pin: create the pin private key on demand
What changed, and why it matters
This commit moves when a special device private key (used for PIN server authentication) is created. Previously it was generated at every boot if missing; now it is created only when actually needed during a PIN operation. This is a code-quality and reliability change that reduces unnecessary key creation at startup, but it does not appear to fix an active security vulnerability.
No immediate action required. Reviewers may want to confirm that lazy key creation does not introduce a race condition or allow the device to reach an inconsistent state if PIN operations are interrupted before the key is persisted.
Security signals we found
Deferred private key generation to first use
Removal of boot-time key initialization dependency
No change to key generation entropy source or storage protections
No explicit vulnerability fix described in commit message
Evidence from the diff
The change removes keychain_init_unit_key() from main.c boot and deletes it from keychain.c/h. It adds pin_get_unit_privatekey() in pinclient.c, which lazily fetches the existing key from storage or generates and persists a new one on first PIN-server interaction. The functional behavior of key generation/storage is preserved, only the timing is deferred. A comment about BLE initialization being delayed because the key was not initialized until first run of keychain_init() was also removed, suggesting the previous boot-time initialization may have been unnecessary or caused ordering issues.
Changed components
main/keychain.cmain/keychain.hmain/main.cmain/process/pinclient.cPIN server authentication key lifecycleInspect captured patch +21 / −29
diff --git a/main/keychain.c b/main/keychain.c
index aa8a97e..87e03c9 100644
--- a/main/keychain.c
+++ b/main/keychain.c
@@ -712,27 +712,4 @@ void keychain_init_cache(void)
// Cache the user key/passphrase preferences
key_flags = storage_get_key_flags();
}
-
-bool keychain_init_unit_key(void)
-{
- uint8_t privatekey[EC_PRIVATE_KEY_LEN];
- SENSITIVE_PUSH(privatekey, sizeof(privatekey));
-
- bool res = storage_get_pin_privatekey(privatekey, sizeof(privatekey));
- if (!res) {
- if (!keychain_get_new_privatekey(privatekey, sizeof(privatekey))) {
- JADE_LOGE("Failed to create new hw private key");
- SENSITIVE_POP(privatekey);
- return false;
- }
- res = storage_set_pin_privatekey(privatekey, sizeof(privatekey));
- if (res) {
- JADE_LOGI("Initialised new hw private key");
- } else {
- JADE_LOGE("Failed to set new hw private key");
- }
- }
- SENSITIVE_POP(privatekey);
- return res;
-}
#endif // AMALGAMATED_BUILD
diff --git a/main/keychain.h b/main/keychain.h
index ac7632b..34aeb54 100644
--- a/main/keychain.h
+++ b/main/keychain.h
@@ -25,7 +25,6 @@ typedef enum { PASSPHRASE_NEVER, PASSPHRASE_ONCE, PASSPHRASE_ALWAYS } passphrase
typedef enum { PASSPHRASE_WORDLIST, PASSPHRASE_FREETEXT } passphrase_type_t;
void keychain_init_cache(void);
-bool keychain_init_unit_key(void);
void keychain_set(const keychain_t* src, uint8_t userdata, bool temporary);
const struct ext_key* keychain_cached_service(const struct ext_key* service, bool subaccount_root);
void keychain_clear(void);
diff --git a/main/main.c b/main/main.c
index 38904b9..cc12bec 100644
--- a/main/main.c
+++ b/main/main.c
@@ -251,10 +251,6 @@ static void boot_process(void)
jade_wally_init();
wallet_init();
- if (!keychain_init_unit_key()) {
- JADE_ABORT();
- }
-
#ifdef CONFIG_BT_ENABLED
// Delay BLE initialisation as uses the hw unit key which is not initialised until
// the first run of keychain_init() (on a new or factory-reset unit).
diff --git a/main/process/pinclient.c b/main/process/pinclient.c
index b6f6813..91d5bc9 100644
--- a/main/process/pinclient.c
+++ b/main/process/pinclient.c
@@ -126,6 +126,26 @@ static void send_http_request_reply(jade_process_t* process, const char* documen
free(buf);
}
+/* Get/Create the devices unit private key */
+static bool pin_get_unit_privatekey(uint8_t* privatekey, const size_t key_len)
+{
+ bool res = storage_get_pin_privatekey(privatekey, key_len);
+ if (!res) {
+ // Unit key not found: create a new one on demand
+ if (!keychain_get_new_privatekey(privatekey, key_len)) {
+ JADE_LOGE("Failed to create new unit private key");
+ return false;
+ }
+ res = storage_set_pin_privatekey(privatekey, key_len);
+ if (res) {
+ JADE_LOGI("Initialised new unit private key");
+ } else {
+ JADE_LOGE("Failed to set new unit private key");
+ }
+ }
+ return res;
+}
+
// Hepler to tweak the server static key into a session key
static bool generate_ske(pin_keys_t* pinkeys)
{
@@ -457,7 +477,7 @@ static pinserver_result_t pinserver_interaction(jade_process_t* process, const u
size_t written = 0;
char data[2 * (sizeof(pinkeys.cke) + sizeof(pinkeys.replay_counter) + sizeof(payload))]; // sufficient
- if (!storage_get_pin_privatekey(pin_privatekey, sizeof(pin_privatekey))
+ if (!pin_get_unit_privatekey(pin_privatekey, sizeof(pin_privatekey))
|| !get_pin_secret(pin, pin_len, pin_privatekey, sizeof(pin_privatekey), pinsecret, sizeof(pinsecret))
|| !sign_payload(pin_privatekey, sizeof(pin_privatekey), &pinkeys, pinsecret, sizeof(pinsecret), entropy,
entropy_len, sig, sizeof(sig))
Why this scored 35/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.