← Watch feed
rust/keystore: wrap lock() in bitbox02-rust
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Marko Bencun
rust/keystore: wrap lock() in bitbox02-rust
Original is renamed so the compiler can error if we missed one.
By moving all keystore calls to bitbox02_rust::keystore, we can more
easily replace the function bodies with native Rust
implementations (without wrapping C).
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a routine internal code reorganization. It moves the keystore lock() function call from a low-level C wrapper into a higher-level Rust module and renames the original function so the compiler can catch any missed references. There is no change to what the function does or to any security behavior of the device.
Recommended action
No security action required. Review as normal refactoring if desired.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch wraps bitbox02::keystore::lock() as bitbox02_rust::keystore::lock() (which calls keystore::_lock()). The underlying unsafe C call bitbox02_sys::keystore_lock() and the ROOT_FINGERPRINT cache clear remain unchanged. All call sites in tests and production code are updated to use the new wrapper. A unit test for lock() is moved from the bitbox02 crate to the bitbox02-rust crate. This is purely refactoring to prepare for future native Rust implementations.
Changed components
src/rust/bitbox02-rust/src/keystore.rssrc/rust/bitbox02/src/keystore.rsInspect captured patch +70 / −66
diff --git a/src/rust/bitbox02-rust/src/hww.rs b/src/rust/bitbox02-rust/src/hww.rs
index 99c4056..1c49d63 100644
--- a/src/rust/bitbox02-rust/src/hww.rs
+++ b/src/rust/bitbox02-rust/src/hww.rs
@@ -343,7 +343,7 @@ mod tests {
// Can reboot when seeded and locked. This happens when the user sets a password and then
// reconnects the device.
- bitbox02::keystore::lock();
+ crate::keystore::lock();
let mut mock_hal = TestingHal::new();
let reboot_called = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
make_request(&mut mock_hal, reboot_request.encode_to_vec().as_ref()).unwrap();
@@ -372,7 +372,7 @@ mod tests {
let mut make_request = init_noise();
- bitbox02::keystore::lock();
+ crate::keystore::lock();
let mut mock_hal = TestingHal::new();
mock_hal.sd.inserted = Some(true);
mock_hal
@@ -434,7 +434,7 @@ mod tests {
};
// Can't reboot when initialized but locked.
- bitbox02::keystore::lock();
+ crate::keystore::lock();
let mut mock_hal = TestingHal::new();
let response_encoded =
make_request(&mut mock_hal, &reboot_request.encode_to_vec()).unwrap();
@@ -490,7 +490,7 @@ mod tests {
&b"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"[..],
&b"aaaaaaaaaaaaaaaa"[..],
] {
- bitbox02::keystore::lock();
+ crate::keystore::lock();
mock_memory();
bitbox02::memory::set_device_name("test device name").unwrap();
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/multisig.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/multisig.rs
index 499c1c1..03fb2de 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/multisig.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/multisig.rs
@@ -589,7 +589,7 @@ mod tests {
};
// Keystore locked.
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert!(validate(&multisig, keypath).is_err());
// Ok.
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
index 817fa4b..f74663a 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
@@ -1705,7 +1705,7 @@ mod tests {
{
// test keystore locked
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert_eq!(
block_on(process(&mut TestingHal::new(), &init_req_valid,)),
Err(Error::InvalidState)
diff --git a/src/rust/bitbox02-rust/src/hww/api/cardano/address.rs b/src/rust/bitbox02-rust/src/hww/api/cardano/address.rs
index 130af4e..b765944 100644
--- a/src/rust/bitbox02-rust/src/hww/api/cardano/address.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/cardano/address.rs
@@ -488,7 +488,7 @@ mod tests {
#[test]
fn test_pubkey_hash_at_keypath() {
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert!(
pubkey_hash_at_keypath(&[1852 + HARDENED, 1815 + HARDENED, HARDENED, 0, 0]).is_err()
);
@@ -515,7 +515,7 @@ mod tests {
);
// Keystore locked
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert_eq!(
do_pkh_skh(
&[1852 + HARDENED, 1815 + HARDENED, HARDENED, 0, 0],
diff --git a/src/rust/bitbox02-rust/src/hww/api/cardano/xpubs.rs b/src/rust/bitbox02-rust/src/hww/api/cardano/xpubs.rs
index 8601d8a..8bd114c 100644
--- a/src/rust/bitbox02-rust/src/hww/api/cardano/xpubs.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/cardano/xpubs.rs
@@ -47,7 +47,7 @@ mod tests {
#[test]
fn test_process() {
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert_eq!(
process(&pb::CardanoXpubsRequest { keypaths: vec![] }),
Ok(Response::Xpubs(pb::CardanoXpubsResponse { xpubs: vec![] })),
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/pubrequest.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/pubrequest.rs
index 53f827f..cc8cc01 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/pubrequest.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/pubrequest.rs
@@ -138,7 +138,7 @@ mod tests {
);
// xpub fetching/encoding failed.
- bitbox02::keystore::lock();
+ keystore::lock();
assert_eq!(
block_on(process(&mut TestingHal::new(), &request)),
Err(Error::InvalidInput)
@@ -231,7 +231,7 @@ mod tests {
);
// Keystore locked.
- bitbox02::keystore::lock();
+ keystore::lock();
assert_eq!(
block_on(process(
&mut TestingHal::new(),
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
index 9f0f64a..723aa39 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
@@ -1177,7 +1177,7 @@ mod tests {
{
// Keystore locked.
- keystore::lock();
+ crate::keystore::lock();
assert_eq!(
block_on(process(
&mut TestingHal::new(),
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
index 48ba348..2b6cd91 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/signmsg.rs
@@ -267,7 +267,7 @@ mod tests {
);
// Keystore locked.
- keystore::lock();
+ crate::keystore::lock();
assert_eq!(
block_on(process(
&mut TestingHal::new(),
diff --git a/src/rust/bitbox02-rust/src/hww/api/restore.rs b/src/rust/bitbox02-rust/src/hww/api/restore.rs
index 9362015..55924c7 100644
--- a/src/rust/bitbox02-rust/src/hww/api/restore.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/restore.rs
@@ -176,7 +176,7 @@ mod tests {
#[test]
fn test_from_mnemonic() {
mock_memory();
- keystore::lock();
+ crate::keystore::lock();
let mut counter = 0u32;
let mut mock_hal = TestingHal::new();
mock_hal.ui.set_enter_string(Box::new(|params| {
diff --git a/src/rust/bitbox02-rust/src/hww/api/rootfingerprint.rs b/src/rust/bitbox02-rust/src/hww/api/rootfingerprint.rs
index ea48e65..4c7f7b3 100644
--- a/src/rust/bitbox02-rust/src/hww/api/rootfingerprint.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/rootfingerprint.rs
@@ -33,12 +33,11 @@ pub fn process() -> Result<Response, Error> {
mod tests {
use super::*;
- use bitbox02::keystore::lock;
use bitbox02::testing::mock_unlocked_using_mnemonic;
#[test]
fn test_process() {
- lock();
+ keystore::lock();
assert_eq!(process(), Err(Error::Generic));
mock_unlocked_using_mnemonic(
diff --git a/src/rust/bitbox02-rust/src/hww/api/set_password.rs b/src/rust/bitbox02-rust/src/hww/api/set_password.rs
index 49465f4..36cd2aa 100644
--- a/src/rust/bitbox02-rust/src/hww/api/set_password.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/set_password.rs
@@ -57,7 +57,7 @@ mod tests {
#[test]
fn test_process() {
mock_memory();
- keystore::lock();
+ crate::keystore::lock();
let mut counter = 0u32;
let mut mock_hal = TestingHal::new();
mock_hal.ui.set_enter_string(Box::new(|params| {
@@ -91,7 +91,7 @@ mod tests {
#[test]
fn test_process_16_bytes() {
mock_memory();
- keystore::lock();
+ crate::keystore::lock();
let mut mock_hal = TestingHal::new();
mock_hal
.ui
@@ -113,7 +113,7 @@ mod tests {
#[test]
fn test_process_invalid_host_entropy() {
mock_memory();
- keystore::lock();
+ crate::keystore::lock();
let mut mock_hal = TestingHal::new();
mock_hal
.ui
@@ -134,7 +134,7 @@ mod tests {
#[test]
fn test_process_2nd_password_doesnt_match() {
mock_memory();
- keystore::lock();
+ crate::keystore::lock();
let mut counter = 0u32;
let mut mock_hal = TestingHal::new();
mock_hal.ui.set_enter_string(Box::new(|_params| {
diff --git a/src/rust/bitbox02-rust/src/keystore.rs b/src/rust/bitbox02-rust/src/keystore.rs
index 4b46901..34e257e 100644
--- a/src/rust/bitbox02-rust/src/keystore.rs
+++ b/src/rust/bitbox02-rust/src/keystore.rs
@@ -27,6 +27,11 @@ use crate::secp256k1::SECP256K1;
use bitcoin::hashes::{Hash, HashEngine, Hmac, HmacEngine, sha256, sha512};
+/// Locks the keystore (resets to state before `keystore::unlock()`).
+pub fn lock() {
+ keystore::_lock();
+}
+
/// Returns the keystore's seed encoded as a BIP-39 mnemonic.
pub fn get_bip39_mnemonic() -> Result<zeroize::Zeroizing<String>, ()> {
keystore::bip39_mnemonic_from_seed(&keystore::copy_seed()?)
@@ -332,9 +337,32 @@ mod tests {
use bitcoin::secp256k1;
+ #[test]
+ fn test_lock() {
+ lock();
+ assert!(keystore::is_locked());
+
+ let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
+ .unwrap();
+ assert!(keystore::encrypt_and_store_seed(&seed, "password").is_ok());
+ assert!(keystore::is_locked()); // still locked, it is only unlocked after unlock_bip39.
+ assert!(
+ block_on(keystore::unlock_bip39(
+ &secp256k1::Secp256k1::new(),
+ &seed,
+ "foo",
+ async || {}
+ ))
+ .is_ok()
+ );
+ assert!(!keystore::is_locked());
+ lock();
+ assert!(keystore::is_locked());
+ }
+
#[test]
fn test_secp256k1_get_private_key() {
- keystore::lock();
+ lock();
let keypath = &[84 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 0];
assert!(secp256k1_get_private_key(keypath).is_err());
@@ -353,7 +381,7 @@ mod tests {
#[test]
fn test_secp256k1_get_private_key_twice() {
- keystore::lock();
+ lock();
let keypath = &[84 + HARDENED, 0 + HARDENED, 0 + HARDENED, 0, 0];
assert!(secp256k1_get_private_key_twice(keypath).is_err());
@@ -372,7 +400,7 @@ mod tests {
#[test]
fn test_get_bip39_mnemonic() {
- keystore::lock();
+ lock();
assert!(get_bip39_mnemonic().is_err());
mock_unlocked();
@@ -386,7 +414,7 @@ mod tests {
// Also test with unhardened and non-zero elements.
let keypath_5 = &[44 + HARDENED, 1 + HARDENED, 10 + HARDENED, 1, 100];
- keystore::lock();
+ lock();
assert!(get_xpub_twice(keypath).is_err());
// 24 words
@@ -451,7 +479,7 @@ mod tests {
#[test]
fn test_get_xpubs_twice() {
- keystore::lock();
+ lock();
assert!(get_xpubs_twice(&[]).is_err());
mock_unlocked_using_mnemonic(
@@ -488,7 +516,7 @@ mod tests {
#[test]
fn test_root_fingerprint() {
- keystore::lock();
+ lock();
assert_eq!(root_fingerprint(), Err(()));
mock_unlocked_using_mnemonic(
@@ -507,7 +535,7 @@ mod tests {
);
assert_eq!(root_fingerprint(), Ok(vec![0xf4, 0x0b, 0x46, 0x9a]));
- keystore::lock();
+ lock();
assert_eq!(root_fingerprint(), Err(()));
}
@@ -575,7 +603,7 @@ mod tests {
#[test]
fn test_bip85_bip39() {
- keystore::lock();
+ lock();
assert!(bip85_bip39(12, 0).is_err());
// Test fixtures generated using:
@@ -621,7 +649,7 @@ mod tests {
#[test]
fn test_bip85_ln() {
- keystore::lock();
+ lock();
assert!(bip85_ln(0).is_err());
mock_unlocked_using_mnemonic(
@@ -698,7 +726,7 @@ mod tests {
for test in tests {
mock_memory();
- keystore::lock();
+ lock();
let seed = &seed[..test.seed_len];
assert!(
diff --git a/src/rust/bitbox02-rust/src/keystore/ed25519.rs b/src/rust/bitbox02-rust/src/keystore/ed25519.rs
index c61e39c..7cf23c3 100644
--- a/src/rust/bitbox02-rust/src/keystore/ed25519.rs
+++ b/src/rust/bitbox02-rust/src/keystore/ed25519.rs
@@ -150,7 +150,7 @@ mod tests {
#[test]
fn test_get_xpub() {
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert!(get_xpub(&[]).is_err());
mock_unlocked();
@@ -166,7 +166,7 @@ mod tests {
#[test]
fn test_get_xprv() {
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert!(get_xprv(&[]).is_err());
mock_unlocked();
@@ -180,7 +180,7 @@ mod tests {
#[test]
fn test_sign() {
let msg = &[0u8; 32];
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert!(sign(&[10 + HARDENED_OFFSET, 10], msg).is_err());
mock_unlocked();
diff --git a/src/rust/bitbox02-rust/src/workflow/unlock.rs b/src/rust/bitbox02-rust/src/workflow/unlock.rs
index ae5fa0b..ad346ca 100644
--- a/src/rust/bitbox02-rust/src/workflow/unlock.rs
+++ b/src/rust/bitbox02-rust/src/workflow/unlock.rs
@@ -207,7 +207,7 @@ mod tests {
bitbox02::memory::set_initialized().unwrap();
// Lock the keystore to simulate the normal locked state
- bitbox02::keystore::lock();
+ crate::keystore::lock();
let mut password_entered = false;
diff --git a/src/rust/bitbox02-rust/src/xpubcache.rs b/src/rust/bitbox02-rust/src/xpubcache.rs
index bac1987..93edfb0 100644
--- a/src/rust/bitbox02-rust/src/xpubcache.rs
+++ b/src/rust/bitbox02-rust/src/xpubcache.rs
@@ -250,7 +250,7 @@ mod tests {
);
// Make sure the following xpubs are derived using the cache only, not touching the seed.
- bitbox02::keystore::lock();
+ crate::keystore::lock();
assert_eq!(
&cache
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
index a0f5b23..107c9ab 100644
--- a/src/rust/bitbox02/src/keystore.rs
+++ b/src/rust/bitbox02/src/keystore.rs
@@ -96,7 +96,7 @@ pub fn unlock(password: &str) -> Result<zeroize::Zeroizing<Vec<u8>>, Error> {
}
}
-pub fn lock() {
+pub fn _lock() {
unsafe { bitbox02_sys::keystore_lock() }
unsafe { ROOT_FINGERPRINT.write(None) }
@@ -454,33 +454,10 @@ mod tests {
assert!(bip39_mnemonic_from_seed(b"foo").is_err());
}
- #[test]
- fn test_lock() {
- lock();
- assert!(is_locked());
-
- let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
- .unwrap();
- assert!(encrypt_and_store_seed(&seed, "password").is_ok());
- assert!(is_locked()); // still locked, it is only unlocked after unlock_bip39.
- assert!(
- block_on(unlock_bip39(
- &secp256k1::Secp256k1::new(),
- &seed,
- "foo",
- async || {}
- ))
- .is_ok()
- );
- assert!(!is_locked());
- lock();
- assert!(is_locked());
- }
-
#[test]
fn test_unlock() {
mock_memory();
- lock();
+ _lock();
assert!(matches!(unlock("password"), Err(Error::Unseeded)));
@@ -493,7 +470,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();
+ _lock();
// First call: unlock. The first one does a seed rentention (1 securechip event).
crate::securechip::fake_event_counter_reset();
@@ -618,7 +595,7 @@ mod tests {
#[test]
fn test_unlock_bip39() {
mock_memory();
- lock();
+ _lock();
let seed = hex::decode("1111111111111111222222222222222233333333333333334444444444444444")
.unwrap();
@@ -716,7 +693,7 @@ mod tests {
mock_memory();
crate::random::fake_reset();
crate::memory::set_salt_root(mock_salt_root.as_slice().try_into().unwrap()).unwrap();
- lock();
+ _lock();
assert!(create_and_store_seed("password", &host_entropy[..size]).is_ok());
assert_eq!(copy_seed().unwrap().as_slice(), &expected_seed[..size]);
@@ -742,7 +719,7 @@ mod tests {
#[test]
fn test_create_and_unlock_twice() {
mock_memory();
- lock();
+ _lock();
let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
.unwrap();
@@ -762,7 +739,7 @@ mod tests {
for seed_size in [16, 24, 32] {
mock_memory();
- lock();
+ _lock();
// Can repeat until initialized - initialized means backup has been created.
for _ in 0..2 {
@@ -771,7 +748,7 @@ mod tests {
// Also unlocks, so we can get the retained seed.
assert_eq!(copy_seed().unwrap().as_slice(), &seed[..seed_size]);
- lock();
+ _lock();
// Can't get seed before unlock.
assert!(copy_seed().is_err());
Risk score
Our methodology →Why this scored 15/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.