From f393064ce7cef9e6e49ff4583b7322370e8aeed1 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Tue, 18 Jun 2024 20:20:27 +0200 Subject: [PATCH] feat(core): Improve PIN progress precision. --- core/embed/trezorhal/optiga.h | 8 +++-- core/embed/trezorhal/optiga/optiga.c | 14 ++++----- storage/storage.c | 44 +++++++++++++++++----------- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/core/embed/trezorhal/optiga.h b/core/embed/trezorhal/optiga.h index 2335313672..a58c223d7c 100644 --- a/core/embed/trezorhal/optiga.h +++ b/core/embed/trezorhal/optiga.h @@ -42,9 +42,11 @@ // Size of secrets used in PIN processing, e.g. salted PIN, master secret etc. #define OPTIGA_PIN_SECRET_SIZE 32 -// The number of milliseconds it takes to execute optiga_pin_set() or -// optiga_pin_verify(). -#define OPTIGA_PIN_DERIVE_MS 1200 +// The number of milliseconds it takes to execute optiga_pin_set(). +#define OPTIGA_PIN_SET_MS 1300 + +// The number of milliseconds it takes to execute optiga_pin_verify(). +#define OPTIGA_PIN_VERIFY_MS 900 typedef secbool (*OPTIGA_UI_PROGRESS)(uint32_t elapsed_ms); diff --git a/core/embed/trezorhal/optiga/optiga.c b/core/embed/trezorhal/optiga/optiga.c index 7fb5084aba..8191f9cf06 100644 --- a/core/embed/trezorhal/optiga/optiga.c +++ b/core/embed/trezorhal/optiga/optiga.c @@ -401,8 +401,6 @@ static int optiga_pin_stretch_common( hmac_sha256_Update(ctx, buffer, size); } - ui_progress(200); - // Combine intermediate result with OID_PIN_ECDH uint8_t encoded_point[BIT_STRING_HEADER_SIZE + 65] = {0x03, 0x42, 0x00}; if (!hash_to_curve_optiga(input, &encoded_point[BIT_STRING_HEADER_SIZE])) { @@ -417,7 +415,7 @@ static int optiga_pin_stretch_common( return res; } - ui_progress(200); + ui_progress(250); hmac_sha256_Update(ctx, buffer, size); memzero(buffer, sizeof(buffer)); @@ -517,7 +515,7 @@ int optiga_pin_set(OPTIGA_UI_PROGRESS ui_progress, goto end; } - ui_progress(200); + ui_progress(300); // Stretch the PIN more with stretching secrets from the Optiga. This step // ensures that if an attacker extracts the value of OID_STRETCHED_PIN or @@ -596,6 +594,8 @@ int optiga_pin_set(OPTIGA_UI_PROGRESS ui_progress, goto end; } + ui_progress(250); + // Authorise using OID_STRETCHED_PIN so that we can write to OID_PIN_HMAC and // OID_PIN_HMAC_CTR. res = optiga_set_auto_state(OPTIGA_OID_SESSION_CTX, OID_STRETCHED_PIN, digest, @@ -626,7 +626,7 @@ int optiga_pin_set(OPTIGA_UI_PROGRESS ui_progress, goto end; } - ui_progress(200); + ui_progress(250); // Stretch the PIN more with the counter-protected PIN secret. This method // ensures that if the user chooses a high-entropy PIN, then even if the @@ -781,6 +781,8 @@ int optiga_pin_verify(OPTIGA_UI_PROGRESS ui_progress, return error_code + OPTIGA_COMMAND_ERROR_OFFSET; } + ui_progress(200); + // Reset the counter which limits the use of OID_PIN_HMAC. res = optiga_set_data_object(OID_PIN_HMAC_CTR, false, COUNTER_RESET, sizeof(COUNTER_RESET)); @@ -789,8 +791,6 @@ int optiga_pin_verify(OPTIGA_UI_PROGRESS ui_progress, return res; } - ui_progress(200); - // Read the counter-protected PIN secret from OID_PIN_SECRET. uint8_t pin_secret[OPTIGA_PIN_SECRET_SIZE] = {0}; size_t size = 0; diff --git a/storage/storage.c b/storage/storage.c index 2e39218bef..4614e9f986 100644 --- a/storage/storage.c +++ b/storage/storage.c @@ -91,11 +91,18 @@ const uint32_t V0_PIN_EMPTY = 1; // The number of milliseconds required to execute PBKDF2. #define PIN_PBKDF2_MS 1280 -// The number of milliseconds required to derive the KEK and KEIV. +// The number of milliseconds required to set the PIN. #if USE_OPTIGA -#define PIN_DERIVE_MS (PIN_PBKDF2_MS + OPTIGA_PIN_DERIVE_MS) +#define PIN_SET_MS (PIN_PBKDF2_MS + OPTIGA_PIN_SET_MS) #else -#define PIN_DERIVE_MS PIN_PBKDF2_MS +#define PIN_SET_MS PIN_PBKDF2_MS +#endif + +// The number of milliseconds required to verify the PIN. +#if USE_OPTIGA +#define PIN_VERIFY_MS (PIN_PBKDF2_MS + OPTIGA_PIN_VERIFY_MS) +#else +#define PIN_VERIFY_MS PIN_PBKDF2_MS #endif // The length of the hashed hardware salt in bytes. @@ -451,6 +458,16 @@ static secbool is_not_wipe_code(const uint8_t *pin, size_t pin_len) { return sectrue; } +static void ui_total_init(uint32_t total_ms) { + ui_total = total_ms; + ui_rem = total_ms; +} + +static void ui_total_add(uint32_t added_ms) { + ui_total += added_ms; + ui_rem += added_ms; +} + static secbool ui_progress(uint32_t elapsed_ms) { ui_rem -= elapsed_ms; if (ui_callback && ui_message) { @@ -721,8 +738,7 @@ static void init_wiped_storage(void) { ensure(set_wipe_code(WIPE_CODE_EMPTY, WIPE_CODE_EMPTY_LEN), "set_wipe_code failed"); - ui_total = PIN_DERIVE_MS; - ui_rem = ui_total; + ui_total_init(PIN_SET_MS); ui_message = PROCESSING_MSG; ensure(set_pin(PIN_EMPTY, PIN_EMPTY_LEN, NULL), "init_pin failed"); } @@ -921,8 +937,7 @@ static secbool unlock(const uint8_t *pin, size_t pin_len, // In case of an upgrade from version 4 or earlier bump the total time of UI // progress to account for the set_pin() call in storage_upgrade_unlocked(). if (get_lock_version() <= 4) { - ui_total += PIN_DERIVE_MS; - ui_rem += PIN_DERIVE_MS; + ui_total_add(PIN_SET_MS); } // Now we can check for wipe code. @@ -945,8 +960,7 @@ static secbool unlock(const uint8_t *pin, size_t pin_len, // Sleep for 2^ctr - 1 seconds before checking the PIN. uint32_t wait = (1 << ctr) - 1; - ui_total += wait * 1000; - ui_rem += wait * 1000; + ui_total_add(wait * 1000); ui_progress(0); for (uint32_t i = 0; i < 10 * wait; i++) { if (sectrue == ui_progress(100)) { @@ -1015,8 +1029,7 @@ secbool storage_unlock(const uint8_t *pin, size_t pin_len, return secfalse; } - ui_total = PIN_DERIVE_MS; - ui_rem = ui_total; + ui_total_init(PIN_VERIFY_MS); if (pin_len == 0) { if (ui_message == NO_MSG) { ui_message = STARTING_MSG; @@ -1311,8 +1324,7 @@ secbool storage_change_pin(const uint8_t *oldpin, size_t oldpin_len, return secfalse; } - ui_total = 2 * PIN_DERIVE_MS; - ui_rem = ui_total; + ui_total_init(PIN_VERIFY_MS + PIN_SET_MS); ui_message = (oldpin_len != 0 && newpin_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG; @@ -1360,8 +1372,7 @@ secbool storage_change_wipe_code(const uint8_t *pin, size_t pin_len, return secfalse; } - ui_total = PIN_DERIVE_MS; - ui_rem = ui_total; + ui_total_init(PIN_VERIFY_MS); ui_message = (pin_len != 0 && wipe_code_len == 0) ? VERIFYING_PIN_MSG : PROCESSING_MSG; @@ -1537,8 +1548,7 @@ static secbool storage_upgrade(void) { } // Set EDEK_PVC_KEY and PIN_NOT_SET_KEY. - ui_total = PIN_DERIVE_MS; - ui_rem = ui_total; + ui_total_init(PIN_SET_MS); ui_message = PROCESSING_MSG; uint8_t pin[V0_MAX_PIN_LEN] = {0}; size_t pin_len = 0;