From 16306e9c03df786c1d604c3886d4094a715bca0d Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 9 Aug 2017 15:54:25 -0500 Subject: [PATCH] Ref #7: Upgrade AuthorityChecker I want two things from AuthorityChecker that it wasn't doing yet: 1. I want it to track which of the provided keys were used, so I can reject transactions which bear more signatures than are necessary 2. To sign a transaction with no unnecessary keys, I want it to support taking a list of available public keys and an authority, then telling me which of those keys I should use to fully satisfy the authority, without having any unnecessary keys As an added bonus, having both of these operations handled by AuthorityChecker means that determining the set of keys needed to sign a transaction, and determining whether a transaction is properly signed, use the same code. :D --- libraries/chain/chain_controller.cpp | 4 + .../chain/include/eos/chain/authority.hpp | 51 +------ .../include/eos/chain/authority_checker.hpp | 127 +++++++++++++++++ tests/tests/misc_tests.cpp | 128 ++++++++++++++++-- tests/tests/native_contract_tests.cpp | 1 + 5 files changed, 252 insertions(+), 59 deletions(-) create mode 100644 libraries/chain/include/eos/chain/authority_checker.hpp diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index 2f68c5f34..b1a5d8897 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -505,6 +506,9 @@ void chain_controller::check_transaction_authorization(const SignedTransaction& ("auth", declaredAuthority)); } } + + EOS_ASSERT(checker.all_keys_used(), tx_irrelevant_sig, + "Transaction bears irrelevant signatures from these keys: ${keys}", ("keys", checker.unused_keys())); } ProcessedTransaction chain_controller::apply_transaction(const SignedTransaction& trx, uint32_t skip) diff --git a/libraries/chain/include/eos/chain/authority.hpp b/libraries/chain/include/eos/chain/authority.hpp index 3b68ab211..13a218ecd 100644 --- a/libraries/chain/include/eos/chain/authority.hpp +++ b/libraries/chain/include/eos/chain/authority.hpp @@ -30,55 +30,8 @@ struct shared_authority { shared_vector keys; }; -/** - * @brief This class determines whether a set of signing keys are sufficient to satisfy an authority or not - * - * To determine whether an authority is satisfied or not, we first determine which keys have approved of a message, and - * then determine whether that list of keys is sufficient to satisfy the authority. This class takes a list of keys and - * provides the @ref satisfied method to determine whether that list of keys satisfies a provided authority. - * - * @tparam F A callable which takes a single argument of type @ref AccountPermission and returns the corresponding - * authority - */ -template -class AuthorityChecker { - F PermissionToAuthority; - flat_set signingKeys; - -public: - AuthorityChecker(F PermissionToAuthority, const flat_set& signingKeys) - : PermissionToAuthority(PermissionToAuthority), signingKeys(signingKeys) {} - - bool satisfied(const types::AccountPermission& permission) const { - return satisfied(PermissionToAuthority(permission)); - } - template - bool satisfied(const AuthorityType& authority) const { - UInt32 weight = 0; - for (const auto& kpw : authority.keys) { - if (signingKeys.count(kpw.key)) { - weight += kpw.weight; - if (weight >= authority.threshold) - return true; - } - } - for (const auto& apw : authority.accounts) -//#warning TODO: Recursion limit? Yes: implement as producer-configurable parameter - if (satisfied(apw.permission)) { - weight += apw.weight; - if (weight >= authority.threshold) - return true; - } - return false; - } -}; - -inline bool operator < ( const types::AccountPermission& a, const types::AccountPermission& b ) { - return std::tie( a.account, a.permission ) < std::tie( b.account, b.permission ); -} -template -AuthorityChecker MakeAuthorityChecker(F&& pta, const flat_set& signingKeys) { - return AuthorityChecker(std::forward(pta), signingKeys); +inline bool operator< (const types::AccountPermission& a, const types::AccountPermission& b) { + return std::tie(a.account, a.permission) < std::tie(b.account, b.permission); } /** diff --git a/libraries/chain/include/eos/chain/authority_checker.hpp b/libraries/chain/include/eos/chain/authority_checker.hpp new file mode 100644 index 000000000..dca8b39a1 --- /dev/null +++ b/libraries/chain/include/eos/chain/authority_checker.hpp @@ -0,0 +1,127 @@ +#pragma once + +#include +#include + +#include + +#include + +#include +#include + +namespace eos { namespace chain { + +namespace detail { +using MetaPermission = static_variant; + +struct GetWeightVisitor { + using result_type = UInt32; + + template + UInt32 operator()(const Permission& permission) { return permission.weight; } +}; + +// Orders permissions descending by weight, and breaks ties with Key permissions being less than Account permissions +struct MetaPermissionComparator { + bool operator()(const MetaPermission& a, const MetaPermission& b) const { + GetWeightVisitor scale; + if (a.visit(scale) > b.visit(scale)) return true; + return a.contains() && !b.contains(); + } +}; + +using MetaPermissionSet = boost::container::flat_multiset; +} + +/** + * @brief This class determines whether a set of signing keys are sufficient to satisfy an authority or not + * + * To determine whether an authority is satisfied or not, we first determine which keys have approved of a message, and + * then determine whether that list of keys is sufficient to satisfy the authority. This class takes a list of keys and + * provides the @ref satisfied method to determine whether that list of keys satisfies a provided authority. + * + * @tparam F A callable which takes a single argument of type @ref AccountPermission and returns the corresponding + * authority + */ +template +class AuthorityChecker { + F PermissionToAuthority; + vector signingKeys; + vector usedKeys; + + struct WeightTallyVisitor { + using result_type = UInt32; + + AuthorityChecker& checker; + UInt32 totalWeight = 0; + + WeightTallyVisitor(AuthorityChecker& checker) + : checker(checker) {} + + UInt32 operator()(const types::KeyPermissionWeight& permission) { + auto itr = boost::find(checker.signingKeys, permission.key); + if (itr != checker.signingKeys.end()) { + checker.usedKeys[itr - checker.signingKeys.begin()] = true; + totalWeight += permission.weight; + } + return totalWeight; + } + UInt32 operator()(const types::AccountPermissionWeight& permission) { + //TODO: Recursion limit? Yes: implement as producer-configurable parameter + if (checker.satisfied(permission.permission)) + totalWeight += permission.weight; + return totalWeight; + } + }; + +public: + AuthorityChecker(F PermissionToAuthority, const flat_set& signingKeys) + : PermissionToAuthority(PermissionToAuthority), + signingKeys(signingKeys.begin(), signingKeys.end()), + usedKeys(signingKeys.size(), false) + {} + + bool satisfied(const types::AccountPermission& permission) { + return satisfied(PermissionToAuthority(permission)); + } + template + bool satisfied(const AuthorityType& authority) { + // Save the current used keys; if we do not satisfy this authority, the newly used keys aren't actually used + auto KeyReverter = fc::make_scoped_exit([this, keys = usedKeys] () mutable { + usedKeys = keys; + }); + + // Sort key permissions and account permissions together into a single set of MetaPermissions + detail::MetaPermissionSet permissions; + permissions.insert(authority.keys.begin(), authority.keys.end()); + permissions.insert(authority.accounts.begin(), authority.accounts.end()); + + // Check all permissions, from highest weight to lowest, seeing if signingKeys satisfies them or not + WeightTallyVisitor visitor(*this); + for (const auto& permission : permissions) + // If we've got enough weight, to satisfy the authority, return! + if (permission.visit(visitor) >= authority.threshold) { + KeyReverter.cancel(); + return true; + } + return false; + } + + bool all_keys_used() const { return boost::algorithm::all_of_equal(usedKeys, true); } + flat_set used_keys() const { + auto range = utilities::FilterDataByMarker(signingKeys, usedKeys, true); + return {range.begin(), range.end()}; + } + flat_set unused_keys() const { + auto range = utilities::FilterDataByMarker(signingKeys, usedKeys, false); + return {range.begin(), range.end()}; + } +}; + +template +AuthorityChecker MakeAuthorityChecker(F&& pta, const flat_set& signingKeys) { + return AuthorityChecker(std::forward(pta), signingKeys); +} + +}} // namespace eos::chain diff --git a/tests/tests/misc_tests.cpp b/tests/tests/misc_tests.cpp index e197926d3..a3e5b6d45 100644 --- a/tests/tests/misc_tests.cpp +++ b/tests/tests/misc_tests.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include @@ -88,13 +88,39 @@ BOOST_AUTO_TEST_CASE(authority_checker) Make_Key(c); auto& c = c_public_key; - auto GetNullAuthority = [](auto){return Authority();}; + auto GetNullAuthority = [](auto){abort(); return Authority();}; auto A = Complex_Authority(2, ((a,1))((b,1)),); - BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a, b}).satisfied(A)); - BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a, b, c}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {a, c}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {b, c}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {a, b}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 0); + } + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {a, c}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2); + } + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {a, b, c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.used_keys().count(a), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } + { + auto checker = MakeAuthorityChecker(GetNullAuthority, {b, c}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + } A = Complex_Authority(3, ((a,1))((b,1))((c,1)),); BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {c, b, a}).satisfied(A)); @@ -115,10 +141,44 @@ BOOST_AUTO_TEST_CASE(authority_checker) auto GetCAuthority = [c_public_key](auto){return Complex_Authority(1, ((c, 1)),);}; A = Complex_Authority(2, ((a, 2))((b, 1)), (("hello", "world", 1))); - BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {b}).satisfied(A)); - BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {c}).satisfied(A)); - BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {b,c}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetCAuthority, {a}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(checker.all_keys_used()); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {b}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(b), 1); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {c}); + BOOST_CHECK(!checker.satisfied(A)); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {b,c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 0); + BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1); + } + { + auto checker = MakeAuthorityChecker(GetCAuthority, {b,c,a}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(a), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } A = Complex_Authority(2, ((a, 1))((b, 1)), (("hello", "world", 1))); BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A)); @@ -127,12 +187,60 @@ BOOST_AUTO_TEST_CASE(authority_checker) BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,b}).satisfied(A)); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {b,c}).satisfied(A)); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,c}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetCAuthority, {a,b,c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1); + } A = Complex_Authority(2, ((a, 1))((b, 1)), (("hello", "world", 2))); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,b}).satisfied(A)); BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {c}).satisfied(A)); BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A)); BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {b}).satisfied(A)); + { + auto checker = MakeAuthorityChecker(GetCAuthority, {a,b,c}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1); + } + + Make_Key(d); + auto& d = d_public_key; + Make_Key(e); + auto& e = e_public_key; + + auto GetAuthority = [d_public_key, e] (const types::AccountPermission& perm) { + if (perm.account == "top") + return Complex_Authority(2, ((d, 1)), (("bottom", "bottom", 1))); + return Key_Authority(e); + }; + + A = Complex_Authority(5, ((a, 2))((b, 2))((c, 2)), (("top", "top", 5))); + { + auto checker = MakeAuthorityChecker(GetAuthority, {a,b,c,d,e}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 2); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 3); + BOOST_CHECK_EQUAL(checker.used_keys().count(d), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(e), 1); + } + { + auto checker = MakeAuthorityChecker(GetAuthority, {a,b,c,e}); + BOOST_CHECK(checker.satisfied(A)); + BOOST_CHECK(!checker.all_keys_used()); + BOOST_CHECK_EQUAL(checker.used_keys().size(), 3); + BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(a), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1); + BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1); + } } FC_LOG_AND_RETHROW() } /// Test creating the wallet diff --git a/tests/tests/native_contract_tests.cpp b/tests/tests/native_contract_tests.cpp index 4ccf99f08..09d07443b 100644 --- a/tests/tests/native_contract_tests.cpp +++ b/tests/tests/native_contract_tests.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include -- GitLab