From aed5912fbfd9aa85bb29eb9d3e97f5211d6b3cd8 Mon Sep 17 00:00:00 2001
From: Andrew Kozlik <andrew.kozlik@gmail.com>
Date: Mon, 2 Oct 2023 19:18:11 +0200
Subject: [PATCH] feat(core): Distinguish Optiga errors from invalid PIN.

---
 core/embed/trezorhal/optiga.h |  8 +++++++-
 storage/storage.c             | 28 +++++++++++++++++-----------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/core/embed/trezorhal/optiga.h b/core/embed/trezorhal/optiga.h
index 3b170f7a5..a09d08307 100644
--- a/core/embed/trezorhal/optiga.h
+++ b/core/embed/trezorhal/optiga.h
@@ -30,9 +30,15 @@
 #define OPTIGA_DEVICE_ECC_KEY_INDEX 0
 #define OPTIGA_COMMAND_ERROR_OFFSET 0x100
 
-// Error code 7: Access conditions not satisfied
+// Error code 0x07: Access conditions not satisfied
 #define OPTIGA_ERR_ACCESS_COND_NOT_SAT (OPTIGA_COMMAND_ERROR_OFFSET + 0x07)
 
+// Error code 0x0E: Counter threshold limit exceeded
+#define OPTIGA_ERR_COUNTER_EXCEEDED (OPTIGA_COMMAND_ERROR_OFFSET + 0x0E)
+
+// Error code 0x2F: Authorization failure
+#define OPTIGA_ERR_AUTH_FAIL (OPTIGA_COMMAND_ERROR_OFFSET + 0x2F)
+
 // Size of secrets used in PIN processing, e.g. salted PIN, master secret etc.
 #define OPTIGA_PIN_SECRET_SIZE 32
 
diff --git a/storage/storage.c b/storage/storage.c
index 3f7050277..8befa1e1b 100644
--- a/storage/storage.c
+++ b/storage/storage.c
@@ -602,9 +602,9 @@ static secbool __wur derive_kek_set(const uint8_t *pin, size_t pin_len,
   uint8_t optiga_secret[OPTIGA_PIN_SECRET_SIZE] = {0};
   uint8_t stretched_pin[OPTIGA_PIN_SECRET_SIZE] = {0};
   stretch_pin_optiga(pin, pin_len, storage_salt, ext_salt, stretched_pin);
-  bool ret = optiga_pin_set(ui_progress, stretched_pin, optiga_secret);
+  int ret = optiga_pin_set(ui_progress, stretched_pin, optiga_secret);
   memzero(stretched_pin, sizeof(stretched_pin));
-  if (!ret) {
+  if (ret != OPTIGA_SUCCESS) {
     memzero(optiga_secret, sizeof(optiga_secret));
     return secfalse;
   }
@@ -625,10 +625,17 @@ static secbool __wur derive_kek_unlock(const uint8_t *pin, size_t pin_len,
   uint8_t optiga_secret[OPTIGA_PIN_SECRET_SIZE] = {0};
   uint8_t stretched_pin[OPTIGA_PIN_SECRET_SIZE] = {0};
   stretch_pin_optiga(pin, pin_len, storage_salt, ext_salt, stretched_pin);
-  bool ret = optiga_pin_verify(ui_progress, stretched_pin, optiga_secret);
+  int ret = optiga_pin_verify(ui_progress, stretched_pin, optiga_secret);
   memzero(stretched_pin, sizeof(stretched_pin));
-  if (!ret) {
+  if (ret != OPTIGA_SUCCESS) {
     memzero(optiga_secret, sizeof(optiga_secret));
+    if (ret == OPTIGA_ERR_COUNTER_EXCEEDED) {
+      // Unreachable code. Wipe should have already been triggered in unlock().
+      storage_wipe();
+      show_pin_too_many_screen();
+    }
+    ensure(ret == OPTIGA_ERR_AUTH_FAIL ? sectrue : secfalse,
+           "optiga_pin_verify failed");
     return secfalse;
   }
   derive_kek_optiga(optiga_secret, kek, keiv);
@@ -1196,15 +1203,13 @@ static secbool unlock(const uint8_t *pin, size_t pin_len,
   }
   uint8_t kek[SHA256_DIGEST_LENGTH] = {0};
   uint8_t keiv[SHA256_DIGEST_LENGTH] = {0};
-  if (sectrue != derive_kek_unlock(unlock_pin, unlock_pin_len,
-                                   (const uint8_t *)rand_salt, ext_salt, kek,
-                                   keiv)) {
-    return secfalse;
-  }
-  memzero(&legacy_pin, sizeof(legacy_pin));
 
   // Check whether the entered PIN is correct.
-  if (sectrue != decrypt_dek(kek, keiv)) {
+  if (sectrue != derive_kek_unlock(unlock_pin, unlock_pin_len,
+                                   (const uint8_t *)rand_salt, ext_salt, kek,
+                                   keiv) ||
+      sectrue != decrypt_dek(kek, keiv)) {
+    memzero(&legacy_pin, sizeof(legacy_pin));
     // Wipe storage if too many failures
     wait_random();
     if (ctr + 1 >= PIN_MAX_TRIES) {
@@ -1213,6 +1218,7 @@ static secbool unlock(const uint8_t *pin, size_t pin_len,
     }
     return secfalse;
   }
+  memzero(&legacy_pin, sizeof(legacy_pin));
   memzero(kek, sizeof(kek));
   memzero(keiv, sizeof(keiv));