diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68f5d798..a8c4817e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -77,6 +77,8 @@ jobs: run: bash testing/check-action-pins.sh - name: Check every source comment resolves in-repo run: bash testing/check-comment-refs.sh + - name: Check no non-test code uses std 64-bit atomics + run: python3 testing/check-portable-atomics.py # Hermetic: synthetic ping functions, no containers, ~45s. Lives beside # the other two so both runners gate on it identically — putting it in # only one would create exactly the drift check-ci-parity.sh exists to @@ -89,6 +91,10 @@ jobs: # step with ci-local.sh's run_deb_version by hand. - name: Check the Debian version derived for tags and branches run: bash testing/check-deb-version.sh + # The unit-test jobs' flaky-test reporter, against recorded nextest + # reports. Kept in step with ci-local.sh's run_nextest_flaky by hand. + - name: Check the flaky-test reporter against its fixtures + run: bash testing/nextest-flaky/test.sh fmt: name: Format check @@ -338,10 +344,26 @@ jobs: - name: Install cargo-nextest uses: taiki-e/install-action@nextest + # The cache restores target/, including the previous run's report. Remove + # it so the flaky-test check below reads only this run's, and reports a + # missing one when nextest wrote none. + - name: Remove a cached nextest report + shell: bash + run: rm -f target/nextest/ci/junit.xml - name: Run unit tests run: cargo nextest run --all --profile ci + # The ci profile retries a failing test, so a test that passed only on + # retry leaves the job green. Annotate each one, and list it in the run + # summary, so a flake is seen without failing an unrelated run. The + # Linux job's two junit reporters ignore nextest's + # elements, and the macOS and Windows jobs have no reporter at all. + - name: Report tests that passed only on retry + if: success() || failure() + shell: bash + run: bash testing/check-nextest-flaky.sh target/nextest/ci/junit.xml + # The bind-success half. Every other unit-test leg runs unprivileged, so # `PacketSocket::open` cannot succeed on any of them and everything past # a successful bind — the post-store shutdown check, the `Present` arm of @@ -460,9 +482,26 @@ jobs: # one fails when the fixture is missing instead of skipping silently. echo "FIPS_TEST_REQUIRE_FIXTURES=1" >> "$GITHUB_ENV" + # The cache restores target/, including the previous run's report. Remove + # it so the flaky-test check below reads only this run's, and reports a + # missing one when nextest wrote none. + - name: Remove a cached nextest report + shell: bash + run: rm -f target/nextest/ci/junit.xml + - name: Run unit tests run: cargo nextest run --all --profile ci + # The ci profile retries a failing test, so a test that passed only on + # retry leaves the job green. Annotate each one, and list it in the run + # summary, so a flake is seen without failing an unrelated run. The + # Linux job's two junit reporters ignore nextest's + # elements, and the macOS and Windows jobs have no reporter at all. + - name: Report tests that passed only on retry + if: success() || failure() + shell: bash + run: bash testing/check-nextest-flaky.sh target/nextest/ci/junit.xml + # ───────────────────────────────────────────────────────────────────────────── # Job 2bb – Unit tests (musl) # @@ -574,10 +613,26 @@ jobs: - name: Install cargo-nextest uses: taiki-e/install-action@nextest + # The cache restores target/, including the previous run's report. Remove + # it so the flaky-test check below reads only this run's, and reports a + # missing one when nextest wrote none. + - name: Remove a cached nextest report + shell: bash + run: rm -f target/nextest/ci/junit.xml - name: Run unit tests run: cargo nextest run --all --profile ci + # The ci profile retries a failing test, so a test that passed only on + # retry leaves the job green. Annotate each one, and list it in the run + # summary, so a flake is seen without failing an unrelated run. The + # Linux job's two junit reporters ignore nextest's + # elements, and the macOS and Windows jobs have no reporter at all. + - name: Report tests that passed only on retry + if: success() || failure() + shell: bash + run: bash testing/check-nextest-flaky.sh target/nextest/ci/junit.xml + # ───────────────────────────────────────────────────────────────────────────── # Job 2d – PowerShell lint (Windows packaging scripts) # @@ -762,6 +817,11 @@ jobs: # per-distro images and no TUN. ~2-3 min. - suite: native-api type: native-api + # mDNS LAN discovery: two daemons on a user-defined bridge with LAN + # rendezvous on and no configured peers, which must find and peer + # with each other by mDNS alone. Seconds when healthy. + - suite: mdns + type: mdns # Moves a multi-homed node's default route between two live paths # while mesh traffic is in flight, and asserts the peering survives @@ -1028,6 +1088,17 @@ jobs: docker rm -f "$c" >/dev/null 2>&1 || true done + # ── mDNS LAN discovery ────────────────────────────────────────────── + # Reads FIPS_TEST_IMAGE, so it runs against the image this workflow + # built. Creates and removes its own docker network; the harness prints + # both nodes' discovery log lines itself when a check fails. + - name: Run mDNS LAN discovery test + if: matrix.type == 'mdns' + timeout-minutes: 10 + env: + FIPS_TEST_IMAGE: fips-test:latest + run: bash testing/mdns/test.sh + # ───────────────────────────────────────────────────────────────────────────── # Job 4 – The .deb the install suite installs # diff --git a/CHANGELOG.md b/CHANGELOG.md index 18cc5362..33bf9156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -355,6 +355,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the release. The tarball, artifact and `.deb` file names keep the tag's `-rcN`. +#### Windows + +- `install-service.ps1` stops when `\etc\fips\fips.key` exists on the system + drive and `C:\ProgramData\fips\fips.key` does not. A service that an + earlier release ran from `\etc\fips` reads only `C:\ProgramData\fips` + after the installer runs, and came up with a new identity with no warning. + Move the key and the settings it needs into `C:\ProgramData\fips`, or + delete it if you did not put it there, then run the installer again. +- The daemon warns when `fips.yaml` or `fips.key` in `\etc\fips` is present + but not used by the run, since a node that ran from them before an upgrade + now runs on another config or identity. + ### Deprecated #### Sessions and rekey @@ -363,6 +375,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 not supported, and the option is removed in v2. It still works as before; the configuration reference and the mesh-layer design now say so. +#### Windows + +- The config search's probe of `\etc\fips\fips.yaml` on the current drive. + The search still reads that file first and merges it under + `C:\ProgramData\fips\fips.yaml`, but any local user can create it, so the + daemon now warns when it loads a config from there, and from v0.6.0 the + search no longer looks there. Move the settings you need into + `C:\ProgramData\fips\fips.yaml`. + ### Removed - **Source-breaking for consumers of the library crate**: `ActivePeer` no @@ -522,6 +543,81 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 through `Requires=fips.service`, then restarted only fips, so `.fips` resolution stayed down and the gateway stayed stopped until started by hand. +#### OpenWrt + +- dnsmasq forwards `.fips` to fips-gateway only while the gateway is + listening, and back to the daemon's resolver whenever the gateway exits. A + gateway that failed to start, on a config it could not parse or on an error + after binding its DNS port, left every LAN client's `.fips` lookups going to + a port nothing held until the gateway was started by hand. +- A package built from the OpenWrt SDK feed `Makefile` carries the released + packages' maintainer scripts. It installs with fips-gateway disabled and + stopped, keeps the gateway's state across an upgrade, and keeps an edited + `/etc/fips/fips.yaml`. The SDK's package scan also no longer stops on the + `Makefile`'s architecture check, which had kept the package out of the build. + +#### Sessions and rekey + +- A link rekey whose msg2 is lost now completes on the initiator's next msg1 + resend. The responder refused every resend while it held the session it had + answered with, so the initiator abandoned its cycles and the link went + without rekeying until the responder's 120 s hold retired that session. The + responder now keeps the msg2 it sent and answers a resend of the same msg1 + with it, on the peer's established address only; any other msg1 is still + refused while the session is held. The wire format is unchanged. +- Link quality estimates no longer freeze after a link rekey. Frames the peer + sent on the old session just before it switched were counted into the new + session's MMP state, so one node's loss estimate, or the other's SRTT and + loss estimate, could stay fixed for most of a rekey interval, and link cost + and parent selection used the stale values. Those frames are still + delivered, but no longer feed MMP, and a ReceiverReport they carry is + dropped. They also no longer hold the link alive: after a rekey the + link-dead timer runs from the switch until a frame on the new session + arrives. +- A forged FSP SessionMsg3 or SessionSetup can no longer split a session's + key epochs during a rekey. Either one, delivered under the peer's address + after the peer had read this node's SessionAck, discarded the handshake the + peer's genuine msg3 needed; the peer then cut over to keys this node never + derived, and frames from it stopped decoding until a later rekey. An + unreadable msg3 now leaves the handshake in place, and a setup arriving + while a handshake the peer armed awaits its msg3 is dropped and counted as + `rekey_held`. A genuine retry that meets such a handshake completes one + handshake timeout later. +- An unreadable SessionMsg3 no longer discards the half-open session of an + initial handshake, which left the initiator's genuine msg3 to an unknown + session. + +#### Windows + +- The ZIP's `README.txt` lists `\etc\fips\fips.yaml` as the first file a + foreground run reads, which it omitted, and says to stop the service before + rerunning `install-service.ps1` to upgrade: with the service running, the + installer fails copying `fips.exe`. + +### Security + +#### Sessions and rekey + +- A copy of a peer's link rekey msg1 can no longer stop link key rotation. A + msg1 carries nothing that ties it to one rekey, so one captured off the + network and replayed after its rekey had completed was taken as a new + request: the node held a session nobody could adopt, refused the peer's + genuine rekeys and skipped its own until the 120 s hold expired, and one + replay per hold kept rotation stopped. The node now remembers the msg1s of + the last 256 rekeys it answered for each peer and refuses a copy of one. A + msg1 from an older rekey, or one captured before the peering last formed + while the peer kept running, is not recognized; closing that needs a wire + change. +- A msg1 from an established peer is answered at the peer's established + address, not at the address it came from. This covers the rekey msg2 and + the link setup msg2 resent for a duplicate msg1 in the 30 s after a link is + formed or rekeyed. Anyone holding a copy of a peer's msg1 could have the + node send a msg2 to an address of their choosing. A peer whose address + changed is answered at the old one until its next frame from the new + address arrives. A msg1 from a node this one holds no link with, or one + that carries a different startup epoch, still starts a new link and is + answered at its source, as any new connection is. + ## [0.5.2] - 2026-09-28 ### Added diff --git a/docs/tutorials/deploy-fips-gateway.md b/docs/tutorials/deploy-fips-gateway.md index 20474bd4..52adfdb1 100644 --- a/docs/tutorials/deploy-fips-gateway.md +++ b/docs/tutorials/deploy-fips-gateway.md @@ -176,7 +176,10 @@ Behind that single command, the init script the LAN's port 53 are forwarded to the gateway's loopback listener on port 5365 instead of going straight to the daemon's resolver on port 5354. (Dnsmasq still owns 53; the gateway sits - in front of the daemon for `.fips` only.) + in front of the daemon for `.fips` only.) The switch happens once + the gateway is listening, and dnsmasq goes back to port 5354 + whenever the gateway exits, so a gateway that fails to start + leaves `.fips` resolving through the daemon. 3. **Adds a global-scope IPv6 prefix** to `br-lan`. Without a non-ULA address on the local interface, Android and Chrome suppress AAAA queries entirely — they assume the LAN has no diff --git a/packaging/openwrt-ipk/Makefile b/packaging/openwrt-ipk/Makefile index cabd0380..a3fdbfa7 100644 --- a/packaging/openwrt-ipk/Makefile +++ b/packaging/openwrt-ipk/Makefile @@ -38,7 +38,10 @@ else ifeq ($(ARCH),arm) # OpenWrt ARM targets predominantly use hardfloat ABI. # Override RUST_TARGET in your build if your target uses softfloat. RUST_TARGET:=arm-unknown-linux-musleabihf -else +else ifeq ($(DUMP),) + # Not while OpenWrt scans package metadata (DUMP=1): the scan does not read + # the target config, so ARCH is empty then, and stopping here would leave the + # package out of the build entirely. $(error Unsupported architecture: $(ARCH). Add a RUST_TARGET mapping in packaging/openwrt-ipk/Makefile.) endif @@ -107,7 +110,8 @@ define Package/fips/install $(INSTALL_BIN) $(CURDIR)/files/etc/init.d/fips $(1)/etc/init.d/fips $(INSTALL_BIN) $(CURDIR)/files/etc/init.d/fips-gateway $(1)/etc/init.d/fips-gateway - # Default config — installed as CONF so opkg will not overwrite it on upgrade + # Default config, mode 0600. Package/fips/conffiles below is what keeps an + # edited copy across an upgrade. $(INSTALL_DIR) $(1)/etc/fips $(INSTALL_CONF) $(CURDIR)/files/etc/fips/fips.yaml $(1)/etc/fips/fips.yaml @@ -132,4 +136,26 @@ define Package/fips/install $(INSTALL_DATA) $(CURDIR)/files/lib/upgrade/keep.d/fips $(1)/lib/upgrade/keep.d/fips endef +# A user's edits to the config survive an upgrade. +define Package/fips/conffiles +/etc/fips/fips.yaml +endef + +# Maintainer scripts, read from scripts/ so this package runs the same +# postinst and prerm bodies that build-ipk.sh and build-apk.sh install. OpenWrt +# wraps those two in its generated scripts, around default_postinst and +# default_prerm. The preinst is used only here: it keeps fips-gateway disabled +# and stopped through default_postinst's enable-and-start loop. +define Package/fips/preinst +$(file < $(CURDIR)/scripts/preinst) +endef + +define Package/fips/postinst +$(file < $(CURDIR)/scripts/postinst) +endef + +define Package/fips/prerm +$(file < $(CURDIR)/scripts/prerm) +endef + $(eval $(call BuildPackage,fips)) diff --git a/packaging/openwrt-ipk/README.md b/packaging/openwrt-ipk/README.md index 30f84c22..fac25610 100644 --- a/packaging/openwrt-ipk/README.md +++ b/packaging/openwrt-ipk/README.md @@ -95,11 +95,12 @@ make package/fips/compile V=s The resulting `.ipk` is placed in `bin/packages//`. -A package built from this `Makefile` carries none of the maintainer scripts in -`scripts/`. Those scripts enable and start `fips` on install and implement the -gateway-enablement and upgrade behavior described below, so that description -does not cover a package built this way. Released packages are built by -`build-ipk.sh` (and `../openwrt-apk/build-apk.sh`), which install those scripts. +Installed on a router, the package enables and starts `fips` and leaves +`fips-gateway` disabled. Built into a firmware image, it is different: the +maintainer scripts do nothing at image build time, and the image build enables +every init script a package ships, `fips-gateway` included. To build an image +with the gateway off, pass `DISABLED_SERVICES="fips-gateway"` to the image +builder's `make image`. ### 4. Pin the source version diff --git a/packaging/openwrt-ipk/files/etc/init.d/fips-gateway b/packaging/openwrt-ipk/files/etc/init.d/fips-gateway index 2c12326a..bc897b79 100755 --- a/packaging/openwrt-ipk/files/etc/init.d/fips-gateway +++ b/packaging/openwrt-ipk/files/etc/init.d/fips-gateway @@ -12,6 +12,10 @@ USE_PROCD=1 START=96 STOP=09 +# procd runs "supervise" (below) rather than the gateway itself. +EXTRA_COMMANDS="supervise" +EXTRA_HELP="\tsupervise Run the gateway, pointing dnsmasq at it while it listens (run by procd)\n" + PROG=/usr/bin/fips-gateway CONFIG=/etc/fips/fips.yaml @@ -21,6 +25,17 @@ CONFIG=/etc/fips/fips.yaml GW_DNS_DEFAULT=5365 # Port the FIPS daemon DNS listens on. DAEMON_DNS_PORT=5354 +# Held while dnsmasq's .fips upstream is changed, so a stopping gateway's swap +# back to the daemon and a starting gateway's swap to itself cannot interleave. +DNS_LOCK=/var/lock/fips-gateway-dns.lock +# The pid of the gateway the current supervise runs, so an exiting instance +# can tell its successor's socket from anything else holding the port. +GW_PIDFILE=/var/run/fips-gateway.pid +# Seconds a stopping gateway gets after SIGTERM before it is killed; under +# procd's own 5 s, after which procd kills only the supervise shell. +GW_KILL_AFTER=3 +# Left by the package's preinst when the gateway is not enabled; see below. +INSTALL_HOLD=/var/run/fips-gateway-install-hold # Global-scope IPv6 prefix assigned to br-lan so Android/Chrome clients # believe they have full IPv6 and actually send AAAA queries. @@ -28,6 +43,18 @@ DAEMON_DNS_PORT=5354 GLOBAL_PREFIX="2001:2:f1b5::1/64" start_service() { + # OpenWrt's generic postinst enables and starts every init script a + # package ships. A package built from the SDK feed Makefile runs it, and + # its preinst leaves this hold unless the gateway was enabled, so that + # installing or upgrading the package does not switch the gateway on. + # Honoured once. + if [ -e "$INSTALL_HOLD" ]; then + rm -f "$INSTALL_HOLD" + disable + logger -t fips-gateway "left disabled and stopped by the package installation" + return 0 + fi + # The gateway daemon exits when gateway.enabled is not true, so without # this check starting a disabled gateway would still take dnsmasq's .fips # upstream away from the daemon and point it at a port nothing listens on. @@ -42,13 +69,6 @@ start_service() { # Load conntrack module for /proc/net/nf_conntrack. modprobe nf_conntrack 2>/dev/null || true - # Redirect dnsmasq .fips forwarding from the daemon (5354) to the port the - # gateway listens on, so LAN clients get virtual IPs instead of raw mesh - # addresses. Done early and synchronously so dnsmasq is ready before the - # gateway starts accepting DNS queries. - dnsmasq_swap_fips_upstream "$(gateway_dns_port)" - sleep 1 - # Add a global-scope IPv6 prefix to br-lan so Android/Chrome clients # send AAAA queries (they suppress AAAA when only ULA addresses exist). # Also set ra_default=2 so odhcpd advertises a default route even @@ -59,8 +79,12 @@ start_service() { # learn a route to the pool automatically. gateway_add_ra_route + # dnsmasq's .fips forwarding moves to the gateway only once the gateway + # holds its DNS port, and back to the daemon (5354) whenever the gateway + # exits, so a gateway that fails to start, before or after binding, does + # not leave LAN clients' .fips lookups going to a dead port. See supervise. procd_open_instance - procd_set_param command "$PROG" --config "$CONFIG" + procd_set_param command /etc/init.d/fips-gateway supervise procd_set_param respawn 3600 5 5 procd_set_param stdout 1 procd_set_param stderr 1 @@ -69,7 +93,7 @@ start_service() { stop_service() { # Restore dnsmasq .fips forwarding back to the daemon. - dnsmasq_swap_fips_upstream "$DAEMON_DNS_PORT" + dns_locked dnsmasq_swap_fips_upstream "$DAEMON_DNS_PORT" # Remove the RA route for the virtual IP pool. gateway_remove_ra_route @@ -82,6 +106,119 @@ reload_service() { restart } +# Run the gateway as procd's instance. dnsmasq's .fips upstream is pointed at +# the gateway's DNS port once the gateway itself has that port bound, and back +# at the daemon's port when the gateway exits for any reason: a config it +# cannot parse, a port it cannot bind, a failure after binding, a crash, or the +# SIGTERM procd sends to stop it. That SIGTERM is passed on so the gateway can +# remove its NAT table and routes, and a gateway still running $GW_KILL_AFTER +# seconds later is killed, because procd kills only this shell. The exit +# status is the gateway's, so procd's respawn sees the gateway's failures. +supervise() { + local port pid="" watcher rc stopping="" killer="" + + port="$(gateway_dns_port)" + # Set before the gateway starts, so a stop that arrives first still + # reaches it. + trap 'stopping=1; gateway_stop' TERM INT + "$PROG" --config "$CONFIG" & + pid=$! + mkdir -p "${GW_PIDFILE%/*}" 2>/dev/null + echo "$pid" > "$GW_PIDFILE" + [ -n "$stopping" ] && gateway_stop + + # Polled from a child of its own so this shell sits in wait and reaps the + # gateway the moment it exits; a polling loop here would see an unreaped + # gateway as still alive. + ( + while kill -0 "$pid" 2>/dev/null; do + dns_locked gateway_dns_claim "$port" "$pid" && exit 0 + sleep 1 + done + exit 0 + ) & + watcher=$! + + # A trapped signal ends wait early, so wait until the gateway is gone. + rc=0 + wait "$pid" || rc=$? + while kill -0 "$pid" 2>/dev/null; do + rc=0 + wait "$pid" || rc=$? + done + [ -n "$killer" ] && kill "$killer" 2>/dev/null + # The watcher has nothing left to do, and would otherwise sleep out its + # poll; any change it is part way through finishes under the lock first. + kill "$watcher" 2>/dev/null + wait "$watcher" + + dns_locked gateway_dns_release "$port" "$pid" + [ "$(cat "$GW_PIDFILE" 2>/dev/null)" = "$pid" ] && rm -f "$GW_PIDFILE" + logger -t fips-gateway "gateway exited with status $rc" + return "$rc" +} + +# supervise's stop: SIGTERM to the gateway now, SIGKILL if it is still running +# $GW_KILL_AFTER seconds later. Reads and sets supervise's pid and killer. +gateway_stop() { + [ -n "$pid" ] || return 0 + kill -TERM "$pid" 2>/dev/null + if [ -z "$killer" ]; then + ( sleep "$GW_KILL_AFTER"; kill -KILL "$pid" 2>/dev/null ) & + killer=$! + fi + return 0 +} + +# Run "$@" holding $DNS_LOCK. The lock is released when the subshell, and +# every process that inherited its descriptor, has exited. +dns_locked() { + mkdir -p "${DNS_LOCK%/*}" 2>/dev/null + ( + flock 9 || logger -t fips-gateway "could not lock $DNS_LOCK; changing dnsmasq anyway" + "$@" + ) 9>"$DNS_LOCK" +} + +# Succeed when process $2 itself holds a UDP socket bound to port $1, on any +# address, IPv4 or IPv6: a socket inode from /proc/net/udp{,6} for that port +# is among the process's open descriptors. Another process holding the port, +# such as an mDNS responder on 5353, does not count. +gateway_dns_held_by() { + local hex inode fd + [ -n "$2" ] || return 1 + hex="$(printf '%04X' "$1")" + for inode in $(cat /proc/net/udp /proc/net/udp6 2>/dev/null | + awk -v want=":$hex" 'substr($2, length($2) - 4) == want { print $10 }'); do + for fd in /proc/"$2"/fd/*; do + [ "$(readlink "$fd" 2>/dev/null)" = "socket:[$inode]" ] && return 0 + done + done + return 1 +} + +# Point dnsmasq at the gateway's port $1 once gateway $2 holds it. Fails, +# changing nothing, until then. +gateway_dns_claim() { + gateway_dns_held_by "$1" "$2" || return 1 + dnsmasq_swap_fips_upstream "$1" + logger -t fips-gateway "gateway DNS is listening on port $1; dnsmasq forwards .fips to it" + return 0 +} + +# Point dnsmasq back at the daemon after gateway $2 on port $1 has exited, +# unless the gateway procd started in its place (the pid in $GW_PIDFILE) holds +# the port already; that one claims dnsmasq itself. +gateway_dns_release() { + local next + next="$(cat "$GW_PIDFILE" 2>/dev/null)" + if [ -n "$next" ] && [ "$next" != "$2" ] && gateway_dns_held_by "$1" "$next"; then + return 0 + fi + dnsmasq_swap_fips_upstream "$DAEMON_DNS_PORT" + return 0 +} + # Extract the gateway "enabled" flag from fips.yaml. # Prints the value indented under the top-level "gateway:" block, or nothing # when there is no such block. diff --git a/packaging/openwrt-ipk/scripts/postinst b/packaging/openwrt-ipk/scripts/postinst index 18361729..68f4ab83 100755 --- a/packaging/openwrt-ipk/scripts/postinst +++ b/packaging/openwrt-ipk/scripts/postinst @@ -23,8 +23,24 @@ # Under apk, a fresh install runs this as post-install and the gateway stays # off. An upgrade runs it as post-upgrade, after the .apk pre-upgrade script # (the prerm body) has stopped the services and left the marker. +# +# A package built from the SDK feed Makefile runs this body from OpenWrt's +# generated postinst, beside a generic loop that enables and starts every init +# script; the preinst there leaves a hold that keeps the gateway out of that +# loop. When this body decides the gateway is to be enabled, it removes the +# hold first. See scripts/preinst. +# +# The upgrade marker stays in /tmp: an installed 0.5.2 package's prerm writes +# it there, and this script must find it to leave a disabled gateway disabled. + +# An image build (OpenWrt's image builder or buildroot) runs this on the build +# host with IPKG_INSTROOT naming the image's root. Nothing here may touch the +# host, so it does nothing there; see the README for the gateway in images. + +[ -n "${IPKG_INSTROOT:-}" ] && exit 0 UPGRADE_MARKER=/tmp/fips-prerm-upgrade +INSTALL_HOLD=/var/run/fips-gateway-install-hold # Run first-boot UCI setup (the script deletes itself when done). if [ -x /etc/uci-defaults/90-fips-setup ]; then @@ -38,6 +54,7 @@ if [ "${PKG_UPGRADE:-0}" = "1" ]; then if [ -e "$UPGRADE_MARKER" ]; then rm -f "$UPGRADE_MARKER" else + rm -f "$INSTALL_HOLD" /etc/init.d/fips-gateway enable fi diff --git a/packaging/openwrt-ipk/scripts/preinst b/packaging/openwrt-ipk/scripts/preinst new file mode 100755 index 00000000..96b11c20 --- /dev/null +++ b/packaging/openwrt-ipk/scripts/preinst @@ -0,0 +1,43 @@ +#!/bin/sh +# Maintainer script run before the FIPS package's files are unpacked. +# +# Used only by the OpenWrt SDK feed Makefile, as the package's preinst under +# opkg and its pre-install and pre-upgrade scripts under apk. build-ipk.sh and +# build-apk.sh ship a postinst of their own and do not need it. +# +# A package built in the SDK gets OpenWrt's generated postinst, whose +# default_postinst enables every init script the package installs on a fresh +# install and starts every one on every install and upgrade. The package's own +# postinst body runs before that loop under opkg and after it under apk, so the +# body cannot keep the gateway off by itself. Instead, unless the gateway is +# enabled now, this leaves a hold that init.d/fips-gateway honours once: its +# start_service removes the hold, disables the service and starts nothing. A +# fresh install therefore leaves the gateway disabled and stopped, and an +# upgrade does not start a gateway the operator disabled. +# +# An apk upgrade runs no script of the outgoing package, so nothing else stops +# the services or tells the postinst body that enablement survived the upgrade. +# This does what the outgoing package's prerm does on an opkg upgrade. + +# An image build (OpenWrt's image builder or buildroot) runs this on the build +# host with IPKG_INSTROOT naming the image's root. Nothing here may touch the +# host, so it does nothing there; see the README for the gateway in images. + +[ -n "${IPKG_INSTROOT:-}" ] && exit 0 + +INSTALL_HOLD=/var/run/fips-gateway-install-hold +UPGRADE_MARKER=/tmp/fips-prerm-upgrade + +# opkg passes "install" or "upgrade "; apk passes versions only. +if [ "${PKG_UPGRADE:-0}" = "1" ] && [ "${1:-}" != "upgrade" ]; then + : > "$UPGRADE_MARKER" 2>/dev/null || true + /etc/init.d/fips-gateway stop 2>/dev/null || true + /etc/init.d/fips stop 2>/dev/null || true +fi + +if ! /etc/init.d/fips-gateway enabled 2>/dev/null; then + mkdir -p "${INSTALL_HOLD%/*}" 2>/dev/null + : > "$INSTALL_HOLD" 2>/dev/null || true +fi + +exit 0 diff --git a/packaging/openwrt-ipk/scripts/prerm b/packaging/openwrt-ipk/scripts/prerm index 09dcf365..c3911d7e 100755 --- a/packaging/openwrt-ipk/scripts/prerm +++ b/packaging/openwrt-ipk/scripts/prerm @@ -10,10 +10,21 @@ # because nothing records it anywhere else, so an upgrade only stops them and # leaves a marker telling the incoming postinst that enablement survived. # A real removal stops and disables both, as before. +# +# A package built from the SDK feed Makefile runs this body from OpenWrt's +# generated prerm, which passes the script's own path first, so "upgrade" is +# then the second argument. opkg exports PKG_UPGRADE=1 for an upgrade either +# way, and apk runs this only on a removal. + +# An image build (OpenWrt's image builder or buildroot) runs this on the build +# host with IPKG_INSTROOT naming the image's root. Nothing here may touch the +# host, so it does nothing there; see the README for the gateway in images. + +[ -n "${IPKG_INSTROOT:-}" ] && exit 0 UPGRADE_MARKER=/tmp/fips-prerm-upgrade -if [ "$1" = "upgrade" ]; then +if [ "${1:-}" = "upgrade" ] || [ "${PKG_UPGRADE:-0}" = "1" ]; then : > "$UPGRADE_MARKER" 2>/dev/null || true /etc/init.d/fips-gateway stop 2>/dev/null || true /etc/init.d/fips stop 2>/dev/null || true diff --git a/packaging/windows/build-zip.ps1 b/packaging/windows/build-zip.ps1 index a098fff4..d54c64e2 100644 --- a/packaging/windows/build-zip.ps1 +++ b/packaging/windows/build-zip.ps1 @@ -83,6 +83,13 @@ Windows Service: # Install (requires Administrator) powershell -File install-service.ps1 + # Upgrade: stop the service first, or the installer fails + # copying fips.exe after it has already changed the config + # directory. Then rerun the installer and start the service. + sc stop fips + powershell -File install-service.ps1 + sc start fips + # Manage sc start fips sc stop fips @@ -125,10 +132,26 @@ Configuration: saying so, or may fail with an access error. A foreground run takes -c , or reads + \etc\fips\fips.yaml on the current drive, then C:\ProgramData\fips\fips.yaml and then, as per-user overrides the service does not read, %APPDATA%\fips\fips.yaml, %USERPROFILE%\.fips.yaml and .\fips.yaml. The key file sits - beside the last config loaded. + beside the last config loaded. \etc\fips\fips.yaml was the + system config of earlier releases, and any local user can + create it; the daemon warns when it loads it, and from v0.6.0 + the search no longer looks there. Move what you need from it + into C:\ProgramData\fips\fips.yaml and delete it. + + A service that earlier releases ran from \etc\fips reads + only C:\ProgramData\fips once install-service.ps1 has run, + so it loses that config and may come up with a new identity. + The installer stops if it finds \etc\fips\fips.key with no + fips.key in C:\ProgramData\fips. If the key is this node's, + move it there, carry the settings you need from + \etc\fips\fips.yaml, node.identity.persistent: true among + them, into C:\ProgramData\fips\fips.yaml, delete + \etc\fips\fips.yaml, and run the installer again. If you did + not put the key there, delete it. fipsctl keygen writes to C:\ProgramData\fips by default and needs an elevated prompt. Run install-service.ps1 before it: diff --git a/packaging/windows/install-service.ps1 b/packaging/windows/install-service.ps1 index ee293629..21a04c81 100644 --- a/packaging/windows/install-service.ps1 +++ b/packaging/windows/install-service.ps1 @@ -134,9 +134,9 @@ Write-Host " Restricted $ConfigDir to SYSTEM and Administrators" # the system drive, where any local user can create files, and the service # still reads a file there when it is missing from the config directory. # Stop rather than enforce, or silently drop, a list nobody has reviewed. -$legacyAclDir = "$env:SystemDrive\etc\fips" +$legacyDir = "$env:SystemDrive\etc\fips" foreach ($name in @("peers.allow", "peers.deny")) { - $legacy = Join-Path $legacyAclDir $name + $legacy = Join-Path $legacyDir $name $current = "$ConfigDir\$name" if ((Test-Path -LiteralPath $legacy) -and -not (Test-Path -LiteralPath $current)) { Write-Error "$legacy exists and $current does not, so the service would enforce the old file. Earlier releases read it, and any local user can write there. Review it, then move it to $current or delete it, and run install-service.ps1 again." @@ -144,6 +144,17 @@ foreach ($name in @("peers.allow", "peers.deny")) { } } +# A service that earlier releases ran from \etc\fips reads only $ConfigDir +# once FIPS_CONFIG is set below, and would come up with a new identity. Stop +# until the key is moved into $ConfigDir or deleted. The installer does not +# move it itself: any local user can write \etc\fips, so a key there may not +# be this node's. +$legacyKey = Join-Path $legacyDir "fips.key" +if ((Test-Path -LiteralPath $legacyKey) -and -not (Test-Path -LiteralPath "$ConfigDir\fips.key")) { + Write-Error "$legacyKey exists and $ConfigDir\fips.key does not, so a service that ran from \etc\fips would come up with a new identity. If that key is this node's, move it to $ConfigDir\fips.key and carry the settings you need from \etc\fips\fips.yaml, node.identity.persistent: true among them, into $ConfigDir\fips.yaml, then delete \etc\fips\fips.yaml, which the service warns about on every start until it is gone. If you did not put the key there, delete it: any local user can write to \etc\fips. Then run install-service.ps1 again." + exit 1 +} + # Empty peer ACL files allow every peer. Having them here means the service # never falls back to the \etc\fips copies. Empty them to clear a list; do not # delete them. diff --git a/src/bin/fips.rs b/src/bin/fips.rs index 52bfaf00..f89276c9 100644 --- a/src/bin/fips.rs +++ b/src/bin/fips.rs @@ -145,6 +145,11 @@ async fn run_daemon( #[cfg(windows)] fips::config::warn_legacy(&loaded_paths); + // Earlier releases could run a Windows node from \etc\fips, where any + // local user can create files; flag a config the search loaded there. + #[cfg(windows)] + fips::config::warn_legacy_etc_config(&loaded_paths); + // Identity provisioning: config nsec > key file > generate ephemeral let mut resolved = match resolve_identity(&config, &loaded_paths) { Ok(r) => r, @@ -164,6 +169,11 @@ async fn run_daemon( IdentitySource::Ephemeral => info!("Using ephemeral identity (new keypair each start)"), } + // Flag a config or key left in \etc\fips that this run did not use. After + // identity resolution, which decides whether that key was used. + #[cfg(windows)] + fips::config::warn_legacy_etc_unused(&loaded_paths, &resolved.source); + // Create node with resolved identity let mut config = config; // Take the nsec rather than move it: `ResolvedIdentity` clears its copy diff --git a/src/config/mod.rs b/src/config/mod.rs index e2462582..5d1f2c4d 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -172,6 +172,128 @@ pub fn warn_legacy(loaded: &[PathBuf]) { } } +/// A file in the drive-relative `\etc\fips` that a Windows run reports. +/// +/// Earlier releases could run a Windows node from `\etc\fips`, a directory +/// any local user can create files in, and the config search still probes +/// `fips.yaml` there. +#[cfg(any(windows, test))] +#[derive(Debug, PartialEq, Eq)] +enum EtcFile { + /// `fips.yaml` there was loaded, found by the config search or named + /// with `-c` or `FIPS_CONFIG`. + Loaded(PathBuf), + /// `fips.yaml` or `fips.key` is there and this run did not use it, so a + /// node that used to run from it now runs on another config or identity. + Unused(PathBuf), +} + +/// Whether two paths name the same file in the way Windows compares them: +/// either separator, any case, and with any drive prefix ignored. +/// +/// The drive-relative `/etc/fips\fips.yaml` the config search probes and an +/// explicit `C:\etc\fips\fips.yaml` given with `-c` or `FIPS_CONFIG` are +/// different `Path`s, since one has a drive prefix, and `Path` comparison is +/// case-sensitive while Windows file names are not. Comparing the text lets +/// the rule run on every platform. +#[cfg(any(windows, test))] +fn same_windows_path(a: &Path, b: &Path) -> bool { + fn key(path: &Path) -> String { + let text = path.to_string_lossy().replace('/', "\\"); + let text = text.strip_prefix(r"\\?\").unwrap_or(&text); + let text = match text.as_bytes() { + [drive, b':', ..] if drive.is_ascii_alphabetic() => &text[2..], + _ => text, + }; + text.to_ascii_lowercase() + } + key(a) == key(b) +} + +/// Decide what to report about `fips.yaml` and `fips.key` in `etc`. +/// +/// `loaded` is the list of config files the daemon loaded, and `key_used` +/// the key file its identity came from, if any. `present` reports whether a +/// file exists; nothing else about the files is looked at, and the key is +/// never read. +#[cfg(any(windows, test))] +fn etc_files( + loaded: &[PathBuf], + key_used: Option<&Path>, + etc: &Path, + present: impl Fn(&Path) -> bool, +) -> Vec { + let config = etc.join(CONFIG_FILENAME); + let key = etc.join(KEY_FILENAME); + let mut found = Vec::new(); + if let Some(path) = loaded.iter().find(|p| same_windows_path(p, &config)) { + found.push(EtcFile::Loaded(path.clone())); + } else if present(config.as_path()) { + found.push(EtcFile::Unused(config)); + } + if !key_used.is_some_and(|k| same_windows_path(k, &key)) && present(key.as_path()) { + found.push(EtcFile::Unused(key)); + } + found +} + +/// The key file this run's identity is held in: the one it loaded, or the one +/// it generated and saved. An identity from the config, or an ephemeral one, +/// uses no key file. +#[cfg(any(windows, test))] +fn key_in_use(identity: &IdentitySource) -> Option<&Path> { + match identity { + IdentitySource::KeyFile(path) | IdentitySource::Generated(path) => Some(path), + IdentitySource::Config | IdentitySource::Ephemeral => None, + } +} + +/// Warn about a config loaded from the drive-relative `\etc\fips`. +/// +/// Any local user may have written it, whether the config search found it or +/// it was named explicitly, and from v0.6.0 the search no longer looks +/// there. Called before identity resolution, so the warning is logged even +/// when a key the file supplies fails to resolve. +#[cfg(windows)] +pub fn warn_legacy_etc_config(loaded: &[PathBuf]) { + let etc = Path::new(LEGACY_SYSTEM_CONFIG_DIR); + // With nothing reported present, only a loaded config can be found. + for file in etc_files(loaded, None, etc, |_| false) { + if let EtcFile::Loaded(path) = file { + tracing::warn!( + path = %path.display(), + current = %Path::new(SYSTEM_CONFIG_DIR).join(CONFIG_FILENAME).display(), + "Config loaded from \\etc\\fips, where any local user can create files; \ + from v0.6.0 the config search no longer looks there. Move the settings \ + this node needs into C:\\ProgramData\\fips\\fips.yaml and delete the file" + ); + } + } +} + +/// Warn about `fips.yaml` or `fips.key` left in the drive-relative +/// `\etc\fips` and not used by this run. +/// +/// A node that ran from them before an upgrade now has a different config, +/// and possibly a different identity. Called after identity resolution, +/// which decides which key file the run uses. +#[cfg(windows)] +pub fn warn_legacy_etc_unused(loaded: &[PathBuf], identity: &IdentitySource) { + let etc = Path::new(LEGACY_SYSTEM_CONFIG_DIR); + for file in etc_files(loaded, key_in_use(identity), etc, Path::exists) { + if let EtcFile::Unused(path) = file { + tracing::warn!( + path = %path.display(), + current = %Path::new(SYSTEM_CONFIG_DIR).display(), + "File in \\etc\\fips is not used by this run; a node that ran from it \ + before now runs on another config or identity. If it is this node's, \ + move fips.yaml and fips.key into C:\\ProgramData\\fips and run \ + install-service.ps1 again; if not, delete it" + ); + } + } +} + /// Find an identity key stranded at the legacy system config directory. /// /// Adding a second system config directory to the search path moves the @@ -1065,7 +1187,7 @@ impl Config { // System config — /etc/fips is always probed so existing installs // keep working after an upgrade. - paths.push(PathBuf::from("/etc/fips").join(CONFIG_FILENAME)); + paths.push(PathBuf::from(LEGACY_SYSTEM_CONFIG_DIR).join(CONFIG_FILENAME)); // macOS and FreeBSD packaging install config under /usr/local/etc/fips, // and the Windows service installer under C:\ProgramData\fips; probe @@ -1973,6 +2095,153 @@ node: ); } + /// A presence check that reports exactly `files` as existing. + fn only(files: &[&Path]) -> impl Fn(&Path) -> bool { + let files: Vec = files.iter().map(|f| f.to_path_buf()).collect(); + move |p| files.iter().any(|f| f == p) + } + + #[test] + fn etc_files_reports_a_config_the_search_loaded_from_etc_fips() { + let etc = Path::new("/etc-legacy/fips"); + let planted = etc.join(CONFIG_FILENAME); + let system = Path::new("/sys-cfg/fips").join(CONFIG_FILENAME); + + assert_eq!( + etc_files( + &[planted.clone(), system.clone()], + None, + etc, + only(&[&planted]) + ), + [EtcFile::Loaded(planted.clone())], + "a config loaded from \\etc\\fips under the real one must be reported as loaded" + ); + assert_eq!( + etc_files( + std::slice::from_ref(&planted), + Some(&etc.join(KEY_FILENAME)), + etc, + only(&[&planted, &etc.join(KEY_FILENAME)]) + ), + [EtcFile::Loaded(planted)], + "a node running from \\etc\\fips is told the config goes away, and its key, \ + which it uses, is not reported" + ); + } + + #[test] + fn etc_files_reports_a_config_and_key_left_in_etc_fips_that_the_run_did_not_use() { + let etc = Path::new("/etc-legacy/fips"); + let old_config = etc.join(CONFIG_FILENAME); + let old_key = etc.join(KEY_FILENAME); + let system = Path::new("/sys-cfg/fips"); + let loaded = [system.join(CONFIG_FILENAME)]; + + assert_eq!( + etc_files(&loaded, None, etc, only(&[&old_config, &old_key])), + [ + EtcFile::Unused(old_config.clone()), + EtcFile::Unused(old_key.clone()) + ], + "after an upgrade to FIPS_CONFIG the old config and key must both be reported" + ); + assert_eq!( + etc_files( + &loaded, + Some(&system.join(KEY_FILENAME)), + etc, + only(&[&old_key]) + ), + [EtcFile::Unused(old_key)], + "a key left in \\etc\\fips while the identity comes from another key file \ + must be reported" + ); + assert_eq!( + etc_files(&loaded, None, etc, only(&[&old_config])), + [EtcFile::Unused(old_config)], + "a config left in \\etc\\fips and not loaded must be reported" + ); + } + + #[test] + fn same_windows_path_ignores_the_drive_the_separator_and_case() { + let probed = PathBuf::from("/etc/fips").join(CONFIG_FILENAME); + for explicit in [ + r"C:\etc\fips\fips.yaml", + r"c:/ETC/Fips/FIPS.yaml", + r"\\?\C:\etc\fips\fips.yaml", + r"\etc\fips\fips.yaml", + ] { + assert!( + same_windows_path(&probed, Path::new(explicit)), + "{explicit} must match the probed /etc/fips\\fips.yaml" + ); + } + for other in [ + r"C:\ProgramData\fips\fips.yaml", + r"C:\etc\fips\fips.key", + r"C:etc\fips\fips.yaml", + r"etc\fips\fips.yaml", + r"C:\x\etc\fips\fips.yaml", + ] { + assert!( + !same_windows_path(&probed, Path::new(other)), + "{other} must not match the probed /etc/fips\\fips.yaml" + ); + } + } + + #[test] + fn etc_files_reports_an_explicit_drive_path_to_etc_fips_as_loaded_and_its_key_as_used() { + let etc = PathBuf::from("/etc/fips"); + let explicit = PathBuf::from(r"C:\etc\fips\fips.yaml"); + let key = PathBuf::from(r"C:\etc\fips\fips.key"); + + assert_eq!( + etc_files(std::slice::from_ref(&explicit), Some(&key), &etc, |_| true), + [EtcFile::Loaded(explicit)], + "a config named as C:\\etc\\fips\\fips.yaml is the legacy file, loaded, and \ + the key beside it is in use" + ); + } + + #[test] + fn key_in_use_counts_a_key_generated_this_run_as_well_as_one_loaded() { + let key = PathBuf::from("/etc-legacy/fips").join(KEY_FILENAME); + assert_eq!( + key_in_use(&IdentitySource::KeyFile(key.clone())), + Some(key.as_path()) + ); + assert_eq!( + key_in_use(&IdentitySource::Generated(key.clone())), + Some(key.as_path()), + "a key generated and saved this run is the one in use, not an unused leftover" + ); + assert_eq!(key_in_use(&IdentitySource::Config), None); + assert_eq!(key_in_use(&IdentitySource::Ephemeral), None); + } + + #[test] + fn etc_files_reports_nothing_when_etc_fips_holds_neither_file() { + let etc = Path::new("/etc-legacy/fips"); + let system = Path::new("/sys-cfg/fips"); + let config = system.join(CONFIG_FILENAME); + let key = system.join(KEY_FILENAME); + + assert_eq!( + etc_files( + std::slice::from_ref(&config), + Some(&key), + etc, + only(&[&config, &key]) + ), + Vec::new(), + "files present only in the current directory must not be reported" + ); + assert_eq!(etc_files(&[], None, etc, only(&[])), Vec::new()); + } + #[test] fn windows_key_fallback_finds_a_key_in_the_old_appdata_dir() { let root = TempDir::new().unwrap(); diff --git a/src/instr/capture.rs b/src/instr/capture.rs index 71b27e1e..9d231187 100644 --- a/src/instr/capture.rs +++ b/src/instr/capture.rs @@ -10,11 +10,12 @@ //! connection is served by its own spawned task, so two simultaneous `on` //! requests are genuinely concurrent and must not both create a writer. +use portable_atomic::AtomicU64; use std::fs::File; use std::io::Write; use std::path::{Component, Path, PathBuf}; use std::sync::Mutex; -use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use super::recorder; diff --git a/src/instr/recorder.rs b/src/instr/recorder.rs index 491602ca..93bd51b6 100644 --- a/src/instr/recorder.rs +++ b/src/instr/recorder.rs @@ -9,8 +9,8 @@ //! `swap(0)`, so there are no "previous value" arrays to carry and the counters //! are per-interval by construction. +use portable_atomic::{AtomicU64, Ordering::Relaxed}; use std::sync::LazyLock; -use std::sync::atomic::{AtomicU64, Ordering::Relaxed}; use std::time::{Duration, Instant}; /// Measurement domain. Structural only: one variant today. diff --git a/src/native/mod.rs b/src/native/mod.rs index b7c65f25..57a772f4 100644 --- a/src/native/mod.rs +++ b/src/native/mod.rs @@ -81,11 +81,12 @@ mod unix_impl { use crate::config::NativeApiConfig; use crate::control::protocol::{Request, Response}; use crate::identity::{NodeAddr, decode_npub, encode_npub}; + use portable_atomic::AtomicU64; use secp256k1::XOnlyPublicKey; use std::collections::HashMap; use std::os::fd::{AsFd, OwnedFd}; use std::path::PathBuf; - use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use tokio::io::BufReader; use tokio::net::{UnixListener, UnixStream}; diff --git a/src/node/dataplane/connected_udp.rs b/src/node/dataplane/connected_udp.rs index b22419f8..090dd315 100644 --- a/src/node/dataplane/connected_udp.rs +++ b/src/node/dataplane/connected_udp.rs @@ -34,7 +34,7 @@ use crate::node::Node; #[cfg(any(target_os = "linux", target_os = "macos"))] use crate::transport::TransportHandle; #[cfg(any(target_os = "linux", target_os = "macos"))] -use std::sync::atomic::{AtomicU64, Ordering::Relaxed}; +use portable_atomic::{AtomicU64, Ordering::Relaxed}; #[cfg(any(target_os = "linux", target_os = "macos"))] use tracing::{debug, warn}; diff --git a/src/node/dataplane/encrypted.rs b/src/node/dataplane/encrypted.rs index 72aff90b..0e20af93 100644 --- a/src/node/dataplane/encrypted.rs +++ b/src/node/dataplane/encrypted.rs @@ -5,12 +5,33 @@ use crate::noise::NoiseError; use crate::proto::fmp::wire::{ EncryptedHeader, FLAG_CE, FLAG_KEY_EPOCH, FLAG_SP, strip_inner_header, }; +use crate::proto::link::LinkMessageType; use crate::transport::ReceivedPacket; use tracing::{debug, trace, warn}; /// Force-remove a peer after this many consecutive decryption failures. const DECRYPT_FAILURE_THRESHOLD: u32 = 20; +/// Which of a peer's link sessions authenticated an inbound frame. +/// +/// After a rekey cutover the previous session still decrypts during the +/// drain, so frames the peer sealed before it moved are delivered. They +/// describe the session the cutover retired, though, and the new session's +/// MMP state was reset at the cutover: a previous-session frame is not +/// counted by the MMP receiver or the spin bit, and a ReceiverReport it +/// carries is not processed. A frame authenticated by a pending session is +/// promoted to current before it is processed, so it is `Current`. +/// +/// The link-dead check reads the MMP receiver's last-received time, which the +/// cutover clears, so previous-session frames no longer hold the link alive: +/// after a cutover the link-dead timer runs from the cutover until a frame on +/// the new session arrives. Peer `touch` and link statistics still see them. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(in crate::node) enum LinkSlot { + Current, + Previous, +} + impl Node { /// Handle an encrypted frame (phase 0x0). /// @@ -137,6 +158,7 @@ impl Node { let sp_flag = header.flags & FLAG_SP != 0; self.process_authentic_fmp_plaintext( &node_addr, + LinkSlot::Current, packet.transport_id, &packet.remote_addr, packet.timestamp_ms, @@ -195,7 +217,7 @@ impl Node { // Decrypt: try current session first, then previous (drain fallback) let ciphertext = &packet.data[header.ciphertext_offset()..]; - let plaintext = { + let (plaintext, slot) = { let peer = self.peers.get_mut(&node_addr).unwrap(); let session = match peer.noise_session_mut() { Some(s) => s, @@ -215,7 +237,7 @@ impl Node { ) { Ok(p) => { peer.reset_decrypt_failures(); - p + (p, LinkSlot::Current) } Err(e) => { // Current session failed — try previous session (drain window) @@ -227,7 +249,7 @@ impl Node { ) { Ok(p) => { peer.reset_decrypt_failures(); - p + (p, LinkSlot::Previous) } Err(_) => { self.log_decrypt_failure(&node_addr, &header, &e); @@ -266,7 +288,9 @@ impl Node { let mut address_changed = false; if let Some(peer) = self.peers.get_mut(&node_addr) { - if let Some(mmp) = peer.mmp_mut() { + if slot == LinkSlot::Current + && let Some(mmp) = peer.mmp_mut() + { mmp.receiver.record_recv( header.counter, timestamp, @@ -299,7 +323,32 @@ impl Node { let _ = address_changed; // Dispatch to link message handler - self.dispatch_link_message(&node_addr, link_message, ce_flag) + self.dispatch_authentic(&node_addr, slot, link_message, ce_flag) + .await; + } + + /// Dispatch the link message of an authenticated frame. A ReceiverReport + /// on the previous session reports on this node's traffic in the session + /// a cutover retired, and is dropped rather than judged against the new + /// session's metrics; every other message is dispatched whichever session + /// carried it. + async fn dispatch_authentic( + &mut self, + node_addr: &crate::NodeAddr, + slot: LinkSlot, + link_message: &[u8], + ce_flag: bool, + ) { + if slot == LinkSlot::Previous + && link_message.first() == Some(&(LinkMessageType::ReceiverReport as u8)) + { + trace!( + peer = %self.peer_display_name(node_addr), + "Dropping a ReceiverReport carried on the previous link session" + ); + return; + } + self.dispatch_link_message(node_addr, link_message, ce_flag) .await; } @@ -353,6 +402,7 @@ impl Node { pub(in crate::node) async fn process_authentic_fmp_plaintext( &mut self, node_addr: &crate::NodeAddr, + slot: LinkSlot, transport_id: crate::transport::TransportId, remote_addr: &crate::transport::TransportAddr, packet_timestamp_ms: u64, @@ -381,7 +431,9 @@ impl Node { peer.link_stats_mut() .record_recv(packet_len, packet_timestamp_ms); peer.touch(packet_timestamp_ms); - if let Some(mmp) = peer.mmp_mut() { + if slot == LinkSlot::Current + && let Some(mmp) = peer.mmp_mut() + { mmp.receiver .record_recv(fmp_counter, inner_ts, packet_len, ce_flag, now_ms); let _spin_rtt = mmp.spin_bit.rx_observe(sp_flag, fmp_counter, now_ms); @@ -399,7 +451,7 @@ impl Node { let _ = address_changed; } let link_message = &fmp_plaintext[INNER_TIMESTAMP_LEN..]; - self.dispatch_link_message(node_addr, link_message, ce_flag) + self.dispatch_authentic(node_addr, slot, link_message, ce_flag) .await; } @@ -414,8 +466,23 @@ impl Node { let sp_flag = fallback.fmp_flags & FLAG_SP != 0; let plaintext = &fallback.packet_data[fallback.fmp_plaintext_offset ..fallback.fmp_plaintext_offset + fallback.fmp_plaintext_len]; + // The worker decrypted with the session registered under the + // frame's receiver index. Only the current session's index is + // current; any other is the previous session draining, or one + // already gone by the time the bounce is processed. + let current = self + .peers + .get(&fallback.source_node_addr) + .and_then(|p| p.our_index()) + .is_some_and(|idx| idx.as_u32() == fallback.receiver_idx); + let slot = if current { + LinkSlot::Current + } else { + LinkSlot::Previous + }; self.process_authentic_fmp_plaintext( &fallback.source_node_addr, + slot, fallback.transport_id, &fallback.remote_addr, fallback.timestamp_ms, diff --git a/src/node/decrypt_worker.rs b/src/node/decrypt_worker.rs index 10fb7bb6..a5d3e7aa 100644 --- a/src/node/decrypt_worker.rs +++ b/src/node/decrypt_worker.rs @@ -39,10 +39,10 @@ use crate::NodeAddr; use crate::transport::{TransportAddr, TransportId}; use crossbeam_channel::{Receiver, Sender, TrySendError, bounded}; +use portable_atomic::{AtomicU64, Ordering}; use ring::aead::{Aad, LessSafeKey, Nonce}; use std::collections::HashMap; use std::sync::Arc; -use std::sync::atomic::{AtomicU64, Ordering}; use tokio::sync::mpsc::UnboundedSender; use tracing::{debug, trace, warn}; @@ -150,6 +150,10 @@ pub(crate) struct DecryptFallback { /// MMP's 30-second link-dead timer fires even though packets /// are arriving fine. pub packet_len: usize, + /// The frame's receiver index: the index of the session that decrypted + /// it, so rx_loop can tell a current-session frame from one on the + /// previous session during a rekey drain. + pub receiver_idx: u32, pub fmp_counter: u64, pub fmp_flags: u8, /// Original received wire buffer, mutated in place by the FMP @@ -521,6 +525,7 @@ fn handle_job( remote_addr, timestamp_ms, packet_len, + receiver_idx: cache_key.1, fmp_counter, fmp_flags, packet_data, diff --git a/src/node/encrypt_worker.rs b/src/node/encrypt_worker.rs index e29dde73..1c94a268 100644 --- a/src/node/encrypt_worker.rs +++ b/src/node/encrypt_worker.rs @@ -513,8 +513,7 @@ impl EncryptWorkerPool { match self.senders[idx].try_push(job) { Ok(()) => {} Err(MacWorkerTryPushError::Full(job)) => { - static FULL_COUNT: std::sync::atomic::AtomicU64 = - std::sync::atomic::AtomicU64::new(0); + static FULL_COUNT: portable_atomic::AtomicU64 = portable_atomic::AtomicU64::new(0); let n = FULL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); if n < 8 || n.is_multiple_of(10000) { warn!( @@ -538,8 +537,7 @@ impl EncryptWorkerPool { match self.senders[idx].try_send(job) { Ok(()) => {} Err(TrySendError::Full(job)) => { - static FULL_COUNT: std::sync::atomic::AtomicU64 = - std::sync::atomic::AtomicU64::new(0); + static FULL_COUNT: portable_atomic::AtomicU64 = portable_atomic::AtomicU64::new(0); let n = FULL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); if n < 8 || n.is_multiple_of(10000) { warn!( @@ -571,7 +569,7 @@ struct MacSendFlowKey { #[derive(Default)] struct MacSequencedSendFlows { flows: Mutex>>, - last_prune_ms: std::sync::atomic::AtomicU64, + last_prune_ms: portable_atomic::AtomicU64, } #[cfg(target_os = "macos")] @@ -719,8 +717,8 @@ struct MacSequencedSendFlow { socket: AsyncUdpSocket, connected_socket: Option>, dest_addr: SocketAddr, - next_seq: std::sync::atomic::AtomicU64, - last_used_ms: std::sync::atomic::AtomicU64, + next_seq: portable_atomic::AtomicU64, + last_used_ms: portable_atomic::AtomicU64, state: Mutex, ready_cv: Condvar, space_cv: Condvar, @@ -763,8 +761,8 @@ impl MacSequencedSendFlow { socket, connected_socket, dest_addr, - next_seq: std::sync::atomic::AtomicU64::new(0), - last_used_ms: std::sync::atomic::AtomicU64::new(now_ms), + next_seq: portable_atomic::AtomicU64::new(0), + last_used_ms: portable_atomic::AtomicU64::new(now_ms), state: Mutex::new(MacSendFlowState::default()), ready_cv: Condvar::new(), space_cv: Condvar::new(), @@ -1386,8 +1384,8 @@ impl SendBackpressurePacer { return false; } - static SEND_BACKPRESSURE_COUNT: std::sync::atomic::AtomicU64 = - std::sync::atomic::AtomicU64::new(0); + static SEND_BACKPRESSURE_COUNT: portable_atomic::AtomicU64 = + portable_atomic::AtomicU64::new(0); let n = SEND_BACKPRESSURE_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); if n < 8 || n.is_multiple_of(100_000) { warn!( @@ -1493,8 +1491,8 @@ fn default_send_backpressure_drop_after() -> u32 { #[cfg(all(unix, not(target_os = "linux")))] fn record_udp_send_backpressure_drop(err: &std::io::Error) { - static SEND_BACKPRESSURE_DROP_COUNT: std::sync::atomic::AtomicU64 = - std::sync::atomic::AtomicU64::new(0); + static SEND_BACKPRESSURE_DROP_COUNT: portable_atomic::AtomicU64 = + portable_atomic::AtomicU64::new(0); let n = SEND_BACKPRESSURE_DROP_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); if n < 8 || n.is_multiple_of(100_000) { warn!( diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index a20454c4..d6c00362 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -14,8 +14,8 @@ use crate::peer::machine::{ }; use crate::proto::fmp::wire::{Msg1Header, Msg2Header, build_msg2}; use crate::proto::fmp::{ - EstablishSnapshot, EstablishView, InboundDecision, InboundReject, OutboundSnapshot, - PromotionResult, WireOutcome, cross_connection_winner, + EstablishSnapshot, EstablishView, InboundDecision, InboundReject, Msg1Digest, OutboundSnapshot, + PromotionResult, RekeyAnswer, WireOutcome, cross_connection_winner, }; use crate::transport::{Link, LinkDirection, LinkId, ReceivedPacket}; use crate::utils::index::SessionIndex; @@ -69,7 +69,7 @@ pub(in crate::node) enum Msg1Waiver { } impl EstablishView for Node { - fn establish_snapshot(&self, peer_addr: &NodeAddr) -> EstablishSnapshot { + fn establish_snapshot(&self, peer_addr: &NodeAddr, msg1: &Msg1Digest) -> EstablishSnapshot { let existing = self.peers.get(peer_addr); let max_peers = self.max_peers(); EstablishSnapshot { @@ -83,6 +83,8 @@ impl EstablishView for Node { .map(|p| p.pending_new_session().is_some()) .unwrap_or(false), rekey_in_progress: existing.map(|p| p.rekey_in_progress()).unwrap_or(false), + held_answer: existing.and_then(|p| p.rekey_answer().cloned()), + msg1_answered_before: existing.is_some_and(|p| p.answered_before(msg1)), existing_msg2: existing.and_then(|p| p.handshake_msg2().map(|m| m.to_vec())), at_max_peers: max_peers > 0 && self.peers.len() >= max_peers, has_pending_outbound_to_peer: self.connections().any(|(_, machine)| { @@ -310,6 +312,19 @@ impl Node { .is_none_or(|t| t.accept_connections()) } + /// The transport and address of `peer`'s established link, where a rekey + /// msg2 is sent whatever address its msg1 arrived from. + fn established_link( + &self, + peer: &NodeAddr, + ) -> Option<( + crate::transport::TransportId, + crate::transport::TransportAddr, + )> { + let p = self.peers.get(peer)?; + Some((p.transport_id()?, p.current_addr()?.clone())) + } + /// Handle handshake message 1 (phase 0x1). /// /// This creates a new inbound connection. Rate limiting is applied @@ -552,6 +567,7 @@ impl Node { remote_epoch: machine.conn_remote_epoch(), their_index: header.sender_idx, msg2_payload: msg2_response, + msg1_digest: Msg1Digest::of(&packet.data), }; // === PHASE C input === @@ -560,7 +576,7 @@ impl Node { // session age resolved here, the max-peers cap, our own address for the // tie-break). Taken before this connection is inserted into the // registry, matching the pre-refactor read points. - let est = self.establish_snapshot(&peer_node_addr); + let est = self.establish_snapshot(&peer_node_addr, &wire.msg1_digest); // === PHASE C: structured classification === // Evaluate the inbound decision once on a local establish leg and route @@ -598,7 +614,10 @@ impl Node { .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); } InboundDecision::Reject { - reason: reason @ (InboundReject::PendingSession | InboundReject::DualRekeyWon), + reason: + reason @ (InboundReject::PendingSession + | InboundReject::DualRekeyWon + | InboundReject::AnsweredBefore), } => { // Existing-peer rekey rejects: the classification took the // fresh-context fail path (no actions) and the local machine is @@ -613,6 +632,11 @@ impl Node { peer = %self.peer_display_name(&peer_node_addr), "Dual rekey initiation: we win (smaller addr), dropping their msg1" ), + InboundReject::AnsweredBefore => debug!( + peer = %self.peer_display_name(&peer_node_addr), + remote_addr = %packet.remote_addr, + "Rekey msg1 answered in an ended cycle, dropping the copy" + ), InboundReject::AtMaxPeers => unreachable!(), } // `conn`/`link_id` were never inserted into the registry, so the @@ -623,12 +647,16 @@ impl Node { InboundDecision::ResendMsg2 { msg2 } => { // Duplicate msg1 at the same epoch: the decision carries the // stored msg2 bytes and the inline resend below owns the send; - // the classification touched no state. + // the classification touched no state. It goes on the peer's + // established link, as a rekey msg2 does: a genuine duplicate + // comes from the address the peering was just formed with, + // while a copy can come from anywhere. debug_assert!(actions.is_empty()); if let Some(msg2) = msg2.as_deref() - && let Some(transport) = self.transports.get(&packet.transport_id) + && let Some((tid, addr)) = self.established_link(&peer_node_addr) + && let Some(transport) = self.transports.get(&tid) { - match transport.send(&packet.remote_addr, msg2).await { + match transport.send(&addr, msg2).await { Ok(_) => debug!( peer = %self.peer_display_name(&peer_node_addr), "Resent msg2 for duplicate msg1 (same epoch)" @@ -641,6 +669,32 @@ impl Node { } } } + InboundDecision::ResendRekeyMsg2 { peer, msg2 } => { + // A resend of the msg1 that armed the pending we hold: our + // msg2 was lost, so give the same answer again, on the peer's + // established link as the first answer went. + debug_assert!(actions.is_empty()); + if let Some((tid, addr)) = self.established_link(&peer) + && let Some(transport) = self.transports.get(&tid) + { + match transport.send(&addr, &msg2).await { + Ok(_) => debug!( + peer = %self.peer_display_name(&peer), + "Resent rekey msg2 for a resent msg1" + ), + Err(e) => debug!( + peer = %self.peer_display_name(&peer), + error = %e, + "Failed to resend rekey msg2" + ), + } + } else { + debug!( + peer = %self.peer_display_name(&peer), + "No established link to resend rekey msg2 on" + ); + } + } InboundDecision::RekeyRespond { peer, abandon_first, @@ -691,42 +745,64 @@ impl Node { } }; - // Send msg2 response using the new handshake. + // Send msg2 response using the new handshake, on the peer's + // established link rather than to the msg1's source. A copy + // of a msg1 authenticates as the peer from any address, so + // answering its source would reflect to an address the + // sender chose. A peer whose address changed is answered at + // the old one until a frame from the new address moves it. let wire_msg2 = build_msg2(our_new_index, wire.their_index, &wire.msg2_payload); - if let Some(transport) = self.transports.get(&packet.transport_id) { - match transport.send(&packet.remote_addr, &wire_msg2).await { - Ok(_) => { - debug!( - peer = %self.peer_display_name(&peer), - new_our_index = %our_new_index, - "Sent rekey msg2 response" - ); - } - Err(e) => { - warn!( - peer = %self.peer_display_name(&peer), - error = %e, - "Failed to send rekey msg2" - ); - let _ = self.index_allocator.free(our_new_index); - self.stats_mut() - .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); - return; - } + let sent = match self.established_link(&peer) { + Some((tid, addr)) => match self.transports.get(&tid) { + Some(transport) => transport + .send(&addr, &wire_msg2) + .await + .map(|_| tid) + .map_err(|e| e.to_string()), + None => Err("no transport for the peer's link".to_string()), + }, + None => Err("the peer has no established link".to_string()), + }; + let link_transport = match sent { + Ok(tid) => tid, + Err(e) => { + warn!( + peer = %self.peer_display_name(&peer), + error = %e, + "Failed to send rekey msg2" + ); + let _ = self.index_allocator.free(our_new_index); + self.stats_mut() + .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); + return; } - } + }; + debug!( + peer = %self.peer_display_name(&peer), + new_our_index = %our_new_index, + "Sent rekey msg2 response" + ); // Store the new session as the responder's pending session. It // is promoted by the initiator's first new-epoch frame, not by // our own tick. + // The answer is kept with it, so a resend of this msg1 draws + // the same msg2 if this one is lost. if let Some(existing) = self.peers.get_mut(&peer) { - existing.answer_rekey(noise_session, our_new_index, wire.their_index); + let answer = RekeyAnswer { + msg1: wire.msg1_digest, + msg2: wire_msg2, + }; + existing.answer_rekey(noise_session, our_new_index, wire.their_index, answer); existing.record_peer_rekey(); } - // Register new index in peers_by_index. + // Register new index in peers_by_index, under the transport + // the msg2 went out on: the peer's frames on the new session + // arrive there, and retirement removes the entry by the + // peer's transport, not the one the msg1 came in on. self.peers_by_index - .insert((packet.transport_id, our_new_index.as_u32()), peer); + .insert((link_transport, our_new_index.as_u32()), peer); // Do NOT touch addr_to_link — the entry must keep pointing at the // original link so future msg1s from this address are recognized diff --git a/src/node/handlers/session.rs b/src/node/handlers/session.rs index 15aad913..cb343bce 100644 --- a/src/node/handlers/session.rs +++ b/src/node/handlers/session.rs @@ -684,15 +684,30 @@ impl Node { // the veto returns without touching them, and the // fall-through only arms a handshake beside them. Adopting // them is still an authenticated msg3's job alone. No arm - // below drops one either: the dual-initiation arm did until - // it was narrowed to abandon only the handshake, for the - // reason recorded at that site. + // below drops one either. let pending_outranks = existing.pending_new_session().is_some() && !existing.pending_stale( Self::now_ms(), self.config().node.session.idle_timeout_secs * 1000, ); + // A handshake the peer armed is held until its msg3 or its + // timeout. Once the peer has read our SessionAck it holds the + // new keys, so this handshake is the only one its msg3 can + // complete, and discarding it for an unauthenticated setup + // splits the session's epochs. A genuine retry that meets a + // stale handshake here completes one handshake timeout + // later, once the handshake has expired. + if rekey_in_progress && !existing.is_rekey_initiator() { + debug!( + src = %self.peer_display_name(src_addr), + "FSP rekey msg1 received while the peer's handshake awaits msg3, dropping" + ); + self.stats_mut() + .record_reject(RejectReason::Session(SessionReject::RekeyHeld)); + return; + } + // Dual-initiation detection: both sides sent SessionSetup // simultaneously. Apply tie-breaker — smaller NodeAddr // wins as initiator (same as initial session setup). @@ -707,22 +722,16 @@ impl Node { .record_reject(RejectReason::Session(SessionReject::RekeyTiebreak)); return; } - // We lose — abandon the armed handshake, become responder - // below. + // We lose — abandon our armed handshake, become + // responder below. // - // `abandon_handshake`, not `abandon_rekey`: the gate - // above is `has_rekey_in_progress`, which says only that - // *some* handshake is armed, not that we armed it. A - // handshake the peer armed carries `rekey_initiator == - // false` and can sit beside a completed epoch that a - // stale `pending_outranks` no longer vetoes, so a - // stranger reaches this line with two unauthenticated - // setup messages: one to arm the handshake, one to lose - // the tie-break against it. Dropping the pending session - // there kills the epoch the peer may already have cut - // over to. Only the handshake is ours to discard, and - // discarding it costs nothing, since an armed handshake - // holds no key material either endpoint is using. + // `abandon_handshake`, not `abandon_rekey`, although the + // arm above leaves only handshakes this node initiated, + // and an initiator handshake never sits beside a pending + // session (see the SessionAck handler). Only the handshake + // is ours to discard, and discarding it costs nothing: + // the peer has not answered it, so it holds no key + // material either endpoint is using. debug!( src = %self.peer_display_name(src_addr), "Dual FSP rekey initiation: we lose (larger addr), abandoning ours" @@ -1132,15 +1141,18 @@ impl Node { // Rekey path: entry is Established with rekey_state (responder side) // - // Every failure below abandons only the handshake. Nothing in a msg3 - // is authenticated until `read_xk_message_3` has both succeeded and - // produced a static key matching this session's peer, so a failure - // here proves nothing about the sender and must not cost the entry - // anything it would miss. A `pending_new_session` beside the - // handshake is the epoch the real peer may already have cut over to, - // and dropping it kills the reverse direction on two unauthenticated - // messages: a forged msg1 to arm the handshake, then any garbage - // msg3. `abandon_handshake` keeps it; `abandon_rekey` does not. + // Nothing in a msg3 is authenticated until the read has both + // succeeded and produced a static key matching this session's peer, + // so a failure here proves nothing about the sender and must not cost + // the entry anything it would miss. An unreadable msg3 therefore + // costs nothing at all: the handshake goes back, rolled back, for the + // genuine msg3. Every later failure follows a read that + // authenticated its sender and abandons only the handshake. A + // `pending_new_session` beside the handshake is the epoch the real + // peer may already have cut over to, and dropping it kills the + // reverse direction on two unauthenticated messages: a forged msg1 to + // arm the handshake, then any garbage msg3. `abandon_handshake` keeps + // it; `abandon_rekey` does not. // // What `abandon_handshake` leaves behind, and why each is safe here: // `rekey_completed_ms` must survive, since `pending_stale` reads it @@ -1158,14 +1170,23 @@ impl Node { } }; - // Process XK msg3 - if let Err(e) = handshake.read_xk_message_3(&msg3.handshake_payload) { + // Process XK msg3. The only tie between this msg3 and the + // handshake is the datagram's source address, which the sender + // chooses, and the peer that read our SessionAck may already hold + // the new keys. Abandoning here would let anyone able to name the + // session discard the handshake the peer's genuine msg3 needs, + // splitting the session's epochs. So the handshake goes back, + // rolled back to its pre-read state, because the read advances + // the cipher nonce before it authenticates. The restore leaves + // the deadline alone, which runs from the peer's setup, so a + // spray cannot hold the handshake open. + if let Err(e) = handshake.try_read_xk_message_3(&msg3.handshake_payload) { debug!( src = %self.peer_display_name(src_addr), error = %e, - "Failed to process rekey XK msg3" + "Failed to process rekey XK msg3, keeping the handshake" ); - entry.abandon_handshake(); + entry.set_rekey_state(handshake, false); self.sessions.insert(*src_addr, entry); return; } @@ -1252,10 +1273,23 @@ impl Node { _ => unreachable!("checked is_awaiting_msg3 above"), }; - // Process XK msg3: read_xk_message_3 (extracts initiator's static key and epoch) - if let Err(e) = handshake.read_xk_message_3(&msg3.handshake_payload) { - debug!(error = %e, "Failed to process Noise XK msg3"); - return; // Entry was already removed + // Process XK msg3 (extracts the initiator's static key and epoch). + // + // Nothing here has been authenticated: the only tie to this half-open + // entry is the datagram's source address, which the sender chooses, + // and the initiator considers the session established once it has + // sent msg3. Dropping the entry would let anyone able to name the + // initiator discard the handshake its genuine msg3 and resends need, + // so the entry goes back with the handshake rolled back to its + // pre-read state, as the SessionAck arm does. `touch()` is + // deliberately not called, so a spray cannot push the handshake + // sweep's deadline out. The drops below stay drops: each follows a + // read that authenticated the sender. + if let Err(e) = handshake.try_read_xk_message_3(&msg3.handshake_payload) { + debug!(error = %e, "Failed to process Noise XK msg3, keeping the handshake"); + entry.set_state(EndToEndState::AwaitingMsg3(handshake)); + self.sessions.insert(*src_addr, entry); + return; } // Extract the initiator's static public key (now available after msg3) diff --git a/src/node/metrics.rs b/src/node/metrics.rs index b5727d04..77f9ca6d 100644 --- a/src/node/metrics.rs +++ b/src/node/metrics.rs @@ -11,7 +11,7 @@ //! The remaining families (session, handshake, mmp, transport) stay on //! `NodeStats`. -use std::sync::atomic::{AtomicU64, Ordering}; +use portable_atomic::{AtomicU64, Ordering}; use crate::cache::HintOutcome; use crate::node::reject::{BloomReject, DiscoveryReject, ForwardingReject, TreeReject}; diff --git a/src/node/reject.rs b/src/node/reject.rs index 40bfbd9d..34ed5e32 100644 --- a/src/node/reject.rs +++ b/src/node/reject.rs @@ -251,6 +251,11 @@ pub enum SessionReject { /// is being suppressed. Tracked via /// [`SessionStats::rekey_yielded`](crate::node::stats::SessionStats). RekeyYielded, + /// A setup message named an established peer while a handshake that + /// peer armed was still waiting for its msg3, so the message was dropped + /// and the handshake kept. Tracked via + /// [`SessionStats::rekey_held`](crate::node::stats::SessionStats). + RekeyHeld, /// A setup message named an established peer that already holds a /// completed rekey awaiting cut-over, so the message was dropped /// rather than arming a second handshake. Tracked via diff --git a/src/node/stats.rs b/src/node/stats.rs index a638f131..e4fd7d04 100644 --- a/src/node/stats.rs +++ b/src/node/stats.rs @@ -56,6 +56,12 @@ pub struct SessionStats { /// abandoned our own rekey and answered as responder. A sustained /// rate here means local key rotation is being suppressed. pub rekey_yielded: u64, + /// A setup message named an established peer while a handshake that + /// peer armed was still waiting for its msg3, so the message was dropped + /// and the handshake kept. A genuine retry lands here when the peer + /// abandoned a rekey this node answered; a sustained rate means setups + /// are being sprayed at the session. + pub rekey_held: u64, /// A setup message named an established peer that already holds a /// completed rekey awaiting cut-over, so the message was dropped /// rather than arming a second handshake. @@ -105,6 +111,7 @@ impl SessionStats { rekey_armed: self.rekey_armed, rekey_tiebreak: self.rekey_tiebreak, rekey_yielded: self.rekey_yielded, + rekey_held: self.rekey_held, rekey_pending: self.rekey_pending, rekey_expired: self.rekey_expired, rekey_unanswered: self.rekey_unanswered, @@ -124,6 +131,7 @@ impl SessionStats { SessionReject::RekeyKeyMismatch => self.rekey_key_mismatch += 1, SessionReject::RekeyTiebreak => self.rekey_tiebreak += 1, SessionReject::RekeyYielded => self.rekey_yielded += 1, + SessionReject::RekeyHeld => self.rekey_held += 1, SessionReject::RekeyPending => self.rekey_pending += 1, SessionReject::AckHandshakeFailed => self.ack_handshake_failed += 1, SessionReject::SetupRateLimited => self.setup_rate_limited += 1, @@ -411,6 +419,7 @@ pub struct SessionStatsSnapshot { pub rekey_armed: u64, pub rekey_tiebreak: u64, pub rekey_yielded: u64, + pub rekey_held: u64, pub rekey_pending: u64, pub rekey_expired: u64, pub rekey_unanswered: u64, @@ -538,14 +547,18 @@ mod tests { } #[test] - fn session_stats_record_reject_separates_the_three_rekey_arming_refusals() { + fn session_stats_record_reject_separates_the_four_rekey_arming_refusals() { let mut stats = SessionStats::default(); stats.record_reject(SessionReject::RekeyTiebreak); stats.record_reject(SessionReject::RekeyYielded); stats.record_reject(SessionReject::RekeyYielded); + stats.record_reject(SessionReject::RekeyHeld); + stats.record_reject(SessionReject::RekeyHeld); + stats.record_reject(SessionReject::RekeyHeld); stats.record_reject(SessionReject::RekeyPending); assert_eq!(stats.rekey_tiebreak, 1); assert_eq!(stats.rekey_yielded, 2); + assert_eq!(stats.rekey_held, 3); assert_eq!(stats.rekey_pending, 1); assert_eq!(stats.rekey_armed, 0); } @@ -554,11 +567,13 @@ mod tests { fn session_stats_snapshot_carries_the_rekey_arming_counters() { let mut stats = SessionStats::default(); stats.record_reject(SessionReject::RekeyTiebreak); + stats.record_reject(SessionReject::RekeyHeld); stats.rekey_armed = 7; stats.rekey_expired = 3; stats.pending_replaced = 2; let snap = stats.snapshot(); assert_eq!(snap.rekey_tiebreak, 1); + assert_eq!(snap.rekey_held, 1); assert_eq!(snap.rekey_armed, 7); assert_eq!(snap.rekey_expired, 3); assert_eq!(snap.pending_replaced, 2); diff --git a/src/node/tests/bloom.rs b/src/node/tests/bloom.rs index 70104193..0c32848b 100644 --- a/src/node/tests/bloom.rs +++ b/src/node/tests/bloom.rs @@ -1485,10 +1485,12 @@ fn next_counter(node: &Node, peer: &NodeAddr) -> u64 { .current_send_counter() } -/// Around a rekey, reports that describe the previous session reach both -/// ends of the link: the initiator accepts one the responder built before it -/// switched, and the responder's frames from the old session pollute the -/// initiator's receiver. Neither kind of report may trigger a resend. +/// Around a rekey, frames the responder sent on the old session reach the +/// initiator after it cut over: a report about the initiator's old-session +/// traffic, and data frames carrying the responder's old-session counters. +/// Neither may feed the new session's MMP state, so the reports either end +/// holds after the cutover describe the new session, an announce sent after +/// it is confirmed by them, and nothing is resent. #[tokio::test] async fn test_bloom_reports_from_the_previous_session_do_not_trigger_resends() { let mut fx = flip_fixture(true).await; @@ -1567,17 +1569,46 @@ async fn test_bloom_reports_from_the_previous_session_do_not_trigger_resends() { for packet in held { fx.nodes[M].node.handle_encrypted_frame(packet).await; } + assert_eq!( + rr_counters(&fx.nodes[M].node, &p), + None, + "M must not take P's report about M's previous session" + ); mmp_round(&mut fx.nodes).await; - - let m_rr = rr_counters(&fx.nodes[M].node, &p).expect("setup: M holds a report"); - let p_rr = rr_counters(&fx.nodes[P].node, &m).expect("setup: P holds a report"); + let describes_new = |rr: Option<(u64, u64, u32)>, next: u64| rr.is_none_or(|rr| rr.0 < next); assert!( - m_rr.0 >= next_counter(&fx.nodes[M].node, &p), - "setup: M's report must describe M's previous session" + describes_new( + rr_counters(&fx.nodes[M].node, &p), + next_counter(&fx.nodes[M].node, &p) + ), + "M's report must describe M's new session" ); assert!( - p_rr.0 >= next_counter(&fx.nodes[P].node, &m), - "setup: P's report must carry P's previous-session counter" + describes_new( + rr_counters(&fx.nodes[P].node, &m), + next_counter(&fx.nodes[P].node, &m) + ), + "P's report must not carry P's previous-session counter" + ); + + // Both ends come to hold a usable report on the new session, which an + // announce sent next is measured from. Under the old behaviour the + // previous-session reports kept both ends from ever holding one here. + let usable = |fx: &FlipFixture| { + [(M, p), (P, m)].into_iter().all(|(i, remote)| { + rr_counters(&fx.nodes[i].node, &remote) + .is_some_and(|rr| rr.0 < next_counter(&fx.nodes[i].node, &remote)) + }) + }; + for _ in 0..5 { + if usable(&fx) { + break; + } + mmp_round(&mut fx.nodes).await; + } + assert!( + usable(&fx), + "both ends must hold a usable new-session report" ); fx.nodes[M].node.bloom_state.mark_update_needed(p); @@ -1594,49 +1625,31 @@ async fn test_bloom_reports_from_the_previous_session_do_not_trigger_resends() { let sent_m = fx.nodes[M].node.metrics().bloom.sent.get(); let sent_p = fx.nodes[P].node.metrics().bloom.sent.get(); let guard = switch_guard(&fx); - let mut last = rr_counters(&fx.nodes[P].node, &m); - let mut changes = 0; for _ in 0..5 { mmp_round(&mut fx.nodes).await; fx.nodes[M].node.check_bloom_state().await; fx.nodes[P].node.check_bloom_state().await; process_available_packets(&mut fx.nodes).await; - let now = rr_counters(&fx.nodes[P].node, &m); - if now != last { - changes += 1; - } - last = now; } - assert!( - changes >= 2, - "setup: P must accept at least two reports from M, saw {changes}" - ); assert_unswitched(&fx, &guard); - let m_rr = rr_counters(&fx.nodes[M].node, &p).expect("setup: M holds a report"); - let p_rr = rr_counters(&fx.nodes[P].node, &m).expect("setup: P holds a report"); - assert!( - m_rr.0 >= next_counter(&fx.nodes[M].node, &p) - && p_rr.0 >= next_counter(&fx.nodes[P].node, &m), - "setup: both reports must still describe a previous session" - ); assert_eq!( fx.nodes[M].node.metrics().bloom.sent.get(), sent_m, - "M must not resend on reports from the previous session" + "M must not resend a delivered announce after the rekey" ); assert_eq!( fx.nodes[P].node.metrics().bloom.sent.get(), sent_p, - "P must not resend on reports carrying its previous-session counter" + "P must not resend a delivered announce after the rekey" ); assert!( - fx.nodes[M].node.bloom_state.announce_outstanding(&p), - "M must still hold its announce to P" + !fx.nodes[M].node.bloom_state.announce_outstanding(&p), + "M's announce must be confirmed by a new-session report" ); assert!( - fx.nodes[P].node.bloom_state.announce_outstanding(&m), - "P must still hold its announce to M" + !fx.nodes[P].node.bloom_state.announce_outstanding(&m), + "P's announce must be confirmed by a new-session report" ); cleanup_nodes(&mut fx.nodes).await; } diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index f56f1d90..aba0d69a 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -1222,9 +1222,10 @@ async fn rekey_cutover_preserves_data_plane() { cleanup_nodes(&mut nodes).await; } -/// A two-node pair caught mid FMP rekey, with node 1's msg2 held back from -/// node 0. Built by [`rekey_pair_with_held_msg2`]. -struct HeldMsg2Pair { +/// A two-node pair with an FSP session over an FMP link whose link sessions +/// have been aged, with a TUN receiver on each node. Built by +/// [`aged_link_pair`]. +struct AgedLinkPair { nodes: Vec, node0_addr: NodeAddr, node1_addr: NodeAddr, @@ -1232,36 +1233,16 @@ struct HeldMsg2Pair { fips1: crate::FipsAddress, tun0_rx: std::sync::mpsc::Receiver>, tun1_rx: std::sync::mpsc::Receiver>, - node0_idx_before: Option, - node1_idx_before: Option, - rekey_idx: crate::utils::index::SessionIndex, - held_msg2: crate::transport::ReceivedPacket, } -/// Build a two-node pair with an FSP session, age both link sessions past -/// both rekey gates, start node 0's FMP rekey, deliver its msg1 to node 1 -/// only, and pull node 1's real msg2 out of node 0's queue. -/// -/// node 0 rekeys on time and node 1 only ever responds, so node 1 holds the -/// new session it committed at msg1 and node 0 is mid-cycle when this -/// returns. Both directions are shown to decode before the rekey, so a later -/// delivery failure is the rekey's and not the harness's. -async fn rekey_pair_with_held_msg2() -> HeldMsg2Pair { - use crate::proto::fmp::wire::{CommonPrefix, PHASE_MSG2}; - use crate::transport::ReceivedPacket; - - const REKEY_AFTER_SECS: u64 = 60; - - // node 0 rekeys on time; node 1 only ever responds. - let mut cfg0 = crate::config::Config::new(); - cfg0.node.rekey.enabled = true; - cfg0.node.rekey.after_secs = REKEY_AFTER_SECS; - cfg0.node.rekey.after_messages = u64::MAX; - let mut cfg1 = crate::config::Config::new(); - cfg1.node.rekey.enabled = true; - cfg1.node.rekey.after_secs = u64::MAX; - cfg1.node.rekey.after_messages = u64::MAX; - +/// Build a two-node pair from `cfg0` and `cfg1`, peer node 0 to node 1 over +/// FMP, open an FSP session from node 0, show that both directions decode, +/// then backdate both link sessions by `age`. +async fn aged_link_pair( + cfg0: crate::config::Config, + cfg1: crate::config::Config, + age: Duration, +) -> AgedLinkPair { let mut nodes = vec![ make_test_node_with_config(cfg0, 1280).await, make_test_node_with_config(cfg1, 1280).await, @@ -1304,8 +1285,8 @@ async fn rekey_pair_with_held_msg2() -> HeldMsg2Pair { let fips0 = crate::FipsAddress::from_node_addr(&node0_addr); let fips1 = crate::FipsAddress::from_node_addr(&node1_addr); - // Baseline: both directions decode before the rekey, so a failure below - // is the rekey's and not the harness's. + // Baseline: both directions decode before the caller's scenario, so a + // later failure is the scenario's and not the harness's. let pre_fwd = build_ipv6_packet(&fips0, &fips1, b"pre-rekey 0 to 1"); let pre_rev = build_ipv6_packet(&fips1, &fips0, b"pre-rekey 1 to 0"); nodes[0].node.handle_tun_outbound(pre_fwd.clone()).await; @@ -1316,9 +1297,7 @@ async fn rekey_pair_with_held_msg2() -> HeldMsg2Pair { let got: Vec> = std::iter::from_fn(|| tun0_rx.try_recv().ok()).collect(); assert_eq!(got, vec![pre_rev], "baseline node 1 to node 0 must decode"); - // Age both sessions past both rekey gates: node 0's jittered time trigger, - // and node 1's 30 s floor below which a msg1 is a duplicate, not a rekey. - let age = Duration::from_secs(REKEY_AFTER_SECS + crate::node::REKEY_JITTER_SECS as u64 + 1); + // Age both link sessions. nodes[0] .node .get_peer_mut(&node1_addr) @@ -1329,6 +1308,70 @@ async fn rekey_pair_with_held_msg2() -> HeldMsg2Pair { .get_peer_mut(&node0_addr) .unwrap() .test_backdate_session_established(age); + + AgedLinkPair { + nodes, + node0_addr, + node1_addr, + fips0, + fips1, + tun0_rx, + tun1_rx, + } +} + +/// A two-node pair caught mid FMP rekey, with node 1's msg2 held back from +/// node 0. Built by [`rekey_pair_with_held_msg2`]. +struct HeldMsg2Pair { + nodes: Vec, + node0_addr: NodeAddr, + node1_addr: NodeAddr, + fips0: crate::FipsAddress, + fips1: crate::FipsAddress, + tun0_rx: std::sync::mpsc::Receiver>, + tun1_rx: std::sync::mpsc::Receiver>, + node0_idx_before: Option, + node1_idx_before: Option, + rekey_idx: crate::utils::index::SessionIndex, + held_msg2: crate::transport::ReceivedPacket, +} + +/// Build a two-node pair with an FSP session, age both link sessions past +/// both rekey gates, start node 0's FMP rekey, deliver its msg1 to node 1 +/// only, and pull node 1's real msg2 out of node 0's queue. +/// +/// node 0 rekeys on time and node 1 only ever responds, so node 1 holds the +/// new session it committed at msg1 and node 0 is mid-cycle when this +/// returns. Both directions are shown to decode before the rekey, so a later +/// delivery failure is the rekey's and not the harness's. +async fn rekey_pair_with_held_msg2() -> HeldMsg2Pair { + use crate::proto::fmp::wire::{CommonPrefix, PHASE_MSG2}; + use crate::transport::ReceivedPacket; + + const REKEY_AFTER_SECS: u64 = 60; + + // node 0 rekeys on time; node 1 only ever responds. + let mut cfg0 = crate::config::Config::new(); + cfg0.node.rekey.enabled = true; + cfg0.node.rekey.after_secs = REKEY_AFTER_SECS; + cfg0.node.rekey.after_messages = u64::MAX; + let mut cfg1 = crate::config::Config::new(); + cfg1.node.rekey.enabled = true; + cfg1.node.rekey.after_secs = u64::MAX; + cfg1.node.rekey.after_messages = u64::MAX; + + // Age both sessions past both rekey gates: node 0's jittered time trigger, + // and node 1's 30 s floor below which a msg1 is a duplicate, not a rekey. + let age = Duration::from_secs(REKEY_AFTER_SECS + crate::node::REKEY_JITTER_SECS as u64 + 1); + let AgedLinkPair { + mut nodes, + node0_addr, + node1_addr, + fips0, + fips1, + tun0_rx, + tun1_rx, + } = aged_link_pair(cfg0, cfg1, age).await; let node0_idx_before = nodes[0].node.get_peer(&node1_addr).unwrap().our_index(); let node1_idx_before = nodes[1].node.get_peer(&node0_addr).unwrap().our_index(); @@ -1397,16 +1440,30 @@ async fn pump_until_quiet(nodes: &mut [TestNode]) { /// Drive node 0's rekey msg1 resend ladder on the synthetic clock from /// `base_ms`: at the default 1 s interval and 2x backoff, resends at +1, +3, -/// +7, +15 and +31 s, then the abandon past the budget at +63 s, pumping -/// after each. -async fn walk_ladder(nodes: &mut [TestNode], base_ms: u64) { +/// +7, +15 and +31 s, then the abandon past the budget at +63 s. Each resend +/// is delivered to node 1, and every msg2 node 1 sends back is lost. Returns +/// the number of msg2s lost. +async fn walk_ladder_losing_msg2s(nodes: &mut [TestNode], base_ms: u64) -> usize { + use crate::proto::fmp::wire::{CommonPrefix, PHASE_MSG2}; + + let mut lost = 0; for offset_s in [1u64, 3, 7, 15, 31, 63] { nodes[0] .node .resend_pending_rekeys(base_ms + offset_s * 1000) .await; + process_available_packets(&mut nodes[1..]).await; + while let Ok(packet) = nodes[0].packet_rx.try_recv() { + assert_eq!( + CommonPrefix::parse(&packet.data).map(|p| p.phase), + Some(PHASE_MSG2), + "only node 1's msg2s may be queued at node 0 during the ladder" + ); + lost += 1; + } pump_until_quiet(nodes).await; } + lost } /// A forged rekey msg2 that carries the initiator's live rekey index must not @@ -1522,9 +1579,10 @@ async fn forged_rekey_msg2_does_not_split_the_link() { /// node 1 answers node 0's rekey msg1 and holds its new session as pending. /// node 1's msg2 never arrives, so node 0 never shows it holds the new keys, /// and node 1's own rekey tick must not cut over to them. node 0 walks its -/// whole msg1 resend ladder and re-fires after the abandon; node 1 refuses -/// each of those msg1s while it holds the pending. Data must flow both ways on -/// the original sessions throughout. The assertion that tells the outcomes +/// whole msg1 resend ladder, each resend's msg2 lost too, and re-fires after +/// the abandon; node 1 answers each resend with the msg2 it already sent and +/// refuses the re-fired msg1 while it holds the pending. Data must flow both +/// ways on the original sessions throughout. The assertion that tells the outcomes /// apart is node 1 to node 0: a node 1 that cut over would seal to the rekey /// index node 0 abandoned. /// @@ -1569,7 +1627,11 @@ async fn dropped_rekey_msg2_does_not_split_the_link() { ); let node1_rejects_before = nodes[1].node.stats().handshake.bad_state; - walk_ladder(&mut nodes, Node::now_ms()).await; + assert_eq!( + walk_ladder_losing_msg2s(&mut nodes, Node::now_ms()).await, + 5, + "node 1 must answer each of node 0's five resends" + ); assert!( !nodes[0] .node @@ -1588,8 +1650,8 @@ async fn dropped_rekey_msg2_does_not_split_the_link() { pump_until_quiet(&mut nodes).await; assert_eq!( nodes[1].node.stats().handshake.bad_state - node1_rejects_before, - 6, - "node 1 must refuse node 0's five resends and its re-fired msg1 while it holds the pending" + 1, + "node 1 must refuse node 0's re-fired msg1 while it holds the pending" ); let post_fwd = build_ipv6_packet(&fips0, &fips1, b"post-loss 0 to 1"); @@ -1622,6 +1684,78 @@ async fn dropped_rekey_msg2_does_not_split_the_link() { cleanup_nodes(&mut nodes).await; } +/// A rekey responder whose msg2 was lost must not cut over on its own tick, +/// and must not answer the initiator's resent msg1 with the msg2 it stored at +/// startup. +/// +/// A responder that cut over on its own tick reset its session age, so the +/// resent msg1 fell under the 30 s floor below which a msg1 is taken as a +/// duplicate of the startup one, and the responder resent its startup msg2, +/// addressed to the startup index the initiator no longer dispatches. Every +/// resend in the ladder met the same answer, and the rekey stalled until a +/// fresh handshake replaced the link. +/// +/// Today the responder refuses the resent msg1 while it holds the pending and +/// sends no msg2 at all. A responder that instead resends the msg2 it answered +/// the rekey with is also correct, so the assertion admits a msg2 addressed to +/// the rekey index and refuses only one addressed anywhere else. +#[tokio::test] +async fn a_lost_rekey_msg2_is_not_followed_by_a_cutover_or_the_startup_msg2() { + use crate::proto::fmp::wire::{CommonPrefix, Msg2Header, PHASE_MSG2}; + + let HeldMsg2Pair { + mut nodes, + node0_addr, + node1_idx_before, + rekey_idx, + held_msg2, + .. + } = rekey_pair_with_held_msg2().await; + let startup_msg2 = nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .handshake_msg2() + .map(|m| m.to_vec()); + + // The msg2 is lost, and node 1's rekey tick runs before node 0 resends. + drop(held_msg2); + nodes[1].node.check_rekey().await; + let node1_idx_after_tick = nodes[1].node.get_peer(&node0_addr).unwrap().our_index(); + + // node 0's first msg1 resend, delivered to node 1 only. + nodes[0] + .node + .resend_pending_rekeys(Node::now_ms() + 1000) + .await; + assert_eq!( + process_available_packets(&mut nodes[1..]).await, + 1, + "node 1 must have exactly node 0's resent rekey msg1 queued" + ); + + let msg2s: Vec> = std::iter::from_fn(|| nodes[0].packet_rx.try_recv().ok()) + .filter(|p| CommonPrefix::parse(&p.data).map(|c| c.phase) == Some(PHASE_MSG2)) + .map(|p| p.data) + .collect(); + for msg2 in &msg2s { + let receiver = Msg2Header::parse(msg2).map(|h| h.receiver_idx); + assert_eq!( + receiver, + Some(rekey_idx), + "node 1 answered the resent rekey msg1 with a msg2 not addressed to the rekey \ + (it is the stored startup msg2: {})", + startup_msg2.as_deref() == Some(msg2.as_slice()) + ); + } + assert_eq!( + node1_idx_after_tick, node1_idx_before, + "node 1 must not cut over on its own tick to a session node 0 never adopted" + ); + + cleanup_nodes(&mut nodes).await; +} + /// A rekey responder whose msg2 was lost holds the pending session it /// answered with until the hold passes, then retires it: the pending slot and /// its role are emptied, its index is unregistered and freed, and the current @@ -1659,9 +1793,10 @@ async fn a_responder_retires_an_unadopted_rekey_and_the_next_rekey_completes() { .pending_our_index() .expect("node 1 must still hold the session it answered with"); - // 2. node 0 walks its ladder to the abandon and re-fires; node 1 refuses - // the re-fired msg1 while it holds the pending. - walk_ladder(&mut nodes, Node::now_ms()).await; + // 2. node 0 walks its ladder to the abandon, losing node 1's answer to + // every resend, and re-fires; node 1 refuses the re-fired msg1 while it + // holds the pending. + walk_ladder_losing_msg2s(&mut nodes, Node::now_ms()).await; nodes[0].node.check_rekey().await; pump_until_quiet(&mut nodes).await; assert!( @@ -1770,6 +1905,932 @@ async fn a_responder_retires_an_unadopted_rekey_and_the_next_rekey_completes() { cleanup_nodes(&mut nodes).await; } +/// A rekey responder whose msg2 was lost answers the initiator's resend of the +/// same msg1 with the msg2 it already sent, so the cycle completes one resend +/// interval after the loss rather than after the responder hold. +/// +/// node 1 must not allocate or arm a second pending for the resend: the msg2 +/// it repeats is bound to the pending it already holds, and node 0 reads that +/// msg2 to complete the handshake it started. The rekey then finishes as an +/// unbroken one would, with node 1 promoting on node 0's first new-epoch frame. +#[tokio::test] +async fn a_resent_rekey_msg1_draws_the_held_msg2_and_the_rekey_completes() { + let HeldMsg2Pair { + mut nodes, + node0_addr, + node1_addr, + fips0, + fips1, + tun0_rx, + tun1_rx, + node0_idx_before, + node1_idx_before, + held_msg2, + .. + } = rekey_pair_with_held_msg2().await; + + // The msg2 is lost. + let lost_msg2 = held_msg2.data.clone(); + drop(held_msg2); + let pending_idx = nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_our_index() + .expect("node 1 must hold the session it answered with"); + let node1_rejects_before = nodes[1].node.stats().handshake.bad_state; + + // node 0's first resend, at +1 s, reaches node 1 only. + nodes[0] + .node + .resend_pending_rekeys(Node::now_ms() + 1_000) + .await; + assert_eq!( + process_available_packets(&mut nodes[1..]).await, + 1, + "node 1 must have exactly node 0's resent msg1 queued" + ); + assert_eq!( + nodes[1].node.stats().handshake.bad_state, + node1_rejects_before, + "node 1 must answer the resent msg1, not refuse it" + ); + assert_eq!( + nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_our_index(), + Some(pending_idx), + "node 1 must keep the pending it holds, not arm another" + ); + + // node 1's answer is the msg2 that was lost, byte for byte. + let answered: Vec<_> = std::iter::from_fn(|| nodes[0].packet_rx.try_recv().ok()).collect(); + assert_eq!(answered.len(), 1, "node 0 must have one answer queued"); + assert_eq!( + answered[0].data, lost_msg2, + "node 1 must resend the msg2 it sent for this msg1" + ); + nodes[0] + .node + .handle_msg2(answered.into_iter().next().unwrap()) + .await; + assert!( + nodes[0] + .node + .get_peer(&node1_addr) + .unwrap() + .pending_new_session() + .is_some(), + "node 0 must complete its rekey on the resent msg2" + ); + + // node 0 cuts over; node 1 promotes on node 0's first new-epoch frame. + nodes[0].node.check_rekey().await; + nodes[1].node.check_rekey().await; + pump_until_quiet(&mut nodes).await; + let post_fwd = build_ipv6_packet(&fips0, &fips1, b"post-resend 0 to 1"); + let post_rev = build_ipv6_packet(&fips1, &fips0, b"post-resend 1 to 0"); + nodes[0].node.handle_tun_outbound(post_fwd.clone()).await; + pump_until_quiet(&mut nodes).await; + nodes[1].node.handle_tun_outbound(post_rev.clone()).await; + pump_until_quiet(&mut nodes).await; + + let got: Vec> = std::iter::from_fn(|| tun1_rx.try_recv().ok()).collect(); + assert_eq!(got, vec![post_fwd], "node 0 to node 1 must decode"); + let got: Vec> = std::iter::from_fn(|| tun0_rx.try_recv().ok()).collect(); + assert_eq!(got, vec![post_rev], "node 1 to node 0 must decode"); + assert_ne!( + nodes[0].node.get_peer(&node1_addr).unwrap().our_index(), + node0_idx_before, + "node 0 must have cut over" + ); + let node1_peer = nodes[1].node.get_peer(&node0_addr).unwrap(); + assert_eq!( + node1_peer.our_index(), + Some(pending_idx), + "node 1 must have promoted the pending it answered with" + ); + assert_ne!(node1_peer.our_index(), node1_idx_before); + + cleanup_nodes(&mut nodes).await; +} + +/// A copy of the msg1 that armed a responder's pending, arriving from an +/// address that is not the peer's, draws the held msg2 only on the peer's +/// established link. Nothing goes back to the address the copy came from. +/// +/// A captured msg1 still authenticates as the peer when replayed, so the +/// responder cannot tell a replay from a resend; answering the datagram's +/// source would hand anyone who sends one a reflector. +#[tokio::test] +async fn a_held_rekey_msg2_is_resent_only_on_the_peers_established_link() { + use crate::node::tests::spanning_tree::make_test_node; + + let HeldMsg2Pair { + mut nodes, + node0_addr, + held_msg2, + .. + } = rekey_pair_with_held_msg2().await; + let lost_msg2 = held_msg2.data.clone(); + drop(held_msg2); + let mut third = vec![make_test_node().await]; + + // Capture node 0's resend and deliver it to node 1 under the third node's + // address, as a spoofed source would. + nodes[0] + .node + .resend_pending_rekeys(Node::now_ms() + 1_000) + .await; + let mut msg1 = nodes[1] + .packet_rx + .try_recv() + .expect("node 0's resent msg1 must be queued at node 1"); + msg1.remote_addr = third[0].addr.clone(); + nodes[1].node.handle_msg1(msg1).await; + + assert!( + third[0].packet_rx.try_recv().is_err(), + "nothing may be sent to the source of the copy" + ); + let answered: Vec<_> = std::iter::from_fn(|| nodes[0].packet_rx.try_recv().ok()).collect(); + assert_eq!( + answered.iter().map(|p| &p.data).collect::>(), + vec![&lost_msg2], + "the held msg2 must go to node 0 on its established link" + ); + assert!( + nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_new_session() + .is_some(), + "node 1 must still hold its pending" + ); + + cleanup_nodes(&mut nodes).await; + cleanup_nodes(&mut third).await; +} + +/// A ReceiverReport about `highest` frames, as the link-layer message a peer +/// sends: the type byte followed by the body. +fn receiver_report_message(highest: u64) -> Vec { + crate::proto::mmp::ReceiverReport { + highest_counter: highest, + cumulative_packets_recv: highest, + cumulative_bytes_recv: highest * 100, + timestamp_echo: 0, + dwell_time: 0, + max_burst_loss: 0, + mean_burst_loss: 0, + jitter: 0, + ecn_ce_count: 0, + owd_trend: 0, + burst_loss_count: 0, + cumulative_reorder_count: 0, + interval_packets_recv: highest as u32, + interval_bytes_recv: (highest * 100) as u32, + } + .encode() +} + +/// A link rekey initiator that has cut over keeps its new session's MMP state +/// free of frames the responder sealed on the old session. +/// +/// node 0 cuts over and resets its MMP receiver and metrics. node 1 has not +/// yet seen a frame on the new epoch, so it still sends on the old session, +/// and node 0 decrypts those frames against its previous session during the +/// drain. Their payload is still delivered, but they describe the old +/// session: a data frame's counter must not become the new session's highest +/// counter, which would make every new-session frame count as a reorder, and +/// a ReceiverReport about node 0's old-session traffic must not become the +/// baseline later reports are judged against, which would reject every +/// new-session report as regressed. Once node 1 promotes, its frames and +/// reports on the new session are counted and accepted. +#[tokio::test] +async fn frames_on_the_previous_link_session_do_not_feed_the_new_sessions_mmp() { + let HeldMsg2Pair { + mut nodes, + node0_addr, + node1_addr, + fips0, + fips1, + tun0_rx, + tun1_rx, + node0_idx_before, + node1_idx_before, + held_msg2, + .. + } = rekey_pair_with_held_msg2().await; + + // node 0 completes its rekey and cuts over on its own tick; nothing is + // delivered to node 1, which stays on the old session. + nodes[0].node.handle_msg2(held_msg2).await; + nodes[0].node.check_rekey().await; + let peer = nodes[0].node.get_peer(&node1_addr).unwrap(); + assert_ne!(peer.our_index(), node0_idx_before, "node 0 must cut over"); + let mmp = peer.mmp().expect("node 0 must run link MMP"); + assert_eq!(mmp.receiver.highest_counter(), 0, "the cutover resets MMP"); + assert_eq!(mmp.metrics.rr_counters(), None, "the cutover resets MMP"); + let reports_before = mmp.metrics.reports_seen(); + + // node 1, still on the old session, sends data and then a report about + // node 0's old-session traffic. Only node 0's queue is delivered. + let old_rev = build_ipv6_packet(&fips1, &fips0, b"old session 1 to 0"); + nodes[1].node.handle_tun_outbound(old_rev.clone()).await; + nodes[1] + .node + .send_encrypted_link_message(&node0_addr, &receiver_report_message(1_000)) + .await + .unwrap(); + assert_eq!( + nodes[1].node.get_peer(&node0_addr).unwrap().our_index(), + node1_idx_before, + "node 1 must still be on the old session" + ); + for _ in 0..3 { + tokio::time::sleep(Duration::from_millis(10)).await; + process_available_packets(&mut nodes[..1]).await; + } + + let got: Vec> = std::iter::from_fn(|| tun0_rx.try_recv().ok()).collect(); + assert_eq!( + got, + vec![old_rev], + "an old-session frame's payload must still be delivered" + ); + let mmp = nodes[0].node.get_peer(&node1_addr).unwrap().mmp().unwrap(); + assert_eq!( + mmp.metrics.reports_seen(), + reports_before, + "an old-session ReceiverReport must not reach the new session's metrics" + ); + assert_eq!(mmp.metrics.rr_counters(), None); + assert_eq!( + mmp.receiver.highest_counter(), + 0, + "an old-session frame's counter must not become the new session's highest" + ); + + // node 0's first new-epoch frame promotes node 1; node 1's frames and + // reports on the new session then feed node 0's MMP. + let new_fwd = build_ipv6_packet(&fips0, &fips1, b"new session 0 to 1"); + nodes[0].node.handle_tun_outbound(new_fwd.clone()).await; + pump_until_quiet(&mut nodes).await; + let got: Vec> = std::iter::from_fn(|| tun1_rx.try_recv().ok()).collect(); + assert_eq!(got, vec![new_fwd]); + assert_ne!( + nodes[1].node.get_peer(&node0_addr).unwrap().our_index(), + node1_idx_before, + "node 1 must promote on node 0's first new-epoch frame" + ); + let new_rev = build_ipv6_packet(&fips1, &fips0, b"new session 1 to 0"); + nodes[1].node.handle_tun_outbound(new_rev.clone()).await; + nodes[1] + .node + .send_encrypted_link_message(&node0_addr, &receiver_report_message(5)) + .await + .unwrap(); + pump_until_quiet(&mut nodes).await; + let got: Vec> = std::iter::from_fn(|| tun0_rx.try_recv().ok()).collect(); + assert_eq!(got, vec![new_rev]); + let mmp = nodes[0].node.get_peer(&node1_addr).unwrap().mmp().unwrap(); + assert!( + mmp.receiver.highest_counter() > 0, + "new-session frames must be counted" + ); + assert_eq!( + mmp.metrics.rr_counters().map(|(highest, _, _)| highest), + Some(5), + "a new-session ReceiverReport must be accepted" + ); + + cleanup_nodes(&mut nodes).await; +} + +/// The decrypt-worker path tells the sessions apart too. A frame the worker +/// decrypted under the previous session's index, bounced back after the +/// cutover, does not feed the new session's MMP; the same frame under the +/// current session's index does. +#[cfg(unix)] +#[tokio::test] +async fn a_worker_decrypted_frame_on_the_previous_link_session_does_not_feed_mmp() { + use crate::node::decrypt_worker::DecryptFallback; + + let HeldMsg2Pair { + mut nodes, + node1_addr, + node0_idx_before, + held_msg2, + .. + } = rekey_pair_with_held_msg2().await; + nodes[0].node.handle_msg2(held_msg2).await; + nodes[0].node.check_rekey().await; + let peer = nodes[0].node.get_peer(&node1_addr).unwrap(); + let previous_idx = peer.previous_our_index().expect("node 0 must be draining"); + assert_eq!(Some(previous_idx), node0_idx_before); + let current_idx = peer.our_index().expect("node 0 must have cut over"); + + // A bounced worker plaintext: the 4-byte session timestamp, then a + // ReceiverReport about 1000 frames. + let (transport_id, remote_addr) = (nodes[0].transport_id, nodes[1].addr.clone()); + let bounce = |receiver_idx: u32, counter: u64| { + let mut data = 0u32.to_le_bytes().to_vec(); + data.extend(receiver_report_message(1_000)); + DecryptFallback { + source_node_addr: node1_addr, + transport_id, + remote_addr: remote_addr.clone(), + timestamp_ms: Node::now_ms(), + packet_len: data.len() + 32, + receiver_idx, + fmp_counter: counter, + fmp_flags: 0, + fmp_plaintext_len: data.len(), + packet_data: data, + fmp_plaintext_offset: 0, + } + }; + + let previous = bounce(previous_idx.as_u32(), 900); + nodes[0].node.process_decrypt_fallback(previous).await; + let mmp = nodes[0].node.get_peer(&node1_addr).unwrap().mmp().unwrap(); + assert_eq!( + mmp.receiver.highest_counter(), + 0, + "a previous-session frame's counter must not be counted" + ); + assert_eq!( + mmp.metrics.rr_counters(), + None, + "a previous-session ReceiverReport must not be processed" + ); + + let current = bounce(current_idx.as_u32(), 1); + nodes[0].node.process_decrypt_fallback(current).await; + let mmp = nodes[0].node.get_peer(&node1_addr).unwrap().mmp().unwrap(); + assert_eq!(mmp.receiver.highest_counter(), 1); + assert_eq!(mmp.metrics.rr_counters().map(|(h, _, _)| h), Some(1_000)); + + cleanup_nodes(&mut nodes).await; +} + +/// Where a captured link rekey msg1 is replayed from, once the cycle it +/// started has completed and the new link session is past the 30 s rekey +/// floor. +#[derive(Clone, Copy, Debug)] +enum ReplaySource { + /// No replay: the control the other two are measured against. + Nothing, + /// An address the node has no link with. + ThirdAddress, + /// The peer's own link address. + PeerAddress, +} + +/// The rekey that is attempted after the replay. +#[derive(Clone, Copy, Debug)] +enum ReplayProbe { + /// node 0, the peer, starts its next rekey. + PeerRekey, + /// node 1's own trigger, past its time threshold and its dampening. + OwnTrigger, +} + +/// What a replayed link msg1 left behind. +#[derive(Debug, PartialEq, Eq)] +struct ReplayOutcome { + /// node 1 holds a pending responder session after the replay. + armed_pending: bool, + /// Rekey msg2s node 1 sent to the replay's source address. + msg2_to_source: usize, + /// The probed rekey went ahead: node 0 completed on node 1's answer, or + /// node 1 started one of its own. + probe_proceeded: bool, +} + +/// The outcome when a replay changes nothing. +const REPLAY_HARMLESS: ReplayOutcome = ReplayOutcome { + armed_pending: false, + msg2_to_source: 0, + probe_proceeded: true, +}; + +/// Deliver `packets` to `node` as the network would have. +async fn dispatch_link_packets( + node: &mut TestNode, + packets: Vec, +) { + use crate::proto::fmp::wire::{CommonPrefix, PHASE_ESTABLISHED, PHASE_MSG1, PHASE_MSG2}; + + for packet in packets { + match CommonPrefix::parse(&packet.data).map(|p| p.phase) { + Some(PHASE_MSG1) => node.node.handle_msg1(packet).await, + Some(PHASE_MSG2) => node.node.handle_msg2(packet).await, + Some(PHASE_ESTABLISHED) => node.node.handle_encrypted_frame(packet).await, + _ => {} + } + } +} + +/// Run `cycles` genuine FMP rekeys from node 0 to completion while keeping a +/// copy of the first one's msg1, age node 1's new link session past the 30 s +/// rekey floor, replay the copy to node 1 from `source`, then attempt the +/// rekey `probe` names. +/// +/// Both nodes rekey on time, so node 1 has a trigger of its own to probe; its +/// tick is never run before the probe, so it never initiates earlier. A third +/// node with no link to either stands in for the third address, so a msg2 sent +/// there is observable. +async fn replay_link_msg1_after_its_cycle( + source: ReplaySource, + probe: ReplayProbe, + cycles: usize, +) -> ReplayOutcome { + replay_link_msg1(source, probe, cycles, true).await +} + +/// [`replay_link_msg1_after_its_cycle`], with the replay sent either past the +/// 30 s floor (`past_floor`) or straight after the last cutover, while a +/// same-epoch msg1 is still taken as a duplicate of the link setup. +async fn replay_link_msg1( + source: ReplaySource, + probe: ReplayProbe, + cycles: usize, + past_floor: bool, +) -> ReplayOutcome { + use crate::proto::fmp::wire::{CommonPrefix, PHASE_MSG1, PHASE_MSG2}; + use crate::transport::ReceivedPacket; + + const REKEY_AFTER_SECS: u64 = 60; + let trigger_age = + Duration::from_secs(REKEY_AFTER_SECS + crate::node::REKEY_JITTER_SECS as u64 + 1); + let config = || { + let mut config = crate::config::Config::new(); + config.node.rekey.enabled = true; + config.node.rekey.after_secs = REKEY_AFTER_SECS; + config.node.rekey.after_messages = u64::MAX; + config + }; + let AgedLinkPair { + mut nodes, + node0_addr, + node1_addr, + fips0, + fips1, + tun1_rx, + .. + } = aged_link_pair(config(), config(), trigger_age).await; + nodes.push(make_test_node_with_config(crate::config::Config::new(), 1280).await); + + // The genuine cycles, with a copy of the first one's msg1 kept on the way. + let mut captured = None; + for cycle in 0..cycles { + if cycle > 0 { + nodes[0] + .node + .get_peer_mut(&node1_addr) + .unwrap() + .test_backdate_session_established(trigger_age); + nodes[1] + .node + .get_peer_mut(&node0_addr) + .unwrap() + .test_backdate_session_established(Duration::from_secs(31)); + } + let node1_idx_before = nodes[1].node.get_peer(&node0_addr).unwrap().our_index(); + nodes[0].node.check_rekey().await; + let msg1 = nodes[1] + .packet_rx + .try_recv() + .expect("node 0's rekey msg1 must be queued at node 1"); + assert_eq!( + CommonPrefix::parse(&msg1.data).map(|p| p.phase), + Some(PHASE_MSG1), + "the captured packet must be node 0's rekey msg1" + ); + captured.get_or_insert_with(|| msg1.data.clone()); + nodes[1].node.handle_msg1(msg1).await; + pump_until_quiet(&mut nodes).await; + nodes[0].node.check_rekey().await; + let first = build_ipv6_packet(&fips0, &fips1, b"first frame on the new link epoch"); + nodes[0].node.handle_tun_outbound(first.clone()).await; + pump_until_quiet(&mut nodes).await; + let got: Vec> = std::iter::from_fn(|| tun1_rx.try_recv().ok()).collect(); + assert_eq!( + got, + vec![first], + "node 0's first new-epoch frame must decode" + ); + let peer1 = nodes[1].node.get_peer(&node0_addr).unwrap(); + assert!( + peer1.pending_new_session().is_none() && peer1.our_index() != node1_idx_before, + "node 1 must have promoted the genuine cycle's session" + ); + } + let captured = captured.expect("at least one cycle must run"); + + // Past the floor below which a same-epoch msg1 is a duplicate. + if past_floor { + nodes[1] + .node + .get_peer_mut(&node0_addr) + .unwrap() + .test_backdate_session_established(Duration::from_secs(31)); + } + + let from = match source { + ReplaySource::Nothing => None, + ReplaySource::ThirdAddress => Some(2), + ReplaySource::PeerAddress => Some(0), + }; + let mut msg2_to_source = 0; + if let Some(from) = from { + let replay = ReceivedPacket::new(nodes[1].transport_id, nodes[from].addr.clone(), captured); + nodes[1].node.handle_msg1(replay).await; + tokio::time::sleep(Duration::from_millis(10)).await; + let reached: Vec = + std::iter::from_fn(|| nodes[from].packet_rx.try_recv().ok()).collect(); + msg2_to_source = reached + .iter() + .filter(|p| CommonPrefix::parse(&p.data).map(|p| p.phase) == Some(PHASE_MSG2)) + .count(); + dispatch_link_packets(&mut nodes[from], reached).await; + pump_until_quiet(&mut nodes).await; + } + let armed_pending = nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_new_session() + .is_some(); + + let probe_proceeded = match probe { + ReplayProbe::PeerRekey => { + nodes[0] + .node + .get_peer_mut(&node1_addr) + .unwrap() + .test_backdate_session_established(trigger_age); + nodes[0].node.check_rekey().await; + assert!( + nodes[0] + .node + .get_peer(&node1_addr) + .unwrap() + .rekey_in_progress(), + "node 0 must start its next rekey" + ); + pump_until_quiet(&mut nodes).await; + nodes[0] + .node + .get_peer(&node1_addr) + .unwrap() + .pending_new_session() + .is_some() + } + ReplayProbe::OwnTrigger => { + let peer = nodes[1].node.get_peer_mut(&node0_addr).unwrap(); + peer.test_backdate_session_established(trigger_age); + peer.backdate_dampener(Duration::from_secs(31)); + nodes[1].node.check_rekey().await; + nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .rekey_in_progress() + } + }; + + cleanup_nodes(&mut nodes).await; + ReplayOutcome { + armed_pending, + msg2_to_source, + probe_proceeded, + } +} + +/// The control for the peer's next rekey: with nothing replayed, the +/// procedure that measures the replays sees node 1 answer node 0's next rekey. +#[tokio::test] +async fn a_peers_next_link_rekey_is_answered_when_nothing_is_replayed() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::Nothing, ReplayProbe::PeerRekey, 1).await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// The control for the node's own trigger: with nothing replayed, node 1 +/// starts its own rekey once past its threshold and its dampening. +#[tokio::test] +async fn a_nodes_own_link_rekey_trigger_fires_when_nothing_is_replayed() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::Nothing, ReplayProbe::OwnTrigger, 1).await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A link msg1 from a completed cycle, replayed from an address the node has +/// no link with, must not hold a pending that refuses the peer's next rekey. +#[tokio::test] +async fn a_link_msg1_replayed_from_a_third_address_does_not_block_the_peers_next_rekey() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::ThirdAddress, ReplayProbe::PeerRekey, 1) + .await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A link msg1 from a completed cycle, replayed from an address the node has +/// no link with, must not hold a pending that suppresses the node's own rekey. +#[tokio::test] +async fn a_link_msg1_replayed_from_a_third_address_does_not_suppress_the_nodes_own_rekey() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::ThirdAddress, ReplayProbe::OwnTrigger, 1) + .await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A link msg1 from a completed cycle, replayed from the peer's own address, +/// must not hold a pending that refuses the peer's next rekey. +#[tokio::test] +async fn a_link_msg1_replayed_from_the_peers_address_does_not_block_the_peers_next_rekey() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::PeerAddress, ReplayProbe::PeerRekey, 1) + .await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A link msg1 from a completed cycle, replayed from the peer's own address, +/// must not hold a pending that suppresses the node's own rekey. +#[tokio::test] +async fn a_link_msg1_replayed_from_the_peers_address_does_not_suppress_the_nodes_own_rekey() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::PeerAddress, ReplayProbe::OwnTrigger, 1) + .await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A link msg1 from an earlier cycle than the last one, replayed from an +/// address the node has no link with, is refused as well: the node keeps +/// the msg1s of its ended cycles, not only the latest. +#[tokio::test] +async fn a_link_msg1_replayed_from_an_earlier_cycle_does_not_block_the_peers_next_rekey() { + let outcome = + replay_link_msg1_after_its_cycle(ReplaySource::ThirdAddress, ReplayProbe::PeerRekey, 3) + .await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A link msg1 replayed from an address the node has no link with, inside the +/// 30 s after a cutover, is taken as a duplicate of the link setup. The stored +/// setup msg2 it draws goes to the peer's established link, never to the +/// address the copy came from, and nothing else changes. +#[tokio::test] +async fn a_link_msg1_replayed_inside_the_rekey_floor_draws_nothing_to_its_source() { + let outcome = replay_link_msg1( + ReplaySource::ThirdAddress, + ReplayProbe::OwnTrigger, + 1, + false, + ) + .await; + assert_eq!(outcome, REPLAY_HARMLESS); +} + +/// A fresh rekey msg1 that arrives from an address other than the peer's is +/// answered on the peer's established link, not at the address it came from, +/// and the rekey completes there. +#[tokio::test] +async fn a_rekey_msg2_answers_on_the_peers_established_link_whatever_the_msg1_source() { + use crate::node::tests::spanning_tree::make_test_node; + use crate::proto::fmp::wire::{CommonPrefix, PHASE_MSG2}; + + const REKEY_AFTER_SECS: u64 = 60; + let trigger_age = + Duration::from_secs(REKEY_AFTER_SECS + crate::node::REKEY_JITTER_SECS as u64 + 1); + let mut cfg0 = crate::config::Config::new(); + cfg0.node.rekey.after_secs = REKEY_AFTER_SECS; + cfg0.node.rekey.after_messages = u64::MAX; + let mut cfg1 = crate::config::Config::new(); + cfg1.node.rekey.after_secs = u64::MAX; + cfg1.node.rekey.after_messages = u64::MAX; + let AgedLinkPair { + mut nodes, + node0_addr, + node1_addr, + .. + } = aged_link_pair(cfg0, cfg1, trigger_age).await; + let mut third = vec![make_test_node().await]; + + nodes[0].node.check_rekey().await; + let mut msg1 = nodes[1] + .packet_rx + .try_recv() + .expect("node 0's rekey msg1 must be queued at node 1"); + msg1.remote_addr = third[0].addr.clone(); + nodes[1].node.handle_msg1(msg1).await; + + assert!( + third[0].packet_rx.try_recv().is_err(), + "nothing may be sent to the address the msg1 came from" + ); + let answered: Vec<_> = std::iter::from_fn(|| nodes[0].packet_rx.try_recv().ok()).collect(); + assert_eq!( + answered + .iter() + .map(|p| CommonPrefix::parse(&p.data).map(|p| p.phase)) + .collect::>(), + vec![Some(PHASE_MSG2)], + "the msg2 must go to node 0 on its established link" + ); + assert!( + nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_new_session() + .is_some(), + "node 1 must hold the session it answered with" + ); + nodes[0] + .node + .handle_msg2(answered.into_iter().next().unwrap()) + .await; + assert!( + nodes[0] + .node + .get_peer(&node1_addr) + .unwrap() + .pending_new_session() + .is_some(), + "node 0 must complete its rekey on the answer" + ); + + cleanup_nodes(&mut nodes).await; + cleanup_nodes(&mut third).await; +} + +/// A rekey msg1 that arrives on a transport other than the peer's link is +/// answered on the link, and the pending session's index is registered under +/// the link's transport: the peer's frames on the new session arrive there, +/// and retirement removes the entry by the peer's transport. +#[tokio::test] +async fn a_rekey_answered_on_the_established_link_registers_its_index_on_that_transport() { + use crate::transport::TransportId; + + const REKEY_AFTER_SECS: u64 = 60; + let trigger_age = + Duration::from_secs(REKEY_AFTER_SECS + crate::node::REKEY_JITTER_SECS as u64 + 1); + let mut cfg0 = crate::config::Config::new(); + cfg0.node.rekey.after_secs = REKEY_AFTER_SECS; + cfg0.node.rekey.after_messages = u64::MAX; + let mut cfg1 = crate::config::Config::new(); + cfg1.node.rekey.after_secs = u64::MAX; + cfg1.node.rekey.after_messages = u64::MAX; + let AgedLinkPair { + mut nodes, + node0_addr, + node1_addr, + fips0, + fips1, + tun1_rx, + .. + } = aged_link_pair(cfg0, cfg1, trigger_age).await; + let link_transport = nodes[1].transport_id; + let other_transport = TransportId::new(link_transport.as_u32() + 1); + + nodes[0].node.check_rekey().await; + let mut msg1 = nodes[1] + .packet_rx + .try_recv() + .expect("node 0's rekey msg1 must be queued at node 1"); + msg1.transport_id = other_transport; + nodes[1].node.handle_msg1(msg1).await; + let pending_idx = nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_our_index() + .expect("node 1 must answer the msg1 and hold its new session"); + assert!( + nodes[1] + .node + .peers_by_index + .contains_key(&(link_transport, pending_idx.as_u32())), + "the pending index must be registered under the link's transport" + ); + assert!( + !nodes[1] + .node + .peers_by_index + .contains_key(&(other_transport, pending_idx.as_u32())), + "the pending index must not be registered under the msg1's transport" + ); + + // node 0 completes and cuts over; its first new-epoch frame, on the link, + // must find node 1's pending and promote it. + pump_until_quiet(&mut nodes).await; + nodes[0].node.check_rekey().await; + let first = build_ipv6_packet(&fips0, &fips1, b"first frame on the new link epoch"); + nodes[0].node.handle_tun_outbound(first.clone()).await; + pump_until_quiet(&mut nodes).await; + let got: Vec> = std::iter::from_fn(|| tun1_rx.try_recv().ok()).collect(); + assert_eq!( + got, + vec![first], + "node 0's first new-epoch frame must decode" + ); + assert_eq!( + nodes[1].node.get_peer(&node0_addr).unwrap().our_index(), + Some(pending_idx), + "node 1 must promote the pending on node 0's first new-epoch frame" + ); + assert!( + nodes[0] + .node + .get_peer(&node1_addr) + .unwrap() + .pending_new_session() + .is_none() + ); + + cleanup_nodes(&mut nodes).await; +} + +/// The record of answered msg1s lives with the peer, so a msg1 captured before +/// the peering was formed again, with the peer's epoch unchanged, is not +/// recognized. This node restarting reaches the same state, as does a link +/// torn down and re-formed. Kept as the record of that residual: it stays red +/// until a msg1 carries something that ties it to one cycle, which is a wire +/// change. +#[tokio::test] +#[ignore = "residual: a link msg1 captured before the peering was re-formed in the same peer epoch still arms a responder pending; the answered-msg1 record does not survive the peering"] +async fn a_link_msg1_captured_before_the_peering_was_re_formed_does_not_arm_a_pending() { + use crate::transport::ReceivedPacket; + + const REKEY_AFTER_SECS: u64 = 60; + let trigger_age = + Duration::from_secs(REKEY_AFTER_SECS + crate::node::REKEY_JITTER_SECS as u64 + 1); + let mut cfg0 = crate::config::Config::new(); + cfg0.node.rekey.after_secs = REKEY_AFTER_SECS; + cfg0.node.rekey.after_messages = u64::MAX; + let mut cfg1 = crate::config::Config::new(); + cfg1.node.rekey.after_secs = u64::MAX; + cfg1.node.rekey.after_messages = u64::MAX; + let AgedLinkPair { + mut nodes, + node0_addr, + node1_addr, + .. + } = aged_link_pair(cfg0, cfg1, trigger_age).await; + + // node 1 answers node 0's rekey; a copy of the msg1 is kept. + nodes[0].node.check_rekey().await; + let msg1 = nodes[1] + .packet_rx + .try_recv() + .expect("node 0's rekey msg1 must be queued at node 1"); + let captured = msg1.data.clone(); + nodes[1].node.handle_msg1(msg1).await; + assert!( + nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_new_session() + .is_some(), + "setup: node 1 must answer the genuine msg1" + ); + + // The peering is torn down and formed again; neither node restarts, so + // node 0's epoch, which the captured msg1 carries, is unchanged. + nodes[0].node.remove_active_peer(&node1_addr); + nodes[1].node.remove_active_peer(&node0_addr); + pump_until_quiet(&mut nodes).await; + initiate_handshake(&mut nodes, 0, 1).await; + drain_all_packets(&mut nodes, false).await; + nodes[1] + .node + .get_peer_mut(&node0_addr) + .expect("setup: the peering must be formed again") + .test_backdate_session_established(Duration::from_secs(31)); + + let replay = ReceivedPacket::new(nodes[1].transport_id, nodes[0].addr.clone(), captured); + nodes[1].node.handle_msg1(replay).await; + assert!( + nodes[1] + .node + .get_peer(&node0_addr) + .unwrap() + .pending_new_session() + .is_none(), + "a copy of a msg1 from before the peering was re-formed must not arm a pending" + ); + + cleanup_nodes(&mut nodes).await; +} + /// The responder hold is the drain ceiling at stock settings, and a raised /// link-dead timeout or heartbeat interval raises it past that ceiling, taking /// the larger of the two rather than their sum. @@ -4450,8 +5511,15 @@ fn arm_stranger_handshake_beside_stale_pending( async fn test_forged_msg3_against_a_peer_armed_handshake_leaves_the_completed_epoch_intact() { let peer = Identity::generate(); let stranger = Identity::generate(); - let (mut node, peer_addr, _valid_msg3) = + let (mut node, peer_addr, valid_msg3) = install_stale_pending_beside_a_stranger_armed_handshake(&peer, &stranger); + // Backdate the stamp the handshake's deadline runs from, so a restore + // that restamped it to the current time could not match by accident. + let armed_at = wall_clock_ms() - 5_000; + node.sessions + .get_mut(&peer_addr) + .unwrap() + .record_peer_rekey(armed_at); // Garbage of the right length: `read_xk_message_3` fails on the AEAD. let forged = SessionMsg3::new(vec![0u8; crate::noise::XK_HANDSHAKE_MSG3_SIZE]).encode(); @@ -4459,19 +5527,44 @@ async fn test_forged_msg3_against_a_peer_armed_handshake_leaves_the_completed_ep .await; let entry = node.sessions.get(&peer_addr).expect("session present"); + assert_eq!( + entry.last_peer_rekey_ms(), + armed_at, + "the restore must not restamp the handshake's deadline, or a spray \ + would hold it open" + ); assert!( entry.pending_new_session().is_some(), "an unauthenticated msg3 must not discard the key epoch the peer may \ - already have cut over to; only the handshake it failed belongs to it" + already have cut over to" ); assert!( entry.is_established(), "the running session must be left intact alongside the pending one" ); assert!( - !entry.has_rekey_in_progress(), - "the handshake the msg3 failed against must still be abandoned" + entry.has_rekey_in_progress() && !entry.is_rekey_initiator(), + "an unreadable msg3 must not discard the handshake it failed against" ); + + // The handshake went back rolled back: the msg3 that genuinely finishes + // it still reads, and the key-mismatch arm, not the read, refuses it. + node.handle_session_payload( + &peer_addr, + &stub_link_peer(), + &SessionMsg3::new(valid_msg3).encode(), + 1280, + false, + ) + .await; + assert_eq!( + node.stats().session.rekey_key_mismatch, + 1, + "the kept handshake must still read the msg3 that finishes it" + ); + let entry = node.sessions.get(&peer_addr).expect("session present"); + assert!(entry.pending_new_session().is_some()); + assert!(!entry.has_rekey_in_progress()); } #[tokio::test] @@ -5284,6 +6377,75 @@ async fn test_forged_session_ack_leaves_the_initiation_able_to_complete_on_the_g cleanup_nodes(&mut nodes).await; } +/// A garbage msg3 that reaches the responder of an initial handshake after +/// the initiator has sent its genuine msg3 must not cost the genuine one the +/// half-open entry it completes. +#[tokio::test] +async fn test_forged_initial_msg3_leaves_the_responder_able_to_complete_on_the_genuine_msg3() { + let mut nodes = make_rekey_disabled_pair().await; + let node0_addr = *nodes[0].node.node_addr(); + let node1_addr = *nodes[1].node.node_addr(); + let node1_pubkey = nodes[1].node.identity().pubkey_full(); + + nodes[0] + .node + .initiate_session(node1_addr, node1_pubkey) + .await + .expect("initiate_session failed"); + tokio::time::sleep(Duration::from_millis(20)).await; + process_available_packets(&mut nodes[1..]).await; + let activity_before = nodes[1] + .node + .get_session(&node0_addr) + .filter(|e| e.is_awaiting_msg3()) + .expect("node 1 must be awaiting msg3 after answering the setup") + .last_activity(); + tokio::time::sleep(Duration::from_millis(20)).await; + process_available_packets(&mut nodes[..1]).await; + assert!( + nodes[0] + .node + .get_session(&node1_addr) + .is_some_and(|e| e.is_established()), + "node 0 must be established once it has sent msg3" + ); + + // Hold node 0's genuine msg3 and deliver the forgery first. + tokio::time::sleep(Duration::from_millis(20)).await; + let held: Vec<_> = std::iter::from_fn(|| nodes[1].packet_rx.try_recv().ok()).collect(); + assert!(!held.is_empty(), "node 0's msg3 must be queued at node 1"); + let forged = SessionMsg3::new(vec![0u8; crate::noise::XK_HANDSHAKE_MSG3_SIZE]).encode(); + nodes[1] + .node + .handle_session_payload(&node0_addr, &node0_addr, &forged, 1280, false) + .await; + let entry = nodes[1] + .node + .get_session(&node0_addr) + .expect("an unauthenticated msg3 must not destroy the half-open entry"); + assert!(entry.is_awaiting_msg3()); + assert_eq!( + entry.last_activity(), + activity_before, + "the reinsert must not push the handshake sweep's deadline out, or a \ + spray would keep a dead entry alive" + ); + + for packet in held { + nodes[1].node.handle_encrypted_frame(packet).await; + } + pump_until_quiet(&mut nodes).await; + assert!( + nodes[1] + .node + .get_session(&node0_addr) + .is_some_and(|e| e.is_established()), + "the genuine msg3 must still complete the session at node 1" + ); + + cleanup_nodes(&mut nodes).await; +} + // ============================================================================ // Integration tests: a lost initial msg3 // ============================================================================ @@ -6357,25 +7519,23 @@ async fn test_setup_naming_a_peer_whose_address_sorts_below_ours_yields_our_reke } #[tokio::test] -async fn test_losing_the_tiebreak_against_a_peer_armed_handshake_keeps_the_completed_epoch() { +async fn test_a_setup_against_a_peer_armed_handshake_is_dropped_and_keeps_the_handshake_and_the_completed_epoch() + { let mut config = Config::new(); config.node.rekey.enabled = false; let mut node = make_node_with(config); - // Our address sorts larger, so the second setup loses the tie-break. - // Which side of it a given pair lands on is fixed by the two addresses, - // not chosen by the sender, so this is half of all peers rather than - // something an attacker selects. + // Our address sorts larger, which is the end that used to yield: the + // tie-break would make us the responder to the second setup. let peer = peer_identity_sorting_below(node.node_addr()); let peer_addr = install_established_peer(&mut node, &peer); - // What a first forged setup leaves: a handshake the *stranger* armed, - // beside a completed epoch too stale for `pending_outranks` to veto. The - // tie-break arm gates on `has_rekey_in_progress`, which this satisfies, - // so a second forged setup reaches the yield with a pending session - // present. Nothing here required us to be the rekey initiator. + // What a first setup leaves: a handshake the sender armed, beside a + // completed epoch too stale for `pending_outranks` to veto, so only the + // armed handshake stands between a second setup and the arming path. let stranger = Identity::generate(); - arm_stranger_handshake_beside_stale_pending(&mut node, &peer_addr, &peer, &stranger); + let valid_msg3 = + arm_stranger_handshake_beside_stale_pending(&mut node, &peer_addr, &peer, &stranger); assert!( !node.sessions.get(&peer_addr).unwrap().is_rekey_initiator(), "the state under test is a handshake we did not arm" @@ -6385,26 +7545,37 @@ async fn test_losing_the_tiebreak_against_a_peer_armed_handshake_keeps_the_compl node.handle_session_payload(&peer_addr, &stub_link_peer(), &forged, 1280, false) .await; - let entry = node.sessions.get(&peer_addr).expect("session present"); + let stats = &node.stats().session; assert_eq!( - node.stats().session.rekey_yielded, - 1, - "the test must actually reach the yield arm, or it proves nothing" + stats.rekey_held, 1, + "a setup meeting a peer-armed handshake must be dropped and counted" + ); + assert_eq!( + (stats.rekey_yielded, stats.rekey_tiebreak, stats.rekey_armed), + (0, 0, 0), + "the tie-break is for two initiators; nothing may yield or arm here" + ); + let entry = node.sessions.get(&peer_addr).expect("session present"); + assert!( + entry.pending_new_session().is_some() && entry.is_established(), + "the completed epoch and the running session must be left intact" ); assert!( - entry.pending_new_session().is_some(), - "yielding a tie-break to an unauthenticated setup must not discard \ - the key epoch the peer may already have cut over to; two forged \ - setups would otherwise kill the reverse direction" - ); - assert!( - entry.is_established(), - "the running session must be left intact alongside the pending one" - ); - assert!( - !entry.has_rekey_in_progress(), - "the handshake we yielded must still be abandoned" + entry.has_rekey_in_progress() && !entry.is_rekey_initiator(), + "the handshake the peer armed must survive the setup, since a peer \ + that read our SessionAck needs it for its msg3" ); + + // The kept handshake is the same one: its own msg3 still reads. + node.handle_session_payload( + &peer_addr, + &stub_link_peer(), + &SessionMsg3::new(valid_msg3).encode(), + 1280, + false, + ) + .await; + assert_eq!(node.stats().session.rekey_key_mismatch, 1); } #[tokio::test] @@ -6891,19 +8062,19 @@ async fn test_a_rekey_whose_session_ack_was_lost_is_retired_after_the_handshake_ /// /// Both nodes would rekey after one message; the initiator is picked at run /// time so that the responder holds the smaller address when -/// `responder_wins`, and the larger otherwise. Only the initiator's tick is -/// run before the responder has armed, after which the responder is +/// `responder_smaller`, and the larger otherwise. Only the initiator's tick +/// is run before the responder has armed, after which the responder is /// dampened and cannot start a rekey of its own inside the test. /// -/// A responder that wins the tie-break drops the retry before arming -/// anything, so both handshakes expire and the next retry completes one -/// timeout later. A responder that loses yields and answers the retry at -/// once. -async fn lostack_retry(responder_wins: bool) { +/// A responder holding a handshake its peer armed drops the retry before +/// arming anything, at either address, since that handshake is the only one +/// a peer that read the SessionAck could finish. Both handshakes then +/// expire, and the next retry completes one timeout later. +async fn lostack_retry(responder_smaller: bool) { let mut nodes = rekey_pair([true, true], Some(1)).await; let node0_smaller = crate::proto::fsp::initiation_winner(nodes[0].node.node_addr(), nodes[1].node.node_addr()); - let resp = if responder_wins == node0_smaller { + let resp = if responder_smaller == node0_smaller { 0 } else { 1 @@ -6948,56 +8119,52 @@ async fn lostack_retry(responder_wins: bool) { tokio::time::sleep(Duration::from_millis(20)).await; process_available_packets(&mut nodes[resp..=resp]).await; let stats = &nodes[resp].node.stats().session; - let (tiebreak, yielded) = (stats.rekey_tiebreak, stats.rekey_yielded); assert_eq!( - tiebreak + yielded, - 1, + stats.rekey_held, 1, "the retry must have met the responder's stale handshake" ); - if responder_wins { - assert_eq!(tiebreak, 1, "the smaller responder must win"); - } else { - assert_eq!(yielded, 1, "the larger responder must yield"); - } + assert_eq!( + (stats.rekey_tiebreak, stats.rekey_yielded), + (0, 0), + "a handshake the peer armed is not a dual initiation at either address" + ); pump_all(&mut nodes).await; - if responder_wins { - assert!( - !holds_pending(&nodes[init], &resp_addr) && !holds_pending(&nodes[resp], &init_addr), - "a retry the responder dropped completes nothing" - ); - tokio::time::sleep(Duration::from_millis(1200)).await; - nodes[resp].node.check_session_rekey().await; - assert_eq!( - nodes[resp].node.stats().session.rekey_expired, - 1, - "the responder's stale handshake must expire on its own rule" - ); - nodes[init].node.check_session_rekey().await; - assert!( - !nodes[init] - .node - .get_session(&resp_addr) - .unwrap() - .has_rekey_in_progress(), - "the retry the responder dropped must itself be retired" - ); - nodes[init].node.check_session_rekey().await; - assert!( - rekey_initiated(&nodes[init], &resp_addr), - "the trigger must start a second retry" - ); - pump_all(&mut nodes).await; - } + assert!( + !holds_pending(&nodes[init], &resp_addr) && !holds_pending(&nodes[resp], &init_addr), + "a retry the responder dropped completes nothing" + ); + tokio::time::sleep(Duration::from_millis(1200)).await; + nodes[resp].node.check_session_rekey().await; + assert_eq!( + nodes[resp].node.stats().session.rekey_expired, + 1, + "the responder's stale handshake must expire on its own rule" + ); + nodes[init].node.check_session_rekey().await; + assert!( + !nodes[init] + .node + .get_session(&resp_addr) + .unwrap() + .has_rekey_in_progress(), + "the retry the responder dropped must itself be retired" + ); + nodes[init].node.check_session_rekey().await; + assert!( + rekey_initiated(&nodes[init], &resp_addr), + "the trigger must start a second retry" + ); + pump_all(&mut nodes).await; assert!( holds_pending(&nodes[init], &resp_addr) && holds_pending(&nodes[resp], &init_addr), "the retried rekey must complete on both nodes" ); - // The first handshake always; the retry too when the responder dropped it. + // The first handshake and the retry the responder dropped. assert_eq!( nodes[init].node.stats().session.rekey_unanswered, - if responder_wins { 2 } else { 1 }, + 2, "every retired handshake must be counted once" ); assert_eq!(nodes[resp].node.stats().session.rekey_unanswered, 0); @@ -7013,13 +8180,274 @@ async fn test_a_retry_dropped_by_a_smaller_responders_stale_handshake_completes_ lostack_retry(true).await; } -/// A retry that a larger responder, still holding its stale handshake, -/// yields to completes at once. +/// A retry dropped by a larger responder still holding its stale handshake +/// completes once both handshakes have expired, as at a smaller one: the +/// larger end no longer yields a handshake its peer armed. #[tokio::test] -async fn test_a_retry_that_a_larger_responder_yields_to_completes_at_once() { +async fn test_a_retry_dropped_by_a_larger_responders_stale_handshake_completes_one_timeout_later() { lostack_retry(false).await; } +// ============================================================================ +// Integration tests: a forgery inside a rekey responder's msg2-to-msg3 window +// ============================================================================ + +/// An unauthenticated message delivered to a rekey responder after the +/// initiator has read its SessionAck and before the initiator's msg3 arrives. +#[derive(Clone, Copy, Debug)] +enum WindowForgery { + /// Nothing is forged: the control the other two are measured against. + Nothing, + /// A SessionMsg3 of the right size whose first AEAD cannot open. + Msg3, + /// A SessionSetup carrying a stranger's ephemeral under the initiator's + /// address, delivered at the larger address, where the dual-initiation + /// tie-break would yield to it. + Setup, +} + +/// What a rekey left behind once a forgery may have reached its window. +#[derive(Debug, PartialEq, Eq)] +struct WindowOutcome { + /// The responder completed the handshake on the initiator's genuine msg3. + responder_completed: bool, + /// The initiator cut over on its liveness timer. + initiator_cut_over: bool, + /// A frame the initiator sent after its cutover and after its whole msg3 + /// resend budget decoded at the responder. + initiator_to_responder: bool, + /// A frame the responder sent decoded at the initiator. + responder_to_initiator: bool, + /// After the responder's own next rekey, frames decode both ways. + next_cycle_carries_both_ways: bool, +} + +/// The outcome of a rekey nothing interfered with. +const WINDOW_HEALTHY: WindowOutcome = WindowOutcome { + responder_completed: true, + initiator_cut_over: true, + initiator_to_responder: true, + responder_to_initiator: true, + next_cycle_carries_both_ways: true, +}; + +/// Send one data frame from `nodes[from]` to `nodes[to]`, deliver it, and +/// report whether it decoded at `nodes[to]`. +async fn session_frame_decodes(nodes: &mut [TestNode], from: usize, to: usize) -> bool { + let from_addr = *nodes[from].node.node_addr(); + let to_addr = *nodes[to].node.node_addr(); + let received = |nodes: &[TestNode]| { + nodes[to] + .node + .get_session(&from_addr) + .expect("the session must survive") + .traffic_counters() + .1 + }; + let before = received(nodes); + nodes[from] + .node + .send_session_data(&to_addr, 0, 0, b"window forgery probe") + .await + .expect("send_session_data failed"); + pump_all(nodes).await; + received(nodes) == before + 1 +} + +/// Cut `nodes[init]` over to the rekey it initiated by running its tick with +/// the liveness timer already elapsed, and report whether it did. +async fn cut_over_initiator(nodes: &mut [TestNode], init: usize, resp_addr: &NodeAddr) -> bool { + nodes[init] + .node + .sessions + .get_mut(resp_addr) + .unwrap() + .set_rekey_completed_ms(wall_clock_ms() - 10_000); + nodes[init].node.check_session_rekey().await; + let entry = nodes[init].node.get_session(resp_addr).unwrap(); + entry.pending_new_session().is_none() && !entry.has_rekey_in_progress() +} + +/// Drive a genuine FSP rekey to the point where the initiator has read the +/// SessionAck and derived the new session, deliver `forgery` to the +/// responder, then release the initiator's genuine msg3 and follow the cycle +/// through the initiator's cutover, its msg3 resend budget, and one data frame +/// each way. Then let the responder start the next rekey and check that it +/// carries frames both ways. +/// +/// The responder is chosen at run time to hold the larger address, the end +/// at which the dual-initiation tie-break yields; a smaller responder wins it +/// and drops the forged setup regardless. The forged msg3 reaches the same +/// arm at either end. +/// +/// Returns the outcome and a note of the responder's counters for the +/// failure message: msg3s refused for arriving with no handshake to read +/// them, and dual-initiation yields. +async fn rekey_with_window_forgery(forgery: WindowForgery) -> (WindowOutcome, String) { + use crate::transport::ReceivedPacket; + + let mut nodes = rekey_pair([true, true], None).await; + let node0_smaller = + crate::proto::fsp::initiation_winner(nodes[0].node.node_addr(), nodes[1].node.node_addr()); + let (init, resp) = if node0_smaller { (0, 1) } else { (1, 0) }; + let init_addr = *nodes[init].node.node_addr(); + let resp_addr = *nodes[resp].node.node_addr(); + assert!( + !crate::proto::fsp::initiation_winner(&resp_addr, &init_addr), + "the responder must hold the larger address" + ); + + // The setup reaches the responder only; it arms and answers. + start_rekey(&mut nodes, init).await; + tokio::time::sleep(Duration::from_millis(20)).await; + process_available_packets(&mut nodes[resp..=resp]).await; + assert_eq!( + nodes[resp].node.stats().session.rekey_armed, + 1, + "the responder must have armed" + ); + + // The SessionAck reaches the initiator only; it derives the new session + // and sends msg3, which waits in the responder's queue. + tokio::time::sleep(Duration::from_millis(20)).await; + process_available_packets(&mut nodes[init..=init]).await; + assert!( + holds_pending(&nodes[init], &resp_addr) + && !nodes[init] + .node + .get_session(&resp_addr) + .unwrap() + .has_rekey_in_progress(), + "the initiator must have read the SessionAck and hold the new session" + ); + assert!( + nodes[resp] + .node + .get_session(&init_addr) + .is_some_and(|e| e.has_rekey_in_progress() && !e.is_rekey_initiator()), + "the responder must still be waiting for msg3" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + let held: Vec = + std::iter::from_fn(|| nodes[resp].packet_rx.try_recv().ok()).collect(); + assert!( + !held.is_empty(), + "the initiator's msg3 must be queued at the responder" + ); + + match forgery { + WindowForgery::Nothing => {} + WindowForgery::Msg3 => { + let forged = SessionMsg3::new(vec![0u8; crate::noise::XK_HANDSHAKE_MSG3_SIZE]).encode(); + nodes[resp] + .node + .handle_session_payload(&init_addr, &init_addr, &forged, 1280, false) + .await; + } + WindowForgery::Setup => { + let forged = forge_setup_for(&nodes[resp].node); + nodes[resp] + .node + .handle_session_payload(&init_addr, &init_addr, &forged, 1280, false) + .await; + assert_eq!( + nodes[resp].node.stats().session.rekey_held, + 1, + "the forged setup must have reached the armed handshake and \ + been held off, or the test measures nothing" + ); + } + } + + // Release the genuine msg3. + for packet in held { + nodes[resp].node.handle_encrypted_frame(packet).await; + } + pump_all(&mut nodes).await; + let responder_completed = holds_pending(&nodes[resp], &init_addr); + + // The initiator cuts over on its timer, then spends its msg3 resend + // budget: a resend is what recovers a msg3 that was merely lost. + let initiator_cut_over = cut_over_initiator(&mut nodes, init, &resp_addr).await; + let max_resends = nodes[init] + .node + .config() + .node + .rate_limit + .handshake_max_resends; + let base_ms = Node::now_ms(); + for step in 1..=u64::from(max_resends) + 1 { + nodes[init] + .node + .resend_pending_session_msg3(base_ms + step * 64_000) + .await; + pump_all(&mut nodes).await; + } + + let initiator_to_responder = session_frame_decodes(&mut nodes, init, resp).await; + let responder_to_initiator = session_frame_decodes(&mut nodes, resp, init).await; + + // The responder's own next rekey, once past the dampening that the + // initiator's setup started. Its data frame above crossed its trigger. + nodes[resp] + .node + .sessions + .get_mut(&init_addr) + .unwrap() + .record_peer_rekey(wall_clock_ms() - 60_000); + nodes[resp].node.check_session_rekey().await; + assert!( + rekey_initiated(&nodes[resp], &init_addr), + "the responder must start the next rekey" + ); + pump_all(&mut nodes).await; + let next_cycle_carries_both_ways = cut_over_initiator(&mut nodes, resp, &init_addr).await + && session_frame_decodes(&mut nodes, resp, init).await + && session_frame_decodes(&mut nodes, init, resp).await; + + let stats = &nodes[resp].node.stats().session; + let note = format!( + "the responder refused {} msg3s and yielded {} times", + stats.bad_state, stats.rekey_yielded + ); + cleanup_nodes(&mut nodes).await; + ( + WindowOutcome { + responder_completed, + initiator_cut_over, + initiator_to_responder, + responder_to_initiator, + next_cycle_carries_both_ways, + }, + note, + ) +} + +/// The control: with nothing forged, the procedure that measures the two +/// forgeries completes the rekey and carries frames both ways on both cycles. +#[tokio::test] +async fn a_rekey_with_nothing_forged_in_the_responders_window_completes_and_carries_traffic_both_ways() + { + let (outcome, note) = rekey_with_window_forgery(WindowForgery::Nothing).await; + assert_eq!(outcome, WINDOW_HEALTHY, "{note}"); +} + +/// A garbage msg3 that reaches a rekey responder after the initiator has read +/// the SessionAck must not cost the genuine msg3 its handshake. +#[tokio::test] +async fn a_forged_msg3_in_the_responders_window_does_not_split_the_rekey() { + let (outcome, note) = rekey_with_window_forgery(WindowForgery::Msg3).await; + assert_eq!(outcome, WINDOW_HEALTHY, "{note}"); +} + +/// A forged setup at a larger rekey responder, after the initiator has read +/// the SessionAck, must not cost the genuine msg3 its handshake. +#[tokio::test] +async fn a_forged_setup_at_the_larger_responder_in_its_window_does_not_split_the_rekey() { + let (outcome, note) = rekey_with_window_forgery(WindowForgery::Setup).await; + assert_eq!(outcome, WINDOW_HEALTHY, "{note}"); +} + /// A forged SessionAck arriving midway through an unanswered rekey must not /// restart its deadline: the rekey is retired on the timeout measured from /// the setup this node sent. diff --git a/src/noise/handshake.rs b/src/noise/handshake.rs index 51dcbd49..035d4b4a 100644 --- a/src/noise/handshake.rs +++ b/src/noise/handshake.rs @@ -15,9 +15,10 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; /// /// Maintains the chaining key (ck), handshake hash (h), and current cipher. /// -/// `Clone` exists for [`HandshakeState::try_read_message_2`] and -/// [`HandshakeState::try_read_xk_message_2`], which have to put the pre-read -/// state back after a message that mixed material in before failing to +/// `Clone` exists for [`HandshakeState::try_read_message_2`], +/// [`HandshakeState::try_read_xk_message_2`] and +/// [`HandshakeState::try_read_xk_message_3`], which have to put the pre-read +/// state back after a message that advanced it before failing to /// authenticate. /// /// `ck` and `h` are cleared on drop, including on the clone above once it @@ -1018,6 +1019,40 @@ impl HandshakeState { Ok(()) } + /// Read XK message 3, leaving the handshake untouched when the message + /// does not authenticate. + /// + /// `read_xk_message_3` advances the symmetric state's nonce before the + /// first AEAD opens, and a message that fails later has already mixed a + /// DH result into the key, so a failed read leaves a handshake that can + /// never read the genuine msg3 afterwards. A responder that keeps its + /// handshake across a failed read, because the message may be a forgery + /// rather than the initiator's corrupt msg3, needs the pre-read state + /// back. + /// + /// The saved set is exactly what `read_xk_message_3` writes: + /// `symmetric`, `remote_static`, `remote_epoch` and `progress`. **That + /// mirror is manual.** A later edit that adds a write to + /// `read_xk_message_3` without adding it here silently reintroduces the + /// poisoning, and no caller can detect it. + pub fn try_read_xk_message_3(&mut self, message: &[u8]) -> Result<(), NoiseError> { + let symmetric = self.symmetric.clone(); + let remote_static = self.remote_static; + let remote_epoch = self.remote_epoch; + let progress = self.progress; + + match self.read_xk_message_3(message) { + Ok(()) => Ok(()), + Err(e) => { + self.symmetric = symmetric; + self.remote_static = remote_static; + self.remote_epoch = remote_epoch; + self.progress = progress; + Err(e) + } + } + } + /// Complete the handshake and return a NoiseSession. /// /// Must be called after the handshake is complete. diff --git a/src/noise/tests.rs b/src/noise/tests.rs index baabfc04..4c5e86b3 100644 --- a/src/noise/tests.rs +++ b/src/noise/tests.rs @@ -693,6 +693,67 @@ fn test_xk_wrong_state_errors() { ); } +/// Drive an XK handshake to the point where the responder is waiting for +/// msg3, and return the initiator's genuine msg3 with the responder. +fn xk_responder_awaiting_msg3() -> (HandshakeState, Vec) { + let initiator_keypair = generate_keypair(); + let responder_keypair = generate_keypair(); + let mut initiator = + HandshakeState::new_xk_initiator(initiator_keypair, responder_keypair.public_key()); + initiator.set_local_epoch(generate_epoch()); + let mut responder = HandshakeState::new_xk_responder(responder_keypair); + responder.set_local_epoch(generate_epoch()); + + let msg1 = initiator.write_xk_message_1().unwrap(); + responder.read_xk_message_1(&msg1).unwrap(); + let msg2 = responder.write_xk_message_2().unwrap(); + initiator.read_xk_message_2(&msg2).unwrap(); + let msg3 = initiator.write_xk_message_3().unwrap(); + (responder, msg3) +} + +#[test] +fn test_a_failed_rolling_back_xk_msg3_read_still_reads_the_genuine_msg3() { + let (mut responder, msg3) = xk_responder_awaiting_msg3(); + + // Fails at the first AEAD, after the nonce has advanced. + assert!( + responder + .try_read_xk_message_3(&[0u8; XK_HANDSHAKE_MSG3_SIZE]) + .is_err() + ); + // Fails at the epoch AEAD, after the static was learned and the se DH + // mixed into the key. + let mut tampered = msg3.clone(); + let last = tampered.len() - 1; + tampered[last] ^= 0x01; + assert!(responder.try_read_xk_message_3(&tampered).is_err()); + assert!( + responder.remote_static().is_none(), + "a failed read must not leave the static it decrypted behind" + ); + assert!(!responder.is_complete()); + + responder + .try_read_xk_message_3(&msg3) + .expect("the genuine msg3 must still read after two failed reads"); + assert!(responder.is_complete()); + assert!(responder.into_session().is_ok()); +} + +#[test] +fn test_a_failed_plain_xk_msg3_read_cannot_read_the_genuine_msg3() { + // The control for the rolling-back read: without the rollback, the + // failed read leaves a handshake the genuine msg3 no longer opens. + let (mut responder, msg3) = xk_responder_awaiting_msg3(); + assert!( + responder + .read_xk_message_3(&[0u8; XK_HANDSHAKE_MSG3_SIZE]) + .is_err() + ); + assert!(responder.read_xk_message_3(&msg3).is_err()); +} + #[test] fn test_xk_handshake_hash_differs_from_ik() { // XK and IK should produce different handshake hashes (different protocol names) diff --git a/src/nostr/runtime.rs b/src/nostr/runtime.rs index 557441b0..7d1ae7b6 100644 --- a/src/nostr/runtime.rs +++ b/src/nostr/runtime.rs @@ -1,7 +1,8 @@ +use portable_atomic::AtomicU64; use std::collections::{HashMap, HashSet}; use std::net::SocketAddr; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::time::{Duration, Instant}; use nostr::nips::nip17; diff --git a/src/packaging_tests.rs b/src/packaging_tests.rs index a361af42..e68df0a9 100644 --- a/src/packaging_tests.rs +++ b/src/packaging_tests.rs @@ -700,6 +700,110 @@ fn windows_installer_restricts_config_dir_before_any_path_inside_it() { } } +/// Guards what install-service.ps1's ACL grants and who it makes owner, not +/// only the order of the steps. +/// +/// The ACL must allow SYSTEM and Administrators full control and grant no one +/// else anything: granting Users read would expose the identity key and make +/// an unelevated `fipsctl address` work again. Both `/setowner` calls must +/// name Administrators. The PowerShell 7 `FileSystemAclExtensions` type must +/// be used only on Core, since Windows PowerShell 5.1's .NET Framework lacks +/// it. +#[test] +fn windows_installer_acl_allows_only_system_and_administrators_and_makes_administrators_owner() { + let lines = ps_lines(&repo_file("packaging/windows/install-service.ps1")); + + let sid_loops: Vec = lines + .iter() + .enumerate() + .filter(|(_, l)| l.starts_with("foreach ($sid in ")) + .map(|(i, _)| i) + .collect(); + assert_eq!( + sid_loops.len(), + 1, + "install-service.ps1: expected one loop over the SIDs the ACL allows" + ); + let sid_loop = sid_loops[0]; + assert_eq!( + lines[sid_loop], r#"foreach ($sid in @("S-1-5-18", "S-1-5-32-544")) {"#, + "install-service.ps1: the ACL must allow exactly SYSTEM and Administrators" + ); + let body = &lines[sid_loop + 1..block_end(&lines, sid_loop)]; + for (what, want) in [ + ( + "the rule's account", + "[System.Security.Principal.SecurityIdentifier]::new($sid),", + ), + ( + "the rule's rights", + "[System.Security.AccessControl.FileSystemRights]::FullControl,", + ), + ( + "the rule's type", + "[System.Security.AccessControl.AccessControlType]::Allow)", + ), + ("the rule's addition to $acl", "$acl.AddAccessRule($rule)"), + ] { + assert!( + body.iter().any(|l| l == want), + "install-service.ps1: the SID loop does not have {what} as {want}" + ); + } + for needle in [ + "FileSystemAccessRule", + "FileSystemRights]::", + "AccessControlType]::", + "AccessRule(", + ] { + let uses = lines.iter().filter(|l| l.contains(needle)).count(); + let inside = body.iter().filter(|l| l.contains(needle)).count(); + assert!( + uses == 1 && inside == 1, + "install-service.ps1: expected {needle} exactly once, inside the SID loop; \ + found {uses}, {inside} inside" + ); + } + + let setowners: Vec<&String> = lines.iter().filter(|l| l.contains("/setowner")).collect(); + assert!( + !setowners.is_empty(), + "install-service.ps1: no /setowner line" + ); + for l in setowners { + assert!( + l.contains(r#"/setowner "*S-1-5-32-544" "#), + "install-service.ps1: /setowner must make Administrators the owner: {l}" + ); + } + + let editions: Vec = lines + .iter() + .enumerate() + .filter(|(_, l)| l.contains("PSEdition")) + .map(|(i, _)| i) + .collect(); + assert_eq!( + editions.len(), + 1, + "install-service.ps1: expected one PSEdition test" + ); + let at = editions[0]; + let want = [ + r#"if ($PSVersionTable.PSEdition -eq "Core") {"#, + "[System.IO.FileSystemAclExtensions]::CreateDirectory($acl, $ConfigDir) | Out-Null", + "} else {", + "[System.IO.Directory]::CreateDirectory($ConfigDir, $acl) | Out-Null", + "}", + ]; + assert_eq!( + lines.get(at..at + want.len()).unwrap_or_default(), + want, + "install-service.ps1: FileSystemAclExtensions must be used only when PSEdition is \ + Core, and Directory.CreateDirectory otherwise" + ); +} + /// Guards the conditions under which install-service.ps1 refuses its config /// directory, not only their position. /// @@ -729,17 +833,33 @@ fn windows_installer_refusal_conditions_stop_the_install() { let trusted = find("list of trusted owners", &|l| { l.starts_with("$trustedOwners = @(") }); - for needle in [ - "\"S-1-5-18\"", - "\"S-1-5-32-544\"", - "WindowsIdentity]::GetCurrent().User.Value", - ] { - assert!( - lines[trusted[0]].contains(needle), - "install-service.ps1: the trusted owners do not include {needle}: {}", - lines[trusted[0]] - ); - } + assert_eq!( + trusted.len(), + 1, + "install-service.ps1: expected one list of trusted owners" + ); + let owners: Vec<&str> = lines[trusted[0]] + .strip_prefix("$trustedOwners = @(") + .and_then(|l| l.strip_suffix(')')) + .unwrap_or_else(|| { + panic!( + "install-service.ps1: the trusted owners are not one @( ) list: {}", + lines[trusted[0]] + ) + }) + .split(',') + .map(str::trim) + .collect(); + assert_eq!( + owners, + [ + "\"S-1-5-18\"", + "\"S-1-5-32-544\"", + "[System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value", + ], + "install-service.ps1: the trusted owners must be exactly SYSTEM, Administrators \ + and the installing account" + ); for i in find("owner refusal", &|l| { l == "if ($trustedOwners -notcontains $ownerSid) {" }) { @@ -874,11 +994,9 @@ fn windows_installer_creates_empty_peer_acl_files_and_refuses_legacy_ones() { .unwrap_or_else(|| panic!("install-service.ps1: no line {what}")) }; - let legacy_dir = first("assigning $legacyAclDir", &|l| { - l.starts_with("$legacyAclDir = ") - }); + let legacy_dir = first("assigning $legacyDir", &|l| l.starts_with("$legacyDir = ")); assert_eq!( - lines[legacy_dir], r#"$legacyAclDir = "$env:SystemDrive\etc\fips""#, + lines[legacy_dir], r#"$legacyDir = "$env:SystemDrive\etc\fips""#, "install-service.ps1: the legacy peer ACL directory is not \\etc\\fips on the \ system drive" ); @@ -906,7 +1024,7 @@ fn windows_installer_creates_empty_peer_acl_files_and_refuses_legacy_ones() { let creation_body = creation + 1..creation_end; for text in [ - "$legacy = Join-Path $legacyAclDir $name", + "$legacy = Join-Path $legacyDir $name", r#"$current = "$ConfigDir\$name""#, ] { assert!( @@ -967,7 +1085,7 @@ fn windows_installer_creates_empty_peer_acl_files_and_refuses_legacy_ones() { let is_check = |l: &str| l == "& $refuseEntries"; let order = [ ("check after the reset", all(&is_check).get(2).copied()), - ("$legacyAclDir", Some(legacy_dir)), + ("$legacyDir", Some(legacy_dir)), ("refusal loop", Some(refusal)), ("refusal of a legacy file", Some(check)), ("end of the refusal loop", Some(refusal_end)), @@ -998,6 +1116,93 @@ fn windows_installer_creates_empty_peer_acl_files_and_refuses_legacy_ones() { } } +/// Guards install-service.ps1's refusal of an identity key left in +/// `\etc\fips`. +/// +/// A service that earlier releases ran from `\etc\fips` reads only +/// `C:\ProgramData\fips` once the installer sets `FIPS_CONFIG`, so a key left +/// in `\etc\fips` with none in the config directory means the node would +/// come up with a new identity. The installer must stop in exactly that case, +/// and must decide it before it creates any file in the config directory, +/// copies the binaries or registers the service, so a refusal leaves an +/// existing install as it was. +#[test] +fn windows_installer_refuses_a_legacy_identity_key_with_none_in_the_config_dir() { + let lines = ps_lines(&repo_file("packaging/windows/install-service.ps1")); + let first = |what: &str, pred: &dyn Fn(&str) -> bool| -> usize { + lines + .iter() + .position(|l| pred(l)) + .unwrap_or_else(|| panic!("install-service.ps1: no line {what}")) + }; + + let dir = first("assigning $legacyDir", &|l| l.starts_with("$legacyDir = ")); + let key = first("assigning $legacyKey", &|l| l.starts_with("$legacyKey = ")); + assert_eq!( + lines[key], r#"$legacyKey = Join-Path $legacyDir "fips.key""#, + "install-service.ps1: the legacy key is not fips.key in $legacyDir" + ); + let checks: Vec = lines + .iter() + .enumerate() + .filter(|(_, l)| l.starts_with("if (") && l.contains("$legacyKey")) + .map(|(i, _)| i) + .collect(); + assert_eq!( + checks.len(), + 1, + "install-service.ps1: expected one test of the legacy key, found {}", + checks.len() + ); + let check = checks[0]; + assert_eq!( + lines[check], + r#"if ((Test-Path -LiteralPath $legacyKey) -and -not (Test-Path -LiteralPath "$ConfigDir\fips.key")) {"#, + "install-service.ps1: the refusal must hold only when the legacy key exists and \ + the config directory has none" + ); + refuses_at(&lines, check, "refusal of a legacy identity key"); + + let order = [ + ("$legacyDir", Some(dir)), + ("$legacyKey", Some(key)), + ("refusal of a legacy identity key", Some(check)), + ( + "creation of a peer ACL file", + lines + .iter() + .position(|l| l.contains("New-Item") && l.contains("-ItemType File")), + ), + ( + "binary copy", + lines.iter().position(|l| l.contains("$Binaries")), + ), + ( + "default config copy", + lines + .iter() + .position(|l| l.contains("Copy-Item") && l.contains("$ConfigDir\\fips.yaml")), + ), + ( + "service registration", + lines.iter().position(|l| l.contains("--install-service")), + ), + ]; + for pair in order.windows(2) { + let [(a, ia), (b, ib)] = pair else { + unreachable!("windows(2) yields pairs") + }; + let (ia, ib) = ( + ia.unwrap_or_else(|| panic!("install-service.ps1: no {a}")), + ib.unwrap_or_else(|| panic!("install-service.ps1: no {b}")), + ); + assert!( + ia < ib, + "install-service.ps1: {a} (code line {ia}) must come before {b} (code line {ib})" + ); + } +} + const COMMON_CONFIG: &str = "packaging/common/fips.yaml"; const OPENWRT_CONFIG: &str = "packaging/openwrt-ipk/files/etc/fips/fips.yaml"; diff --git a/src/peer/active.rs b/src/peer/active.rs index 0c7e4291..2e37b1af 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -7,7 +7,7 @@ use crate::config::MmpConfig; use crate::node::REKEY_JITTER_SECS; use crate::noise::{HandshakeState as NoiseHandshakeState, NoiseError, NoiseSession}; use crate::proto::bloom::BloomFilter; -use crate::proto::fmp::RekeyRole; +use crate::proto::fmp::{AnsweredMsg1s, Msg1Digest, RekeyAnswer, RekeyRole}; use crate::proto::mmp::MmpPeerState; use crate::proto::stp::{ParentDeclaration, TreeCoordinate}; use crate::transport::{LinkId, LinkStats, TransportAddr, TransportId}; @@ -274,6 +274,11 @@ pub struct ActivePeer { pending_role: Option, /// When the pending session was installed, for the responder hold. pending_since: Option, + /// The rekey msg1s this node answered for this peer as the responder: the + /// whole answer that armed a pending it holds, so a resend of that msg1 + /// can be answered again, and the digests of ended cycles, so a copy of + /// one is refused. Not cycle state: it outlives every pending. + answered: AnsweredMsg1s, // === Published active-send-state (two-tier boundary) === /// The send-critical subset read (and, on roam/responder-cutover, written) @@ -317,6 +322,7 @@ impl ActivePeer { rekey_msg1_resend_count: 0, pending_role: None, pending_since: None, + answered: AnsweredMsg1s::default(), send: PeerSendState::new(link_id, now, authenticated_at), } } @@ -398,6 +404,7 @@ impl ActivePeer { rekey_msg1_resend_count: 0, pending_role: None, pending_since: None, + answered: AnsweredMsg1s::default(), send, } } @@ -937,6 +944,16 @@ impl ActivePeer { .map(|t| t.checked_sub(age).unwrap_or_else(Instant::now)); } + /// Test-only seam: backdate the last peer-initiated rekey so a test can + /// move past the rekey dampening window without waiting it out. Shifts + /// only the private timestamp; compiled out of release builds. + #[cfg(test)] + pub(crate) fn backdate_dampener(&mut self, age: Duration) { + self.last_peer_rekey = self + .last_peer_rekey + .map(|t| t.checked_sub(age).unwrap_or_else(Instant::now)); + } + /// Test-only seam: install link-layer MMP state with a chosen operating /// mode on a peer that was constructed without a Noise session (the bare /// `new` constructor leaves `mmp` as `None`). This only attaches the same @@ -1050,6 +1067,17 @@ impl ActivePeer { self.pending_role } + /// The answer that armed the pending session, when this node holds it as + /// the rekey responder; `None` otherwise. + pub(crate) fn rekey_answer(&self) -> Option<&RekeyAnswer> { + self.answered.held() + } + + /// Whether `msg1` armed a responder cycle with this peer that has ended. + pub(crate) fn answered_before(&self, msg1: &Msg1Digest) -> bool { + self.answered.ended(msg1) + } + /// Check whether the pending session has been held for at least `hold` /// since it was installed. False when no pending session is held. pub(crate) fn pending_expired(&self, hold: Duration) -> bool { @@ -1069,12 +1097,14 @@ impl ActivePeer { their_index: SessionIndex, ) { self.install_pending(session, our_index, their_index, RekeyRole::Initiator); + self.answered.end(); } /// Store the session this node produced by answering the peer's rekey - /// msg1. It is held until a frame on the new epoch from the peer - /// authenticates against it ([`handle_peer_kbit_flip`](Self::handle_peer_kbit_flip)), - /// or until the responder hold passes and the node retires it + /// msg1, with the answer it sent. It is held until a frame on the new + /// epoch from the peer authenticates against it + /// ([`handle_peer_kbit_flip`](Self::handle_peer_kbit_flip)), or until the + /// responder hold passes and the node retires it /// ([`retire_pending`](Self::retire_pending)); it is never cut over on this /// node's own schedule. pub(crate) fn answer_rekey( @@ -1082,8 +1112,19 @@ impl ActivePeer { session: NoiseSession, our_index: SessionIndex, their_index: SessionIndex, + answer: RekeyAnswer, ) { self.install_pending(session, our_index, their_index, RekeyRole::Responder); + self.answered.arm(answer); + } + + /// Clear what is recorded beside the pending slot: its role and install + /// time, and the answer that armed it, of which only the msg1 digest is + /// kept, as an ended cycle. Every path that empties the slot calls this. + fn release_pending(&mut self) { + self.pending_role = None; + self.pending_since = None; + self.answered.end(); } /// Store a pending session with the role that produced it and the time it @@ -1123,8 +1164,7 @@ impl ActivePeer { let new_session = self.send.pending_new_session.take()?; let new_our_index = self.send.pending_our_index.take(); let new_their_index = self.send.pending_their_index.take(); - self.pending_role = None; - self.pending_since = None; + self.release_pending(); // Demote current to previous self.send.previous_session = self.send.noise_session.take(); @@ -1167,8 +1207,7 @@ impl ActivePeer { let new_session = self.send.pending_new_session.take()?; let new_our_index = self.send.pending_our_index.take(); let new_their_index = self.send.pending_their_index.take(); - self.pending_role = None; - self.pending_since = None; + self.release_pending(); // Demote current to previous self.send.previous_session = self.send.noise_session.take(); @@ -1236,8 +1275,7 @@ impl ActivePeer { } self.send.pending_new_session.take()?; self.send.pending_their_index = None; - self.pending_role = None; - self.pending_since = None; + self.release_pending(); debug_assert_eq!( self.pending_role.is_some(), self.send.pending_new_session.is_some(), @@ -1261,8 +1299,7 @@ impl ActivePeer { let freed = self.rekey_our_index.take().or_else(|| { self.send.pending_new_session = None; self.send.pending_their_index = None; - self.pending_role = None; - self.pending_since = None; + self.release_pending(); self.send.pending_our_index.take() }); debug_assert_eq!( @@ -1840,6 +1877,60 @@ mod tests { assert_eq!(cur_pt.as_deref(), Some(&b"steady"[..])); } + /// A stand-in for the answer a responder records when it arms a pending. + fn answer() -> RekeyAnswer { + RekeyAnswer { + msg1: crate::proto::fmp::Msg1Digest::of(b"msg1"), + msg2: vec![0x02; 8], + } + } + + /// The answer that armed a responder pending is held exactly as long as + /// the pending: it leaves on retirement, on promotion, and on abandon, + /// leaving its msg1 recorded as an ended cycle, and a pending this node + /// initiated carries none. + #[test] + fn the_answer_that_armed_a_pending_leaves_with_it() { + type Exit = fn(&mut ActivePeer) -> Option; + let exits: [(&str, Exit); 3] = [ + ("retire", ActivePeer::retire_pending), + ("promote", ActivePeer::handle_peer_kbit_flip), + ("abandon", ActivePeer::abandon_rekey), + ]; + for (name, exit) in exits { + let (_cur_send, cur_recv) = ik_session_pair(); + let (_pend_send, pend_recv) = ik_session_pair(); + let mut peer = peer_with_current(cur_recv); + peer.answer_rekey( + pend_recv, + SessionIndex::new(3), + SessionIndex::new(4), + answer(), + ); + assert_eq!( + peer.rekey_answer().map(|a| a.msg2.clone()), + Some(vec![0x02; 8]), + "{name}: the answer must be held with the pending" + ); + assert!(exit(&mut peer).is_some(), "{name}: the pending must exit"); + assert!(peer.pending_new_session().is_none()); + assert!( + peer.rekey_answer().is_none(), + "{name}: the answer must leave with the pending" + ); + assert!( + peer.answered_before(&answer().msg1), + "{name}: the answered msg1 must be remembered as an ended cycle" + ); + } + + let (_cur_send, cur_recv) = ik_session_pair(); + let (_pend_send, pend_recv) = ik_session_pair(); + let mut peer = peer_with_current(cur_recv); + peer.set_pending_session(pend_recv, SessionIndex::new(3), SessionIndex::new(4)); + assert!(peer.rekey_answer().is_none()); + } + /// Retiring a pending session this node answered hands back its index for /// the caller to free, empties the pending slot and its role, and leaves /// the current session alone. @@ -1848,7 +1939,12 @@ mod tests { let (_cur_send, cur_recv) = ik_session_pair(); let (_pend_send, pend_recv) = ik_session_pair(); let mut peer = peer_with_current(cur_recv); - peer.answer_rekey(pend_recv, SessionIndex::new(3), SessionIndex::new(4)); + peer.answer_rekey( + pend_recv, + SessionIndex::new(3), + SessionIndex::new(4), + answer(), + ); assert_eq!(peer.pending_role(), Some(RekeyRole::Responder)); assert_eq!(peer.retire_pending(), Some(SessionIndex::new(3))); @@ -1882,7 +1978,12 @@ mod tests { let (_cur_send, cur_recv) = ik_session_pair(); let (_pend_send, pend_recv) = ik_session_pair(); let mut peer = peer_with_current(cur_recv); - peer.answer_rekey(pend_recv, SessionIndex::new(3), SessionIndex::new(4)); + peer.answer_rekey( + pend_recv, + SessionIndex::new(3), + SessionIndex::new(4), + answer(), + ); assert!(peer.handle_peer_kbit_flip().is_some()); assert_eq!(peer.pending_role(), None); @@ -1896,7 +1997,12 @@ mod tests { let (_cur_send, cur_recv) = ik_session_pair(); let (_pend_send, pend_recv) = ik_session_pair(); let mut peer = peer_with_current(cur_recv); - peer.answer_rekey(pend_recv, SessionIndex::new(3), SessionIndex::new(4)); + peer.answer_rekey( + pend_recv, + SessionIndex::new(3), + SessionIndex::new(4), + answer(), + ); assert!(!peer.pending_expired(Duration::from_secs(60))); peer.backdate_pending(Duration::from_secs(61)); diff --git a/src/peer/machine.rs b/src/peer/machine.rs index 47030771..684ab952 100644 --- a/src/peer/machine.rs +++ b/src/peer/machine.rs @@ -1257,6 +1257,9 @@ impl PeerMachine { // The decision carries the stored msg2 bytes; the driver's inline // resend owns the send. No machine state is touched. InboundDecision::ResendMsg2 { .. } => Vec::new(), + // Decision-only: the driver resends the held rekey msg2 on the + // peer's established link. No machine state is touched. + InboundDecision::ResendRekeyMsg2 { .. } => Vec::new(), // Decision-only: the driver's inline body owns the abandon, the // index allocation, the framed msg2 send, the pending-session // store, and the dampening stamp. The machine mutates nothing. @@ -1865,7 +1868,7 @@ fn disconnect_frame(reason: CloseReason) -> Vec { #[cfg(test)] mod tests { use super::*; - use crate::proto::fmp::PromotionResult; + use crate::proto::fmp::{Msg1Digest, PromotionResult}; use crate::{Identity, PeerIdentity}; fn peer_identity() -> PeerIdentity { @@ -1892,6 +1895,7 @@ mod tests { remote_epoch: epoch, their_index: SessionIndex::new(their), msg2_payload: vec![0xAB; 8], + msg1_digest: Msg1Digest::of(&[0xCD; 8]), } } @@ -1903,6 +1907,8 @@ mod tests { has_session: false, pending_new_session: false, rekey_in_progress: false, + held_answer: None, + msg1_answered_before: false, existing_msg2: None, at_max_peers: false, has_pending_outbound_to_peer: false, diff --git a/src/perf_profile.rs b/src/perf_profile.rs index 2745823a..66442954 100644 --- a/src/perf_profile.rs +++ b/src/perf_profile.rs @@ -36,8 +36,8 @@ //! * `FMP_WORKER_QUEUE_WAIT` — rx_loop FMP job dispatch → worker //! * `ENDPOINT_EVENT_WAIT` — rx_loop endpoint delivery → endpoint recv +use portable_atomic::{AtomicU64, Ordering::Relaxed}; use std::sync::OnceLock; -use std::sync::atomic::{AtomicU64, Ordering::Relaxed}; use std::time::Instant; /// Number of measurement buckets. Indices match `Stage`. diff --git a/src/proto/fmp/core.rs b/src/proto/fmp/core.rs index 6df59200..eefc59bb 100644 --- a/src/proto/fmp/core.rs +++ b/src/proto/fmp/core.rs @@ -21,6 +21,7 @@ use super::state::Fmp; use crate::transport::LinkId; use crate::utils::index::SessionIndex; use crate::{NodeAddr, PeerIdentity}; +use std::collections::VecDeque; /// Determine winner of cross-connection tie-breaker. /// @@ -144,6 +145,95 @@ pub(crate) enum RekeyRole { Responder, } +/// A digest of one link handshake msg1 exactly as it arrived, header included. +/// +/// The initiator's resend ladder retransmits its stored msg1 bytes unchanged, +/// so a resend digests equal to the original and any other msg1 does not. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct Msg1Digest([u8; 32]); + +impl Msg1Digest { + /// Digest the wire bytes of one msg1. + pub(crate) fn of(wire_msg1: &[u8]) -> Self { + use sha2::{Digest, Sha256}; + Self(Sha256::digest(wire_msg1).into()) + } +} + +/// What this node sent when it answered a peer's rekey msg1 as the responder: +/// the msg1 it answered, by digest, and the framed msg2 it sent back. +/// +/// Kept with the pending session that answer armed, so a resend of that msg1 +/// draws the same msg2 again rather than a refusal while the pending is held. +#[derive(Clone, Debug)] +pub(crate) struct RekeyAnswer { + /// The msg1 that armed the pending session. + pub msg1: Msg1Digest, + /// The framed msg2 sent in answer, opaque to the core. + pub msg2: Vec, +} + +/// How many msg1s of ended cycles one peer's [`AnsweredMsg1s`] remembers. +/// +/// A msg1 is answered as a rekey only on a session at least +/// [`REKEY_MIN_SESSION_AGE_SECS`] old, so this covers at least two hours of +/// the peer's cycles, and eight and a half at the default 120 s interval when +/// the message-count trigger does not fire first. 32 bytes each, so 8 KiB per +/// peer at most. +pub(crate) const ENDED_MSG1_RECORD: usize = 256; + +/// The rekey msg1s this node answered as the link-rekey responder for one +/// peer, by digest. +/// +/// A link msg1 carries nothing that ties it to one cycle, so a copy taken off +/// the wire still authenticates as the peer, in the peer's current epoch, +/// after the cycle it started has ended. This record is how a copy is told +/// from a fresh msg1. The answer that armed the pending session this node +/// holds is kept whole, so a resend of that msg1 draws the same msg2. When +/// the pending leaves (adopted, retired or abandoned) its digest moves to the +/// ended list, and a msg1 matching an ended cycle is refused instead of arming +/// a new pending. +/// +/// Retention is bounded: the ended list keeps the last +/// [`ENDED_MSG1_RECORD`] digests, so a msg1 from an older cycle of the same +/// epoch is not recognized. The record lives with the peer, so it starts empty +/// whenever the peering is established again while the peer's epoch stays the +/// same, as after this node restarts or the link is torn down and re-formed. +#[derive(Debug, Default)] +pub(crate) struct AnsweredMsg1s { + held: Option, + ended: VecDeque, +} + +impl AnsweredMsg1s { + /// Record the answer that armed a new responder pending. + pub(crate) fn arm(&mut self, answer: RekeyAnswer) { + self.end(); + self.held = Some(answer); + } + + /// The pending the held answer armed has left: keep only its digest. + pub(crate) fn end(&mut self) { + if let Some(answer) = self.held.take() { + if self.ended.len() == ENDED_MSG1_RECORD { + self.ended.pop_front(); + } + self.ended.push_back(answer.msg1); + } + } + + /// The answer that armed the pending this node holds, if it holds one it + /// answered. + pub(crate) fn held(&self) -> Option<&RekeyAnswer> { + self.held.as_ref() + } + + /// Whether `msg1` armed a cycle that has since ended. + pub(crate) fn ended(&self, msg1: &Msg1Digest) -> bool { + self.ended.contains(msg1) + } +} + /// A snapshot of one active peer's rekey-relevant state, taken by the shell. /// /// Every clock read is resolved shell-side into a plain `u64`/`bool` before the @@ -225,6 +315,9 @@ pub(crate) struct WireOutcome { /// The opaque Noise msg2 payload the responder produced (empty only if no /// msg2 is to be sent). pub msg2_payload: Vec, + /// Digest of the msg1 as it arrived, matched against the msg1 that armed + /// a held responder pending. + pub msg1_digest: Msg1Digest, } /// A snapshot of the `Node` registry state the inbound establish decision reads @@ -250,6 +343,13 @@ pub(crate) struct EstablishSnapshot { pub pending_new_session: bool, /// The existing peer has a rekey handshake in flight. pub rekey_in_progress: bool, + /// When the pending session is one this node answered as the rekey + /// responder, the answer that armed it. `None` with no pending, or with a + /// pending this node initiated. + pub held_answer: Option, + /// This msg1 armed a responder cycle of this peer's that has since ended + /// (pre-evaluated shell-side against the peer's [`AnsweredMsg1s`]). + pub msg1_answered_before: bool, /// The existing peer's stored msg2 wire bytes (an opaque blob), resent on a /// same-epoch duplicate msg1. `None` when there is no existing peer or it /// has no stored msg2. @@ -393,6 +493,12 @@ pub(crate) enum InboundDecision { /// only on the dual-initiation *loser* path, where we first abandon our own /// in-flight rekey. `peer` is the rekey target. RekeyRespond { peer: NodeAddr, abandon_first: bool }, + /// A resend of the rekey msg1 that armed the responder pending this node + /// holds: send `msg2`, the answer already given, again. The shell sends it + /// only on the peer's established link and never to the msg1's source + /// address, since a captured msg1 replayed from anywhere authenticates + /// the same as a resend. Nothing else changes; the pending stays held. + ResendRekeyMsg2 { peer: NodeAddr, msg2: Vec }, /// Same-epoch duplicate msg1 (not a rekey): resend the existing peer's stored /// msg2. `msg2` is the opaque stored bytes (`None` → nothing to resend, the /// silent no-op preserved from the pre-refactor path). @@ -404,7 +510,7 @@ pub(crate) enum InboundDecision { } /// Why an inbound msg1 was rejected. Distinguishes only the diagnostic log -/// message; all three reject identically (BadState stat, rate-limiter complete, +/// message; all four reject identically (BadState stat, rate-limiter complete, /// the local not-yet-registered connection dropped). #[derive(Debug)] pub(crate) enum InboundReject { @@ -412,11 +518,15 @@ pub(crate) enum InboundReject { /// bypass the cap: silent-drop before any msg2 build/send. AtMaxPeers, /// The peer already holds a pending post-rekey session awaiting K-bit - /// cutover; a second rekey msg1 must not overwrite it. + /// cutover, and this msg1 is not the one that armed it; a second rekey + /// msg1 must not overwrite it. PendingSession, /// Dual rekey initiation and we are the tie-break *winner* (smaller /// NodeAddr): drop the peer's msg1 and keep driving our own rekey. DualRekeyWon, + /// The msg1 armed a rekey cycle with this peer that has already ended: a + /// copy, not a fresh request, and it must not arm a pending. + AnsweredBefore, } /// The classification outcome for one outbound `handle_msg2` completion, decided @@ -463,7 +573,9 @@ pub(crate) trait EstablishView { /// `peer_addr`: the existing peer's epoch/session/rekey state (with the /// session age resolved shell-side), the max-peers cap, and this node's own /// address for the tie-break. - fn establish_snapshot(&self, peer_addr: &NodeAddr) -> EstablishSnapshot; + /// `msg1` is the digest of the msg1 being classified, checked against the + /// peer's record of answered msg1s. + fn establish_snapshot(&self, peer_addr: &NodeAddr, msg1: &Msg1Digest) -> EstablishSnapshot; /// Snapshot the registry state relevant to classifying an outbound msg2 /// completion for `peer_addr`: whether the identity is already an active @@ -663,11 +775,30 @@ impl Fmp { }; } if snap.pending_new_session { - // A completed rekey is already pending cutover. + // A completed rekey is already pending cutover. A + // resend of the msg1 this node answered to arm it + // means the answer was lost: give it again. Any other + // msg1 is refused. + if let Some(answer) = &snap.held_answer + && answer.msg1 == wire.msg1_digest + { + return InboundDecision::ResendRekeyMsg2 { + peer: peer_addr, + msg2: answer.msg2.clone(), + }; + } return InboundDecision::Reject { reason: InboundReject::PendingSession, }; } + if snap.msg1_answered_before { + // A copy of a msg1 whose cycle has ended: refuse it + // before it can arm a pending, or, on a tie-break we + // lose, abandon our own rekey. + return InboundDecision::Reject { + reason: InboundReject::AnsweredBefore, + }; + } if snap.rekey_in_progress { // Dual initiation — smaller NodeAddr wins as initiator. // Our own rekey is the outbound/initiator side, so reuse diff --git a/src/proto/fmp/mod.rs b/src/proto/fmp/mod.rs index 24262eb7..9e3f8953 100644 --- a/src/proto/fmp/mod.rs +++ b/src/proto/fmp/mod.rs @@ -35,9 +35,9 @@ pub(crate) mod wire; mod tests; pub(crate) use core::{ - ConnAction, ConnSnapshot, EstablishSnapshot, EstablishView, InboundDecision, InboundReject, - LifecycleView, OutboundDecision, OutboundSnapshot, PeerSnapshot, RekeyCfg, RekeyResendSnapshot, - RekeyRole, WireOutcome, + AnsweredMsg1s, ConnAction, ConnSnapshot, EstablishSnapshot, EstablishView, InboundDecision, + InboundReject, LifecycleView, Msg1Digest, OutboundDecision, OutboundSnapshot, PeerSnapshot, + RekeyAnswer, RekeyCfg, RekeyResendSnapshot, RekeyRole, WireOutcome, }; pub use core::{PromotionResult, cross_connection_winner}; pub(crate) use limits::backoff_ms; diff --git a/src/proto/fmp/tests/core.rs b/src/proto/fmp/tests/core.rs index ad81608e..4ca5d615 100644 --- a/src/proto/fmp/tests/core.rs +++ b/src/proto/fmp/tests/core.rs @@ -5,9 +5,10 @@ use super::util::{ wire_outcome, }; use crate::NodeAddr; +use crate::proto::fmp::core::ENDED_MSG1_RECORD; use crate::proto::fmp::{ - ConnAction, Fmp, InboundDecision, InboundReject, OutboundDecision, OutboundSnapshot, RekeyCfg, - RekeyRole, cross_connection_winner, + AnsweredMsg1s, ConnAction, Fmp, InboundDecision, InboundReject, Msg1Digest, OutboundDecision, + OutboundSnapshot, RekeyAnswer, RekeyCfg, RekeyRole, cross_connection_winner, }; use crate::testutil::make_node_addr; use crate::transport::LinkId; @@ -495,6 +496,158 @@ fn establish_inbound_pending_session_rejects() { )); } +/// An aged existing peer holding a responder pending armed by the msg1 whose +/// wire bytes are `armed_by`, answered with msg2 `[0x02; 4]`. +fn snapshot_holding_an_answer(armed_by: &[u8]) -> crate::proto::fmp::EstablishSnapshot { + let mut snap = establish_snapshot(); + snap.has_existing_peer = true; + snap.existing_peer_epoch = Some([7u8; 8]); + snap.has_session = true; + snap.existing_session_age_secs = 31; + snap.pending_new_session = true; + snap.held_answer = Some(RekeyAnswer { + msg1: Msg1Digest::of(armed_by), + msg2: vec![0x02; 4], + }); + snap +} + +#[test] +fn a_resent_msg1_matching_the_held_answer_resends_its_msg2() { + // The msg2 answering a held pending was lost and the initiator resent the + // same msg1: answer it again with the same msg2, for the same peer. + let fmp = Fmp::new(); + let snap = snapshot_holding_an_answer(b"the msg1 that armed it"); + let mut wire = wire_outcome(Some([7u8; 8])); + wire.msg1_digest = Msg1Digest::of(b"the msg1 that armed it"); + let peer = *wire.peer_identity.node_addr(); + match fmp.establish_inbound(&snap, &wire) { + InboundDecision::ResendRekeyMsg2 { peer: p, msg2 } => { + assert_eq!(p, peer); + assert_eq!(msg2, vec![0x02; 4]); + } + other => panic!("expected ResendRekeyMsg2, got {other:?}"), + } +} + +#[test] +fn a_different_msg1_is_refused_while_an_answered_pending_is_held() { + // Only the msg1 that armed the pending draws its answer; a new msg1 from + // the same peer must not, and must not replace the pending either. + let fmp = Fmp::new(); + let snap = snapshot_holding_an_answer(b"the msg1 that armed it"); + let mut wire = wire_outcome(Some([7u8; 8])); + wire.msg1_digest = Msg1Digest::of(b"a fresh msg1"); + assert!(matches!( + fmp.establish_inbound(&snap, &wire), + InboundDecision::Reject { + reason: InboundReject::PendingSession + } + )); +} + +#[test] +fn a_pending_this_node_initiated_answers_no_msg1() { + // A pending this node initiated has no answer recorded, so every msg1 is + // refused while it is held, as before. + let fmp = Fmp::new(); + let mut snap = snapshot_holding_an_answer(b"msg1"); + snap.held_answer = None; + let mut wire = wire_outcome(Some([7u8; 8])); + wire.msg1_digest = Msg1Digest::of(b"msg1"); + assert!(matches!( + fmp.establish_inbound(&snap, &wire), + InboundDecision::Reject { + reason: InboundReject::PendingSession + } + )); +} + +#[test] +fn a_msg1_from_an_ended_cycle_is_refused_and_arms_nothing() { + // A copy of a msg1 whose cycle has ended must not arm a new pending, even + // with no pending held and the session aged past the rekey floor. + let fmp = Fmp::new(); + let mut snap = snapshot_holding_an_answer(b"msg1"); + snap.pending_new_session = false; + snap.held_answer = None; + snap.msg1_answered_before = true; + let wire = wire_outcome(Some([7u8; 8])); + assert!(matches!( + fmp.establish_inbound(&snap, &wire), + InboundDecision::Reject { + reason: InboundReject::AnsweredBefore + } + )); +} + +#[test] +fn a_msg1_from_an_ended_cycle_does_not_make_us_abandon_our_own_rekey() { + // Mid-rekey and on the losing side of the tie-break, a fresh msg1 makes + // us abandon ours and respond; a copy of an ended cycle's msg1 must not. + let fmp = Fmp::new(); + let mut snap = snapshot_holding_an_answer(b"msg1"); + snap.pending_new_session = false; + snap.held_answer = None; + snap.rekey_in_progress = true; + snap.our_node_addr = max_node_addr(); + let wire = wire_outcome(Some([7u8; 8])); + assert!(matches!( + fmp.establish_inbound(&snap, &wire), + InboundDecision::RekeyRespond { + abandon_first: true, + .. + } + )); + snap.msg1_answered_before = true; + assert!(matches!( + fmp.establish_inbound(&snap, &wire), + InboundDecision::Reject { + reason: InboundReject::AnsweredBefore + } + )); +} + +#[test] +fn the_answered_record_holds_the_armed_answer_then_remembers_its_msg1_once_ended() { + let mut record = AnsweredMsg1s::default(); + let first = Msg1Digest::of(b"first"); + record.arm(RekeyAnswer { + msg1: first, + msg2: vec![0x02; 4], + }); + assert_eq!(record.held().map(|a| a.msg1), Some(first)); + assert!(!record.ended(&first), "a held cycle has not ended"); + + record.end(); + assert!(record.held().is_none()); + assert!(record.ended(&first)); + assert!(!record.ended(&Msg1Digest::of(b"never answered"))); + + // Ending with nothing held records nothing. + record.end(); + assert!(record.ended(&first)); +} + +#[test] +fn the_answered_record_keeps_the_most_recent_ended_cycles_up_to_its_bound() { + let mut record = AnsweredMsg1s::default(); + let digest = |i: usize| Msg1Digest::of(&i.to_le_bytes()); + for i in 0..=ENDED_MSG1_RECORD { + record.arm(RekeyAnswer { + msg1: digest(i), + msg2: Vec::new(), + }); + record.end(); + } + assert!( + !record.ended(&digest(0)), + "the oldest beyond the bound is forgotten" + ); + assert!(record.ended(&digest(1))); + assert!(record.ended(&digest(ENDED_MSG1_RECORD))); +} + #[test] fn establish_inbound_dual_init_we_win_rejects() { // rekey in progress + our addr < peer addr (our = 0x10, peer = pubkey-derived diff --git a/src/proto/fmp/tests/util.rs b/src/proto/fmp/tests/util.rs index ebe5cd85..9e2ac333 100644 --- a/src/proto/fmp/tests/util.rs +++ b/src/proto/fmp/tests/util.rs @@ -1,7 +1,7 @@ //! Shared test helpers for the FMP connection-lifecycle unit tests. use crate::proto::fmp::{ - ConnSnapshot, EstablishSnapshot, PeerSnapshot, RekeyResendSnapshot, WireOutcome, + ConnSnapshot, EstablishSnapshot, Msg1Digest, PeerSnapshot, RekeyResendSnapshot, WireOutcome, }; use crate::testutil::make_node_addr; use crate::transport::LinkId; @@ -84,6 +84,8 @@ pub(super) fn establish_snapshot() -> EstablishSnapshot { has_session: false, pending_new_session: false, rekey_in_progress: false, + held_answer: None, + msg1_answered_before: false, existing_msg2: None, at_max_peers: false, has_pending_outbound_to_peer: false, @@ -102,5 +104,6 @@ pub(super) fn wire_outcome(remote_epoch: Option<[u8; 8]>) -> WireOutcome { remote_epoch, their_index: SessionIndex::new(0x1234), msg2_payload: Vec::new(), + msg1_digest: Msg1Digest::of(b"msg1"), } } diff --git a/src/proto/mmp/tests/state.rs b/src/proto/mmp/tests/state.rs index 34b959e6..07336644 100644 --- a/src/proto/mmp/tests/state.rs +++ b/src/proto/mmp/tests/state.rs @@ -797,3 +797,37 @@ fn test_gap_tracker_saturates_when_advancing_onto_the_ceiling_counter() { "a saturated expectation must not keep opening bursts" ); } + +/// Why a previous-session ReceiverReport must not reach the metrics after a +/// rekey: the reset leaves no baseline, so a report about the old session's +/// high counters is accepted as the first one, and every report on the new +/// session then reads as regressed against it until the new session's +/// counters pass the old ones. This pins the mechanism the data plane guards +/// against by dropping such reports; it is not a red-first test of that fix, +/// which lives in the node-level rekey tests. +#[test] +fn a_report_about_the_old_session_after_a_rekey_reset_rejects_the_new_sessions_reports() { + let mut m = MmpMetrics::new(); + m.process_receiver_report(&make_rr(1_000, 1_000, 100_000, 1_000, 0, 0), 1_050, 0); + m.reset_for_rekey(); + assert_eq!(m.rr_counters(), None, "the reset clears the baseline"); + + // A report the peer sent about the old session, arriving after the reset. + m.process_receiver_report(&make_rr(1_200, 1_200, 120_000, 1_100, 0, 0), 1_150, 1_000); + assert_eq!(m.rr_counters().map(|(h, _, _)| h), Some(1_200)); + + // The new session's reports start from small counters and are refused. + m.process_receiver_report(&make_rr(10, 10, 1_000, 1_200, 0, 0), 1_250, 2_000); + assert_eq!( + m.rr_counters().map(|(h, _, _)| h), + Some(1_200), + "the new session's report is rejected as regressed" + ); + + // Without the old-session report, the same new-session report is taken. + let mut m = MmpMetrics::new(); + m.process_receiver_report(&make_rr(1_000, 1_000, 100_000, 1_000, 0, 0), 1_050, 0); + m.reset_for_rekey(); + m.process_receiver_report(&make_rr(10, 10, 1_000, 1_200, 0, 0), 1_250, 2_000); + assert_eq!(m.rr_counters().map(|(h, _, _)| h), Some(10)); +} diff --git a/testing/check-deb-depends.sh b/testing/check-deb-depends.sh index 63180d84..dc15fd50 100755 --- a/testing/check-deb-depends.sh +++ b/testing/check-deb-depends.sh @@ -26,11 +26,18 @@ # drops it. The equality rule is what keeps that hand-written floor honest: if # the toolchain stops needing it, or starts needing a newer one, this fails. # +# It also compares the package's Recommends with the recommends Cargo.toml's +# [package.metadata.deb] declares, entry for entry and in any order. Nothing +# else in the tree reads that field (deb-install installs with +# --no-install-recommends), so without this a packaging change that dropped or +# altered it would ship unnoticed. +# # Run it inside the build image (build-deb-container.sh does), so the symbols # files and the C library it reads are the ones cargo-deb read. On a host of a # different distribution a difference could come from the host instead. # # Usage: check-deb-depends.sh ... +# Cargo.toml is read from the checkout this script sits in. # Exit: 0 every package matches, 1 a mismatch, 2 could not establish a result. set -euo pipefail @@ -48,6 +55,32 @@ done exit 2 } +CARGO_TOML="$(cd "$(dirname "$0")/.." && pwd)/Cargo.toml" + +# The recommends value of Cargo.toml's [package.metadata.deb], or empty when +# the section declares none. Fails when the file or the section cannot be read, +# or the key is not a one-line string, so an unreadable declaration is never +# compared as an empty one. +declared_recommends() { + awk ' + /^\[/ { insec = ($0 == "[package.metadata.deb]"); if (insec) found = 1; next } + insec && /^recommends[[:space:]]*=/ { + if (match($0, /^recommends[[:space:]]*=[[:space:]]*"[^"]*"[[:space:]]*$/)) { + sub(/^recommends[[:space:]]*=[[:space:]]*"/, ""); sub(/"[[:space:]]*$/, "") + print; done = 1; exit 0 + } + bad = 1; exit 0 + } + END { if (!found || bad) exit 1 } + ' "$CARGO_TOML" +} + +if ! DECLARED_RECOMMENDS=$(declared_recommends 2>/dev/null); then + echo "check-deb-depends: cannot read a one-line recommends from [package.metadata.deb] in $CARGO_TOML." >&2 + echo " Refusing to report a pass I did not establish." >&2 + exit 2 +fi + FAILED=0 UNKNOWN=0 CHECKED=0 @@ -59,6 +92,7 @@ SIMPLE_RE='^([a-z0-9][a-z0-9+.-]*)( \(>= ([^)]+)\))?$' # Split a Depends value on commas into one trimmed entry per line. split_deps() { printf '%s\n' "$1" | tr ',' '\n' | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//' | sed '/^$/d' + return 0 } check_deb() { @@ -178,11 +212,22 @@ check_deb() { echo " hand-declared $e" done < <(split_deps "$shipped") + # Recommends: the same entries as Cargo.toml declares, order aside. + local recommends want got + recommends=$(dpkg-deb -f "$deb" Recommends) + echo " recommends shipped: ${recommends:-(none)}; declared: ${DECLARED_RECOMMENDS:-(none)}" + want=$(split_deps "$DECLARED_RECOMMENDS" | sort) + got=$(split_deps "$recommends" | sort) + if [ "$want" != "$got" ]; then + echo " FAIL $label: Recommends '${recommends}' differs from Cargo.toml's recommends '${DECLARED_RECOMMENDS}'" >&2 + bad=1 + fi + CHECKED=$((CHECKED + 1)) [ "$bad" -eq 0 ] || FAILED=$((FAILED + 1)) } -echo "=== Depends check (shipped Depends against dpkg-shlibdeps) ===" +echo "=== Depends check (shipped Depends against dpkg-shlibdeps, Recommends against Cargo.toml) ===" for arg in "$@"; do if [ ! -f "$arg" ]; then echo " ERROR $arg does not exist" >&2 @@ -193,9 +238,11 @@ for arg in "$@"; do done if [ "$FAILED" -ne 0 ]; then - echo "check-deb-depends: $FAILED of $CHECKED package(s) declare Depends that differ from what their binaries need." >&2 - echo " A missing or low entry installs where the binaries cannot run; a high one" >&2 + echo "check-deb-depends: $FAILED of $CHECKED package(s) declare Depends that differ from what their binaries need," >&2 + echo " or Recommends that differ from what Cargo.toml declares." >&2 + echo " A missing or low Depends entry installs where the binaries cannot run; a high one" >&2 echo " is a hand-written floor that no longer tracks them. Fix Cargo.toml's depends." >&2 + echo " A Recommends difference means the packaging did not carry Cargo.toml's recommends." >&2 exit 1 fi diff --git a/testing/check-nextest-flaky.sh b/testing/check-nextest-flaky.sh new file mode 100644 index 00000000..5065a6a5 --- /dev/null +++ b/testing/check-nextest-flaky.sh @@ -0,0 +1,95 @@ +#!/bin/bash +# ── Surface tests that passed only on retry ───────────────────────────────── +# The ci nextest profile (.config/nextest.toml) retries a failing test twice, +# so a test that fails and then passes reports green. nextest says so only in +# its summary count ("N passed (1 flaky)") and a FLAKY line in the job log, +# where nobody looks on a green run; that is how a real race in a test went +# unnoticed until someone happened to read the output. The profile's own +# comment names the remedy: surface retried-but-passed tests, and keep the +# retries so a flake does not red an unrelated run. +# +# This reads the profile's JUnit report and, for every test case carrying a +# (an attempt that failed before the final one passed), emits a +# GitHub warning annotation and a line in the step summary. A test that failed +# every attempt carries instead and is the nextest step's red, not +# this script's concern. +# +# The report is parsed with awk rather than an XML library so the script runs +# unchanged on the Linux, macOS and Windows (Git Bash) runners. It relies on +# quick-junit's layout, one element per line, and on the two element names +# appearing only as elements: in text and attributes `<` is always escaped. +# +# Usage: check-nextest-flaky.sh [junit.xml] +# Default path: target/nextest/ci/junit.xml, the ci profile's report. +# Exit 0 = the report was read (flaky tests, if any, were annotated; they do +# not fail the step). Exit 2 = no report, or one with no test cases: nothing +# was checked, and that is never reported as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +REPORT="${1:-target/nextest/ci/junit.xml}" + +if [[ ! -s "$REPORT" ]]; then + echo "::error title=Flaky-test check::no JUnit report at $REPORT; flaky tests were not checked" + exit 2 +fi + +# One line per flaky test: "", +# then a final "cases" line so an empty or truncated report is +# told apart from a clean one. +if ! parsed="$(awk ' + function attr(line, key, m, v) { + if (match(line, " " key "=\"[^\"]*\"")) { + v = substr(line, RSTART + length(key) + 3, RLENGTH - length(key) - 4) + gsub(/</, "<", v); gsub(/>/, ">", v); gsub(/"/, "\"", v) + gsub(/'/, "\047", v); gsub(/&/, "\\&", v) + return v + } + return "" + } + function flush() { + if (fails > 0) printf "%d\t%s\t%s\n", fails, cls, name + fails = 0 + } + /]/ { flush(); cases++; name = attr($0, "name"); cls = attr($0, "classname") } + /]/ { fails++ } + END { flush(); printf "cases\t%d\n", cases } +' "$REPORT")"; then + echo "::error title=Flaky-test check::could not parse $REPORT; flaky tests were not checked" + exit 2 +fi + +cases="$(printf '%s\n' "$parsed" | awk -F'\t' '$1 == "cases" { print $2 }')" +if [[ -z "$cases" || "$cases" -eq 0 ]]; then + echo "::error title=Flaky-test check::$REPORT lists no test cases; flaky tests were not checked" + exit 2 +fi + +flaky=0 +summary="" +while IFS=$'\t' read -r fails cls name; do + [[ "$fails" == "cases" ]] && continue + flaky=$((flaky + 1)) + # Workflow-command values escape %, CR and LF; names carry none of the + # latter, but a % in a test name would otherwise be read as an escape. + msg="$cls $name failed $fails attempt(s) before passing on retry" + echo "::warning title=Flaky test::${msg//%/%25}" + summary+="- \`$cls $name\`: failed $fails attempt(s), then passed"$'\n' +done <<< "$parsed" + +if [[ "$flaky" -eq 0 ]]; then + echo "check-nextest-flaky: $cases test case(s), none passed only on retry" + exit 0 +fi + +echo "check-nextest-flaky: $flaky of $cases test case(s) passed only on retry" +if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then + { + echo "### Flaky tests ($flaky)" + echo "" + echo "These failed at least once and passed on a retry, so the run is green." + echo "" + printf '%s' "$summary" + } >> "$GITHUB_STEP_SUMMARY" +fi +exit 0 diff --git a/testing/check-portable-atomics.py b/testing/check-portable-atomics.py new file mode 100755 index 00000000..d9443754 --- /dev/null +++ b/testing/check-portable-atomics.py @@ -0,0 +1,206 @@ +#!/usr/bin/env python3 +"""Fail when non-test code under src/ uses the std 64-bit atomics. + +`std::sync::atomic::AtomicU64` and `AtomicI64` exist only on targets with +64-bit atomics. The 32-bit MIPS targets the OpenWrt packages are meant to +cover (mips-unknown-linux-musl, mipsel-unknown-linux-musl) have none, so one +such use anywhere in the crate stops the whole crate building there. Code +that needs a 64-bit atomic uses `portable_atomic::AtomicU64` instead, which is +the std type where the target has one and a lock-based fallback where it does +not. No CI leg builds for MIPS yet, so without this check the first anyone +would hear of a new std use is a failed build on a router target. + +What is flagged, read from the files committed at HEAD: + + * a `use` tree rooted at `std` or `core` that reaches + `sync::atomic::AtomicU64` or `sync::atomic::AtomicI64`, in any form: + a plain path, a grouped `{...}` list over one or several lines, a nested + group such as `std::sync::{Arc, atomic::{AtomicU64, Ordering}}`, or a + rename with `as`; + * a glob import of `std::sync::atomic::*` or `core::sync::atomic::*`, + because it brings both types in unnamed; + * a path-qualified use such as `std::sync::atomic::AtomicU64::new(0)`, and + `atomic::AtomicU64` after `use std::sync::atomic;`, anywhere in the text; + * `sa::AtomicU64` where the file renames the module with + `use std::sync::atomic as sa;` or `use std::sync::{atomic::{self as sa}}`. + +Scope is src/, minus files under a `tests/` directory and files named +`tests.rs` or `*_tests.rs`, which are only ever built for the host. A +`#[cfg(test)]` module inside an ordinary file is NOT exempt: telling it apart +needs a Rust parser, and holding test code in those files to the same rule +costs nothing today (no such module uses the std types). + +One file is exempt by name, with its reason: src/transport/ble/io_android.rs +is compiled only for Android, and every Android target Rust supports has +64-bit atomics. + +Known gaps, recorded rather than discovered: a std 64-bit atomic reached +through a re-export from another crate, or through a type alias defined +outside src/, is not seen; nor is one written with a macro that assembles the +path from pieces. + +Exit codes: + 0 - no non-test file under src/ uses a std 64-bit atomic + 1 - at least one does; every hit is printed + 2 - the check could not look (not a git work tree, git failed, or src/ + matched no Rust files); never a pass +""" + +from __future__ import annotations + +import re +import subprocess +import sys + +EXEMPT = { + "src/transport/ble/io_android.rs": "Android targets all have 64-bit atomics", +} + +WIDE = ("AtomicU64", "AtomicI64") + +USE_RE = re.compile(r"\buse\s+([^;]+);", re.S) +# Any `atomic::AtomicU64` whose `atomic` is not the tail of a longer name, so +# `portable_atomic::AtomicU64` is not matched and every std spelling is. +PATH_RE = re.compile(r"(? str: + """Run a git command and return its stdout, exiting 2 if it fails.""" + proc = subprocess.run(["git", *args], capture_output=True, text=True) + if proc.returncode != 0: + print(f"check-portable-atomics: git {' '.join(args)} failed: {proc.stderr.strip()}", + file=sys.stderr) + sys.exit(2) + return proc.stdout + + +def is_test_path(path: str) -> bool: + """True for files only ever built as part of the test harness.""" + parts = path.split("/") + name = parts[-1] + return "tests" in parts[:-1] or name == "tests.rs" or name.endswith("_tests.rs") + + +def split_top(text: str) -> list[str]: + """Split a use-tree list on the commas that are not inside braces.""" + items, depth, cur = [], 0, [] + for ch in text: + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if ch == "," and depth == 0: + items.append("".join(cur)) + cur = [] + else: + cur.append(ch) + items.append("".join(cur)) + return [i.strip() for i in items if i.strip()] + + +def flatten(tree: str, prefix: tuple[str, ...] = ()) -> list[tuple[str, ...]]: + """Expand a use tree into the full paths it imports. + + A rename, written `name@alias` by the caller, stays on the last segment. + """ + tree = re.sub(r"\s+", "", tree) + brace = tree.find("{") + if brace == -1: + segs = tuple(s for s in tree.split("::") if s) + return [prefix + segs] + head = tuple(s for s in tree[:brace].split("::") if s) + inner = tree[brace + 1:tree.rfind("}")] + out = [] + for item in split_top(inner): + if item == "self" or item.startswith("self@"): + alias = item[len("self"):] + out.append(prefix + head[:-1] + (head[-1] + alias,)) + else: + out.extend(flatten(item, prefix + head)) + return out + + +def use_hits(text: str) -> list[tuple[int, str]]: + """Return (line, imported path) for each std/core wide-atomic import.""" + hits = [] + for m in USE_RE.finditer(text): + body = re.sub(r"\s+as\s+(\w+)", r"@\1", m.group(1)) + line = text.count("\n", 0, m.start()) + 1 + for path in flatten(body): + if not path: + continue + last, _, alias = path[-1].partition("@") + path = path[:-1] + (last,) + if path[0] not in ("std", "core") or path[1:3] != ("sync", "atomic"): + continue + if len(path) == 3 and alias: + hits.extend(alias_hits(text, alias, "::".join(path))) + elif len(path) >= 4 and (path[3] in WIDE or path[3] == "*"): + hits.append((line, "::".join(path[:4]))) + return hits + + +def alias_hits(text: str, alias: str, module: str) -> list[tuple[int, str]]: + """Return (line, use) for each wide atomic named through a module alias.""" + hits = [] + pat = re.compile(r"(? list[tuple[int, str]]: + """Return (line, matched text) for each path-qualified wide atomic.""" + hits = [] + for m in PATH_RE.finditer(text): + line = text.count("\n", 0, m.start()) + 1 + hits.append((line, re.sub(r"\s+", "", m.group(0)))) + return hits + + +def main() -> int: + """Scan the committed src/ tree and report every std wide-atomic use.""" + root = git("rev-parse", "--show-toplevel").strip() + if not root: + print("check-portable-atomics: empty work-tree root", file=sys.stderr) + return 2 + files = [ + f for f in git("-C", root, "ls-tree", "-r", "--name-only", "HEAD", "--", "src/").splitlines() + if f.endswith(".rs") + ] + if not files: + print("check-portable-atomics: src/ matched no Rust files at HEAD", file=sys.stderr) + return 2 + + scanned = 0 + findings = [] + for path in files: + if is_test_path(path) or path in EXEMPT: + continue + text = git("-C", root, "show", f"HEAD:{path}") + scanned += 1 + seen = set() + for line, what in use_hits(text) + path_hits(text): + if (line, what) not in seen: + seen.add((line, what)) + findings.append(f"{path}:{line}: {what}") + + if scanned == 0: + print("check-portable-atomics: every file under src/ was excluded", file=sys.stderr) + return 2 + + if findings: + for f in findings: + print(f) + print("", file=sys.stderr) + print("check-portable-atomics: std 64-bit atomics do not exist on 32-bit MIPS, so the", + file=sys.stderr) + print("crate stops building there. Use portable_atomic::AtomicU64 (or AtomicI64).", + file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/testing/ci-local.sh b/testing/ci-local.sh index 759286bc..83355007 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -32,7 +32,7 @@ # chaos-churn-mixed-10, chaos-ethernet-mesh, # chaos-ethernet-only, chaos-ethernet-churn, chaos-tcp-mesh, # chaos-congestion-stress, -# sidecar, dns-resolver, deb-install, medium-change +# sidecar, native-api, mdns, dns-resolver, deb-install, medium-change # # Opt-in (require --with-tor; depend on live Tor network): # tor-socks5, tor-directory @@ -220,6 +220,7 @@ STUN_FAULTS_SUITES=(stun-faults) DNS_RESOLVER_SUITES=(dns-resolver) NATIVE_API_SUITES=(native-api) MEDIUM_CHANGE_SUITES=(medium-change) +MDNS_SUITES=(mdns) DEB_INSTALL_SUITES=(deb-install) TOR_SUITES=(tor-socks5 tor-directory) @@ -285,6 +286,8 @@ list_suites() { echo "" echo " Medium change:" for s in "${MEDIUM_CHANGE_SUITES[@]}"; do echo " $s"; done + echo " mDNS LAN discovery:" + for s in "${MDNS_SUITES[@]}"; do echo " $s"; done echo "" echo " DNS resolver:" for s in "${DNS_RESOLVER_SUITES[@]}"; do echo " $s"; done @@ -1250,6 +1253,18 @@ run_medium_change() { ci_release_mc_networks } +# Run the mDNS LAN discovery harness: two nodes on a user-defined bridge that +# must find and peer with each other by mDNS alone. Reads FIPS_TEST_IMAGE, and +# creates and removes its own network. +run_mdns() { + info "[mdns] Running mDNS LAN discovery test" + if FIPS_TEST_IMAGE="$CI_IMAGE_TEST" bash testing/mdns/test.sh 2>&1; then + record "mdns" 0 + else + record "mdns" 1 + fi +} + # Run dns-resolver harness (multi-distro + e2e scenarios) # # Its e2e scenarios run the fips binaries from the package build_ci_deb @@ -1552,6 +1567,9 @@ run_integration() { # Native datagram API (light — one single-node run plus a two-node pair) run_native_api + # mDNS LAN discovery (light — one two-node pair, seconds when healthy) + run_mdns + # DNS resolver multi-distro suite (heavy — per-distro systemd images) run_dns_resolver @@ -1616,6 +1634,8 @@ run_suite() { run_native_api ;; medium-change) run_medium_change ;; + mdns) + run_mdns ;; deb-install) run_deb_install ;; tor-socks5) @@ -1661,9 +1681,6 @@ print_summary() { echo "" } -# Verify the local default suite set and the GitHub matrix still cover the -# same work. Runs first: it takes about a second, and a divergence should be -# reported before a half-hour suite rather than after it. # The OpenWrt maintainer scripts and the fips-gateway init script ship to # routers and run there under ash, never under bash. This runs them under ash # in a busybox container against stubbed init scripts, so an install, an @@ -1689,6 +1706,9 @@ run_tarball_install() { return $rc } +# Verify the local default suite set and the GitHub matrix still cover the +# same work. Runs first: it takes about a second, and a divergence should be +# reported before a half-hour suite rather than after it. run_ci_parity() { local rc=0 info "[ci-parity] Comparing the local suite set against the GitHub matrix" @@ -1729,6 +1749,16 @@ run_comment_refs() { record "comment-refs" $rc } +# No non-test code may use std's 64-bit atomics. They do not exist on 32-bit +# MIPS, so one such use stops the crate building for the OpenWrt MIPS targets, +# and no leg builds for MIPS to notice. Static, and it needs nothing built. +run_portable_atomics() { + local rc=0 + info "[portable-atomics] Checking that no non-test code uses std 64-bit atomics" + python3 "$SCRIPT_DIR/check-portable-atomics.py" || rc=$? + record "portable-atomics" $rc +} + # Every daemon log string a test matches on must still be emitted by src/. # A stale one does not fail — it stops observing, and an expect-zero assertion # built on it then passes for the wrong reason. @@ -1779,6 +1809,18 @@ run_deb_version() { record "deb-version" $rc } +# The GitHub unit-test jobs run check-nextest-flaky.sh after nextest to +# surface tests that passed only on retry. Nothing local runs nextest under the +# retrying ci profile, so the checker itself never runs here; its fixture tests +# do, so a checker that stopped seeing flaky tests fails here rather than going +# quiet on GitHub. Static, about a second, and needs nothing built. +run_nextest_flaky() { + local rc=0 + info "[nextest-flaky] Checking the flaky-test reporter against its fixtures" + bash "$SCRIPT_DIR/nextest-flaky/test.sh" || rc=$? + record "nextest-flaky" $rc +} + # ── Main ─────────────────────────────────────────────────────────────────── main() { @@ -1799,8 +1841,10 @@ main() { run_image_scoping run_action_pins run_comment_refs + run_portable_atomics run_wait_converge run_deb_version + run_nextest_flaky if [[ "$TEST_ONLY" == true ]]; then run_tests diff --git a/testing/deb-install/test.sh b/testing/deb-install/test.sh index a1419491..c43aad94 100755 --- a/testing/deb-install/test.sh +++ b/testing/deb-install/test.sh @@ -4,11 +4,12 @@ # Each scenario takes the .deb from --deb, or builds (or reuses) it # through packaging/debian/build-deb-container.sh, boots a systemd # container with TUN access for the target distro, installs the .deb -# via `apt install ./fips_*.deb`, waits for fips.service + fips-dns.service -# to come up, and verifies that `dig @127.0.0.53 AAAA .fips` -# returns a non-empty AAAA answer through the resolver backend that -# fips-dns-setup configured. Then exercises fips-gateway against the -# same daemon to verify the gateway/daemon default-pairing. Finally it +# via `apt-get install -y --no-install-recommends ./fips_*.deb`, waits +# for fips.service + fips-dns.service to come up, and verifies that +# `dig @127.0.0.53 AAAA .fips` returns a non-empty AAAA answer +# through the resolver backend that fips-dns-setup configured. Then +# exercises fips-gateway against the same daemon to verify the +# gateway/daemon default-pairing. Finally it # purges the package with the DNS routing file planted and fips-dns # stopped, and checks the file is removed and systemd-resolved restarted. # @@ -21,9 +22,10 @@ # - The fips, fips-dns, and (optionally) fips-gateway systemd units # - End-to-end .fips resolution as a real user would experience it # -# Usage: ./test.sh [scenario ...] +# Usage: ./test.sh [--deb PATH] [scenario ...] # No args = run all scenarios. # Named args = run only those (e.g., ./test.sh ubuntu26 debian12) +# --deb PATH = install that package instead of building one. # # Requirements: Docker able to grant SYS_ADMIN and NET_ADMIN and an # unconfined AppArmor profile (the containers are not privileged; see diff --git a/testing/interop/README.md b/testing/interop/README.md index 2dd5abf2..bb9d5d0a 100644 --- a/testing/interop/README.md +++ b/testing/interop/README.md @@ -123,7 +123,8 @@ per node-spec into `generated-configs/docker-compose.generated.yml`. with its ref + short SHA. 5. Remove the temp worktree (done per-ref so peak disk stays at one worktree). -It also writes `.build/refs.env`, recording each slot's ref and SHA. The +It also writes `.build/refs.env`, recording each slot's ref and the short SHA +of the commit it resolves to (an annotated tag is peeled to its commit). The driver reads it to know which pairs are mixed-version. (If absent, it falls back to the image labels.) @@ -244,19 +245,20 @@ working copy at all. ## How to read the output -The driver runs eight phases (0 to 7), plus 1b and 5b when data-plane +The driver runs nine phases (0 to 8), plus 1b and 5b when data-plane streams are on: | Phase | Check | | ----- | ---------------------------------------------------------------- | | 0 | Bring up the mesh (+ optional netem). | | 1 | All nodes reach N-1 authenticated peers; all directed pairs ping over `fips0` (the definitive FSP-session check). | -| 2 | First FMP rekey cutover completes within the timeout. | +| 2 | First FMP rekey cutover completes within the timeout (the count is role-blind: builds before v0.5.2 log a responder's cutover with the same line). | | 3 | All pairs still ping after the first rekey. | | 4 | Wait out a second rekey cycle. | | 5 | All pairs still ping after the second rekey. | | 6 | Per-node / per-pair interop log analysis. | -| 7 | Every node's mesh-size estimate within ±25% of N after warmup. | +| 7 | Every node's mesh-size estimate within ±25% of N after warmup, and every node lists all its direct peers in every poll round, warmup included (one isolated round in which a node cannot be asked is tolerated). | +| 8 | Whole-run log health (the global negative checks below). | When data-plane streams are on (`--topology`, or `FIPS_INTEROP_STREAMS`), Phase 1b measures stream loss over a quiet control window and Phase 5b @@ -270,12 +272,16 @@ many reps Phase 5b abstained and in how many it re-measured. Phase 6 is the interop-specific part. It reports: -- **Global health** — panics, `ERROR` lines, `unknown FMP version` drops, - link teardowns, decrypt failures, handshake failures, rekey-msg2 failures. - Any non-zero count is broken down per node, attributed to a specific build. - **Rekey machinery exercised** — both FMP and FSP rekey cutovers fired. - **Per-pair interop summary** — each unordered pair, classified - same-version vs MIXED, with whether it stayed healthy through the run. + same-version vs MIXED, with whether it stayed healthy through Phase 6. + +Phase 8 runs the global health checks last, so they cover the whole run, +the Phase 7 warmup included: panics, `ERROR` lines, `unknown FMP version` +drops, link teardowns (`MMP link teardown`, which the generated config +makes visible by setting `fips::node::handlers::mmp` to debug), decrypt +failures, handshake failures, rekey-msg2 failures. Any non-zero count is +broken down per node, attributed to a specific build. The final verdict lists every failure attributed to a specific `x[ref@sha] <-> y[ref@sha]` pair or build, then states the attribution: diff --git a/testing/interop/build-images.sh b/testing/interop/build-images.sh index 9abab1be..9accb0e4 100755 --- a/testing/interop/build-images.sh +++ b/testing/interop/build-images.sh @@ -144,7 +144,9 @@ build_one() { local slot="$1" local ref="$2" local sha - sha="$(git -C "$REPO_ROOT" rev-parse --short "$ref")" + # Peel to the commit: an annotated tag's own name resolves to the tag + # object, which is not what was built. + sha="$(git -C "$REPO_ROOT" rev-parse --short "${ref}^{commit}")" echo "" echo "=== Building slot '$slot' ref='$ref' sha=$sha ===" @@ -224,7 +226,7 @@ MANIFEST="$WORK_BASE/refs.env" for i in 0 1 2; do slot="${SLOTS[$i]}" ref="${REFS[$i]}" - sha="$(git -C "$REPO_ROOT" rev-parse --short "$ref")" + sha="$(git -C "$REPO_ROOT" rev-parse --short "${ref}^{commit}")" upper="$(echo "$slot" | tr '[:lower:]' '[:upper:]')" echo "INTEROP_REF_${upper}=$ref" echo "INTEROP_SHA_${upper}=$sha" diff --git a/testing/interop/generate-configs.sh b/testing/interop/generate-configs.sh index d21266a1..feab1dc2 100755 --- a/testing/interop/generate-configs.sh +++ b/testing/interop/generate-configs.sh @@ -307,7 +307,10 @@ COMPOSE_FILE="$OUT_DIR/docker-compose.generated.yml" echo " - net.ipv6.conf.all.disable_ipv6=0" echo " restart: \"no\"" echo " environment:" - echo " - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug" + # handlers::mmp at debug so the driver's "MMP link teardown" scan, which + # catches a peer removed at any point of the run, can match: the line is + # a debug! in that module. + echo " - RUST_LOG=info,fips::node::handlers::rekey=debug,fips::node::handlers::handshake=debug,fips::node::handlers::mmp=debug" echo "" echo "services:" for nid in "${NODE_IDS[@]}"; do diff --git a/testing/interop/interop-test.sh b/testing/interop/interop-test.sh index 857e4b59..5e10cf19 100755 --- a/testing/interop/interop-test.sh +++ b/testing/interop/interop-test.sh @@ -41,6 +41,11 @@ # Phase 5b), and MESH-SIZE estimate convergence across versions # (Phase 7). # +# A link lost at any point of a run turns it red: the ping phases cover +# Phases 1 to 5, Phase 7 checks every node's direct peers on each poll +# round of its warmup and settle loops, and Phase 8 scans the whole run's +# logs for peer removals and the other global failure signatures. +# # Environment: # FIPS_INTEROP_NETEM tc-netem arg string applied to every container's # eth0, e.g. "delay 10ms 5ms 25% loss 1%". Unset = @@ -537,7 +542,14 @@ count_log_pattern() { return 0 } -# Count completed FMP initiator rekey cutovers across all node logs. +# Count completed FMP rekey cutovers across all node logs. +# +# The line says "(initiator)", but it does not show direction in a mixed +# mesh: builds before v0.5.2 also log it as a responder, on the self-cutover +# one tick after sending msg2. So this counts cutovers of either role, and +# every caller reports it as a role-blind count. None needs direction: the +# control window wants any cutover, and the rekey phases want proof that +# rekeys completed. # # The pattern is a literal argument so the log-string guard can check it # against the daemon source. Prints the count, or `unreadable:` with @@ -597,6 +609,114 @@ mesh_estimate() { || echo null } +# The direct neighbours does not list in `fipsctl show peers`, as +# space-separated node ids (empty when all are listed). Prints `unreadable` +# with status 1 when the node cannot be asked or its answer cannot be +# parsed, so a dead reader is never taken for a full peer list. +missing_peers() { + local n="$1" out listed m missing="" + if ! out="$(docker exec "${CONTAINER[$n]}" fipsctl show peers 2>/dev/null)" \ + || ! listed="$(python3 -c 'import sys,json; print(" ".join(p["npub"] for p in json.load(sys.stdin)["peers"]))' <<<"$out" 2>/dev/null)"; then + echo "unreadable" + return 1 + fi + for m in "${NODES[@]}"; do + pair_is_direct "$n" "$m" || continue + case " $listed " in + *" ${NPUB_OF[$m]} "*) ;; + *) missing+=" $m" ;; + esac + done + echo "${missing# }" + return 0 +} + +# One link-continuity round over every node: record each direct neighbour +# a node has stopped listing, and each node that could not be asked. A +# peer drops out of `show peers` only when its link is removed (a stale +# but live link stays listed), so a miss here is a lost link, not loss +# noise. Phase 7 runs no pings, so without this a link lost during its +# warmup would pass the run. +# +# A node that cannot be asked is a different matter: one failed +# `docker exec` says nothing about its links. An isolated unreadable round +# is counted and reported but tolerated; two in a row, or an unreadable +# final round, fail the node, because then the harness has stopped +# observing it. A link lost and re-established inside the unread gap +# still leaves an "MMP link teardown" line for Phase 8 to find. +declare -A LINK_LOST_AT LINK_LOST_ROUNDS LINK_UNREADABLE LINK_UNREAD_RUN LINK_UNREAD_WORST +LINK_ROUNDS=0 +check_links_round() { + local n m missing at=$(( SECONDS - MESH_UP_AT )) + LINK_ROUNDS=$((LINK_ROUNDS + 1)) + for n in "${NODES[@]}"; do + if ! missing="$(missing_peers "$n")"; then + LINK_UNREADABLE[$n]=$(( ${LINK_UNREADABLE[$n]:-0} + 1 )) + LINK_UNREAD_RUN[$n]=$(( ${LINK_UNREAD_RUN[$n]:-0} + 1 )) + if [ "${LINK_UNREAD_RUN[$n]}" -gt "${LINK_UNREAD_WORST[$n]:-0}" ]; then + LINK_UNREAD_WORST[$n]="${LINK_UNREAD_RUN[$n]}" + fi + echo " UNREADABLE $n: show peers could not be read (${at}s after mesh start)" + continue + fi + LINK_UNREAD_RUN[$n]=0 + for m in $missing; do + if [ -z "${LINK_LOST_AT[$n|$m]:-}" ]; then + LINK_LOST_AT[$n|$m]="$at" + echo " LINK LOST $n no longer lists direct peer $m (${at}s after mesh start)" + fi + LINK_LOST_ROUNDS[$n|$m]=$(( ${LINK_LOST_ROUNDS[$n|$m]:-0} + 1 )) + done + done + return 0 +} + +# Global negative checks — these must be zero on EVERY node regardless +# of version pairing. A non-zero count is attributed to the node and, +# where the count is asymmetric across versions, flagged as interop. +# Every pattern must be live at the level generate-configs.sh sets. +declare -A GLOBAL_PATTERNS=( + ["PANIC|panicked"]="panics" + ["ERROR"]="error-level log lines" + ["unknown FMP version|Unknown FMP version"]="unknown-FMP-version drops" + ["MMP link teardown"]="MMP link teardowns (a peer removed)" + ["Excessive decryption failures"]="excessive-decryption-failure removals" + ["Session AEAD decryption failed"]="FSP AEAD decrypt failures" + ["Rekey msg2 processing failed"]="rekey msg2 failures" + ["Handshake failed|handshake failed"]="handshake failures" +) + +scan_global_patterns() { + local pat desc total n c s u + for pat in "${!GLOBAL_PATTERNS[@]}"; do + desc="${GLOBAL_PATTERNS[$pat]}" + if ! total="$(count_log_pattern "$pat")"; then + echo " FAIL $desc: node logs unreadable ($total), zero not established" + FAILED=$((FAILED + 1)) + INTEROP_FAILURES+=("[log] $desc: node logs unreadable ($total)") + continue + fi + if [ "$total" -eq 0 ]; then + echo " PASS $desc: 0" + PASSED=$((PASSED + 1)) + else + echo " FAIL $desc: $total (expected 0)" + FAILED=$((FAILED + 1)) + # Per-node breakdown so the count can be attributed to a build. + for n in "${NODES[@]}"; do + c="$(count_node_pattern "$n" "$pat")" + if [ "$c" -gt 0 ]; then + s="${SLOT_OF[$n]}" + u="$(echo "$s" | tr '[:lower:]' '[:upper:]')" + echo " $n [$u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}]: $c" + INTEROP_FAILURES+=("[log] node $n ($u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}): $c x '$desc'") + fi + done + fi + done + return 0 +} + # ── Data-plane continuity streams ──────────────────────────────────── # # A sustained ping6 stream over the overlay (.fips) is data-plane @@ -876,7 +996,7 @@ if ! fmp_cutovers="$(count_log_pattern 'Rekey cutover complete \(initiator\), K- FAILED=$((FAILED + 1)) INTEROP_FAILURES+=("[log] FMP rekey cutovers: node logs unreadable ($fmp_cutovers)") elif [ "$fmp_cutovers" -ge 1 ]; then - echo " PASS FMP rekey initiator cutovers: $fmp_cutovers" + echo " PASS FMP rekey cutovers (either role; see fmp_cutover_count): $fmp_cutovers" PASSED=$((PASSED + 1)) else echo " FAIL no FMP rekey cutover observed within ${FIRST_REKEY_TIMEOUT}s" @@ -983,49 +1103,9 @@ PASSED=0; FAILED=0 # at least one cutover before the final assertions. wait_for_log_pattern_count "FSP rekey cutover complete" 1 "$REKEY_SETTLE" || true -# Global negative checks — these must be zero on EVERY node regardless -# of version pairing. A non-zero count is attributed to the node and, -# where the count is asymmetric across versions, flagged as interop. -echo "" -echo " -- Global health (all $NUM_NODES nodes) --" - -declare -A GLOBAL_PATTERNS=( - ["PANIC|panicked"]="panics" - ["ERROR"]="error-level log lines" - ["unknown FMP version|Unknown FMP version"]="unknown-FMP-version drops" - ["MMP link teardown"]="MMP link teardowns" - ["Excessive decryption failures"]="excessive-decryption-failure removals" - ["Session AEAD decryption failed"]="FSP AEAD decrypt failures" - ["Rekey msg2 processing failed"]="rekey msg2 failures" - ["Handshake failed|handshake failed"]="handshake failures" -) - -for pat in "${!GLOBAL_PATTERNS[@]}"; do - desc="${GLOBAL_PATTERNS[$pat]}" - if ! total="$(count_log_pattern "$pat")"; then - echo " FAIL $desc: node logs unreadable ($total), zero not established" - FAILED=$((FAILED + 1)) - INTEROP_FAILURES+=("[log] $desc: node logs unreadable ($total)") - continue - fi - if [ "$total" -eq 0 ]; then - echo " PASS $desc: 0" - PASSED=$((PASSED + 1)) - else - echo " FAIL $desc: $total (expected 0)" - FAILED=$((FAILED + 1)) - # Per-node breakdown so the count can be attributed to a build. - for n in "${NODES[@]}"; do - c="$(count_node_pattern "$n" "$pat")" - if [ "$c" -gt 0 ]; then - s="${SLOT_OF[$n]}" - u="$(echo "$s" | tr '[:lower:]' '[:upper:]')" - echo " $n [$u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}]: $c" - INTEROP_FAILURES+=("[log] node $n ($u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}): $c x '$desc'") - fi - done - fi -done +# The global negative checks (panics, errors, link teardowns, ...) run +# after Phase 7 instead, so they cover the whole run including the +# mesh-size warmup; see Phase 8. # Positive checks — the rekey machinery actually exercised both layers. echo "" @@ -1036,7 +1116,7 @@ if ! fmp_total="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit echo " FAIL FMP rekey cutovers: node logs unreadable ($fmp_total), not established" FAILED=$((FAILED + 1)) elif [ "$fmp_total" -ge 1 ]; then - echo " PASS FMP rekey cutovers across mesh: $fmp_total" + echo " PASS FMP rekey cutovers across mesh (either role): $fmp_total" PASSED=$((PASSED + 1)) else echo " FAIL FMP rekey cutovers: $fmp_total (expected >= 1)" @@ -1056,8 +1136,9 @@ else fi # Per-pair summary: classify each unordered pair and report whether it -# stayed healthy through the run. "Healthy" = no connectivity failure -# recorded for either direction of the pair. +# stayed healthy through Phases 1 to 6. "Healthy" = no connectivity +# failure recorded for either direction of the pair. Phase 7's link +# checks and Phase 8's log scan run later and record their own failures. echo "" echo " -- Per-pair interop summary --" for p in "${PAIRS[@]}"; do @@ -1074,10 +1155,10 @@ for p in "${PAIRS[@]}"; do done fi if [ "$pair_failed" -eq 0 ]; then - echo " PASS $kind pair $label: stayed healthy" + echo " PASS $kind pair $label: healthy through Phase 6" PASSED=$((PASSED + 1)) else - echo " FAIL $kind pair $label: connectivity failed during the run" + echo " FAIL $kind pair $label: connectivity failed by Phase 6" FAILED=$((FAILED + 1)) fi done @@ -1109,21 +1190,28 @@ declare -A MS_EST MS_OK MS_INBAND_SINCE ms_accept_after=$(( MESH_UP_AT + MESH_SIZE_WARMUP )) echo " band [$ms_lo, $ms_hi], warmup ends $(( ms_accept_after - SECONDS ))s from now, then ${MESH_SIZE_SETTLE}s settled, poll up to ${MESH_SIZE_TIMEOUT}s beyond that" -# Poll through the warmup as well. Nothing here is asserted on — it is -# the trajectory the phase has never recorded, and it is what an -# undercount that never recovers would show up in. +# Poll through the warmup as well. The estimates are not asserted on here — +# they are the trajectory the phase has never recorded, and what an +# undercount that never recovers would show up in — but every round +# checks that each node still lists all its direct peers. +ms_next_print=$SECONDS while [ "$SECONDS" -lt "$ms_accept_after" ]; do - ms_line="" - for n in "${NODES[@]}"; do - MS_EST[$n]="$(mesh_estimate "$n")" - ms_line+=" $n=${MS_EST[$n]}" - done - echo " warmup, $(( ms_accept_after - SECONDS ))s to go:$ms_line" - sleep 30 + check_links_round + if [ "$SECONDS" -ge "$ms_next_print" ]; then + ms_line="" + for n in "${NODES[@]}"; do + MS_EST[$n]="$(mesh_estimate "$n")" + ms_line+=" $n=${MS_EST[$n]}" + done + echo " warmup, $(( ms_accept_after - SECONDS ))s to go:$ms_line" + ms_next_print=$(( SECONDS + 30 )) + fi + sleep "$MESH_SIZE_POLL" done ms_deadline=$(( SECONDS + MESH_SIZE_SETTLE + MESH_SIZE_TIMEOUT )) while :; do + check_links_round all_ok=1 for n in "${NODES[@]}"; do est="$(mesh_estimate "$n")" @@ -1165,7 +1253,44 @@ done # that the nodes agreed. Print the spread so nobody has to infer it. ms_spread="$(printf '%s\n' "${MS_EST[@]}" | sort -n | awk '/^[0-9]+$/{ if (lo=="") lo=$1; hi=$1 } END{ if (lo=="") print "no numeric estimates"; else if (lo==hi) print "all nodes agree on " lo; else print "nodes disagree: " lo " to " hi }')" echo " NOTE: final estimates — $ms_spread (agreement is reported, not asserted)" -phase_result "Mesh-size estimate convergence" + +# Link continuity across Phase 7, one check per node. +for n in "${NODES[@]}"; do + s="${SLOT_OF[$n]}"; u="$(echo "$s" | tr '[:lower:]' '[:upper:]')" + node_ok=1 + if [ "${LINK_UNREAD_WORST[$n]:-0}" -ge 2 ] || [ "${LINK_UNREAD_RUN[$n]:-0}" -ge 1 ]; then + node_ok=0 + echo " FAIL $n [$u]: show peers unreadable in ${LINK_UNREADABLE[$n]} of $LINK_ROUNDS rounds (longest run ${LINK_UNREAD_WORST[$n]}, final round $([ "${LINK_UNREAD_RUN[$n]:-0}" -ge 1 ] && echo unreadable || echo read)); link continuity not established" + INTEROP_FAILURES+=("[link] node $n ($u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}): show peers unreadable in ${LINK_UNREADABLE[$n]} of $LINK_ROUNDS Phase 7 rounds") + elif [ -n "${LINK_UNREADABLE[$n]:-}" ]; then + echo " NOTE $n [$u]: show peers unreadable in ${LINK_UNREADABLE[$n]} isolated round(s) of $LINK_ROUNDS; tolerated" + fi + for m in "${NODES[@]}"; do + [ -n "${LINK_LOST_AT[$n|$m]:-}" ] || continue + node_ok=0 + echo " FAIL $n [$u]: lost direct peer $m (first ${LINK_LOST_AT[$n|$m]}s after mesh start, missing in ${LINK_LOST_ROUNDS[$n|$m]} of $LINK_ROUNDS rounds)" + # NOTE: keep "$kind pair" contiguous — interop-stress.sh greps it. + INTEROP_FAILURES+=("[link] $(pair_kind "$n" "$m") pair $(pair_label "$n" "$m") [$(hop_label "$n" "$m")]: $n lost $m during Phase 7 (first ${LINK_LOST_AT[$n|$m]}s after mesh start)") + done + if [ "$node_ok" -eq 1 ]; then + echo " PASS $n [$u]: listed every direct peer in every readable round of $LINK_ROUNDS Phase 7 rounds" + PASSED=$((PASSED + 1)) + else + FAILED=$((FAILED + 1)) + fi +done +phase_result "Mesh-size estimate convergence and link continuity" +echo "" + +# ── Phase 8: whole-run log health ──────────────────────────────────── +# +# The global negative checks, run last so they cover every phase. A +# link lost and re-established inside the Phase 7 warmup leaves an +# "MMP link teardown" line even if no poll round caught it missing. +echo "Phase 8: Whole-run log health (all $NUM_NODES nodes)" +PASSED=0; FAILED=0 +scan_global_patterns +phase_result "Whole-run log health" echo "" # ── Summary ────────────────────────────────────────────────────────── diff --git a/testing/mdns/test.sh b/testing/mdns/test.sh new file mode 100644 index 00000000..dbfb8de7 --- /dev/null +++ b/testing/mdns/test.sh @@ -0,0 +1,255 @@ +#!/bin/bash +# ── mDNS LAN discovery, end to end ────────────────────────────────────────── +# Two fips daemons in separate containers on one user-defined Docker bridge, +# with LAN rendezvous on, matching scopes, no configured peers and Nostr off. +# The only way they can find each other is the mDNS advert one multicasts and +# the other browses for, so each node listing the other as an authenticated +# peer, at the other's address on this bridge, is the receive path working end +# to end. +# +# This is the coverage the unit tests in src/mdns/tests.rs cannot give. Their +# positive test needs two mDNS daemons in one process, which multicast +# loopback does not support reliably, so it is #[ignore]d; the two that run +# assert only that something was NOT seen, and would pass with the receive +# path removed. Separate containers are separate processes in separate +# network namespaces, which is the real cross-daemon path, and a user-defined +# bridge forwards link-local multicast between its containers the way a +# switch does (tested for 224.0.0.251:5353 before this harness was written). +# +# The network takes a docker-assigned subnet rather than a fixed one, so two +# concurrent runs cannot collide on address space; nothing here depends on +# the addresses beyond reading them back. +# +# Two settings exist for break-checking the harness, and leave it red when +# used; a normal run sets neither: +# MDNS_SCOPE_B= give node B a scope other than node A's +# MDNS_SPLIT_BRIDGES=1 put node B on a bridge of its own +# +# Image: FIPS_TEST_IMAGE if set (ci-local.sh passes its per-run image; the +# GitHub job passes the image it built). Otherwise a minimal image is built +# from target/release, refusing binaries older than the source. +# +# Usage: ./test.sh +# Exit 0 = both nodes discovered and peered with each other. Exit 1 = they did +# not. Exit 2 = the harness could not run; never treated as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +LABEL="com.corganlabs.fips-ci=1" +NET_A="fips-mdns-net-$$" +NET_B="fips-mdns-net-b-$$" +NODE_A="fips-mdns-a-$$" +NODE_B="fips-mdns-b-$$" +UDP_PORT=2121 +SCOPE_A="fips-mdns-test" +SCOPE_B="${MDNS_SCOPE_B:-$SCOPE_A}" +SPLIT_BRIDGES="${MDNS_SPLIT_BRIDGES:-0}" +# Seconds from both daemons running until each must list the other. mDNS +# resolution and the handshake take well under a second on a quiet host; the +# rest is headroom for a loaded CI runner. +DISCOVERY_TIMEOUT="${MDNS_DISCOVERY_TIMEOUT:-60}" +IMAGE="" +BUILT_IMAGE="" +WORK="$(mktemp -d)" + +PASS=0 +FAIL=0 +log() { echo "=== $*"; } +pass() { echo " PASS: $*"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $*"; FAIL=$((FAIL + 1)); } + +cleanup() { + docker rm -f "$NODE_A" "$NODE_B" >/dev/null 2>&1 + docker network rm "$NET_A" "$NET_B" >/dev/null 2>&1 + [[ -n "$BUILT_IMAGE" ]] && docker rmi -f "$BUILT_IMAGE" >/dev/null 2>&1 + rm -rf "$WORK" + return 0 +} +trap cleanup EXIT + +# Set IMAGE to FIPS_TEST_IMAGE, or build a minimal one from target/release. +resolve_image() { + if [[ -n "${FIPS_TEST_IMAGE:-}" ]]; then + IMAGE="$FIPS_TEST_IMAGE" + log "Using FIPS_TEST_IMAGE=$IMAGE" + return 0 + fi + local bin="$REPO_ROOT/target/release" + if [[ ! -x "$bin/fips" || ! -x "$bin/fipsctl" ]]; then + echo "No FIPS_TEST_IMAGE and no release fips/fipsctl; build them: cargo build --release" >&2 + return 1 + fi + # A stale daemon would give a verdict about code that is not the tree's. + local newer + newer="$(find "$REPO_ROOT/src" "$REPO_ROOT/Cargo.toml" -newer "$bin/fips" -print -quit 2>/dev/null)" + if [[ -n "$newer" ]]; then + echo "target/release/fips is older than $newer; rebuild: cargo build --release" >&2 + return 1 + fi + BUILT_IMAGE="fips-mdns-test:$$" + log "Building $BUILT_IMAGE from target/release" + local context="$WORK/context" + mkdir -p "$context" + cp "$bin/fips" "$bin/fipsctl" "$context/" + cat > "$context/Dockerfile" <<'DOCKERFILE' +FROM debian:trixie-slim +# libdbus-1-3 and libsystemd0 are the daemon's dynamic dependencies. +RUN apt-get update && \ + apt-get install -y --no-install-recommends ca-certificates libdbus-1-3 libsystemd0 && \ + rm -rf /var/lib/apt/lists/* +COPY fips fipsctl /usr/local/bin/ +RUN chmod +x /usr/local/bin/fips /usr/local/bin/fipsctl && mkdir -p /run/fips +DOCKERFILE + docker build -t "$BUILT_IMAGE" --label "$LABEL" "$context" --quiet >/dev/null || return 1 + IMAGE="$BUILT_IMAGE" + return 0 +} + +# write_config : LAN rendezvous on, Nostr off, no peers. +write_config() { + cat > "$1" < +start_node() { + # fips::mdns at debug so a failed run shows what the browser saw. + docker create --name "$1" --hostname "$1" --label "$LABEL" --network "$2" \ + -e RUST_LOG=info,fips::mdns=debug \ + --entrypoint /usr/local/bin/fips "$IMAGE" --config /fips-mdns.yaml >/dev/null \ + && docker cp "$3" "$1:/fips-mdns.yaml" >/dev/null \ + && docker start "$1" >/dev/null +} + +# wait_for_log : 0 once the line appears. +wait_for_log() { + local waited=0 + while [[ $waited -lt $3 ]]; do + docker logs "$1" 2>&1 | grep -qF "$2" && return 0 + sleep 1 + waited=$((waited + 1)) + done + return 1 +} + +# peer_addr : the transport address at which the node lists +# as a peer. Empty when it does not list it; status 1 when the node +# could not be asked, so a dead reader is never taken for "not yet". +peer_addr() { + local out + out="$(docker exec "$1" fipsctl show peers 2>/dev/null)" || return 1 + python3 -c ' +import json, sys +peers = json.loads(sys.argv[1])["peers"] +print(next((p.get("transport_addr", "?") for p in peers if p["npub"] == sys.argv[2]), "")) +' "$out" "$2" 2>/dev/null +} + +# container_ip +container_ip() { + docker inspect -f "{{(index .NetworkSettings.Networks \"$2\").IPAddress}}" "$1" 2>/dev/null +} + +# ───────────────────────────────────────────────────────────────────── + +resolve_image || exit 2 + +keys_a="$(python3 "$REPO_ROOT/testing/lib/derive_keys.py" fips-mdns node-a)" +keys_b="$(python3 "$REPO_ROOT/testing/lib/derive_keys.py" fips-mdns node-b)" +nsec_a="$(sed -n 's/^nsec=//p' <<<"$keys_a")"; npub_a="$(sed -n 's/^npub=//p' <<<"$keys_a")" +nsec_b="$(sed -n 's/^nsec=//p' <<<"$keys_b")"; npub_b="$(sed -n 's/^npub=//p' <<<"$keys_b")" +if [[ -z "$nsec_a" || -z "$npub_a" || -z "$nsec_b" || -z "$npub_b" ]]; then + echo "could not derive node keys" >&2 + exit 2 +fi +write_config "$WORK/a.yaml" "$nsec_a" "$SCOPE_A" +write_config "$WORK/b.yaml" "$nsec_b" "$SCOPE_B" + +net_b="$NET_A" +where="one user-defined bridge" +if [[ "$SPLIT_BRIDGES" == "1" ]]; then + net_b="$NET_B" + where="two separate bridges" +fi +log "Two nodes on $where, scopes '$SCOPE_A' and '$SCOPE_B'" +for net in $(printf '%s\n' "$NET_A" "$net_b" | sort -u); do + if ! docker network create --driver bridge --label "$LABEL" "$net" >/dev/null; then + echo "could not create network $net" >&2 + exit 2 + fi +done +if ! start_node "$NODE_A" "$NET_A" "$WORK/a.yaml" || ! start_node "$NODE_B" "$net_b" "$WORK/b.yaml"; then + echo "could not start the two nodes" >&2 + exit 2 +fi + +# Both daemons must be up with LAN discovery running before an absence of +# peers means anything: a node that never started mDNS cannot find a peer, and +# that is a setup failure, not a discovery verdict. +for node in "$NODE_A" "$NODE_B"; do + if ! wait_for_log "$node" "lan: mDNS discovery started" 30; then + echo "$node did not start LAN discovery; last log lines:" >&2 + docker logs "$node" 2>&1 | tail -20 >&2 + exit 2 + fi +done +pass "both daemons running with LAN discovery started" + +ip_a="$(container_ip "$NODE_A" "$NET_A")" +ip_b="$(container_ip "$NODE_B" "$net_b")" +log "Waiting up to ${DISCOVERY_TIMEOUT}s for each to list the other ($NODE_A $ip_a, $NODE_B $ip_b)" +addr_ab=""; addr_ba=""; unreadable=0 +for ((waited = 0; waited < DISCOVERY_TIMEOUT; waited++)); do + addr_ab="$(peer_addr "$NODE_A" "$npub_b")" || { unreadable=$((unreadable + 1)); addr_ab=""; } + addr_ba="$(peer_addr "$NODE_B" "$npub_a")" || { unreadable=$((unreadable + 1)); addr_ba=""; } + [[ -n "$addr_ab" && -n "$addr_ba" ]] && break + sleep 1 +done +[[ "$unreadable" -gt 0 ]] && echo " note: show peers could not be read $unreadable time(s) while waiting" + +for row in "$NODE_A|$addr_ab|$ip_b|B" "$NODE_B|$addr_ba|$ip_a|A"; do + IFS='|' read -r node addr want other <<<"$row" + # transport_addr is host:port; the host must be the other node's address on + # this bridge, which is where its advert came from. + if [[ -z "$addr" ]]; then + fail "$node does not list node $other as a peer after ${DISCOVERY_TIMEOUT}s" + elif [[ "${addr%:*}" == "$want" ]]; then + pass "$node lists node $other as an authenticated peer at $addr" + else + fail "$node lists node $other at $addr, not at its bridge address $want" + fi +done + +if [[ "$FAIL" -ne 0 ]]; then + for node in "$NODE_A" "$NODE_B"; do + echo "--- $node: lan and handshake lines" + docker logs "$node" 2>&1 | grep -iE "lan:|mdns|handshake|peer" | tail -20 + done +fi + +echo "" +echo "=== mdns: $PASS passed, $FAIL failed" +[[ "$FAIL" -eq 0 ]] || exit 1 +exit 0 diff --git a/testing/nextest-flaky/clean.xml b/testing/nextest-flaky/clean.xml new file mode 100644 index 00000000..aa9acb3b --- /dev/null +++ b/testing/nextest-flaky/clean.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/testing/nextest-flaky/failed.xml b/testing/nextest-flaky/failed.xml new file mode 100644 index 00000000..b44c9108 --- /dev/null +++ b/testing/nextest-flaky/failed.xml @@ -0,0 +1,72 @@ + + + + + + + + thread 'tests::always_fails' (191023) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + thread 'tests::always_fails' (191028) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::always_fails ... FAILED + +failures: + +failures: + tests::always_fails + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::always_fails' (191028) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + thread 'tests::always_fails' (191032) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::always_fails ... FAILED + +failures: + +failures: + tests::always_fails + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.01s + + + +thread 'tests::always_fails' (191032) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + +running 1 test +test tests::always_fails ... FAILED + +failures: + +failures: + tests::always_fails + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::always_fails' (191023) panicked at src/lib.rs:22:67: +nope +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + + diff --git a/testing/nextest-flaky/flaky.xml b/testing/nextest-flaky/flaky.xml new file mode 100644 index 00000000..69d48e14 --- /dev/null +++ b/testing/nextest-flaky/flaky.xml @@ -0,0 +1,53 @@ + + + + + + + thread 'tests::flips_once' (174785) panicked at src/lib.rs:10:13: +first attempt fails +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::flips_once ... FAILED + +failures: + +failures: + tests::flips_once + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::flips_once' (174785) panicked at src/lib.rs:10:13: +first attempt fails +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + + + thread 'tests::also_flips' (174790) panicked at src/lib.rs:18:13: +first attempt fails <&> "quoted" +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +running 1 test +test tests::also_flips ... FAILED + +failures: + +failures: + tests::also_flips + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s + + + +thread 'tests::also_flips' (174790) panicked at src/lib.rs:18:13: +first attempt fails <&> "quoted" +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + + + + diff --git a/testing/nextest-flaky/test.sh b/testing/nextest-flaky/test.sh new file mode 100644 index 00000000..67535d39 --- /dev/null +++ b/testing/nextest-flaky/test.sh @@ -0,0 +1,98 @@ +#!/bin/bash +# ── Fixture tests for check-nextest-flaky.sh ──────────────────────────────── +# The checker only runs on GitHub, after the ci-profile nextest steps, and a +# flaky test there is rare: a checker that stopped seeing them would stay quiet +# for months and look exactly like a healthy one. These fixtures pin its +# behaviour instead. Each is a real JUnit report from cargo-nextest 0.9.146 +# under this repository's ci profile (retries = 2), run over a four-test crate: +# +# clean.xml every test passed on its first attempt +# flaky.xml two tests failed once and passed on retry (FLAKY 2/3) +# failed.xml one test failed all three attempts (, ) +# +# The flaky fixture must produce one warning per flaky test and a step-summary +# entry for each; the clean and failed fixtures must produce none, since a test +# that never passed is the nextest step's red, not a flake. A missing report +# must not read as clean. +# +# Exit 0 = every case behaved. Exit 1 = a case did not. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +CHECK="$SCRIPT_DIR/../check-nextest-flaky.sh" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +FAILED=0 +ok() { echo " ok $*"; } +bad() { echo " FAIL $*"; FAILED=$((FAILED + 1)); } + +# expect : +# records ok when the command succeeds and a failure otherwise. +expect() { + local good="$1" poor="$2" + shift 2 + if "$@"; then ok "$good"; else bad "$poor"; fi + return 0 +} + +# run_case : runs the checker with a fresh step summary, leaving +# its output in $WORK/out, the summary in $WORK/summary and its status in RC. +run_case() { + : > "$WORK/summary" + GITHUB_STEP_SUMMARY="$WORK/summary" bash "$CHECK" "$1" > "$WORK/out" 2>&1 + RC=$? + WARNINGS="$(grep -c '^::warning title=Flaky test::' "$WORK/out")" + return 0 +} + +echo "check-nextest-flaky fixtures" + +run_case "$SCRIPT_DIR/flaky.xml" +expect "flaky: exit 0, so a flake does not red the run" "flaky: exit $RC, expected 0" \ + test "$RC" -eq 0 +expect "flaky: one warning per flaky test" "flaky: $WARNINGS warning(s), expected 2" \ + test "$WARNINGS" -eq 2 +for t in tests::flips_once tests::also_flips; do + if grep -q "^::warning title=Flaky test::flakydemo $t failed 1 attempt(s)" "$WORK/out" \ + && grep -q "flakydemo $t\`: failed 1 attempt(s)" "$WORK/summary"; then + ok "flaky: $t named in a warning and in the step summary" + else + bad "flaky: $t missing from the warnings or the step summary" + fi +done +if grep -q 'tests::steady\|tests::always_fails' "$WORK/out" "$WORK/summary"; then + bad "flaky: a test that passed first time was reported" +else + ok "flaky: tests that passed first time are not reported" +fi + +run_case "$SCRIPT_DIR/clean.xml" +expect "clean: exit 0, no warning, no summary" \ + "clean: exit $RC, $WARNINGS warning(s), summary $(wc -c < "$WORK/summary") bytes" \ + test "$RC" -eq 0 -a "$WARNINGS" -eq 0 -a ! -s "$WORK/summary" +expect "clean: all four cases were read" "clean: did not report reading four cases" \ + grep -q '4 test case(s), none passed only on retry' "$WORK/out" + +run_case "$SCRIPT_DIR/failed.xml" +expect "failed: a test that never passed is not reported as flaky" \ + "failed: exit $RC, $WARNINGS warning(s)" \ + test "$RC" -eq 0 -a "$WARNINGS" -eq 0 + +run_case "$WORK/no-such-report.xml" +expect "missing report: exit 2, not a clean pass" "missing report: exit $RC, expected 2" \ + test "$RC" -eq 2 -a "$WARNINGS" -eq 0 + +echo '' > "$WORK/empty.xml" +run_case "$WORK/empty.xml" +expect "report with no test cases: exit 2, not a clean pass" \ + "report with no test cases: exit $RC, expected 2" \ + test "$RC" -eq 2 + +if [[ "$FAILED" -ne 0 ]]; then + echo "check-nextest-flaky fixtures: $FAILED case(s) failed" + exit 1 +fi +echo "check-nextest-flaky fixtures: all cases passed" +exit 0 diff --git a/testing/openwrt/fixtures/released-fips.yaml b/testing/openwrt/fixtures/released-fips.yaml index 2f15160d..6bb09aee 100644 --- a/testing/openwrt/fixtures/released-fips.yaml +++ b/testing/openwrt/fixtures/released-fips.yaml @@ -162,11 +162,17 @@ transports: # auto_connect: true # accept_connections: true - # No BLE transport: OpenWrt builds target musl, which has no BlueZ backend. + # Bluetooth Low Energy transport — requires BlueZ and the 'ble' feature. + # ble: + # adapter: "hci0" + # mtu: 2048 + # advertise: true + # scan: true + # auto_connect: true + # accept_connections: true # Outbound LAN gateway. dnsmasq forwards .fips queries to listen=[::1]:5353 -# while it runs (configured by the fips-gateway init script). Requires IPv6 -# forwarding enabled. +# (configured by the fips init script). Requires IPv6 forwarding enabled. gateway: enabled: true pool: "fd01::/112" diff --git a/testing/openwrt/maintainer-scripts-test.sh b/testing/openwrt/maintainer-scripts-test.sh index ad707410..a2cf9643 100755 --- a/testing/openwrt/maintainer-scripts-test.sh +++ b/testing/openwrt/maintainer-scripts-test.sh @@ -17,9 +17,12 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" -# Pinned rather than :latest so the shell under test does not change under a -# run. Overridable for trying another ash build. -IMAGE="${OPENWRT_ASH_IMAGE:-busybox:1.37}" +# Pinned by digest so the shell under test does not change under a run: the +# 1.37 tag moves with every 1.37.x rebuild. This is the multi-arch index digest +# of busybox:1.37.0 as of 2026-10-01. Bump it deliberately, reading the new +# digest with `docker buildx imagetools inspect busybox:`. +# Overridable for trying another ash build. +IMAGE="${OPENWRT_ASH_IMAGE:-busybox:1.37.0@sha256:bdf57e528e45e4433820e045b29b4597825a1c9e38353532d90a01445013f82e}" if ! command -v docker >/dev/null 2>&1; then echo "openwrt-scripts: docker not found; cannot run the ash scenarios" >&2 @@ -54,6 +57,8 @@ docker run --rm --network none \ -e APK_SCRIPTS=/apk \ -e "POSTINST=${POSTINST:-}" \ -e "PRERM=${PRERM:-}" \ + -e "PREINST=${PREINST:-}" \ + -e "INIT_GATEWAY=${INIT_GATEWAY:-}" \ "$IMAGE" sh /src/testing/openwrt/scenarios.sh rc=$? diff --git a/testing/openwrt/package-test.sh b/testing/openwrt/package-test.sh index e6f8056c..b71f38cd 100755 --- a/testing/openwrt/package-test.sh +++ b/testing/openwrt/package-test.sh @@ -281,7 +281,8 @@ for path in ./postinst ./prerm; do done # ── P4. No source still names the drop-in ─────────────────────────────────── -# This is the only check on the SDK feed Makefile, which nothing here builds. +# With P5, one of the two checks on the SDK feed Makefile, which nothing here +# builds. # grep exits 1 when nothing matches and 2 when it could not read a path; only # the first is a pass. (cd "$PROJECT_ROOT" && grep -rlF 'dnsmasq.d/fips.conf' \ @@ -301,6 +302,31 @@ else ok "P4 the drop-in source file is gone" fi +# ── P5. The SDK feed Makefile ships the same maintainer scripts ───────────── +# Read, not built: building it needs the OpenWrt SDK. Each script must be the +# file in scripts/ that the scenarios run, and fips.yaml must be a conffile. +MAKEFILE="$PROJECT_ROOT/packaging/openwrt-ipk/Makefile" +[[ -r "$MAKEFILE" ]] || harness_fail "cannot read $MAKEFILE" +for script in preinst postinst prerm; do + if awk -v want="define Package/fips/$script" -v body="\$(file < \$(CURDIR)/scripts/$script)" ' + $0 == want { inside = 1; next } + inside && $0 == body { found = 1 } + inside && $0 == "endef" { inside = 0 } + END { exit !found }' "$MAKEFILE"; then + ok "P5 the SDK Makefile's $script is scripts/$script" + else + bad "P5 the SDK Makefile does not define Package/fips/$script as scripts/$script" + fi +done +if awk '$0 == "define Package/fips/conffiles" { inside = 1; next } + inside && $0 == "/etc/fips/fips.yaml" { found = 1 } + inside && $0 == "endef" { inside = 0 } + END { exit !found }' "$MAKEFILE"; then + ok "P5 the SDK Makefile lists /etc/fips/fips.yaml as a conffile" +else + bad "P5 the SDK Makefile does not list /etc/fips/fips.yaml as a conffile" +fi + # ── Hand the scripts to the ash scenarios ─────────────────────────────────── if [[ -n "$KEEP" ]]; then for phase in $WANT_PHASES; do diff --git a/testing/openwrt/scenarios.sh b/testing/openwrt/scenarios.sh index fe78548f..bb91d279 100755 --- a/testing/openwrt/scenarios.sh +++ b/testing/openwrt/scenarios.sh @@ -8,9 +8,9 @@ # exercised is the scripts' behaviour given that contract, not opkg itself. # A real `opkg upgrade` on a router image stays uncovered. # -# POSTINST and PRERM may be pointed at other files. That is the seam used to -# see a scenario red against the previously released scripts, and to re-break -# the fixed ones during a break-check. +# PREINST, POSTINST, PRERM and INIT_GATEWAY may be pointed at other files. That is the +# seam used to see a scenario red against the previously released scripts, and +# to re-break the fixed ones during a break-check. # # APK_SCRIPTS names a directory holding the four scripts the .apk registers # (post-install, pre-upgrade, post-upgrade, pre-deinstall), as captured from @@ -23,13 +23,15 @@ set -u REPO="${REPO:-/src}" POSTINST="${POSTINST:-$REPO/packaging/openwrt-ipk/scripts/postinst}" +PREINST="${PREINST:-$REPO/packaging/openwrt-ipk/scripts/preinst}" PRERM="${PRERM:-$REPO/packaging/openwrt-ipk/scripts/prerm}" RELEASED_PRERM="$REPO/testing/openwrt/fixtures/released-prerm" -INIT_GATEWAY="$REPO/packaging/openwrt-ipk/files/etc/init.d/fips-gateway" +INIT_GATEWAY="${INIT_GATEWAY:-$REPO/packaging/openwrt-ipk/files/etc/init.d/fips-gateway}" APK_SCRIPTS="${APK_SCRIPTS:-}" SHIPPED_YAML="$REPO/packaging/openwrt-ipk/files/etc/fips/fips.yaml" -# The fips.yaml every release up to 0.5.1 shipped, from before the gateway's -# default DNS port moved. +# The fips.yaml v0.5.0 and v0.5.1 shipped, byte for byte (from the v0.5.1 tag), +# from before the gateway's default DNS port moved. v0.3.0 to v0.4.2 shipped an +# older file with the same legacy listen line. RELEASED_YAML="$REPO/testing/openwrt/fixtures/released-fips.yaml" GATEWAY_RS="$REPO/src/config/gateway.rs" SETUP_SCRIPT="$REPO/packaging/openwrt-ipk/files/etc/uci-defaults/90-fips-setup" @@ -440,41 +442,44 @@ YAML # ── 7. start_service refuses to touch dnsmasq for a disabled gateway ──────── # The init script's helpers are redefined after sourcing it, so start_service # runs its own decision against recorded stubs instead of uci, procd and the -# network. +# network. Where dnsmasq ends up while the gateway runs is scenario 15's. +stub_start_service_helpers() { + # stub_start_service_helpers [keep-swap]: keep-swap leaves the real + # dnsmasq_swap_fips_upstream in place, for a caller with the uci stub. + if [ "${1:-}" != "keep-swap" ]; then + dnsmasq_swap_fips_upstream() { echo "dnsmasq_swap $1" >> "$CALLS"; return 0; } + fi + sysctl() { return 0; } + modprobe() { return 0; } + logger() { return 0; } + procd_set_param() { + if [ "$1" = "command" ]; then + echo "$*" >> "$CALLS" + fi + return 0 + } + procd_close_instance() { return 0; } + gateway_add_global_prefix() { echo "add_global_prefix" >> "$CALLS"; return 0; } + gateway_add_ra_route() { echo "add_ra_route" >> "$CALLS"; return 0; } + procd_open_instance() { echo "procd_open_instance" >> "$CALLS"; return 0; } + return 0 +} + scenario_start_service_guard() { note "scenario 7: start_service guard" # shellcheck source=/dev/null . "$INIT_GATEWAY" - - sysctl() { return 0; } - modprobe() { return 0; } - logger() { return 0; } - sleep() { return 0; } - procd_set_param() { return 0; } - procd_close_instance() { return 0; } - dnsmasq_swap_fips_upstream() { echo "dnsmasq_swap $1" >> "$CALLS"; return 0; } - gateway_add_global_prefix() { echo "add_global_prefix" >> "$CALLS"; return 0; } - gateway_add_ra_route() { echo "add_ra_route" >> "$CALLS"; return 0; } - procd_open_instance() { echo "procd_open_instance" >> "$CALLS"; return 0; } + stub_start_service_helpers reset_state CONFIG="$SHIPPED_YAML" start_service >/dev/null 2>&1 - assert_called "dnsmasq_swap 5365" "an enabled gateway redirects dnsmasq to the default port" + assert_none_called_with_prefix "dnsmasq_swap " \ + "an enabled gateway does not point dnsmasq at a gateway that is not yet listening" assert_called "procd_open_instance" "an enabled gateway still starts the daemon" - - reset_state - CONFIG="$WORK/explicit-5353.yaml" - cat > "$CONFIG" <<'YAML' -gateway: - enabled: true - pool: "fd01::/112" - dns: - listen: "[::1]:5353" -YAML - start_service >/dev/null 2>&1 - assert_called "dnsmasq_swap 5353" "dnsmasq follows an explicit gateway.dns.listen" + assert_called "command /etc/init.d/fips-gateway supervise" \ + "procd runs the gateway through the init script's supervise command" reset_state CONFIG="$WORK/disabled.yaml" @@ -833,9 +838,507 @@ scenario_listen_migration() { return 0 } +# ── 15. dnsmasq points at the gateway only while it listens ─────────────── +# start_service runs against recorded procd stubs, and the command it hands +# procd is then executed as procd would execute it: the real init script is +# installed at /etc/init.d/fips-gateway, whose #! line runs a stand-in +# /etc/rc.common, and /usr/bin/fips-gateway is a stub gateway. A stub that +# binds does so by becoming nc, so the socket is the stub's own, as the real +# gateway's is. dnsmasq's loopback .fips forward lives in the uci stub and +# starts on the daemon's port, as it is while the gateway is stopped. However +# the gateway exits, it must end there. + +GW_STUB=/tmp/fips-gw-stub + +install_supervise_env() { + # install_supervise_env + install_uci_stub + printf '#!/bin/sh\nexit 0\n' > "$STUB_BIN/logger" + chmod 0755 "$STUB_BIN/logger" + + rm -rf "$GW_STUB" + mkdir -p "$GW_STUB" /etc/fips /usr/bin + rm -f /var/run/fips-gateway.pid + echo "$1" > "$GW_STUB/port" + cat > /etc/fips/fips.yaml < /etc/rc.common <<'SHIM' +#!/bin/sh +# Stand-in for OpenWrt's rc.common: runs one of the init script's commands. +initscript="$1" +action="$2" +shift 2 +. "$initscript" +case " ${EXTRA_COMMANDS:-} " in +*" $action "*) "$action" "$@" ;; +*) echo "rc.common stand-in: $action is not a command of $initscript" >&2; exit 2 ;; +esac +SHIM + chmod 0755 /etc/rc.common + + cat > /usr/bin/fips-gateway <<'STUB' +#!/bin/sh +# Stub gateway, by $GW_STUB/mode: +# parse exits 1 at once, as on a config it cannot parse; +# taken exits 1 after two seconds without binding, as when another +# process holds its port; +# bind becomes nc listening on the port, until signalled; +# stubborn ignores SIGTERM and runs until killed, binding nothing itself. +d=/tmp/fips-gw-stub +echo $$ > "$d/pid" +port="$(cat "$d/port")" +case "$(cat "$d/mode")" in +parse) exit 1 ;; +taken) sleep 2; exit 1 ;; +stubborn) + trap '' TERM + while :; do sleep 1; done + ;; +esac +exec nc -u -l -p "$port" -s ::1 /dev/null 2>&1 +STUB + chmod 0755 /usr/bin/fips-gateway + + uci_seed "$DNSMASQ_OPT" "/fips/::1#5354" "/lan/192.168.1.2" + return 0 +} + +remove_supervise_env() { + rm -f /etc/rc.common /usr/bin/fips-gateway /var/run/fips-gateway.pid + rm -rf /etc/fips "$GW_STUB" "$STUB_BIN" + return 0 +} + +fips_upstream() { + # The loopback .fips forwards dnsmasq has, sorted, on one line. + "$STUB_BIN/uci" -q get "$DNSMASQ_OPT" | tr ' ' '\n' | grep '^/fips/' | sort | tr '\n' ' ' + return 0 +} + +wait_for_upstream() { + # wait_for_upstream : succeeds once dnsmasq + # forwards .fips to ::1# and nowhere else on loopback. + n=0 + while [ "$n" -lt "$2" ]; do + [ "$(fips_upstream)" = "/fips/::1#$1 " ] && return 0 + sleep 0.1 + n=$((n + 1)) + done + return 1 +} + +start_gateway_service() { + # Runs start_service and then, in the background, the command it gave + # procd, with the uci and logger stubs first on PATH. Sets CMD_PID. + ( + PATH="$STUB_BIN:$PATH" + # shellcheck source=/dev/null + . "$INIT_GATEWAY" + stub_start_service_helpers keep-swap + CONFIG=/etc/fips/fips.yaml + start_service >/dev/null 2>&1 + ) + sed -n 's/^command //p' "$CALLS" > "$WORK/command" + # shellcheck disable=SC2046 + set -- $(cat "$WORK/command") + if [ $# -eq 0 ]; then + CMD_PID="" + bad "start_service gave procd no command: $(calls_oneline)" + return 1 + fi + ( PATH="$STUB_BIN:$PATH" exec "$@" ) > "$WORK/command.out" 2>&1 & + CMD_PID=$! + return 0 +} + +wait_command() { + # wait_command: waits up to 20 s for the procd command; sets CMD_RC. + ( sleep 20; kill -KILL "$CMD_PID" 2>/dev/null ) & + dog=$! + CMD_RC=0 + wait "$CMD_PID" || CMD_RC=$? + kill "$dog" 2>/dev/null + wait "$dog" 2>/dev/null + return 0 +} + +stub_pid() { + # stub_pid: the stub gateway's pid once it has started, else empty. + n=0 + while [ ! -s "$GW_STUB/pid" ] && [ "$n" -lt 100 ]; do sleep 0.1; n=$((n + 1)); done + cat "$GW_STUB/pid" 2>/dev/null + return 0 +} + +assert_gone() { + # assert_gone + if [ -n "$1" ] && kill -0 "$1" 2>/dev/null; then + bad "$2 — pid $1 is still running" + kill -KILL "$1" 2>/dev/null + else + ok "$2" + fi + return 0 +} + +assert_on_daemon() { + # assert_on_daemon + assert_equals "$(fips_upstream)" "/fips/::1#5354 " "$1" + return 0 +} + +hold_port() { + # hold_port : an unrelated process binds the port. Sets HOLDER. + nc -u -l -p "$1" -s ::1 /dev/null 2>&1 & + HOLDER=$! + n=0 + while ! grep -q ":$(printf '%04X' "$1") " /proc/net/udp6 && [ "$n" -lt 50 ]; do + sleep 0.1 + n=$((n + 1)) + done + return 0 +} + +scenario_supervise() { + note "scenario 15: dnsmasq follows the gateway's life" + unset -f sleep logger sysctl modprobe 2>/dev/null + + # 1. The gateway exits before binding, as on a config it cannot parse. + reset_state + install_supervise_env 5365 + echo parse > "$GW_STUB/mode" + if start_gateway_service; then + wait_command + assert_on_daemon "a gateway that exits before binding leaves dnsmasq on the daemon" + if [ "$CMD_RC" -ne 0 ]; then + ok "the gateway's failure is the exit status procd sees" + else + bad "the procd command exited 0 although the gateway failed" + fi + fi + remove_supervise_env + + # 2. Another process holds the gateway's port, as an mDNS responder holds + # 5353, so the gateway cannot bind it and exits. + reset_state + install_supervise_env 5365 + echo taken > "$GW_STUB/mode" + hold_port 5365 + if start_gateway_service; then + if wait_for_upstream 5365 15; then + bad "dnsmasq was pointed at a port the gateway does not hold" + else + ok "dnsmasq is not pointed at a port another process holds" + fi + wait_command + assert_on_daemon "a gateway whose port is taken leaves dnsmasq on the daemon" + fi + kill "$HOLDER" 2>/dev/null + wait "$HOLDER" 2>/dev/null + remove_supervise_env + + # 3. The gateway binds, then fails, as on a NAT or route setup error. + reset_state + install_supervise_env 5365 + echo bind > "$GW_STUB/mode" + if start_gateway_service; then + if wait_for_upstream 5365 100; then + ok "dnsmasq forwards .fips to the gateway once it is listening" + else + bad "dnsmasq never moved to the listening gateway: $(fips_upstream)" + fi + kill -USR1 "$(stub_pid)" 2>/dev/null + wait_command + assert_on_daemon "a gateway that fails after binding hands dnsmasq back to the daemon" + assert_equals "$(uci_sorted "$DNSMASQ_OPT")" \ + "$(sorted_words /lan/192.168.1.2 /fips/::1#5354)" \ + "the swaps keep dnsmasq's other servers" + fi + remove_supervise_env + + # 4. procd stops a running gateway with SIGTERM, here on its own (as when + # procd restarts an instance), without stop_service. Port from the config. + reset_state + install_supervise_env 5400 + echo bind > "$GW_STUB/mode" + if start_gateway_service; then + if wait_for_upstream 5400 100; then + ok "dnsmasq follows an explicit gateway.dns.listen" + else + bad "dnsmasq never moved to the gateway's port 5400: $(fips_upstream)" + fi + gw="$(stub_pid)" + kill -TERM "$CMD_PID" 2>/dev/null + wait_command + assert_equals "$CMD_RC" "143" "procd's SIGTERM reaches the gateway, which exits on it" + assert_gone "$gw" "the gateway does not outlive the procd command" + assert_on_daemon "a gateway stopped by SIGTERM hands dnsmasq back to the daemon" + fi + remove_supervise_env + + # 5. A gateway that ignores SIGTERM is killed before procd's own timeout, + # after which procd would kill only the supervise shell. + reset_state + install_supervise_env 5365 + echo stubborn > "$GW_STUB/mode" + if start_gateway_service; then + gw="$(stub_pid)" + kill -TERM "$CMD_PID" 2>/dev/null + # Tenths of a second until the gateway is gone; the supervise shell + # reaps it at once, so kill -0 fails as soon as it dies. + tenths=0 + while [ -n "$gw" ] && kill -0 "$gw" 2>/dev/null && [ "$tenths" -lt 100 ]; do + sleep 0.1 + tenths=$((tenths + 1)) + done + wait_command + assert_equals "$CMD_RC" "137" "a gateway that ignores SIGTERM is killed" + assert_gone "$gw" "a gateway that ignores SIGTERM does not outlive the procd command" + if [ "$tenths" -lt 45 ]; then + ok "it is killed inside procd's 5 s stop timeout (after ${tenths} tenths of a second)" + else + bad "it was killed after ${tenths} tenths of a second, too close to or past procd's 5 s" + fi + assert_on_daemon "a killed gateway hands dnsmasq back to the daemon" + fi + remove_supervise_env + + # 6. The swap back after a gateway exits is skipped only when the gateway + # procd started in its place, named in the pid file, holds the port. + reset_state + install_uci_stub + ( + PATH="$STUB_BIN:$PATH" + # shellcheck source=/dev/null + . "$INIT_GATEWAY" + mkdir -p /var/run + hold_port 5365 + uci_seed "$DNSMASQ_OPT" "/fips/::1#5365" + echo "$HOLDER" > "$GW_PIDFILE" + gateway_dns_release 5365 99999 >/dev/null 2>&1 + fips_upstream > "$WORK/successor" + echo 99998 > "$GW_PIDFILE" + gateway_dns_release 5365 99999 >/dev/null 2>&1 + fips_upstream > "$WORK/other" + kill "$HOLDER" + wait "$HOLDER" 2>/dev/null + rm -f "$GW_PIDFILE" + ) + assert_file_is "$WORK/successor" "/fips/::1#5365 " \ + "the swap back is skipped while the successor gateway holds the port" + assert_file_is "$WORK/other" "/fips/::1#5354 " \ + "the swap back happens while only some other process holds the port" + rm -rf "$STUB_BIN" + return 0 +} + +# ── 16. A package built from the SDK feed Makefile ───────────────────────── +# OpenWrt generates that package's postinst and prerm around default_postinst +# and default_prerm (package/base-files/files/lib/functions.sh, read at OpenWrt +# main and openwrt-24.10). After its own steps, default_postinst runs a loop +# over the package's init scripts: "enable" unless PKG_UPGRADE is 1, then +# "start". Under opkg the package's postinst body is sourced before that loop; +# under apk it is appended to the generated script and runs after it. +# default_prerm sources the prerm body with the generated script's own path as +# $1, then disables (on a removal) and stops each init script. The preinst is +# the package's own, run as opkg runs it ("install" or "upgrade ") or as +# apk runs it (versions only, PKG_UPGRADE=1 exported on an upgrade). +# +# /etc/init.d/fips is the recording stub. /etc/init.d/fips-gateway is the real +# script, run through a stand-in rc.common that keeps enablement as the +# /etc/rc.d links OpenWrt's does and records whether start_service opened a +# procd instance, which is what starting the gateway means. + +install_sdk_env() { + install_uci_stub + printf '#!/bin/sh\nexit 0\n' > "$STUB_BIN/logger" + chmod 0755 "$STUB_BIN/logger" + rm -rf /etc/rc.d + mkdir -p /etc/rc.d /etc/fips /var/run + cp "$SHIPPED_YAML" /etc/fips/fips.yaml + cp "$INIT_GATEWAY" /etc/init.d/fips-gateway + chmod 0755 /etc/init.d/fips-gateway + rm -f /var/run/fips-gateway-install-hold + cat > /etc/rc.common <<'SHIM' +#!/bin/sh +# Stand-in for OpenWrt's rc.common: enablement as /etc/rc.d links, and start +# runs start_service with procd's calls recorded instead of made. +initscript="$1" +action="$2" +shift 2 +name="${initscript##*/}" +enable() { ln -sf "../init.d/$name" "/etc/rc.d/S${START}$name"; ln -sf "../init.d/$name" "/etc/rc.d/K${STOP}$name"; } +disable() { rm -f /etc/rc.d/S??"$name" /etc/rc.d/K??"$name"; } +enabled() { [ -L "/etc/rc.d/S${START}$name" ]; } +procd_open_instance() { echo "$name instance" >> "$CALLS"; } +procd_set_param() { :; } +procd_close_instance() { :; } +stop_service() { :; } +. "$initscript" +sysctl() { :; } +modprobe() { :; } +gateway_add_global_prefix() { :; } +gateway_add_ra_route() { :; } +stop_service() { :; } +echo "$name $action" >> "$CALLS" +case "$action" in +start) start_service ;; +stop) stop_service ;; +*) "$action" "$@" ;; +esac +SHIM + chmod 0755 /etc/rc.common + return 0 +} + +remove_sdk_env() { + rm -rf /etc/rc.d /etc/fips "$STUB_BIN" + rm -f /etc/rc.common /var/run/fips-gateway-install-hold + return 0 +} + +sdk_loop() { + # The init-script loop of default_postinst. + for i in /etc/init.d/fips /etc/init.d/fips-gateway; do + if [ "${PKG_UPGRADE:-0}" != "1" ]; then + "$i" enable + fi + "$i" start + done + return 0 +} + +sdk_postinst() { + # sdk_postinst ipk|apk + if [ "$1" = "ipk" ]; then + ( set -- /usr/lib/opkg/info/fips.postinst configure; . "$POSTINST" ) >/dev/null 2>&1 + sdk_loop >/dev/null 2>&1 + else + sdk_loop >/dev/null 2>&1 + sh "$POSTINST" 2.0-r1 >/dev/null 2>&1 + fi + return 0 +} + +sdk_prerm_upgrade_ipk() { + # default_prerm for the outgoing SDK-built package on an opkg upgrade. + ( set -- /usr/lib/opkg/info/fips.prerm upgrade 2.0-r1; . "$PRERM" ) >/dev/null 2>&1 + for i in /etc/init.d/fips /etc/init.d/fips-gateway; do + "$i" stop >/dev/null 2>&1 + done + return 0 +} + +sdk_preinst() { + # sdk_preinst , run as a script, as opkg and apk run it. + if [ ! -f "$PREINST" ]; then + bad "there is no preinst at $PREINST" + return 0 + fi + sh "$PREINST" "$@" >/dev/null 2>&1 + return 0 +} + +assert_gateway() { + # assert_gateway + if [ -L /etc/rc.d/S96fips-gateway ]; then got=enabled; else got=disabled; fi + assert_equals "$got" "$1" "$3: fips-gateway ends $1" + if grep -qxF "fips-gateway instance" "$CALLS"; then got=started; else got=stopped; fi + assert_equals "$got" "$2" "$3: fips-gateway is $2" + assert_absent /var/run/fips-gateway-install-hold "$3: no install hold is left behind" + return 0 +} + +scenario_sdk_package() { + note "scenario 16: SDK feed package, default_postinst and default_prerm" + saved_path="$PATH" + PATH="$STUB_BIN:$PATH" + export PATH + + for fmt in ipk apk; do + reset_state + install_sdk_env + if [ "$fmt" = "ipk" ]; then + PKG_UPGRADE=0 sdk_preinst install + PKG_UPGRADE=0 sdk_postinst ipk + else + sdk_preinst 2.0-r1 + sdk_postinst apk + fi + assert_called "fips start" "$fmt fresh install: the daemon is started" + assert_file_is "$FIPS_STATE" "1" "$fmt fresh install: the daemon is enabled" + assert_gateway disabled stopped "$fmt fresh install" + remove_sdk_env + + for gw in enabled disabled; do + reset_state + install_sdk_env + echo 1 > "$FIPS_STATE" + [ "$gw" = "enabled" ] && /etc/init.d/fips-gateway enable >/dev/null 2>&1 + : > "$CALLS" + if [ "$fmt" = "ipk" ]; then + PKG_UPGRADE=1 sdk_prerm_upgrade_ipk + PKG_UPGRADE=1 sdk_preinst upgrade 1.0-r1 + PKG_UPGRADE=1 sdk_postinst ipk + else + PKG_UPGRADE=1 sdk_preinst 2.0-r1 1.0-r1 + PKG_UPGRADE=1 sdk_postinst apk + fi + if [ "$gw" = "enabled" ]; then + assert_gateway enabled started "$fmt upgrade, gateway enabled" + else + assert_gateway disabled stopped "$fmt upgrade, gateway disabled" + fi + assert_absent "$UPGRADE_MARKER" "$fmt upgrade, gateway $gw: the upgrade marker is removed" + remove_sdk_env + done + done + + # An opkg upgrade from a released package, whose prerm disabled the + # gateway and left no marker: the postinst body re-enables and starts it, + # as with build-ipk.sh, which needs it to clear the hold first. + reset_state + install_sdk_env + echo 1 > "$FIPS_STATE" + /etc/init.d/fips-gateway enable >/dev/null 2>&1 + : > "$CALLS" + PKG_UPGRADE=1 sh "$RELEASED_PRERM" upgrade 2.0-r1 >/dev/null 2>&1 + PKG_UPGRADE=1 sdk_preinst upgrade 0.5.1 + PKG_UPGRADE=1 sdk_postinst ipk + assert_gateway enabled started "ipk upgrade from a released package" + remove_sdk_env + + # An image build runs the scripts on the build host, with IPKG_INSTROOT + # naming the image root: they must not touch the host at all. + reset_state + rm -f /var/run/fips-gateway-install-hold "$UPGRADE_MARKER" + for pkg_upgrade in 0 1; do + IPKG_INSTROOT=/tmp/fips-image-root PKG_UPGRADE=$pkg_upgrade sh "$PREINST" install >/dev/null 2>&1 + IPKG_INSTROOT=/tmp/fips-image-root PKG_UPGRADE=$pkg_upgrade sh "$POSTINST" configure >/dev/null 2>&1 + IPKG_INSTROOT=/tmp/fips-image-root PKG_UPGRADE=$pkg_upgrade sh "$PRERM" upgrade 2.0-r1 >/dev/null 2>&1 + IPKG_INSTROOT=/tmp/fips-image-root PKG_UPGRADE=$pkg_upgrade sh "$PRERM" remove >/dev/null 2>&1 + done + assert_equals "$(calls_oneline)" "" "image build: no script touches the build host's services" + assert_absent /var/run/fips-gateway-install-hold "image build: the preinst leaves no hold on the build host" + assert_absent "$UPGRADE_MARKER" "image build: the prerm leaves no marker on the build host" + + PATH="$saved_path" + return 0 +} + echo "OpenWrt maintainer-script scenarios (shell: $(readlink -f /proc/$$/exe 2>/dev/null || echo sh))" echo " postinst: $POSTINST" echo " prerm: $PRERM" +echo " preinst: $PREINST" echo " apk: ${APK_SCRIPTS:-(not set)}" scenario_fresh_install @@ -852,6 +1355,8 @@ scenario_dns_port_reader scenario_default_port_parity scenario_swap_cleanup scenario_listen_migration +scenario_supervise +scenario_sdk_package echo "" if [ "$FAILURES" -eq 0 ]; then