Sign explicit acks under Strict; bind request_id/reply_id in the XEdDSA signing buffer

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
 

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,15 +145,17 @@ 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) {
         curve_to_ed_pub(pubKey, cached_ed_pubkey);
         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);

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

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);
                 }