keystore: split keystore_unlock_bip39() into multiple functions
What changed, and why it matters
This commit is a simple internal code reorganization. It splits one existing function, keystore_unlock_bip39(), into three smaller functions: a check, a finalize step, and the original wrapper that calls both. The actual behavior and security checks appear unchanged. There is no indication this fixes or introduces a security issue.
No security action required; treat as routine refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors keystore_unlock_bip39() in src/keystore.c into keystore_unlock_bip39_check(), keystore_unlock_bip39_finalize(), and a thin keystore_unlock_bip39() that calls both. The check still verifies _is_unlocked_device and hashes/compares the seed before resetting the USB timeout. The finalize step still retains the BIP39 seed and sets _is_unlocked_bip39. The public header exposes the two new helpers. No logic changes are visible in the diff.
Changed components
src/keystore.csrc/keystore.hInspect captured patch +41 / −11
diff --git a/src/keystore.c b/src/keystore.c
index 3162983..5350e75 100644
--- a/src/keystore.c
+++ b/src/keystore.c
@@ -485,16 +485,11 @@ keystore_error_t keystore_unlock(
return result;
}
-bool keystore_unlock_bip39(
- const uint8_t* seed,
- size_t seed_length,
- const char* mnemonic_passphrase,
- uint8_t* root_fingerprint_out)
+bool keystore_unlock_bip39_check(const uint8_t* seed, size_t seed_length)
{
if (!_is_unlocked_device) {
return false;
}
- usb_processing_timeout_reset(LONG_TIMEOUT);
uint8_t seed_hashed[32] = {0};
UTIL_CLEANUP_32(seed_hashed);
@@ -505,6 +500,30 @@ bool keystore_unlock_bip39(
return false;
}
+ usb_processing_timeout_reset(LONG_TIMEOUT);
+
+ return true;
+}
+
+bool keystore_unlock_bip39_finalize(const uint8_t* bip39_seed)
+{
+ if (!_retain_bip39_seed(bip39_seed)) {
+ return false;
+ }
+ _is_unlocked_bip39 = true;
+ return true;
+}
+
+bool keystore_unlock_bip39(
+ const uint8_t* seed,
+ size_t seed_length,
+ const char* mnemonic_passphrase,
+ uint8_t* root_fingerprint_out)
+{
+ if (!keystore_unlock_bip39_check(seed, seed_length)) {
+ return false;
+ }
+
uint8_t bip39_seed[64] = {0};
UTIL_CLEANUP_64(bip39_seed);
rust_derive_bip39_seed(
@@ -513,11 +532,7 @@ bool keystore_unlock_bip39(
rust_util_bytes_mut(bip39_seed, sizeof(bip39_seed)),
rust_util_bytes_mut(root_fingerprint_out, 4));
- if (!_retain_bip39_seed(bip39_seed)) {
- return false;
- }
- _is_unlocked_bip39 = true;
- return true;
+ return keystore_unlock_bip39_finalize(bip39_seed);
}
void keystore_lock(void)
diff --git a/src/keystore.h b/src/keystore.h
index 848ea83..1a5ecfe 100644
--- a/src/keystore.h
+++ b/src/keystore.h
@@ -104,6 +104,15 @@ USE_RESULT keystore_error_t keystore_create_and_store_seed(
USE_RESULT keystore_error_t
keystore_unlock(const char* password, uint8_t* remaining_attempts_out, int* securechip_result_out);
+/**
+ * Checks if bip39 unlocking can be performed. It can be performed if `keystore_unlock()`
+ * successfully and the input seed matches the keystore seed (i.e. must match the output
+ * of `keystore_copy_seed()`).
+ * @param[in] seed the input seed to BIP39.
+ * @param[in] seed_length the size of the seed
+ */
+USE_RESULT bool keystore_unlock_bip39_check(const uint8_t* seed, size_t seed_length);
+
/** Unlocks the bip39 seed. The input seed must be the keystore seed (i.e. must match the output
* of `keystore_copy_seed()`).
* @param[in] seed the input seed to BIP39.
@@ -120,6 +129,12 @@ USE_RESULT bool keystore_unlock_bip39(
const char* mnemonic_passphrase,
uint8_t* root_fingerprint_out);
+/**
+ * Retains the given bip39 seed and marks the keystore as unlocked.
+ * @param[in] bip39_seed 64 byte bip39 seed.
+ */
+USE_RESULT bool keystore_unlock_bip39_finalize(const uint8_t* bip39_seed);
+
/**
* Locks the keystore (resets to state before `keystore_unlock()`).
*/
Why this scored 15/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.