From ca280c52ed6309163b5959e4285b684a96130ee6 Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:20:01 +0200 Subject: [PATCH] vcontact key fix --- docs/ARCHITECTURE.md | 15 +++- releasenotes/RELEASE_NOTES_1.17.4-zephcore.md | 23 +++++- zephcore/app/CompanionMesh.cpp | 75 +++++++++++++++++-- zephcore/app/CompanionMesh.h | 9 ++- 4 files changed, 107 insertions(+), 15 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6526ade..3bd816c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -611,9 +611,18 @@ unsolicited notices: a one-shot low-battery alert and a restart-reason message (all causes: PIN/SOFTWARE/BROWNOUT/POR/WATCHDOG/LOCKUP — offline-queue only, so routine power-on "noise" costs nothing over the air). -**Identity**: pubkey = `SHA256("zc-vcontact" || self_pubkey)` — stable per -node, unique per device, and deliberately **not a real keypair**: no private -key exists anywhere. +**Identity**: seed = `SHA256("zc-vcontact" || self_prv_key || counter)`, +pubkey = that seed's Ed25519 public point — stable per node, unique per device. +It is a **real curve point**, which the bare hash it replaced was not: a random +32-byte string decompresses to a valid Ed25519 point only ~50% of the time, so +half of all nodes advertised a v-contact that strict clients reject on contact +upsert and DM build ("peer pub_key is not a valid Ed25519 point"). ZephCore +never noticed because it only `memcmp`s this key. The `counter` byte re-rolls +the key on the protocol-reserved `0x00`/`0xFF` prefix (P = 2/256 per try). + +Seeding from the *private* key keeps the v-contact unlinkable to its node by +outsiders, and its private half is **derived and dropped** — never stored, +never signs, never does ECDH. Only this node can recompute it. **No-RF invariants** (all enforced in `CompanionMesh`): 1. `vcontactHandleFrame()` intercepts `CMD_SEND_TXT_MSG` (and the handful of diff --git a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md index 3f2d9a4..f087dd4 100644 --- a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md +++ b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md @@ -4,7 +4,7 @@ Storage housekeeping, plus a listen-before-talk fix. The repeater's `erase` neve a node flashed from another firmware could start out with somebody else's leftovers underneath it, and switching a node between companion and repeater firmware quietly let the two share the same 128 KB. Separately, the channel-activity detector was using the wrong reference table on LR1110 -boards. +boards, and the companion's built-in `v` contact carried an unusable key on about half of all nodes. > [!IMPORTANT] > **Read the role-switching section before you flash a different role onto an existing node.** A @@ -115,6 +115,27 @@ unless you run a 250 or 500 kHz bandwidth, where the old value was up to ten cou --- +## The built-in `v` contact had an unusable key on half of all nodes + +Every companion offers a contact named after itself with a `v` in front — the loopback chat that runs +the console commands. Its key was built in a way that produced something key-shaped but, on roughly +half of all nodes, not a valid key for the curve the protocol uses. + +The official app never checked and so never minded. Other clients do check, and refused either to add +the contact or to send it a message. That is why the v-contact has worked for some people and not for +others with no apparent pattern: it was a coin flip settled when the node's identity was created, and +nothing the owner did afterwards could change the outcome. + +The key is now derived properly and is valid on every node. Nothing else about the v-contact moves — +it is still local to the app, still never touches the radio, and still has no private key stored +anywhere. + +> [!IMPORTANT] +> **The v-contact's key changes with this release, so your app will keep showing the old one.** Delete +> the leftover `v` entry by hand. The new one arrives on its own the next time the app connects. + +--- + ## Also in this release Nothing here changes how a node behaves. diff --git a/zephcore/app/CompanionMesh.cpp b/zephcore/app/CompanionMesh.cpp index 105b3db..e340d0d 100644 --- a/zephcore/app/CompanionMesh.cpp +++ b/zephcore/app/CompanionMesh.cpp @@ -232,8 +232,9 @@ void CompanionMesh::begin() BaseChatMesh::begin(); /* Derive the v-contact pubkey from our identity: stable per node, unique - * per device. Deliberately NOT a real keypair — no private key exists - * anywhere, so nothing addressed to this key is decryptable by anyone. */ + * per device. A real Ed25519 point (strict clients reject anything else), + * but its private half is derived-and-dropped — never stored, never used — + * and only this node can recompute it. */ deriveVContactKey(); /* Stamp lastmod only if a time source already ran (hardware RTC restore * happens before begin()). Otherwise stay deferred (lastmod = 0) until @@ -1052,13 +1053,71 @@ void CompanionMesh::vcontactFlushConfirm() void CompanionMesh::deriveVContactKey() { - /* v-contact pubkey = SHA256("zc-vcontact" || self pubkey). Re-run whenever - * the identity changes (boot, CMD_IMPORT_PRIVATE_KEY) so the key always - * tracks the current identity. */ + /* v-contact pubkey = the Ed25519 public point of a keypair derived from + * SHA256("zc-vcontact" || self prv_key || counter). Re-run whenever the + * identity changes (boot, CMD_IMPORT_PRIVATE_KEY) so the key always tracks + * the current identity. + * + * This used to be the bare hash SHA256("zc-vcontact" || self pubkey) placed + * straight into a pub_key field. That is wrong on the wire: a uniformly + * random 32-byte string decompresses to a valid Ed25519 point only about + * half the time (the recovered x^2 must be a quadratic residue), so ~50% of + * nodes advertised a v-contact whose key strict clients reject outright — + * they decompress peer keys on contact upsert and on DM build, and error + * with "peer pub_key is not a valid Ed25519 point". ZephCore itself never + * noticed because it only ever memcmp()s this key (see isVContactKey(); + * buildVContact() sets shared_secret_valid = false). Deriving the point via + * scalarbase instead makes it valid by construction, for every node. + * + * Seeded from the PRIVATE key, not the public one: the seed then sits + * behind a value no one else holds, so an outsider cannot link a v-contact + * to the node it belongs to, and cannot derive the matching private key. + * Nothing needs that link — the app receives the v-contact through an + * explicit PUSH_CODE_NEW_ADVERT and never derives it. The private half is + * computed here and dropped: it is never stored, never signs, and never + * takes part in a key exchange. Publishing the point leaks nothing about + * the identity — reaching prv from it means solving the Ed25519 discrete + * log AND inverting SHA-512 AND inverting SHA-256. + * + * The counter byte serves the reserved-prefix guard: MeshCore treats + * pub_key[0] of 0x00/0xFF as a protocol marker (same rule as + * ZephyrRNG::generateFirstBootIdentity and validatePrivateKey), and a + * deterministic derivation cannot simply "draw again" without one. Each + * bump re-rolls the whole key; P(a single miss) = 2/256, so the loop + * essentially always ends on the first pass and stays deterministic. */ static const char vc_salt[] = "zc-vcontact"; - mesh::Utils::sha256(_vcontact_pubkey, PUB_KEY_SIZE, - (const uint8_t *)vc_salt, sizeof(vc_salt) - 1, - self_id.pub_key, PUB_KEY_SIZE); + uint8_t material[PRV_KEY_SIZE + 1]; + uint8_t seed[SEED_SIZE]; + mesh::LocalIdentity vc; + + /* writeTo()'s buffer format is prv || pub; capping max_len at PRV_KEY_SIZE + * asks for the private half alone. */ + if (self_id.writeTo(material, PRV_KEY_SIZE) != PRV_KEY_SIZE) { + /* Cannot happen — kept so a future signature change fails loudly + * rather than seeding off an uninitialised stack buffer. */ + LOG_ERR("vcontact: private key unavailable, key not derived"); + mesh::Utils::secureZeroize(material, sizeof(material)); + return; + } + + for (int counter = 0; counter < 256; counter++) { + material[PRV_KEY_SIZE] = (uint8_t)counter; + mesh::Utils::sha256(seed, SEED_SIZE, + (const uint8_t *)vc_salt, sizeof(vc_salt) - 1, + material, sizeof(material)); + vc.fromSeed(seed); + if (vc.pub_key[0] != 0x00 && vc.pub_key[0] != 0xFF) break; + } + memcpy(_vcontact_pubkey, vc.pub_key, PUB_KEY_SIZE); + + /* material holds the real identity private key, and vc holds the v-key's + * discarded private half. Neither has any business outliving this call. + * LocalIdentity keeps prv_key private and has no wipe of its own, so the + * object is cleared wholesale — it has no virtuals, so there is no vtable + * pointer to destroy. */ + mesh::Utils::secureZeroize(material, sizeof(material)); + mesh::Utils::secureZeroize(seed, sizeof(seed)); + mesh::Utils::secureZeroize(&vc, sizeof(vc)); } void CompanionMesh::vcontactPushAdvert() diff --git a/zephcore/app/CompanionMesh.h b/zephcore/app/CompanionMesh.h index a1f90cb..cff007e 100644 --- a/zephcore/app/CompanionMesh.h +++ b/zephcore/app/CompanionMesh.h @@ -164,9 +164,12 @@ public: * A synthesized CHAT contact visible only to the connected BLE/USB app. * Messages to it are short-circuited into the CLI before any packet is * created — nothing ever reaches the dispatcher or the radio. Its pubkey - * is SHA256("zc-vcontact" || self pubkey); no private key exists and it - * is never registered in the RF RX matching path, so over-the-air - * traffic addressed to it is inert. */ + * is the Ed25519 point of a keypair seeded from + * SHA256("zc-vcontact" || self prv_key || counter) — a real point, because + * strict clients decompress peer keys and reject anything else. The private + * half is derived and dropped: never stored, never used. The key is never + * registered in the RF RX matching path, so over-the-air traffic addressed + * to it is inert. */ void setVContactCLICallback(VContactCLICallback cb) { _vcontact_cli_cb = cb; } bool isVContactEnabled() const { return prefs.v_contact_enabled != 0; } /** Queue an unsolicited v-contact message (battery alert, restart reason).