keystore: reduce SC operations in the seed check
What the developer wrote
Authored by Marko Bencun
keystore: reduce SC operations in the seed check
verify_seed() after storing a seed currently re-stretches the
password, which needs 5 securechip events.
We can skip that because we still have access to the stretched
password.
If the check succeeds, but there was an error involving the password
or secure chip (e.g. the stretching result was corrupted the first
time), the consequence is that the user won't be able to unlock, but
the seed itself on the MCU, on the backup and retained (encrypted) in
seed is unaffected. That trade-off is good, because a failure
there (securechip returns no error but actually had a corruption)
should be exceedingly rare.
This effort is part of mitigating Optiga's throttling mechanism that
kicks in after 133 events - users can run into this by repeatedly
resetting/restoring).
What changed, and why it matters
This commit changes how the BitBox02 hardware wallet verifies a newly stored seed. Instead of re-stretching the user's password through the secure chip (which consumes several secure-chip operations), it reuses the already-stretched encryption key. This is a performance/reliability improvement to avoid hitting a secure-chip throttling limit after many device resets or restores. The commit itself is a defensive hardening change, not an obvious vulnerability fix, but it slightly changes the security assumptions around seed verification.
Review the new _verify_seed() implementation for correct key lifecycle handling, ensure the encryption_key buffer is zeroized after use, and confirm that callers cannot pass a stale or incorrect key. Verify that the reduced secure-chip event count does not weaken anti-brute-force or tamper-evident properties. Consider whether an independent security review is warranted because the change touches seed storage verification.
Security signals we found
Reduces secure-chip (Optiga) operations to mitigate throttling after 133 events
Reuses stretched password/encryption key instead of re-deriving it during seed verification
Changes seed verification from password-based to key-based decryption
Adds explicit length underflow check (encrypted_len < 49) before buffer arithmetic
Clears decrypted buffer with util_zero on all exit paths
Commit message acknowledges a rare failure mode where a corrupted stretch would prevent unlock but not harm seed integrity
Evidence from the diff
The patch refactors _verify_seed() in src/keystore.c to accept a pre-derived encryption key rather than a password. It reads the encrypted seed and HMAC from memory and decrypts it with the supplied key, comparing the result to the expected seed. Previously, verify_seed() called _get_and_decrypt_seed(), which re-derived the key via the secure chip (5 additional securechip events). The Rust test expectations are updated from 12-14 events down to 7-9 events. The commit message explicitly notes a trade-off: if the stretched password or secure chip silently corrupted the first stretch, the user would later be unable to unlock, but the stored seed, backup, and retained encrypted seed are unaffected.
Changed components
src/keystore.csrc/rust/bitbox02-rust/src/hww/api/restore.rssrc/rust/bitbox02-rust/src/hww/api/set_password.rssrc/rust/bitbox02-rust/src/keystore.rsInspect captured patch +24 / −11
diff --git a/src/keystore.c b/src/keystore.c
index 9d38bf4..3162983 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -208,22 +208,35 @@ static keystore_error_t _get_and_decrypt_seed(
}
static bool _verify_seed(
- const char* password,
+ const uint8_t* encryption_key,
const uint8_t* expected_seed,
size_t expected_seed_len)
{
- uint8_t decrypted_seed[KEYSTORE_MAX_SEED_LENGTH] = {0};
- size_t seed_len;
- UTIL_CLEANUP_32(decrypted_seed);
- if (_get_and_decrypt_seed(password, decrypted_seed, &seed_len, NULL) != KEYSTORE_OK) {
+ uint8_t encrypted_seed_and_hmac[96];
+ UTIL_CLEANUP_32(encrypted_seed_and_hmac);
+ uint8_t encrypted_len;
+ if (!memory_get_encrypted_seed_and_hmac(encrypted_seed_and_hmac, &encrypted_len)) {
return false;
}
- if (expected_seed_len != seed_len) {
+ if (encrypted_len < 49) {
+ Abort("_verify_seed: underflow / zero size");
+ }
+ size_t decrypted_len = encrypted_len - 48;
+ uint8_t decrypted[decrypted_len];
+ bool password_correct = cipher_aes_hmac_decrypt(
+ encrypted_seed_and_hmac, encrypted_len, decrypted, &decrypted_len, encryption_key);
+ if (!password_correct) {
return false;
}
- if (!MEMEQ(expected_seed, decrypted_seed, seed_len)) {
+ if (expected_seed_len != decrypted_len) {
+ util_zero(decrypted, sizeof(decrypted));
+ return false;
+ }
+ if (!MEMEQ(expected_seed, decrypted, expected_seed_len)) {
+ util_zero(decrypted, sizeof(decrypted));
return false;
}
+ util_zero(decrypted, sizeof(decrypted));
return true;
}
@@ -356,7 +369,7 @@ keystore_error_t keystore_encrypt_and_store_seed(
if (!memory_set_encrypted_seed_and_hmac(encrypted_seed, encrypted_seed_len_u8)) {
return KEYSTORE_ERR_MEMORY;
}
- if (!_verify_seed(password, seed, seed_length)) {
+ if (!_verify_seed(secret, seed, seed_length)) {
if (!memory_reset_hww()) {
return KEYSTORE_ERR_MEMORY;
}
diff --git a/src/rust/bitbox02-rust/src/hww/api/restore.rs b/src/rust/bitbox02-rust/src/hww/api/restore.rs
index bc8d180..83bf95d 100644
--- a/src/rust/bitbox02-rust/src/hww/api/restore.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/restore.rs
@@ -200,7 +200,7 @@ mod tests {
)),
Ok(Response::Success(pb::Success {}))
);
- assert_eq!(bitbox02::securechip::fake_event_counter(), 13);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 8);
drop(mock_hal); // to remove mutable borrow of counter
assert_eq!(counter, 2);
assert!(!keystore::is_locked());
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 c36ac3a..b040d7d 100644
--- a/src/rust/bitbox02-rust/src/hww/api/set_password.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/set_password.rs
@@ -80,7 +80,7 @@ mod tests {
)),
Ok(Response::Success(pb::Success {}))
);
- assert_eq!(bitbox02::securechip::fake_event_counter(), 14);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 9);
drop(mock_hal); // to remove mutable borrow of counter
assert_eq!(counter, 2);
assert!(!keystore::is_locked());
diff --git a/src/rust/bitbox02-rust/src/keystore.rs b/src/rust/bitbox02-rust/src/keystore.rs
index 2e74b80..169d385 100644
--- a/src/rust/bitbox02-rust/src/keystore.rs
+++ b/src/rust/bitbox02-rust/src/keystore.rs
@@ -577,7 +577,7 @@ mod tests {
bitbox02::securechip::fake_event_counter_reset();
assert!(keystore::encrypt_and_store_seed(seed, "foo").is_ok());
- assert_eq!(bitbox02::securechip::fake_event_counter(), 12);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 7);
assert!(keystore::is_locked());
Why this scored 29/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.