From 5d0fa7e2aa252f24a93d28c6c1b24f78515f3d20 Mon Sep 17 00:00:00 2001 From: italo sampaio Date: Mon, 14 Nov 2022 12:03:43 -0300 Subject: [PATCH] Some additional fixes addressed on code review --- ledger/src/ui/src/bolos_ux.c | 2 +- ledger/src/ui/src/onboard.c | 9 +++++---- ledger/src/ui/src/onboard.h | 2 -- ledger/src/ui/src/pin.c | 4 ++-- ledger/src/ui/src/pin.h | 4 ++-- ledger/src/ui/src/unlock.c | 2 +- 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/ledger/src/ui/src/bolos_ux.c b/ledger/src/ui/src/bolos_ux.c index 0c5ffab4..f0f9be71 100644 --- a/ledger/src/ui/src/bolos_ux.c +++ b/ledger/src/ui/src/bolos_ux.c @@ -534,7 +534,7 @@ void bolos_ux_main(void) { // PIN is invalidated so we must check it again. The pin value // used here is the same as in RSK_UNLOCK_CMD, so we also // don't have a prepended length byte - validate_pin(false); + unlock_with_pin(false); G_bolos_ux_context.exit_code = BOLOS_UX_OK; clear_pin(); break; diff --git a/ledger/src/ui/src/onboard.c b/ledger/src/ui/src/onboard.c index 6c7314a6..ca0369a5 100644 --- a/ledger/src/ui/src/onboard.c +++ b/ledger/src/ui/src/onboard.c @@ -32,7 +32,7 @@ #include "onboard.h" // Global onboarding flag -const unsigned char *N_onboarded_ui[1]; +const unsigned char N_onboarded_ui[1]; /* * Reset the given onboard context @@ -76,7 +76,8 @@ unsigned int onboard_device(onboard_t *onboard_ctx) { onboard_ctx->seed[i] ^= onboard_ctx->host_seed[i]; } // The seed is now in onboard_ctx->seed, generate the mnemonic - os_memset(onboard_ctx->words_buffer, 0, sizeof(onboard_ctx->words_buffer)); + explicit_bzero(onboard_ctx->words_buffer, + sizeof(onboard_ctx->words_buffer)); onboard_ctx->words_buffer_length = bolos_ux_mnemonic_from_data((unsigned char *)onboard_ctx->seed, sizeof(onboard_ctx->seed), @@ -106,7 +107,7 @@ unsigned int onboard_device(onboard_t *onboard_ctx) { os_global_pin_invalidate(); unsigned char output_index = CMDPOS; SET_APDU_AT(output_index++, 2); - SET_APDU_AT(output_index++, validate_pin(true)); + SET_APDU_AT(output_index++, unlock_with_pin(true)); // Turn the onboarding flag on to mark onboarding // has been done using the UI @@ -135,7 +136,7 @@ unsigned int set_host_seed(volatile unsigned int rx, onboard_t *onboard_ctx) { } unsigned char index = APDU_OP(); - if ((index >= 0) && ((size_t)index <= sizeof(onboard_ctx->host_seed))) { + if ((index >= 0) && ((size_t)index < sizeof(onboard_ctx->host_seed))) { onboard_ctx->host_seed[index] = APDU_AT(3); } diff --git a/ledger/src/ui/src/onboard.h b/ledger/src/ui/src/onboard.h index 049e1f24..e9e8255f 100644 --- a/ledger/src/ui/src/onboard.h +++ b/ledger/src/ui/src/onboard.h @@ -25,8 +25,6 @@ #ifndef __ONBOARD #define __ONBOARD -#include "os.h" -#include "os_io_seproxyhal.h" #include "pin.h" // 128 of words (215 => hashed to 64, or 128) + HMAC_LENGTH*2 = 256 diff --git a/ledger/src/ui/src/pin.c b/ledger/src/ui/src/pin.c index 72eecf0c..a19898b8 100644 --- a/ledger/src/ui/src/pin.c +++ b/ledger/src/ui/src/pin.c @@ -124,13 +124,13 @@ void clear_pin() { } /* - * Validates the pin currently saved to the internal pin buffer + * Uses the pin currently saved to the internal pin buffer to unlock the device * * @arg[in] prepended_length true if the internal buffer includes a prepended * length byte, false otherwise * @ret 1 if pin validated successfully, 0 otherwise */ -unsigned int validate_pin(bool prepended_length) { +unsigned int unlock_with_pin(bool prepended_length) { if (prepended_length) { return os_global_pin_check(GET_PIN(), GET_PIN_LENGTH()); } else { diff --git a/ledger/src/ui/src/pin.h b/ledger/src/ui/src/pin.h index 2ba801fb..5589954f 100644 --- a/ledger/src/ui/src/pin.h +++ b/ledger/src/ui/src/pin.h @@ -69,13 +69,13 @@ bool is_pin_valid(); void clear_pin(); /* - * Validates the pin currently saved to the internal pin buffer + * Uses the pin currently saved to the internal pin buffer to unlock the device * * @arg[in] prepended_length true if the internal buffer includes a prepended * length byte, false otherwise * @ret 1 if pin validated successfully, 0 otherwise */ -unsigned int validate_pin(bool prepended_length); +unsigned int unlock_with_pin(bool prepended_length); /* * Sets the pin currently saved to the internal pin buffer as the device's pin. diff --git a/ledger/src/ui/src/unlock.c b/ledger/src/ui/src/unlock.c index 8195d9d1..b041b786 100644 --- a/ledger/src/ui/src/unlock.c +++ b/ledger/src/ui/src/unlock.c @@ -36,6 +36,6 @@ */ unsigned int unlock() { unsigned char output_index = OP; - SET_APDU_AT(output_index++, validate_pin(false)); + SET_APDU_AT(output_index++, unlock_with_pin(false)); return output_index; }