From cccfd8b84af4c95e71a6707bf4a8649b7faab11f Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Mon, 26 Oct 2026 19:29:37 -0400 Subject: [PATCH 1/2] firmware: a Computer pairing again keeps its new keys after a restart (#155) NimBLE's NVS store saves a bond only when the number of bonds changes. A pairing that replaced an existing bond for the same identity updated RAM only; after a restart the old keys came back, often without the new IRK, so a Computer on private addresses was never resolved and couldn't reconnect. When a pairing replaces the Computer, its keys are now deleted and written again, which makes both reach NVS. Subscriptions are kept. Assisted-by: Claude Code:claude-opus-5-5 --- firmware/src/ble_keyboard.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/firmware/src/ble_keyboard.cpp b/firmware/src/ble_keyboard.cpp index 596129e..8c02dca 100644 --- a/firmware/src/ble_keyboard.cpp +++ b/firmware/src/ble_keyboard.cpp @@ -124,6 +124,20 @@ bool storedLtk(const ble_addr_t& addr, uint8_t out[16]) { return true; } +// NimBLE's NVS store saves a bond only when the number of bonds changes: a pairing that replaces an +// existing bond for the same identity updates RAM and leaves the old keys in NVS, which come back at +// the next start without the new IRK or LTK (#155). Deleting and writing the keys again makes the +// count drop then grow, so both reach NVS. Only the keys: the subscriptions stay. +bool persistBond(const ble_addr_t& addr) { + ble_store_key_sec key{}; + key.peer_addr = addr; + ble_store_value_sec ours{}, peers{}; + if (ble_store_read_our_sec(&key, &ours) != 0 || ble_store_read_peer_sec(&key, &peers) != 0) return false; + ble_store_delete_our_sec(&key); + ble_store_delete_peer_sec(&key); + return ble_store_write_our_sec(&ours) == 0 && ble_store_write_peer_sec(&peers) == 0; +} + bool isComputer(const ble_addr_t& addr) { uint8_t ltk[16]; return g_hasComputer && g_hasLtk && ble_addr_cmp(&addr, &g_computer) == 0 && storedLtk(addr, ltk) && @@ -290,7 +304,8 @@ class SecurityCallbacks : public BLESecurityCallbacks { // the Computer couldn't reconnect. Saved first: after a power cut in between, the // next start forgets the old bonds. uint8_t ltk[16]; - if (!storedLtk(d->peer_id_addr, ltk) || !saveComputer(d->peer_id_addr)) { + if (!persistBond(d->peer_id_addr) || !storedLtk(d->peer_id_addr, ltk) || + !saveComputer(d->peer_id_addr)) { reject(*d); return; } From ec3fafb0f8fbd655dcc22c031eaeb06e4ce5a052 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Mon, 26 Oct 2026 19:33:00 -0400 Subject: [PATCH 2/2] firmware: rebond rewrite refreshes the resolving list (#155) - Remove the Computer's resolving-list entry before rewriting its peer keys: the controller already holds it (from the pairing, or from boot with the old IRK) and would refuse the re-add, failing every pairing that sends an IRK. Advertising is stopped first, as HCI requires. - Rewrite each record in turn, and judge the peer record by reading it back, since a resolving-list error is returned after it's saved. Assisted-by: Claude Code:claude-opus-5-5 --- firmware/src/ble_keyboard.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/firmware/src/ble_keyboard.cpp b/firmware/src/ble_keyboard.cpp index 8c02dca..43242c5 100644 --- a/firmware/src/ble_keyboard.cpp +++ b/firmware/src/ble_keyboard.cpp @@ -124,18 +124,33 @@ bool storedLtk(const ble_addr_t& addr, uint8_t out[16]) { return true; } +// Not in a public header; ble_gap_unpair() would also end the link and drop the subscriptions. +extern "C" int ble_hs_pvcy_remove_entry(uint8_t addr_type, const uint8_t* addr); + // NimBLE's NVS store saves a bond only when the number of bonds changes: a pairing that replaces an // existing bond for the same identity updates RAM and leaves the old keys in NVS, which come back at -// the next start without the new IRK or LTK (#155). Deleting and writing the keys again makes the -// count drop then grow, so both reach NVS. Only the keys: the subscriptions stay. +// the next start without the new IRK or LTK (#155). Deleting and writing each record again makes the +// count drop then grow, so it reaches NVS. Runs for every new Computer; only the keys, the +// subscriptions stay. Each record is rewritten in turn, so a power cut loses at most one. bool persistBond(const ble_addr_t& addr) { ble_store_key_sec key{}; key.peer_addr = addr; ble_store_value_sec ours{}, peers{}; if (ble_store_read_our_sec(&key, &ours) != 0 || ble_store_read_peer_sec(&key, &peers) != 0) return false; ble_store_delete_our_sec(&key); + if (ble_store_write_our_sec(&ours) != 0) return false; + // The peer write adds the IRK to the controller's resolving list, which already holds this + // address (from the pairing, or from boot with the old IRK) and would refuse a second entry. + // The list can't change while advertising. + BLEDevice::stopAdvertising(); + ble_hs_pvcy_remove_entry(addr.type, addr.val); ble_store_delete_peer_sec(&key); - return ble_store_write_our_sec(&ours) == 0 && ble_store_write_peer_sec(&peers) == 0; + const int rc = ble_store_write_peer_sec(&peers); + // A resolving-list failure is returned after the record is saved: go by what's stored. + ble_store_value_sec back{}; + if (ble_store_read_peer_sec(&key, &back) != 0 || memcmp(back.ltk, peers.ltk, sizeof(back.ltk)) != 0) return false; + if (rc != 0) log_w("Computer's IRK not added to the resolving list: rc=%d", rc); + return true; } bool isComputer(const ble_addr_t& addr) {