diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c4bab51..eac2231f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -521,6 +521,921 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 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 + single semaphore before any identity check, with the sender's npub used only + as a log field, so one sender could hold every slot and deny traversal + onboarding to every other peer for as long as it kept offering. Admission now + takes a per-npub permit and a global permit together. A sender over its own + allowance is refused at debug rather than warn, because the party tripping it + is by definition sending faster than the node wants and a record per + rejection would turn the spam into log volume; the global bound being reached + keeps its warn, which is the operator's signal that the node is genuinely + saturated. **This does not make the pool inexhaustible.** Nostr identities + cost nothing to generate and the signal subscription carries no author + restriction, so an attacker running four throwaway npubs still saturates the + shipped 16-slot pool at an unchanged total offer rate. What the change buys + is that one identity can no longer do it alone, and that the two refusals are + distinguishable in the log. The permit is still held across the whole + attempt; that duration remains inferred from the attempt timeout rather than + measured. + +- Config validation now rejects a `node.discovery.nostr.signal_ttl_secs` that + is too large for the configured `replay_window_secs`. A traversal signal is + acceptable over its TTL plus 60s of clock-skew grace on each side, and that + span has to stay strictly inside the replay window, or a session id evicted + from the replay cache on expiry is still fresh enough to be accepted a second + time. The relation was documented but unenforced, so raising the TTL past + 180s silently voided it. The bound is derived from the skew constant rather + than restated, and is checked whether or not nostr discovery is enabled, for + the same reason the rekey rules are. The shipped defaults (120s against 300s) + are unaffected, but a configuration that had widened the TTL or narrowed the + replay window now fails to load, with an error naming the concrete floor for + `replay_window_secs`. The NAT lab's config generator was one such + configuration and its generated `replay_window_secs` moves from 60 to 180. + Note that this covers eviction on expiry only: `seen_sessions_max_entries` + remains a separate capacity-eviction route that no config relation bounds. + +- The peer-retry tick no longer awaits the Nostr advert refetch. It ran inline + on the 1-second rx-loop tick, awaiting a fetch with a 2-second timeout for + each due peer and discarding the result; with up to sixteen due peers the + timeouts stacked, and field profiling measured single 2.00 s stalls as the + common case and a worst tick of 12.4 s against a 1 s period, delaying every + other rx-loop arm by as much as 4.2 s. The refetch is now spawned, so a dial + uses the advert cached at that moment and the refreshed one lands for that + peer's next retry. + +- The `Adopted NAT traversal socket` log line now carries the transport id and + the local address alongside the peer npub. Without the local address an + operator cannot join a host socket table against adoption events, and without + the transport id several peers sharing one adopted transport are + indistinguishable from several separate adopted transports. + +#### Admission / rate limiting + +- Inbound msg1 is classified before it is rate limited, and rekey or restart + msg1 arriving on an established link now draws on its own token bucket + instead of competing with stranger admission for a single shared one. On a + node with many peers the shared bucket refused a large share of ordinary + rekey traffic: a field node at roughly 245 peers refused 8753 msg1 in 25 + minutes, and 159 of the 201 distinct sources were peers it already held + sessions with. Nodes upgrade with no config change. The `Msg1 rate limited` + log line now reports which limb refused, the pending count or the token + bucket, which it previously did not distinguish. + +#### Data-plane / metrics / observability + +- Peer bloom filters are computed for every recipient in one prefix and suffix + union sweep rather than rebuilt per recipient. Announcing to R peers + previously did R full map builds and R by T merges; at 240 peers that was + 20.6 ms per tick, roughly half the tick body, with a median per-interval + maximum of 34.5 ms. The result is exactly equal rather than approximately: + merging is a bytewise OR, so regrouping the unions cannot change it. The + trade-off, measured rather than assumed, is that the sweep does its full work + regardless of how many peers are ready, so a tick announcing to one or two + peers now costs about twice what it did; break-even is around three ready + peers. Cadence, the debounce, the sequence rule and the fill-ratio cap are + unchanged. + +- Each peer's npub is derived once at construction instead of once per tick. + The per-tick stats snapshot ran a bech32 encode for every tracked peer, and a + second one for the common peer with no hosts-file entry and no alias, since + the display-name fallback bottoms out in the same encode: 14.1 ms per tick at + 240 peers. The display name itself is deliberately not cached, because the + alias map and the host map both mutate at runtime. + +#### Transports & config + +- `fipsctl keygen` no longer exits non-zero when only the `fips.pub` write + fails. The private key is already on disk at that point, so failing the run + reported failure for a keygen that did produce the identity; the failure is + now a warning and the run succeeds. The pre-existing-key guard also moves + from `exists` to `symlink_metadata`, so a dangling symlink at the key path + now blocks keygen without `--force` instead of being overwritten silently. + +#### Library API + +- **Source-breaking for consumers of the library crate**: four public types now + implement `Drop`, so their fields can no longer be moved out. `Identity`, + `ResolvedIdentity`, `IdentityConfig` and `HandshakeState` each gained one as + part of clearing key material at end of scope. `IdentityConfig` is the one + most likely to be reached in practice, since it hangs off the public `Config` + as `node.identity`, so code that moved the nsec out of a configuration value + no longer compiles and needs `Option::take` instead. Nothing about the + behaviour of the shipped binaries changes; this affects only callers using + `fips` as a library. + +#### CI & test-harness reliability + +- Two CI runs on one machine can no longer collide. Every suite derives its own + docker build context, image tag, container names, network range and host + interface names per run, so concurrent runs cannot reap each other's + containers or contend for a fixed subnet. This is what a contributor running + `testing/ci-local.sh` alongside a GitHub run, or two local runs at once, sees + change: the runs stay independent instead of one killing the other. + +- A test that does not run, or whose result cannot be read, no longer passes + silently. A failed scenario now fails the run rather than being logged and + stepped over, a node whose logs cannot be read no longer counts as clean, an + unanswered control query no longer reads as zero, an unknown scenario key is + rejected instead of matching nothing, and a skipped check appears in the + final verdict rather than only in scrollback. + +- New guards run in both the local and GitHub runners, so the two gates agree. + They check that trailing-log call sites are wired, that the log strings the + harness matches on are still emitted by the daemon, that the two runners' + integration-suite sets match per leg rather than as a folded token, that + every GitHub Action reference is pinned in the required form, and that source + comments do not cite references a reader of the published tree cannot + resolve. + +- Coverage moved from Docker to deterministic in-process tests, and dead + scenarios were retired. The six cost-selection chaos scenarios, the + admission-cap and acl-allowlist Docker suites, the smoke-10 scenario, the + tcp-chain and mesh-public static topologies, and three ignored Ethernet + tests are gone, with their behaviour asserted in unit and integration tests + instead. A local CI run is correspondingly shorter and less dependent on + container timing. + +- The `bloom-storm` chaos scenario no longer runs on either the local or the + cloud runner. Unlike the retirements above it has no replacement: the + scenario files remain in the tree and it stays runnable by hand, but nothing + now exercises downstream containment of a mid-chain ancestor swap on a + schedule. This is recorded as a coverage gap rather than as a completed + migration. + +- A failing harness now says why it failed. The dns-resolver suite sent build + and container-start output to `/dev/null`, so a failed scenario reported + that it had failed and nothing else; output is now captured and emitted on + failure, naming the command, and the systemd readiness wait dumps container + state, failed units and the journal when it gives up. The NAT-lab path + assertions exited bare, printing neither what they expected nor what they + saw and triggering none of the scenario diagnostics their siblings already + call; all twelve call sites now report the container, the expectation, the + observation and a projection of the peer or link table, and distinguish a + failed control-socket exec from unparseable output from a genuine mismatch. + The convergence gate could not tell a tree that did not converge from + connectivity that failed, and could exit non-zero while reporting "20 + passed, 0 failed"; it now records the outcome, the count reached and the + count pending, and its failure messages name the condition. A passing run + is as quiet as before, and no timing, threshold or control-flow behaviour + changed in any of the three. + +- A dns-resolver scenario no longer burns the full 30-second boot timeout and + warns about a container that booted correctly. The readiness poll ran under + `pipefail` and piped `systemctl is-system-running` into `grep`, and that + command exits non-zero when the system is degraded, which is where systemd + inside a container always settles; the pipeline therefore failed even when + the pattern matched, leaving the degraded branch dead. The poll now matches + on the captured state instead of piping into `grep`. + +- The chaos harness now checks that teardown and node stops did what they + report. `docker compose down` exits 0 while leaving a run's containers + alive, so a partly-failed bring-up leaked named containers with nothing to + detect it; teardown now asks whether the containers this run owns are gone, + treats a survivor that forced removal clears as a warning, and aborts with + the names written to an artifact when one survives that or the query cannot + run at all. The check is scoped to a run's own names, so concurrent runs + cannot trip each other. Node churn separately marked a node down whether or + not `docker stop` succeeded, so the simulation's model of the mesh diverged + from reality, and `nodes_down`, the `max_down_nodes` cap and the + connectivity guard are all computed from that model; a failed stop now + warns, carries the daemon's own message, and leaves the node out of the + down set for the next churn tick to retry against an honest model. + +#### Docs & contributor tooling + +- Comments throughout the source tree, the packaging files and the test scripts + no longer cite internal identifiers, planning documents or private stage names + that a reader of the published tree cannot resolve; each now states the thing + the citation stood for. A handful of comments that described behaviour the + code does not have (the control-plane read path, its snapshot dispatch, and + the MMP report types) have been corrected rather than merely reworded. One of + the edited files, the DNS setup helper, installs to `/usr/lib/fips` on every + packaging path, so its comment reached users. No code changed. + +### Fixed + +#### FMP/FSP rekey reliability + +- Inbound session-setup messages are now rate limited, keyed on the + authenticated link peer the datagram arrived over. The setup path allocated + a session entry and sent a routed SessionAck for every well-formed message + naming an address it had no entry for, and that address is an envelope field + the sender picks, so one neighbour could grow the session table at whatever + rate it could transmit and buy an ack per entry to a destination of its + choosing. The limiter sits ahead of every send and both handshake + constructions in the handler, so a refused message emits nothing and costs + no cryptography. The key is the link peer rather than the claimed source + address, which is what makes it a limit at all: keying on the source would + hand a single sender a fresh full bucket per forged message. + + Two consequences worth stating rather than discovering. The limiter bounds + each neighbour's contribution and makes a flood attributable; it does not + give the node an absolute ceiling, which stays at roughly + `peers * rate * handshake_timeout_secs`. And a legitimate peer reaching this + node over the *same* link as an attacker shares that attacker's bucket, so + 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`. + +- 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 XK msg2 read failed, returned without putting it back. Nothing + in that message is authenticated (the only thing tying it to the initiation + is the datagram's source address, which the sender chooses), so any node able + to reach the victim could cancel any initiation with 57 bytes of the right + length, and hold establishment down by repeating it. The entry is now kept. + Keeping it is not enough on its own, and the second half is the part worth + naming: the msg2 read mixes the sender's ephemeral into the symmetric state + before it authenticates anything, so an entry put back as the failed read + left it holds a handshake that can never read the genuine msg2, which trades + a one-round-trip denial for one lasting the full handshake timeout. The read + is therefore rolled back to its pre-read state before the entry goes back. + The three later failure paths in the same handler still drop the entry: each + is downstream of a msg2 that authenticated, so it is a local failure rather + than a possible forgery. The entry's activity stamp is deliberately not + refreshed on the failure path, so a spray cannot hold a dead initiation past + its original sweep deadline, and a new `ack_handshake_failed` counter makes + the refusals visible at the default log level. Only the XK handshake on this + branch is covered; the additional drop sites in the XX handshake on the + development branch are not. + +- An unauthenticated session msg3 no longer discards a completed key epoch. + Five sites discarded the whole rekey, which nulls a `pending` session sitting + beside the handshake, when only the handshake had failed: the four failure + paths in the responder-side rekey arm of the msg3 handler, and the + dual-initiation yield arm in the setup handler, which is gated on a rekey + being in progress rather than on this node having initiated it, so an entry + the peer armed reaches it too. That `pending` session is the epoch the real + peer may already have cut over to, so discarding it kills the reverse + direction until the session idles out. Two unauthenticated messages reached + it: a forged setup message arms a handshake beside a completed rekey once + that rekey has waited a full idle timeout for a peer that never appeared on + the new epoch, and any garbage msg3 of the right length then finishes the + job. All five now abandon only the handshake. The four remaining sites in the + ack initiator arm are deliberately left alone and the reason is recorded + there: an entry with the initiator flag set holds no pending session, so the + two calls are the same action at those sites. + +#### NAT traversal / Nostr discovery + +- Nostr NAT traversal signals are now sent only to relays the client pool + actually holds. A signal is addressed to the merge of the peer's NIP-17 inbox + relays, the relays its advert nominates for signaling, and our own DM relays, + but the pool is built once at startup from the configured relays and the send + is rejected outright, before anything is contacted, if any single URL in that + list is outside it. One unconfigured relay anywhere in the merge therefore + killed the whole attempt, including the sends to relays both sides shared. On + a public node in open mode this made discovery non-functional: 309 traversal + attempts, 290 explicit failures, zero successes, every failure on `relay not + found`. Configured peers were unaffected, since they run a matching relay + set. Comparison is on the normalized relay URL rather than the raw string, so + a configured relay spelled with a trailing slash or different host case is + not discarded. Two smaller fixes ride along: the responder resolves its + relays before binding a socket and running STUN, rather than spending a STUN + round trip and holding an offer slot only to find it has nowhere to answer, + and it gained the empty-relay-list guard the initiator already had. + +- Nostr NAT traversal no longer breaks after the host suspends. The traversal + clock cached a Unix timestamp once at startup and advanced it with a + monotonic `Instant`, which does not tick while a machine is asleep, so after + a suspend the daemon's idea of the time trailed real time by the suspend + duration for the rest of the process lifetime. Every NIP-40 expiration it + computed was therefore published already in the past: relays dropped the + offers as expired, the initiator logged a signal timeout waiting for an + answer, and traversal stayed broken until the daemon was restarted. The + clock now reads the wall clock on every call. This is not macOS-specific, + though a laptop that sleeps is where it is easiest to hit; any host that + suspends or hibernates was affected. Reported in + [#128](https://github.com/jmcorgan/fips/issues/128). + +#### Spanning-tree / mesh-size / routing + +- Flap dampening can now engage more than once in the lifetime of a node. + The arming check tested whether a dampening deadline had ever been set + rather than whether one was still in effect, so the first episode + disarmed the mechanism permanently: a node in a second flap storm went on + switching parents under hold-down alone, and neither the `flap_dampened` + counter nor the "Flap dampening engaged" warning fired again, so the + storm was invisible to anyone watching that counter. A lapsed episode is + now retired explicitly, clearing both the deadline and the switch + counter, so a second episode requires a fresh threshold of switches + within one window rather than re-engaging on the first switch after + lapse. Hold-down was unaffected throughout and continued to limit + discretionary switching, which is why the practical effect at shipped + settings was lost visibility and a lost escalation tier rather than + unrestrained flapping. Every path that can engage an episode now reports + it, including a re-engagement during parent-loss recovery, which was + previously silent. The warning names which path armed the episode + (`trigger`) and how long discretionary parent switching stays suppressed + (`dampening_secs`), using the same `trigger` values as the parent-switch + logs beside it, so the two can be read together. A + `node.tree.flap_dampening_secs` large enough to overflow the monotonic + clock is capped at one year, beyond which an episode is + indistinguishable from permanent, so an extreme setting no longer panics + the node when dampening engages. + +#### Data-plane / metrics / observability + +- A SessionDatagram carrying a truncated inner FSP payload no longer panics the + forwarding path. The coordinate-cache warm path sliced the inner payload at + the full 12-byte header offset while guarding only with the 4-byte common + prefix parser, so an inner payload of 4 to 11 bytes with phase 0x0 and the + Coords Present flag set indexed past the end of the slice. Because the + receive loop is the process's main future, the panic terminated the daemon + rather than a task, and under the packaged systemd unit the node restarted + into the same frame. The warm path now applies the same + `FspEncryptedHeader` guard the local-delivery path already used, which + additionally means a malformed frame carrying a non-zero protocol version or + the Unencrypted flag alongside Coords Present is dropped rather than having + its body read as coordinates. Any peer that had completed a link handshake + could trigger this, and admission is default-open. Frames rejected by that + guard are now counted in the forwarding statistics as + `warm_malformed_packets` and `warm_malformed_bytes`, the byte counter + charging the whole outer frame, visible over the control socket and on the + fipstop Routing State pane, so a node being fed malformed frames is + distinguishable from a quiet one at the default log level. The count is not a + packet drop: the frame is still delivered or forwarded, and only the + coordinate-cache warm attempt is abandoned. The existing debug log now also + carries the frame's protocol version and flags, which separate a short frame + from a bad-version or Unencrypted-flagged one. + +- `SessionDatagram` hop-limit handling now follows IP semantics. Delivery to + the addressed node is no longer TTL-gated, and a forwarder decrements before + deciding rather than after, so a datagram that would leave with a TTL of zero + is dropped instead of transmitted. Previously the TTL check ran ahead of the + local-delivery test, so a datagram addressed to this node that arrived with + TTL 0 was dropped, and a forwarder receiving a transit datagram at TTL 1 + transmitted it at TTL 0 for the next hop to discard, wasting one transmission + per expiring datagram. `SessionDatagram::decrement_ttl` and + `SessionDatagram::can_forward` were aligned to the same semantics: + `decrement_ttl` decrements first and reports false when the result is zero, + and `can_forward` is true only at a TTL of 2 or more. The reachable radius is + unchanged, because the two behaviors compensated exactly: a path of `h` links + still delivers for any source TTL of `h` or more. During a rolling upgrade, an + unupgraded forwarder feeding an upgraded destination delivers one hop further + than either version does on its own; no version mix delivers less far. The + `TtlExhausted` reject counter now charges at the node that makes the decision + rather than at the hop after it. + +#### Transports & config + +- A failed private-key write no longer leaves a node silently running an + ephemeral identity. Six write results in the identity path were discarded, + and the sharpest was in `persistent` mode: a failed write to `fips.key` fell + through to an ephemeral identity with no message, so a node that had been + asked for a stable identity changed its npub, its routing address and its + mesh IPv6 on every start, and nothing said so. All six now report. An + ephemeral start that is about to overwrite an existing key file now warns + first, naming the path and the setting that would have preserved the + identity, which is the warning `fipsctl keygen` has always given and the + daemon never did. Existence is tested with `symlink_metadata` rather than + `exists`, because a dangling symlink reports absent from the latter while + still being a file the write acts on. The persistent read path additionally + warns when it finds a key file whose mode is looser than 0600, or one that is + a symlink; it does not repair either, since the daemon does not own a file it + did not create. + +#### Peer lifecycle / gateway + +- A failed log write can no longer panic the thread or task that logged. The + subscriber was built with the default internal-error reporting, which sends a + failed write to `eprintln!`, and that macro panics when stderr has also + failed. The shipped supervisor configurations make that a single condition + rather than two: the macOS plist points both standard streams at one + unrotated file, and the systemd units route both to journald, so one full + disk fails both sinks together. In the daemon a crypto worker was the case + that mattered: it logs a warning on send backpressure, and a worker that + dies takes its share of the peer space with it permanently, while the panic + message is discarded along the same broken path. In `fips-gateway`, which + built its subscriber the same way, the casualty is a spawned task: the DNS + resolver, the control accept loop or the pool tick, none of which is observed + until shutdown, so the process would keep running and reporting healthy with + mesh name resolution or lease expiry and NAT cleanup silently stopped. + +#### macOS install layout + +- macOS: `peers.allow`, `peers.deny`, and the `hosts` file are now read + from `/usr/local/etc/fips/`, matching the install layout the macOS + packaging ships (`packaging/macos/`). That layout is what the three fixes + in this group align the daemon and `fipsctl` to. The default-path constants + were hardcoded to `/etc/fips/...` with only a `#[cfg(unix)]` / + `#[cfg(windows)]` split, so on macOS the daemon looked in a directory that + does not exist: `load_file` / `load_hosts_file` hit their `NotFound` no-op + arm and silently returned an empty ACL / empty host map. A populated + `peers.deny` therefore reported `effective_mode: "default_open"` and + `enforcement_active: false` via `fipsctl acl show`, and host-file aliases + went unloaded, with no error or warning. The default constants now follow + the platform's packaging (`/usr/local/etc/fips/` on macOS, `/etc/fips/` on + Linux and other Unix for the ACL files, and `/etc/fips/` on Linux and + `%ProgramData%\fips\` on Windows for the hosts file) and are pinned by + platform-gated unit tests so the layout cannot silently drift again. At + startup the daemon warns once if any of these files exist at the old + `/etc/fips/` location but not at the current default. Linux and Windows + behavior is unchanged. Contributed by + [@sh1ftred](https://github.com/sh1ftred). + **macOS users with existing files in `/etc/fips/` should move them to + `/usr/local/etc/fips/`.** + +- macOS: `fipsctl keygen` now writes `fips.key` / `fips.pub` to + `/usr/local/etc/fips/` by default. The default output directory was + hardcoded to `/etc/fips` for all Unix, but the daemon derives its identity + key paths from the config file's directory, which is + `/usr/local/etc/fips/fips.yaml` on macOS, so a generated identity landed + where the daemon never reads it and the node silently kept an ephemeral + identity. Linux and other Unix keep `/etc/fips`, Windows is unchanged, and + the values are pinned by platform-gated unit tests. + +- macOS: the system-wide config search path now includes + `/usr/local/etc/fips/fips.yaml` in addition to `/etc/fips/fips.yaml`. + Previously only `/etc/fips/fips.yaml` was probed, so a bare `fips` run + without `--config` skipped the installed config and derived identity key + paths from a non-existent directory. `/etc/fips/fips.yaml` is still probed + first so existing installs keep working. Both the macOS entry in the search + path and the directory `fipsctl keygen` writes to read the shared + `SYSTEM_CONFIG_DIR` constant, so the two cannot drift apart. The + launchd-installed daemon was unaffected (it always passes `--config`). + Linux and Windows behavior is unchanged. Because the daemon derives the + identity key directory from whichever config file loaded last, a macOS host + carrying `fips.yaml` at both locations would have resolved `fips.key` to the + new directory, found none, and under `persistent` generated a fresh + identity, silently changing its npub, routing address and mesh IPv6. The + daemon now adopts a key stranded at `/etc/fips/fips.key` and warns to move + it, instead of generating one. The fallback is confined to keys resolved + from the system config directory, so a run using `./fips.yaml` or a user + config is never redirected to a system key. + +#### Packaging & deployment + +- The maintainer address published in package metadata no longer bounces. The + crate authors field, the Debian package maintainer and upstream contact, and + both AUR PKGBUILD maintainer lines carried an address that no longer accepts + mail, so the contact of record in every artifact we ship was unreachable. + +### Security + +#### FMP/FSP session integrity + +- 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 + false: it ran a fresh responder handshake and overwrote the entry, discarding + the live keys. The message carries no authenticator and its source address is + an envelope field, so anyone able to reach a node could name an established + peer and take that session down, repeatedly, and hold it down by repeating + the message. The established case now always arms the handshake alongside the + running session and adopts the new keys only after a msg3 whose authenticated + static key matches the key the session was opened with, which is the check + the rekey path already applied; a peer that genuinely restarted still + re-establishes, and a forged setup leaves the session carrying traffic. This + 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. + +- 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 + address a signed offer named, including loopback, link-local, multicast, + broadcast, unspecified and CGNAT addresses, and placed no limit on how + many candidates one offer could carry. Any npub could + therefore have a node emit a burst of UDP packets at addresses of the + sender's choosing, carrying the node's own source address. Candidates in + the never-routable ranges are now rejected, IPv4-mapped IPv6 forms are + canonicalized before the check so they cannot slip past it, candidates + with port 0 are dropped, private-range candidates are punched only when + they share a /24 with one of our own addresses (which is what same-LAN + traversal already required of its own path), and the planned target list + is capped at eight. A peer's reflexive address is checked against the + never-routable ranges but not against the /24 rule, so a deployment whose + STUN server sits inside the private network keeps working. A malformed + address in a peer's signal now drops that one candidate instead of + failing the whole traversal. A node also records what it declined: one + log record per planning attempt carries how many candidates the peer + offered, how many were planned, the count refused in each class and one + sample address, at warning level for the shapes no honest peer produces + and at debug level for the routine off-subnet case. Same-LAN and + reflexive traversal are otherwise unaffected. + +- Traversal offers and answers dated in the future are now rejected. The + freshness check measured a message's age with a saturating subtraction, which + yields zero for any timestamp ahead of the local clock, so the age test could + not fail for a future-dated signal and no other term bounded the issue time + from above. A signal claiming to be issued arbitrarily far in the future was + accepted as strictly fresh, which voided the property that the freshness + window is narrower than the session-id replay window (300s by default) and + left the replay cache as the sole defence against a captured offer being + replayed. Forward-dating is now tolerated only up to the same 60s of clock + skew already allowed in the other direction, and a signal accepted under that + grace reports the skew outcome, so the existing clock-skew log fires for a + peer whose clock is ahead just as it does for one whose clock is behind. The + declared expiry timestamp is also no longer trusted beyond the issue time plus + the configured TTL, so a sender cannot widen its own acceptance window by + inflating that field. A single timestamp is now acceptable over at most the + signalling TTL plus 60s on each side, 240s under the shipped defaults. + Rejections are also now distinguishable in the log: a stale signal and a + future-dated one no longer share one reason string, and the inbound-offer + path, whose only surface was an unattributed debug line below the default log + level, now names the peer and the session and warns for the rejection classes + that relay delivery lag cannot produce (future-dated, identity-mismatch and + malformed offers), leaving an ordinary stale offer quiet. As with the existing + inbound rate-limit warning, an unauthenticated remote peer can drive that + line. A failure of our own offer's freshness during answer validation is + reported against the offer rather than mislabelled as the answer's, and the + tolerated-acceptance log now carries the issue and expiry stamps and no longer + attributes the acceptance to clock skew, since a peer configured with a longer + signalling TTL than ours now reaches it too. + +#### Data-plane / routing signals + +- 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 + they name, either by initiating a session toward it or by completing the + Noise handshake that binds an address to a peer's static key. These signals + carry no end-to-end authentication, so until now any admitted mesh member + could send one naming any address and have its effects applied: a path-MTU + clamp written for an arbitrary address, a cached-coordinate flush for an + arbitrary address, and a discovery and warmup cycle for an arbitrary address. + The `MtuExceeded` case was the sharpest, because its write into the + address-keyed path-MTU lookup that the TUN reader consults at TCP MSS clamp + time sat outside the session guard and so required no session, no peer + relationship and no prior state at all. A half-open session created by an + inbound handshake that has not yet proved its address does not admit these + signals, so a forged session opening cannot be used to unlock them. Signals + from a genuine on-path forwarder are unaffected: the reporter may be any node + at any distance. This does not make the sender authentic, which nothing + short of a wire format change can do. Rejected signals are counted as + unknown-session rejections, and additionally on four new error-signal + counters visible through `show routing`, `show metrics` and the fipstop + routing pane: `unbound_coords`, `unbound_broken` and `unbound_mtu` give the + refused count per signal type, against the existing per-type arrival + counters as the denominator, and `unbound_forged` counts the subset whose + 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 + the longest any secret lives here, every Noise handshake left its static and + ephemeral keypairs, its per-message Diffie-Hellman results and its chaining + key in freed memory, and each session's ChaCha20-Poly1305 keys were dropped + intact. Clearing now covers the retained cipher key on each cipher state, the + chaining key and the handshake hash, the 64-byte HKDF outputs and the two + session keys derived from them, the static and ephemeral keypairs a handshake + 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 + 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 + type's clearing destructor, so a configuration that already carried a key left + the superseded copy in the heap whenever a second source replaced it. **This + clears the copies the crate owns, not every copy that ever existed.** The + secp256k1 key types are copyable, so the compiler may duplicate them where no + code here can name them, which is why that library calls its own erase + non-secure. The hash and key-derivation states, and the cipher keys cached + inside `ring`, offer no clearing route at the versions pinned here and are + 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. + +#### Supply chain + +- 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 + associated-data field where standard Noise `EncryptAndHash` uses the handshake + hash `h`. The published tables named `Noise_IK_secp256k1_ChaChaPoly_SHA256` + and `Noise_XK_secp256k1_ChaChaPoly_SHA256` unqualified, so anyone auditing the + stack against the Noise specification had nothing telling them where to look. + Domain separation and Diffie-Hellman binding survive through the chaining key, + which is seeded from the protocol name and chained at every step; transcript + 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 + the associated data carries `h`. This is a correction to what is documented + and claimed; no code behaviour and nothing on the wire changed. + ## [0.4.1] - 2026-07-19 ### Changed diff --git a/README.md b/README.md index 4990ca06..98cd7ecf 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.5.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 73eec368..248e8cec 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 2d119510..ff7db8ab 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/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.