diff --git a/.github/workflows/package-openwrt.yml b/.github/workflows/package-openwrt.yml index 5d0037fd..6de57606 100644 --- a/.github/workflows/package-openwrt.yml +++ b/.github/workflows/package-openwrt.yml @@ -997,11 +997,34 @@ jobs: | xargs sha256sum \ > checksums-openwrt.txt - - name: Create release - uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2 - with: - files: | - dist/*.ipk - dist/*.apk - dist/checksums-openwrt.txt - generate_release_notes: true + # This job used to create the Release object itself, with + # generate_release_notes. The three other packaging workflows wait for + # that object instead, so whichever got there first decided what the + # release page said: if this one won, the written notes were stranded + # behind an auto-generated commit list. Wait like the others, so the + # release is created once, deliberately, by whoever pushes the tag. + - name: Wait for tag release + env: + GH_TOKEN: ${{ github.token }} + run: | + for attempt in $(seq 1 20); do + if gh release view "${GITHUB_REF_NAME}" --repo "${GITHUB_REPOSITORY}" >/dev/null 2>&1; then + exit 0 + fi + echo "Release ${GITHUB_REF_NAME} not available yet; waiting..." + sleep 15 + done + + echo "Timed out waiting for release ${GITHUB_REF_NAME}" >&2 + exit 1 + + - name: Upload OpenWrt assets + env: + GH_TOKEN: ${{ github.token }} + run: | + gh release upload "${GITHUB_REF_NAME}" \ + dist/*.ipk \ + dist/*.apk \ + dist/checksums-openwrt.txt \ + --clobber \ + --repo "${GITHUB_REPOSITORY}" diff --git a/CHANGELOG.md b/CHANGELOG.md index b7c5ee68..b114365a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -856,65 +856,11 @@ with v0.4.x or earlier peers. permissive umask; a capture carries the node npub, build, platform and a timing series. -## [0.4.2] - unreleased +## [0.4.2] - 2026-08-25 -### Added +### FMP/FSP sessions and rekey -#### Admission / rate limiting - -- `node.rate_limit.session_setup_burst` (64) and - `node.rate_limit.session_setup_rate` (16.0), the parameters of the new - per-link-peer session-setup limiter. This is the FSP session-setup bucket, - and it is distinct from the link-layer msg1 bucket described below; the two - meter different messages and are sized independently. Setup messages naming - a peer this node is already established with are metered on a second - per-link bucket derived from `node.limits.max_peers`, - `node.rekey.after_secs` and `node.rate_limit.handshake_max_resends`, so - raising the peer limit sizes it automatically. A zero burst or a - non-positive rate is rejected at config validation rather than silently - refusing every session. - -- `node.limits.max_sessions`, defaulting to 1024, which bounds the end-to-end - session table. Zero means unlimited, which restores the previous behaviour - exactly and is the way to back the change out on a running node. The default - is four times the adjacent `node.session.pending_max_destinations`. A - session entry measures 6608 bytes of inline state plus heap, so the table - holds to roughly 7 MB, and a test pins that per-entry figure so the - arithmetic behind the default fails loudly if an entry grows. Existing - configurations parse unchanged, the key being optional. - -- `node.rate_limit.established_handshake_burst` and - `node.rate_limit.established_handshake_rate`, the parameters of the new - established-link msg1 token bucket, which meters link-layer msg1 rather - than FSP session setup. Both are optional; omitting them (the normal case) - derives the bucket from `node.limits.max_peers`, `node.rekey.after_secs` - and `node.rate_limit.handshake_max_resends`, so raising the peer limit - sizes the bucket automatically. An explicit zero burst or a non-positive - rate is rejected at config validation rather than silently refusing all - rekey traffic. - -#### NAT traversal / Nostr discovery - -- `node.discovery.nostr.max_concurrent_offers_per_npub`, defaulting to 4, which - bounds how many inbound traversal offers one sender npub may have in flight - at once. It sits inside `max_concurrent_incoming_offers`, which remains the - outer bound, so a value above that is inert; zero is rejected at config - validation, since it refuses every inbound offer rather than disabling the - limit, and so is a value above the maximum permit count a semaphore can - hold, which would otherwise fail at construction rather than at load. - Existing configurations parse unchanged, the key being optional. - -#### Docs & contributor tooling - -- `SECURITY.md`, stating a private channel for vulnerability reports, what a - useful report contains, what a reporter can expect back and on what timing, - and which branches receive fixes. The repository previously documented no - reporting channel at all, so someone with a finding had to guess at an - address or open a public issue. - -### Changed - -#### FMP/FSP rekey reliability +#### Changed - Config validation now rejects two `node.rekey` settings that appear to disable the trigger and in fact fire it continuously. `after_messages` of @@ -929,209 +875,7 @@ with v0.4.x or earlier peers. supported way to disable one arm. A config carrying either setting now fails to load instead of starting a node that rekeys constantly. -#### NAT traversal / Nostr discovery - -- Inbound traversal offers are now admitted against a per-sender allowance as - well as the global pool. The intake path previously took a permit from a - single semaphore before any identity check, with the sender's npub used only - as a log field, so one sender could hold every slot and deny traversal - onboarding to every other peer for as long as it kept offering. Admission now - takes a per-npub permit and a global permit together. A sender over its own - allowance is refused at debug rather than warn, because the party tripping it - is by definition sending faster than the node wants and a record per - rejection would turn the spam into log volume; the global bound being reached - keeps its warn, which is the operator's signal that the node is genuinely - saturated. **This does not make the pool inexhaustible.** Nostr identities - cost nothing to generate and the signal subscription carries no author - restriction, so an attacker running four throwaway npubs still saturates the - shipped 16-slot pool at an unchanged total offer rate. What the change buys - is that one identity can no longer do it alone, and that the two refusals are - distinguishable in the log. The permit is still held across the whole - attempt; that duration remains inferred from the attempt timeout rather than - measured. - -- Config validation now rejects a `node.discovery.nostr.signal_ttl_secs` that - is too large for the configured `replay_window_secs`. A traversal signal is - acceptable over its TTL plus 60s of clock-skew grace on each side, and that - span has to stay strictly inside the replay window, or a session id evicted - from the replay cache on expiry is still fresh enough to be accepted a second - time. The relation was documented but unenforced, so raising the TTL past - 180s silently voided it. The bound is derived from the skew constant rather - than restated, and is checked whether or not nostr discovery is enabled, for - the same reason the rekey rules are. The shipped defaults (120s against 300s) - are unaffected, but a configuration that had widened the TTL or narrowed the - replay window now fails to load, with an error naming the concrete floor for - `replay_window_secs`. The NAT lab's config generator was one such - configuration and its generated `replay_window_secs` moves from 60 to 180. - Note that this covers eviction on expiry only: `seen_sessions_max_entries` - remains a separate capacity-eviction route that no config relation bounds. - -- The peer-retry tick no longer awaits the Nostr advert refetch. It ran inline - on the 1-second rx-loop tick, awaiting a fetch with a 2-second timeout for - each due peer and discarding the result; with up to sixteen due peers the - timeouts stacked, and field profiling measured single 2.00 s stalls as the - common case and a worst tick of 12.4 s against a 1 s period, delaying every - other rx-loop arm by as much as 4.2 s. The refetch is now spawned, so a dial - uses the advert cached at that moment and the refreshed one lands for that - peer's next retry. - -- The `Adopted NAT traversal socket` log line now carries the transport id and - the local address alongside the peer npub. Without the local address an - operator cannot join a host socket table against adoption events, and without - the transport id several peers sharing one adopted transport are - indistinguishable from several separate adopted transports. - -#### Admission / rate limiting - -- Inbound msg1 is classified before it is rate limited, and rekey or restart - msg1 arriving on a link belonging to a promoted peer now draws on its own - token bucket instead of competing with stranger admission for a single - shared one. On a node with many peers the shared bucket refused a large - share of ordinary rekey traffic: a field node at roughly 245 peers refused - 8753 msg1 in 25 minutes, and 159 of the 201 distinct sources were peers it - already held sessions with. On the XX handshake path the classifier keys on - promotion state rather than on the presence of an address-map entry, because - msg1 creates such an entry for a still-pending inbound connection before any - identity is known; a still-handshaking stranger therefore stays in the - stranger class for its whole lifetime, retransmits included. Nodes upgrade - with no config change. The `Msg1 rate limited` log line now reports which - limb refused, the pending count or the token bucket, which it previously did - not distinguish. - -#### Data-plane / metrics / observability - -- Peer bloom filters are computed for every recipient in one prefix and suffix - union sweep rather than rebuilt per recipient. Announcing to R peers - previously did R full map builds and R by T merges; at 240 peers that was - 20.6 ms per tick, roughly half the tick body, with a median per-interval - maximum of 34.5 ms. The result is exactly equal rather than approximately: - merging is a bytewise OR, so regrouping the unions cannot change it. The - trade-off, measured rather than assumed, is that the sweep does its full work - regardless of how many peers are ready, so a tick announcing to one or two - peers now costs about twice what it did; break-even is around three ready - peers. Cadence, the debounce, the sequence rule and the fill-ratio cap are - unchanged. - -- Each peer's npub is derived once at construction instead of once per tick. - The per-tick stats snapshot ran a bech32 encode for every tracked peer, and a - second one for the common peer with no hosts-file entry and no alias, since - the display-name fallback bottoms out in the same encode: 14.1 ms per tick at - 240 peers. The display name itself is deliberately not cached, because the - alias map and the host map both mutate at runtime. - -#### Transports & config - -- `fipsctl keygen` no longer exits non-zero when only the `fips.pub` write - fails. The private key is already on disk at that point, so failing the run - reported failure for a keygen that did produce the identity; the failure is - now a warning and the run succeeds. The pre-existing-key guard also moves - from `exists` to `symlink_metadata`, so a dangling symlink at the key path - now blocks keygen without `--force` instead of being overwritten silently. - -#### Library API - -- **Source-breaking for consumers of the library crate**: four public types now - implement `Drop`, so their fields can no longer be moved out. `Identity`, - `ResolvedIdentity`, `IdentityConfig` and `HandshakeState` each gained one as - part of clearing key material at end of scope. `IdentityConfig` is the one - most likely to be reached in practice, since it hangs off the public `Config` - as `node.identity`, so code that moved the nsec out of a configuration value - no longer compiles and needs `Option::take` instead. Nothing about the - behaviour of the shipped binaries changes; this affects only callers using - `fips` as a library. - -#### CI & test-harness reliability - -- Two CI runs on one machine can no longer collide. Every suite derives its own - docker build context, image tag, container names, network range and host - interface names per run, so concurrent runs cannot reap each other's - containers or contend for a fixed subnet. This is what a contributor running - `testing/ci-local.sh` alongside a GitHub run, or two local runs at once, sees - change: the runs stay independent instead of one killing the other. - -- A test that does not run, or whose result cannot be read, no longer passes - silently. A failed scenario now fails the run rather than being logged and - stepped over, a node whose logs cannot be read no longer counts as clean, an - unanswered control query no longer reads as zero, an unknown scenario key is - rejected instead of matching nothing, and a skipped check appears in the - final verdict rather than only in scrollback. - -- New guards run in both the local and GitHub runners, so the two gates agree. - They check that trailing-log call sites are wired, that the log strings the - harness matches on are still emitted by the daemon, that the two runners' - integration-suite sets match per leg rather than as a folded token, that - every GitHub Action reference is pinned in the required form, and that source - comments do not cite references a reader of the published tree cannot - resolve. - -- Coverage moved from Docker to deterministic in-process tests, and dead - scenarios were retired. The six cost-selection chaos scenarios, the - admission-cap and acl-allowlist Docker suites, the smoke-10 scenario, the - tcp-chain and mesh-public static topologies, and three ignored Ethernet - tests are gone, with their behaviour asserted in unit and integration tests - instead. A local CI run is correspondingly shorter and less dependent on - container timing. - -- The `bloom-storm` chaos scenario no longer runs on either the local or the - cloud runner. Unlike the retirements above it has no replacement: the - scenario files remain in the tree and it stays runnable by hand, but nothing - now exercises downstream containment of a mid-chain ancestor swap on a - schedule. This is recorded as a coverage gap rather than as a completed - migration. - -- A failing harness now says why it failed. The dns-resolver suite sent build - and container-start output to `/dev/null`, so a failed scenario reported - that it had failed and nothing else; output is now captured and emitted on - failure, naming the command, and the systemd readiness wait dumps container - state, failed units and the journal when it gives up. The NAT-lab path - assertions exited bare, printing neither what they expected nor what they - saw and triggering none of the scenario diagnostics their siblings already - call; all twelve call sites now report the container, the expectation, the - observation and a projection of the peer or link table, and distinguish a - failed control-socket exec from unparseable output from a genuine mismatch. - The convergence gate could not tell a tree that did not converge from - connectivity that failed, and could exit non-zero while reporting "20 - passed, 0 failed"; it now records the outcome, the count reached and the - count pending, and its failure messages name the condition. A passing run - is as quiet as before, and no timing, threshold or control-flow behaviour - changed in any of the three. - -- A dns-resolver scenario no longer burns the full 30-second boot timeout and - warns about a container that booted correctly. The readiness poll ran under - `pipefail` and piped `systemctl is-system-running` into `grep`, and that - command exits non-zero when the system is degraded, which is where systemd - inside a container always settles; the pipeline therefore failed even when - the pattern matched, leaving the degraded branch dead. The poll now matches - on the captured state instead of piping into `grep`. - -- The chaos harness now checks that teardown and node stops did what they - report. `docker compose down` exits 0 while leaving a run's containers - alive, so a partly-failed bring-up leaked named containers with nothing to - detect it; teardown now asks whether the containers this run owns are gone, - treats a survivor that forced removal clears as a warning, and aborts with - the names written to an artifact when one survives that or the query cannot - run at all. The check is scoped to a run's own names, so concurrent runs - cannot trip each other. Node churn separately marked a node down whether or - not `docker stop` succeeded, so the simulation's model of the mesh diverged - from reality, and `nodes_down`, the `max_down_nodes` cap and the - connectivity guard are all computed from that model; a failed stop now - warns, carries the daemon's own message, and leaves the node out of the - down set for the next churn tick to retry against an honest model. - -#### Docs & contributor tooling - -- Comments throughout the source tree, the packaging files and the test scripts - no longer cite internal identifiers, planning documents or private stage names - that a reader of the published tree cannot resolve; each now states the thing - the citation stood for. A handful of comments that described behaviour the - code does not have (the control-plane read path, its snapshot dispatch, and - the MMP report types) have been corrected rather than merely reworded. One of - the edited files, the DNS setup helper, installs to `/usr/lib/fips` on every - packaging path, so its comment reached users. No code changed. - -### Fixed - -#### FMP/FSP rekey reliability +#### Fixed - Inbound session-setup messages are now rate limited, keyed on the authenticated link peer the datagram arrived over. The setup path allocated @@ -1158,10 +902,10 @@ with v0.4.x or earlier peers. - A forged SessionAck no longer destroys an in-flight session initiation. The handler removed the session entry to take ownership of the handshake state - and, when the XX msg2 read failed, returned without putting it back. Nothing + and, when the XK msg2 read failed, returned without putting it back. Nothing in that message is authenticated (the only thing tying it to the initiation is the datagram's source address, which the sender chooses), so any node able - to reach the victim could cancel any initiation with 106 bytes of the right + to reach the victim could cancel any initiation with 57 bytes of the right length, and hold establishment down by repeating it. The entry is now kept. Keeping it is not enough on its own, and the second half is the part worth naming: the msg2 read mixes the sender's ephemeral into the symmetric state @@ -1169,274 +913,33 @@ with v0.4.x or earlier peers. left it holds a handshake that can never read the genuine msg2, which trades a one-round-trip denial for one lasting the full handshake timeout. The read is therefore rolled back to its pre-read state before the entry goes back. - The entry's activity stamp is deliberately not refreshed on the failure path, - so a spray cannot hold a dead initiation past its original sweep deadline, - and a new `ack_handshake_failed` counter makes the refusals visible at the - default log level. + The three later failure paths in the same handler still drop the entry: each + is downstream of a msg2 that authenticated, so it is a local failure rather + than a possible forgery. The entry's activity stamp is deliberately not + refreshed on the failure path, so a spray cannot hold a dead initiation past + its original sweep deadline, and a new `ack_handshake_failed` counter makes + the refusals visible at the default log level. Only the XK handshake on this + branch is covered; the additional drop sites in the XX handshake on the + development branch are not. - Two further exits from the same handler are covered here that the - maintenance branch has no equivalent of, because they exist only on the XX - handshake: the negotiation payload carried alongside msg2, and the check - that the responder's static key is the one this node dialled. Both are still - handling an unauthenticated message. A successful XX msg2 read proves the - sender holds the static it presented and nothing about *which* static that - should be, and identities are free, so any node at all can answer a msg1 - with a well-formed msg2 of its own and reach either exit. The identity - mismatch is the sharper of the two and still not a reason to drop: what it - establishes is that the sender of that datagram is not who we dialled, not - that the peer is, since the source address it claimed is an envelope field. - Both now roll the handshake back and keep the entry, and the identity - mismatch — previously a silent return — gets its own `ack_identity_mismatch` - counter rather than sharing the first, so a stale npub-to-address mapping - and somebody answering initiations under their own identity stay - distinguishable. The failure paths downstream of the identity check still - drop the entry: each is past a msg2 that both authenticated and matched the - dialled peer, so it is a local failure rather than a possible forgery. +- An unauthenticated session msg3 no longer discards a completed key epoch. + Five sites discarded the whole rekey, which nulls a `pending` session sitting + beside the handshake, when only the handshake had failed: the four failure + paths in the responder-side rekey arm of the msg3 handler, and the + dual-initiation yield arm in the setup handler, which is gated on a rekey + being in progress rather than on this node having initiated it, so an entry + the peer armed reaches it too. That `pending` session is the epoch the real + peer may already have cut over to, so discarding it kills the reverse + direction until the session idles out. Two unauthenticated messages reached + it: a forged setup message arms a handshake beside a completed rekey once + that rekey has waited a full idle timeout for a peer that never appeared on + the new epoch, and any garbage msg3 of the right length then finishes the + job. All five now abandon only the handshake. The four remaining sites in the + ack initiator arm are deliberately left alone and the reason is recorded + there: an entry with the initiator flag set holds no pending session, so the + two calls are the same action at those sites. -- An unauthenticated session msg3 no longer discards a completed key epoch. Six - sites discarded the whole rekey, which nulls a `pending` session sitting - beside the handshake, when only the handshake had failed: the five failure - paths in the responder-side rekey arm of the msg3 handler, and the dual- - initiation yield arm in the setup handler, which is gated on a rekey being in - progress rather than on this node having initiated it, so an entry the peer - armed reaches it too. The fifth msg3 path, the negotiation payload split out - of that message, is specific to the XX handshake and has no counterpart on the - maintenance branch. That `pending` session is the epoch the real peer may - already have cut over to, so discarding it kills the reverse direction until - the session idles out. Two unauthenticated messages reached it: a forged setup - message arms a handshake beside a completed rekey once that rekey has waited a - full idle timeout for a peer that never appeared on the new epoch, and any - garbage msg3 of the right length then finishes the job. All six now abandon - only the handshake. The four remaining sites in the ack initiator arm are - deliberately left alone and the reason is recorded there: an entry with the - initiator flag set holds no pending session, so the two calls are the same - action at those sites. - -#### NAT traversal / Nostr discovery - -- Nostr NAT traversal signals are now sent only to relays the client pool - actually holds. A signal is addressed to the merge of the peer's NIP-17 inbox - relays, the relays its advert nominates for signaling, and our own DM relays, - but the pool is built once at startup from the configured relays and the send - is rejected outright, before anything is contacted, if any single URL in that - list is outside it. One unconfigured relay anywhere in the merge therefore - killed the whole attempt, including the sends to relays both sides shared. On - a public node in open mode this made discovery non-functional: 309 traversal - attempts, 290 explicit failures, zero successes, every failure on `relay not - found`. Configured peers were unaffected, since they run a matching relay - set. Comparison is on the normalized relay URL rather than the raw string, so - a configured relay spelled with a trailing slash or different host case is - not discarded. Two smaller fixes ride along: the responder resolves its - relays before binding a socket and running STUN, rather than spending a STUN - round trip and holding an offer slot only to find it has nowhere to answer, - and it gained the empty-relay-list guard the initiator already had. - -- Nostr NAT traversal no longer breaks after the host suspends. The traversal - clock cached a Unix timestamp once at startup and advanced it with a - monotonic `Instant`, which does not tick while a machine is asleep, so after - a suspend the daemon's idea of the time trailed real time by the suspend - duration for the rest of the process lifetime. Every NIP-40 expiration it - computed was therefore published already in the past: relays dropped the - offers as expired, the initiator logged a signal timeout waiting for an - answer, and traversal stayed broken until the daemon was restarted. The - clock now reads the wall clock on every call. This is not macOS-specific, - though a laptop that sleeps is where it is easiest to hit; any host that - suspends or hibernates was affected. Reported in - [#128](https://github.com/jmcorgan/fips/issues/128). - -#### Spanning-tree / mesh-size / routing - -- Flap dampening can now engage more than once in the lifetime of a node. - The arming check tested whether a dampening deadline had ever been set - rather than whether one was still in effect, so the first episode - disarmed the mechanism permanently: a node in a second flap storm went on - switching parents under hold-down alone, and neither the `flap_dampened` - counter nor the "Flap dampening engaged" warning fired again, so the - storm was invisible to anyone watching that counter. A lapsed episode is - now retired explicitly, clearing both the deadline and the switch - counter, so a second episode requires a fresh threshold of switches - within one window rather than re-engaging on the first switch after - lapse. Hold-down was unaffected throughout and continued to limit - discretionary switching, which is why the practical effect at shipped - settings was lost visibility and a lost escalation tier rather than - unrestrained flapping. Every path that can engage an episode now reports - it, including a re-engagement during parent-loss recovery, which was - previously silent. The warning names which path armed the episode - (`trigger`) and how long discretionary parent switching stays suppressed - (`dampening_secs`), using the same `trigger` values as the parent-switch - logs beside it, so the two can be read together. A - `node.tree.flap_dampening_secs` large enough to overflow the monotonic - clock is capped at one year, beyond which an episode is - indistinguishable from permanent, so an extreme setting no longer panics - the node when dampening engages. - -#### Data-plane / metrics / observability - -- A SessionDatagram carrying a truncated inner FSP payload no longer panics the - forwarding path. The coordinate-cache warm path sliced the inner payload at - the full 12-byte header offset while guarding only with the 4-byte common - prefix parser, so an inner payload of 4 to 11 bytes with phase 0x0 and the - Coords Present flag set indexed past the end of the slice. Because the - receive loop is the process's main future, the panic terminated the daemon - rather than a task, and under the packaged systemd unit the node restarted - into the same frame. The warm path now applies the same - `FspEncryptedHeader` guard the local-delivery path already used, which - additionally means a malformed frame carrying a non-zero protocol version or - the Unencrypted flag alongside Coords Present is dropped rather than having - its body read as coordinates. Any peer that had completed a link handshake - could trigger this, and admission is default-open. Frames rejected by that - guard are now counted in the forwarding statistics as - `warm_malformed_packets` and `warm_malformed_bytes`, the byte counter - charging the whole outer frame, visible over the control socket and on the - fipstop Routing State pane, so a node being fed malformed frames is - distinguishable from a quiet one at the default log level. The count is not a - packet drop: the frame is still delivered or forwarded, and only the - coordinate-cache warm attempt is abandoned. The existing debug log now also - carries the frame's protocol version and flags, which separate a short frame - from a bad-version or Unencrypted-flagged one. - -- `SessionDatagram` hop-limit handling now follows IP semantics. Delivery to - the addressed node is no longer TTL-gated, and a forwarder decrements before - deciding rather than after, so a datagram that would leave with a TTL of zero - is dropped instead of transmitted. Previously the TTL check ran ahead of the - local-delivery test, so a datagram addressed to this node that arrived with - TTL 0 was dropped, and a forwarder receiving a transit datagram at TTL 1 - transmitted it at TTL 0 for the next hop to discard, wasting one transmission - per expiring datagram. `SessionDatagram::decrement_ttl` and - `SessionDatagram::can_forward` were aligned to the same semantics: - `decrement_ttl` decrements first and reports false when the result is zero, - and `can_forward` is true only at a TTL of 2 or more. The reachable radius is - unchanged, because the two behaviors compensated exactly: a path of `h` links - still delivers for any source TTL of `h` or more. During a rolling upgrade, an - unupgraded forwarder feeding an upgraded destination delivers one hop further - than either version does on its own; no version mix delivers less far. The - `TtlExhausted` reject counter now charges at the node that makes the decision - rather than at the hop after it. - -#### Transports & config - -- The UDP transport's DNS cache is now bounded and actually evicts. The map - held one entry per distinct hostname string ever dialed, and the TTL was - applied only on the read, so a stale entry was overwritten on the next dial - of the same name and otherwise stayed for the life of the process. Under a - rendezvous policy that accepts advertised endpoints the keys are strings a - remote party chose, which made the growth theirs to drive. A store now - sweeps entries past their TTL and, if the map is still full, drops the - oldest, holding it to 256 hostnames. Refreshing a name already cached - evicts nothing. Eviction is by insertion time rather than last use, so a - rarely dialed name in a very large peer list may re-resolve more often; the - cost of a wrong eviction is one DNS lookup, not a failed dial. - -- macOS: stopping an Ethernet transport under load no longer hangs the - process. The BPF reader thread handed each frame to the async consumer with - `blocking_send`, which parks with no way to be woken. Stopping the transport - aborts the consumer first, so nothing drains the 1024-frame channel, and the - socket's `Drop` then joined a thread that could never return: on a busy - interface the daemon had to be killed. The socket now drops the receiver - before joining, which releases a parked send at once, and the reader thread - sends through a helper that watches the same shutdown pipe its `select()` - already honours, so a send waiting for room cannot outlive a shutdown - request. The helper yields before it sleeps, so the saturated-path handoff - rate is unchanged. **Not covered by CI**: the reader thread is macOS-only - and Linux CI compiles none of it. What the tests prove is that the helper - the thread now waits in is cancellable; that a real BPF thread exits under - load still needs a manual check on a Mac. - -- A failed private-key write no longer leaves a node silently running an - ephemeral identity. Six write results in the identity path were discarded, - and the sharpest was in `persistent` mode: a failed write to `fips.key` fell - through to an ephemeral identity with no message, so a node that had been - asked for a stable identity changed its npub, its routing address and its - mesh IPv6 on every start, and nothing said so. All six now report. An - ephemeral start that is about to overwrite an existing key file now warns - first, naming the path and the setting that would have preserved the - identity, which is the warning `fipsctl keygen` has always given and the - daemon never did. Existence is tested with `symlink_metadata` rather than - `exists`, because a dangling symlink reports absent from the latter while - still being a file the write acts on. The persistent read path additionally - warns when it finds a key file whose mode is looser than 0600, or one that is - a symlink; it does not repair either, since the daemon does not own a file it - did not create. - -#### Peer lifecycle / gateway - -- A failed log write can no longer panic the thread or task that logged. The - subscriber was built with the default internal-error reporting, which sends a - failed write to `eprintln!`, and that macro panics when stderr has also - failed. The shipped supervisor configurations make that a single condition - rather than two: the macOS plist points both standard streams at one - unrotated file, and the systemd units route both to journald, so one full - disk fails both sinks together. In the daemon a crypto worker was the case - that mattered: it logs a warning on send backpressure, and a worker that - dies takes its share of the peer space with it permanently, while the panic - message is discarded along the same broken path. In `fips-gateway`, which - built its subscriber the same way, the casualty is a spawned task: the DNS - resolver, the control accept loop or the pool tick, none of which is observed - until shutdown, so the process would keep running and reporting healthy with - mesh name resolution or lease expiry and NAT cleanup silently stopped. - -#### macOS install layout - -- macOS: `peers.allow`, `peers.deny`, and the `hosts` file are now read - from `/usr/local/etc/fips/`, matching the install layout the macOS - packaging ships (`packaging/macos/`). That layout is what the three fixes - in this group align the daemon and `fipsctl` to. The default-path constants - were hardcoded to `/etc/fips/...` with only a `#[cfg(unix)]` / - `#[cfg(windows)]` split, so on macOS the daemon looked in a directory that - does not exist: `load_file` / `load_hosts_file` hit their `NotFound` no-op - arm and silently returned an empty ACL / empty host map. A populated - `peers.deny` therefore reported `effective_mode: "default_open"` and - `enforcement_active: false` via `fipsctl acl show`, and host-file aliases - went unloaded, with no error or warning. The default constants now follow - the platform's packaging (`/usr/local/etc/fips/` on macOS, `/etc/fips/` on - Linux and other Unix for the ACL files, and `/etc/fips/` on Linux and - `%ProgramData%\fips\` on Windows for the hosts file) and are pinned by - platform-gated unit tests so the layout cannot silently drift again. At - startup the daemon warns once if any of these files exist at the old - `/etc/fips/` location but not at the current default. Linux and Windows - behavior is unchanged. Contributed by - [@sh1ftred](https://github.com/sh1ftred). - **macOS users with existing files in `/etc/fips/` should move them to - `/usr/local/etc/fips/`.** - -- macOS: `fipsctl keygen` now writes `fips.key` / `fips.pub` to - `/usr/local/etc/fips/` by default. The default output directory was - hardcoded to `/etc/fips` for all Unix, but the daemon derives its identity - key paths from the config file's directory, which is - `/usr/local/etc/fips/fips.yaml` on macOS, so a generated identity landed - where the daemon never reads it and the node silently kept an ephemeral - identity. Linux and other Unix keep `/etc/fips`, Windows is unchanged, and - the values are pinned by platform-gated unit tests. - -- macOS: the system-wide config search path now includes - `/usr/local/etc/fips/fips.yaml` in addition to `/etc/fips/fips.yaml`. - Previously only `/etc/fips/fips.yaml` was probed, so a bare `fips` run - without `--config` skipped the installed config and derived identity key - paths from a non-existent directory. `/etc/fips/fips.yaml` is still probed - first so existing installs keep working. Both the macOS entry in the search - path and the directory `fipsctl keygen` writes to read the shared - `SYSTEM_CONFIG_DIR` constant, so the two cannot drift apart. The - launchd-installed daemon was unaffected (it always passes `--config`). - Linux and Windows behavior is unchanged. Because the daemon derives the - identity key directory from whichever config file loaded last, a macOS host - carrying `fips.yaml` at both locations would have resolved `fips.key` to the - new directory, found none, and under `persistent` generated a fresh - identity, silently changing its npub, routing address and mesh IPv6. The - daemon now adopts a key stranded at `/etc/fips/fips.key` and warns to move - it, instead of generating one. The fallback is confined to keys resolved - from the system config directory, so a run using `./fips.yaml` or a user - config is never redirected to a system key. - -#### Packaging & deployment - -- The maintainer address published in package metadata no longer bounces. The - crate authors field, the Debian package maintainer and upstream contact, and - both AUR PKGBUILD maintainer lines carried an address that no longer accepts - mail, so the contact of record in every artifact we ship was unreachable. - -### Security - -#### FMP/FSP session integrity +#### Security - A frame whose counter is `u64::MAX` is now refused by the replay window instead of being accepted as a new high-water mark. Accepting it pinned @@ -1597,7 +1100,104 @@ with v0.4.x or earlier peers. phase dispatch and it applies to established data frames as well as to handshakes; that counter is not yet readable through the control socket. -#### NAT traversal / Nostr discovery +### NAT traversal and Nostr discovery + +#### Added + +- `node.discovery.nostr.max_concurrent_offers_per_npub`, defaulting to 4, which + bounds how many inbound traversal offers one sender npub may have in flight + at once. It sits inside `max_concurrent_incoming_offers`, which remains the + outer bound, so a value above that is inert; zero is rejected at config + validation, since it refuses every inbound offer rather than disabling the + limit, and so is a value above the maximum permit count a semaphore can + hold, which would otherwise fail at construction rather than at load. + Existing configurations parse unchanged, the key being optional. + +#### Changed + +- Inbound traversal offers are now admitted against a per-sender allowance as + well as the global pool. The intake path previously took a permit from a + single semaphore before any identity check, with the sender's npub used only + as a log field, so one sender could hold every slot and deny traversal + onboarding to every other peer for as long as it kept offering. Admission now + takes a per-npub permit and a global permit together. A sender over its own + allowance is refused at debug rather than warn, because the party tripping it + is by definition sending faster than the node wants and a record per + rejection would turn the spam into log volume; the global bound being reached + keeps its warn, which is the operator's signal that the node is genuinely + saturated. **This does not make the pool inexhaustible.** Nostr identities + cost nothing to generate and the signal subscription carries no author + restriction, so an attacker running four throwaway npubs still saturates the + shipped 16-slot pool at an unchanged total offer rate. What the change buys + is that one identity can no longer do it alone, and that the two refusals are + distinguishable in the log. The permit is still held across the whole + attempt; that duration remains inferred from the attempt timeout rather than + measured. + +- Config validation now rejects a `node.discovery.nostr.signal_ttl_secs` that + is too large for the configured `replay_window_secs`. A traversal signal is + acceptable over its TTL plus 60s of clock-skew grace on each side, and that + span has to stay strictly inside the replay window, or a session id evicted + from the replay cache on expiry is still fresh enough to be accepted a second + time. The relation was documented but unenforced, so raising the TTL past + 180s silently voided it. The bound is derived from the skew constant rather + than restated, and is checked whether or not nostr discovery is enabled, for + the same reason the rekey rules are. The shipped defaults (120s against 300s) + are unaffected, but a configuration that had widened the TTL or narrowed the + replay window now fails to load, with an error naming the concrete floor for + `replay_window_secs`. The NAT lab's config generator was one such + configuration and its generated `replay_window_secs` moves from 60 to 180. + Note that this covers eviction on expiry only: `seen_sessions_max_entries` + remains a separate capacity-eviction route that no config relation bounds. + +- The peer-retry tick no longer awaits the Nostr advert refetch. It ran inline + on the 1-second rx-loop tick, awaiting a fetch with a 2-second timeout for + each due peer and discarding the result; with up to sixteen due peers the + timeouts stacked, and field profiling measured single 2.00 s stalls as the + common case and a worst tick of 12.4 s against a 1 s period, delaying every + other rx-loop arm by as much as 4.2 s. The refetch is now spawned, so a dial + uses the advert cached at that moment and the refreshed one lands for that + peer's next retry. + +- The `Adopted NAT traversal socket` log line now carries the transport id and + the local address alongside the peer npub. Without the local address an + operator cannot join a host socket table against adoption events, and without + the transport id several peers sharing one adopted transport are + indistinguishable from several separate adopted transports. + +#### Fixed + +- Nostr NAT traversal signals are now sent only to relays the client pool + actually holds. A signal is addressed to the merge of the peer's NIP-17 inbox + relays, the relays its advert nominates for signaling, and our own DM relays, + but the pool is built once at startup from the configured relays and the send + is rejected outright, before anything is contacted, if any single URL in that + list is outside it. One unconfigured relay anywhere in the merge therefore + killed the whole attempt, including the sends to relays both sides shared. On + a public node in open mode this made discovery non-functional: 309 traversal + attempts, 290 explicit failures, zero successes, every failure on `relay not + found`. Configured peers were unaffected, since they run a matching relay + set. Comparison is on the normalized relay URL rather than the raw string, so + a configured relay spelled with a trailing slash or different host case is + not discarded. Two smaller fixes ride along: the responder resolves its + relays before binding a socket and running STUN, rather than spending a STUN + round trip and holding an offer slot only to find it has nowhere to answer, + and it gained the empty-relay-list guard the initiator already had. + +- Nostr NAT traversal no longer breaks after the host suspends. The traversal + clock cached a Unix timestamp once at startup and advanced it with a + monotonic `Instant`, which does not tick while a machine is asleep, so after + a suspend the daemon's idea of the time trailed real time by the suspend + duration for the rest of the process lifetime. Every NIP-40 expiration it + computed was therefore published already in the past: relays dropped the + offers as expired, the initiator logged a signal timeout waiting for an + answer, and traversal stayed broken until the daemon was restarted. The + clock now reads the wall clock on every call. This is not macOS-specific, + though a laptop that sleeps is where it is easiest to hit; any host that + suspends or hibernates was affected. Reported in + [#128](https://github.com/jmcorgan/fips/issues/128). + +#### Security - An advert or inbox-relay list returned by a relay is now checked against the peer it claims to describe before anything else looks at it. The relay @@ -1796,30 +1396,73 @@ with v0.4.x or earlier peers. attributes the acceptance to clock skew, since a peer configured with a longer signalling TTL than ours now reaches it too. -#### DNS responder +### Data plane, routing signals and metrics -- The DNS responder's mesh-interface filter now works on macOS and FreeBSD, - where it had never run. The filter drops `.fips` queries that arrive over the - mesh TUN, which is what keeps a widened `dns.bind_addr` from exposing the - hosts file's alias space to every mesh peer. It was keyed on the interface - index resolved from the *configured* TUN name, but macOS and FreeBSD assign - the device a name of the kernel's choosing (`utunN`, `tunN`), so the lookup - found nothing, the index came back `None`, and `None` disables the filter. - The index is now resolved from the name of the device the node actually - created, which the TUN startup path already records, and a live device whose - index will not resolve is logged rather than passed off as "no mesh - interface". Linux is unaffected, since the configured name is the device's - name there. **Behaviour change on macOS and FreeBSD**: a node with a - non-loopback `dns.bind_addr` stops answering `.fips` queries that arrive over - the mesh interface. **What this does not close**: with an app-owned TUN the - node never learns a device name, so the filter stays off there. **Not - measured**: whether macOS and FreeBSD attribute a locally originated query - sent to the node's own mesh address to the TUN interface, as Linux does. If - they do, such a query is now dropped on those platforms; the shipped resolver - drop-in targets `[::1]` rather than the mesh address, so the packaged path is - not affected. +#### Changed -#### Data-plane / routing signals +- Peer bloom filters are computed for every recipient in one prefix and suffix + union sweep rather than rebuilt per recipient. Announcing to R peers + previously did R full map builds and R by T merges; at 240 peers that was + 20.6 ms per tick, roughly half the tick body, with a median per-interval + maximum of 34.5 ms. The result is exactly equal rather than approximately: + merging is a bytewise OR, so regrouping the unions cannot change it. The + trade-off, measured rather than assumed, is that the sweep does its full work + regardless of how many peers are ready, so a tick announcing to one or two + peers now costs about twice what it did; break-even is around three ready + peers. Cadence, the debounce, the sequence rule and the fill-ratio cap are + unchanged. + +- Each peer's npub is derived once at construction instead of once per tick. + The per-tick stats snapshot ran a bech32 encode for every tracked peer, and a + second one for the common peer with no hosts-file entry and no alias, since + the display-name fallback bottoms out in the same encode: 14.1 ms per tick at + 240 peers. The display name itself is deliberately not cached, because the + alias map and the host map both mutate at runtime. + +#### Fixed + +- A SessionDatagram carrying a truncated inner FSP payload no longer panics the + forwarding path. The coordinate-cache warm path sliced the inner payload at + the full 12-byte header offset while guarding only with the 4-byte common + prefix parser, so an inner payload of 4 to 11 bytes with phase 0x0 and the + Coords Present flag set indexed past the end of the slice. Because the + receive loop is the process's main future, the panic terminated the daemon + rather than a task, and under the packaged systemd unit the node restarted + into the same frame. The warm path now applies the same + `FspEncryptedHeader` guard the local-delivery path already used, which + additionally means a malformed frame carrying a non-zero protocol version or + the Unencrypted flag alongside Coords Present is dropped rather than having + its body read as coordinates. Any peer that had completed a link handshake + could trigger this, and admission is default-open. Frames rejected by that + guard are now counted in the forwarding statistics as + `warm_malformed_packets` and `warm_malformed_bytes`, the byte counter + charging the whole outer frame, visible over the control socket and on the + fipstop Routing State pane, so a node being fed malformed frames is + distinguishable from a quiet one at the default log level. The count is not a + packet drop: the frame is still delivered or forwarded, and only the + coordinate-cache warm attempt is abandoned. The existing debug log now also + carries the frame's protocol version and flags, which separate a short frame + from a bad-version or Unencrypted-flagged one. + +- `SessionDatagram` hop-limit handling now follows IP semantics. Delivery to + the addressed node is no longer TTL-gated, and a forwarder decrements before + deciding rather than after, so a datagram that would leave with a TTL of zero + is dropped instead of transmitted. Previously the TTL check ran ahead of the + local-delivery test, so a datagram addressed to this node that arrived with + TTL 0 was dropped, and a forwarder receiving a transit datagram at TTL 1 + transmitted it at TTL 0 for the next hop to discard, wasting one transmission + per expiring datagram. `SessionDatagram::decrement_ttl` and + `SessionDatagram::can_forward` were aligned to the same semantics: + `decrement_ttl` decrements first and reports false when the result is zero, + and `can_forward` is true only at a TTL of 2 or more. The reachable radius is + unchanged, because the two behaviors compensated exactly: a path of `h` links + still delivers for any source TTL of `h` or more. During a rolling upgrade, an + unupgraded forwarder feeding an upgraded destination delivers one hop further + than either version does on its own; no version mix delivers less far. The + `TtlExhausted` reject counter now charges at the node that makes the decision + rather than at the hop after it. + +#### Security - A transit node's induced routing errors are now bounded by the authenticated link peer that induced them. The 100 ms suppression gate on @@ -1859,6 +1502,7 @@ with v0.4.x or earlier peers. is an emission change only: an unmodified peer parses the frame exactly as before. Which of the two signals is emitted still discloses whether the entry exists. + - A reactive `MtuExceeded` is now believed only when this node has actually sent a frame larger than the bottleneck it reports. The signal is unauthenticated: the admission gate narrows which destination may be named @@ -2025,7 +1669,54 @@ with v0.4.x or earlier peers. the fipstop routing pane. A refused request keeps its dedup entry, and retries carry fresh request_ids, so a refusal cannot suppress the retry. -#### Admission / peer caps +### Admission, rate limiting and peer caps + +#### Added + +- `node.rate_limit.session_setup_burst` (64) and + `node.rate_limit.session_setup_rate` (16.0), the parameters of the new + per-link-peer session-setup limiter. This is the FSP session-setup bucket, + and it is distinct from the link-layer msg1 bucket described below; the two + meter different messages and are sized independently. Setup messages naming + a peer this node is already established with are metered on a second + per-link bucket derived from `node.limits.max_peers`, + `node.rekey.after_secs` and `node.rate_limit.handshake_max_resends`, so + raising the peer limit sizes it automatically. A zero burst or a + non-positive rate is rejected at config validation rather than silently + refusing every session. + +- `node.limits.max_sessions`, defaulting to 1024, which bounds the end-to-end + session table. Zero means unlimited, which restores the previous behaviour + exactly and is the way to back the change out on a running node. The default + is four times the adjacent `node.session.pending_max_destinations`. A + session entry measures 6608 bytes of inline state plus heap, so the table + holds to roughly 7 MB, and a test pins that per-entry figure so the + arithmetic behind the default fails loudly if an entry grows. Existing + configurations parse unchanged, the key being optional. + +- `node.rate_limit.established_handshake_burst` and + `node.rate_limit.established_handshake_rate`, the parameters of the new + established-link msg1 token bucket, which meters link-layer msg1 rather + than FSP session setup. Both are optional; omitting them (the normal case) + derives the bucket from `node.limits.max_peers`, `node.rekey.after_secs` + and `node.rate_limit.handshake_max_resends`, so raising the peer limit + sizes the bucket automatically. An explicit zero burst or a non-positive + rate is rejected at config validation rather than silently refusing all + rekey traffic. + +#### Changed + +- Inbound msg1 is classified before it is rate limited, and rekey or restart + msg1 arriving on an established link now draws on its own token bucket + instead of competing with stranger admission for a single shared one. On a + node with many peers the shared bucket refused a large share of ordinary + rekey traffic: a field node at roughly 245 peers refused 8753 msg1 in 25 + minutes, and 159 of the 201 distinct sources were peers it already held + sessions with. Nodes upgrade with no config change. The `Msg1 rate limited` + log line now reports which limb refused, the pending count or the token + bucket, which it previously did not distinguish. + +#### Security - The Ethernet transport's discovery buffer is now bounded and no longer costs a linear scan per beacon. Beacons are unauthenticated broadcast frames, and @@ -2118,7 +1809,135 @@ with v0.4.x or earlier peers. well-formed frame and then goes silent still holds its slot. Closing that needs a rolling idle deadline. -#### Gateway +### Transports and configuration + +#### Changed + +- `fipsctl keygen` no longer exits non-zero when only the `fips.pub` write + fails. The private key is already on disk at that point, so failing the run + reported failure for a keygen that did produce the identity; the failure is + now a warning and the run succeeds. The pre-existing-key guard also moves + from `exists` to `symlink_metadata`, so a dangling symlink at the key path + now blocks keygen without `--force` instead of being overwritten silently. + +#### Fixed + +- The UDP transport's DNS cache is now bounded and actually evicts. The map + held one entry per distinct hostname string ever dialed, and the TTL was + applied only on the read, so a stale entry was overwritten on the next dial + of the same name and otherwise stayed for the life of the process. Under a + rendezvous policy that accepts advertised endpoints the keys are strings a + remote party chose, which made the growth theirs to drive. A store now + sweeps entries past their TTL and, if the map is still full, drops the + oldest, holding it to 256 hostnames. Refreshing a name already cached + evicts nothing. Eviction is by insertion time rather than last use, so a + rarely dialed name in a very large peer list may re-resolve more often; the + cost of a wrong eviction is one DNS lookup, not a failed dial. + +- macOS: stopping an Ethernet transport under load no longer hangs the + process. The BPF reader thread handed each frame to the async consumer with + `blocking_send`, which parks with no way to be woken. Stopping the transport + aborts the consumer first, so nothing drains the 1024-frame channel, and the + socket's `Drop` then joined a thread that could never return: on a busy + interface the daemon had to be killed. The socket now drops the receiver + before joining, which releases a parked send at once, and the reader thread + sends through a helper that watches the same shutdown pipe its `select()` + already honours, so a send waiting for room cannot outlive a shutdown + request. The helper yields before it sleeps, so the saturated-path handoff + rate is unchanged. **Not covered by CI**: the reader thread is macOS-only + and Linux CI compiles none of it. What the tests prove is that the helper + the thread now waits in is cancellable; that a real BPF thread exits under + load still needs a manual check on a Mac. + +- A failed private-key write no longer leaves a node silently running an + ephemeral identity. Six write results in the identity path were discarded, + and the sharpest was in `persistent` mode: a failed write to `fips.key` fell + through to an ephemeral identity with no message, so a node that had been + asked for a stable identity changed its npub, its routing address and its + mesh IPv6 on every start, and nothing said so. All six now report. An + ephemeral start that is about to overwrite an existing key file now warns + first, naming the path and the setting that would have preserved the + identity, which is the warning `fipsctl keygen` has always given and the + daemon never did. Existence is tested with `symlink_metadata` rather than + `exists`, because a dangling symlink reports absent from the latter while + still being a file the write acts on. The persistent read path additionally + warns when it finds a key file whose mode is looser than 0600, or one that is + a symlink; it does not repair either, since the daemon does not own a file it + did not create. + +### Spanning tree, mesh size and routing + +#### Fixed + +- Flap dampening can now engage more than once in the lifetime of a node. + The arming check tested whether a dampening deadline had ever been set + rather than whether one was still in effect, so the first episode + disarmed the mechanism permanently: a node in a second flap storm went on + switching parents under hold-down alone, and neither the `flap_dampened` + counter nor the "Flap dampening engaged" warning fired again, so the + storm was invisible to anyone watching that counter. A lapsed episode is + now retired explicitly, clearing both the deadline and the switch + counter, so a second episode requires a fresh threshold of switches + within one window rather than re-engaging on the first switch after + lapse. Hold-down was unaffected throughout and continued to limit + discretionary switching, which is why the practical effect at shipped + settings was lost visibility and a lost escalation tier rather than + unrestrained flapping. Every path that can engage an episode now reports + it, including a re-engagement during parent-loss recovery, which was + previously silent. The warning names which path armed the episode + (`trigger`) and how long discretionary parent switching stays suppressed + (`dampening_secs`), using the same `trigger` values as the parent-switch + logs beside it, so the two can be read together. A + `node.tree.flap_dampening_secs` large enough to overflow the monotonic + clock is capped at one year, beyond which an episode is + indistinguishable from permanent, so an extreme setting no longer panics + the node when dampening engages. + +### DNS responder + +#### Security + +- The DNS responder's mesh-interface filter now works on macOS and FreeBSD, + where it had never run. The filter drops `.fips` queries that arrive over the + mesh TUN, which is what keeps a widened `dns.bind_addr` from exposing the + hosts file's alias space to every mesh peer. It was keyed on the interface + index resolved from the *configured* TUN name, but macOS and FreeBSD assign + the device a name of the kernel's choosing (`utunN`, `tunN`), so the lookup + found nothing, the index came back `None`, and `None` disables the filter. + The index is now resolved from the name of the device the node actually + created, which the TUN startup path already records, and a live device whose + index will not resolve is logged rather than passed off as "no mesh + interface". Linux is unaffected, since the configured name is the device's + name there. **Behaviour change on macOS and FreeBSD**: a node with a + non-loopback `dns.bind_addr` stops answering `.fips` queries that arrive over + the mesh interface. **What this does not close**: with an app-owned TUN the + node never learns a device name, so the filter stays off there. **Not + measured**: whether macOS and FreeBSD attribute a locally originated query + sent to the node's own mesh address to the TUN interface, as Linux does. If + they do, such a query is now dropped on those platforms; the shipped resolver + drop-in targets `[::1]` rather than the mesh address, so the packaged path is + not affected. + +### Gateway and peer lifecycle + +#### Fixed + +- A failed log write can no longer panic the thread or task that logged. The + subscriber was built with the default internal-error reporting, which sends a + failed write to `eprintln!`, and that macro panics when stderr has also + failed. The shipped supervisor configurations make that a single condition + rather than two: the macOS plist points both standard streams at one + unrotated file, and the systemd units route both to journald, so one full + disk fails both sinks together. In the daemon a crypto worker was the case + that mattered: it logs a warning on send backpressure, and a worker that + dies takes its share of the peer space with it permanently, while the panic + message is discarded along the same broken path. In `fips-gateway`, which + built its subscriber the same way, the casualty is a spawned task: the DNS + resolver, the control accept loop or the pool tick, none of which is observed + until shutdown, so the process would keep running and reporting healthy with + mesh name resolution or lease expiry and NAT cleanup silently stopped. + +#### Security - The gateway DNS forwarder now validates an upstream answer before it becomes a NAT mapping. It previously accepted whatever datagram arrived: the upstream @@ -2139,7 +1958,9 @@ with v0.4.x or earlier peers. NXDOMAIN. Connecting the socket also means a dead upstream surfaces ECONNREFUSED immediately instead of stalling for five seconds. -#### Control socket +### Control socket + +#### Security - The control socket and the directory holding it are now created with a restrictive mode rather than created wide and narrowed afterwards. `bind(2)` @@ -2163,7 +1984,9 @@ with v0.4.x or earlier peers. holding it can deny the daemon its socket more simply by squatting the path first. -#### Key material and identity files +### Key material and identity files + +#### Security - Private key writes no longer follow a symlink, and the key file's mode is enforced rather than merely requested. The single write path opened with @@ -2209,7 +2032,23 @@ with v0.4.x or earlier peers. See the `### Changed` note above for the source-breaking effect the four new `Drop` implementations have on library consumers. -#### Supply chain +### Library API + +#### Changed + +- **Source-breaking for consumers of the library crate**: four public types now + implement `Drop`, so their fields can no longer be moved out. `Identity`, + `ResolvedIdentity`, `IdentityConfig` and `HandshakeState` each gained one as + part of clearing key material at end of scope. `IdentityConfig` is the one + most likely to be reached in practice, since it hangs off the public `Config` + as `node.identity`, so code that moved the nsec out of a configuration value + no longer compiles and needs `Option::take` instead. Nothing about the + behaviour of the shipped binaries changes; this affects only callers using + `fips` as a library. + +### Supply chain + +#### Security - The dependency lockfile is refreshed past a set of advisories against the pinned `nostr` 0.44.3 and `nostr-relay-pool` 0.44.1, both of which were also @@ -2262,7 +2101,163 @@ with v0.4.x or earlier peers. that is integrity, not authenticity, because upstream publishes no detached sums. -#### Docs & contributor tooling +### Packaging, install and platform layout + +#### Fixed + +- macOS: `peers.allow`, `peers.deny`, and the `hosts` file are now read + from `/usr/local/etc/fips/`, matching the install layout the macOS + packaging ships (`packaging/macos/`). That layout is what the three fixes + in this group align the daemon and `fipsctl` to. The default-path constants + were hardcoded to `/etc/fips/...` with only a `#[cfg(unix)]` / + `#[cfg(windows)]` split, so on macOS the daemon looked in a directory that + does not exist: `load_file` / `load_hosts_file` hit their `NotFound` no-op + arm and silently returned an empty ACL / empty host map. A populated + `peers.deny` therefore reported `effective_mode: "default_open"` and + `enforcement_active: false` via `fipsctl acl show`, and host-file aliases + went unloaded, with no error or warning. The default constants now follow + the platform's packaging (`/usr/local/etc/fips/` on macOS, `/etc/fips/` on + Linux and other Unix for the ACL files, and `/etc/fips/` on Linux and + `%ProgramData%\fips\` on Windows for the hosts file) and are pinned by + platform-gated unit tests so the layout cannot silently drift again. At + startup the daemon warns once if any of these files exist at the old + `/etc/fips/` location but not at the current default. Linux and Windows + behavior is unchanged. Contributed by + [@sh1ftred](https://github.com/sh1ftred). + **macOS users with existing files in `/etc/fips/` should move them to + `/usr/local/etc/fips/`.** + +- macOS: `fipsctl keygen` now writes `fips.key` / `fips.pub` to + `/usr/local/etc/fips/` by default. The default output directory was + hardcoded to `/etc/fips` for all Unix, but the daemon derives its identity + key paths from the config file's directory, which is + `/usr/local/etc/fips/fips.yaml` on macOS, so a generated identity landed + where the daemon never reads it and the node silently kept an ephemeral + identity. Linux and other Unix keep `/etc/fips`, Windows is unchanged, and + the values are pinned by platform-gated unit tests. + +- macOS: the system-wide config search path now includes + `/usr/local/etc/fips/fips.yaml` in addition to `/etc/fips/fips.yaml`. + Previously only `/etc/fips/fips.yaml` was probed, so a bare `fips` run + without `--config` skipped the installed config and derived identity key + paths from a non-existent directory. `/etc/fips/fips.yaml` is still probed + first so existing installs keep working. Both the macOS entry in the search + path and the directory `fipsctl keygen` writes to read the shared + `SYSTEM_CONFIG_DIR` constant, so the two cannot drift apart. The + launchd-installed daemon was unaffected (it always passes `--config`). + Linux and Windows behavior is unchanged. Because the daemon derives the + identity key directory from whichever config file loaded last, a macOS host + carrying `fips.yaml` at both locations would have resolved `fips.key` to the + new directory, found none, and under `persistent` generated a fresh + identity, silently changing its npub, routing address and mesh IPv6. The + daemon now adopts a key stranded at `/etc/fips/fips.key` and warns to move + it, instead of generating one. The fallback is confined to keys resolved + from the system config directory, so a run using `./fips.yaml` or a user + config is never redirected to a system key. + +- The maintainer address published in package metadata no longer bounces. The + crate authors field, the Debian package maintainer and upstream contact, and + both AUR PKGBUILD maintainer lines carried an address that no longer accepts + mail, so the contact of record in every artifact we ship was unreachable. + +### Docs, CI and contributor tooling + +#### Added + +- `SECURITY.md`, stating a private channel for vulnerability reports, what a + useful report contains, what a reporter can expect back and on what timing, + and which branches receive fixes. The repository previously documented no + reporting channel at all, so someone with a finding had to guess at an + address or open a public issue. + +#### Changed + +- Two CI runs on one machine can no longer collide. Every suite derives its own + docker build context, image tag, container names, network range and host + interface names per run, so concurrent runs cannot reap each other's + containers or contend for a fixed subnet. This is what a contributor running + `testing/ci-local.sh` alongside a GitHub run, or two local runs at once, sees + change: the runs stay independent instead of one killing the other. + +- A test that does not run, or whose result cannot be read, no longer passes + silently. A failed scenario now fails the run rather than being logged and + stepped over, a node whose logs cannot be read no longer counts as clean, an + unanswered control query no longer reads as zero, an unknown scenario key is + rejected instead of matching nothing, and a skipped check appears in the + final verdict rather than only in scrollback. + +- New guards run in both the local and GitHub runners, so the two gates agree. + They check that trailing-log call sites are wired, that the log strings the + harness matches on are still emitted by the daemon, that the two runners' + integration-suite sets match per leg rather than as a folded token, that + every GitHub Action reference is pinned in the required form, and that source + comments do not cite references a reader of the published tree cannot + resolve. + +- Coverage moved from Docker to deterministic in-process tests, and dead + scenarios were retired. The six cost-selection chaos scenarios, the + admission-cap and acl-allowlist Docker suites, the smoke-10 scenario, the + tcp-chain and mesh-public static topologies, and three ignored Ethernet + tests are gone, with their behaviour asserted in unit and integration tests + instead. A local CI run is correspondingly shorter and less dependent on + container timing. + +- The `bloom-storm` chaos scenario no longer runs on either the local or the + cloud runner. Unlike the retirements above it has no replacement: the + scenario files remain in the tree and it stays runnable by hand, but nothing + now exercises downstream containment of a mid-chain ancestor swap on a + schedule. This is recorded as a coverage gap rather than as a completed + migration. + +- A failing harness now says why it failed. The dns-resolver suite sent build + and container-start output to `/dev/null`, so a failed scenario reported + that it had failed and nothing else; output is now captured and emitted on + failure, naming the command, and the systemd readiness wait dumps container + state, failed units and the journal when it gives up. The NAT-lab path + assertions exited bare, printing neither what they expected nor what they + saw and triggering none of the scenario diagnostics their siblings already + call; all twelve call sites now report the container, the expectation, the + observation and a projection of the peer or link table, and distinguish a + failed control-socket exec from unparseable output from a genuine mismatch. + The convergence gate could not tell a tree that did not converge from + connectivity that failed, and could exit non-zero while reporting "20 + passed, 0 failed"; it now records the outcome, the count reached and the + count pending, and its failure messages name the condition. A passing run + is as quiet as before, and no timing, threshold or control-flow behaviour + changed in any of the three. + +- A dns-resolver scenario no longer burns the full 30-second boot timeout and + warns about a container that booted correctly. The readiness poll ran under + `pipefail` and piped `systemctl is-system-running` into `grep`, and that + command exits non-zero when the system is degraded, which is where systemd + inside a container always settles; the pipeline therefore failed even when + the pattern matched, leaving the degraded branch dead. The poll now matches + on the captured state instead of piping into `grep`. + +- The chaos harness now checks that teardown and node stops did what they + report. `docker compose down` exits 0 while leaving a run's containers + alive, so a partly-failed bring-up leaked named containers with nothing to + detect it; teardown now asks whether the containers this run owns are gone, + treats a survivor that forced removal clears as a warning, and aborts with + the names written to an artifact when one survives that or the query cannot + run at all. The check is scoped to a run's own names, so concurrent runs + cannot trip each other. Node churn separately marked a node down whether or + not `docker stop` succeeded, so the simulation's model of the mesh diverged + from reality, and `nodes_down`, the `max_down_nodes` cap and the + connectivity guard are all computed from that model; a failed stop now + warns, carries the daemon's own message, and leaves the node out of the + down set for the next churn tick to retry against an honest model. + +- Comments throughout the source tree, the packaging files and the test scripts + no longer cite internal identifiers, planning documents or private stage names + that a reader of the published tree cannot resolve; each now states the thing + the citation stood for. A handful of comments that described behaviour the + code does not have (the control-plane read path, its snapshot dispatch, and + the MMP report types) have been corrected rather than merely reworded. One of + the edited files, the DNS setup helper, installs to `/usr/lib/fips` on every + packaging path, so its comment reached users. No code changed. + +#### Security - The security reference now records that both Noise patterns deviate from the standard construction in one respect: the handshake AEAD passes an empty diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 6bab5620..8855a003 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -1,146 +1,676 @@ -# FIPS v0.4.1 +# FIPS v0.4.2 -**Released**: 2026-07-19 +**Released**: 2026-08-25 -v0.4.1 is a maintenance release on the v0.4.x line. It raises the default -antipoison cap on inbound bloom filter announcements, removes a redundant -spanning-tree metric counter, fixes two convergence and path-MTU bugs, and -cuts per-packet CPU in the bloom and identity paths. There is no wire -format change and no new feature surface. +v0.4.2 is a maintenance release on the v0.4.x line, and the largest one +this line has carried: 144 commits since v0.4.1. Most of it is security +work. It closes several paths by which a party that could merely reach a +node could take its sessions down, have its traffic attributed to +another node, or steer that node's path MTU; it bounds a set of tables +an unauthenticated party could grow without limit; it closes the +fail-open cases where a local error widened what a node accepted; it +fixes NAT traversal in two places where it was simply not working; and +it protects private key material on disk and clears it in memory. There +is no wire format change. -v0.4.1 is wire-compatible with v0.4.0. Nodes can be upgraded one at a time -with no coordinated restart, though one behavior change below is worth -reading before you start a rolling upgrade. +The new configuration surface is small and optional: three admission and +rate-limiting keys, each with a default that needs no action. The +repository also gains a `SECURITY.md`, so someone with a finding no +longer has to guess at an address or open a public issue. + +v0.4.2 is wire-compatible with v0.4.1. No frame gains, loses, or resizes +a field, so a mixed mesh works and nodes can be upgraded one at a time +with no coordinated restart. Three changes narrow what a node accepts, +or change how it acts on a field it already read: the session datagram +hop limit, the path MTU floor, and routing-signal admission. Those three +are what the interop gate is pointed at deliberately, rather than at +connectivity alone. Compatibility is the release's intent and what that +gate checks; it is not a claim that every mixed pairing was exercised. + +**Read the upgrade notes before you start.** Two configuration shapes +that loaded in v0.4.1 now refuse to start. ## At a glance -- `node.bloom.max_inbound_fpr` default moves from `0.10` to `0.20`. -- The `parent_switched` metric counter is gone. Use `parent_switches`. -- Spanning tree no longer serves stale coordinates after a parent link is - lost through peer removal. -- Discovery no longer loosens a path MTU clamp it had correctly tightened. -- Bloom probing and identity operations do measurably less work per call, - with identical results. +### Before you upgrade + +- Two configuration shapes now fail to load: a `node.rekey` interval + that fires the trigger continuously, and a traversal signal TTL too + large for the replay window. Both are start-time failures, so a node + carrying either will not come back after a package upgrade. +- macOS installs now read the ACL, hosts, config and identity files from + `/usr/local/etc/fips/`, which is where the packaging puts them. + +### Security + +- Six paths by which an unauthenticated or misattributed packet changed + a node's session state are closed. One of them terminated the daemon. +- A remote party can no longer drive a destination's path MTU to zero, + or aim a node's UDP punch packets at addresses of its choosing. +- Nineteen fixes harden a node against a party that can reach it but is + not an admitted peer: unbounded tables, unauthenticated writes and + fail-open paths. None changes the wire format and none needs action. +- Private keys are no longer written through a symlink, an existing + `fips.key` has its mode retightened on every write, and a failed key + write no longer leaves a node silently running an ephemeral identity. + +### Connectivity and performance + +- NAT traversal works in two deployments where it did not: a public node + in open mode, and any host that suspends. +- Roughly 35 ms per tick comes back at 240 peers, and multi-second + rx-loop stalls during peer retry are gone. + +### New configuration, all optional + +- `node.rate_limit.session_setup_burst` / `_rate`, + `node.rate_limit.established_handshake_burst` / `_rate`, + `node.discovery.nostr.max_concurrent_offers_per_npub`, and + `node.limits.max_sessions` (default 1024). Each default needs no + action. + +### Dependencies + +- `cargo audit` reports no vulnerability, against twelve before, and + every GitHub Action is pinned to a commit SHA. + +## A note on the security content + +Most of this release is security work, and most of that work began with +reviews the project did not commission. Over the past month a number of +unsolicited security reviews have arrived, and they share a character: +they are driven by current frontier language models, their authors say +so, and they arrive as specific, carefully written reports citing the +code they describe rather than as vague claims. + +The findings have been legitimate. Not every one survived a second +reading, and several described documented behaviour as a defect. But +enough held up under adversarial re-reading that treating this class of +report as noise would have been a mistake, and a substantial part of +what this release fixes was found that way, including issues in code +that had been reviewed before. + +None of it has been reported active in a deployment. What these reviews +have produced are reachable defects rather than observed incidents, and +finding them at that stage is the outcome everyone would choose. + +This looks like a broader shift rather than something particular to this +project. The cost of a competent first pass over an unfamiliar codebase +has fallen sharply, and open source is benefiting from it: small +projects are now getting the kind of attention that was previously +reserved for large ones. We welcome it, and we would rather receive a +report of this kind than not. `SECURITY.md` describes how to send one. +The most useful reports are the ones that say plainly which parts were +machine-generated and which were verified by a person, because that is +the difference between a lead and a finding, and we assess the two +differently. ## Behavior changes worth flagging -### The inbound filter FPR cap default doubles again +### The session datagram hop limit now follows IP semantics -`node.bloom.max_inbound_fpr` goes from `0.10` to `0.20`. The cap rejects -inbound `FilterAnnounce` frames whose advertised false positive rate -exceeds it. On the fixed 1 KB, k=5 filter, `0.10` corresponds to a fill of -0.631 and roughly 1,630 reachable entries, and the busiest nodes' -aggregates had started reaching that ceiling as the mesh grew. `0.20` -corresponds to a fill of 0.7248 and roughly 2,114 entries. +Delivery to the addressed node is no longer gated on the hop limit, and +a forwarder decrements before deciding rather than after. Two cases +change on a deployed line: -Be aware that this is the second time in two releases that this default -has doubled, for the same reason both times. That is worth stating plainly -rather than repeating the previous release's framing: raising the cap buys -headroom, it does not fix anything. The real constraint is the fixed 1 KB -filter size, which is a protocol constant. The structural remedy is the v2 -filter work, where filter capacity scales with the mesh instead of being -pinned. This release is an interim step to keep legitimate aggregates from -being rejected until that lands. It is not the start of a pattern of -raising the cap once per release, and if you are sizing capacity planning -around this number, plan against the v2 work rather than against a third -raise. +| Case | v0.4.1 | v0.4.2 | +| ---- | ------ | ------ | +| Addressed to this node, hop limit 0 | dropped | delivered | +| Transit datagram, hop limit 1 | forwarded at 0 | dropped here | -The antipoison property the cap exists for is preserved. A saturated or -deliberately poisoned filter still presents an FPR near 100% and is still -rejected. +The reachable radius is unchanged, because the two behaviors compensate +exactly: a path of `h` links still delivers for any source hop limit of +`h` or more. During a rolling upgrade no version mix delivers less far, +and an unupgraded forwarder feeding an upgraded destination delivers one +hop further than either version does on its own. -**This matters during a rolling upgrade.** A v0.4.1 node accepts a -`FilterAnnounce` with a derived FPR between 0.10 and 0.20; a v0.4.0 node -drops the same frame, and the drop is silent on the wire with no NACK. The -cap also gates the mesh size estimator, which declines to produce a value -when any contributing filter is over the cap. So while a mesh is partly -upgraded, upgraded and not-yet-upgraded nodes can legitimately report -different mesh sizes, or one can report a size while the other reports -unknown. This resolves once every node is on v0.4.1. If you want to avoid -the window entirely, set `node.bloom.max_inbound_fpr: 0.10` explicitly in -your config before upgrading and remove it after the last node is done. +What an operator will see move is the counter. `TtlExhausted` now +charges at the node that makes the decision rather than at the hop after +it, so its distribution across a mixed mesh shifts by one hop while the +upgrade is in progress. That is expected and is not a loss of traffic. -### The `parent_switched` counter is removed +### `node.rekey.enabled` governs periodic rekey only -`parent_switched` was incremented on the line immediately before -`parent_switches` at every site and never independently, so the two -counters always held the same value. `parent_switched` is now gone from -the tree metrics, the control socket snapshot, and the `fipstop` tree -view. `parent_switches` remains and is unchanged. +This is a correction to what the setting has always meant rather than a +new field. `enabled` controls whether this node *initiates* periodic +rekey. A rekey a peer drives is still answered when it is off, and two +things that used to sit behind the same gate no longer do: the session +drain sweep and the cut-over that retires an old key epoch now run +either way. A node with rekey disabled previously held superseded keys +for the life of the session. -If you scrape the control socket, or have dashboards or alerts referencing -`parent_switched`, point them at `parent_switches`. Anything still asking -for `parent_switched` will find nothing rather than a zero. +### New admission defaults you may feel + +None of these needs configuration, and none changes an existing key's +value. They are new bounds where there was none. + +- `node.discovery.nostr.max_concurrent_offers_per_npub` defaults to 4. + It sits inside `max_concurrent_incoming_offers` (16), which remains + the outer bound, so a value above that is inert. +- `node.rate_limit.session_setup_burst` (64) and `session_setup_rate` + (16.0) meter inbound FSP session setup per authenticated link peer. A + legitimate peer arriving over the same link as a flooding one shares + that link's bucket, so establishment behind a flooded neighbour is + refused until it refills. +- `node.rate_limit.established_handshake_burst` and `_rate` are optional + and normally omitted; the bucket is then derived from + `node.limits.max_peers`, `node.rekey.after_secs` and + `node.rate_limit.handshake_max_resends`, so raising the peer limit + sizes it automatically. +- An accepted inbound TCP or onion connection now has a deadline for its + first frame. This is a module constant, not a configuration key. +- A remote-supplied path MTU below an actionable minimum is ignored + rather than applied or stored. Locally derived MTUs are exempt at both + the seed and the TCP MSS clamp, so a genuinely narrow link, which the + mesh does use, still adapts. + +### Control socket snapshots carry new fields + +`show_routing` and `show_status` gained counters this cycle: +`warm_malformed_packets` and `warm_malformed_bytes`, and the four +error-signal counters `unbound_coords`, `unbound_broken`, `unbound_mtu` +and `unbound_forged`. An older `fipsctl` or `fipstop` reading a newer +daemon gains unknown fields rather than losing known ones. If you scrape +those snapshots, expect additions, not removals. Two other new counters, +the framing `payload_len_mismatch` and the setup-message refusal +counters, are not yet readable over the control socket. + +### The bloom announce sweep changes where its cost sits + +Peer bloom filters are now computed for every recipient in one union +sweep instead of being rebuilt per recipient. The result is exactly +equal, not approximately: merging is a bytewise OR. The trade-off, +measured rather than assumed, is that the sweep does its full work +regardless of how many peers are ready, so a tick announcing to one or +two peers costs about twice what it did. Break-even is around three +ready peers, so a small mesh pays slightly more and a large one pays a +great deal less. Cadence, the debounce, the sequence rule and the +fill-ratio cap are unchanged. ## Notable bug fixes -### Stale coordinates after losing a parent through peer removal +Every item here is a fix for a defect that shipped in v0.4.1. Fixes for +defects introduced and resolved inside this cycle are in the CHANGELOG +and are not repeated here. The CHANGELOG is the complete record; this +section is a selection. -When a node's parent link dropped via peer removal, the node correctly -reparented or self-rooted, but skipped the coordinate cache invalidation -that every other position-change path performs. Cached entries for -downstream destinations kept the node's old coordinate prefix. This did -not self-correct the way a stale cache entry normally would: routing -access refreshes an entry's TTL, so an entry that was actively being -routed through never expired, and was only fixed by an unrelated fresh -insert. Both invalidation classes now run on this path, matching the -loop-detection branch. +### Session and handshake authentication -### Discovery could loosen a tightened path MTU clamp +This is the release's centre of gravity. Six paths are closed, each of +which let a packet that authenticated nothing change a node's session +state. -An originator handling a `LookupResponse` overwrote its cached path MTU -unconditionally. If a reactive `MtuExceeded` or `PathMtuNotification` had -already taught it a tighter value, a later, looser discovery estimate -would clobber that and re-loosen the clamp, risking a return to dropped -oversized packets. The cached and received values are now compared and the -tighter one is kept. +- **A truncated inner payload terminated the daemon.** A + `SessionDatagram` whose inner FSP payload was 4 to 11 bytes, with + phase 0x0 and the Coords Present flag set, indexed past the end of a + slice on the coordinate-cache warm path. The receive loop is the + process's main future, so the panic took the daemon down rather than a + task, and under the packaged systemd unit the node restarted into the + same frame. Any peer past a link handshake could send it, and + admission is default-open. +- **An unauthenticated setup message could hold a session down.** With + `node.rekey.enabled` false, a setup message naming an established peer + replaced that peer's session outright, discarding the live keys. The + message carries no authenticator and its source address is an envelope + field the sender picks. The established case now arms a handshake + beside the running session and adopts new keys only after a msg3 whose + authenticated static key matches the one the session was opened with. +- **A forged `SessionAck` cancelled an in-flight initiation**, and an + unauthenticated msg3 discarded a completed key epoch. Both took 57 + bytes of the right length from anyone who could reach the node, and + both were repeatable. The setup path is now also rate limited, keyed + on the authenticated link peer the datagram arrived over rather than + on the address the sender claims. +- **A peer could complete a genuine handshake under another node's + address.** The responder recorded a session under the source address + in the datagram without checking it against the static key it had just + authenticated, so the identity cache, the session map and the + reconstructed mesh IPv6 all attributed that traffic to the node it + named. The address is now derived from the authenticated key, on both + the initial and the rekey path. +- **Routing signals were acted on for any address.** `CoordsRequired`, + `PathBroken` and `MtuExceeded` carry no end-to-end authentication, and + a node applied their effects for any destination they named. They are + now refused unless this node has itself bound that destination, and + each refusal is counted. Signals from a genuine on-path forwarder at + any distance are unaffected. +- **The established-address waiver admitted the wrong party.** A + transport with `accept_connections` false still admits an inbound msg1 + sourced from an established peer's address, so a peer re-handshaking + after a restart is not locked out. Nothing checked that the sender was + that peer, so any off-path party sourcing from the address obtained a + full link handshake from a node configured to accept none. The + handshake is now dropped once the key exchange reveals a static key + that does not belong to the identity owning that address. + +A frame whose declared payload length disagrees with the length that +arrived is also now dropped at the single dispatch point, before that +field can be used as a parsing input. This closes no known defect: on +the stream transports the comparison holds by construction, and on the +datagram transports a short frame already failed the AEAD tag. What +changes is which reason it is dropped for. + +### NAT traversal and Nostr discovery + +- **Traversal was non-functional on a public node in open mode.** A + signal is addressed to the merge of the peer's inbox relays, the + relays its advert nominates, and our own, but the send was rejected + outright if any single URL in that merge was outside the client pool + built at startup. One unconfigured relay killed the whole attempt, + including the sends to relays both sides shared. Measured in an + open-mode window: 309 attempts, 290 explicit failures, zero successes, + every failure on `relay not found`. Configured peers were unaffected, + since they run a matching relay set. Comparison is now on the + normalized relay URL, so a trailing slash or a different host case + does not discard a relay that is in fact configured. +- **Traversal broke permanently after the host suspended.** The + traversal clock cached a Unix timestamp at startup and advanced it + with a monotonic instant, which does not tick while a machine is + asleep, so the daemon's idea of the time trailed real time by the + suspend duration for the rest of the process lifetime. Every + expiration it published was already in the past, relays dropped the + offers, and traversal stayed dead until restart. The clock now reads + the wall clock on every call. A laptop is where this is easiest to + hit, but any host that suspends or hibernates was affected. Reported + in [#128](https://github.com/jmcorgan/fips/issues/128). +- **A node could be aimed at third parties.** A rendezvous-enabled node + punched every address a signed offer named, with no limit on how many + one offer could carry, so any npub could have it emit a burst of UDP + packets carrying its own source address at loopback, link-local, + multicast, broadcast, unspecified or CGNAT addresses. Never-routable + ranges are now rejected, IPv4-mapped forms are canonicalized first so + they cannot slip past, port 0 is dropped, private-range candidates are + punched only when they share a /24 with one of our own addresses, and + the planned list is capped at eight. Each planning attempt logs what + it declined and why. +- **A future-dated traversal signal was accepted as strictly fresh.** + The freshness check measured age with a saturating subtraction, which + yields zero for any timestamp ahead of the local clock, and nothing + else bounded the issue time from above. Forward-dating is now + tolerated only to the same 60s of clock skew already allowed in the + other direction, and a declared expiry is no longer trusted past the + issue time plus the configured TTL. +- **One sender could hold every inbound offer slot.** Admission took a + permit from a single pool before any identity check, with the sender's + npub used only as a log field. Admission now takes a per-npub permit + and a global permit together. This does not make the pool + inexhaustible: Nostr identities cost nothing to generate, so four + throwaway npubs still saturate the shipped 16-slot pool at an + unchanged total offer rate. What it buys is that one identity can no + longer do it alone, and that the two refusals are distinguishable in + the log. + +### Path MTU + +A single `MtuExceeded` carrying a very small value drove a session's +path MTU to zero, after which every packet to that destination was +answered with an ICMPv6 Packet Too Big instead of being sent: a +blackhole lasting until the daemon restarted. The same value reached the +SYN-time TCP MSS clamp, where anything at or below 137 saturates the +segment size to zero. The `path_mtu` field is an unsigned per-hop +annotation carried outside the signed proof, and `MtuExceeded` and +`PathBroken` arrive unencrypted with no sender check, so any forwarder, +or anyone able to reach the node, could lower it. + +Remote values below an actionable minimum are now ignored at the three +places a remote value is acted on, each with its own warning and +counter, and the per-destination cache has a way back: an entry is +released on a `PathBroken` report, on session idle expiry, and on +handshake timeout, with the local link MTU reseeded in its place. +Entries written by the discovery lookup carrier age out on a deadline of +their own, because a destination this node never opens a session with +reaches none of those three routes. Without it, one response carrying a +floor value pinned that destination's clamp until the daemon restarted. + +### Inbound connection slots and rekey admission + +- **An unauthenticated remote could lock out inbound peering by staying + silent.** The peer cap was tested at accept, with no read in between, + and the frame reader's reads carried no deadline. Pool keys are + `ip:port`, so N sockets from one address took N slots, and at the 256 + default that closed the node to new peers for as long as the sockets + stayed open. The first frame now has a deadline, the onion listener + gets the same treatment, and the handshake reaper now closes the + transport connection it used to forget. This does not close the whole + case: a peer that sends one well-formed frame and then goes silent + still holds its slot. +- **Rekey traffic was being refused on busy nodes, silently.** Rekey and + restart msg1 on an established link competed with stranger admission + for one shared token bucket. Measured on a field node at roughly 245 + peers: 8753 msg1 refused in 25 minutes, with 159 of the 201 distinct + sources being peers it already held sessions with. Nothing errored and + no session dropped, so the only symptom was a flat `rekey_armed`. + Inbound msg1 is now classified before it is limited and draws on its + own bucket. Nodes upgrade with no config change, and the + `Msg1 rate limited` line now says which limb refused. + +### Identity and key files on disk + +- **A private key write followed a symlink**, because the single write + path opened with create and truncate and no `O_NOFOLLOW`. Both writers + now share an open helper that carries it. +- **An existing `fips.key` kept a loose mode forever.** The mode was + supplied only through `open(2)`, which the kernel honours on creation + and ignores otherwise, so a key file at 0644 stayed 0644 through every + rewrite. That needs no attacker: one `chmod`, or a restore that did + not preserve modes, leaves the key readable indefinitely. The mode is + now applied to the open descriptor before any secret bytes are + written. On Windows neither protection applies and the file inherits + the parent directory's ACLs; that exclusion is deliberate. +- **A failed key write left a node running an ephemeral identity in + silence.** Six write results in the identity path were discarded, and + the sharpest was in `persistent` mode: a failed write to `fips.key` + fell through to an ephemeral identity with no message, so a node asked + for a stable identity changed its npub, its routing address and its + mesh IPv6 on every start, and nothing said so. All six now report. +- **Key material is now cleared when it goes out of scope.** Nothing in + the crate erased a key before this. Clearing now covers the session + and handshake keys, the identity keypair, the temporary copies the + elliptic-curve operations make, encoded secrets, and the private key + on its way through configuration, including the config file's text, + since `node.identity.nsec` is read straight out of it. This clears the + copies the crate owns, not every copy that ever existed: the secp256k1 + key types are copyable, and the hash, key-derivation and cached cipher + states of the pinned libraries offer no clearing route. Reading the + residue needs access to the process's memory, or to a core dump or + swap image of it. **This carries a source-breaking change for library + consumers; see the upgrade notes.** + +### Gateway DNS answers + +The gateway's DNS forwarder accepted whatever datagram arrived on its +upstream socket. The upstream query reused the client's own transaction +ID, the socket was wildcard-bound and never connected, the receive +discarded the sender, neither the response ID nor the question was +compared against what was asked, and the returned address was not +checked against the mesh prefix. Because the extracted address is +installed as a DNAT rule with no interface constraint, a forged answer +redirected traffic rather than only poisoning a lookup. + +The query now carries a random transaction ID, the socket is connected +so the kernel drops foreign sources, a response must match on ID, +question and type, and the address goes through the validating parser +before any allocation. One deliberate behaviour change: validation sits +before the rcode check, so an upstream answering FORMERR or REFUSED with +an empty question section now yields SERVFAIL rather than having its +rcode relayed. Checking after the rcode would admit a forged NXDOMAIN. + +### macOS install layout + +On macOS the daemon and `fipsctl` read `/etc/fips/`, a directory the +macOS packaging does not create, while the packaging installs to +`/usr/local/etc/fips/`. The effect was silent in the worst way: a +populated `peers.deny` reported `effective_mode: "default_open"` with +`enforcement_active: false` through `fipsctl acl show`, host-file +aliases went unloaded, and `fipsctl keygen` wrote an identity where the +daemon never read it, so the node kept an ephemeral one. The default +paths now follow the platform's packaging, the system config search path +includes `/usr/local/etc/fips/fips.yaml`, and a key stranded at the +legacy path is adopted with a warning rather than a fresh identity being +generated. Linux and Windows behavior is unchanged. + +**macOS users with existing files in `/etc/fips/` should move them to +`/usr/local/etc/fips/`.** + +### Robustness under adverse local conditions + +- **A failed log write could panic the thread or task that logged.** The + subscriber reported its own internal errors through `eprintln!`, which + panics when stderr has also failed, and the shipped supervisor + configurations make that one condition rather than two: the macOS + plist points both standard streams at one file, and the systemd units + route both to journald, so one full disk fails both sinks together. In + the daemon the casualty was a crypto worker, which takes its share of + the peer space with it permanently. In `fips-gateway` it was a spawned + task: the DNS resolver, the control accept loop or the pool tick, none + of which is observed until shutdown, so the process kept running and + reporting healthy with mesh name resolution or lease expiry and NAT + cleanup stopped. +- **Flap dampening could engage only once in a node's lifetime.** The + arming check tested whether a deadline had ever been set rather than + whether one was still in effect, so after the first episode a node in + a second flap storm went on switching parents under hold-down alone, + and neither the `flap_dampened` counter nor the warning fired again. + Hold-down was unaffected throughout, which is why the practical cost + at shipped settings was lost visibility rather than unrestrained + flapping. Separately, a `node.tree.flap_dampening_secs` large enough + to overflow the monotonic clock is now capped at one year instead of + panicking the node when dampening engages. + +### Supply chain + +- The dependency lockfile moves past a set of advisories against the + pinned `nostr` 0.44.3 and `nostr-relay-pool` 0.44.1, both of which + were also yanked. The ones that matter here are the relay-pool + advisories describing forged events bypassing signature validation and + unverified relay events being processed: that is the path a node + learns peer adverts on, and it performs no independent verification of + its own, so the exposure was a misattributed advert rather than the + denial of service the summaries lead with. `cargo audit` now reports + no vulnerability, against twelve before. Four warnings remain that no + version move fixes. +- Every GitHub Action reference is now pinned to a commit SHA. None of + the sixty-six was pinned before, including the jobs holding the AUR + deploy key, the jobs with release write scope, and the packaging jobs + that run with a signing key in the environment. Sixty-two are full + SHAs; four are justified in one place, since two actions read the tool + to install from the ref name itself. The sharper hole was not the + tags: the OpenWrt workflow fetched a helper binary and a toolchain + from release URLs with no verification at all, in two jobs holding a + signing key. Both downloads now check a per-architecture pinned + SHA-256. + +### Limits, provenance checks and fail-closed defaults + +Nineteen fixes harden a node against a party that can reach it but has +not been admitted to it. Every claim behind them was assessed and then +re-read by a separate reviewer briefed to refute it, and only what +survived that pass is here. **None changes the wire format**, and none +needs configuration. + +They fall into four shapes. + +**Tables that an unauthenticated party could grow.** The established +session table had no population cap and now defaults to 1024, tunable +with `node.limits.max_sessions`. The UDP transport's DNS cache grew one +entry per hostname ever dialed and is now bounded at 256 with eviction. +The Ethernet discovery buffer deduplicated beacons with a full scan and +had no cap; it is now a map bounded at 1024 distinct MACs. The lookup +dedup cache was fail-closed at its bound, so a flood stopped every +lookup transiting the node; it now evicts instead of refusing. + +**State an unauthenticated packet could change.** A relay-returned Nostr +advert was cached without checking that the peer it named had signed it. +A lookup response was acted on with no correlation against a lookup this +node had issued. A STUN binding response was accepted from any source. A +NAT punch packet was accepted from any address whose digest matched. +Each now checks the thing that binds it. + +**Denial paths reachable from off the path.** An epoch-mismatch `msg1` +is authentic but replayable, and accepting it tore down a working +peering; it is now refused while that peering is still carrying +authenticated traffic, and dampened against repetition. The +routing-error limiter was keyed on the field the attacker chooses. The +Nostr notify loop ran two decrypts and a signature verify per event +ahead of any limiter. A retired FSP key epoch could be held resident +indefinitely by a peer that kept using it. + +**Local fail-open surfaces.** A read error on `peers.allow` or +`peers.deny` was swallowed and published an empty ACL, which took a +strict allowlist node to admitting everyone; the last good policy is now +held and retried. The control socket and its parent directory were +created under the ambient umask and only tightened afterwards. The DNS +mesh-interface filter was keyed on the configured TUN name and so had +never run on macOS or FreeBSD. + +Some findings from the same review pass are not addressed here. Where a +fix requires a wire-format change it is not a candidate for the 0.4.x +line at all, which takes none; that work belongs to a later release. +`SECURITY.md` sets out the trust model this protocol assumes, and it is +worth reading if you are deciding how far to rely on a mesh whose +membership you do not control. + +Two portability defects are fixed alongside them: a Windows build +failure and a set of Windows and macOS unit-test failures. Both were +caught by CI on those platforms rather than by review, and the coverage +gap that let them through is recorded at the sites. + +### A documented claim that was wrong + +The security reference named both Noise patterns unqualified, which told +anyone auditing the stack against the Noise specification that the +construction was standard. It is not, in one respect: the handshake AEAD +passes an empty associated-data field where standard Noise +`EncryptAndHash` uses the handshake hash. Domain separation and +Diffie-Hellman binding survive through the chaining key; transcript +binding is the property actually absent. Nothing in the daemon reads the +handshake hash, so no shipped behaviour rests on it, but anything later +built on it (channel binding, an exporter, cookie binding) would +silently not work. The reference now says so. ## Upgrade notes -This is a drop-in upgrade from v0.4.0 with no wire format change, no -config migration, and no coordinated restart. Upgrade nodes in whatever -order you like. +There is no wire format change and no coordinated restart. Nodes can be +upgraded one at a time in any order. **One thing must be done before you +upgrade, not after**, because it is a start-time failure rather than a +degradation. -Two things to do rather than assume: +### Check two configuration relations before upgrading -1. If you monitor `parent_switched`, move to `parent_switches` before - upgrading, or your dashboards will go blank rather than error. -2. During the rolling window, expect upgraded and not-yet-upgraded nodes - to potentially disagree about mesh size, per the FPR cap section above. - This is expected and self-resolves. Do not chase it as a bug unless it - persists after every node reports `0.4.1`. +Two configuration shapes that loaded in v0.4.1 are now rejected at +config validation. A node carrying either will not start after the +package upgrade. Both were settings that looked like they disabled +something and in fact made it fire continuously, so a rejection is the +correct behaviour, but it arrives at the least convenient moment if you +meet it for the first time on a restart. -If you have pinned `node.bloom.max_inbound_fpr` explicitly in your config, -your setting is honored and nothing changes for you. The change only -affects nodes taking the default. +Check your config file before you upgrade: -Downgrading to v0.4.0 is supported and needs no special handling. +```bash +grep -nE 'after_messages|after_secs|signal_ttl_secs|replay_window_secs' \ + /etc/fips/fips.yaml +``` -## Getting v0.4.1 +On macOS the file is at `/usr/local/etc/fips/fips.yaml`. + +**1. `node.rekey.after_messages` must be at least 1.** Zero makes the +message-count arm true on every poll, because the trigger compares with +greater-or-equal, so a node rekeyed on sight rather than never. The +default is 65536. If you set it to 0 intending to disable the arm, use a +very large value instead; there is no upper bound. + +**2. `node.rekey.after_secs` must be greater than 15**, the per-session +rekey jitter. Each session offsets the interval by a random value within +plus or minus that bound, so a smaller interval saturates to zero on a +negative draw and rekeys on sight for roughly half of sessions. The +default is 120. Both rekey checks run whether or not `node.rekey.enabled` +is true, so turning rekey on later cannot surface the error at a +surprising moment. + +**3. `node.discovery.nostr.signal_ttl_secs` plus 120 must be less than +`node.discovery.nostr.replay_window_secs`.** A traversal signal is +acceptable over its TTL plus 60s of clock-skew grace on each side, and +that span has to stay strictly inside the replay window, or a session id +evicted from the replay cache on expiry is still fresh enough to be +accepted a second time. The relation was documented but unenforced, so +raising the TTL past 180s silently voided it. The shipped defaults, a +TTL of 120 against a window of 300, are unaffected. The error names the +concrete floor for `replay_window_secs`, so if you hit it on a test +start the fix is in the message. + +The safest sequence is to run that grep on every node's config first, +correct anything that trips one of the three rules, and only then +upgrade. + +### If you use `fips` as a library + +**Binaries are unaffected. Skip this section unless you build against +the `fips` crate.** + +Four public types gained a `Drop` implementation as part of clearing key +material at end of scope: `Identity`, `ResolvedIdentity`, +`IdentityConfig` and `HandshakeState`. A type that implements `Drop` +cannot have its fields moved out, so this is source-breaking for a +consumer of the library crate even though nothing about the shipped +binaries changes. + +`IdentityConfig` is the one most likely to be reached in practice, +because it hangs off the public `Config` as `node.identity`. Code that +moved the nsec out of a configuration value no longer compiles. The fix +is `Option::take` on the field rather than moving the value out. + +This is a source break in a patch release, which semantic versioning +does not sanction. It ships anyway because the alternative was holding a +security fix for the next minor, and because the crate is not published +to a registry, so the reachable population is small. + +### During and after a rolling upgrade + +- `TtlExhausted` charges at a different node than it did, so its + distribution shifts by one hop while the mesh is mixed. This settles + once every node reports `0.4.2`. +- If you scrape the control socket, expect `show_routing` and + `show_status` to carry new counters. An older `fipsctl` or `fipstop` + gains unknown fields rather than losing known ones. +- On macOS, move `peers.allow`, `peers.deny`, `hosts`, `fips.yaml` and + `fips.key` from `/etc/fips/` to `/usr/local/etc/fips/`. The daemon + warns once at startup if it finds any of them only at the old + location, and it will adopt a key stranded there rather than + generating a new identity, but the warning is the signal to move them + rather than to leave them. +- A mesh whose peers advertise a legitimately narrow path MTU should be + watched once after the upgrade. Locally derived values are exempt from + the new floor at both the seed and the clamp, so a narrow link is + expected to adapt as before, but that exemption is asserted in the + code rather than proven by a test that drives a genuinely narrow path. + +Downgrading to v0.4.1 is supported. A config corrected for the three +rules above still loads on v0.4.1, so the correction does not have to be +reverted. + +## Getting v0.4.2 - **Linux x86_64 / aarch64**: `.deb` and tarball at the - [v0.4.1 release page](https://github.com/jmcorgan/fips/releases/tag/v0.4.1). + [v0.4.2 release page](https://github.com/jmcorgan/fips/releases/tag/v0.4.2). - **Arch Linux**: `fips` from the AUR. -- **macOS**: `.pkg` at the v0.4.1 release page. -- **Windows**: ZIP at the v0.4.1 release page. +- **macOS**: `.pkg` at the v0.4.2 release page. +- **Windows**: ZIP at the v0.4.2 release page. - **OpenWrt**: `.ipk` (OpenWrt 24.x and earlier) or `.apk` (OpenWrt 25+) - at the v0.4.1 release page. -- **From source**: `cargo build --release` from a checkout of the v0.4.1 + at the v0.4.2 release page. +- **From source**: `cargo build --release` from a checkout of the v0.4.2 tag (Rust 1.94.1 per `rust-toolchain.toml`; `libclang-dev` is a required Linux build prerequisite). -- **Nix / NixOS**: `nix build .#fips` from a checkout of the v0.4.1 tag - builds the binaries from source with the pinned toolchain and no manual - prerequisites (see the Nix section of `packaging/README.md`). +- **Nix / NixOS**: `nix build .#fips` from a checkout of the v0.4.2 tag + builds the binaries from source with the pinned toolchain and no + manual prerequisites (see the Nix section of `packaging/README.md`). The full per-commit changelog lives in [`CHANGELOG.md`](../../CHANGELOG.md). Issues and discussion at [github.com/jmcorgan/fips](https://github.com/jmcorgan/fips). +Security reports have a private channel as of this release; see +[`SECURITY.md`](../../SECURITY.md). + ## Contributors Thanks to everyone who contributed code, packaging work, bug reports, or reviews to this release. -- [@jcorgan](https://github.com/jmcorgan): release shepherd, spanning-tree - and discovery fixes, bloom and identity performance work, antipoison cap - change, and testing. +- [@jmcorgan](https://github.com/jmcorgan) (Johnathan Corgan): release + shepherd; the session and handshake authentication work, path MTU + bounding, key material protection and clearing, gateway DNS answer + validation, Action pinning and the dependency refresh, the traversal + clock fix, spanning-tree and rate-limiting work, and the test harness. +- [@sh1ftred](https://github.com/sh1ftred): the macOS install layout + fix, so config, ACL and identity paths follow the platform packaging + ([#132](https://github.com/jmcorgan/fips/pull/132)). First + contribution to FIPS. + +Bug reports and reviews that shaped this release: + +- [@Theleifless](https://github.com/Theleifless): reported + [#128](https://github.com/jmcorgan/fips/issues/128), NAT traversal + breaking after the host sleeps. +- [@ngmisl](https://github.com/ngmisl): filed + [#137](https://github.com/jmcorgan/fips/issues/137), the security + review most of this release's security work answers. diff --git a/docs/releases/release-notes-v0.4.2.md b/docs/releases/release-notes-v0.4.2.md new file mode 100644 index 00000000..8855a003 --- /dev/null +++ b/docs/releases/release-notes-v0.4.2.md @@ -0,0 +1,676 @@ +# FIPS v0.4.2 + +**Released**: 2026-08-25 + +v0.4.2 is a maintenance release on the v0.4.x line, and the largest one +this line has carried: 144 commits since v0.4.1. Most of it is security +work. It closes several paths by which a party that could merely reach a +node could take its sessions down, have its traffic attributed to +another node, or steer that node's path MTU; it bounds a set of tables +an unauthenticated party could grow without limit; it closes the +fail-open cases where a local error widened what a node accepted; it +fixes NAT traversal in two places where it was simply not working; and +it protects private key material on disk and clears it in memory. There +is no wire format change. + +The new configuration surface is small and optional: three admission and +rate-limiting keys, each with a default that needs no action. The +repository also gains a `SECURITY.md`, so someone with a finding no +longer has to guess at an address or open a public issue. + +v0.4.2 is wire-compatible with v0.4.1. No frame gains, loses, or resizes +a field, so a mixed mesh works and nodes can be upgraded one at a time +with no coordinated restart. Three changes narrow what a node accepts, +or change how it acts on a field it already read: the session datagram +hop limit, the path MTU floor, and routing-signal admission. Those three +are what the interop gate is pointed at deliberately, rather than at +connectivity alone. Compatibility is the release's intent and what that +gate checks; it is not a claim that every mixed pairing was exercised. + +**Read the upgrade notes before you start.** Two configuration shapes +that loaded in v0.4.1 now refuse to start. + +## At a glance + +### Before you upgrade + +- Two configuration shapes now fail to load: a `node.rekey` interval + that fires the trigger continuously, and a traversal signal TTL too + large for the replay window. Both are start-time failures, so a node + carrying either will not come back after a package upgrade. +- macOS installs now read the ACL, hosts, config and identity files from + `/usr/local/etc/fips/`, which is where the packaging puts them. + +### Security + +- Six paths by which an unauthenticated or misattributed packet changed + a node's session state are closed. One of them terminated the daemon. +- A remote party can no longer drive a destination's path MTU to zero, + or aim a node's UDP punch packets at addresses of its choosing. +- Nineteen fixes harden a node against a party that can reach it but is + not an admitted peer: unbounded tables, unauthenticated writes and + fail-open paths. None changes the wire format and none needs action. +- Private keys are no longer written through a symlink, an existing + `fips.key` has its mode retightened on every write, and a failed key + write no longer leaves a node silently running an ephemeral identity. + +### Connectivity and performance + +- NAT traversal works in two deployments where it did not: a public node + in open mode, and any host that suspends. +- Roughly 35 ms per tick comes back at 240 peers, and multi-second + rx-loop stalls during peer retry are gone. + +### New configuration, all optional + +- `node.rate_limit.session_setup_burst` / `_rate`, + `node.rate_limit.established_handshake_burst` / `_rate`, + `node.discovery.nostr.max_concurrent_offers_per_npub`, and + `node.limits.max_sessions` (default 1024). Each default needs no + action. + +### Dependencies + +- `cargo audit` reports no vulnerability, against twelve before, and + every GitHub Action is pinned to a commit SHA. + +## A note on the security content + +Most of this release is security work, and most of that work began with +reviews the project did not commission. Over the past month a number of +unsolicited security reviews have arrived, and they share a character: +they are driven by current frontier language models, their authors say +so, and they arrive as specific, carefully written reports citing the +code they describe rather than as vague claims. + +The findings have been legitimate. Not every one survived a second +reading, and several described documented behaviour as a defect. But +enough held up under adversarial re-reading that treating this class of +report as noise would have been a mistake, and a substantial part of +what this release fixes was found that way, including issues in code +that had been reviewed before. + +None of it has been reported active in a deployment. What these reviews +have produced are reachable defects rather than observed incidents, and +finding them at that stage is the outcome everyone would choose. + +This looks like a broader shift rather than something particular to this +project. The cost of a competent first pass over an unfamiliar codebase +has fallen sharply, and open source is benefiting from it: small +projects are now getting the kind of attention that was previously +reserved for large ones. We welcome it, and we would rather receive a +report of this kind than not. `SECURITY.md` describes how to send one. +The most useful reports are the ones that say plainly which parts were +machine-generated and which were verified by a person, because that is +the difference between a lead and a finding, and we assess the two +differently. + +## Behavior changes worth flagging + +### The session datagram hop limit now follows IP semantics + +Delivery to the addressed node is no longer gated on the hop limit, and +a forwarder decrements before deciding rather than after. Two cases +change on a deployed line: + +| Case | v0.4.1 | v0.4.2 | +| ---- | ------ | ------ | +| Addressed to this node, hop limit 0 | dropped | delivered | +| Transit datagram, hop limit 1 | forwarded at 0 | dropped here | + +The reachable radius is unchanged, because the two behaviors compensate +exactly: a path of `h` links still delivers for any source hop limit of +`h` or more. During a rolling upgrade no version mix delivers less far, +and an unupgraded forwarder feeding an upgraded destination delivers one +hop further than either version does on its own. + +What an operator will see move is the counter. `TtlExhausted` now +charges at the node that makes the decision rather than at the hop after +it, so its distribution across a mixed mesh shifts by one hop while the +upgrade is in progress. That is expected and is not a loss of traffic. + +### `node.rekey.enabled` governs periodic rekey only + +This is a correction to what the setting has always meant rather than a +new field. `enabled` controls whether this node *initiates* periodic +rekey. A rekey a peer drives is still answered when it is off, and two +things that used to sit behind the same gate no longer do: the session +drain sweep and the cut-over that retires an old key epoch now run +either way. A node with rekey disabled previously held superseded keys +for the life of the session. + +### New admission defaults you may feel + +None of these needs configuration, and none changes an existing key's +value. They are new bounds where there was none. + +- `node.discovery.nostr.max_concurrent_offers_per_npub` defaults to 4. + It sits inside `max_concurrent_incoming_offers` (16), which remains + the outer bound, so a value above that is inert. +- `node.rate_limit.session_setup_burst` (64) and `session_setup_rate` + (16.0) meter inbound FSP session setup per authenticated link peer. A + legitimate peer arriving over the same link as a flooding one shares + that link's bucket, so establishment behind a flooded neighbour is + refused until it refills. +- `node.rate_limit.established_handshake_burst` and `_rate` are optional + and normally omitted; the bucket is then derived from + `node.limits.max_peers`, `node.rekey.after_secs` and + `node.rate_limit.handshake_max_resends`, so raising the peer limit + sizes it automatically. +- An accepted inbound TCP or onion connection now has a deadline for its + first frame. This is a module constant, not a configuration key. +- A remote-supplied path MTU below an actionable minimum is ignored + rather than applied or stored. Locally derived MTUs are exempt at both + the seed and the TCP MSS clamp, so a genuinely narrow link, which the + mesh does use, still adapts. + +### Control socket snapshots carry new fields + +`show_routing` and `show_status` gained counters this cycle: +`warm_malformed_packets` and `warm_malformed_bytes`, and the four +error-signal counters `unbound_coords`, `unbound_broken`, `unbound_mtu` +and `unbound_forged`. An older `fipsctl` or `fipstop` reading a newer +daemon gains unknown fields rather than losing known ones. If you scrape +those snapshots, expect additions, not removals. Two other new counters, +the framing `payload_len_mismatch` and the setup-message refusal +counters, are not yet readable over the control socket. + +### The bloom announce sweep changes where its cost sits + +Peer bloom filters are now computed for every recipient in one union +sweep instead of being rebuilt per recipient. The result is exactly +equal, not approximately: merging is a bytewise OR. The trade-off, +measured rather than assumed, is that the sweep does its full work +regardless of how many peers are ready, so a tick announcing to one or +two peers costs about twice what it did. Break-even is around three +ready peers, so a small mesh pays slightly more and a large one pays a +great deal less. Cadence, the debounce, the sequence rule and the +fill-ratio cap are unchanged. + +## Notable bug fixes + +Every item here is a fix for a defect that shipped in v0.4.1. Fixes for +defects introduced and resolved inside this cycle are in the CHANGELOG +and are not repeated here. The CHANGELOG is the complete record; this +section is a selection. + +### Session and handshake authentication + +This is the release's centre of gravity. Six paths are closed, each of +which let a packet that authenticated nothing change a node's session +state. + +- **A truncated inner payload terminated the daemon.** A + `SessionDatagram` whose inner FSP payload was 4 to 11 bytes, with + phase 0x0 and the Coords Present flag set, indexed past the end of a + slice on the coordinate-cache warm path. The receive loop is the + process's main future, so the panic took the daemon down rather than a + task, and under the packaged systemd unit the node restarted into the + same frame. Any peer past a link handshake could send it, and + admission is default-open. +- **An unauthenticated setup message could hold a session down.** With + `node.rekey.enabled` false, a setup message naming an established peer + replaced that peer's session outright, discarding the live keys. The + message carries no authenticator and its source address is an envelope + field the sender picks. The established case now arms a handshake + beside the running session and adopts new keys only after a msg3 whose + authenticated static key matches the one the session was opened with. +- **A forged `SessionAck` cancelled an in-flight initiation**, and an + unauthenticated msg3 discarded a completed key epoch. Both took 57 + bytes of the right length from anyone who could reach the node, and + both were repeatable. The setup path is now also rate limited, keyed + on the authenticated link peer the datagram arrived over rather than + on the address the sender claims. +- **A peer could complete a genuine handshake under another node's + address.** The responder recorded a session under the source address + in the datagram without checking it against the static key it had just + authenticated, so the identity cache, the session map and the + reconstructed mesh IPv6 all attributed that traffic to the node it + named. The address is now derived from the authenticated key, on both + the initial and the rekey path. +- **Routing signals were acted on for any address.** `CoordsRequired`, + `PathBroken` and `MtuExceeded` carry no end-to-end authentication, and + a node applied their effects for any destination they named. They are + now refused unless this node has itself bound that destination, and + each refusal is counted. Signals from a genuine on-path forwarder at + any distance are unaffected. +- **The established-address waiver admitted the wrong party.** A + transport with `accept_connections` false still admits an inbound msg1 + sourced from an established peer's address, so a peer re-handshaking + after a restart is not locked out. Nothing checked that the sender was + that peer, so any off-path party sourcing from the address obtained a + full link handshake from a node configured to accept none. The + handshake is now dropped once the key exchange reveals a static key + that does not belong to the identity owning that address. + +A frame whose declared payload length disagrees with the length that +arrived is also now dropped at the single dispatch point, before that +field can be used as a parsing input. This closes no known defect: on +the stream transports the comparison holds by construction, and on the +datagram transports a short frame already failed the AEAD tag. What +changes is which reason it is dropped for. + +### NAT traversal and Nostr discovery + +- **Traversal was non-functional on a public node in open mode.** A + signal is addressed to the merge of the peer's inbox relays, the + relays its advert nominates, and our own, but the send was rejected + outright if any single URL in that merge was outside the client pool + built at startup. One unconfigured relay killed the whole attempt, + including the sends to relays both sides shared. Measured in an + open-mode window: 309 attempts, 290 explicit failures, zero successes, + every failure on `relay not found`. Configured peers were unaffected, + since they run a matching relay set. Comparison is now on the + normalized relay URL, so a trailing slash or a different host case + does not discard a relay that is in fact configured. +- **Traversal broke permanently after the host suspended.** The + traversal clock cached a Unix timestamp at startup and advanced it + with a monotonic instant, which does not tick while a machine is + asleep, so the daemon's idea of the time trailed real time by the + suspend duration for the rest of the process lifetime. Every + expiration it published was already in the past, relays dropped the + offers, and traversal stayed dead until restart. The clock now reads + the wall clock on every call. A laptop is where this is easiest to + hit, but any host that suspends or hibernates was affected. Reported + in [#128](https://github.com/jmcorgan/fips/issues/128). +- **A node could be aimed at third parties.** A rendezvous-enabled node + punched every address a signed offer named, with no limit on how many + one offer could carry, so any npub could have it emit a burst of UDP + packets carrying its own source address at loopback, link-local, + multicast, broadcast, unspecified or CGNAT addresses. Never-routable + ranges are now rejected, IPv4-mapped forms are canonicalized first so + they cannot slip past, port 0 is dropped, private-range candidates are + punched only when they share a /24 with one of our own addresses, and + the planned list is capped at eight. Each planning attempt logs what + it declined and why. +- **A future-dated traversal signal was accepted as strictly fresh.** + The freshness check measured age with a saturating subtraction, which + yields zero for any timestamp ahead of the local clock, and nothing + else bounded the issue time from above. Forward-dating is now + tolerated only to the same 60s of clock skew already allowed in the + other direction, and a declared expiry is no longer trusted past the + issue time plus the configured TTL. +- **One sender could hold every inbound offer slot.** Admission took a + permit from a single pool before any identity check, with the sender's + npub used only as a log field. Admission now takes a per-npub permit + and a global permit together. This does not make the pool + inexhaustible: Nostr identities cost nothing to generate, so four + throwaway npubs still saturate the shipped 16-slot pool at an + unchanged total offer rate. What it buys is that one identity can no + longer do it alone, and that the two refusals are distinguishable in + the log. + +### Path MTU + +A single `MtuExceeded` carrying a very small value drove a session's +path MTU to zero, after which every packet to that destination was +answered with an ICMPv6 Packet Too Big instead of being sent: a +blackhole lasting until the daemon restarted. The same value reached the +SYN-time TCP MSS clamp, where anything at or below 137 saturates the +segment size to zero. The `path_mtu` field is an unsigned per-hop +annotation carried outside the signed proof, and `MtuExceeded` and +`PathBroken` arrive unencrypted with no sender check, so any forwarder, +or anyone able to reach the node, could lower it. + +Remote values below an actionable minimum are now ignored at the three +places a remote value is acted on, each with its own warning and +counter, and the per-destination cache has a way back: an entry is +released on a `PathBroken` report, on session idle expiry, and on +handshake timeout, with the local link MTU reseeded in its place. +Entries written by the discovery lookup carrier age out on a deadline of +their own, because a destination this node never opens a session with +reaches none of those three routes. Without it, one response carrying a +floor value pinned that destination's clamp until the daemon restarted. + +### Inbound connection slots and rekey admission + +- **An unauthenticated remote could lock out inbound peering by staying + silent.** The peer cap was tested at accept, with no read in between, + and the frame reader's reads carried no deadline. Pool keys are + `ip:port`, so N sockets from one address took N slots, and at the 256 + default that closed the node to new peers for as long as the sockets + stayed open. The first frame now has a deadline, the onion listener + gets the same treatment, and the handshake reaper now closes the + transport connection it used to forget. This does not close the whole + case: a peer that sends one well-formed frame and then goes silent + still holds its slot. +- **Rekey traffic was being refused on busy nodes, silently.** Rekey and + restart msg1 on an established link competed with stranger admission + for one shared token bucket. Measured on a field node at roughly 245 + peers: 8753 msg1 refused in 25 minutes, with 159 of the 201 distinct + sources being peers it already held sessions with. Nothing errored and + no session dropped, so the only symptom was a flat `rekey_armed`. + Inbound msg1 is now classified before it is limited and draws on its + own bucket. Nodes upgrade with no config change, and the + `Msg1 rate limited` line now says which limb refused. + +### Identity and key files on disk + +- **A private key write followed a symlink**, because the single write + path opened with create and truncate and no `O_NOFOLLOW`. Both writers + now share an open helper that carries it. +- **An existing `fips.key` kept a loose mode forever.** The mode was + supplied only through `open(2)`, which the kernel honours on creation + and ignores otherwise, so a key file at 0644 stayed 0644 through every + rewrite. That needs no attacker: one `chmod`, or a restore that did + not preserve modes, leaves the key readable indefinitely. The mode is + now applied to the open descriptor before any secret bytes are + written. On Windows neither protection applies and the file inherits + the parent directory's ACLs; that exclusion is deliberate. +- **A failed key write left a node running an ephemeral identity in + silence.** Six write results in the identity path were discarded, and + the sharpest was in `persistent` mode: a failed write to `fips.key` + fell through to an ephemeral identity with no message, so a node asked + for a stable identity changed its npub, its routing address and its + mesh IPv6 on every start, and nothing said so. All six now report. +- **Key material is now cleared when it goes out of scope.** Nothing in + the crate erased a key before this. Clearing now covers the session + and handshake keys, the identity keypair, the temporary copies the + elliptic-curve operations make, encoded secrets, and the private key + on its way through configuration, including the config file's text, + since `node.identity.nsec` is read straight out of it. This clears the + copies the crate owns, not every copy that ever existed: the secp256k1 + key types are copyable, and the hash, key-derivation and cached cipher + states of the pinned libraries offer no clearing route. Reading the + residue needs access to the process's memory, or to a core dump or + swap image of it. **This carries a source-breaking change for library + consumers; see the upgrade notes.** + +### Gateway DNS answers + +The gateway's DNS forwarder accepted whatever datagram arrived on its +upstream socket. The upstream query reused the client's own transaction +ID, the socket was wildcard-bound and never connected, the receive +discarded the sender, neither the response ID nor the question was +compared against what was asked, and the returned address was not +checked against the mesh prefix. Because the extracted address is +installed as a DNAT rule with no interface constraint, a forged answer +redirected traffic rather than only poisoning a lookup. + +The query now carries a random transaction ID, the socket is connected +so the kernel drops foreign sources, a response must match on ID, +question and type, and the address goes through the validating parser +before any allocation. One deliberate behaviour change: validation sits +before the rcode check, so an upstream answering FORMERR or REFUSED with +an empty question section now yields SERVFAIL rather than having its +rcode relayed. Checking after the rcode would admit a forged NXDOMAIN. + +### macOS install layout + +On macOS the daemon and `fipsctl` read `/etc/fips/`, a directory the +macOS packaging does not create, while the packaging installs to +`/usr/local/etc/fips/`. The effect was silent in the worst way: a +populated `peers.deny` reported `effective_mode: "default_open"` with +`enforcement_active: false` through `fipsctl acl show`, host-file +aliases went unloaded, and `fipsctl keygen` wrote an identity where the +daemon never read it, so the node kept an ephemeral one. The default +paths now follow the platform's packaging, the system config search path +includes `/usr/local/etc/fips/fips.yaml`, and a key stranded at the +legacy path is adopted with a warning rather than a fresh identity being +generated. Linux and Windows behavior is unchanged. + +**macOS users with existing files in `/etc/fips/` should move them to +`/usr/local/etc/fips/`.** + +### Robustness under adverse local conditions + +- **A failed log write could panic the thread or task that logged.** The + subscriber reported its own internal errors through `eprintln!`, which + panics when stderr has also failed, and the shipped supervisor + configurations make that one condition rather than two: the macOS + plist points both standard streams at one file, and the systemd units + route both to journald, so one full disk fails both sinks together. In + the daemon the casualty was a crypto worker, which takes its share of + the peer space with it permanently. In `fips-gateway` it was a spawned + task: the DNS resolver, the control accept loop or the pool tick, none + of which is observed until shutdown, so the process kept running and + reporting healthy with mesh name resolution or lease expiry and NAT + cleanup stopped. +- **Flap dampening could engage only once in a node's lifetime.** The + arming check tested whether a deadline had ever been set rather than + whether one was still in effect, so after the first episode a node in + a second flap storm went on switching parents under hold-down alone, + and neither the `flap_dampened` counter nor the warning fired again. + Hold-down was unaffected throughout, which is why the practical cost + at shipped settings was lost visibility rather than unrestrained + flapping. Separately, a `node.tree.flap_dampening_secs` large enough + to overflow the monotonic clock is now capped at one year instead of + panicking the node when dampening engages. + +### Supply chain + +- The dependency lockfile moves past a set of advisories against the + pinned `nostr` 0.44.3 and `nostr-relay-pool` 0.44.1, both of which + were also yanked. The ones that matter here are the relay-pool + advisories describing forged events bypassing signature validation and + unverified relay events being processed: that is the path a node + learns peer adverts on, and it performs no independent verification of + its own, so the exposure was a misattributed advert rather than the + denial of service the summaries lead with. `cargo audit` now reports + no vulnerability, against twelve before. Four warnings remain that no + version move fixes. +- Every GitHub Action reference is now pinned to a commit SHA. None of + the sixty-six was pinned before, including the jobs holding the AUR + deploy key, the jobs with release write scope, and the packaging jobs + that run with a signing key in the environment. Sixty-two are full + SHAs; four are justified in one place, since two actions read the tool + to install from the ref name itself. The sharper hole was not the + tags: the OpenWrt workflow fetched a helper binary and a toolchain + from release URLs with no verification at all, in two jobs holding a + signing key. Both downloads now check a per-architecture pinned + SHA-256. + +### Limits, provenance checks and fail-closed defaults + +Nineteen fixes harden a node against a party that can reach it but has +not been admitted to it. Every claim behind them was assessed and then +re-read by a separate reviewer briefed to refute it, and only what +survived that pass is here. **None changes the wire format**, and none +needs configuration. + +They fall into four shapes. + +**Tables that an unauthenticated party could grow.** The established +session table had no population cap and now defaults to 1024, tunable +with `node.limits.max_sessions`. The UDP transport's DNS cache grew one +entry per hostname ever dialed and is now bounded at 256 with eviction. +The Ethernet discovery buffer deduplicated beacons with a full scan and +had no cap; it is now a map bounded at 1024 distinct MACs. The lookup +dedup cache was fail-closed at its bound, so a flood stopped every +lookup transiting the node; it now evicts instead of refusing. + +**State an unauthenticated packet could change.** A relay-returned Nostr +advert was cached without checking that the peer it named had signed it. +A lookup response was acted on with no correlation against a lookup this +node had issued. A STUN binding response was accepted from any source. A +NAT punch packet was accepted from any address whose digest matched. +Each now checks the thing that binds it. + +**Denial paths reachable from off the path.** An epoch-mismatch `msg1` +is authentic but replayable, and accepting it tore down a working +peering; it is now refused while that peering is still carrying +authenticated traffic, and dampened against repetition. The +routing-error limiter was keyed on the field the attacker chooses. The +Nostr notify loop ran two decrypts and a signature verify per event +ahead of any limiter. A retired FSP key epoch could be held resident +indefinitely by a peer that kept using it. + +**Local fail-open surfaces.** A read error on `peers.allow` or +`peers.deny` was swallowed and published an empty ACL, which took a +strict allowlist node to admitting everyone; the last good policy is now +held and retried. The control socket and its parent directory were +created under the ambient umask and only tightened afterwards. The DNS +mesh-interface filter was keyed on the configured TUN name and so had +never run on macOS or FreeBSD. + +Some findings from the same review pass are not addressed here. Where a +fix requires a wire-format change it is not a candidate for the 0.4.x +line at all, which takes none; that work belongs to a later release. +`SECURITY.md` sets out the trust model this protocol assumes, and it is +worth reading if you are deciding how far to rely on a mesh whose +membership you do not control. + +Two portability defects are fixed alongside them: a Windows build +failure and a set of Windows and macOS unit-test failures. Both were +caught by CI on those platforms rather than by review, and the coverage +gap that let them through is recorded at the sites. + +### A documented claim that was wrong + +The security reference named both Noise patterns unqualified, which told +anyone auditing the stack against the Noise specification that the +construction was standard. It is not, in one respect: the handshake AEAD +passes an empty associated-data field where standard Noise +`EncryptAndHash` uses the handshake hash. Domain separation and +Diffie-Hellman binding survive through the chaining key; transcript +binding is the property actually absent. Nothing in the daemon reads the +handshake hash, so no shipped behaviour rests on it, but anything later +built on it (channel binding, an exporter, cookie binding) would +silently not work. The reference now says so. + +## Upgrade notes + +There is no wire format change and no coordinated restart. Nodes can be +upgraded one at a time in any order. **One thing must be done before you +upgrade, not after**, because it is a start-time failure rather than a +degradation. + +### Check two configuration relations before upgrading + +Two configuration shapes that loaded in v0.4.1 are now rejected at +config validation. A node carrying either will not start after the +package upgrade. Both were settings that looked like they disabled +something and in fact made it fire continuously, so a rejection is the +correct behaviour, but it arrives at the least convenient moment if you +meet it for the first time on a restart. + +Check your config file before you upgrade: + +```bash +grep -nE 'after_messages|after_secs|signal_ttl_secs|replay_window_secs' \ + /etc/fips/fips.yaml +``` + +On macOS the file is at `/usr/local/etc/fips/fips.yaml`. + +**1. `node.rekey.after_messages` must be at least 1.** Zero makes the +message-count arm true on every poll, because the trigger compares with +greater-or-equal, so a node rekeyed on sight rather than never. The +default is 65536. If you set it to 0 intending to disable the arm, use a +very large value instead; there is no upper bound. + +**2. `node.rekey.after_secs` must be greater than 15**, the per-session +rekey jitter. Each session offsets the interval by a random value within +plus or minus that bound, so a smaller interval saturates to zero on a +negative draw and rekeys on sight for roughly half of sessions. The +default is 120. Both rekey checks run whether or not `node.rekey.enabled` +is true, so turning rekey on later cannot surface the error at a +surprising moment. + +**3. `node.discovery.nostr.signal_ttl_secs` plus 120 must be less than +`node.discovery.nostr.replay_window_secs`.** A traversal signal is +acceptable over its TTL plus 60s of clock-skew grace on each side, and +that span has to stay strictly inside the replay window, or a session id +evicted from the replay cache on expiry is still fresh enough to be +accepted a second time. The relation was documented but unenforced, so +raising the TTL past 180s silently voided it. The shipped defaults, a +TTL of 120 against a window of 300, are unaffected. The error names the +concrete floor for `replay_window_secs`, so if you hit it on a test +start the fix is in the message. + +The safest sequence is to run that grep on every node's config first, +correct anything that trips one of the three rules, and only then +upgrade. + +### If you use `fips` as a library + +**Binaries are unaffected. Skip this section unless you build against +the `fips` crate.** + +Four public types gained a `Drop` implementation as part of clearing key +material at end of scope: `Identity`, `ResolvedIdentity`, +`IdentityConfig` and `HandshakeState`. A type that implements `Drop` +cannot have its fields moved out, so this is source-breaking for a +consumer of the library crate even though nothing about the shipped +binaries changes. + +`IdentityConfig` is the one most likely to be reached in practice, +because it hangs off the public `Config` as `node.identity`. Code that +moved the nsec out of a configuration value no longer compiles. The fix +is `Option::take` on the field rather than moving the value out. + +This is a source break in a patch release, which semantic versioning +does not sanction. It ships anyway because the alternative was holding a +security fix for the next minor, and because the crate is not published +to a registry, so the reachable population is small. + +### During and after a rolling upgrade + +- `TtlExhausted` charges at a different node than it did, so its + distribution shifts by one hop while the mesh is mixed. This settles + once every node reports `0.4.2`. +- If you scrape the control socket, expect `show_routing` and + `show_status` to carry new counters. An older `fipsctl` or `fipstop` + gains unknown fields rather than losing known ones. +- On macOS, move `peers.allow`, `peers.deny`, `hosts`, `fips.yaml` and + `fips.key` from `/etc/fips/` to `/usr/local/etc/fips/`. The daemon + warns once at startup if it finds any of them only at the old + location, and it will adopt a key stranded there rather than + generating a new identity, but the warning is the signal to move them + rather than to leave them. +- A mesh whose peers advertise a legitimately narrow path MTU should be + watched once after the upgrade. Locally derived values are exempt from + the new floor at both the seed and the clamp, so a narrow link is + expected to adapt as before, but that exemption is asserted in the + code rather than proven by a test that drives a genuinely narrow path. + +Downgrading to v0.4.1 is supported. A config corrected for the three +rules above still loads on v0.4.1, so the correction does not have to be +reverted. + +## Getting v0.4.2 + +- **Linux x86_64 / aarch64**: `.deb` and tarball at the + [v0.4.2 release page](https://github.com/jmcorgan/fips/releases/tag/v0.4.2). +- **Arch Linux**: `fips` from the AUR. +- **macOS**: `.pkg` at the v0.4.2 release page. +- **Windows**: ZIP at the v0.4.2 release page. +- **OpenWrt**: `.ipk` (OpenWrt 24.x and earlier) or `.apk` (OpenWrt 25+) + at the v0.4.2 release page. +- **From source**: `cargo build --release` from a checkout of the v0.4.2 + tag (Rust 1.94.1 per `rust-toolchain.toml`; `libclang-dev` is a + required Linux build prerequisite). +- **Nix / NixOS**: `nix build .#fips` from a checkout of the v0.4.2 tag + builds the binaries from source with the pinned toolchain and no + manual prerequisites (see the Nix section of `packaging/README.md`). + +The full per-commit changelog lives in +[`CHANGELOG.md`](../../CHANGELOG.md). Issues and discussion at +[github.com/jmcorgan/fips](https://github.com/jmcorgan/fips). + +Security reports have a private channel as of this release; see +[`SECURITY.md`](../../SECURITY.md). + +## Contributors + +Thanks to everyone who contributed code, packaging work, bug reports, or +reviews to this release. + +- [@jmcorgan](https://github.com/jmcorgan) (Johnathan Corgan): release + shepherd; the session and handshake authentication work, path MTU + bounding, key material protection and clearing, gateway DNS answer + validation, Action pinning and the dependency refresh, the traversal + clock fix, spanning-tree and rate-limiting work, and the test harness. +- [@sh1ftred](https://github.com/sh1ftred): the macOS install layout + fix, so config, ACL and identity paths follow the platform packaging + ([#132](https://github.com/jmcorgan/fips/pull/132)). First + contribution to FIPS. + +Bug reports and reviews that shaped this release: + +- [@Theleifless](https://github.com/Theleifless): reported + [#128](https://github.com/jmcorgan/fips/issues/128), NAT traversal + breaking after the host sleeps. +- [@ngmisl](https://github.com/ngmisl): filed + [#137](https://github.com/jmcorgan/fips/issues/137), the security + review most of this release's security work answers. diff --git a/src/cache/coord_cache.rs b/src/cache/coord_cache.rs index e832915e..d2acd8ae 100644 --- a/src/cache/coord_cache.rs +++ b/src/cache/coord_cache.rs @@ -17,6 +17,32 @@ pub const DEFAULT_COORD_CACHE_SIZE: usize = 50_000; /// Default TTL for coordinate cache entries (5 minutes in milliseconds). pub const DEFAULT_COORD_CACHE_TTL_MS: u64 = 300_000; +/// What a hint write did, which is the only place the precedence rule is +/// observable. +/// +/// `#[must_use]` on purpose. A hint write can be refused, and a caller that +/// drops the outcome cannot tell a stored coordinate from a rejected one. It +/// also makes the compiler, rather than review, the thing that notices when a +/// write site is left on the hint path that should have been verified. +#[must_use] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum HintOutcome { + /// No entry existed; the hint was stored. + Inserted, + /// An entry existed and the hint replaced it with a different value. + /// + /// This is the security-interesting outcome. A destination's coordinates + /// changing is ordinary when it moves in the tree and is also exactly what + /// a poisoning looks like, so the two are not distinguishable here and the + /// counter is a rate to watch rather than an alarm. + Changed, + /// An entry existed and the hint carried the same value. + Unchanged, + /// An entry existed, was verified and still within its verification + /// window, so the hint was refused. + Rejected, +} + /// Coordinate cache for routing decisions. /// /// Maps node addresses to their tree coordinates, enabling data packets @@ -62,12 +88,41 @@ impl CoordCache { self.default_ttl_ms = ttl_ms; } - /// Insert or update a cache entry. - pub fn insert(&mut self, addr: NodeAddr, coords: TreeCoordinate, current_time_ms: u64) { - // Update existing entry if present + /// Insert or update a cache entry from an unauthenticated hint. + /// + /// **This is the only way to write a coordinate learned off the wire, and + /// it is deliberately the obvious name.** A hint never displaces an entry + /// that a verified lookup established and whose verification has not yet + /// aged out; see [`CacheEntry::is_verified`]. Conferring trust requires + /// asking for it by name, with [`CoordCache::insert_verified`]. + pub fn insert( + &mut self, + addr: NodeAddr, + coords: TreeCoordinate, + current_time_ms: u64, + ) -> HintOutcome { + self.insert_hint_with_ttl(addr, coords, current_time_ms, self.default_ttl_ms) + } + + /// Insert or update a cache entry from a hint, with an explicit TTL. + fn insert_hint_with_ttl( + &mut self, + addr: NodeAddr, + coords: TreeCoordinate, + current_time_ms: u64, + ttl_ms: u64, + ) -> HintOutcome { if let Some(entry) = self.entries.get_mut(&addr) { - entry.update(coords, current_time_ms, self.default_ttl_ms); - return; + if entry.is_verified(current_time_ms) { + return HintOutcome::Rejected; + } + let changed = entry.coords() != &coords; + entry.update(coords, current_time_ms, ttl_ms); + return if changed { + HintOutcome::Changed + } else { + HintOutcome::Unchanged + }; } // Evict if at capacity @@ -75,15 +130,47 @@ impl CoordCache { self.evict_one(current_time_ms); } - let entry = CacheEntry::new(coords, current_time_ms, self.default_ttl_ms); + // Eviction can decline to free a slot when every entry is a live + // verified one, which is the case the hint must not be allowed to + // force. Refuse rather than grow past the cap. + if self.entries.len() >= self.max_entries { + return HintOutcome::Rejected; + } + + let entry = CacheEntry::new(coords, current_time_ms, ttl_ms); + self.entries.insert(addr, entry); + HintOutcome::Inserted + } + + /// Insert or update a cache entry from a lookup whose proof was verified. + /// + /// Unconditional: a verified value displaces whatever was there, which is + /// the point — it is how a poisoned entry gets corrected. + pub fn insert_verified( + &mut self, + addr: NodeAddr, + coords: TreeCoordinate, + current_time_ms: u64, + ) { + if let Some(entry) = self.entries.get_mut(&addr) { + entry.update_verified(coords, current_time_ms, self.default_ttl_ms); + return; + } + + if self.entries.len() >= self.max_entries { + self.evict_one(current_time_ms); + } + + let entry = CacheEntry::new_verified(coords, current_time_ms, self.default_ttl_ms); self.entries.insert(addr, entry); } - /// Insert or update a cache entry with path MTU information. + /// Insert or update a verified cache entry with path MTU information. /// /// Used by discovery response handling to store the discovered path MTU - /// alongside the target's coordinates. - pub fn insert_with_path_mtu( + /// alongside the target's coordinates. Verified for the same reason + /// [`CoordCache::insert_verified`] is: the caller checked the proof. + pub fn insert_verified_with_path_mtu( &mut self, addr: NodeAddr, coords: TreeCoordinate, @@ -91,7 +178,7 @@ impl CoordCache { path_mtu: u16, ) { if let Some(entry) = self.entries.get_mut(&addr) { - entry.update(coords, current_time_ms, self.default_ttl_ms); + entry.update_verified(coords, current_time_ms, self.default_ttl_ms); entry.set_path_mtu(path_mtu); return; } @@ -100,7 +187,7 @@ impl CoordCache { self.evict_one(current_time_ms); } - let mut entry = CacheEntry::new(coords, current_time_ms, self.default_ttl_ms); + let mut entry = CacheEntry::new_verified(coords, current_time_ms, self.default_ttl_ms); entry.set_path_mtu(path_mtu); self.entries.insert(addr, entry); } @@ -112,18 +199,8 @@ impl CoordCache { coords: TreeCoordinate, current_time_ms: u64, ttl_ms: u64, - ) { - if let Some(entry) = self.entries.get_mut(&addr) { - entry.update(coords, current_time_ms, ttl_ms); - return; - } - - if self.entries.len() >= self.max_entries { - self.evict_one(current_time_ms); - } - - let entry = CacheEntry::new(coords, current_time_ms, ttl_ms); - self.entries.insert(addr, entry); + ) -> HintOutcome { + self.insert_hint_with_ttl(addr, coords, current_time_ms, ttl_ms) } /// Look up coordinates for an address (without touching). @@ -253,10 +330,19 @@ impl CoordCache { return; } - // Otherwise evict LRU (oldest last_used) + // Otherwise evict the LRU among entries that are not live-verified. + // + // Restricting the victim pool is what stops a hint flood from + // manufacturing the empty slot the precedence rule depends on: without + // it, an attacker fills the cache with hints until a verified entry + // becomes the LRU, evicts it, and then plants into a slot that is now + // empty and so accepts an ordinary first write. Declining to evict is + // the correct outcome when every entry is live-verified; the caller + // refuses the hint rather than growing past the cap. let lru_key = self .entries .iter() + .filter(|(_, e)| !e.is_verified(current_time_ms)) .max_by_key(|(_, e)| e.idle_time(current_time_ms)) .map(|(k, _)| *k); @@ -299,6 +385,7 @@ impl Default for CoordCache { #[cfg(test)] mod tests { use super::*; + use crate::cache::entry::VERIFIED_TTL_MS; fn make_node_addr(val: u8) -> NodeAddr { let mut bytes = [0u8; 16]; @@ -316,7 +403,7 @@ mod tests { let addr = make_node_addr(1); let coords = make_coords(&[1, 0]); - cache.insert(addr, coords.clone(), 0); + let _ = cache.insert(addr, coords.clone(), 0); assert!(cache.contains(&addr, 0)); assert_eq!(cache.get(&addr, 0), Some(&coords)); @@ -329,7 +416,7 @@ mod tests { let addr = make_node_addr(1); let coords = make_coords(&[1, 0]); - cache.insert(addr, coords, 0); + let _ = cache.insert(addr, coords, 0); assert!(cache.contains(&addr, 500)); assert!(!cache.contains(&addr, 1500)); @@ -340,8 +427,8 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let addr = make_node_addr(1); - cache.insert(addr, make_coords(&[1, 0]), 0); - cache.insert(addr, make_coords(&[1, 2, 0]), 500); + let _ = cache.insert(addr, make_coords(&[1, 0]), 0); + let _ = cache.insert(addr, make_coords(&[1, 2, 0]), 500); assert_eq!(cache.len(), 1); let coords = cache.get(&addr, 500).unwrap(); @@ -356,14 +443,14 @@ mod tests { let addr2 = make_node_addr(2); let addr3 = make_node_addr(3); - cache.insert(addr1, make_coords(&[1, 0]), 0); - cache.insert(addr2, make_coords(&[2, 0]), 100); + let _ = cache.insert(addr1, make_coords(&[1, 0]), 0); + let _ = cache.insert(addr2, make_coords(&[2, 0]), 100); // Touch addr2 to make it more recent let _ = cache.get_and_touch(&addr2, 200); // Insert addr3, should evict addr1 (LRU) - cache.insert(addr3, make_coords(&[3, 0]), 300); + let _ = cache.insert(addr3, make_coords(&[3, 0]), 300); assert!(!cache.contains(&addr1, 300)); assert!(cache.contains(&addr2, 300)); @@ -374,11 +461,11 @@ mod tests { fn test_coord_cache_evict_expired_first() { let mut cache = CoordCache::new(2, 100); - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 50); + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 50); // At time 150, addr1 is expired, addr2 is not - cache.insert(make_node_addr(3), make_coords(&[3, 0]), 150); + let _ = cache.insert(make_node_addr(3), make_coords(&[3, 0]), 150); // addr1 should be evicted (expired), not addr2 (LRU but not expired) assert!(!cache.contains(&make_node_addr(1), 150)); @@ -390,9 +477,9 @@ mod tests { fn test_coord_cache_purge_expired() { let mut cache = CoordCache::new(100, 100); - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); // expires at 100 - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 50); // expires at 150 - cache.insert(make_node_addr(3), make_coords(&[3, 0]), 200); // expires at 300 + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); // expires at 100 + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 50); // expires at 150 + let _ = cache.insert(make_node_addr(3), make_coords(&[3, 0]), 200); // expires at 300 assert_eq!(cache.len(), 3); @@ -408,8 +495,8 @@ mod tests { fn test_coord_cache_stats() { let mut cache = CoordCache::new(100, 100); - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 50); + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 50); let stats = cache.stats(150); @@ -424,7 +511,7 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let addr = make_node_addr(1); - cache.insert_with_ttl(addr, make_coords(&[1, 0]), 0, 200); + let _ = cache.insert_with_ttl(addr, make_coords(&[1, 0]), 0, 200); // Should expire at 200, not the default 1000 assert!(cache.contains(&addr, 100)); @@ -436,8 +523,8 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let addr = make_node_addr(1); - cache.insert_with_ttl(addr, make_coords(&[1, 0]), 0, 200); - cache.insert_with_ttl(addr, make_coords(&[1, 2, 0]), 100, 300); + let _ = cache.insert_with_ttl(addr, make_coords(&[1, 0]), 0, 200); + let _ = cache.insert_with_ttl(addr, make_coords(&[1, 2, 0]), 100, 300); assert_eq!(cache.len(), 1); let coords = cache.get(&addr, 100).unwrap(); @@ -452,7 +539,7 @@ mod tests { let mut cache = CoordCache::new(100, 100); let addr = make_node_addr(1); - cache.insert(addr, make_coords(&[1, 0]), 0); + let _ = cache.insert(addr, make_coords(&[1, 0]), 0); assert_eq!(cache.len(), 1); // Entry expired at time 200 @@ -467,7 +554,7 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let addr = make_node_addr(1); - cache.insert(addr, make_coords(&[1, 0]), 500); + let _ = cache.insert(addr, make_coords(&[1, 0]), 500); let entry = cache.get_entry(&addr).unwrap(); assert_eq!(entry.created_at(), 500); @@ -481,7 +568,7 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let addr = make_node_addr(1); - cache.insert(addr, make_coords(&[1, 0]), 0); + let _ = cache.insert(addr, make_coords(&[1, 0]), 0); assert_eq!(cache.len(), 1); let removed = cache.remove(&addr); @@ -498,8 +585,8 @@ mod tests { assert!(cache.is_empty()); - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); assert!(!cache.is_empty()); @@ -525,7 +612,7 @@ mod tests { cache.set_default_ttl_ms(200); assert_eq!(cache.default_ttl_ms(), 200); - cache.insert(addr, make_coords(&[1, 0]), 0); + let _ = cache.insert(addr, make_coords(&[1, 0]), 0); // New TTL applies: expires at 200 assert!(cache.contains(&addr, 100)); assert!(!cache.contains(&addr, 201)); @@ -550,7 +637,7 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let target = make_node_addr(1); - cache.insert(target, make_coords(&[1, 0]), 0); + let _ = cache.insert(target, make_coords(&[1, 0]), 0); assert_eq!(cache.len(), 1); let removed = cache.invalidate_via_node(&target); @@ -564,7 +651,7 @@ mod tests { let mut cache = CoordCache::new(100, 1000); let dest = make_node_addr(5); // Path: 5 -> 3 -> 1 -> 0 (root). Target 3 appears at depth 1. - cache.insert(dest, make_coords(&[5, 3, 1, 0]), 0); + let _ = cache.insert(dest, make_coords(&[5, 3, 1, 0]), 0); let removed = cache.invalidate_via_node(&make_node_addr(3)); assert_eq!(removed, 1); @@ -576,7 +663,7 @@ mod tests { // Entry whose ancestry does NOT contain the target must be retained. let mut cache = CoordCache::new(100, 1000); let dest = make_node_addr(5); - cache.insert(dest, make_coords(&[5, 3, 1, 0]), 0); + let _ = cache.insert(dest, make_coords(&[5, 3, 1, 0]), 0); let removed = cache.invalidate_via_node(&make_node_addr(99)); assert_eq!(removed, 0); @@ -596,8 +683,8 @@ mod tests { fn test_invalidate_other_roots_current_root_kept() { let mut cache = CoordCache::new(100, 1000); // Entries rooted at addr(0) - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); let removed = cache.invalidate_other_roots(&make_node_addr(0)); assert_eq!(removed, 0); @@ -608,10 +695,10 @@ mod tests { fn test_invalidate_other_roots_different_root_dropped() { let mut cache = CoordCache::new(100, 1000); // Three entries rooted at addr(0), one rooted at addr(9) - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); - cache.insert(make_node_addr(3), make_coords(&[3, 0]), 0); - cache.insert(make_node_addr(4), make_coords(&[4, 9]), 0); + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); + let _ = cache.insert(make_node_addr(3), make_coords(&[3, 0]), 0); + let _ = cache.insert(make_node_addr(4), make_coords(&[4, 9]), 0); let removed = cache.invalidate_other_roots(&make_node_addr(0)); assert_eq!(removed, 1); @@ -623,8 +710,8 @@ mod tests { #[test] fn test_invalidate_other_roots_all_match() { let mut cache = CoordCache::new(100, 1000); - cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); - cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); + let _ = cache.insert(make_node_addr(1), make_coords(&[1, 0]), 0); + let _ = cache.insert(make_node_addr(2), make_coords(&[2, 0]), 0); let removed = cache.invalidate_other_roots(&make_node_addr(0)); assert_eq!(removed, 0); @@ -638,4 +725,123 @@ mod tests { assert_eq!(removed, 0); assert_eq!(cache.len(), 0); } + + #[test] + fn a_hint_does_not_displace_a_live_verified_entry() { + let mut cache = CoordCache::new(100, 1000); + let addr = make_node_addr(1); + let good = make_coords(&[1, 0]); + let forged = make_coords(&[1, 2, 0]); + + cache.insert_verified(addr, good.clone(), 0); + assert_eq!(cache.insert(addr, forged, 10), HintOutcome::Rejected); + assert_eq!( + cache.get(&addr, 10), + Some(&good), + "the verified value must survive the hint" + ); + } + + #[test] + fn a_verified_write_displaces_a_hint() { + let mut cache = CoordCache::new(100, 1000); + let addr = make_node_addr(1); + let hint = make_coords(&[1, 2, 0]); + let good = make_coords(&[1, 0]); + + assert_eq!(cache.insert(addr, hint, 0), HintOutcome::Inserted); + cache.insert_verified(addr, good.clone(), 10); + assert_eq!( + cache.get(&addr, 10), + Some(&good), + "a proof must be able to correct a poisoned entry" + ); + } + + #[test] + fn verification_ages_out_so_a_stale_verified_entry_stops_refusing_hints() { + let mut cache = CoordCache::new(100, u64::MAX / 4); + let addr = make_node_addr(1); + cache.insert_verified(addr, make_coords(&[1, 0]), 0); + + // Inside the window: refused. + assert_eq!( + cache.insert(addr, make_coords(&[1, 2, 0]), VERIFIED_TTL_MS), + HintOutcome::Rejected + ); + // One millisecond past it: accepted, so a destination that genuinely + // moved is not locked out forever by a verification nobody renews. + let moved = make_coords(&[1, 3, 0]); + assert_eq!( + cache.insert(addr, moved.clone(), VERIFIED_TTL_MS + 1), + HintOutcome::Changed + ); + assert_eq!(cache.get(&addr, VERIFIED_TTL_MS + 1), Some(&moved)); + } + + #[test] + fn ordinary_traffic_does_not_extend_the_verification_window() { + // The entry TTL has to be long enough that the touches below keep the + // entry alive; the test is about the verification clock, not expiry. + let mut cache = CoordCache::new(100, VERIFIED_TTL_MS); + let addr = make_node_addr(1); + cache.insert_verified(addr, make_coords(&[1, 0]), 0); + + // Touch it repeatedly the way forwarding does, right up to the edge. + for t in [100, 1000, 100_000, VERIFIED_TTL_MS] { + let _ = cache.get_and_touch(&addr, t); + } + + // The entry is alive but its verification has aged out on its own + // clock, which is the whole point of keeping the two clocks separate. + assert_eq!( + cache.insert(addr, make_coords(&[1, 2, 0]), VERIFIED_TTL_MS + 1), + HintOutcome::Changed, + "refresh must not carry the verification forward" + ); + } + + #[test] + fn eviction_prefers_an_unverified_victim_over_a_verified_one() { + let mut cache = CoordCache::new(2, 1_000_000); + let verified = make_node_addr(1); + let hint = make_node_addr(2); + let newcomer = make_node_addr(3); + + // The verified entry is the least recently used, so an unrestricted + // LRU would take it. That is exactly the eviction an attacker would + // drive to manufacture an empty slot. + cache.insert_verified(verified, make_coords(&[1, 0]), 0); + assert_eq!( + cache.insert(hint, make_coords(&[2, 0]), 100), + HintOutcome::Inserted + ); + assert_eq!( + cache.insert(newcomer, make_coords(&[3, 0]), 200), + HintOutcome::Inserted + ); + + assert!( + cache.contains(&verified, 200), + "the verified entry must not be the eviction victim" + ); + assert!( + !cache.contains(&hint, 200), + "the unverified entry should have been evicted instead" + ); + } + + #[test] + fn a_cache_full_of_verified_entries_refuses_a_hint_rather_than_evicting_one() { + let mut cache = CoordCache::new(2, 1_000_000); + cache.insert_verified(make_node_addr(1), make_coords(&[1, 0]), 0); + cache.insert_verified(make_node_addr(2), make_coords(&[2, 0]), 0); + + assert_eq!( + cache.insert(make_node_addr(3), make_coords(&[3, 0]), 10), + HintOutcome::Rejected + ); + assert!(cache.contains(&make_node_addr(1), 10)); + assert!(cache.contains(&make_node_addr(2), 10)); + } } diff --git a/src/cache/entry.rs b/src/cache/entry.rs index 69c30ff8..8fe95ae0 100644 --- a/src/cache/entry.rs +++ b/src/cache/entry.rs @@ -2,6 +2,30 @@ use crate::proto::stp::TreeCoordinate; +/// How long a verification outranks a hint, in milliseconds. +/// +/// Deliberately independent of the entry's own TTL. An entry carrying live +/// traffic is refreshed on every use and so never expires, and if verification +/// rode that same clock a once-verified entry would outrank every hint forever +/// — including the hints that would carry a destination's genuine move. This +/// clock is never refreshed: verification ages out on its own, and the entry +/// stays usable afterwards, it just stops winning. +pub const VERIFIED_TTL_MS: u64 = 300_000; + +/// Where a cached coordinate came from, which is what decides whether it may +/// be overwritten. +/// +/// The distinction is the whole of the defence: `Verified` values arrive with +/// a proof this node checked, `Hint` values are copied off a passing packet +/// and are attacker-supplied in the general case. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CoordSource { + /// Established by a lookup whose response proof this node verified. + Verified, + /// Copied from a packet in transit. Unauthenticated. + Hint, +} + /// A cached coordinate entry. #[derive(Clone, Debug)] pub struct CacheEntry { @@ -19,10 +43,21 @@ pub struct CacheEntry { /// response is cached. `None` when populated from SessionSetup or /// other sources that don't carry path MTU information. path_mtu: Option, + /// Where the current coordinates came from. + source: CoordSource, + /// Until when a `Verified` source outranks a hint (Unix milliseconds). + /// + /// Zero for a hint. Never extended by `refresh` or `touch`; see + /// [`VERIFIED_TTL_MS`]. + verified_until: u64, } impl CacheEntry { - /// Create a new cache entry. + /// Create a new cache entry, carrying a hint. + /// + /// Hint is the safe default: a caller that means to confer trust has to say + /// so with [`CacheEntry::new_verified`], rather than trust being what you + /// get by reaching for the obvious constructor. pub fn new(coords: TreeCoordinate, current_time_ms: u64, ttl_ms: u64) -> Self { Self { coords, @@ -30,9 +65,44 @@ impl CacheEntry { last_used: current_time_ms, expires_at: current_time_ms.saturating_add(ttl_ms), path_mtu: None, + source: CoordSource::Hint, + verified_until: 0, } } + /// Create a new cache entry from a verified lookup. + pub fn new_verified(coords: TreeCoordinate, current_time_ms: u64, ttl_ms: u64) -> Self { + let mut entry = Self::new(coords, current_time_ms, ttl_ms); + entry.mark_verified(current_time_ms); + entry + } + + /// Where the current coordinates came from. + pub fn source(&self) -> CoordSource { + self.source + } + + /// Whether this entry's verification still outranks a hint at this time. + /// + /// A `Verified` entry whose `verified_until` has passed answers `false`: + /// the coordinates remain usable, they just no longer refuse an update. + pub fn is_verified(&self, current_time_ms: u64) -> bool { + self.source == CoordSource::Verified && current_time_ms <= self.verified_until + } + + /// Mark the current coordinates as verified, starting the verification + /// clock at `current_time_ms`. + pub fn mark_verified(&mut self, current_time_ms: u64) { + self.source = CoordSource::Verified; + self.verified_until = current_time_ms.saturating_add(VERIFIED_TTL_MS); + } + + /// Mark the current coordinates as an unauthenticated hint. + pub fn mark_hint(&mut self) { + self.source = CoordSource::Hint; + self.verified_until = 0; + } + /// Get the cached coordinates. pub fn coords(&self) -> &TreeCoordinate { &self.coords @@ -79,11 +149,23 @@ impl CacheEntry { self.last_used = current_time_ms; } - /// Update the coordinates and refresh timestamps. + /// Update the coordinates and refresh timestamps, as a hint. + /// + /// New coordinates are new provenance: whatever the entry held before, the + /// value now present came from this caller, so an update by the hint path + /// demotes the entry rather than inheriting the old verification. pub fn update(&mut self, coords: TreeCoordinate, current_time_ms: u64, ttl_ms: u64) { self.coords = coords; self.last_used = current_time_ms; self.expires_at = current_time_ms.saturating_add(ttl_ms); + self.mark_hint(); + } + + /// Update the coordinates from a verified lookup and restart the + /// verification clock. + pub fn update_verified(&mut self, coords: TreeCoordinate, current_time_ms: u64, ttl_ms: u64) { + self.update(coords, current_time_ms, ttl_ms); + self.mark_verified(current_time_ms); } /// Time since last use (for LRU eviction). diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 4144126c..bc8763b0 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -8,8 +8,10 @@ mod entry; use thiserror::Error; -pub use coord_cache::{CoordCache, DEFAULT_COORD_CACHE_SIZE, DEFAULT_COORD_CACHE_TTL_MS}; -pub use entry::CacheEntry; +pub use coord_cache::{ + CoordCache, DEFAULT_COORD_CACHE_SIZE, DEFAULT_COORD_CACHE_TTL_MS, HintOutcome, +}; +pub use entry::{CacheEntry, CoordSource, VERIFIED_TTL_MS}; /// Errors related to cache operations. #[derive(Debug, Error)] diff --git a/src/control/snapshots/show_routing.json b/src/control/snapshots/show_routing.json index 0e1dad20..99e8ddd2 100644 --- a/src/control/snapshots/show_routing.json +++ b/src/control/snapshots/show_routing.json @@ -51,6 +51,10 @@ "unbound_mtu": 0 }, "forwarding": { + "coord_hint_changed": 0, + "coord_hint_rejected": 0, + "coord_warm_foreign_root": 0, + "coord_warm_key_mismatch": 0, "decode_error_bytes": 0, "decode_error_packets": 0, "delivered_bytes": 0, diff --git a/src/control/snapshots/show_status.json b/src/control/snapshots/show_status.json index fd2efe5f..8bed4e5a 100644 --- a/src/control/snapshots/show_status.json +++ b/src/control/snapshots/show_status.json @@ -6,6 +6,10 @@ "estimated_mesh_size": null, "exe_path": "", "forwarding": { + "coord_hint_changed": 0, + "coord_hint_rejected": 0, + "coord_warm_foreign_root": 0, + "coord_warm_key_mismatch": 0, "decode_error_bytes": 0, "decode_error_packets": 0, "delivered_bytes": 0, diff --git a/src/node/dataplane/forwarding.rs b/src/node/dataplane/forwarding.rs index 725a8de5..76164e66 100644 --- a/src/node/dataplane/forwarding.rs +++ b/src/node/dataplane/forwarding.rs @@ -17,8 +17,9 @@ use crate::proto::fsp::wire::{ use crate::proto::fsp::{SessionAck, SessionSetup}; use crate::proto::link::{SessionDatagram, SessionDatagramRef}; use crate::proto::routing::{DropReason, LimitVerdict, NextHop, RouteAction, RouteOutcome}; +use crate::proto::stp::TreeCoordinate; use std::time::{Duration, Instant}; -use tracing::{debug, warn}; +use tracing::{debug, trace, warn}; impl Node { /// Handle an incoming SessionDatagram from a peer. @@ -226,6 +227,45 @@ impl Node { /// reconstructed from the header size so the malformed-frame byte counter /// measures the same population as its siblings — which are charged the /// outer slice — instead of the inner FSP payload. + /// Warm one coordinate-cache entry from a plaintext session header, after + /// the two write-side sanity checks. + /// + /// The key and the value both come off the wire unauthenticated, so this + /// is the only place a warm write can be filtered at all. Two checks, and + /// they are deliberately of different strengths: + /// + /// **Foreign root: refused.** A coordinate under a root other than ours + /// can never route. `StpState::find_next_hop` returns `None` outright on a + /// root mismatch, and the bloom fallback compares against a `my_distance` + /// of `usize::MAX`, so no candidate is ever strictly closer. Caching one + /// therefore buys nothing and costs something real: the entry's presence + /// is what `synth_routing_error` reads to choose `PathBroken` over + /// `CoordsRequired`, so a foreign-root plant turns this node into a + /// one-packet reflector aimed at whatever source the datagram claimed. + /// `CoordCache::invalidate_other_roots` already applies this same + /// invariant whenever our own tree position moves; this applies it at + /// write time instead of waiting for the next move. + /// + /// **Key mismatch: counted only.** A coordinate whose first element is not + /// the address it is filed under is wrong, but refusing it here would also + /// refuse a write honest nodes make: a sender whose own cache missed puts + /// its *own* coordinates in `SessionSetup.dest_coords`, by way of + /// `get_dest_coords`. What that costs a transit node on first contact is + /// not established, so this counts and does not refuse. It is **not** a + /// security check either way — an attacker satisfies it by naming the + /// victim as its own child, which is the forgery worth making. + fn warm_coord(&mut self, key: NodeAddr, coords: TreeCoordinate, now_ms: u64) { + if coords.root_id() != self.tree_state.my_coords().root_id() { + self.metrics().forwarding.record_warm_foreign_root(); + trace!(addr = %key, "Warm write names a foreign root; not caching"); + return; + } + if *coords.node_addr() != key { + self.metrics().forwarding.record_warm_key_mismatch(); + } + self.insert_coord_hint(key, coords, now_ms); + } + fn try_warm_coord_cache_ref(&mut self, datagram: &SessionDatagramRef<'_>, outer_len: usize) { let prefix = match FspCommonPrefix::parse(datagram.payload) { Some(p) => p, @@ -242,10 +282,8 @@ impl Node { match prefix.phase { FSP_PHASE_MSG1 => match SessionSetup::decode(inner) { Ok(setup) => { - self.coord_cache_mut() - .insert(datagram.src_addr, setup.src_coords, now_ms); - self.coord_cache_mut() - .insert(datagram.dest_addr, setup.dest_coords, now_ms); + self.warm_coord(datagram.src_addr, setup.src_coords, now_ms); + self.warm_coord(datagram.dest_addr, setup.dest_coords, now_ms); debug!( src = %datagram.src_addr, dest = %datagram.dest_addr, @@ -258,10 +296,8 @@ impl Node { }, FSP_PHASE_MSG2 => match SessionAck::decode(inner) { Ok(ack) => { - self.coord_cache_mut() - .insert(datagram.src_addr, ack.src_coords, now_ms); - self.coord_cache_mut() - .insert(datagram.dest_addr, ack.dest_coords, now_ms); + self.warm_coord(datagram.src_addr, ack.src_coords, now_ms); + self.warm_coord(datagram.dest_addr, ack.dest_coords, now_ms); debug!( src = %datagram.src_addr, dest = %datagram.dest_addr, @@ -297,12 +333,10 @@ impl Node { match parse_encrypted_coords(coord_data) { Ok((src_coords, dest_coords, _bytes_consumed)) => { if let Some(coords) = src_coords { - self.coord_cache_mut() - .insert(datagram.src_addr, coords, now_ms); + self.warm_coord(datagram.src_addr, coords, now_ms); } if let Some(coords) = dest_coords { - self.coord_cache_mut() - .insert(datagram.dest_addr, coords, now_ms); + self.warm_coord(datagram.dest_addr, coords, now_ms); } debug!( src = %datagram.src_addr, diff --git a/src/node/handlers/lookup.rs b/src/node/handlers/lookup.rs index 5d515e00..9feafbd9 100644 --- a/src/node/handlers/lookup.rs +++ b/src/node/handlers/lookup.rs @@ -389,10 +389,10 @@ impl Node { caching coordinates without it" ); self.metrics().errors.lookup_resp_mtu_below_floor.inc(); - self.coord_cache.insert(target, coords, now_ms); + self.coord_cache.insert_verified(target, coords, now_ms); } else { self.coord_cache - .insert_with_path_mtu(target, coords, now_ms, path_mtu); + .insert_verified_with_path_mtu(target, coords, now_ms, path_mtu); } } LookupAction::WritePathMtu { diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 6ce7bb83..ccc3f7c6 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -253,7 +253,7 @@ impl Node { .plan_cache_coords(*src_addr, my_addr, src_coords, dest_coords) { if let FspAction::CacheCoords { addr, coords } = action { - self.coord_cache.insert(addr, coords, now_ms); + self.insert_coord_hint(addr, coords, now_ms); } } ciphertext_offset += bytes_consumed; @@ -1169,7 +1169,7 @@ impl Node { entry.clear_handshake_payload(); entry.touch(now_ms); self.sessions.insert(*src_addr, entry); - self.coord_cache.insert(*src_addr, ack.src_coords, now_ms); + self.insert_coord_hint(*src_addr, ack.src_coords.clone(), now_ms); // Flush any queued outbound packets for this destination self.flush_pending_packets(src_addr).await; diff --git a/src/node/metrics.rs b/src/node/metrics.rs index 1c39cb38..6359c5dd 100644 --- a/src/node/metrics.rs +++ b/src/node/metrics.rs @@ -13,6 +13,7 @@ use std::sync::atomic::{AtomicU64, Ordering}; +use crate::cache::HintOutcome; use crate::node::reject::{BloomReject, DiscoveryReject, ForwardingReject, TreeReject}; use crate::node::stats::{ BloomStatsSnapshot, CongestionStatsSnapshot, ErrorSignalStatsSnapshot, ForwardingStatsSnapshot, @@ -69,6 +70,27 @@ pub struct ForwardingMetrics { pub decode_error_bytes: Counter, pub warm_malformed_packets: Counter, pub warm_malformed_bytes: Counter, + /// Coordinate-cache warm writes refused because the coordinate names a + /// root other than this node's. Such an entry can never route — both + /// selectors reject a foreign root — so a write of one is either tree + /// churn or a plant, and the counter is the only place the difference + /// shows. + pub coord_warm_foreign_root: Counter, + /// Coordinate-cache warm writes whose coordinate does not name the address + /// it is filed under. Counted, not refused. **This is not a security + /// signal**: an attacker forges a passing coordinate by naming the victim + /// as its own child. It counts a defect honest nodes make, where a sender + /// whose own cache missed sends its own coordinates as the destination's. + pub coord_warm_key_mismatch: Counter, + /// Hint writes that replaced an existing entry with a different value. + /// A destination moving in the tree produces this, and so does a + /// poisoning; the two are not distinguishable here, so this is a rate to + /// watch rather than an alarm. + pub coord_hint_changed: Counter, + /// Hint writes refused because the entry they targeted was verified and + /// still inside its verification window, or because the cache was full of + /// live-verified entries and declined to evict one. + pub coord_hint_rejected: Counter, pub ttl_exhausted_packets: Counter, pub ttl_exhausted_bytes: Counter, pub delivered_packets: Counter, @@ -134,6 +156,25 @@ impl ForwardingMetrics { self.warm_malformed_bytes.add(bytes as u64); } + /// Record a warm write refused for naming a foreign root. + pub fn record_warm_foreign_root(&self) { + self.coord_warm_foreign_root.inc(); + } + + /// Record a warm write whose coordinate does not name its own key. + pub fn record_warm_key_mismatch(&self) { + self.coord_warm_key_mismatch.inc(); + } + + /// Record the outcome of a hint write against the coordinate cache. + pub fn record_hint_outcome(&self, outcome: HintOutcome) { + match outcome { + HintOutcome::Changed => self.coord_hint_changed.inc(), + HintOutcome::Rejected => self.coord_hint_rejected.inc(), + HintOutcome::Inserted | HintOutcome::Unchanged => {} + } + } + /// Record a forwarded (transit) packet of `bytes` payload. #[inline] pub fn record_forwarded(&self, bytes: usize) { @@ -202,6 +243,10 @@ impl ForwardingMetrics { decode_error_bytes: self.decode_error_bytes.get(), warm_malformed_packets: self.warm_malformed_packets.get(), warm_malformed_bytes: self.warm_malformed_bytes.get(), + coord_warm_foreign_root: self.coord_warm_foreign_root.get(), + coord_warm_key_mismatch: self.coord_warm_key_mismatch.get(), + coord_hint_changed: self.coord_hint_changed.get(), + coord_hint_rejected: self.coord_hint_rejected.get(), ttl_exhausted_packets: self.ttl_exhausted_packets.get(), ttl_exhausted_bytes: self.ttl_exhausted_bytes.get(), delivered_packets: self.delivered_packets.get(), diff --git a/src/node/mod.rs b/src/node/mod.rs index 95faf14a..124b5a8b 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -3214,6 +3214,23 @@ impl Node { /// cannot make loop-free forwarding decisions. The caller should signal /// `CoordsRequired` back to the source when `None` is returned for a /// non-local destination. + /// Write one unauthenticated coordinate hint, counting the outcome. + /// + /// Every production hint write goes through here, so the precedence rule + /// has exactly one enforcement point and the counters have exactly one + /// increment site. The verified path is deliberately not routed through + /// this: a caller that has checked a proof calls + /// `CoordCache::insert_verified` directly and says so. + pub(crate) fn insert_coord_hint( + &mut self, + addr: NodeAddr, + coords: TreeCoordinate, + now_ms: u64, + ) { + let outcome = self.coord_cache.insert(addr, coords, now_ms); + self.metrics().forwarding.record_hint_outcome(outcome); + } + pub fn find_next_hop(&mut self, dest_node_addr: &NodeAddr) -> Option<&ActivePeer> { // 1. Local delivery if dest_node_addr == self.node_addr() { diff --git a/src/node/stats.rs b/src/node/stats.rs index 187748f9..dda3a6fe 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -309,6 +309,10 @@ pub struct ForwardingStatsSnapshot { pub decode_error_bytes: u64, pub warm_malformed_packets: u64, pub warm_malformed_bytes: u64, + pub coord_warm_foreign_root: u64, + pub coord_warm_key_mismatch: u64, + pub coord_hint_changed: u64, + pub coord_hint_rejected: u64, pub ttl_exhausted_packets: u64, pub ttl_exhausted_bytes: u64, pub delivered_packets: u64, diff --git a/src/node/tests/discovery.rs b/src/node/tests/discovery.rs index 65280156..fb26b1e9 100644 --- a/src/node/tests/discovery.rs +++ b/src/node/tests/discovery.rs @@ -1307,7 +1307,7 @@ async fn test_response_path_mtu_three_node_chain() { #[tokio::test] async fn test_cache_entry_path_mtu_stored() { - // Verify that insert_with_path_mtu stores the path_mtu in the cache entry + // Verify that insert_verified_with_path_mtu stores the path_mtu in the cache entry let mut node = make_node(); let target = make_node_addr(0xBB); @@ -1315,7 +1315,7 @@ async fn test_cache_entry_path_mtu_stored() { let now_ms = 1000u64; node.coord_cache_mut() - .insert_with_path_mtu(target, coords, now_ms, 1280); + .insert_verified_with_path_mtu(target, coords, now_ms, 1280); let entry = node.coord_cache().get_entry(&target).unwrap(); assert_eq!(entry.path_mtu(), Some(1280)); @@ -1330,7 +1330,7 @@ async fn test_cache_entry_no_path_mtu_from_regular_insert() { let coords = TreeCoordinate::from_addrs(vec![target, make_node_addr(0)]).unwrap(); let now_ms = 1000u64; - node.coord_cache_mut().insert(target, coords, now_ms); + let _ = node.coord_cache_mut().insert(target, coords, now_ms); let entry = node.coord_cache().get_entry(&target).unwrap(); assert_eq!(entry.path_mtu(), None); diff --git a/src/node/tests/forwarding.rs b/src/node/tests/forwarding.rs index fe19f636..c092e2d8 100644 --- a/src/node/tests/forwarding.rs +++ b/src/node/tests/forwarding.rs @@ -204,13 +204,142 @@ async fn test_forwarding_direct_peer() { // Coordinate Cache Warming Tests // ============================================================================ +#[tokio::test] +async fn a_forged_warm_cannot_displace_a_coordinate_established_by_a_verified_lookup() { + let mut node = make_node(); + let attacker_link = make_node_addr(0xAA); + let victim_dest = make_node_addr(0x02); + let root_addr = *node.tree_state.my_coords().root_id(); + + // The state a completed lookup leaves behind. + let real_coords = TreeCoordinate::from_addrs(vec![victim_dest, root_addr]).unwrap(); + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + node.coord_cache_mut() + .insert_verified(victim_dest, real_coords.clone(), now_ms); + + // One packet, claiming to be from the destination, carrying a different + // position for it under the same root. This is the whole attack. + let forged = + TreeCoordinate::from_addrs(vec![victim_dest, make_node_addr(0x77), root_addr]).unwrap(); + let src_coords = TreeCoordinate::from_addrs(vec![victim_dest, root_addr]).unwrap(); + let payload = SessionSetup::new(src_coords, forged.clone()).encode(); + let encoded = SessionDatagram::new(victim_dest, victim_dest, payload).encode(); + + let rejected_before = node.metrics().forwarding.coord_hint_rejected.get(); + node.handle_session_datagram(&attacker_link, &encoded[1..], false) + .await; + + assert_eq!( + node.coord_cache().get(&victim_dest, now_ms), + Some(&real_coords), + "a forged warm displaced a verified coordinate" + ); + assert!( + node.metrics().forwarding.coord_hint_rejected.get() > rejected_before, + "the refusal should be counted" + ); +} + +#[tokio::test] +async fn warming_refuses_a_coordinate_rooted_in_a_tree_this_node_is_not_in() { + let mut node = make_node(); + let from = make_node_addr(0xAA); + let src_addr = make_node_addr(0x01); + let dest_addr = make_node_addr(0x02); + // Deliberately NOT this node's root. Such an entry can never route: both + // selectors reject a foreign root, so caching it only occupies a slot and + // flips the error-PDU choice in `synth_routing_error` from CoordsRequired + // to PathBroken, which is the primitive this guard removes. + let foreign_root = make_node_addr(0xF0); + assert_ne!( + &foreign_root, + node.tree_state.my_coords().root_id(), + "fixture must not accidentally share the node's root" + ); + + let src_coords = TreeCoordinate::from_addrs(vec![src_addr, foreign_root]).unwrap(); + let dest_coords = TreeCoordinate::from_addrs(vec![dest_addr, foreign_root]).unwrap(); + let setup_payload = SessionSetup::new(src_coords, dest_coords).encode(); + let encoded = SessionDatagram::new(src_addr, dest_addr, setup_payload).encode(); + + let before = node.metrics().forwarding.coord_warm_foreign_root.get(); + node.handle_session_datagram(&from, &encoded[1..], false) + .await; + + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + assert!( + node.coord_cache().get(&src_addr, now_ms).is_none(), + "a foreign-root src coordinate was cached" + ); + assert!( + node.coord_cache().get(&dest_addr, now_ms).is_none(), + "a foreign-root dest coordinate was cached" + ); + assert_eq!( + node.metrics().forwarding.coord_warm_foreign_root.get(), + before + 2, + "both refusals should be counted" + ); +} + +#[tokio::test] +async fn warming_counts_but_still_caches_a_coordinate_that_does_not_name_its_own_key() { + let mut node = make_node(); + let from = make_node_addr(0xAA); + let src_addr = make_node_addr(0x01); + let dest_addr = make_node_addr(0x02); + let root_addr = *node.tree_state.my_coords().root_id(); + let someone_else = make_node_addr(0x09); + + // dest_coords names 0x09, not the 0x02 it will be filed under. This is the + // shape an honest sender produces when its own cache missed and + // `get_dest_coords` fell back to the sender's own coordinates, so it is + // counted and NOT refused. + let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); + let dest_coords = TreeCoordinate::from_addrs(vec![someone_else, root_addr]).unwrap(); + let setup_payload = SessionSetup::new(src_coords, dest_coords).encode(); + let encoded = SessionDatagram::new(src_addr, dest_addr, setup_payload).encode(); + + let before = node.metrics().forwarding.coord_warm_key_mismatch.get(); + node.handle_session_datagram(&from, &encoded[1..], false) + .await; + + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_millis() as u64; + assert!( + node.coord_cache().get(&dest_addr, now_ms).is_some(), + "the mismatching entry should still be cached; this check counts only" + ); + assert_eq!( + node.metrics().forwarding.coord_warm_key_mismatch.get(), + before + 1, + "the mismatch should be counted exactly once" + ); + assert_eq!( + node.metrics().forwarding.coord_warm_key_mismatch.get() - before, + 1, + "the well-formed src coordinate must not be counted as a mismatch" + ); +} + #[tokio::test] async fn test_coord_cache_warming_session_setup() { let mut node = make_node(); let from = make_node_addr(0xAA); let src_addr = make_node_addr(0x01); let dest_addr = make_node_addr(0x02); - let root_addr = make_node_addr(0xF0); + // The warming path refuses a coordinate under a root other than this + // node's, so a fixture that wants the write to land has to share the + // node's root. A fresh node is its own root. + let root_addr = *node.tree_state.my_coords().root_id(); let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); let dest_coords = TreeCoordinate::from_addrs(vec![dest_addr, root_addr]).unwrap(); @@ -254,7 +383,10 @@ async fn test_coord_cache_warming_session_ack() { let from = make_node_addr(0xAA); let src_addr = make_node_addr(0x01); let dest_addr = make_node_addr(0x02); - let root_addr = make_node_addr(0xF0); + // The warming path refuses a coordinate under a root other than this + // node's, so a fixture that wants the write to land has to share the + // node's root. A fresh node is its own root. + let root_addr = *node.tree_state.my_coords().root_id(); let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); let dest_coords = TreeCoordinate::from_addrs(vec![dest_addr, root_addr]).unwrap(); @@ -298,7 +430,10 @@ async fn test_coord_cache_warming_encrypted_msg_with_coords() { let from = make_node_addr(0xAA); let src_addr = make_node_addr(0x01); let dest_addr = make_node_addr(0x02); - let root_addr = make_node_addr(0xF0); + // The warming path refuses a coordinate under a root other than this + // node's, so a fixture that wants the write to land has to share the + // node's root. A fresh node is its own root. + let root_addr = *node.tree_state.my_coords().root_id(); let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); let dest_coords = TreeCoordinate::from_addrs(vec![dest_addr, root_addr]).unwrap(); @@ -392,7 +527,10 @@ async fn test_coord_cache_warming_ttl_zero_local_delivery() { let from = make_node_addr(0xAA); let my_addr = *node.node_addr(); let src_addr = make_node_addr(0x01); - let root_addr = make_node_addr(0xF0); + // The warming path refuses a coordinate under a root other than this + // node's, so a fixture that wants the write to land has to share the + // node's root. A fresh node is its own root. + let root_addr = *node.tree_state.my_coords().root_id(); let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); let dest_coords = TreeCoordinate::from_addrs(vec![my_addr, root_addr]).unwrap(); @@ -438,7 +576,10 @@ async fn test_coord_cache_warming_ttl_zero_transit_drop() { let from = make_node_addr(0xAA); let src_addr = make_node_addr(0x01); let dest_addr = make_node_addr(0x02); - let root_addr = make_node_addr(0xF0); + // The warming path refuses a coordinate under a root other than this + // node's, so a fixture that wants the write to land has to share the + // node's root. A fresh node is its own root. + let root_addr = *node.tree_state.my_coords().root_id(); let src_coords = TreeCoordinate::from_addrs(vec![src_addr, root_addr]).unwrap(); let dest_coords = TreeCoordinate::from_addrs(vec![dest_addr, root_addr]).unwrap(); @@ -795,7 +936,7 @@ async fn test_forwarding_with_cache_warming_enables_routing() { // Node 0 gets full cache for (addr, coords) in &all_coords { if addr != nodes[0].node.node_addr() { - nodes[0] + let _ = nodes[0] .node .coord_cache_mut() .insert(*addr, coords.clone(), now_ms); @@ -824,7 +965,7 @@ async fn test_forwarding_with_cache_warming_enables_routing() { .unwrap() .1 .clone(); - nodes[i] + let _ = nodes[i] .node .coord_cache_mut() .insert(j_addr, coords, now_ms); diff --git a/src/node/tests/probe.rs b/src/node/tests/probe.rs index f1f3138e..cbe2cf88 100644 --- a/src/node/tests/probe.rs +++ b/src/node/tests/probe.rs @@ -344,7 +344,7 @@ async fn preview_next_hop_reports_why_it_could_name_no_hop() { let alien_root = crate::NodeAddr::from_bytes([0x77; 16]); let coords = crate::proto::stp::TreeCoordinate::from_addrs(vec![stranger, alien_root]) .expect("non-empty coordinate"); - nodes[0] + let _ = nodes[0] .node .coord_cache_mut() .insert(stranger, coords, wall_ms); diff --git a/src/node/tests/routing.rs b/src/node/tests/routing.rs index 007e47e1..a9d51708 100644 --- a/src/node/tests/routing.rs +++ b/src/node/tests/routing.rs @@ -89,7 +89,7 @@ fn test_routing_bloom_filter_hit() { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - node.coord_cache_mut().insert(dest, dest_coords, now_ms); + let _ = node.coord_cache_mut().insert(dest, dest_coords, now_ms); // Add dest to peer1's bloom filter only let peer1 = node.get_peer_mut(&peer1_addr).unwrap(); @@ -140,7 +140,7 @@ fn test_routing_bloom_filter_multiple_hits_tiebreak() { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - node.coord_cache_mut().insert(dest, dest_coords, now_ms); + let _ = node.coord_cache_mut().insert(dest, dest_coords, now_ms); // Add dest to ALL peers' bloom filters for &addr in &peer_addrs { @@ -192,7 +192,7 @@ fn test_routing_tree_fallback() { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - node.coord_cache_mut().insert(dest, dest_coords, now_ms); + let _ = node.coord_cache_mut().insert(dest, dest_coords, now_ms); // No bloom filter hit — should fall back to tree routing. // Our distance to dest: 2 (root → peer → dest) @@ -268,7 +268,7 @@ fn test_routing_bloom_hit_not_closer_falls_through_to_tree() { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - node.coord_cache_mut().insert(dest, dest_coords, now_ms); + let _ = node.coord_cache_mut().insert(dest, dest_coords, now_ms); // dest is in bloom_peer's filter only (the "bloom hit" candidate), // but bloom_peer's tree distance (3) is NOT strictly less than our @@ -342,7 +342,8 @@ fn test_routing_refreshes_coord_cache_ttl() { .map(|d| d.as_millis() as u64) .unwrap_or(0); let short_ttl = 10_000; // 10 seconds - node.coord_cache_mut() + let _ = node + .coord_cache_mut() .insert_with_ttl(dest, dest_coords, now_ms, short_ttl); let original_expiry = node.coord_cache().get_entry(&dest).unwrap().expires_at(); @@ -438,7 +439,7 @@ fn test_routing_discovery_coord_cache() { assert!(node.find_next_hop(&dest).is_none()); // Now populate coord_cache (as discovery would do) - node.coord_cache_mut().insert(dest, dest_coords, now_ms); + let _ = node.coord_cache_mut().insert(dest, dest_coords, now_ms); // find_next_hop should succeed via coord_cache let result = node.find_next_hop(&dest); @@ -484,14 +485,14 @@ async fn test_routing_chain_topology() { let node3_addr = *nodes[3].node.node_addr(); let node3_coords = nodes[3].node.tree_state().my_coords().clone(); - nodes[0] + let _ = nodes[0] .node .coord_cache_mut() .insert(node3_addr, node3_coords, now_ms); let node0_addr = *nodes[0].node.node_addr(); let node0_coords = nodes[0].node.tree_state().my_coords().clone(); - nodes[3] + let _ = nodes[3] .node .coord_cache_mut() .insert(node0_addr, node0_coords, now_ms); @@ -551,7 +552,7 @@ async fn test_routing_bloom_preferred_over_tree() { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - nodes[0] + let _ = nodes[0] .node .coord_cache_mut() .insert(dest, dest_coords, now_ms); @@ -705,7 +706,8 @@ async fn test_routing_reachability_100_nodes() { for node in &mut nodes { for (addr, coords) in &all_coords { if addr != node.node.node_addr() { - node.node + let _ = node + .node .coord_cache_mut() .insert(*addr, coords.clone(), now_ms); } @@ -840,7 +842,8 @@ async fn test_routing_stops_after_peer_removal() { for node in &mut nodes { for (addr, coords) in &all_coords { if addr != node.node.node_addr() { - node.node + let _ = node + .node .coord_cache_mut() .insert(*addr, coords.clone(), now_ms); } @@ -945,7 +948,7 @@ async fn test_routing_bloom_only_transit() { .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_millis() as u64) .unwrap_or(0); - nodes[0] + let _ = nodes[0] .node .coord_cache_mut() .insert(node3_addr, node3_coords, now_ms); @@ -1055,7 +1058,7 @@ async fn test_routing_source_only_coords_100_nodes() { // Inject dest coords ONLY at the source let (dest_addr, dest_coords) = &all_coords[dst]; - nodes[src] + let _ = nodes[src] .node .coord_cache_mut() .insert(*dest_addr, dest_coords.clone(), now_ms); @@ -1098,7 +1101,8 @@ async fn test_routing_source_only_coords_100_nodes() { for node in &mut nodes { for (addr, coords) in &all_coords { if addr != node.node.node_addr() { - node.node + let _ = node + .node .coord_cache_mut() .insert(*addr, coords.clone(), now_ms); } @@ -1145,7 +1149,7 @@ fn test_classify_forward_tree_up() { // Destination somewhere above us; routed via the parent. let dest = make_node_addr(50); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( dest, TreeCoordinate::from_addrs(vec![dest, root]).unwrap(), now_ms(), @@ -1175,7 +1179,7 @@ fn test_classify_forward_tree_down() { // Destination below the child; routed down to it. let dest = make_node_addr(60); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( dest, TreeCoordinate::from_addrs(vec![dest, child, me, root]).unwrap(), now_ms(), @@ -1209,7 +1213,7 @@ fn test_classify_forward_tree_down_cross() { // Destination lives elsewhere (directly under root), NOT under the child; // reachable from the child only via a cross-link. let dest = make_node_addr(60); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( dest, TreeCoordinate::from_addrs(vec![dest, root]).unwrap(), now_ms(), @@ -1241,7 +1245,7 @@ fn test_classify_forward_crosslink_descend() { // Destination is under the cross-link peer. let dest = make_node_addr(70); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( dest, TreeCoordinate::from_addrs(vec![dest, peer, sibling_parent, root]).unwrap(), now_ms(), @@ -1273,7 +1277,7 @@ fn test_classify_forward_crosslink_ascend() { // Destination lives elsewhere (under root directly), NOT under the peer. let dest = make_node_addr(80); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( dest, TreeCoordinate::from_addrs(vec![dest, root]).unwrap(), now_ms(), @@ -1382,14 +1386,14 @@ fn test_parent_loss_reparent_invalidates_coord_cache() { // via-node class: a downstream destination that routes through us. let downstream = make_node_addr(10); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( downstream, TreeCoordinate::from_addrs(vec![downstream, my_addr, root]).unwrap(), now_ms, ); // survivor: same root, does not route through us. let sibling_dest = make_node_addr(11); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( sibling_dest, TreeCoordinate::from_addrs(vec![sibling_dest, alt, root]).unwrap(), now_ms, @@ -1436,14 +1440,14 @@ fn test_parent_loss_selfroot_invalidates_coord_cache() { // via-node class: routes through us. let downstream = make_node_addr(10); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( downstream, TreeCoordinate::from_addrs(vec![downstream, my_addr, old_root]).unwrap(), now_ms, ); // other-roots class: on the old root, does not route through us. let foreign = make_node_addr(11); - node.coord_cache_mut().insert( + let _ = node.coord_cache_mut().insert( foreign, TreeCoordinate::from_addrs(vec![foreign, parent, old_root]).unwrap(), now_ms, @@ -1549,7 +1553,8 @@ fn seam_two_equidistant_peers(node: &mut Node) -> (NodeAddr, NodeAddr, NodeAddr) .update_peer(ParentDeclaration::new(far, dest, 3, 1000), far_coords); let dest_coords = TreeCoordinate::from_addrs(vec![dest, near, my_addr]).unwrap(); - node.coord_cache_mut() + let _ = node + .coord_cache_mut() .insert(dest, dest_coords, seam_now_ms()); (near, far, dest) @@ -1593,7 +1598,8 @@ fn seam_distance_ladder(node: &mut Node) -> (NodeAddr, NodeAddr, NodeAddr, NodeA .update_peer(ParentDeclaration::new(rung1, rung2, 3, 1000), rung1_coords); let dest_coords = TreeCoordinate::from_addrs(vec![dest, rung1, rung2, rung3, my_addr]).unwrap(); - node.coord_cache_mut() + let _ = node + .coord_cache_mut() .insert(dest, dest_coords, seam_now_ms()); (rung1, rung2, rung3, dest) @@ -1901,7 +1907,8 @@ fn test_seam_routing_view_reads_match_live_peer_state() { // not only the present/absent arms. let stale = make_node_addr(201); let stale_coords = TreeCoordinate::from_addrs(vec![stale, near, my_addr]).unwrap(); - node.coord_cache_mut() + let _ = node + .coord_cache_mut() .insert_with_ttl(stale, stale_coords, 1_000_000, 10); let unknown = make_node_addr(202); diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 74055d6e..3aa50d5e 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -3270,7 +3270,7 @@ async fn test_path_broken_naming_a_dest_with_no_session_does_not_flush_cached_co let dest = NodeAddr::from_bytes([0xCC; 16]); let reporter = NodeAddr::from_bytes([0xBB; 16]); let coords = node.tree_state().my_coords().clone(); - node.coord_cache_mut().insert(dest, coords, 1000); + let _ = node.coord_cache_mut().insert(dest, coords, 1000); let encoded = PathBroken::new(dest, reporter).encode(); node.handle_path_broken(&reporter, &encoded[5..]).await; @@ -3314,7 +3314,7 @@ async fn test_path_broken_naming_a_dest_whose_entry_is_an_unauthenticated_respon let dest = NodeAddr::from_bytes([0xCC; 16]); let reporter = NodeAddr::from_bytes([0xBB; 16]); let coords = node.tree_state().my_coords().clone(); - node.coord_cache_mut().insert(dest, coords, 1000); + let _ = node.coord_cache_mut().insert(dest, coords, 1000); // One forged SessionSetup naming `dest` would leave exactly this entry. install_halfopen(&mut node, dest); @@ -3352,7 +3352,7 @@ async fn test_path_broken_for_a_session_we_initiated_still_flushes_cached_coords let dest = *remote.node_addr(); let reporter = NodeAddr::from_bytes([0xBB; 16]); let coords = node.tree_state().my_coords().clone(); - node.coord_cache_mut().insert(dest, coords, 1000); + let _ = node.coord_cache_mut().insert(dest, coords, 1000); let encoded = PathBroken::new(dest, reporter).encode(); node.handle_path_broken(&reporter, &encoded[5..]).await; diff --git a/src/node/tests/spanning_tree.rs b/src/node/tests/spanning_tree.rs index 1ec4d513..89529411 100644 --- a/src/node/tests/spanning_tree.rs +++ b/src/node/tests/spanning_tree.rs @@ -1003,7 +1003,8 @@ pub(super) fn populate_all_coord_caches(nodes: &mut [TestNode]) { for tn in nodes.iter_mut() { for (addr, coords) in &all_coords { if addr != tn.node.node_addr() { - tn.node + let _ = tn + .node .coord_cache_mut() .insert(*addr, coords.clone(), now_ms); } diff --git a/src/transport/udp/io/unix.rs b/src/transport/udp/io/unix.rs index 0b4c70f5..66e8e5e3 100644 --- a/src/transport/udp/io/unix.rs +++ b/src/transport/udp/io/unix.rs @@ -377,7 +377,18 @@ pub(super) fn sockaddr_to_socket_addr( unsafe { &*(storage as *const _ as *const libc::sockaddr_in6) }; let ip = std::net::Ipv6Addr::from(addr.sin6_addr.s6_addr); let port = u16::from_be(addr.sin6_port); - Ok(SocketAddr::from((ip, port))) + // Carry `sin6_scope_id` through. A link-local source (fe80::/10) + // identifies a host only together with its interface scope — the + // same address can be present on several interfaces — so an + // address parsed without it cannot be replied to. Sources outside + // the link-local range carry scope 0, for which this is identical + // to the unscoped form. + Ok(SocketAddr::V6(std::net::SocketAddrV6::new( + ip, + port, + 0, + addr.sin6_scope_id, + ))) } family => Err(std::io::Error::new( std::io::ErrorKind::InvalidData, @@ -385,3 +396,92 @@ pub(super) fn sockaddr_to_socket_addr( )), } } + +#[cfg(test)] +mod tests { + use super::sockaddr_to_socket_addr; + use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; + + /// Build an `AF_INET6` `sockaddr_storage` the way the kernel fills one in + /// on receive: network-order port, raw address bytes, host-order scope. + fn sockaddr_v6(ip: Ipv6Addr, port: u16, scope_id: u32) -> libc::sockaddr_storage { + let mut storage: libc::sockaddr_storage = unsafe { std::mem::zeroed() }; + // SAFETY: `sockaddr_storage` is defined to be large enough for, and + // aligned for, every concrete `sockaddr_*`; we write the `AF_INET6` + // variant and then tag `ss_family` to match. + let addr = unsafe { &mut *(&mut storage as *mut _ as *mut libc::sockaddr_in6) }; + addr.sin6_family = libc::AF_INET6 as libc::sa_family_t; + addr.sin6_port = port.to_be(); + addr.sin6_addr = libc::in6_addr { + s6_addr: ip.octets(), + }; + addr.sin6_scope_id = scope_id; + storage + } + + fn sockaddr_v4(ip: Ipv4Addr, port: u16) -> libc::sockaddr_storage { + let mut storage: libc::sockaddr_storage = unsafe { std::mem::zeroed() }; + // SAFETY: as above, for the `AF_INET` variant. + let addr = unsafe { &mut *(&mut storage as *mut _ as *mut libc::sockaddr_in) }; + addr.sin_family = libc::AF_INET as libc::sa_family_t; + addr.sin_port = port.to_be(); + addr.sin_addr = libc::in_addr { + s_addr: u32::from(ip).to_be(), + }; + storage + } + + /// The regression this function exists to prevent: a link-local source is + /// routable only with its interface scope, so dropping `sin6_scope_id` + /// leaves an address that cannot be replied to. Every address on a Wi-Fi + /// Aware NDP interface is link-local, so losing it there stalls the Noise + /// handshake — msg1 arrives, msg2 has nowhere to go. + #[test] + fn link_local_source_keeps_its_scope_id() { + let ip: Ipv6Addr = "fe80::1".parse().unwrap(); + let storage = sockaddr_v6(ip, 4871, 42); + + match sockaddr_to_socket_addr(&storage).expect("AF_INET6 converts") { + SocketAddr::V6(addr) => { + assert_eq!(*addr.ip(), ip); + assert_eq!(addr.port(), 4871); + assert_eq!(addr.scope_id(), 42, "scope id must survive conversion"); + } + other => panic!("expected V6, got {other:?}"), + } + } + + /// A scoped address is not equal to its unscoped twin, which is precisely + /// why the bug was silent: both parse, both look right in a log line, and + /// only the reply fails. + #[test] + fn scoped_and_unscoped_addresses_are_distinct() { + let ip: Ipv6Addr = "fe80::1".parse().unwrap(); + let scoped = sockaddr_to_socket_addr(&sockaddr_v6(ip, 4871, 42)).unwrap(); + let unscoped = sockaddr_to_socket_addr(&sockaddr_v6(ip, 4871, 0)).unwrap(); + assert_ne!(scoped, unscoped); + } + + /// Sources outside the link-local range carry scope 0, and must convert + /// exactly as they did before. + #[test] + fn global_v6_source_is_unchanged() { + let ip: Ipv6Addr = "2001:db8::1".parse().unwrap(); + let addr = sockaddr_to_socket_addr(&sockaddr_v6(ip, 4871, 0)).unwrap(); + assert_eq!(addr, SocketAddr::from((ip, 4871))); + } + + #[test] + fn v4_source_is_unchanged() { + let ip = Ipv4Addr::new(192, 168, 8, 238); + let addr = sockaddr_to_socket_addr(&sockaddr_v4(ip, 2121)).unwrap(); + assert_eq!(addr, SocketAddr::from((ip, 2121))); + } + + #[test] + fn unsupported_family_is_an_error() { + let mut storage: libc::sockaddr_storage = unsafe { std::mem::zeroed() }; + storage.ss_family = libc::AF_UNIX as libc::sa_family_t; + assert!(sockaddr_to_socket_addr(&storage).is_err()); + } +}