Merge branch 'unlock-reduce-sc-events'

benma · benma · commit 7363fdee6335 · 2025-10-13T09:33:51.000+02:00
diff --git a/src/keystore.c b/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();
diff --git a/src/keystore.h b/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()`
diff --git a/src/rust/bitbox02-rust/src/hww/api/backup.rs b/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() {
@@ -186,6 +186,7 @@ mod tests {
 
         let mut mock_hal = TestingHal::new();
         mock_hal.sd.inserted = Some(true);
+        bitbox02::securechip::fake_event_counter_reset();
         assert_eq!(
             block_on(create(
                 &mut mock_hal,
@@ -196,6 +197,7 @@ mod tests {
             )),
             Ok(Response::Success(pb::Success {}))
         );
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 1);
         assert_eq!(EXPECTED_TIMESTMAP, bitbox02::memory::get_seed_birthdate());
         assert_eq!(
             mock_hal.ui.screens,
@@ -223,6 +225,68 @@ mod tests {
         );
     }
 
+    /// Test backup creation on a initialized keystore. The sdcard does not contain the backup yet.
+    #[test]
+    pub fn test_create_initialized_new() {
+        const TIMESTMAP: u32 = 1601281809;
+
+        mock_memory();
+
+        let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
+            .unwrap();
+        bitbox02::keystore::encrypt_and_store_seed(&seed, "password").unwrap();
+        bitbox02::memory::set_initialized().unwrap();
+
+        let mut password_entered: bool = false;
+
+        let mut mock_hal = TestingHal::new();
+        mock_hal.sd.inserted = Some(true);
+        mock_hal.ui.set_enter_string(Box::new(|_params| {
+            password_entered = true;
+            Ok("password".into())
+        }));
+        bitbox02::securechip::fake_event_counter_reset();
+        assert_eq!(
+            block_on(create(
+                &mut mock_hal,
+                &pb::CreateBackupRequest {
+                    timestamp: TIMESTMAP,
+                    timezone_offset: 18000,
+                }
+            )),
+            Ok(Response::Success(pb::Success {}))
+        );
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 5);
+        assert_eq!(
+            mock_hal.ui.screens,
+            vec![
+                Screen::Confirm {
+                    title: "Is today?".into(),
+                    body: "Mon 2020-09-28".into(),
+                    longtouch: false
+                },
+                Screen::Status {
+                    title: "Backup created".into(),
+                    success: true
+                }
+            ]
+        );
+
+        mock_hal.ui.remove_enter_string(); // no more password entry needed
+        assert_eq!(
+            block_on(check(
+                &mut mock_hal,
+                &pb::CheckBackupRequest { silent: true }
+            )),
+            Ok(Response::CheckBackup(pb::CheckBackupResponse {
+                id: backup::id(&seed),
+            }))
+        );
+
+        drop(mock_hal); // to remove mutable borrow of `password_entered`
+        assert!(password_entered);
+    }
+
     /// Use backup file fixtures generated using firmware v9.12.0 and perform tests on it. This
     /// should catch regressions when changing backup loading/verification in the firmware code.
     #[test]
diff --git a/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs b/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 {
@@ -139,17 +141,20 @@ mod tests {
 
         bitbox02::memory::set_initialized().unwrap();
 
+        let mut password_entered: bool = false;
+
         let mut mock_hal = TestingHal::new();
-        mock_hal
-            .ui
-            .set_enter_string(Box::new(|_params| Ok("password".into())));
+        mock_hal.ui.set_enter_string(Box::new(|_params| {
+            password_entered = true;
+            Ok("password".into())
+        }));
 
         bitbox02::securechip::fake_event_counter_reset();
         assert_eq!(
             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,
@@ -173,6 +178,9 @@ mod tests {
                 },
             ]
         );
+
+        drop(mock_hal); // to remove mutable borrow of `password_entered`
+        assert!(password_entered);
     }
 
     /// When initialized, a password check is prompted before displaying the mnemonic.
diff --git a/src/rust/bitbox02-rust/src/workflow/testing.rs b/src/rust/bitbox02-rust/src/workflow/testing.rs
@@ -206,4 +206,8 @@ impl<'a> TestingWorkflows<'a> {
     pub fn set_enter_string(&mut self, cb: EnterStringCb<'a>) {
         self._enter_string = Some(cb);
     }
+
+    pub fn remove_enter_string(&mut self) {
+        self._enter_string = None;
+    }
 }
diff --git a/src/rust/bitbox02-rust/src/workflow/unlock.rs b/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,11 +173,63 @@ 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()
-    {}
+    loop {
+        if let Ok(seed) = unlock_keystore(hal, "Enter password", password::CanCancel::No).await {
+            unlock_bip39(hal, &seed).await;
+            return Ok(());
+        }
+    }
+}
 
-    unlock_bip39(hal, &bitbox02::keystore::copy_seed()?).await;
-    Ok(())
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::hal::testing::TestingHal;
+    use crate::workflow::testing::Screen;
+    use alloc::boxed::Box;
+    use bitbox02::testing::{mock_memory, mock_unlocked, mock_unlocked_using_mnemonic};
+    use util::bb02_async::block_on;
+
+    #[test]
+    fn test_unlock_success() {
+        mock_memory();
+
+        // Set up an initialized wallet with password
+        bitbox02::keystore::encrypt_and_store_seed(
+            hex::decode("c7940c13479b8d9a6498f4e50d5a42e0d617bc8e8ac9f2b8cecf97e94c2b035c")
+                .unwrap()
+                .as_slice(),
+            "password",
+        )
+        .unwrap();
+
+        bitbox02::memory::set_initialized().unwrap();
+
+        // Lock the keystore to simulate the normal locked state
+        bitbox02::keystore::lock();
+
+        let mut password_entered = false;
+
+        let mut mock_hal = TestingHal::new();
+        mock_hal.ui.set_enter_string(Box::new(|_params| {
+            password_entered = true;
+            Ok("password".into())
+        }));
+        bitbox02::securechip::fake_event_counter_reset();
+        assert_eq!(block_on(unlock(&mut mock_hal)), Ok(()));
+        // 6 for keystore unlock, 1 for keystore bip39 unlock.
+        assert_eq!(bitbox02::securechip::fake_event_counter(), 7);
+
+        assert!(!bitbox02::keystore::is_locked());
+
+        assert_eq!(
+            bitbox02::keystore::copy_bip39_seed().unwrap().as_slice(),
+            hex::decode("cff4b263e5b0eb299e5fd35fcd09988f6b14e5b464f8d18fb84b152f889dd2a30550f4c2b346cae825ffedd4a87fc63fc12a9433de5125b6c7fdbc5eab0c590b")
+                .unwrap(),
+        );
+
+        drop(mock_hal); // to remove mutable borrow of `password_entered`
+        assert!(password_entered);
+    }
 }
diff --git a/src/rust/bitbox02/src/keystore.rs b/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]);
 

Original file line number	Diff line number	Diff line change
`@@ -206,4 +206,8 @@ impl<'a> TestingWorkflows<'a> {`
`206`	`206`	`pub fn set_enter_string(&mut self, cb: EnterStringCb<'a>) {`
`207`	`207`	`self._enter_string = Some(cb);`
`208`	`208`	`}`
	`209`	`+`
	`210`	`+ pub fn remove_enter_string(&mut self) {`
	`211`	`+ self._enter_string = None;`
	`212`	`+ }`
`209`	`213`	`}`