diff --git a/SECURITY.md b/SECURITY.md index 34977e15a29..37d1ba84173 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -35,7 +35,7 @@ A client connected to a node over Bluetooth, USB serial, WiFi, or Ethernet has f There is no central authority to sign node keys. The first public key a node hears for a given node number is the one it binds to that node number, a Trust On First Use (TOFU) model that is a hard requirement of a decentralized mesh. Clients and firmware reduce the impact of this by keeping favorited nodes from rolling out of the node database and by flagging public-key changes in the client UI. -Firmware 2.8.X adds XEdDSA packet signing to further secure node identity claims and the authenticity of subsequent messages. It reuses each node's existing x25519 key pair to produce signatures, so a receiver can verify that a packet came from the holder of the bound key. Once a node has been seen signing, unsigned packets claiming that identity can be rejected. +Firmware 2.8.X adds XEdDSA packet signing to further secure node identity claims and the authenticity of subsequent messages. It reuses each node's existing x25519 key pair to produce signatures, so a receiver can verify that a packet came from the holder of the bound key. Once a node has been seen signing, unsigned packets claiming that identity can be rejected. Signatures also cover a packet's request/reply linkage, so a signed acknowledgement or reply cannot be retargeted at a different message, and nodes on the Strict signature policy additionally sign the explicit delivery acks/naks they send, allowing Strict peers to authenticate delivery reports. ### Known limitations diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 95c640d539c..a62f45f443a 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -89,31 +89,50 @@ bool CryptoEngine::regeneratePublicKey(uint8_t *pubKey, uint8_t *privKey) #if !(MESHTASTIC_EXCLUDE_XEDDSA) /** * Build a signing buffer that covers packet metadata and payload: - * [fromNode(4) | packetId(4) | portnum(4) | payload(N)] - * This prevents replay, reattribution, and portnum redirection attacks. + * [fromNode(4) | packetId(4) | portnum(4) | payload(N)] (base) + * [fromNode(4) | packetId(4) | portnum(4) | request_id(4) | reply_id(4) | payload(N)] (extended) + * The extended layout is used exactly when request_id or reply_id is nonzero, binding the + * request/reply linkage so a signed ack/reply cannot be retargeted at a different outstanding + * request (or a signed tapback re-pointed at a different message). Packets without either field + * keep the base layout, byte-identical to the pre-2.8.0 draft format, so their signatures stay + * verifiable across that boundary. Both sides derive the layout from the packet's own decoded + * fields, so no format flag is transmitted. The conditional layout is theoretically ambiguous (a + * base-layout payload could begin with bytes that parse as the extended header), but a forgery + * additionally requires identical fromNode/packetId/portnum and an honest signer emitting a + * zero-request packet with an attacker-useful payload prefix on the same portnum - signed + * ROUTING_APP packets always carry a request_id, so no such packet exists for the ack case. + * Covering the metadata prevents replay, reattribution, and portnum redirection attacks. */ static size_t buildSigningBuffer(uint8_t *buf, size_t bufSize, uint32_t fromNode, uint32_t packetId, uint32_t portnum, - const uint8_t *payload, size_t payloadLen) + uint32_t requestId, uint32_t replyId, const uint8_t *payload, size_t payloadLen) { - const size_t headerLen = sizeof(uint32_t) * 3; - size_t totalLen = headerLen + payloadLen; + static_assert(sizeof(uint32_t) * 5 + sizeof(meshtastic_Data_payload_t::bytes) <= MAX_BLOCKSIZE, + "signing buffer must hold the header plus a maximum Data payload"); + size_t headerLen = sizeof(uint32_t) * 3; + size_t totalLen = headerLen + payloadLen + (requestId != 0 || replyId != 0 ? sizeof(uint32_t) * 2 : 0); if (totalLen > bufSize) return 0; // May need endian conversion for oddball platforms. memcpy(buf, &fromNode, sizeof(uint32_t)); memcpy(buf + sizeof(uint32_t), &packetId, sizeof(uint32_t)); memcpy(buf + sizeof(uint32_t) * 2, &portnum, sizeof(uint32_t)); + if (requestId != 0 || replyId != 0) { + memcpy(buf + sizeof(uint32_t) * 3, &requestId, sizeof(uint32_t)); + memcpy(buf + sizeof(uint32_t) * 4, &replyId, sizeof(uint32_t)); + headerLen += sizeof(uint32_t) * 2; + } memcpy(buf + headerLen, payload, payloadLen); return totalLen; } -bool CryptoEngine::xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t portnum, const uint8_t *payload, size_t payloadLen, - uint8_t *signature) +bool CryptoEngine::xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t portnum, uint32_t requestId, uint32_t replyId, + const uint8_t *payload, size_t payloadLen, uint8_t *signature) { if (memfll(xeddsa_private_key, 0, sizeof(xeddsa_private_key))) return false; uint8_t sigBuf[MAX_BLOCKSIZE]; - size_t sigLen = buildSigningBuffer(sigBuf, sizeof(sigBuf), fromNode, packetId, portnum, payload, payloadLen); + size_t sigLen = + buildSigningBuffer(sigBuf, sizeof(sigBuf), fromNode, packetId, portnum, requestId, replyId, payload, payloadLen); if (sigLen == 0) return false; // XEdDSA::sign mixes signature[0..31] into the nonce as the spec's random Z (meshtastic/Crypto#3) @@ -126,7 +145,8 @@ bool CryptoEngine::xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t po } bool CryptoEngine::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) + uint32_t requestId, uint32_t replyId, const uint8_t *payload, size_t payloadLen, + const uint8_t *signature) { // Use cached Ed25519 key if the Curve25519 key matches, avoiding expensive field inversion if (memcmp(pubKey, cached_curve_pubkey, 32) != 0) { @@ -134,7 +154,8 @@ bool CryptoEngine::xeddsa_verify(const uint8_t *pubKey, uint32_t fromNode, uint3 memcpy(cached_curve_pubkey, pubKey, 32); } uint8_t sigBuf[MAX_BLOCKSIZE]; - size_t sigLen = buildSigningBuffer(sigBuf, sizeof(sigBuf), fromNode, packetId, portnum, payload, payloadLen); + size_t sigLen = + buildSigningBuffer(sigBuf, sizeof(sigBuf), fromNode, packetId, portnum, requestId, replyId, payload, payloadLen); if (sigLen == 0) return false; return XEdDSA::verify(signature, cached_ed_pubkey, sigBuf, sigLen); diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index 95c7eb8ece3..01386eedaf6 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -43,10 +43,10 @@ class CryptoEngine virtual bool ensurePkiKeys(meshtastic_Config_SecurityConfig &security, meshtastic_User &user); #endif #if !(MESHTASTIC_EXCLUDE_XEDDSA) - bool xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t portnum, const uint8_t *payload, size_t payloadLen, - uint8_t *signature); - 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); + bool xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t portnum, uint32_t requestId, uint32_t replyId, + const uint8_t *payload, size_t payloadLen, uint8_t *signature); + bool xeddsa_verify(const uint8_t *pubKey, uint32_t fromNode, uint32_t packetId, uint32_t portnum, uint32_t requestId, + uint32_t replyId, const uint8_t *payload, size_t payloadLen, const uint8_t *signature); #endif void setDHPrivateKey(uint8_t *_private_key); // The remotePublic key parameter takes the public_key bytes container from diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 4b2c426938c..43202008d5e 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -608,8 +608,9 @@ static NodeInfoBootstrapResult verifyFirstContactNodeInfo(meshtastic_MeshPacket meshtastic_User user = meshtastic_User_init_zero; if (!pb_decode_from_bytes(p->decoded.payload.bytes, p->decoded.payload.size, &meshtastic_User_msg, &user) || user.public_key.size != 32 || crc32Buffer(user.public_key.bytes, user.public_key.size) != p->from || - !crypto->xeddsa_verify(user.public_key.bytes, p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, - p->decoded.payload.size, p->decoded.xeddsa_signature.bytes)) { + !crypto->xeddsa_verify(user.public_key.bytes, p->from, p->id, p->decoded.portnum, p->decoded.request_id, + p->decoded.reply_id, p->decoded.payload.bytes, p->decoded.payload.size, + p->decoded.xeddsa_signature.bytes)) { return NodeInfoBootstrapResult::INVALID; } @@ -638,9 +639,9 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p) // Authoritative keys only: verifying against an opportunistic cache key would let a planted // key mark its own node a signer, the trust loop #11116 closed on the decrypt path. if (nodeDB->copyPublicKeyAuthoritative(p->from, senderKey)) { - p->xeddsa_signed = - crypto->xeddsa_verify(senderKey.bytes, p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, - p->decoded.payload.size, p->decoded.xeddsa_signature.bytes); + p->xeddsa_signed = crypto->xeddsa_verify(senderKey.bytes, p->from, p->id, p->decoded.portnum, p->decoded.request_id, + p->decoded.reply_id, p->decoded.payload.bytes, p->decoded.payload.size, + p->decoded.xeddsa_signature.bytes); if (p->xeddsa_signed) { // Learn this node as a signer, so a later unsigned signable broadcast from it is dropped // A warm-tier key must be re-admitted before setting the signer bit; otherwise Balanced @@ -686,7 +687,11 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p) return true; // Balanced rejects only what a signer always signs: non-PKI broadcasts whose signed encoding - // would have fit, plus unicasts on ham where licensed senders sign too. Mirrors perhapsEncode. + // would have fit, plus unicasts on ham where licensed senders sign too. Mirrors perhapsEncode + // EXCEPT for its Strict-signs-acks clause, which is deliberately not mirrored: a receiver + // cannot know the sender's policy, so an unsigned unicast ack must never be treated as a + // downgrade - "fixing" that asymmetry here would drop every ack from Balanced/Compatible + // senders the moment they are known signers. if (nodeDB->isKnownXeddsaSigner(p->from) && (isBroadcast(p->to) || owner.is_licensed)) { size_t canonicalSize; if (!canonicalSignableSize(&p->decoded, &canonicalSize)) @@ -1082,14 +1087,22 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) // verification at every XEdDSA-enabled receiver that knows our key. p->decoded.xeddsa_signature.size = 0; #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) - // Licensed packets stay plaintext, so sign both broadcasts and unicasts. Normal mode - // continues to sign broadcasts only. Use the exact encoded size: a payload-size heuristic - // where we sign-then-fail-TOO_LARGE breaks packets that - // were deliverable unsigned, and perhapsDecode() applies the mirror-image rule when - // deciding whether an unsigned broadcast from a known signer is a downgrade. - if (!p->pki_encrypted && (owner.is_licensed || isBroadcast(p->to)) && signedDataFits(&p->decoded)) { - if (crypto->xeddsa_sign(p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, p->decoded.payload.size, - p->decoded.xeddsa_signature.bytes)) { + // Three classes get signed: broadcasts (normal mode), everything when licensed (ham + // packets stay plaintext, so unicasts are signed too), and explicit acks/naks under the + // Strict policy. Strict receivers drop unsigned non-PKI packets, so without signed acks + // two Strict peers can never complete reliable delivery. An explicit ack/nak is exactly + // a self-originated ROUTING_APP unicast: RoutingModule::allocReply() returns NULL, so + // every such packet reaching this gate came from MeshModule::allocAckNak. Use the exact + // encoded size: a payload-size heuristic where we sign-then-fail-TOO_LARGE breaks + // packets that were deliverable unsigned, and perhapsDecode() applies the mirror-image + // rule when deciding whether an unsigned broadcast from a known signer is a downgrade. + const bool strictSignsAck = + config.security.packet_signature_policy == + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT && + p->decoded.portnum == meshtastic_PortNum_ROUTING_APP && !isBroadcast(p->to); + if (!p->pki_encrypted && (owner.is_licensed || isBroadcast(p->to) || strictSignsAck) && signedDataFits(&p->decoded)) { + if (crypto->xeddsa_sign(p->from, p->id, p->decoded.portnum, p->decoded.request_id, p->decoded.reply_id, + p->decoded.payload.bytes, p->decoded.payload.size, p->decoded.xeddsa_signature.bytes)) { p->decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; LOG_DEBUG("XEdDSA signed packet 0x%08x", p->id); } diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index 448942ffe62..f11d6bfa168 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -178,6 +178,9 @@ void test_XEdDSA(void) uint32_t fromNode = 0x1234; uint32_t packetId = 0xDEADBEEF; uint32_t portnum = 1; + // Nonzero on purpose so the v2 buffer's request/reply binding is exercised, not just zero-padded. + uint32_t requestId = 0xCAFE0001; + uint32_t replyId = 0xCAFE0002; for (int times = 0; times < 10; times++) { printf("Start of time %u\n", times); crypto->generateKeyPair(x_public_key, private_key); @@ -186,24 +189,33 @@ void test_XEdDSA(void) TEST_ASSERT_EQUAL_MEMORY(ed_public_key, ed_public_key2, 32); // Sign and verify with metadata - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), signature)); - TEST_ASSERT(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, message, sizeof(message), signature)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), signature)); + TEST_ASSERT(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), + signature)); // Different payload fails - TEST_ASSERT_FALSE( - crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, message2, sizeof(message2), signature)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, requestId, replyId, message2, + sizeof(message2), signature)); // Different fromNode fails - TEST_ASSERT_FALSE( - crypto->xeddsa_verify(x_public_key, fromNode + 1, packetId, portnum, message, sizeof(message), signature)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, fromNode + 1, packetId, portnum, requestId, replyId, message, + sizeof(message), signature)); // Different packetId fails - TEST_ASSERT_FALSE( - crypto->xeddsa_verify(x_public_key, fromNode, packetId + 1, portnum, message, sizeof(message), signature)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, fromNode, packetId + 1, portnum, requestId, replyId, message, + sizeof(message), signature)); // Different portnum fails - TEST_ASSERT_FALSE( - crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum + 1, message, sizeof(message), signature)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum + 1, requestId, replyId, message, + sizeof(message), signature)); + + // Retargeting an ack/reply at a different request fails + TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, requestId + 1, replyId, message, + sizeof(message), signature)); + + // Re-pointing a reply/tapback at a different message fails + TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, requestId, replyId + 1, message, + sizeof(message), signature)); } } @@ -214,18 +226,21 @@ void test_XEdDSA_cross_key_reject(void) uint8_t pubB[32], privB[32]; uint8_t signature[64]; uint8_t message[] = "cross-key check"; - uint32_t fromNode = 0x4242, packetId = 0xABCD1234, portnum = 7; + uint32_t fromNode = 0x4242, packetId = 0xABCD1234, portnum = 7, requestId = 0x77, replyId = 0x88; crypto->generateKeyPair(pubA, privA); // engine now holds key A - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), signature)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), signature)); crypto->generateKeyPair(pubB, privB); // unrelated key pair - TEST_ASSERT_TRUE(crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, message, sizeof(message), signature)); - TEST_ASSERT_FALSE(crypto->xeddsa_verify(pubB, fromNode, packetId, portnum, message, sizeof(message), signature)); + TEST_ASSERT_TRUE( + crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), signature)); + TEST_ASSERT_FALSE( + crypto->xeddsa_verify(pubB, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), signature)); uint8_t zeroKey[32] = {0}; - TEST_ASSERT_FALSE(crypto->xeddsa_verify(zeroKey, fromNode, packetId, portnum, message, sizeof(message), signature)); + TEST_ASSERT_FALSE( + crypto->xeddsa_verify(zeroKey, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), signature)); } // Signing with an unset (all-zero) private key must fail rather than emit a bogus signature. @@ -234,7 +249,7 @@ void test_XEdDSA_empty_key_sign_fails(void) CryptoEngine fresh; // freshly constructed: xeddsa_private_key is all zero uint8_t signature[64]; uint8_t message[] = "no key"; - TEST_ASSERT_FALSE(fresh.xeddsa_sign(0x1, 0x2, 0x3, message, sizeof(message), signature)); + TEST_ASSERT_FALSE(fresh.xeddsa_sign(0x1, 0x2, 0x3, 0x4, 0x5, message, sizeof(message), signature)); } // curve_to_ed_pub caches the last converted key; verifying A, then B, then A must stay correct. @@ -243,18 +258,22 @@ void test_XEdDSA_curve_to_ed_cache(void) uint8_t pubA[32], privA[32], sigA[64]; uint8_t pubB[32], privB[32], sigB[64]; uint8_t message[] = "cache check"; - uint32_t fromNode = 0x11, packetId = 0x22, portnum = 3; + uint32_t fromNode = 0x11, packetId = 0x22, portnum = 3, requestId = 0x44, replyId = 0x55; crypto->generateKeyPair(pubA, privA); - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), sigA)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sigA)); crypto->generateKeyPair(pubB, privB); - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), sigB)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sigB)); // Interleave keys to exercise both cache hits and cache invalidation. - TEST_ASSERT_TRUE(crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, message, sizeof(message), sigA)); - TEST_ASSERT_TRUE(crypto->xeddsa_verify(pubB, fromNode, packetId, portnum, message, sizeof(message), sigB)); - TEST_ASSERT_TRUE(crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, message, sizeof(message), sigA)); - TEST_ASSERT_FALSE(crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, message, sizeof(message), sigB)); + TEST_ASSERT_TRUE( + crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sigA)); + TEST_ASSERT_TRUE( + crypto->xeddsa_verify(pubB, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sigB)); + TEST_ASSERT_TRUE( + crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sigA)); + TEST_ASSERT_FALSE( + crypto->xeddsa_verify(pubA, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sigB)); } // A payload at the maximum signable size (DATA_PAYLOAD_LEN - signature) round-trips and detects tampering. @@ -267,12 +286,12 @@ void test_XEdDSA_max_payload(void) uint8_t pub[32], priv[32], signature[64]; crypto->generateKeyPair(pub, priv); - uint32_t fromNode = 0xFEED, packetId = 0xC0DE, portnum = 1; + uint32_t fromNode = 0xFEED, packetId = 0xC0DE, portnum = 1, requestId = 0xF00D, replyId = 0xBEAD; - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, payload, len, signature)); - TEST_ASSERT(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, payload, len, signature)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, payload, len, signature)); + TEST_ASSERT(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, requestId, replyId, payload, len, signature)); payload[0] ^= 0x01; - TEST_ASSERT_FALSE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, payload, len, signature)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, requestId, replyId, payload, len, signature)); } // XEdDSA is a randomized (hedged) scheme: the nonce mixes in Z, caller-supplied randomness @@ -284,16 +303,52 @@ void test_XEdDSA_repeated_sign_is_randomized(void) { uint8_t pub[32], priv[32], sig1[64], sig2[64]; uint8_t message[] = "same message"; - uint32_t fromNode = 0x9, packetId = 0x9, portnum = 9; + uint32_t fromNode = 0x9, packetId = 0x9, portnum = 9, requestId = 0x9, replyId = 0x9; crypto->generateKeyPair(pub, priv); - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), sig1)); - TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), sig2)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sig1)); + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sig2)); TEST_ASSERT_TRUE_MESSAGE(memcmp(sig1, sig2, sizeof(sig1)) != 0, "signatures must differ - XEdDSA Z randomization is not wired through"); - TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, message, sizeof(message), sig1)); - TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, message, sizeof(message), sig2)); + TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sig1)); + TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, requestId, replyId, message, sizeof(message), sig2)); +} + +// A packet with no request/reply linkage keeps the base signing layout, byte-identical to the +// pre-request_id-binding format: a signature built by hand over [from|id|portnum|payload] - what +// an existing 2.8 draft signer emits - must verify through the current engine with 0/0 fields, +// and must NOT verify when reinterpreted with nonzero request/reply fields (or vice versa). +void test_XEdDSA_legacy_layout_compat(void) +{ + uint8_t pub[32], priv[32], ed_priv[32], ed_pub[32]; + uint8_t message[] = "legacy signer"; + uint8_t signature[64]; + uint32_t fromNode = 0x77, packetId = 0x1CEB00DA, portnum = 1; + + crypto->generateKeyPair(pub, priv); + XEdDSA::priv_curve_to_ed_keys(priv, ed_priv, ed_pub); + + // Hand-build the base-format buffer exactly as pre-binding firmware does. + uint8_t legacyBuf[12 + sizeof(message)]; + memcpy(legacyBuf, &fromNode, 4); + memcpy(legacyBuf + 4, &packetId, 4); + memcpy(legacyBuf + 8, &portnum, 4); + memcpy(legacyBuf + 12, message, sizeof(message)); + memset(signature, 0x42, 32); // hedge nonce seed, any value works + XEdDSA::sign(signature, ed_priv, ed_pub, legacyBuf, sizeof(legacyBuf)); + + // Legacy signature verifies through the new engine when both fields are zero... + TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, 0, 0, message, sizeof(message), signature)); + // ...and cannot be re-framed as a signature over a request/reply-bearing packet. + TEST_ASSERT_FALSE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, 0xA1, 0, message, sizeof(message), signature)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, 0, 0xB2, message, sizeof(message), signature)); + + // The mirror image: an extended-format signature must not verify with the fields zeroed. + uint8_t extSig[64]; + TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, 0xA1, 0xB2, message, sizeof(message), extSig)); + TEST_ASSERT_TRUE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, 0xA1, 0xB2, message, sizeof(message), extSig)); + TEST_ASSERT_FALSE(crypto->xeddsa_verify(pub, fromNode, packetId, portnum, 0, 0, message, sizeof(message), extSig)); } void test_AES_CTR(void) @@ -380,6 +435,7 @@ void setup() RUN_TEST(test_XEdDSA_curve_to_ed_cache); RUN_TEST(test_XEdDSA_max_payload); RUN_TEST(test_XEdDSA_repeated_sign_is_randomized); + RUN_TEST(test_XEdDSA_legacy_layout_compat); exit(UNITY_END()); // stop unit testing } diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index e2d006e3820..25683dc92ac 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -762,8 +762,8 @@ void test_receiveVerifiesSignedDecodedDownlink(void) memcpy(mockNodeDB->emptyNode.public_key.bytes, pub, 32); meshtastic_MeshPacket p = makeDecodedBroadcast(); - TEST_ASSERT_TRUE(crypto->xeddsa_sign(p.from, p.id, p.decoded.portnum, p.decoded.payload.bytes, p.decoded.payload.size, - p.decoded.xeddsa_signature.bytes)); + TEST_ASSERT_TRUE(crypto->xeddsa_sign(p.from, p.id, p.decoded.portnum, p.decoded.request_id, p.decoded.reply_id, + p.decoded.payload.bytes, p.decoded.payload.size, p.decoded.xeddsa_signature.bytes)); p.decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; unitTest->publish(&p); @@ -773,6 +773,47 @@ void test_receiveVerifiesSignedDecodedDownlink(void) TEST_ASSERT_TRUE(mockNodeDB->emptyNode.bitfield & NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK); } +// A signed unicast ROUTING ack from a Strict sender, delivered decoded by a plaintext broker, +// verifies at ingress with its request_id binding intact; retargeting the same signed ack at a +// different request_id must be dropped. +void test_receiveVerifiesSignedAckAndDropsRetargeted(void) +{ + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->emptyNode.public_key.size = 32; + memcpy(mockNodeDB->emptyNode.public_key.bytes, pub, 32); + + meshtastic_MeshPacket p = makeDecodedBroadcast(); + p.to = myNodeInfo.my_node_num; + p.decoded.portnum = meshtastic_PortNum_ROUTING_APP; + p.decoded.request_id = 0xA5A5A5A5; + 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_TRUE(crypto->xeddsa_sign(p.from, p.id, p.decoded.portnum, p.decoded.request_id, p.decoded.reply_id, + p.decoded.payload.bytes, p.decoded.payload.size, p.decoded.xeddsa_signature.bytes)); + p.decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; + + unitTest->publish(&p); + + TEST_ASSERT_EQUAL(1, mockRouter->packets_.size()); + TEST_ASSERT_TRUE(mockRouter->packets_.front().xeddsa_signed); + + // Same signed bytes, aimed at a different outstanding request: verification must fail. + meshtastic_MeshPacket retargeted = p; + retargeted.id++; // dodge any dedup by id; the signature binds the original id too, but the + // point pinned here is the request_id binding + TEST_ASSERT_TRUE(crypto->xeddsa_sign( + retargeted.from, retargeted.id, retargeted.decoded.portnum, retargeted.decoded.request_id, retargeted.decoded.reply_id, + retargeted.decoded.payload.bytes, retargeted.decoded.payload.size, retargeted.decoded.xeddsa_signature.bytes)); + retargeted.decoded.request_id = 0x5A5A5A5A; // retarget after signing + unitTest->publish(&retargeted); + + TEST_ASSERT_EQUAL(1, mockRouter->packets_.size()); // still only the first ack +} + // A decoded downlink carrying a signature that fails verification is dropped. void test_receiveDropsBadSignatureOnDecodedDownlink(void) { @@ -784,8 +825,8 @@ void test_receiveDropsBadSignatureOnDecodedDownlink(void) memcpy(mockNodeDB->emptyNode.public_key.bytes, pub, 32); meshtastic_MeshPacket p = makeDecodedBroadcast(); - TEST_ASSERT_TRUE(crypto->xeddsa_sign(p.from, p.id, p.decoded.portnum, p.decoded.payload.bytes, p.decoded.payload.size, - p.decoded.xeddsa_signature.bytes)); + TEST_ASSERT_TRUE(crypto->xeddsa_sign(p.from, p.id, p.decoded.portnum, p.decoded.request_id, p.decoded.reply_id, + p.decoded.payload.bytes, p.decoded.payload.size, p.decoded.xeddsa_signature.bytes)); p.decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; p.decoded.xeddsa_signature.bytes[0] ^= 0xFF; @@ -1160,6 +1201,7 @@ void setup() RUN_TEST(test_receiveDropsUnsignedBroadcastFromSigner); RUN_TEST(test_receiveAcceptsUnsignedBroadcastFromNonSigner); RUN_TEST(test_receiveVerifiesSignedDecodedDownlink); + RUN_TEST(test_receiveVerifiesSignedAckAndDropsRetargeted); RUN_TEST(test_receiveDropsBadSignatureOnDecodedDownlink); RUN_TEST(test_receiveCompatibleAcceptsUnsignedBroadcastFromSigner); RUN_TEST(test_receiveStrictDropsUnsignedPortnumsAndUnicast); diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index c3abb7bc957..fd4ed5ca2a5 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -33,6 +33,9 @@ #include "modules/NodeInfoModule.h" #include "modules/RoutingModule.h" #include "mqtt/MQTT.h" +#ifdef ARCH_PORTDUINO +#include "platform/portduino/PortduinoGlue.h" +#endif #include #include #include @@ -243,8 +246,8 @@ static meshtastic_MeshPacket makeDecoded(NodeNum from, NodeNum to, meshtastic_Po // because perhapsEncode only auto-signs packets that originate from us. static void signWithCurrentKey(meshtastic_MeshPacket *p) { - bool ok = crypto->xeddsa_sign(p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, p->decoded.payload.size, - p->decoded.xeddsa_signature.bytes); + bool ok = crypto->xeddsa_sign(p->from, p->id, p->decoded.portnum, p->decoded.request_id, p->decoded.reply_id, + p->decoded.payload.bytes, p->decoded.payload.size, p->decoded.xeddsa_signature.bytes); TEST_ASSERT_TRUE_MESSAGE(ok, "xeddsa_sign failed in test setup"); p->decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE; } @@ -398,6 +401,14 @@ void setUp(void) setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED); myNodeInfo.my_node_num = LOCAL_NODE; // drives isFromUs()/getFrom()/isToUs() +#ifdef ARCH_PORTDUINO + // The native test harness boots Portduino in simulated mode (`-s` in test_testing_command), and + // wouldEncryptWithPKC() hard-disables PKC whenever force_simradio is set - so B11/B12, which + // assert the PKC unicast path, can never pass under it. Model a real (non-sim) device instead, + // the same workaround test_admin_session_repro documents in its setUp. + portduino_config.force_simradio = false; +#endif + // Working primary channel with the default PSK so encrypt/decrypt round-trips. channels.initDefaults(); channels.onConfigChanged(); @@ -687,6 +698,32 @@ void test_A14_strict_bootstraps_identity_bound_signed_nodeinfo(void) TEST_ASSERT_TRUE(p.xeddsa_signed); } +// A solicited first-contact NodeInfo (a want_response reply, so request_id != 0) must still +// bootstrap: the v2 signing buffer binds request_id on both ends, so a nonzero value has to +// round-trip through verifyFirstContactNodeInfo exactly like the unsolicited 0/0 case in A14. +void test_A14b_strict_bootstraps_solicited_signed_nodeinfo(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + const NodeNum signer = crc32Buffer(pub, sizeof(pub)); + + meshtastic_User user = meshtastic_User_init_zero; + user.public_key.size = sizeof(pub); + memcpy(user.public_key.bytes, pub, sizeof(pub)); + meshtastic_MeshPacket p = makeDecoded(signer, LOCAL_NODE, meshtastic_PortNum_NODEINFO_APP, 0); + p.decoded.request_id = 0x600DCAFE; + p.decoded.payload.size = + pb_encode_to_bytes(p.decoded.payload.bytes, sizeof(p.decoded.payload.bytes), &meshtastic_User_msg, &user); + signWithCurrentKey(&p); + + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); + const meshtastic_NodeInfoLite *node = mockNodeDB->getMeshNode(signer); + TEST_ASSERT_NOT_NULL(node); + TEST_ASSERT_EQUAL_UINT8_ARRAY(pub, node->public_key.bytes, sizeof(pub)); + TEST_ASSERT_TRUE(p.xeddsa_signed); +} + void test_A15_strict_rejects_nodeinfo_key_without_identity_binding(void) { setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); @@ -912,6 +949,9 @@ void test_B6_rich_shape_sweep_no_deadband(void) TEST_ASSERT_TRUE_MESSAGE(sawUnsigned, "rich sweep never crossed the fit boundary"); } +// Scope: the suite's default BALANCED policy (setUp). Under Balanced/Compatible, infrastructure +// unicasts - acks included - stay unsigned for OTA interop; the Strict-signs-acks behavior is +// pinned separately in B14/B15. void test_B7_infrastructure_port_signing_matrix(void) { uint8_t pub[32], priv[32]; @@ -1066,6 +1106,62 @@ void test_B13_licensed_port_and_destination_signing_matrix(void) } } +// B14: Strict signs explicit acks/naks - the self-originated ROUTING_APP unicasts - so Strict +// peers (which drop unsigned non-PKI packets) can authenticate delivery reports. Built from a +// real allocAckNak product to pin the "explicit ack == self-originated ROUTING unicast" +// equivalence the sign gate relies on. Non-ack infrastructure unicasts must stay unsigned under +// Strict: their roundTrip lands in the Strict unsigned-drop, which would be impossible had +// perhapsEncode signed them (their key is registered, so a signature would have verified). +void test_B14_strict_signs_explicit_acks(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, pub); + + meshtastic_MeshPacket *ack = pipelineRouting->allocAckNak(meshtastic_Routing_Error_NONE, REMOTE_NODE, 0xABCD1234, 0, 0); + TEST_ASSERT_NOT_NULL_MESSAGE(ack, "allocAckNak failed in test setup"); + meshtastic_MeshPacket p = *ack; + packetPool.release(ack); + TEST_ASSERT_EQUAL(LOCAL_NODE, getFrom(&p)); + TEST_ASSERT_EQUAL(0xABCD1234, p.decoded.request_id); + TEST_ASSERT_TRUE_MESSAGE(signedEncodingFits(&p.decoded), "an ack plus signature must always fit a frame"); + + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); + TEST_ASSERT_EQUAL_MESSAGE(XEDDSA_SIGNATURE_SIZE, p.decoded.xeddsa_signature.size, "Strict must sign the explicit ack"); + TEST_ASSERT_TRUE(p.xeddsa_signed); + + const meshtastic_PortNum nonAckPorts[] = { + meshtastic_PortNum_NODEINFO_APP, + meshtastic_PortNum_TRACEROUTE_APP, + meshtastic_PortNum_POSITION_APP, + }; + for (const auto port : nonAckPorts) { + meshtastic_MeshPacket unicast = makeDecoded(LOCAL_NODE, REMOTE_NODE, port, SMALL_PAYLOAD); + TEST_ASSERT_EQUAL_MESSAGE(DECODE_POLICY_REJECT, roundTrip(&unicast), + "non-ack infrastructure unicasts must stay unsigned under Strict"); + } +} + +// B15: only Strict changes ack signing - Balanced and Compatible keep today's unsigned acks so +// default meshes stay interoperable with every deployed receiver. +void test_B15_balanced_and_compatible_do_not_sign_acks(void) +{ + const meshtastic_Config_SecurityConfig_PacketSignaturePolicy policies[] = { + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED, + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE, + }; + for (const auto policy : policies) { + setPolicy(policy); + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_ROUTING_APP, SMALL_PAYLOAD); + p.decoded.request_id = 0x11112222; + TEST_ASSERT_EQUAL(DECODE_SUCCESS, roundTrip(&p)); + TEST_ASSERT_EQUAL_MESSAGE(0, p.decoded.xeddsa_signature.size, "non-Strict policies must not sign acks"); + TEST_ASSERT_FALSE(p.xeddsa_signed); + } +} + // =========================================================================== // Group C - routing pipeline and NodeInfo authentication ordering // =========================================================================== @@ -1898,6 +1994,49 @@ void test_E13_decoded_unsigned_nodeinfo_padded_inside_payload_dropped(void) TEST_ASSERT_FALSE(p.xeddsa_signed); } +// E14: the v2 signing buffer binds request_id. Channel crypto is CTR without a MAC, so without +// this binding a signed ack's request_id would be malleable in flight - an attacker could aim a +// captured "delivered" ack at a different outstanding request. The retargeted copy must drop. +void test_E14_decoded_signed_ack_retargeted_request_id_dropped(void) +{ + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, pub); + + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, LOCAL_NODE, meshtastic_PortNum_ROUTING_APP, SMALL_PAYLOAD); + p.decoded.request_id = 0xAAAA5555; + signWithCurrentKey(&p); + + TEST_ASSERT_TRUE(checkXeddsaReceivePolicy(&p)); + TEST_ASSERT_TRUE(p.xeddsa_signed); + + p.decoded.request_id ^= 1; // same signed bytes, aimed at a different request + TEST_ASSERT_FALSE_MESSAGE(checkXeddsaReceivePolicy(&p), "retargeted ack must fail verification"); + TEST_ASSERT_FALSE(p.xeddsa_signed); +} + +// E15: same binding for reply_id - a signed reply/tapback cannot be re-pointed at a different +// message. +void test_E15_decoded_signed_reply_retargeted_reply_id_dropped(void) +{ + uint8_t pub[32], priv[32]; + crypto->generateKeyPair(pub, priv); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, pub); + + meshtastic_MeshPacket p = makeDecoded(REMOTE_NODE, NODENUM_BROADCAST, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + p.decoded.reply_id = 0x5555AAAA; + signWithCurrentKey(&p); + + TEST_ASSERT_TRUE(checkXeddsaReceivePolicy(&p)); + TEST_ASSERT_TRUE(p.xeddsa_signed); + + p.decoded.reply_id ^= 1; // re-point the tapback at a different message + TEST_ASSERT_FALSE_MESSAGE(checkXeddsaReceivePolicy(&p), "retargeted reply must fail verification"); + TEST_ASSERT_FALSE(p.xeddsa_signed); +} + void setup() { initializeTestEnvironment(); @@ -1938,6 +2077,7 @@ void setup() RUN_TEST(test_A13_strict_accepts_locally_authenticated_pki_packet); RUN_TEST(test_A13b_strict_rejects_spoofed_pki_flag_on_encrypted_ingress); RUN_TEST(test_A14_strict_bootstraps_identity_bound_signed_nodeinfo); + RUN_TEST(test_A14b_strict_bootstraps_solicited_signed_nodeinfo); RUN_TEST(test_A15_strict_rejects_nodeinfo_key_without_identity_binding); RUN_TEST(test_A16_compatible_rejects_invalid_first_contact_nodeinfo); #if WARM_NODE_COUNT > 0 @@ -1960,6 +2100,8 @@ void setup() RUN_TEST(test_B11_normal_unicast_still_uses_pki); RUN_TEST(test_B12_licensed_receiver_does_not_decrypt_pki); RUN_TEST(test_B13_licensed_port_and_destination_signing_matrix); + RUN_TEST(test_B14_strict_signs_explicit_acks); + RUN_TEST(test_B15_balanced_and_compatible_do_not_sign_acks); printf("\n=== Group C: routing pipeline authentication ordering ===\n"); RUN_TEST(test_C1_invalid_first_copy_does_not_poison_valid_same_id); @@ -2007,6 +2149,8 @@ void setup() RUN_TEST(test_E11_decoded_unsigned_oversized_telemetry_from_signer_accepted); RUN_TEST(test_E12_decoded_unsigned_waypoint_padded_inside_payload_dropped); RUN_TEST(test_E13_decoded_unsigned_nodeinfo_padded_inside_payload_dropped); + RUN_TEST(test_E14_decoded_signed_ack_retargeted_request_id_dropped); + RUN_TEST(test_E15_decoded_signed_reply_retargeted_reply_id_dropped); const int result = UNITY_END(); airTime = savedAirTime;