← Watch feed
reduce number of secure chip security events when creating/restoring
Public commit record
78/100 · Adequate
What the developer wrote
Authored by Marko Bencun
reduce number of secure chip security events when creating/restoring
Before, `keystore_encrypt_and_store_seed()` (called when
creating/restoring a seed) would always be followed by
`keystore_unlock(<password>)` with the password the user just chose,
so unlock could never fail. The unlocking part costs many secure chip
operations (for stretching the password).
By making the first function already unlock the keystore, we can avoid
calling `keystore_unlock()`, reducing the number of secure chip events
by 5.
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).
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit is a performance and reliability improvement, not a security vulnerability fix. It changes how a hardware wallet sets up a new wallet seed so that it performs fewer operations on the secure chip. Previously, after creating or restoring a seed, the software immediately unlocked the wallet with the same password, which wasted secure chip operations. Now the creation/restore step also unlocks the wallet, reducing secure chip events by 5. This helps avoid hitting the secure chip's throttling limit (133 events) when users repeatedly reset or restore their device.
Recommended action
No security action required. This is a defensive hardening/optimization change. Reviewers may want to verify that _retain_seed() properly copies the seed into retained memory and that _is_unlocked_device state is correctly managed, but the diff shows no security defect.
Security signals we found
01
Secure chip event counter reduction (5 fewer events per create/restore)
02
Removal of redundant keystore_unlock() calls after seed creation/restoration
03
Reference to Optiga throttling mechanism after 133 events
04
No change to encryption, key stretching, or authentication logic
Evidence from the diff
The patch refactors keystore_encrypt_and_store_seed() in src/keystore.c to call _retain_seed() and set _is_unlocked_device = true before returning. This means the keystore is already unlocked after seed creation/restoration, so callers no longer need to call keystore_unlock() with the same password. The Rust call sites in restore.rs and set_password.rs have their redundant unlock() calls removed. Tests are updated to reflect the reduced secure chip event count (from 19 to 14 in restore/set_password tests, and from 11 to 12 in one keystore test). The change is purely an optimization to reduce Optiga secure chip operations and avoid throttling; it does not alter cryptographic behavior or fix a vulnerability.
Changed components
src/keystore.csrc/keystore.hsrc/rust/bitbox02-rust/src/hww/api/restore.rssrc/rust/bitbox02-rust/src/hww/api/set_password.rssrc/rust/bitbox02-rust/src/keystore.rssrc/rust/bitbox02/src/keystore.rsInspect captured patch +20 / −30
diff --git a/src/keystore.c b/src/keystore.c
index 0336811..6333507 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -345,6 +345,13 @@ keystore_error_t keystore_encrypt_and_store_seed(
}
return KEYSTORE_ERR_MEMORY;
}
+
+ keystore_error_t retain_seed_result = _retain_seed(seed, seed_length);
+ if (retain_seed_result != KEYSTORE_OK) {
+ return retain_seed_result;
+ }
+ _is_unlocked_device = true;
+
return KEYSTORE_OK;
}
diff --git a/src/keystore.h b/src/keystore.h
index 963896a..68dcb86 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -63,7 +63,7 @@ USE_RESULT bool keystore_copy_seed(uint8_t* seed_out, size_t* length_out);
USE_RESULT bool keystore_copy_bip39_seed(uint8_t* bip32_seed_out);
/**
- * Restores a seed.
+ * Restores a seed. This also unlocks the keystore with this seed.
* @param[in] seed The seed that is to be restored.
* @param[in] seed_length The length of the seed (max. 32 bytes).
* @param[in] password The password with which we encrypt the seed.
@@ -75,6 +75,7 @@ keystore_encrypt_and_store_seed(const uint8_t* seed, size_t seed_length, const c
Generates the seed, mixes it with host_entropy, and stores it encrypted with the
password. The size of the host entropy determines the size of the seed. Can be either 16 or 32
bytes, resulting in 12 or 24 BIP39 recovery words.
+ This also unlocks the keystore with the new seed.
@param[in] host_entropy bytes of entropy to be mixed in.
@param[in] host_entropy_size must be 16 or 32.
*/
diff --git a/src/rust/bitbox02-rust/src/hww/api/restore.rs b/src/rust/bitbox02-rust/src/hww/api/restore.rs
index b5a55ee..e64b7dd 100644
--- a/src/rust/bitbox02-rust/src/hww/api/restore.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/restore.rs
@@ -17,7 +17,6 @@ use crate::pb;
use pb::response::Response;
-use crate::general::abort;
use crate::hal::Ui;
use crate::workflow::{confirm, mnemonic, password, unlock};
@@ -84,9 +83,6 @@ pub async fn from_file(
}
bitbox02::memory::set_initialized().or(Err(Error::Memory))?;
- if bitbox02::keystore::unlock(&password).is_err() {
- abort("restore_from_file: unlock failed");
- };
// Ignore non-critical error.
let _ = bitbox02::memory::set_device_name(&metadata.name);
@@ -160,10 +156,6 @@ pub async fn from_mnemonic(
}
bitbox02::memory::set_initialized().or(Err(Error::Memory))?;
- // This should never fail.
- if bitbox02::keystore::unlock(&password).is_err() {
- abort("restore_from_mnemonic: unlock failed");
- };
unlock::unlock_bip39(hal).await;
Ok(Response::Success(pb::Success {}))
@@ -207,7 +199,7 @@ mod tests {
)),
Ok(Response::Success(pb::Success {}))
);
- assert_eq!(bitbox02::securechip::fake_event_counter(), 19);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 14);
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 e81d2b9..5f67c2d 100644
--- a/src/rust/bitbox02-rust/src/hww/api/set_password.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/set_password.rs
@@ -40,9 +40,6 @@ pub async fn process(
hal.ui().status(&format!("Error\n{:?}", err), false).await;
return Err(Error::Generic);
}
- if keystore::unlock(&password).is_err() {
- panic!("Unexpected error during restore: unlock failed.");
- }
unlock::unlock_bip39(hal).await;
Ok(Response::Success(pb::Success {}))
}
@@ -83,7 +80,7 @@ mod tests {
)),
Ok(Response::Success(pb::Success {}))
);
- assert_eq!(bitbox02::securechip::fake_event_counter(), 19);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 14);
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 bab6976..fa2b094 100644
--- a/src/rust/bitbox02-rust/src/keystore.rs
+++ b/src/rust/bitbox02-rust/src/keystore.rs
@@ -577,14 +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(), 11);
-
- assert!(keystore::unlock_bip39(test.mnemonic_passphrase).is_err());
- assert!(keystore::is_locked());
-
- bitbox02::securechip::fake_event_counter_reset();
- assert!(keystore::unlock("foo").is_ok());
- assert_eq!(bitbox02::securechip::fake_event_counter(), 6);
+ assert_eq!(bitbox02::securechip::fake_event_counter(), 12);
assert!(keystore::is_locked());
diff --git a/src/rust/bitbox02/src/keystore.rs b/src/rust/bitbox02/src/keystore.rs
index 8f88265..8ab249b 100644
--- a/src/rust/bitbox02/src/keystore.rs
+++ b/src/rust/bitbox02/src/keystore.rs
@@ -408,7 +408,6 @@ mod tests {
let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
.unwrap();
assert!(encrypt_and_store_seed(&seed, "password").is_ok());
- assert!(unlock("password").is_ok());
assert!(is_locked()); // still locked, it is only unlocked after unlock_bip39.
assert!(unlock_bip39("foo").is_ok());
assert!(!is_locked());
@@ -432,6 +431,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();
// Loop to check that unlocking works while unlocked.
for _ in 0..3 {
@@ -497,7 +497,6 @@ mod tests {
assert!(root_fingerprint().is_err());
assert!(encrypt_and_store_seed(&seed, "password").is_ok());
- assert!(unlock("password").is_ok());
assert!(root_fingerprint().is_err());
assert!(unlock_bip39("foo").is_ok());
assert_eq!(root_fingerprint(), Ok(vec![0xf1, 0xbc, 0x3c, 0x46]),);
@@ -572,7 +571,6 @@ mod tests {
lock();
assert!(create_and_store_seed("password", &host_entropy[..size]).is_ok());
- assert!(unlock("password").is_ok());
assert_eq!(copy_seed().unwrap().as_slice(), &expected_seed[..size]);
// Check the seed has been stored encrypted with the expected encryption key.
// Decrypt and check seed.
@@ -603,14 +601,12 @@ mod tests {
let seed2 = hex::decode("c28135734876aff9ccf4f1d60df8d19a0a38fd02085883f65fc608eb769a635d")
.unwrap();
assert!(encrypt_and_store_seed(&seed, "password").is_ok());
- assert!(unlock("password").is_ok());
// Create new (different) seed.
assert!(encrypt_and_store_seed(&seed2, "password").is_ok());
- assert!(unlock("password").is_ok());
assert_eq!(copy_seed().unwrap().as_slice(), &seed2);
}
- // Functional test to store seeds, unlock, retrieve seed.
+ // Functional test to store seeds, lock/unlock, retrieve seed.
#[test]
fn test_seeds() {
let seed = hex::decode("cb33c20cea62a5c277527e2002da82e6e2b37450a755143a540a54cea8da9044")
@@ -624,6 +620,12 @@ mod tests {
for _ in 0..2 {
assert!(encrypt_and_store_seed(&seed[..seed_size], "foo").is_ok());
}
+ // Also unlocks, so we can get the retained seed.
+ assert_eq!(copy_seed().unwrap().as_slice(), &seed[..seed_size]);
+
+ lock();
+ // Can't get seed before unlock.
+ assert!(copy_seed().is_err());
// Wrong password.
assert!(matches!(
@@ -633,8 +635,6 @@ mod tests {
})
));
- // Can't get seed before unlock.
- assert!(copy_seed().is_err());
// Correct password. First time: unlock. After unlock, it becomes a password check.
for _ in 0..3 {
assert!(unlock("foo").is_ok());
Risk score
Our methodology →Why this scored 30/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.