mirror of
https://github.com/meshtastic/firmware.git
synced 2026-09-17 09:01:47 -04:00
Tom
ee76117835
* fix(router): relay opaque packets per rebroadcast_mode, not only in ALLd6b12ea3f(#10967) moved undecryptable packets onto relayOpaquePacket(), which relays only in ALL and ALL_SKIP_DECODING. A PKI unicast between two other nodes - remote admin, a DM, key verification - is opaque to a relay, so a router in CORE_PORTNUMS_ONLY (the ROUTER role default) stopped carrying any of it, and KNOWN_ONLY / LOCAL_ONLY lost the rule6eabbaf43added in 2024 that relays a PKI-shaped unicast with one known party. The old rule was still in RoutingModule::handleReceivedProtobuf, unreachable: encrypted packets no longer reach modules. opaqueRelayAllowedByMode() gives each mode an explicit opaque rule: ALL / ALL_SKIP_DECODING / CORE_PORTNUMS_ONLY relay (the port list cannot apply to a packet with no readable port); KNOWN_ONLY / LOCAL_ONLY relay a channel-0 unicast whose sender or destination has a User in NodeDB; NONE relays nothing. The dead RoutingModule block is removed; its licensed-party check stays for decoded packets. Nothing here reads packet_signature_policy. * fix(router): NAK, phone delivery and MQTT uplink for packets we cannot read Before #10967 an undecryptable packet addressed to us reached RoutingModule, which handed it to the phone and, via ReliableRouter::sniffReceived, answered a want_ack unicast from an unknown sender with PKI_UNKNOWN_PUBKEY - the NAK that makes the sender transmit its NodeInfo so its retry decrypts. A PKI DM between two other nodes was uplinked to MQTT as ciphertext when encrypted uplink was on. All three stopped: the gate REJECTs a to-us decode failure before any module runs, and the pki_encrypted marking in dispatchReceived is unreachable for opaque ingress. passesRoutingAuthGate() now treats every DECODE_FAILURE not from us as opaque (isFromUs stays REJECT, #11544). The opaque branch calls handleOpaqueForUs(), which NAKs a want_ack unicast to us (PKI_UNKNOWN_PUBKEY when we hold no key for the sender, NO_CHANNEL otherwise) and queues a frame we had no way to read - PKI without the sender's key, or a channel hash matching nothing we hold - straight to the phone via sendToPhone(), bypassing handleFromRadio() so an unverified sender never touches NodeDB. A matched-and-failed frame (bad key, tampering, junk) is NAKed but not delivered. uplinkOpaqueUnicast() restores the MQTT path for channel-0 unicasts not to or from us, gated on mqtt.enabled and mqtt.encryption_enabled; the dead marking is removed. * test(packet_signing): pin what a relay does with traffic it cannot read Group R builds genuine PKI-encrypted packets between two generated identities and runs them through ingress under every rebroadcast_mode: R1 remote admin between two known nodes relays in every mode but NONE R2 between strangers: KNOWN_ONLY / LOCAL_ONLY decline, the rest relay R3 one known party satisfies KNOWN_ONLY / LOCAL_ONLY R4 an unknown-channel broadcast relays in ALL / ALL_SKIP / CORE only R5 undecryptable DM to us: one PKI_UNKNOWN_PUBKEY NAK, phone gets the frame, nothing relayed, sender not added to NodeDB - in every mode R6 the same frame claiming to be from us gets no reaction R7 sender key held but wrong: NAK NO_CHANNEL, no phone delivery R8 opaque PKI unicast is uplinked only with encrypted MQTT uplink R9 unknown-channel broadcast reaches the phone without touching NodeDB R10 the relay decision is identical under all three signature policies C6 no longer lists CORE_PORTNUMS_ONLY as a mode that suppresses opaque relay and expects the phone to see an unreadable frame; the RoutingModule mock records the NAK reason. * test(rebroadcast_mode): give the relay policy its own suite, sharing the ingress harness test/support/AuthPipelineHarness.h now holds the mock NodeDB, the counting radio / router / routing-module / module / MQTT, the packet builders (decoded, channel- encrypted, PKI between two generated identities) and the per-process / per-test lifecycle that test_packet_signing kept locally. test_rebroadcast_mode pins what this node carries for others, per DeviceConfig.rebroadcast_mode: remote admin between two known nodes relays in every mode but NONE; strangers are declined by KNOWN_ONLY / LOCAL_ONLY and carried elsewhere; one known party suffices; an unknown-channel broadcast relays in ALL / ALL_SKIP / CORE only; a licensed node never relays ciphertext and relays plaintext unless a party is known unlicensed; hop_limit 0, id 0, a foreign next_hop and CLIENT_MUTE each stop a relay; the signature policy changes none of it. Registered in state-manifest.tsv and the routing shard. test_packet_signing keeps what follows from the auth gate's verdict: C9-C11 now carry want_ack and hops so their "nothing happens" assertions are no longer vacuous (junk on a held channel relays but never reaches the phone; a legacy DM and a malformed PKI plaintext to us are NAKed NO_CHANNEL once and nothing else), and C18-C22 cover the to-us NAK / phone / MQTT outcomes. Comments on the src side trimmed to the two-line rule. * fix(router): classify an opaque frame from the decode attempt, not the header handleOpaqueForUs() re-derived "unreadable" from the wire header and NodeDB, which disagreed with what perhapsDecode() had just found: a hash-0 broadcast from a sender whose key we hold read as readable although PKI never applies to a broadcast, and a pending-key decrypt rejected as malformed read as unreadable because no stored key existed. Both changed the NAK reason and whether ciphertext reached the phone. passesRoutingAuthGate() now hands the attempt's DecodeState out and the handler takes unreadable = (state == DECODE_OPAQUE). For that to be precise, perhapsDecode() sets pkiAttempted only when a sender, pending or admin key was actually tried, and the KNOWN_ONLY short-circuit - which declines before any attempt - reports OPAQUE for a PKI-shaped unicast to us or an unheld hash and FAILURE for a held channel. isUnreadableToUs() is gone; Channels::hasHash() replaces its loop. * test(support): free the harness AirTime and NodeStatus before restoring the originals pipelineHarnessDestroy() restored the saved pointers and orphaned the two objects it had installed. LeakSanitizer reported the NodeStatus as a 160-byte direct leak and errored test_packet_signing and test_rebroadcast_mode at exit in the coverage shards while every case passed. * fix(router): a failed admin-key fallback does not count as a decrypt attempt Every configured admin key set pkiAttempted, so on a node with any admin key an unknown sender's DM read as DECODE_FAILURE: NO_CHANNEL instead of PKI_UNKNOWN_PUBKEY, and withheld from the phone, so the sender never learned to send its NodeInfo. An admin key that fails says nothing about the sender; only the sender's own (or pending) key counts. A successful-but-malformed admin decrypt already returns DECODE_FAILURE directly. * test(packet_signing): pin decode provenance for admin keys, forged from-us frames, hash-0 broadcasts and KNOWN_ONLY strangers C18 configures an unrelated admin key so the fallback runs and fails. C19 installs our identity so the forged frame is a real decrypt attempt and asserts the gate's REJECT before the side effects. C23: a hash-0 broadcast from a keyed sender on a channel we do not hold is unreadable and reaches the phone. C24: KNOWN_ONLY declines a stranger on a held channel as matched, so the phone never sees it, while the same stranger on an unheld channel is unreadable. * test(support): restore the caller's DH key, model the TX queue and ACK/NAK log, clear per-process state between tests The ingress harness builds its router, radio, routing module and crypto engine once per process, so anything they carry decides the next test's outcome. Three of those carried surfaces were already deciding one. makePkiUnicastBetween() ended by installing a fresh random DH key, so a caller that set its own key before building a frame silently lost it. C19 did exactly that: its REJECT assertion passed because the decrypt failed on a key mismatch, not because the from-us arm rejected the forgery, and would have stayed green with that arm deleted. CryptoEngine::private_key is public under PIO_UNIT_TESTING, so the helper now saves the engine's key and puts it back; the trap is closed for every caller rather than worked around in one. C19 also builds the frame before installing our identity and gains a control assertion: without our key in NodeDB the same frame is OPAQUE_RELAY_ONLY, which is what makes the REJECT attributable. The opaque dedup ring survived a whole suite unreset. That was tolerable while it only gated relay; it is about to gate the NAK, phone delivery and MQTT uplink too, where a stale (from,id) would silently zero a later test's expectations instead of failing it. Cleared per test, along with the DH key, any pending handshake key, and the admin-key fallback budget that C18 drains six tokens from. resetAdminKeyFallbackBudget() is defined under !MESHTASTIC_EXCLUDE_PKI but declared unguarded, so the call site is guarded. installOurIdentity() now marks HAS_USER on our own node, as NodeDB does on a device. The rebroadcast_mode predicates read that bit, and markOurselvesLicensed() keeps owner.is_licensed and our NodeDB record in agreement for the same reason: getLicenseStatus(us) must say Licensed, not NotLicensed. The radio and routing-module mocks become models rather than counters, since every suite including this header gets them. The radio holds a real TX queue that findInTxQueue() consults and that cancelSending()/removePendingTXPacket() take entries out of, and it records each frame it was handed so a test can assert a hop limit or relay_node instead of a call count. The routing module keeps every ACK/NAK with its destination, channel and hop limit, so a second NAK can be pinned without losing the first; its reset() replaces the six sites that zeroed ackCalls by hand, which would otherwise desync the log from the counter. Phone-queue draining moves into the harness for the suites that both need it. * refactor(router): the opaque path lives in Router, not NextHopRouter A pure move. Nothing about handling a frame we cannot read is next-hop specific: relayOpaquePacket() reads iface, isToUs/isFromUs, the device role, owner.is_licensed, the last byte of our node number and the packet's own header, then calls Router::send(). The one thing that held it in the subclass was the (from,id) dedup ring, and that landed in NextHopRouter next to the pending and route-health tables by proximity rather than dependency - its whole purpose is to stay isolated from routing state, which argues for sitting beside PacketHistory instead. So the ring, opaqueWasSeenRecently(), relayOpaquePacket() and the rebroadcast_mode predicate (now Router::opaqueAllowedByMode) move down, and relayOpaquePacket() stops being virtual: there is one router chain, nothing else overrode it, and the base implementation returned false to no one. The alternative was a second virtual to reach the same array from the same caller, which is what the dedup work that follows would otherwise have needed. isRebroadcaster() comes along because relayOpaquePacket() needs it and it reads only config.device - no FloodingRouter state - so it was already misplaced. capEventRelayHops() moves too, and is now declared for NextHopRouter's own rebroadcast path rather than being file-static. * fix(router): apply the packet's own rules to every consumer of an opaque frame Five rules that pre-#10967 applied to a packet we could not read were left applying to the relay alone. This puts them back on all four consumers - relay, NAK, phone, MQTT - which is one change of shape, so it lands as one commit rather than five: the branch head now computes what is true of the frame once and every consumer below reads the same answer. Duplicate suppression. The only dedup sat inside relayOpaquePacket(), behind its isToUs() early return, so it never saw a frame addressed to us. Three neighbours rebroadcasting a stranger's want_ack DM to us cost three PKI_UNKNOWN_PUBKEY NAKs on the air, three encrypted frames queued for the phone, and at a gateway three publishes of every opaque PKI DM between other nodes. Before these packets stopped going through PacketHistory, shouldFilterReceived() ran first and made each of those once per (from,id). The originator's own retransmission keeps its exemption, and gains the two rules the decoded path already applies to a repeat: do not queue a second copy while the first is still in the TX queue, and answer again at hop 0, since only a direct neighbour ever sees hop_start == hop_limit. rebroadcast_mode. LOCAL_ONLY and KNOWN_ONLY say the node ignores what it cannot decrypt; the deleted RoutingModule branch gated phone delivery as well as relay, and only the relay half was carried over, so a stranger's unknown-channel ciphertext reached the phone in every mode. The phone now follows the same predicate. NONE still delivers: it means do not relay, not do not listen. Licence. A licensed station transmits in the clear and may not answer, or hand on, traffic to or from a node it knows to be unlicensed - the rule RoutingModule applies to decoded packets, which the opaque path never got. NAK reason. PKI_UNKNOWN_PUBKEY claimed a missing key even for a channel-0 frame too short to have carried PKI overhead. Such a frame was never a candidate, so the reason is NO_CHANNEL, matching the size test perhapsDecode uses. Uplink. uplinkOpaqueUnicast() read the header only, so a channel-0 unicast that matched a held hash-0 channel and failed its AEAD was published to the PKI topic as ciphertext we never tried to read. It now takes the gate's verdict. One consequence worth stating: the dedup ring records frames the relay gate used to reject before reaching it - to us, from us, hop-exhausted, mode-blocked, and everything on a licensed node - so its 32 slots serve four consumers on nodes that previously never touched it. Two clean-ups ride along because the same rules move: ReliableRouter's sniffReceived() loses its own undecryptable-NAK arms, which radio ingress has not been able to reach since the auth gate started answering those frames before handleReceived() (deliverLocal, the only other caller, is always decoded), and test_rebroadcast_mode stops declaring a warm.dat write it never makes - no case in it reads a signer back from the warm store. * test(rebroadcast_mode): declare the warm-store write again The suite stopped writing warm.dat only until this branch gave it a test that installs an identity of our own, which puts a key through the warm store. The declaration was removed on the evidence of a run that predated that test, in the same commit that added it; the harness caught the undeclared write. * fix(router): put back the undecodable-NAK arms in ReliableRouter::sniffReceived Deleted as dead code, and they are not. Radio ingress genuinely cannot reach them any more - the auth gate answers an unreadable frame in handleOpaqueForUs() and returns before handleReceived(), and deliverLocal(), the only other caller, is always decoded - but sniffReceived() has a contract of its own that test_reliable_ack_matrix drives directly, and a local or SimRadio caller still arrives with an encrypted packet. CI caught it in the misc-4 shard. Restored verbatim, with the reachability noted where the next reader will look rather than in a commit message nobody greps. * fix(router): classify a PKI-shaped unicast from key material alone Addresses the review on #11844. A channel we hold whose hash is 0 matched every PKI DM on the mesh and failed every one, and that failure was read as "we tried". One unlucky 1-in-256 channel hash therefore withheld every PKI DM from the phone and the broker, answered NO_CHANNEL where the sender needs PKI_UNKNOWN_PUBKEY to recover, and classified our own overheard DMs as a forgery so the implicit "Delivered to mesh" ACK never fired. Hash 0 on a unicast is the PKI sentinel, so isPkiShapedUnicast() now decides it in one place, used by the KNOWN_ONLY short-circuit, the decode provenance and the NAK reason. The isToUs asymmetry in the short-circuit is gone with it. Also from the review: - id 0 cannot be deduped by the (from,id) ring, so an undecryptable want_ack DM carrying it drew a NAK and a phone frame on every copy heard. Every consumer now declines it, as relay already did; a NAK for request_id 0 is unmatchable at the sender anyway. - The ring records only frames some consumer can act on. Our own overheard rebroadcasts and unicasts that can neither be relayed nor uplinked were evicting live entries, spending the anti-amplification bound #11522 added it for. - The MQTT uplink applies the licensed-station rule, and deliberately not rebroadcast_mode: that setting governs what goes back on the air, and MQTT has its own switches for what leaves over IP, which is where the pre-#10967 uplink sat. - gateState is initialised rather than relying on the gate's first statement. Comments on the opaque path trimmed to the two-line rule; the reasoning lives in the tests, which are exempt. * ci(size-budget): raise the rak4631 flash budget to 748000 The opaque-relay restore lands at 746,080 bytes on rak4631, 80 bytes over the previous 746,000 limit. Image ends at 0xDC260, 55 KB clear of the warm region. * fix(router): keep #10967's removal of the undecodable-frame NAK The PKI_UNKNOWN_PUBKEY / NO_CHANNEL NAK for a frame we cannot read is not restored. Every input to that decision - to, from, id, want_ack, hop_start, hop_limit - is unauthenticated cleartext, so the NAK is a reflector: one frame in from a node with no key material, one flooded reply out to whichever `from` it names, with a hop budget the sender chooses. #10967 removed it as an ACK side effect on purpose; the security review of this PR shows why, and the relay regression it fixes does not need it. handleOpaqueForUs() now only delivers to the phone. The originator-retx re-NAK and its `repeat` plumbing go with the NAK. A to-us DECODE_FAILURE is REJECT again at the gate, as #10967 had it: nothing on the opaque path acts on a frame we matched and failed on. ReliableRouter::sniffReceived() is back to develop byte for byte; its undecodable arms are reachable by local and SimRadio callers only and stay as they are. A PKI DM to a node that does not hold the sender's key fails silently, as on develop; the receiving phone still sees the frame. The sender-side recovery this NAK used to trigger is the follow-on's problem to solve without a header-driven reply. Tests: C10, C11, C18, C20, C26, C27, C29, C31 assert no NAK; C28 (the NAK's hop budget) is deleted; the matched-failure helper drops its want_ack arm. * fix(router): do not re-decode a frame the gate already classified; scope capEventRelayHops handleOpaqueForUs() hands the phone a copy the auth gate has already run perhapsDecode() on. MeshService::sendToPhone() ran it again, and for a PKI-shaped DM from a sender whose key we lack that second pass re-enters the admin-key fallback and spends a second token from a budget the code documents as global and attacker-facing: the sustained rate halved from 4/s to 2/s on any node with an admin key configured. sendToPhone() takes an alreadyClassified flag and skips the decode; nothing else calls it that way. Nodes with no admin key configured were never affected, since adminKeyFallbackAllowed() returns before touching the bucket. test_C34 freezes the clock, configures an unrelated admin key, and pins one token spent per unreadable frame delivered to the phone. adminKeyFallbackTokensRemaining() is a PIO_UNIT_TESTING accessor beside resetAdminKeyFallbackBudget(). ReliableRouter::sniffReceived()'s PKI_UNKNOWN_PUBKEY arm now requires the frame to be long enough to have carried the PKI overhead, the same shape test Router's opaque classification applies; a shorter channel-0 frame was never a PKI candidate, so no key was missing and it gets NO_CHANNEL. Reachable by local and SimRadio callers; pinned in test_reliable_ack_matrix. capEventRelayHops() moved out of NextHopRouter.cpp as a file-static and became a free function at global scope. Both callers are Router subclasses, so it is now a static member of Router next to isRebroadcaster(). * fix(router): size the opaque dedup ring like PACKETHISTORY_MAX Every ROUTER relays opaque frames now, not just nodes set to ALL, so the churn through this ring is what bounds a duplicate storm on the backbone. 32 untimed slots shared by three consumers is thin for that; 128 on every target but STM32WL, which keeps 32 alongside its 20-entry PacketHistory. 8 B per slot in .bss: +768 B, no flash. * revert(router): return the branch to develop * fix(router): relay opaque packets in CORE_PORTNUMS_ONLY A packet a relay cannot decrypt - a PKI unicast between two other nodes, an unknown-channel broadcast - is relayed only from its header, and only in the rebroadcast modes relayOpaquePacket() lists. CORE_PORTNUMS_ONLY was not among them, so a node in that mode, which is the ROUTER role default, dropped every such packet. The portnum filter that mode exists for cannot be applied to a payload the relay cannot read; add the mode to the list. Fixes #11843. * test(packet_signing): CORE_PORTNUMS_ONLY carries an opaque frame C6 listed CORE_PORTNUMS_ONLY among the modes that must suppress an opaque relay, pinning the behaviour #11843 reports. It now asserts the frame is relayed in that mode, with the same no-side-effect checks as the ALL case, and keeps LOCAL_ONLY and NONE as the suppressing modes.