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.

Sign up for GitHub

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

Jump to bottom

Enforce auth membership limits #2716

Merged
merged 2 commits into from
Aug 7, 2018
Merged

Enforce auth membership limits #2716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5aa4a4
Enfore authority membership limits
mvandeberg Aug 6, 2018
bf7cc0d
Add more validation to more op
mvandeberg Aug 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libraries/chain/database.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2849,7 +2849,7 @@ void database::_apply_transaction(const signed_transaction& trx)

try
{
trx.verify_authority( chain_id, get_active, get_owner, get_posting, STEEMIT_MAX_SIG_CHECK_DEPTH );
trx.verify_authority( chain_id, get_active, get_owner, get_posting, STEEMIT_MAX_SIG_CHECK_DEPTH, is_producing() );
}
catch( protocol::tx_missing_active_auth& e )
{
Expand Down
50 changes: 50 additions & 0 deletions libraries/chain/steem_evaluator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ struct strcmp_equal
}
};

void validate_auth_size( const authority& a )
{
size_t size = a.account_auths.size() + a.key_auths.size();
FC_ASSERT( size <= STEEMIT_MAX_AUTHORITY_MEMBERSHIP, "Authority membership exceeded. Max: 10 Current: ${n}", ("n", size) );
}

void witness_update_evaluator::do_apply( const witness_update_operation& o )
{
_db.get_account( o.owner ); // verify owner exists
Expand Down Expand Up @@ -130,6 +136,13 @@ void account_create_evaluator::do_apply( const account_create_operation& o )
("p", o.fee) );
}

if( _db.is_producing() )
{
validate_auth_size( o.owner );
validate_auth_size( o.active );
validate_auth_size( o.posting );
}

if( _db.has_hardfork( STEEMIT_HARDFORK_0_15__465 ) )
{
for( auto& a : o.owner.account_auths )
Expand Down Expand Up @@ -215,6 +228,13 @@ void account_create_with_delegation_evaluator::do_apply( const account_create_wi
("f", wso.median_props.account_creation_fee)
("p", o.fee) );

if( _db.is_producing() )
{
validate_auth_size( o.owner );
validate_auth_size( o.active );
validate_auth_size( o.posting );
}

for( auto& a : o.owner.account_auths )
{
_db.get_account( a.first );
Expand Down Expand Up @@ -288,6 +308,16 @@ void account_update_evaluator::do_apply( const account_update_operation& o )
const auto& account = _db.get_account( o.account );
const auto& account_auth = _db.get< account_authority_object, by_account >( o.account );

if( _db.is_producing() )
{
if( o.owner )
validate_auth_size( *o.owner );
if( o.active )
validate_auth_size( *o.active );
if( o.posting )
validate_auth_size( *o.posting );
}

if( o.owner )
{
#ifndef IS_TEST_NET
Expand Down Expand Up @@ -1524,14 +1554,21 @@ void custom_evaluator::do_apply( const custom_operation& o )
{
database& d = db();
if( d.is_producing() )
{
FC_ASSERT( o.data.size() <= 8192, "custom_operation data must be less than 8k" );
FC_ASSERT( o.required_auths.size() <= STEEMIT_MAX_AUTHORITY_MEMBERSHIP, "Too many auths specified. Max: 10, Current: ${n}", ("n", o.required_auths.size()) );
}
}

void custom_json_evaluator::do_apply( const custom_json_operation& o )
{
database& d = db();
if( d.is_producing() )
{
FC_ASSERT( o.json.length() <= 8192, "custom_json_operation json must be less than 8k" );
size_t num_auths = o.required_auths.size() + o.required_posting_auths.size();
FC_ASSERT( num_auths <= STEEMIT_MAX_AUTHORITY_MEMBERSHIP, "Too many auths specified. Max: 10, Current: ${n}", ("n", num_auths) );
}

std::shared_ptr< custom_operation_interpreter > eval = d.get_custom_json_evaluator( o.id );
if( !eval )
Expand Down Expand Up @@ -1560,6 +1597,14 @@ void custom_binary_evaluator::do_apply( const custom_binary_operation& o )
{
FC_ASSERT( false, "custom_binary_operation is deprecated" );
FC_ASSERT( o.data.size() <= 8192, "custom_binary_operation data must be less than 8k" );

size_t num_auths = o.required_owner_auths.size() + o.required_active_auths.size() + o.required_posting_auths.size();
for( const auto& auth : o.required_auths )
{
num_auths += auth.key_auths.size() + auth.account_auths.size();
}

FC_ASSERT( num_auths <= STEEMIT_MAX_AUTHORITY_MEMBERSHIP, "Too many auths specified. Max: 10, Current: ${n}", ("n", num_auths) );
}
FC_ASSERT( d.has_hardfork( STEEMIT_HARDFORK_0_14__317 ) );

Expand Down Expand Up @@ -1936,6 +1981,11 @@ void request_account_recovery_evaluator::do_apply( const request_account_recover
FC_ASSERT( !o.new_owner_authority.is_impossible(), "Cannot recover using an impossible authority." );
FC_ASSERT( o.new_owner_authority.weight_threshold, "Cannot recover using an open authority." );

if( _db.is_producing() )
{
validate_auth_size( o.new_owner_authority );
}

// Check accounts in the new authority exist
if( ( _db.has_hardfork( STEEMIT_HARDFORK_0_15__465 ) ) )
{
Expand Down
5 changes: 3 additions & 2 deletions libraries/protocol/include/steemit/protocol/sign_state.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ struct sign_state
* produce a signature for this key, else returns false.
*/
bool signed_by( const public_key_type& k );
bool check_authority( string id );
bool check_authority( string id, bool enforce_membership_limit = false );

/**
* Checks to see if we have signatures of the active authorites of
* the accounts specified in authority or the keys specified.
*/
bool check_authority( const authority& au, uint32_t depth = 0 );
bool check_authority( const authority& au, uint32_t depth = 0, bool enforce_membership_limit = false );
bool check_authority( const authority& au, bool enforce_membership_limit );

bool remove_unused_signatures();

Expand Down
10 changes: 7 additions & 3 deletions libraries/protocol/include/steemit/protocol/transaction.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,18 @@ namespace steemit { namespace protocol {
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
uint32_t max_recursion = STEEMIT_MAX_SIG_CHECK_DEPTH )const;
uint32_t max_recursion = STEEMIT_MAX_SIG_CHECK_DEPTH,
bool enforce_membership_limit = false
)const;

set<public_key_type> minimize_required_signatures(
const chain_id_type& chain_id,
const flat_set<public_key_type>& available_keys,
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
uint32_t max_recursion = STEEMIT_MAX_SIG_CHECK_DEPTH
uint32_t max_recursion = STEEMIT_MAX_SIG_CHECK_DEPTH,
bool enforce_membership_limit = false
) const;

flat_set<public_key_type> get_signature_keys( const chain_id_type& chain_id )const;
Expand All @@ -99,7 +102,8 @@ namespace steemit { namespace protocol {
bool allow_committe = false,
const flat_set< account_name_type >& active_aprovals = flat_set< account_name_type >(),
const flat_set< account_name_type >& owner_aprovals = flat_set< account_name_type >(),
const flat_set< account_name_type >& posting_approvals = flat_set< account_name_type >());
const flat_set< account_name_type >& posting_approvals = flat_set< account_name_type >(),
bool enforce_membership_limit = false );


struct annotated_signed_transaction : public signed_transaction {
Expand Down
26 changes: 22 additions & 4 deletions libraries/protocol/sign_state.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,21 @@ bool sign_state::signed_by( const public_key_type& k )
return itr->second = true;
}

bool sign_state::check_authority( string id )
bool sign_state::check_authority( string id, bool enforce_membership_limit )
{
if( approved_by.find(id) != approved_by.end() ) return true;
return check_authority( get_active(id) );
return check_authority( get_active(id), enforce_membership_limit );
}

bool sign_state::check_authority( const authority& auth, uint32_t depth )
bool sign_state::check_authority( const authority& auth, bool enforce_membership_limit )
{
return check_authority( auth, 0, enforce_membership_limit );
}

bool sign_state::check_authority( const authority& auth, uint32_t depth, bool enforce_membership_limit )
{
uint32_t total_weight = 0;
size_t membership = 0;
for( const auto& k : auth.key_auths )
{
if( signed_by( k.first ) )
Expand All @@ -33,6 +39,12 @@ bool sign_state::check_authority( const authority& auth, uint32_t depth )
if( total_weight >= auth.weight_threshold )
return true;
}

membership++;
if( enforce_membership_limit && membership >= STEEMIT_MAX_AUTHORITY_MEMBERSHIP )
{
return false;
}
}

for( const auto& a : auth.account_auths )
Expand All @@ -41,7 +53,7 @@ bool sign_state::check_authority( const authority& auth, uint32_t depth )
{
if( depth == max_recursion )
continue;
if( check_authority( get_active( a.first ), depth+1 ) )
if( check_authority( get_active( a.first ), depth+1, enforce_membership_limit ) )
{
approved_by.insert( a.first );
total_weight += a.second;
Expand All @@ -55,6 +67,12 @@ bool sign_state::check_authority( const authority& auth, uint32_t depth )
if( total_weight >= auth.weight_threshold )
return true;
}

membership++;
if( enforce_membership_limit && membership >= STEEMIT_MAX_AUTHORITY_MEMBERSHIP )
{
return false;
}
}
return total_weight >= auth.weight_threshold;
}
Expand Down
51 changes: 38 additions & 13 deletions libraries/protocol/transaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t
bool allow_committe,
const flat_set< account_name_type >& active_aprovals,
const flat_set< account_name_type >& owner_approvals,
const flat_set< account_name_type >& posting_approvals
const flat_set< account_name_type >& posting_approvals,
bool enforce_membership_limit
)
{ try {
flat_set< account_name_type > required_active;
Expand Down Expand Up @@ -119,9 +120,9 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t
s.approved_by.insert( id );
for( auto id : required_posting )
{
STEEMIT_ASSERT( s.check_authority(id) ||
s.check_authority(get_active(id)) ||
s.check_authority(get_owner(id)),
STEEMIT_ASSERT( s.check_authority(id, enforce_membership_limit) ||
s.check_authority(get_active(id), enforce_membership_limit) ||
s.check_authority(get_owner(id), enforce_membership_limit),
tx_missing_posting_auth, "Missing Posting Authority ${id}",
("id",id)
("posting",get_posting(id))
Expand All @@ -146,21 +147,21 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t

for( const auto& auth : other )
{
STEEMIT_ASSERT( s.check_authority(auth), tx_missing_other_auth, "Missing Authority", ("auth",auth)("sigs",sigs) );
STEEMIT_ASSERT( s.check_authority(auth, enforce_membership_limit), tx_missing_other_auth, "Missing Authority", ("auth",auth)("sigs",sigs) );
}

// fetch all of the top level authorities
for( auto id : required_active )
{
STEEMIT_ASSERT( s.check_authority(id) ||
s.check_authority(get_owner(id)),
STEEMIT_ASSERT( s.check_authority(id, enforce_membership_limit) ||
s.check_authority(get_owner(id), enforce_membership_limit),
tx_missing_active_auth, "Missing Active Authority ${id}", ("id",id)("auth",get_active(id))("owner",get_owner(id)) );
}

for( auto id : required_owner )
{
STEEMIT_ASSERT( owner_approvals.find(id) != owner_approvals.end() ||
s.check_authority(get_owner(id)),
s.check_authority(get_owner(id), enforce_membership_limit),
tx_missing_owner_auth, "Missing Owner Authority ${id}", ("id",id)("auth",get_owner(id)) );
}

Expand Down Expand Up @@ -210,7 +211,7 @@ set<public_key_type> signed_transaction::get_required_signatures(
FC_ASSERT( !required_owner.size() );
FC_ASSERT( !required_active.size() );
for( auto& posting : required_posting )
s.check_authority( posting );
s.check_authority( posting );

s.remove_unused_signatures();

Expand Down Expand Up @@ -251,7 +252,8 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
uint32_t max_recursion
uint32_t max_recursion,
bool enforce_membership_limit
) const
{
set< public_key_type > s = get_required_signatures( chain_id, available_keys, get_active, get_owner, get_posting, max_recursion );
Expand All @@ -262,7 +264,18 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
result.erase( k );
try
{
steemit::protocol::verify_authority( operations, result, get_active, get_owner, get_posting, max_recursion );
steemit::protocol::verify_authority(
operations,
result,
get_active,
get_owner,
get_posting,
max_recursion,
false,
flat_set< account_name_type >(),
flat_set< account_name_type >(),
flat_set< account_name_type >(),
enforce_membership_limit );
continue; // element stays erased if verify_authority is ok
}
catch( const tx_missing_owner_auth& e ) {}
Expand All @@ -279,9 +292,21 @@ void signed_transaction::verify_authority(
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
uint32_t max_recursion )const
uint32_t max_recursion,
bool enforce_membership_limit )const
{ try {
steemit::protocol::verify_authority( operations, get_signature_keys( chain_id ), get_active, get_owner, get_posting, max_recursion );
steemit::protocol::verify_authority(
operations,
get_signature_keys( chain_id ),
get_active,
get_owner,
get_posting,
max_recursion,
false,
flat_set< account_name_type >(),
flat_set< account_name_type >(),
flat_set< account_name_type >(),
enforce_membership_limit );
} FC_CAPTURE_AND_RETHROW( (*this) ) }

} } // steemit::protocol
5 changes: 3 additions & 2 deletions libraries/wallet/wallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,8 @@ class wallet_api_impl
{ return (get_account_from_lut( account_name ).owner); },
[&]( const string& account_name ) -> const authority&
{ return (get_account_from_lut( account_name ).posting); },
STEEMIT_MAX_SIG_CHECK_DEPTH
STEEMIT_MAX_SIG_CHECK_DEPTH,
true
);

for( const public_key_type& k : minimal_signing_keys )
Expand Down Expand Up @@ -1766,7 +1767,7 @@ annotated_signed_transaction wallet_api::vote_for_witness(string voting_account,
void wallet_api::check_memo( const string& memo, const account_api_obj& account )const
{
vector< public_key_type > keys;

try
{
// Check if memo is a private key
Expand Down