keystore: fewer securechip calls when checking password

The sanity check to see if the seed has changed does not need a
securechip operation, it can use the retained seed hash instead, same
as `unlock_bip39()`. This reduces the number of securechip operations
needed to do a password check, which reduces the risk of running into
the Optiga throttling security mechanism.

Author: benma

src/keystore.c

@@ -422,6 +422,23 @@ keystore_error_t keystore_create_and_store_seed(
     return keystore_encrypt_and_store_seed(seed, host_entropy_size, password);
 }
 
+// Checks if the retained seed matches the passed seed.
+static bool _check_retained_seed(const uint8_t* seed, size_t seed_length)
+{
+    if (!_is_unlocked_device) {
+        return false;
+    }
+    uint8_t seed_hashed[32] = {0};
+    UTIL_CLEANUP_32(seed_hashed);
+    if (_hash_seed(seed, seed_length, seed_hashed) != KEYSTORE_OK) {
+        return false;
+    }
+    if (!MEMEQ(seed_hashed, _retained_seed_hash, sizeof(_retained_seed_hash))) {
+        return false;
+    }
+    return true;
+}
+
 keystore_error_t keystore_unlock(
     const char* password,
     uint8_t* remaining_attempts_out,
@@ -455,12 +472,7 @@ keystore_error_t keystore_unlock(
     if (result == KEYSTORE_OK) {
         if (_is_unlocked_device) {
             // Already unlocked. Fail if the seed changed under our feet (should never happen).
-            uint8_t current_seed[KEYSTORE_MAX_SEED_LENGTH] = {0};
-            size_t current_seed_len = 0;
-            if (!keystore_copy_seed(current_seed, &current_seed_len)) {
-                return KEYSTORE_ERR_DECRYPT;
-            }
-            if (seed_len != current_seed_len || !MEMEQ(current_seed, seed, current_seed_len)) {
+            if (!_check_retained_seed(seed, seed_len)) {
                 Abort("Seed has suddenly changed. This should never happen.");
             }
         } else {
@@ -491,15 +503,9 @@ bool keystore_unlock_bip39_check(const uint8_t* seed, size_t seed_length)
         return false;
     }
 
-    uint8_t seed_hashed[32] = {0};
-    UTIL_CLEANUP_32(seed_hashed);
-    if (_hash_seed(seed, seed_length, seed_hashed) != KEYSTORE_OK) {
+    if (!_check_retained_seed(seed, seed_length)) {
         return false;
     }
-    if (!MEMEQ(seed_hashed, _retained_seed_hash, sizeof(_retained_seed_hash))) {
-        return false;
-    }
-
     usb_processing_timeout_reset(LONG_TIMEOUT);
 
     return true;

src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs

@@ -149,7 +149,7 @@ mod tests {
             block_on(process(&mut mock_hal)),
             Ok(Response::Success(pb::Success {}))
         );
-        assert_eq!(bitbox02::securechip::fake_event_counter(), 7);
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 6);
 
         assert_eq!(
             mock_hal.ui.screens,

src/rust/bitbox02/src/keystore.rs

@@ -466,15 +466,18 @@ mod tests {
         assert!(encrypt_and_store_seed(&seed, "password").is_ok());
         lock();
 
+        // First call: unlock. The first one does a seed rentention (1 securechip event).
+        crate::securechip::fake_event_counter_reset();
+        assert!(unlock("password").is_ok());
+        assert_eq!(crate::securechip::fake_event_counter(), 6);
+
         // Loop to check that unlocking works while unlocked.
-        for _ in 0..3 {
-            // First call: unlock Further calls perform a password check. The first onedoes a seed
-            // rentention (1 securechip event). The password check does not do the rentention but a
-            // copy_seed() instead to check the seed, so they end up hacing the same number of
-            // events.crate::securechip::fake_event_counter_reset();
+        for _ in 0..2 {
+            // Further calls perform a password check.The password check does not do the retention
+            // so it ends up needing one secure chip operation less.
             crate::securechip::fake_event_counter_reset();
             assert!(unlock("password").is_ok());
-            assert_eq!(crate::securechip::fake_event_counter(), 6);
+            assert_eq!(crate::securechip::fake_event_counter(), 5);
         }
 
         // Also check that the retained seed was encrypted with the expected encryption key.