keystore: more consistent treatment of keystore_unlock errors
What the developer wrote
Authored by Marko Bencun
keystore: more consistent treatment of keystore_unlock errors
Before, the device would be reset right after the 10th wrong
password. However, if there was any other error during unlock (e.g. a
securechip error) that is not caused by a wrong password, the device
would not be reset immediately, but at the start of the next unlock
attempt.
This commit unifies all error treatment, leading to a device reset
right away.
The alternative is to decrement the counter on any error other than
incorrect password, but that would drastically change the behavior of
the unlock function, and we'd rather be conservative here.
What changed, and why it matters
This firmware update fixes a timing inconsistency in how the BitBox02 hardware wallet handles device-unlock failures. Previously, only a wrong password triggered an immediate device reset after 10 failed attempts, while other unlock errors (such as a secure-chip malfunction) delayed the reset until the next unlock attempt. Now any serious unlock error causes the reset right away, making behavior more predictable and preventing a window where a failing secure chip could leave the device in an ambiguous state.
Treat as a hardening/defensive fix and include in the next firmware release. Review whether any callers rely on the previous delayed-reset behavior, and verify that reset_reset(false) is safe to invoke from every error path covered by the new branch. Regression-test secure-chip error injection to confirm the device resets immediately and does not allow further unlock attempts.
Security signals we found
Inconsistent error handling between wrong-password and other unlock failures
Delayed device reset on secure-chip or decryption errors
Potential state ambiguity after non-password unlock errors
Refactor centralizes unlock-failure handling and immediate reset behavior
Evidence from the diff
The commit refactors keystore_unlock() in src/keystore.c so that the remaining-attempts check and reset_reset(false) call occur on any non-OK result from _get_and_decrypt_seed(), not only on KEYSTORE_ERR_INCORRECT_PASSWORD. Previously the code returned early for errors other than incorrect password, skipping the immediate max-attempts/reset logic; that logic only ran after a subsequent unlock attempt. The patch moves failed_attempts lookup, the MAX_UNLOCK_ATTEMPTS comparison, and the reset into the error path, and simplifies the success path to reset attempts and return KEYSTORE_OK. Header comments are updated to note that remaining_attempts_out is populated on any error.
Changed components
src/keystore.csrc/keystore.hBitBox02 firmware keystore unlock flowInspect captured patch +26 / −27
diff --git a/src/keystore.c b/src/keystore.c
index 37cc627..401942f 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -131,39 +131,38 @@ keystore_error_t keystore_unlock(
size_t seed_len;
keystore_error_t result =
_get_and_decrypt_seed(password, seed, &seed_len, securechip_result_out);
- if (result != KEYSTORE_OK && result != KEYSTORE_ERR_INCORRECT_PASSWORD) {
+ if (result != KEYSTORE_OK) {
+ // Compute remaining attempts
+ failed_attempts = bitbox02_smarteeprom_get_unlock_attempts();
+
+ if (failed_attempts >= MAX_UNLOCK_ATTEMPTS) {
+ *remaining_attempts_out = 0;
+ reset_reset(false);
+ return KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED;
+ }
+
+ *remaining_attempts_out = MAX_UNLOCK_ATTEMPTS - failed_attempts;
return result;
}
- if (result == KEYSTORE_OK) {
- if (rust_keystore_is_unlocked_device()) {
- // Already unlocked. Fail if the seed changed under our feet (should never happen).
- if (!rust_keystore_check_retained_seed(rust_util_bytes(seed, seed_len))) {
- Abort("Seed has suddenly changed. This should never happen.");
- }
- } else {
- keystore_error_t retain_seed_result = _retain_seed(seed, seed_len);
- if (retain_seed_result != KEYSTORE_OK) {
- return retain_seed_result;
- }
- }
- bitbox02_smarteeprom_reset_unlock_attempts();
- if (seed_out != NULL && seed_len_out != NULL) {
- memcpy(seed_out, seed, seed_len);
- *seed_len_out = seed_len;
+ if (rust_keystore_is_unlocked_device()) {
+ // Already unlocked. Fail if the seed changed under our feet (should never happen).
+ if (!rust_keystore_check_retained_seed(rust_util_bytes(seed, seed_len))) {
+ Abort("Seed has suddenly changed. This should never happen.");
+ }
+ } else {
+ keystore_error_t retain_seed_result = _retain_seed(seed, seed_len);
+ if (retain_seed_result != KEYSTORE_OK) {
+ return retain_seed_result;
}
}
- // Compute remaining attempts
- failed_attempts = bitbox02_smarteeprom_get_unlock_attempts();
+ bitbox02_smarteeprom_reset_unlock_attempts();
- if (failed_attempts >= MAX_UNLOCK_ATTEMPTS) {
- *remaining_attempts_out = 0;
- reset_reset(false);
- return KEYSTORE_ERR_MAX_ATTEMPTS_EXCEEDED;
+ if (seed_out != NULL && seed_len_out != NULL) {
+ memcpy(seed_out, seed, seed_len);
+ *seed_len_out = seed_len;
}
-
- *remaining_attempts_out = MAX_UNLOCK_ATTEMPTS - failed_attempts;
- return result;
+ return KEYSTORE_OK;
}
bool keystore_get_bip39_word_stack(uint16_t idx, char* word_out, size_t word_out_size)
diff --git a/src/keystore.h b/src/keystore.h
index 96b7598..8a5367f 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -58,7 +58,7 @@ typedef enum {
* check the password).
* @param[in] password keystore password, used to decrypt the seed.
* If it is false, the keystore is not unlocked.
- * @param[out] remaining_attempts_out will have the number of remaining attempts.
+ * @param[out] On any error, remaining_attempts_out will have the number of remaining attempts.
* If zero, the keystore is locked until the device is reset.
* @param[out] securechip_result_out, if not NULL, will contain the error code from
* @param[out] seed_out The seed bytes copied from the retained seed.
Why this scored 59/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.