Skip to content

Commit

Permalink
Explictly use signature canonization and accept BIP-0062 canonization…
Browse files Browse the repository at this point in the history 
… after HF20 #1944
  • Loading branch information
@mvandeberg
mvandeberg committed Dec 28, 2017
1 parent 1130c5c commit 29900df
Show file tree
Hide file tree
Showing 20 changed files with 575 additions and 556 deletions.
12 changes: 7 additions & 5 deletions libraries/chain/database.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2924,7 +2924,8 @@ void database::_apply_transaction(const signed_transaction& trx)

try
{
trx.verify_authority( chain_id, get_active, get_owner, get_posting, STEEM_MAX_SIG_CHECK_DEPTH );
trx.verify_authority( chain_id, get_active, get_owner, get_posting, STEEM_MAX_SIG_CHECK_DEPTH,
has_hardfork( STEEM_HARDFORK_0_20__1944 ) ? fc::ecc::bip_0062 : fc::ecc::fc_canonical );
}
catch( protocol::tx_missing_active_auth& e )
{
Expand Down Expand Up @@ -2994,7 +2995,8 @@ const witness_object& database::validate_block_header( uint32_t skip, const sign
const witness_object& witness = get_witness( next_block.witness );

if( !(skip&skip_witness_signature) )
FC_ASSERT( next_block.validate_signee( witness.signing_key ) );
FC_ASSERT( next_block.validate_signee( witness.signing_key,
has_hardfork( STEEM_HARDFORK_0_20__1944 ) ? fc::ecc::canonical_signature_type::bip_0062 : fc::ecc::canonical_signature_type::fc_canonical ) );

if( !(skip&skip_witness_schedule_check) )
{
Expand Down Expand Up @@ -4339,15 +4341,15 @@ void database::validate_smt_invariants()const
const auto& rewards_balance_idx = get_index< account_rewards_balance_index, by_id >();
add_from_balance_index( rewards_balance_idx, theMap );

// - Total vesting
// - Total vesting
#pragma message( "TODO: Add SMT vesting support here once it is implemented." )

// - Market orders
#pragma message( "TODO: Add limit_order_object iteration here once they support SMTs." )

// - Reward funds
#pragma message( "TODO: Add reward_fund_object iteration here once they support SMTs." )

// - Escrow & savings - no support of SMT is expected.

// Do the verification of total balances.
Expand Down
2 changes: 1 addition & 1 deletion libraries/fc
Submodule fc updated 6 files
+12 −3 include/fc/crypto/elliptic.hpp
+48 −7 src/crypto/elliptic_common.cpp
+2 −2 src/crypto/elliptic_impl_priv.cpp
+23 −26 src/crypto/elliptic_secp256k1.cpp
+4 −0 tests/CMakeLists.txt
+72 −0 tests/crypto/ecdsa_canon_test.cpp
8 changes: 4 additions & 4 deletions libraries/net/node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1425,8 +1425,8 @@ namespace graphene { namespace net { namespace detail {
wlog( "Sending a keepalive message to peer ${peer} who hasn't sent us any messages in the last ${timeout} seconds",
( "peer", active_peer->get_remote_endpoint() )("timeout", active_send_keepalive_timeout ) );
peers_to_send_keep_alive.push_back(active_peer);
}
else if (active_peer->we_need_sync_items_from_peer &&
}
else if (active_peer->we_need_sync_items_from_peer &&
!active_peer->is_currently_handling_message() &&
!active_peer->item_ids_requested_from_peer &&
active_peer->ids_of_items_to_get.empty())
Expand Down Expand Up @@ -1913,7 +1913,7 @@ namespace graphene { namespace net { namespace detail {
fc::sha256::encoder shared_secret_encoder;
fc::sha512 shared_secret = originating_peer->get_shared_secret();
shared_secret_encoder.write(shared_secret.data(), sizeof(shared_secret));
fc::ecc::public_key expected_node_public_key(hello_message_received.signed_shared_secret, shared_secret_encoder.result(), false);
fc::ecc::public_key expected_node_public_key(hello_message_received.signed_shared_secret, shared_secret_encoder.result(), fc::ecc::canonical_signature_type::non_canonical);

// store off the data provided in the hello message
originating_peer->user_agent = hello_message_received.user_agent;
Expand Down Expand Up @@ -3344,7 +3344,7 @@ namespace graphene { namespace net { namespace detail {
("endpoint", peer->get_remote_endpoint())("len", peer->ids_of_items_being_processed.size()));

// if we just processed the last item in our list from this peer, we will want to
// send another request to find out if we are now in sync (this is normally handled in
// send another request to find out if we are now in sync (this is normally handled in
// send_sync_block_to_node_delegate)
if (peer->ids_of_items_to_get.empty() &&
peer->number_of_unfetched_item_ids == 0 &&
Expand Down
2 changes: 1 addition & 1 deletion libraries/plugins/apis/block_api/include/steem/plugins/block_api/block_api_objects.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ struct api_signed_block_object : public signed_block
api_signed_block_object( const signed_block& block ) : signed_block( block )
{
block_id = id();
signing_key = signee();
signing_key = signee( fc::ecc::canonical_signature_type::bip_0062 );
transaction_ids.reserve( transactions.size() );
for( const signed_transaction& tx : transactions )
transaction_ids.push_back( tx.id() );
Expand Down
11 changes: 7 additions & 4 deletions libraries/plugins/apis/database_api/database_api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,7 +1373,8 @@ DEFINE_API_IMPL( database_api_impl, get_required_signatures )
[&]( 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 ); },
[&]( string account_name ){ return authority( _db.get< chain::account_authority_object, chain::by_account >( account_name ).posting ); },
STEEM_MAX_SIG_CHECK_DEPTH );
STEEM_MAX_SIG_CHECK_DEPTH,
_db.has_hardfork( STEEM_HARDFORK_0_20__1944 ) ? fc::ecc::canonical_signature_type::bip_0062 : fc::ecc::canonical_signature_type::fc_canonical );

return result;
}
Expand Down Expand Up @@ -1405,7 +1406,8 @@ DEFINE_API_IMPL( database_api_impl, get_potential_signatures )
result.keys.insert( k );
return authority( auth );
},
STEEM_MAX_SIG_CHECK_DEPTH
STEEM_MAX_SIG_CHECK_DEPTH,
_db.has_hardfork( STEEM_HARDFORK_0_20__1944 ) ? fc::ecc::canonical_signature_type::bip_0062 : fc::ecc::canonical_signature_type::fc_canonical
);

return result;
Expand All @@ -1417,7 +1419,8 @@ DEFINE_API_IMPL( database_api_impl, verify_authority )
[&]( 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 ); },
[&]( string account_name ){ return authority( _db.get< chain::account_authority_object, chain::by_account >( account_name ).posting ); },
STEEM_MAX_SIG_CHECK_DEPTH );
STEEM_MAX_SIG_CHECK_DEPTH,
_db.has_hardfork( STEEM_HARDFORK_0_20__1944 ) ? fc::ecc::canonical_signature_type::bip_0062 : fc::ecc::canonical_signature_type::fc_canonical );
return verify_authority_return( { true } );
}

Expand All @@ -1444,7 +1447,7 @@ DEFINE_API_IMPL( database_api_impl, verify_signatures )
for( const auto& sig : args.signatures )
{
STEEM_ASSERT(
sig_keys.insert( fc::ecc::public_key( sig, args.hash ) ).second,
sig_keys.insert( fc::ecc::public_key( sig, args.hash, fc::ecc::canonical_signature_type::bip_0062 ) ).second,
protocol::tx_duplicate_sig,
"Duplicate Signature detected" );
}
Expand Down
2 changes: 1 addition & 1 deletion ...ies/plugins/apis/database_api/include/steem/plugins/database_api/database_api_objects.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ struct api_signed_block_object : public signed_block
api_signed_block_object( const signed_block& block ) : signed_block( block )
{
block_id = id();
signing_key = signee();
signing_key = signee( fc::ecc::canonical_signature_type::bip_0062 );
transaction_ids.reserve( transactions.size() );
for( const signed_transaction& tx : transactions )
transaction_ids.push_back( tx.id() );
Expand Down
12 changes: 6 additions & 6 deletions libraries/protocol/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ namespace steem { namespace protocol {
return result;
}

fc::ecc::public_key signed_block_header::signee()const
fc::ecc::public_key signed_block_header::signee( fc::ecc::canonical_signature_type canon_type )const
{
return fc::ecc::public_key( witness_signature, digest(), true/*enforce canonical*/ );
return fc::ecc::public_key( witness_signature, digest(), canon_type /*enforce canonical*/ );
}

void signed_block_header::sign( const fc::ecc::private_key& signer )
void signed_block_header::sign( const fc::ecc::private_key& signer, fc::ecc::canonical_signature_type canon_type )
{
witness_signature = signer.sign_compact( digest() );
witness_signature = signer.sign_compact( digest(), canon_type );
}

bool signed_block_header::validate_signee( const fc::ecc::public_key& expected_signee )const
bool signed_block_header::validate_signee( const fc::ecc::public_key& expected_signee, fc::ecc::canonical_signature_type canon_type )const
{
return signee() == expected_signee;
return signee( canon_type ) == expected_signee;
}

checksum_type signed_block::calculate_merkle_root()const
Expand Down
1 change: 1 addition & 0 deletions libraries/protocol/hardfork.d/0_20.hf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#define STEEM_HARDFORK_0_20__1765 (STEEM_HARDFORK_0_20)
#define STEEM_HARDFORK_0_20__1811 (STEEM_HARDFORK_0_20)
#define STEEM_HARDFORK_0_20__1815 (STEEM_HARDFORK_0_20)
#define STEEM_HARDFORK_0_20__1944 (STEEM_HARDFORK_0_20)

#define STEEM_HARDFORK_0_20_VERSION hardfork_version( 0, 20 )

Expand Down
6 changes: 3 additions & 3 deletions libraries/protocol/include/steem/protocol/block_header.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ namespace steem { namespace protocol {
struct signed_block_header : public block_header
{
block_id_type id()const;
fc::ecc::public_key signee()const;
void sign( const fc::ecc::private_key& signer );
bool validate_signee( const fc::ecc::public_key& expected_signee )const;
fc::ecc::public_key signee( fc::ecc::canonical_signature_type canon_type )const;
void sign( const fc::ecc::private_key& signer, fc::ecc::canonical_signature_type canon_type = fc::ecc::fc_canonical );
bool validate_signee( const fc::ecc::public_key& expected_signee, fc::ecc::canonical_signature_type canon_type )const;

signature_type witness_signature;
};
Expand Down
20 changes: 13 additions & 7 deletions libraries/protocol/include/steem/protocol/transaction.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

namespace steem { namespace protocol {

using fc::ecc::canonical_signature_type;

struct transaction
{
uint16_t ref_block_num = 0;
Expand Down Expand Up @@ -53,36 +55,40 @@ namespace steem { namespace protocol {
signed_transaction( const transaction& trx = transaction() )
: transaction(trx){}

const signature_type& sign( const private_key_type& key, const chain_id_type& chain_id );
const signature_type& sign( const private_key_type& key, const chain_id_type& chain_id, canonical_signature_type canon_type/* = fc::ecc::fc_canonical*/ );

signature_type sign( const private_key_type& key, const chain_id_type& chain_id )const;
signature_type sign( const private_key_type& key, const chain_id_type& chain_id, canonical_signature_type canon_type/* = fc::ecc::fc_canonical*/ )const;

set<public_key_type> get_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 = STEEM_MAX_SIG_CHECK_DEPTH
uint32_t max_recursion/* = STEEM_MAX_SIG_CHECK_DEPTH */,
canonical_signature_type canon_type/* = fc::ecc::fc_canonical */
)const;

void verify_authority(
const chain_id_type& chain_id,
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
uint32_t max_recursion = STEEM_MAX_SIG_CHECK_DEPTH )const;
uint32_t max_recursion/* = STEEM_MAX_SIG_CHECK_DEPTH */,
canonical_signature_type canon_type/* = fc::ecc::fc_canonical*/
)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 = STEEM_MAX_SIG_CHECK_DEPTH
) const;
uint32_t max_recursion /*= STEEM_MAX_SIG_CHECK_DEPTH*/,
canonical_signature_type canon_type/* = fc::ecc::fc_canonical*/
)const;

flat_set<public_key_type> get_signature_keys( const chain_id_type& chain_id )const;
flat_set<public_key_type> get_signature_keys( const chain_id_type& chain_id, canonical_signature_type canon_type/* = fc::ecc::fc_canonical*/ )const;

vector<signature_type> signatures;

Expand Down
12 changes: 6 additions & 6 deletions libraries/protocol/steem_operations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,10 +361,10 @@ namespace steem { namespace protocol {
void pow::create( const fc::ecc::private_key& w, const digest_type& i )
{
input = i;
signature = w.sign_compact(input,false);
signature = w.sign_compact(input,fc::ecc::canonical_signature_type::non_canonical);

auto sig_hash = fc::sha256::hash( signature );
public_key_type recover = fc::ecc::public_key( signature, sig_hash, false );
public_key_type recover = fc::ecc::public_key( signature, sig_hash, fc::ecc::canonical_signature_type::non_canonical );

work = fc::sha256::hash(recover);
}
Expand All @@ -376,7 +376,7 @@ namespace steem { namespace protocol {

auto prv_key = fc::sha256::hash( input );
auto input = fc::sha256::hash( prv_key );
auto signature = fc::ecc::private_key::regenerate( prv_key ).sign_compact(input);
auto signature = fc::ecc::private_key::regenerate( prv_key ).sign_compact(input, fc::ecc::canonical_signature_type::fc_canonical );

auto sig_hash = fc::sha256::hash( signature );
public_key_type recover = fc::ecc::public_key( signature, sig_hash );
Expand All @@ -399,9 +399,9 @@ namespace steem { namespace protocol {
void pow::validate()const
{
FC_ASSERT( work != fc::sha256() );
FC_ASSERT( public_key_type(fc::ecc::public_key( signature, input, false )) == worker );
FC_ASSERT( public_key_type(fc::ecc::public_key( signature, input, fc::ecc::canonical_signature_type::non_canonical )) == worker );
auto sig_hash = fc::sha256::hash( signature );
public_key_type recover = fc::ecc::public_key( signature, sig_hash, false );
public_key_type recover = fc::ecc::public_key( signature, sig_hash, fc::ecc::canonical_signature_type::non_canonical );
FC_ASSERT( work == fc::sha256::hash(recover) );
}

Expand Down Expand Up @@ -473,7 +473,7 @@ namespace steem { namespace protocol {
validate_account_name( first_block.witness );
FC_ASSERT( first_block.witness == second_block.witness );
FC_ASSERT( first_block.timestamp == second_block.timestamp );
FC_ASSERT( first_block.signee() == second_block.signee() );
FC_ASSERT( first_block.signee( fc::ecc::canonical_signature_type::bip_0062 ) == second_block.signee( fc::ecc::canonical_signature_type::bip_0062 ) );
FC_ASSERT( first_block.id() != second_block.id() );
}

Expand Down
29 changes: 16 additions & 13 deletions libraries/protocol/transaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,19 @@ steem::protocol::transaction_id_type steem::protocol::transaction::id() const
return result;
}

const signature_type& steem::protocol::signed_transaction::sign(const private_key_type& key, const chain_id_type& chain_id)
const signature_type& steem::protocol::signed_transaction::sign( const private_key_type& key, const chain_id_type& chain_id, canonical_signature_type canon_type )
{
digest_type h = sig_digest( chain_id );
signatures.push_back(key.sign_compact(h));
signatures.push_back(key.sign_compact(h, canon_type));
return signatures.back();
}

signature_type steem::protocol::signed_transaction::sign(const private_key_type& key, const chain_id_type& chain_id)const
signature_type steem::protocol::signed_transaction::sign( const private_key_type& key, const chain_id_type& chain_id, canonical_signature_type canon_type )const
{
digest_type::encoder enc;
fc::raw::pack( enc, chain_id );
fc::raw::pack( enc, *this );
return key.sign_compact(enc.result());
return key.sign_compact(enc.result(), canon_type);
}

void transaction::set_expiration( fc::time_point_sec expiration_time )
Expand All @@ -82,14 +82,14 @@ void transaction::get_required_authorities( flat_set< account_name_type >& activ
operation_get_required_authorities( op, active, owner, posting, other );
}

flat_set<public_key_type> signed_transaction::get_signature_keys( const chain_id_type& chain_id )const
flat_set<public_key_type> signed_transaction::get_signature_keys( const chain_id_type& chain_id, canonical_signature_type canon_type )const
{ try {
auto d = sig_digest( chain_id );
flat_set<public_key_type> result;
for( const auto& sig : signatures )
{
STEEM_ASSERT(
result.insert( fc::ecc::public_key(sig,d) ).second,
result.insert( fc::ecc::public_key( sig, d, canon_type ) ).second,
tx_duplicate_sig,
"Duplicate Signature detected" );
}
Expand All @@ -104,7 +104,8 @@ set<public_key_type> signed_transaction::get_required_signatures(
const authority_getter& get_active,
const authority_getter& get_owner,
const authority_getter& get_posting,
uint32_t max_recursion_depth )const
uint32_t max_recursion_depth,
canonical_signature_type canon_type )const
{
flat_set< account_name_type > required_active;
flat_set< account_name_type > required_owner;
Expand All @@ -114,7 +115,7 @@ set<public_key_type> signed_transaction::get_required_signatures(

/** posting authority cannot be mixed with active authority in same transaction */
if( required_posting.size() ) {
sign_state s(get_signature_keys( chain_id ),get_posting,available_keys);
sign_state s(get_signature_keys( chain_id, canon_type ),get_posting,available_keys);
s.max_recursion = max_recursion_depth;

FC_ASSERT( !required_owner.size() );
Expand All @@ -134,7 +135,7 @@ set<public_key_type> signed_transaction::get_required_signatures(
}


sign_state s(get_signature_keys( chain_id ),get_active,available_keys);
sign_state s(get_signature_keys( chain_id, canon_type ),get_active,available_keys);
s.max_recursion = max_recursion_depth;

for( const auto& auth : other )
Expand All @@ -161,10 +162,11 @@ 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,
canonical_signature_type canon_type
) const
{
set< public_key_type > s = get_required_signatures( chain_id, available_keys, get_active, get_owner, get_posting, max_recursion );
set< public_key_type > s = get_required_signatures( chain_id, available_keys, get_active, get_owner, get_posting, max_recursion, canon_type );
flat_set< public_key_type > result( s.begin(), s.end() );

for( const public_key_type& k : s )
Expand All @@ -189,9 +191,10 @@ 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,
canonical_signature_type canon_type )const
{ try {
steem::protocol::verify_authority( operations, get_signature_keys( chain_id ), get_active, get_owner, get_posting, max_recursion );
steem::protocol::verify_authority( operations, get_signature_keys( chain_id, canon_type ), get_active, get_owner, get_posting, max_recursion );
} FC_CAPTURE_AND_RETHROW( (*this) ) }

} } // steem::protocol
Loading

0 comments on commit 29900df

Please sign in to comment.