ElixirForge/firmware
1
0
Fork 0
mirror of https://github.com/alexhopeoconnor/firmware.git synced 2026-10-04 03:18:10 +10:00
Code Releases Activity

fix(NodeDB): don't let an empty contact key erase a stored public key (#11432)

Browse Source 
Clients send `add_contact` before every text-message DM, because a phone
often holds a larger contact database (with public keys) than the radio
can keep. That makes `addFromContact()` the highest-volume key-write path
on the device - and it had no protection against key erasure.

Its only key guard covered the manually-verified case: if the local entry
was marked manually verified and the incoming contact was not, a key
mismatch aborted the update. Every ordinary entry fell straight through to
`CopyUserToNodeInfoLite()`, which assigns `public_key` unconditionally. So
a SharedContact with `has_user` set and an empty `public_key` overwrote a
peer's stored, XEdDSA-proven key with zeros - and `addFromContact()` calls
`saveNodeDatabaseToDisk()`, so the erasure survived a reboot. Subsequent
DMs to that peer then failed with PKI_SEND_FAIL_PUBLIC_KEY, with no way to
recover until the peer's NodeInfo was re-exchanged.

`public_key` is a singular (non-optional) bytes field, so "absent" and
"empty" both decode to size 0; a client that simply has no key for a
contact is indistinguishable on the wire from one asking to clear it.

The fix is deliberately narrow: keep the stored key when the entry already
holds a full 32-byte key and the incoming contact does not. A well-formed
32-byte contact key still updates the entry exactly as before.

Deliberately NOT changed here:
- `updateUser()`'s first-key-wins pin is not applied to this path. Clients
  legitimately use add_contact to supply keys the radio never had and to
  update them (QR-code contact sharing); a blanket pin would break that
  documented flow. Only erasure is refused.
- `CopyUserToNodeInfoLite()` itself is untouched - it has many other
  callers (self-record refresh, updateUser, warm-tier rehydration), so the
  guard lives at this call site.
- The manually-verified branch is unchanged.
- Node-number validation (reserved/broadcast/self) on this path remains
  open and is tracked separately.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Clive Blackledge
2026-08-12 12:22:53 +00:00
committed by GitHub
co-authored by Claude Opus 5
parent 2f6906974e
commit 579d26e1b2
1 changed files with 9 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mesh/NodeDB.cpp
+9
View File
@@ -3482,7 +3482,16 @@ void NodeDB::addFromContact(meshtastic_SharedContact contact)
}
}
info->num = contact.node_num;
// CopyUserToNodeInfoLite assigns public_key unconditionally, and clients send add_contact before every
// DM - often from an entry that carries no key at all. A contact may still supply or update a full
// 32-byte key (that's what add_contact is for), but it must never *erase* a key we already hold, which
// would be persisted below and break subsequent DMs with PKI_SEND_FAIL_PUBLIC_KEY.
const meshtastic_NodeInfoLite_public_key_t storedKey = info->public_key;
TypeConversions::CopyUserToNodeInfoLite(info, contact.user);
if (storedKey.size == 32 && info->public_key.size != 32) {
LOG_INFO("Contact 0x%08x has no key, keep the stored one", contact.node_num);
info->public_key = storedKey;
}
if (contact.should_ignore) {
// Block the contact and drop its rich satellite data, but keep the
// public key copied above - an ignored peer keeps a usable identity