reduce number of secure chip security events when creating/restoring

Before, `keystore_encrypt_and_store_seed()` (called when
creating/restoring a seed) would always be followed by
`keystore_unlock(<password>)` with the password the user just chose,
so unlock could never fail. The unlocking part costs many secure chip
operations (for stretching the password).

By making the first function already unlock the keystore, we can avoid
calling `keystore_unlock()`, reducing the number of secure chip events
by 5.

This effort is part of mitigating Optiga's throttling mechanism that
kicks in after 133 events - users can run into this by repeatedly
resetting/restoring).

Author: benma

src/keystore.c

@@ -345,6 +345,13 @@ keystore_error_t keystore_encrypt_and_store_seed(
         }
         return KEYSTORE_ERR_MEMORY;
     }
+
+    keystore_error_t retain_seed_result = _retain_seed(seed, seed_length);
+    if (retain_seed_result != KEYSTORE_OK) {
+        return retain_seed_result;
+    }
+    _is_unlocked_device = true;
+
     return KEYSTORE_OK;
 }
 

src/keystore.h

@@ -63,7 +63,7 @@ USE_RESULT bool keystore_copy_seed(uint8_t* seed_out, size_t* length_out);
 USE_RESULT bool keystore_copy_bip39_seed(uint8_t* bip32_seed_out);
 
 /**
- * Restores a seed.
+ * Restores a seed. This also unlocks the keystore with this seed.
  * @param[in] seed The seed that is to be restored.
  * @param[in] seed_length The length of the seed (max. 32 bytes).
  * @param[in] password The password with which we encrypt the seed.
@@ -75,6 +75,7 @@ keystore_encrypt_and_store_seed(const uint8_t* seed, size_t seed_length, const c
    Generates the seed, mixes it with host_entropy, and stores it encrypted with the
    password. The size of the host entropy determines the size of the seed. Can be either 16 or 32
    bytes, resulting in 12 or 24 BIP39 recovery words.
+   This also unlocks the keystore with the new seed.
    @param[in] host_entropy bytes of entropy to be mixed in.
    @param[in] host_entropy_size must be 16 or 32.
 */

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

@@ -17,7 +17,6 @@ use crate::pb;
 
 use pb::response::Response;
 
-use crate::general::abort;
 use crate::hal::Ui;
 use crate::workflow::{confirm, mnemonic, password, unlock};
 
@@ -84,9 +83,6 @@ pub async fn from_file(
     }
 
     bitbox02::memory::set_initialized().or(Err(Error::Memory))?;
-    if bitbox02::keystore::unlock(&password).is_err() {
-        abort("restore_from_file: unlock failed");
-    };
 
     // Ignore non-critical error.
     let _ = bitbox02::memory::set_device_name(&metadata.name);
@@ -160,10 +156,6 @@ pub async fn from_mnemonic(
     }
 
     bitbox02::memory::set_initialized().or(Err(Error::Memory))?;
-    // This should never fail.
-    if bitbox02::keystore::unlock(&password).is_err() {
-        abort("restore_from_mnemonic: unlock failed");
-    };
 
     unlock::unlock_bip39(hal).await;
     Ok(Response::Success(pb::Success {}))
@@ -207,7 +199,7 @@ mod tests {
             )),
             Ok(Response::Success(pb::Success {}))
         );
-        assert_eq!(bitbox02::securechip::fake_event_counter(), 19);
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 14);
         drop(mock_hal); // to remove mutable borrow of counter
         assert_eq!(counter, 2);
         assert!(!keystore::is_locked());

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

@@ -40,9 +40,6 @@ pub async fn process(
         hal.ui().status(&format!("Error\n{:?}", err), false).await;
         return Err(Error::Generic);
     }
-    if keystore::unlock(&password).is_err() {
-        panic!("Unexpected error during restore: unlock failed.");
-    }
     unlock::unlock_bip39(hal).await;
     Ok(Response::Success(pb::Success {}))
 }
@@ -83,7 +80,7 @@ mod tests {
             )),
             Ok(Response::Success(pb::Success {}))
         );
-        assert_eq!(bitbox02::securechip::fake_event_counter(), 19);
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 14);
         drop(mock_hal); // to remove mutable borrow of counter
         assert_eq!(counter, 2);
         assert!(!keystore::is_locked());

src/rust/bitbox02-rust/src/keystore.rs

@@ -577,14 +577,7 @@ mod tests {
 
             bitbox02::securechip::fake_event_counter_reset();
             assert!(keystore::encrypt_and_store_seed(seed, "foo").is_ok());
-            assert_eq!(bitbox02::securechip::fake_event_counter(), 11);
-
-            assert!(keystore::unlock_bip39(test.mnemonic_passphrase).is_err());
-            assert!(keystore::is_locked());
-
-            bitbox02::securechip::fake_event_counter_reset();
-            assert!(keystore::unlock("foo").is_ok());
-            assert_eq!(bitbox02::securechip::fake_event_counter(), 6);
+            assert_eq!(bitbox02::securechip::fake_event_counter(), 12);
 
             assert!(keystore::is_locked());
 

src/rust/bitbox02/src/keystore.rs

@@ -408,7 +408,6 @@ mod tests {
         let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
             .unwrap();
         assert!(encrypt_and_store_seed(&seed, "password").is_ok());
-        assert!(unlock("password").is_ok());
         assert!(is_locked()); // still locked, it is only unlocked after unlock_bip39.
         assert!(unlock_bip39("foo").is_ok());
         assert!(!is_locked());
@@ -432,6 +431,7 @@ mod tests {
         crate::memory::set_salt_root(mock_salt_root.as_slice().try_into().unwrap()).unwrap();
 
         assert!(encrypt_and_store_seed(&seed, "password").is_ok());
+        lock();
 
         // Loop to check that unlocking works while unlocked.
         for _ in 0..3 {
@@ -497,7 +497,6 @@ mod tests {
 
         assert!(root_fingerprint().is_err());
         assert!(encrypt_and_store_seed(&seed, "password").is_ok());
-        assert!(unlock("password").is_ok());
         assert!(root_fingerprint().is_err());
         assert!(unlock_bip39("foo").is_ok());
         assert_eq!(root_fingerprint(), Ok(vec![0xf1, 0xbc, 0x3c, 0x46]),);
@@ -572,7 +571,6 @@ mod tests {
             lock();
 
             assert!(create_and_store_seed("password", &host_entropy[..size]).is_ok());
-            assert!(unlock("password").is_ok());
             assert_eq!(copy_seed().unwrap().as_slice(), &expected_seed[..size]);
             // Check the seed has been stored encrypted with the expected encryption key.
             // Decrypt and check seed.
@@ -603,14 +601,12 @@ mod tests {
         let seed2 = hex::decode("c28135734876aff9ccf4f1d60df8d19a0a38fd02085883f65fc608eb769a635d")
             .unwrap();
         assert!(encrypt_and_store_seed(&seed, "password").is_ok());
-        assert!(unlock("password").is_ok());
         // Create new (different) seed.
         assert!(encrypt_and_store_seed(&seed2, "password").is_ok());
-        assert!(unlock("password").is_ok());
         assert_eq!(copy_seed().unwrap().as_slice(), &seed2);
     }
 
-    // Functional test to store seeds, unlock, retrieve seed.
+    // Functional test to store seeds, lock/unlock, retrieve seed.
     #[test]
     fn test_seeds() {
         let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
@@ -624,6 +620,12 @@ mod tests {
             for _ in 0..2 {
                 assert!(encrypt_and_store_seed(&seed[..seed_size], "foo").is_ok());
             }
+            // Also unlocks, so we can get the retained seed.
+            assert_eq!(copy_seed().unwrap().as_slice(), &seed[..seed_size]);
+
+            lock();
+            // Can't get seed before unlock.
+            assert!(copy_seed().is_err());
 
             // Wrong password.
             assert!(matches!(
@@ -633,8 +635,6 @@ mod tests {
                 })
             ));
 
-            // Can't get seed before unlock.
-            assert!(copy_seed().is_err());
             // Correct password. First time: unlock. After unlock, it becomes a password check.
             for _ in 0..3 {
                 assert!(unlock("foo").is_ok());