From b3d96d1b863b9b5460d47f9fb40989fa8fc9ff14 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Fri, 8 Dec 2023 17:11:22 +0100 Subject: [PATCH 1/2] Use standard app crypto helper to simplify io_seproxyhal_touch_tx_ok --- Makefile | 3 ++ src_features/signTx/ui_common_signTx.c | 48 +++++++++++--------------- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index a2f9b02..119abfd 100644 --- a/Makefile +++ b/Makefile @@ -266,6 +266,9 @@ else APP_SOURCE_PATH += src_bagl endif +# Allow usage of function from lib_standard_app/crypto_helpers.c +APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/crypto_helpers.c + ### initialize plugin SDK submodule if needed, rebuild it, and warn if a difference is noticed ifeq ($(CHAIN),ethereum) ifneq ($(shell git submodule status | grep '^[-+]'),) diff --git a/src_features/signTx/ui_common_signTx.c b/src_features/signTx/ui_common_signTx.c index e57bb21..67db726 100644 --- a/src_features/signTx/ui_common_signTx.c +++ b/src_features/signTx/ui_common_signTx.c @@ -1,3 +1,4 @@ +#include "lib_standard_app/crypto_helpers.h" #include "os_io_seproxyhal.h" #include "shared_context.h" #include "utils.h" @@ -5,30 +6,21 @@ #include "handle_swap_sign_transaction.h" unsigned int io_seproxyhal_touch_tx_ok(__attribute__((unused)) const bagl_element_t *e) { - uint8_t privateKeyData[INT256_LENGTH]; - uint8_t signature[100]; - cx_ecfp_private_key_t privateKey; - uint32_t tx = 0; + uint32_t info = 0; int err; - io_seproxyhal_io_heartbeat(); - os_perso_derive_node_bip32(CX_CURVE_256K1, - tmpCtx.transactionContext.bip32.path, - tmpCtx.transactionContext.bip32.length, - privateKeyData, - NULL); - cx_ecfp_init_private_key(CX_CURVE_256K1, privateKeyData, 32, &privateKey); - explicit_bzero(privateKeyData, sizeof(privateKeyData)); - unsigned int info = 0; - io_seproxyhal_io_heartbeat(); - cx_ecdsa_sign(&privateKey, - CX_RND_RFC6979 | CX_LAST, - CX_SHA256, - tmpCtx.transactionContext.hash, - sizeof(tmpCtx.transactionContext.hash), - signature, - sizeof(signature), - &info); - explicit_bzero(&privateKey, sizeof(privateKey)); + if (bip32_derive_ecdsa_sign_rs_hash_256(CX_CURVE_256K1, + tmpCtx.transactionContext.bip32.path, + tmpCtx.transactionContext.bip32.length, + CX_RND_RFC6979 | CX_LAST, + CX_SHA256, + tmpCtx.transactionContext.hash, + sizeof(tmpCtx.transactionContext.hash), + G_io_apdu_buffer + 1, + G_io_apdu_buffer + 1 + 32, + &info) != CX_OK) { + THROW(0x6F00); + } + if (txContext.txType == EIP1559 || txContext.txType == EIP2930) { if (info & CX_ECCINFO_PARITY_ODD) { G_io_apdu_buffer[0] = 1; @@ -57,13 +49,13 @@ unsigned int io_seproxyhal_touch_tx_ok(__attribute__((unused)) const bagl_elemen G_io_apdu_buffer[0] += 2; } } - format_signature_out(signature); - tx = 65; - G_io_apdu_buffer[tx++] = 0x90; - G_io_apdu_buffer[tx++] = 0x00; + + // Write status code at parity_byte + r + s + G_io_apdu_buffer[1 + 64] = 0x90; + G_io_apdu_buffer[1 + 64 + 1] = 0x00; // Send back the response, do not restart the event loop - err = io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, tx); + err = io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, 1 + 64 + 2); if (G_called_from_swap) { PRINTF("G_called_from_swap\n"); From c35398240516da6268c540598e4eb15e31be2f1c Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Fri, 8 Dec 2023 17:26:44 +0100 Subject: [PATCH 2/2] During signature, move parsing and swap fields checking away from main flow --- src_features/signTx/logic_signTx.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 08f031c..b5bb875 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -319,13 +319,14 @@ static int strcasecmp_workaround(const char *str1, const char *str2) { return 0; } -void finalizeParsing(bool direct) { +__attribute__((noinline)) static void finalize_parsing_helper(bool direct, bool *use_standard_UI) { char displayBuffer[50]; uint8_t decimals = WEI_TO_ETHER; uint64_t chain_id = get_tx_chain_id(); const char *ticker = get_displayable_ticker(&chain_id); ethPluginFinalize_t pluginFinalize; - bool use_standard_UI = true; + + *use_standard_UI = true; // Verify the chain if (chainConfig->chainId != ETHEREUM_MAINNET_CHAINID) { @@ -396,7 +397,7 @@ void finalizeParsing(bool direct) { switch (pluginFinalize.uiType) { case ETH_UI_TYPE_GENERIC: // Use the dedicated ETH plugin UI - use_standard_UI = false; + *use_standard_UI = false; tmpContent.txContent.dataPresent = false; // Add the number of screens + the number of additional screens to get the total // number of screens needed. @@ -405,7 +406,7 @@ void finalizeParsing(bool direct) { break; case ETH_UI_TYPE_AMOUNT_ADDRESS: // Use the standard ETH UI as this plugin uses the amount/address UI - use_standard_UI = true; + *use_standard_UI = true; tmpContent.txContent.dataPresent = false; if ((pluginFinalize.amount == NULL) || (pluginFinalize.address == NULL)) { PRINTF("Incorrect amount/address set by plugin\n"); @@ -443,7 +444,7 @@ void finalizeParsing(bool direct) { } // User has just validated a swap but ETH received apdus about a non standard plugin / contract - if (G_called_from_swap && !use_standard_UI) { + if (G_called_from_swap && !*use_standard_UI) { PRINTF("ERR_SILENT_MODE_CHECK_FAILED, G_called_from_swap\n"); THROW(ERR_SILENT_MODE_CHECK_FAILED); } @@ -457,7 +458,7 @@ void finalizeParsing(bool direct) { } // Prepare destination address and amount to display - if (use_standard_UI) { + if (*use_standard_UI) { // Format the address in a temporary buffer, if in swap case compare it with validated // address, else commit it address_to_string(tmpContent.txContent.destination, @@ -532,11 +533,14 @@ void finalizeParsing(bool direct) { // Prepare network field get_network_as_string(strings.common.network_name, sizeof(strings.common.network_name)); PRINTF("Network: %s\n", strings.common.network_name); +} +void finalizeParsing(bool direct) { + bool use_standard_UI; bool no_consent_check; - - // If called from swap, the user as already validated a standard transaction - // We have already checked the fields of this transaction above + finalize_parsing_helper(direct, &use_standard_UI); + // If called from swap, the user has already validated a standard transaction + // And we have already checked the fields of this transaction above no_consent_check = G_called_from_swap && use_standard_UI; #ifdef NO_CONSENT