keystore: fewer securechip calls when checking password
What the developer wrote
Authored by Marko Bencun
keystore: fewer securechip calls when checking password
The sanity check to see if the seed has changed does not need a
securechip operation, it can use the retained seed hash instead, same
as `unlock_bip39()`. This reduces the number of securechip operations
needed to do a password check, which reduces the risk of running into
the Optiga throttling security mechanism.
What changed, and why it matters
This commit is a hardening improvement, not a fix for an active vulnerability. It reduces how often the BitBox02 hardware wallet talks to its secure chip (the Optiga) when checking a password. Previously, an extra secure-chip call was made to fetch and compare the seed; now the device compares a locally retained hash instead. This lowers the chance of hitting the secure chip's throttling/lockout counter during normal use, which could otherwise make the device temporarily unusable or even require reset. The change does not introduce a known exploit path.
Treat as a defensive hardening commit. Review that _check_retained_seed() cannot be called before _retained_seed_hash is initialized, and that the hash is cleared on lock to prevent stale comparisons. No urgent patch or incident response is indicated, but firmware should include this change to improve availability and reduce secure-chip wear.
Security signals we found
Avoids unnecessary secure-chip (Optiga) operations during password verification
Reduces exposure to secure-chip throttling/lockout mechanism
Uses retained in-memory seed hash instead of re-deriving/copying seed for sanity check
Unit tests updated to reflect reduced secure-chip event count
No change to cryptographic primitives or trust boundaries
Evidence from the diff
The patch refactors keystore_unlock() and keystore_unlock_bip39_check() to use a new helper _check_retained_seed(), which compares a supplied seed against the in-memory retained seed hash (_retained_seed_hash) instead of calling keystore_copy_seed(). The latter required an additional secure chip (Optiga) operation. By avoiding that operation during password checks, the commit reduces the per-unlock secure-chip event count from 6 to 5 (and updates unit-test expectations accordingly). The retained seed hash is already computed and stored during the initial unlock/seed retention flow, so the comparison is functionally equivalent for the ‘seed changed under our feet’ sanity check. The change is defensive: it mitigates the risk of exhausting the Optiga’s authentication attempt counter, which can throttle or lock access.
Changed components
src/keystore.csrc/rust/bitbox02-rust/src/hww/api/show_mnemonic.rssrc/rust/bitbox02/src/keystore.rsBitBox02 secure chip (Optiga) interaction layerInspect captured patch +29 / −20
diff --git a/src/keystore.c b/src/keystore.c
index 465f414..1bbb9db 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -422,6 +422,23 @@ keystore_error_t keystore_create_and_store_seed(
return keystore_encrypt_and_store_seed(seed, host_entropy_size, password);
}
+// Checks if the retained seed matches the passed seed.
+static bool _check_retained_seed(const uint8_t* seed, size_t seed_length)
+{
+ if (!_is_unlocked_device) {
+ return false;
+ }
+ uint8_t seed_hashed[32] = {0};
+ UTIL_CLEANUP_32(seed_hashed);
+ if (_hash_seed(seed, seed_length, seed_hashed) != KEYSTORE_OK) {
+ return false;
+ }
+ if (!MEMEQ(seed_hashed, _retained_seed_hash, sizeof(_retained_seed_hash))) {
+ return false;
+ }
+ return true;
+}
+
keystore_error_t keystore_unlock(
const char* password,
uint8_t* remaining_attempts_out,
@@ -455,12 +472,7 @@ keystore_error_t keystore_unlock(
if (result == KEYSTORE_OK) {
if (_is_unlocked_device) {
// Already unlocked. Fail if the seed changed under our feet (should never happen).
- uint8_t current_seed[KEYSTORE_MAX_SEED_LENGTH] = {0};
- size_t current_seed_len = 0;
- if (!keystore_copy_seed(current_seed, ¤t_seed_len)) {
- return KEYSTORE_ERR_DECRYPT;
- }
- if (seed_len != current_seed_len || !MEMEQ(current_seed, seed, current_seed_len)) {
+ if (!_check_retained_seed(seed, seed_len)) {
Abort("Seed has suddenly changed. This should never happen.");
}
} else {
@@ -491,15 +503,9 @@ bool keystore_unlock_bip39_check(const uint8_t* seed, size_t seed_length)
return false;
}
- uint8_t seed_hashed[32] = {0};
- UTIL_CLEANUP_32(seed_hashed);
- if (_hash_seed(seed, seed_length, seed_hashed) != KEYSTORE_OK) {
+ if (!_check_retained_seed(seed, seed_length)) {
return false;
}
- if (!MEMEQ(seed_hashed, _retained_seed_hash, sizeof(_retained_seed_hash))) {
- return false;
- }
-
usb_processing_timeout_reset(LONG_TIMEOUT);
return true;
diff --git a/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs b/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs
index 2415618..b44e112 100644
--- a/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/show_mnemonic.rs
@@ -149,7 +149,7 @@ mod tests {
block_on(process(&mut mock_hal)),
Ok(Response::Success(pb::Success {}))
);
- assert_eq!(bitbox02::securechip::fake_event_counter(), 7);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 6);
assert_eq!(
mock_hal.ui.screens,
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
index e4193b1..e0bc5ea 100644
--- a/src/rust/bitbox02/src/keystore.rs
+++ b/src/rust/bitbox02/src/keystore.rs
@@ -466,15 +466,18 @@ mod tests {
assert!(encrypt_and_store_seed(&seed, "password").is_ok());
lock();
+ // 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!(crate::securechip::fake_event_counter(), 6);
+
// Loop to check that unlocking works while unlocked.
- for _ in 0..3 {
- // First call: unlock Further calls perform a password check. The first onedoes a seed
- // rentention (1 securechip event). The password check does not do the rentention but a
- // copy_seed() instead to check the seed, so they end up hacing the same number of
- // events.crate::securechip::fake_event_counter_reset();
+ 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!(crate::securechip::fake_event_counter(), 6);
+ assert_eq!(crate::securechip::fake_event_counter(), 5);
}
// Also check that the retained seed was encrypted with the expected encryption key.
Why this scored 25/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.