fix(security): reverify lost routing auth handoffs

Author: RCGV1

src/mesh/FloodingRouter.cpp

@@ -68,9 +68,7 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p)
 {
     // isRebroadcaster() is duplicated in perhapsRebroadcast(), but this avoids confusing log messages
     if (isRebroadcaster() && iface && p->hop_limit > 0) {
-        // Verify the replacement before deleting the valid lower-hop copy waiting in the TX queue.
-        // This is intentionally redundant with ReliableRouter's ingress gate: it keeps this helper
-        // safe if another caller is introduced later.
+        // Re-authenticate before replacing the queued lower-hop copy so future callers remain safe.
         if (passesRoutingAuthGate(p) != RoutingAuthVerdict::ACCEPT)
             return true;
 

src/mesh/Router.cpp

@@ -96,6 +96,8 @@ static bool routingAuthCacheMatches(const meshtastic_MeshPacket &packet)
 
 static void storeRoutingAuthCache(const meshtastic_MeshPacket &wire, const meshtastic_MeshPacket &authenticated)
 {
+    if (!routingAuthCacheLock)
+        return;
     concurrency::LockGuard guard(routingAuthCacheLock);
     routingAuthCache.wire = wire;
     routingAuthCache.authenticated = authenticated;
@@ -688,55 +690,60 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p)
 }
 #endif
 
-RoutingAuthVerdict passesRoutingAuthGate(const meshtastic_MeshPacket *p)
+static RoutingAuthVerdict evaluateRoutingAuth(const meshtastic_MeshPacket &wire, meshtastic_MeshPacket &authenticated)
 {
-    // Routing still needs the original encrypted representation for byte-for-byte relay and for
-    // MQTT uplink. Authenticate a copy here; handleReceived() performs the normal in-place decode
-    // only after stateful routing filters have completed.
-    if (routingAuthCacheMatches(*p))
-        return RoutingAuthVerdict::ACCEPT;
-
-    meshtastic_MeshPacket wire = *p;
-    meshtastic_MeshPacket authCandidate = *p;
+    authenticated = wire;
     routingAuthEvaluations++;
-    if (authCandidate.which_payload_variant == meshtastic_MeshPacket_decoded_tag) {
+    if (authenticated.which_payload_variant == meshtastic_MeshPacket_decoded_tag) {
         // Already-decoded remote ingress (notably Portduino SimRadio) did not pass through a
         // decryptor. Never trust serialized local authentication metadata on that boundary.
-        authCandidate.pki_encrypted = false;
-        authCandidate.public_key.size = 0;
-        authCandidate.xeddsa_signed = false;
+        authenticated.pki_encrypted = false;
+        authenticated.public_key.size = 0;
+        authenticated.xeddsa_signed = false;
 #if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA)
         concurrency::LockGuard g(cryptLock);
-        if (!checkXeddsaReceivePolicy(&authCandidate)) {
-            LOG_WARN("Already-decoded packet rejected by signature policy");
+        if (!checkXeddsaReceivePolicy(&authenticated)) {
+            LOG_WARN("Already-decoded packet rejected by routing signature policy");
             return RoutingAuthVerdict::REJECT;
         }
 #endif
-        wire = *p;
-        storeRoutingAuthCache(wire, authCandidate);
         return RoutingAuthVerdict::ACCEPT;
     }
-    const DecodeState state = perhapsDecode(&authCandidate);
+    const DecodeState state = perhapsDecode(&authenticated);
     if (state == DecodeState::DECODE_POLICY_REJECT) {
-        LOG_WARN("Packet rejected by signature policy");
+        LOG_WARN("Packet rejected by routing signature policy");
         return RoutingAuthVerdict::REJECT;
     }
     if (state == DecodeState::DECODE_FATAL) {
-        LOG_WARN("Fatal decode error, dropping packet");
+        LOG_WARN("Fatal decode error during routing authentication");
         return RoutingAuthVerdict::REJECT;
     }
     if (state == DecodeState::DECODE_FAILURE) {
-        LOG_WARN("Decryptable packet failed decoding, dropping packet");
+        LOG_WARN("Decryptable packet failed routing authentication");
         return RoutingAuthVerdict::REJECT;
     }
 
     // Only an explicit unknown-channel result remains eligible for opaque relay.
     if (state == DecodeState::DECODE_OPAQUE)
         return RoutingAuthVerdict::OPAQUE_RELAY_ONLY;
-    storeRoutingAuthCache(wire, authCandidate);
     return RoutingAuthVerdict::ACCEPT;
 }
 
+RoutingAuthVerdict passesRoutingAuthGate(const meshtastic_MeshPacket *p)
+{
+    // Routing still needs the original encrypted representation for byte-for-byte relay and for
+    // MQTT uplink. Authenticate a copy here; handleReceived() performs the normal in-place decode
+    // only after stateful routing filters have completed.
+    if (routingAuthCacheMatches(*p))
+        return RoutingAuthVerdict::ACCEPT;
+
+    meshtastic_MeshPacket authenticated = meshtastic_MeshPacket_init_zero;
+    const RoutingAuthVerdict verdict = evaluateRoutingAuth(*p, authenticated);
+    if (verdict == RoutingAuthVerdict::ACCEPT)
+        storeRoutingAuthCache(*p, authenticated);
+    return verdict;
+}
+
 #if !(MESHTASTIC_EXCLUDE_PKI)
 // The fallback costs three X25519 ops before the AEAD tag is checked. Budget is global because p->from is
 // attacker-controlled; successful runs refund, and their key is then persisted for the fast path.
@@ -1206,7 +1213,7 @@ NodeNum Router::getNodeNum()
  * Handle any packet that is received by an interface on this node.
  * Note: some packets may merely being passed through this node and will be forwarded elsewhere.
  */
-void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src)
+void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src, bool routingAuthRequired)
 {
     bool skipHandle = false;
 
@@ -1221,8 +1228,15 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src)
 
     // Consume the decoded/authenticated handoff after preserving the exact encrypted packet and
     // before mutating any packet fields that participate in the exact cache match.
-    if (src == RX_SRC_RADIO)
-        applyRoutingAuthCache(p);
+    if (routingAuthRequired && !applyRoutingAuthCache(p)) {
+        meshtastic_MeshPacket authenticated = meshtastic_MeshPacket_init_zero;
+        if (evaluateRoutingAuth(*p, authenticated) != RoutingAuthVerdict::ACCEPT) {
+            LOG_WARN("Routing authentication handoff was lost and packet re-verification failed");
+            packetPool.release(p_encrypted);
+            return;
+        }
+        *p = authenticated;
+    }
 
     // Also, we should set the time from the ISR and it should have msec level resolution.
     // Keep the decoded working packet and encrypted MQTT copy on the same local arrival timestamp.
@@ -1426,6 +1440,6 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p)
 
     // Note: we avoid calling shouldFilterReceived if we are supposed to ignore certain nodes - because some overrides might
     // cache/learn of the existence of nodes (i.e. FloodRouter) that they should not
-    handleReceived(p);
+    handleReceived(p, RX_SRC_RADIO, true);
     packetPool.release(p);
 }

src/mesh/Router.h

@@ -99,6 +99,10 @@ class Router : protected concurrency::OSThread, protected PacketHistory
         before us */
     uint32_t rxDupe = 0, txRelayCanceled = 0;
 
+#ifdef PIO_UNIT_TESTING
+    void handleReceivedAfterRoutingGateForTest(meshtastic_MeshPacket *p) { handleReceived(p, RX_SRC_RADIO, true); }
+#endif
+
   protected:
     friend class RoutingModule;
 
@@ -158,7 +162,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory
      * Note: this packet will never be called for messages sent/generated by this node.
      * Note: this method will free the provided packet.
      */
-    void handleReceived(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO);
+    void handleReceived(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO, bool routingAuthRequired = false);
 
     /** Frees the provided packet, and generates a NAK indicating the specifed error while sending */
     void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p);