From 6f3f0bd7c26be715c9fb80bea91dcd38b59eb80f Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Thu, 17 Sep 2026 22:24:29 +0000 Subject: [PATCH] Prove explicit acks with Routing.ack_proof (#11877) * Prove explicit acks with Routing.ack_proof Explicit acks are ROUTING_APP packets, and ROUTING_APP is excluded from PKC, so an ack travels under channel encryption alone - and the default channel key is public. Anyone in range can forge one, and the client grants its strongest delivery claim on the strength of the ack's unauthenticated `from`. Where the acknowledged packet was PKI encrypted the endpoints already share a Curve25519 secret, so the recipient can prove receipt in ~10 encoded bytes: ack_proof = HMAC-SHA256(shared_key, "ack" | LE32(from) | LE32(to) | LE32(request_id) | routing)[0..8) where `routing` is the encoded Routing message without the ack_proof field, taken as received with that byte range removed rather than re-encoded. Excising keeps the value a function of the received bytes alone, so it does not depend on two implementations' encoders agreeing and does not drop fields this build has never heard of. For the same reason the sender appends the field rather than setting it on a decoded struct and re-encoding. What this does and does not buy. It buys an authenticated delivery receipt from the actual recipient, which is the property a forged ack costs a user and which matters where people act on a delivery confirmation. It does NOT protect the retransmission loop, and must not be described as if it does: perhapsGenerateImplicitAckForOwnOverheard clears a pending retransmission on any overheard rebroadcast of our own (from, id), header-only and keyless, so replaying the originator's own ciphertext stops their retries more cheaply than forging an ack. No ack authentication of any kind closes that path. Advisory only, and deliberately not a step toward enforcement. A rule requiring a proof once a peer has sent one would make a missing proof destroy the only delivery signal we have, on state the user cannot see: the proof needs the peer to hold our key, and peer-side eviction, a downgrade or a factory reset are all invisible to us. A valid proof marks the ack verified; anything else behaves exactly as today. The client renders the difference. Verification costs one X25519 and nothing caches the shared secret, so ackProofPermitsAction gates on findPendingPacket first - otherwise a forged ack naming any packet id, which is visible in the cleartext header, would force a DH. Depends on meshtastic/protobufs#1094 for the generated field. * test: correct a comment that predates ack_proof being generated The wire-roundtrip test described ack_proof as an unknown field, which was true while the prototype hand-encoded it. The field is generated now, so this build understands it - but older firmware does not, which is the case the assertion actually covers. * Fix the EXCLUDE_PKI test build, and format Two copies of test_proof_binds_error_reason and test_proof_binds_direction were sitting in the #else branch of test_ack_proof, where Identity, makeIdentity, makeAck, becomeNode, crypto and ACK_PROOF_SIZE do not exist. They were also unregistered, so they were dead code that only served to break the build. The native suite never compiles that branch, so a local run could not see it. Also apply clang-format to a declaration that had been wrapped by hand. * Exempt test_ack_proof from trufflehog's Lob false positive Same detector and same shape as the three suites already listed here: it stitches nearby hex literals into one candidate string, and this suite's node numbers and request ids (0x0A0A0A0A, 0x0B0B0B0B, 0xABCD1234) happen to match a Lob API key. The suite holds no literal key material - every key it uses comes from crypto->generateKeyPair at runtime. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 --------- Co-authored-by: Claude --- .trunk/trunk.yaml | 6 +- src/mesh/AckProof.cpp | 155 +++++++++++ src/mesh/AckProof.h | 94 +++++++ src/mesh/CryptoEngine.cpp | 48 ++++ src/mesh/CryptoEngine.h | 34 +++ src/mesh/MeshModule.cpp | 8 + src/mesh/ReliableRouter.cpp | 41 ++- src/mesh/ReliableRouter.h | 7 + test/test_ack_proof/test_main.cpp | 433 ++++++++++++++++++++++++++++++ 9 files changed, 824 insertions(+), 2 deletions(-) create mode 100644 src/mesh/AckProof.cpp create mode 100644 src/mesh/AckProof.h create mode 100644 test/test_ack_proof/test_main.cpp diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index 765e2889c6..db8f9dd3cf 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -174,9 +174,13 @@ lint: # (0xFFFFFF00u and neighbors). trufflehog's Lob detector stitches nearby # hex literals into one candidate string and the result happens to match # a Lob API key shape. Not secrets - deterministic test fixtures for the - # 32-bit rollover. + # 32-bit rollover. test_ack_proof trips the same detector on its node + # numbers and request ids (0x0A0A0A0A, 0x0B0B0B0B, 0xABCD1234); its key + # material is generated at runtime by crypto->generateKeyPair, so there is + # no literal key in the file to leak. - linters: [trufflehog] paths: + - test/test_ack_proof/test_main.cpp - test/test_airtime/test_main.cpp - test/test_throttle/test_main.cpp - test/test_uptime_clock/test_main.cpp diff --git a/src/mesh/AckProof.cpp b/src/mesh/AckProof.cpp new file mode 100644 index 0000000000..c53ca0c78c --- /dev/null +++ b/src/mesh/AckProof.cpp @@ -0,0 +1,155 @@ +#include "AckProof.h" + +#if !(MESHTASTIC_EXCLUDE_PKI) + +#include "CryptoEngine.h" +#include "NodeDB.h" +#include "concurrency/LockGuard.h" +#include +#include + +/** Is this a packet shaped like an explicit ack/nak we could attach a proof to? */ +static bool isProvableAck(const meshtastic_MeshPacket *p) +{ + return p && p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && + p->decoded.portnum == meshtastic_PortNum_ROUTING_APP && p->decoded.request_id != 0 && !isBroadcast(p->to); +} + +bool ackProofExtract(const uint8_t *payload, size_t payloadLen, uint8_t *proofOut, size_t *fieldStart, size_t *fieldLen) +{ + // Walk the encoded Routing message with the real parser rather than assuming the proof is the + // trailing field: it is only last today because nanopb emits in field-number order and 4 is the + // highest tag, which stops being true the moment Routing gains a field 5. + pb_istream_t stream = pb_istream_from_buffer(payload, payloadLen); + uint32_t tag; + pb_wire_type_t wireType; + bool eof = false; + bool found = false; + + while (true) { + const size_t tagStart = payloadLen - stream.bytes_left; + // pb_decode_tag returns false at a CLEAN end of stream, with eof set - so eof has to be + // checked before treating the false as a parse error, or a well-formed message reads as + // malformed. This mirrors nanopb's own decode loop (pb_decode.c, pb_decode_inner). + if (!pb_decode_tag(&stream, &wireType, &tag, &eof)) { + if (eof) + break; + return false; + } + + if (tag == ACK_PROOF_FIELD_NUMBER && wireType == PB_WT_STRING) { + // Two proofs mean two possible excisions and so two possible verdicts. Refuse rather + // than pick one. + if (found) + return false; + + pb_istream_t substream; + if (!pb_make_string_substream(&stream, &substream)) + return false; + // A wrong length is malformed, not a near miss - an honest sender emits exactly this. + if (substream.bytes_left != ACK_PROOF_SIZE || !pb_read(&substream, proofOut, ACK_PROOF_SIZE)) + return false; + if (!pb_close_string_substream(&stream, &substream)) + return false; + + found = true; + if (fieldStart) + *fieldStart = tagStart; + if (fieldLen) + *fieldLen = (payloadLen - stream.bytes_left) - tagStart; + continue; + } + + if (!pb_skip_field(&stream, wireType)) + return false; + } + return found; +} + +bool ackProofAttachWithKey(meshtastic_MeshPacket *p, const uint8_t *peerPubKey) +{ + if (!isProvableAck(p)) + return false; + + uint8_t proof[ACK_PROOF_SIZE]; + { + // ackProofCompute clobbers the engine's shared_key. cryptLock is not held on the ack + // allocation path (perhapsEncode takes it later, further down the send), so take it here. + concurrency::LockGuard g(cryptLock); + // The payload as it stands IS this message without the ack_proof field - it has not been + // appended yet - so the sender never has to reconstruct anything. + if (!crypto->ackProofCompute(peerPubKey, getFrom(p), p->to, p->decoded.request_id, p->decoded.payload.bytes, + p->decoded.payload.size, proof)) + return false; + } + + // Append the field rather than setting it on a decoded struct and re-encoding. A re-encode + // would drop any field this build does not know, and the receiver derives the MAC input by + // excising ack_proof from the bytes as received - so the two would disagree the first time a + // newer sender includes something we cannot parse. Appending leaves every other byte untouched. + pb_ostream_t stream = pb_ostream_from_buffer(p->decoded.payload.bytes + p->decoded.payload.size, + sizeof(p->decoded.payload.bytes) - p->decoded.payload.size); + if (!pb_encode_tag(&stream, PB_WT_STRING, ACK_PROOF_FIELD_NUMBER) || !pb_encode_string(&stream, proof, ACK_PROOF_SIZE)) + return false; // out of room: send the ack unproven rather than truncating it + + p->decoded.payload.size += stream.bytes_written; + return true; +} + +AckProofResult ackProofVerifyWithKey(const meshtastic_MeshPacket *p, uint32_t requestId, const uint8_t *peerPubKey) +{ + if (!p || p->which_payload_variant != meshtastic_MeshPacket_decoded_tag) + return AckProofResult::ABSENT; + + uint8_t claimed[ACK_PROOF_SIZE]; + size_t fieldStart = 0, fieldLen = 0; + if (!ackProofExtract(p->decoded.payload.bytes, p->decoded.payload.size, claimed, &fieldStart, &fieldLen)) + return AckProofResult::ABSENT; + if (!peerPubKey) + return AckProofResult::NO_KEY; + + // This encoded Routing message without the ack_proof field: the received bytes with that one + // range removed. Never re-encoded, so the value depends only on what arrived - a re-encode + // would make it depend on this build's encoder agreeing with the sender's, and would silently + // drop any field this build does not know about. + uint8_t routing[sizeof(p->decoded.payload.bytes)]; + const size_t tail = p->decoded.payload.size - (fieldStart + fieldLen); + memcpy(routing, p->decoded.payload.bytes, fieldStart); + memcpy(routing + fieldStart, p->decoded.payload.bytes + fieldStart + fieldLen, tail); + const size_t routingLen = fieldStart + tail; + + uint8_t expected[ACK_PROOF_SIZE]; + { + concurrency::LockGuard g(cryptLock); + if (!crypto->ackProofCompute(peerPubKey, getFrom(p), p->to, requestId, routing, routingLen, expected)) + return AckProofResult::NO_KEY; + } + + // memcmp is fine here: both sides are already public once transmitted, and an attacker gets no + // timing oracle they could not get by simply sending a guess. + return memcmp(claimed, expected, ACK_PROOF_SIZE) == 0 ? AckProofResult::VALID : AckProofResult::INVALID; +} + +void ackProofAttach(meshtastic_MeshPacket *p) +{ + if (!isProvableAck(p)) + return; + + // Authoritative keys only. Proving against an opportunistically cached key would let a planted + // key validate its own acks - the same trust loop the decrypt path already closes. + meshtastic_NodeInfoLite_public_key_t peerKey = {0, {0}}; + if (!nodeDB->copyPublicKeyAuthoritative(p->to, peerKey) || peerKey.size != 32) + return; + + if (ackProofAttachWithKey(p, peerKey.bytes)) + LOG_DEBUG("Attached ACK proof for 0x%08x to 0x%08x", p->decoded.request_id, p->to); +} + +AckProofResult ackProofVerify(const meshtastic_MeshPacket *p, uint32_t requestId) +{ + meshtastic_NodeInfoLite_public_key_t peerKey = {0, {0}}; + const bool haveKey = nodeDB->copyPublicKeyAuthoritative(getFrom(p), peerKey) && peerKey.size == 32; + return ackProofVerifyWithKey(p, requestId, haveKey ? peerKey.bytes : nullptr); +} + +#endif diff --git a/src/mesh/AckProof.h b/src/mesh/AckProof.h new file mode 100644 index 0000000000..1614edd39b --- /dev/null +++ b/src/mesh/AckProof.h @@ -0,0 +1,94 @@ +#pragma once + +#include "configuration.h" +#include "mesh/CryptoEngine.h" +#include "mesh/generated/meshtastic/mesh.pb.h" + +#if !(MESHTASTIC_EXCLUDE_PKI) + +/** + * Pairwise ACK proof: an authenticated delivery receipt from the actual recipient. + * + * Explicit acks are ROUTING_APP packets, and ROUTING_APP is excluded from PKC, so an ack is + * protected by nothing but the channel PSK - public on the default channel. Anyone in range can + * forge one, and the client grants its strongest delivery claim ("delivered to recipient") on the + * strength of the ack's unauthenticated `from`. + * + * Where the acknowledged packet was PKI-encrypted the two endpoints already share a Curve25519 + * secret, so the recipient can prove receipt with a short MAC: + * + * proof = HMAC-SHA256( sharedKey, + * "ack" | LE32(from) | LE32(to) | LE32(request_id) | routing ) + * [0 .. ACK_PROOF_SIZE) + * + * where `routing` is this encoded Routing message without the ack_proof field. See + * CryptoEngine::ackProofCompute for why each of those is bound. + * + * Properties, and the honest limits: + * + * - ~10 encoded bytes instead of 66, and one HMAC instead of Ed25519 sign+verify. + * - Nothing is added to the original message: the secret is derived, not transmitted. (Shipping a + * nonce in the payload would also work, but costs bytes on every DM and gains nothing here.) + * - request_id is an input, so a captured proof cannot be retargeted at another pending packet, + * and the Routing bytes are an input, so a proven ack cannot be bit-flipped into a nak. + * - The ack stays channel-encrypted, so relays keep reading request_id for next-hop learning and + * retransmission cancel. Authentication without confidentiality is the point. + * - This does NOT protect the retransmission loop, and must not be described as if it does. + * ReliableRouter::perhapsGenerateImplicitAckForOwnOverheard clears a pending retransmission on + * any overheard rebroadcast of our own (from, id) - header-only, no key, and it fires even on a + * PKI DM we cannot read. Replaying the originator's own ciphertext stops their retries more + * cheaply than forging an ack, so no ack authentication of any kind closes that path. The one + * property on offer here is the receipt. + * - Only the original sender can verify. A relay cannot, so a forged ack still propagates and + * still cancels intermediates' retransmissions; the endpoint just stops believing it. + * - Channel (non-PKI) traffic gets NOTHING from this: the forger holds the PSK, so any secret + * carried under channel encryption is a secret the forger also has. + * - PKI_UNKNOWN_PUBKEY and NO_CHANNEL naks are emitted precisely when we could not decrypt, so no + * shared secret exists and they can never carry a proof. They stay forgeable. + * - Verification costs one X25519 (there is no shared-secret cache), and an attacker chooses when + * we pay it. Callers MUST gate on cheap checks first - see ReliableRouter::ackProofPermitsAction, + * which will not verify unless we actually have a matching packet outstanding. + */ + +// The generated field (meshtastic/protobufs#1094). ACK_PROOF_SIZE comes from CryptoEngine.h, which +// derives it from the generated type so the protocol owns the number. +#define ACK_PROOF_FIELD_NUMBER meshtastic_Routing_ack_proof_tag + +// Advisory by design, and this is not a placeholder for later enforcement. A rule that requires a +// proof once a peer has sent one would make a missing proof destroy the only delivery signal we +// have, on state the user cannot see: the proof needs the PEER to hold OUR key, and peer-side +// NodeDB eviction, a firmware downgrade or a factory reset are all invisible to us and would +// silently kill acks forever. A valid proof marks the ack verified; anything else behaves exactly +// as today and is reported unverified, and the client renders the difference. +static constexpr bool ACK_PROOF_ENFORCE = false; + +enum class AckProofResult { + ABSENT, // no proof attached - today's behavior, and every peer we have no pairwise key with + VALID, // present and correct + INVALID, // present and wrong - a forgery, or a key/identity mismatch + NO_KEY, // present, but we hold no authoritative key for the peer, so we cannot judge it +}; + +/** + * Attach a proof to a freshly built ROUTING ack/nak. No-op unless the packet is a ROUTING unicast + * with a request_id and we hold an authoritative public key for the destination. + */ +void ackProofAttach(meshtastic_MeshPacket *p); + +/** Check the proof on a received ack/nak against the id of the packet it claims to acknowledge. */ +AckProofResult ackProofVerify(const meshtastic_MeshPacket *p, uint32_t requestId); + +// Mechanism, split out from the NodeDB-backed policy above so both halves are unit-testable +// without standing up a node database. +bool ackProofAttachWithKey(meshtastic_MeshPacket *p, const uint8_t *peerPubKey); +AckProofResult ackProofVerifyWithKey(const meshtastic_MeshPacket *p, uint32_t requestId, const uint8_t *peerPubKey); + +/** + * Read the proof out of an encoded Routing payload. False if the field is absent or malformed, + * which includes a message carrying more than one of them. fieldStart/fieldLen (optional) report + * the byte range it occupies, which the verifier removes to rebuild the bytes the sender hashed. + */ +bool ackProofExtract(const uint8_t *payload, size_t payloadLen, uint8_t *proofOut, size_t *fieldStart = nullptr, + size_t *fieldLen = nullptr); + +#endif diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 81d27223cd..80a27cee57 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -287,6 +287,54 @@ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_ return aes_ccm_ad(shared_key, 32, nonce, 8, bytes, numBytes - 12, nullptr, 0, auth, bytesOut); } +// The label is domain separation only - it never needs to be secret. It is fixed length and +// precedes the fixed-width fields, and only the trailing Routing bytes are variable, so the input is +// unambiguous without length prefixes. +static const char ACK_PROOF_LABEL[] = "ack"; +#define ACK_PROOF_LABEL_LEN (sizeof(ACK_PROOF_LABEL) - 1) // no trailing NUL on the wire + +/** Write a little-endian uint32 - the encoding is pinned by the protocol, not by the host. */ +static void ackProofPutLE32(uint8_t *out, uint32_t v) +{ + out[0] = (uint8_t)(v & 0xff); + out[1] = (uint8_t)((v >> 8) & 0xff); + out[2] = (uint8_t)((v >> 16) & 0xff); + out[3] = (uint8_t)((v >> 24) & 0xff); +} + +bool CryptoEngine::ackProofCompute(const uint8_t *peerPubKey, uint32_t ackFrom, uint32_t ackTo, uint32_t requestId, + const uint8_t *routing, size_t routingLen, uint8_t *proofOut) +{ + if (memfll(private_key, 0, sizeof(private_key))) + return false; // no identity yet - nothing to prove with + + // setDHPublicKey takes a mutable buffer (Curve25519::dh2 works in place), so copy the peer key. + uint8_t peer[32]; + memcpy(peer, peerPubKey, 32); + if (!setDHPublicKey(peer)) + return false; // includes the library's weak-point check + hash(shared_key, 32); // same derivation encryptCurve25519/decryptCurve25519 use + + uint8_t header[ACK_PROOF_LABEL_LEN + 3 * sizeof(uint32_t)]; + memcpy(header, ACK_PROOF_LABEL, ACK_PROOF_LABEL_LEN); + ackProofPutLE32(header + ACK_PROOF_LABEL_LEN, ackFrom); + ackProofPutLE32(header + ACK_PROOF_LABEL_LEN + 4, ackTo); + ackProofPutLE32(header + ACK_PROOF_LABEL_LEN + 8, requestId); + + uint8_t digest[32]; + SHA256 mac; + mac.resetHMAC(shared_key, 32); + mac.update(header, sizeof(header)); + if (routing && routingLen) + mac.update(routing, routingLen); + mac.finalizeHMAC(shared_key, 32, digest, sizeof(digest)); + memcpy(proofOut, digest, ACK_PROOF_SIZE); + + memset(digest, 0, sizeof(digest)); + memset(shared_key, 0, sizeof(shared_key)); // do not leave the pairwise secret sitting in the engine + return true; +} + void CryptoEngine::setDHPrivateKey(uint8_t *_private_key) { memcpy(private_key, _private_key, 32); diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index b23df129fc..d2debbcbe1 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -27,6 +27,8 @@ struct CryptoKey { // Encoded size the signature adds to the Data protobuf: 1 tag byte (field 10 < 16) + // 1 length byte (64 < 128) + 64 signature bytes. test_packet_signing asserts this stays exact. #define XEDDSA_SIGNATURE_FIELD_BYTES (XEDDSA_SIGNATURE_SIZE + 2) +// Length of Routing.ack_proof, taken from the generated field so the protocol owns the number. +static constexpr size_t ACK_PROOF_SIZE = sizeof(meshtastic_Routing_ack_proof_t::bytes); class CryptoEngine { @@ -48,6 +50,38 @@ class CryptoEngine bool xeddsa_verify(const uint8_t *pubKey, uint32_t fromNode, uint32_t packetId, uint32_t portnum, const uint8_t *payload, size_t payloadLen, const uint8_t *signature); #endif + /** + * Derive the pairwise ACK proof carried in Routing.ack_proof. + * + * proof = HMAC-SHA256( sharedKey, + * "ack" | LE32(ackFrom) | LE32(ackTo) | LE32(requestId) | routing ) + * [0 .. ACK_PROOF_SIZE) + * + * sharedKey is the same SHA256(X25519(our private, peer public)) packet crypto uses, so only the + * two endpoints can produce or check it. What each input is for: + * + * - ackFrom / ackTo: X25519 is symmetric, so DH(a_priv, B_pub) == DH(b_priv, A_pub). Without + * the direction bound, an A->B proof for a requestId equals the B->A proof for it. + * - requestId: stops a captured proof being retargeted at another outstanding packet. + * - routing: the encoded Routing message WITHOUT ack_proof, as the bytes arrived. An ack and a + * nak for one packet otherwise hash identically, and channel crypto is CTR with no integrity + * check, so a PSK holder could flip a proven success into a failure and it would still verify. + * - HMAC rather than SHA256(key | msg): the raw construction is not breakable here, but HMAC is + * the one with a proof behind it and costs no flash - SHA256 already carries resetHMAC and + * finalizeHMAC in its vtable. + * - Integers are little-endian explicitly, so the value is a property of the protocol and not of + * the compiler that built the node. + * + * Cost: setDHPublicKey runs Curve25519::dh2 on every call and nothing caches the result, so this + * is one X25519 per verify, which an attacker chooses when we pay by sending a forged ack. + * Callers MUST gate on cheap checks first - see ReliableRouter::ackProofPermitsAction. + * + * Clobbers shared_key, so the caller must hold cryptLock (which is NOT recursive - do not call + * this from a context that already holds it, such as perhapsEncode). + */ + bool ackProofCompute(const uint8_t *peerPubKey, uint32_t ackFrom, uint32_t ackTo, uint32_t requestId, const uint8_t *routing, + size_t routingLen, uint8_t *proofOut); + void setDHPrivateKey(uint8_t *_private_key); // The remotePublic key parameter takes the public_key bytes container from // a stored node header. NodeInfoLite is the on-device storage type since diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp index 059f90ee81..621d2bf244 100644 --- a/src/mesh/MeshModule.cpp +++ b/src/mesh/MeshModule.cpp @@ -1,4 +1,5 @@ #include "MeshModule.h" +#include "AckProof.h" #include "Channels.h" #include "MeshService.h" #include "NodeDB.h" @@ -85,6 +86,13 @@ meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, Nod p->rx_rssi = relaySource->rx_rssi; p->rx_snr = relaySource->rx_snr; } +#if !(MESHTASTIC_EXCLUDE_PKI) + // Prove to the original sender that this ack came from the node holding their pairwise key. + // No-op unless we hold an authoritative key for `to`. Must follow the request_id and payload + // assignments above: the proof covers request_id and is appended to the payload. + ackProofAttach(p); +#endif + if (err != meshtastic_Routing_Error_NONE) LOG_WARN("Alloc an err=%d,to=0x%08x,idFrom=0x%08x,id=0x%08x", err, to, idFrom, p->id); diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 7be23ee810..796487157d 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -1,4 +1,5 @@ #include "ReliableRouter.h" +#include "AckProof.h" #include "Default.h" #include "MeshTypes.h" #include "NodeDB.h" @@ -172,7 +173,7 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas PacketId nakId = (c && c->error_reason != meshtastic_Routing_Error_NONE) ? p->decoded.request_id : 0; // We intentionally don't check wasSeenRecently, because it is harmless to delete non existent retransmission records - if ((ackId || nakId) && + if ((ackId || nakId) && ackProofPermitsAction(p, ackId ? ackId : nakId) && // Implicit ACKs from MQTT should not stop retransmissions !(isFromUs(p) && p->transport_mechanism == meshtastic_MeshPacket_TransportMechanism_TRANSPORT_MQTT)) { LOG_DEBUG("Received a %s for 0x%08x, stopping retransmissions", ackId ? "ACK" : "NAK", ackId); @@ -192,6 +193,44 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas isBroadcast(p->to) ? FloodingRouter::sniffReceived(p, c) : NextHopRouter::sniffReceived(p, c); } +bool ReliableRouter::ackProofPermitsAction(const meshtastic_MeshPacket *p, PacketId originalId) +{ +#if !(MESHTASTIC_EXCLUDE_PKI) + // Cheap gate FIRST: verification costs one X25519 per call (setDHPublicKey runs Curve25519::dh2 + // and nothing caches the result), and an attacker with the channel key picks when we pay it. A + // packet id is visible in the cleartext header, so without this gate a forged ack for any id at + // all forces a DH. With it, only ids we genuinely have outstanding can, and only while pending. + PendingPacket *orig = findPendingPacket(p->to, originalId); + if (!orig) + return true; + const bool expected = orig->packet && orig->packet->pki_encrypted; + + switch (ackProofVerify(p, originalId)) { + case AckProofResult::VALID: + LOG_DEBUG("ACK proof OK for 0x%08x", originalId); + return true; + case AckProofResult::INVALID: + // Somebody produced an ack for our packet without holding the shared secret. + LOG_WARN("ACK proof MISMATCH for 0x%08x from 0x%08x%s", originalId, getFrom(p), + ACK_PROOF_ENFORCE ? ", ignoring ack" : " (advisory)"); + return !ACK_PROOF_ENFORCE; + case AckProofResult::NO_KEY: + LOG_DEBUG("ACK proof present for 0x%08x but no authoritative key for 0x%08x", originalId, getFrom(p)); + return true; + case AckProofResult::ABSENT: + // What every current firmware sends, and what every non-PKI peer will always send, so this + // can only ever be reported - never enforced. See ACK_PROOF_ENFORCE in AckProof.h. + if (expected) + LOG_INFO("Unproven ack for PKI packet 0x%08x from 0x%08x", originalId, getFrom(p)); + return true; + } +#else + (void)p; + (void)originalId; +#endif + return true; +} + /** * If we ACK this packet, should we set want_ack=true on the ACK for reliable delivery of the ACK packet? */ diff --git a/src/mesh/ReliableRouter.h b/src/mesh/ReliableRouter.h index 1dafaca801..2a0e34ced6 100644 --- a/src/mesh/ReliableRouter.h +++ b/src/mesh/ReliableRouter.h @@ -42,4 +42,11 @@ class ReliableRouter : public NextHopRouter * Should this packet be ACKed with a want_ack for reliable delivery? */ bool shouldSuccessAckWithWantAck(const meshtastic_MeshPacket *p); + + /** + * May this ack/nak act on our pending send for `originalId`? Always true unless the ack carries + * a pairwise proof that fails to verify AND enforcement is on. An absent proof is not a failure: + * only peers we share a PKI key with can produce one at all. + */ + bool ackProofPermitsAction(const meshtastic_MeshPacket *p, PacketId originalId); }; \ No newline at end of file diff --git a/test/test_ack_proof/test_main.cpp b/test/test_ack_proof/test_main.cpp new file mode 100644 index 0000000000..a37353ec45 --- /dev/null +++ b/test/test_ack_proof/test_main.cpp @@ -0,0 +1,433 @@ +// PROTOTYPE suite for the pairwise ACK proof (src/mesh/AckProof.h). +// +// Covers the mechanism end to end - key agreement, request_id binding, wire encoding, and the +// verify verdicts - without standing up a NodeDB. The NodeDB-backed policy wrappers +// (ackProofAttach/ackProofVerify) and the ReliableRouter hook are deliberately NOT covered here; +// they need the full router harness, and this is a spike. +#include "configuration.h" + +#include "TestUtil.h" +#include + +#if !(MESHTASTIC_EXCLUDE_PKI) + +#include "mesh/AckProof.h" +#include "mesh/CryptoEngine.h" +#include "mesh/MeshTypes.h" +#include +#include + +static constexpr NodeNum ALICE = 0x0A0A0A0A; // sends the DM, verifies the ack +static constexpr NodeNum BOB = 0x0B0B0B0B; // receives the DM, produces the ack +static constexpr uint32_t REQUEST_ID = 0xABCD1234; + +// Stand-in for the encoded Routing message the proof covers. +static const uint8_t ROUTING[] = {0x18, 0x00}; // error_reason = NONE +#define ROUTING_LEN sizeof(ROUTING) + +struct Identity { + uint8_t pub[32]; + uint8_t priv[32]; +}; + +static Identity makeIdentity() +{ + Identity id; + crypto->generateKeyPair(id.pub, id.priv); + return id; +} + +/** Act as this node for subsequent crypto calls. */ +static void becomeNode(const Identity &id) +{ + uint8_t priv[32]; + memcpy(priv, id.priv, sizeof(priv)); + crypto->setDHPrivateKey(priv); +} + +/** A minimal ROUTING ack, shaped the way MeshModule::allocAckNak builds one. */ +static meshtastic_MeshPacket makeAck(NodeNum from, NodeNum to, uint32_t requestId) +{ + meshtastic_MeshPacket p = meshtastic_MeshPacket_init_zero; + p.from = from; + p.to = to; + p.id = 0x5150; + p.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + p.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + p.decoded.request_id = requestId; + + meshtastic_Routing ack = meshtastic_Routing_init_default; + ack.which_variant = meshtastic_Routing_error_reason_tag; + ack.error_reason = meshtastic_Routing_Error_NONE; + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_Routing_msg, &ack); + TEST_ASSERT_GREATER_THAN(0, p.decoded.payload.size); + return p; +} + +void setUp(void) {} +void tearDown(void) {} + +// The whole scheme rests on this: both endpoints derive the same value from opposite halves of the +// key pair, and nobody else can. +void test_proof_is_pairwise_and_symmetric(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + const Identity mallory = makeIdentity(); + + uint8_t fromBob[ACK_PROOF_SIZE], fromAlice[ACK_PROOF_SIZE], fromMallory[ACK_PROOF_SIZE]; + + // Both endpoints compute over the SAME direction tuple (Bob is the acker); only the key half + // they hold differs. + becomeNode(bob); + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, fromBob)); + becomeNode(alice); + TEST_ASSERT_TRUE(crypto->ackProofCompute(bob.pub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, fromAlice)); + TEST_ASSERT_EQUAL_UINT8_ARRAY_MESSAGE(fromBob, fromAlice, ACK_PROOF_SIZE, + "both endpoints must derive the same proof from the shared secret"); + + // A third party holding both public keys cannot reproduce it. + becomeNode(mallory); + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, fromMallory)); + TEST_ASSERT_TRUE_MESSAGE(memcmp(fromBob, fromMallory, ACK_PROOF_SIZE) != 0, "a non-peer must not derive the proof"); +} + +// request_id is an input, so a captured proof cannot be aimed at a different pending packet - the +// retargeting problem that the signature scheme needed an explicit buffer change to solve. +void test_proof_binds_request_id(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + uint8_t a[ACK_PROOF_SIZE], b[ACK_PROOF_SIZE]; + becomeNode(bob); + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, a)); + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, BOB, ALICE, REQUEST_ID + 1, ROUTING, ROUTING_LEN, b)); + TEST_ASSERT_TRUE_MESSAGE(memcmp(a, b, ACK_PROOF_SIZE) != 0, "proof must differ per request_id"); +} + +// Without an identity there is nothing to prove with, and we must fail rather than emit a constant. +void test_proof_requires_private_key(void) +{ + const Identity peer = makeIdentity(); + uint8_t zero[32] = {0}, proof[ACK_PROOF_SIZE]; + crypto->setDHPrivateKey(zero); + TEST_ASSERT_FALSE(crypto->ackProofCompute(peer.pub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, proof)); +} + +// An all-zero peer key is a weak point; Curve25519::dh2 rejects it and so must we. +void test_proof_rejects_weak_peer_key(void) +{ + const Identity self = makeIdentity(); + becomeNode(self); + uint8_t zeroPub[32] = {0}, proof[ACK_PROOF_SIZE]; + TEST_ASSERT_FALSE(crypto->ackProofCompute(zeroPub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, proof)); +} + +// The proof rides as a protobuf field appended to the encoded Routing payload. A decoder still +// reads the ack correctly with it present - which held when ack_proof was an unknown field to this +// build and must keep holding now that it is generated, since older firmware sees it as unknown. +void test_wire_roundtrip_and_unknown_field_tolerance(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + const pb_size_t bare = ack.decoded.payload.size; + TEST_ASSERT_TRUE(ackProofAttachWithKey(&ack, alice.pub)); + + // Encoded cost is the proof plus a tag and a length byte, and only on acks. + TEST_ASSERT_EQUAL_MESSAGE(bare + ACK_PROOF_SIZE + 2, ack.decoded.payload.size, "proof must cost exactly 10 encoded bytes"); + + uint8_t extracted[ACK_PROOF_SIZE], expected[ACK_PROOF_SIZE]; + TEST_ASSERT_TRUE(ackProofExtract(ack.decoded.payload.bytes, ack.decoded.payload.size, extracted)); + uint8_t bareRouting[64]; + memcpy(bareRouting, ack.decoded.payload.bytes, bare); + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, BOB, ALICE, REQUEST_ID, bareRouting, bare, expected)); + TEST_ASSERT_EQUAL_UINT8_ARRAY(expected, extracted, ACK_PROOF_SIZE); + + // Parsing the ack is unaffected by the proof - the case older firmware, which does not know the + // field, will be in. + meshtastic_Routing decoded = meshtastic_Routing_init_default; + TEST_ASSERT_TRUE_MESSAGE( + pb_decode_from_bytes(ack.decoded.payload.bytes, ack.decoded.payload.size, &meshtastic_Routing_msg, &decoded), + "a decoder must still parse the Routing message with a proof present"); + TEST_ASSERT_EQUAL(meshtastic_Routing_error_reason_tag, decoded.which_variant); + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, decoded.error_reason); +} + +// A bare ack (every current firmware) is ABSENT, not INVALID - this is what keeps the scheme +// deployable alongside nodes that know nothing about it. +void test_verify_verdicts(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + const Identity mallory = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket bare = makeAck(BOB, ALICE, REQUEST_ID); + meshtastic_MeshPacket proven = makeAck(BOB, ALICE, REQUEST_ID); + TEST_ASSERT_TRUE(ackProofAttachWithKey(&proven, alice.pub)); + + becomeNode(alice); + TEST_ASSERT_EQUAL(AckProofResult::ABSENT, ackProofVerifyWithKey(&bare, REQUEST_ID, bob.pub)); + TEST_ASSERT_EQUAL(AckProofResult::VALID, ackProofVerifyWithKey(&proven, REQUEST_ID, bob.pub)); + + // Present but unjudgeable, because we hold no key for the acker. + TEST_ASSERT_EQUAL(AckProofResult::NO_KEY, ackProofVerifyWithKey(&proven, REQUEST_ID, nullptr)); + + // Right proof, wrong pending packet: retargeting is caught. + TEST_ASSERT_EQUAL(AckProofResult::INVALID, ackProofVerifyWithKey(&proven, REQUEST_ID + 1, bob.pub)); + + // Same ack attributed to a different signer's key. + TEST_ASSERT_EQUAL(AckProofResult::INVALID, ackProofVerifyWithKey(&proven, REQUEST_ID, mallory.pub)); +} + +// A forger with the channel PSK can rewrite the ack, but cannot produce the proof. +void test_verify_rejects_tampered_proof(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + TEST_ASSERT_TRUE(ackProofAttachWithKey(&ack, alice.pub)); + + becomeNode(alice); + TEST_ASSERT_EQUAL(AckProofResult::VALID, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub)); + ack.decoded.payload.bytes[ack.decoded.payload.size - 1] ^= 0x01; + TEST_ASSERT_EQUAL(AckProofResult::INVALID, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub)); +} + +// A truncated or over-long field is malformed, not a proof - an honest sender emits exactly +// ACK_PROOF_SIZE bytes, so anything else is rejected outright rather than compared. +void test_malformed_proof_field_is_absent(void) +{ + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + uint8_t junk[ACK_PROOF_SIZE - 1]; + memset(junk, 0x5A, sizeof(junk)); + + pb_ostream_t stream = pb_ostream_from_buffer(ack.decoded.payload.bytes + ack.decoded.payload.size, + sizeof(ack.decoded.payload.bytes) - ack.decoded.payload.size); + TEST_ASSERT_TRUE(pb_encode_tag(&stream, PB_WT_STRING, ACK_PROOF_FIELD_NUMBER)); + TEST_ASSERT_TRUE(pb_encode_string(&stream, junk, sizeof(junk))); + ack.decoded.payload.size += stream.bytes_written; + + uint8_t out[ACK_PROOF_SIZE]; + TEST_ASSERT_FALSE(ackProofExtract(ack.decoded.payload.bytes, ack.decoded.payload.size, out)); +} + +// Nothing is attached to packets that are not explicit acks, so no other traffic pays for this. +void test_attach_only_applies_to_routing_unicasts(void) +{ + const Identity peer = makeIdentity(); + const Identity self = makeIdentity(); + becomeNode(self); + + meshtastic_MeshPacket broadcast = makeAck(BOB, NODENUM_BROADCAST, REQUEST_ID); + TEST_ASSERT_FALSE_MESSAGE(ackProofAttachWithKey(&broadcast, peer.pub), "broadcasts have no single peer to prove to"); + + meshtastic_MeshPacket noRequestId = makeAck(BOB, ALICE, 0); + TEST_ASSERT_FALSE_MESSAGE(ackProofAttachWithKey(&noRequestId, peer.pub), "nothing to bind without a request_id"); + + meshtastic_MeshPacket text = makeAck(BOB, ALICE, REQUEST_ID); + text.decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP; + TEST_ASSERT_FALSE_MESSAGE(ackProofAttachWithKey(&text, peer.pub), "only ROUTING acks carry a proof"); +} + +// THE load-bearing property for multi-hop: a repeater that holds the channel key relays by +// decoding and re-encoding the Data message (perhapsDecode decodes in place, then perhapsEncode +// re-runs pb_encode on p->decoded for any packet not from us). That cycle strips unknown fields at +// the Data level - so the proof survives only because it is nested inside Data.payload, which is an +// opaque bytes field the decoder copies through verbatim. Both halves are asserted here: if anyone +// ever "simplifies" this by moving the proof up alongside xeddsa_signature, the second half fails. +void test_proof_survives_relay_reencode(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + TEST_ASSERT_TRUE(ackProofAttachWithKey(&ack, alice.pub)); + + // A field appended at the *Data* level, as a control for what a hop destroys. + uint8_t control[4] = {0xDE, 0xAD, 0xBE, 0xEF}; + uint8_t ctrlBuf[16]; + pb_ostream_t ctrl = pb_ostream_from_buffer(ctrlBuf, sizeof(ctrlBuf)); + TEST_ASSERT_TRUE(pb_encode_tag(&ctrl, PB_WT_STRING, 101)); // not a field of meshtastic_Data + TEST_ASSERT_TRUE(pb_encode_string(&ctrl, control, sizeof(control))); + + uint8_t wire[512]; + const size_t wireLen = pb_encode_to_bytes(wire, sizeof(wire), &meshtastic_Data_msg, &ack.decoded); + TEST_ASSERT_GREATER_THAN(0, wireLen); + memcpy(wire + wireLen, ctrlBuf, ctrl.bytes_written); + const size_t withControl = wireLen + ctrl.bytes_written; + + // Exactly what a repeater does with what it received. + meshtastic_MeshPacket relayed = meshtastic_MeshPacket_init_zero; + relayed.from = BOB; + relayed.to = ALICE; + relayed.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + TEST_ASSERT_TRUE(pb_decode_from_bytes(wire, withControl, &meshtastic_Data_msg, &relayed.decoded)); + uint8_t reencoded[512]; + const size_t reencodedLen = pb_encode_to_bytes(reencoded, sizeof(reencoded), &meshtastic_Data_msg, &relayed.decoded); + TEST_ASSERT_GREATER_THAN(0, reencodedLen); + + // The Data-level field is gone after one hop... + TEST_ASSERT_LESS_THAN_MESSAGE(withControl, reencodedLen, "a Data-level unknown field must NOT survive the relay re-encode"); + + // ...but the proof, nested in the payload blob, is still there and still verifies downstream. + meshtastic_MeshPacket downstream = meshtastic_MeshPacket_init_zero; + downstream.from = BOB; + downstream.to = ALICE; + downstream.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + TEST_ASSERT_TRUE(pb_decode_from_bytes(reencoded, reencodedLen, &meshtastic_Data_msg, &downstream.decoded)); + TEST_ASSERT_EQUAL(REQUEST_ID, downstream.decoded.request_id); + + becomeNode(alice); + TEST_ASSERT_EQUAL_MESSAGE(AckProofResult::VALID, ackProofVerifyWithKey(&downstream, REQUEST_ID, bob.pub), + "proof must survive a repeater's decode/re-encode cycle"); +} + +// REGRESSION: an ack and a nak for the same packet share from/to/portnum/request_id and differ +// only in the Routing payload. Channel crypto is CTR with no MAC, so if the payload were not bound +// a PSK holder could bit-flip a proven "delivered" into a "failed" and the proof would still +// verify. The easy direction is success -> failure; manufacturing a fake success is much narrower, +// needing a captured proofed nak, and the naks that exist either never reach the air or come from +// a node that by definition holds no key. +void test_proof_binds_error_reason(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + TEST_ASSERT_EQUAL_MESSAGE(0x18, ack.decoded.payload.bytes[0], "expected error_reason to be field 3, varint"); + TEST_ASSERT_EQUAL_MESSAGE(meshtastic_Routing_Error_NONE, ack.decoded.payload.bytes[1], "expected a success ack"); + TEST_ASSERT_TRUE(ackProofAttachWithKey(&ack, alice.pub)); + + becomeNode(alice); + TEST_ASSERT_EQUAL(AckProofResult::VALID, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub)); + + // Exactly what a CTR bit-flip buys the attacker: same proof bytes, different verdict. + ack.decoded.payload.bytes[1] = meshtastic_Routing_Error_MAX_RETRANSMIT; + TEST_ASSERT_EQUAL_MESSAGE(AckProofResult::INVALID, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub), + "flipping a proven ack into a nak must invalidate the proof"); +} + +// X25519 is symmetric - DH(a_priv, B_pub) == DH(b_priv, A_pub) - so without the direction bound an +// A->B proof for a request_id would equal the B->A proof for it. Cheap insurance; this pins it. +void test_proof_binds_direction(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + uint8_t forward[ACK_PROOF_SIZE], reverse[ACK_PROOF_SIZE]; + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, BOB, ALICE, REQUEST_ID, ROUTING, ROUTING_LEN, forward)); + TEST_ASSERT_TRUE(crypto->ackProofCompute(alice.pub, ALICE, BOB, REQUEST_ID, ROUTING, ROUTING_LEN, reverse)); + TEST_ASSERT_TRUE_MESSAGE(memcmp(forward, reverse, ACK_PROOF_SIZE) != 0, + "A->B and B->A proofs must differ despite the symmetric shared secret"); +} + +// Two proof fields mean two possible excisions and so two possible verdicts. Refuse rather than +// silently pick one, since an attacker with the channel key can append a second field at will. +void test_duplicate_proof_field_is_rejected(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + TEST_ASSERT_TRUE(ackProofAttachWithKey(&ack, alice.pub)); + + uint8_t first[ACK_PROOF_SIZE]; + TEST_ASSERT_TRUE(ackProofExtract(ack.decoded.payload.bytes, ack.decoded.payload.size, first)); + + // Append a second, byte-identical proof field. + pb_ostream_t stream = pb_ostream_from_buffer(ack.decoded.payload.bytes + ack.decoded.payload.size, + sizeof(ack.decoded.payload.bytes) - ack.decoded.payload.size); + TEST_ASSERT_TRUE(pb_encode_tag(&stream, PB_WT_STRING, ACK_PROOF_FIELD_NUMBER)); + TEST_ASSERT_TRUE(pb_encode_string(&stream, first, ACK_PROOF_SIZE)); + ack.decoded.payload.size += stream.bytes_written; + + uint8_t out[ACK_PROOF_SIZE]; + TEST_ASSERT_FALSE_MESSAGE(ackProofExtract(ack.decoded.payload.bytes, ack.decoded.payload.size, out), + "a message with two proof fields must be rejected as malformed"); + + becomeNode(alice); + TEST_ASSERT_EQUAL(AckProofResult::ABSENT, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub)); +} + +// The proof must cover fields this build does not understand, so that a future Routing field +// outside the oneof is protected and old and new nodes still agree on the hashed bytes. Excision +// gives that for free; re-encoding from the decoded struct would silently drop them. +void test_proof_covers_unknown_routing_fields(void) +{ + const Identity alice = makeIdentity(); + const Identity bob = makeIdentity(); + + becomeNode(bob); + meshtastic_MeshPacket ack = makeAck(BOB, ALICE, REQUEST_ID); + + // A field this build's Routing schema does not define, as a newer sender would emit. + const pb_size_t beforeUnknown = ack.decoded.payload.size; + uint8_t extra[3] = {0x11, 0x22, 0x33}; + pb_ostream_t pre = pb_ostream_from_buffer(ack.decoded.payload.bytes + ack.decoded.payload.size, + sizeof(ack.decoded.payload.bytes) - ack.decoded.payload.size); + TEST_ASSERT_TRUE(pb_encode_tag(&pre, PB_WT_STRING, 9)); // not a field of meshtastic_Routing + TEST_ASSERT_TRUE(pb_encode_string(&pre, extra, sizeof(extra))); + ack.decoded.payload.size += pre.bytes_written; + // Its contents start after the one-byte tag and one-byte length. + const size_t unknownDataStart = beforeUnknown + 2; + + TEST_ASSERT_TRUE(ackProofAttachWithKey(&ack, alice.pub)); + + becomeNode(alice); + TEST_ASSERT_EQUAL(AckProofResult::VALID, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub)); + + // Tamper with the field's CONTENTS, not its framing: the message must stay parseable so that + // the verdict is INVALID (the proof no longer matches) rather than ABSENT (no readable proof). + ack.decoded.payload.bytes[unknownDataStart] ^= 0x01; + TEST_ASSERT_EQUAL_MESSAGE(AckProofResult::INVALID, ackProofVerifyWithKey(&ack, REQUEST_ID, bob.pub), + "an unknown field must be covered by the proof"); +} + +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + RUN_TEST(test_proof_is_pairwise_and_symmetric); + RUN_TEST(test_proof_binds_request_id); + RUN_TEST(test_proof_binds_error_reason); + RUN_TEST(test_proof_binds_direction); + RUN_TEST(test_proof_requires_private_key); + RUN_TEST(test_proof_rejects_weak_peer_key); + RUN_TEST(test_wire_roundtrip_and_unknown_field_tolerance); + RUN_TEST(test_verify_verdicts); + RUN_TEST(test_verify_rejects_tampered_proof); + RUN_TEST(test_malformed_proof_field_is_absent); + RUN_TEST(test_duplicate_proof_field_is_rejected); + RUN_TEST(test_proof_covers_unknown_routing_fields); + RUN_TEST(test_attach_only_applies_to_routing_unicasts); + RUN_TEST(test_proof_survives_relay_reencode); + exit(UNITY_END()); +} + +void loop() {} + +#else // PKI excluded + +void setUp(void) {} +void tearDown(void) {} +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + exit(UNITY_END()); +} +void loop() {} + +#endif