Skip to content

Commit

Permalink
feat(core): Improve PIN progress precision.
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewkozlik committed Jun 21, 2024
1 parent 509e291 commit f393064
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 27 deletions.
8 changes: 5 additions & 3 deletions core/embed/trezorhal/optiga.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 7 additions & 7 deletions core/embed/trezorhal/optiga/optiga.c
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand All @@ -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));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand All @@ -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;
Expand Down
44 changes: 27 additions & 17 deletions storage/storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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.
Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit f393064

Please sign in to comment.