From 427992e1d32621e0d0fbca86908af4fc320b5897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Thu, 7 Mar 2024 10:22:27 +0100 Subject: [PATCH] Sign: force parsing and hashing --- src/apdu_sign.c | 105 ++++++++++++++++++------------------------------ src/apdu_sign.h | 3 -- src/main.c | 2 +- 3 files changed, 41 insertions(+), 69 deletions(-) diff --git a/src/apdu_sign.c b/src/apdu_sign.c index e0bcfa7a..1ce1d5db 100644 --- a/src/apdu_sign.c +++ b/src/apdu_sign.c @@ -19,6 +19,8 @@ #define B2B_BLOCKBYTES 128 +int perform_signature(bool const send_hash); + static inline void conditional_init_hash_state(blake2b_hash_state_t *const state) { check_null(state); if (!state->initialized) { @@ -75,12 +77,12 @@ static inline void clear_data(void) { } static bool sign_without_hash_ok(void) { - delayed_send(perform_signature(true, false)); + delayed_send(perform_signature(false)); return true; } static bool sign_with_hash_ok(void) { - delayed_send(perform_signature(true, true)); + delayed_send(perform_signature(true)); return true; } @@ -97,7 +99,7 @@ size_t baking_sign_complete(bool const send_hash, volatile uint32_t *flags) { case MAGIC_BYTE_PREENDORSEMENT: case MAGIC_BYTE_ENDORSEMENT: guard_baking_authorized(&G.parsed_baking_data, &global.path_with_curve); - result = perform_signature(true, send_hash); + result = perform_signature(send_hash); ux_empty_screen(); break; @@ -129,7 +131,7 @@ size_t baking_sign_complete(bool const send_hash, volatile uint32_t *flags) { if (bip32_path_with_curve_eq(&global.path_with_curve, &N_data.baking_key) && // ops->signing is generated from G.bip32_path and G.curve COMPARE(&G.maybe_ops.v.operation.source, &G.maybe_ops.v.signing) == 0) { - result = perform_signature(true, send_hash); + result = perform_signature(send_hash); } else { THROW(EXC_SECURITY); } @@ -164,10 +166,7 @@ static uint8_t get_magic_byte_or_throw(uint8_t const *const buff, size_t const b } } -static size_t handle_apdu(bool const enable_hashing, - bool const enable_parsing, - uint8_t const instruction, - volatile uint32_t *flags) { +size_t handle_apdu_sign(uint8_t instruction, volatile uint32_t *flags) { if (os_global_pin_is_validated() != BOLOS_UX_OK) { THROW(EXC_SECURITY); } @@ -202,35 +201,28 @@ static size_t handle_apdu(bool const enable_hashing, THROW(EXC_WRONG_PARAM); } - if (enable_parsing) { - if (G.packet_index != 1) { - PARSE_ERROR(); // Only parse a single packet when baking - } - - G.magic_byte = get_magic_byte_or_throw(buff, buff_size); - if (G.magic_byte == MAGIC_BYTE_UNSAFE_OP) { - // Parse the operation. It will be verified in `baking_sign_complete`. - G.maybe_ops.is_valid = parse_operations(&G.maybe_ops.v, - buff, - buff_size, - global.path_with_curve.derivation_type, - &global.path_with_curve.bip32_path); - } else { - // This should be a baking operation so parse it. - if (!parse_baking_data(&G.parsed_baking_data, buff, buff_size)) { - PARSE_ERROR(); - } - } + if (G.packet_index != 1) { + PARSE_ERROR(); // Only parse a single packet when baking } - if (enable_hashing) { - // Hash contents of *previous* message (which may be empty). - blake2b_incremental_hash(G.message_data, - sizeof(G.message_data), - &G.message_data_length, - &G.hash_state); + G.magic_byte = get_magic_byte_or_throw(buff, buff_size); + if (G.magic_byte == MAGIC_BYTE_UNSAFE_OP) { + // Parse the operation. It will be verified in `baking_sign_complete`. + G.maybe_ops.is_valid = parse_operations(&G.maybe_ops.v, + buff, + buff_size, + global.path_with_curve.derivation_type, + &global.path_with_curve.bip32_path); + } else if (!parse_baking_data(&G.parsed_baking_data, buff, buff_size)) { + PARSE_ERROR(); } + // Hash contents of *previous* message (which may be empty). + blake2b_incremental_hash(G.message_data, + sizeof(G.message_data), + &G.message_data_length, + &G.hash_state); + if (G.message_data_length + buff_size > sizeof(G.message_data)) { PARSE_ERROR(); } @@ -239,19 +231,17 @@ static size_t handle_apdu(bool const enable_hashing, G.message_data_length += buff_size; if (last) { - if (enable_hashing) { - // Hash contents of *this* message and then get the final hash value. - blake2b_incremental_hash(G.message_data, - sizeof(G.message_data), - &G.message_data_length, - &G.hash_state); - blake2b_finish_hash(G.final_hash, - sizeof(G.final_hash), - G.message_data, - sizeof(G.message_data), - &G.message_data_length, - &G.hash_state); - } + // Hash contents of *this* message and then get the final hash value. + blake2b_incremental_hash(G.message_data, + sizeof(G.message_data), + &G.message_data_length, + &G.hash_state); + blake2b_finish_hash(G.final_hash, + sizeof(G.final_hash), + G.message_data, + sizeof(G.message_data), + &G.message_data_length, + &G.hash_state); G.maybe_ops.is_valid = parse_operations_final(&G.parse_state, &G.maybe_ops.v); @@ -261,33 +251,18 @@ static size_t handle_apdu(bool const enable_hashing, } } -size_t handle_apdu_sign(uint8_t instruction, volatile uint32_t *flags) { - bool const enable_hashing = instruction != INS_SIGN_UNSAFE; - bool const enable_parsing = enable_hashing; - return handle_apdu(enable_hashing, enable_parsing, instruction, flags); -} - -size_t handle_apdu_sign_with_hash(uint8_t instruction, volatile uint32_t *flags) { - bool const enable_hashing = true; - bool const enable_parsing = true; - return handle_apdu(enable_hashing, enable_parsing, instruction, flags); -} - -int perform_signature(bool const on_hash, bool const send_hash) { +int perform_signature(bool const send_hash) { if (os_global_pin_is_validated() != BOLOS_UX_OK) { THROW(EXC_SECURITY); } write_high_water_mark(&G.parsed_baking_data); size_t tx = 0; - if (send_hash && on_hash) { + if (send_hash) { memcpy(&G_io_apdu_buffer[tx], G.final_hash, sizeof(G.final_hash)); tx += sizeof(G.final_hash); } - uint8_t const *const data = on_hash ? G.final_hash : G.message_data; - size_t const data_length = on_hash ? sizeof(G.final_hash) : G.message_data_length; - key_pair_t key_pair = {0}; size_t signature_size = 0; @@ -305,8 +280,8 @@ int perform_signature(bool const on_hash, bool const send_hash) { MAX_SIGNATURE_SIZE, global.path_with_curve.derivation_type, &key_pair, - data, - data_length); + G.final_hash, + sizeof(G.final_hash)); } CATCH_OTHER(e) { error = e; diff --git a/src/apdu_sign.h b/src/apdu_sign.h index 2ab88a4e..dc3490e3 100644 --- a/src/apdu_sign.h +++ b/src/apdu_sign.h @@ -3,8 +3,5 @@ #include "apdu.h" size_t handle_apdu_sign(uint8_t instruction, volatile uint32_t* flags); -size_t handle_apdu_sign_with_hash(uint8_t instruction, volatile uint32_t* flags); void prompt_register_delegate(ui_callback_t const ok_cb, ui_callback_t const cxl_cb); - -int perform_signature(bool const on_hash, bool const send_hash); diff --git a/src/main.c b/src/main.c index 78f8355d..7eb32715 100644 --- a/src/main.c +++ b/src/main.c @@ -22,7 +22,7 @@ __attribute__((noreturn)) void app_main(void) { global.handlers[APDU_INS(INS_PROMPT_PUBLIC_KEY)] = handle_apdu_get_public_key; global.handlers[APDU_INS(INS_SIGN)] = handle_apdu_sign; global.handlers[APDU_INS(INS_GIT)] = handle_apdu_git; - global.handlers[APDU_INS(INS_SIGN_WITH_HASH)] = handle_apdu_sign_with_hash; + global.handlers[APDU_INS(INS_SIGN_WITH_HASH)] = handle_apdu_sign; global.handlers[APDU_INS(INS_AUTHORIZE_BAKING)] = handle_apdu_get_public_key; global.handlers[APDU_INS(INS_RESET)] = handle_apdu_reset; global.handlers[APDU_INS(INS_QUERY_AUTH_KEY)] = handle_apdu_query_auth_key;