Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #584 authorize transactions with non-immediate owner authorities #1259

Merged
merged 11 commits into from
Feb 10, 2019
12 changes: 10 additions & 2 deletions libraries/app/database_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <graphene/app/database_api.hpp>
#include <graphene/app/util.hpp>
#include <graphene/chain/get_config.hpp>
#include <graphene/chain/hardfork.hpp>

#include <fc/bloom_filter.hpp>
#include <fc/smart_ref_impl.hpp>
Expand Down Expand Up @@ -1971,10 +1972,12 @@ set<public_key_type> database_api::get_required_signatures( const signed_transac

set<public_key_type> database_api_impl::get_required_signatures( const signed_transaction& trx, const flat_set<public_key_type>& available_keys )const
{
bool allow_non_immediate_owner = ( _db.head_block_time() >= HARDFORK_CORE_584_TIME );
auto result = trx.get_required_signatures( _db.get_chain_id(),
available_keys,
[&]( account_id_type id ){ return &id(_db).active; },
[&]( account_id_type id ){ return &id(_db).owner; },
allow_non_immediate_owner,
_db.get_global_properties().parameters.max_authority_depth );
return result;
}
Expand All @@ -1990,6 +1993,7 @@ set<address> database_api::get_potential_address_signatures( const signed_transa

set<public_key_type> database_api_impl::get_potential_signatures( const signed_transaction& trx )const
{
bool allow_non_immediate_owner = ( _db.head_block_time() >= HARDFORK_CORE_584_TIME );
set<public_key_type> result;
trx.get_required_signatures(
_db.get_chain_id(),
Expand All @@ -2008,6 +2012,7 @@ set<public_key_type> database_api_impl::get_potential_signatures( const signed_t
result.insert(k);
return &auth;
},
allow_non_immediate_owner,
_db.get_global_properties().parameters.max_authority_depth
);

Expand Down Expand Up @@ -2055,10 +2060,12 @@ bool database_api::verify_authority( const signed_transaction& trx )const

bool database_api_impl::verify_authority( const signed_transaction& trx )const
{
bool allow_non_immediate_owner = ( _db.head_block_time() >= HARDFORK_CORE_584_TIME );
trx.verify_authority( _db.get_chain_id(),
[this]( account_id_type id ){ return &id(_db).active; },
[this]( account_id_type id ){ return &id(_db).owner; },
_db.get_global_properties().parameters.max_authority_depth );
allow_non_immediate_owner,
_db.get_global_properties().parameters.max_authority_depth );
return true;
}

Expand All @@ -2080,7 +2087,8 @@ bool database_api_impl::verify_account_authority( const string& account_name_or_
{
graphene::chain::verify_authority(ops, keys,
[this]( account_id_type id ){ return &id(_db).active; },
[this]( account_id_type id ){ return &id(_db).owner; } );
[this]( account_id_type id ){ return &id(_db).owner; },
true );
}
catch (fc::exception& ex)
{
Expand Down
7 changes: 6 additions & 1 deletion libraries/chain/db_block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,14 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx

if( !(skip & skip_transaction_signatures) )
{
bool allow_non_immediate_owner = ( head_block_time() >= HARDFORK_CORE_584_TIME );
auto get_active = [&]( account_id_type id ) { return &id(*this).active; };
auto get_owner = [&]( account_id_type id ) { return &id(*this).owner; };
trx.verify_authority( chain_id, get_active, get_owner, get_global_properties().parameters.max_authority_depth );
trx.verify_authority( chain_id,
get_active,
get_owner,
allow_non_immediate_owner,
get_global_properties().parameters.max_authority_depth );
}

//Skip all manner of expiration and TaPoS checking if we're on block 1; It's impossible that the transaction is
Expand Down
5 changes: 5 additions & 0 deletions libraries/chain/hardfork.d/CORE_584.hf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// bitshares-core issue #584 Owner keys of non-immediately required accounts can not authorize a transaction
// https://github.com/bitshares/bitshares-core/issues/584
#ifndef HARDFORK_CORE_584_TIME
#define HARDFORK_CORE_584_TIME (fc::time_point_sec( 1580000000 )) // a temporary date in the future
#endif
37 changes: 35 additions & 2 deletions libraries/chain/include/graphene/chain/protocol/transaction.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ namespace graphene { namespace chain {
return results;
}

void get_required_authorities( flat_set<account_id_type>& active, flat_set<account_id_type>& owner, vector<authority>& other )const;
void get_required_authorities( flat_set<account_id_type>& active,
flat_set<account_id_type>& owner,
vector<authority>& other )const;

protected:
// Calculate the digest used for signature validation
Expand Down Expand Up @@ -146,13 +148,27 @@ namespace graphene { namespace chain {
const flat_set<public_key_type>& available_keys,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
)const;

/**
* Checks whether signatures in this signed transaction are sufficient to authorize the transaction.
* Throws an exception when failed.
*
* @param chain_id the ID of a block chain
* @param get_active callback function to retrieve active authorities of a given account
* @param get_owner callback function to retrieve owner authorities of a given account
* @param allow_non_immediate_owner whether to allow owner authority of non-immediately
* required accounts to authorize operations in the transaction
* @param max_recursion maximum level of recursion when verifying, since an account
* can have another account in active authorities and/or owner authorities
*/
void verify_authority(
const chain_id_type& chain_id,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const;

/**
Expand All @@ -161,12 +177,12 @@ namespace graphene { namespace chain {
* some cases where get_required_signatures() returns a
* non-minimal set.
*/

set<public_key_type> minimize_required_signatures(
const chain_id_type& chain_id,
const flat_set<public_key_type>& available_keys,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
) const;

Expand Down Expand Up @@ -213,9 +229,26 @@ namespace graphene { namespace chain {
mutable bool _validated = false;
};

/**
* Checks whether given public keys and approvals are sufficient to authorize given operations.
* Throws an exception when failed.
*
* @param ops a vector of operations
* @param sigs a set of public keys
* @param get_active callback function to retrieve active authorities of a given account
* @param get_owner callback function to retrieve owner authorities of a given account
* @param allow_non_immediate_owner whether to allow owner authority of non-immediately
* required accounts to authorize operations
* @param max_recursion maximum level of recursion when verifying, since an account
* can have another account in active authorities and/or owner authorities
* @param allow_committee whether to allow the special "committee account" to authorize the operations
* @param active_approvals accounts that approved the operations with their active authories
* @param owner_approvals accounts that approved the operations with their owner authories
*/
void verify_authority( const vector<operation>& ops, const flat_set<public_key_type>& sigs,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH,
bool allow_committe = false,
const flat_set<account_id_type>& active_aprovals = flat_set<account_id_type>(),
Expand Down
3 changes: 3 additions & 0 deletions libraries/chain/proposal_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include <graphene/chain/database.hpp>
#include <graphene/chain/hardfork.hpp>
#include <graphene/chain/proposal_object.hpp>

namespace graphene { namespace chain {
Expand All @@ -31,10 +32,12 @@ bool proposal_object::is_authorized_to_execute(database& db) const
transaction_evaluation_state dry_run_eval(&db);

try {
bool allow_non_immediate_owner = ( db.head_block_time() >= HARDFORK_CORE_584_TIME );
verify_authority( proposed_transaction.operations,
available_key_approvals,
[&]( account_id_type id ){ return &id(db).active; },
[&]( account_id_type id ){ return &id(db).owner; },
allow_non_immediate_owner,
db.get_global_properties().parameters.max_authority_depth,
true, /* allow committeee */
available_active_approvals,
Expand Down
47 changes: 33 additions & 14 deletions libraries/chain/protocol/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ void transaction::set_reference_block( const block_id_type& reference_block )
ref_block_prefix = reference_block._hash[1];
}

void transaction::get_required_authorities( flat_set<account_id_type>& active, flat_set<account_id_type>& owner, vector<authority>& other )const
void transaction::get_required_authorities( flat_set<account_id_type>& active,
flat_set<account_id_type>& owner,
vector<authority>& other )const
{
for( const auto& op : operations )
operation_get_required_authorities( op, active, owner, other );
Expand All @@ -102,7 +104,6 @@ void transaction::get_required_authorities( flat_set<account_id_type>& active, f
}



const flat_set<public_key_type> empty_keyset;

struct sign_state
Expand Down Expand Up @@ -162,7 +163,7 @@ struct sign_state
bool check_authority( account_id_type id )
{
if( approved_by.find(id) != approved_by.end() ) return true;
return check_authority( get_active(id) );
return check_authority( get_active(id) ) || ( allow_non_immediate_owner && check_authority( get_owner(id) ) );
}

/**
Expand Down Expand Up @@ -197,7 +198,8 @@ struct sign_state
{
if( depth == max_recursion )
continue;
if( check_authority( get_active( a.first ), depth+1 ) )
if( check_authority( get_active( a.first ), depth+1 )
|| ( allow_non_immediate_owner && check_authority( get_owner( a.first ), depth+1 ) ) )
{
approved_by.insert( a.first );
total_weight += a.second;
Expand Down Expand Up @@ -228,27 +230,38 @@ struct sign_state
}

sign_state( const flat_set<public_key_type>& sigs,
const std::function<const authority*(account_id_type)>& a,
const std::function<const authority*(account_id_type)>& active,
const std::function<const authority*(account_id_type)>& owner,
bool allow_owner,
uint32_t max_recursion_depth = GRAPHENE_MAX_SIG_CHECK_DEPTH,
const flat_set<public_key_type>& keys = empty_keyset )
:get_active(a),available_keys(keys)
: get_active(active),
get_owner(owner),
allow_non_immediate_owner(allow_owner),
max_recursion(max_recursion_depth),
available_keys(keys)
{
for( const auto& key : sigs )
provided_signatures[ key ] = false;
approved_by.insert( GRAPHENE_TEMP_ACCOUNT );
}

const std::function<const authority*(account_id_type)>& get_active;
const flat_set<public_key_type>& available_keys;
const std::function<const authority*(account_id_type)>& get_owner;

const bool allow_non_immediate_owner;
const uint32_t max_recursion;
const flat_set<public_key_type>& available_keys;

flat_map<public_key_type,bool> provided_signatures;
flat_set<account_id_type> approved_by;
uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH;
};


void verify_authority( const vector<operation>& ops, const flat_set<public_key_type>& sigs,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion_depth,
bool allow_committe,
const flat_set<account_id_type>& active_aprovals,
Expand All @@ -265,8 +278,7 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t
GRAPHENE_ASSERT( required_active.find(GRAPHENE_COMMITTEE_ACCOUNT) == required_active.end(),
invalid_committee_approval, "Committee account may only propose transactions" );

sign_state s(sigs,get_active);
s.max_recursion = max_recursion_depth;
sign_state s( sigs, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth );
for( auto& id : active_aprovals )
s.approved_by.insert( id );
for( auto& id : owner_approvals )
Expand Down Expand Up @@ -322,6 +334,7 @@ set<public_key_type> signed_transaction::get_required_signatures(
const flat_set<public_key_type>& available_keys,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion_depth )const
{
flat_set<account_id_type> required_active;
Expand All @@ -330,8 +343,7 @@ set<public_key_type> signed_transaction::get_required_signatures(
get_required_authorities( required_active, required_owner, other );

const flat_set<public_key_type>& signature_keys = get_signature_keys( chain_id );
sign_state s( signature_keys, get_active, available_keys );
s.max_recursion = max_recursion_depth;
sign_state s( signature_keys, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth, available_keys );

for( const auto& auth : other )
s.check_authority(&auth);
Expand All @@ -357,6 +369,7 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
const flat_set<public_key_type>& available_keys,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion
) const
{
Expand All @@ -368,7 +381,7 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
result.erase( k );
try
{
graphene::chain::verify_authority( operations, result, get_active, get_owner, max_recursion );
graphene::chain::verify_authority( operations, result, get_active, get_owner, allow_non_immediate_owner, max_recursion );
continue; // element stays erased if verify_authority is ok
}
catch( const tx_missing_owner_auth& e ) {}
Expand Down Expand Up @@ -406,9 +419,15 @@ void signed_transaction::verify_authority(
const chain_id_type& chain_id,
const std::function<const authority*(account_id_type)>& get_active,
const std::function<const authority*(account_id_type)>& get_owner,
bool allow_non_immediate_owner,
uint32_t max_recursion )const
{ try {
graphene::chain::verify_authority( operations, get_signature_keys( chain_id ), get_active, get_owner, max_recursion );
graphene::chain::verify_authority( operations,
get_signature_keys( chain_id ),
get_active,
get_owner,
allow_non_immediate_owner,
max_recursion );
} FC_CAPTURE_AND_RETHROW( (*this) ) }

} } // graphene::chain
Loading