Skip to content

Commit

Permalink
Issue bitshares#1129 - Fix for Transaction Malleqbility
Browse files Browse the repository at this point in the history 
This patch creates a hard fork where signature format becomes far more
strict than SSL strictly allows.   It depends upon a recent update to FC
which allows optional strict checking of signatures.

Conflicts:
	libraries/blockchain/include/bts/blockchain/checkpoints.hpp
	libraries/blockchain/include/bts/blockchain/fork_blocks.hpp
	libraries/blockchain/include/bts/blockchain/transaction_evaluation_state.hpp
	libraries/fc
  • Loading branch information
@bytemaster @pmconrad
bytemaster authored and pmconrad committed Dec 15, 2014
1 parent 62a53a2 commit d7440a2
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 46 deletions.
8 changes: 4 additions & 4 deletions libraries/blockchain/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ namespace bts { namespace blockchain {
return fc::ripemd160::hash( enc.result() );
}

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, bool enforce_canonical )const
{
return fc::ecc::public_key( delegate_signature, digest() ) == expected_signee;
return fc::ecc::public_key( delegate_signature, digest(), enforce_canonical ) == expected_signee;
}

public_key_type signed_block_header::signee()const
public_key_type signed_block_header::signee( bool enforce_canonical )const
{
return fc::ecc::public_key( delegate_signature, digest() );
return fc::ecc::public_key( delegate_signature, digest(), enforce_canonical );
}

void signed_block_header::sign( const fc::ecc::private_key& signer )
Expand Down
12 changes: 7 additions & 5 deletions libraries/blockchain/chain_database.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,8 @@ namespace bts { namespace blockchain {
//ilog( "applying ${trx}", ("trx",trx) );
transaction_evaluation_state_ptr trx_eval_state =
std::make_shared<transaction_evaluation_state>(pending_state.get(), _chain_id);
trx_eval_state->evaluate( trx, _skip_signature_verification );
trx_eval_state->evaluate( trx, _skip_signature_verification,
block.block_num > BTS_CHECK_CANONICAL_SIGNATURE_FORK_BLOCK_NUM );
//ilog( "evaluation: ${e}", ("e",*trx_eval_state) );
// TODO: capture the evaluation state with a callback for wallets...
// summary.transaction_states.emplace_back( std::move(trx_eval_state) );
Expand Down Expand Up @@ -1013,7 +1014,7 @@ namespace bts { namespace blockchain {
block_signee = self->get_slot_signee( block_data.timestamp, self->get_active_delegates() ).active_key();
else
/* We need the block_signee's key in several places and computing it is expensive, so compute it here and pass it down */
block_signee = block_data.signee();
block_signee = block_data.signee( block_data.block_num > BTS_CHECK_CANONICAL_SIGNATURE_FORK_BLOCK_NUM );

auto checkpoint_itr = CHECKPOINT_BLOCKS.find(block_data.block_num);
if( checkpoint_itr != CHECKPOINT_BLOCKS.end() && checkpoint_itr->second != block_id )
Expand Down Expand Up @@ -1545,7 +1546,7 @@ namespace bts { namespace blockchain {
pending_chain_state_ptr pend_state = std::make_shared<pending_chain_state>(my->_pending_trx_state);
transaction_evaluation_state_ptr trx_eval_state = std::make_shared<transaction_evaluation_state>(pend_state.get(), my->_chain_id);

trx_eval_state->evaluate( trx );
trx_eval_state->evaluate( trx, false, pend_state->get_head_block_num() > BTS_CHECK_CANONICAL_SIGNATURE_FORK_BLOCK_NUM );
auto fees = trx_eval_state->get_fees() + trx_eval_state->alt_fees_paid.amount;
if( fees < required_fees )
{
Expand All @@ -1565,7 +1566,8 @@ namespace bts { namespace blockchain {
auto pending_state = std::make_shared<pending_chain_state>( shared_from_this() );
transaction_evaluation_state_ptr eval_state = std::make_shared<transaction_evaluation_state>( pending_state.get(), my->_chain_id );

eval_state->evaluate( transaction );
eval_state->evaluate( transaction, false,
pending_state->get_head_block_num() > BTS_CHECK_CANONICAL_SIGNATURE_FORK_BLOCK_NUM );
auto fees = eval_state->get_fees();
if( fees < min_fee )
FC_CAPTURE_AND_THROW( insufficient_relay_fee, (fees)(min_fee) );
Expand Down Expand Up @@ -2038,7 +2040,7 @@ namespace bts { namespace blockchain {

try
{
trx_eval_state->evaluate( item->trx );
trx_eval_state->evaluate( item->trx, false, true );
// TODO: what about fees in other currencies?
total_fees += trx_eval_state->get_fees( 0 );
/* Apply temporary state to block state */
Expand Down
4 changes: 2 additions & 2 deletions libraries/blockchain/include/bts/blockchain/block.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ namespace bts { namespace blockchain {
struct signed_block_header : public block_header
{
block_id_type id()const;
bool validate_signee( const fc::ecc::public_key& expected_signee )const;
public_key_type signee()const;
bool validate_signee( const fc::ecc::public_key& expected_signee, bool enforce_canoncial = false )const;
public_key_type signee( bool enforce_canoncial = false )const;
void sign( const fc::ecc::private_key& signer );

signature_type delegate_signature;
Expand Down
34 changes: 2 additions & 32 deletions libraries/blockchain/include/bts/blockchain/fork_blocks.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,9 @@
#include <stdint.h>
#include <vector>

/** @file bts/blockchain/fork_blocks.hpp
* @brief Defines global block number constants for when hardforks take effect
*/
//#define BTSX_MARKET_FORK_1_BLOCK_NUM 274000
//#define BTSX_MARKET_FORK_2_BLOCK_NUM 316001
//#define BTSX_MARKET_FORK_3_BLOCK_NUM 340000
//#define BTSX_MARKET_FORK_4_BLOCK_NUM 357000
//#define BTSX_MARKET_FORK_5_BLOCK_NUM 408750
//#define BTSX_MARKET_FORK_6_BLOCK_NUM 451500
//#define BTSX_MARKET_FORK_7_BLOCK_NUM 554800
//#define BTSX_MARKET_FORK_8_BLOCK_NUM 578900
//#define BTSX_MARKET_FORK_9_BLOCK_NUM 613200
//#define BTSX_MARKET_FORK_10_BLOCK_NUM 640000
//#define BTSX_MARKET_FORK_11_BLOCK_NUM 820200
//#define BTSX_MARKET_FORK_12_BLOCK_NUM 871000
//
//#define BTSX_MARKET_FORK_TO_UNIX_TIME_LIST ((BTSX_MARKET_FORK_1_BLOCK_NUM, "0.4.0", 1408064036)) \
// ((BTSX_MARKET_FORK_2_BLOCK_NUM, "0.4.9-RC1", 1409096675)) \
// ((BTSX_MARKET_FORK_3_BLOCK_NUM, "0.4.9", 1409193626)) \
// ((BTSX_MARKET_FORK_4_BLOCK_NUM, "0.4.10", 1409437355)) \
// ((BTSX_MARKET_FORK_5_BLOCK_NUM, "0.4.12", 1409846462)) \
// ((BTSX_MARKET_FORK_6_BLOCK_NUM, "0.4.13-RC1", 1410288486)) \
// ((BTSX_MARKET_FORK_7_BLOCK_NUM, "0.4.16", 1411258737)) \
// ((BTSX_MARKET_FORK_8_BLOCK_NUM, "0.4.17", 1411599233)) \
// ((BTSX_MARKET_FORK_9_BLOCK_NUM, "0.4.18", 1411765631)) \
// ((BTSX_MARKET_FORK_10_BLOCK_NUM, "0.4.19", 1412203442)) \
// ((BTSX_MARKET_FORK_11_BLOCK_NUM, "0.4.21", 1413928884))
#define BTSX_MARKET_FORK_TO_UNIX_TIME_LIST
//#define BTSX_YIELD_FORK_1_BLOCK_NUM BTSX_MARKET_FORK_6_BLOCK_NUM
//#define BTSX_YIELD_FORK_2_BLOCK_NUM 494000
//
//#define BTSX_SUPPLY_FORK_1_BLOCK_NUM BTSX_MARKET_FORK_7_BLOCK_NUM
//#define BTSX_SUPPLY_FORK_2_BLOCK_NUM BTSX_MARKET_FORK_8_BLOCK_NUM

#define BTS_CHECK_CANONICAL_SIGNATURE_FORK_BLOCK_NUM BTS_V0_4_25_FORK_BLOCK_NUM

namespace bts { namespace blockchain {
uint32_t estimate_last_known_fork_from_git_revision_timestamp(uint32_t revision_time);
Expand Down
2 changes: 1 addition & 1 deletion libraries/blockchain/include/bts/blockchain/transaction_evaluation_state.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace bts { namespace blockchain {

virtual void reset();

virtual void evaluate( const signed_transaction& trx, bool skip_signature_check = false );
virtual void evaluate( const signed_transaction& trx, bool skip_signature_check = false, bool enforce_canonical = false );
virtual void evaluate_operation( const operation& op );

/** perform any final operations based upon the current state of
Expand Down
4 changes: 2 additions & 2 deletions libraries/blockchain/transaction_evaluation_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ namespace bts { namespace blockchain {

} FC_RETHROW_EXCEPTIONS( warn, "" ) }

void transaction_evaluation_state::evaluate( const signed_transaction& trx_arg, bool skip_signature_check )
void transaction_evaluation_state::evaluate( const signed_transaction& trx_arg, bool skip_signature_check, bool enforce_canonical )
{ try {
reset();
_skip_signature_check = skip_signature_check;
Expand All @@ -201,7 +201,7 @@ namespace bts { namespace blockchain {
auto digest = trx_arg.digest( _chain_id );
for( const auto& sig : trx.signatures )
{
auto key = fc::ecc::public_key( sig, digest ).serialize();
auto key = fc::ecc::public_key( sig, digest, enforce_canonical ).serialize();
signed_keys.insert( address(key) );
signed_keys.insert( address(pts_address(key,false,56) ) );
signed_keys.insert( address(pts_address(key,true,56) ) );
Expand Down

0 comments on commit d7440a2

Please sign in to comment.