Skip to content

Commit

Permalink
Accept only a required authority. Redirection into stronger authoriti…
Browse files Browse the repository at this point in the history
…es is not accepted
  • Loading branch information
Mariusz-Trela committed Nov 25, 2024
1 parent d70d232 commit f9b2f04
Show file tree
Hide file tree
Showing 33 changed files with 1,006 additions and 570 deletions.
3 changes: 2 additions & 1 deletion libraries/chain/database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4400,7 +4400,8 @@ void database::validate_transaction(const std::shared_ptr<full_transaction_type>
const flat_set<public_key_type>& signature_keys = full_transaction->get_signature_keys();
const required_authorities_type& required_authorities = full_transaction->get_required_authorities();

hive::protocol::verify_authority(required_authorities,
hive::protocol::verify_authority(has_hardfork( HIVE_HARDFORK_1_28_STRICT_AUTHORITY_LEVEL ),
required_authorities,
signature_keys,
get_active,
get_owner,
Expand Down
3 changes: 3 additions & 0 deletions libraries/plugins/apis/database_api/database_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,7 @@ DEFINE_API_IMPL( database_api_impl, get_potential_signatures )
DEFINE_API_IMPL( database_api_impl, verify_authority )
{
args.trx.verify_authority(
_db.has_hardfork( HIVE_HARDFORK_1_28_STRICT_AUTHORITY_LEVEL ),
_db.get_chain_id(),
[&]( string account_name ){ return authority( _db.get< chain::account_authority_object, chain::by_account >( account_name ).active ); },
[&]( string account_name ){ return authority( _db.get< chain::account_authority_object, chain::by_account >( account_name ).owner ); },
Expand Down Expand Up @@ -1827,6 +1828,7 @@ DEFINE_API_IMPL( database_api_impl, verify_account_authority )
}

bool ok = hive::protocol::has_authorization(
_db.has_hardfork( HIVE_HARDFORK_1_28_STRICT_AUTHORITY_LEVEL ),
required_authorities,
args.signers,
[&]( string account_name ) { return authority( _db.get< chain::account_authority_object, chain::by_account >( account_name ).active ); },
Expand Down Expand Up @@ -1856,6 +1858,7 @@ DEFINE_API_IMPL( database_api_impl, verify_signatures )
try
{
hive::protocol::verify_authority< verify_signatures_args >(
_db.has_hardfork( HIVE_HARDFORK_1_28_STRICT_AUTHORITY_LEVEL ),
{ args },
sig_keys,
[this]( const string& name ) { return authority( _db.get< chain::account_authority_object, chain::by_account >( name ).owner ); },
Expand Down
2 changes: 1 addition & 1 deletion libraries/plugins/colony/colony_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ void colony_plugin_impl::start( uint32_t block_num )
required_authorities_type required_authorities;
required_authorities.required_active.insert( account.get_name() );

if( hive::protocol::has_authorization( required_authorities, common_keys, get_active, get_owner, get_posting, get_witness_key ) )
if( hive::protocol::has_authorization( _db.has_hardfork( HIVE_HARDFORK_1_28_STRICT_AUTHORITY_LEVEL ), required_authorities, common_keys, get_active, get_owner, get_posting, get_witness_key ) )
{
if( i < _max_threads )
threadI = _threads.emplace( _threads.end(), *this, (uint8_t)i );
Expand Down
2 changes: 2 additions & 0 deletions libraries/protocol/hardfork.d/1_28.hf
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ long next_hf_time();
#define HIVE_HARDFORK_1_28_GLOBAL_WITNESS_PROPS (HIVE_HARDFORK_1_28)
// expiration time for a transaction is longer
#define HIVE_HARDFORK_1_28_EXPIRATION_TIME (HIVE_HARDFORK_1_28)
// accept only required authority. Redirection into stronger authorities is not accepted
#define HIVE_HARDFORK_1_28_STRICT_AUTHORITY_LEVEL (HIVE_HARDFORK_1_28)

#endif
2 changes: 1 addition & 1 deletion libraries/protocol/include/hive/protocol/sign_state.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct sign_state
const authority_getter& a,
const flat_set<public_key_type>& keys );

const authority_getter& get_active;
const authority_getter& get_current_authority;
const flat_set<public_key_type>& available_keys;

flat_map<public_key_type,bool> provided_signatures;
Expand Down
2 changes: 2 additions & 0 deletions libraries/protocol/include/hive/protocol/transaction.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ namespace hive { namespace protocol {
)const;

void verify_authority(
bool strict_authority_level,
const chain_id_type& chain_id,
const authority_getter& get_active,
const authority_getter& get_owner,
Expand All @@ -85,6 +86,7 @@ namespace hive { namespace protocol {
)const;

set<public_key_type> minimize_required_signatures(
bool strict_authority_level,
const chain_id_type& chain_id,
const flat_set<public_key_type>& available_keys,
const authority_getter& get_active,
Expand Down
12 changes: 8 additions & 4 deletions libraries/protocol/include/hive/protocol/transaction_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ required_authorities_type get_required_authorities(const vector<AuthContainerTyp
return result;
} FC_CAPTURE_AND_RETHROW((auth_containers)) }

void verify_authority(const required_authorities_type& required_authorities,
void verify_authority(bool strict_authority_level,
const required_authorities_type& required_authorities,
const flat_set<public_key_type>& sigs,
const authority_getter& get_active,
const authority_getter& get_owner,
Expand All @@ -40,15 +41,17 @@ void verify_authority(const required_authorities_type& required_authorities,
const flat_set<account_name_type>& owner_approvals = flat_set<account_name_type>(),
const flat_set<account_name_type>& posting_approvals = flat_set<account_name_type>());

bool has_authorization( const required_authorities_type& required_authorities,
bool has_authorization( bool strict_authority_level,
const required_authorities_type& required_authorities,
const flat_set<public_key_type>& sigs,
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
const witness_public_key_getter& get_witness_key );

template< typename AuthContainerType >
void verify_authority(const vector<AuthContainerType>& auth_containers,
void verify_authority(bool strict_authority_level,
const vector<AuthContainerType>& auth_containers,
const flat_set<public_key_type>& sigs,
const authority_getter& get_active,
const authority_getter& get_owner,
Expand All @@ -62,7 +65,8 @@ void verify_authority(const vector<AuthContainerType>& auth_containers,
const flat_set<account_name_type>& owner_approvals = flat_set<account_name_type>(),
const flat_set<account_name_type>& posting_approvals = flat_set<account_name_type>())
{
verify_authority(get_required_authorities(auth_containers),
verify_authority(strict_authority_level,
get_required_authorities(auth_containers),
sigs,
get_active,
get_owner,
Expand Down
6 changes: 3 additions & 3 deletions libraries/protocol/sign_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ bool sign_state::check_authority( const string& id )
{
if( approved_by.find(id) != approved_by.end() ) return true;
uint32_t account_auth_count = 1;
return check_authority_impl( get_active(id), 0, &account_auth_count );
return check_authority_impl( get_current_authority(id), 0, &account_auth_count );
}

bool sign_state::check_authority( const authority& auth, uint32_t depth, uint32_t account_auth_count )
Expand Down Expand Up @@ -62,7 +62,7 @@ bool sign_state::check_authority_impl( const authority& auth, uint32_t depth, ui

(*account_auth_count)++;

if( check_authority_impl( get_active( a.first ), depth + 1, account_auth_count ) )
if( check_authority_impl( get_current_authority( a.first ), depth + 1, account_auth_count ) )
{
approved_by.insert( a.first );
total_weight += a.second;
Expand Down Expand Up @@ -102,7 +102,7 @@ sign_state::sign_state(
const flat_set<public_key_type>& sigs,
const authority_getter& a,
const flat_set<public_key_type>& keys
) : get_active(a), available_keys(keys)
) : get_current_authority(a), available_keys(keys)
{
for( const auto& key : sigs )
provided_signatures[ key ] = false;
Expand Down
8 changes: 6 additions & 2 deletions libraries/protocol/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ set<public_key_type> signed_transaction::get_required_signatures(
}

set<public_key_type> signed_transaction::minimize_required_signatures(
bool strict_authority_level,
const chain_id_type& chain_id,
const flat_set< public_key_type >& available_keys,
const authority_getter& get_active,
Expand All @@ -199,6 +200,7 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
try
{
hive::protocol::verify_authority(
strict_authority_level,
operations,
result,
get_active,
Expand All @@ -223,7 +225,8 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
return set<public_key_type>( result.begin(), result.end() );
}

void signed_transaction::verify_authority(const chain_id_type& chain_id,
void signed_transaction::verify_authority(bool strict_authority_level,
const chain_id_type& chain_id,
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
Expand All @@ -233,7 +236,8 @@ void signed_transaction::verify_authority(const chain_id_type& chain_id,
uint32_t max_membership,
uint32_t max_account_auths) const
{ try {
hive::protocol::verify_authority(operations,
hive::protocol::verify_authority(strict_authority_level,
operations,
get_signature_keys(chain_id, pack),
get_active,
get_owner,
Expand Down
34 changes: 26 additions & 8 deletions libraries/protocol/transaction_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum class verify_authority_problem

template< typename PROBLEM_HANDLER, typename OTHER_AUTH_PROBLEM_HANDLER >
void verify_authority_impl(
bool strict_authority_level,
const required_authorities_type& required_authorities,
const flat_set<public_key_type>& sigs,
const authority_getter& get_active,
Expand Down Expand Up @@ -73,8 +74,15 @@ FC_EXPAND_MACRO( \
s.approved_by.insert( id );
for( const auto& id : required_authorities.required_posting )
{
VERIFY_AUTHORITY_CHECK( s.check_authority( id ) || s.check_authority( get_active( id ) ) ||
s.check_authority( get_owner( id ) ), verify_authority_problem::missing_posting, id );
if( strict_authority_level )
{
VERIFY_AUTHORITY_CHECK( s.check_authority( id ), verify_authority_problem::missing_posting, id );
}
else
{
VERIFY_AUTHORITY_CHECK( s.check_authority( id ) || s.check_authority( get_active( id ) ) ||
s.check_authority( get_owner( id ) ), verify_authority_problem::missing_posting, id );
}
}
VERIFY_AUTHORITY_CHECK( !s.remove_unused_signatures(),
verify_authority_problem::unused_signature, account_name_type() );
Expand All @@ -99,8 +107,16 @@ FC_EXPAND_MACRO( \
// fetch all of the top level authorities
for( const auto& id : required_authorities.required_active )
{
VERIFY_AUTHORITY_CHECK( s.check_authority( id ) || s.check_authority( get_owner( id ) ),
verify_authority_problem::missing_active, id );
if( strict_authority_level )
{
VERIFY_AUTHORITY_CHECK( s.check_authority( id ),
verify_authority_problem::missing_active, id );
}
else
{
VERIFY_AUTHORITY_CHECK( s.check_authority( id ) || s.check_authority( get_owner( id ) ),
verify_authority_problem::missing_active, id );
}
}

for( const auto& id : required_authorities.required_owner )
Expand Down Expand Up @@ -132,7 +148,8 @@ FC_EXPAND_MACRO( \
#undef VERIFY_AUTHORITY_CHECK_OTHER_AUTH
}

void verify_authority(const required_authorities_type& required_authorities,
void verify_authority(bool strict_authority_level,
const required_authorities_type& required_authorities,
const flat_set<public_key_type>& sigs,
const authority_getter& get_active,
const authority_getter& get_owner,
Expand All @@ -146,7 +163,7 @@ void verify_authority(const required_authorities_type& required_authorities,
const flat_set<account_name_type>& owner_approvals /* = flat_set<account_name_type>() */,
const flat_set<account_name_type>& posting_approvals /* = flat_set<account_name_type>() */)
{ try {
verify_authority_impl( required_authorities, sigs,
verify_authority_impl( strict_authority_level, required_authorities, sigs,
get_active, get_owner, get_posting, get_witness_key,
max_recursion_depth, max_membership, max_account_auths,
active_approvals, owner_approvals, posting_approvals,
Expand Down Expand Up @@ -196,15 +213,16 @@ FC_EXPAND_MACRO( \
#undef VERIFY_AUTHORITY_THROW
} FC_CAPTURE_AND_RETHROW((sigs)) }

bool has_authorization( const required_authorities_type& required_authorities,
bool has_authorization( bool strict_authority_level,
const required_authorities_type& required_authorities,
const flat_set<public_key_type>& sigs,
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
const witness_public_key_getter& get_witness_key )
{
bool result = true;
verify_authority_impl( required_authorities, sigs,
verify_authority_impl( strict_authority_level, required_authorities, sigs,
get_active, get_owner, get_posting, get_witness_key,
HIVE_MAX_SIG_CHECK_DEPTH, HIVE_MAX_AUTHORITY_MEMBERSHIP, HIVE_MAX_SIG_CHECK_ACCOUNTS,
flat_set<account_name_type>(), flat_set<account_name_type>(), flat_set<account_name_type>(),
Expand Down
5 changes: 5 additions & 0 deletions libraries/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,12 @@ class wallet_api_impl
{
const signed_transaction& signature_source = new_tx->get_transaction();

protocol::hardfork_version _result = _remote_wallet_bridge_api->get_hardfork_version({}, LOCK);

bool _strict_authority_level = _result.minor_v() >= 28;

auto minimal_signing_keys = signature_source.minimize_required_signatures(
_strict_authority_level,
_hive_chain_id,
available_keys,
[&]( const string& account_name ) { return collector.get_account( account_name ).active; },
Expand Down
22 changes: 14 additions & 8 deletions tests/python/functional/authority/test_account_authority.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_owner_account_authority(node: tt.InitNode, authority_level: str, alice:
).account_auths, f"Account {bob.name} was not set as account_auths for the account {alice.name}."

if authority_level == "owner":
assert node.api.database.verify_account_authority(
assert not node.api.database.verify_account_authority(
account=alice.name, signers=[tt.PublicKey(bob.name, secret="active")], level="active"
).valid

Expand Down Expand Up @@ -188,16 +188,22 @@ def test_signing_with_circular_account_authority(
│ │
● posting ● posting
"""
# bob as carols owner account authority set carols active authority to redirect to bob
# bob as carol's owner account authority can't set carol's active authority to redirect to bob
bob.wallet.api.use_authority("active", bob.name)
bob.wallet.api.update_account_auth_account(carol.name, "active", bob.name, 1)
assert (
len(get_authority(node, carol.name, authority_level="active").account_auths) == 1
), "Carol's active-account_auths is not set"
with pytest.raises(tt.exceptions.CommunicationError):
bob.wallet.api.update_account_auth_account(carol.name, "active", bob.name, 1)

# now bob can sign owner type operation (use active bob authority)
# bob can't sign owner type operation (use active bob authority)
bob.wallet.api.use_authority("active", bob.name)
bob.wallet.api.sign_transaction(decline_voting_rights)
with pytest.raises(tt.exceptions.CommunicationError):
bob.wallet.api.sign_transaction(decline_voting_rights)

carol.wallet.api.import_key(tt.PrivateKey(account_name=carol.name, secret="owner"))
assert len(carol.wallet.api.list_keys()) == 1, "Carol's wallet has an incorrect number of imported keys. Expected 1"
carol.wallet.api.use_authority("owner", carol.name)
# carol can't sign owner type operation (use owner carol authority)
with pytest.raises(tt.exceptions.CommunicationError):
carol.wallet.api.sign_transaction(decline_voting_rights)


@run_for("testnet")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_update_account_active_authority(alice: UpdateAccount, authority_type: s
"""
alice.use_authority(authority_type)
new_auth = alice.generate_new_authority()
if authority_type == "posting":
if authority_type == "posting" or authority_type == "owner":
with pytest.raises(RequestError) as exception:
alice.update_account(active=new_auth, use_account_update2=use_account_update2)
alice.assert_if_rc_current_mana_was_unchanged()
Expand All @@ -78,7 +78,7 @@ def test_update_account_posting_authority(alice: UpdateAccount, authority_type:
"""
alice.use_authority(authority_type)
new_auth = alice.generate_new_authority()
if authority_type == "posting":
if authority_type == "posting" or authority_type == "owner":
with pytest.raises(RequestError) as exception:
alice.update_account(posting=new_auth, use_account_update2=use_account_update2)
alice.assert_if_rc_current_mana_was_unchanged()
Expand All @@ -104,7 +104,7 @@ def test_update_account_memo_key(alice: UpdateAccount, authority_type: str, use_
"""
alice.use_authority(authority_type)
new_key = alice.generate_new_key()
if authority_type == "posting":
if authority_type == "posting" or authority_type == "owner":
with pytest.raises(RequestError) as exception:
alice.update_account(memo_key=new_key, use_account_update2=use_account_update2)
alice.assert_if_rc_current_mana_was_unchanged()
Expand All @@ -130,7 +130,7 @@ def test_update_json_metadata(alice: UpdateAccount, authority_type: str, use_acc
"""
alice.use_authority(authority_type)
new_json_meta = '{"foo": "bar"}'
if authority_type == "posting":
if authority_type == "posting" or authority_type == "owner":
with pytest.raises(RequestError) as exception:
alice.update_account(json_metadata=new_json_meta, use_account_update2=use_account_update2)
alice.assert_if_rc_current_mana_was_unchanged()
Expand All @@ -153,9 +153,16 @@ def test_update_posting_json_metadata(alice: UpdateAccount, authority_type: str)
"""
alice.use_authority(authority_type)
new_posting_json_meta = '{"foo": "bar"}'
transaction = alice.update_account(posting_json_metadata=new_posting_json_meta, use_account_update2=True)
alice.assert_if_rc_current_mana_was_reduced(transaction)
alice.assert_account_details_were_changed(new_posting_json_meta=new_posting_json_meta)
if authority_type == "active" or authority_type == "owner":
with pytest.raises(RequestError) as exception:
alice.update_account(posting_json_metadata=new_posting_json_meta, use_account_update2=True)
alice.assert_if_rc_current_mana_was_unchanged()
error_response = exception.value.error
assert "Missing Posting Authority" in error_response
else:
transaction = alice.update_account(posting_json_metadata=new_posting_json_meta, use_account_update2=True)
alice.assert_if_rc_current_mana_was_reduced(transaction)
alice.assert_account_details_were_changed(new_posting_json_meta=new_posting_json_meta)


@pytest.mark.testnet()
Expand Down
Loading

0 comments on commit f9b2f04

Please sign in to comment.