From e5f55d4920e7d0923c026ca8cf71f70cadc41462 Mon Sep 17 00:00:00 2001 From: mcchadwick Date: Thu, 26 Aug 2021 09:33:56 -0600 Subject: [PATCH 01/11] Enforce proper field ordering --- lib/firmware/ethereum.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index b67eec2f..02af7638 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -47,9 +47,9 @@ #define MAX_CHAIN_ID 2147483630 -#define ETHEREUM_TX_TYPE_LEGACY 0 -#define ETHEREUM_TX_TYPE_EIP_2930 1 -#define ETHEREUM_TX_TYPE_EIP_1559 2 +#define ETHEREUM_TX_TYPE_LEGACY 0UL +#define ETHEREUM_TX_TYPE_EIP_2930 1UL +#define ETHEREUM_TX_TYPE_EIP_1559 2UL static bool ethereum_signing = false; static uint32_t data_total, data_left; @@ -801,12 +801,12 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, rlp_length += rlp_calculate_length(msg->nonce.size, msg->nonce.bytes[0]); if (msg->has_max_fee_per_gas) { - rlp_length += rlp_calculate_length(msg->max_fee_per_gas.size, - msg->max_fee_per_gas.bytes[0]); if (msg->has_max_priority_fee_per_gas) { rlp_length += rlp_calculate_length(msg->max_priority_fee_per_gas.size, msg->max_priority_fee_per_gas.bytes[0]); } + rlp_length += rlp_calculate_length(msg->max_fee_per_gas.size, + msg->max_fee_per_gas.bytes[0]); } else { rlp_length += rlp_calculate_length(msg->gas_price.size, msg->gas_price.bytes[0]); } @@ -819,10 +819,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, if (wanchain_tx_type) { rlp_length += rlp_calculate_number_length(wanchain_tx_type); } - - if (ethereum_tx_type) { - rlp_length += rlp_calculate_number_length(ethereum_tx_type); - } + + // TODO: Determins whether type field should be hashed + // if (ethereum_tx_type) { + // rlp_length += rlp_calculate_number_length(ethereum_tx_type); + // } if (chain_id) { rlp_length += rlp_calculate_number_length(chain_id); @@ -833,9 +834,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, /* Stage 2: Store header fields */ hash_rlp_list_length(rlp_length); layoutProgress(_("Signing"), 100); - if (ethereum_tx_type) { - hash_rlp_number(ethereum_tx_type); - } + + // TODO: Determins whether type field should be hashed + // if (ethereum_tx_type) { + // hash_rlp_number(ethereum_tx_type); + // } if (wanchain_tx_type) { hash_rlp_number(wanchain_tx_type); } @@ -843,11 +846,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, hash_rlp_field(msg->nonce.bytes, msg->nonce.size); if (msg->has_max_fee_per_gas) { - hash_rlp_field(msg->max_fee_per_gas.bytes, msg->max_fee_per_gas.size); if (msg->has_max_priority_fee_per_gas) { hash_rlp_field(msg->max_priority_fee_per_gas.bytes, msg->max_priority_fee_per_gas.size); } + hash_rlp_field(msg->max_fee_per_gas.bytes, msg->max_fee_per_gas.size); } else { hash_rlp_field(msg->gas_price.bytes, msg->gas_price.size); } From ffd661a50650d29298346f2e393f43a2510b68f2 Mon Sep 17 00:00:00 2001 From: markrypto Date: Thu, 2 Sep 2021 09:59:42 -0600 Subject: [PATCH 02/11] eip1559 improvements --- lib/firmware/ethereum.c | 72 ++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index 02af7638..fa8db4ee 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -270,12 +270,14 @@ static void send_signature(void) { uint8_t v; layoutProgress(_("Signing"), 1000); - /* eip-155 replay protection */ - if (chain_id) { - /* hash v=chain_id, r=0, s=0 */ - hash_rlp_number(chain_id); - hash_rlp_length(0, 0); - hash_rlp_length(0, 0); + if (ethereum_tx_type == ETHEREUM_TX_TYPE_LEGACY) { + /* legacy eip-155 replay protection */ + if (chain_id) { + /* hash v=chain_id, r=0, s=0 */ + hash_rlp_number(chain_id); + hash_rlp_length(0, 0); + hash_rlp_length(0, 0); + } } keccak_Final(&keccak_ctx, hash); @@ -574,7 +576,7 @@ static void layoutEthereumFee(const EthereumSignTx *msg, bool is_token, } /* - * RLP fields: + * RLP fields: (legacy) * - nonce (0 .. 32) * - gas_price (0 .. 32) * - gas_limit (0 .. 32) @@ -799,6 +801,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, uint32_t rlp_length = 0; layoutProgress(_("Signing"), 0); + if (ethereum_tx_type == 2) { + // This is the chain ID length for 1559 tx + rlp_length += 1; + } + rlp_length += rlp_calculate_length(msg->nonce.size, msg->nonce.bytes[0]); if (msg->has_max_fee_per_gas) { if (msg->has_max_priority_fee_per_gas) { @@ -816,33 +823,48 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, rlp_length += rlp_calculate_length(msg->value.size, msg->value.bytes[0]); rlp_length += rlp_calculate_length(data_total, msg->data_initial_chunk.bytes[0]); + if (ethereum_tx_type == 2) { + // access list size + rlp_length += 1; // c0, keepkey does not support >0 length access list at this time + } + if (wanchain_tx_type) { rlp_length += rlp_calculate_number_length(wanchain_tx_type); } - - // TODO: Determins whether type field should be hashed - // if (ethereum_tx_type) { - // rlp_length += rlp_calculate_number_length(ethereum_tx_type); - // } - - if (chain_id) { - rlp_length += rlp_calculate_number_length(chain_id); - rlp_length += rlp_calculate_length(0, 0); - rlp_length += rlp_calculate_length(0, 0); + + if (ethereum_tx_type == ETHEREUM_TX_TYPE_LEGACY) { + // legacy EIP-155 replay protection + if (chain_id) { + rlp_length += rlp_calculate_number_length(chain_id); + rlp_length += rlp_calculate_length(0, 0); + rlp_length += rlp_calculate_length(0, 0); + } + } + + // Start the hash: + // keccak256(0x02 || rlp([chain_id, nonce, max_priority_fee_per_gas, max_fee_per_gas, + // gas_limit, destination, amount, data, access_list])) + + // tx type should never be greater than one byte in length + // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2718.md#transactiontype-only-goes-up-to-0x7f + if (ethereum_tx_type == 2) { + uint8_t datbuf[1] = {0x02}; + hash_data(datbuf, sizeof(datbuf)); } + layoutProgress(_("Signing"), 100); /* Stage 2: Store header fields */ hash_rlp_list_length(rlp_length); - layoutProgress(_("Signing"), 100); - // TODO: Determins whether type field should be hashed - // if (ethereum_tx_type) { - // hash_rlp_number(ethereum_tx_type); - // } if (wanchain_tx_type) { hash_rlp_number(wanchain_tx_type); } + if (ethereum_tx_type == 2) { + // chain id goes here for 1559 + hash_rlp_field((uint8_t *)(&chain_id), sizeof(chain_id)); + } + hash_rlp_field(msg->nonce.bytes, msg->nonce.size); if (msg->has_max_fee_per_gas) { @@ -862,6 +884,12 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, hash_data(msg->data_initial_chunk.bytes, msg->data_initial_chunk.size); data_left = data_total - msg->data_initial_chunk.size; + if (ethereum_tx_type == 2) { + // Keepkey does not support an access list size >0 at this time + uint8_t datbuf[1] = {0xC0}; // size of empty access list + hash_data(datbuf, sizeof(datbuf)); + } + memcpy(privkey, node->private_key, 32); if (data_left > 0) { From fcbe1b7c06f89e28fdb987f7555b559f7170e488 Mon Sep 17 00:00:00 2001 From: markrypto Date: Thu, 2 Sep 2021 17:02:42 -0600 Subject: [PATCH 03/11] fixed chain_id size --- lib/firmware/ethereum.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index fa8db4ee..398ef1e0 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -136,6 +136,17 @@ void bn_from_bytes(const uint8_t *value, size_t value_len, bignum256 *val) { static inline void hash_data(const uint8_t *buf, size_t size) { sha3_Update(&keccak_ctx, buf, size); + // { + // // let's see the data + // char pstr[65] = {0}; + // uint16_t ctr; + + // for (ctr=0; ctr<=size; ctr+=2) { + // sprintf(&pstr[ctr], "%02x", buf[ctr/2]); + // } + // review(ButtonRequestType_ButtonRequest_Other, "hashed data", "%s", pstr); + // } + } /* @@ -288,6 +299,17 @@ static void send_signature(void) { return; } + // { + // // let's see the digest + // char pstr[65] = {0}; + // uint16_t ctr; + + // for (ctr=0; ctr<=64; ctr+=2) { + // sprintf(&pstr[ctr], "%02x", hash[ctr/2]); + // } + // review(ButtonRequestType_ButtonRequest_Other, "eth tx hash", "%s", pstr); + // } + memzero(privkey, sizeof(privkey)); /* Send back the result */ @@ -802,8 +824,10 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, layoutProgress(_("Signing"), 0); if (ethereum_tx_type == 2) { - // This is the chain ID length for 1559 tx - rlp_length += 1; + // This is the chain ID length for 1559 tx (only one byte for now) + rlp_length += rlp_calculate_number_length(chain_id); + + //rlp_length += 1; } rlp_length += rlp_calculate_length(msg->nonce.size, msg->nonce.bytes[0]); @@ -861,8 +885,8 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, } if (ethereum_tx_type == 2) { - // chain id goes here for 1559 - hash_rlp_field((uint8_t *)(&chain_id), sizeof(chain_id)); + // chain id goes here for 1559 (only one byte for now) + hash_rlp_field((uint8_t *)(&chain_id), sizeof(uint8_t)); } hash_rlp_field(msg->nonce.bytes, msg->nonce.size); From 1a8a0d9002823f46ca3f43f908c3bf398ff91509 Mon Sep 17 00:00:00 2001 From: markrypto Date: Thu, 9 Sep 2021 16:13:42 -0600 Subject: [PATCH 04/11] clean up source --- lib/firmware/ethereum.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index 398ef1e0..7bf4d686 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -136,17 +136,6 @@ void bn_from_bytes(const uint8_t *value, size_t value_len, bignum256 *val) { static inline void hash_data(const uint8_t *buf, size_t size) { sha3_Update(&keccak_ctx, buf, size); - // { - // // let's see the data - // char pstr[65] = {0}; - // uint16_t ctr; - - // for (ctr=0; ctr<=size; ctr+=2) { - // sprintf(&pstr[ctr], "%02x", buf[ctr/2]); - // } - // review(ButtonRequestType_ButtonRequest_Other, "hashed data", "%s", pstr); - // } - } /* @@ -299,17 +288,6 @@ static void send_signature(void) { return; } - // { - // // let's see the digest - // char pstr[65] = {0}; - // uint16_t ctr; - - // for (ctr=0; ctr<=64; ctr+=2) { - // sprintf(&pstr[ctr], "%02x", hash[ctr/2]); - // } - // review(ButtonRequestType_ButtonRequest_Other, "eth tx hash", "%s", pstr); - // } - memzero(privkey, sizeof(privkey)); /* Send back the result */ From 7816b02239b469455dbd88666e739d06067244e9 Mon Sep 17 00:00:00 2001 From: markrypto Date: Fri, 10 Sep 2021 11:21:17 -0600 Subject: [PATCH 05/11] updated python tests for eip1559, bump version --- CMakeLists.txt | 2 +- deps/python-keepkey | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 98f286af..7d5a5d99 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.7.2) project(KeepKeyFirmware - VERSION 7.2.0 + VERSION 7.2.1 LANGUAGES C CXX ASM) diff --git a/deps/python-keepkey b/deps/python-keepkey index 0eddb5bc..1584e53d 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 0eddb5bc317307af43765fd4ebe64f869fa33870 +Subproject commit 1584e53d81a8fa402dc7b9daa419f418ef816f24 From 1418708f8f5eed4f3896d7b6d1006d6d74476b86 Mon Sep 17 00:00:00 2001 From: markrypto Date: Fri, 10 Sep 2021 11:32:51 -0600 Subject: [PATCH 06/11] clarified tx type for eth --- lib/firmware/ethereum.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index 7bf4d686..e7c6c34c 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -801,7 +801,7 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, uint32_t rlp_length = 0; layoutProgress(_("Signing"), 0); - if (ethereum_tx_type == 2) { + if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { // This is the chain ID length for 1559 tx (only one byte for now) rlp_length += rlp_calculate_number_length(chain_id); @@ -825,7 +825,7 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, rlp_length += rlp_calculate_length(msg->value.size, msg->value.bytes[0]); rlp_length += rlp_calculate_length(data_total, msg->data_initial_chunk.bytes[0]); - if (ethereum_tx_type == 2) { + if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { // access list size rlp_length += 1; // c0, keepkey does not support >0 length access list at this time } @@ -849,7 +849,7 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, // tx type should never be greater than one byte in length // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2718.md#transactiontype-only-goes-up-to-0x7f - if (ethereum_tx_type == 2) { + if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { uint8_t datbuf[1] = {0x02}; hash_data(datbuf, sizeof(datbuf)); } @@ -862,7 +862,7 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, hash_rlp_number(wanchain_tx_type); } - if (ethereum_tx_type == 2) { + if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { // chain id goes here for 1559 (only one byte for now) hash_rlp_field((uint8_t *)(&chain_id), sizeof(uint8_t)); } @@ -886,7 +886,7 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, hash_data(msg->data_initial_chunk.bytes, msg->data_initial_chunk.size); data_left = data_total - msg->data_initial_chunk.size; - if (ethereum_tx_type == 2) { + if (ethereum_tx_type == ETHEREUM_TX_TYPE_EIP_1559) { // Keepkey does not support an access list size >0 at this time uint8_t datbuf[1] = {0xC0}; // size of empty access list hash_data(datbuf, sizeof(datbuf)); From 594a39014a1320a43b83eace287ceee81a13071d Mon Sep 17 00:00:00 2001 From: Reid Rankin Date: Fri, 10 Sep 2021 13:57:41 -0400 Subject: [PATCH 07/11] disallow illegal ethereum tx types --- deps/python-keepkey | 2 +- lib/firmware/ethereum.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index 1584e53d..92623366 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 1584e53d81a8fa402dc7b9daa419f418ef816f24 +Subproject commit 926233668933a91f4d6732694cd025a7cf7ab6f1 diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index e7c6c34c..145cf19f 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -653,11 +653,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, /* Ethereum tx type */ if (msg->has_type) { - if (msg->type == 0 || msg->type == 1 || msg->type == 2) { + if (msg->type == ETHEREUM_TX_TYPE_EIP_1559) { ethereum_tx_type = msg->type; } else { fsm_sendFailure(FailureType_Failure_SyntaxError, - _("Ethereum tx type out of bounds")); + _("Ethereum tx type not supported")); ethereum_signing_abort(); return; } From d0f0bc71141aaf6260b1e89ed22dbb5dbb3432a8 Mon Sep 17 00:00:00 2001 From: markrypto Date: Fri, 10 Sep 2021 15:15:33 -0600 Subject: [PATCH 08/11] latest python deps --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index 92623366..b697b08a 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 926233668933a91f4d6732694cd025a7cf7ab6f1 +Subproject commit b697b08a29676c780f3fb16106d902efcbabbe5a From 0ecb1e9eeebb3331c06b9264db24d6fbd291db3a Mon Sep 17 00:00:00 2001 From: markrypto Date: Fri, 10 Sep 2021 15:43:38 -0600 Subject: [PATCH 09/11] revert last two commits --- deps/python-keepkey | 2 +- lib/firmware/ethereum.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index b697b08a..1584e53d 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit b697b08a29676c780f3fb16106d902efcbabbe5a +Subproject commit 1584e53d81a8fa402dc7b9daa419f418ef816f24 diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index 145cf19f..e7c6c34c 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -653,11 +653,11 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, /* Ethereum tx type */ if (msg->has_type) { - if (msg->type == ETHEREUM_TX_TYPE_EIP_1559) { + if (msg->type == 0 || msg->type == 1 || msg->type == 2) { ethereum_tx_type = msg->type; } else { fsm_sendFailure(FailureType_Failure_SyntaxError, - _("Ethereum tx type not supported")); + _("Ethereum tx type out of bounds")); ethereum_signing_abort(); return; } From ef9c3b12887aa2f2fbf54e74d87f5e84fba2ed55 Mon Sep 17 00:00:00 2001 From: markrypto Date: Fri, 10 Sep 2021 15:50:26 -0600 Subject: [PATCH 10/11] python deps updated --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index 1584e53d..b697b08a 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 1584e53d81a8fa402dc7b9daa419f418ef816f24 +Subproject commit b697b08a29676c780f3fb16106d902efcbabbe5a From 577e22887fdb247fd7c788c312c5cb7394107a79 Mon Sep 17 00:00:00 2001 From: markrypto Date: Fri, 10 Sep 2021 15:54:19 -0600 Subject: [PATCH 11/11] hard fail on eth tx type not legacy or eip1559 --- lib/firmware/ethereum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/firmware/ethereum.c b/lib/firmware/ethereum.c index e7c6c34c..89345c63 100644 --- a/lib/firmware/ethereum.c +++ b/lib/firmware/ethereum.c @@ -653,7 +653,7 @@ void ethereum_signing_init(EthereumSignTx *msg, const HDNode *node, /* Ethereum tx type */ if (msg->has_type) { - if (msg->type == 0 || msg->type == 1 || msg->type == 2) { + if (msg->type == 0 || msg->type == 2) { ethereum_tx_type = msg->type; } else { fsm_sendFailure(FailureType_Failure_SyntaxError,