From 3a7c49972209c7b9adbda245b0d3c1d8337fd8b1 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sun, 16 Aug 2026 20:33:21 -0500 Subject: [PATCH] fix(mesh): dedup opaque relays to prevent an undecryptable-frame broadcast storm (#11522) * NextHopRouter: dedup opaque relays to prevent a broadcast storm Undecryptable ("opaque") frames are relayed by relayOpaquePacket(), which by design never enters PacketHistory - so unauthenticated frames can't poison next-hop learning or ACK matching (packet-authenticity policy, d6b12ea3f). But PacketHistory admission was also the *only* deduplication on that path. With none, a dense mesh re-relays every copy of every opaque frame and the copy count multiplies at each hop into an unbounded broadcast storm; "let hop exhaustion bound it" caps depth, not count. Add a small, isolated (from,id) seen-set checked in relayOpaquePacket() before rebroadcast: a second PacketHistory-style table (fixed 32-slot ring, round-robin eviction) that only suppresses duplicate opaque rebroadcasts and never feeds routing/ACK/next-hop, preserving the security property. Genuine originator (re)transmissions (hop_start == hop_limit) are still relayed so reliable opaque unicast propagates (mirrors FloodingRouter's isRepeated). Observed on a mixed-channel mesh: one node relayed a single undecryptable broadcast 23x (every overheard copy) with TX queues saturated, while decodable traffic on the same node deduped normally. Co-Authored-By: Claude Opus 4.8 * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Change log level from WARN to TRACE for duplicates --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/mesh/NextHopRouter.cpp | 30 ++++++++++++++++++++++++++++++ src/mesh/NextHopRouter.h | 20 ++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/mesh/NextHopRouter.cpp b/src/mesh/NextHopRouter.cpp index 5b4511120..c86f35ec7 100644 --- a/src/mesh/NextHopRouter.cpp +++ b/src/mesh/NextHopRouter.cpp @@ -37,6 +37,18 @@ bool NextHopRouter::relayOpaquePacket(const meshtastic_MeshPacket *p) (p->next_hop != NO_NEXT_HOP_PREFERENCE && p->next_hop != nodeDB->getLastByteOfNodeNum(getNodeNum()))) return false; + // Dedup opaque relays. Opaque frames deliberately never enter PacketHistory (so unauthenticated + // traffic can't influence routing/ACK/next-hop) - but with NO dedup at all, a dense mesh re-relays + // every copy of every frame, multiplying at each hop into an unbounded broadcast storm ("let hop + // exhaustion bound it" caps depth, not count). Suppress duplicate opaque rebroadcasts with a small, + // routing-isolated seen-set. Genuine originator (re)transmissions (hop_start == hop_limit) are + // always relayed so reliable opaque unicast still propagates (mirrors FloodingRouter's isRepeated). + const bool isOriginatorTx = p->hop_start > 0 && p->hop_start == p->hop_limit; + if (opaqueWasSeenRecently(getFrom(p), p->id) && !isOriginatorTx) { + LOG_TRACE("Drop duplicate opaque relay from 0x%08x id 0x%08x", getFrom(p), p->id); + return false; + } + meshtastic_MeshPacket *relay = packetPool.allocCopy(*p); if (!relay) return false; @@ -53,6 +65,24 @@ bool NextHopRouter::relayOpaquePacket(const meshtastic_MeshPacket *p) return res == ERRNO_OK; } +// Isolated dedup for opaque relays (see relayOpaquePacket). Returns true if (from,id) is already in the +// ring; otherwise records it (round-robin eviction) and returns false. A separate table from +// PacketHistory on purpose: opaque frames must never influence routing/ACK/next-hop. No timestamps - +// a stale (from,id) can't false-match a later packet because ids are effectively random. +bool NextHopRouter::opaqueWasSeenRecently(NodeNum from, PacketId id) +{ + for (uint8_t i = 0; i < OPAQUE_SEEN_MAX; i++) { + if (opaqueSeen[i].sender == from && opaqueSeen[i].id == id) + return true; + } + // Not seen: record it, overwriting the oldest-written slot (FIFO). Empty slots hold id 0, which a + // real entry never has (relayOpaquePacket drops id 0), so they simply never match above. + opaqueSeen[opaqueSeenNext].sender = from; + opaqueSeen[opaqueSeenNext].id = id; + opaqueSeenNext = (uint8_t)((opaqueSeenNext + 1) % OPAQUE_SEEN_MAX); + return false; +} + PendingPacket::PendingPacket(meshtastic_MeshPacket *p, uint8_t numRetransmissions) { packet = p; diff --git a/src/mesh/NextHopRouter.h b/src/mesh/NextHopRouter.h index 26cda830a..0d6d971fc 100644 --- a/src/mesh/NextHopRouter.h +++ b/src/mesh/NextHopRouter.h @@ -124,6 +124,8 @@ class NextHopRouter : public FloodingRouter constexpr static uint32_t ROUTE_TTL_MSEC = 30UL * 60 * 1000; // re-discover a route unconfirmed for 30 min constexpr static uint8_t ROUTE_FAILURE_THRESHOLD = 3; // consecutive un-ACKed directed deliveries -> dead + constexpr static uint8_t OPAQUE_SEEN_MAX = 32; // opaque-relay dedup slots (see relayOpaquePacket); ~8B/slot -> ~256B + protected: /** * Pending retransmissions @@ -135,6 +137,21 @@ class NextHopRouter : public FloodingRouter */ RouteHealth routeHealth[ROUTE_HEALTH_MAX] = {}; + /** + * Recently-seen opaque (undecryptable) frames, keyed on the outer (from,id) header. A second, + * isolated PacketHistory-style dedup: it bounds broadcast amplification of frames we can't decrypt + * WITHOUT admitting them to the real PacketHistory/NodeDB, so unauthenticated traffic can never + * influence routing / ACK / next-hop decisions. Fixed-size ring, round-robin (FIFO) eviction, no + * timestamps (a stale (from,id) can't false-match: packet ids are effectively random, and a real + * entry never has id 0 - relayOpaquePacket drops id 0 before this). RAM-only. + */ + struct OpaqueSeen { + NodeNum sender = 0; + PacketId id = 0; // 0 == empty/unused slot + }; + OpaqueSeen opaqueSeen[OPAQUE_SEEN_MAX] = {}; + uint8_t opaqueSeenNext = 0; // ring write cursor (round-robin eviction) + /** * Should this incoming filter be dropped? * @@ -143,6 +160,9 @@ class NextHopRouter : public FloodingRouter */ virtual bool shouldFilterReceived(const meshtastic_MeshPacket *p) override; bool relayOpaquePacket(const meshtastic_MeshPacket *p) override; + // Dedup helper for relayOpaquePacket: true if (from,id) is already recorded; otherwise records it + // (round-robin eviction) and returns false. Pure function of the table - no clock. + bool opaqueWasSeenRecently(NodeNum from, PacketId id); /** * Look for packets we need to relay