mirror of
https://github.com/jmcorgan/fips.git
synced 2026-09-14 00:45:08 +00:00
master
986
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Johnathan Corgan
|
ff92b378c6 |
Merge maint into master, carrying the show links counter fix
show links and its control-socket snapshot now report the traffic counters of the peer bound to each link, instead of counters on the link record that nothing ever wrote, so the query agrees with show peers for the same link_id. show_links, the snapshot block and the peer accessors are identical on both lines, so the change applies unchanged, and its changelog entry lands under master's Unreleased section. |
||
|
Johnathan Corgan
|
4c2f0143e4 |
fix(control): report the bound peer's traffic counters in show links
show_links rendered packets_sent, packets_recv, bytes_sent, bytes_recv and last_recv_ms from the LinkStats held on the Link record, but nothing on the data plane ever wrote that copy. Every send and receive counter write goes to the separate LinkStats held on the active peer, which show_peers reads. So every link reported zero however much traffic it carried, while show_peers showed the traffic for the same link_id (GitHub issue #158). The two copies were never connected: the Link counters have had no production writer since they were added, and show_links read them from the day it was introduced. Both render sites now take a link's counters from the peer bound to it, and fall back to the link's own (zero) counters when no peer is bound yet, as for a link still in handshake. That covers the on-loop handler and the tick-published snapshot the control socket serves from, which must stay byte-identical. A small helper on Node maps each peer's link_id to its counters once per render. The fix reads the counters rather than writing a second copy at each send and receive site. Writing both would add a link lookup per packet on the hot path and keep two sources of truth that every future writer must keep in step, which is how this defect arose. Removing the unused Link counters would change public methods, so that is left for a separate cleanup. The counters follow the peer across address changes, while the row's transport_id and remote_addr remain those the link was created with; that matches how show_peers already keys the same counters by link_id. The counters cover authenticated link frames only, so they are not expected to equal the transport totals in show_transports. The response shape is unchanged. The new test establishes a real two-node link over the loopback transport and checks each node's show_links row against that node's peer counters, after first requiring the counters to be non-zero so a run with no traffic cannot pass. It also requires node 1's received count to be non-zero and no greater than node 0's sent count, a bound that does not come from the same node's peer copy. It fails on the unfixed code with packets_sent 0 against 3, and breaking only the snapshot site fails its on-loop and snapshot equality check. A second test keeps a link with no peer in the output with zero counters on both render paths. Fixes #158 |
||
 fr34akyandJohnathan Corgan
|
429d77731b |
add a pfSense package
pfSense is FreeBSD underneath, but the FreeBSD package does not work there, failing in three silent ways. pfSense runs only /usr/local/etc/rc.d/*.sh at boot and re-runs them when WAN gets a new address, so a suffixless rc script never starts; unbound.conf is generated from config.xml with no conf.d, so a drop-in is never read; and on a firewall where the default-on "Allow IPv6" has been turned off, unbound is then generated with do-ip6: no and a responder on ::1 is unreachable. So this ships fips.sh, wires the fips. zone into the DNS Resolver through config.xml, and binds the responder on 127.0.0.1 for robustness against that last case. The rc script is plain sh: what pfSense imposes is the .sh name and that a re-run leave a running daemon alone and exit 0. It identifies the daemon by process name and recovers an orphaned daemon(8) supervisor found via fstat, since a locked empty pidfile makes daemon(8) report pid -1. The DNS setup is a manual step, never run from post-install, and validates the merged options with unbound-checkconf (pfSense's test_unbound_config) before touching config.xml, so a bad merge cannot take DNS from every client behind the firewall. The daemon runs under daemon(8) -H so newsyslog can rotate its log by signalling a reopen. Packages link statically by default: pfSense runs a FreeBSD base that cannot be obtained to link against. A firmware upgrade keeps the package (pfSense-upgrade removes only pfSense-pkg-*; confirmed on a live Plus 26.03.1 -> 26.07 upgrade, aarch64 — the package survived and the daemon restarted at boot. That is a minor, FreeBSD 16 -> 16 change; the cross-major compat case is still only source-reasoned). aarch64 is refused, where a static binary faults at posix_spawn. The mechanics the two builders share — version derivation, the stage layout, the manifest fields, the @sample scripts and pkg create — live in packaging/common/pkg-lib.sh, which both source; the FreeBSD package is byte-identical before and after that extraction. One ABI can serve more than one product: CE 2.9 and Plus 26.x on Intel are both FreeBSD:16:amd64 with a byte-identical artifact, named ...-ce2.9-plus26-amd64.pkg. The pfSense package is built and checked in its own CI job — separate from the FreeBSD package, and not a dependency of the release job, so a pfSense-only failure reds that job alone and is never a release asset. It is kept as a workflow artifact until it has been installed on a real pfSense box. CI produces the CE 2.8.1 (FreeBSD:15:amd64) package; CE 2.9, Plus 26.x Intel and ARM need a FreeBSD 16 build host the CI does not have, and ARM stays build-it-yourself because rustup ships no toolchain for it. testing/check-pfsense-pkg.sh validates a built package on any FreeBSD host and runs in that CI job: contents, modes, a positive boot-script lifecycle against a stub daemon, php -l and a fips_strip_block unit test of the config.xml helper. Installing on a real pfSense box, and the firmware-upgrade behaviour, are covered only by an aarch64 hardware run and pfSense-upgrade's source; the README records what is and is not tested. Co-authored-by: Johnathan Corgan <johnathan@corganlabs.com> |
||
|
Johnathan Corgan
|
38622e5dac |
Merge maint into master, carrying the mesh-lab empty-array fix
The mesh-lab rekey loop expands its env_args array in a form bash 3.2 accepts when the array is empty, so the plain rekey variant runs on macOS instead of skipping setup and reporting a false rekey failure. run-loop.sh is identical on both lines, so the change applies unchanged. |
||
 SherryandJohnathan Corgan
|
31ebec6208 |
fix(testing): guard the empty env_args splat in the mesh-lab loop
Running `run-loop.sh rekey` on macOS aborts the setup subshell with
"env_args[@]: unbound variable", so generate-configs.sh, the rekey
inject-config step and `docker compose up -d` never run. The script
carries on and runs the suite against a stack that was never started,
so the failure surfaces as a bogus rekey failure rather than a setup
error, and setup.log is empty because the expansion fails before the
redirection is applied.
run_rekey_family() declares `local env_args=()` and fills it only for
the rekey-accept-off and rekey-outbound-only variants, so the plain
`rekey` variant reaches the `env "${env_args[@]}"` call sites with an
empty array. Under `set -u` (line 49) bash 3.2 treats an empty array
expansion as unbound. Bash 4.4 stopped doing so, which is why Linux
and CI never see this and only macOS's /bin/bash 3.2 is affected.
Expand the array as `${env_args[@]+"${env_args[@]}"}` at those three
call sites. It yields nothing for an empty array and the quoted
elements otherwise, on every bash version, so the plain variant runs
`env` with no assignments and the other two keep their overrides. The
nat-lan call site is left as it is: its env_args is declared with an
element and can never be empty.
Co-authored-by: Johnathan Corgan <johnathan@corganlabs.com>
|
||
|
Johnathan Corgan
|
21faff5269 |
Merge maint into master, carrying the systemd test container isolation
The deb-install and dns-resolver harnesses start their systemd containers without --privileged, with a check that fails the suite if a container can reach the host's consoles or kernel parameters. The harness files are identical on both lines, so the change applies unchanged. |
||
|
Johnathan Corgan
|
ddaf4ff5f5 |
fix(testing): boot the systemd install containers without --privileged
The deb-install and dns-resolver harnesses boot systemd inside containers started with --privileged. A privileged container sees the host's real VT and serial devices and a writable /proc/sys and /sys, so the image's systemd acts on the host: it starts a getty on the host's tty1, where each side's hangup kills the other's getty until the host unit hits its start limit and the machine has no console login; logind holds tty6; and systemd-sysctl applies the image's sysctl.d to the host's kernel parameters. The Fedora image also sets up the host's virtual consoles and runs a udev coldplug against the host's /sys. Start the containers with SYS_ADMIN and NET_ADMIN and an unconfined AppArmor profile instead. That is what the suites use privilege for: mount namespaces for the units' sandboxing, TUN and dummy links, and nftables in the container's own network namespace. The default AppArmor profile denies the mounts systemd makes, so the profile override is needed on AppArmor hosts. The containers then have no host console devices, and docker's /proc/sys and /sys mounts are read-only, so the getty, logind VT, sysctl, vconsole and udev units above skip on their own conditions and nothing has to be listed per image. The cgroup tree the harnesses bind in for systemd stays writable, as before. IPv6 forwarding for the gateway checks is now set with --sysctl at start, since /proc/sys is no longer writable from inside. Forwarding is therefore on for the whole scenario, including the install and resolver checks that run before the gateway, where it used to be off until the gateway step. A check after each container start fails the suite if the container can see a host console device or write /proc/sys or /sys, so restoring --privileged turns the run red rather than quietly reaching the host. Measured: all 13 dns-resolver and 5 deb-install scenarios pass, with the check passing once per container. The four host kernel parameters the Fedora image's sysctl.d sets to other values were unchanged across the dns-resolver run. In the e2e-debian12 container, no tty device nodes exist, no process holds a tty and /proc/sys and /sys are read-only; before the change the same container had agetty on the host's tty1 and logind on tty6. With --privileged restored on one start function the check fails and the suite exits non-zero. |
||
|
Johnathan Corgan
|
c2ef48f6ac |
Merge maint into master, carrying two fixes from fr34aky
The invalid-hostname test now resolves an absolute name, so a wildcard search domain cannot answer it, and the FreeBSD package README uses version placeholders in place of a stale concrete version. Both apply to master unchanged. |
||
|
fr34akyandJohnathan Corgan
|
93d45191c4 |
docs(packaging/freebsd): use version placeholders in the examples
The build and install examples named v0.5.1 — the release current when they were written. Every release since has left them describing a version nobody builds, and nothing gates them, so the drift is silent and only a reader hitting a stale copy-pasteable command notices. The install command and the output path become <version>/<arch>. The example illustrating the '-' and '+' to '.' mapping keeps a concrete shape, since a placeholder alone would not show the transformation, but uses <x.y.z> rather than a real release so it cannot go stale either. |
||
|
fr34akyandJohnathan Corgan
|
a1c0cd42f5 |
fix(transport): make the invalid-hostname test independent of the search domain
test_resolve_socket_addr_invalid asserts that nonexistent.invalid does not resolve. On a host whose /etc/resolv.conf search domain has a wildcard A record, it does: libc appends the search domain, the wildcard answers for nonexistent.invalid.<domain>, and the assertion inverts. The test then fails on that host and nowhere else, which reads as a flake. The name is now written absolute, with a trailing dot, so search-list expansion never applies and the reserved .invalid TLD returns NXDOMAIN from the root wherever the test runs. Verified on a host that reproduced the failure: fails before, passes after. |
||
|
Johnathan Corgan
|
d5e4533c1e |
fix(testing): make the chaos veth restore and the iface-binding suite hold on a loaded CI host
ethernet-churn failed on master and next alike, as a tree that did not converge. The daemon was not the cause: the harness left ring links down and reported them restored, and under host load those dead links lined up until every link was down at once. Finding that turned up several more harness defects, fixed together here. The iface-binding suite built its host veth names from FIPS_CI_NAME_SUFFIX, and an interface name gets fifteen characters. On a runner that sets the suffix to a timestamp and a pid, ip(8) refused the name before the first pair existed. GitHub's job does not set the suffix, so it passed there. The names now use the four-hex-character token from sim.naming, as the chaos simulation and the NAT topology script already do, and the reaper in ci-cleanup.sh matches the new shape under both the scoped and the unscoped sweep. A churned node's restart recreates each veth pair it shared with its neighbours. A stopped container's network namespace can outlive the stop by about two minutes, and while it does, renaming the survivor's new end fails with "File exists". The harness ignored that, read the old interface's MAC, logged success, and left the new end down. The restore now deletes any interface holding the final or temporary name first, checks every add, move and rename, and waits for both ends to report operstate up. A restore that still fails raises as a harness fault. A container PID docker cannot report now raises instead of reading as "not running", and a pair is deferred only for a neighbour churn itself stopped. The survivor's end of a recreated link also gets its netem parameters back; before, that direction ran unshaped. The runner hands one down-node set to every manager, and node churn and traffic stored it as `down_nodes or set()`. The set is empty when they are built, so each kept a private copy: traffic started iperf3 on stopped containers, and netem and link flaps tried to shape them. Every manager and event schedule drew from one random stream in wall-clock order, so host load changed which node churn stopped next. Each consumer now has its own stream derived from the seed. The topology and ephemeral node choice stay on the seed's own stream, so generated topologies do not change, but every other runtime draw does. The final tree snapshot now waits for three consecutive agreeing reads, five seconds apart and bounded at ninety seconds, instead of being taken the moment the stopped nodes were restored. A red chaos scenario lost its results directory with the worktree the CI worker deletes. Each scenario's results are now scoped to the run, and a red prints its status, assertions, final tree and each node's log tail into the run log. ethernet-churn's baseline had been calibrated on the broken restore. On the fixed harness, sixteen runs across master-line and next-line code, twelve of them under contention and across three seeds, all ended with 4 nodes answering, 1 root and 3 parented, and the scenario now asserts exactly that. The scenario loader also checked the parented floor against one root only; it now checks it against max_roots, since a mesh with R roots can parent at most n - R nodes. |
||
 ArjenandJohnathan Corgan
|
a354501514 |
refactor(transport): lift presence out of the ethernet module
The presence machinery was written inside `transport::ethernet` because that is
where it was needed first, not because it belongs to ethernet. `presence.rs`
imports nothing but `std` — its only `super::` references were a doc link and
its own test module's `use super::*`. It is a standalone state machine that
happened to live in a transport's directory.
So this is a move, not a generalisation. `PresenceState`, `Presence`,
`AbsencePolicy`, `ChurnGuard` and `bind_backoff` go up to `transport::presence`;
ethernet re-exports what it used to own, and the duplicate `watcher.rs` is
deleted in favour of the shared one this branch now starts from.
What deliberately does *not* move is the binder loop. `BinderContext` holds an
`EthernetConfig`, a `NeighborBuffer`, `EthernetStats` and a pubkey for beacons,
and `bind_now` spawns the ethernet receive loop and beacon sender. Making that
generic needs associated types or `dyn`, and every implementer would still
supply its own bind-and-spawn body — abstracting thirty lines of control flow
while leaving two hundred lines of substance per transport, with an indirection
sitting between the reader and the platform-specific unsafe. The shared part
really is just the state and the decisions.
The line matters because ethernet is not the only transport bound to something
that can disappear. BLE binds an HCI adapter — a USB dongle that unplugs,
rfkills, or resets — and today has no presence handling at all: a missing
`hci0` at start returns an error and the transport is skipped for the life of
the process, which is precisely the boot race this branch exists to fix. A
future BLE binder writes its own loop and reuses the state machine, which is
the half that took a review to get right: the churn damping, the
announce/retract pairing, the episode clock, the detach classification.
The move itself is behaviour-neutral: `presence.rs` changes by a single
doc-link line and nothing else. The one suite change is the deletion of the
branch's duplicate `src/transport/ethernet/watcher.rs`, which takes its three
tests with it. `watcher_constructs_and_reports_its_backing`,
`a_persistently_failing_source_gives_up_instead_of_spinning` and
`a_sourceless_watcher_never_fires` are byte-identical to three of the five in
the shared `src/transport/watcher.rs`, which this commit does not touch, so
what goes is three duplicates and no coverage.
The module is narrowed to `pub(crate)` while it is being moved. `pub` here was
inherited from `ethernet::presence` rather than chosen, nothing outside the
crate names it, and it matches what `transport::watcher` already is. The two
types an embedder could reach stay reachable: `ethernet` re-exports
`AbsencePolicy` and `Presence`, so `transport::ethernet::{AbsencePolicy,
Presence}` is unchanged. Everything else under the module — `PresenceState`,
`ChurnGuard`, `BindOutcome`, `DetachOutcome`, `bind_backoff` and the three
constants — becomes crate-only, which is a public-surface narrowing riding
inside a move and is named here for that reason. It carries ethernet's target
gate, since ethernet is its only consumer and a crate-private module with no
consumer is dead code on a target where ethernet is cfg'd out.
|
||
|
ArjenandJohnathan Corgan
|
40b24cc9df |
chore(openwrt): ship the radio transports enabled and optional
The mesh0/mesh1 and ap0/ap1 Ethernet transports shipped commented out, and fips-mesh-setup / fips-ap-setup awk-toggled the comment prefix in fips.yaml when they created or removed an interface. That existed for one reason: a transport whose interface was missing at startup was skipped and never retried, so a stock install that never ran the helpers would have logged a bind warning every boot. The daemon now waits for the interface and binds it when it appears, so the toggling has nothing left to protect. The blocks ship enabled with optional: true — which is the honest statement about a radio the router may never configure — and the helpers create the interface and stop there. No config rewrite, and no "restart fips AFTER the interface is up" step anywhere in either procedure. phy0-sta0 (wwan) gets optional: true for the same reason: it only exists while a radio is in station mode. eth0 and br-lan stay required, and the test pins that they do — marking the whole ethernet block optional would silence exactly the failures this policy exists to surface. With presence on IFF_UP rather than IFF_UP|IFF_RUNNING, that stays correct for a router with nothing plugged into its LAN ports: absence now means the netdev is gone or admin-down, a real fault, rather than an empty switch port. fips-ap-setup still edits node.rendezvous.lan, and that one does still need a restart: it is a config value, not an interface. The changelog entry gains an upgrade note. fips.yaml is a package conffile, so a router whose setup script had already uncommented a block keeps that block untouched and never receives the new key; with optional defaulting to false the block is required, and an absent interface there stays Degraded and errors once at ten seconds where the shipped file is silent. |
||
|
ArjenandJohnathan Corgan
|
062efd8045 |
feat(fipstop): surface interface presence in the transports view
The observability work shipped as JSON only, which left the operator's live
view saying `up` for a transport bound to nothing. That is the exact shape
of the bug the whole mechanism exists to end: the original OpenWrt failure
was expensive because the 802.11s link formed regardless, so nothing anyone
could see said the node was deaf. Putting the data in show_transports and
not in fipstop reproduced that at one remove.
The list gains real columns. Instance name and the thing a transport is
bound to are separate facts about separate columns, so they get separate
columns: `Bound to` answers one question with different answers per
transport type — a netdev for the interface-bound ones, the bound socket
address for UDP and TCP, a truncated onion for Tor, a remote MAC for a link
row. Packed into one label they left the netdev names ragged down the list,
and that is the column an operator scans to find the interface they are
looking for.
The State column carries presence for an interface-bound transport rather
than the lifecycle state. `up` is true from the moment the transport starts
and stays true while its interface is missing, so it is precisely the wrong
answer in the one case someone is scanning that column for; there is no room
to show both and only one of them is news.
A Policy column reads required or optional, and severity follows it: an
absent interface whose absence is normal — a dock adapter that is not
plugged in, a radio this board never had — is yellow, while an absent
interface the config says to expect is red. That is the same split the
daemon makes between staying Full and reporting Degraded; painting both red
would train the operator to ignore red.
Ordering is no longer arbitrary. show_transports iterated a HashMap, so the
array order was whatever the hash seed produced, different on every daemon
restart. Both render sites sort by ascending transport id, which is creation
order, so the list groups by transport type for free — and it fixes fipsctl
output as well as the view, which sorting in fipstop alone would not have.
The identifying columns sit at the left. The first column was Min, so it
absorbed every spare column of a wide terminal and shoved Instance and
Bound-to into the middle, away from the names being scanned. It is
fixed-width now, sized for the widest label that actually lives in it — a
link's tree glyph and direction — with Peer taking the slack.
The detail pane gains an Interface block: netdev, presence and how long it
has been held, carrier, absence policy spelled out as its consequence rather
than its config key, bind count (flagged when it has rebound) and failed
binds when there are any. Carrier is spelled out because presence is IFF_UP
— a bound interface with no carrier is a bridge with nothing plugged into
it, which is normal and should not have to be inferred from silence — and
the two counters separate an interface that is flapping from one that is
there and refusing to bind.
test(control): pin the show_transports interface block
docs/reference/control-socket.md states the schema of each query response is
pinned by the snapshots in src/control/snapshots/. For the interface block it
was not: show_transports.json is `{"transports": []}`, because build_test_node
keeps every runtime list empty, so the only thing pinned was the empty form.
show_routing.json was regenerated for req_own_loopback in the same series,
which is what makes the omission look accidental rather than considered.
The block is emitted by two hand-duplicated sites — the live handler and the
read-handle variant — that agree today with nothing enforcing it.
A separate node and a separate snapshot rather than a richer build_test_node,
so the nineteen existing snapshots keep the empty-state determinism they were
built for. The fixture starts a real transport on an interface no host has, so
presence is deterministically absent and carrier deterministically false
everywhere this runs, and the captured shape is the one an operator actually
meets: state `up` with the interface absent. That pairing is the whole reason
the block exists, so it is worth having a committed artifact that shows it.
since_secs joins VOLATILE_KEYS — it is elapsed time, so redaction pins the
key's presence without pinning a value that changes between runs.
insert_transport_for_test mirrors isolate_peer_acl_for_test: the snapshot
tests live in crate::control and cannot reach Node's private transports map,
and a narrow hook keeps the fixture honest rather than hand-authoring JSON
that nothing in the daemon produces.
fix(fipstop): make the transports table fit an 80-column terminal
The table's fixed columns sum to 90, plus six single-column gaps: 96 against
the ~77 usable inside an 80-column terminal's border and scrollbar. Ratatui
resolves an over-subscribed layout by shrinking every column proportionally,
so the overflow does not clip the rightmost column — it clips all of them, and
`mesh0 (optional)` rendered as `mesh0 (optio`. The marker is the one thing on
that row worth reading, and the absence of it is what says `required`.
80x24 is the OpenWrt serial console and the xterm/tmux default, so this is the
deployment target rather than an edge case.
Below 100 columns the table drops Tx and Rx and sizes the rest down. Those two
are the only columns whose absence costs nothing an operator is scanning this
table to find — they are byte counters, and the detail pane carries them in
full — while Instance keeps 18 because `mesh0 (optional)` is 16, and Bound-to
keeps 17 because that is a full MAC. Both row shapes carry the same seven
cells in the same order, so the narrow variant is the same list with its tail
cut, keeping one place where the column count is decided rather than two that
have to agree.
The detail view stacks instead of splitting below 110 columns. A 40% split of
an 80-column terminal leaves the table 32 columns for a layout needing 65 even
narrow, and ratatui spends all of it on the trailing columns — rendering
Transport, Instance, Bound-to and State at width zero, so the operator gets
blank rows. Stacking keeps both panes readable instead of both unreadable.
The regression test renders at 80 and asserts the marker. That is precisely
the gap that let this through: the existing test asserting `mesh0 (optional)`
renders at 110, and the one rendering at 80 asserted only the netdev name, so
between them neither covered the width where the layout breaks. Verified
against the defect — with the narrow tier disabled, the new test fails and the
other two still pass.
The insert_transport_for_test hook now lands here rather than two commits
earlier. Nothing called it before this commit's snapshot fixture, so the
commits in between failed cargo clippy --all-targets -- -D warnings on dead
code. It also carries the target gate its only caller has, so the definition
and the call are present on the same platforms.
Changelog entry for the view.
Document the view in docs/reference/cli-fipstop.md. The reference page
described the Transports tab as a tree of instances with per-link children and
said nothing about an interface block, so the page and the tab disagreed the
moment this commit landed. The tab-table row now points at a new Interface
block section covering Interface, Presence, Carrier, Bound to, On absence and
the two bind counters.
|
||
|
ArjenandJohnathan Corgan
|
fdc127e75f |
fix(peering): stop re-dialling an active peer on an alternate path
A peer reachable over two interfaces was re-dialled on whichever path it was not currently using, once per discovery tick, forever. Each dial that completed promoted and displaced the incumbent, so the peer's link migrated back and forth on a fixed cadence and tore down its session each time. That is the ordinary result of two machines sharing a LAN and a cable: each beacons on both, so each discovers the other twice. Measured on real hardware — seventeen dials to one peer in fifteen minutes, alternating wifi and cable, displacing a link reporting etx 1.0 and loss 0.0. When the peer is the parent, which the best path usually is, every migration also switched parents, invalidated the downstream coordinate cache and re-announced to every peer, so the cost was mesh-wide while the benefit was nil. The gate already existed and already said it was for this: "skip a candidate whose path is already the current, still-fresh one (avoid churning a healthy link)". It only ever matched the *same* path, so it covered exactly the case that could not churn anything, and the alternate path — the only one that could — went straight through. Liveness is the right question, not the candidate's path: a live link should not be replaced by any path, and a dead one should be replaced by whichever answers. `active_peer_link_is_live` replaces the candidate-matching wrapper at the discovery call site accordingly. Failover is unaffected. A peer that stops answering goes stale within a heartbeat interval and every path, alternate included, is dialled again. What is given up is switching away from a link that is working, which is not worth doing. The configured-peer refresh is deliberately untouched: it prefers alternative addresses on purpose, for NAT and multi-address peers, and that is a different question from beacon discovery re-dialling a peer already on the wire. One consequence goes with the change, named here rather than left silent. A peer held on an adopted NAT-traversal transport was re-dialled by any Ethernet or BLE beacon that named it, because such a beacon can never match that peer's current path: the addresses cannot be equal (a six-byte MAC or BLE address against an ip:port) and the transport kinds differ. A peer that was both hole-punched and locally adjacent therefore drifted onto the local path on the next discovery tick. Liveness gates that too, so it now stays on the traversed path for as long as that path answers. Nothing else repeats the migration on a timer: adopt_established_traversal refuses a peer that is already connected, so it is the way on to a bootstrap transport and not the way off. What is left is the configured-peer refresh, which keeps its bootstrap carve-out and does perform the migration, and the traversed link going quiet, which reopens every path. Upgrading a live traversed link to a local one is worth having and belongs in a change that asks for it, not in the accident this one removes. Coverage is honest but partial. The liveness predicate is unit-tested in both directions, and the path-matching those tests used as a vehicle is retargeted onto the function that still uses it. The call-site change itself is NOT covered: I restored the old same-path-only behaviour and the suite stayed green, so the tests pin the predicate rather than the decision. Driving `poll_transport_discovery` needs two bound Ethernet transports and a seeded neighbour buffer; an absent transport fails the dial anyway, so link count cannot discriminate. Verification is the live daemon that produced the measurements above. a_bootstrap_held_peer_is_never_its_own_configured_candidate pins that remaining off-ramp, which had no test anywhere: with the bootstrap carve-out in active_peer_matches_candidate deleted, the peer's own traversal address compares equal on both the address and the transport kind, has_alternative goes false, and the configured-peer refresh can no longer move it. The two predicate tests are renamed to what they assert. Neither looks at a candidate or a transport any more, so "same_path" and "discovery" named things the bodies no longer touch, and the candidate each still bound was kept alive only by a `let _ =` suppression. Both suppressions go with the bindings. Changelog entry, including the traversal consequence above. |
||
|
ArjenandJohnathan Corgan
|
7c8cf01905 |
refactor(transport): classify send failures centrally
InterfaceUnavailable existed to make absence branchable, and then stopped
being branchable at the transport boundary: every non-MTU error was flattened
into NodeError::SendFailed { reason: format!(...) }, so no caller downstream
could tell a two-second interface flap from a permanent fault. Both got the
same treatment, which for a half-built handshake means being torn down and
filed as peer misbehaviour.
The classification belongs on the error rather than at each call site, and the
question worth asking is not what went wrong but whether waiting fixes it: a
transient failure was refused by a condition the daemon is already working to
resolve, so the state built around it — a half-finished handshake, a route, a
queued packet — is worth keeping. TransportError::is_transient answers that
once, and NodeError::SendUnavailable carries the answer across the node
boundary instead of discarding it.
Deliberately narrow: only InterfaceUnavailable. Timeout and ConnectionRefused
describe a remote that did not answer, which is a statement about the peer
rather than about this node's ability to transmit, and their retry paths sit
at a different layer. The test pins that narrowness in both directions,
because the failure mode of this abstraction is someone adding a variant to
the transient list and quietly making callers hold state open for a fault that
will never clear.
No behaviour change yet. This is the plumbing half; the callers that should
act on it — route withdrawal on detach, and not counting a local interface
flap as a handshake reject — are recorded in reference/ and deferred, because
both are routing changes that want their own test story.
feat(node): withdraw a transport's peers when its interface goes away
Losing an interface withdrew nothing. The peers stayed in the registry, the
routes through them stayed selectable, and this node kept advertising
reachability it no longer had — so transit traffic was dropped in silence and
other nodes kept routing toward us for those destinations, until the liveness
reaper noticed up to link_dead_timeout_secs (30 s) later.
Measured on real hardware: a dongle detached at 07:37:19 took the node's parent
with it, and no new parent was chosen until 07:37:46. Twenty-seven seconds
routing through a link that had already gone, with four alternative peers
available the whole time. The alternatives are the point — a mesh that can
route around a dead link should not be the last to hear the link is dead.
The detach edge is both earlier and more certain than inactivity, so it is the
better trigger. reap_peers_on_transport routes through the same
route_link_dead the liveness reaper uses rather than open-coding a second
teardown: every consequence of losing a peer — sessions, path MTU release,
session indices, decrypt-worker unregistration, the link, the control machine,
tree cleanup and re-announce, bloom withdrawal — already hangs off that one
path, and a parallel one would drift from it.
Not policy-filtered. Whether an interface's absence is normal is a statement
about node *health*; it says nothing about whether the routes over it still
work. An optional interface's peers are exactly as unreachable.
PathBroken needs no new wiring. Once the peers are gone resolve_next_hop
returns None, which takes the NoRoute path — and that one already synthesises
the routing error, rate limiting included. The cure for the silent drop was to
stop having a route, not to add a second error path.
Deliberately undamped. A flapping interface cannot drive a reap storm through
here: ChurnGuard suppresses `announce` after three short-lived bindings, which
leaves `announced` false, which makes `detached()` return `retract: false` —
so no presence edge is published at all during churn. The edges this reacts to
are already rate-limited at the source, reaping an already-reaped transport is
a no-op, and a second damper would only add a way for the two to disagree.
The trade taken: immediate reaping costs a re-peer for an absence shorter than
the dead timeout that then recovers — a `wifi reload` returns in ~5 s and today
costs nothing, where this costs ~15 s of re-peering. Accepted, because
black-holing is silent, poisons other nodes' routing and needs the full timeout
to clear, where a re-peer is bounded, visible and self-healing. A grace period
remains available if that proves wrong; reference/ records its shape.
The integration assertion is the one that proves the wiring rather than the
unit: link_dead_timeout_secs is left at its 30 s default, so a withdrawal
inside 15 s can only have come from the detach edge. Verified against the
defect — with the reap disabled, that assertion fails and every other case in
the suite still passes.
fix(node): keep a half-built link when msg2 hits a transient transport
A send refused because the interface is absent or mid-rebind was treated as a
failed handshake: the link was removed, the reverse-address entry dropped, the
session index freed, the control machine torn down, the queued PromoteToActive
aborted — and the whole thing recorded as
RejectReason::Handshake(HandshakeReject::BadState).
That counter means "the remote sent something invalid". A local interface flap
is not the remote's fault, and an operator reading the rejects would conclude
it was. The initiator, meanwhile, resends msg1 into a link that no longer
exists and has to rebuild from nothing.
The binder is already working to bring the interface back, so the half-built
link is now left exactly where it is for that resend to land on. Nothing leaks
by staying: an initiator that never resends leaves a stale connection, which
`check_timeouts` reaps at `handshake_timeout_secs` like every other abandoned
handshake. Only a genuinely terminal error still tears down.
This is the first consumer of `TransportError::is_transient`, which is what
the central classification was for — before it, the distinction did not
survive as far as this call site.
The rekey msg1 send site gets the severity half only. Its teardown was already
benign: it returns before `set_rekey_state`, so the cycle simply does not
start and is retried when rekey next comes due, with nothing torn down and
nothing charged to the peer. Only the `warn!` was wrong for a local,
self-clearing condition the presence machine has already reported.
The test drives a real absent Ethernet transport rather than a stub, so the
error under test is the one production raises, from the code path that raises
it. Verified against the defect: with the transient branch disabled, the link
is destroyed and the assertion fails.
The deferral gives the epoch-mismatch restart arm in handle_msg1 a third
outcome, and its post-promote debug_assert! did not admit it. That arm's
assertion required the machine to be Established or absent; a transient msg2
failure returns before PromoteToActive and leaves it registered at
Handshaking{ReceivedMsg1}, so a debug build panics there. The assertion is
widened to name that phase exactly, which keeps it red for any other state,
and a_transient_msg2_failure_on_the_restart_path_leaves_the_fresh_leg_pending
covers the arm the existing transient test does not reach. Three comments
around those two arms claimed a send failure always removes the machine; each
now names the transient case as well. Release builds were never affected: the
tail is gated on Established, so a deferred machine simply skips it.
The route_link_dead doc comment is put back on route_link_dead. Inserting
reap_peers_on_transport between the comment and the function it described left
both blocks running together, so rustdoc attached the whole thing to the new
function and route_link_dead lost its documentation. The restored text also
names the second caller this commit adds, and generalises the sentence about
where now_ms comes from, since both callers now hoist it once per batch.
The transport-layer design's grace-period paragraph is rewritten. It argued
that no linger timer was needed because peers survive a detach untouched and
the liveness reaper is the effective bound. The detach reap makes both halves
false, so the paragraph now states the trade it actually makes: peers go at
the edge, a half-built link is still held, and a short absence that recovers
costs a re-peer, which is preferred to silent black-holing.
Changelog entries for both user-visible halves.
The insert_transport_for_test helper is no longer added here. Nothing used it
until two commits later, so cargo clippy --all-targets -- -D warnings failed
on dead code at this point in the history; it now lands with its caller.
|
||
|
ArjenandJohnathan Corgan
|
95866b7c7c |
test(iface-binding): cover the presence machine end to end
Two daemons whose only transports are interface-bound, run against a veth pair the harness creates, downs, deletes and recreates underneath them. Asserts the boot race (a daemon whose only interface is missing starts, reports the transport absent and the node Degraded, rather than exiting on NoTransports or skipping the transport for the life of the process), the late attach and discovery over it, the flap in both directions, destroy-and-recreate, and that an optional interface which never appears never moves node health. Also the log policy, which is the half that is easy to regress silently: absence is logged once on the edge and not once per retry; a required interface still absent past the ten-second bring-up window errors exactly once, while the optional one — absent just as long — stays silent; and that error is not repeated on a schedule. The detach edge is checked not to error, guarded by how long detection actually took, so a slow runner skips the check rather than failing on the harness's own latency. The containers run FIPS_TEST_MODE=default, not chaos. The chaos entrypoint waits up to 30 s for every configured Ethernet interface before starting the daemon, which is precisely the workaround under test — the daemon has to do its own waiting here or the suite proves nothing. Host-namespace ip(8) runs in a short-lived privileged container sharing the host network and PID namespaces, for the reason chaos/sim/veth.py documents: on macOS the containers live in the Docker VM, so ip(8) run on the macOS host could never reach them. Chaos ethernet transports are marked optional: true. In that harness a neighbour's interface disappearing is the scenario, not a fault — node_churn stops a container, which destroys its netns and with it both ends of every veth it held, so a surviving node watches a required interface vanish for the 30-90 s the neighbour is down, once per churn event. Reporting that at error is right for a deployment and wrong for a harness that tears the interface down on purpose; the mesh-wide zero-ERROR ceiling would have failed on injected chaos rather than on a defect. test(iface-binding): cover an interface present before the daemon starts Every scenario in the suite created its interface after the daemons were already running — that ordering is the boot race the suite was written for. But it means both nodes could only ever reach Present through binder_loop, so the inline bind in start_async, which is the ordinary case on a booted router, had no end-to-end coverage at all. That is where the churn guard went unseeded and the first detach stopped reaching node health, and no existing case could reach it: they all detach from a binding the loop created, which seeds the guard as a side effect. Case (f) adds a third node whose single required interface exists before its daemon does. The gate is what buys that ordering — the harness needs a running container to have a netns to move a veth into, but the daemon must not start until after the move, so node-c comes up parked on a file and the harness releases it once the interface is in place. Then one detach, on a binding the loop did not create, and the node must degrade. Verified against the defect rather than only against the fix: with the guard seed reverted, cases (a) through (e) all still pass and (f) is the only failure. A regression test that has never been seen to fail is a claim, not a test. It also asserts the reverse edge, so Degraded stays a level rather than a latch on this path too. test(iface-binding): assert the fast path and the churn guard Three gaps, two of them in tests that existed and asserted nothing. **The netlink path was never asserted to be in use.** The 1 s poll is a complete fallback and covers every wait in the suite, so the whole thing passed with `open_link_socket()` hardcoded to Err — the fast path could have been dead for a release and no test would have said so. The binder reports which backing it got at startup, so case (g) asks it directly rather than inferring from timing the poll would also satisfy, and the unit test that used to write `let _ = w.is_event_driven();` now asserts it on Linux, where the source is an unprivileged `AF_NETLINK` socket and falling back is a real loss rather than a sandbox's prerogative. **Churn damping had no end-to-end coverage**, which now matters twice over: it bounds the recovery announcements, and since the detach edge withdraws peers it is also the only thing bounding how often that withdrawal fires. Every flap elsewhere in the suite is a single down/up with long settles either side — exactly the shape the damper ignores. Case (h) drives four bindings that each die inside `MIN_STABLE_BINDING`, asserts the guard engages, asserts it then *suppresses* rather than merely counting, and asserts it is not a latch. **`a_poisoned_binding_does_not_strand_the_transport` discarded its result.** `let _ = eth.binding.tasks_alive();` left the entire point unasserted: reading a poisoned lock as "alive" would have the binder believe a dead binding healthy and never rebind, and treating it as an error would strand the transport. `false` is what routes it back through detach and rebind, so say so. `a_stop_racing_a_bind_leaves_nothing_behind` now asserts the error *kind*. `bind_and_spawn` refuses at its presence probe long before the post-store shutdown check, so `is_err()` alone passed on absence and would still pass with that check deleted. The test keeps the coverage it genuinely has — stop raises the flag before teardown, teardown leaves no socket and no loops — and says plainly that the race it is named for needs a bind that succeeds, which needs privilege no unit test has. Both new cases were verified against the defect: with the netlink source forced to Err, (g) fails; with `CHURN_THRESHOLD` raised out of reach, (h) fails. Nothing else in the suite notices either. One case was attempted and removed rather than shipped: `"interface replaced"` cannot be produced deterministically, because the delete that changes an ifindex fires a netlink event the binder acts on within microseconds, so `gone` wins the race. It passed about one run in three. reference/notes.md records the measurement and the two approaches that could work. Also fixes a real bug in the harness: `grep -q` under `set -o pipefail` exits on its first match, `docker logs` takes SIGPIPE, and the pipeline reports failure even though the line was found. That cost two false failures before it was spotted; `log_count` reads the stream to the end. test(chaos): cover an Ethernet rebind under active traffic The one case dynamic interface binding had no coverage for anywhere: a datagram crossing an Ethernet link while the interface underneath it goes away and comes back. No existing scenario could reach it, for two separate reasons. `ethernet-only` and `ethernet-mesh` both run with `traffic.enabled: false`, so no datagram crosses an Ethernet link in any test — `ethernet-only`'s own comment says exactly that, and names framing, the length field that trims NIC minimum-frame padding, and AEAD over Ethernet as unexercised because of it. And `link_flaps` cannot produce a rebind whatever it is pointed at: it simulates a down link with netem 100% loss, so the interface stays IFF_UP and the presence machine never sees an edge. `ethernet-mesh` has had link flaps enabled all along without once exercising a rebind. `node_churn` is what actually moves an interface. Stopping a container destroys its network namespace, deleting every veth in it — and deleting one end of a veth deletes its peer — so a *surviving* node watches its Ethernet interface disappear outright, and watches it return when the harness recreates the pair on restart. That is a real detach and a real rebind, driven from outside the daemon. The new scenario is a 4-node Ethernet ring with traffic on and one node churned at a time, with link flaps deliberately off so the only outage is a genuine interface removal and a traffic shortfall cannot be ambiguous between the two. Measured across four runs: 206-388 MB moved over Ethernet links while interfaces were being taken away underneath. It also needed an assertion that did not exist. Traffic results have always been written to `iperf3-results.json` and never read, so a scenario carrying `traffic.enabled: true` could have every session fail and still exit 0 on a green control plane — and a rebind under load is precisely what a tree snapshot cannot see. `min_traffic` counts sessions that finished with bytes actually received, treating iperf3's top-level `error` and a missing `end` block as zero, so a session only counts when it moved data. The baseline is calibrated against four runs rather than assumed: `max_roots` starts at the observed maximum plus one, and the site records the sample, its size, and why four runs is thin. The first draft asserted a single root and failed every run — the harness restores stopped nodes immediately before the final snapshot, so a just-restarted node has not re-parented yet and is briefly its own root. That is the scenario working. Wired into both runners, since a chaos scenario on one side only makes "local green" and "GitHub green" stop meaning the same thing; check-ci-parity was confirmed to fail on a one-sided addition before this was committed. The iface-binding suite's entry in the GitHub workflow's integration matrix moves here from the commit that introduced the presence machine. That commit declared the suite on GitHub before testing/iface-binding/ existed and before testing/ci-local.sh knew about it, so testing/check-ci-parity.sh failed there and the three workflow steps named files that were not yet in the tree. Registering both runners in the commit that adds the suite settles both. |
||
|
ArjenandJohnathan Corgan
|
abe3f0b40a |
feat(transport): bind and rebind network interfaces dynamically
An interface-bound transport is a long-lived object that is *sometimes bound*. The interface it names may not exist when the daemon starts, may appear minutes later, may vanish and return mid-operation, and may never appear at all. Until now the first observation was final: an interface missing at start was logged once and skipped for the life of the process, and one that disappeared at runtime published a health change but was never rebound. On OpenWrt that is a live bug. procd starts fips before wifi has created fips-mesh0 / fips-ap0, both transports are skipped and never retried, and the 802.11s peer link forms anyway (that is mac80211, not the daemon) — so the node looks healthy and reaches nothing. Give every interface-bound transport presence state and a binder task. start_async now returns Ok with the transport ABSENT rather than failing; the binder binds when the interface appears, tears the socket down when it goes away, and rebinds when it returns. Start-time absence and runtime detach are one transition and one code path. TransportError::InterfaceUnavailable makes absence branchable: a missing interface and a typo'd interface name were the same flat StartFailed (String), so nothing downstream could tell a state from a fault. A bind failure that is *not* absence — no CAP_NET_RAW, no readable /dev/bpf* — still fails the start, because retrying a socket that can never open behind a Degraded nobody is watching is worse than dying loudly at boot. **Presence is IFF_UP, not IFF_UP|IFF_RUNNING.** Carrier and bindability are different questions and only the second belongs in a bind gate. An AF_PACKET socket on a carrier-less bridge is valid and starts carrying traffic the instant a member port comes up, with no rebind. Gating on carrier would report a wifi-only router Degraded forever for an empty br-lan, turn every carrier flap the socket would have survived into an unbind/rebind cycle, and deadlock an 802.11s interface that reports RUNNING only once it has peered — peering needs beacons, beacons need a bound socket. Carrier is reported beside presence in show_transports instead. **A name is not a device.** Both backends bind by device: AF_PACKET stores sll_ifindex, a BPF descriptor follows the interface it was attached to. A netdev deleted and recreated under the same name leaves the socket attached to something gone while the name resolves perfectly well, and nothing notices — a stale AF_PACKET socket never becomes readable, so the receive loop neither errors nor exits, and send failures go to the caller rather than the binder. A listen-only node (announce: false, no beacon sender to fail) sat present and deaf indefinitely after a wifi reload. The bound index is captured at bind and compared on every poll. A name reappearing with a different MAC is different hardware: cached neighbors are dropped rather than resumed onto. **Detection is event-driven** where the kernel offers a source — netlink RTNLGRP_LINK on Linux, PF_ROUTE on macOS and FreeBSD — with a 1 s getifaddrs poll underneath as a backstop. Link-event payloads are not parsed: an event is a hint to re-run the probe, which is cheap and authoritative, and a parser's bugs would be presence bugs. Probes are coalesced to ten a second because PF_ROUTE has no group filter and delivers every routing message on the host; a persistently failing event source is logged once, backed off, and abandoned for the poll after five errors rather than spinning a core silently on ENOBUFS. **Degraded becomes a level, not a latch.** The supervisor's reason set was monotonic, which was correct while no child could recover; with recovery it would have come to mean "something broke at some point since boot" rather than "something is broken now". Absence lives in its own reversible set, health is recomputed on every transition in both directions, and an absent transport still counts as up so a single-interface node that boots before its wifi degrades rather than exiting on NoTransports. There is deliberately no restart action in the FSM: rebinding belongs next to the file descriptor, and a supervisor-authored retry would be a second mechanism racing the first for the same socket. **The node's egress MTU follows the bound set.** transport_mtu filters on is_bound(), not is_operational() — an interface-bound transport is operational from the moment it starts, so the weaker predicate let hardware that had never appeared clamp the whole node's IPv6 MTU. Because that minimum now moves at runtime, the TUN reader and writer read the TCP MSS ceiling from a shared atomic instead of a u16 captured at spawn; every other consumer (show_status, the snapshot, the session-layer fragmentation check) already read it live, so the clamp was the one place the daemon could report one effective MTU and enforce another. It moves in both directions: a narrow interface appearing tightens it, its departure releases it. MSS is negotiated per connection, so a change binds connections opened after it. **Logging is per edge, never per attempt**, with one deadline after it. The edge itself is not an error — info at boot, warn on a runtime detach, since an interface bound a moment later is the ordinary case this mechanism exists to absorb and crying error at t=0 then "recovered" at t=0.2s is the failure the rule exists to prevent. Ten seconds is the whole grace: past it, absence is no longer a race against a radio or a container, so a required interface still missing is reported once at error. Start-time absence and a runtime detach share that one deadline, as they share everything else here. Said once, not repeated: how long an absence has lasted is state, published as interface.since_secs and as Degraded, and a monitor can threshold it per deployment rather than the daemon compiling a schedule in. Successful rebinds are damped. Backoff covers failed binds; the nastier case is binds that keep succeeding into a socket that dies moments later, which a receive loop giving up on a persistent error while the interface stays UP produces once per second forever. Consecutive bindings dying inside ten seconds back off on the 1 s → 30 s curve, and past three the binder stops announcing each bind as a recovery until one lasts. The receive loop backs off and exits on a dead socket instead of spinning on Err with a warn per iteration, and the ad-hoc ENXIO socket reopen in the beacon sender is gone: both hand recovery to the presence machine, one mechanism for every cause rather than one hack per symptom. Beacons pause while absent because the task simply does not exist then. transports.ethernet.*.optional (default false) decides how loudly absence is reported. Naming an interface is a statement that you expect it, so the default complains: Degraded, and the error at the deadline. optional: true is silent — info on the edge, no health impact, no error — for hardware legitimately not always there. It describes the interface's presence, not the transport's importance, and no value of it makes a missing interface fatal at startup. show_transports grows an `interface` block — presence, carrier, policy, how long the phase has been held, bind count, failed attempts. The original boot race was expensive precisely because nothing an operator could see said the node was deaf. ci: exercise the presence probe against musl and an address-less interface Two legs, both about the assumption the presence probe rests on. OpenWrt — the platform dynamic interface binding exists for — is musl, and musl reimplements getifaddrs independently of glibc. `interface_present` reads ifa_flags out of it, and the interfaces this feature exists for (fips-mesh0, fips-ap0) are unbridged with no IP address at all, which is exactly where getifaddrs implementations differ. Every other leg is glibc, so without this one the probe was asserted on a libc no test had ever run it against, on the target it was written for. Building for the musl target rather than inside an Alpine container keeps the leg to a cross-build plus a run, without a second toolchain image to maintain. The address-less interface is created on both this leg and the glibc one. Pinning the contract on both libcs is what turns "glibc and musl agree about getifaddrs" from an assumption into a checked fact, and makes the glibc leg fail first if glibc is the one that changes. Loopback cannot stand in: it has 127.0.0.1, so probing it asks whether getifaddrs works rather than whether it reports this. Deliberately not tolerant of failure — a guard that quietly does not run is worse than no guard. The iface-binding suite is deliberately not added to the integration matrix here. Its files and its testing/ci-local.sh registration arrive in the next commit, and the GitHub workflow's matrix entry now goes with them, so both runners gain the suite together and testing/check-ci-parity.sh holds at every point in this history. docs: document dynamic interface binding The transport-layer design gains an Interface Presence section: the three deployment scenarios that motivated it, the presence machine and its two invariants, why presence is IFF_UP and not IFF_UP|IFF_RUNNING, interface identity and the recreated-netdev case, the detection sources and their two rate limits, the optional policy, health as a level rather than a latch, the egress-MTU consequence, the logging rules, and what the mechanism retires. It records why the error deadline is a single ten-second window rather than a repeating severity ladder, so the ladder does not come back. The control-socket and fipsctl references described show_transports without its `interface` block, and the transport-layer state machine still implied that Up meant bound. Both now say what Up and presence each describe and how they differ, since "transport up, interface absent" is a normal state an operator will meet and would otherwise read as a contradiction. The configuration reference documents `optional` with the absence, log and retry behaviour of each setting, and the ground-up tutorial's dongle example gains optional: true, which is what that example is actually for. fix(transport): pair every presence edge the binder publishes Six defects, all in the wiring around the presence machine rather than in the machine itself. The state machines were well tested; what went untested was how the binder feeds them, and every one of these lives there. **The first detach after a clean start never reached node health.** start_async binds inline and publishes that edge itself, before binder_loop exists. The loop then built a fresh ChurnGuard, which had therefore recorded no bind — and detached() takes `announced` to decide whether an edge is owed, so it asked for no retraction. A node that booted with its interface present and then lost it kept reporting Full for as long as the interface stayed away, while show_transports said absent and the log said detached. stabilized() could not repair it either: it early-returns while bound_at is None, which it is for a bind the loop did not perform, so the guard stayed unseeded until a second detach happened to fix it. The loop now seeds the guard from the binding it inherits. This is not the cable-unplug case. Presence is IFF_UP, so a carrier loss is correctly not a detach at all; it takes an admin down, a netdev delete, or a device removal — a wifi reload, a hostapd restart, a dongle pulled. **An optional interface published nothing, and the MTU floor rode the same channel.** Filtering the edge at the source conflated two questions that happen to share a transport: whether node health should move, and whether the bound set changed. Only the first is policy. The second determines the node's egress MTU floor, and refresh_tun_mss_ceiling has no other trigger — so an optional transport binding or unbinding at runtime left the TUN MSS clamp derived from a bound set that no longer existed, reporting one effective MTU and clamping to another. That is the defect the MssCeiling work was written to remove, reintroduced for exactly the transports the shipped OpenWrt config marks optional: five of seven. TransportPresence gains health_relevant; the edge always goes out and only health is filtered. **A permanent bind fault went unlogged if any absence race preceded it.** record_attempt ran ahead of the InterfaceUnavailable arm, so a probe that won a race the bind then lost burned the counter that gates the only error a real fault ever gets — emitted on attempts == 1. A flapping interface followed by CAP_NET_RAW being dropped or /dev/bpf* exhausting therefore reported nothing at all, for the life of the process, and the operator got report_sustained_absence's "still missing" instead: wrong, for an interface that is present. An absence race is not a bind attempt and no longer counts as one. **The watcher's give-up did not stick.** Abandoning the event source parked on pending() *inside the changed() future*, and that future is constructed fresh on every pass of the binder's select! and dropped whenever the poll ticker wins. So the next pass re-read the dead socket, re-counted the error and re-logged "not recoverable" — once per wake-up, forever, which at a 1 s tick across the shipped seven-transport config is seven warnings and seven failing syscalls a second on flash-backed logging. The 100 ms backoff lived in the dropped future too and never applied across passes. Abandonment is now state on the watcher. **A zero-length read livelocked the binder.** try_io clears readiness only on WouldBlock, which the sibling error arm handles by hand and this one did not — so breaking out left readable() instantly ready with nothing to read, and the loop never returned Pending. That starves the select! of its ticker entirely and takes presence detection down with it. Cleared and treated as a fault so the give-up path applies. **A failed presence probe read as an absent interface.** getifaddrs is a netlink dump and fails for reasons that have nothing to do with the interface: ENOBUFS under memory pressure, EMFILE or ENFILE under fd exhaustion, since it opens a socket of its own. Answering "not present" there tore down a working socket and degraded health over a transient syscall failure, undiagnosably, and under fd exhaustion the rebind could not have succeeded anyway. interface_has_flags now distinguishes the two; the detach gate holds its binding when the kernel will not answer, while the bind gate still treats it as absence and retries. **On macOS the reader thread could not die, so a dead socket read as live.** Any read() failure other than EBADF — ENXIO being the one that matters, which is what BPF answers once the interface it was attached to is torn away — reset the parse buffer and continued. The thread never returned, so the channel never closed, so recv_from never failed, so the tokio task never exited, so tasks_alive() reported a dead socket as a live one. Detach detection on macOS reduced to the name and the index, and an interface reset in place left the transport present and deaf. It now gives up after a streak, and the return closes the channel the binder is actually watching. Two smaller pairings while here: stop_async retracts the edge it would otherwise leave standing for a socket that is gone, and start_async hands a refused edge to the binder to retry rather than dropping the only edge either consumer will see until the interface next moves. The absence deadline is stamped at start rather than at construction, so a transport staged for longer than the bring-up window no longer reports sustained absence on its first tick having given the interface no window at all. feat(config): reject impossible ethernet interface names at load Config::validate never inspected transports.ethernet, so an empty interface name, a name past the kernel's 15-byte limit, and two transports naming the same netdev all loaded cleanly and failed only at runtime — the first two as a permanent absence indistinguishable from an interface that has not been created yet. That indistinguishability is deliberate and worth keeping: waiting is the right answer for an interface the operator has not made yet, and the daemon cannot know which of the two it is looking at. Which is exactly why the syntactic gate earns its place. A name that is *impossible* is the one case still separable from "not there yet", and without the check a typo costs a permanently Degraded node whose only symptom is an interface that never arrives — the failure mode the presence machine exists to make legible, reintroduced one level up. Syntax only. Whether a well-formed name exists stays the binder's question, asked once a second, forever. The duplicate check is a different fault: two transports on one netdev means two sockets on the same device at the same ethertype, each receiving every frame the other does. test(transport): close the vacuous and uncovered branches Six tests that asserted nothing, or asserted less than they claimed. `a_bind_fault_still_fails_the_start` returned early whenever the socket *could* be opened, so it was vacuous as root and on any developer machine with a group-readable /dev/bpf* — the fail-fast path it is named for went unchecked exactly where someone was most likely to run it. It now asserts in both halves, and the privileged half is worth more than the fix: a present, bindable interface binding inline is the ordinary case on a booted router, and no other unit test reaches it, because every other one here names an interface that does not exist. The bind-success path had no unit coverage at all. `an_interface_with_no_addresses_is_still_present` returned early when the fixture was absent — no fixture, pass. It still has to skip on a machine with no address-less interface, so the guard is the runner declaring that it has fixtures: CI now sets FIPS_TEST_REQUIRE_FIXTURES beside FIPS_TEST_ADDRLESS_IFACE, and the test fails rather than skips if the fixture step is ever removed or renamed. Its `let _ = interface_carrier(...)` is now asserted too: if presence and carrier ever collapsed into one read, a carrier-less bridge would report absent and the whole IFF_UP-not-IFF_RUNNING decision would be silently undone. `policy_labels` checked `Required.as_str()` and not `Optional.as_str()`, so a swapped pair would paint every expected interface as the tolerated kind and stay green. `Presence::as_str` had no test at all — `binding` was never observed by anything, anywhere. Three binder branches had no coverage: a transport restarting (the second `start_async` clearing the previous run's stop flag — only a second *stop* was tested, so a transport that could never restart passed everything), the episode clock being restamped at start rather than at construction, and the refused-edge retry actually delivering. The last one matters most: the existing test filled the channel and dropped the receiver, so a slot that captured an edge and never re-sent it would pass while health sat on a stale level forever. And the hardware-change boundary `record_bind` returns, which the neighbour flush hangs off: false on a first bind (or every clean start would drop a cache it had just built) and true once, not stickily, on a MAC change. Each new test was verified against the defect it guards — comment out the `shutdown.store(false)`, the `mark_starting()`, or the seeded `unpublished`, and the corresponding test goes red while the rest stay green. fix(test): correct the dummy-carrier assertion, and pin Darwin's presence probe The carrier assertion added in |
||
|
Johnathan Corgan
|
268e529125 |
Give the connected-socket test helper one home
Carrying the in-line decrypt tests up brought a second copy of install_connected_udp with them: netmon's tests already had one, and the two differed only in which local address they bound. Both now call one helper alongside make_node, on the netmon copy's wildcard bind, so the suite that was already passing keeps the socket it was passing with. |
||
|
Johnathan Corgan
|
f7df7c2a89 |
Merge maint into master, keeping master's rotation-fix text
Both lines had grown the same fix: bind the return of set_current_addr on the in-line decrypt path and drop the peer's connected UDP socket when the address actually changed. The code is functionally identical, so the conflict resolves to master's side, whose comment also explains what netmon's first-sight rule now rests on. maint's changelog paragraph for the same fix is dropped for the same reason: master already documents it. What the merge is actually for is the two tests maint grew alongside it. They drive handle_encrypted_frame directly, in both directions, which master's nearest tests do not: those exercise the medium-change path. The chacha20 lockfile move comes across with it. |
||
|
Johnathan Corgan
|
195fe1e700 |
build(deps): move chacha20 off the yanked 0.10.1
chacha20 0.10.1 is yanked on crates.io. It reaches this tree through rand, which is a direct dependency, so it sits on the built path rather than off to one side. The requirement in Cargo.toml already admitted 0.10.2, so this is a lockfile change and nothing else moved: the diff is the chacha20 package entry and rand's reference to it. This is not a security fix. cargo audit reports nothing against chacha20 at either version, and 0.10.1 was withdrawn by its maintainer rather than flagged by an advisory. What the move buys is that a fresh checkout resolves without reaching for a yanked version. Gated on the branch: fmt, build, clippy with -D warnings and the library tests all pass, 2314 tests and 0 failed, and the six repo guards exit 0. |
||
|
Johnathan Corgan
|
ecd1debebe |
fix(dataplane): drop a peer's connected UDP socket when its address rotates
A per-peer connect(2)-ed UDP socket is pinned to one 5-tuple. When the peer moves, that address is gone, but the socket stays installed and the send path keeps preferring it over the wildcard listen socket. set_current_addr returns whether the address actually changed, and its doc says the return exists so callers can invalidate such a socket. Two callers sit in the same file and treated it differently. The decrypt-worker completion path binds the return and clears the socket on a rotation; the in-line decrypt path discarded it as a bare statement, so a peer that roamed through that path kept its stale socket. bool carries no must_use, so the discard was silent under -D warnings. Bind the return in handle_encrypted_frame and clear the connected socket on a rotation, with the same cfg gating and the same ordering as the sibling twenty lines below. The two tests cover the defect and the healthy path, and both were checked against the defect rather than only against the fix. Reverting the change fails the rotation test on its surviving-socket assertion, and making the clear unconditional fails the healthy-path test on its socket-still-installed assertion. maint carries no test module for the network-change handler, so the connected-socket helper is built here from the same three primitives the production activation path uses. How much real traffic reaches the in-line decrypt path rather than the worker path is not established. The fix rests on the defect itself and on the sibling path's existing treatment of the same flag, not on a measured impact. |
||
|
Johnathan Corgan
|
cdfe0ba369 |
Merge maint into master, keeping the tested heartbeat gate
The heartbeat-send accounting fix was authored on maint, and the branch that landed the per-peer medium-change probe carried its own fix for the same defect. They agree on behaviour: both record the attempt before the send and the delivery only on success, and both consult the retry floor only when the last attempt was newer than the last success. So this merge is a reconciliation of text, not a decision about behaviour. Four files conflicted and none of them on behaviour. The gate keeps maint's shape, a pure `heartbeat_due` function that can be tested without driving a send, replacing the inline block; the constant keeps master's documentation, including the note that the retry floor can come down to the tick now that the connection-oriented write is bounded. The peer accessors keep `pub(crate)` rather than `pub`, since the type is publicly re-exported and every caller is in-crate. The remaining two conflicts were documentation wording alone. The changelog had the same fix described twice, once under each line's heading. The entry written for this line is kept and maint's duplicate dropped, so the release notes describe the fix once. One hazard this merge does not announce: the heartbeat test file merged with no conflict marker while both sides had added the same `set_heartbeat_interval` helper in different places, which does not compile. One copy is removed here, keeping the doc line that names the knob the retry gate must not floor. The second defective send site, in the medium-change fan-out, does not exist on maint and is already fixed on this line. |
||
|
Johnathan Corgan
|
139f2af9b0 |
docs(netmon): describe the scoped reaction, and make two tests observe it
Two tests passed with the thing they exist to check removed, and seven places still described the reaction as node-wide. The exclusion test captured the peer's last-heartbeat-sent timestamp, fired a change, and asserted it had not moved. Splitting the fan-out into an attempt and a success made that assertion vacuous: delete the filter that excludes connection-oriented transports and the stream peer is selected, its send fails at the connection-readiness gate, the sent timestamp is untouched, and the assertion passes anyway. The attempt timestamp is the observation that sees the exclusion, because the fan-out writes it for every peer it picks, before any send. Asserting on that fails when the filter is removed. That test's own justification was also stale on this base: the write it called unbounded is now bounded by the writer task, so the filter is kept for a different reason, that widening the fan-out should be its own change with its own evidence. The test comment now says so, and says that widening it is the edit that would record the decision. The probe's bind address reached the sampler through three sites and no test touched any of them; substituting a null at either end left the suite green. The new test starts a UDP transport on a loopback address, pins a peer onto it with a numeric endpoint, publishes the snapshot, and asserts the probe target carries the transport's bind address. It needs no privileges and no route. Nulling either end fails it. On the prose: four places in the handler module plus two operator-facing pages still described the old node-wide reaction. One is the doc summary of a function whose own name and signature say it acts on the peers that moved. Another is an intra-doc link to a name the scoping renamed away, which resolves nowhere and which nothing catches, since there is no rustdoc gate. The pacing constant's rationale said the reaction drops every peer's socket. It drops the socket of each peer the change names; what makes the pacing argument hold is that a cleanly flapping interface is the worst case, because a medium change moves the whole table at once, so the scoped set is every peer anyway. The bound is unchanged and the argument is now exact. The test that pins the pacing carried the same sentence. The statement that nothing is left stranded by the scoping rested on one of the two ways a peer can be absent from the sample. The other is a peer whose transport address is not a numeric endpoint, which never reaches the sample at all while holding a connected socket. That is transient where the node dialled out, because the address is replaced by the observed numeric source on the first authentic frame, but it is a different argument from the one written. Which side supplies the address on an inbound peering is not established here, so the sentence does not claim the group is empty. The reference page also said the sampling cost is three syscalls per peer per sample, bounded by the peer limit. It is five, read off the sampling function rather than measured, and the bound does not hold when that limit is zero, which the configuration defines as unlimited. The upgrade note is the part with a consequence. The new cross-field check refuses startup when eight settling rounds of the debounce interval meet or exceed the liveness timeout. Detection is on by default, so a node that shortened its liveness timeout to one or two seconds for fast failover is refused after the upgrade, citing a key its operator never set. A timeout of zero is exempt. The changelog now says so, with the three ways out. |
||
|
Johnathan Corgan
|
d9be9c7013 |
fix(netmon): re-derive a dropped change instead of losing the peers it named
The detector advanced its baseline immediately before offering the change to the channel, and the channel holds one slot: when it is full the newer change is discarded with a debug line. That was sound while the reaction was node-wide, because any later change would rebind everything anyway, and the doc on the spawning function said so in as many words. Scoping the reaction to the peers a change names broke it. The handler now reads only that peer set, so a discarded change loses those peers, and because the baseline had already moved the ordinary diff compares the post-change sample against itself and never names them again. Nothing else repairs them: the session activation early-returns while a connected socket is present, and the send path prefers that socket, so the peer transmits from the abandoned source until the liveness timeout reaps the peering. The baseline now advances only on a send that was accepted. Holding it where it was makes the next sample re-derive the move, and repairs are idempotent, so re-reporting a peer that was already handled costs nothing and stops as soon as the queue drains. The alternative, restoring the previous baseline on a full channel, needs the map cloned on every reported change to serve the rare case, so it was not taken. The coalescing test could not see any of this: all three of its scripted fingerprints named the same peer, so it passed under either policy. It is rescripted over disjoint peer sets, and it now fails on the old ordering with a message that names the defect. Three prose corrections in the same module, all of them consequences of the same scoping change. The claim that the handler still re-evaluates every peer is now false and is replaced by what the handler does. The first-sight residual said the ordinary diff recovers the peer on the next sample, which it does not, because the socket's pinned source is read only on the first-sight arm; the bound is the liveness timeout, and the node-wide reaction is what used to repair it as collateral. And the sampling cost is five syscalls per target, read off the sampling function rather than measured; the earlier correction reached one site of three. |
||
|
fr34akyandJohnathan Corgan
|
10b4e6ea60 |
fix(node): scope the medium-change reaction to the peers that moved
The reaction dropped every peer's connected socket and heartbeated every connectionless peer, whatever the change actually named. That was defensible while the fingerprint was host-wide and could not say which peering had moved. Keying it on peers removed that excuse, and introduced a reason to care. `probe_target` is the observed source address of the last authentic packet a peer sent, updated with no throttle. So the trigger is now within reach of a remote party for the first time: a peer alternating between two of its own addresses that leave this host by different interfaces moves the fingerprint at will. Node-wide, that one peer could drive every other peering's socket teardown and heartbeat, repeatedly, bounded only by the poll interval — up to `max_peers` drain threads torn down and respawned per period. Scoped, the only peer in the set is the roamer itself, whose connected socket the data plane has already cleared on the address change. The lever closes by construction rather than by a rate limit. Nothing is left stranded by the narrowing. A peer absent from the set is one whose local source address the kernel still resolves to the same place, and that is the entire content of the fingerprint: a peer that did not move is a peer whose socket is not stale. To be clear about what this was worth: nothing black-holed before it. The sockets reinstall on a later tick and sends continue over the wildcard socket meanwhile, so the cost was internal teardown and respawn work rather than an outage. It is done here because this change is what created the lever, not because it was urgent. The test holds two peers, moves one, and asserts the other keeps both its socket and its heartbeat timestamp. The medium-change lab still passes 17/17, which is the end-to-end check that the peer whose route really did move is still in the set and still repaired. |
||
|
fr34akyandJohnathan Corgan
|
0264c9e275 |
fix(node): clear the connected socket when a peer's address rotates
`handle_encrypted_frame` updated the peer's current address and discarded the flag saying it had changed, so a peer that roamed kept its per-peer `connect()`-ed UDP socket pinned to the old 5-tuple and went on sending where it used to be. The decrypt-worker completion path already captures that flag and clears the socket for exactly this reason; this path did not. Pre-existing, and not something this branch touched — but the first-sight rule added here now rests on the invariant it breaks. That rule compares the socket's pinned source against a probe to the peer's *current* address, so a socket left behind after a roam makes those two disagree for as long as it survives, and the peer reports a move on first sight that nothing local caused. Fixed rather than documented as an assumption, because the stale socket is a defect on its own terms: it is aimed at an address the peer has left. |
||
|
fr34akyandJohnathan Corgan
|
c0aa7ebf53 |
fix(node): stop a failed heartbeat from suppressing the next one
Two send sites recorded a heartbeat as delivered before the send was attempted, and neither looked at what came back. `check_link_heartbeats` stamped the peer's "last heartbeat" timestamp and left it alone whatever the send returned, so a peer whose heartbeat could not go out was treated as heartbeated and was not tried again for a whole `node.heartbeat_interval_secs` — although it had heard nothing and its own `link_dead_timeout_secs` was already running. On a 10s interval against a 30s dead timeout, three failures in a row are the whole budget. The timestamp now moves only on a send that returned cleanly, and a separate record of the *attempt* spaces the retries, so a peer that keeps failing is retried in seconds rather than either hammered every tick or left for a full interval. The retry spacing is deliberately consulted on the failure path only: a successful send stamps both timestamps with the same instant, so gating the healthy path on it as well would have floored a configured `node.heartbeat_interval_secs` at the two-second retry interval, silently and with no validation refusing the value. An attempt strictly newer than the last success is the only state that means "the last one did not land". The retry floor is not shorter than two seconds because the send behind it awaits an unbounded `write_all` on a connection-oriented transport, on the rx loop; retrying that every tick would make a stranded stream a stalled node. The medium-change fan-out had the same defect and one of its own. It reported `heartbeated` as the number of peers it selected, before any of them was sent to. A medium change is exactly the condition under which sends start failing, so that count read identically whether every frame left or none did — the one number in the line an operator would use to tell those apart. It now counts sends that returned cleanly, and the peer's timestamp moves only for those, so a failed fan-out neither inflates the log nor suppresses the peer's next due heartbeat. Tests, each checked against the defect rather than only against the fix. The predicate is driven through the real sweep with elapsed time staged on the peer, because `check_link_heartbeats` reads `std::time::Instant`, which tokio's paused clock does not move. Deleting the retry gate, flipping the `&&` to `||`, and raising the constant to minutes each fail it. A healthy peer configured below the retry floor is asserted to keep the interval it configured, which the unconditional gate fails. The fan-out test drives a peer re-pinned onto a UDP transport that was never started: connectionless, so the fan-out selects it, and failing with `NotStarted` before touching a socket. Reverting either half fails it. |
||
|
fr34akyandJohnathan Corgan
|
47f8f01e2f |
fix(config): validate node.netmon.*
Two checks, both about the detector being able to do its job at all rather than taste in numbers. A zero `poll_interval_secs` was silently clamped to one second, so a typo produced a node polling twenty times more often than configured and saying nothing about it. It is now refused, and only while detection is enabled, since a disabled detector imposes no constraint on its own knobs. A `debounce_ms` whose worst case — the full `MAX_DEBOUNCE_ROUNDS` of settling — meets or exceeds `link_dead_timeout_secs` is refused too, because the liveness reaper would tear the peering down before the change was ever reported: the machinery would run and could not help. The constant is shared with the detector rather than restated, so the two cannot drift. Both multiplications saturate. The operands are operator-supplied `u64`s, and an overflow would wrap to a small number and silently accept the very configuration this refuses. Documented in the reference, because neither refusal was predictable from it. The debounce one couples two keys in different blocks, so an operator shortening `link_dead_timeout_secs` for fast failover can make an untouched `debounce_ms` illegal and meet a hard startup refusal citing a netmon key they never set. The multiplier its own error message uses is now on the page. A test asserts the shipped defaults still validate, which is the failure mode a cross-field check invites. Another asserts the refusal message carries no run of literal spaces: it is the whole diagnostic for the only refusal reachable by editing `debounce_ms`, substring assertions cannot see how it reads, and rustfmt does not touch string literals — a continuation join had already left twenty-two of them mid-sentence. |
||
|
fr34akyandJohnathan Corgan
|
957e05b283 |
feat(netmon): fingerprint the path to each peer, not the host
The medium-change detector sampled two host-wide signals: the source address the routing table would pick for an off-link destination, and the set of up, non-loopback interface addresses. The second one was the problem. It enumerated every address the host had, so a docker bridge coming up, a VPN connecting or a container network appearing moved the fingerprint with no peering affected at all — and the reaction to a moved fingerprint is to drop every connected UDP socket and heartbeat every peer. Self-healing, so it cost work rather than connectivity, but on any host running containers it could fire repeatedly for nothing. Ask the question per peer instead. For each peer whose transport address is a numeric IP endpoint, `connect(2)` a UDP socket to it and read back the local address — the same no-packets operation, aimed at the peers we actually hold rather than at a documentation prefix. The fingerprint becomes the set of local addresses the kernel would use to reach our peers. That is the quantity the reaction cares about. The stale `connect(2)` this subsystem exists to repair pinned a local source address chosen for one destination, so measuring the same thing for the same destinations asks the kernel the question the bug is about rather than a proxy for it. Three things follow: - Interfaces the node does not peer over cannot move it, by construction rather than by a filter guessing which interface names are infrastructure. `docker compose up` moves nothing. - On-link peers become visible. A peer on the same LAN is reached by its subnet route, and the old probe followed the *default* route by construction, so it looked straight past that path. - A more specific route moving under one peer is representable at all, which no single host-wide sample could be. Samples are compared over the *intersection* of their peer sets, never the union, so peers joining and leaving cannot fire the fan-out on their own. The sample is still adopted on the no-change path, or `last` would freeze on the peer set the detector started with. A peer whose probe stops answering is a move to "no route" and does count: that peer is exactly the one now stranded. **A peer's first sample is judged against its socket, not against nothing.** The intersection rule skips a peer present in only one sample, which is right for peer churn and wrong for the sample in which a peer first appears, because that sample may already be the post-change one. `last` gains a peer only at the first wake after it shows up in the entity snapshot, so a medium change inside that window is consumed rather than delayed: the peer's connected socket stays pinned to the path the host has just left, and the peering black-holes until the liveness timeout tears it down. A first-seen peer is therefore compared against the source its own connected socket is bound to, where it has one. One residual on that rule, stated exactly rather than understated: the tick publishes the entity snapshot *before* it installs connected sockets, so a socket installed on tick N is first visible on tick N+1, and a peer whose path moves inside that window is first seen with `bound` still `None` while genuinely holding a pinned socket. About one `tick_interval_secs` per join. A hole, not a harmless skip. **The probe binds the way the send path binds.** `open_connected_fd` binds `local_addr` verbatim before connecting, so the socket keeps a configured address whatever the route says, while the probe took the kernel's choice. Under a non-wildcard `bind_addr` the two answered different questions and every first-seen peer reported a phantom move. The probe now binds what the transport binds, address only and port 0; under the default wildcard bind nothing changes. Operator-facing corrections in the same surface. `PeerSourceMove::before` was recorded on every move and then wildcarded away by the only thing that read it, so the log said where a peer moved to but not where from — and the address it moved *from* is the one the stale `connect(2)` had pinned. Both ends are rendered now. The peer id used a private four-byte hex helper that duplicated `NodeAddr::short_hex` and dropped the `...` suffix every other operator surface prints; the duplicate is gone. The cost note said three syscalls per target: `UdpSocket::bind` is a `socket(2)` and a `bind(2)`, and the socket takes a `close(2)` on drop, so it is five. In the same place, "bounded by `node.limits.max_peers`" does not hold when that value is 0, which the configuration defines as unlimited. This deletes `interface_addrs()`'s only call site, and with it the `getifaddrs` walk and its `sockaddr` decoding. It therefore absorbs the Android `getifaddrs` issue rather than leaving it to be fixed separately. The peer table is reached through `entities_snapshot` rather than a new sharing primitive, so the detector stays a detached task holding no node state and taking no node lock. `PeerRow` gains a typed `probe_target` rather than having the detector re-parse the display string next to it, so a change to that string's rendering cannot silently leave the detector with an empty table and no way to notice. Peers that are not probeable IP destinations contribute nothing and need no per-transport special-casing here: a MAC on Ethernet or BLE, a .onion or Nym recipient behind a local proxy, a scoped IPv6 literal, and a peer still carrying its configured hostname all arrive as `None`. The last is deliberate — resolving one would put a DNS lookup with its timeouts on the sample path — and the window is small, since the address is replaced by the observed numeric source the first time an authenticated packet arrives. A node with no peers detects nothing, which is right: nothing is bound to the old path. Also corrects two doc claims that did not match the code, both in the text being rewritten: `transport::watcher` has no consumer besides this module, so it is not "shared with the interface binder"; and a connection-oriented transport does not "re-dial on send" in the case that matters, because `send_async` only dials when the pool holds no connection for the address and a connection stranded by a medium change is still in the pool — it is evicted after a write to it fails. Tests, each run against the defect it guards rather than only against the fix: - The churn rules are mutation-checked: iterating the union instead of the intersection fails four tests, and dropping the sample-adoption fails the one that pins a newly joined peer entering the comparison. - The snapshot seam is table-driven over six address shapes and fails if the publish site stops populating `probe_target` — nothing renders that field, so nothing else would have caught it. - A namespace test brings up a dummy interface with its own subnet and asserts the fingerprint does not move, then puts a more specific route to the peer out of that same interface and asserts it does, so the negative half cannot pass because sampling quietly stopped working. The same test now asserts that an unconstrained probe answers with the carrier and a constrained one with the other address, and that the two differ — the disagreement that would otherwise report a phantom move on every first-seen peer. Ignoring the constraint in `preferred_source` fails it. - The two links carrying the pinned source were asserted by nothing. Substituting `None` where `probe_targets` reads the row, or where `sample` writes the fingerprint, left the whole suite green while silently restoring the bug the first-sight rule exists to fix. Both are asserted now, and both mutations fail. - Every live-probe test passed if `preferred_source` returned `None` for everything: two all-`None` samples are self-consistent, the recorded-keys test never inspected a value, and the loopback assertion skipped through its `if let`. A probe to loopback must now answer with loopback, which holds on any host that can run the suite, including one started with `--network none`. - `reports_are_spaced_out_under_clean_flapping` polled at one second against a one-second pacing floor, so the two were indistinguishable and deleting the pacing block still passed. The wake is 100ms now. - The pinned-source publish test was Linux-gated though `open_connected_fd` and the field it asserts are available on macOS too. Widened, along with the two sibling tests on the same helper. - The detector's netlink subscription asserts the route groups rather than logging a decline, so a wrong group mask reds instead of passing quietly. |
||
|
Johnathan Corgan
|
4990525b62 |
fix(peering): do not count a failed heartbeat send as a delivered one
The heartbeat was recorded as sent before the send was attempted, so a peer
whose heartbeat could not go out was treated as heartbeated and was not tried
again for a whole heartbeat_interval_secs, although it had heard nothing and
its own link-dead timer was running.
Record the attempt and the delivery separately. The interval that paces a
healthy peer now advances only on a send that returned cleanly, and a peer
whose send failed is retried after a shorter fixed interval instead of after a
full heartbeat interval.
The retry interval gates only a peer whose last attempt failed. On a healthy
peer the two timestamps are equal, so consulting it there would clamp a
heartbeat_interval_secs configured below the retry interval, and that setting
has no validation floor.
The due-or-not decision moves into a small pure function so it can be tested
without driving a send. The tests cover both halves and both were checked
against the defect rather than only against the fix: restoring the old ordering
fails the integration test on the recorded-as-landed assertion, and removing
the retry gate fails it on the not-retried-every-tick assertion.
Also widen the generated-config ignore glob. The rule matched
generated-configs/ exactly, while the scripts write
generated-configs${FIPS_CI_NAME_SUFFIX}, so every suffixed run left an
untracked directory behind.
|
||
|
fr34akyandJohnathan Corgan
|
813c50bb14 |
fix(transport): take the stream write off the caller's task
TCP, Tor, Nym and BLE wrote to their links from whatever task called `send`. `write_all` on a stream blocks once the peer's receive window and this node's send buffer are both full, and it blocks for as long as that lasts — there is no bound on it. A peer that has stopped reading, or one whose path has just changed medium, produces exactly that state. The callers are the rx loop's tick handlers. The heartbeat sweep is one of them, which is how this surfaced: filtering connection-oriented peers out of the medium-change fan-out took the write out of that handler, but not out of the sweep ten seconds behind it — same peer, same state. And while the rx loop sits in that write it is not serving anything else, so one unresponsive peer stalls every other peer's liveness, the forwarding path, and the control socket. Each connection now owns a writer task holding the write half, and `send` enqueues onto a bounded channel. The only code that can await the wire is a task with nothing else to do, so the property holds by construction rather than by a timeout that has to be tuned against a healthy slow link — a Tor circuit legitimately stalls for seconds, and any budget short enough to protect the rx loop is short enough to kill one of those. `try_send`, not `send`: awaiting a full queue would reinstate the same block one level up. A full queue means the writer has not drained a frame in the time it took to offer 64, which is a peer that is not receiving, so the send fails and the caller's existing retry and liveness handling takes over — the same shape as the connect gate above it, which already refuses rather than waits. Frames are written whole by a single task, so ordering is preserved and the half-written-frame hazard cannot arise: a write error takes the connection down with it, and the peer sees a closed connection rather than a frame it cannot resynchronise from. Teardown mirrors the receive loop's existing contract exactly — the pool entry is removed and the direction counter decremented only when the removal returned `Some`, so a concurrent close or stop of the same address cannot double-count. BLE was the worst of the four. `send_async` awaited the L2CAP write while holding the connection-pool mutex, so a peer that stopped draining its link blocked not only its own sender but every other BLE operation behind that guard: connects, evictions, and each receive loop's teardown. The other three only blocked the caller. The fix is the same shape, and the pooled stream was already an `Arc` with a `Send` future, so the writer task holds a clone and no generic surgery was needed. Queue depth is 16 there rather than 64: a BLE link carries a fraction of the throughput, so the same depth would be seconds of backlog rather than a burst. This also settles the Android backend without changing it. Its `BleStream::send` pushes onto the embedder's queue and waits for a slot rather than dropping, which was an unbounded wait on whatever task called it. That call now happens only in the writer task, where waiting is the job, and the layer above it is the connection's own bounded queue, which fills and refuses without waiting on anything. Two costs, both deliberate. The byte count `send` returns is now what was queued rather than what reached the wire, which is the prediction the UDP fast path already reports when it dispatches to the encrypt workers; bytes actually written are recorded by the writer task as they go. And the frame is copied into the queue, because `write_all` borrows and a queue must own — one memcpy of at most an MTU, against an unbounded stall. Tor and Nym share `socks5::pool`, so the writer loop is written once there and once for TCP, matching each transport's existing receive loop. Both regression tests drive a peer that accepts and then never reads, push far more than any buffer holds, and assert every send returns promptly. Both are mutation-checked: replacing `try_send` with an awaiting `send` fails the TCP one on the timeout, and reinstating the write under the pool guard fails the BLE one, which also asserts the pool stays lockable throughout. What the evidence does and does not cover. The BLE change is exercised through `MockBleIo`, not over the radio, so the argument that a stalled radio link behaves like a stalled mock channel is reasoning rather than measurement. The Android backend is `target_os = "android"` and there is no NDK on the author's host, so it has no local compile evidence; the `android-check` leg is the only one. Integration suites were run one at a time rather than as a single pass, since the full runner exhausts memory there: `chaos-tcp-mesh`, `chaos-congestion-stress` and `chaos-churn-mixed-10` all pass. Folded in on landing, all documentation and no behaviour change. Two doc comments were left attached to the wrong function when the writer loop was inserted above the receive loop, so `tcp_receive_loop` and BLE's `receive_loop` each lost their own documentation to the new function above them; both are moved back, and the TCP section banner now names both loops. The Android backend's safety comment claimed `BleStream::send` is reached from the writer task and from nowhere else, which is not so — `pubkey_exchange` calls it too, safely, because it wraps the call in a timeout — so the comment now names both callers and the reason each is bounded. And the netmon fan-out's filter documented itself by the hazard it avoided, an unbounded `write_all` reached from the rx loop; that hazard is gone, so the rationale and the matching changelog clause now record that the filter has outlived it and that widening the fan-out is open work left out here so the two changes stay separable. Co-authored-by: Johnathan Corgan <johnathan@corganlabs.com> |
||
|
fr34aky
|
eaca285ad2 |
fix(testing): claim the medium-change lab's subnets instead of pinning them
The suite pinned three fixed /24s — 172.31.60, .61 and .62. Compose
project names are unique per run, so two concurrent runs got distinct
container and network *names*, but the address pools are constants and
both runs asked for the same ones. Whichever created a network first
won; the other died at topology start with
failed to create network ..._mc-far: invalid pool request:
Pool overlaps with other one on this address space
having tested nothing. Pushing maint, master and next within a second of
each other is enough to hit it, and the branch that loses looks broken
when it is fine.
The three `MC_*_PREFIX` overrides existed from the start but nothing
ever set them, so the defaults were the only values ever used.
This does what the nat suite already does. A free /24 per network is
claimed under 10.42.0.0/16 before the lab starts, with docker's own
`network create` as the atomic arbiter of who owns what — the run-id
derived offset is deliberately not used here for the same reason it was
rejected there: it makes a collision unlikely rather than impossible,
and a collision is the failure being removed. Both CI labels are stamped
so ci-cleanup.sh's label sweep recovers the networks when a run is
SIGKILLed, which no inline removal can cover.
Since the claim creates the networks, compose has to attach rather than
create, so `docker-compose.external-net.yml` declares the three
external, applied through a new `MC_EXTRA_COMPOSE` hook. Nothing else
sets it: the GitHub matrix runs one job per runner and invokes
`test.sh` directly, and a bare `docker compose up` is a single lab, so
both keep the fixed defaults and the addresses in the README stay
literal. The hook appends to the whole COMPOSE array rather than to the
`up` alone, so teardown addresses the same project — a `down` without
the overlay would not know the networks are external.
The claim lives in ci-local.sh rather than the suite script for the
reason the nat comment gives: the workflow invokes the script directly
and tears down with the base file only, and does not want a claim.
Release does a `compose down` before removing the networks. That order
is load-bearing rather than tidy: `docker network rm` silently no-ops on
a network that still has endpoints attached and reports success, so
without the `down` the removal fails exactly on the path it exists for.
A network left behind would not merely leak — the next invocation in the
run would hit `network with name ... already exists`, which is not a
pool overlap, so the allocator correctly refuses to advance and fails.
Also gives the suite its own compose project. It set none, so it
inherited whichever COMPOSE_PROJECT_NAME the previous suite exported —
which is why the failure named a medium-change network under the nat
project, `fipsci_<runid>_nat_mc-far`. That is not what caused the
overlap, but filing one suite's resources under another's project is a
teardown hazard: `down --remove-orphans` on either would consider the
other's containers orphans.
The /24 claim loop is now shared rather than copied a third time.
`ci_claim_nat_net` keeps its name, its `[nat]` log tag and its exported
prefix, and becomes a two-line caller.
Verified rather than assumed. The lab runs on the claimed prefixes, not
just alongside them: `node-b re-pinned to 10.42.1.10:2121`. With the
first three candidates occupied by squatter networks — the collision
path itself — the allocator advances to 10.42.3/4/5 and the suite passes
on those. Run standalone with no overlay it still renders 172.31.6x and
passes 8/8, so the workflow path is untouched. `nat-cone` passes and
still claims 10.41.0/1, so the shared loop did not disturb it. Networks
are gone after each run. `ci-local.sh --only medium-change` is 17/17.
Not exercised: the partial-claim rollback, which needs a /16 exhausted
part-way to reach. It mirrors ci_claim_nat_networks' shape.
|
||
|
Johnathan Corgan
|
d1e7dc1a79 | Merge branch 'maint' | ||
|
Johnathan Corgan
|
70002baf20 |
Settle the native-api counter reads instead of racing the reader task
The suite asserted a happens-before the native API does not offer. A client's write on a flow descriptor lands in the kernel buffer of an AF_UNIX socketpair and runs no daemon code; the counters advance only inside the per-flow reader task, after that task's own recv().await. `stats` is answered on a different task and loads the same atomics, and the daemon is a single-threaded runtime, so a stats reply can be produced while the datagrams are still queued and the reader task has not been polled. The reply is then a well-formed status ok with a live flow_id and local_port and both counters at zero, which is the shape that redded maint at |
||
|
Johnathan Corgan
|
4c345f3ffd |
Make the malformed-advert phase inject the stimulus it asserts against
Two test helpers piped a heredoc into `docker exec <container> python3 -` with no `-i`. Without it docker attaches no stdin, so `python3 -` reads an empty program, runs nothing and exits 0; `set -euo pipefail` cannot catch a success. Measured on this host at docker 29.1.3 against a live container: without the flag the program produced no output and returned 0, with it the program ran. The relay half is the serious one. The publisher it silently skipped is the malformed Kind-37195 event that phase 3 exists to inject, and the three assertions that follow hold whether or not anything was injected, so the phase has passed in all 51 archived runs without exercising the path it covers. The NAT half only makes a diagnostic vacuous, but it did real damage once: every tcpdump wrapped around the STUN probe reported 0 packets captured, and that was read as a packet leaving the node and vanishing. Adding `-i` alone is not enough. The publisher printed its completion line unconditionally, after reading the relay's reply and discarding it, so a refused event would still have satisfied a check that only looked for that line. strfry verifies the event id and the signature and answers ["OK",<id>,false,"invalid: ..."] on refusal, and a refused event is never stored and never broadcast. The publisher now parses the relay's verdict and exits non-zero on anything but an acceptance, and phase 3 reds unless the relay reports the event stored. The verdict is read by decoding the websocket frame rather than by matching a substring. NIP-01's OK is a four-element array whose message is mandatory even on success, so `,true]` never occurs in a conformant acceptance; and a 91-byte payload puts a literal '[' in the frame's length byte, so searching for the JSON finds the header. Both were found by testing the guard against frames built the way a relay builds them. Three comments are corrected while here. The publisher's header claimed the daemons log a parse error and that the content fails to deserialize; neither holds, because the `protocol` tag is checked first and the discard emits no diagnostic. The kind and `d` tag are now documented as duplicating the consumers' subscription filter, which is what makes them able to drift. Expect phase 3 to exercise the missing-protocol-tag branch for the first time. A red there is a finding rather than a regression this introduces. |
||
|
Johnathan Corgan
|
fc5b4ade24 |
refactor(transport): keep the link watcher out of the public library API
`transport::watcher` was published as `pub mod`, which makes `LinkWatcher` and its group constants part of the crate's compatibility surface. Every caller is in-crate — `node::netmon` and its tests, reached as `crate::transport::watcher` — so nothing outside the crate is relying on it, and publishing it would commit the library to the shape before anyone has asked for that shape. Narrowed to `pub(crate) mod`. This is a one-line change now and a breaking one after a release carries it. |
||
|
fr34akyandJohnathan Corgan
|
b922568dca |
fix(node): re-pin connected UDP sockets when the host changes medium
An established UDP peer gets its own `connect()`-ed socket for the send fast path. `open_connected_fd` binds the wildcard and then calls `connect(2)`, which makes the kernel resolve the route once and auto-bind the local source address to whichever interface was carrying it at that moment. It never re-evaluates. So after the host changed transport medium — a laptop between WLAN and LAN, a phone between Wi-Fi and cellular — every established peer went on transmitting from an address the routing table had abandoned. The peer, which re-pins to whatever address it last heard from, answered somewhere the node was no longer sending from. The peering stayed marked connected and carried nothing until `link_dead_timeout_secs` tore it down: 60-90s of black-holed traffic per switch on a live node, then a full re-handshake and tree re-convergence. The mirror-image case, the peer rotating its address, was already handled where the rotation is observed. This is the local half, and it had no signal to hang off, because a local move is invisible in the data plane. Medium-change detection supplies that signal. `node.netmon.*` controls it and it is on by default. The node samples a coarse fingerprint of its network attachment — the source addresses the routing table would pick for an off-link destination, plus the set of up, non-loopback interface addresses — and reports a change once the picture settles. A handover is not atomic, so a short debounce coalesces the burst into one event, and a fingerprint that settles back where it started reports nothing. Linux and Android subscribe to `NETLINK_ROUTE` multicast and macOS and FreeBSD to a `PF_ROUTE` socket, both reacting in milliseconds; every other platform samples on a timer, which also runs underneath the kernel sources as a backstop. A backend decides only when to look, so the remaining ones land behind the same seam. The reaction is two steps. Drop the stale connected sockets, which is self-healing rather than disruptive: the wildcard listen socket resolves a route per packet, so sends keep working immediately, and a correctly-bound socket is reinstalled on a later tick. Then heartbeat every peer whose send path cannot block, so the far side re-pins at once rather than waiting out its own interval. That filter is the whole point rather than an optimisation. A connectionless send completes without awaiting the wire. A connection-oriented one awaits an unbounded `write_all` on a stream that the medium change has very likely just stranded, and this reaction runs on the rx loop, so it would hold every other arm of the select for as long as that socket took to fail. A peer on such a transport keeps the periodic heartbeat it had before, with `link_dead_timeout_secs` as the backstop. Covered by unit tests, by a regression test that pins the fan-out filter, and by a new `medium-change` integration suite: a multi-homed node whose default route moves between two live access paths while mesh traffic is in flight, with the far peer off-link behind a router. The changelog entries land under Unreleased rather than in the released `0.5.1` section, since none of this is in that release. |
||
|
ArjenandJohnathan Corgan
|
0b24097db3 |
feat(transport): kernel link-event watcher
A watcher that resolves when the kernel reports a change to the host's network links, so callers can react to interface state in sub-second time instead of polling for it. Netlink `RTNLGRP_LINK` on Linux, `PF_ROUTE` on the BSDs. Self-contained and unused by anything yet: it lands separately because more than one caller wants it, and a second netlink socket beside this one would be the wrong answer to that. Three properties are the reason it is worth sharing rather than reimplementing, and each has a test: - **A sourceless watcher parks rather than fires.** A kernel or sandbox that refuses the socket yields a watcher whose `changed()` never resolves, which is what makes it safe to `select!` against a poll ticker — the ticker simply always wins and the caller degrades to polling with no special case. - **A zero-length read does not spin.** `try_io` clears readiness only on `WouldBlock`, so breaking out of a zero-length read leaves `readable()` instantly ready with nothing to read, and the loop never returns `Pending`. That starves the caller's `select!` of every other arm — it is not a busy loop in the watcher, it is a livelock in whatever owns it. - **Giving up is sticky.** `changed()` is constructed fresh on every pass of a caller's `select!` and dropped whenever another arm wins, so a `pending()` inside the future parks nothing beyond the current pass. Without a flag on the watcher itself, the next pass re-reads the dead socket, re-counts the error and re-logs the give-up warning — once per wake-up, forever. The messages are deliberately not parsed. An event is a hint to re-ask whatever question the caller actually has, which is cheap and authoritative; decoding `nlmsghdr`/`ifinfomsg` to reach the same answer would add a parser whose bugs would become the caller's bugs. `RTNLGRP_LINK` carries link state only. A caller needing route or address events should extend `open_link_socket` with a group mask rather than opening its own socket — the note is in the module docs so the next caller finds it. |
||
|
Johnathan Corgan
|
e8f3f2bf1c |
Merge branch 'maint'
# Conflicts: # Cargo.lock # Cargo.toml # README.md |
||
|
Johnathan Corgan
|
c5aeef39ef |
Release v0.5.1
Version only. This is the last source mutation before the tag.v0.5.1 |
||
|
Johnathan Corgan
|
30a283907b |
Correct the release documents the readiness pass found wrong
The closing code fence in the release notes carried an info string, so under CommonMark it was not a closer and the block ran to the end of the file. Rendered through GitHub's own markdown endpoint the published body stopped emitting headings after "Upgrade notes" and ended inside a code block, so the download section, the changelog and security links and the contributor credits all became preformatted text. Rendering the corrected file the same way returns all seven headings and a working release link. A lint fix had rewritten both fences rather than only the opening one. The post-upgrade instruction named a subcommand that does not exist: it said to run fipsctl status, which exits 2, where the command is fipsctl show status. That instruction is aimed at exactly the users this release exists for. Several claims were broader than the tree supports, and each is narrowed to what is true. The two discovery fixes carry no platform gating, so the notes no longer tell macOS, Windows, FreeBSD and OpenWrt users that nothing changed for them, and no longer say the release contains no behavioral change. The supported-distribution paragraph now covers the .deb and the tarball rather than all Linux artifacts, since Arch and NixOS build from source and OpenWrt is a musl target that does not depend on the glibc floor. The install-test sentence says x86_64, because the arm64 package is install-tested by nothing. The changelog no longer claims every producer runs the floor check, since the deprecated host-build targets do not. The counter guidance was wrong about which counter moves, the objdump line prints the symbol beside the version, and the two relative links in the notes resolve inside the repository but 404 in a release body, so they are absolute now. That last change also makes the root mirror byte-identical to the versioned copy for the first time. The FreeBSD install line takes a concrete filename again: a placeholder is not pasteable, which is the whole point of an install instruction. |
||
|
Johnathan Corgan
|
f0ee81ec7c |
Prepare the v0.5.1 release content
Move the staged changelog entries under a 0.5.1 heading dated 2026-09-06, and add an entry for the deb-install hang: the suite started a oneshot unit that requires the daemon, so a daemon that could not execute left the start job undispatched and the suite reported nothing at all. That is the whole class of fault the suite exists to find, and it protects the run that gates artifact publication, so it is owed an entry. Add the release notes and mirror them to the root file. The notes lead with who should upgrade and who is unaffected, because for most users this release changes nothing and for Debian 12 and Ubuntu 22.04 users the daemon has never run at all. They state what was measured and what was not, and give an objdump line that reads the floor of a binary already installed: 2.34 from this release, 2.39 from any earlier one. The root mirror is not byte-identical to the versioned copy and cannot be. The two files sit at different depths, so a link that resolves in one breaks in the other; the mirror is content-identical with the relative link paths rewritten, and the check is a diff whose every hunk is a link path. Here that is two hunks, both confirmed to resolve. The currency audit found five stale version sites and two claims that do not match the tree, all corrected here. The status badge, the release-notes link and the status sentence in the README follow the release, as they did at v0.4.2. The FreeBSD install line gave a filename that does not exist, since the tree is 0.5.1-dev; it now takes a version placeholder so it stops going stale at every bump, and the example above it keeps a concrete name. The design document recorded the first lookup fix and not the second: it still said a returning copy is dropped as a duplicate, which the next commit in this release exists to stop doing. It now names the counter that actually receives the drop. The README claimed the .deb is exercised per release. The install suite is real and covers the five distributions it names, but it runs on push and pull request, not at a tag, and no workflow installs the published artifact. The sentence now says that, so the released package being checked by hand is written down rather than assumed. Getting-started gains the statement this release should have produced: which distributions are supported and why the binaries run on all of them. Until now the policy and the floor lived only in the build environment file, so an operator had nowhere to read either. |
||
|
Johnathan Corgan
|
f149cbac55 | Merge the maintenance line up | ||
|
Johnathan Corgan
|
867f5f81b4 |
build: produce every Linux artifact in a pinned container and check its floor
The released .deb installs cleanly on Debian 12 and Ubuntu 22.04 and the daemon then cannot start, with the loader reporting GLIBC_2.39 not found. Three binaries are affected, fips, fipstop and fips-gateway; fipsctl runs, which is why it stayed quiet, since an install checked by running fipsctl gets a clean answer while the daemon is dead. It is not a v0.5.0 regression: every release artifact from v0.3.0 onward carries the same floor and the same unversioned dependency. The cause is the build machine. Rust's standard library references pidfd_spawnp and pidfd_getpid as weak undefined symbols behind a runtime check, so a binary should fall back where the C library lacks them. Linking against a C library that has them records a hard version dependency instead, and the loader refuses the image on that entry alone. No Rust changed here. One script now produces the Linux artifacts. It builds in a container pinned to the oldest distribution still supported for free by its distributor, named with the floor in packaging/build-floor.env, and runs the floor check on the package it produced, so every producer is gated rather than one workflow. The floor guard reads readelf's Version needs section: the obvious objdump formulation returns 2.2.5 for the shipped fips and would have passed every affected release. The script prints the package path as the only thing on its stdout, which is what lets a caller take it without parsing, and it takes --features. Both required care. The container's own stdout reaches the caller, so the build runs with its output on stderr; without that, a caller using a plain command substitution captures four lines of build chatter along with the path. And a feature build must keep the +<features> marker that distinguishes it from the default build of the same commit, or dpkg sees two packages at one version and a revert silently no-ops. The version is derived on the host, because the image has no git and the source is mounted read-only, so build-deb.sh now applies that marker to an explicit version as well as to one it derives. Both runners now build once through that script and install the artifact. The five deb-install legs previously built their own package each, so one CI run performed five complete release builds and four were waste; they now live in a job of their own that downloads one built package, which also stops the rest of the integration matrix waiting on it. The parity guard read one hardcoded job and now sweeps every job's matrix. The release workflow builds both architectures through the same script, and the systemd tarball takes its binaries out of that package instead of from a second, unchecked set on the runner, then is floor-checked after the strip. Cargo.toml derives the dependency with $auto rather than stating a bare libc6 that nothing can fail. Note the ordering this implies for any pipeline that builds on a current distribution: until it builds through this script, its packages will declare libc6 (>= 2.39). Measured: the container build produces four binaries at 2.34, and one artifact passes all five distributions, 95 checks, in about two minutes. The floor check fails the released 0.5.0 package on three binaries and passes this one. A profiling build produces fips_0.5.1~dev+git<date>.<sha>+profiling-1_amd64.deb. |
||
|
Johnathan Corgan
|
25daa4de1a |
fix(testing): bound the deb-install service starts so a dead daemon reds instead of hanging
The deb-install suite started fips-dns.service with no timeout. That unit is Type=oneshot with Requires=fips.service, so when the daemon cannot execute -- a broken package, a bad config, a missing capability -- systemd restarts it every five seconds for ever, the oneshot start job is never dispatched, and systemctl start never returns. The suite then produced no FAIL line, no Results line and no exit status at all. Observed at 21 minutes against a package whose binaries could not load. That is the whole class of fault this suite exists to find, so the suite stopped reporting at exactly the point it was most needed. It also matters beyond one test: a hang here blocks the local run that gates artifact publication. Queue that one unit rather than waiting on it, and wait for it to become active before reading /run/fips/dns-backend. RemainAfterExit=yes makes is-active a correct readiness test for the oneshot, and the wait carries a timeout and dumps the journal on failure, so the verdict lands on an assertion instead of on a stalled call. Keep every other start blocking, because the call returning is what synchronises the checks after it -- fips-gateway.service waits up to thirty seconds for fips0 in ExecStartPre, and a caller that does not wait races it. What those calls lacked was a bound, not the wait, so they get one. Bound the whole suite from ci-local.sh as a backstop, and say so explicitly when it fires: a timeout means no assertion was reached, which is not the same as an assertion failing. Verified by breaking what it guards. Against the released 0.5.0 package, whose binaries require a newer glibc than Debian 12 provides, the suite now reds in 36 seconds with the loader error in the journal dump. Against a working package it passes 38 checks across debian12 and ubuntu26, covering both resolver backends. |
||
|
Johnathan Corgan
|
8494c53ff8 | Merge the maintenance line up | ||
|
Johnathan Corgan
|
e5586cb333 |
fix(lookup): count a request of our own that came back apart from a duplicate
Dropping our own flooded request when a bloom false positive circulates it back to us recorded the drop under `req_duplicate`, whose documented meaning is that a peer resent a request. The two events are not the same, and only one of them says anything about the peer that delivered the frame. A returning copy has a nonzero floor in healthy operation and rises with the bloom fill ratio, so folding it into `req_duplicate` puts a permanent number on a counter an operator reads as neighbour misbehaviour, and leaves no way to tell a resending peer from this node's own fan-out coming home. It now carries its own rejection reason and counter, `req_own_loopback`, shown in fipstop as "Own Loopback", and its own log line naming the target. The control socket's show routing fixture gains the field. Two comments record what the guard rests on. `request.origin` is the obvious cheaper identity test and is unusable: it is unsigned and set by whoever sends the frame, so a peer could put this node's address on any request and make it refuse to transit that request. And the test reaches only as far as the last MAX_RECORDED_IDS a target's ladder issued, so a ladder configured with more rungs than that loses its earliest ids. The request-side regression test now builds its request with this node's own address as the origin, which is the shape the defect actually has. It was using a helper whose origin is a third party, so it exercised the guard without reproducing the case. |
||
|
ArjenandJohnathan Corgan
|
2787062436 |
fix(lookup): accept our own lookup response instead of relaying it away
An originator could misfile the answer to its own lookup as transit, so discovery reported "requests went unanswered" while the replies were in fact arriving and being reverse-path forwarded to a peer. A request is flooded to every tree peer whose bloom filter claims the target. At a high fill ratio a false positive sends a copy out into the wider network, which can circulate it back to the originator. On arrival `classify_request` applied its only identity test, `target == my_addr`, which a lookup we originated never satisfies, so the copy was recorded in `recent_requests` as an ordinary transit entry keyed on our own `request_id`. When the target answered, `classify_response` consulted `recent_requests` first, matched that entry, and relayed our own answer to the peer that looped the request. The pending lookup was never satisfied, the ladder retried with a fresh id, and the same race repeated. The `None` arm's reasoning — "not a request we transited, so it claims to answer one of ours" — held only while our own ids stayed out of the dedup cache, and nothing enforced that. `resp_unsolicited` pinned at zero while `resp_received == resp_forwarded` was the tell: control never reached the originator arm. Affected nodes showed `resp_accepted` at 1 of 291. It is a race, not a hard failure: when the genuine reply beats the looped copy the cache is still clean and the lookup succeeds. It bites hardest at the junction between a quiet subtree and a dense public network, where a lookup for a descendant goes both correctly downward and outward through a false positive, and the wide network returns it first. Two changes close it, on both sides of the invariant: - `classify_response` tests the pending lookups before `recent_requests`. A response naming a target with a lookup outstanding, carrying an id that lookup issued, is ours whatever the dedup cache holds. The id is fresh 64-bit randomness per attempt and the target signs over it, so an id we never issued still cannot match and the replay properties the `Unsolicited` arm protects are unchanged. - `classify_request` drops a request whose id a pending lookup for that target issued, rather than recording it. Our own id never enters the transit cache, so a duplicate reply cannot be misrouted after we accept the first, and the returning copy is not forwarded a second time. The guard keys on the id, so another node's lookup for the same target still transits normally. Verified against the reported deployment: discovery now completes where it previously exhausted the four-attempt ladder. |
||
|
Johnathan Corgan
|
66f0de8ad8 |
Merge the maintenance line up
Carries the maintenance branch's dev-open commit onto the development line, restoring containment after the release. The version conflict resolves to this branch's own 0.6.0-dev, which is what a forward merge always does with a version: each line keeps the one it is building toward. Nothing else differs. Both branches made the same changelog edit when they opened, so it merged without a conflict. |