Public commit record
90/100 · Strong
What the developer wrote
Authored by cedwies
Move salt_root functions to HAL
Centralizes salt root access behind the HAL abstraction,
enabling full test isolation without C memory dependencies.
TestingHal initializes with a default salt root to preserve
existing test behavior without manual setup in each test.
The 2 rust_salt_hash_data tests keep using mock_memory() +
C-backed salt root because the extern "C" shim internally
constructs BitBox02Hal—these tests verify the FFI path works.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification
What changed, and why it matters
This commit is a straightforward internal refactoring: it moves access to a device secret called the 'salt root' behind an existing hardware-abstraction layer so that automated tests can use a fake memory instead of real device memory. There is no user-facing change, no bug fix, and no security vulnerability visible in the diff.
Recommended action
No security action required. Treat as normal code-review/merge for test maintainability.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors salt root retrieval from a direct C memory call (bitbox02::memory::get_salt_root) into a Memory trait method on the Rust HAL. hash_data now takes &mut impl Hal and calls hal.memory().get_salt_root(). A TestingMemory implementation is added with a configurable 32-byte salt root and a sentinel value of all 0xff to simulate retrieval failure. Existing tests are updated to set the salt root on the mock HAL rather than on the C memory layer. The production FFI shim (rust_salt_hash_data) constructs a fresh BitBox02Hal, preserving the previous behavior. No cryptographic algorithm, secret handling, or error path is changed.
Changed components
src/rust/bitbox02-rust/src/hal.rssrc/rust/bitbox02-rust/src/keystore.rssrc/rust/bitbox02-rust/src/salt.rsInspect captured patch +65 / −37
diff --git a/src/rust/bitbox02-rust/src/hal.rs b/src/rust/bitbox02-rust/src/hal.rs
index 0249b18..51df79e 100644
--- a/src/rust/bitbox02-rust/src/hal.rs
+++ b/src/rust/bitbox02-rust/src/hal.rs
@@ -78,6 +78,7 @@ pub trait Memory {
fn get_unlock_attempts(&mut self) -> u8;
fn increment_unlock_attempts(&mut self);
fn reset_unlock_attempts(&mut self);
+ fn get_salt_root(&mut self) -> Result<zeroize::Zeroizing<Vec<u8>>, ()>;
}
/// Hardware abstraction layer for BitBox devices.
@@ -256,6 +257,10 @@ impl Memory for BitBox02Memory {
fn reset_unlock_attempts(&mut self) {
bitbox02::memory::smarteeprom_reset_unlock_attempts()
}
+
+ fn get_salt_root(&mut self) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
+ bitbox02::memory::get_salt_root()
+ }
}
pub struct BitBox02Hal {
@@ -417,6 +422,7 @@ pub mod testing {
encrypted_seed_and_hmac: Option<Vec<u8>>,
device_name: Option<String>,
unlock_attempts: u8,
+ salt_root: [u8; 32],
}
impl TestingSecureChip {
@@ -537,6 +543,7 @@ pub mod testing {
encrypted_seed_and_hmac: None,
device_name: None,
unlock_attempts: 0,
+ salt_root: *b"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
}
}
@@ -551,6 +558,10 @@ pub mod testing {
pub fn set_unlock_attempts_for_testing(&mut self, attempts: u8) {
self.unlock_attempts = attempts;
}
+
+ pub fn set_salt_root(&mut self, salt_root: &[u8; 32]) {
+ self.salt_root = *salt_root;
+ }
}
impl super::Memory for TestingMemory {
@@ -639,6 +650,14 @@ pub mod testing {
fn reset_unlock_attempts(&mut self) {
self.unlock_attempts = 0;
}
+
+ fn get_salt_root(&mut self) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
+ if self.salt_root.iter().all(|&b| b == 0xff) {
+ Err(())
+ } else {
+ Ok(zeroize::Zeroizing::new(self.salt_root.to_vec()))
+ }
+ }
}
pub struct TestingHal<'a> {
diff --git a/src/rust/bitbox02-rust/src/keystore.rs b/src/rust/bitbox02-rust/src/keystore.rs
index ac77f07..725f09a 100644
--- a/src/rust/bitbox02-rust/src/keystore.rs
+++ b/src/rust/bitbox02-rust/src/keystore.rs
@@ -192,9 +192,9 @@ fn verify_seed(
decrypted.as_slice() == expected_seed
}
-fn hash_seed(seed: &[u8]) -> Result<[u8; 32], Error> {
+fn hash_seed(hal: &mut impl crate::hal::Hal, seed: &[u8]) -> Result<[u8; 32], Error> {
let salted_key =
- crate::salt::hash_data(&[], "keystore_retain_seed_hash").map_err(|_| Error::Salt)?;
+ crate::salt::hash_data(hal, &[], "keystore_retain_seed_hash").map_err(|_| Error::Salt)?;
let mut engine = HmacEngine::<sha256::Hash>::new(salted_key.as_slice());
engine.input(seed);
@@ -207,7 +207,7 @@ fn retain_seed(hal: &mut impl crate::hal::Hal, seed: &[u8]) -> Result<(), Error>
seed,
"keystore_retained_seed_access",
)?));
- RETAINED_SEED_HASH.write(Some(hash_seed(seed)?));
+ RETAINED_SEED_HASH.write(Some(hash_seed(hal, seed)?));
Ok(())
}
@@ -300,11 +300,11 @@ pub fn re_encrypt_seed(
}
// Checks if the retained seed matches the passed seed.
-fn check_retained_seed(seed: &[u8]) -> Result<(), ()> {
+fn check_retained_seed(hal: &mut impl crate::hal::Hal, seed: &[u8]) -> Result<(), ()> {
if RETAINED_SEED.read().is_none() {
return Err(());
}
- if hash_seed(seed).map_err(|_| ())? != RETAINED_SEED_HASH.read().ok_or(())? {
+ if hash_seed(hal, seed).map_err(|_| ())? != RETAINED_SEED_HASH.read().ok_or(())? {
return Err(());
}
Ok(())
@@ -365,7 +365,7 @@ pub async fn unlock(
if RETAINED_SEED.read().is_some() {
// Already unlocked. Fail if the seed changed under our feet (should never happen).
- if check_retained_seed(&seed).is_err() {
+ if check_retained_seed(hal, &seed).is_err() {
panic!("Seed has suddenly changed. This should never happen.");
}
} else {
@@ -392,7 +392,7 @@ pub async fn unlock_bip39(
mnemonic_passphrase: &str,
yield_now: impl AsyncFn(),
) -> Result<(), Error> {
- check_retained_seed(seed).map_err(|_| Error::CannotUnlockBIP39)?;
+ check_retained_seed(hal, seed).map_err(|_| Error::CannotUnlockBIP39)?;
let (bip39_seed, root_fingerprint) =
crate::bip39::derive_seed(seed, mnemonic_passphrase, &yield_now).await;
@@ -449,7 +449,7 @@ pub fn create_and_store_seed(
// Mix in entropy derived from the user password.
let password_salted_hashed =
- crate::salt::hash_data(password.as_bytes(), "keystore_seed_generation")
+ crate::salt::hash_data(hal, password.as_bytes(), "keystore_seed_generation")
.map_err(|_| Error::Salt)?;
for (i, &hash_byte) in password_salted_hashed.iter().take(seed_len).enumerate() {
@@ -585,12 +585,13 @@ pub fn stretch_retained_seed_encryption_key(
purpose_in: &str,
purpose_out: &str,
) -> Result<zeroize::Zeroizing<Vec<u8>>, Error> {
- let salted_in = crate::salt::hash_data(encryption_key, purpose_in).map_err(|_| Error::Salt)?;
+ let salted_in =
+ crate::salt::hash_data(hal, encryption_key, purpose_in).map_err(|_| Error::Salt)?;
let kdf = hal.securechip().kdf(salted_in.as_slice())?;
let salted_out =
- crate::salt::hash_data(encryption_key, purpose_out).map_err(|_| Error::Salt)?;
+ crate::salt::hash_data(hal, encryption_key, purpose_out).map_err(|_| Error::Salt)?;
let mut engine = HmacEngine::<sha256::Hash>::new(salted_out.as_slice());
engine.input(kdf.as_slice());
@@ -902,12 +903,9 @@ mod tests {
.collect();
for size in [16, 32] {
- mock_memory();
- bitbox02::random::fake_reset();
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
- lock();
-
let mut hal = TestingHal::new();
+ hal.memory.set_salt_root(&mock_salt_root);
+
hal.random.mock_next(seed_random);
assert!(create_and_store_seed(&mut hal, "password", &host_entropy[..size]).is_ok());
assert_eq!(
@@ -1130,7 +1128,7 @@ mod tests {
let mock_salt_root =
hex!("3333333333333333444444444444444411111111111111112222222222222222");
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&mock_salt_root);
assert!(encrypt_and_store_seed(&mut mock_hal, &seed, "password").is_ok());
lock();
@@ -1215,7 +1213,7 @@ mod tests {
let seed = hex!("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044");
let mock_salt_root =
hex!("3333333333333333444444444444444411111111111111112222222222222222");
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&mock_salt_root);
assert!(encrypt_and_store_seed(&mut mock_hal, &seed, "password").is_ok());
lock();
@@ -1263,7 +1261,7 @@ mod tests {
let seed = hex!("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044");
let mock_salt_root =
hex!("3333333333333333444444444444444411111111111111112222222222222222");
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&mock_salt_root);
assert!(encrypt_and_store_seed(&mut mock_hal, &seed, "password").is_ok());
lock();
@@ -1297,7 +1295,7 @@ mod tests {
let seed = hex!("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044");
let mock_salt_root =
hex!("3333333333333333444444444444444411111111111111112222222222222222");
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&mock_salt_root);
assert!(encrypt_and_store_seed(&mut mock_hal, &seed, "password").is_ok());
lock();
@@ -1341,7 +1339,7 @@ mod tests {
let seed = hex!("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044");
let mock_salt_root =
hex!("3333333333333333444444444444444411111111111111112222222222222222");
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&mock_salt_root);
assert!(encrypt_and_store_seed(&mut mock_hal, &seed, "password").is_ok());
lock();
@@ -1382,20 +1380,21 @@ mod tests {
fn test_unlock_bip39() {
mock_memory();
lock();
+ let mut mock_hal = TestingHal::new();
let seed = hex!("1111111111111111222222222222222233333333333333334444444444444444");
let mock_salt_root =
hex!("3333333333333333444444444444444411111111111111112222222222222222");
- bitbox02::memory::set_salt_root(&mock_salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&mock_salt_root);
assert!(root_fingerprint().is_err());
- assert!(encrypt_and_store_seed(&mut TestingHal::new(), &seed, "password").is_ok());
+ assert!(encrypt_and_store_seed(&mut mock_hal, &seed, "password").is_ok());
assert!(root_fingerprint().is_err());
// Incorrect seed passed
assert!(
block_on(unlock_bip39(
- &mut TestingHal::new(),
+ &mut mock_hal,
b"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"foo",
async || {}
@@ -1403,7 +1402,7 @@ mod tests {
.is_err()
);
// Correct seed passed.
- let mut mock_hal = TestingHal::new();
+
// Mock random value used for creating the unstretched bip39 seed encryption key.
mock_hal.random.mock_next(hex!(
"9b44c7048893faaf6e2d7625d13d8f1cab0765fd61f159d9713e08155d06717c"
@@ -1640,14 +1639,15 @@ mod tests {
#[test]
fn test_stretch_retained_seed_encryption_key_success() {
mock_memory();
+ let mut mock_hal = TestingHal::new();
let salt_root = hex!("0000000000000000111111111111111122222222222222223333333333333333");
- bitbox02::memory::set_salt_root(&salt_root).unwrap();
+ mock_hal.memory.set_salt_root(&salt_root);
let encryption_key =
hex!("00112233445566778899aabbccddeeff112233445566778899aabbccddeeff00");
let stretched = stretch_retained_seed_encryption_key(
- &mut TestingHal::new(),
+ &mut mock_hal,
&encryption_key,
"keystore_retained_seed_access_in",
"keystore_retained_seed_access_out",
@@ -1661,11 +1661,12 @@ mod tests {
#[test]
fn test_stretch_retained_seed_encryption_key_salt_error() {
mock_memory();
- bitbox02::memory::set_salt_root(&[0xffu8; 32]).unwrap();
+ let mut mock_hal = TestingHal::new();
+ mock_hal.memory.set_salt_root(&[0xffu8; 32]);
let encryption_key = [0u8; 32];
let result = stretch_retained_seed_encryption_key(
- &mut TestingHal::new(),
+ &mut mock_hal,
&encryption_key,
"purpose_in",
"purpose_out",
diff --git a/src/rust/bitbox02-rust/src/salt.rs b/src/rust/bitbox02-rust/src/salt.rs
index 6166481..464df95 100644
--- a/src/rust/bitbox02-rust/src/salt.rs
+++ b/src/rust/bitbox02-rust/src/salt.rs
@@ -15,7 +15,7 @@
use alloc::vec::Vec;
use core::ffi::c_char;
-use bitbox02::memory;
+use crate::hal::Memory;
use sha2::Digest;
use util::bytes::{Bytes, BytesMut};
use zeroize::Zeroizing;
@@ -25,8 +25,12 @@ use zeroize::Zeroizing;
/// the salt, and the provided `data` slice is hashed alongside it.
///
/// Returns `Err(())` if the salt root cannot be retrieved from persistent storage.
-pub fn hash_data(data: &[u8], purpose: &str) -> Result<Zeroizing<Vec<u8>>, ()> {
- let salt_root = memory::get_salt_root()?;
+pub fn hash_data(
+ hal: &mut impl crate::hal::Hal,
+ data: &[u8],
+ purpose: &str,
+) -> Result<Zeroizing<Vec<u8>>, ()> {
+ let salt_root = hal.memory().get_salt_root()?;
let mut hasher = sha2::Sha256::new();
hasher.update(salt_root.as_slice());
@@ -49,7 +53,8 @@ pub unsafe extern "C" fn rust_salt_hash_data(
Ok(purpose) => purpose,
Err(()) => return false,
};
- match hash_data(data.as_ref(), purpose_str) {
+ let mut hal = crate::hal::BitBox02Hal::new();
+ match hash_data(&mut hal, data.as_ref(), purpose_str) {
Ok(hash) => {
hash_out.as_mut()[..32].copy_from_slice(&hash);
true
@@ -61,7 +66,8 @@ pub unsafe extern "C" fn rust_salt_hash_data(
#[cfg(test)]
mod tests {
use super::*;
- use bitbox02::testing::mock_memory;
+ use crate::hal::testing::TestingHal;
+ use bitbox02::{memory, testing::mock_memory};
use core::convert::TryInto;
use core::ptr;
use hex_lit::hex;
@@ -72,23 +78,25 @@ mod tests {
#[test]
fn test_hash_data() {
mock_memory();
- memory::set_salt_root(&MOCK_SALT_ROOT).unwrap();
+ let mut mock_hal = TestingHal::new();
+ mock_hal.memory.set_salt_root(&MOCK_SALT_ROOT);
let data = hex!("001122334455667788");
let expected = hex!("62db8dcd47ddf8e81809c377ed96643855d3052bb73237100ca81f0f5a7611e6");
- let hash = hash_data(&data, "test purpose").unwrap();
+ let hash = hash_data(&mut mock_hal, &data, "test purpose").unwrap();
assert_eq!(hash.as_slice(), &expected);
}
#[test]
fn test_hash_data_empty_inputs() {
mock_memory();
- memory::set_salt_root(&MOCK_SALT_ROOT).unwrap();
+ let mut mock_hal = TestingHal::new();
+ mock_hal.memory.set_salt_root(&MOCK_SALT_ROOT);
let expected = hex!("2dbb05dd73d94edba6946611aaca367f76c809e96f20499ad674e596050f9833");
- let hash = hash_data(&[], "").unwrap();
+ let hash = hash_data(&mut mock_hal, &[], "").unwrap();
assert_eq!(hash.as_slice(), &expected);
}
Risk score
Our methodology →Why this scored 12/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.