diff --git a/.github/workflows/package-freebsd.yml b/.github/workflows/package-freebsd.yml index 3c12da74..3a3cb6e8 100644 --- a/.github/workflows/package-freebsd.yml +++ b/.github/workflows/package-freebsd.yml @@ -58,6 +58,13 @@ jobs: # major (FreeBSD:15:amd64) — pkg on other majors refuses the package. runs-on: ubuntu-latest needs: determine-versioning + # Successful runs of this job take 8 to 11 minutes. Five consecutive runs + # in August 2026 instead sat in the VM step for 70, 190, 360, 360 and 360 + # minutes and ended cancelled, the last three at GitHub's own six-hour job + # ceiling. Nothing here bounded them. This bound is deliberately loose + # enough that a slow-but-working run still passes, and tight enough that a + # stall fails in half an hour instead of burning a runner for six. + timeout-minutes: 30 steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 diff --git a/CHANGELOG.md b/CHANGELOG.md index fe237b67..80713d32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,19 +114,269 @@ with v0.4.x or earlier peers. ### Added -- `node.rate_limit.established_handshake_burst` and - `node.rate_limit.established_handshake_rate`, the parameters of the new - established-link msg1 token bucket. 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. - - The receive-path `RejectReason` classification (shipped in 0.4.0) is additionally wired into the Noise XX handshake cluster (msg1/msg2/msg3) and the rekey-initiator outbound sites on `next`. +### Changed + +- `node.rekey.enabled` now means "initiate rekeys" and nothing else. The + responder half of the establish decision was also gated on it, and once the + rekey is declared in the msg3 negotiation payload that flag was the only + thing that could divert a msg3 whose marker matched the session we hold — it + diverted it to a msg2 resend while the initiator had already installed its + pending session and would cut over on its own timer regardless. A pair + configured with the flag true on one end and false on the other therefore + parted company at the initiator's cutover and carried no traffic in either + direction until the link-dead timer. Removing that gate exposed a second + reading of the same flag: the rekey poll returned early when rekey was + disabled, and its drain-expiry arm is the only thing that releases a demoted + session and its index, so a node that does not initiate but now accepts a + rekey would have pinned the previous session and its allocator index + forever. The trigger and the polled cutover stay gated on the flag; the + drain no longer is. + +- Rekey timer jitter is enabled on next's XX FMP rekey path + (`REKEY_JITTER_SECS = 15` at `src/node/mod.rs`), matching the + IK-line behavior on maint/master. It had been temporarily set to + `0` on next because variable-interval rekeys exposed three XX + rekey-path defects that left the two endpoints on divergent Noise + sessions; those defects are fixed (see `### Fixed`), so the + per-session signed jitter over `[-15, +15]` seconds is restored. + `node.rekey.after_secs` is the nominal interval rather than a floor; + mean is preserved. + +- On the XX FMP handshake, an over-cap inbound connection is rejected + solely by the late `promote_connection` check, and the resulting + `MaxPeersExceeded` rejection is logged at debug rather than warn so a + saturated node under sustained inbound pressure does not emit WARN + spam for these expected policy rejections. There is no early cap gate: + on XX the peer's identity is not known until the third handshake + message, by which point Msg1, Msg2, and Msg3 have all crossed the + wire, so an early gate would save no wire bytes and would govern + exactly the same net-new-peer set as the late check. The known-peer / + cross-connection bypass — which also covers peers the node is itself + dialing, e.g. configured `auto_connect` peers — is handled by that + late check, since those peers return earlier via the cross-connection + paths and are not subject to the cap. + +- A dial that names a peer now requires that peer to answer. Under Noise XX + the responder's static key arrives during the handshake instead of being + pinned before it, so a cryptographically valid msg2 proves only that + somebody answered, not that the peer we asked for did. The initiator holds + the dial-time identity in its own field and compares it against the + identity msg2 carried, dropping the leg on a mismatch. Two situations that + used to end in a connection no longer do, both of them intended. + + A LAN peer advertised under the wrong npub is refused rather than peered + under its real identity. mDNS adverts are unauthenticated and the npub in + the TXT record is taken as the identity to dial, so anyone on the LAN can + point a node at a real host under someone else's name; that dial now ends + in a rejection instead of promoting whoever answered. It is a repeating + refusal and not a permanent stop: the candidate comes back each time the + advert is resolved again, and each return costs one handshake. + + A peer whose key has rotated stops connecting for as long as configuration + or a discovery record still names its old npub. This is what pinning means + and there is no form of it that keeps the previous behaviour, but it is a + change in a situation with no attacker in it: a rotated peer used to come + back quietly under its new identity, and the stale npub went unnoticed. + + What an operator sees in both cases is a peer that never comes up, and one + warning per attempt reading `msg2 answered by a different static than the + one dialed, dropping the leg`, carrying the link, the identity dialed and + the identity learned. The rejection is charged to the undifferentiated + `BadState` handshake reject counter and has none of its own, so the log + line is the only thing that names the cause. A configured peer is put back + on the retry schedule under the identity that was dialed, never under the + one that answered, so the warning repeats on backoff until the mismatch is + resolved. + + For a legitimate key rotation the fix is to update the peer's npub in + configuration, or to let discovery republish it under the new key; the next + dial then matches and the peer comes up. Dials that name nobody, meaning + shared-media legs and every inbound leg, are unaffected and still promote + whoever answers. + +### Fixed + +- A leaf-profile node no longer self-elects as tree root. A leaf holding the + smallest node address elected itself, but its peers refuse a non-full node as + a parent, so it formed an isolated second root and partitioned the mesh: a + multi-hop session from the leaf to a non-adjacent full node then failed, + because the far node could not route a handshake reply back into the leaf's + separate coordinate tree. A leaf now attaches under its full upstream and + holds that subtree's coordinate for its own routing; it never announces that + coordinate, reaching peers via the ones carried on its session frames, and + the upstream already advertises it in the upstream's bloom filter. + `Node::with_identity` also derives the node profile, leaf-only flag and bloom + state from the config as `Node::new` does, where it previously hardcoded the + full profile and silently dropped a configured leaf or non-routing profile. + +- Every XX handshake reject arm now releases the session index and the link it + holds. The msg2 self-connect drop and the msg3 reject arms disposed of a leg + without returning what that leg had allocated, so each rejected handshake + leaked an index and a link entry. One arm is deliberately not fixed like the + others: its index comes from the receiver field of the incoming header, which + the peer supplies, so freeing it unconditionally would let a hostile peer + release an index belonging to an unrelated live session, trading a memory + leak for a remote session teardown. That arm frees only after a + transport-blind predicate establishes the index is not claimed elsewhere. + The outbound ACL-reject arm also regains the reschedule call its dial-gate + sibling makes, so a configured peer no longer drops off the dial schedule. + +- XX FMP rekey no longer diverges under timer jitter, which unblocked + re-enabling the rekey jitter on next (`REKEY_JITTER_SECS = 15`; see + `### Changed`). With jitter the two directions of a link rekey close + together in time, and three defects specific to the XX three-message + rekey state machine could each leave the endpoints committed to + different Noise sessions — silent session divergence that starved the + receiver into ~50% post-rekey ping loss and a 30-second heartbeat + link-dead teardown (tree parent loss, routing failure) while every + crypto, transport, and link-state gate stayed green. All three are + fixed: + - The K-bit-flip handler promoted whatever pending session existed the + instant the header bit flipped, which under interleaved rekeys could + be a stale pending from an earlier epoch. It now trial-decrypts the + inbound frame against the pending session and promotes only on an + authenticated decrypt, delivering that plaintext through the + canonical path and leaving the pending untouched otherwise — the + same cutover discipline used on FSP. + - The FMP rekey msg3 was sent once, so a lost datagram left the + responder without the new session. The msg3 payload is now retained + and retransmitted over the existing link until a peer frame + authenticates against the pending or post-cutover current session, + abandoning after the configured handshake-resend budget. Per-link + rekeys are also serialized: a new rekey does not start while one + awaits cutover or is still retransmitting msg3. + - The `handle_msg3` cross-connection and rekey-responder paths were + partitioned by a fixed 30-second session-age threshold, but a rekey + resets the session-age clock, so under jitter a rekey-aged msg3 was + frequently under 30 seconds and got swallowed by the + initial-handshake cross-connection branch, which discarded the + peer's rekey session with no pending slot while the peer cut over to + it anyway. The cross-connection branch is now bounded by the same + jitter-aware session-age floor the rekey responder uses, so the two + paths partition with no overlap. At zero jitter the floor equals the + previous 30-second constant, so default-cadence behavior is + unchanged. + +- XX rekey dual-initiation race that broke six pair-directions + post-rekey when both endpoints initiated rekey simultaneously. + The `handle_msg3` tie-breaker only fired when `rekey_in_progress` + was still true, but XX's three-message handshake lets both sides + clear that flag (via `set_pending_session`) before either's msg3 + lands. The drop-on-pending-session guard then silently discarded + the peer's msg3, each side cut over to its own initiator session, + and the link broke asymmetrically. The tie-breaker now also fires + when `pending_new_session().is_some()`, applying the same + smaller-NodeAddr resolution rule. Mirrored to the FSP rekey msg1 + path for symmetry. + +### Security + +- The peer static key is verified on both FMP handshake paths, not only at + rekey. Under Noise XX the static is learned during the handshake rather than + pinned in advance, and neither path that learns one checked it against what + we already knew, so an attacker able to observe and inject on path could + substitute their own identity on a fresh dial and on an established link. On + a fresh dial we recorded who we meant to reach and then overwrote it with + whoever answered without comparing the two, so an attacker who raced the real + peer to msg2 became the peer — promotion, the ACL check and the peer registry + all ran on the answering identity — and the intended node was never reached. + On an established link a rekey msg2 was matched to its peer only by the + session index we had put in the cleartext msg1 header, so anyone who saw that + header could answer with their own static and take the link over at cutover. + Both are now compared before anything is committed, with the dial-time + expectation held in a field that has no setter so the handshake cannot + overwrite it. Anonymous dials still promote whoever answers, which is what + shared-media discovery means. + +## [0.5.0] - unreleased + +### Added + +#### Platforms + +- FreeBSD support for the daemon, `fipsctl`, and `fipstop`, on x86_64 only: + native TUN datapath (TUNSIFHEAD address-family framing, kernel-assigned + `tunN` device name as with `utun` on macOS), clean service teardown, + `/usr/local/etc/fips` config search path, and `/var/run/fips` + control-socket default (both shared with macOS). The `hosts`, + `peers.allow` / `peers.deny` and `fipsctl keygen` defaults follow the + same `/usr/local/etc/fips` layout as macOS; that move and its startup + warning are described by the three macOS path entries under `[0.4.2]` + below, which this platform inherits. + `fips-gateway` remains Linux-only. Native `.pkg` packaging under + `packaging/freebsd/` + (`make freebsd`) with rc.d services, a `fips` control-socket group, + service stop/restart across `pkg upgrade`, and `.fips` DNS integration + for `local_unbound`/`unbound`/`dnsmasq`. mDNS LAN discovery works via + `mdns-sd` 0.20 (`socket-pktinfo` 0.4.1, the first release that builds + on FreeBSD). Daemon logs now disable ANSI color when stdout is not a + terminal (all platforms). No aarch64 FreeBSD artifact is produced and + that combination is not verified here. + +- Android-ready core, offered as an embedding seam rather than as a supported + platform: there is no Android artifact and none is planned. The daemon's + desktop transports and TUN operations are + gated by `target_os` rather than by Cargo features, so a plain `cargo build` + compiles for every target with no flags and Android self-excludes the raw + Ethernet transport as Windows already did. `Node::enable_app_owned_tun()` + gives an embedder that owns the TUN file descriptor (an Android + `VpnService`, for instance) a channel pair for exchanging IPv6 packet bytes + with FIPS instead of FIPS creating a system TUN device, and `start()` then + performs no system-TUN or `CAP_NET_ADMIN` operations. Packets entering this + way bypass `handle_tun_packet`, so the embedder must push only + `fd00::/8`-destined packets and clamp TCP MSS on outbound SYNs. Desktop + builds are unchanged and no Cargo features are introduced. + +- `Node::dns_local_addr()`, the DNS companion to the app-owned TUN seam above. + An embedder whose resolver is pointed into the tunnel has no system socket + aimed at the built-in `.fips` responder, so the accessor reports the address + read back off the bound socket: `dns.port = 0` therefore yields the + kernel-assigned port, and it returns `Some` only while the responder is up. + Read it once, after `start()` returns and before the node is moved into a + background task; it is not a liveness feed (#136). + +#### Native datagram API + +- An **experimental** native datagram API addressed by public key, off by + default and not a stable interface. A client process opens a flow to a + peer's public key on a chosen port and sends and receives datagrams on a + file descriptor the daemon hands it: no IPv6 emulation, no TUN device and no + DNS, a datagram travelling from key to key. **The wire needs no change and + gets none.** Every FSP data packet has carried a port pair inside its AEAD + envelope since v0.2.0 and port 256 is simply the IPv6 shim, so what was + missing was a way for a program to ask for a port of its own and be handed + the traffic. The x-only public key is the address and an npub is that key + written in bech32, so converting between them is a local encoding rather + than a lookup or a name service; the 16-byte node address that travels on + the wire is a truncated hash of the key, does not invert, and appears + nowhere a client can see. A listener is a descriptor: the daemon writes one + message per arrival to it, carrying the new flow's descriptor and the peer's + address, so poll, select and epoll work on a listener and accepting is a + `recvmsg`. There is no accept command and no reject command, and refusing a + flow is closing the descriptor you were handed. The Rust surface mirrors + `std::net`, with `FipsStream::connect`, `FipsListener::bind`, `incoming`, + `accept`, `io::Result` and an errno mapping rather than a bespoke error + type, plus `set_nonblocking`, `AsFd` and four deadline methods under the + names and signatures `std::net` uses for the same jobs. One rule has no + counterpart in Berkeley sockets and a client author must know it: the v1 + wire carries no half-close, so nothing peer-driven ever closes a flow, and a + server written to read until the flow ends waits for a signal that cannot + arrive. The listener uses `SOCK_SEQPACKET` on Linux and FreeBSD and + `SOCK_DGRAM` on macOS, which does not implement `SOCK_SEQPACKET` for + `AF_UNIX`; both keep the message boundaries the API's contract with its + clients rests on. The two kernels signal a closed peer differently and were + measured rather than reasoned about, so the receive path treats a Darwin + `ECONNRESET` as end of file alongside the `POLLHUP` and zero-byte read that + Linux gives. `EAGAIN` is deliberately not in that company: it means the + socket is empty and the peer alive, so it stays an error and the caller + waits again. + +#### OpenWrt mesh + - OpenWrt 802.11s open-mesh backhaul: router-to-router radio links with FIPS providing all encryption, authentication and routing over bare L2 neighbor links. The mesh runs open with `mesh_fwding 0`, since SAE would duplicate the @@ -150,32 +400,14 @@ with v0.4.x or earlier peers. alphabetically ordered network pickers, and the encryption type must be uniform across routers or clients treat the ESS as different saved networks. `fips-ap-setup` is an opt-in UCI helper creating the `fips-ap0` open AP on an - isolated network with a static ULA /64 and RA-only odhcpd addressing — + isolated network with a static ULA /64 and RA-only odhcpd addressing: stateless SLAAC with no DHCP, the minimum that satisfies Android's - provisioning check — behind a locked-down `fips_ap` firewall zone with no + provisioning check, behind a locked-down `fips_ap` firewall zone with no path to `br-lan` or the WAN, reaching only ICMPv6, mDNS and the FIPS transports. There is no internet by design, so phones keep cellular as their default route (#126). -- Android-ready core: the daemon's desktop transports and TUN operations are - gated by `target_os` rather than by Cargo features, so a plain `cargo build` - compiles for every target with no flags and Android self-excludes the raw - Ethernet transport as Windows already did. `Node::enable_app_owned_tun()` - gives an embedder that owns the TUN file descriptor — an Android - `VpnService`, for instance — a channel pair for exchanging IPv6 packet bytes - with FIPS instead of FIPS creating a system TUN device, and `start()` then - performs no system-TUN or `CAP_NET_ADMIN` operations. Packets entering this - way bypass `handle_tun_packet`, so the embedder must push only - `fd00::/8`-destined packets and clamp TCP MSS on outbound SYNs. Desktop - builds are unchanged and no Cargo features are introduced. - -- `Node::dns_local_addr()`, the DNS companion to the app-owned TUN seam above. - An embedder whose resolver is pointed into the tunnel has no system socket - aimed at the built-in `.fips` responder, so the accessor reports the address - read back off the bound socket: `dns.port = 0` therefore yields the - kernel-assigned port, and it returns `Some` only while the responder is up. - Read it once, after `start()` returns and before the node is moved into a - background task; it is not a liveness feed (#136). +#### Node lifecycle - A bounded graceful-shutdown drain phase, controlled by the new `node.drain_timeout_secs` (default 2s). On the shutdown signal the node @@ -186,76 +418,7 @@ with v0.4.x or earlier peers. via control queries during the window. The immediate stop path used by non-daemon callers is unchanged. -- FreeBSD support for the daemon, `fipsctl`, and `fipstop`: native TUN - datapath (TUNSIFHEAD address-family framing, kernel-assigned `tunN` - device name as with `utun` on macOS), clean service teardown, - `/usr/local/etc/fips` config search path, and `/var/run/fips` - control-socket default (both shared with macOS). The `hosts`, - `peers.allow` / `peers.deny` and `fipsctl keygen` defaults follow the - same `/usr/local/etc/fips` layout as macOS — see the corresponding - entry under Fixed, which describes that move and its startup warning. - `fips-gateway` remains Linux-only. Native `.pkg` packaging under - `packaging/freebsd/` - (`make freebsd`) with rc.d services, a `fips` control-socket group, - service stop/restart across `pkg upgrade`, and `.fips` DNS integration - for `local_unbound`/`unbound`/`dnsmasq`. mDNS LAN discovery works via - `mdns-sd` 0.20 (`socket-pktinfo` 0.4.1, the first release that builds - on FreeBSD). Daemon logs now disable ANSI color when stdout is not a - terminal (all platforms). -- An optional tick-body profiler behind the new `profiling` Cargo feature, - **off by default**. When enabled, `fipsctl profile tick on [--dir PATH]` / - `off` / `status` starts and stops a capture at runtime with no restart. Each - capture writes one tab-separated file (default `/var/log/fips`, capped at - 32 MB) carrying, per ten-second interval, the exact count, max and total for - every step of the rx-loop tick arm, the whole-tick span, and gauges for ticks, - peer count, the gap between successive tick-arm entries and the resulting - arm-starvation delay. With the feature off the instrumentation macro is a pure - pass-through, so a default build contains no timing code on the tick path. - `LogsDirectory=fips` was added to the packaged systemd units so the capture - directory is created and cleaned up declaratively. - -- `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. - -- `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. 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.rate_limit.established_handshake_burst` and - `node.rate_limit.established_handshake_rate`, the parameters of the new - established-link msg1 token bucket. 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. - -- `packaging/debian/build-deb.sh --features ` builds the `.deb` with a - Cargo feature list, which is how an instrumented package is produced for a - measurement run. The auto-derived dev Version gains a matching `+` - marker, so a feature build and a default build of the same commit are no - longer indistinguishable: without it the two carry byte-identical versions, - an install of one over the other is an apt no-op, and the running node offers - no way to tell which one it has. The marker sorts above the unmarked build, so - installing a feature build is an upgrade and reverting to the default build is - a downgrade — revert with `dpkg -i` rather than `apt install`. `--features` is - refused together with `--no-build`, which would stamp the marker onto binaries - the features never reached. - -- `node.rendezvous.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. Existing configurations parse unchanged, the key being optional. +#### Transports & config - The UDP transport's listen socket descriptor can now be handed to an embedder, for hosts that associate a socket with one interface or network @@ -308,6 +471,20 @@ with v0.4.x or earlier peers. invisible for a peer that has a second address that works: the lane would never carry traffic and nothing above debug logging would say so. +#### Observability & measurement + +- An optional tick-body profiler behind the new `profiling` Cargo feature, + **off by default**. When enabled, `fipsctl profile tick on [--dir PATH]` / + `off` / `status` starts and stops a capture at runtime with no restart. Each + capture writes one tab-separated file (default `/var/log/fips`, capped at + 32 MB) carrying, per ten-second interval, the exact count, max and total for + every step of the rx-loop tick arm, the whole-tick span, and gauges for ticks, + peer count, the gap between successive tick-arm entries and the resulting + arm-starvation delay. With the feature off the instrumentation macro is a pure + pass-through, so a default build contains no timing code on the tick path. + `LogsDirectory=fips` was added to the packaged systemd units so the capture + directory is created and cleaned up declaratively. + - `fipsctl probe ` answers, for one target, where it sits in the spanning tree relative to this node and whether this node can actually reach it. The work runs as five stages that report separately, `bloom`, @@ -339,71 +516,124 @@ with v0.4.x or earlier peers. terminal leaves behind. `--json` emits exactly one document at the end, so a script parsing the report does not have to skip past progress output. -- An **experimental** native datagram API addressed by public key, off by - default and not a stable interface. A client process opens a flow to a - peer's public key on a chosen port and sends and receives datagrams on a - file descriptor the daemon hands it: no IPv6 emulation, no TUN device and no - DNS, a datagram travelling from key to key. **The wire needs no change and - gets none.** Every FSP data packet has carried a port pair inside its AEAD - envelope since v0.2.0 and port 256 is simply the IPv6 shim, so what was - missing was a way for a program to ask for a port of its own and be handed - the traffic. The x-only public key is the address and an npub is that key - written in bech32, so converting between them is a local encoding rather - than a lookup or a name service; the 16-byte node address that travels on - the wire is a truncated hash of the key, does not invert, and appears - nowhere a client can see. A listener is a descriptor: the daemon writes one - message per arrival to it, carrying the new flow's descriptor and the peer's - address, so poll, select and epoll work on a listener and accepting is a - `recvmsg`. There is no accept command and no reject command, and refusing a - flow is closing the descriptor you were handed. The Rust surface mirrors - `std::net`, with `FipsStream::connect`, `FipsListener::bind`, `incoming`, - `accept`, `io::Result` and an errno mapping rather than a bespoke error - type, plus `set_nonblocking`, `AsFd` and four deadline methods under the - names and signatures `std::net` uses for the same jobs. One rule has no - counterpart in Berkeley sockets and a client author must know it: the v1 - wire carries no half-close, so nothing peer-driven ever closes a flow, and a - server written to read until the flow ends waits for a signal that cannot - arrive. The listener uses `SOCK_SEQPACKET` on Linux and FreeBSD and - `SOCK_DGRAM` on macOS, which does not implement `SOCK_SEQPACKET` for - `AF_UNIX`; both keep the message boundaries the API's contract with its - clients rests on. The two kernels signal a closed peer differently and were - measured rather than reasoned about, so the receive path treats a Darwin - `ECONNRESET` as end of file alongside the `POLLHUP` and zero-byte read that - Linux gives. `EAGAIN` is deliberately not in that company: it means the - socket is empty and the peer alive, so it stays an error and the caller - waits again. +#### Packaging & deployment + +- `packaging/debian/build-deb.sh --features ` builds the `.deb` with a + Cargo feature list, which is how an instrumented package is produced for a + measurement run. The auto-derived dev Version gains a matching `+` + marker, so a feature build and a default build of the same commit are no + longer indistinguishable: without it the two carry byte-identical versions, + an install of one over the other is an apt no-op, and the running node offers + no way to tell which one it has. The marker sorts above the unmarked build, so + installing a feature build is an upgrade and reverting to the default build is + a downgrade: revert with `dpkg -i` rather than `apt install`. `--features` is + refused together with `--no-build`, which would stamp the marker onto binaries + the features never reached. ### Changed -- `node.rekey.enabled` now means "initiate rekeys" and nothing else. The - responder half of the establish decision was also gated on it, and once the - rekey is declared in the msg3 negotiation payload that flag was the only - thing that could divert a msg3 whose marker matched the session we hold — it - diverted it to a msg2 resend while the initiator had already installed its - pending session and would cut over on its own timer regardless. A pair - configured with the flag true on one end and false on the other therefore - parted company at the initiator's cutover and carried no traffic in either - direction until the link-dead timer. Removing that gate exposed a second - reading of the same flag: the rekey poll returned early when rekey was - disabled, and its drain-expiry arm is the only thing that releases a demoted - session and its index, so a node that does not initiate but now accepts a - rekey would have pinned the previous session and its allocator index - forever. The trigger and the polled cutover stay gated on the flag; the - drain no longer is. +#### Naming: discovery split into lookup and rendezvous + +- The mesh-lookup control-metrics family is now emitted under the key + `lookup` in `fipsctl stats metrics` and `show routing`. The former key + `discovery` is still emitted as a deprecated alias carrying identical + counters; update dashboards and alerts to read `lookup`. + +- The overloaded `node.discovery.*` config table was split into + `node.lookup.*` (mesh-lookup scalars: `ttl`, `attempt_timeouts_secs`, + `recent_expiry_secs`, `backoff_base_secs`, `backoff_max_secs`, + `forward_min_interval_secs`) and `node.rendezvous.*` (peer rendezvous: + `nostr.*`, `lan.*`). A deployed `node.discovery:` block still loads and is + folded into the new tables with a one-time deprecation warning; migrate your + `fips.yaml` to the new keys. This includes the two keys v0.4.2 introduces + under the old spelling, so an operator upgrading straight from 0.4.2 does not + have to infer them: `node.discovery.nostr.max_concurrent_offers_per_npub` and + `node.discovery.nostr.signal_ttl_secs` are now + `node.rendezvous.nostr.max_concurrent_offers_per_npub` and + `node.rendezvous.nostr.signal_ttl_secs`. + +- The Ethernet transport's per-interface `discovery` flag was renamed to + `listen` (`transports.ethernet.*`) to match the symmetric `announce` + (transmit) / `listen` (receive) neighbor-beacon vocabulary. The old + `discovery:` key is still accepted via a serde alias, so deployed configs + continue to load unchanged; `Config::to_yaml()` re-emits it under the + canonical `listen:` name. Update your `fips.yaml` to `listen:`. + +- `fipstop`'s Routing State pane renames its `Discovery Requests` and + `Discovery Responses` sections to `Lookup Requests` and `Lookup Responses`, + and reads those counters from the canonical `lookup` key rather than from the + deprecated `discovery` alias. The counters themselves are unchanged, so an + operator who knows the pane by its old section labels is reading the same + numbers under new names. + +#### Library surface and internals + +- The protocol layers were restructured into sans-IO cores with the I/O kept in + a thin shell, and two new crate-root modules, `nostr` and `mdns`, own peer + rendezvous and LAN discovery. The crate root also gains the `is_punch_packet` + helper and the `CoordError`, `MtuExceeded`, `COORDS_REQUIRED_SIZE` and + `MTU_EXCEEDED_SIZE` exports. This is an internal reorganization with no + operator-visible behaviour change and no wire change: encoded bytes and decode + decisions are identical. Its two consequences a reader will feel are the + library surface under Removed below and the tracing targets in the next entry. + +- Tracing targets follow module paths, so the module relocations in this release + move them. `fips::discovery::nostr::*` becomes `fips::nostr::*`, mDNS moves to + `fips::mdns::*`, and the protocol subsystems move under `fips::proto::*` + (`fips::tree` to `fips::proto::stp`, `fips::bloom` to `fips::proto::bloom`, + `fips::protocol` to `fips::proto::*`, and the mesh-lookup subsystem from + `fips::discovery` to `fips::proto::lookup`). An existing `RUST_LOG` filter + naming an old target still parses and simply stops matching, so the symptom is + missing log lines rather than an error, and a filter that has gone blind looks + exactly like a subsystem that has gone quiet. Update `RUST_LOG` filters, + journal-watch recipes and any log-scraping alert accordingly. The four targets + named explicitly in source rather than derived from a module path + (`fips::config`, `fips::instr`, `fips::node::handlers::handshake`, + `fips::node::handlers::rekey`) are unaffected. + +#### Node health - Node health is determined at start completion instead of unconditionally reaching a single running state. **Zero transports up is now fatal**: the node tears down cleanly and the daemon exits with an error, where it previously came up and served nothing. Any configured optional child that - failed to start — a transport beyond the first, Nostr, mDNS, TUN, DNS, or a - worker pool — leaves the node degraded but serving, with a warning naming + failed to start, meaning a transport beyond the first, Nostr, mDNS, TUN, DNS, + or a worker pool, leaves the node degraded but serving, with a warning naming what failed, and all configured children up is full health. A child the node was never asked to run does not count against it. The published node state gains `Degraded` and `Failed`, both visible via control queries, with degraded operational and failed not. Exit detection for the DNS task, the two - TUN threads, mDNS and Nostr also re-evaluates health at runtime, so a child - that dies after a healthy start now shows as degraded; transports and worker - pools expose no runtime-exit signal yet and are unchanged. + TUN threads and mDNS also re-evaluates health at runtime, so a child that dies + after a healthy start now shows as degraded. For Nostr the watched children + are the three service loops that cannot return by design (the inbound notify + loop, the advert publisher and the refresh ticker); `connect_task` and + `relay_startup_task` are deliberately not watched, because `Client::connect()` + returns as soon as it has spawned the per-relay background tasks, so a + finished handle there carries no health signal. Transports and worker pools + expose no runtime-exit signal yet and are unchanged. + +#### Peer handshake + +- `node.rate_limit.handshake_resend_interval_ms` no longer governs the **first** + outbound handshake resend. When msg1 is sent, the peer state machine arms the + first retransmit deadline from a hardcoded 1000 ms constant + (`HANDSHAKE_RETRANSMIT_INTERVAL_MS` in `src/peer/machine.rs`), because the + machine-armed timer rather than the connection's stored deadline is now the + due signal the retransmit driver fires on. The key still governs the second + and later resends, alongside `node.rate_limit.handshake_resend_backoff` and + `node.rate_limit.handshake_max_resends`. The constant equals the shipped + default of 1000, so a deployment that never overrode the key sees no change; + one that raised or lowered it will find the first resend still firing at + 1000 ms. The limitation is recorded at the site in the source. + +#### Data-plane / transports + +- Connected UDP peer drains now batch macOS receives with `recvmsg_x(2)`, + matching the wildcard UDP receive path instead of issuing one `recv(2)` + syscall per queued datagram. + +- Routing next-hop selection now visits borrowed peers and coordinates instead + of allocating candidate snapshots for each forwarded packet. - A connected UDP socket that cannot open now names the syscall that failed and the address it was operating on, the local address for `bind` and the peer @@ -415,67 +645,234 @@ with v0.4.x or earlier peers. local address. A node at roughly 245 peers was emitting this three times a second across nine peers with no way to diagnose it. -- Connected UDP peer drains now batch macOS receives with `recvmsg_x(2)`, - matching the wildcard UDP receive path instead of issuing one `recv(2)` - syscall per queued datagram. -- Routing next-hop selection now visits borrowed peers and coordinates instead - of allocating candidate snapshots for each forwarded packet. +#### Observability -- 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. +- The warning raised when a lookup response carries a path MTU below the + actionable floor now names the request it refused, as a `request_id` field on + the log line. Every other warning that handler emits already carried the + correlator, and this one could not: it is raised while the cached-coordinates + effect is applied, after the response itself has gone out of scope. An + operator reading the refusal therefore had a counter and a peer name but no + way to tie the line to a particular exchange, or to the sibling lines logged + for it. **Only the log line changes**: the response is still accepted, the + coordinates are still cached, the sub-floor path MTU is still discarded, and + the same counter is still charged. -- Rekey timer jitter is enabled on next's XX FMP rekey path - (`REKEY_JITTER_SECS = 15` at `src/node/mod.rs`), matching the - IK-line behavior on maint/master. It had been temporarily set to - `0` on next because variable-interval rekeys exposed three XX - rekey-path defects that left the two endpoints on divergent Noise - sessions; those defects are fixed (see `### Fixed`), so the - per-session signed jitter over `[-15, +15]` seconds is restored. - `node.rekey.after_secs` is the nominal interval rather than a floor; - mean is preserved. -- On the XX FMP handshake, an over-cap inbound connection is rejected - solely by the late `promote_connection` check, and the resulting - `MaxPeersExceeded` rejection is logged at debug rather than warn so a - saturated node under sustained inbound pressure does not emit WARN - spam for these expected policy rejections. There is no early cap gate: - on XX the peer's identity is not known until the third handshake - message, by which point Msg1, Msg2, and Msg3 have all crossed the - wire, so an early gate would save no wire bytes and would govern - exactly the same net-new-peer set as the late check. The known-peer / - cross-connection bypass — which also covers peers the node is itself - dialing, e.g. configured `auto_connect` peers — is handled by that - late check, since those peers return earlier via the cross-connection - paths and are not subject to the cap. -- The Ethernet transport's per-interface `discovery` flag was renamed to - `listen` (`transports.ethernet.*`) to match the symmetric `announce` - (transmit) / `listen` (receive) neighbor-beacon vocabulary. The old - `discovery:` key is still accepted via a serde alias, so deployed configs - continue to load unchanged; `Config::to_yaml()` re-emits it under the - canonical `listen:` name. Update your `fips.yaml` to `listen:`. +#### Docs & contributor tooling -- The mesh-lookup control-metrics family is now emitted under the key - `lookup` in `fipsctl stats metrics` and `show routing`. The former key - `discovery` is still emitted as a deprecated alias carrying identical - counters; update dashboards and alerts to read `lookup`. -- The overloaded `node.discovery.*` config table was split into - `node.lookup.*` (mesh-lookup scalars: `ttl`, `attempt_timeouts_secs`, - `recent_expiry_secs`, `backoff_base_secs`, `backoff_max_secs`, - `forward_min_interval_secs`) and `node.rendezvous.*` (peer rendezvous: - `nostr.*`, `lan.*`). A deployed `node.discovery:` block still loads and is - folded into the new tables with a one-time deprecation warning; migrate your - `fips.yaml` to the new keys. +- Source comments and doc comments were corrected across the modules this + release restructured. Unresolvable internal references and planning-phase + locators were removed from comments that exist only on this line, stale + view-trait documentation was rewritten to describe the shape the restructuring + produced, and stale liveness claims in the peer machine, executor and timer + documentation were corrected. No code changed. + +#### Packaging & deployment + +- Every GitHub Actions reference in the workflows and composite actions that + exist only on this line is now pinned to a full commit SHA, so the whole + `.github` tree is pinned or explicitly justified. The branch carries 75 + action references, of which 71 are pinned to a 40-character commit SHA with + the mandatory trailing version comment and 4 are left on mutable tags by + explicit allowance (`dtolnay/rust-toolchain@nightly` and three + `taiki-e/install-action@nextest`); `testing/check-action-pins.sh` reports the + same 75 and passes. Nine of those references were pinned here, in + `package-freebsd.yml` and an `android-check` job block, neither of which + exists on the maintenance branch: the original pinning sweep was authored + there, never saw these files, and they arrived through the merge still on + mutable tags, at which point the guard that shipped with the sweep failed the + branch exactly as intended. The general lesson is worth keeping with the + guard: a checker authored on the earliest branch is only as complete as that + branch's file set, and merging it upward gates files it has never swept. + +### Deprecated + +- The `discovery` metric-family key (control-socket JSON), renamed to `lookup`. + It is dual-emitted alongside the new `lookup` key during a migration window + and will be removed. + Migrate dashboards/alerts from `discovery.*` to `lookup.*`. +- The `node.discovery.*` config table. Its keys were split into `node.lookup.*` + (mesh-lookup) and `node.rendezvous.*` (peer rendezvous). A legacy + `node.discovery:` block still applies for now with a deprecation warning and + will be removed; migrate to `node.lookup.*` / `node.rendezvous.*`. +- The Ethernet `transports.ethernet.discovery` flag, renamed to + `transports.ethernet.listen`. The old key is still accepted via a serde + alias and will be removed at the v2 cutover; migrate to `listen`. + +### Removed + +- **Source-breaking for consumers of the library crate**: the crate-root modules + `bloom`, `discovery`, `mmp`, `protocol` and `tree` are gone, along with the + crate-root `HandshakeState`, `PeerConnection`, `PeerSlot` and `ProtocolError` + re-exports. The protocol cores moved into an internal `proto` module and are + reached through crate-root re-exports instead: tree types through + `proto::stp`, bloom types through `proto::bloom`, the FSP, STP, lookup, + routing and FMP wire types through their matching `proto::*` submodules, and + `PromotionResult` / `cross_connection_winner` through `proto::fmp` rather + than `peer`. **The `HandshakeState` removed here is the peer connection-phase + enum, not the Noise handshake type of the same name.** The Noise type is + untouched, still lives at `fips::noise::HandshakeState`, and it is that type, + not this one, that the `[0.4.2]` entry below about four public types gaining + `Drop` describes. `ProtocolError` is replaced by `fips::Error`, whose + `Malformed` variant now carries a `&'static str` rather than a `String` and + which gained `BadSizeClass`, `BadCoord` and `BadBloom` variants, so the + diagnostic text changed with it. `PeerSlot` and the `PeerConnection` resend + API were unused and are deleted. `Node::connections()` is now `pub(crate)` + and yields the internal peer machine rather than a `PeerConnection`; a + consumer that walked links through it should use `Node::peers()`, + `Node::get_peer()` and `Node::peer_count()` over `ActivePeer`, which remain + public. Nothing about the behaviour of the shipped binaries changes, and + nothing on the wire changes: encoded bytes and decode decisions are identical. + +### Fixed + +#### Control socket + +- `disconnect` on the control socket now closes the transport connection + rather than only the peer. It notified the peer and freed every node-side + structure, sessions, indices, links, address mapping, tree and bloom state, + and never touched the transport, so on a connection-oriented transport (TCP, + Tor, Nym, BLE) the pool entry, the socket and its inbound-slot accounting + survived the peer the node had just forgotten, until the far end closed or + the receive loop errored. An operator who disconnected a peer to free a slot + did not free the slot. No effect on UDP, Ethernet or loopback, whose + `close_connection` is the connectionless no-op. Still not addressed: + `disconnect` reports `peer not found` for an identity that is only + mid-handshake, so withdrawing a peer during its handshake leaves that leg + resending msg1 until the handshake timeout bounds it. + +- `connect` on the control socket now tries the address it was given for a + peer the node is already connected to, instead of reporting success without + doing anything. The command built an ephemeral peer configuration and handed + it to the ordinary dial path, which returns success the moment the peer is + already held, so `fipsctl connect` printed success and the node never + attempted the path. An operator moving a peer onto a freshly provisioned + link, or a supervising process that has just seen a second path come up, had + no way to make the node use it: the peer stayed where it first authenticated + until that path died. The address is now tried as an alternate path + alongside the live one, through the same helper a runtime peer refresh uses, + so promotion happens only after the alternate handshake authenticates and a + wrong address cannot displace a healthy link. The response gains an additive + `refreshed` field distinguishing "started an alternate-path handshake" from + "already on this exact path and it is fresh". `connect` stays ephemeral: the + peer is not written to configuration and gets no auto-reconnect. + +- A failed probe now names the condition it actually hit. Probing an absent + key reported one of two different failures depending on whether a bloom + filter had returned a false positive, and the reason shown was wrong in the + branch that gets likelier as the mesh grows: the verdict was right, the + explanation was not. The core already held the fact and discarded it, since + the lookup gate proceeds only when a peer's filter claimed the address, so a + sent lookup proves the claim; that is now its own failure kind, carried to + the control socket through the existing reason field. `fipsctl` renders the + two differently and closes with a note naming the false-positive mechanism, + which is the part that helps, because the reason string alone does not tell + an operator that a filter cannot miss a key that is present. The + seventeen-second wait is unchanged, being the retry ladder running to + completion; what changes is that the operator is told the wait carried no + information rather than given a wrong reason for it. Still not addressed: + naming which peer's filter made the claim. + +#### Data-plane / transports + +- A path MTU measured on one link no longer clamps a peer that has moved to + another. Every writer of the per-destination path-MTU cache keeps the + smaller of the existing and incoming value, which is right while a peer + stays put, but the entry is keyed by destination alone. So a peer first + reached over a narrow link stayed clamped to that link's ceiling for the + lifetime of the process: when it later became reachable over a wider + transport, promotion re-seeded, the seed saw a tighter existing value and + declined, and traffic kept running at the old link's ceiling on a link that + could carry far more, with nothing reporting it because the clamp was doing + exactly what it was told. The node now records which transport last seeded + each destination and treats a seed from a different one as authoritative + rather than as a loosening to refuse. Re-seeding the same transport still + keeps the tighter value, so repeated promotion does not reset discovery, and + a destination with no prior seed is unchanged. + +#### macOS + +- The packaged macOS daemon recreates and binds its control socket at + `/var/run/fips/control.sock`. A privileged macOS process selects that private + runtime path before its leaf exists, so bind creates it and clients follow + once it is there, and socket setup now changes ownership and mode only for a + private parent directory it creates or recognizes as a canonical FIPS runtime + directory. Previously the packaged daemon fell through to the shared + `/tmp/fips-control.sock` path after every boot, and because socket setup + changed the parent directory unconditionally, the root daemon also took group + ownership of `/tmp` itself. + +#### Packaging & deployment + +- The FreeBSD package manifest carried the same bounced maintainer address the + other packaging paths did, so the contact of record in the new `.pkg` artifact + would have been unreachable at its first release. + +## [0.4.2] - unreleased + +### Added + +#### 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.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 + +- Config validation now rejects two `node.rekey` settings that appear to + disable the trigger and in fact fire it continuously. `after_messages` of + zero makes the message-count arm true on every poll, because the trigger + compares the counter with greater-or-equal. `after_secs` at or below the + per-session jitter bound is the same trap on the timer arm: 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. Both are checked whether or not rekey is + enabled, so switching it on later cannot surface the error at a surprising + moment, and neither gains an upper bound; a very large value remains the + 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 @@ -496,7 +893,7 @@ with v0.4.x or earlier peers. attempt; that duration remains inferred from the attempt timeout rather than measured. -- Config validation now rejects a `node.rendezvous.nostr.signal_ttl_secs` that +- 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 @@ -512,18 +909,39 @@ with v0.4.x or earlier peers. Note that this covers eviction on expiry only: `seen_sessions_max_entries` remains a separate capacity-eviction route that no config relation bounds. -- Config validation now rejects two `node.rekey` settings that appear to - disable the trigger and in fact fire it continuously. `after_messages` of - zero makes the message-count arm true on every poll, because the trigger - compares the counter with greater-or-equal. `after_secs` at or below the - per-session jitter bound is the same trap on the timer arm: 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. Both are checked whether or not rekey is - enabled, so switching it on later cannot surface the error at a surprising - moment, and neither gains an upper bound — a very large value remains the - supported way to disable one arm. A config carrying either setting now fails - to load instead of starting a node that rekeys constantly. +- 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 @@ -544,34 +962,16 @@ with v0.4.x or earlier peers. 240 peers. The display name itself is deliberately not cached, because the alias map and the host map both mutate at runtime. -- 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. +#### Transports & config -- 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. +- `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. -- `SessionDatagram::decrement_ttl` and `SessionDatagram::can_forward` now match - the forwarder's IP hop-limit 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. - -- 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. +#### 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`, @@ -583,94 +983,98 @@ with v0.4.x or earlier peers. behaviour of the shipped binaries changes; this affects only callers using `fips` as a library. -- The warning raised when a lookup response carries a path MTU below the - actionable floor now names the request it refused, as a `request_id` field on - the log line. Every other warning that handler emits already carried the - correlator, and this one could not: it is raised while the cached-coordinates - effect is applied, after the response itself has gone out of scope. An - operator reading the refusal therefore had a counter and a peer name but no - way to tie the line to a particular exchange, or to the sibling lines logged - for it. **Only the log line changes** — the response is still accepted, the - coordinates are still cached, the sub-floor path MTU is still discarded, and - the same counter is still charged. +#### CI & test-harness reliability -- A dial that names a peer now requires that peer to answer. Under Noise XX - the responder's static key arrives during the handshake instead of being - pinned before it, so a cryptographically valid msg2 proves only that - somebody answered, not that the peer we asked for did. The initiator holds - the dial-time identity in its own field and compares it against the - identity msg2 carried, dropping the leg on a mismatch. Two situations that - used to end in a connection no longer do, both of them intended. +- 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 LAN peer advertised under the wrong npub is refused rather than peered - under its real identity. mDNS adverts are unauthenticated and the npub in - the TXT record is taken as the identity to dial, so anyone on the LAN can - point a node at a real host under someone else's name; that dial now ends - in a rejection instead of promoting whoever answered. It is a repeating - refusal and not a permanent stop: the candidate comes back each time the - advert is resolved again, and each return costs one handshake. +- 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. - A peer whose key has rotated stops connecting for as long as configuration - or a discovery record still names its old npub. This is what pinning means - and there is no form of it that keeps the previous behaviour, but it is a - change in a situation with no attacker in it: a rotated peer used to come - back quietly under its new identity, and the stale npub went unnoticed. +- 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. - What an operator sees in both cases is a peer that never comes up, and one - warning per attempt reading `msg2 answered by a different static than the - one dialed, dropping the leg`, carrying the link, the identity dialed and - the identity learned. The rejection is charged to the undifferentiated - `BadState` handshake reject counter and has none of its own, so the log - line is the only thing that names the cause. A configured peer is put back - on the retry schedule under the identity that was dialed, never under the - one that answered, so the warning repeats on backoff until the mismatch is - resolved. +- 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. - For a legitimate key rotation the fix is to update the peer's npub in - configuration, or to let discovery republish it under the new key; the next - dial then matches and the peer comes up. Dials that name nobody, meaning - shared-media legs and every inbound leg, are unaffected and still promote - whoever answers. +- 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. -### Deprecated +- 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. -- The `discovery` metric-family key (control-socket JSON). It is dual-emitted - alongside the new `lookup` key during a migration window and will be removed. - Migrate dashboards/alerts from `discovery.*` to `lookup.*`. -- The `node.discovery.*` config table. Its keys were split into `node.lookup.*` - (mesh-lookup) and `node.rendezvous.*` (peer rendezvous). A legacy - `node.discovery:` block still applies for now with a deprecation warning and - will be removed; migrate to `node.lookup.*` / `node.rendezvous.*`. -- The Ethernet `transports.ethernet.discovery` flag, renamed to - `transports.ethernet.listen`. The old key is still accepted via a serde - alias and will be removed at the v2 cutover; migrate to `listen`. +- 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 -- A leaf-profile node no longer self-elects as tree root. A leaf holding the - smallest node address elected itself, but its peers refuse a non-full node as - a parent, so it formed an isolated second root and partitioned the mesh: a - multi-hop session from the leaf to a non-adjacent full node then failed, - because the far node could not route a handshake reply back into the leaf's - separate coordinate tree. A leaf now attaches under its full upstream and - holds that subtree's coordinate for its own routing; it never announces that - coordinate, reaching peers via the ones carried on its session frames, and - the upstream already advertises it in the upstream's bloom filter. - `Node::with_identity` also derives the node profile, leaf-only flag and bloom - state from the config as `Node::new` does, where it previously hardcoded the - full profile and silently dropped a configured leaf or non-routing profile. - -- Every XX handshake reject arm now releases the session index and the link it - holds. The msg2 self-connect drop and the msg3 reject arms disposed of a leg - without returning what that leg had allocated, so each rejected handshake - leaked an index and a link entry. One arm is deliberately not fixed like the - others: its index comes from the receiver field of the incoming header, which - the peer supplies, so freeing it unconditionally would let a hostile peer - release an index belonging to an unrelated live session, trading a memory - leak for a remote session teardown. That arm frees only after a - transport-blind predicate establishes the index is not claimed elsewhere. - The outbound ACL-reject arm also regains the reschedule call its dial-gate - sibling makes, so a configured peer no longer drops off the dial schedule. +#### FMP/FSP rekey reliability - Inbound session-setup messages are now rate limited, keyed on the authenticated link peer the datagram arrived over. The setup path allocated @@ -692,14 +1096,14 @@ with v0.4.x or earlier peers. establishment behind a flooded neighbour is refused until it refills. Rekey and restart traffic is deliberately not subject to that: setup messages naming an already-established peer draw on a separate per-link bucket, - because suppressed key rotation is silent — nothing errors and no session - drops — and would have shown up only as a flat `rekey_armed`. + because suppressed key rotation is silent (nothing errors and no session + drops) and would have shown up only as a flat `rekey_armed`. - 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 - 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 + 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 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 @@ -732,58 +1136,58 @@ with v0.4.x or earlier peers. 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. - The five failure paths in the responder-side rekey arm of the msg3 handler - abandoned the whole rekey, which nulls a `pending` session sitting beside the - handshake, when only the handshake had failed. 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 fifth, the negotiation - payload split out of msg3, is specific to the XX handshake and has no - counterpart on the maintenance branch. The dual-initiation arm of the setup - handler was the sixth instance of the same defect and is fixed with them: it - is gated on a rekey being in progress rather than on this node having - initiated one, so a handshake the peer armed beside a completed epoch - reaches it on nothing more than a second unauthenticated setup message. The - four remaining calls in the ack handler's rekey-initiator arm are left - alone, and the reason is recorded at the site: an entry carrying the - initiator flag holds no pending session, so the destructive and - non-destructive forms are the same action there. +- 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. -- The packaged macOS daemon now recreates and binds its control socket at - `/var/run/fips/control.sock` instead of falling through to the shared - `/tmp/fips-control.sock` path after boot. The previous fallback also made the - root daemon attempt to chown `/tmp` itself to the `fips` group because socket - setup unconditionally changed the parent directory. A privileged macOS - process now selects the private runtime path before its leaf exists, clients - follow it once created, and socket setup changes ownership and mode only for - a private parent directory it creates or recognizes as a canonical FIPS - runtime directory. +#### NAT traversal / Nostr discovery -- 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`, 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. +- 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 @@ -803,35 +1207,74 @@ with v0.4.x or earlier peers. 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. + 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. -- A `node.tree.flap_dampening_secs` large enough to overflow the monotonic - clock no longer panics the node when dampening engages; the value is - capped at one year, beyond which an episode is indistinguishable from - permanent. +#### Data-plane / metrics / observability -- The maintainer address published in package metadata no longer bounces. The - crate authors field, the Debian package maintainer and upstream contact, - both AUR PKGBUILD maintainer lines and the FreeBSD package manifest carried - an address that no longer accepts mail, so the contact of record in every - artifact we ship was unreachable. +- 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. -- 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. +- `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 + +- 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 @@ -840,7 +1283,7 @@ with v0.4.x or earlier peers. 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 + 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 @@ -848,230 +1291,68 @@ with v0.4.x or earlier peers. 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/`). 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. + 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, matching the install layout the macOS - packaging ships. 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 — `/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. + `/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`, - matching the install layout the macOS packaging ships. 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 + `/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 + 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. -- 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). +#### Packaging & deployment -- XX FMP rekey no longer diverges under timer jitter, which unblocked - re-enabling the rekey jitter on next (`REKEY_JITTER_SECS = 15`; see - `### Changed`). With jitter the two directions of a link rekey close - together in time, and three defects specific to the XX three-message - rekey state machine could each leave the endpoints committed to - different Noise sessions — silent session divergence that starved the - receiver into ~50% post-rekey ping loss and a 30-second heartbeat - link-dead teardown (tree parent loss, routing failure) while every - crypto, transport, and link-state gate stayed green. All three are - fixed: - - The K-bit-flip handler promoted whatever pending session existed the - instant the header bit flipped, which under interleaved rekeys could - be a stale pending from an earlier epoch. It now trial-decrypts the - inbound frame against the pending session and promotes only on an - authenticated decrypt, delivering that plaintext through the - canonical path and leaving the pending untouched otherwise — the - same cutover discipline used on FSP. - - The FMP rekey msg3 was sent once, so a lost datagram left the - responder without the new session. The msg3 payload is now retained - and retransmitted over the existing link until a peer frame - authenticates against the pending or post-cutover current session, - abandoning after the configured handshake-resend budget. Per-link - rekeys are also serialized: a new rekey does not start while one - awaits cutover or is still retransmitting msg3. - - The `handle_msg3` cross-connection and rekey-responder paths were - partitioned by a fixed 30-second session-age threshold, but a rekey - resets the session-age clock, so under jitter a rekey-aged msg3 was - frequently under 30 seconds and got swallowed by the - initial-handshake cross-connection branch, which discarded the - peer's rekey session with no pending slot while the peer cut over to - it anyway. The cross-connection branch is now bounded by the same - jitter-aware session-age floor the rekey responder uses, so the two - paths partition with no overlap. At zero jitter the floor equals the - previous 30-second constant, so default-cadence behavior is - unchanged. -- XX rekey dual-initiation race that broke six pair-directions - post-rekey when both endpoints initiated rekey simultaneously. - The `handle_msg3` tie-breaker only fired when `rekey_in_progress` - was still true, but XX's three-message handshake lets both sides - clear that flag (via `set_pending_session`) before either's msg3 - lands. The drop-on-pending-session guard then silently discarded - the peer's msg3, each side cut over to its own initiator session, - and the link broke asymmetrically. The tie-breaker now also fires - when `pending_new_session().is_some()`, applying the same - smaller-NodeAddr resolution rule. Mirrored to the FSP rekey msg1 - path for symmetry. -- `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. 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. - -- `disconnect` on the control socket now closes the transport connection - rather than only the peer. It notified the peer and freed every node-side - structure, sessions, indices, links, address mapping, tree and bloom state, - and never touched the transport, so on a connection-oriented transport (TCP, - Tor, Nym, BLE) the pool entry, the socket and its inbound-slot accounting - survived the peer the node had just forgotten, until the far end closed or - the receive loop errored. An operator who disconnected a peer to free a slot - did not free the slot. No effect on UDP, Ethernet or loopback, whose - `close_connection` is the connectionless no-op. Still not addressed: - `disconnect` reports `peer not found` for an identity that is only - mid-handshake, so withdrawing a peer during its handshake leaves that leg - resending msg1 until the handshake timeout bounds it. - -- `connect` on the control socket now tries the address it was given for a - peer the node is already connected to, instead of reporting success without - doing anything. The command built an ephemeral peer configuration and handed - it to the ordinary dial path, which returns success the moment the peer is - already held, so `fipsctl connect` printed success and the node never - attempted the path. An operator moving a peer onto a freshly provisioned - link, or a supervising process that has just seen a second path come up, had - no way to make the node use it: the peer stayed where it first authenticated - until that path died. The address is now tried as an alternate path - alongside the live one, through the same helper a runtime peer refresh uses, - so promotion happens only after the alternate handshake authenticates and a - wrong address cannot displace a healthy link. The response gains an additive - `refreshed` field distinguishing "started an alternate-path handshake" from - "already on this exact path and it is fresh". `connect` stays ephemeral: the - peer is not written to configuration and gets no auto-reconnect. - -- A path MTU measured on one link no longer clamps a peer that has moved to - another. Every writer of the per-destination path-MTU cache keeps the - smaller of the existing and incoming value, which is right while a peer - stays put, but the entry is keyed by destination alone. So a peer first - reached over a narrow link stayed clamped to that link's ceiling for the - lifetime of the process: when it later became reachable over a wider - transport, promotion re-seeded, the seed saw a tighter existing value and - declined, and traffic kept running at the old link's ceiling on a link that - could carry far more, with nothing reporting it because the clamp was doing - exactly what it was told. The node now records which transport last seeded - each destination and treats a seed from a different one as authoritative - rather than as a loosening to refuse. Re-seeding the same transport still - keeps the tighter value, so repeated promotion does not reset discovery, and - a destination with no prior seed is unchanged. +- 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 -- The peer static key is verified on both FMP handshake paths, not only at - rekey. Under Noise XX the static is learned during the handshake rather than - pinned in advance, and neither path that learns one checked it against what - we already knew, so an attacker able to observe and inject on path could - substitute their own identity on a fresh dial and on an established link. On - a fresh dial we recorded who we meant to reach and then overwrote it with - whoever answered without comparing the two, so an attacker who raced the real - peer to msg2 became the peer — promotion, the ACL check and the peer registry - all ran on the answering identity — and the intended node was never reached. - On an established link a rekey msg2 was matched to its peer only by the - session index we had put in the cleartext msg1 header, so anyone who saw that - header could answer with their own static and take the link over at cutover. - Both are now compared before anything is committed, with the dial-time - expectation held in a field that has no setter so the handshake cannot - overwrite it. Anonymous dials still promote whoever answers, which is what - shared-media discovery means. - -- The influence a remote party has over path MTU is now bounded, and the - per-destination path MTU cache has a way back. The `path_mtu` field is an - unsigned per-hop transit annotation carried outside the signed proof, and the - `MtuExceeded` and `PathBroken` signals arrive unencrypted with no sender - check, so any forwarder — or anyone who can reach the node — could lower it, - and it was accepted with no minimum. 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 that lasted until the daemon restarted. The same value - reached the SYN-time TCP MSS clamp, where anything at or below 137 saturates - to a segment size of zero and the band just above it yields single digits. - Values below an actionable minimum are now ignored rather than applied or - stored, at the three places a remote value is acted on: the path MTU state - machine, the reactive `MtuExceeded` write, and the discovery response, whose - coordinates are still cached so refusing the annotation cannot become a way - to deny discovery. The MSS clamp additionally refuses to write a zero. Each - of the three refusals logs a warning and increments its own counter in the - error-signal family, so an operator can tell them apart without scraping - logs: they carry different meanings, one being an authenticated peer inside - an established session, one an unencrypted signal anyone able to reach the - node can send at will, and one a verified discovery response whose unsigned - annotation a forwarder on the reverse path rewrote. Because those three - refusals are the only way a remote value reaches the per-destination store, - the SYN-time clamp does not apply the minimum a second time when it reads - that store: a small value there is one the node derived from its own outgoing - link, which is exact rather than suspect, and BLE in particular negotiates a - link MTU per connection that lands under the minimum routinely. The clamp - refuses only a stored value admitting no TCP payload byte at all, at 137 or - below, where the segment size saturates to zero and the clamp would be - skipped entirely; it logs that at trace rather than warn, since it sits on - the per-packet path, and the peer's link promotion reports it once instead. - A stored per-destination path MTU is released when the path is invalidated by - a `PathBroken` report, by session idle expiry, or by handshake timeout, and - the link MTU read from the local transport is reseeded in its place, so a - directly connected peer does not lose its own measurement along with the - remote claim. Locally derived MTUs are not subject to the minimum, at the - seed or at the clamp. Legitimate narrow paths are unaffected: adaptation to - hops well below the IPv6 minimum, which the mesh does use, continues to work. +#### FMP/FSP session integrity - A session setup message naming an already-established peer no longer replaces that peer's session. The handler did this whenever `node.rekey.enabled` was @@ -1087,35 +1368,97 @@ with v0.4.x or earlier peers. changes no wire format and adds no configuration: a node with rekey disabled already answered such a message, it simply destroyed the session afterwards. +- A session rekey armed by a peer's setup message is abandoned if the matching + msg3 never arrives, rather than persisting for the life of the session, so an + arming that never completes cannot make the node read a later genuine setup + message as a simultaneous initiation and drop it. Only the armed handshake + expires, and only it: a rekey that completed is the key epoch the peer has + already moved to, since it exists only because a msg3 carrying that peer's + authenticated key arrived and the sender of that msg3 promotes the new epoch + on an unconditional two-second timer. Expiring those keys on any timer would + drop every later frame from that peer, so they are held until the peer's own + frame promotes them, a newer completed rekey replaces them, or the session + goes away. What the wait does bound is precedence, not the keys: a completed + rekey outranks a fresh setup message from that peer only until it has waited a + full idle timeout, after which the setup is answered normally, so a peer that + restarted while we held such a session is not refused for as long as our own + sends keep the session from idling out. The handshake timeout logs at INFO, + since it costs nothing, and a completed session displaced by a newer one at + WARN, since that does throw away keys the peer may hold. Session counters + record the arming of a handshake by a setup message, each of the three ways + such a message is refused, and each displaced session, so a node under a + sustained spray of setup messages shows a rate rather than nothing; the + per-message log lines stay at DEBUG because an unauthenticated sender can + drive them at line rate. These counters are not yet readable through the + control socket. + - The session drain sweep and the cut-over that retires an old key epoch now run whether or not periodic rekey is enabled. Both sat behind the periodic-rekey gate, so a node with rekey disabled that adopted new keys held the superseded ones for the life of the session. -- A session rekey armed by a peer's setup message is now abandoned if the - matching msg3 never arrives, rather than persisting for the life of the - session. A stuck one made the node treat a later genuine setup message as a - simultaneous initiation and drop it, which would otherwise have turned the - fix above into a lasting block on re-establishment for roughly half of peer - pairs. Only the armed handshake expires, and only it: a rekey that completed - is the key epoch the peer has already moved to, since it exists only because - a msg3 carrying that peer's authenticated key arrived and the sender of that - msg3 promotes the new epoch on an unconditional two-second timer. Expiring - those keys on any timer would drop every later frame from that peer, so they - are now held until the peer's own frame promotes them, a newer completed - rekey replaces them, or the session goes away. What the wait does bound is - precedence, not the keys: a completed rekey outranks a fresh setup message - from that peer only until it has waited a full idle timeout, after which the - setup is answered normally, so a peer that restarted while we held such a - session is no longer refused for as long as our own sends keep the session - from idling out. The handshake timeout logs at INFO, since it costs nothing, - and a completed session displaced by a newer one at WARN, since that does - throw away keys the peer may hold. Session counters record the arming of a - handshake by a setup message, each of the three ways such a message is - refused, and each displaced session, so a node under a sustained spray of - setup messages shows a rate rather than nothing; the per-message log lines - stay at DEBUG because an unauthenticated sender can drive them at line rate. - These counters are not yet readable through the control socket. +- The FSP session address is now bound to the peer key the Noise handshake + authenticated, on both the initial and the rekey path. The responder recorded + a session under the source address carried in the datagram without ever + checking that address against the static key it had just authenticated, so a + peer could complete a genuine handshake while claiming another node's + address, and the identity cache, the session map and the address the IPv6 + shim reconstructs on delivery would all attribute its traffic to the node it + named. The address is now derived from the authenticated key at the point it + first becomes available in msg3, and a mismatch drops the half-open session + without recording either the identity or the session. The rekey responder + needed its own check: it returns before that code is reached and never read + the peer's static key at all, so a rekey could complete under an established + session with a different key than the one that opened it. It now requires the + key to be unchanged and abandons the rekey while leaving the existing session + intact, rather than tearing the session down, which would have handed an + attacker a way to kill established sessions. Both comparisons are on x-only + keys, because a stored key may carry a synthesized parity while the handshake + learns the true point. The two rejections are counted separately in the + session reject statistics. + +- A link handshake admitted by the established-address waiver is now confirmed + against the identity that owns that address, instead of on the address alone. + A transport configured with `accept_connections` false still admits an inbound + msg1 whose source matches an established peer, so that a peer re-handshaking + after a restart or a rekey is not locked out, but nothing checked that the + party sourcing from that address was the peer. Any off-path party able to send + from it therefore obtained a full link handshake from a node configured to + accept none. Once the key exchange reveals the initiator's static key, the + handshake is now dropped unless that key belongs to the identity the matched + address is attributed to, and dropped as well when the waiver was used and no + identity owns the address at all, which fails closed rather than skipping the + check for that case. The cheap refusal is unchanged: a stranger reaching a + transport that refuses inbound connections is still turned away before any + cryptography. Attribution consults both the reverse-address lookup and the + scan over established peers rather than stopping at whichever answers first, + because the reverse lookup can name a link that no longer exists, and stopping + there would refuse a peer the scan can still attribute, permanently, since + the confirmation returns above the code that repairs that lookup. Both + refusals log at warning level, naming the expected and actual peers, and + charge the existing handshake bad-state rejection counter rather than one of + their own. + +- An inbound frame whose header disagrees with the frame that arrived is now + dropped, at the single dispatch point every transport converges on, before the + declared length can be used as a parsing input. The 4-byte common prefix + carries a payload length that the node never read, so on the datagram + transports (UDP, Ethernet and BLE, which deliver one whole frame per packet + and where the arrived length is therefore known exactly) nothing compared the + two. This closes no known defect, and it is worth being exact about what it + drops on a deployed line. The stream transports (TCP, Tor, Nym) read their + frame boundary out of that same field, so the comparison holds by construction + and never fires for them. A short datagram is a truncated frame, which already + failed the AEAD tag or the exact-size handshake parse, so what changes there + is which reason it is dropped for rather than whether it is dropped. A frame + whose phase the node does not recognize carries no fixed relationship between + the two and is left alone rather than rejected on a guess. The drop takes its + own rejection reason and its own `payload_len_mismatch` counter rather than + reusing the admission one, since it is a framing rejection decided before the + 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 - Traversal punch targets taken from a peer's offer or answer are now filtered and bounded. A rendezvous-enabled node previously punched every @@ -1169,114 +1512,56 @@ 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. -- The FSP session address is now bound to the peer key the Noise handshake - authenticated, on both the initial and the rekey path. The responder recorded - a session under the source address carried in the datagram without ever - checking that address against the static key it had just authenticated, so a - peer could complete a genuine handshake while claiming another node's - address, and the identity cache, the session map and the address the IPv6 - shim reconstructs on delivery would all attribute its traffic to the node it - named. The address is now derived from the authenticated key at the point it - first becomes available in msg3, and a mismatch drops the half-open session - without recording either the identity or the session. The rekey responder - needed its own check: it returns before that code is reached and never read - the peer's static key at all, so a rekey could complete under an established - session with a different key than the one that opened it. It now requires the - key to be unchanged and abandons the rekey while leaving the existing session - intact, rather than tearing the session down, which would have handed an - attacker a way to kill established sessions. Both comparisons are on x-only - keys, because a stored key may carry a synthesized parity while the handshake - learns the true point. The two rejections are counted separately in the - session reject statistics. +#### Data-plane / routing signals -- 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 - yanked. `nostr` moves to 0.44.8 and `nostr-relay-pool` to 0.44.3; the - requirements in `Cargo.toml` already admitted both, so this is a lockfile - change and no code changed with it. The advisories that matter here are the - relay-pool ones, RUSTSEC-2026-0224 and RUSTSEC-2026-0232, which describe - forged events bypassing signature validation and unverified relay events - being processed: that is the path this 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 advisory summaries - lead with. RUSTSEC-2026-0231 (auth-challenge memory exhaustion) is on the - same path, and RUSTSEC-2026-0216 and RUSTSEC-2026-0227 reach the NIP-44 - decryption of relay-supplied content. The remaining advisories in that set - cover NIP-04, NIP-46, NIP-50, NIP-60, NIP-98 and the wallet parsers, none of - which this code calls. The refresh was taken over the whole lockfile rather - than the two crates alone, which additionally clears RUSTSEC-2026-0204 in - `crossbeam-epoch` and leaves no yanked crate in the tree; `cargo audit` now - reports no vulnerability, against twelve before. Four warnings remain and are - not fixable by a version move: `instant` and `paste` are unmaintained, `lru` - 0.16.4 carries an unsoundness advisory, and `nostr-relay-pool` itself is now - marked unmaintained. - -- The gateway DNS forwarder now validates an upstream answer before it becomes - a NAT mapping. It previously accepted whatever datagram arrived: the upstream - query reused the client's own transaction ID, the upstream socket was - wildcard-bound and never connected, the receive discarded the sender, neither - the response ID nor the question section 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 that carries no interface - constraint, a forged answer redirected traffic rather than only poisoning a - lookup. The upstream 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 or it is discarded while the receive continues against the - original deadline, and the address goes through the validating parser with a - non-mesh answer refused before any allocation. One deliberate behaviour - change: the 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. Connecting the socket also means a dead upstream surfaces - ECONNREFUSED immediately instead of stalling for five seconds. - -- 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 - create and truncate and no `O_NOFOLLOW`, so a symlink planted at the key path - was followed and its target overwritten, and it supplied the mode only - through `open(2)`, which the kernel honours on creation and ignores - otherwise, so a `fips.key` that already existed at 0644 stayed 0644 through - every rewrite. That second half needs no attacker: one `chmod`, or a restore - that did not preserve modes, leaves the key readable indefinitely. Both - writers now share an open helper carrying `O_NOFOLLOW`, and the private key - has its mode applied to the open descriptor before any secret bytes are - written. The public key keeps create-time mode instead, since forcing it - would reopen an operator-tightened `fips.pub` on every start. On Windows - neither protection applies and the file inherits the parent directory's - ACLs; that exclusion is deliberate and recorded at both writers. - -- An accepted inbound TCP connection no longer holds a slot indefinitely - without sending anything. The cap was tested at accept and the pool insert - and counter bump followed with no read in between, while the frame reader's - reads carried no deadline, so an unauthenticated remote held a slot by - connecting and staying silent. Pool keys are `ip:port`, so N sockets from one - address took N slots, and at the 256 default that locked out inbound peering - for as long as the sockets stayed open. The first frame on an inbound - connection now has a deadline, as a module constant rather than a new - configuration key, and the onion listener gets the same treatment for the - same accept-then-count ordering. Separately, the node's handshake reaper tore - down session state without closing the transport connection, so a peer that - sent msg1 and then stalled was forgotten by the node while its socket and - slot survived; the reaper now closes the connection too. **What this does not - close**: the deadline covers the first frame only, so a peer that sends one - well-formed frame and then goes silent still holds its slot. Closing that - needs a rolling idle deadline. - -- Every GitHub Action is pinned to a commit SHA, and the OpenWrt packaging - workflow verifies the helper binary it downloads. No reference in the - repository was pinned before: all sixty-six named a mutable tag and one named - a branch, 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 now full commit SHAs with the original tag - retained as a trailing comment. Four are left unpinned and justified in one - place: two actions read the tool to install from the ref name itself, so a - SHA would hand them a hex string where a toolchain name belongs. A guard - enforces the form on every sweep, treats an unreadable tree as an error - rather than a pass, and documents what it does not cover. The sharper hole - was not the tags: the OpenWrt workflow fetched a helper binary from a release - URL with no verification at all, in two jobs holding a signing key. That - download now checks a per-architecture pinned SHA-256, with the hash - provenance recorded honestly, upstream publishing no checksum document. +- The influence a remote party has over path MTU is now bounded, and the + per-destination path MTU cache has a way back. The `path_mtu` field is an + unsigned per-hop transit annotation carried outside the signed proof, and the + `MtuExceeded` and `PathBroken` signals arrive unencrypted with no sender + check, so any forwarder, or anyone who can reach the node, could lower it, + and it was accepted with no minimum. 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 that lasted until the daemon restarted. The same value + reached the SYN-time TCP MSS clamp, where anything at or below 137 saturates + to a segment size of zero and the band just above it yields single digits. + Values below an actionable minimum are now ignored rather than applied or + stored, at the three places a remote value is acted on: the path MTU state + machine, the reactive `MtuExceeded` write, and the discovery response, whose + coordinates are still cached so refusing the annotation cannot become a way + to deny discovery. The MSS clamp additionally refuses to write a zero. Each + of the three refusals logs a warning and increments its own counter in the + error-signal family, so an operator can tell them apart without scraping + logs: they carry different meanings, one being an authenticated peer inside + an established session, one an unencrypted signal anyone able to reach the + node can send at will, and one a verified discovery response whose unsigned + annotation a forwarder on the reverse path rewrote. Because those three + refusals are the only way a remote value reaches the per-destination store, + the SYN-time clamp does not apply the minimum a second time when it reads + that store: a small value there is one the node derived from its own outgoing + link, which is exact rather than suspect, and BLE in particular negotiates a + link MTU per connection that lands under the minimum routinely. The clamp + refuses only a stored value admitting no TCP payload byte at all, at 137 or + below, where the segment size saturates to zero and the clamp would be + skipped entirely; it logs that at trace rather than warn, since it sits on + the per-packet path, and the peer's link promotion reports it once instead. + A stored per-destination path MTU is released when the path is invalidated by + a `PathBroken` report, by session idle expiry, or by handshake timeout, and + the link MTU read from the local transport is reseeded in its place, so a + directly connected peer does not lose its own measurement along with the + remote claim. The release also resets the session's own current path MTU + alongside the address-keyed entry, rather than reseeding the link value and + leaving the tightened one in place, so a path declared dead recovers at once + instead of only through the increase ladder. Entries written by the discovery + lookup carrier carry their own deadline and age out, since a destination this + node never opens a session with reaches none of the three release routes: + without that, a single response carrying a floor value pinned that + destination's clamp until restart, and an unknown request id still classifies + as originator, so a captured response could be replayed indefinitely. The + notification mirror deliberately carries no deadline. Locally derived MTUs + are not subject to the minimum, at the seed or at the clamp. Legitimate + narrow paths are unaffected: adaptation to hops well below the IPv6 minimum, + which the mesh does use, continues to work. - The three routing signals (`CoordsRequired`, `PathBroken`, `MtuExceeded`) are no longer acted on unless this node has itself bound the destination address @@ -1303,6 +1588,63 @@ with v0.4.x or earlier peers. claimed source and destination pairing no honest forwarder could produce. The drop log line now carries the signal type and the refusal class. +#### Admission / peer caps + +- An accepted inbound TCP connection no longer holds a slot indefinitely + without sending anything. The cap was tested at accept and the pool insert + and counter bump followed with no read in between, while the frame reader's + reads carried no deadline, so an unauthenticated remote held a slot by + connecting and staying silent. Pool keys are `ip:port`, so N sockets from one + address took N slots, and at the 256 default that locked out inbound peering + for as long as the sockets stayed open. The first frame on an inbound + connection now has a deadline, as a module constant rather than a new + configuration key, and the onion listener gets the same treatment for the + same accept-then-count ordering. Separately, the node's handshake reaper tore + down session state without closing the transport connection, so a peer that + sent msg1 and then stalled was forgotten by the node while its socket and + slot survived; the reaper now closes the connection too. **What this does not + close**: the deadline covers the first frame only, so a peer that sends one + well-formed frame and then goes silent still holds its slot. Closing that + needs a rolling idle deadline. + +#### Gateway + +- The gateway DNS forwarder now validates an upstream answer before it becomes + a NAT mapping. It previously accepted whatever datagram arrived: the upstream + query reused the client's own transaction ID, the upstream socket was + wildcard-bound and never connected, the receive discarded the sender, neither + the response ID nor the question section 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 that carries no interface + constraint, a forged answer redirected traffic rather than only poisoning a + lookup. The upstream 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 or it is discarded while the receive continues against the + original deadline, and the address goes through the validating parser with a + non-mesh answer refused before any allocation. One deliberate behaviour + change: the 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. Connecting the socket also means a dead upstream surfaces + ECONNREFUSED immediately instead of stalling for five seconds. + +#### Key material and identity files + +- 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 + create and truncate and no `O_NOFOLLOW`, so a symlink planted at the key path + was followed and its target overwritten, and it supplied the mode only + through `open(2)`, which the kernel honours on creation and ignores + otherwise, so a `fips.key` that already existed at 0644 stayed 0644 through + every rewrite. That second half needs no attacker: one `chmod`, or a restore + that did not preserve modes, leaves the key readable indefinitely. Both + writers now share an open helper carrying `O_NOFOLLOW`, and the private key + has its mode applied to the open descriptor before any secret bytes are + written. The public key keeps create-time mode instead, since forcing it + would reopen an operator-tightened `fips.pub` on every start. On Windows + neither protection applies and the file inherits the parent directory's + ACLs; that exclusion is deliberate and recorded at both writers. + - Private and symmetric key material is now cleared when it goes out of scope. Nothing in the crate erased a key before this: the node's private key sat in the loaded configuration in plaintext for the whole process lifetime, which is @@ -1315,7 +1657,7 @@ with v0.4.x or earlier peers. holds for its whole duration, the identity's long-term keypair, the temporary copy each of the fourteen elliptic-curve operations makes from a keypair, the bech32 and hex encodings of a secret, and the private key on its way through - configuration — including the config file's whole text, which is treated as + configuration, including the config file's whole text, which is treated as secret for as long as it is held, since `node.identity.nsec` is read straight out of it. Two places that assigned over an already-loaded key now clear the old value first: assignment frees the previous string without running the @@ -1329,47 +1671,63 @@ with v0.4.x or earlier peers. deliberately left alone; the residue any of this leaves needs access to the process's memory, or to a core dump or swap image of it, to read. Adds a dependency on `zeroize`. Nothing on the wire and no configuration changes. + See the `### Changed` note above for the source-breaking effect the four new + `Drop` implementations have on library consumers. -- A link handshake admitted by the established-address waiver is now confirmed - against the identity that owns that address, instead of on the address alone. - A transport configured with `accept_connections` false still admits an inbound - msg1 whose source matches an established peer, so that a peer re-handshaking - after a restart or a rekey is not locked out, but nothing checked that the - party sourcing from that address was the peer. Any off-path party able to send - from it therefore obtained a full link handshake from a node configured to - accept none. Once the key exchange reveals the initiator's static key, the - handshake is now dropped unless that key belongs to the identity the matched - address is attributed to, and dropped as well when the waiver was used and no - identity owns the address at all, which fails closed rather than skipping the - check for that case. The cheap refusal is unchanged: a stranger reaching a - transport that refuses inbound connections is still turned away before any - cryptography. Attribution consults both the reverse-address lookup and the - scan over established peers rather than stopping at whichever answers first, - because the reverse lookup can name a link that no longer exists, and stopping - there would refuse a peer the scan can still attribute — permanently, since - the confirmation returns above the code that repairs that lookup. Both - refusals log at warning level, naming the expected and actual peers, and - charge the existing handshake bad-state rejection counter rather than one of - their own. +#### Supply chain -- An inbound frame whose header disagrees with the frame that arrived is now - dropped, at the single dispatch point every transport converges on, before the - declared length can be used as a parsing input. The 4-byte common prefix - carries a payload length that the node never read, so on the datagram - transports — UDP, Ethernet and BLE, which deliver one whole frame per packet - and where the arrived length is therefore known exactly — nothing compared the - two. This closes no known defect, and it is worth being exact about what it - drops on a deployed line. The stream transports (TCP, Tor, Nym) read their - frame boundary out of that same field, so the comparison holds by construction - and never fires for them. A short datagram is a truncated frame, which already - failed the AEAD tag or the exact-size handshake parse, so what changes there - is which reason it is dropped for rather than whether it is dropped. A frame - whose phase the node does not recognize carries no fixed relationship between - the two and is left alone rather than rejected on a guess. The drop takes its - own rejection reason and its own `payload_len_mismatch` counter rather than - reusing the admission one, since it is a framing rejection decided before the - phase dispatch and it applies to established data frames as well as to - handshakes; that counter is not yet readable through the control socket. +- 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 + yanked. `nostr` moves to 0.44.8 and `nostr-relay-pool` to 0.44.3; the + requirements in `Cargo.toml` already admitted both, so this is a lockfile + change and no code changed with it. The advisories that matter here are the + relay-pool ones, RUSTSEC-2026-0224 and RUSTSEC-2026-0232, which describe + forged events bypassing signature validation and unverified relay events + being processed: that is the path this 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 advisory summaries + lead with. RUSTSEC-2026-0231 (auth-challenge memory exhaustion) is on the + same path, and RUSTSEC-2026-0216 and RUSTSEC-2026-0227 reach the NIP-44 + decryption of relay-supplied content. The remaining advisories in that set + cover NIP-04, NIP-46, NIP-50, NIP-60, NIP-98 and the wallet parsers, none of + which this code calls. The refresh was taken over the whole lockfile rather + than the two crates alone, which additionally clears RUSTSEC-2026-0204 in + `crossbeam-epoch` and leaves no yanked crate in the tree; `cargo audit` now + reports no vulnerability, against twelve before. Four warnings remain and are + not fixable by a version move: `instant` and `paste` are unmaintained, `lru` + 0.16.4 carries an unsoundness advisory, and `nostr-relay-pool` itself is now + marked unmaintained. + +- Every GitHub Action is pinned to a commit SHA, and the OpenWrt packaging + workflow verifies both of the artifacts it downloads. No reference in the + repository was pinned before: all sixty-six named a mutable tag and one named + a branch, 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 now full commit SHAs with the original tag + retained as a trailing comment. Four are left unpinned and justified in one + place: two actions read the tool to install from the ref name itself, so a + SHA would hand them a hex string where a toolchain name belongs. A guard + enforces the form on every sweep, treats an unreadable tree as an error + rather than a pass, and documents what it does not cover. The sharper hole + was not the tags: the OpenWrt workflow fetched a helper binary from a release + URL with no verification at all, in two jobs holding a signing key. That + download now checks a per-architecture pinned SHA-256, with the hash + provenance recorded honestly, upstream publishing no checksum document. + + The same workflow's Zig toolchain fetch is verified the same way. It ran as + `curl | tar`, which leaves nowhere to check the bytes, so a short read + reached `tar` as a truncated archive and failed the build with "Unexpected + EOF in archive"; curl's `--retry` does not cover that exit. The download now + stages to a temporary directory, checks a per-architecture pinned SHA-256 + with a guard that fails if an architecture is added without one, and only + then extracts, with three attempts at 10s and 20s backoff and an early exit + when two attempts return identical bytes, since a stable mismatch is a wrong + pin rather than a bad transfer. As with the helper binary, the hashes come + from the upstream download index and were checked against the tarball bytes: + that is integrity, not authenticity, because upstream publishes no detached + sums. + +#### Docs & contributor tooling - The security reference now records that both Noise patterns deviate from the standard construction in one respect: the handshake AEAD passes an empty @@ -1382,8 +1740,8 @@ with v0.4.x or earlier peers. binding is the property actually absent. Nothing in the daemon reads the handshake hash, so no shipped behaviour rests on it, but the comments that called it transcript binding or channel binding overstated it and now describe - what the field is, and the field records that anything later built on it — - channel binding, an exporter, cookie binding — will silently not work until + what the field is, and the field records that anything later built on it + (channel binding, an exporter, cookie binding) will silently not work until the associated data carries `h`. This is a correction to what is documented and claimed; no code behaviour and nothing on the wire changed. diff --git a/README.md b/README.md index 8e4cb3e9..be739adf 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ ![banner](docs/logos/fips_banner.png) [![License: MIT](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE) -[![Rust](https://img.shields.io/badge/rust-1.85%2B-orange.svg)](https://www.rust-lang.org/) +[![Rust](https://img.shields.io/badge/rust-orange.svg)](https://www.rust-lang.org/) [![Status](https://img.shields.io/badge/status-v0.6.0--dev-green.svg)](#status--roadmap) A self-organizing encrypted mesh network built on Nostr identities, diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index a826276c..7d8b02be 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -20,8 +20,10 @@ locations, lowest to highest priority: All found files are loaded and merged in priority order. Values from higher priority files override those from lower priority files. This allows a system -administrator to set site-wide defaults in `/etc/fips/fips.yaml` while -individual deployments override specific values in `./fips.yaml`. +administrator to set site-wide defaults in the priority 1 path above, +`/usr/local/etc/fips/fips.yaml` on macOS and `/etc/fips/fips.yaml` on other +Unix systems, while individual deployments override specific values in +`./fips.yaml`. ### CLI Option diff --git a/docs/tutorials/persistent-identity.md b/docs/tutorials/persistent-identity.md index 86359217..7cefa5cb 100644 --- a/docs/tutorials/persistent-identity.md +++ b/docs/tutorials/persistent-identity.md @@ -28,6 +28,11 @@ The whole exercise should take about ten minutes. your stable nsec / npub ``` +The diagram shows the Linux layout. On macOS the same three files +live under `/usr/local/etc/fips/`; read +[Where these files live](#where-these-files-live) before running any +command below. + After this tutorial your node will have: - A keypair on disk that the daemon reuses across restarts. @@ -36,6 +41,28 @@ After this tutorial your node will have: - A clear understanding of which file holds the secret and how to keep it that way. +## Where these files live + +Every path in this tutorial is written in its Linux form. The macOS +package (`.pkg`) installs config and keys under +`/usr/local/etc/fips/` instead of `/etc/fips/`, so on macOS +substitute as you go: + +| Linux / other Unix | macOS | +| --- | --- | +| `/etc/fips/fips.yaml` | `/usr/local/etc/fips/fips.yaml` | +| `/etc/fips/fips.key` | `/usr/local/etc/fips/fips.key` | +| `/etc/fips/fips.pub` | `/usr/local/etc/fips/fips.pub` | + +`fipsctl keygen` writes to `/usr/local/etc/fips/` by default on +macOS. The daemon still probes `/etc/fips/fips.yaml` as a fallback, +so an existing install is not broken by an upgrade, but the macOS +packaging only installs files under `/usr/local/etc/fips/`. If a +macOS host already carries key files at the old `/etc/fips/` path, +the daemon uses the old key and warns rather than minting a new +identity; the migration recipe is in the +[how-to guide](../how-to/persistent-identity.md). + ## Why a stable identity matters In FIPS your Nostr keypair *is* your node's identity in the most @@ -68,7 +95,8 @@ The daemon supports two ways of holding that keypair: > identity unless you explicitly ask for one. > - *Persistent*: the daemon reads (or, on first start, > generates and writes) a keypair stored at -> `/etc/fips/fips.key`. The npub stays the same across +> `/etc/fips/fips.key` (`/usr/local/etc/fips/fips.key` on +> macOS). The npub stays the same across > restarts, reboots, and reinstalls as long as that file is > preserved. You take on the cost of protecting an on-disk > secret in exchange for being addressable by a stable name. @@ -117,9 +145,9 @@ Make a note of it. We expect this to change. ## Step 2: Enable persistent identity in the config -Open `/etc/fips/fips.yaml` and find the `node:` block. The -shipped default has the relevant fragment commented out; make it -look like this: +Open `/etc/fips/fips.yaml` (`/usr/local/etc/fips/fips.yaml` on +macOS) and find the `node:` block. The shipped default has the +relevant fragment commented out; make it look like this: ```yaml node: @@ -137,6 +165,9 @@ The daemon's behavior on the next restart: `/etc/fips/fips.{key,pub}` with the correct file modes, and use that. +The daemon derives the key directory from whichever config file it +loaded, so on macOS both files land in `/usr/local/etc/fips/`. + ## Step 3: Restart the daemon ```sh @@ -164,6 +195,9 @@ The daemon wrote two files: sudo ls -l /etc/fips/fips.key /etc/fips/fips.pub ``` +On macOS, list `/usr/local/etc/fips/fips.key` and +`/usr/local/etc/fips/fips.pub` instead. + Expect: ```text @@ -239,7 +273,7 @@ old one will be stale. persistent identity. - **Where it lives.** `/etc/fips/fips.key` and `/etc/fips/fips.pub`, mode `0600` and `0644`, owned - `root:root`. + `root:root`; under `/usr/local/etc/fips/` on macOS. - **What to share.** `fips.pub` is public; `fips.key` is not. - **What it buys you.** A npub other operators can add to their `peers:` list once, and that addresses the services your node @@ -250,11 +284,13 @@ old one will be stale. If the post-restart npub does not match `fips.pub`: - **Check file permissions.** - `sudo ls -l /etc/fips/fips.key`. If the mode is not `0600` or - the owner is not `root:root`, the daemon may have refused to - read it. Restore with + `sudo ls -l /etc/fips/fips.key`, or + `sudo ls -l /usr/local/etc/fips/fips.key` on macOS. If the mode + is not `0600` or the owner is not `root:root`, the daemon may + have refused to read it. Restore with `sudo chmod 0600 /etc/fips/fips.key && sudo chown root:root - /etc/fips/fips.key`. + /etc/fips/fips.key`, substituting the macOS path where it + applies. - **Check the journal.** `sudo journalctl -u fips -n 100` after the restart will show one of: - `Loaded persistent identity from key file path=...` — good. diff --git a/src/bin/fipsctl.rs b/src/bin/fipsctl.rs index 737dedee..8fb7fe3e 100644 --- a/src/bin/fipsctl.rs +++ b/src/bin/fipsctl.rs @@ -1088,6 +1088,15 @@ fn settled_text(name: &str, stage: &serde_json::Value) -> String { Some(n) => format!("no reply to {n} requests"), None => "no reply".to_string(), }, + "bloom_unconfirmed" => match stage.get("attempts").and_then(|v| v.as_u64()) { + Some(1) => { + "a peer filter claimed this address; 1 request went unanswered".to_string() + } + Some(n) => { + format!("a peer filter claimed this address; {n} requests went unanswered") + } + None => "a peer filter claimed this address; nothing answered for it".to_string(), + }, "already_pending" => "joined a lookup already in flight, which failed".to_string(), other => other.replace('_', " "), }, @@ -1287,9 +1296,34 @@ fn rtt_text(rtt: &serde_json::Value) -> String { /// The closing paragraph, which must not claim reachability it did not observe. fn overall_note(report: &serde_json::Value) -> Option<&'static str> { - if field(report, "overall") != "partial" { - return None; + match field(report, "overall") { + "partial" => partial_note(report), + "failed" => failed_note(report), + _ => None, } +} + +/// A failed probe's closing paragraph. Only the unconfirmed-claim case earns +/// one: it is the reading the operator cannot make from the reason alone, and +/// it is the one that otherwise reads as a network fault. +fn failed_note(report: &serde_json::Value) -> Option<&'static str> { + match field(report.get("discovery")?, "reason") { + "bloom_unconfirmed" => Some( + " A peer's bloom filter claimed this address and then no lookup\n\ + \x20 answered for it. A filter cannot miss a key that IS on the\n\ + \x20 mesh, so an absent address produces exactly this whenever some\n\ + \x20 peer's filter false-positives on it -- the same finding the\n\ + \x20 fast bloom_miss reports, reached the slow way. An address that\n\ + \x20 is present but whose lookups are being lost produces it too.\n\ + \x20 The wait says nothing about which; it is the ladder running to\n\ + \x20 the end.", + ), + _ => None, + } +} + +/// The partial-verdict paragraph, keyed on what the rtt stage settled on. +fn partial_note(report: &serde_json::Value) -> Option<&'static str> { match field(report.get("rtt")?, "reason") { "no_report" => Some( " The handshake completed, so our packets reached them. Nothing has\n\ @@ -1598,6 +1632,97 @@ mod tests { assert_eq!(rows[0].text, "no peer filter holds this address"); } + /// A finished probe of an address that is not on the mesh, under a given + /// gate outcome. `bloom_miss` is the run where no filter claimed it; + /// `bloom_unconfirmed` is the run where one did and no lookup answered. + fn absent_key_report(claimed: bool) -> serde_json::Value { + if claimed { + serde_json::json!({ + "overall": "failed", + "elapsed_ms": 17000, + "bloom": {"verdict": "ok", "reason": null, "elapsed_ms": 1000, "fanout": 1}, + "discovery": {"verdict": "failed", "reason": "bloom_unconfirmed", + "elapsed_ms": 16000, "attempts": 4, + "attempt_timeouts_secs": [1, 2, 4, 8]}, + "path": {"verdict": "skipped", "reason": "not_reached"}, + "session": {"verdict": "skipped", "reason": "not_reached"}, + "rtt": {"verdict": "skipped", "reason": "not_reached"}, + }) + } else { + serde_json::json!({ + "overall": "failed", + "elapsed_ms": 1900, + "bloom": {"verdict": "failed", "reason": "bloom_miss", "elapsed_ms": 1900, + "fanout": null}, + "discovery": {"verdict": "skipped", "reason": "not_reached"}, + "path": {"verdict": "skipped", "reason": "not_reached"}, + "session": {"verdict": "skipped", "reason": "not_reached"}, + "rtt": {"verdict": "skipped", "reason": "not_reached"}, + }) + } + } + + #[test] + fn the_two_absent_key_paths_read_differently_to_the_operator() { + let clean = absent_key_report(false); + let claimed = absent_key_report(true); + + let clean_rows = stage_rows(&clean); + let clean_text = clean_rows[row_at(&clean_rows, "bloom")].text.clone(); + let claimed_rows = stage_rows(&claimed); + let claimed_text = claimed_rows[row_at(&claimed_rows, "discovery")] + .text + .clone(); + + assert_eq!(clean_text, "no peer filter holds this address"); + assert_ne!( + clean_text, claimed_text, + "the two paths must not print the same line" + ); + assert!( + claimed_text.contains("claimed this address"), + "the slow line must say a filter claimed it: {claimed_text}" + ); + assert!( + claimed_text.contains('4'), + "and how many requests went unanswered: {claimed_text}" + ); + + // The line `no_response` used to print. It says nothing about why a + // request went out, which is the whole finding here. + assert_ne!( + claimed_text, "no reply to 4 requests", + "the claimed path must not fall back to the bare no-reply wording" + ); + + // The closing paragraph is where the operator is told what the wait + // meant. Only the claimed path earns one. + let note = overall_note(&claimed).unwrap_or_default(); + assert!( + note.contains("false-positive"), + "the note must name the mechanism: {note}" + ); + assert!( + note.contains("bloom_miss"), + "and tie it to the fast answer: {note}" + ); + assert!(overall_note(&clean).is_none()); + } + + #[test] + fn a_bare_no_response_still_reads_as_a_missing_reply() { + // The reason survives for the case it still describes: the gate's + // answer never arrived, so no claim was ever made. + let mut report = absent_key_report(true); + report["discovery"]["reason"] = serde_json::json!("no_response"); + let rows = stage_rows(&report); + assert_eq!( + rows[row_at(&rows, "discovery")].text, + "no reply to 4 requests" + ); + assert!(overall_note(&report).is_none()); + } + #[test] fn a_failed_path_keeps_the_stages_that_ran_after_it() { // The path preview names no hop and the session succeeds anyway, diff --git a/src/node/dataplane/peer_actions.rs b/src/node/dataplane/peer_actions.rs index 53b254b7..4794cc42 100644 --- a/src/node/dataplane/peer_actions.rs +++ b/src/node/dataplane/peer_actions.rs @@ -483,7 +483,7 @@ impl Node { handler and must never reach the executor" ); } - PeerAction::SwapSendState { .. } => { + PeerAction::SwapSendState => { // Initiator cutover: the live authoritative rekey-cadence // path, routed here from `check_rekey` via // `route_rekey_cadence` → `PeerEvent::RekeyConsume`; the diff --git a/src/peer/machine.rs b/src/peer/machine.rs index c9d42ecb..f3bef93a 100644 --- a/src/peer/machine.rs +++ b/src/peer/machine.rs @@ -448,8 +448,9 @@ pub(crate) enum PeerAction { /// resolution; it must never reach the action executor. ResolveCrossConnection { swap: bool }, /// Initiator-side rekey cutover: swap the published send-state to the pending - /// epoch. - SwapSendState { epoch: [u8; 8] }, + /// epoch. The remote epoch is not carried here: `conn` is its sole carrier + /// and promotion reads it from there via `conn_remote_epoch`. + SwapSendState, /// Complete an initiator-side rekey drain: retire the previous session slot /// (drop its `peers_by_index`/decrypt-worker entry, free its index). The /// executor reads the REAL previous index from `ActivePeer::complete_drain` @@ -576,8 +577,6 @@ pub(crate) struct PeerMachine { /// Pure handshake-phase bookkeeping (link/direction/indices/transport/ /// stored handshake bytes/epoch). Reused verbatim from the FMP state core. conn: ConnectionState, - /// Remote startup epoch (establish-path-only; NOT in send-state). - remote_epoch: Option<[u8; 8]>, /// A stored-handshake send failure was observed on this leg. The failure is /// carried as a flag (not a `PeerState::Failed` transition) so retransmit /// eligibility (`is_handshaking_sent_msg1`) survives until the @@ -635,7 +634,6 @@ impl PeerMachine { Some(id) => ConnectionState::outbound(link, id, now), None => ConnectionState::outbound_anonymous(link, now), }, - remote_epoch: None, send_failed: false, rekey_in_progress: false, rekey_our_index: None, @@ -663,7 +661,6 @@ impl PeerMachine { node_addr: None, leg: None, conn: ConnectionState::inbound(link, now), - remote_epoch: None, send_failed: false, rekey_in_progress: false, rekey_our_index: None, @@ -739,7 +736,6 @@ impl PeerMachine { node_addr: Some(addr), leg: None, conn, - remote_epoch, send_failed: false, rekey_in_progress: false, rekey_our_index: None, @@ -1578,7 +1574,6 @@ impl PeerMachine { // Identity crystallizes at msg3 on XX (WireOutcome carries only the node // address + epoch; the full static key stays shell-side). self.node_addr = Some(wire.peer_node_addr); - self.remote_epoch = wire.remote_epoch; // Seed the leg's index from the event. On a fresh classification machine // the index was allocated at msg1 shell-side and is not otherwise known // here; the cross-connection and rekey-responder decisions read it back to @@ -1887,9 +1882,7 @@ impl PeerMachine { addr: peer, kind: MaintainKind::Rekey(RekeyPhase::Draining), }; - let mut actions = vec![PeerAction::SwapSendState { - epoch: self.remote_epoch.unwrap_or_default(), - }]; + let mut actions = vec![PeerAction::SwapSendState]; if let Some(idx) = self.conn.our_index() { actions.push(PeerAction::RegisterDecryptSession { index: idx }); } @@ -2527,7 +2520,7 @@ mod tests { link: LinkId::new(7), }, PeerAction::ResolveCrossConnection { swap: true }, - PeerAction::SwapSendState { epoch: [1u8; 8] }, + PeerAction::SwapSendState, PeerAction::CompleteDrain { peer }, PeerAction::InvalidateSendState, PeerAction::RegisterDecryptSession { @@ -2573,7 +2566,7 @@ mod tests { | PeerAction::SendLinkMessage { .. } | PeerAction::PromoteToActive { .. } | PeerAction::ResolveCrossConnection { .. } - | PeerAction::SwapSendState { .. } + | PeerAction::SwapSendState | PeerAction::CompleteDrain { .. } | PeerAction::InvalidateSendState | PeerAction::RegisterDecryptSession { .. } @@ -2627,7 +2620,12 @@ mod tests { }; m.rekey_our_index = Some(SessionIndex::new(0x2222)); m.conn.set_our_index(SessionIndex::new(0x1111)); - m.remote_epoch = Some([9u8; 8]); + // The remote startup epoch lives on the surviving carrier, written + // there by BOTH handshake legs (`complete_handshake` from msg2 on the + // outbound leg, `complete_handshake_msg3` from msg3 on the inbound one) + // through this same setter. Seed it the way production does, so the + // value asserted below is one an outbound machine can actually hold. + m.conn.set_remote_epoch(Some([9u8; 8])); m.session_established_at_ms = 0; let actions = m.step( @@ -2641,7 +2639,7 @@ mod tests { assert_eq!( actions, vec![ - PeerAction::SwapSendState { epoch: [9u8; 8] }, + PeerAction::SwapSendState, PeerAction::RegisterDecryptSession { index: SessionIndex::new(0x2222) }, @@ -2673,6 +2671,9 @@ mod tests { vec![PeerAction::CompleteDrain { peer: addr }] ); assert_eq!(m.state(), PeerState::Active { addr }); + // The cutover carries no epoch of its own; `conn` is the sole carrier + // and the cutover must leave it exactly as the handshake wrote it. + assert_eq!(m.conn_remote_epoch(), Some([9u8; 8])); } // ---- Test 2: responder cutover (data-plane owned) --------------------- @@ -2701,7 +2702,7 @@ mod tests { assert!( !actions .iter() - .any(|a| matches!(a, PeerAction::SwapSendState { .. })) + .any(|a| matches!(a, PeerAction::SwapSendState)) ); assert_eq!(m.state(), PeerState::Active { addr }); } @@ -3672,7 +3673,8 @@ mod tests { }; m.rekey_our_index = Some(SessionIndex::new(0x2222)); m.conn.set_our_index(SessionIndex::new(0x1111)); - m.remote_epoch = Some([9u8; 8]); + // Seeded through the setter both handshake legs use; see Test 1. + m.conn.set_remote_epoch(Some([9u8; 8])); // Consume the shell-decided Cutover. let cut = m.step( @@ -3685,7 +3687,7 @@ mod tests { assert_eq!( cut, vec![ - PeerAction::SwapSendState { epoch: [9u8; 8] }, + PeerAction::SwapSendState, PeerAction::RegisterDecryptSession { index: SessionIndex::new(0x2222) }, @@ -4045,7 +4047,6 @@ mod tests { }; m.rekey_our_index = Some(SessionIndex::new(0x2222)); m.conn.set_our_index(SessionIndex::new(0x1111)); - m.remote_epoch = Some([9u8; 8]); m.session_established_at_ms = 0; let actions = m.step( diff --git a/src/proto/probe/core.rs b/src/proto/probe/core.rs index a6532fd3..7d26e4f9 100644 --- a/src/proto/probe/core.rs +++ b/src/proto/probe/core.rs @@ -408,12 +408,23 @@ impl Probe { let gave_up = self.lookup_was_pending && !obs.lookup_pending; if gave_up || obs.now_ms.saturating_sub(self.stage_started_ms) >= self.budgets.discovery_ms { - let reason = if self.lookup_outcome == Some(LookupOutcomeKind::Deduplicated) { - FailKind::AlreadyPending - } else { - FailKind::NoResponse - }; - self.fail_stage(reason, obs); + self.fail_stage(self.discovery_fail_kind(), obs); + } + } + + /// Which "the lookup did not answer" finding this probe has earned. + /// + /// The gate proceeds only when some peer's filter claimed the target, so + /// a `Sent` lookup that goes unanswered is a claim nobody could confirm. + /// Reporting that as a bare `NoResponse` reads as a network fault and + /// hides the one fact the resolver does know: a filter put the key on the + /// mesh and the mesh disagreed. A joined lookup is its own finding again, + /// because this probe never chose to issue it. + fn discovery_fail_kind(&self) -> FailKind { + match self.lookup_outcome { + Some(LookupOutcomeKind::Deduplicated) => FailKind::AlreadyPending, + Some(LookupOutcomeKind::Sent) => FailKind::BloomUnconfirmed, + _ => FailKind::NoResponse, } } @@ -644,7 +655,8 @@ impl Probe { /// the reason its own budget would have given. fn expire_running_stage(&mut self) { let kind = match self.stage { - Stage::Bloom | Stage::Discovery => FailKind::NoResponse, + Stage::Bloom => FailKind::NoResponse, + Stage::Discovery => self.discovery_fail_kind(), Stage::Path => FailKind::NoNextHop, Stage::Session => { if self.owns_session { diff --git a/src/proto/probe/state.rs b/src/proto/probe/state.rs index 0f6ebe1d..80cd47f7 100644 --- a/src/proto/probe/state.rs +++ b/src/proto/probe/state.rs @@ -64,6 +64,12 @@ pub(crate) enum FailKind { NoTreePeers, /// discovery: the attempt ladder was exhausted, or the budget expired. NoResponse, + /// discovery: the gate proceeded on a peer filter's claim and the lookup + /// was never answered, so the claim was never confirmed. Distinct from + /// `NoResponse` because the reason a request went out at all is part of + /// the finding: a filter cannot miss a key that is present, so on an + /// absent key this is that filter false-positiving. + BloomUnconfirmed, /// path: the two coordinates have different spanning-tree roots. DisjointTrees, /// path: no send-ready peer is strictly closer to the target. @@ -96,6 +102,7 @@ impl FailKind { FailKind::AlreadyPending => "already_pending", FailKind::NoTreePeers => "no_tree_peers", FailKind::NoResponse => "no_response", + FailKind::BloomUnconfirmed => "bloom_unconfirmed", FailKind::DisjointTrees => "disjoint_trees", FailKind::NoNextHop => "no_next_hop", FailKind::Preexisting => "preexisting", diff --git a/src/proto/probe/tests/core.rs b/src/proto/probe/tests/core.rs index cc883665..962b2a1b 100644 --- a/src/proto/probe/tests/core.rs +++ b/src/proto/probe/tests/core.rs @@ -4,8 +4,8 @@ use crate::NodeAddr; use crate::proto::probe::core::{Budgets, Observation, Probe, ProbeAction}; use crate::proto::probe::state::{ - FailKind, LeftIntact, LookupOutcomeKind, NextHopFacts, PathFacts, Preflight, ResolveSource, - RttCounters, StageVerdict, + FailKind, LeftIntact, LookupOutcomeKind, NextHopFacts, Overall, PathFacts, Preflight, + ResolveSource, RttCounters, StageVerdict, }; use crate::proto::routing::RouteClass; @@ -166,7 +166,7 @@ fn discovery_times_out_at_its_own_budget_measured_from_the_request() { assert_eq!(probe.step(&o), vec![ProbeAction::Finish]); let snap = probe.snapshot(); assert_eq!(snap.discovery.verdict, StageVerdict::Failed); - assert_eq!(snap.discovery.reason, Some(FailKind::NoResponse)); + assert_eq!(snap.discovery.reason, Some(FailKind::BloomUnconfirmed)); assert_eq!( snap.bloom.verdict, StageVerdict::Ok, @@ -198,7 +198,7 @@ fn discovery_fails_early_when_the_pending_entry_clears() { assert_eq!(probe.step(&o), vec![ProbeAction::Finish]); assert_eq!( probe.snapshot().discovery.reason, - Some(FailKind::NoResponse) + Some(FailKind::BloomUnconfirmed) ); assert!(T0 + 2_000 < T0 + b.discovery_ms); } @@ -250,6 +250,111 @@ fn each_gate_decision_lands_on_the_stage_that_owns_it() { } } +/// Drive a probe for a key that is **not** on the mesh under a given gate +/// decision, and return the finished snapshot. +/// +/// `BloomMiss` is the run where no peer's filter claimed the key. `Sent` is +/// the run where at least one did — which, for a genuinely absent key, can +/// only be a false positive, since a bloom filter has no false negatives. +/// The mesh then answers nothing and the ladder runs out. +fn absent_key_probe(gate: LookupOutcomeKind) -> crate::proto::probe::ProbeSnapshot { + let mut pre = preflight(); + pre.coords_cached = false; + let mut probe = Probe::new(T0, budgets(), pre); + let claimed = gate == LookupOutcomeKind::Sent; + + for tick in 0..40u64 { + let mut o = obs(T0 + tick * 1_000); + o.coords_cached = false; + if tick > 0 { + o.lookup_outcome = Some(gate); + } + // The claimed run holds a pending entry while the ladder retries, then + // clears it with no coordinates: nobody answered for the address. + o.lookup_pending = claimed && tick < 16; + if probe.step(&o).contains(&ProbeAction::Finish) { + break; + } + } + probe.snapshot() +} + +#[test] +fn absent_key_names_the_filter_claim_instead_of_collapsing_into_no_response() { + let clean = absent_key_probe(LookupOutcomeKind::BloomMiss); + let claimed = absent_key_probe(LookupOutcomeKind::Sent); + + assert_eq!( + clean.bloom.verdict, + StageVerdict::Failed, + "no filter claimed it, so the bloom stage is where it ended" + ); + assert_eq!( + claimed.bloom.verdict, + StageVerdict::Ok, + "a filter claimed it, so a request did go out" + ); + + assert_eq!(clean.bloom.reason, Some(FailKind::BloomMiss)); + assert_eq!( + claimed.discovery.reason, + Some(FailKind::BloomUnconfirmed), + "a lookup issued on a filter claim and left unanswered is its own finding" + ); + + // The defect this guards: the claimed run must not land on any reason a + // run that never issued a request can also produce. `no_response` was + // exactly such a reason — the bloom stage reaches it too — so reporting + // it here told the operator nothing about which case they were in. + for shared in [ + FailKind::BloomMiss, + FailKind::NoResponse, + FailKind::BackoffSuppressed, + FailKind::NoTreePeers, + ] { + assert_ne!( + claimed.discovery.reason, + Some(shared), + "{} cannot distinguish a claimed lookup from one never issued", + shared.name() + ); + } + + // The verdict is the answer and the answer was already right: absent + // either way. Only the reason changes. + assert_eq!(clean.overall, Overall::Failed); + assert_eq!( + clean.overall, claimed.overall, + "the key is absent on both paths; the verdict must not move" + ); +} + +#[test] +fn cancelling_mid_discovery_still_names_the_filter_claim() { + // `expire_running_stage` writes the reason for a stage that never got to + // settle. It must give the same finding the budget path would. + let mut pre = preflight(); + pre.coords_cached = false; + let mut probe = Probe::new(T0, budgets(), pre); + + let mut o = obs(T0); + o.coords_cached = false; + probe.step(&o); + + let mut o = obs(T0 + 1_000); + o.coords_cached = false; + o.lookup_pending = true; + o.lookup_outcome = Some(LookupOutcomeKind::Sent); + probe.step(&o); + assert_eq!(probe.snapshot().discovery.verdict, StageVerdict::Running); + + probe.cancel(T0 + 2_000); + assert_eq!( + probe.snapshot().discovery.reason, + Some(FailKind::BloomUnconfirmed) + ); +} + // ---- ownership ---------------------------------------------------------- #[test] diff --git a/src/transport/nym/mod.rs b/src/transport/nym/mod.rs index 5a34b26b..c465ab56 100644 --- a/src/transport/nym/mod.rs +++ b/src/transport/nym/mod.rs @@ -642,6 +642,15 @@ fn parse_target_addr(addr: &TransportAddr) -> Result( mut reader: OwnedReadHalf, @@ -151,6 +159,7 @@ pub(crate) async fn proxied_receive_loop( mtu: u16, stats: Arc, label: &'static str, + first_frame_timeout: Option, on_remove: impl Fn(&S, &M), ) { debug!( @@ -160,8 +169,34 @@ pub(crate) async fn proxied_receive_loop( label ); + let mut first = true; loop { - match read_fmp_packet(&mut reader, mtu).await { + let read = match first_frame_timeout { + // Bound the first read only. A silent remote otherwise holds its + // inbound slot for as long as it keeps the socket open. + Some(d) if first => { + match tokio::time::timeout(d, read_fmp_packet(&mut reader, mtu)).await { + Ok(result) => result, + Err(_) => { + // Not a recv error: `record_recv_error` means framing + // or I/O failure, and folding deadline expiries into + // it corrupts that counter. + debug!( + transport_id = %transport_id, + remote_addr = %remote_addr, + timeout_secs = d.as_secs_f64(), + "No complete frame within the first-frame deadline, dropping inbound {} connection", + label + ); + break; + } + } + } + _ => read_fmp_packet(&mut reader, mtu).await, + }; + first = false; + + match read { Ok(data) => { stats.record_recv(data.len()); diff --git a/src/transport/tor/mod.rs b/src/transport/tor/mod.rs index 483a2a93..9e213c7c 100644 --- a/src/transport/tor/mod.rs +++ b/src/transport/tor/mod.rs @@ -33,6 +33,7 @@ use crate::transport::socks5::{ ConnectingEntry, ConnectingPool, DialError, ProxiedConnection, ProxiedPool, Socks5Auth, Socks5Dialer, SocksTarget, poll_connecting, proxied_receive_loop, }; +use crate::transport::tcp::INBOUND_FIRST_FRAME_TIMEOUT; use control::{ControlAuth, TorControlClient, TorMonitoringInfo}; use stats::TorStats; @@ -367,6 +368,7 @@ impl TorTransport { pool, mtu, max_inbound, + INBOUND_FIRST_FRAME_TIMEOUT, stats, ) .await; @@ -769,6 +771,7 @@ impl TorTransport { mtu, recv_stats, Direction::Outbound, + None, ) .await; }); @@ -934,6 +937,7 @@ impl TorTransport { mtu, recv_stats, Direction::Outbound, + None, ) .await; }); @@ -1046,6 +1050,10 @@ impl Transport for TorTransport { /// actually removing it (so a concurrent close/stop never drives the counter /// below zero). `direction` is retained for the terminal "receive loop /// stopped" debug field the shared loop deliberately leaves to each transport. +/// +/// `first_frame_timeout` is `Some` for an inbound connection, which holds a +/// capped pool slot from the moment it is accepted, and `None` for an +/// outbound one, which holds no such slot. #[allow(clippy::too_many_arguments)] async fn tor_receive_loop( reader: tokio::net::tcp::OwnedReadHalf, @@ -1056,6 +1064,7 @@ async fn tor_receive_loop( mtu: u16, stats: Arc, direction: Direction, + first_frame_timeout: Option, ) { proxied_receive_loop( reader, @@ -1066,6 +1075,7 @@ async fn tor_receive_loop( mtu, stats, "Tor", + first_frame_timeout, |stats, meta| match meta { Direction::Inbound => stats.record_pool_inbound_removed(), Direction::Outbound => stats.record_pool_outbound_removed(), @@ -1091,6 +1101,13 @@ async fn tor_receive_loop( /// connections to a local TCP listener; we accept them, configure /// socket options, split the stream, and spawn a per-connection /// receive task. +/// +/// `first_frame_timeout` is the deadline from accept to the first complete +/// inbound frame, handed to each spawned receive loop. An accepted socket +/// takes an inbound slot against `max_inbound` before any byte is read, so +/// without it a remote that connects and stays silent holds that slot for as +/// long as it keeps the socket open. +#[allow(clippy::too_many_arguments)] async fn tor_accept_loop( listener: TcpListener, transport_id: TransportId, @@ -1098,6 +1115,7 @@ async fn tor_accept_loop( pool: ProxiedPool, mtu: u16, max_inbound: usize, + first_frame_timeout: Duration, stats: Arc, ) { debug!( @@ -1185,6 +1203,7 @@ async fn tor_accept_loop( mtu, recv_stats, Direction::Inbound, + Some(first_frame_timeout), ) .await; }); @@ -1893,4 +1912,143 @@ mod tests { let err = format!("{}", result.unwrap_err()); assert!(err.contains("directory")); } + + // ======================================================================== + // Inbound first-frame deadline (onion listener) + // ======================================================================== + + /// Poll `f` every 10ms until it holds or `limit` elapses. + async fn wait_until bool>(mut f: F, limit: Duration) -> bool { + let deadline = Instant::now() + limit; + loop { + if f() { + return true; + } + if Instant::now() >= deadline { + return false; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + /// Drives `tor_accept_loop` directly: the only production path to it is + /// `start_directory_mode`, which needs a Tor-managed hostname file and a + /// running daemon, so it is not reachable from a unit test. + fn spawn_onion_accept_loop( + listener: TcpListener, + packet_tx: PacketTx, + first_frame_timeout: Duration, + ) -> (ProxiedPool, Arc, JoinHandle<()>) { + let pool: ProxiedPool = Arc::new(Mutex::new(HashMap::new())); + let stats = Arc::new(TorStats::new()); + let handle = tokio::spawn(tor_accept_loop( + listener, + TransportId::new(1), + packet_tx, + pool.clone(), + 1400, + 64, + first_frame_timeout, + stats.clone(), + )); + (pool, stats, handle) + } + + /// Mirror of the TCP case: a silent onion-side socket must lose its + /// inbound slot at the deadline. Break-check: with the timeout wrapper + /// removed from the shared loop the count stays at 1 and the second + /// assertion fails. + #[tokio::test] + async fn idle_inbound_onion_socket_releases_its_slot() { + let (tx, _rx) = packet_channel(32); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let listen = listener.local_addr().unwrap(); + let (pool, stats, accept) = + spawn_onion_accept_loop(listener, tx, Duration::from_millis(200)); + + // Held open for the whole test: any release is the deadline's doing. + let squatter = TcpStream::connect(listen).await.unwrap(); + + assert!( + wait_until(|| stats.pool_inbound_count() == 1, Duration::from_secs(2)).await, + "an accepted onion socket should take an inbound slot" + ); + assert!( + wait_until(|| stats.pool_inbound_count() == 0, Duration::from_secs(2)).await, + "a silent onion socket should lose its slot at the first-frame deadline" + ); + assert!(pool.lock().await.is_empty()); + + drop(squatter); + accept.abort(); + } + + /// The deadline covers a *complete* first frame, not merely the first + /// byte: a remote that dribbles a prefix inside the deadline and the + /// remainder after it must still lose its slot, and the late frame must + /// not be delivered. + #[tokio::test] + async fn byte_dripped_first_onion_frame_past_deadline_is_dropped() { + let (tx, mut rx) = packet_channel(32); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let listen = listener.local_addr().unwrap(); + let (_pool, stats, accept) = + spawn_onion_accept_loop(listener, tx, Duration::from_millis(300)); + + let frame = build_msg1_frame(); + let mut peer = TcpStream::connect(listen).await.unwrap(); + // Prefix inside the deadline, remainder well past it. + peer.write_all(&frame[..4]).await.unwrap(); + tokio::time::sleep(Duration::from_millis(600)).await; + let _ = peer.write_all(&frame[4..]).await; + + assert!( + tokio::time::timeout(Duration::from_millis(500), rx.recv()) + .await + .is_err(), + "a first onion frame completing after the deadline must not be delivered" + ); + assert!( + wait_until(|| stats.pool_inbound_count() == 0, Duration::from_secs(2)).await, + "the dripped onion connection should have released its slot" + ); + + drop(peer); + accept.abort(); + } + + /// The healthy path, and a regression guard as for TCP: the deadline is + /// scoped to the first iteration, so an established onion connection that + /// then goes quiet keeps its slot. It exists so a future general idle + /// deadline cannot start reaping quiet onion links without a test going + /// red. + #[tokio::test] + async fn established_onion_connection_survives_long_idle() { + let (tx, mut rx) = packet_channel(32); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let listen = listener.local_addr().unwrap(); + let (pool, stats, accept) = + spawn_onion_accept_loop(listener, tx, Duration::from_millis(200)); + + let mut peer = TcpStream::connect(listen).await.unwrap(); + peer.write_all(&build_msg1_frame()).await.unwrap(); + let packet = tokio::time::timeout(Duration::from_secs(2), rx.recv()) + .await + .expect("timeout") + .expect("packet channel closed"); + assert_eq!(packet.data, build_msg1_frame()); + + // Four deadlines' worth of silence after the first frame. + tokio::time::sleep(Duration::from_millis(800)).await; + + assert_eq!( + stats.pool_inbound_count(), + 1, + "an established onion connection must not be dropped by the first-frame deadline" + ); + assert!(!pool.lock().await.is_empty()); + + drop(peer); + accept.abort(); + } } diff --git a/testing/README.md b/testing/README.md index f7d35e3d..0c2e143f 100644 --- a/testing/README.md +++ b/testing/README.md @@ -45,8 +45,8 @@ and a local STUN responder. Automated network testing with configurable node counts, topology algorithms (random geometric, Erdos-Renyi, chain, explicit), and fault injection (netem mutation, link flaps, traffic generation, node -churn). 20 scenarios covering general stress testing, cost-based parent -selection, mixed link technologies (fiber/Bluetooth/WiFi), +churn). 10 scenarios covering general stress and node churn, discovery +over sparse topologies, spanning-tree and bloom-propagation regression, transport-specific validation (UDP, TCP, Ethernet), and ECN/congestion testing. Scenarios are defined in YAML and executed via a Python harness that manages the full diff --git a/testing/chaos/README.md b/testing/chaos/README.md index 994ae382..ec9a9341 100644 --- a/testing/chaos/README.md +++ b/testing/chaos/README.md @@ -3,10 +3,11 @@ Automated network testing for FIPS. Generates random or explicit topologies, spins up Docker containers, and applies configurable stressors (network impairment, link flaps, traffic generation, node -churn) over a timed simulation run. Scenarios cover general stress -testing, cost-based parent selection, mixed link technologies -(fiber/Bluetooth/WiFi), and transport-specific validation (UDP, TCP, -Ethernet). Logs are collected and analyzed automatically. +churn) over a timed simulation run. Scenarios cover general stress and +node churn, discovery over sparse topologies, spanning-tree and +bloom-propagation regression, transport-specific validation (UDP, TCP, +Ethernet), and ECN/congestion testing. Logs are collected and analyzed +automatically. ## Prerequisites @@ -23,24 +24,56 @@ Ethernet). Logs are collected and analyzed automatically. ## Available Scenarios -### General stress tests +### General stress and churn -Random topologies with increasing stressor intensity. +Random topologies with increasing stressor intensity. All three enable +netem mutation, link flaps, iperf traffic, node churn and bandwidth +tiers, and differ in transport mix, density, and whether the peer set +itself churns. Each takes a `--nodes N` override, so the node counts +below are defaults rather than fixed sizes. -| Scenario | Nodes | Topology | Duration | Netem | Link Flaps | Traffic | Node Churn | Bandwidth | -| -------- | ----- | ---------------- | -------- | ----- | ---------- | ------- | ---------- | --------- | -| chaos-10 | 10 | random_geometric | 120s | yes | yes | yes | -- | -- | -| churn-10 | 10 | random_geometric | 600s | yes | yes | yes | yes | -- | -| churn-20 | 20 | erdos_renyi | 600s | yes | yes | yes | yes | yes | +| Scenario | Nodes | Topology | Duration | Peer churn | +| ---------------- | ----- | ---------------- | -------- | ---------- | +| churn-mixed | 20 | erdos_renyi | 600s | -- | +| maelstrom | 20 | erdos_renyi | 600s | yes | +| maelstrom-sparse | 50 | random_geometric | 600s | yes | -- **chaos-10**: Network degradation (5-50ms delay, 0-2% loss), link flaps (max 2 - down, 10-30s), and iperf traffic (max 3 concurrent). Netem mutates 30% of - links every 15-30s between normal and degraded policies. -- **churn-10**: Extended run with node churn (1 node down at a time, 30-90s). - Tests tree re-convergence after node departure/rejoin. -- **churn-20**: Aggressive scale test. Erdos-Renyi topology, up to 5 nodes down - simultaneously, bandwidth tiers (1/10/100/1000 Mbps), `protect_connectivity` - disabled (partitions allowed). +- **churn-mixed**: Mixed transports on one mesh (60% UDP, 20% Ethernet, 20% + TCP). Netem mutates 30% of links every 20-45s between normal and degraded + policies; link flaps (max 3 down, 10-30s, connectivity protected); node + churn (max 5 down, 30-90s, partitions allowed); bandwidth tiers + (1/10/100/1000 Mbps). Carries baseline assertions, so a run in which the + mesh never formed cannot report success. Local CI runs it as + `churn-mixed --nodes 10 --duration 120`, which is the invocation its + thresholds are calibrated for. +- **maelstrom**: The same stressors plus peer-level topology mutation + (connect/disconnect every 8-12s) and ephemeral identities on half the + nodes, with `coord_ttl_secs: 10` so coordinate cache entries expire during + the run. Tests re-convergence when the peer set and the identities behind + it both move. +- **maelstrom-sparse**: 50-node sparse random geometric graph (radius 0.20, + roughly 3-4 peers per node), which forces multi-hop routing and heavy + discovery use. The short coordinate TTL expires transit-warmed entries, so + nodes must rediscover rather than coast on the cache. + +### Spanning-tree and bloom propagation + +Explicit topology with an induced parent flap. **No runner invokes this +scenario.** It was retired from both the local and the cloud runner and is +hand-run only; the files remain in the tree and the retirement is recorded as a +coverage gap rather than as a migration to other tests. + +| Scenario | Nodes | Topology | Duration | What it tests | +| ----------- | ----- | -------- | -------- | ------------------------------- | +| bloom-storm | 6 | explicit | 180s | Bloom rate under sustained flap | + +- **bloom-storm**: Six-node depth-4 mesh. The two candidate uplinks at depth 2 + swap netem delay (5ms against 100ms) every 4s with parent-flap dampening + disabled, so the node switches parents each round. Asserts a ceiling on the + `stats.bloom.sent` delta per node over the trailing 30s, and a floor of 10 + parent switches so a harness that never produced a real switch cannot pass + trivially. `scenarios/bloom-storm.README.md` carries the bug-class + description and the threshold derivation. ### Cost-based parent selection — retired, now sans-IO unit tests @@ -65,7 +98,7 @@ Explicit topologies exercising non-UDP transports. | Scenario | Nodes | Transport | Shape | Duration | Netem | Link Flaps | What it tests | | ------------- | ----- | -------------- | ----- | -------- | ----- | ---------- | ------------------------------------------ | -| ethernet-only | 4 | Ethernet | Ring | 90s | yes | -- | AF_PACKET transport with beacon discovery | +| ethernet-only | 4 | Ethernet | Ring | 30s | yes | -- | AF_PACKET transport with beacon discovery | | ethernet-mesh | 6 | UDP + Ethernet | Mesh | 120s | yes | yes | Mixed UDP/Ethernet, netem mutation + flaps | | tcp-mesh | 6 | UDP + TCP | Mesh | 120s | yes | yes | Mixed UDP/TCP, netem mutation + flaps | @@ -137,27 +170,32 @@ scenario runs. | `--duration secs` | Override the scenario's duration | | `--list` | List available scenarios | -The scenario argument accepts either a name (`churn-10`) or a file -path (`scenarios/churn-10.yaml`). +The scenario argument accepts either a name (`churn-mixed`) or a file +path (`scenarios/churn-mixed.yaml`). `--list` prints the names that +resolve. ## Scenario YAML Format -Annotated example based on `churn-10.yaml`: +Annotated example based on `churn-mixed.yaml`: ```yaml scenario: - name: "churn-10" + name: "churn-mixed" seed: 42 # deterministic RNG seed duration_secs: 600 # total simulation time topology: - num_nodes: 10 - algorithm: random_geometric # or erdos_renyi, chain + num_nodes: 20 + algorithm: erdos_renyi # or random_geometric, chain, explicit params: - radius: 0.5 # algorithm-specific parameter + p: 0.3 # algorithm-specific parameter ensure_connected: true # retry until graph is connected - subnet: "172.20.0.0/24" + subnet: "172.20.0.0/16" ip_start: 10 # first node gets .10 + transport_mix: # fraction of edges per transport + udp: 0.6 + ethernet: 0.2 + tcp: 0.2 netem: enabled: true @@ -180,33 +218,44 @@ netem: link_flaps: enabled: true interval_secs: { min: 30, max: 60 } - max_down_links: 2 + max_down_links: 3 down_duration_secs: { min: 10, max: 30 } protect_connectivity: true # never partition the graph traffic: enabled: true - max_concurrent: 3 - interval_secs: { min: 10, max: 30 } - duration_secs: { min: 5, max: 15 } + max_concurrent: 10 + interval_secs: { min: 0, max: 30 } + duration_secs: { min: 5, max: 90 } parallel_streams: 4 node_churn: enabled: true - interval_secs: { min: 60, max: 180 } - max_down_nodes: 1 + interval_secs: { min: 60, max: 90 } + max_down_nodes: 5 down_duration_secs: { min: 30, max: 90 } - protect_connectivity: true # never kill the last path + protect_connectivity: false # partitions allowed bandwidth: - enabled: false # per-link HTB rate limiting + enabled: true # per-link HTB rate limiting tiers_mbps: [1, 10, 100, 1000] # each link randomly assigned a tier +assertions: # evaluated after the run + baseline: + min_nodes_reporting: 10 + max_roots: 6 + min_nodes_parented: 4 + min_sessions: 10 + logging: rust_log: "debug" output_dir: "./sim-results" ``` +The assertion thresholds in the shipped file are calibrated against +recorded runs at the invocation CI uses, and the file's own comments say +what they were derived from. Read those before retuning them. + ## Topology Algorithms | Algorithm | Parameters | Description | diff --git a/testing/chaos/sim/docker_exec.py b/testing/chaos/sim/docker_exec.py index fcd328c3..063f4f69 100644 --- a/testing/chaos/sim/docker_exec.py +++ b/testing/chaos/sim/docker_exec.py @@ -133,3 +133,54 @@ def is_container_running(container: str) -> bool: return result.returncode == 0 and result.stdout.strip() == "true" except subprocess.TimeoutExpired: return False + + +def existing_containers(names: list[str], timeout: int = 30) -> list[str] | None: + """Return which of `names` docker still knows about, running or not. + + Deliberately scoped to the names the caller passes in. Enumerating by + compose project label would also sweep up whatever a concurrent run or + a different project owns, and under the parallel CI matrix that turns a + leak check into a flake generator. + + Returns `None` when docker could not be asked -- a query that failed has + observed nothing, and reporting "no survivors" for it would recreate the + silence this check exists to break. + """ + try: + result = subprocess.run( + ["docker", "ps", "-a", "--format", "{{.Names}}"], + capture_output=True, + text=True, + timeout=timeout, + ) + except subprocess.TimeoutExpired: + log.error("docker ps timed out after %ds", timeout) + return None + if result.returncode != 0: + log.error("docker ps exited %d\nstderr: %s", result.returncode, _tail(result.stderr)) + return None + present = set(result.stdout.split()) + return [name for name in names if name in present] + + +def force_remove(names: list[str], timeout: int = 60) -> None: + """Remove the named containers outright, whatever state they are in. + + Best effort and never raises: this runs on a teardown path that has + already reported a leak, and the report is the part that must survive. + """ + if not names: + return + try: + result = subprocess.run( + ["docker", "rm", "-f"] + names, + capture_output=True, + text=True, + timeout=timeout, + ) + except subprocess.TimeoutExpired: + log.error("docker rm -f timed out after %ds", timeout) + return + if result.returncode != 0: + log.error("docker rm -f exited %d\nstderr: %s", result.returncode, _tail(result.stderr)) diff --git a/testing/chaos/sim/nodes.py b/testing/chaos/sim/nodes.py index bff2d6a1..2a69f7c3 100644 --- a/testing/chaos/sim/nodes.py +++ b/testing/chaos/sim/nodes.py @@ -104,17 +104,34 @@ class NodeManager: self._start_node(nid) def _stop_node(self, node_id: str, duration: float): - """Stop a container.""" + """Stop a container, and record it as down only if it stopped. + + A `docker stop` that exits non-zero -- the container already gone, + the daemon refusing -- once left the node marked down anyway. Every + figure the scenario reasons with is derived from that mark: + `down_count` and so the `max_down_nodes` cap, `_would_disconnect` + and so the `protect_connectivity` guard, and the shared + `down_nodes` set that netem, links and traffic all skip. Returning + before the mutation keeps the model equal to the mesh and lets the + next churn tick retry the node. + """ container = self.topology.container_name(node_id) docker_exec_quiet(container, "kill 1", timeout=5) # SIGTERM to PID 1 # Use docker stop with a short grace period import subprocess - subprocess.run( + result = subprocess.run( ["docker", "stop", "-t", "2", container], capture_output=True, + text=True, timeout=15, ) + if result.returncode != 0: + log.warning( + "Failed to stop %s: %s; leaving %s marked up", + container, result.stderr.strip(), node_id, + ) + return now = time.time() state = self.node_states[node_id] diff --git a/testing/chaos/sim/runner.py b/testing/chaos/sim/runner.py index 8f661cae..75993411 100644 --- a/testing/chaos/sim/runner.py +++ b/testing/chaos/sim/runner.py @@ -25,7 +25,7 @@ from .assertions import ( from .compose import generate_compose from .config_gen import write_configs from .control import snapshot_all_congestion, snapshot_all_mmp, snapshot_all_trees -from .docker_exec import docker_compose +from .docker_exec import docker_compose, existing_containers, force_remove from .link_swap import LinkSwapManager from .links import LinkManager from .logs import AnalysisResult, analyze_logs, collect_logs, write_sim_metadata @@ -569,6 +569,85 @@ class SimRunner: except Exception: log.exception("Could not write status file") + def _own_containers(self) -> list[str]: + """Name every container this run asked compose to create. + + Empty before the topology exists, which is the only window in which + a teardown can run with nothing of its own on the host. + """ + if not self.topology: + return [] + return [self.topology.container_name(nid) for nid in self.topology.nodes] + + def _stop_mesh(self) -> None: + """Take the containers down, then check that they actually went. + + `docker compose down` exits 0 while leaving containers behind when it + races an `up -d` that failed part way through: compose enumerates the + project before the stragglers have registered, finds nothing to + remove, and says so successfully. Nothing downstream looks at what + survived, so the leak is invisible at the one moment it is cheap to + see. + """ + log.info("Stopping containers...") + docker_compose(self.compose_file, ["down"], check=False) + self._check_teardown() + + def _check_teardown(self) -> None: + """Report, then clear, any of this run's containers that outlived `down`. + + Scoped to names this run generated. A check that reasoned about the + compose project label, or about container names in general, would + answer for whatever a concurrent scenario happens to own, and the CI + matrix runs plenty of those at once. + """ + wanted = self._own_containers() + if not wanted: + return + + survivors = existing_containers(wanted) + if survivors is None: + # Detection failed, which is not the same as detecting nothing. + log.error("Could not ask docker what survived `down`; leak undetectable") + return + if not survivors: + return + log.warning( + "`compose down` exited 0 but left %d of this run's %d containers: %s", + len(survivors), len(wanted), ", ".join(survivors), + ) + + # Remedy, kept apart from the detection above on purpose: deleting + # everything from here down leaves the report intact. + force_remove(survivors) + leaked = existing_containers(survivors) + if not leaked: + return + # Either docker could not be asked a second time, or the containers + # are still there after a forced removal. Neither is the transient + # race, and neither clears itself, so this is the run's verdict. + detail = "unknown" if leaked is None else ", ".join(leaked) + log.error("Containers survived forced removal, leaked to the host: %s", detail) + self._write_leak(leaked if leaked is not None else wanted) + self.aborted = True + + def _write_leak(self, names: list[str]) -> None: + """Record the leaked names beside the run's other artifacts. + + Never raises, for the same reason `_write_status` does not: this runs + on a teardown path that has already gone wrong. Written alongside + status.txt rather than into it: the status is the simulation's own + outcome, and a leak on the way out does not retract it. + """ + try: + os.makedirs(self.output_dir, exist_ok=True) + path = os.path.join(self.output_dir, "leaked-containers.txt") + with open(path, "w") as f: + for name in names: + f.write(name + "\n") + except Exception: + log.exception("Could not write leaked-containers file") + def _release_network(self) -> None: """Give this run's claimed /24 back. @@ -593,8 +672,7 @@ class SimRunner: if self.compose_file: # `up -d` can fail part way through, so this run may own # containers or a network even with no mesh to speak of. - log.info("Stopping containers...") - docker_compose(self.compose_file, ["down"], check=False) + self._stop_mesh() self._release_network() return None @@ -755,12 +833,7 @@ class SimRunner: # Status first: it is the one artifact that must exist whatever # else happens, and stopping containers can still time out. self._write_status(status) - log.info("Stopping containers...") - docker_compose( - self.compose_file, - ["down"], - check=False, - ) + self._stop_mesh() self._release_network() return result diff --git a/testing/dns-resolver/test.sh b/testing/dns-resolver/test.sh index 1c799ff7..bf4ed80b 100755 --- a/testing/dns-resolver/test.sh +++ b/testing/dns-resolver/test.sh @@ -55,50 +55,115 @@ cleanup_container() { docker rm -f "$name" >/dev/null 2>&1 || true } +# Emit a captured output file to stderr, delimited and labelled with the +# command it came from. Callers use this only on failure: a command that +# succeeds leaves no trace, so the suite stays quiet when it is green. +dump_output() { + local label="$1" file="$2" + { + echo " --- $label failed; captured output follows ---" + if [ -s "$file" ]; then + cat "$file" + else + echo " (no output)" + fi + echo " --- end captured output ---" + } >&2 +} + +# Run a command with both streams captured. Discard the capture on success; +# on failure emit it, so the reason a build or a container start died is not +# thrown away. Stdin is inherited, so a caller may pipe into it. +run_quiet() { + local label="$1" + shift + local out rc=0 + out=$(mktemp) + "$@" >"$out" 2>&1 || rc=$? + [ "$rc" -eq 0 ] || dump_output "$label" "$out" + rm -f "$out" + return "$rc" +} + # Build an image from an inline Dockerfile. build_image() { local tag="$1" shift - echo "$@" | docker build -t "$tag" -f - "$REPO_ROOT" >/dev/null 2>&1 + echo "$@" | run_quiet "docker build -t $tag" \ + docker build -t "$tag" -f - "$REPO_ROOT" } # Start a systemd container in the background. start_systemd_container() { local name="$1" image="$2" cleanup_container "$name" - docker run -d --name "$name" \ + run_quiet "docker run $name" \ + docker run -d --name "$name" \ --label com.corganlabs.fips-ci=1 \ --privileged \ --cgroupns=host \ -v /sys/fs/cgroup:/sys/fs/cgroup:rw \ --tmpfs /run --tmpfs /run/lock \ - "$image" >/dev/null 2>&1 + "$image" } # Same, but with TUN device for the e2e scenario. start_systemd_container_with_tun() { local name="$1" image="$2" cleanup_container "$name" - docker run -d --name "$name" \ + run_quiet "docker run $name (with tun)" \ + docker run -d --name "$name" \ --label com.corganlabs.fips-ci=1 \ --privileged \ --cgroupns=host \ --device /dev/net/tun \ -v /sys/fs/cgroup:/sys/fs/cgroup:rw \ --tmpfs /run --tmpfs /run/lock \ - "$image" >/dev/null 2>&1 + "$image" +} + +# Report why systemd never reached a running state. Every probe is +# best-effort and captured with its own stderr: the container may have +# exited, or never have been created at all, and the docker error text +# saying so is itself the diagnosis. +dump_systemd_state() { + local name="$1" out + out=$(mktemp) + { + echo "== docker ps -a" + docker ps -a --filter "name=^${name}$" 2>&1 + echo "== systemctl is-system-running" + docker exec "$name" systemctl is-system-running 2>&1 + echo "== systemctl list-units --failed" + docker exec "$name" systemctl list-units --failed --no-pager 2>&1 + echo "== journalctl -b (last 100 lines)" + docker exec "$name" journalctl -b --no-pager -n 100 2>&1 + echo "== docker logs (last 100 lines)" + docker logs --tail 100 "$name" 2>&1 + } >"$out" 2>&1 + dump_output "systemd boot of $name" "$out" + rm -f "$out" } # Wait for systemd to reach a bootable state inside the container. wait_for_systemd() { local name="$1" + local state for _i in $(seq 1 "$BOOT_TIMEOUT"); do - if docker exec "$name" systemctl is-system-running --wait 2>/dev/null | grep -qE 'running|degraded'; then - return 0 - fi + # Read the state rather than piping it into grep. systemctl exits + # non-zero for "degraded", and pipefail turns that into a failed + # pipeline even when grep matched, so the piped form could never + # accept a degraded boot: in a container systemd-modules-load + # always fails, so every scenario burned the full timeout and + # warned about a container that had in fact booted. + state=$(docker exec "$name" systemctl is-system-running --wait 2>/dev/null) + case "$state" in + *running*|*degraded*) return 0 ;; + esac sleep 1 done echo " WARNING: systemd did not reach running state in ${BOOT_TIMEOUT}s (may still work)" + dump_systemd_state "$name" return 0 } diff --git a/testing/lib/wait-converge-test.sh b/testing/lib/wait-converge-test.sh index 7b7c0ab9..48763531 100755 --- a/testing/lib/wait-converge-test.sh +++ b/testing/lib/wait-converge-test.sh @@ -111,8 +111,31 @@ ping_backcompat_hold() { fi } +# Case 6 trace: converges quickly, so the verdict case that needs a +# converged run does not spend the near-converged hold's twelve seconds. +ping_quick_converge() { + set_pt; local t=$PT + if (( t < 2 )); then + PASSED=18; FAILED=2 + else + PASSED=20; FAILED=0 + fi +} + HOLD_MSG="holding for full budget" STUCK_MSG="STUCK" +NOCONV_MSG="tree did not converge" + +# Run the gate in THIS shell (not a subshell) so the CONVERGE_* verdict +# globals survive, capturing its output to a file instead. `$(...)` runs the +# gate in a fork, which discards those assignments — that is why cases 1-4 +# can only assert on text. +VERDICT_OUT=$(mktemp) +run_gate() { + reset_ping + CONVERGE_OUTCOME=""; CONVERGE_REACHED=-1; CONVERGE_PENDING=-1 + wait_until_connected "$@" >"$VERDICT_OUT" 2>&1 +} # --- Case 1: near-converged hold -------------------------------------- echo @@ -221,6 +244,56 @@ check "case5: floor of 1 polled its full budget" "$c5_polled_ok" "elapsed=${elap unset -f docker +# --- Case 6: the verdict discriminates non-convergence from connectivity -- +# +# This is the break-what-it-guards check for the verdict itself. The recorded +# failure exited 1 while reporting "20 passed, 0 failed": every connectivity +# pair passed and only the tree fell short, and nothing in the summary told +# the two apart. The gate now names its verdict, so drive it into each +# outcome and assert the verdict is the one that outcome deserves. +echo +echo "== Case 6: verdict names which condition failed ==" + +echo "-- Case 6a: genuinely unconverged tree, hard cap --" +run_gate ping_never_converges 6 3 1 2; rc=$? +cat "$VERDICT_OUT" +c6a_rc_ok=1; [ "$rc" -ne 0 ] && c6a_rc_ok=0 +check "case6a: unconverged tree still reds" "$c6a_rc_ok" "rc=$rc" +c6a_out_ok=1; [ "$CONVERGE_OUTCOME" = "timeout" ] && c6a_out_ok=0 +check "case6a: verdict is timeout" "$c6a_out_ok" "CONVERGE_OUTCOME=$CONVERGE_OUTCOME" +c6a_cnt_ok=1 +[ "$CONVERGE_REACHED" -eq 19 ] && [ "$CONVERGE_PENDING" -eq 1 ] && c6a_cnt_ok=0 +check "case6a: verdict carries the shortfall" "$c6a_cnt_ok" \ + "reached=$CONVERGE_REACHED pending=$CONVERGE_PENDING" +c6a_msg_ok=1; grep -q "$NOCONV_MSG" "$VERDICT_OUT" && c6a_msg_ok=0 +check "case6a: message says the tree did not converge" "$c6a_msg_ok" + +echo "-- Case 6b: wedged far from convergence, stall bail --" +run_gate ping_far_stall 30 4 1 2; rc=$? +cat "$VERDICT_OUT" +c6b_rc_ok=1; [ "$rc" -ne 0 ] && c6b_rc_ok=0 +check "case6b: wedged tree still reds" "$c6b_rc_ok" "rc=$rc" +c6b_out_ok=1; [ "$CONVERGE_OUTCOME" = "stalled" ] && c6b_out_ok=0 +check "case6b: verdict is stalled" "$c6b_out_ok" "CONVERGE_OUTCOME=$CONVERGE_OUTCOME" +c6b_msg_ok=1; grep -q "$NOCONV_MSG" "$VERDICT_OUT" && c6b_msg_ok=0 +check "case6b: message says the tree did not converge" "$c6b_msg_ok" + +echo "-- Case 6c: converged tree, and the verdict does not cry non-convergence --" +run_gate ping_quick_converge 20 4 1 2; rc=$? +cat "$VERDICT_OUT" +c6c_rc_ok=1; [ "$rc" -eq 0 ] && c6c_rc_ok=0 +check "case6c: converged tree still passes" "$c6c_rc_ok" "rc=$rc" +c6c_out_ok=1; [ "$CONVERGE_OUTCOME" = "converged" ] && c6c_out_ok=0 +check "case6c: verdict is converged" "$c6c_out_ok" "CONVERGE_OUTCOME=$CONVERGE_OUTCOME" +c6c_cnt_ok=1 +[ "$CONVERGE_REACHED" -eq 20 ] && [ "$CONVERGE_PENDING" -eq 0 ] && c6c_cnt_ok=0 +check "case6c: verdict carries a clean tree" "$c6c_cnt_ok" \ + "reached=$CONVERGE_REACHED pending=$CONVERGE_PENDING" +c6c_quiet_ok=0; grep -q "$NOCONV_MSG" "$VERDICT_OUT" && c6c_quiet_ok=1 +check "case6c: no non-convergence message on a clean run" "$c6c_quiet_ok" + +rm -f "$VERDICT_OUT" + # --- Summary ---------------------------------------------------------- echo echo "==============================================" diff --git a/testing/lib/wait-converge.sh b/testing/lib/wait-converge.sh index 17933a5c..a9588b15 100644 --- a/testing/lib/wait-converge.sh +++ b/testing/lib/wait-converge.sh @@ -9,6 +9,9 @@ # wait_until_connected [poll_secs] \ # [near_converged_slack] # +# wait_until_connected also sets CONVERGE_OUTCOME / CONVERGE_REACHED / +# CONVERGE_PENDING; see the block above it. +# # There was a wait_for_links() here. It was removed rather than kept for # symmetry: it had no caller anywhere in the tree on any branch, and its # reader carried the same failure-to-zero fallback wait_for_peers does. An @@ -56,6 +59,30 @@ wait_for_peers() { return 1 } +# Verdict of the most recent wait_until_connected() call, so a caller can +# report WHICH condition failed rather than only that one did: +# CONVERGE_OUTCOME converged | stalled | timeout +# CONVERGE_REACHED reachable pairs at the moment of the verdict +# CONVERGE_PENDING unreachable pairs at that moment +# +# These exist because the gate's own probe is strictly harsher than the +# assertion it guards, so a run can fail the gate at 18/20 and then pass +# the strict all-pairs assertion 20/20. Without them the caller's summary +# line reads "20 passed, 0 failed" on a non-convergence exit, which a +# reader cannot tell from a connectivity failure. +CONVERGE_OUTCOME="" +CONVERGE_REACHED=0 +CONVERGE_PENDING=0 + +# Record the verdict of a wait_until_connected() return. +# +# shellcheck disable=SC2034 # read by sourcing suites, not within this file +_converge_verdict() { + CONVERGE_OUTCOME="$1" + CONVERGE_REACHED="$PASSED" + CONVERGE_PENDING="$FAILED" +} + # Wait until a connectivity check reports every pair reachable, using a # progress-aware deadline instead of a fixed one. # @@ -102,6 +129,7 @@ wait_until_connected() { while (( SECONDS - start_secs < max_secs )); do "$ping_fn" if (( FAILED == 0 )); then + _converge_verdict converged echo " converge: all $PASSED pair(s) reachable after $((SECONDS - start_secs))s" return 0 fi @@ -111,7 +139,8 @@ wait_until_connected() { echo " converge: $PASSED reachable, $FAILED pending (progressing) after $((SECONDS - start_secs))s" elif (( SECONDS - last_progress >= stall_secs )); then if (( FAILED > near_converged_slack )); then - echo " converge: STUCK at $PASSED reachable / $FAILED pending — no progress for ${stall_secs}s (after $((SECONDS - start_secs))s)" + _converge_verdict stalled + echo " converge: STUCK — tree did not converge: $PASSED reachable / $FAILED pending, no progress for ${stall_secs}s (after $((SECONDS - start_secs))s)" return 1 fi if (( held_for_budget == 0 )); then @@ -122,6 +151,7 @@ wait_until_connected() { sleep "$poll_secs" done - echo " converge: TIMEOUT at $PASSED reachable / $FAILED pending after ${max_secs}s" + _converge_verdict timeout + echo " converge: TIMEOUT — tree did not converge: $PASSED reachable / $FAILED pending after ${max_secs}s" return 1 } diff --git a/testing/nat/scripts/nat-test.sh b/testing/nat/scripts/nat-test.sh index b7cf2efe..0d9f8052 100755 --- a/testing/nat/scripts/nat-test.sh +++ b/testing/nat/scripts/nat-test.sh @@ -306,41 +306,97 @@ require_docker_daemon() { fi } +# The two path assertions below decide whether a converged mesh took the path +# the scenario expects. Every failure branch names the container, what was +# expected and what was read instead, and callers wrap them in the same +# `|| { dump_*_diagnostics; return 1; }` shape the convergence waits use, so the +# container state behind a mismatch is captured with it. Both stay silent on +# success: this suite passes most of the time. assert_peer_path() { local container="$1" local expected_transport="$2" local expected_prefix="$3" - docker exec "$container" fipsctl show peers \ - | python3 -c " + local peers errfile rc=0 + # stdout and stderr are kept apart deliberately: any warning fipsctl writes + # to stderr would otherwise be spliced into the JSON and turn a healthy read + # into a parse failure. + errfile="$(mktemp)" + peers="$(docker exec "$container" fipsctl show peers 2>"$errfile")" || rc=$? + if [ "$rc" != 0 ]; then + echo "ASSERT FAIL: peer path $container: expected transport ${expected_transport} to a peer at ${expected_prefix}*, but 'fipsctl show peers' exited ${rc}:" >&2 + cat "$errfile" >&2 + rm -f "$errfile" + return 1 + fi + rm -f "$errfile" + if ! python3 -c " import json, sys -data = json.load(sys.stdin) -peers = [p for p in data.get('peers', []) if p.get('connectivity') == 'connected'] +container, want_transport, want_prefix = sys.argv[1], sys.argv[2], sys.argv[3] +raw = sys.stdin.read() +want = f'transport {want_transport!r} to a peer at {want_prefix}*' +head = f'ASSERT FAIL: peer path {container}: expected {want}, ' +try: + data = json.loads(raw) +except ValueError as exc: + raise SystemExit(head + f'but the peer JSON did not parse: {exc}; read: {raw!r}') +reported = data.get('peers', []) +seen = [ + {k: p.get(k) for k in + ('npub', 'connectivity', 'transport_type', 'transport_addr', 'direction', 'last_seen_ms')} + for p in reported +] +peers = [p for p in reported if p.get('connectivity') == 'connected'] if not peers: - raise SystemExit(1) + raise SystemExit(head + f'observed no connected peer; {len(reported)} peer(s) reported: {seen}') peer = peers[0] transport = peer.get('transport_type', '') addr = peer.get('transport_addr', '') -if transport != sys.argv[1]: - raise SystemExit(f'transport mismatch: expected {sys.argv[1]!r}, got {transport!r}') -if not addr.startswith(sys.argv[2]): - raise SystemExit(f'addr mismatch: expected prefix {sys.argv[2]!r}, got {addr!r}') -" "$expected_transport" "$expected_prefix" +if transport != want_transport: + raise SystemExit(head + f'observed transport {transport!r} at {addr!r}; peers: {seen}') +if not addr.startswith(want_prefix): + raise SystemExit(head + f'observed addr {addr!r} on transport {transport!r}; peers: {seen}') +" "$container" "$expected_transport" "$expected_prefix" <<<"$peers"; then + return 1 + fi } assert_link_path() { local container="$1" local expected_prefix="$2" - docker exec "$container" fipsctl show links \ - | python3 -c " + local links errfile rc=0 + # See assert_peer_path: stderr is kept out of the JSON on purpose. + errfile="$(mktemp)" + links="$(docker exec "$container" fipsctl show links 2>"$errfile")" || rc=$? + if [ "$rc" != 0 ]; then + echo "ASSERT FAIL: link path $container: expected a link to ${expected_prefix}*, but 'fipsctl show links' exited ${rc}:" >&2 + cat "$errfile" >&2 + rm -f "$errfile" + return 1 + fi + rm -f "$errfile" + if ! python3 -c " import json, sys -data = json.load(sys.stdin) +container, want_prefix = sys.argv[1], sys.argv[2] +raw = sys.stdin.read() +want = f'a link to {want_prefix}*' +head = f'ASSERT FAIL: link path {container}: expected {want}, ' +try: + data = json.loads(raw) +except ValueError as exc: + raise SystemExit(head + f'but the link JSON did not parse: {exc}; read: {raw!r}') links = data.get('links', []) +seen = [ + {k: link.get(k) for k in ('link_id', 'remote_addr', 'direction', 'state')} + for link in links +] if not links: - raise SystemExit(1) + raise SystemExit(head + 'observed no links at all') addr = links[0].get('remote_addr', '') -if not addr.startswith(sys.argv[1]): - raise SystemExit(f'link addr mismatch: expected prefix {sys.argv[1]!r}, got {addr!r}') -" "$expected_prefix" +if not addr.startswith(want_prefix): + raise SystemExit(head + f'observed {addr!r}; links: {seen}') +" "$container" "$expected_prefix" <<<"$links"; then + return 1 + fi } require_bootstrap_activity() { @@ -373,10 +429,22 @@ run_cone() { dump_cone_diagnostics return 1 } - assert_peer_path fips-nat-cone-a${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_WAN}. - assert_peer_path fips-nat-cone-b${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_WAN}. - assert_link_path fips-nat-cone-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}. - assert_link_path fips-nat-cone-b${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}. + assert_peer_path fips-nat-cone-a${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_WAN}. || { + dump_cone_diagnostics + return 1 + } + assert_peer_path fips-nat-cone-b${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_WAN}. || { + dump_cone_diagnostics + return 1 + } + assert_link_path fips-nat-cone-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}. || { + dump_cone_diagnostics + return 1 + } + assert_link_path fips-nat-cone-b${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}. || { + dump_cone_diagnostics + return 1 + } # shellcheck disable=SC1090 source "$CONFIG_DIR/cone/npubs.env" ping_peer fips-nat-cone-a${FIPS_CI_NAME_SUFFIX:-} "$NPUB_B" @@ -398,10 +466,22 @@ run_symmetric() { dump_symmetric_diagnostics return 1 } - assert_peer_path fips-nat-symmetric-a${FIPS_CI_NAME_SUFFIX:-} tcp ${NAT_WAN}.11: - assert_peer_path fips-nat-symmetric-b${FIPS_CI_NAME_SUFFIX:-} tcp ${NAT_WAN}.10: - assert_link_path fips-nat-symmetric-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}.11: - assert_link_path fips-nat-symmetric-b${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}.10: + assert_peer_path fips-nat-symmetric-a${FIPS_CI_NAME_SUFFIX:-} tcp ${NAT_WAN}.11: || { + dump_symmetric_diagnostics + return 1 + } + assert_peer_path fips-nat-symmetric-b${FIPS_CI_NAME_SUFFIX:-} tcp ${NAT_WAN}.10: || { + dump_symmetric_diagnostics + return 1 + } + assert_link_path fips-nat-symmetric-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}.11: || { + dump_symmetric_diagnostics + return 1 + } + assert_link_path fips-nat-symmetric-b${FIPS_CI_NAME_SUFFIX:-} ${NAT_WAN}.10: || { + dump_symmetric_diagnostics + return 1 + } require_bootstrap_activity fips-nat-symmetric-a${FIPS_CI_NAME_SUFFIX:-} require_bootstrap_activity fips-nat-symmetric-b${FIPS_CI_NAME_SUFFIX:-} # shellcheck disable=SC1090 @@ -424,10 +504,22 @@ run_lan() { dump_lan_diagnostics return 1 } - assert_peer_path fips-nat-lan-a${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_LAN}. - assert_peer_path fips-nat-lan-b${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_LAN}. - assert_link_path fips-nat-lan-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_LAN}. - assert_link_path fips-nat-lan-b${FIPS_CI_NAME_SUFFIX:-} ${NAT_LAN}. + assert_peer_path fips-nat-lan-a${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_LAN}. || { + dump_lan_diagnostics + return 1 + } + assert_peer_path fips-nat-lan-b${FIPS_CI_NAME_SUFFIX:-} udp ${NAT_LAN}. || { + dump_lan_diagnostics + return 1 + } + assert_link_path fips-nat-lan-a${FIPS_CI_NAME_SUFFIX:-} ${NAT_LAN}. || { + dump_lan_diagnostics + return 1 + } + assert_link_path fips-nat-lan-b${FIPS_CI_NAME_SUFFIX:-} ${NAT_LAN}. || { + dump_lan_diagnostics + return 1 + } # shellcheck disable=SC1090 source "$CONFIG_DIR/lan/npubs.env" ping_peer fips-nat-lan-a${FIPS_CI_NAME_SUFFIX:-} "$NPUB_B" diff --git a/testing/static/scripts/rekey-test.sh b/testing/static/scripts/rekey-test.sh index 4a4d39c2..1e774c40 100755 --- a/testing/static/scripts/rekey-test.sh +++ b/testing/static/scripts/rekey-test.sh @@ -196,6 +196,11 @@ PASSED=0 FAILED=0 TOTAL_PASSED=0 TOTAL_FAILED=0 +# Counted separately from TOTAL_FAILED on purpose: a convergence-gate +# failure and a connectivity failure are different outcomes, and folding +# the first into the second is what made the recorded transcript report +# "20 passed, 0 failed" on an exit-1 run. +TOTAL_UNCONVERGED=0 # Node identities ENV_FILE="$SCRIPT_DIR/../generated-configs${FIPS_CI_NAME_SUFFIX:-}/npubs.env" @@ -274,6 +279,24 @@ _baseline_ping() { ping_all quiet "$CONVERGENCE_PING_TIMEOUT" } +# Emit the one-line run summary. +# +# When the convergence gate is what failed, the line says so and names the +# shortfall. The gate's probe is strictly harsher than the strict all-pairs +# assertion it guards, so a run can fail the gate at 18/20 relationships and +# still pass the assertion 20/20 — which is exactly the recorded failure this +# discriminator exists for. Without it both outcomes print the same counts. +results_line() { + local line="=== Results: $TOTAL_PASSED passed, $TOTAL_FAILED failed" + if [ "$TOTAL_UNCONVERGED" -ne 0 ]; then + line+=", tree did not converge" + line+=" ($CONVERGE_REACHED/$((CONVERGE_REACHED + CONVERGE_PENDING))" + line+=" relationships, $CONVERGE_OUTCOME)" + fi + echo "$line ===" + return 0 +} + phase_result() { local phase="$1" TOTAL_PASSED=$((TOTAL_PASSED + PASSED)) @@ -404,16 +427,19 @@ if wait_until_connected _baseline_ping "$BASELINE_CONVERGENCE_TIMEOUT" 20; then if [ "$FAILED" -ne 0 ]; then echo "" dump_peer_connectivity - echo "=== Results: $TOTAL_PASSED passed, $TOTAL_FAILED failed ===" + results_line exit 1 fi else - echo " Mesh did not reach a converged tree before timeout" + TOTAL_UNCONVERGED=$((TOTAL_UNCONVERGED + 1)) + echo " Mesh did not reach a converged tree before timeout" \ + "($CONVERGE_OUTCOME at $CONVERGE_REACHED reachable /" \ + "$CONVERGE_PENDING pending)" ping_all quiet "$CONVERGENCE_PING_TIMEOUT" phase_result "Pre-rekey baseline (all 20 pairs)" echo "" dump_peer_connectivity - echo "=== Results: $TOTAL_PASSED passed, $TOTAL_FAILED failed ===" + results_line exit 1 fi echo "" @@ -570,9 +596,9 @@ phase_result "Log analysis" echo "" # ── Summary ──────────────────────────────────────────────────────────── -echo "=== Results: $TOTAL_PASSED passed, $TOTAL_FAILED failed ===" +results_line -if [ "$TOTAL_FAILED" -eq 0 ]; then +if [ "$TOTAL_FAILED" -eq 0 ] && [ "$TOTAL_UNCONVERGED" -eq 0 ]; then exit 0 else # Dump logs on failure for diagnostics.