Moves "unlock_attempts" functions to HAL

Author: cedwies

src/memory/bitbox02_smarteeprom.h

@@ -20,6 +20,9 @@
 /**
  * After this many failed unlock attempts, the keystore becomes locked until a
  * device reset.
+ *
+ * Must match MAX_UNLOCK_ATTEMPTS in rust keystore.rs and
+ * SMALL_MONOTONIC_COUNTER_MAX_USE in optiga.c.
  */
 #define MAX_UNLOCK_ATTEMPTS (10)
 

src/optiga/optiga.c

@@ -50,6 +50,8 @@
 
 // Number of times the password can be entered incorrectly before further password stretching fails.
 // The counter is reset when the correct password is entered.
+// Must match MAX_UNLOCK_ATTEMPTS in bitbox02_smarteeprom.h and
+// MAX_UNLOCK_ATTEMPTS in rust keystore.rs.
 #define SMALL_MONOTONIC_COUNTER_MAX_USE (MAX_UNLOCK_ATTEMPTS)
 
 // Maximum size of metadata. See "Metadata Update Identifier":

src/rust/bitbox02-rust/src/hal.rs

@@ -75,6 +75,9 @@ pub trait Memory {
     fn get_encrypted_seed_and_hmac(&mut self) -> Result<alloc::vec::Vec<u8>, ()>;
     fn set_encrypted_seed_and_hmac(&mut self, data: &[u8]) -> Result<(), ()>;
     fn reset_hww(&mut self) -> Result<(), ()>;
+    fn get_unlock_attempts(&mut self) -> u8;
+    fn increment_unlock_attempts(&mut self);
+    fn reset_unlock_attempts(&mut self);
 }
 
 /// Hardware abstraction layer for BitBox devices.
@@ -243,6 +246,16 @@ impl Memory for BitBox02Memory {
     fn reset_hww(&mut self) -> Result<(), ()> {
         bitbox02::memory::reset_hww()
     }
+
+    fn get_unlock_attempts(&mut self) -> u8 {
+        bitbox02::memory::smarteeprom_get_unlock_attempts()
+    }
+    fn increment_unlock_attempts(&mut self) {
+        bitbox02::memory::smarteeprom_increment_unlock_attempts()
+    }
+    fn reset_unlock_attempts(&mut self) {
+        bitbox02::memory::smarteeprom_reset_unlock_attempts()
+    }
 }
 
 pub struct BitBox02Hal {
@@ -403,6 +416,7 @@ pub mod testing {
         seed_birthdate: u32,
         encrypted_seed_and_hmac: Option<Vec<u8>>,
         device_name: Option<String>,
+        unlock_attempts: u8,
     }
 
     impl TestingSecureChip {
@@ -522,6 +536,7 @@ pub mod testing {
                 seed_birthdate: 0,
                 encrypted_seed_and_hmac: None,
                 device_name: None,
+                unlock_attempts: 0,
             }
         }
 
@@ -532,6 +547,10 @@ pub mod testing {
         pub fn set_platform(&mut self, platform: bitbox02::memory::Platform) {
             self.platform = platform;
         }
+
+        pub fn set_unlock_attempts_for_testing(&mut self, attempts: u8) {
+            self.unlock_attempts = attempts;
+        }
     }
 
     impl super::Memory for TestingMemory {
@@ -608,6 +627,18 @@ pub mod testing {
             self.device_name = None;
             Ok(())
         }
+
+        fn get_unlock_attempts(&mut self) -> u8 {
+            self.unlock_attempts
+        }
+
+        fn increment_unlock_attempts(&mut self) {
+            self.unlock_attempts += 1;
+        }
+
+        fn reset_unlock_attempts(&mut self) {
+            self.unlock_attempts = 0;
+        }
     }
 
     pub struct TestingHal<'a> {

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

@@ -40,6 +40,10 @@ const ENCRYPTION_OVERHEAD: usize = 64;
 // to roughly seven seconds so we don't assume communication was lost mid-unlock.
 const LONG_TIMEOUT: i16 = -70;
 
+// Must match MAX_UNLOCK_ATTEMPTS in bitbox02_smarteeprom.h and
+// SMALL_MONOTONIC_COUNTER_MAX_USE in optiga.c.
+const MAX_UNLOCK_ATTEMPTS: u8 = 10;
+
 #[derive(Debug)]
 pub enum Error {
     InvalidState,
@@ -337,7 +341,7 @@ pub async fn unlock(
     if !hal.memory().is_seeded() {
         return Err(Error::Unseeded);
     }
-    if get_remaining_unlock_attempts() == 0 {
+    if get_remaining_unlock_attempts(hal) == 0 {
         //
         //  We reset the device as soon as the MAX_UNLOCK_ATTEMPTSth attempt
         //  is made. So we should never enter this branch...
@@ -347,11 +351,11 @@ pub async fn unlock(
         return Err(Error::MaxAttemptsExceeded);
     }
     bitbox02::usb_processing::timeout_reset(LONG_TIMEOUT);
-    bitbox02::memory::smarteeprom_increment_unlock_attempts();
+    hal.memory().increment_unlock_attempts();
     let seed = match get_and_decrypt_seed(hal, password) {
         Ok(seed) => seed,
         err @ Err(_) => {
-            if get_remaining_unlock_attempts() == 0 {
+            if get_remaining_unlock_attempts(hal) == 0 {
                 crate::reset::reset(hal, false).await;
                 return Err(Error::MaxAttemptsExceeded);
             }
@@ -367,15 +371,15 @@ pub async fn unlock(
     } else {
         retain_seed(hal, &seed)?;
     }
-    bitbox02::memory::smarteeprom_reset_unlock_attempts();
+    hal.memory().reset_unlock_attempts();
     Ok(seed)
 }
 
 /// Returns the number of remaining unlock attempts (calls to `unlock()`) that are allowed before
 /// the device resets itself.
-pub fn get_remaining_unlock_attempts() -> u8 {
-    let failed_attempts: u8 = bitbox02::memory::smarteeprom_get_unlock_attempts();
-    bitbox02::memory::MAX_UNLOCK_ATTEMPTS.saturating_sub(failed_attempts)
+pub fn get_remaining_unlock_attempts(hal: &mut impl crate::hal::Hal) -> u8 {
+    let failed_attempts: u8 = hal.memory().get_unlock_attempts();
+    MAX_UNLOCK_ATTEMPTS.saturating_sub(failed_attempts)
 }
 
 /// Unlocks the bip39 seed. The input seed must be the keystore seed (i.e. must match the output
@@ -1173,14 +1177,14 @@ mod tests {
         assert_eq!(decrypted.as_slice(), seed.as_slice());
 
         // First 9 wrong attempts.
-        for i in 1..bitbox02::memory::MAX_UNLOCK_ATTEMPTS {
+        for i in 1..MAX_UNLOCK_ATTEMPTS {
             assert!(matches!(
                 block_on(unlock(&mut mock_hal, "invalid password")),
                 Err(Error::IncorrectPassword)
             ));
             assert_eq!(
-                get_remaining_unlock_attempts(),
-                bitbox02::memory::MAX_UNLOCK_ATTEMPTS - i
+                get_remaining_unlock_attempts(&mut mock_hal),
+                MAX_UNLOCK_ATTEMPTS - i
             );
             // Still seeded.
             assert!(mock_hal.memory.is_seeded());
@@ -1218,15 +1222,15 @@ mod tests {
         assert!(is_locked());
         assert!(copy_seed(&mut mock_hal).is_err());
 
-        for attempt in 1..bitbox02::memory::MAX_UNLOCK_ATTEMPTS {
+        for attempt in 1..MAX_UNLOCK_ATTEMPTS {
             assert!(matches!(
                 block_on(unlock(&mut mock_hal, "invalid password")),
                 Err(Error::IncorrectPassword),
             ));
 
             assert_eq!(
-                get_remaining_unlock_attempts(),
-                bitbox02::memory::MAX_UNLOCK_ATTEMPTS - attempt
+                get_remaining_unlock_attempts(&mut mock_hal),
+                MAX_UNLOCK_ATTEMPTS - attempt
             );
             assert!(is_locked());
             assert!(copy_seed(&mut mock_hal).is_err());
@@ -1265,12 +1269,11 @@ mod tests {
         lock();
         assert!(is_locked());
 
-        bitbox02::memory::set_unlock_attempts_for_testing(bitbox02::memory::MAX_UNLOCK_ATTEMPTS);
-        assert_eq!(get_remaining_unlock_attempts(), 0);
-        assert_eq!(
-            bitbox02::memory::smarteeprom_get_unlock_attempts(),
-            bitbox02::memory::MAX_UNLOCK_ATTEMPTS
-        );
+        let max = MAX_UNLOCK_ATTEMPTS;
+        mock_hal.memory.set_unlock_attempts_for_testing(max);
+
+        assert_eq!(get_remaining_unlock_attempts(&mut mock_hal), 0);
+        assert_eq!(mock_hal.memory.get_unlock_attempts(), MAX_UNLOCK_ATTEMPTS);
 
         assert!(matches!(
             block_on(unlock(&mut mock_hal, "password")),
@@ -1303,10 +1306,7 @@ mod tests {
                 block_on(unlock(hal, "wrong")),
                 Err(Error::IncorrectPassword)
             ));
-            assert_eq!(
-                get_remaining_unlock_attempts(),
-                bitbox02::memory::MAX_UNLOCK_ATTEMPTS - 1
-            );
+            assert_eq!(get_remaining_unlock_attempts(hal), MAX_UNLOCK_ATTEMPTS - 1);
         }
 
         wrong_attempt(&mut mock_hal);
@@ -1358,10 +1358,7 @@ mod tests {
                 block_on(unlock(hal, "wrong")),
                 Err(Error::IncorrectPassword)
             ));
-            assert_eq!(
-                get_remaining_unlock_attempts(),
-                bitbox02::memory::MAX_UNLOCK_ATTEMPTS - 1
-            );
+            assert_eq!(get_remaining_unlock_attempts(hal), MAX_UNLOCK_ATTEMPTS - 1);
         }
 
         wrong_attempt(&mut mock_hal);
@@ -2049,7 +2046,7 @@ mod tests {
                 block_on(unlock(&mut mock_hal, "bar")),
                 Err(Error::IncorrectPassword)
             ));
-            assert_eq!(get_remaining_unlock_attempts(), 9);
+            assert_eq!(get_remaining_unlock_attempts(&mut mock_hal), 9);
 
             // Correct password. First time: unlock. After unlock, it becomes a password check.
             for _ in 0..3 {

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

@@ -92,7 +92,7 @@ pub async fn unlock_keystore(
     match crate::keystore::unlock(hal, &password).await {
         Ok(seed) => Ok(seed),
         Err(crate::keystore::Error::IncorrectPassword) => {
-            let msg = match crate::keystore::get_remaining_unlock_attempts() {
+            let msg = match crate::keystore::get_remaining_unlock_attempts(hal) {
                 1 => "Wrong password\n1 try remains".into(),
                 n => format!("Wrong password\n{} tries remain", n),
             };

src/rust/bitbox02/src/memory.rs

@@ -17,8 +17,6 @@ extern crate alloc;
 use alloc::string::String;
 use alloc::vec::Vec;
 
-pub const MAX_UNLOCK_ATTEMPTS: u8 = bitbox02_sys::MAX_UNLOCK_ATTEMPTS as u8;
-
 // deduct one for the null terminator.
 pub const DEVICE_NAME_MAX_LEN: usize = bitbox02_sys::MEMORY_DEVICE_NAME_MAX_LEN as usize - 1;
 
@@ -186,17 +184,6 @@ pub fn smarteeprom_reset_unlock_attempts() {
     }
 }
 
-/// Testing helper to set the recorded number of unlock attempts to a precise value.
-/// Panics if `attempts` exceeds the maximum supported by the firmware.
-#[cfg(feature = "testing")]
-pub fn set_unlock_attempts_for_testing(attempts: u8) {
-    assert!(attempts <= MAX_UNLOCK_ATTEMPTS);
-    smarteeprom_reset_unlock_attempts();
-    for _ in 0..attempts {
-        smarteeprom_increment_unlock_attempts();
-    }
-}
-
 pub fn multisig_set_by_hash(hash: &[u8], name: &str) -> Result<(), MemoryError> {
     if hash.len() != 32 {
         return Err(MemoryError::MEMORY_ERR_INVALID_INPUT);