pinserver: retain unit private key on url change
What the developer wrote
Authored by Mike Tolkachev
pinserver: retain unit private key on url change
What changed, and why it matters
This commit changes how Blockstream Jade handles its PIN server settings. Previously, changing the PIN server web address (URL) also wiped the device's private PIN encryption key, forcing the device to create a fresh one. The patch keeps the existing private key when only the URL changes, while still erasing it when the PIN server's public key changes. This avoids unnecessary key rotation and reduces the risk that a legitimate URL-only update silently breaks PIN security by generating a new key.
Review the new `pubkey_changed` logic to confirm it cannot be bypassed by an attacker-controlled parameter (e.g., supplying a pubkey that compares equal to the stored or default pubkey). Verify that `storage_set_pinserver_details` and `storage_erase_pinserver_details` callers always pass an explicit action. Consider whether retaining the PIN private key across URL changes could allow a malicious URL to reuse an old key; ensure URL/certificate validation and user confirmation still prevent connecting to an attacker-controlled pinserver.
Security signals we found
PIN private key lifecycle change: key retained on URL-only pinserver update
Public-key change still triggers private-key erasure/regeneration
Debug-clean and full reset still erase private key
Logic moved outside CONFIG_DEBUG_MODE guard so URL-vs-pubkey decision applies uniformly
No explicit CVE, advisory, or security disclosure referenced in commit
Evidence from the diff
The patch introduces a storage_pin_privkey_action_t enum (STORAGE_PIN_KEEP_PRIVKEY / STORAGE_PIN_ERASE_PRIVKEY) and threads it through storage_set_pinserver_details() and storage_erase_pinserver_details(). In update_pinserver(), it now computes whether the public key is actually changing. If only the URL/certificate is updated, the stored PIN private key is retained; if the pubkey changes or details are reset, the private key is erased so it will be regenerated on next use. The debug-clean path and full reset path still erase the private key. The previous logic only allowed URL-only updates in non-debug builds but unconditionally erased the private key on any stored update, which could rotate the PIN encryption key unexpectedly.
Changed components
main/process/update_pinserver.cmain/process/debug_clean.cmain/storage.cmain/storage.hJade PIN server configuration and PIN private key storageInspect captured patch +38 / −32
### main/process/debug_clean.c
@@ -29,7 +29,7 @@ void debug_clean_reset_process(void* process_ptr)
// Clean pinserver overrides from storage
storage_erase_pinserver_cert();
- storage_erase_pinserver_details();
+ storage_erase_pinserver_details(STORAGE_PIN_ERASE_PRIVKEY);
// Clean multisig registrations from storage
char multisig_names[MAX_MULTISIG_REGISTRATIONS][NVS_KEY_NAME_MAX_SIZE]; // Sufficient
### main/process/update_pinserver.c
@@ -122,30 +122,26 @@ int update_pinserver(const CborValue* const params, const char** errmsg)
}
}
-#ifndef CONFIG_DEBUG_MODE
- if (keychain_has_pin()) {
- // Check that we are not trying to update the pinserver pubkey on a Jade unit
- // that already has a wallet set up/persisted in flash.
- // NOTE: we do allow an update of just the url/certs, as this may be a url change
- // that still connects to the same backend pinserver instance.
- uint8_t user_pubkey[EC_PUBLIC_KEY_LEN];
- const bool have_user_pubkey = storage_get_pinserver_pubkey(user_pubkey, sizeof(user_pubkey));
-
- // Cannot reset a non-default pubkey to the default value
- if (reset_details && have_user_pubkey && memcmp(server_public_key_start, user_pubkey, sizeof(user_pubkey))) {
- *errmsg = "Cannot update initialized unit";
- goto cleanup;
- }
+ uint8_t stored_pubkey[EC_PUBLIC_KEY_LEN];
+ const bool have_stored_pubkey = storage_get_pinserver_pubkey(stored_pubkey, sizeof(stored_pubkey));
+ const uint8_t* const old_pubkey = have_stored_pubkey ? stored_pubkey : server_public_key_start;
+ const uint8_t* const new_pubkey = pubkey ? pubkey : (reset_details ? server_public_key_start : old_pubkey);
+ const bool pubkey_changed = memcmp(old_pubkey, new_pubkey, EC_PUBLIC_KEY_LEN) != 0;
- // Cannot set new pubkey unless effectively unchanged
- const uint8_t* effective_pubkey = have_user_pubkey ? user_pubkey : server_public_key_start;
- if (pubkey && memcmp(effective_pubkey, pubkey, pubkey_len)) {
- *errmsg = "Cannot update initialized unit";
- goto cleanup;
- }
+#ifndef CONFIG_DEBUG_MODE
+ // Check that we are not trying to update the pinserver pubkey on a Jade unit
+ // that already has a wallet set up/persisted in flash.
+ // NOTE: we do allow an update of just the url/certs, as this may be a url change
+ // that still connects to the same backend pinserver instance.
+ if (keychain_has_pin() && pubkey_changed) {
+ *errmsg = "Cannot update initialized unit";
+ goto cleanup;
}
#endif // CONFIG_DEBUG_MODE
+ const storage_pin_privkey_action_t privkey_action
+ = pubkey_changed ? STORAGE_PIN_ERASE_PRIVKEY : STORAGE_PIN_KEEP_PRIVKEY;
+
if (urlA_len) {
char* pubkey_hex = NULL;
if (pubkey && pubkey_len > 0) {
@@ -221,15 +217,15 @@ int update_pinserver(const CborValue* const params, const char** errmsg)
// Ok, now user confirmed actions, actually set the pinserver details in storage
if (urlA_len) {
JADE_LOGI("Setting user pinserver details");
- if (!storage_set_pinserver_details(urlA, urlB, pubkey, pubkey_len)) {
+ if (!storage_set_pinserver_details(urlA, urlB, pubkey, pubkey_len, privkey_action)) {
JADE_LOGE("Failed to persist pinserver details");
*errmsg = "Failed to persist Oracle details";
retval = CBOR_RPC_INTERNAL_ERROR;
goto cleanup;
}
} else if (reset_details) {
JADE_LOGI("Erasing user pinserver details - resetting to default");
- if (!storage_erase_pinserver_details()) {
+ if (!storage_erase_pinserver_details(privkey_action)) {
JADE_LOGE("Failed to erase pinserver details");
*errmsg = "Failed to erase Oracle details";
retval = CBOR_RPC_INTERNAL_ERROR;
@@ -269,7 +265,7 @@ bool reset_pinserver(void)
JADE_LOGI("Erasing user pinserver details and certificate - resetting to default");
bool retval = true;
- if (!storage_erase_pinserver_details()) {
+ if (!storage_erase_pinserver_details(STORAGE_PIN_ERASE_PRIVKEY)) {
JADE_LOGE("Failed to erase pinserver details");
retval = false;
}
### main/storage.c
@@ -550,17 +550,20 @@ bool storage_get_replay_counter(uint32_t* replay_counter)
return true;
}
-bool storage_set_pinserver_details(const char* urlA, const char* urlB, const uint8_t* pubkey, const size_t pubkey_len)
+bool storage_set_pinserver_details(const char* urlA, const char* urlB, const uint8_t* pubkey, const size_t pubkey_len,
+ const storage_pin_privkey_action_t privkey_action)
{
- JADE_ASSERT(urlA);
- JADE_ASSERT(urlB);
+ JADE_ASSERT(urlA && urlB);
+ JADE_ASSERT(privkey_action == STORAGE_PIN_KEEP_PRIVKEY || privkey_action == STORAGE_PIN_ERASE_PRIVKEY);
// Commit all values, or none
nvs_handle handle;
STORAGE_OPEN(handle, DEFAULT_NAMESPACE, NVS_READWRITE);
STORAGE_SET_STRING(handle, USER_PINSERVER_URL_A, urlA);
STORAGE_SET_STRING(handle, USER_PINSERVER_URL_B, urlB);
- STORAGE_ERASE(handle, PIN_PRIVATEKEY_FIELD); // Re-create on first use later
+ if (privkey_action == STORAGE_PIN_ERASE_PRIVKEY) {
+ STORAGE_ERASE(handle, PIN_PRIVATEKEY_FIELD); // Re-create on first use later
+ }
// Pubkey is optional (as just server public address may change)
if (pubkey && pubkey_len > 0) {
@@ -586,15 +589,19 @@ bool storage_get_pinserver_pubkey(uint8_t* pubkey, const size_t pubkey_len)
return read_blob_fixed(DEFAULT_NAMESPACE, USER_PINSERVER_PUBKEY, pubkey, pubkey_len);
}
-bool storage_erase_pinserver_details(void)
+bool storage_erase_pinserver_details(const storage_pin_privkey_action_t privkey_action)
{
+ JADE_ASSERT(privkey_action == STORAGE_PIN_KEEP_PRIVKEY || privkey_action == STORAGE_PIN_ERASE_PRIVKEY);
+
// Erase all of the pinserver fields, or none of them
nvs_handle handle;
STORAGE_OPEN(handle, DEFAULT_NAMESPACE, NVS_READWRITE);
STORAGE_ERASE(handle, USER_PINSERVER_URL_A);
STORAGE_ERASE(handle, USER_PINSERVER_URL_B);
STORAGE_ERASE(handle, USER_PINSERVER_PUBKEY);
- STORAGE_ERASE(handle, PIN_PRIVATEKEY_FIELD); // Re-create on first use later
+ if (privkey_action == STORAGE_PIN_ERASE_PRIVKEY) {
+ STORAGE_ERASE(handle, PIN_PRIVATEKEY_FIELD); // Re-create on first use later
+ }
STORAGE_COMMIT(handle);
STORAGE_CLOSE(handle);
return true;
### main/storage.h
@@ -36,6 +36,8 @@
#define MAX_PINSVR_CERTIFICATE_LENGTH 2048
#define MAX_PINSVR_URL_LENGTH 120
+typedef enum { STORAGE_PIN_KEEP_PRIVKEY, STORAGE_PIN_ERASE_PRIVKEY } storage_pin_privkey_action_t;
+
bool storage_init(void);
bool storage_erase(void);
bool storage_get_stats(size_t* entries_used, size_t* entries_free);
@@ -60,11 +62,12 @@ bool storage_set_wallet_erase_pin(const uint8_t* pin, size_t pin_len);
bool storage_get_wallet_erase_pin(uint8_t* pin, size_t pin_len);
bool storage_erase_wallet_erase_pin(void);
-bool storage_set_pinserver_details(const char* urlA, const char* urlB, const uint8_t* pubkey, size_t pubkey_len);
+bool storage_set_pinserver_details(const char* urlA, const char* urlB, const uint8_t* pubkey, size_t pubkey_len,
+ storage_pin_privkey_action_t privkey_action);
bool storage_get_pinserver_urlA(char* url, size_t len, size_t* written);
bool storage_get_pinserver_urlB(char* url, size_t len, size_t* written);
bool storage_get_pinserver_pubkey(uint8_t* pubkey, size_t pubkey_len);
-bool storage_erase_pinserver_details(void);
+bool storage_erase_pinserver_details(storage_pin_privkey_action_t privkey_action);
bool storage_set_pinserver_cert(const char* cert);
bool storage_get_pinserver_cert(char* cert, size_t len, size_t* written);Why this scored 43/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.