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

@@ -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<permission_object, by_owner>(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);
    }

libraries/chain/include/eos/chain/account_object.hpp

@@ -29,33 +29,6 @@
 
 namespace eos { namespace chain {
 
-   struct shared_authority {
-      shared_authority( chainbase::allocator<char> 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<types::AccountPermissionWeight> accounts;
-      shared_vector<types::KeyPermissionWeight>     keys;
-   };
-
    class account_object : public chainbase::object<account_object_type, account_object> {
       OBJECT_CTOR(account_object,(code))
 
@@ -78,52 +51,10 @@ namespace eos { namespace chain {
       >
    >;
 
-   class permission_object : public chainbase::object<permission_object_type, permission_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<tag<by_id>, member<permission_object, permission_object::id_type, &permission_object::id>>,
-         ordered_unique<tag<by_parent>,
-            composite_key<permission_object,
-               member<permission_object, permission_object::id_type, &permission_object::parent>,
-               member<permission_object, permission_object::id_type, &permission_object::id>
-            >
-         >,
-         ordered_unique<tag<by_owner>,
-            composite_key<permission_object,
-               member<permission_object, AccountName, &permission_object::owner>,
-               member<permission_object, PermissionName, &permission_object::name>,
-               member<permission_object, permission_object::id_type, &permission_object::id>
-            >
-         >,
-         ordered_unique<tag<by_name>,
-            composite_key<permission_object,
-               member<permission_object, PermissionName, &permission_object::name>,
-               member<permission_object, permission_object::id_type, &permission_object::id>
-            >
-         >
-      >
-   >;
-
 } } // 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<eos::chain::permission_object>, (_id))
 FC_REFLECT(chainbase::oid<eos::chain::account_object>, (_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))

libraries/chain/include/eos/chain/action_objects.hpp

@@ -23,7 +23,7 @@
  */
 #pragma once
 #include <eos/chain/types.hpp>
-#include <eos/chain/account_object.hpp>
+#include <eos/chain/permission_object.hpp>
 
 #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<tag<by_id>, member<action_permission_object, action_permission_object::id_type, &action_permission_object::id>>,
          ordered_unique<tag<by_owner_scope>, 
             composite_key< action_permission_object,
-               member<action_permission_object, account_id_type, &action_permission_object::owner>,
+               member<action_permission_object, AccountName, &action_permission_object::owner>,
                member<action_permission_object, permission_object::id_type, &action_permission_object::scope_permission>
             >
          >

libraries/chain/include/eos/chain/authority.hpp

@@ -2,26 +2,101 @@
 #include <eos/chain/types.hpp>
 #include <eos/types/generated.hpp>
 
-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<char> 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<types::AccountPermissionWeight> accounts;
+   shared_vector<types::KeyPermissionWeight>     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<typename F>
+class AuthorityChecker {
+   F PermissionToAuthority;
+   const flat_set<public_key_type>& signingKeys;
+
+public:
+   AuthorityChecker(F PermissionToAuthority, const flat_set<public_key_type>& signingKeys)
+      : PermissionToAuthority(PermissionToAuthority), signingKeys(signingKeys) {}
+
+   bool satisfied(const types::AccountPermission& permission) const {
+      return satisfied(PermissionToAuthority(permission));
+   }
+   template<typename AuthorityType>
+   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<typename F>
+AuthorityChecker<F> MakeAuthorityChecker(F&& pta, const flat_set<public_key_type>& signingKeys) {
+   return AuthorityChecker<F>(std::forward<F>(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))

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" )

libraries/chain/include/eos/chain/permission_object.hpp

+/*
+ * 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 <eos/chain/authority.hpp>
+
+#include "multi_index_includes.hpp"
+
+namespace eos { namespace chain {
+   class permission_object : public chainbase::object<permission_object_type, permission_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<tag<by_id>, member<permission_object, permission_object::id_type, &permission_object::id>>,
+         ordered_unique<tag<by_parent>,
+            composite_key<permission_object,
+               member<permission_object, permission_object::id_type, &permission_object::parent>,
+               member<permission_object, permission_object::id_type, &permission_object::id>
+            >
+         >,
+         ordered_unique<tag<by_owner>,
+            composite_key<permission_object,
+               member<permission_object, AccountName, &permission_object::owner>,
+               member<permission_object, PermissionName, &permission_object::name>
+            >
+         >,
+         ordered_unique<tag<by_name>,
+            composite_key<permission_object,
+               member<permission_object, PermissionName, &permission_object::name>,
+               member<permission_object, permission_object::id_type, &permission_object::id>
+            >
+         >
+      >
+   >;
+
+} } // eos::chain
+
+CHAINBASE_SET_INDEX_TYPE(eos::chain::permission_object, eos::chain::permission_index)
+
+FC_REFLECT(chainbase::oid<eos::chain::permission_object>, (_id))
+FC_REFLECT(eos::chain::permission_object, (id)(owner)(parent)(name)(auth))

libraries/chain/transaction.cpp

@@ -27,6 +27,8 @@
 #include <fc/smart_ref_impl.hpp>
 #include <algorithm>
 
+#include <boost/range/adaptor/transformed.hpp>
+
 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<const types::Transaction&>(*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<public_key_type> SignedTransaction::get_signature_keys( const chain_id_type& chain_id )const
 { try {
-   auto d = sig_digest( chain_id );
-   flat_set<public_key_type> 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;

libraries/native_contract/system_contract.cpp

 #include <eos/native_contract/system_contract.hpp>
 
 #include <eos/chain/message_handling_contexts.hpp>
+#include <eos/chain/permission_object.hpp>
 #include <eos/chain/account_object.hpp>
 #include <eos/chain/exceptions.hpp>
 #include <eos/chain/global_property_object.hpp>
@@ -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) {

tests/tests/misc_tests.cpp

 #include <eos/chain/BlockchainConfiguration.hpp>
+#include <eos/chain/authority.hpp>
 
 #include <eos/utilities/rand.hpp>
 
@@ -72,9 +73,62 @@ BOOST_AUTO_TEST_CASE(deterministic_distributions)
    rng.shuffle(nums);
    std::vector<int> 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()