From 210dfa0370bcd0e888e65fb95f619d40d72e5064 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 28 Sep 2022 16:06:51 +0200 Subject: [PATCH 01/10] Add Swap feature to Solana application --- src/globals.h | 4 +- src/main.c | 100 ++++++++++++++++++++-- src/signMessage.c | 78 +++++++++++++++-- src/swap/handle_check_address.c | 73 ++++++++++++++++ src/swap/handle_check_address.h | 5 ++ src/swap/handle_get_printable_amount.c | 33 +++++++ src/swap/handle_get_printable_amount.h | 5 ++ src/swap/handle_swap_sign_transaction.c | 109 ++++++++++++++++++++++++ src/swap/handle_swap_sign_transaction.h | 9 ++ src/swap/swap_lib_calls.c | 16 ++++ src/swap/swap_lib_calls.h | 74 ++++++++++++++++ 11 files changed, 490 insertions(+), 16 deletions(-) create mode 100644 src/swap/handle_check_address.c create mode 100644 src/swap/handle_check_address.h create mode 100644 src/swap/handle_get_printable_amount.c create mode 100644 src/swap/handle_get_printable_amount.h create mode 100644 src/swap/handle_swap_sign_transaction.c create mode 100644 src/swap/handle_swap_sign_transaction.h create mode 100644 src/swap/swap_lib_calls.c create mode 100644 src/swap/swap_lib_calls.h diff --git a/src/globals.h b/src/globals.h index 727505ba..6bb92771 100644 --- a/src/globals.h +++ b/src/globals.h @@ -52,6 +52,8 @@ typedef enum InstructionCode { InsSignOffchainMessage = 0x07 } InstructionCode; +extern bool G_called_from_swap; + // display stepped screens extern unsigned int ux_step; extern unsigned int ux_step_count; @@ -84,4 +86,4 @@ typedef struct internalStorage_t { extern const internalStorage_t N_storage_real; #define N_storage (*(volatile internalStorage_t*) PIC(&N_storage_real)) -#endif \ No newline at end of file +#endif diff --git a/src/main.c b/src/main.c index 60c00aed..5d7d1e41 100644 --- a/src/main.c +++ b/src/main.c @@ -22,8 +22,15 @@ #include "apdu.h" #include "menu.h" +// Swap feature +#include "swap_lib_calls.h" +#include "handle_swap_sign_transaction.h" +#include "handle_get_printable_amount.h" +#include "handle_check_address.h" + ApduCommand G_command; unsigned char G_io_seproxyhal_spi_buffer[IO_SEPROXYHAL_BUFFER_SIZE_B]; +bool G_called_from_swap; static void reset_main_globals(void) { MEMCLEAR(G_command); @@ -141,6 +148,8 @@ void app_main(void) { tx += 2; } FINALLY { + // Restrict silent swap mode to the first APDU received + G_called_from_swap = false; } } END_TRY; @@ -260,13 +269,8 @@ void nv_app_state_init() { } } -__attribute__((section(".boot"))) int main(void) { - // exit critical section - __asm volatile("cpsie i"); - - // ensure exception will work as planned - os_boot(); - +void coin_main(void) { + G_called_from_swap = false; for (;;) { UX_INIT(); @@ -307,5 +311,87 @@ __attribute__((section(".boot"))) int main(void) { END_TRY; } app_exit(); +} + +static void start_app_from_lib(void) { + G_called_from_swap = true; + UX_INIT(); + io_seproxyhal_init(); + nv_app_state_init(); + USB_power(0); + USB_power(1); +#ifdef HAVE_BLE + // Erase globals that may inherit values from exchange + MEMCLEAR(G_io_asynch_ux_callback); + // grab the current plane mode setting + G_io_app.plane_mode = os_setting_get(OS_SETTING_PLANEMODE, NULL, 0); + BLE_power(0, NULL); + BLE_power(1, "Nano X"); +#endif // HAVE_BLE + app_main(); +} + +static void library_main_helper(libargs_t *args) { + check_api_level(CX_COMPAT_APILEVEL); + switch (args->command) { + case CHECK_ADDRESS: + // ensure result is zero if an exception is thrown + args->check_address->result = 0; + args->check_address->result = handle_check_address(args->check_address); + break; + case SIGN_TRANSACTION: + if (copy_transaction_parameters(args->create_transaction)) { + // never returns + start_app_from_lib(); + } + break; + case GET_PRINTABLE_AMOUNT: + handle_get_printable_amount(args->get_printable_amount); + break; + default: + break; + } +} + +static void library_main(libargs_t *args) { + bool end = false; + /* This loop ensures that library_main_helper and os_lib_end are called + * within a try context, even if an exception is thrown */ + while (1) { + BEGIN_TRY { + TRY { + if (!end) { + library_main_helper(args); + } + os_lib_end(); + } + FINALLY { + end = true; + } + } + END_TRY; + } +} + +__attribute__((section(".boot"))) int main(int arg0) { + // exit critical section + __asm volatile("cpsie i"); + + // ensure exception will work as planned + os_boot(); + + if (arg0 == 0) { + // called from dashboard as standalone app + coin_main(); + } else { + // Called as library from another app + libargs_t *args = (libargs_t *) arg0; + if (args->id == 0x100) { + library_main(args); + } else { + app_exit(); + } + } + return 0; } diff --git a/src/signMessage.c b/src/signMessage.c index 2c559c0f..693edea3 100644 --- a/src/signMessage.c +++ b/src/signMessage.c @@ -12,6 +12,8 @@ #include "globals.h" #include "apdu.h" +#include "handle_swap_sign_transaction.h" + static uint8_t set_result_sign_message() { uint8_t signature[SIGNATURE_LENGTH]; cx_ecfp_private_key_t privateKey; @@ -137,6 +139,12 @@ void handle_sign_message_parse_message(volatile unsigned int *tx) { //*tx = set_result_sign_message(); // THROW(ApduReplySuccess); UNUSED(tx); + } + + if ((G_command.non_confirm || G_called_from_swap) && + !(G_command.non_confirm && G_called_from_swap)) { + // Blind sign requested NOT in swap context + // Or no blind sign requested while in swap context THROW(ApduReplySdkNotSupported); } @@ -168,22 +176,76 @@ void handle_sign_message_parse_message(volatile unsigned int *tx) { } } +static bool check_swap_validity(const SummaryItemKind_t kinds[MAX_TRANSACTION_SUMMARY_ITEMS], + size_t num_summary_steps) { + bool amount_ok = false; + bool recipient_ok = false; + bool fee_payer_ok = false; + if (num_summary_steps != 3) { + PRINTF("3 steps expected for transaction in swap context, not %u\n", num_summary_steps); + return false; + } + for (size_t i = 0; i < num_summary_steps; ++i) { + transaction_summary_display_item(i, DisplayFlagNone | DisplayFlagLongPubkeys); + PRINTF("G_transaction_summary_title = %s\n", G_transaction_summary_title); + PRINTF("G_transaction_summary_text = %s\n", G_transaction_summary_text); + switch (kinds[i]) { + case SummaryItemAmount: + if (strcmp(G_transaction_summary_title, "Transfer") == 0) { + amount_ok = check_swap_amount(G_transaction_summary_text); + } else { + PRINTF("Refused field '%s'\n", G_transaction_summary_title); + return false; + } + break; + case SummaryItemPubkey: + if (strcmp(G_transaction_summary_title, "Recipient") == 0) { + recipient_ok = check_swap_recipient(G_transaction_summary_text); + } else if (strcmp(G_transaction_summary_title, "Fee payer") == 0) { + fee_payer_ok = check_swap_fee_payer(G_transaction_summary_text); + } else { + PRINTF("Refused field '%s'\n", G_transaction_summary_title); + return false; + } + break; + default: + PRINTF("Refused kind '%u'\n", SummaryItemAmount); + return false; + } + } + return amount_ok && recipient_ok && fee_payer_ok; +} + void handle_sign_message_ui(volatile unsigned int *flags) { // Display the transaction summary SummaryItemKind_t summary_step_kinds[MAX_TRANSACTION_SUMMARY_ITEMS]; size_t num_summary_steps = 0; if (transaction_summary_finalize(summary_step_kinds, &num_summary_steps) == 0) { - size_t num_flow_steps = 0; + // If we are in swap context, do not redisplay the message data + // Instead, ensure they are identitical with what was previously displayed + if (G_called_from_swap) { + if (check_swap_validity(summary_step_kinds, num_summary_steps)) { + send_result_sign_message(); + // Quit app, we are in limited mode and our work is done + os_sched_exit(0); + } else { + PRINTF("Refused blind signing incorrect Swap transaction\n"); + sendResponse(0, false); + } + } else { + MEMCLEAR(flow_steps); + size_t num_flow_steps = 0; - for (size_t i = 0; i < num_summary_steps; i++) { - flow_steps[num_flow_steps++] = &ux_summary_step; - } + for (size_t i = 0; i < num_summary_steps; i++) { + flow_steps[num_flow_steps++] = &ux_summary_step; + } - flow_steps[num_flow_steps++] = &ux_approve_step; - flow_steps[num_flow_steps++] = &ux_reject_step; - flow_steps[num_flow_steps++] = FLOW_END_STEP; + flow_steps[num_flow_steps++] = &ux_approve_step; + flow_steps[num_flow_steps++] = &ux_reject_step; + flow_steps[num_flow_steps++] = FLOW_END_STEP; - ux_flow_init(0, flow_steps, NULL); + ux_flow_init(0, flow_steps, NULL); + } } else { THROW(ApduReplySolanaSummaryFinalizeFailed); } diff --git a/src/swap/handle_check_address.c b/src/swap/handle_check_address.c new file mode 100644 index 00000000..0f0a70c4 --- /dev/null +++ b/src/swap/handle_check_address.c @@ -0,0 +1,73 @@ +#include + +#include "handle_check_address.h" +#include "os.h" +#include "utils.h" +#include "sol/printer.h" + +static int derive_public_key(const uint8_t *buffer, + uint16_t buffer_length, + uint8_t public_key[PUBKEY_LENGTH], + char public_key_str[BASE58_PUBKEY_LENGTH]) { + uint32_t derivation_path[MAX_BIP32_PATH_LENGTH]; + uint32_t path_length; + int ret; + + ret = read_derivation_path(buffer, buffer_length, derivation_path, &path_length) != 0; + if (ret != 0) { + return ret; + } + + get_public_key(public_key, derivation_path, path_length); + + return encode_base58(public_key, PUBKEY_LENGTH, public_key_str, BASE58_PUBKEY_LENGTH); +} + +int handle_check_address(check_address_parameters_t *params) { + PRINTF("Inside Solana handle_check_address\n"); + PRINTF("Params on the address %d\n", (unsigned int) params); + + if (params->coin_configuration != NULL || params->coin_configuration_length != 0) { + PRINTF("No coin_configuration expected\n"); + return 0; + } + + if (params->address_parameters == NULL) { + PRINTF("derivation path expected\n"); + return 0; + } + + if (params->address_to_check == NULL) { + PRINTF("Address to check expected\n"); + return 0; + } + PRINTF("Address to check %s\n", params->address_to_check); + + if (params->extra_id_to_check == NULL) { + PRINTF("extra_id_to_check expected\n"); + return 0; + } else if (params->extra_id_to_check[0] != '\0') { + PRINTF("extra_id_to_check expected empty, not '%s'\n", params->extra_id_to_check); + return 0; + } + + uint8_t public_key[PUBKEY_LENGTH]; + char public_key_str[BASE58_PUBKEY_LENGTH]; + if (derive_public_key(params->address_parameters, + params->address_parameters_length, + public_key, + public_key_str) != 0) { + PRINTF("Failed to derive public key\n"); + return 0; + } + // Only public_key_str is usefull in this context + UNUSED(public_key); + + if (strcmp(params->address_to_check, public_key_str) != 0) { + PRINTF("Adress %s != %s\n", params->address_to_check, public_key_str); + return 0; + } + + PRINTF("Addresses match\n"); + return 1; +} diff --git a/src/swap/handle_check_address.h b/src/swap/handle_check_address.h new file mode 100644 index 00000000..264a00a8 --- /dev/null +++ b/src/swap/handle_check_address.h @@ -0,0 +1,5 @@ +#pragma once + +#include "swap_lib_calls.h" + +int handle_check_address(check_address_parameters_t* params); diff --git a/src/swap/handle_get_printable_amount.c b/src/swap/handle_get_printable_amount.c new file mode 100644 index 00000000..7cf1a9ff --- /dev/null +++ b/src/swap/handle_get_printable_amount.c @@ -0,0 +1,33 @@ +#include "handle_get_printable_amount.h" +#include "swap_lib_calls.h" +#include "utils.h" +#include "sol/printer.h" + +/* return 0 on error, 1 otherwise */ +int handle_get_printable_amount(get_printable_amount_parameters_t* params) { + PRINTF("Inside Solana handle_get_printable_amount\n"); + MEMCLEAR(params->printable_amount); + + // Fees are displayed normally + // params->is_fee + + if (params->coin_configuration != NULL || params->coin_configuration_length != 0) { + PRINTF("No coin_configuration expected\n"); + return 0; + } + + uint64_t amount; + if (!swap_str_to_u64(params->amount, params->amount_length, &amount)) { + PRINTF("Amount is too big"); + return 0; + } + + if (print_amount(amount, params->printable_amount, sizeof(params->printable_amount)) != 0) { + PRINTF("print_amount failed"); + return 0; + } + + PRINTF("Amount %s\n", params->printable_amount); + + return 1; +} diff --git a/src/swap/handle_get_printable_amount.h b/src/swap/handle_get_printable_amount.h new file mode 100644 index 00000000..36481fdd --- /dev/null +++ b/src/swap/handle_get_printable_amount.h @@ -0,0 +1,5 @@ +#pragma once + +#include "swap_lib_calls.h" + +int handle_get_printable_amount(get_printable_amount_parameters_t* params); diff --git a/src/swap/handle_swap_sign_transaction.c b/src/swap/handle_swap_sign_transaction.c new file mode 100644 index 00000000..01f48592 --- /dev/null +++ b/src/swap/handle_swap_sign_transaction.c @@ -0,0 +1,109 @@ +#include "handle_swap_sign_transaction.h" +#include "utils.h" +#include "swap_lib_calls.h" +#include "sol/printer.h" + +typedef struct swap_validated_s { + bool initialized; + uint64_t amount; + char recipient[BASE58_PUBKEY_LENGTH]; + // uint64_t fees; + // char extra_id[20]; +} swap_validated_t; + +static swap_validated_t G_swap_validated; + +// Save the data validated during the Exchange app flow +bool copy_transaction_parameters(create_transaction_parameters_t *params) { + // Ensure no subcoin configuration + if (params->coin_configuration != NULL || params->coin_configuration_length != 0) { + PRINTF("No coin_configuration expected\n"); + return false; + } + + // Ensure no extraid + if (params->destination_address_extra_id == NULL) { + PRINTF("destination_address_extra_id expected\n"); + return false; + } else if (params->destination_address_extra_id[0] != '\0') { + PRINTF("destination_address_extra_id expected empty, not '%s'\n", + params->destination_address_extra_id); + return false; + } + + // first copy parameters to stack, and then to global data. + // We need this "trick" as the input data position can overlap with app globals + swap_validated_t swap_validated; + memset(&swap_validated, 0, sizeof(swap_validated)); + + // Save recipient + strncpy(swap_validated.recipient, + params->destination_address, + sizeof(swap_validated.recipient)); + if (swap_validated.recipient[sizeof(swap_validated.recipient) - 1] != '\0') { + PRINTF("Address copy error\n"); + return false; + } + + // Save amount + if (!swap_str_to_u64(params->amount, params->amount_length, &swap_validated.amount)) { + return false; + } + + // TODO: what to do with fees ??? + // if (!swap_str_to_u64(params->fee_amount, params->fee_amount_length, &swap_validated.fee)) { + // return false; + // } + + swap_validated.initialized = true; + + // Commit from stack to global data, params becomes tainted but we won't access it anymore + memcpy(&G_swap_validated, &swap_validated, sizeof(swap_validated)); + return true; +} + +// Check that the amount in parameter is the same as the previously saved amount +bool check_swap_amount(const char *amount) { + if (!G_swap_validated.initialized) { + return false; + } + + char validated_amount[MAX_PRINTABLE_AMOUNT_SIZE]; + if (print_amount(G_swap_validated.amount, validated_amount, sizeof(validated_amount)) != 0) { + PRINTF("Conversion failed\n"); + return false; + } + if (strcmp(amount, validated_amount) == 0) { + return true; + } else { + PRINTF("Amount requested in this transaction = %s\n", amount); + PRINTF("Amount validated in swap = %s\n", validated_amount); + return false; + } +} + +// Check that the recipient in parameter is the same as the previously saved recipient +bool check_swap_recipient(const char *recipient) { + if (!G_swap_validated.initialized) { + return false; + } + + if (strcmp(G_swap_validated.recipient, recipient) == 0) { + return true; + } else { + PRINTF("Recipient requested in this transaction = %s\n", recipient); + PRINTF("Recipient validated in swap = %s\n", G_swap_validated.recipient); + return false; + } +} + +// What to do with fees ? +bool check_swap_fee_payer(const char *fee_payer) { + if (!G_swap_validated.initialized) { + return false; + } + + // TODO ? + PRINTF("Fee payer requested in this transaction = %s\n", fee_payer); + return true; +} diff --git a/src/swap/handle_swap_sign_transaction.h b/src/swap/handle_swap_sign_transaction.h new file mode 100644 index 00000000..f4bbdb48 --- /dev/null +++ b/src/swap/handle_swap_sign_transaction.h @@ -0,0 +1,9 @@ +#pragma once + +#include "swap_lib_calls.h" + +bool copy_transaction_parameters(create_transaction_parameters_t *sign_transaction_params); + +bool check_swap_amount(const char *amount); +bool check_swap_recipient(const char *recipient); +bool check_swap_fee_payer(const char *fee_payer); diff --git a/src/swap/swap_lib_calls.c b/src/swap/swap_lib_calls.c new file mode 100644 index 00000000..009a9bc5 --- /dev/null +++ b/src/swap/swap_lib_calls.c @@ -0,0 +1,16 @@ +#include + +#include "swap_lib_calls.h" + +bool swap_str_to_u64(const uint8_t* src, size_t length, uint64_t* result) { + if (length > sizeof(uint64_t)) { + return false; + } + uint64_t value = 0; + for (size_t i = 0; i < length; i++) { + value <<= 8; + value |= src[i]; + } + *result = value; + return true; +} diff --git a/src/swap/swap_lib_calls.h b/src/swap/swap_lib_calls.h new file mode 100644 index 00000000..8f61f8b3 --- /dev/null +++ b/src/swap/swap_lib_calls.h @@ -0,0 +1,74 @@ +#pragma once + +#include +#include +#include + +#define RUN_APPLICATION 1 + +#define SIGN_TRANSACTION 2 + +#define CHECK_ADDRESS 3 + +#define GET_PRINTABLE_AMOUNT 4 + +/* + * Amounts are stored as bytes, with a max size of 16 (see protobuf + * specifications). Max 16B integer is 340282366920938463463374607431768211455 + * in decimal, which is a 32-long char string. + * The printable amount also contains spaces, the ticker symbol (with variable + * size, up to 12 in Ethereum for instance) and a terminating null byte, so 50 + * bytes total should be a fair maximum. + */ +#define MAX_PRINTABLE_AMOUNT_SIZE 50 + +// structure that should be send to specific coin application to get address +typedef struct check_address_parameters_s { + // IN + unsigned char *coin_configuration; + unsigned char coin_configuration_length; + // serialized path, segwit, version prefix, hash used, dictionary etc. + // fields and serialization format depends on spesific coin app + unsigned char *address_parameters; + unsigned char address_parameters_length; + char *address_to_check; + char *extra_id_to_check; + // OUT + int result; +} check_address_parameters_t; + +// structure that should be send to specific coin application to get printable amount +typedef struct get_printable_amount_parameters_s { + // IN + unsigned char *coin_configuration; + unsigned char coin_configuration_length; + unsigned char *amount; + unsigned char amount_length; + bool is_fee; + // OUT + char printable_amount[MAX_PRINTABLE_AMOUNT_SIZE]; +} get_printable_amount_parameters_t; + +typedef struct create_transaction_parameters_s { + unsigned char *coin_configuration; + unsigned char coin_configuration_length; + unsigned char *amount; + unsigned char amount_length; + unsigned char *fee_amount; + unsigned char fee_amount_length; + char *destination_address; + char *destination_address_extra_id; +} create_transaction_parameters_t; + +bool swap_str_to_u64(const uint8_t *src, size_t length, uint64_t *result); + +typedef struct libargs_s { + unsigned int id; + unsigned int command; + unsigned int unused; + union { + check_address_parameters_t *check_address; + create_transaction_parameters_t *create_transaction; + get_printable_amount_parameters_t *get_printable_amount; + }; +} libargs_t; From 7c49b4be6dc0acaa45a3d1dab2c3c2d65cb02614 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 28 Sep 2022 17:53:41 +0200 Subject: [PATCH 02/10] Fix mispellings and improve misspelling CI --- .github/workflows/lint-workflow.yml | 2 +- libsol/message_test.c | 2 +- src/swap/handle_check_address.c | 4 ++-- src/swap/swap_lib_calls.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/lint-workflow.yml b/.github/workflows/lint-workflow.yml index 975ed6f3..fbc07d41 100644 --- a/.github/workflows/lint-workflow.yml +++ b/.github/workflows/lint-workflow.yml @@ -46,4 +46,4 @@ jobs: # Use Config file when the github action supports it builtin: clear,rare check_filenames: true - skip: ./libsol,./tests + skip: ./libsol/printer_test.c,./tests/Cargo.lock diff --git a/libsol/message_test.c b/libsol/message_test.c index 25a0b65d..1d78d9b8 100644 --- a/libsol/message_test.c +++ b/libsol/message_test.c @@ -6,7 +6,7 @@ #include #include -// Disable clang format for this file to keep clear buffer formating +// Disable clang format for this file to keep clear buffer formatting /* clang-format off */ void test_process_message_body_ok() { diff --git a/src/swap/handle_check_address.c b/src/swap/handle_check_address.c index 0f0a70c4..35f84d31 100644 --- a/src/swap/handle_check_address.c +++ b/src/swap/handle_check_address.c @@ -60,11 +60,11 @@ int handle_check_address(check_address_parameters_t *params) { PRINTF("Failed to derive public key\n"); return 0; } - // Only public_key_str is usefull in this context + // Only public_key_str is useful in this context UNUSED(public_key); if (strcmp(params->address_to_check, public_key_str) != 0) { - PRINTF("Adress %s != %s\n", params->address_to_check, public_key_str); + PRINTF("Address %s != %s\n", params->address_to_check, public_key_str); return 0; } diff --git a/src/swap/swap_lib_calls.h b/src/swap/swap_lib_calls.h index 8f61f8b3..27a4436a 100644 --- a/src/swap/swap_lib_calls.h +++ b/src/swap/swap_lib_calls.h @@ -28,7 +28,7 @@ typedef struct check_address_parameters_s { unsigned char *coin_configuration; unsigned char coin_configuration_length; // serialized path, segwit, version prefix, hash used, dictionary etc. - // fields and serialization format depends on spesific coin app + // fields and serialization format depends on specific coin app unsigned char *address_parameters; unsigned char address_parameters_length; char *address_to_check; From bbb514a711461f3e2c7e64a386f241336106b49d Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 12 Oct 2022 13:51:50 +0200 Subject: [PATCH 03/10] Review from Askibin --- src/globals.h | 2 +- src/main.c | 7 +++---- src/signMessage.c | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/globals.h b/src/globals.h index 6bb92771..f4ac8b7e 100644 --- a/src/globals.h +++ b/src/globals.h @@ -52,7 +52,7 @@ typedef enum InstructionCode { InsSignOffchainMessage = 0x07 } InstructionCode; -extern bool G_called_from_swap; +extern volatile bool G_called_from_swap; // display stepped screens extern unsigned int ux_step; diff --git a/src/main.c b/src/main.c index 5d7d1e41..e10d779d 100644 --- a/src/main.c +++ b/src/main.c @@ -30,7 +30,7 @@ ApduCommand G_command; unsigned char G_io_seproxyhal_spi_buffer[IO_SEPROXYHAL_BUFFER_SIZE_B]; -bool G_called_from_swap; +volatile bool G_called_from_swap; static void reset_main_globals(void) { MEMCLEAR(G_command); @@ -126,6 +126,7 @@ void app_main(void) { THROW(ApduReplySdkExceptionIoReset); } CATCH_OTHER(e) { + G_called_from_swap = false; switch (e & 0xF000) { case 0x6000: sw = e; @@ -148,8 +149,6 @@ void app_main(void) { tx += 2; } FINALLY { - // Restrict silent swap mode to the first APDU received - G_called_from_swap = false; } } END_TRY; @@ -354,7 +353,7 @@ static void library_main_helper(libargs_t *args) { } static void library_main(libargs_t *args) { - bool end = false; + volatile bool end = false; /* This loop ensures that library_main_helper and os_lib_end are called * within a try context, even if an exception is thrown */ while (1) { diff --git a/src/signMessage.c b/src/signMessage.c index 693edea3..282a636f 100644 --- a/src/signMessage.c +++ b/src/signMessage.c @@ -230,7 +230,7 @@ void handle_sign_message_ui(volatile unsigned int *flags) { os_sched_exit(0); } else { PRINTF("Refused blind signing incorrect Swap transaction\n"); - sendResponse(0, false); + THROW(ApduReplySolanaSummaryFinalizeFailed); } } else { MEMCLEAR(flow_steps); From a12689e195fe89f184ac2f801bc01caa6ff7f84f Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Thu, 13 Oct 2022 10:07:21 +0200 Subject: [PATCH 04/10] Update changelog --- doc/api.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/api.md b/doc/api.md index 98a1392d..66acb019 100644 --- a/doc/api.md +++ b/doc/api.md @@ -3,6 +3,7 @@ ## 1.3.0 - Add SIGN SOLANA OFF-CHAIN MESSAGE +- Add compatibility with the Exchange Application to SWAP, FUND, or SELL SOL tokens ## About From 0918d87250d0dcfc18c712133d9e01ef0c0e135a Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Thu, 13 Oct 2022 16:38:35 +0200 Subject: [PATCH 05/10] Review --- src/main.c | 2 ++ src/signMessage.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main.c b/src/main.c index e10d779d..3729e88f 100644 --- a/src/main.c +++ b/src/main.c @@ -48,8 +48,10 @@ void handleApdu(volatile unsigned int *flags, volatile unsigned int *tx, int rx) const int ret = apdu_handle_message(G_io_apdu_buffer, rx, &G_command); if (ret != 0) { + MEMCLEAR(G_command); THROW(ret); } + if (G_command.state == ApduStatePayloadInProgress) { THROW(ApduReplySuccess); } diff --git a/src/signMessage.c b/src/signMessage.c index 282a636f..03ba82db 100644 --- a/src/signMessage.c +++ b/src/signMessage.c @@ -143,8 +143,8 @@ void handle_sign_message_parse_message(volatile unsigned int *tx) { if ((G_command.non_confirm || G_called_from_swap) && !(G_command.non_confirm && G_called_from_swap)) { - // Blind sign requested NOT in swap context - // Or no blind sign requested while in swap context + // User validation bypass requested NOT in swap context + // Or user validation requested while in swap context THROW(ApduReplySdkNotSupported); } From ae6737589e6fbf94f999beb225dec1a8d00d8f7f Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 12 Oct 2022 14:10:04 +0200 Subject: [PATCH 06/10] Temporary version --- Makefile | 4 ++-- doc/api.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 0656ba2a..53da6070 100644 --- a/Makefile +++ b/Makefile @@ -39,9 +39,9 @@ APP_LOAD_PARAMS += $(COMMON_LOAD_PARAMS) APPNAME = "Solana" APPVERSION_M = 1 -APPVERSION_N = 3 +APPVERSION_N = 4 APPVERSION_P = 0 -APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)" +APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)-swap" ifeq ($(TARGET_NAME),TARGET_NANOS) ICONNAME=icons/nanos_app_solana.gif diff --git a/doc/api.md b/doc/api.md index 66acb019..1ac45771 100644 --- a/doc/api.md +++ b/doc/api.md @@ -1,6 +1,6 @@ # Solana application : Common Technical Specifications -## 1.3.0 +## 1.4.0 - Add SIGN SOLANA OFF-CHAIN MESSAGE - Add compatibility with the Exchange Application to SWAP, FUND, or SELL SOL tokens From 6c9f5c26f4bd4323af465638ae63ba0b52bebdfa Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 12 Oct 2022 18:52:17 +0200 Subject: [PATCH 07/10] Use sizeof instead of strlen --- libsol/parser.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libsol/parser.c b/libsol/parser.c index 1de6da49..f0e71dbe 100644 --- a/libsol/parser.c +++ b/libsol/parser.c @@ -5,6 +5,8 @@ "\xff" \ "solana offchain" +#define OFFCHAIN_MESSAGE_SIGNING_DOMAIN_LEN (sizeof(OFFCHAIN_MESSAGE_SIGNING_DOMAIN) - 1) + static int check_buffer_length(Parser* parser, size_t num) { return parser->buffer_length < num ? 1 : 0; } @@ -130,13 +132,14 @@ int parse_message_header(Parser* parser, MessageHeader* header) { } int parse_offchain_message_header(Parser* parser, OffchainMessageHeader* header) { - const size_t domain_len = strlen(OFFCHAIN_MESSAGE_SIGNING_DOMAIN); - BAIL_IF(check_buffer_length(parser, domain_len)); + BAIL_IF(check_buffer_length(parser, OFFCHAIN_MESSAGE_SIGNING_DOMAIN_LEN)); int res; - if ((res = memcmp(OFFCHAIN_MESSAGE_SIGNING_DOMAIN, parser->buffer, domain_len)) != 0) { + if ((res = memcmp(OFFCHAIN_MESSAGE_SIGNING_DOMAIN, + parser->buffer, + OFFCHAIN_MESSAGE_SIGNING_DOMAIN_LEN)) != 0) { return res; } - advance(parser, domain_len); + advance(parser, OFFCHAIN_MESSAGE_SIGNING_DOMAIN_LEN); BAIL_IF(parse_u8(parser, &header->version)); BAIL_IF(parse_u8(parser, &header->format)); From 9ad620198f1b7c6b2dac243cf7b6aa044d753b9b Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 12 Oct 2022 18:55:04 +0200 Subject: [PATCH 08/10] Factorize signing code --- src/signMessage.c | 30 -------------- src/signOffchainMessage.c | 30 -------------- src/utils.c | 86 +++++++++++++++++++++++++++------------ src/utils.h | 8 +--- 4 files changed, 60 insertions(+), 94 deletions(-) diff --git a/src/signMessage.c b/src/signMessage.c index 03ba82db..b6d725d5 100644 --- a/src/signMessage.c +++ b/src/signMessage.c @@ -14,36 +14,6 @@ #include "handle_swap_sign_transaction.h" -static uint8_t set_result_sign_message() { - uint8_t signature[SIGNATURE_LENGTH]; - cx_ecfp_private_key_t privateKey; - BEGIN_TRY { - TRY { - get_private_key_with_seed(&privateKey, - G_command.derivation_path, - G_command.derivation_path_length); - cx_eddsa_sign(&privateKey, - CX_LAST, - CX_SHA512, - G_command.message, - G_command.message_length, - NULL, - 0, - signature, - SIGNATURE_LENGTH, - NULL); - memcpy(G_io_apdu_buffer, signature, SIGNATURE_LENGTH); - } - CATCH_OTHER(e) { - THROW(e); - } - FINALLY { - MEMCLEAR(privateKey); - } - } - END_TRY; - return SIGNATURE_LENGTH; -} static void send_result_sign_message(void) { sendResponse(set_result_sign_message(), true); diff --git a/src/signOffchainMessage.c b/src/signOffchainMessage.c index f1dabf62..d11bf57a 100644 --- a/src/signOffchainMessage.c +++ b/src/signOffchainMessage.c @@ -76,36 +76,6 @@ static bool is_data_ascii(const uint8_t *data, size_t length) { return true; } -static uint8_t set_result_sign_message() { - uint8_t signature[SIGNATURE_LENGTH]; - cx_ecfp_private_key_t privateKey; - BEGIN_TRY { - TRY { - get_private_key_with_seed(&privateKey, - G_command.derivation_path, - G_command.derivation_path_length); - cx_eddsa_sign(&privateKey, - CX_LAST, - CX_SHA512, - G_command.message, - G_command.message_length, - NULL, - 0, - signature, - SIGNATURE_LENGTH, - NULL); - memcpy(G_io_apdu_buffer, signature, SIGNATURE_LENGTH); - } - CATCH_OTHER(e) { - THROW(e); - } - FINALLY { - MEMCLEAR(privateKey); - } - } - END_TRY; - return SIGNATURE_LENGTH; -} ////////////////////////////////////////////////////////////////////// diff --git a/src/utils.c b/src/utils.c index 258a1fcc..405cb45e 100644 --- a/src/utils.c +++ b/src/utils.c @@ -5,37 +5,12 @@ #include "utils.h" #include "menu.h" -void get_public_key(uint8_t *publicKeyArray, const uint32_t *derivationPath, size_t pathLength) { - cx_ecfp_private_key_t privateKey; - cx_ecfp_public_key_t publicKey; - - get_private_key(&privateKey, derivationPath, pathLength); - BEGIN_TRY { - TRY { - cx_ecfp_generate_pair(CX_CURVE_Ed25519, &publicKey, &privateKey, 1); - } - CATCH_OTHER(e) { - THROW(e); - } - FINALLY { - MEMCLEAR(privateKey); - } - } - END_TRY; - - for (int i = 0; i < PUBKEY_LENGTH; i++) { - publicKeyArray[i] = publicKey.W[PUBKEY_LENGTH + PRIVATEKEY_LENGTH - i]; - } - if ((publicKey.W[PUBKEY_LENGTH] & 1) != 0) { - publicKeyArray[PUBKEY_LENGTH - 1] |= 0x80; - } -} uint32_t readUint32BE(uint8_t *buffer) { return ((buffer[0] << 24) | (buffer[1] << 16) | (buffer[2] << 8) | (buffer[3])); } -void get_private_key(cx_ecfp_private_key_t *privateKey, +static void get_private_key(cx_ecfp_private_key_t *privateKey, const uint32_t *derivationPath, size_t pathLength) { uint8_t privateKeyData[PRIVATEKEY_LENGTH]; @@ -64,7 +39,7 @@ void get_private_key(cx_ecfp_private_key_t *privateKey, END_TRY; } -void get_private_key_with_seed(cx_ecfp_private_key_t *privateKey, +static void get_private_key_with_seed(cx_ecfp_private_key_t *privateKey, const uint32_t *derivationPath, uint8_t pathLength) { uint8_t privateKeyData[PRIVATEKEY_LENGTH]; @@ -93,6 +68,32 @@ void get_private_key_with_seed(cx_ecfp_private_key_t *privateKey, END_TRY; } +void get_public_key(uint8_t *publicKeyArray, const uint32_t *derivationPath, size_t pathLength) { + cx_ecfp_private_key_t privateKey; + cx_ecfp_public_key_t publicKey; + + get_private_key(&privateKey, derivationPath, pathLength); + BEGIN_TRY { + TRY { + cx_ecfp_generate_pair(CX_CURVE_Ed25519, &publicKey, &privateKey, 1); + } + CATCH_OTHER(e) { + THROW(e); + } + FINALLY { + MEMCLEAR(privateKey); + } + } + END_TRY; + + for (int i = 0; i < PUBKEY_LENGTH; i++) { + publicKeyArray[i] = publicKey.W[PUBKEY_LENGTH + PRIVATEKEY_LENGTH - i]; + } + if ((publicKey.W[PUBKEY_LENGTH] & 1) != 0) { + publicKeyArray[PUBKEY_LENGTH - 1] |= 0x80; + } +} + int read_derivation_path(const uint8_t *data_buffer, size_t data_size, uint32_t *derivation_path, @@ -132,6 +133,37 @@ void sendResponse(uint8_t tx, bool approve) { ui_idle(); } +uint8_t set_result_sign_message(void) { + uint8_t signature[SIGNATURE_LENGTH]; + cx_ecfp_private_key_t privateKey; + BEGIN_TRY { + TRY { + get_private_key_with_seed(&privateKey, + G_command.derivation_path, + G_command.derivation_path_length); + cx_eddsa_sign(&privateKey, + CX_LAST, + CX_SHA512, + G_command.message, + G_command.message_length, + NULL, + 0, + signature, + SIGNATURE_LENGTH, + NULL); + memcpy(G_io_apdu_buffer, signature, SIGNATURE_LENGTH); + } + CATCH_OTHER(e) { + THROW(e); + } + FINALLY { + MEMCLEAR(privateKey); + } + } + END_TRY; + return SIGNATURE_LENGTH; +} + unsigned int ui_prepro(const bagl_element_t *element) { unsigned int display = 1; if (element->component.userid > 0) { diff --git a/src/utils.h b/src/utils.h index af8f2889..6692bb32 100644 --- a/src/utils.h +++ b/src/utils.h @@ -25,13 +25,7 @@ void get_public_key(uint8_t *publicKeyArray, const uint32_t *derivationPath, siz uint32_t readUint32BE(uint8_t *buffer); -void get_private_key(cx_ecfp_private_key_t *privateKey, - const uint32_t *derivationPath, - size_t pathLength); - -void get_private_key_with_seed(cx_ecfp_private_key_t *privateKey, - const uint32_t *derivationPath, - uint8_t pathLength); +uint8_t set_result_sign_message(void); /** * Deserialize derivation path from raw bytes. From 502794a3820865924a33fd0bcb7ba17410eb3a66 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 12 Oct 2022 18:55:26 +0200 Subject: [PATCH 09/10] Align max path length with sdk value --- src/utils.c | 4 ++-- src/utils.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils.c b/src/utils.c index 405cb45e..6f0fa0ce 100644 --- a/src/utils.c +++ b/src/utils.c @@ -41,7 +41,7 @@ static void get_private_key(cx_ecfp_private_key_t *privateKey, static void get_private_key_with_seed(cx_ecfp_private_key_t *privateKey, const uint32_t *derivationPath, - uint8_t pathLength) { + size_t pathLength) { uint8_t privateKeyData[PRIVATEKEY_LENGTH]; BEGIN_TRY { TRY { @@ -97,7 +97,7 @@ void get_public_key(uint8_t *publicKeyArray, const uint32_t *derivationPath, siz int read_derivation_path(const uint8_t *data_buffer, size_t data_size, uint32_t *derivation_path, - uint32_t *derivation_path_length) { + size_t *derivation_path_length) { if (!data_buffer || !derivation_path || !derivation_path_length) { return ApduReplySdkInvalidParameter; } diff --git a/src/utils.h b/src/utils.h index 6692bb32..d6c26748 100644 --- a/src/utils.h +++ b/src/utils.h @@ -45,7 +45,7 @@ uint8_t set_result_sign_message(void); int read_derivation_path(const uint8_t *data_buffer, size_t data_size, uint32_t *derivation_path, - uint32_t *derivation_path_length); + size_t *derivation_path_length); void sendResponse(uint8_t tx, bool approve); From 44eb43e144cdc70a2f2defd7c96a7e4a8ef9e942 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Wed, 12 Oct 2022 18:55:51 +0200 Subject: [PATCH 10/10] clang format --- src/signMessage.c | 1 - src/signOffchainMessage.c | 1 - src/utils.c | 9 ++++----- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/signMessage.c b/src/signMessage.c index b6d725d5..3951c5e3 100644 --- a/src/signMessage.c +++ b/src/signMessage.c @@ -14,7 +14,6 @@ #include "handle_swap_sign_transaction.h" - static void send_result_sign_message(void) { sendResponse(set_result_sign_message(), true); } diff --git a/src/signOffchainMessage.c b/src/signOffchainMessage.c index d11bf57a..d219dc2a 100644 --- a/src/signOffchainMessage.c +++ b/src/signOffchainMessage.c @@ -76,7 +76,6 @@ static bool is_data_ascii(const uint8_t *data, size_t length) { return true; } - ////////////////////////////////////////////////////////////////////// UX_STEP_NOCB(ux_sign_msg_text_step, diff --git a/src/utils.c b/src/utils.c index 6f0fa0ce..4e5d96c1 100644 --- a/src/utils.c +++ b/src/utils.c @@ -5,14 +5,13 @@ #include "utils.h" #include "menu.h" - uint32_t readUint32BE(uint8_t *buffer) { return ((buffer[0] << 24) | (buffer[1] << 16) | (buffer[2] << 8) | (buffer[3])); } static void get_private_key(cx_ecfp_private_key_t *privateKey, - const uint32_t *derivationPath, - size_t pathLength) { + const uint32_t *derivationPath, + size_t pathLength) { uint8_t privateKeyData[PRIVATEKEY_LENGTH]; BEGIN_TRY { TRY { @@ -40,8 +39,8 @@ static void get_private_key(cx_ecfp_private_key_t *privateKey, } static void get_private_key_with_seed(cx_ecfp_private_key_t *privateKey, - const uint32_t *derivationPath, - size_t pathLength) { + const uint32_t *derivationPath, + size_t pathLength) { uint8_t privateKeyData[PRIVATEKEY_LENGTH]; BEGIN_TRY { TRY {