keystore: reduce secure chip operations when unlocking

Every call to `keystore::unlock` is followed by `copy_seed()`. The latter
uses a secure chip operation. We can remove that by returning the seed
from unlock and re-using it.

This reduces the number of securechip operations by 1 when:

- when unlocking the device
- showing mnemonic on initialized device
- creating backu on initialized device

This is to reduce the risk of running into Optiga's throttling
security mechanism.

Author: benma

src/keystore.c

@@ -445,7 +445,9 @@ static bool _check_retained_seed(const uint8_t* seed, size_t seed_length)
 keystore_error_t keystore_unlock(
     const char* password,
     uint8_t* remaining_attempts_out,
-    int* securechip_result_out)
+    int* securechip_result_out,
+    uint8_t* seed_out,
+    size_t* seed_len_out)
 {
     if (!memory_is_seeded()) {
         return KEYSTORE_ERR_UNSEEDED;
@@ -486,6 +488,11 @@ keystore_error_t keystore_unlock(
             _is_unlocked_device = true;
         }
         bitbox02_smarteeprom_reset_unlock_attempts();
+
+        if (seed_out != NULL && seed_len_out != NULL) {
+            memcpy(seed_out, seed, seed_len);
+            *seed_len_out = seed_len;
+        }
     }
     // Compute remaining attempts
     failed_attempts = bitbox02_smarteeprom_get_unlock_attempts();

src/keystore.h

@@ -95,14 +95,22 @@ USE_RESULT keystore_error_t keystore_create_and_store_seed(
  * @param[out] remaining_attempts_out will have the number of remaining attempts.
  * If zero, the keystore is locked until the device is reset.
  * @param[out] securechip_result_out, if not NULL, will contain the error code from
+ * @param[out] seed_out The seed bytes copied from the retained seed.
+ * The buffer should be KEYSTORE_MAX_SEED_LENGTH bytes long. The caller must
+ * zero the seed once it is no longer needed.
+ * @param[out] seed_len_out The seed length.
  * `securechip_kdf()` if there was a secure chip error, and 0 otherwise.
  * @return
  * - KEYSTORE_OK if they keystore was successfully unlocked
  * - KEYSTORE_ERR_* if unsuccessful.
  * Only call this if memory_is_seeded() returns true.
  */
-USE_RESULT keystore_error_t
-keystore_unlock(const char* password, uint8_t* remaining_attempts_out, int* securechip_result_out);
+USE_RESULT keystore_error_t keystore_unlock(
+    const char* password,
+    uint8_t* remaining_attempts_out,
+    int* securechip_result_out,
+    uint8_t* seed_out,
+    size_t* seed_len_out);
 
 /**
  * Checks if bip39 unlocking can be performed. It can be performed if `keystore_unlock()`

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

@@ -99,15 +99,15 @@ pub async fn create(
 
     let is_initialized = bitbox02::memory::is_initialized();
 
-    if is_initialized {
-        unlock::unlock_keystore(hal, "Unlock device", unlock::CanCancel::Yes).await?;
-    }
-
-    let seed = bitbox02::keystore::copy_seed()?;
-
-    // Yield now to give executor a chance to process USB/BLE communication, as copy_seed() causes
-    // some delay.
-    futures_lite::future::yield_now().await;
+    let seed = if is_initialized {
+        unlock::unlock_keystore(hal, "Unlock device", unlock::CanCancel::Yes).await?
+    } else {
+        let seed = bitbox02::keystore::copy_seed()?;
+        // Yield now to give executor a chance to process USB/BLE communication, as copy_seed() causes
+        // some delay.
+        futures_lite::future::yield_now().await;
+        seed
+    };
 
     let seed_birthdate = if !is_initialized {
         if bitbox02::memory::set_seed_birthdate(timestamp).is_err() {
@@ -256,7 +256,7 @@ mod tests {
             )),
             Ok(Response::Success(pb::Success {}))
         );
-        assert_eq!(bitbox02::securechip::fake_event_counter(), 6);
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 5);
         assert_eq!(
             mock_hal.ui.screens,
             vec![

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

@@ -22,18 +22,20 @@ use pb::response::Response;
 use crate::hal::Ui;
 use crate::workflow::{confirm, unlock};
 
-use crate::keystore;
-
 /// Handle the ShowMnemonic API call. This shows the seed encoded as
 /// 12/18/24 BIP39 English words. Afterwards, for each word, the user
 /// is asked to pick the right word among 5 words, to check if they
 /// wrote it down correctly.
 pub async fn process(hal: &mut impl crate::hal::Hal) -> Result<Response, Error> {
-    if bitbox02::memory::is_initialized() {
-        unlock::unlock_keystore(hal, "Unlock device", unlock::CanCancel::Yes).await?;
-    }
+    let mnemonic_sentence = {
+        let seed = if bitbox02::memory::is_initialized() {
+            unlock::unlock_keystore(hal, "Unlock device", unlock::CanCancel::Yes).await?
+        } else {
+            bitbox02::keystore::copy_seed()?
+        };
 
-    let mnemonic_sentence = keystore::get_bip39_mnemonic()?;
+        bitbox02::keystore::bip39_mnemonic_from_seed(&seed)?
+    };
 
     hal.ui()
         .confirm(&confirm::Params {
@@ -152,7 +154,7 @@ mod tests {
             block_on(process(&mut mock_hal)),
             Ok(Response::Success(pb::Success {}))
         );
-        assert_eq!(bitbox02::securechip::fake_event_counter(), 6);
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 5);
 
         assert_eq!(
             mock_hal.ui.screens,

src/rust/bitbox02-rust/src/workflow/unlock.rs

@@ -19,6 +19,8 @@ use bitbox02::keystore;
 
 pub use password::CanCancel;
 
+use alloc::vec::Vec;
+
 /// Confirm the entered mnemonic passphrase with the user. Returns true if the user confirmed it,
 /// false if the user rejected it.
 async fn confirm_mnemonic_passphrase(
@@ -79,7 +81,7 @@ pub async fn unlock_keystore(
     hal: &mut impl crate::hal::Hal,
     title: &str,
     can_cancel: password::CanCancel,
-) -> Result<(), UnlockError> {
+) -> Result<zeroize::Zeroizing<Vec<u8>>, UnlockError> {
     let password = password::enter(
         hal,
         title,
@@ -89,7 +91,7 @@ pub async fn unlock_keystore(
     .await?;
 
     match keystore::unlock(&password) {
-        Ok(()) => Ok(()),
+        Ok(seed) => Ok(seed),
         Err(keystore::Error::IncorrectPassword { remaining_attempts }) => {
             let msg = match remaining_attempts {
                 1 => "Wrong password\n1 try remains".into(),
@@ -171,13 +173,12 @@ pub async fn unlock(hal: &mut impl crate::hal::Hal) -> Result<(), ()> {
     }
 
     // Loop unlock until the password is correct or the device resets.
-    while unlock_keystore(hal, "Enter password", password::CanCancel::No)
-        .await
-        .is_err()
-    {}
-
-    unlock_bip39(hal, &bitbox02::keystore::copy_seed()?).await;
-    Ok(())
+    loop {
+        if let Ok(seed) = unlock_keystore(hal, "Enter password", password::CanCancel::No).await {
+            unlock_bip39(hal, &seed).await;
+            return Ok(());
+        }
+    }
 }
 
 #[cfg(test)]
@@ -217,7 +218,8 @@ mod tests {
         }));
         bitbox02::securechip::fake_event_counter_reset();
         assert_eq!(block_on(unlock(&mut mock_hal)), Ok(()));
-        assert_eq!(bitbox02::securechip::fake_event_counter(), 8);
+        // 6 for keystore unlock, 1 for keystore bip39 unlock.
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 7);
 
         assert!(!bitbox02::keystore::is_locked());
 

src/rust/bitbox02/src/keystore.rs

@@ -67,9 +67,11 @@ impl core::convert::From<keystore_error_t> for Error {
     }
 }
 
-pub fn unlock(password: &str) -> Result<(), Error> {
+pub fn unlock(password: &str) -> Result<zeroize::Zeroizing<Vec<u8>>, Error> {
     let mut remaining_attempts: u8 = 0;
     let mut securechip_result: i32 = 0;
+    let mut seed = zeroize::Zeroizing::new([0u8; MAX_SEED_LENGTH].to_vec());
+    let mut seed_len: usize = 0;
     match unsafe {
         bitbox02_sys::keystore_unlock(
             crate::util::str_to_cstr_vec(password)
@@ -78,9 +80,14 @@ pub fn unlock(password: &str) -> Result<(), Error> {
                 .cast(),
             &mut remaining_attempts,
             &mut securechip_result,
+            seed.as_mut_ptr(),
+            &mut seed_len,
         )
     } {
-        keystore_error_t::KEYSTORE_OK => Ok(()),
+        keystore_error_t::KEYSTORE_OK => {
+            seed.truncate(seed_len);
+            Ok(seed)
+        }
         keystore_error_t::KEYSTORE_ERR_INCORRECT_PASSWORD => {
             Err(Error::IncorrectPassword { remaining_attempts })
         }
@@ -483,15 +490,15 @@ mod tests {
 
         // 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!(unlock("password").unwrap().as_slice(), seed);
         assert_eq!(crate::securechip::fake_event_counter(), 6);
 
         // Loop to check that unlocking works while unlocked.
         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!(unlock("password").unwrap().as_slice(), seed);
             assert_eq!(crate::securechip::fake_event_counter(), 5);
         }
 
@@ -630,7 +637,9 @@ mod tests {
             .is_err()
         );
         // Correct seed passed.
+        crate::securechip::fake_event_counter_reset();
         assert!(block_on(unlock_bip39(&secp, &seed, "foo", async || {})).is_ok());
+        assert_eq!(crate::securechip::fake_event_counter(), 1);
         assert_eq!(root_fingerprint(), Ok(vec![0xf1, 0xbc, 0x3c, 0x46]),);
 
         let expected_bip39_seed = hex::decode("2b3c63de86f0f2b13cc6a36c1ba2314fbc1b40c77ab9cb64e96ba4d5c62fc204748ca6626a9f035e7d431bce8c9210ec0bdffc2e7db873dee56c8ac2153eee9a").unwrap();
@@ -769,7 +778,7 @@ mod tests {
 
             // Correct password. First time: unlock. After unlock, it becomes a password check.
             for _ in 0..3 {
-                assert!(unlock("foo").is_ok());
+                assert_eq!(unlock("foo").unwrap().as_slice(), &seed[..seed_size]);
             }
             assert_eq!(copy_seed().unwrap().as_slice(), &seed[..seed_size]);