From 15898a06f8396e07a0f53fb808a4d8f7cf2f5f95 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Tue, 4 Jul 2017 16:46:19 -0500 Subject: [PATCH] Ref #2: Check transaction carries expected signatures All transactions must declare a list of permissions they utilize. The chain now checks that the signatures are present to satisfy these permissions, at least theoretically (only partially tested). As the transaction is evaluated and applied, the message handlers will check that the required permissions were declared on the transaction. Also, define the logic to check that an authority is satisfied (only this part is tested so far) TODO: Test that transactions are rejected if they do not bear sufficient signatures TODO: Make message handlers check the declared permissions are sufficient, and reject the transaction if they are not. --- libraries/chain/chain_controller.cpp | 11 +- .../include/eos/chain/account_object.hpp | 69 ----------- .../include/eos/chain/action_objects.hpp | 6 +- .../chain/include/eos/chain/authority.hpp | 111 +++++++++++++++--- .../chain/include/eos/chain/exceptions.hpp | 4 +- .../include/eos/chain/permission_object.hpp | 73 ++++++++++++ libraries/chain/transaction.cpp | 28 ++--- libraries/native_contract/system_contract.cpp | 7 +- tests/tests/misc_tests.cpp | 56 ++++++++- 9 files changed, 250 insertions(+), 115 deletions(-) create mode 100644 libraries/chain/include/eos/chain/permission_object.hpp diff --git a/libraries/chain/chain_controller.cpp b/libraries/chain/chain_controller.cpp index ad0091d5922..79899fa1803 100644 --- a/libraries/chain/chain_controller.cpp +++ b/libraries/chain/chain_controller.cpp @@ -639,11 +639,20 @@ void chain_controller::apply_message( apply_context& context ) } FC_CAPTURE_AND_RETHROW((context.msg)) } - void chain_controller::_apply_transaction(const SignedTransaction& trx) { try { validate_transaction(trx); + auto getAuthority = [&db=_db](const types::AccountPermission& permission) { + auto key = boost::make_tuple(permission.account, permission.permission); + return db.get(key).auth; + }; +#warning TODO: Use a real chain_id here (where is this stored? Do we still need it?) + auto checker = MakeAuthorityChecker(std::move(getAuthority), trx.get_signature_keys(chain_id_type{})); + + for (const auto& requiredAuthority : trx.authorizations) + EOS_ASSERT(checker.satisfied(requiredAuthority), tx_missing_auth, "Transaction is not authorized."); + for (const auto& message : trx.messages) { process_message(message); } diff --git a/libraries/chain/include/eos/chain/account_object.hpp b/libraries/chain/include/eos/chain/account_object.hpp index 13b5aebcce4..80877fa92ff 100644 --- a/libraries/chain/include/eos/chain/account_object.hpp +++ b/libraries/chain/include/eos/chain/account_object.hpp @@ -29,33 +29,6 @@ namespace eos { namespace chain { - struct shared_authority { - shared_authority( chainbase::allocator alloc ) - :accounts(alloc),keys(alloc) - {} - - shared_authority& operator=(const Authority& a) { - threshold = a.threshold; - accounts = decltype(accounts)(a.accounts.begin(), a.accounts.end(), accounts.get_allocator()); - keys = decltype(keys)(a.keys.begin(), a.keys.end(), keys.get_allocator()); - return *this; - } - shared_authority& operator=(Authority&& a) { - threshold = a.threshold; - accounts.reserve(a.accounts.size()); - for (auto& p : a.accounts) - accounts.emplace_back(std::move(p)); - keys.reserve(a.keys.size()); - for (auto& p : a.keys) - keys.emplace_back(std::move(p)); - return *this; - } - - UInt32 threshold = 0; - shared_vector accounts; - shared_vector keys; - }; - class account_object : public chainbase::object { OBJECT_CTOR(account_object,(code)) @@ -78,52 +51,10 @@ namespace eos { namespace chain { > >; - class permission_object : public chainbase::object { - OBJECT_CTOR(permission_object, (auth) ) - - id_type id; - AccountName owner; ///< the account this permission belongs to - id_type parent; ///< parent permission - PermissionName name; ///< human-readable name for the permission - shared_authority auth; ///< authority required to execute this permission - }; - - struct by_parent; - struct by_owner; - using permission_index = chainbase::shared_multi_index_container< - permission_object, - indexed_by< - ordered_unique, member>, - ordered_unique, - composite_key, - member - > - >, - ordered_unique, - composite_key, - member, - member - > - >, - ordered_unique, - composite_key, - member - > - > - > - >; - } } // eos::chain CHAINBASE_SET_INDEX_TYPE(eos::chain::account_object, eos::chain::account_index) -CHAINBASE_SET_INDEX_TYPE(eos::chain::permission_object, eos::chain::permission_index) -FC_REFLECT(chainbase::oid, (_id)) FC_REFLECT(chainbase::oid, (_id)) -FC_REFLECT(eos::chain::shared_authority, (threshold)(accounts)(keys)) FC_REFLECT(eos::chain::account_object, (id)(name)(vm_type)(vm_version)(code_version)(code)(creation_date)) -FC_REFLECT(eos::chain::permission_object, (id)(owner)(parent)(name)(auth)) diff --git a/libraries/chain/include/eos/chain/action_objects.hpp b/libraries/chain/include/eos/chain/action_objects.hpp index 9c1abdbbecd..d32bd09178a 100644 --- a/libraries/chain/include/eos/chain/action_objects.hpp +++ b/libraries/chain/include/eos/chain/action_objects.hpp @@ -23,7 +23,7 @@ */ #pragma once #include -#include +#include #include "multi_index_includes.hpp" @@ -57,7 +57,7 @@ namespace eos { namespace chain { OBJECT_CTOR(action_permission_object) id_type id; - account_id_type owner; ///< the account whose permission we seek + AccountName owner; ///< the account whose permission we seek permission_object::id_type scope_permission; ///< the scope permission defined by the contract for the action permission_object::id_type owner_permission; ///< the owner permission that is required }; @@ -69,7 +69,7 @@ namespace eos { namespace chain { ordered_unique, member>, ordered_unique, composite_key< action_permission_object, - member, + member, member > > diff --git a/libraries/chain/include/eos/chain/authority.hpp b/libraries/chain/include/eos/chain/authority.hpp index 06f5f90ec50..6cbc7fe649c 100644 --- a/libraries/chain/include/eos/chain/authority.hpp +++ b/libraries/chain/include/eos/chain/authority.hpp @@ -2,26 +2,101 @@ #include #include -namespace eos { - inline bool operator < ( const types::AccountPermission& a, const types::AccountPermission& b ) { - return std::tie( a.account, a.permission ) < std::tie( b.account, b.permission ); +namespace eos { namespace chain { +struct shared_authority { + shared_authority( chainbase::allocator alloc ) + :accounts(alloc),keys(alloc) + {} + + shared_authority& operator=(const Authority& a) { + threshold = a.threshold; + accounts = decltype(accounts)(a.accounts.begin(), a.accounts.end(), accounts.get_allocator()); + keys = decltype(keys)(a.keys.begin(), a.keys.end(), keys.get_allocator()); + return *this; + } + shared_authority& operator=(Authority&& a) { + threshold = a.threshold; + accounts.reserve(a.accounts.size()); + for (auto& p : a.accounts) + accounts.emplace_back(std::move(p)); + keys.reserve(a.keys.size()); + for (auto& p : a.keys) + keys.emplace_back(std::move(p)); + return *this; } - /** - * Makes sure all keys are unique and sorted and all account permissions are unique and sorted - */ - inline bool validate( types::Authority& auth ) { - const types::KeyPermissionWeight* prev = nullptr; - for( const auto& k : auth.keys ) { - if( !prev ) prev = &k; - else if( prev->key < k.key ) return false; - } - const types::AccountPermissionWeight* pa = nullptr; - for( const auto& a : auth.accounts ) { - if( !pa ) pa = &a; - else if( pa->permission < a.permission ) return false; - } - return true; + UInt32 threshold = 0; + shared_vector accounts; + 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; + const 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? + 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); +} + +/** + * Makes sure all keys are unique and sorted and all account permissions are unique and sorted + */ +inline bool validate( types::Authority& auth ) { + const types::KeyPermissionWeight* prev = nullptr; + for( const auto& k : auth.keys ) { + if( !prev ) prev = &k; + else if( prev->key < k.key ) return false; + } + const types::AccountPermissionWeight* pa = nullptr; + for( const auto& a : auth.accounts ) { + if( !pa ) pa = &a; + else if( pa->permission < a.permission ) return false; + } + return true; +} + +} } // namespace eos::chain +FC_REFLECT(eos::chain::shared_authority, (threshold)(accounts)(keys)) diff --git a/libraries/chain/include/eos/chain/exceptions.hpp b/libraries/chain/include/eos/chain/exceptions.hpp index 70f4770499e..22e341d1308 100644 --- a/libraries/chain/include/eos/chain/exceptions.hpp +++ b/libraries/chain/include/eos/chain/exceptions.hpp @@ -42,9 +42,7 @@ namespace eos { namespace chain { FC_DECLARE_DERIVED_EXCEPTION( black_swan_exception, eos::chain::chain_exception, 3100000, "black swan" ) FC_DECLARE_DERIVED_EXCEPTION( unknown_block_exception, eos::chain::chain_exception, 3110000, "unknown block" ) - FC_DECLARE_DERIVED_EXCEPTION( tx_missing_active_auth, eos::chain::transaction_exception, 3030001, "missing required active authority" ) - FC_DECLARE_DERIVED_EXCEPTION( tx_missing_owner_auth, eos::chain::transaction_exception, 3030002, "missing required owner authority" ) - FC_DECLARE_DERIVED_EXCEPTION( tx_missing_other_auth, eos::chain::transaction_exception, 3030003, "missing required other authority" ) + FC_DECLARE_DERIVED_EXCEPTION( tx_missing_auth, eos::chain::transaction_exception, 3030001, "missing required authority" ) FC_DECLARE_DERIVED_EXCEPTION( tx_irrelevant_sig, eos::chain::transaction_exception, 3030004, "irrelevant signature included" ) FC_DECLARE_DERIVED_EXCEPTION( tx_duplicate_sig, eos::chain::transaction_exception, 3030005, "duplicate signature included" ) FC_DECLARE_DERIVED_EXCEPTION( invalid_committee_approval, eos::chain::transaction_exception, 3030006, "committee account cannot directly approve transaction" ) diff --git a/libraries/chain/include/eos/chain/permission_object.hpp b/libraries/chain/include/eos/chain/permission_object.hpp new file mode 100644 index 00000000000..fda72bbc0a1 --- /dev/null +++ b/libraries/chain/include/eos/chain/permission_object.hpp @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2017, Respective Authors. + * + * The MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#pragma once +#include + +#include "multi_index_includes.hpp" + +namespace eos { namespace chain { + class permission_object : public chainbase::object { + OBJECT_CTOR(permission_object, (auth) ) + + id_type id; + AccountName owner; ///< the account this permission belongs to + id_type parent; ///< parent permission + PermissionName name; ///< human-readable name for the permission + shared_authority auth; ///< authority required to execute this permission + }; + + struct by_parent; + struct by_owner; + struct by_name; + using permission_index = chainbase::shared_multi_index_container< + permission_object, + indexed_by< + ordered_unique, member>, + ordered_unique, + composite_key, + member + > + >, + ordered_unique, + composite_key, + member + > + >, + ordered_unique, + composite_key, + member + > + > + > + >; + +} } // eos::chain + +CHAINBASE_SET_INDEX_TYPE(eos::chain::permission_object, eos::chain::permission_index) + +FC_REFLECT(chainbase::oid, (_id)) +FC_REFLECT(eos::chain::permission_object, (id)(owner)(parent)(name)(auth)) diff --git a/libraries/chain/transaction.cpp b/libraries/chain/transaction.cpp index 001d901aa2a..94b202229a4 100644 --- a/libraries/chain/transaction.cpp +++ b/libraries/chain/transaction.cpp @@ -27,6 +27,8 @@ #include #include +#include + namespace eos { namespace chain { digest_type SignedTransaction::digest()const { @@ -50,16 +52,12 @@ eos::chain::transaction_id_type SignedTransaction::id() const { } const signature_type& eos::chain::SignedTransaction::sign(const private_key_type& key, const chain_id_type& chain_id) { - digest_type h = sig_digest( chain_id ); - signatures.push_back(key.sign_compact(h)); + signatures.push_back(key.sign_compact(sig_digest(chain_id))); return signatures.back(); } signature_type eos::chain::SignedTransaction::sign(const private_key_type& key, const chain_id_type& chain_id)const { - digest_type::encoder enc; - fc::raw::pack( enc, chain_id ); - fc::raw::pack( enc, static_cast(*this) ); - return key.sign_compact(enc.result()); + return key.sign_compact(sig_digest(chain_id)); } void SignedTransaction::set_reference_block(const block_id_type& reference_block) { @@ -74,17 +72,13 @@ bool SignedTransaction::verify_reference_block(const block_id_type& reference_bl flat_set SignedTransaction::get_signature_keys( const chain_id_type& chain_id )const { try { - auto d = sig_digest( chain_id ); - flat_set result; - for( const auto& sig : signatures ) - { - EOS_ASSERT( - result.insert( fc::ecc::public_key(sig,d) ).second, - tx_duplicate_sig, - "Duplicate Signature detected" ); - } - return result; -} FC_CAPTURE_AND_RETHROW() } + using boost::adaptors::transformed; + auto SigToKey = transformed([digest = sig_digest(chain_id)](const fc::ecc::compact_signature& signature) { + return public_key_type(fc::ecc::public_key(signature, digest)); + }); + auto keyRange = signatures | SigToKey; + return {keyRange.begin(), keyRange.end()}; + } FC_CAPTURE_AND_RETHROW() } eos::chain::digest_type SignedTransaction::merkle_digest() const { digest_type::encoder enc; diff --git a/libraries/native_contract/system_contract.cpp b/libraries/native_contract/system_contract.cpp index 01c731f3f4d..3e1d2b3e675 100644 --- a/libraries/native_contract/system_contract.cpp +++ b/libraries/native_contract/system_contract.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -50,9 +51,9 @@ void validate_system_newaccount(message_validate_context& context) { EOS_ASSERT(context.msg.has_notify(config::StakedBalanceContractName), message_validate_exception, "Must notify Staked Balance Contract (${name})", ("name", config::StakedBalanceContractName)); - EOS_ASSERT( eos::validate(create.owner), message_validate_exception, "Invalid owner authority"); - EOS_ASSERT( eos::validate(create.active), message_validate_exception, "Invalid active authority"); - EOS_ASSERT( eos::validate(create.recovery), message_validate_exception, "Invalid recovery authority"); + EOS_ASSERT( validate(create.owner), message_validate_exception, "Invalid owner authority"); + EOS_ASSERT( validate(create.active), message_validate_exception, "Invalid active authority"); + EOS_ASSERT( validate(create.recovery), message_validate_exception, "Invalid recovery authority"); } void precondition_system_newaccount(precondition_validate_context& context) { diff --git a/tests/tests/misc_tests.cpp b/tests/tests/misc_tests.cpp index 127e705a442..0220ab36a51 100644 --- a/tests/tests/misc_tests.cpp +++ b/tests/tests/misc_tests.cpp @@ -1,4 +1,5 @@ #include +#include #include @@ -72,9 +73,62 @@ BOOST_AUTO_TEST_CASE(deterministic_distributions) rng.shuffle(nums); std::vector c{1, 0, 2}; BOOST_CHECK(std::equal(nums.begin(), nums.end(), c.begin())); - } FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE(authority_checker) +{ try { + #define KEY(x) auto x = fc::ecc::private_key::regenerate(fc::sha256::hash(#x)).get_public_key() + + KEY(a); + KEY(b); + KEY(c); + + auto GetNullAuthority = [](auto){return Authority();}; + + Authority A(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)); + + A = Authority(3, {{a,1},{b,1},{c,1}}, {}); + BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {c, b, a}).satisfied(A)); + BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {a, b}).satisfied(A)); + BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {a, c}).satisfied(A)); + BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {b, c}).satisfied(A)); + + A = Authority(1, {{a, 1}, {b, 1}}, {}); + BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a}).satisfied(A)); + BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {b}).satisfied(A)); + BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {c}).satisfied(A)); + + A = Authority(1, {{a, 2}, {b, 1}}, {}); + BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a}).satisfied(A)); + BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {b}).satisfied(A)); + BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {c}).satisfied(A)); + + auto GetCAuthority = [c](auto){return Authority(1, {{c, 1}}, {});}; + + A = 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)); + + A = Authority(2, {{a, 1}, {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, {a,b}).satisfied(A)); + BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {b,c}).satisfied(A)); + BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,c}).satisfied(A)); + + A = 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)); +} FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_SUITE_END()