The responder recorded a session under the source address carried in the
datagram without ever checking that address against the static key the
Noise handshake had just authenticated. A peer could therefore complete a
genuine handshake while claiming another node's address, and the identity
cache, the session map and the address the IPv6 shim reconstructs on
delivery would all attribute its traffic to the node it named.
Derive the address from the authenticated key at the point the key first
becomes available in msg3, and reject the handshake when it does not match
the claimed source. The entry has already been removed by that point, so
returning drops the half-open session and neither the identity nor the
session is recorded.
The rekey responder path needed its own check rather than inheriting that
one. It returns before the initial path's code is reached, and it never
read the peer's static key at all, so a rekey could complete under an
established session with a different key than the one that opened it. It
now requires the key to be unchanged, which is the stronger comparison
available there, and abandons the rekey while keeping the existing session
intact on mismatch. Tearing the session down instead would have handed an
attacker a way to kill established sessions.
Both comparisons are on x-only keys. A stored peer key may carry a
synthesized even parity because npubs encode no parity, while the
handshake learns the true point, so comparing full keys would reject
roughly half of legitimate peers on every rekey.
Both rejections are counted separately in the session reject statistics.
The tests drive real Noise handshakes through the datagram entry point and
construct the mismatch rather than asserting the comparison exists.
The forwarding path was corrected to full IP semantics: local delivery is not
hop-limit gated, and forwarding decrements first and drops at zero. Two helpers
on SessionDatagram were left implementing the old rule. decrement_ttl still
checked before decrementing, and can_forward still answered true at a hop limit
of one, where the decrement leaves zero and the datagram is dropped. Neither is
called anywhere today, which is precisely why they are worth correcting rather
than leaving as an invitation to reintroduce the defect.
The TtlExhausted reject doc described behaviour that no longer exists. The
reject is charged for a transit arrival at one as well as at zero, and is never
charged for a datagram addressed to this node, whose delivery is decided ahead
of the test.
Both helpers are pinned by tests at the boundary, which fail against the
previous implementations.
The discovery request dedup cache (recent_requests) silently dropped
LookupRequests once it reached MAX_RECENT_DISCOVERY_REQUESTS, with no
counter to surface the condition. Add a DiscoveryReject::ReqDedupCacheFull
reject reason backed by a req_dedup_cache_full counter on DiscoveryStats,
mirroring the existing duplicate-request counter, and record it at the
drop site so the rejection is visible in show_routing.
Introduce a typed RejectReason enum and a NodeStats::record_reject
dispatch so every receive-path rejection-and-return site bumps a
machine-readable per-subsystem counter while keeping its operator-facing
log line. The top-level variants mirror the existing NodeStats subsystem
split (Tree, Bloom, Discovery, Forwarding) and add Handshake, Session,
Mmp, and Transport categories; HandshakeStats, SessionStats, and MmpStats
are new sub-stats.
Wired clusters: tree and MMP outbound sign-failure; the FSP session
unknown-session and state-machine cluster; the Noise IK handshake
state-machine cluster (msg1/msg2); and the decode / crypto / cap /
semantic tail across bloom, discovery, forwarding, mmp, and tree. The
TreeStats::ancestry_invalid counter, present since the scaffold but never
incremented, is now bumped from the validate_semantics ancestry rejection.
Several handshake, MMP, tree, and discovery paths that previously had no
counter at all are now counted, including the send_lookup_response
no-route drop (DiscoveryStats::resp_no_route).
Existing direct counters at the bloom / discovery / forwarding sites are
retained alongside the new dispatch while the rollout is in progress (the
bloom_poison tests expect the transitional +2 delta); a later change
collapses the duplicate increment.
Johnathan Corgan