Merge maint into master

This commit is contained in:
Johnathan Corgan
2026-10-01 22:42:02 +00:00
74 changed files with 4689 additions and 718 deletions
+30 -8
View File
@@ -79,6 +79,17 @@ jobs:
run: bash testing/check-comment-refs.sh
- name: Check no non-test code uses std 64-bit atomics
run: python3 testing/check-portable-atomics.py
# The OpenWrt Package workflow runs this too, but only on trunk pushes,
# tags and pull requests; here a branch push sees a finding first.
# Kept in step with ci-local.sh's run_shellcheck by hand.
- name: Install shellcheck (if missing)
run: |
if ! command -v shellcheck >/dev/null 2>&1; then
sudo apt-get update && sudo apt-get install -y --no-install-recommends shellcheck
fi
shellcheck --version
- name: Check the OpenWrt package's shell scripts with shellcheck
run: bash testing/check-shellcheck.sh
# Hermetic: synthetic ping functions, no containers, ~45s. Lives beside
# the other two so both runners gate on it identically — putting it in
# only one would create exactly the drift check-ci-parity.sh exists to
@@ -86,15 +97,20 @@ jobs:
# a matrix suite.
- name: Run convergence-gate unit tests
run: bash testing/lib/wait-converge-test.sh
# Runs package-linux.yml's own version derivation on a release tag, a
# candidate tag and a branch. Not a matrix suite either, so it is kept in
# step with ci-local.sh's run_deb_version by hand.
- name: Check the Debian version derived for tags and branches
run: bash testing/check-deb-version.sh
# Runs the packaging workflows' own version derivations on a release
# tag, a candidate tag and a branch. Not a matrix suite either, so it is
# kept in step with ci-local.sh's run_package_versions by hand.
- name: Check the package versions derived for tags and branches
run: bash testing/check-package-versions.sh
# The unit-test jobs' flaky-test reporter, against recorded nextest
# reports. Kept in step with ci-local.sh's run_nextest_flaky by hand.
- name: Check the flaky-test reporter against its fixtures
run: bash testing/nextest-flaky/test.sh
# The glibc floor check's cases, built from the host's own true
# executable. Not a matrix suite, so it is kept in step with
# ci-local.sh's run_glibc_floor by hand.
- name: Check the glibc floor check against its cases
run: bash testing/glibc-floor/test.sh
fmt:
name: Format check
@@ -342,7 +358,9 @@ jobs:
${{ runner.os }}-cargo-
- name: Install cargo-nextest
uses: taiki-e/install-action@nextest
uses: taiki-e/install-action@fcf5432d9f50d67e37ee6e29bdb7a224ff67b4a7 # v2
with:
tool: nextest
# The cache restores target/, including the previous run's report. Remove
# it so the flaky-test check below reads only this run's, and reports a
@@ -455,7 +473,9 @@ jobs:
${{ runner.os }}-cargo-
- name: Install cargo-nextest
uses: taiki-e/install-action@nextest
uses: taiki-e/install-action@fcf5432d9f50d67e37ee6e29bdb7a224ff67b4a7 # v2
with:
tool: nextest
# The Darwin half of the address-less presence contract. The Linux legs
# pin that `getifaddrs` reports an interface with no addresses as
@@ -611,7 +631,9 @@ jobs:
${{ runner.os }}-cargo-
- name: Install cargo-nextest
uses: taiki-e/install-action@nextest
uses: taiki-e/install-action@fcf5432d9f50d67e37ee6e29bdb7a224ff67b4a7 # v2
with:
tool: nextest
# The cache restores target/, including the previous run's report. Remove
# it so the flaky-test check below reads only this run's, and reports a
+1 -1
View File
@@ -46,7 +46,7 @@ jobs:
# dpkg reads X.Y.Z-rcN as revision rcN of X.Y.Z and sorts it above
# the release; X.Y.Z~rcN sorts below it. git refuses '~' in a ref
# name, so the tag carries '-' and this maps it, for the .deb only.
# testing/check-deb-version.sh runs this step's text.
# testing/check-package-versions.sh runs this step's text.
DEB_VERSION="$VERSION"
if [[ "$GITHUB_REF" == refs/tags/* ]] \
&& [[ "$VERSION" =~ ^([0-9]+\.[0-9]+\.[0-9]+)-((alpha|beta|pre|rc)[0-9]*)$ ]]; then
+22 -46
View File
@@ -25,6 +25,7 @@ jobs:
outputs:
package_version: ${{ steps.version.outputs.package_version }}
apk_version: ${{ steps.version.outputs.apk_version }}
ipk_version: ${{ steps.version.outputs.ipk_version }}
release_channel: ${{ steps.channel.outputs.release_channel }}
steps:
- uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
@@ -41,15 +42,28 @@ jobs:
# in the .apk metadata. apk_version is built directly from the same
# structured inputs (tag, or commit height) — no reparse of the
# flattened package_version. See packaging/openwrt-apk/apk-version.sh.
#
# ipk_version is the .ipk control Version. opkg compares versions as
# dpkg does, so it reads a tag's vX.Y.Z-rcN as revision rcN and sorts
# it above the release; vX.Y.Z~rcN sorts below it. git refuses '~' in
# a ref name, so the tag carries '-' and this maps it. The leading 'v'
# stays: every released .ipk carries it, and under opkg 0.5.4 sorts
# below v0.5.3. testing/check-package-versions.sh runs this step's text.
if [[ "$GITHUB_REF" == refs/tags/* ]]; then
echo "package_version=${GITHUB_REF_NAME}" >> "$GITHUB_OUTPUT"
echo "apk_version=$(sh packaging/openwrt-apk/apk-version.sh tag "${GITHUB_REF_NAME}")" >> "$GITHUB_OUTPUT"
IPK_VERSION="${GITHUB_REF_NAME}"
if [[ "$GITHUB_REF_NAME" =~ ^(v[0-9]+\.[0-9]+\.[0-9]+)-((alpha|beta|pre|rc)[0-9]*)$ ]]; then
IPK_VERSION="${BASH_REMATCH[1]}~${BASH_REMATCH[2]}"
fi
echo "ipk_version=${IPK_VERSION}" >> "$GITHUB_OUTPUT"
else
BRANCH=$(echo "$GITHUB_REF_NAME" | sed 's|/|-|g')
HEIGHT=$(git rev-list --count HEAD)
HASH=$(git rev-parse --short HEAD)
echo "package_version=${BRANCH}.${HEIGHT}.${HASH}" >> "$GITHUB_OUTPUT"
echo "apk_version=$(sh packaging/openwrt-apk/apk-version.sh dev "${HEIGHT}")" >> "$GITHUB_OUTPUT"
echo "ipk_version=${BRANCH}.${HEIGHT}.${HASH}" >> "$GITHUB_OUTPUT"
fi
- name: Determine release channel
@@ -120,8 +134,9 @@ jobs:
- name: Install Rust toolchain (nightly, Tier 3)
if: matrix.rust_channel == 'nightly'
uses: dtolnay/rust-toolchain@nightly
uses: dtolnay/rust-toolchain@be39649afda95dbf70f87cce95f68b8d5797b296 # nightly
with:
toolchain: nightly
components: rust-src
- name: Cache Cargo registry + build
@@ -331,6 +346,7 @@ jobs:
- name: Build .ipk
env:
PKG_VERSION: ${{ needs.determine-versioning.outputs.package_version }}
IPK_VERSION: ${{ needs.determine-versioning.outputs.ipk_version }}
run: ./packaging/openwrt-ipk/build-ipk.sh --arch ${{ matrix.build_arch }} --bin-dir "$GITHUB_WORKSPACE/bins"
- name: Install shellcheck (if missing)
@@ -341,52 +357,12 @@ jobs:
fi
shellcheck --version
# Its own step, and its own shell dialect. The shipped-scripts lint below
# runs --shell=sh with the OpenWrt rc.common exclude set, which misfires
# on a bash script; install-nak.sh is also not shipped in the package.
- name: Lint install-nak.sh
# The scripts this package ships, as sh, and install-nak.sh, as bash.
# The guard is the one copy of the lint; ci.yml and ci-local.sh run it
# too, so a branch push sees a finding before it reaches a trunk.
- name: Lint shell scripts
shell: bash
run: shellcheck --shell=bash .github/scripts/install-nak.sh
- name: Lint shipped shell scripts
shell: bash
run: |
set -euo pipefail
FILES_DIR=packaging/openwrt-ipk/files
# Scripts shipped inside the .ipk. The init scripts use the OpenWrt
# `#!/bin/sh /etc/rc.common` shebang; tell shellcheck to treat them
# as POSIX sh and silence the unrecognized-shebang warning (SC1008).
# SC2317 (unreachable command) fires on rc.common's externally-invoked
# start_service/stop_service/reload_service hooks.
TARGETS=(
"$FILES_DIR/etc/init.d/fips"
"$FILES_DIR/etc/init.d/fips-gateway"
"$FILES_DIR/etc/fips/firewall.sh"
"$FILES_DIR/etc/hotplug.d/net/99-fips"
"$FILES_DIR/etc/uci-defaults/90-fips-setup"
"$FILES_DIR/usr/bin/fips-mesh-setup"
"$FILES_DIR/usr/bin/fips-ap-setup"
)
fail=0
for f in "${TARGETS[@]}"; do
if [ ! -f "$f" ]; then
echo "FAIL: missing $f"
fail=1
continue
fi
echo "==> shellcheck $f"
if shellcheck --shell=sh --exclude=SC1008,SC2317,SC2034,SC3043,SC2086,SC2089,SC2090 "$f"; then
echo " PASS"
else
echo " FAIL"
fail=1
fi
done
if [ "$fail" -ne 0 ]; then
echo "shellcheck FAILED"
exit 1
fi
echo "shellcheck PASS (${#TARGETS[@]} scripts)"
run: bash testing/check-shellcheck.sh
- name: Sysctl drop-in syntax check
shell: bash
+65
View File
@@ -355,6 +355,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
the release. The tarball, artifact and `.deb` file names keep the tag's
`-rcN`.
#### Native datagram API
- An accepted native API flow that its program closes without ever sending on
now stays open, holding its port and a slot against
`node.native_api.max_flows`, until its listener is closed.
#### OpenWrt
- Release-candidate OpenWrt `.ipk` packages are now versioned `vX.Y.Z~rcN`, so
opkg sorts them below the final release and the release upgrades a router
that ran the candidate. The package file name keeps the tag's `-rcN`.
#### Windows
- `install-service.ps1` stops when `\etc\fips\fips.key` exists on the system
@@ -543,6 +555,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
through `Requires=fips.service`, then restarted only fips, so `.fips`
resolution stayed down and the gateway stayed stopped until started by hand.
#### macOS
- If an encrypt worker thread exits, the daemon no longer stops once that
worker's send queue fills. Packets for that worker are now dropped instead
of blocking forever. This applies to the default sender.
#### Native datagram API
- On macOS, a flow accepted through the native API could arrive already
closed, losing the peer's first datagram, when the kernel's descriptor
garbage collector ran before the client read the arrival message. The same
exposure on connect and listen replies is closed too.
#### OpenWrt
- dnsmasq forwards `.fips` to fips-gateway only while the gateway is
@@ -555,6 +580,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
stopped, keeps the gateway's state across an upgrade, and keeps an edited
`/etc/fips/fips.yaml`. The SDK's package scan also no longer stops on the
`Makefile`'s architecture check, which had kept the package out of the build.
- The shipped config and README now agree that the LAN Ethernet transport
binds the LAN bridge (`br-lan`). A socket on a bridge member port does not
receive FIPS frames, whether or not br_netfilter is loaded. If you followed
the earlier README and changed the `lan` entry in `/etc/fips/fips.yaml` to a
member port (for example `lan1` or `eth1`), change it back to `br-lan`. Your
edited config is kept across upgrades, along with its old "physical port
names, NOT bridge names" comment, so an upgrade alone will not correct it.
#### Sessions and rekey
@@ -596,6 +628,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Security
#### Links and transports
- An inbound TCP or Tor onion connection is now dropped when it goes longer
than the node's own link-liveness bound without delivering a complete frame.
The bound is about 64 seconds at default settings. A remote that sends one
frame and then goes silent no longer holds an inbound connection slot
indefinitely. The bound follows `node.heartbeat_interval_secs`,
`node.link_dead_timeout_secs`, `node.tick_interval_secs` and the handshake
resend settings.
#### Routing and discovery
- A single forged or reflected `PathBroken` signal no longer deletes
coordinates a node verified by lookup. A verified entry is kept while a
fresh lookup re-validates it. It is demoted to an unverified hint only when
`PathBroken` signals naming the destination arrive over two different links
within 15 seconds. The vote is the authenticated link peer the signal
arrived over, not the reporter it names, so a sender on one link cannot
reach the quorum by inventing reporters. Forged reports that arrive over two
different links still demote the entry. The re-lookup now runs on every such
signal, and when the destination's identity is not cached the node first
caches it from the session's key so the answer can be verified. New
error-signal counters `broken_below_quorum`, `broken_demoted`,
`broken_link_mismatch` and `broken_reporter_mismatch` appear in
`show_routing`; the last two only count and never refuse a signal.
#### Sessions and rekey
- A copy of a peer's link rekey msg1 can no longer stop link key rotation. A
@@ -617,6 +675,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
address arrives. A msg1 from a node this one holds no link with, or one
that carries a different startup epoch, still starts a new link and is
answered at its source, as any new connection is.
- The SHA-256 and HMAC states used by the Noise handshake are now cleared when
dropped. The connection's handshake slot is cleared when a handshake
completes, and its session slot when a rekey session is taken out. The
handshake keypair is erased in place on every early return from starting a
handshake. The security reference now states what clearing key material in
memory does and does not cover in a release build, including copies left
behind by moves and the identity loaded from a secret string.
## [0.5.2] - 2026-09-28
Generated
+3
View File
@@ -333,6 +333,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa"
dependencies = [
"hybrid-array",
"zeroize",
]
[[package]]
@@ -1028,6 +1029,7 @@ dependencies = [
"const-oid",
"crypto-common 0.2.2",
"ctutils",
"zeroize",
]
[[package]]
@@ -1175,6 +1177,7 @@ dependencies = [
"futures",
"hex",
"hkdf",
"hmac 0.13.0",
"libc",
"libm",
"mdns-sd",
+5 -1
View File
@@ -22,8 +22,12 @@ profiling = []
[dependencies]
ratatui = "0.30"
secp256k1 = { version = "0.30", features = ["rand", "global-context"] }
sha2 = "0.11"
# The `zeroize` features clear the SHA-256 and HMAC states on drop. `hmac`'s
# feature only forwards to `digest`'s, which `sha2`'s also turns on; it is
# listed so the HMAC inside `hkdf` keeps it if `sha2`'s ever stops doing so.
sha2 = { version = "0.11", features = ["zeroize"] }
hkdf = "0.13"
hmac = { version = "0.13", features = ["zeroize"] }
ring = "0.17"
libm = "0.2"
zeroize = { version = "1.9", features = ["zeroize_derive"] }
+40 -13
View File
@@ -272,18 +272,28 @@ follows the same path as the request. Greedy tree routing toward the
`origin_coords` is used only as a fallback if the reverse-path entry has
expired.
**Originator check first**: a node tests its own outstanding lookups before
consulting `recent_requests`. A request is flooded to every bloom-matching
tree peer, and a bloom false positive can send a copy out into the wider
network and back to the originator, which would otherwise file its own
`request_id` as an ordinary transit entry and relay its own answer away. The
originator arm therefore wins: a response naming a target with a lookup
**Originator check**: on the response path, a node tests its own outstanding
lookups before consulting `recent_requests`. A request is flooded to every
bloom-matching tree peer, and a bloom false positive can send a copy out into
the wider network and back to the originator, which would otherwise file its
own `request_id` as an ordinary transit entry and relay its own answer away.
The originator arm therefore wins: a response naming a target with a lookup
outstanding, carrying a `request_id` that lookup issued, is accepted here
whatever the dedup cache holds. A returning copy of the request is likewise
dropped rather than recorded, so the originator's id never enters the transit
cache. The drop is counted as `req_own_loopback` rather than as a duplicate: a
returning copy has a nonzero floor in healthy operation and says nothing about
the peer that delivered it.
whatever the dedup cache holds. A returning copy of the request is dropped
rather than recorded, so while the check recognises the id, it never enters the
transit cache. The drop is counted as `req_own_loopback` rather than as a
duplicate, because the cause differs: the node's own fan-out returning, not a
request id it has already recorded arriving again. Neither counter identifies
the peer that delivered the copy. The request path runs the two tests the other
way round, the dedup test first, and the order is immaterial there: an id the
originator check recognises is never in `recent_requests`, because the
originator records the ids it issues only in its pending lookups and a request
is recorded only after it has passed the check. Separately, the check
recognises an id only while its lookup is outstanding and the id is among the
last `MAX_RECORDED_IDS` (eight) that the lookup's retry ladder issued for that
target. A copy that returns after the lookup completed or timed out, or a copy
of an older attempt on a longer ladder, is recorded and forwarded as transit,
and a later copy of it is dropped as a duplicate.
**Response-forwarded flag**: Each `recent_requests` entry tracks whether a
response has already been forwarded for that `request_id`. If a second
@@ -372,10 +382,27 @@ source.
1. Immediately send a standalone CoordsWarmup (0x14) message (rate-limited,
same per-destination interval as CoordsRequired response)
2. Remove stale coordinates from cache
3. Initiate discovery for the destination
2. Handle the cached coordinates by where they came from. Unverified
coordinates (a hint copied off a passing packet, or a lookup result
whose verification has aged out) are removed. Coordinates a lookup
verified are kept while discovery re-validates them, because the signal
is unauthenticated and removing them would let the next forged warm
replace them. They are demoted to an unverified hint, keeping their value,
only when PathBroken signals naming the destination arrive over two
different links within 15 seconds. The vote is the authenticated link
peer, not the reporter the signal names, which the sender chooses. A node
whose signals all arrive over one link never demotes this way; its
verified coordinates last until discovery replaces them or their
verification ages out after 300 seconds.
3. Initiate discovery for the destination. If its identity is not cached,
cache it first from the session's key, so the response can be verified
4. Reset CP warmup counter
The source also counts, without refusing anything, a PathBroken that
arrives over a link other than its forward link to the destination, and one
whose reporter is not closer to the destination than the source is. Both
have a non-zero healthy floor.
### MtuExceeded
**Trigger**: A transit node receives a SessionDatagram but the total
+6 -3
View File
@@ -133,9 +133,12 @@ and silent drops on its listeners.
**Not a connection in the TCP sense.** A successful `connect` is a local
registration and contacts no peer. There is no handshake, no keepalive and no
notification that a peer went away. A flow ends when its descriptor closes, and
in no other way. In particular **a peer cannot end your flow: it has no close to
send.** That single fact shapes every program written against this interface,
and the consequences are drawn out in
in no other way. The one delay is an accepted flow never sent on, which ends
only once its listener has closed as well (see
[../reference/native-api.md](../reference/native-api.md#fipslistener)). In
particular **a peer cannot end your flow: it has no close to send.** That
single fact shapes every program written against this interface, and the
consequences are drawn out in
[../how-to/use-the-native-datagram-api.md](../how-to/use-the-native-datagram-api.md#four-things-that-will-bite-you).
## See also
@@ -227,6 +227,16 @@ leaves the flows already accepted from it untouched. A program that parks
streams in a `Vec` and never removes them holds ports and flow slots
exactly as if it had leaked descriptors.
**An accepted flow you drop without ever sending on stays open until you
drop its listener.** Until your program has sent on an accepted flow, the
daemon keeps its own copy of the flow's descriptor, because on macOS the
kernel can otherwise destroy the flow while its descriptor is still on the
way to you. Your first `send` on the flow, or dropping the listener, lets
that copy go. So a server that refuses flows by dropping them unanswered
holds a port and a flow slot for each one until its listener goes, and a
long-lived listener that refuses many flows can walk the node into its flow
ceiling.
**Nothing peer-driven ever ends a flow, so your program has to.** The v1
wire carries no half-close. Nothing closes the daemon's half of a live
accepted flow, so a loop written as "echo until the flow closes", or one
+10 -2
View File
@@ -124,7 +124,7 @@ one producer on the surface, which is what lets a caller read it.
## Step 6: Keep descriptor hygiene
Five rules. Each one leaks a flow or loses one when broken.
Six rules. Each one leaks a flow or loses one when broken.
**Request close-on-exec** with `MSG_CMSG_CLOEXEC` on the `recvmsg`, rather than
setting it afterwards. Without it the descriptor survives an `exec` into a
@@ -140,7 +140,15 @@ descriptors rather than dropping them on the floor.
**Lift the descriptor out of an arrival you cannot parse** before discarding
the message. Refusing a flow is closing its descriptor; discarding the message
without taking it leaks the flow instead.
without taking it leaks the flow instead. A refused flow ends only when the
listener closes, though, unless you wrote on it first: the daemon keeps its
own copy of an accepted flow's descriptor until your first write or the
listener's close.
**Close the setup connection once you have the reply.** The daemon keeps its
own copy of the descriptor in its last reply until your next command on that
connection or the connection's close. A flow or listener you close while the
connection sits idle stays open until one of those happens.
**Bound the partial line.** A daemon that stopped sending newlines would
otherwise grow your buffer without end. The shipped client caps it at 64 KiB,
+34 -3
View File
@@ -195,7 +195,10 @@ That is what lets the blanket reference implementation cover `&str` and
One datagram flow, and the descriptor it rides on. A flow is an exact match of
both ends and both ports. **The descriptor is the flow**: it lives while a
process holds that descriptor and ends when the last one closes it.
process holds that descriptor and ends when the last one closes it. The one
exception is an accepted flow that has never been sent on: the daemon keeps
its own copy of that descriptor until the first `send` or until the listener
is dropped. See `accept` below.
`Send + Sync + 'static`, with no `Arc` and no borrow. There is **no `Clone` and
no `try_clone`**. Because `send` and `recv` both take `&self`, a shared borrow
@@ -340,6 +343,15 @@ no other way to refuse one. An unparseable arrival is therefore reported only
after the descriptor it carried has been taken into ownership, so a parse
failure refuses the flow rather than leaking it.
**A refused flow stays open until its listener is dropped**, unless the
program sent on it first. Until the first `send` on an accepted flow, the
daemon keeps its own copy of the flow's descriptor, because on macOS the
kernel can otherwise destroy a socket whose descriptor is still in an unread
arrival. That copy goes at the first `send`, when the listener is dropped, or
when the flow ends any other way, and the flow then ends with the program's
own close. Until then a dropped flow holds its port and its slot against
`max_flows`.
**`incoming()`** returns an `Incoming<'_>`, which borrows the listener for the
iterator's lifetime, so the listener cannot be moved or dropped mid-iteration.
@@ -360,7 +372,8 @@ none either. A bounded accept is `set_nonblocking` plus a wait of the caller's
own on the descriptor.
**Dropping** closes the descriptor and unbinds the port. Flows already accepted
from it are untouched; flows still pending on it go with it.
from it and still held are untouched; flows still pending on it go with it, and
so do flows accepted from it and dropped without ever being sent on.
### Incoming
@@ -613,6 +626,20 @@ own half non-blocking and leaves the client's half blocking. `SOCK_SEQPACKET`
is what preserves message boundaries in both directions, which is why the
payload needs no framing.
**The daemon keeps a copy of the client's half after sending it.** While a
descriptor sits unread in a message, the message can be its only reference,
and the macOS kernel's descriptor collector destroys a socket in that state:
the client then receives a flow that reads as end of file with its datagrams
gone. So the daemon keeps its copy until one of these:
- For a `connect` or `listen` reply, at the client's next command on the same
connection, or when that connection closes.
- For an arrival on a listener, at the client's first write on the flow, when
the listener closes, or when the flow ends any other way.
A flow or listener the client closes before then ends when the daemon's copy
goes, not at the client's close.
A refused `connect` leaves the port free: the socket pair is built before the
port is claimed, so a failure to build it needs no rollback.
@@ -626,7 +653,11 @@ returning.
**The connection owns nothing.** Closing it releases no flow and no listener,
and a descriptor kept across the close keeps working. What owns the flow is the
descriptor.
descriptor. The connection does delay one thing: a descriptor from its last
reply that the client closes while the connection is still open, with no
further command sent, stays open until the next command or the connection's
close (see Passing the descriptor). The shipped client closes the connection
as soon as it has the reply, so it never meets this.
## Command reference
+86 -9
View File
@@ -1,9 +1,10 @@
# Security Reference
Consolidated security reference covering the nftables baseline, peer
ACL file format, cryptographic primitives, rekey defaults, replay
window, filesystem permissions, threat-resistance matrix, and default
network exposures per transport. For the threat-model design and
ACL file format, cryptographic primitives, key material in memory,
rekey defaults, replay window, filesystem permissions,
threat-resistance matrix, and default network exposures per
transport. For the threat-model design and
rationale, see [../design/fips-security.md](../design/fips-security.md).
For the operator activation steps and drop-in recipes, see
[../how-to/enable-mesh-firewall.md](../how-to/enable-mesh-firewall.md).
@@ -86,6 +87,68 @@ Domain separation and DH binding survive through the chaining key `ck`, which
`h` is maintained at every step and is never fed to the AEAD, so it binds
nothing.
## Key Material in Memory
The daemon clears the copies of secret material that its own code
holds once they are no longer needed: the node's long-term private
key when the identity is dropped, the static and ephemeral keypairs a
Noise handshake holds, the chaining key and handshake hash, the
per-message Diffie-Hellman results, the key-derivation outputs and the
two session keys derived from them, the retained key on each cipher
state, the bech32 and hex encodings of a secret, and the configuration
text that carries `node.identity.nsec`. The SHA-256 state that hashes
each Diffie-Hellman result and the HMAC states inside HKDF clear
themselves on drop, through the opt-in `zeroize` features of `sha2`
and `hmac`, which the daemon turns on. A completed session keeps its
own copy of the handshake hash and does not clear it, on purpose:
nothing derives a key from it, and the session hands it out to any
caller.
Each erase is a volatile write, and the optimiser does not remove a
volatile write as a dead store. `secp256k1`'s erase follows the write
with a compiler fence, and `zeroize`'s with an optimisation barrier (an
empty `asm!` block on x86_64); those limit how the compiler may reorder
code around the write, but it is the volatile write that keeps the
store. This was checked against generated code rather than assumed: in
an x86_64 release build (Rust 1.94.1, `secp256k1` 0.30.0, `zeroize`
1.9.0), every erase in the Noise handshake and identity code that is
linked into the daemon is present as stores in the machine code.
An erase reaches only the place it is called on. What it does not
reach:
- **Copies left by moves.** Moving a value copies its bytes and leaves
the old bytes where they were. A handshake state is built on the
stack and moved several times between being created and being
dropped. Each of those moves leaves behind, in a stack frame that is
no longer in use, a copy of the node's long-term private key and,
once the handshake has started, of its ephemeral key and chaining
key. Two moves out of the slots on a connection's control machine
are cleared: when a completed handshake leaves the slot that held it,
and when a session is taken out of its slot for a rekey, the slot is
overwritten as the value leaves. Other moves are not. When a
connection is promoted to an active peer, or reaped as stale, its
whole handshake state is moved off its control machine, which leaves
the session's two traffic keys, or an unfinished handshake's private
keys, in the heap memory the machine occupies.
- **Loading the identity from a secret string.** Building the node's
identity from its key file or from `node.identity.nsec` leaves
copies of the private key, among them a whole intermediate identity,
in that constructor's stack frame, and they are not cleared.
- **Registers and spilled temporaries**, which no code in the daemon
can name.
- **Library state.** The cipher keys cached inside `ring`'s
`LessSafeKey` have no clearing route. The daemon cannot clear the
internal temporaries of the `libsecp256k1` C library either; the
library clears some of its own, such as the nonce and secret scalar
used in signing, on a best-effort basis.
Clearing therefore shortens how long secret material stays in memory
and removes it from the places the daemon's own code keeps it; it does
not guarantee that a secret is gone from the process. Reading what
remains requires access to the daemon's memory, or to a core dump or
swap image of it.
## Rekey Defaults
Both link-layer and session-layer Noise sessions rekey under one of
@@ -245,12 +308,14 @@ machine.
**The file descriptor carries the grant, not the connection.** A setup call
hands the client a socket descriptor and the connection it was made on is then
closed; the flow or the held port lives until that descriptor is closed. A
descriptor is an ordinary kernel object, so it survives `fork`, survives
`exec` unless the client asked for it close-on-exec when it received it, and
can be handed to another process over `SCM_RIGHTS`. A process holding one can
send as this node on that flow, or receive on that port, without ever opening
the API socket and without being in the `fips` group.
closed; the flow or the held port lives until that descriptor is closed and
the daemon has let go of the copy it keeps while the descriptor is being
handed over. A descriptor is an ordinary kernel object, so it survives
`fork`, survives `exec` unless the client asked for it close-on-exec when it
received it, and can be handed to another process over `SCM_RIGHTS`. A
process holding one can send as this node on that flow, or receive on that
port, without ever opening the API socket and without being in the `fips`
group.
Nothing revokes a descriptor already handed out. Restarting the daemon closes
its own halves and ends every flow and listener at once, and that is the only
revocation there is.
@@ -269,6 +334,18 @@ peer had sent it, reaching any listener on this node under any peer identity
the caller names. Leave it off outside a test harness; a packaged node does
not enable it.
**A remote peer can fill the node's flow ceiling through a server that
refuses flows by dropping them.** Until a program first sends on a flow it
accepted, the daemon keeps its own copy of that flow's descriptor, so a flow
accepted and dropped unanswered keeps its slot against the node-wide
`node.native_api.max_flows` until its listener is dropped. A peer that opens
flows to such a listener from many source ports can therefore exhaust the
ceiling, and every other program on the node then gets `EMFILE` on `connect`
and silently loses arrivals on its listeners. This is the accepted cost of
keeping a flow alive while its descriptor is on the way to the program; see
[../how-to/use-the-native-datagram-api.md](../how-to/use-the-native-datagram-api.md)
for what releases the daemon's copy.
The socket is local only. It is not reachable over the network, and nothing
about it changes the mesh's own authentication: a peer still verifies the
node's signature, which is precisely why a local caller that can send through
+3 -1
View File
@@ -118,7 +118,9 @@ define Package/fips/install
# Firewall helper script (called by UCI include and hotplug)
$(INSTALL_BIN) $(CURDIR)/files/etc/fips/firewall.sh $(1)/etc/fips/firewall.sh
# sysctl: enable br_netfilter so AF_PACKET sees frames on bridge member ports
# sysctl: turn off br_netfilter's call hooks (fips-bridge.conf). br_netfilter
# does not make bridge member ports usable for the Ethernet transport; the
# shipped config binds the LAN bridge instead. See fips-bridge.conf.
$(INSTALL_DIR) $(1)/etc/sysctl.d
$(INSTALL_DATA) $(CURDIR)/files/etc/sysctl.d/fips-bridge.conf $(1)/etc/sysctl.d/fips-bridge.conf
$(INSTALL_DATA) $(CURDIR)/files/etc/sysctl.d/fips-gateway.conf $(1)/etc/sysctl.d/fips-gateway.conf
+19 -7
View File
@@ -17,7 +17,7 @@ OpenWrt 22.03+ router via the standard `opkg` package system.
| `/etc/init.d/fips-gateway` | procd service for the gateway (disabled by default) |
| `/etc/fips/fips.yaml` | Node configuration (edit before first start) |
| `/etc/fips/firewall.sh` | Firewall helper — accepts traffic on `fips0` |
| `/etc/sysctl.d/fips-bridge.conf` | `br_netfilter` settings for Ethernet transport |
| `/etc/sysctl.d/fips-bridge.conf` | Turns off `br_netfilter`'s firewall call hooks |
| `/etc/sysctl.d/fips-gateway.conf` | `proxy_ndp` and IPv6 forwarding for the gateway |
| `/etc/hotplug.d/net/99-fips` | Applies firewall rules when `fips0` comes up |
| `/etc/uci-defaults/90-fips-setup` | First-boot kernel module, firewall and dnsmasq `.fips` forwarding setup |
@@ -38,7 +38,7 @@ OpenWrt 22.03+ router via the standard `opkg` package system.
| Requirement | Notes |
|---|---|
| `kmod-tun` | Required for `fips0` TUN interface |
| `kmod-br-netfilter` | Required for Ethernet transport on bridge member ports |
| `kmod-br-netfilter` | Loaded, hooks off (`fips-bridge.conf`); see notes below |
Both kernel modules are listed as package dependencies (`DEPENDS`) and will be
installed automatically by `opkg`.
@@ -141,11 +141,23 @@ The default config enables:
- Ethernet transport, including the `wan`, `wwan` and `lan` entries
For Ethernet transport, edit the interface names in the `ethernet:` section to
match your router. **Always use physical port names
(`eth0`, `eth1`, or DSA port names like `wan`/`lan1`), never bridge names
(`br-lan`).** The shipped default WAN port is `eth0` (OpenWrt 24); on OpenWrt
25 (DSA) boards the WAN port is named `wan` — the `.apk` package ships that
default. Run `ip link show` to confirm the names on your board.
match your router. **For the LAN, bind the LAN bridge (`br-lan`), never one of
its member ports** (on DSA boards `lan1`..`lanN`; on others whichever `ethN` the
bridge holds; `bridge link` lists them). A socket on a bridge member port sends
frames but never forms a link, because the bridge takes the frames that arrive
on its members; loading `br_netfilter` does not change that.
A lab test with a two-member Linux bridge found this with `br_netfilter`
unloaded, loaded with its call hooks off, and loaded with them on, while a
socket on `br-lan` worked both with `br_netfilter` unloaded and with it loaded
as shipped. Ports outside any bridge bind by their own name. The shipped
default WAN port is `eth0` (OpenWrt 24); on OpenWrt 25 (DSA) boards the WAN
port is named `wan`, and the `.apk` package ships that default. Run
`ip link show` to confirm the names on your board.
The lab used software bridges only. A DSA switch with hardware bridge offload
has not been checked, so confirm the LAN entry forms links on such a router.
`kmod-br-netfilter`, `fips-bridge.conf` and the module load in `90-fips-setup`
are kept until their removal has been checked on a router.
## Service management
+13 -2
View File
@@ -14,7 +14,13 @@
# arm 32-bit ARM routers (Cortex-A7)
# x86_64 x86 routers / VMs
#
# Output: dist/fips_<version>_<openwrt-arch>.ipk
# Output: dist/fips_<PKG_VERSION>_<openwrt-arch>.ipk
#
# Environment:
# PKG_VERSION label for the file name (default: git describe)
# IPK_VERSION control file Version (default: PKG_VERSION). CI passes a
# release candidate as vX.Y.Z~rcN, which opkg sorts below the
# release, while PKG_VERSION keeps the tag's vX.Y.Z-rcN.
#
# Prerequisites:
# cargo install cargo-zigbuild
@@ -86,8 +92,13 @@ DIST_DIR="$PROJECT_ROOT/dist"
PKG_NAME="fips"
PKG_VERSION="${PKG_VERSION:-$(cd "$PROJECT_ROOT" && git describe --tags --always --dirty 2>/dev/null || echo "0.1.0")}"
IPK_VERSION="${IPK_VERSION:-$PKG_VERSION}"
if [ "$IPK_VERSION" = "$PKG_VERSION" ]; then
echo "==> Building $PKG_NAME $PKG_VERSION for $OPENWRT_ARCH ($RUST_TARGET)"
else
echo "==> Building $PKG_NAME $PKG_VERSION (control Version $IPK_VERSION) for $OPENWRT_ARCH ($RUST_TARGET)"
fi
# ---------------------------------------------------------------------------
# 1. Obtain binaries
@@ -192,7 +203,7 @@ PKG_SIZE=$(du -sk "$DATA_DIR" | cut -f1)
cat > "$CONTROL_DIR/control" <<EOF
Package: $PKG_NAME
Version: $PKG_VERSION
Version: $IPK_VERSION
Architecture: $OPENWRT_ARCH
Maintainer: FIPS Network
Section: net
@@ -90,8 +90,14 @@ transports:
bind_addr: "0.0.0.0:8443"
# advertise_on_nostr: true
# Ethernet transport — physical port names, NOT bridge names.
# Run 'ip link show' on the router to identify port names.
# Ethernet transport. Each entry names one interface; run 'ip link show'
# on the router to see the names. For the LAN, bind the LAN bridge
# (br-lan), never one of its member ports: a socket on a bridge member
# port sends frames but never forms a link, because the bridge takes the
# frames that arrive on its members, whether or not br_netfilter is
# loaded. Ports outside any bridge (the WAN port, phy0-sta0) bind by
# their own name. A DSA switch with hardware bridge offload has not been
# checked and needs a check on the router.
ethernet:
wan:
interface: "eth0"
@@ -185,11 +185,15 @@ dns_locked() {
# is among the process's open descriptors. Another process holding the port,
# such as an mDNS responder on 5353, does not count.
gateway_dns_held_by() {
local hex inode fd
local hex inodes inode fd
[ -n "$2" ] || return 1
hex="$(printf '%04X' "$1")"
for inode in $(cat /proc/net/udp /proc/net/udp6 2>/dev/null |
awk -v want=":$hex" 'substr($2, length($2) - 4) == want { print $10 }'); do
# cat rather than awk's own file arguments: busybox awk gives up on a
# missing /proc/net/udp6. A while-read loop on the pipe would run in a
# subshell under ash, where the return below could not leave this function.
inodes="$(cat /proc/net/udp /proc/net/udp6 2>/dev/null |
awk -v want=":$hex" 'substr($2, length($2) - 4) == want { print $10 }')"
for inode in $inodes; do
for fd in /proc/"$2"/fd/*; do
[ "$(readlink "$fd" 2>/dev/null)" = "socket:[$inode]" ] && return 0
done
@@ -1,12 +1,17 @@
# FIPS: bridge netfilter settings
#
# kmod-br-netfilter must be loaded for AF_PACKET sockets to receive frames
# on bridge member ports (e.g. eth1 when it is a member of br-lan).
# Without it, the bridge's rx_handler intercepts frames before they reach
# the packet socket layer.
# The package loads br_netfilter (kmod-br-netfilter) and this file turns
# off its IP/IPv6/ARP call hooks, so frames the bridge forwards are not
# also run through the firewall's IP, IPv6 and ARP tables.
#
# We load br_netfilter for the AF_PACKET visibility benefit but disable its
# IP/IPv6/ARP call hooks to avoid double-processing of routed traffic.
# Loading br_netfilter does not make a bridge member port usable for the
# FIPS Ethernet transport. A lab test with a two-member Linux bridge found
# that a socket bound to a member port never forms a link, with
# br_netfilter unloaded, loaded with these hooks off, or loaded with them
# on. A socket bound to the bridge itself (br-lan) works, both with
# br_netfilter unloaded and with it loaded as this file sets it.
# Bind the bridge. The module, its dependency and this file stay until
# their removal has been checked on a router.
net.bridge.bridge-nf-call-iptables=0
net.bridge.bridge-nf-call-ip6tables=0
net.bridge.bridge-nf-call-arptables=0
@@ -18,7 +18,10 @@
modprobe tun 2>/dev/null || true
echo "tun" > /etc/modules.d/tun
# kmod-br-netfilter makes AF_PACKET visible on bridge member ports.
# br_netfilter is loaded with its call hooks off (fips-bridge.conf). It does
# not make a bridge member port usable for the Ethernet transport: a lab
# test found a member-port socket never forms a link either way, so the
# shipped config binds the LAN bridge. Its removal waits on a router check.
modprobe br_netfilter 2>/dev/null || true
echo "br_netfilter" > /etc/modules.d/br-netfilter
+69
View File
@@ -247,6 +247,27 @@ impl CoordCache {
self.entries.remove(addr)
}
/// Demote an entry to an unverified hint, keeping its coordinates and TTL.
///
/// The entry goes on routing, but a hint may now replace it. Returns
/// whether an entry existed.
pub fn demote(&mut self, addr: &NodeAddr) -> bool {
match self.entries.get_mut(addr) {
Some(entry) => {
entry.mark_hint();
true
}
None => false,
}
}
/// Forget the path MTU stored with an entry, keeping the entry.
pub fn clear_path_mtu(&mut self, addr: &NodeAddr) {
if let Some(entry) = self.entries.get_mut(addr) {
entry.clear_path_mtu();
}
}
/// Check if an address is cached (and not expired).
pub fn contains(&self, addr: &NodeAddr, current_time_ms: u64) -> bool {
self.get(addr, current_time_ms).is_some()
@@ -844,4 +865,52 @@ mod tests {
assert!(cache.contains(&make_node_addr(1), 10));
assert!(cache.contains(&make_node_addr(2), 10));
}
#[test]
fn demote_keeps_the_value_and_lets_a_hint_replace_it() {
let mut cache = CoordCache::new(100, 1000);
let addr = make_node_addr(1);
let real = make_coords(&[1, 0]);
cache.insert_verified_with_path_mtu(addr, real.clone(), 10, 1400);
assert_eq!(
cache.insert(addr, make_coords(&[1, 2, 0]), 10),
HintOutcome::Rejected
);
assert!(cache.demote(&addr));
let entry = cache.get_entry(&addr).unwrap();
assert_eq!(entry.coords(), &real);
assert_eq!(entry.source(), crate::cache::CoordSource::Hint);
assert!(!entry.is_verified(10));
assert_eq!(
entry.path_mtu(),
Some(1400),
"demote leaves the path MTU to its caller"
);
assert_eq!(
cache.insert(addr, make_coords(&[1, 2, 0]), 11),
HintOutcome::Changed
);
}
#[test]
fn demote_of_an_absent_entry_reports_none() {
let mut cache = CoordCache::new(100, 1000);
assert!(!cache.demote(&make_node_addr(1)));
assert!(cache.is_empty());
}
#[test]
fn clear_path_mtu_keeps_the_entry() {
let mut cache = CoordCache::new(100, 1000);
let addr = make_node_addr(1);
cache.insert_verified_with_path_mtu(addr, make_coords(&[1, 0]), 10, 1400);
cache.clear_path_mtu(&addr);
let entry = cache.get_entry(&addr).unwrap();
assert_eq!(entry.path_mtu(), None);
assert!(entry.is_verified(10));
}
}
+5
View File
@@ -133,6 +133,11 @@ impl CacheEntry {
self.path_mtu = Some(mtu);
}
/// Forget the path MTU, as when the path it described is released.
pub fn clear_path_mtu(&mut self) {
self.path_mtu = None;
}
/// Check if this entry has expired.
pub fn is_expired(&self, current_time_ms: u64) -> bool {
current_time_ms > self.expires_at
+1 -1
View File
@@ -3238,7 +3238,7 @@ mod tests {
/// renders each equal their on-loop oracle byte-for-byte, and all three are
/// served off-loop via `snapshot_dispatch`.
#[test]
fn snapshot_dispatch_serves_r5_queries() {
fn snapshot_dispatch_serves_acl_and_stats_peer_queries_off_loop_byte_identical() {
use super::super::protocol::Request;
use super::super::read_handle::snapshot_dispatch;
+4
View File
@@ -36,6 +36,10 @@
"resp_unsolicited": 0
},
"error_signals": {
"broken_below_quorum": 0,
"broken_demoted": 0,
"broken_link_mismatch": 0,
"broken_reporter_mismatch": 0,
"coords_required": 0,
"emit_limiter_at_capacity": 0,
"emit_over_dest_interval": 0,
+24 -19
View File
@@ -16,9 +16,13 @@ use super::{FipsAddress, IdentityError, NodeAddr, sha256};
/// The keypair is the node's long-term private key. It is erased when the
/// identity is dropped, and every constructor below erases the intermediate
/// secret it built the identity from. All of that clears the copies this
/// crate owns, not every copy that ever existed: `secp256k1` names its erase
/// non-secure because the compiler may duplicate or move the bytes to places
/// no code here can name.
/// crate owns, not every copy that ever existed. Each erase is a volatile
/// write, so the optimiser keeps it, but it reaches only the place it is
/// called on: a copy made before it runs is not cleared, and that includes
/// the bytes a move leaves behind wherever the value used to be, such as
/// the frame of the constructor that built it. `from_secret_str` is a known
/// case: it leaves copies of the private key, a whole intermediate
/// `Identity` among them, in its own frame.
#[derive(Clone)]
pub struct Identity {
keypair: Keypair,
@@ -141,35 +145,36 @@ impl Drop for Identity {
}
}
/// A `Keypair` copy that is erased when it goes out of scope.
/// Erases a `Keypair` in place when it goes out of scope.
///
/// `Keypair` is `Copy` and so cannot clear itself on drop. A frame that holds
/// a copy of the node's long-term private key across several exit paths —
/// early error returns, `?`, a normal return — would otherwise need an erase
/// written at each one, and a missed path is invisible. Holding the copy here
/// instead makes the clearing structural.
/// written at each one, and a missed path is invisible. Guarding the copy
/// makes the clearing structural.
///
/// This clears the copy this guard owns, not every copy that ever existed:
/// `secp256k1` names its erase non-secure because the compiler may duplicate
/// or move the bytes to places no code here can name.
pub(crate) struct ErasingKeypair(Keypair);
/// The guard borrows the keypair rather than holding one, so making it and
/// moving it copy no key bytes: the erase lands on the caller's own binding,
/// wherever that lies, and there is no second copy for an early return to
/// leave behind. It clears that one binding, not every copy that ever
/// existed. The erase is a volatile write, so the optimiser keeps it, but a
/// copy made before the guard existed, or taken out through
/// [`ErasingKeypair::get`], is not cleared by it.
pub(crate) struct ErasingKeypair<'a>(&'a mut Keypair);
impl ErasingKeypair {
/// Take a copy of `source` into the guard and erase `source` in place, so
/// the caller's own binding does not outlive the move.
pub(crate) fn take(source: &mut Keypair) -> Self {
let guarded = Self(*source);
source.non_secure_erase();
guarded
impl<'a> ErasingKeypair<'a> {
/// Guard `source` so it is erased where it lies when the guard drops.
pub(crate) fn new(source: &'a mut Keypair) -> Self {
Self(source)
}
/// Borrow the guarded keypair.
pub(crate) fn get(&self) -> &Keypair {
&self.0
self.0
}
}
impl Drop for ErasingKeypair {
impl Drop for ErasingKeypair<'_> {
fn drop(&mut self) {
self.0.non_secure_erase();
}
+28
View File
@@ -634,3 +634,31 @@ fn test_identity_debug() {
assert!(!debug.contains("keypair"));
assert!(debug.contains(".."));
}
/// Hold a guard over `keypair` and return early or normally, the way the
/// handshake entry points do.
fn guarded_secret(keypair: &mut Keypair, fail: bool) -> Result<[u8; 32], ()> {
let guard = ErasingKeypair::new(keypair);
if fail {
return Err(());
}
Ok(guard.get().secret_bytes())
}
#[test]
fn test_erasing_keypair_erases_the_callers_binding_in_place_on_every_exit() {
let identity = Identity::generate();
let original = identity.keypair().secret_bytes();
// `non_secure_erase` overwrites a keypair with a fixed dummy whose
// secret is 32 bytes of 0x01.
let erased = [1u8; 32];
assert_ne!(original, erased);
let mut keypair = identity.keypair();
assert_eq!(guarded_secret(&mut keypair, false), Ok(original));
assert_eq!(keypair.secret_bytes(), erased);
let mut keypair = identity.keypair();
assert_eq!(guarded_secret(&mut keypair, true), Err(()));
assert_eq!(keypair.secret_bytes(), erased);
}
+11 -4
View File
@@ -373,7 +373,10 @@ fn expired(error: io::Error) -> io::Error {
///
/// The protocol has no close command. Dropping the stream closes its
/// descriptor, and that is what releases the flow and its local port at the
/// daemon.
/// daemon. An accepted flow that was never sent on is the exception: the daemon
/// keeps its own copy of its descriptor until the first [`FipsStream::send`] or
/// until the listener is dropped, so dropping the stream before either releases
/// nothing yet.
#[derive(Debug)]
pub struct FipsStream {
fd: OwnedFd,
@@ -627,8 +630,9 @@ impl AsFd for FipsStream {
/// **The listener is a descriptor**, which is what makes it pollable: it joins
/// an existing `poll`, `select` or `epoll` loop with no new mechanism, and
/// [`FipsListener::accept`] is one `recvmsg` on it. Dropping the listener closes
/// that descriptor, which unbinds the port; flows already accepted from it are
/// untouched.
/// that descriptor, which unbinds the port; flows already accepted from it and
/// still held are untouched, and those accepted and dropped without ever being
/// sent on end with it.
#[derive(Debug)]
pub struct FipsListener {
fd: OwnedFd,
@@ -676,7 +680,10 @@ impl FipsListener {
/// Refusing a flow is dropping the stream, which closes its descriptor.
/// There is no other way to refuse one, which is why an unreadable arrival
/// message is reported after the descriptor it carried has been taken: the
/// flow is then refused rather than leaked.
/// flow is then refused rather than leaked. A refused flow that was never
/// sent on still holds its port and its slot against the node's flow limit
/// until this listener is dropped, because the daemon keeps its own copy of
/// the descriptor until then.
pub fn accept(&self) -> io::Result<(FipsStream, FipsAddr)> {
let mut buf = [0u8; CHUNK];
let chunk = fdpass::recv(self.fd.as_raw_fd(), &mut buf)?;
+99
View File
@@ -20,6 +20,17 @@
//! every unix so the platforms can be compared without the test itself being a
//! variable.
//!
//! **A second measurement lives here: what the kernel does to a socket that is
//! in flight.** When the daemon hands a flow's descriptor to a listener's
//! client, the descriptor sits inside an `SCM_RIGHTS` message until the client
//! reads it. xnu's descriptor garbage collector takes its roots only from files
//! that are in flight, and treats one whose only references are messages as
//! unreachable unless it is found in the receive buffer of another in-flight
//! socket. The listener's client half is not in flight, so a flow socket whose
//! daemon copy has been closed is flushed by any collection that runs before
//! the client reads it. Linux keeps such a socket. The tests at the end of this
//! file measure that difference directly.
//!
//! `SOCK_CLOEXEC` is deliberately not passed in the type argument, though
//! `super::seqpacket::pair` does pass it. Linux and FreeBSD accept it there and
//! macOS does not, and that difference belongs to the port rather than to this
@@ -464,3 +475,91 @@ fn freebsd_seqpacket_drops_a_zero_length_message_instead_of_delivering_it() {
diagnosis of the stalled FreeBSD runs is wrong."
);
}
/// How long a collection is given to run after it has been queued.
///
/// xnu runs its descriptor collector as an asynchronous thread call, so the
/// close that queues it returns before it has run. A fixed wait is the only
/// handle a test has on it; if the Darwin probe below ever misses, this is the
/// first number to raise.
const COLLECTION_WAIT: std::time::Duration = std::time::Duration::from_millis(100);
/// Give the kernel's descriptor collector a reason to run, then time to run.
///
/// Freeing any `AF_UNIX` socket queues xnu's collector, so a fresh pair closed
/// at once is enough. That is also why the collector can run at any moment on
/// a busy host: every unix socket any process closes queues it. Linux runs its
/// collector here too: in 6.8, the kernel this was read against, closing a unix
/// socket while any descriptor is in flight runs it before the close returns.
/// So the Linux probe below checks that Linux's collector keeps the socket,
/// rather than only that no collector ran.
pub(super) fn provoke_collection() {
let (a, b) = dgram_pair().expect("AF_UNIX SOCK_DGRAM socketpair");
drop(a);
drop(b);
std::thread::sleep(COLLECTION_WAIT);
}
/// Hand a flow's client half across a listener pair the way the daemon does,
/// close the sender's copy, provoke a collection, and read what reaches the
/// receiver.
///
/// Built from the product's own pair type and hand-off code rather than from
/// [`dgram_pair`], so the measurement is of exactly the sockets the daemon
/// uses. Returns the first read on the received descriptor: the held bytes if
/// the socket survived, zero bytes if the collector flushed it.
#[cfg(any(target_os = "linux", target_os = "macos"))]
fn read_after_collection_in_flight() -> io::Result<usize> {
use super::{fdpass, seqpacket};
use std::os::fd::AsFd;
let (daemon, flow) = seqpacket::pair()?;
let (sender, receiver) = seqpacket::pair()?;
assert_eq!(send(daemon.as_raw_fd(), b"held")?, 4);
fdpass::try_send(sender.as_raw_fd(), b"arrival", Some(flow.as_fd()))?;
// The message is now the only reference to the flow's client half.
drop(flow);
provoke_collection();
let mut buf = [0u8; 64];
let chunk = fdpass::recv(receiver.as_raw_fd(), &mut buf)?;
let flow = chunk
.fd
.ok_or_else(|| io::Error::other("the message arrived without its descriptor"))?;
recv(flow.as_raw_fd(), &mut buf)
}
/// Darwin flushes an in-flight socket whose only reference is the message.
///
/// This is the defect behind the native API's intermittent macOS failures, in
/// isolation: the listener's client receives a flow descriptor that reads as
/// end of file, with the datagram written to it before the hand-off gone.
/// A failure here means the collector did not run within
/// [`COLLECTION_WAIT`], or that Darwin no longer collects such a socket. In
/// the second case the daemon's hold on a handed-over descriptor is no longer
/// needed there.
#[cfg(target_os = "macos")]
#[test]
fn darwin_collects_an_in_flight_socket_whose_only_reference_is_the_message() {
let read = read_after_collection_in_flight();
assert!(
matches!(read, Ok(0)),
"expected the collector to flush the in-flight flow socket, so its first \
read returns end of file; got {read:?}"
);
}
/// Linux keeps the same socket through the collection the close provokes: a
/// queue held by a socket that is not in flight counts as a reference to what
/// it holds.
#[cfg(target_os = "linux")]
#[test]
fn linux_keeps_an_in_flight_socket_whose_only_reference_is_the_message() {
let read = read_after_collection_in_flight();
assert!(
matches!(read, Ok(4)),
"expected the in-flight flow socket to survive with its datagram; got {read:?}"
);
}
+526 -39
View File
@@ -18,7 +18,20 @@
//! replies and then has no further part in anything it opened. A flow lives
//! until its own descriptor reaches end of file and a listener until its own
//! does, whichever task holds them, which is what makes a descriptor this API
//! hands back behave like one a syscall would have.
//! hands back behave like one a syscall would have. The one exception: the
//! connection keeps a copy of the descriptor in its last reply until the
//! client's next command or its close, for the same reason a listener keeps a
//! copy of a flow it hands over (below). The shipped client closes the
//! connection as soon as it has the reply, so for it the copy is gone at once.
//!
//! **A listener keeps a copy of a flow it handed over.** While a flow's
//! descriptor sits unread in an arrival message, the message can be the only
//! reference to it, and xnu's descriptor collector flushes a socket in that
//! state. So `hand_over` keeps the daemon's copy until the client has written
//! on the flow or closed the listener, and a flow then reaches end of file once
//! both the client and that copy have let go. The cost is that a client which
//! accepts a flow and closes it without writing leaves it open until the
//! listener closes. See `dgram_probe.rs` for the measurement.
//!
//! **The wire is connected.** A datagram a client writes leaves this node over
//! FSP, and one arriving on a held port reaches its flow. `max_payload` is the
@@ -233,6 +246,15 @@ mod unix_impl {
/// writes into.
sock: Arc<Seqpacket>,
counts: Arc<Counts>,
/// The daemon's copy of the client's half, kept while that half may
/// still be in flight to a listener's client.
///
/// Holding it keeps the socket reachable from outside the message that
/// carries it, which is what stops xnu's collector flushing it before
/// the client reads the arrival. `None` once released, and always for
/// a connected flow, whose descriptor went back in an RPC reply and is
/// held by the connection that sent it instead; see `Connection::run`.
pin: Option<OwnedFd>,
}
/// What `stats` reports about one flow.
@@ -283,6 +305,31 @@ mod unix_impl {
self.table().remove(&id);
}
/// Let go of the daemon's copy of a flow's client half.
///
/// The copy is dropped after the lock is released: if the client has
/// already closed its own, this is the last reference, and the close it
/// causes is the flow's end of file.
fn unpin(&self, id: u64) {
let pin = self.table().get_mut(&id).and_then(|flow| flow.pin.take());
drop(pin);
}
/// Let go of the daemon's copy of every flow on a local port.
///
/// Called when a listener ends. Every pinned flow on its port came from
/// it: a connected flow carries no pin, and no later listener can hold
/// the port until this one's release has been served.
fn unpin_port(&self, port: u16) {
let pins: Vec<OwnedFd> = self
.table()
.values_mut()
.filter(|flow| flow.local == port)
.filter_map(|flow| flow.pin.take())
.collect();
drop(pins);
}
/// What the debug `stats` command reports, or `None` for a flow this
/// node does not hold.
fn stats(&self, id: u64) -> Option<FlowStats> {
@@ -503,6 +550,7 @@ mod unix_impl {
key,
peer,
wiring,
None,
&self.outbound,
&self.node,
&self.flows,
@@ -700,6 +748,19 @@ mod unix_impl {
#[cfg(test)]
impl Connection {
/// Another connection to the same node, sharing its flow table, as a
/// second client of one daemon has.
pub(super) fn sibling(&self) -> Self {
Self::new(
self.node.clone(),
self.outbound.clone(),
Arc::clone(&self.flows),
self.limits,
Arc::clone(&self.npub),
self.debug,
)
}
/// Build a connection wired to a channel a test serves.
pub(super) fn for_test(
node: mpsc::Sender<NativeMessage>,
@@ -730,14 +791,24 @@ mod unix_impl {
}
}
/// Wait until a flow's reader task has observed the client's close.
/// Wait until a flow's reader task has observed the client's close,
/// failing by name if it never does.
///
/// A flow the node no longer holds counts as closed: the reader sets
/// the flag and forgets the flow in the same turn, so the flag alone is
/// almost never there to be seen. Bounded by the clock rather than by
/// turns of the runtime, for the reason given at `CLOSE_WAIT`.
pub(super) async fn settle_closed(&self, flow: u64) {
for _ in 0..1000 {
match self.flows.stats(flow) {
Some(stats) if stats.closed => return,
_ => tokio::task::yield_now().await,
}
}
let seen = super::tests::eventually(async || match self.flows.stats(flow) {
Some(stats) if !stats.closed => None,
_ => Some(()),
})
.await;
assert!(
seen.is_some(),
"the reader never saw flow {flow} close within {:?}",
super::tests::CLOSE_WAIT
);
}
}
@@ -780,35 +851,48 @@ mod unix_impl {
/// The reader cannot start any earlier: it stamps every datagram it
/// forwards with the flow's key and the peer's address, and the local port
/// is not known until the registry has answered.
///
/// The flow is recorded before the reader is spawned. A handed-over client
/// can already have written, and on a multi-thread runtime the reader can
/// run at once, so recording afterwards would let its unpin find no flow
/// and leave the pin in place for the flow's whole life.
#[allow(
clippy::too_many_arguments,
reason = "one hand-off of plumbing to two tasks, with no state to group"
)]
fn start(
id: u64,
key: FlowKey,
peer: XOnlyPublicKey,
wiring: Wiring,
pin: Option<OwnedFd>,
outbound: &mpsc::Sender<Outbound>,
node: &mpsc::Sender<NativeMessage>,
flows: &Arc<Flows>,
) {
let counts = Arc::new(Counts::default());
let pinned = pin.is_some();
flows.record(
id,
Flow {
local: key.local,
sock: Arc::clone(&wiring.sock),
counts: Arc::clone(&counts),
pin,
},
);
tokio::spawn(drain(
id,
key,
peer,
Arc::clone(&wiring.sock),
Arc::clone(&counts),
counts,
pinned,
outbound.clone(),
node.clone(),
Arc::clone(flows),
));
tokio::spawn(feed(Arc::clone(&wiring.sock), wiring.inbound));
flows.record(
id,
Flow {
local: key.local,
sock: wiring.sock,
counts,
},
);
tokio::spawn(feed(wiring.sock, wiring.inbound));
}
/// Serve one listener until its client closes the descriptor.
@@ -872,6 +956,15 @@ mod unix_impl {
}
debug!(port, "Native API listener closed by its client");
// Before the release, so the port cannot have passed to another
// listener whose flows this would also let go of. When the client has
// closed the listener, an arrival it never read went with the
// listener's receive queue, so no descriptor from this port is still
// in flight to it. The other two ways out of the loop, the node going
// away and a failed read, can leave the client's half open with
// arrivals unread on it. Both are teardown, and the hold goes with the
// listener rather than outliving it.
flows.unpin_port(port);
let _ = node
.send(NativeMessage::Release {
flows: Vec::new(),
@@ -891,6 +984,15 @@ mod unix_impl {
/// Both writes are try-sends. They go onto a socket pair whose client half
/// has not been sent yet, so no process can read either one and a task that
/// parked on one would stop serving this listener entirely.
///
/// The daemon keeps its copy of the client's half after the arrival is
/// written, until the client writes on the flow or closes the listener.
/// Until the client reads the arrival, the message carrying the descriptor
/// is otherwise its only reference, and xnu's collector flushes a socket in
/// that state: the client then receives a flow that reads as end of file,
/// with its held datagrams gone. Nothing in the protocol says when the
/// client has read the arrival, so a client that closes the flow without
/// writing leaves it open until the listener closes.
async fn hand_over(
arrival: Arrival,
listener: &Seqpacket,
@@ -968,13 +1070,11 @@ mod unix_impl {
accepted.key,
accepted.peer,
wiring,
Some(theirs),
outbound,
node,
flows,
);
// Dropping our copy leaves the client holding the only reference to its
// half, so its close tears the flow down.
drop(theirs);
}
/// What a failed hand-off write says about the client, for the counter.
@@ -1098,6 +1198,11 @@ mod unix_impl {
/// Counting continues alongside the forwarding: `stats` is how a check
/// observes that a datagram reached the daemon, independently of whether it
/// then reached a peer.
///
/// `pinned` says the daemon still holds a copy of the client's half. The
/// first datagram the client writes proves it holds the descriptor, so the
/// copy is let go then, once, and the client's close ends the flow from
/// there on.
#[allow(
clippy::too_many_arguments,
reason = "one hand-off of plumbing to a task, with no state to group"
@@ -1108,6 +1213,7 @@ mod unix_impl {
peer: XOnlyPublicKey,
sock: Arc<Seqpacket>,
counts: Arc<Counts>,
mut pinned: bool,
outbound: mpsc::Sender<Outbound>,
node: mpsc::Sender<NativeMessage>,
flows: Arc<Flows>,
@@ -1116,6 +1222,10 @@ mod unix_impl {
loop {
match sock.recv(&mut buf).await {
Ok(Received::Datagram(len)) => {
if pinned {
flows.unpin(id);
pinned = false;
}
counts.datagrams.fetch_add(1, Ordering::Relaxed);
counts.bytes.fetch_add(len as u64, Ordering::Relaxed);
let sent = outbound
@@ -1210,22 +1320,43 @@ mod unix_impl {
npub: Arc<str>,
debug: bool,
) -> Result<(), std::io::Error> {
let mut connection = Connection::new(node, outbound, flows, limits, npub, debug);
Connection::new(node, outbound, flows, limits, npub, debug)
.run(stream)
.await
}
impl Connection {
/// Answer the commands on one client connection, in order, until it
/// closes or misbehaves.
///
/// **The daemon keeps its copy of the descriptor in the last reply**
/// until the client's next command arrives or the connection ends.
/// Until the client reads the reply, the message carrying the
/// descriptor can be its only reference, and xnu's collector flushes a
/// socket in that state, as it does an unread arrival's. A client that
/// sends another command has read the reply first, unless it pipelined,
/// which the shipped client never does. Once the copy is gone the
/// client holds the only reference, so its close tears the flow or the
/// listener down; until then a close it makes waits for the copy.
pub(super) async fn run(mut self, stream: UnixStream) -> Result<(), std::io::Error> {
let mut reader = BufReader::new(stream);
let mut line = Vec::new();
let mut kept: Option<OwnedFd> = None;
while read_command(&mut reader, &mut line).await? {
let (response, fd) = connection.answer(&line).await;
drop(kept.take());
let (response, fd) = self.answer(&line).await;
let mut json = serde_json::to_vec(&response)?;
json.push(b'\n');
fdpass::reply(reader.get_ref(), &json, fd.as_ref().map(AsFd::as_fd)).await?;
// Dropping our copy leaves the client holding the only reference to
// its half, so its close tears the flow or the listener down.
drop(fd);
kept = fd;
}
// End of file, or an early return above: either way `kept` goes
// with this frame, and with it the last copy the daemon holds.
Ok(())
}
}
/// Read one newline-terminated command into `line`, refusing an oversized
/// one.
@@ -1365,6 +1496,47 @@ mod tests {
sock
}
/// How long a test waits for the daemon to notice that a client closed a
/// descriptor.
///
/// On Linux the close wakes the task reading the daemon's half, so a wait
/// that will succeed does so within a few turns of the runtime. On macOS
/// and FreeBSD nothing wakes it: the reader sees the close only when its
/// bounded wait expires and it retries the read, as much as one
/// `CLOSE_RETRY` interval (`seqpacket.rs`) after the close, and a
/// listener's close that ends a flow's hold costs two of those in a row.
/// Counting turns of the runtime bounds nothing there: a thousand of them
/// ran out well inside one interval on a macOS runner. A wait that is
/// going to succeed still returns as soon as it does; this is how long one
/// that is not takes to say so.
pub(super) const CLOSE_WAIT: std::time::Duration = std::time::Duration::from_secs(5);
// Four times the longest run of unnoticed closes a test waits through, so
// that lengthening `CLOSE_RETRY` cannot quietly use up the margin.
const _: () = assert!(
CLOSE_WAIT.as_millis() >= 8 * super::seqpacket::CLOSE_LATENCY.as_millis(),
"CLOSE_WAIT no longer covers two unnoticed closes four times over"
);
/// Retry `attempt` until it produces a value, for up to [`CLOSE_WAIT`].
///
/// `None` means the bound ran out. Attempts are a millisecond apart rather
/// than a yield apart, so a wait that lasts a quarter second on macOS is
/// not spent spinning, which for [`rebind`] would mean opening and closing
/// a socket pair on every turn.
pub(super) async fn eventually<T>(mut attempt: impl AsyncFnMut() -> Option<T>) -> Option<T> {
tokio::time::timeout(CLOSE_WAIT, async {
loop {
if let Some(value) = attempt().await {
return value;
}
tokio::time::sleep(std::time::Duration::from_millis(1)).await;
}
})
.await
.ok()
}
/// Send one command that opens a flow, returning the reply and descriptor.
async fn open(connection: &mut Connection, line: &str) -> (serde_json::Value, StdUnixStream) {
let (response, fd) = connection.answer(line.as_bytes()).await;
@@ -1726,6 +1898,332 @@ mod tests {
assert_eq!(&buf[..3], &[0x00, 0xff, 0x10]);
}
/// Wait until a listener descriptor has an arrival to read.
///
/// Polled without blocking and yielding in between, because the arrival is
/// written by the listener's task on this same runtime and a blocking wait
/// would stop the task it is waiting for. A readable listener means
/// `hand_over` has finished: it writes the arrival and settles what happens
/// to the daemon's copy of the descriptor in one synchronous step.
async fn readable(listener: &StdUnixStream) {
for _ in 0..1000 {
let mut poll = libc::pollfd {
fd: listener.as_raw_fd(),
events: libc::POLLIN,
revents: 0,
};
// SAFETY: `poll` points at one live pollfd and the call cannot block.
let rc = unsafe { libc::poll(&mut poll, 1, 0) };
if rc > 0 && (poll.revents & libc::POLLIN) != 0 {
return;
}
tokio::task::yield_now().await;
}
panic!("no arrival became readable on the listener");
}
/// Wait until the node no longer holds a flow, failing if it never lets go.
///
/// The reader marks a flow closed before it gives the registry entry back
/// and forgets the flow, so `stats` can still find a closed flow for a few
/// turns of the runtime, and on macOS and FreeBSD the reader notices the
/// close itself only when it next retries. Bounded by [`CLOSE_WAIT`], so a
/// flow that is never released names itself.
async fn forgotten(connection: &mut Connection, flow: u64) {
let line = format!(r#"{{"command":"stats","params":{{"flow_id":{flow}}}}}"#);
let mut last = serde_json::Value::Null;
let gone = eventually(async || {
last = ask(connection, &line).await;
(last["data"]["errno"] == "ENOENT").then_some(())
})
.await;
assert!(
gone.is_some(),
"the node still holds flow {flow} after {CLOSE_WAIT:?}: {last}"
);
}
/// Bind a listener on a port a closed one held, retrying until the closed
/// one has given it back.
///
/// The release reaches the registry from the closed listener's own task
/// once that task has noticed the close, which takes a few turns of the
/// runtime, or on macOS and FreeBSD up to a retry interval. Bounded by
/// [`CLOSE_WAIT`], so a port that is never given back fails here by name.
async fn rebind(connection: &mut Connection, port: u16) -> OwnedFd {
let line = format!(r#"{{"command":"listen","params":{{"local_port":{port}}}}}"#);
eventually(async || {
let (response, fd) = connection.answer(line.as_bytes()).await;
(serde_json::to_value(response).unwrap()["status"] == "ok")
.then(|| fd.expect("a rebound listener still gets a descriptor"))
})
.await
.unwrap_or_else(|| {
panic!("the closed listener never gave port {port} back within {CLOSE_WAIT:?}")
})
}
/// Bind a listener on 4242, deliver one datagram to it from a new peer,
/// and accept the flow that announced, returning everything a test needs.
async fn arrive_and_accept(
connection: &mut Connection,
) -> (StdUnixStream, u64, serde_json::Value, StdUnixStream) {
let (_value, listener) = listen(connection, 4242).await;
let value = ask(connection, &arrival(5000, 4242, "00ff10")).await;
assert_eq!(value["data"]["outcome"], "announced", "{value}");
let flow = value["data"]["flow_id"].as_u64().unwrap();
let (message, client) = accept(&listener);
(listener, flow, message, client)
}
#[tokio::test]
async fn an_arrival_survives_a_kernel_collection_before_its_client_reads_it() {
// The macOS failure, made deterministic. Between the daemon writing the
// arrival and the client reading it, the flow's descriptor exists only
// inside the arrival message. xnu's descriptor collector flushes a
// socket in that state, so unless the daemon still holds its own copy,
// the client receives a flow that reads as end of file with its held
// datagram gone. On Linux the kernel keeps the socket either way, so
// this test can only fail on macOS.
let (mut connection, _outbound) = connect();
let (_value, listener) = listen(&mut connection, 4242).await;
let value = ask(&mut connection, &arrival(5000, 4242, "00ff10")).await;
assert_eq!(value["data"]["outcome"], "announced", "{value}");
readable(&listener).await;
super::dgram_probe::provoke_collection();
let (message, mut client) = accept(&listener);
assert_eq!(message["held"], 1);
let mut buf = [0u8; 64];
assert_eq!(
client.read(&mut buf).unwrap(),
3,
"the accepted flow lost its held datagram to the kernel's collector"
);
assert_eq!(&buf[..3], &[0x00, 0xff, 0x10]);
}
#[tokio::test]
async fn a_handed_over_flow_outlives_its_dropped_descriptor_until_its_listener_closes() {
// The daemon keeps its copy of a handed-over descriptor until the
// client has shown it holds one, because dropping it is what exposes
// the descriptor to the macOS collector. On Linux the hold is
// observable this way: a client that closes the flow without ever
// writing does not end it while the listener that produced it is open.
let (mut connection, _outbound) = connect();
let (listener, flow, _message, client) = arrive_and_accept(&mut connection).await;
drop(client);
still_open(
&mut connection,
flow,
"the flow ended while the daemon should still hold its descriptor",
)
.await;
// Closing the listener ends the hold: whatever the client did with the
// arrival, the descriptor is no longer in flight in a live socket.
drop(listener);
forgotten(&mut connection, flow).await;
}
#[tokio::test]
async fn a_handed_over_flow_closes_with_its_descriptor_once_its_client_has_written() {
// A datagram from the client proves it holds the descriptor, so the
// daemon lets its copy go and the client's close ends the flow at once,
// with the listener still open.
let (mut connection, _outbound) = connect();
let (listener, flow, _message, mut client) = arrive_and_accept(&mut connection).await;
client.write_all(b"x").unwrap();
connection.settle(flow, 1).await;
drop(client);
forgotten(&mut connection, flow).await;
drop(listener);
}
#[tokio::test]
async fn a_handed_over_flow_its_client_still_holds_outlives_its_listener_and_closes_with_its_descriptor()
{
// Closing the listener lets the daemon's copy go, and the client's own
// copy then carries the flow by itself: it keeps working after the
// listener has gone, and the client's close ends it with no write ever
// made. Letting the copy go must not end a flow the client still holds.
let (mut connection, _outbound) = connect();
let (listener, flow, _message, mut client) = arrive_and_accept(&mut connection).await;
drop(listener);
// The port comes back only after the listener's task has let its
// flows' copies go, so a rebound port means that has happened.
let _rebound = rebind(&mut connection, 4242).await;
let mut buf = [0u8; 64];
assert_eq!(client.read(&mut buf).unwrap(), 3);
let value = ask(
&mut connection,
&format!(r#"{{"command":"inject","params":{{"flow_id":{flow},"data":"ab"}}}}"#),
)
.await;
assert_eq!(value["status"], "ok", "{value}");
assert_eq!(client.read(&mut buf).unwrap(), 1);
assert_eq!(buf[0], 0xab);
drop(client);
forgotten(&mut connection, flow).await;
}
/// Serve a sibling of `connection` over a real socket, the way the daemon
/// serves a client, returning the client's end and the serving task.
///
/// Through `run` rather than `answer`, because what these tests observe is
/// what the serving loop does with the descriptor in a reply it has sent.
fn serve_socket(
connection: &Connection,
) -> (StdUnixStream, tokio::task::JoinHandle<std::io::Result<()>>) {
let (ours, theirs) = StdUnixStream::pair().expect("AF_UNIX socketpair");
ours.set_nonblocking(true).expect("the socket is open");
let ours = tokio::net::UnixStream::from_std(ours).expect("inside a runtime");
let task = tokio::spawn(connection.sibling().run(ours));
(bounded(theirs), task)
}
/// Write one command on a client socket and read its reply line, with the
/// descriptor it carried.
async fn call(client: &StdUnixStream, line: &str) -> (serde_json::Value, Option<OwnedFd>) {
let mut writer = client;
writer.write_all(line.as_bytes()).unwrap();
writer.write_all(b"\n").unwrap();
readable(client).await;
let mut buf = [0u8; 4096];
let chunk = super::fdpass::recv(client.as_raw_fd(), &mut buf)
.expect("a reply should be readable on the connection");
let reply = buf[..chunk.len]
.strip_suffix(b"\n")
.expect("one whole reply line per read");
(serde_json::from_slice(reply).unwrap(), chunk.fd)
}
/// Open a flow through a served socket, returning its id and descriptor.
async fn connect_over(client: &StdUnixStream) -> (u64, OwnedFd) {
let line = format!(
r#"{{"command":"connect","params":{{"peer":"{PEER}","remote_port":4242,"local_port":4243}}}}"#
);
let (value, fd) = call(client, &line).await;
assert_eq!(value["status"], "ok", "{value}");
let flow = value["data"]["flow_id"].as_u64().unwrap();
(
flow,
fd.expect("a connect reply carries the flow's descriptor"),
)
}
/// Assert that the node still holds a flow open, after giving its reader
/// every chance to notice a close.
///
/// Where a close wakes the reader, a few turns of the runtime are that
/// chance. On macOS and FreeBSD the reader notices a close only when its
/// bounded wait expires and it retries, so a check made sooner passes
/// whether or not the flow has closed. There this also waits out two of
/// those intervals: a reader that parked just before a close has retried
/// by then, with one interval to spare for a loaded runner. Elsewhere the
/// interval is zero and the wait costs nothing.
async fn still_open(connection: &mut Connection, flow: u64, why: &str) {
for _ in 0..1000 {
tokio::task::yield_now().await;
}
tokio::time::sleep(super::seqpacket::CLOSE_LATENCY * 2).await;
let value = ask(
connection,
&format!(r#"{{"command":"stats","params":{{"flow_id":{flow}}}}}"#),
)
.await;
assert_eq!(value["status"], "ok", "{why}: {value}");
assert_eq!(value["data"]["closed"], false, "{why}: {value}");
}
/// Any command at all, sent only so the serving loop reads one.
const NEXT: &str = r#"{"command":"stats","params":{"flow_id":0}}"#;
#[tokio::test]
async fn a_descriptor_sent_in_a_reply_is_kept_until_the_clients_next_command() {
// Until the client reads a reply, the message carrying its descriptor
// can be the only reference to it, which is what xnu's collector
// flushes. So the daemon keeps its copy until the client sends another
// command, which it does only after reading the reply. On Linux the
// hold is observable as a flow that outlives the client's close.
let (mut probe, _outbound) = connect();
let (client, _task) = serve_socket(&probe);
let (flow, fd) = connect_over(&client).await;
drop(fd);
still_open(
&mut probe,
flow,
"the flow ended while the connection should still hold its descriptor",
)
.await;
call(&client, NEXT).await;
forgotten(&mut probe, flow).await;
}
#[tokio::test]
async fn a_descriptor_sent_in_a_reply_is_let_go_when_the_connection_ends() {
// The connection's close ends its hold. The client's own copy then
// carries the flow alone, so the flow survives the connection and ends
// with the client's close.
let (mut probe, _outbound) = connect();
let (client, task) = serve_socket(&probe);
let (flow, fd) = connect_over(&client).await;
drop(client);
task.await
.unwrap()
.expect("a connection closed between commands ends cleanly");
still_open(
&mut probe,
flow,
"the flow ended with the connection while the client still holds it",
)
.await;
drop(fd);
forgotten(&mut probe, flow).await;
}
#[tokio::test]
async fn a_flow_its_client_closes_after_its_next_command_ends_at_once() {
// Once the client has sent another command the hold is over, so the
// client's close is the flow's end of file with the connection still
// open.
let (mut probe, _outbound) = connect();
let (client, _task) = serve_socket(&probe);
let (flow, fd) = connect_over(&client).await;
call(&client, NEXT).await;
drop(fd);
forgotten(&mut probe, flow).await;
drop(client);
}
#[tokio::test]
async fn a_flow_whose_reply_is_never_followed_by_a_command_ends_with_the_connection() {
// A client that closes the flow and then the connection, sending
// nothing more, still ends the flow: the connection's end of file is
// the last point at which the daemon lets its copy go.
let (mut probe, _outbound) = connect();
let (client, task) = serve_socket(&probe);
let (flow, fd) = connect_over(&client).await;
drop(fd);
drop(client);
task.await
.unwrap()
.expect("a connection closed between commands ends cleanly");
forgotten(&mut probe, flow).await;
}
#[test]
fn a_listener_that_closed_and_one_that_stopped_reading_are_counted_apart() {
// Both take the same cleanup, so the counter is the only place the
@@ -1946,19 +2444,8 @@ mod tests {
// The unbind is what `close(listen_fd)` means in Berkeley, and the
// daemon can only observe it by reading its own half. Without the read
// arm the port is held for the node's lifetime and every attempt below
// fails.
for _ in 0..1000 {
let (response, fd) = connection
.answer(br#"{"command":"listen","params":{"local_port":4242}}"#)
.await;
if serde_json::to_value(response).unwrap()["status"] == "ok" {
assert!(fd.is_some(), "a rebound listener still gets a descriptor");
return;
}
tokio::task::yield_now().await;
}
panic!("the closed listener never gave its port back");
// arm the port is held for the node's lifetime and every attempt fails.
rebind(&mut connection, 4242).await;
}
#[tokio::test]
+13
View File
@@ -180,6 +180,19 @@ pub fn set_sndbuf(fd: &OwnedFd, bytes: usize) -> io::Result<()> {
#[cfg(any(target_os = "macos", target_os = "freebsd"))]
const CLOSE_RETRY: Duration = Duration::from_millis(250);
/// How long a reader can leave a closed peer unnoticed, for the native API
/// tests that assert a flow is still open and so must wait long enough to have
/// seen it close.
///
/// `CLOSE_RETRY` where the reactor cannot see a close, because the reader then
/// notices one only when that bound expires and it retries the read.
#[cfg(all(test, any(target_os = "macos", target_os = "freebsd")))]
pub(super) const CLOSE_LATENCY: Duration = CLOSE_RETRY;
/// Zero where a close wakes the reader itself.
#[cfg(all(test, not(any(target_os = "macos", target_os = "freebsd"))))]
pub(super) const CLOSE_LATENCY: Duration = Duration::ZERO;
/// The daemon's half of a flow's or a listener's socket pair, registered with
/// the reactor.
pub struct Seqpacket {
+10 -13
View File
@@ -1,11 +1,10 @@
//! Off-task FMP + FSP decrypt + delivery worker.
//!
//! First incremental step of the data-plane shard restructure (per the
//! architectural plan): each worker now **owns its session state
//! directly** in a local `HashMap`, with no `Arc<RwLock<HashMap>>`
//! cache on the Node side and no `Arc<Mutex<ReplayWindow>>` shared
//! with the rx_loop. The worker is the sole authority over the replay
//! window and the recv-side ciphers for every session it owns.
//! Each worker **owns its session state directly** in a local
//! `HashMap`, with no `Arc<RwLock<HashMap>>` cache on the Node side and
//! no `Arc<Mutex<ReplayWindow>>` shared with the rx_loop. The worker is
//! the sole authority over the replay window and the recv-side ciphers
//! for every session it owns.
//!
//! Dispatch is **deterministic by session key**: rx_loop computes
//! `worker_idx = hash(cache_key) % N` and routes both
@@ -484,8 +483,8 @@ fn handle_job(
// **decrypted-in-place** FMP plaintext back to rx_loop.
//
// Two problems with that path:
// 1. After the shard-owned-sessions refactor (01f6c62), the FSP
// replay window is owned by **this worker thread**. Once we
// 1. Since sessions became shard-owned, the FSP replay
// window is owned by **this worker thread**. Once we
// `state.fsp_replay.accept(fsp_counter)`, the rx_loop's
// `noise::Session::replay_window` is stale — it still has
// old counters. When rx_loop tries to FSP-decrypt the
@@ -511,11 +510,9 @@ fn handle_job(
// still offloads the FMP AEAD (~half the per-packet decrypt
// CPU). Correctness over micro-optimisation.
//
// The DataShard end-state (per the architectural plan) re-
// introduces the EndpointData fast path correctly by having the
// shard worker also own the rx_loop side for its sessions — at
// that point there's no "rx_loop legacy path" for the worker to
// conflict with.
// A worker that also owned the rx_loop side of its sessions could
// restore the EndpointData fast path correctly: there would then
// be no "rx_loop legacy path" for the worker to conflict with.
// Pass the buffer through by ownership + offset/length. No
// per-packet allocation; rx_loop slices into `packet_data`.
let _ = link_msg; // sanity-check borrow before sending buffer onward
+210 -34
View File
@@ -56,15 +56,17 @@ use crate::transport::udp::io::AsyncUdpSocket;
#[cfg(not(target_os = "macos"))]
use crossbeam_channel::{Receiver, SendError, Sender, TrySendError, bounded};
use ring::aead::{Aad, LessSafeKey, Nonce};
#[cfg(any(target_os = "macos", test))]
use std::collections::VecDeque;
#[cfg(target_os = "macos")]
use std::collections::{BTreeMap, HashMap, VecDeque};
use std::collections::{BTreeMap, HashMap};
use std::net::SocketAddr;
#[cfg(unix)]
use std::os::unix::io::AsRawFd;
use std::sync::Arc;
use std::sync::OnceLock;
#[cfg(target_os = "macos")]
use std::sync::{Condvar, Mutex};
#[cfg(any(target_os = "macos", test))]
use std::sync::{Condvar, Mutex, PoisonError};
use tracing::{debug, trace, warn};
/// A pre-cooked FMP-encrypt-and-send job. All state-touching work
@@ -218,43 +220,42 @@ impl QueuedFmpSendJob {
/// same rationale as the bounded endpoint_commands channel upstream.
const WORKER_CHANNEL_CAP: usize = 1024;
#[cfg(target_os = "macos")]
struct MacWorkerSender {
inner: Arc<MacWorkerQueueInner>,
#[cfg(any(target_os = "macos", test))]
struct MacWorkerSender<T> {
inner: Arc<MacWorkerQueueInner<T>>,
}
#[cfg(target_os = "macos")]
struct MacWorkerReceiver {
inner: Arc<MacWorkerQueueInner>,
#[cfg(any(target_os = "macos", test))]
struct MacWorkerReceiver<T> {
inner: Arc<MacWorkerQueueInner<T>>,
}
#[cfg(target_os = "macos")]
struct MacWorkerQueueInner {
state: Mutex<MacWorkerQueueState>,
#[cfg(any(target_os = "macos", test))]
struct MacWorkerQueueInner<T> {
state: Mutex<MacWorkerQueueState<T>>,
not_empty: Condvar,
not_full: Condvar,
cap: usize,
}
#[cfg(target_os = "macos")]
#[derive(Default)]
struct MacWorkerQueueState {
queue: VecDeque<QueuedFmpSendJob>,
#[cfg(any(target_os = "macos", test))]
struct MacWorkerQueueState<T> {
queue: VecDeque<T>,
waiting: bool,
closed: bool,
}
#[cfg(target_os = "macos")]
enum MacWorkerTryPushError {
Full(Box<QueuedFmpSendJob>),
#[cfg(any(target_os = "macos", test))]
enum MacWorkerTryPushError<T> {
Full(Box<T>),
Closed,
}
#[cfg(target_os = "macos")]
#[cfg(any(target_os = "macos", test))]
struct MacWorkerPushError;
#[cfg(target_os = "macos")]
fn mac_worker_channel(cap: usize) -> (MacWorkerSender, MacWorkerReceiver) {
#[cfg(any(target_os = "macos", test))]
fn mac_worker_channel<T>(cap: usize) -> (MacWorkerSender<T>, MacWorkerReceiver<T>) {
let inner = Arc::new(MacWorkerQueueInner {
state: Mutex::new(MacWorkerQueueState {
queue: VecDeque::with_capacity(cap),
@@ -273,9 +274,9 @@ fn mac_worker_channel(cap: usize) -> (MacWorkerSender, MacWorkerReceiver) {
)
}
#[cfg(target_os = "macos")]
impl MacWorkerSender {
fn try_push(&self, job: QueuedFmpSendJob) -> Result<(), MacWorkerTryPushError> {
#[cfg(any(target_os = "macos", test))]
impl<T> MacWorkerSender<T> {
fn try_push(&self, job: T) -> Result<(), MacWorkerTryPushError<T>> {
let mut state = self
.inner
.state
@@ -298,7 +299,7 @@ impl MacWorkerSender {
Ok(())
}
fn push_blocking(&self, job: QueuedFmpSendJob) -> Result<(), MacWorkerPushError> {
fn push_blocking(&self, job: T) -> Result<(), MacWorkerPushError> {
let mut state = self
.inner
.state
@@ -328,8 +329,8 @@ impl MacWorkerSender {
}
}
#[cfg(target_os = "macos")]
impl Drop for MacWorkerSender {
#[cfg(any(target_os = "macos", test))]
impl<T> Drop for MacWorkerSender<T> {
fn drop(&mut self) {
let mut state = self
.inner
@@ -343,9 +344,34 @@ impl Drop for MacWorkerSender {
}
}
#[cfg(target_os = "macos")]
impl MacWorkerReceiver {
fn recv_batch(&self, batch: &mut Vec<QueuedFmpSendJob>, max: usize) -> bool {
/// Closing from the receiver side matters because a sender waiting in
/// `push_blocking` on a full queue sleeps on `not_full`, and only the
/// receiver draining the queue wakes it. If the worker thread exits (a
/// panic unwinding included), nothing else would, and the rx_loop behind
/// that sender would block forever.
#[cfg(any(target_os = "macos", test))]
impl<T> Drop for MacWorkerReceiver<T> {
fn drop(&mut self) {
// This can run while the worker thread unwinds from a panic, where
// a second panic would abort the process, so tolerate poisoning.
let mut state = self
.inner
.state
.lock()
.unwrap_or_else(PoisonError::into_inner);
state.closed = true;
let queued = std::mem::take(&mut state.queue);
drop(state);
// Queued jobs hold key copies and sockets; free them outside the lock.
drop(queued);
self.inner.not_full.notify_all();
self.inner.not_empty.notify_all();
}
}
#[cfg(any(target_os = "macos", test))]
impl<T> MacWorkerReceiver<T> {
fn recv_batch(&self, batch: &mut Vec<T>, max: usize) -> bool {
debug_assert!(batch.is_empty());
let mut state = self
.inner
@@ -378,7 +404,7 @@ impl MacWorkerReceiver {
}
#[cfg(target_os = "macos")]
type WorkerSender = MacWorkerSender;
type WorkerSender = MacWorkerSender<QueuedFmpSendJob>;
#[cfg(not(target_os = "macos"))]
type WorkerSender = Sender<QueuedFmpSendJob>;
@@ -397,7 +423,7 @@ type WorkerSender = Sender<QueuedFmpSendJob>;
///
/// **Ordering: hash-by-destination** so single-flow TCP keeps its
/// FIFO ordering (round-robin caused 8000 retransmits in an earlier
/// experiment — see the git log for the 56e0ca8 fix). Multi-peer /
/// experiment, which is why dispatch hashes by destination). Multi-peer /
/// multi-flow benches still get parallelism since different
/// destinations hash to different workers.
#[derive(Clone)]
@@ -955,7 +981,7 @@ fn run_worker(idx: usize, rx: Receiver<QueuedFmpSendJob>) {
}
#[cfg(target_os = "macos")]
fn run_worker_macos(idx: usize, rx: MacWorkerReceiver) {
fn run_worker_macos(idx: usize, rx: MacWorkerReceiver<QueuedFmpSendJob>) {
trace!(worker = idx, "FMP encrypt worker thread starting");
let batch_size = macos_worker_batch_size();
@@ -2402,3 +2428,153 @@ fn send_one_raw(
Ok(r as usize)
}
}
/// Tests for the bounded worker queue the macOS encrypt pool uses. The
/// queue is generic, so these run on every platform against small item
/// types. Every wait is bounded so a regression fails instead of hanging.
#[cfg(test)]
mod mac_queue_tests {
use super::*;
use std::panic::{AssertUnwindSafe, catch_unwind};
use std::sync::mpsc;
use std::thread;
use std::time::Duration;
const WAIT: Duration = Duration::from_secs(5);
/// Run `push_blocking(item)` on a helper thread and return a channel
/// that yields its result.
fn spawn_pusher<T: Send + 'static>(
tx: MacWorkerSender<T>,
item: T,
) -> mpsc::Receiver<Result<(), MacWorkerPushError>> {
let (done_tx, done_rx) = mpsc::channel();
thread::spawn(move || {
let result = tx.push_blocking(item);
let _ = done_tx.send(result);
});
done_rx
}
#[test]
fn push_blocking_returns_error_when_worker_thread_panics_with_full_queue() {
let (tx, rx) = mac_worker_channel::<u32>(2);
assert!(tx.try_push(1).is_ok());
assert!(tx.try_push(2).is_ok());
match tx.try_push(9) {
Err(MacWorkerTryPushError::Full(job)) => assert_eq!(*job, 9),
_ => panic!("try_push on a full queue should hand the job back"),
}
// The worker owns the receiver and dies without draining, once the
// pusher below has had time to start waiting for space.
let (die_tx, die_rx) = mpsc::channel::<()>();
let worker = thread::spawn(move || {
let _rx = rx;
let _ = die_rx.recv();
panic!("simulated encrypt worker panic");
});
let done = spawn_pusher(tx, 3);
thread::sleep(Duration::from_millis(200));
assert!(
matches!(done.try_recv(), Err(mpsc::TryRecvError::Empty)),
"push_blocking returned while the queue was full and the worker alive"
);
die_tx
.send(())
.expect("worker thread gone before its signal");
let result = done
.recv_timeout(WAIT)
.expect("push_blocking still blocked after the worker thread died");
assert!(matches!(result, Err(MacWorkerPushError)));
assert!(worker.join().is_err(), "worker thread should have panicked");
}
#[test]
fn try_push_returns_closed_after_receiver_dropped() {
let (tx, rx) = mac_worker_channel::<u32>(2);
drop(rx);
assert!(matches!(tx.try_push(1), Err(MacWorkerTryPushError::Closed)));
let done = spawn_pusher(tx, 2);
let result = done
.recv_timeout(WAIT)
.expect("push_blocking blocked on a queue whose receiver is gone");
assert!(matches!(result, Err(MacWorkerPushError)));
}
#[test]
fn receiver_drop_releases_queued_items() {
let marker = Arc::new(());
let (tx, rx) = mac_worker_channel::<Arc<()>>(4);
assert!(tx.try_push(Arc::clone(&marker)).is_ok());
assert!(tx.try_push(Arc::clone(&marker)).is_ok());
assert_eq!(Arc::strong_count(&marker), 3);
drop(rx);
assert_eq!(
Arc::strong_count(&marker),
1,
"queued items must be freed when the receiver goes away"
);
drop(tx);
}
#[test]
fn push_blocking_completes_when_worker_drains_full_queue() {
let (tx, rx) = mac_worker_channel::<u32>(2);
assert!(tx.try_push(1).is_ok());
assert!(tx.try_push(2).is_ok());
let done = spawn_pusher(tx, 3);
thread::sleep(Duration::from_millis(100));
assert!(
matches!(done.try_recv(), Err(mpsc::TryRecvError::Empty)),
"push_blocking returned while the queue was still full"
);
let mut batch = Vec::new();
assert!(rx.recv_batch(&mut batch, 16));
assert_eq!(batch, vec![1, 2]);
let result = done
.recv_timeout(WAIT)
.expect("push_blocking not woken after the worker drained");
assert!(result.is_ok());
batch.clear();
assert!(rx.recv_batch(&mut batch, 16));
assert_eq!(batch, vec![3]);
}
#[test]
fn recv_batch_drains_then_reports_closed_after_sender_drop() {
let (tx, rx) = mac_worker_channel::<u32>(4);
for i in 1..=3 {
assert!(tx.try_push(i).is_ok());
}
drop(tx);
let mut batch = Vec::new();
assert!(rx.recv_batch(&mut batch, 16));
assert_eq!(batch, vec![1, 2, 3]);
batch.clear();
assert!(!rx.recv_batch(&mut batch, 16));
assert!(batch.is_empty());
}
#[test]
fn receiver_drop_does_not_panic_on_poisoned_lock() {
let (tx, rx) = mac_worker_channel::<u32>(2);
// The sender's own Drop still expects an unpoisoned lock, so it must
// never run here: a panic there while a failed assertion unwinds
// would abort the whole test binary.
let _tx = std::mem::ManuallyDrop::new(tx);
let inner = Arc::clone(&rx.inner);
let poisoner = thread::spawn(move || {
let _guard = inner.state.lock().unwrap();
panic!("poison the queue lock");
});
assert!(poisoner.join().is_err());
assert!(rx.inner.state.is_poisoned());
let dropped = catch_unwind(AssertUnwindSafe(move || drop(rx)));
assert!(dropped.is_ok(), "receiver drop panicked on a poisoned lock");
}
}
+8 -5
View File
@@ -141,11 +141,11 @@ impl Node {
}
RequestOutcome::OwnRequestLooped => {
// Our own flooded request, come back to us through a bloom
// false positive. Counted apart from ReqDuplicate: that one
// describes a peer resending, and this one describes our own
// fan-out returning, so folding them together would put a
// permanent healthy floor on a counter an operator reads as
// neighbour misbehaviour.
// false positive. Counted apart from ReqDuplicate by cause:
// this is our own fan-out returning, that is a request id we
// already recorded, seen again. Neither counter says which
// peer delivered the copy; this debug line is the only
// per-peer record.
self.metrics()
.lookup
.record_reject(DiscoveryReject::ReqOwnLoopback);
@@ -375,6 +375,9 @@ impl Node {
now_ms,
path_mtu,
} => {
// Reports about the path this verified value replaces are
// not evidence against it.
self.broken_quorum.clear(&target);
// The annotation is unsigned and accumulates hop by hop, so
// any forwarder on the reverse path can lower it. A value
// below the actionable floor cannot describe a usable path,
+36 -11
View File
@@ -79,6 +79,36 @@ fn ladder_ms(rate_limit: &crate::config::RateLimitConfig) -> u64 {
total
}
/// The longest silence, in milliseconds, the node tolerates on a link it
/// keeps.
///
/// A peer's handshake resend ladder, three ticks of scheduling slack, and
/// the longer of one heartbeat interval and the link-dead timeout. The
/// link-dead reap is suppressed while a rekey still has msg1 resends left,
/// which is what the ladder term covers. 64 s at stock settings.
pub(in crate::node) fn link_silence_ms(node: &crate::config::NodeConfig) -> u64 {
let after_ms = node
.heartbeat_interval_secs
.max(node.link_dead_timeout_secs)
.saturating_mul(1000);
ladder_ms(&node.rate_limit)
.saturating_add(node.tick_interval_secs.saturating_mul(3000))
.saturating_add(after_ms)
}
/// The inbound idle deadline for stream transports: how long an accepted
/// connection may go without delivering a complete frame once it has
/// delivered one.
///
/// Set to `link_silence_ms`, so a connection carrying a link the node would
/// keep is never dropped by it, whatever the heartbeat, link-dead, tick and
/// resend settings are; a connection carrying no live link is reclaimed.
pub(in crate::node) fn inbound_idle_timeout(
node: &crate::config::NodeConfig,
) -> std::time::Duration {
std::time::Duration::from_millis(link_silence_ms(node))
}
/// How long a rekey responder holds a pending session its initiator has
/// not adopted before retiring it.
///
@@ -92,18 +122,13 @@ fn ladder_ms(rate_limit: &crate::config::RateLimitConfig) -> u64 {
/// interval later, and the link-dead reap if every frame from the
/// initiator is lost. Both are allowed one more tick. Retiring before
/// either turns a late but legitimate adoption into a split. That floor
/// is 64 s at stock settings; the drain ceiling, which bounds residence
/// for the same recovering-peer reason, is 120 s and is used unless the
/// configured timers push the floor above it.
/// is `link_silence_ms`, 64 s at stock settings; the drain ceiling, which
/// bounds residence for the same recovering-peer reason, is 120 s and is
/// used unless the configured timers push the floor above it.
pub(in crate::node) fn pending_hold(node: &crate::config::NodeConfig) -> std::time::Duration {
let after_ms = node
.heartbeat_interval_secs
.max(node.link_dead_timeout_secs)
.saturating_mul(1000);
let floor_ms = ladder_ms(&node.rate_limit)
.saturating_add(node.tick_interval_secs.saturating_mul(3000))
.saturating_add(after_ms);
std::time::Duration::from_millis(drain_max_retention_ms(&node.rate_limit).max(floor_ms))
std::time::Duration::from_millis(
drain_max_retention_ms(&node.rate_limit).max(link_silence_ms(node)),
)
}
impl Node {
+133 -23
View File
@@ -18,6 +18,7 @@ use crate::noise::{
use crate::proto::fmp::wire::{
ESTABLISHED_HEADER_SIZE, FLAG_KEY_EPOCH, FLAG_SP, build_established_header,
};
use crate::proto::fsp::quorum::QuorumVerdict;
use crate::proto::fsp::wire::{
FSP_COMMON_PREFIX_SIZE, FSP_FLAG_CP, FSP_FLAG_K, FSP_HEADER_SIZE, FSP_PHASE_ESTABLISHED,
FSP_PHASE_MSG1, FSP_PHASE_MSG2, FSP_PHASE_MSG3, FSP_PORT_HEADER_SIZE, FSP_PORT_IPV6_SHIM,
@@ -193,7 +194,8 @@ impl Node {
self.handle_coords_required(src_addr, error_body).await;
}
Some(RoutingSignalType::PathBroken) => {
self.handle_path_broken(src_addr, error_body).await;
self.handle_path_broken(src_addr, link_peer, error_body)
.await;
}
Some(RoutingSignalType::MtuExceeded) => {
self.handle_mtu_exceeded(src_addr, error_body).await;
@@ -1919,12 +1921,28 @@ impl Node {
/// Handle a PathBroken error signal from a transit router.
///
/// The router has coordinates but still can't route to the destination.
/// Send a standalone CoordsWarmup immediately (rate-limited), invalidate
/// cached coordinates, trigger re-discovery, and reset the warmup counter.
/// Send a standalone CoordsWarmup immediately (rate-limited), re-validate
/// the destination's coordinates by lookup, release its path MTU, and
/// reset the warmup counter.
///
/// Cached coordinates that are only a hint are removed. Coordinates a
/// lookup verified are kept while the lookup re-validates them, and are
/// demoted to a hint only once reports arriving over distinct links reach
/// the quorum: the signal is unauthenticated, and deleting a verified
/// entry on one report is what let the next forged warm replace it.
///
/// `src_addr` is the datagram's claimed source and is not
/// end-to-end authenticated; see `signal_verdict`.
pub(in crate::node) async fn handle_path_broken(&mut self, src_addr: &NodeAddr, inner: &[u8]) {
/// end-to-end authenticated; see `signal_verdict`. `link_peer` is the
/// authenticated peer the datagram arrived over. It is the report's vote
/// in the quorum, because the body's reporter is whatever the sender
/// wrote, and it is compared with the forward path to count mismatches;
/// it never refuses the signal.
pub(in crate::node) async fn handle_path_broken(
&mut self,
src_addr: &NodeAddr,
link_peer: &NodeAddr,
inner: &[u8],
) {
self.metrics().errors.path_broken.inc();
let msg = match PathBroken::decode(inner) {
@@ -1936,11 +1954,10 @@ impl Node {
};
// The premise: this signal carries no end-to-end authentication, so
// the body's `dest_addr` is attacker-chosen. `plan_path_broken` emits
// its coord-cache invalidation unconditionally, and the path-MTU
// release below is likewise unguarded, so both act on whatever address
// the body names unless the gate refuses it here, in the shell, which
// is the only layer that knows who sent the datagram.
// the body's `dest_addr` is attacker-chosen. The coord-cache action,
// the lookup and the path-MTU release below all act on whatever
// address the body names unless the gate refuses it here, in the
// shell, which is the only layer that knows who sent the datagram.
let verdict = self.signal_verdict(src_addr, &msg.dest_addr);
if verdict != SignalVerdict::Admit {
debug!(src = %src_addr, dest = %msg.dest_addr, reporter = %msg.reporter,
@@ -1960,6 +1977,7 @@ impl Node {
reporter = %msg.reporter,
"PathBroken: transit router reports routing failure"
);
self.count_path_broken_mismatches(link_peer, &msg);
// Send standalone CoordsWarmup immediately (rate-limited)
if self
@@ -1978,46 +1996,69 @@ impl Node {
"PathBroken response rate-limited, skipping standalone CoordsWarmup");
}
// Invalidate stale cached coordinates, then (only if the target's
// identity is cached — else the LookupResponse proof cannot be verified,
// e.g. when the XK responder receives PathBroken before msg3 completes)
// trigger re-discovery. The core emits invalidate-then-lookup in order.
let has_cached_identity = self.has_cached_identity(&msg.dest_addr);
let actions = self
.fsp
.plan_path_broken(msg.dest_addr, has_cached_identity);
// Only a live verified entry has anything for the quorum to protect,
// so only a report against one is recorded; that also bounds the
// quorum's keys by the destinations this node has looked up. The vote
// is the link the report arrived over, so a sender on one link counts
// once however many reporters it names.
let now = Self::now_ms();
let verified = self
.coord_cache
.get_entry(&msg.dest_addr)
.is_some_and(|e| e.is_verified(now));
let quorum = if verified {
self.broken_quorum.record(msg.dest_addr, *link_peer, now)
} else {
QuorumVerdict::Below { distinct: 0 }
};
let actions = self.fsp.plan_path_broken(msg.dest_addr, verified, quorum);
for action in actions {
match action {
FspAction::InvalidateCoords { addr } => {
self.coord_cache.remove(&addr);
}
FspAction::DemoteCoords { addr } => {
self.coord_cache.demote(&addr);
self.metrics().errors.broken_demoted.inc();
debug!(dest = %addr,
"PathBroken quorum reached; demoted verified coordinates to a hint");
}
FspAction::InitiateLookup { dest } => {
self.cache_session_identity(&dest);
self.maybe_initiate_lookup(&dest).await;
}
_ => {}
}
}
if let QuorumVerdict::Below { distinct } = quorum
&& verified
{
self.metrics().errors.broken_below_quorum.inc();
debug!(dest = %msg.dest_addr, distinct,
"PathBroken below quorum; keeping verified coordinates pending the lookup");
}
// The path this destination's stored MTU described is gone, so release
// it rather than carrying it onto whatever path replaces it. Rate
// limited per destination on its own budget: PathBroken is
// unauthenticated, and an unlimited release discards a genuinely
// learned bottleneck as fast as it is relearned. The budget is not
// shared with any other signal, so nothing else can spend it.
//
// A cache entry kept above still carries the MTU the lookup stored
// with it, which describes the same path; clear it with the map so
// the two do not disagree.
if self
.path_mtu_release_limiter
.should_send(&msg.dest_addr, Self::now_ms())
{
self.path_mtu_lookup_release(&msg.dest_addr);
self.coord_cache.clear_path_mtu(&msg.dest_addr);
} else {
trace!(dest = %msg.dest_addr,
"PathBroken path MTU release rate-limited, keeping the stored value");
}
if !has_cached_identity {
debug!(dest = %msg.dest_addr,
"Skipping discovery after PathBroken: no cached identity for target");
}
// Reset coords warmup counter so the next N packets include
// COORDS_PRESENT, re-warming transit caches along the new path.
let n = self.config().node.session.coords_warmup_packets;
@@ -2031,6 +2072,75 @@ impl Node {
}
}
/// Cache the identity of `dest` from its session entry if the identity
/// cache has none.
///
/// A lookup's answer is verified against the target's cached key, so a
/// lookup for a destination whose identity has been evicted runs to its
/// timeout and drops the packets queued for it as unreachable. A session
/// already holds the key: the one this node initiated to, or the one the
/// responder handshake authenticated.
fn cache_session_identity(&mut self, dest: &NodeAddr) {
if self.has_cached_identity(dest) {
return;
}
if let Some(pubkey) = self.sessions.get(dest).map(|e| *e.remote_pubkey()) {
self.register_identity(*dest, pubkey);
}
}
/// Count, without acting on, the two ways an admitted PathBroken can
/// disagree with this node's own view of the path.
///
/// Advisory only: both checks read state an attacker can influence and
/// both have a non-zero healthy floor, so they size the problem rather
/// than refuse anything.
///
/// - Link: the signal arrived over a link other than the one this node
/// would forward to the destination on. A genuine report can also do
/// this when the reverse path differs from the forward one.
/// - Reporter: the reporter is this node or the destination, or its known
/// coordinates are not strictly closer to the destination than this
/// node's. Forwarding makes strict progress under each forwarder's own
/// view of the destination, and a PathBroken arises exactly where that
/// view may differ from this node's, so a genuine report can count here
/// too. A reporter's coordinates are rarely known unless it is a direct
/// peer, so this mostly reads as unknown and is not counted.
fn count_path_broken_mismatches(&self, link_peer: &NodeAddr, msg: &PathBroken) {
let now = Self::now_ms();
let dest = &msg.dest_addr;
let reporter = &msg.reporter;
if let (Some(hop), _) = self.preview_next_hop(dest, now)
&& hop.node_addr != *link_peer
{
self.metrics().errors.broken_link_mismatch.inc();
debug!(dest = %dest, link_peer = %link_peer, next_hop = %hop.node_addr,
"PathBroken arrived off the forward link");
}
let implausible = if reporter == self.node_addr() || reporter == dest {
true
} else {
let dest_coords = self.coord_cache.get(dest, now);
let reporter_coords = self
.tree_state
.peer_coords(reporter)
.or_else(|| self.coord_cache.get(reporter, now));
match (dest_coords, reporter_coords) {
(Some(d), Some(r)) => {
r.distance_to(d) >= self.tree_state.my_coords().distance_to(d)
}
_ => false,
}
};
if implausible {
self.metrics().errors.broken_reporter_mismatch.inc();
debug!(dest = %dest, reporter = %reporter,
"PathBroken reporter is not closer to the destination than this node");
}
}
/// Handle an MtuExceeded error signal from a transit router.
///
/// A transit router couldn't forward our packet because it exceeded the
+37 -9
View File
@@ -1,10 +1,10 @@
//! Lock-free metric counters backed by atomics.
//!
//! Mirrors the `NodeStats` counter surface (`stats.rs`) but stores each
//! counter in an `AtomicU64`, so it can be bumped through `&self` and, in
//! a later step, sampled without dispatching through the rx_loop task. The
//! hottest counters are cache-line padded to avoid false sharing once
//! reads move off-thread.
//! counter in an `AtomicU64`, so it can be bumped through `&self` and read
//! off the rx_loop task (the control read handle serves `show_metrics` from
//! it directly). The hottest counters are cache-line padded to avoid false
//! sharing between the writer and off-thread readers.
//!
//! The forwarding, discovery, tree, bloom, congestion, and error families
//! live here exclusively and are both written and served from the registry.
@@ -43,11 +43,10 @@ impl Counter {
/// Cache-line padding wrapper for the hottest counters.
///
/// Padding keeps a hot counter off shared cache lines so that concurrent
/// reads (introduced when metric sampling moves off the rx_loop task) do
/// not false-share with the writer. With a single writer today the padding
/// is forward-looking insurance. Derefs to the inner counter so the call
/// sites are identical to an unpadded one.
/// Padding keeps a hot counter off shared cache lines so that reads from
/// off the rx_loop task (the control read handle) do not false-share with
/// the single writer. Derefs to the inner counter so the call sites are
/// identical to an unpadded one.
#[repr(align(64))]
#[derive(Default)]
pub struct Padded<T>(pub T);
@@ -536,6 +535,31 @@ pub struct ErrorMetrics {
/// rising count is a forged or stale reactive signal.
pub mtu_exceeded_uncorroborated: Counter,
pub unbound: UnboundSignals,
/// Admitted `PathBroken` signals against coordinates a lookup verified
/// that left them in place, because reports over distinct links had not
/// yet reached the quorum. The lookup still ran. A genuine failure's
/// reports usually arrive over one link, so this is the ordinary outcome
/// of a real broken path as well as of signals forged or reflected
/// through one neighbour.
pub broken_below_quorum: Counter,
/// Verified coordinates demoted to a hint because reports arriving over
/// distinct links reached the quorum. A genuine failure reported from two
/// directions produces this. Forged reports produce it only when they
/// arrive over two different links.
pub broken_demoted: Counter,
/// Admitted `PathBroken` signals that arrived over a link other than the
/// one this node would forward to the destination on. Counted, never
/// refused. The healthy floor is not zero: 4 to 10 percent of genuine
/// reports arrived off the forward link in a loopback measurement, and a
/// real topology with asymmetric paths will see more.
pub broken_link_mismatch: Counter,
/// Admitted `PathBroken` signals whose reporter is this node, the
/// destination, or a node whose known coordinates are no closer to the
/// destination than this node's. Counted, never refused. The healthy
/// floor is not zero, since the reporter's view of the destination can
/// differ from this node's, and most reporters' coordinates are unknown
/// here and are not counted at all.
pub broken_reporter_mismatch: Counter,
/// Routing errors this node declined to emit because the authenticated
/// link peer that induced them had spent its budget. A rising count is
/// either a peer flooding unroutable traffic or a hub relaying more
@@ -565,6 +589,10 @@ impl ErrorMetrics {
unbound_broken: self.unbound.broken.get(),
unbound_mtu: self.unbound.mtu.get(),
unbound_forged: self.unbound.forged.get(),
broken_below_quorum: self.broken_below_quorum.get(),
broken_demoted: self.broken_demoted.get(),
broken_link_mismatch: self.broken_link_mismatch.get(),
broken_reporter_mismatch: self.broken_reporter_mismatch.get(),
emit_over_peer_budget: self.emit_over_peer_budget.get(),
emit_over_dest_interval: self.emit_over_dest_interval.get(),
emit_limiter_at_capacity: self.emit_limiter_at_capacity.get(),
+17 -2
View File
@@ -55,6 +55,7 @@ use crate::proto::fmp::wire::{
build_established_header, prepend_inner_header,
};
use crate::proto::fsp::Fsp;
use crate::proto::fsp::quorum::LinkQuorum;
use crate::proto::lookup::{Lookup, LookupBackoff, LookupForwardRateLimiter};
use crate::proto::mmp::Mmp;
use crate::proto::routing::{self, Router, RoutingErrorRateLimiter};
@@ -668,6 +669,11 @@ pub struct Node {
/// not a bound on this one, and one PathBroken drives both responses, so
/// a shared limiter would let the coord-warmup arm pay for the release.
path_mtu_release_limiter: RoutingErrorRateLimiter,
/// Distinct links that recently delivered a PathBroken naming a
/// destination whose coordinates a lookup verified. A verified entry is
/// demoted only when this reaches its quorum; any number of reports over
/// one link leave it in place while the lookup re-validates it.
broken_quorum: LinkQuorum,
// === Peering Homeostasis ===
/// Owner of the peering-reconciler state relocated off `Node`: the sans-IO
@@ -930,6 +936,7 @@ impl Node {
path_mtu_release_limiter: RoutingErrorRateLimiter::with_interval_ms(
handlers::session::PATH_MTU_RELEASE_MIN_INTERVAL.as_millis() as u64,
),
broken_quorum: LinkQuorum::new(),
probes: handlers::probe::ProbeRegistry::new(),
lookup: Lookup::new(
LookupBackoff::with_params(backoff_base_secs, backoff_max_secs),
@@ -1099,6 +1106,7 @@ impl Node {
path_mtu_release_limiter: RoutingErrorRateLimiter::with_interval_ms(
handlers::session::PATH_MTU_RELEASE_MIN_INTERVAL.as_millis() as u64,
),
broken_quorum: LinkQuorum::new(),
probes: handlers::probe::ProbeRegistry::new(),
lookup: Lookup::new(LookupBackoff::new(), LookupForwardRateLimiter::new()),
discovery_sign_limiter: LookupSignRateLimiter::new(),
@@ -1208,10 +1216,15 @@ impl Node {
// raising `node.limits.max_connections` actually raises the inbound
// ceiling rather than being silently capped at the transport default.
let node_max_connections = self.config().node.limits.max_connections;
// Inbound stream connections are dropped after this long without a
// complete frame. Derived from the node's own liveness timers so it
// cannot drop a connection whose link the node would keep.
let idle_timeout = handlers::rekey::inbound_idle_timeout(&self.config().node);
for (name, tcp_config) in tcp_instances {
let transport_id = self.allocate_transport_id();
let mut tcp = TcpTransport::new(transport_id, name, tcp_config, packet_tx.clone());
tcp.set_node_max_connections(node_max_connections);
tcp.set_inbound_idle_timeout(idle_timeout);
transports.push(TransportHandle::Tcp(tcp));
}
@@ -1226,7 +1239,8 @@ impl Node {
for (name, tor_config) in tor_instances {
let transport_id = self.allocate_transport_id();
let tor = TorTransport::new(transport_id, name, tor_config, packet_tx.clone());
let mut tor = TorTransport::new(transport_id, name, tor_config, packet_tx.clone());
tor.set_inbound_idle_timeout(idle_timeout);
transports.push(TransportHandle::Tor(tor));
}
@@ -1890,7 +1904,8 @@ impl Node {
// advance together. What is published is data, not a rendered response,
// and it is published only here, rather than as a monolithic per-tick
// rebuild of every query's result. It also is not gated behind any slow
// I/O on the tick the way the abandoned 2edc8a1 republish was.
// I/O on the tick, which was the shape of an earlier, abandoned
// republish design.
// Per-stats-history-peer metadata. `show_stats_peers` /
// `show_stats_history_all_peers` need each tracked peer's live
// membership (`is_active`), resolved npub, and display name — all
+6 -3
View File
@@ -123,9 +123,12 @@ pub enum DiscoveryReject {
/// forwarded to the peer that looped it. Tracked via
/// [`DiscoveryStats::req_own_loopback`](crate::node::stats::DiscoveryStats).
///
/// **This has a nonzero floor in healthy operation** and rises with the
/// bloom fill ratio. It says nothing about the peer that delivered the
/// copy, which is why it is not counted as [`Self::ReqDuplicate`].
/// **This can occur in healthy operation**: a bloom false positive is
/// enough to send a copy back, so the rate rises with the bloom fill
/// ratio. It is counted apart from [`Self::ReqDuplicate`] by cause: this
/// is the node's own fan-out returning, while `ReqDuplicate` is a request
/// id the node has already recorded, seen again. Neither identifies the
/// peer that delivered the copy.
ReqOwnLoopback,
/// Request dedup cache (`recent_requests`) is at capacity, so the
/// `LookupRequest` is dropped without being forwarded. Tracked via
+4
View File
@@ -460,6 +460,10 @@ pub struct ErrorSignalStatsSnapshot {
pub unbound_broken: u64,
pub unbound_mtu: u64,
pub unbound_forged: u64,
pub broken_below_quorum: u64,
pub broken_demoted: u64,
pub broken_link_mismatch: u64,
pub broken_reporter_mismatch: u64,
pub emit_over_peer_budget: u64,
pub emit_over_dest_interval: u64,
pub emit_limiter_at_capacity: u64,
+860
View File
@@ -0,0 +1,860 @@
//! Forged routing signals against a destination whose coordinates a lookup
//! verified.
//!
//! The attack these tests describe: a `PathBroken` naming a destination this
//! node has a session with, followed by a forged `SessionSetup` carrying a
//! different position for that destination under the same root. Every packet
//! is driven through `handle_session_datagram`, the entry point a real one
//! reaches, and the observable is the next hop `find_next_hop` picks.
use super::*;
use crate::node::session::EndToEndState;
use crate::node::tests::spanning_tree::{TestNode, cleanup_nodes, run_tree_test};
use crate::proto::fsp::SessionSetup;
use crate::proto::link::SessionDatagram;
use crate::proto::routing::PathBroken;
use crate::proto::stp::TreeCoordinate;
/// Index of the victim in the fixture's node vector.
const V: usize = 0;
/// Index of the peer the destination genuinely sits under.
const P1: usize = 1;
/// Index of the peer the forged coordinates point at.
const P2: usize = 2;
/// The victim, its two tree peers, and a destination it holds a session
/// with whose real and forged coordinates differ in the peer they hang off.
struct Fixture {
nodes: Vec<TestNode>,
dest: NodeAddr,
p1: NodeAddr,
p2: NodeAddr,
real: TreeCoordinate,
forged: TreeCoordinate,
}
/// Wall-clock milliseconds, the clock `handle_path_broken` and
/// `find_next_hop` read. A verification stamped on any other clock reads as
/// aged out to the handler.
fn wall_ms() -> u64 {
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_millis() as u64
}
/// `dest` hung directly under `parent`, sharing its root.
fn coords_under(dest: NodeAddr, parent: &TreeCoordinate) -> TreeCoordinate {
let mut addrs = vec![dest];
addrs.extend(parent.node_addrs().copied());
TreeCoordinate::from_addrs(addrs).unwrap()
}
/// Install the entry `initiate_session` creates for `remote`, which is what
/// makes the admission gate accept a signal naming it.
fn install_initiating(node: &mut Node, remote: &Identity) {
use crate::noise::HandshakeState;
let handshake =
HandshakeState::new_xk_initiator(node.identity().keypair(), remote.pubkey_full());
let entry = crate::node::session::SessionEntry::new(
*remote.node_addr(),
remote.pubkey_full(),
EndToEndState::Initiating(handshake),
1000,
true,
);
node.sessions.insert(*remote.node_addr(), entry);
}
/// Build the fixture and assert the two preconditions every test relies on.
async fn fixture() -> Fixture {
let mut nodes = run_tree_test(3, &[(V, P1), (V, P2)], false).await;
let p1 = *nodes[P1].node.node_addr();
let p2 = *nodes[P2].node.node_addr();
let remote = Identity::generate();
let dest = *remote.node_addr();
install_initiating(&mut nodes[V].node, &remote);
let real = coords_under(dest, nodes[P1].node.tree_state().my_coords());
let forged = coords_under(dest, nodes[P2].node.tree_state().my_coords());
nodes[V]
.node
.coord_cache_mut()
.insert_verified(dest, real.clone(), wall_ms());
let mut fx = Fixture {
nodes,
dest,
p1,
p2,
real,
forged,
};
assert_eq!(
fx.next_hop(),
Some(fx.p1),
"precondition: the verified coordinates route via P1"
);
let forged_hop = fx.nodes[V]
.node
.tree_state()
.find_next_hop(&fx.forged, &std::collections::BTreeSet::new());
assert_eq!(
forged_hop,
Some(fx.p2),
"precondition: the forged coordinates would route via P2, so a \
successful plant is visible as a flip"
);
fx
}
impl Fixture {
/// The next hop the victim picks toward the destination.
fn next_hop(&mut self) -> Option<NodeAddr> {
let dest = self.dest;
self.nodes[V]
.node
.find_next_hop(&dest)
.map(|p| *p.node_addr())
}
/// The victim's cache entry for the destination: its value and whether it
/// is still verified on the handler's clock.
fn entry(&self) -> Option<(TreeCoordinate, bool)> {
self.nodes[V]
.node
.coord_cache()
.get_entry(&self.dest)
.map(|e| (e.coords().clone(), e.is_verified(wall_ms())))
}
/// Deliver a `PathBroken` naming the destination, claiming `reporter` as
/// both the datagram source and the body's reporter, arriving over the
/// link to `link_peer`.
async fn path_broken(&mut self, reporter: NodeAddr, link_peer: NodeAddr) {
self.path_broken_from(reporter, reporter, link_peer).await;
}
/// Deliver a `PathBroken` naming the destination from datagram source
/// `src`, whose body names `reporter`, arriving over the link to
/// `link_peer`. Both identities are the sender's to choose.
async fn path_broken_from(&mut self, src: NodeAddr, reporter: NodeAddr, link_peer: NodeAddr) {
let victim = *self.nodes[V].node.node_addr();
let payload = PathBroken::new(self.dest, reporter).encode();
let encoded = SessionDatagram::new(src, victim, payload).encode();
self.nodes[V]
.node
.handle_session_datagram(&link_peer, &encoded[1..], false)
.await;
}
/// Deliver a forged `SessionSetup` in transit, claiming to be from the
/// destination and carrying the forged coordinates for it, over the link
/// to P2.
async fn forged_setup(&mut self) {
let payload = SessionSetup::new(self.forged.clone(), self.forged.clone()).encode();
let encoded = SessionDatagram::new(self.dest, self.dest, payload).encode();
let p2 = self.p2;
self.nodes[V]
.node
.handle_session_datagram(&p2, &encoded[1..], false)
.await;
}
}
/// One forged warm against a verified destination changes nothing: the
/// precedence rule refuses the hint, and the route stays on P1.
#[tokio::test]
async fn a_forged_same_root_warm_does_not_move_the_route_to_a_verified_destination() {
let mut fx = fixture().await;
let rejected = fx.nodes[V]
.node
.metrics()
.forwarding
.coord_hint_rejected
.get();
fx.forged_setup().await;
assert_eq!(fx.next_hop(), Some(fx.p1), "a forged warm moved the route");
assert!(
fx.nodes[V]
.node
.metrics()
.forwarding
.coord_hint_rejected
.get()
> rejected,
"the refused hint should be counted"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// One forged `PathBroken` followed by one forged warm: the two-packet attack.
/// The signal must not strip the verification that refuses the warm.
#[tokio::test]
async fn a_forged_path_broken_then_a_forged_warm_does_not_move_the_route_to_a_verified_destination()
{
let mut fx = fixture().await;
let p2 = fx.p2;
fx.path_broken(make_node_addr(0xB1), p2).await;
fx.forged_setup().await;
assert_eq!(
fx.next_hop(),
Some(fx.p1),
"the forged warm moved the route"
);
assert_eq!(
fx.entry(),
Some((fx.real.clone(), true)),
"the verified entry must survive one PathBroken with its value and \
its verification"
);
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_below_quorum.get(), 1);
assert_eq!(errors.broken_demoted.get(), 0);
cleanup_nodes(&mut fx.nodes).await;
}
/// A direct forger on one link inventing two reporters is still one vote: the
/// entry stays verified and the forged warm that follows is refused. Keyed on
/// the body's reporter, the two invented names would have reached the quorum
/// and the warm would have moved the route.
#[tokio::test]
async fn a_forger_inventing_two_reporters_over_one_link_does_not_demote_a_verified_entry() {
let mut fx = fixture().await;
let p2 = fx.p2;
fx.path_broken(make_node_addr(0xB1), p2).await;
fx.path_broken(make_node_addr(0xB2), p2).await;
fx.forged_setup().await;
assert_eq!(
fx.entry(),
Some((fx.real.clone(), true)),
"two invented reporters over one link demoted the verified entry"
);
assert_eq!(
fx.next_hop(),
Some(fx.p1),
"the forged warm moved the route"
);
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_below_quorum.get(), 2);
assert_eq!(errors.broken_demoted.get(), 0);
cleanup_nodes(&mut fx.nodes).await;
}
/// Two genuinely different reporters whose signals both arrive over one link
/// are one vote too: what counts is the authenticated link, not the
/// reporter, nor the pair of the two.
#[tokio::test]
async fn two_reporters_over_the_same_link_do_not_reach_the_quorum() {
let mut fx = fixture().await;
let (p1, p2) = (fx.p1, fx.p2);
fx.path_broken(p1, p1).await;
fx.path_broken(p2, p1).await;
assert_eq!(fx.entry(), Some((fx.real.clone(), true)));
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_below_quorum.get(), 2);
assert_eq!(errors.broken_demoted.get(), 0);
cleanup_nodes(&mut fx.nodes).await;
}
/// Reports arriving over two different links reach the quorum and demote the
/// entry, and the forged warm that follows then moves the route. This is the
/// residual the quorum leaves: forged reports demote the entry once they
/// arrive over two different links, as a genuine failure reported from two
/// directions does. Asserted so that it stays visible.
#[tokio::test]
async fn reports_over_two_different_links_demote_a_verified_entry_and_a_following_warm_then_moves_the_route()
{
let mut fx = fixture().await;
let (p1, p2) = (fx.p1, fx.p2);
fx.path_broken(make_node_addr(0xB1), p1).await;
fx.path_broken(make_node_addr(0xB2), p2).await;
assert_eq!(
fx.entry(),
Some((fx.real.clone(), false)),
"a demoted entry keeps its value and loses its verification"
);
assert_eq!(
fx.nodes[V]
.node
.coord_cache()
.get_entry(&fx.dest)
.map(|e| e.source()),
Some(crate::cache::CoordSource::Hint)
);
assert_eq!(
fx.next_hop(),
Some(fx.p1),
"demotion alone does not move the route"
);
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_demoted.get(), 1);
assert_eq!(errors.broken_below_quorum.get(), 1);
fx.forged_setup().await;
assert_eq!(
fx.next_hop(),
Some(fx.p2),
"once demoted, the entry is a hint and a forged warm replaces it"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// A node with a single peer receives every report over one link, so it never
/// demotes by quorum, however many reporters the reports name. What bounds a
/// stale verified entry there is its verification age: one still inside
/// `VERIFIED_TTL_MS` is kept, one past it is removed by the next report like
/// any hint.
#[tokio::test]
async fn on_a_single_link_node_only_the_verification_age_bounds_a_verified_entry() {
use crate::cache::VERIFIED_TTL_MS;
// V - P1, and V holds a session with a destination beyond P1.
let mut nodes = run_tree_test(2, &[(V, P1)], false).await;
let p1 = *nodes[P1].node.node_addr();
let victim = *nodes[V].node.node_addr();
let remote = Identity::generate();
let dest = *remote.node_addr();
install_initiating(&mut nodes[V].node, &remote);
let coords = coords_under(dest, nodes[P1].node.tree_state().my_coords());
let report = |reporter: NodeAddr| {
let payload = PathBroken::new(dest, reporter).encode();
SessionDatagram::new(reporter, victim, payload).encode()
};
let verified = |node: &Node| {
node.coord_cache()
.get_entry(&dest)
.map(|e| e.is_verified(wall_ms()))
};
// A cache TTL long enough that expiry never removes the entry, so every
// removal below is the handler's. Ten seconds of margin inside the
// bound, so the test's own run time cannot age the entry out before the
// reports land.
let cache = nodes[V].node.coord_cache_mut();
cache.set_default_ttl_ms(4 * VERIFIED_TTL_MS);
cache.insert_verified(dest, coords.clone(), wall_ms() - VERIFIED_TTL_MS + 10_000);
for r in 0..5u8 {
let encoded = report(make_node_addr(0xB0 + r));
nodes[V]
.node
.handle_session_datagram(&p1, &encoded[1..], false)
.await;
}
assert_eq!(
verified(&nodes[V].node),
Some(true),
"reports over one link demoted or removed an entry still inside its \
verification"
);
let errors = &nodes[V].node.metrics().errors;
assert_eq!(errors.broken_below_quorum.get(), 5);
assert_eq!(errors.broken_demoted.get(), 0);
// Past the bound the entry no longer refuses anything, and the next
// report removes it.
nodes[V].node.coord_cache_mut().insert_verified(
dest,
coords,
wall_ms() - VERIFIED_TTL_MS - 1_000,
);
assert_eq!(
verified(&nodes[V].node),
Some(false),
"precondition: the entry is present and its verification has aged out"
);
let encoded = report(make_node_addr(0xC0));
nodes[V]
.node
.handle_session_datagram(&p1, &encoded[1..], false)
.await;
assert_eq!(
verified(&nodes[V].node),
None,
"an aged-out entry is removed"
);
cleanup_nodes(&mut nodes).await;
}
/// One reporter repeating itself over one link is one vote: the entry stays
/// verified and the forged warm is still refused.
#[tokio::test]
async fn one_reporter_repeating_a_path_broken_does_not_reach_the_quorum() {
let mut fx = fixture().await;
let p2 = fx.p2;
fx.path_broken(make_node_addr(0xB1), p2).await;
fx.path_broken(make_node_addr(0xB1), p2).await;
fx.forged_setup().await;
assert_eq!(fx.next_hop(), Some(fx.p1));
assert_eq!(fx.entry(), Some((fx.real.clone(), true)));
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_below_quorum.get(), 2);
assert_eq!(errors.broken_demoted.get(), 0);
cleanup_nodes(&mut fx.nodes).await;
}
/// A verification that has aged out protects nothing, so the entry is
/// removed as a hint would be.
#[tokio::test]
async fn a_path_broken_removes_a_verified_entry_whose_verification_has_aged_out() {
let mut fx = fixture().await;
let dest = fx.dest;
let real = fx.real.clone();
let p1 = fx.p1;
let cache = fx.nodes[V].node.coord_cache_mut();
// A TTL long enough that the entry is still live when its verification
// is not, so the removal below is the handler's and not expiry's.
cache.set_default_ttl_ms(4 * crate::cache::VERIFIED_TTL_MS);
cache.insert_verified(
dest,
real,
wall_ms() - crate::cache::VERIFIED_TTL_MS - 1_000,
);
assert_eq!(
fx.entry().map(|(_, verified)| verified),
Some(false),
"precondition: the entry is present and no longer verified"
);
fx.path_broken(make_node_addr(0xB1), p1).await;
assert!(fx.entry().is_none(), "an unverified entry is removed");
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_below_quorum.get(), 0);
assert_eq!(errors.broken_demoted.get(), 0);
cleanup_nodes(&mut fx.nodes).await;
}
/// When the path-MTU release fires, a kept entry forgets the MTU the lookup
/// stored with it, so it does not go on displaying a released value.
#[tokio::test]
async fn a_kept_entry_forgets_its_path_mtu_when_the_release_fires() {
let mut fx = fixture().await;
let dest = fx.dest;
let real = fx.real.clone();
let p2 = fx.p2;
fx.nodes[V]
.node
.coord_cache_mut()
.insert_verified_with_path_mtu(dest, real, wall_ms(), 1200);
fx.path_broken(make_node_addr(0xB1), p2).await;
let entry = fx.nodes[V].node.coord_cache().get_entry(&dest).unwrap();
assert!(
entry.is_verified(wall_ms()),
"precondition: the entry was kept"
);
assert_eq!(entry.path_mtu(), None);
cleanup_nodes(&mut fx.nodes).await;
}
/// When the path-MTU release is rate limited, a kept entry keeps its MTU, as
/// the path-MTU map does: the two describe the same path and must not
/// disagree.
#[tokio::test]
async fn a_kept_entry_keeps_its_path_mtu_when_the_release_is_rate_limited() {
let mut fx = fixture().await;
let dest = fx.dest;
let real = fx.real.clone();
let p2 = fx.p2;
let fips = crate::FipsAddress::from_node_addr(&dest);
// The first report spends the release budget for this destination.
fx.path_broken(make_node_addr(0xB1), p2).await;
// A value learned again since, in both stores.
fx.nodes[V]
.node
.coord_cache_mut()
.insert_verified_with_path_mtu(dest, real, wall_ms(), 1200);
fx.nodes[V].node.path_mtu_lookup_insert(fips, 1200);
// A second report inside the release interval (same link, so it stays
// below the quorum and the entry is kept).
fx.path_broken(make_node_addr(0xB2), p2).await;
let node = &fx.nodes[V].node;
let entry = node.coord_cache().get_entry(&dest).unwrap();
assert!(
entry.is_verified(wall_ms()),
"precondition: the entry was kept"
);
assert_eq!(
node.path_mtu_lookup_get(&fips),
Some(1200),
"precondition: the release was rate limited"
);
assert_eq!(
entry.path_mtu(),
Some(1200),
"a rate-limited release cleared the kept entry's MTU but not the map's"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// The sum of the lookup counters `maybe_initiate_lookup` moves on every
/// outcome, so a test can see that it ran whatever it decided.
fn lookup_attempts(node: &Node) -> u64 {
let l = &node.metrics().lookup;
l.req_initiated.get()
+ l.req_bloom_miss.get()
+ l.req_deduplicated.get()
+ l.req_backoff_suppressed.get()
}
/// An admitted `PathBroken` re-validates the destination by lookup even when
/// its identity is not cached, which is the fixture's state: the session entry
/// alone does not register the identity.
#[tokio::test]
async fn an_admitted_path_broken_starts_a_lookup_without_a_cached_identity() {
let mut fx = fixture().await;
let dest = fx.dest;
assert!(
!fx.nodes[V].node.has_cached_identity(&dest),
"precondition: the destination's identity is not cached"
);
let before = lookup_attempts(&fx.nodes[V].node);
let p1 = fx.p1;
fx.path_broken(make_node_addr(0xB1), p1).await;
assert!(
lookup_attempts(&fx.nodes[V].node) > before,
"the PathBroken should have run the lookup path"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// The lookup a PathBroken starts must be answerable when the destination's
/// identity is not cached. The session already holds the destination's key,
/// so the lookup's answer can be verified with it: the position is learned,
/// the lookup does not run to its timeout, and packets queued for the
/// destination are not dropped as unreachable when it would have.
#[tokio::test]
async fn a_path_broken_without_a_cached_identity_starts_a_lookup_this_node_can_verify() {
// V - P1 - D, with D a real node V has a session with but whose identity
// V has not cached.
let mut nodes = run_tree_test(3, &[(0, 1), (1, 2)], false).await;
let p1 = *nodes[1].node.node_addr();
let dest = *nodes[2].node.node_addr();
let dest_pubkey = nodes[2].node.identity().pubkey_full();
let real: Vec<NodeAddr> = nodes[2]
.node
.tree_state()
.my_coords()
.node_addrs()
.copied()
.collect();
let handshake = crate::noise::HandshakeState::new_xk_initiator(
nodes[0].node.identity().keypair(),
dest_pubkey,
);
nodes[0].node.sessions.insert(
dest,
crate::node::session::SessionEntry::new(
dest,
dest_pubkey,
EndToEndState::Initiating(handshake),
1000,
true,
),
);
assert!(
!nodes[0].node.has_cached_identity(&dest),
"precondition: the destination's identity is not cached"
);
nodes[0]
.node
.queue_pending_tun_packet_for_test(dest, vec![0x60; 40]);
let lookup = &nodes[0].node.metrics().lookup;
let (accepted, miss, timed_out) = (
lookup.resp_accepted.get(),
lookup.resp_identity_miss.get(),
lookup.resp_timed_out.get(),
);
let victim = *nodes[0].node.node_addr();
let payload = PathBroken::new(dest, p1).encode();
let encoded = SessionDatagram::new(p1, victim, payload).encode();
let start = wall_ms();
nodes[0]
.node
.handle_session_datagram(&p1, &encoded[1..], false)
.await;
for _ in 0..10 {
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
crate::node::tests::spanning_tree::process_available_packets(&mut nodes).await;
}
let lookup = &nodes[0].node.metrics().lookup;
assert_eq!(
lookup.resp_identity_miss.get(),
miss,
"the lookup's answer could not be verified for want of an identity"
);
assert!(
lookup.resp_accepted.get() > accepted,
"the lookup's answer must be verified and accepted"
);
let entry = nodes[0].node.coord_cache().get_entry(&dest).unwrap();
assert_eq!(
entry.coords().node_addrs().copied().collect::<Vec<_>>(),
real
);
assert!(entry.is_verified(wall_ms()));
// Run the lookup schedule well past its end: an answered lookup has
// nothing left to time out, so the queued packet survives.
for step in 1..=6 {
nodes[0]
.node
.check_pending_lookups(start + step * 20_000)
.await;
}
assert_eq!(
nodes[0].node.metrics().lookup.resp_timed_out.get(),
timed_out,
"the lookup ran to its timeout"
);
assert_eq!(
nodes[0].node.pending_tun_total_packets(),
1,
"the packet queued for the destination was dropped"
);
cleanup_nodes(&mut nodes).await;
}
/// The healthy path for keeping a verified entry below quorum: a genuine
/// failure reported once is still recovered, because the lookup the report
/// starts replaces the stale value with the destination's real position.
#[tokio::test]
async fn a_genuine_path_broken_still_replaces_stale_verified_coordinates_by_lookup() {
// V - P1 - D, with D a real node V has a session with, and a second peer
// P2 on its own, so a later report can arrive over a different link.
let mut nodes = run_tree_test(4, &[(0, 1), (1, 2), (0, 3)], false).await;
let p1 = *nodes[1].node.node_addr();
let p2 = *nodes[3].node.node_addr();
let dest = *nodes[2].node.node_addr();
let dest_pubkey = nodes[2].node.identity().pubkey_full();
let real: Vec<NodeAddr> = nodes[2]
.node
.tree_state()
.my_coords()
.node_addrs()
.copied()
.collect();
nodes[0].node.register_identity(dest, dest_pubkey);
let handshake = crate::noise::HandshakeState::new_xk_initiator(
nodes[0].node.identity().keypair(),
dest_pubkey,
);
nodes[0].node.sessions.insert(
dest,
crate::node::session::SessionEntry::new(
dest,
dest_pubkey,
EndToEndState::Initiating(handshake),
1000,
true,
),
);
// D "moved": V holds a verified position for it that is no longer true.
let stale = coords_under(dest, nodes[0].node.tree_state().my_coords());
assert_ne!(stale.node_addrs().copied().collect::<Vec<_>>(), real);
nodes[0]
.node
.coord_cache_mut()
.insert_verified(dest, stale, wall_ms());
let lookup = &nodes[0].node.metrics().lookup;
let (initiated, accepted) = (lookup.req_initiated.get(), lookup.resp_accepted.get());
let victim = *nodes[0].node.node_addr();
let payload = PathBroken::new(dest, p1).encode();
let encoded = SessionDatagram::new(p1, victim, payload).encode();
nodes[0]
.node
.handle_session_datagram(&p1, &encoded[1..], false)
.await;
assert_eq!(nodes[0].node.metrics().errors.broken_below_quorum.get(), 1);
for _ in 0..10 {
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
crate::node::tests::spanning_tree::process_available_packets(&mut nodes).await;
}
let lookup = &nodes[0].node.metrics().lookup;
assert!(
lookup.req_initiated.get() > initiated,
"the report must start a lookup"
);
assert!(
lookup.resp_accepted.get() > accepted,
"the lookup must be answered"
);
let entry = nodes[0].node.coord_cache().get_entry(&dest).unwrap();
assert_eq!(
entry.coords().node_addrs().copied().collect::<Vec<_>>(),
real,
"the lookup must replace the stale position with the real one"
);
assert!(entry.is_verified(wall_ms()));
// The lookup verified the destination again, so the report about the old
// path no longer counts: a report over a second link now starts a new
// quorum rather than completing the old one.
let payload = PathBroken::new(dest, make_node_addr(0xB2)).encode();
let encoded = SessionDatagram::new(make_node_addr(0xB2), victim, payload).encode();
nodes[0]
.node
.handle_session_datagram(&p2, &encoded[1..], false)
.await;
let errors = &nodes[0].node.metrics().errors;
assert_eq!(
errors.broken_demoted.get(),
0,
"a report from before the re-verification combined with one after it"
);
assert_eq!(errors.broken_below_quorum.get(), 2);
cleanup_nodes(&mut nodes).await;
}
/// A PathBroken arriving off the forward link is counted and still acted on
/// in full: the advisory check refuses nothing.
#[tokio::test]
async fn a_path_broken_off_the_forward_link_is_counted_and_still_acted_on() {
let mut fx = fixture().await;
let dest = fx.dest;
let p2 = fx.p2;
fx.nodes[V]
.node
.sessions
.get_mut(&dest)
.unwrap()
.set_coords_warmup_remaining(0);
let lookups = lookup_attempts(&fx.nodes[V].node);
fx.path_broken(make_node_addr(0xB1), p2).await;
let node = &fx.nodes[V].node;
assert_eq!(node.metrics().errors.broken_link_mismatch.get(), 1);
assert_eq!(
node.sessions.get(&dest).unwrap().coords_warmup_remaining(),
node.config().node.session.coords_warmup_packets,
"the warmup counter is reset whatever the link"
);
assert!(
node.config().node.session.coords_warmup_packets > 0,
"precondition: a reset is distinguishable from the zero set above"
);
assert!(
lookup_attempts(node) > lookups,
"the lookup runs whatever the link"
);
assert_eq!(node.metrics().errors.broken_below_quorum.get(), 1);
cleanup_nodes(&mut fx.nodes).await;
}
/// The same report over the forward link is not counted.
#[tokio::test]
async fn a_path_broken_over_the_forward_link_is_not_counted_as_a_mismatch() {
let mut fx = fixture().await;
let p1 = fx.p1;
fx.path_broken(make_node_addr(0xB1), p1).await;
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(errors.broken_link_mismatch.get(), 0);
assert_eq!(
errors.broken_below_quorum.get(),
1,
"the signal was admitted and handled, so the zero above is a real zero"
);
cleanup_nodes(&mut fx.nodes).await;
}
/// The reporter-mismatch count after one PathBroken naming the fixture's
/// destination, reported by the address `pick` chooses, over the forward link.
async fn reporter_mismatches(pick: impl FnOnce(&Fixture) -> NodeAddr) -> u64 {
mismatches_from(|fx| {
let reporter = pick(fx);
(reporter, reporter)
})
.await
}
/// The reporter-mismatch count after one PathBroken naming the fixture's
/// destination over the forward link, with the datagram source and the
/// body's reporter that `pick` returns, in that order.
async fn mismatches_from(pick: impl FnOnce(&Fixture) -> (NodeAddr, NodeAddr)) -> u64 {
let mut fx = fixture().await;
let (src, reporter) = pick(&fx);
let p1 = fx.p1;
fx.path_broken_from(src, reporter, p1).await;
let errors = &fx.nodes[V].node.metrics().errors;
assert_eq!(
errors.broken_below_quorum.get(),
1,
"precondition: the signal reached the handler's verified-entry path, \
so the count below observed it"
);
let n = errors.broken_reporter_mismatch.get();
cleanup_nodes(&mut fx.nodes).await;
n
}
#[tokio::test]
async fn a_reporter_farther_from_the_destination_than_this_node_is_counted() {
// P2 is a direct peer on the far side of this node from P1, under which
// the destination sits.
assert_eq!(reporter_mismatches(|fx| fx.p2).await, 1);
}
#[tokio::test]
async fn a_reporter_closer_to_the_destination_than_this_node_is_not_counted() {
assert_eq!(reporter_mismatches(|fx| fx.p1).await, 0);
}
#[tokio::test]
async fn a_reporter_with_unknown_coordinates_is_not_counted() {
assert_eq!(reporter_mismatches(|_| make_node_addr(0xB1)).await, 0);
}
#[tokio::test]
async fn this_node_named_as_the_reporter_is_counted() {
assert_eq!(
reporter_mismatches(|fx| *fx.nodes[V].node.node_addr()).await,
1
);
}
/// The destination named as its own reporter cannot be reporting a broken
/// path to itself. The datagram source differs from the reporter here,
/// because a datagram claiming to come from the destination is refused by the
/// admission gate before this check is reached.
#[tokio::test]
async fn the_destination_named_as_the_reporter_is_counted() {
assert_eq!(
mismatches_from(|fx| (make_node_addr(0xB1), fx.dest)).await,
1
);
}
+1
View File
@@ -13,6 +13,7 @@ mod bloom_poison;
mod bootstrap;
mod connected_udp;
mod control;
mod coord_forgery;
mod decrypt_failure;
mod disconnect;
mod discovery;
+15 -8
View File
@@ -4306,7 +4306,9 @@ async fn test_path_broken_releases_path_mtu_lookup_entry() {
assertion below observes nothing"
);
tn.node.handle_path_broken(&reporter, inner).await;
tn.node
.handle_path_broken(&reporter, &reporter, inner)
.await;
assert_eq!(
tn.node.path_mtu_lookup_get(&dest_fips),
@@ -4361,7 +4363,9 @@ async fn test_path_broken_resets_the_session_source_path_mtu() {
assertion below observes nothing"
);
tn.node.handle_path_broken(&reporter, inner).await;
tn.node
.handle_path_broken(&reporter, &reporter, inner)
.await;
assert_eq!(
tn.node
@@ -4699,7 +4703,8 @@ async fn test_path_broken_naming_a_dest_with_no_session_does_not_flush_cached_co
let _ = node.coord_cache_mut().insert(dest, coords, 1000);
let encoded = PathBroken::new(dest, reporter).encode();
node.handle_path_broken(&reporter, &encoded[5..]).await;
node.handle_path_broken(&reporter, &reporter, &encoded[5..])
.await;
assert!(
node.coord_cache().get(&dest, 1000).is_some(),
@@ -4746,7 +4751,8 @@ async fn test_path_broken_naming_a_dest_whose_entry_is_an_unauthenticated_respon
install_halfopen(&mut node, dest);
let encoded = PathBroken::new(dest, reporter).encode();
node.handle_path_broken(&reporter, &encoded[5..]).await;
node.handle_path_broken(&reporter, &reporter, &encoded[5..])
.await;
assert!(
node.coord_cache().get(&dest, 1000).is_some(),
@@ -4781,7 +4787,8 @@ async fn test_path_broken_for_a_session_we_initiated_still_flushes_cached_coords
let _ = node.coord_cache_mut().insert(dest, coords, 1000);
let encoded = PathBroken::new(dest, reporter).encode();
node.handle_path_broken(&reporter, &encoded[5..]).await;
node.handle_path_broken(&reporter, &reporter, &encoded[5..])
.await;
assert!(
node.coord_cache().get(&dest, 1000).is_none(),
@@ -6993,7 +7000,7 @@ async fn deliver_path_broken(
let encoded = PathBroken::new(dest, reporter).encode();
nodes[at]
.node
.handle_path_broken(&reporter, &encoded[5..])
.handle_path_broken(&reporter, &reporter, &encoded[5..])
.await;
}
@@ -8994,7 +9001,7 @@ async fn a_path_broken_flood_releases_the_stored_path_mtu_only_once_per_interval
let inner = &encoded[5..];
node.path_mtu_lookup_insert(dest_fips, 700);
node.handle_path_broken(&reporter, inner).await;
node.handle_path_broken(&reporter, &reporter, inner).await;
assert_eq!(
node.path_mtu_lookup_get(&dest_fips),
None,
@@ -9002,7 +9009,7 @@ async fn a_path_broken_flood_releases_the_stored_path_mtu_only_once_per_interval
);
node.path_mtu_lookup_insert(dest_fips, 700);
node.handle_path_broken(&reporter, inner).await;
node.handle_path_broken(&reporter, &reporter, inner).await;
assert_eq!(
node.path_mtu_lookup_get(&dest_fips),
Some(700),
+88 -1
View File
@@ -12,7 +12,8 @@ use crate::transport::{
ConnectionState, TransportAddr, TransportHandle, TransportId, packet_channel,
};
use spanning_tree::{
TestNode, cleanup_nodes, drain_all_packets, initiate_handshake, verify_tree_convergence,
TestNode, cleanup_nodes, drain_all_packets, initiate_handshake, process_available_packets,
verify_tree_convergence,
};
use std::time::Duration;
@@ -29,6 +30,13 @@ async fn make_test_node_tcp() -> TestNode {
/// so immutable fields (e.g. heartbeat/link-dead timeouts) are set before the
/// `NodeContext` is built rather than poked afterward.
async fn make_test_node_tcp_with(config: Config) -> TestNode {
make_test_node_tcp_idle(config, None).await
}
/// Like `make_test_node_tcp_with`, and also sets the transport's inbound
/// idle deadline when `idle` is given, as `create_transports` does for a
/// node built from config.
async fn make_test_node_tcp_idle(config: Config, idle: Option<Duration>) -> TestNode {
let mut node = make_node_with(config);
let transport_id = TransportId::new(1);
@@ -40,6 +48,9 @@ async fn make_test_node_tcp_with(config: Config) -> TestNode {
let (packet_tx, packet_rx) = packet_channel(256);
let mut transport = TcpTransport::new(transport_id, None, config, packet_tx);
if let Some(d) = idle {
transport.set_inbound_idle_timeout(d);
}
transport.start_async().await.unwrap();
let local_addr = transport
@@ -218,6 +229,82 @@ async fn test_tcp_connection_loss_detection() {
cleanup_nodes(&mut nodes).await;
}
/// A TCP link whose only traffic is heartbeats outlives several inbound idle
/// deadlines when the deadline is the one the node derives from its own
/// liveness timers.
///
/// This is the healthy-path check for the idle deadline: it uses the real
/// derivation (`inbound_idle_timeout`), not a hand-picked value, and
/// short timers so three deadlines fit in a test. Break-check: give the
/// listening node's transport an idle deadline below the heartbeat interval
/// and its inbound connection is dropped, so the inbound count falls to 0.
#[tokio::test]
async fn tcp_link_kept_alive_only_by_heartbeats_survives_several_idle_deadlines() {
use crate::node::handlers::rekey::inbound_idle_timeout;
let mut config = Config::new();
config.node.heartbeat_interval_secs = 1;
config.node.link_dead_timeout_secs = 3;
config.node.rate_limit.handshake_resend_interval_ms = 100;
config.node.rate_limit.handshake_max_resends = 1;
let idle = inbound_idle_timeout(&config.node);
// 100 ms of ladder, three 1 s ticks, 3 s link-dead.
assert_eq!(idle, Duration::from_millis(6100));
let mut nodes = vec![
make_test_node_tcp_idle(config.clone(), Some(idle)).await,
make_test_node_tcp_idle(config, Some(idle)).await,
];
// Node 0 dials node 1, so node 1 holds the inbound connection.
initiate_handshake(&mut nodes, 0, 1).await;
assert!(drain_all_packets(&mut nodes, false).await > 0);
let addr_0 = *nodes[0].node.node_addr();
let addr_1 = *nodes[1].node.node_addr();
let inbound = |nodes: &[TestNode]| match nodes[1].node.transports.get(&nodes[1].transport_id) {
Some(TransportHandle::Tcp(t)) => t.stats().pool_inbound_count(),
_ => panic!("node 1 should have its TCP transport"),
};
assert_eq!(
inbound(&nodes),
1,
"node 1 should hold one inbound connection"
);
// Heartbeats only, for three idle deadlines and a little more.
let start = tokio::time::Instant::now();
let mut processed = 0;
while start.elapsed() < idle * 3 + Duration::from_millis(500) {
nodes[0].node.check_link_heartbeats().await;
nodes[1].node.check_link_heartbeats().await;
processed += process_available_packets(&mut nodes).await;
assert_eq!(
inbound(&nodes),
1,
"the inbound connection was dropped after {:?} of heartbeat-only traffic",
start.elapsed()
);
tokio::time::sleep(Duration::from_millis(250)).await;
}
// Sibling control: heartbeats were actually exchanged, so the kept
// connection is not an artefact of nothing having run.
assert!(
processed >= 18,
"expected at least one heartbeat per second each way, processed {processed}"
);
assert!(
nodes[0].node.get_peer(&addr_1).is_some(),
"node 0 lost node 1"
);
assert!(
nodes[1].node.get_peer(&addr_0).is_some(),
"node 1 lost node 0"
);
cleanup_nodes(&mut nodes).await;
}
/// TCP reconnection after link death: connect-on-send re-establishes the link.
///
/// After both peers detect a dead link and remove each other, a fresh
+104
View File
@@ -3854,6 +3854,110 @@ async fn a_node_without_ble_config_draws_no_ble_warning() {
);
}
/// The inbound idle deadline is the node's link-silence bound: 64 s at stock
/// settings, moving one for one with the link-dead timeout, and never below
/// the link-dead timeout plus a tick, the shortest silence after which the
/// node itself would reap the link.
#[test]
fn the_inbound_idle_deadline_is_the_link_silence_bound_and_tracks_the_link_dead_timeout() {
use crate::node::handlers::rekey::inbound_idle_timeout;
use crate::transport::tcp::INBOUND_IDLE_TIMEOUT;
// 31 s of msg1 ladder (1+2+4+8+16), three 1 s ticks, 30 s link-dead.
let stock = crate::config::NodeConfig::default();
assert_eq!(inbound_idle_timeout(&stock), Duration::from_secs(64));
// The transport default a TCP or Tor instance holds before the node sets
// it must be the same stock value, so the two cannot drift apart.
assert_eq!(inbound_idle_timeout(&stock), INBOUND_IDLE_TIMEOUT);
let raised = crate::config::NodeConfig {
link_dead_timeout_secs: 300,
..Default::default()
};
assert_eq!(
inbound_idle_timeout(&raised),
inbound_idle_timeout(&stock) + Duration::from_secs(270)
);
for node in [
stock,
raised,
crate::config::NodeConfig {
heartbeat_interval_secs: 90,
..Default::default()
},
crate::config::NodeConfig {
tick_interval_secs: 5,
link_dead_timeout_secs: 10,
..Default::default()
},
] {
let floor = Duration::from_secs(
node.link_dead_timeout_secs
.max(node.heartbeat_interval_secs)
+ node.tick_interval_secs,
);
assert!(
inbound_idle_timeout(&node) > floor,
"idle deadline {:?} must exceed {:?} for heartbeat {} s, link-dead {} s, tick {} s",
inbound_idle_timeout(&node),
floor,
node.heartbeat_interval_secs,
node.link_dead_timeout_secs,
node.tick_interval_secs,
);
}
}
/// `create_transports` hands every TCP and Tor instance the idle deadline
/// derived from the node's own liveness timers, not the transport default.
///
/// Break-check: remove either `set_inbound_idle_timeout` call and that
/// transport keeps `INBOUND_IDLE_TIMEOUT`, which the non-default timers here
/// are chosen to differ from.
#[tokio::test]
async fn create_transports_sets_the_inbound_idle_deadline_from_the_node_liveness_timers() {
use crate::node::handlers::rekey::inbound_idle_timeout;
use crate::transport::tcp::INBOUND_IDLE_TIMEOUT;
let mut config = crate::Config::new();
config.node.control.enabled = false;
config.node.heartbeat_interval_secs = 5;
config.node.link_dead_timeout_secs = 120;
config.transports.tcp =
crate::config::TransportInstances::Single(crate::config::TcpConfig::default());
config.transports.tor =
crate::config::TransportInstances::Single(crate::config::TorConfig::default());
let expected = inbound_idle_timeout(&config.node);
// 31 s of ladder, three 1 s ticks, 120 s link-dead.
assert_eq!(expected, Duration::from_secs(154));
assert_ne!(expected, INBOUND_IDLE_TIMEOUT);
let mut node = make_node_with(config);
let (tx, _rx) = packet_channel(8);
let transports = node.create_transports(&tx).await;
let mut seen = (0, 0);
for handle in &transports {
match handle {
crate::transport::TransportHandle::Tcp(t) => {
assert_eq!(t.inbound_idle_timeout(), expected, "TCP idle deadline");
seen.0 += 1;
}
crate::transport::TransportHandle::Tor(t) => {
assert_eq!(t.inbound_idle_timeout(), expected, "Tor idle deadline");
seen.1 += 1;
}
_ => {}
}
}
assert_eq!(
seen,
(1, 1),
"one TCP and one Tor transport should be built"
);
}
#[cfg(all(ble_available, any(target_os = "android", test)))]
mod test_radio {
use crate::transport::ble::addr::BleAddr;
+16 -10
View File
@@ -344,10 +344,10 @@ impl HandshakeState {
SecretKey::from_slice(&secret_bytes).expect("32 random bytes is valid secret key");
self.ephemeral_keypair = Some(Keypair::from_secret_key(&self.secp, &secret_key));
secret_bytes.zeroize();
// The erase is called non-secure because the private key may sit in
// further copies this function cannot name. It still clears the copy
// this frame owns; the keypair the key was just stored in is cleared
// by `Drop for HandshakeState`.
// This clears the copy this frame owns, with a volatile write the
// optimiser keeps; a copy made before it, such as one a call above
// left in a register or a stack slot, is not reached. The keypair the
// key was just stored in is cleared by `Drop for HandshakeState`.
secret_key.non_secure_erase();
}
@@ -364,9 +364,9 @@ impl HandshakeState {
/// caller below binds the value `Keypair::secret_key` hands back and
/// erases that binding once the DH is done, because the returned
/// `SecretKey` is a whole private key rather than a handle to one. Those
/// erases clear the copies this crate owns; `secp256k1` names its erase
/// non-secure because the compiler may hold further copies that no code
/// here can name.
/// erases clear the copies this crate owns. Each is a volatile write the
/// optimiser keeps, but a copy the compiler made before it runs, in a
/// register or a stack slot, is not reached.
fn ecdh(&self, our_secret: &SecretKey, their_public: &PublicKey) -> [u8; 32] {
// Get raw (x, y) coordinates (64 bytes) without any hashing
let mut point = shared_secret_point(their_public, our_secret);
@@ -1098,9 +1098,15 @@ impl Drop for HandshakeState {
/// where clearing them matters most. `Keypair` is `Copy` and so cannot
/// clear itself on drop; `HandshakeState` is not, so it does it for both.
///
/// This clears the copies this crate owns, not every copy that ever
/// existed: `secp256k1` names its erase non-secure because the compiler
/// may duplicate or move the bytes to places no code here can name.
/// This clears the copy being dropped, not every copy that ever existed.
/// Each erase is a volatile write, so the optimiser keeps it, but a
/// `HandshakeState` is built on the stack and moved several times before
/// it is dropped, and every move leaves the old bytes, both private keys
/// included, where the value used to be. A plain `take()` out of an
/// `Option` leaves its full contents in the slot. When a connection's
/// handshake completes, the connection's heap slot is cleared with
/// `clear_slot` once the handshake has left it; a move of the whole
/// connection state out of its machine does not clear anything.
fn drop(&mut self) {
self.static_keypair.non_secure_erase();
if let Some(ephemeral) = self.ephemeral_keypair.as_mut() {
+44
View File
@@ -489,5 +489,49 @@ pub(crate) fn open(
Ok(buf)
}
/// Move the value out of `slot`, leaving `None`, and clear the bytes the
/// value occupied.
///
/// `Option::take` moves the value out but writes only the `None` marker, so
/// the old value's bytes stay where they were. For a slot holding a
/// handshake or a session that is a full copy of its keys, which nothing
/// will clear because nothing owns it any more. Here, once the value has
/// moved out, the slot is cleared with [`clear_slot`].
///
/// This clears the slot only. The moved value is the caller's to clear, and
/// any copy made on the way out, such as in a register or a stack slot, is
/// out of reach as it is for every other move. A caller that unwraps the
/// result straight away should use `take().expect(..)` followed by
/// [`clear_slot`] instead: unwrapping after the slot has been cleared makes
/// the optimiser keep a second copy of the value on the stack, because it
/// can no longer copy the value straight from the slot to where it ends up.
pub(crate) fn take_cleared<T>(slot: &mut Option<T>) -> Option<T> {
let value = slot.take();
clear_slot(slot);
value
}
/// Drop whatever `slot` holds, overwrite every byte of it with zeros, and
/// leave it `None`.
///
/// The zeros are volatile writes, which the optimiser keeps. Meant for a
/// slot whose value has just been moved out, so the bytes the move left
/// behind are cleared.
pub(crate) fn clear_slot<T>(slot: &mut Option<T>) {
*slot = None;
let raw: *mut Option<T> = slot;
// SAFETY: `raw` comes from a live `&mut`, so it is valid and aligned for
// `size_of::<Option<T>>()` bytes. Seen as `MaybeUninit`, those bytes have
// no drop glue, own nothing and may be all zero, which is what
// `zeroize_flat_type` asks of its target. The slot holds `None` after the
// assignment above, so zeroing it discards nothing, and writing `None`
// (which does not drop the zeroed bytes) leaves the slot valid before
// anything reads it again.
unsafe {
zeroize::zeroize_flat_type(raw.cast::<std::mem::MaybeUninit<Option<T>>>());
raw.write(None);
}
}
#[cfg(test)]
mod tests;
+8
View File
@@ -7,6 +7,14 @@ use std::fmt;
/// Provides bidirectional authenticated encryption with replay protection.
/// The send counter is monotonically incremented; received counters are
/// validated against a sliding window to prevent replay attacks.
///
/// The two traffic keys are cleared when their `CipherState`s are dropped,
/// which clears only the copy being dropped. A session is moved out of
/// `HandshakeState::into_session` and into a connection's session slot, and
/// every move leaves both keys where the value used to be. Taking it out of
/// that slot through `take_cleared`, as the machine's `take_session` does,
/// clears the slot; a plain `take()`, or a move of the whole connection
/// state out of its machine, leaves both keys there.
pub struct NoiseSession {
/// Our role in the original handshake.
role: HandshakeRole,
+131
View File
@@ -915,3 +915,134 @@ fn test_xk_invalid_msg3_size() {
.is_err()
);
}
/// The `zeroize` features of `sha2` and `hmac` are on, so the SHA-256 state
/// that hashes each Diffie-Hellman result, and the two SHA-256 cores inside
/// the HMAC that HKDF runs on, are cleared when dropped. Without `sha2`'s
/// feature this does not compile; `hmac`'s forwards to the same `digest`
/// feature, so dropping it alone changes nothing while `sha2`'s is on.
#[test]
fn test_sha256_states_used_by_hashing_and_hkdf_are_cleared_on_drop() {
fn clears_on_drop<T: zeroize::ZeroizeOnDrop>() {}
clears_on_drop::<sha2::Sha256>();
clears_on_drop::<<sha2::Sha256 as hmac::EagerHash>::Core>();
}
/// A value that counts its drops, so a test can tell a value dropped once
/// from one dropped twice or leaked.
struct DropCounter<'a> {
bytes: [u8; 48],
heap: Vec<u8>,
drops: &'a std::cell::Cell<usize>,
}
impl Drop for DropCounter<'_> {
fn drop(&mut self) {
self.drops.set(self.drops.get() + 1);
}
}
#[test]
fn test_take_cleared_moves_the_value_out_intact_and_leaves_the_slot_none() {
let drops = std::cell::Cell::new(0);
let mut slot = Some(DropCounter {
bytes: [0xa5; 48],
heap: vec![7u8; 100],
drops: &drops,
});
let taken = take_cleared(&mut slot).expect("the slot held a value");
assert!(slot.is_none());
assert_eq!(taken.bytes, [0xa5; 48]);
assert_eq!(taken.heap, vec![7u8; 100]);
assert_eq!(drops.get(), 0, "clearing the slot must not drop the value");
drop(taken);
assert_eq!(drops.get(), 1);
}
#[test]
fn test_take_cleared_slot_is_reusable_and_an_empty_slot_stays_empty() {
let drops = std::cell::Cell::new(0);
let mut slot: Option<DropCounter<'_>> = None;
assert!(take_cleared(&mut slot).is_none());
assert!(slot.is_none());
slot = Some(DropCounter {
bytes: [1; 48],
heap: vec![2; 3],
drops: &drops,
});
let first = take_cleared(&mut slot).expect("first value");
slot = Some(DropCounter {
bytes: [3; 48],
heap: vec![4; 5],
drops: &drops,
});
let second = take_cleared(&mut slot).expect("second value");
assert_eq!((first.bytes[0], second.bytes[0]), (1, 3));
assert!(slot.is_none());
drop((first, second));
assert_eq!(drops.get(), 2);
}
/// For `Option<bool>`, all-zero bytes read as `Some(false)`, not `None`.
/// The slot must still come out as `None`, so zeroing alone is not enough.
#[test]
fn test_take_cleared_leaves_none_where_zero_bytes_would_read_as_some() {
let mut slot = Some(true);
assert_eq!(take_cleared(&mut slot), Some(true));
assert_eq!(slot, None);
let mut slot = Some((true, [9u8; 32]));
assert_eq!(take_cleared(&mut slot), Some((true, [9u8; 32])));
assert_eq!(slot, None);
}
#[test]
fn test_take_cleared_session_still_decrypts_what_its_peer_encrypts() {
let initiator_keypair = generate_keypair();
let responder_keypair = generate_keypair();
let mut initiator =
HandshakeState::new_initiator(initiator_keypair, responder_keypair.public_key());
let mut responder = HandshakeState::new_responder(responder_keypair);
initiator.set_local_epoch(generate_epoch());
responder.set_local_epoch(generate_epoch());
let msg1 = initiator.write_message_1().unwrap();
responder.read_message_1(&msg1).unwrap();
let msg2 = responder.write_message_2().unwrap();
let mut handshake_slot = Some(initiator);
let mut initiator = take_cleared(&mut handshake_slot).expect("handshake in slot");
assert!(handshake_slot.is_none());
initiator.read_message_2(&msg2).unwrap();
let mut session_slot = Some(initiator.into_session().unwrap());
let mut sender = take_cleared(&mut session_slot).expect("session in slot");
assert!(session_slot.is_none());
let mut receiver = responder.into_session().unwrap();
let ciphertext = sender.encrypt(b"after the move").unwrap();
assert_eq!(receiver.decrypt(&ciphertext).unwrap(), b"after the move");
}
#[test]
fn test_clear_slot_drops_a_present_value_once_and_leaves_none() {
let drops = std::cell::Cell::new(0);
let mut slot = Some(DropCounter {
bytes: [5; 48],
heap: vec![6; 7],
drops: &drops,
});
clear_slot(&mut slot);
assert!(slot.is_none());
assert_eq!(drops.get(), 1);
clear_slot(&mut slot);
assert!(slot.is_none());
assert_eq!(drops.get(), 1, "an empty slot has nothing to drop");
let mut flag = Some(true);
flag.take();
clear_slot(&mut flag);
assert_eq!(flag, None);
}
+16 -5
View File
@@ -622,8 +622,9 @@ impl PeerMachine {
) -> Result<Vec<u8>, NoiseError> {
// The parameter is this frame's own copy of the node's long-term
// private key, and the state checks below return before it is used.
// The guard clears it on every exit path.
let our_keypair = ErasingKeypair::take(&mut our_keypair);
// The guard clears it in place on every exit path, and makes no copy
// of its own for an early return to leave behind.
let our_keypair = ErasingKeypair::new(&mut our_keypair);
let msg1 = {
let direction = self.conn.direction();
@@ -668,8 +669,9 @@ impl PeerMachine {
current_time_ms: u64,
) -> Result<Vec<u8>, NoiseError> {
// Same as `start_handshake`: the parameter copy outlives two early
// returns, so the guard owns it rather than an erase per exit path.
let our_keypair = ErasingKeypair::take(&mut our_keypair);
// returns, so the guard clears it in place rather than an erase per
// exit path.
let our_keypair = ErasingKeypair::new(&mut our_keypair);
let (msg2, learned_identity, remote_epoch) = {
let direction = self.conn.direction();
@@ -738,10 +740,15 @@ impl PeerMachine {
});
}
// The slot is heap memory that outlives this call; clearing it once
// the handshake has left keeps both private keys from staying
// there. Unwrapping before clearing lets the handshake move
// straight from the slot to `hs`, with no second stack copy.
let mut hs = leg
.noise_handshake
.take()
.expect("noise handshake must exist in SentMsg1 state");
noise::clear_slot(&mut leg.noise_handshake);
hs.read_message_2(message)?;
@@ -766,7 +773,11 @@ impl PeerMachine {
pub(crate) fn take_session(&mut self) -> Option<NoiseSession> {
// The session exists iff the handshake reached `Complete`, so taking it
// unconditionally is byte-equivalent to the old `== Complete` gate.
self.leg.as_mut().and_then(|leg| leg.noise_session.take())
// The slot is cleared as the session leaves, so its two traffic keys
// do not stay behind in the machine.
self.leg
.as_mut()
.and_then(|leg| noise::take_cleared(&mut leg.noise_session))
}
/// Check if we have a completed session ready to take.
+2 -18
View File
@@ -1,9 +1,3 @@
// Some entry points (e.g. `stamp`, `record_since`) are only called from
// paths that aren't yet wired up in this PR (FSP-pipelined dispatch,
// per-stage worker telemetry). Keep them in tree so the follow-up wiring
// PR is a pure call-site change.
#![allow(dead_code)]
//! Runtime perf profiler for the FMP/FSP hot path and queue handoffs.
//!
//! Avoids external dependencies (`perf`, samply, etc.) by instrumenting
@@ -191,19 +185,9 @@ pub(crate) fn enabled() -> bool {
})
}
/// Capture a timestamp for a future queue-wait measurement. Returns
/// `None` when tracing is disabled so callers can store it cheaply in
/// packet/job structs without paying `Instant::now()` in production.
#[inline]
pub(crate) fn stamp() -> Option<Instant> {
if enabled() {
Some(Instant::now())
} else {
None
}
}
/// Record time elapsed since a previously captured stamp.
// Unix-only because its one caller, the encrypt worker, is.
#[cfg(unix)]
#[inline]
pub(crate) fn record_since(stage: Stage, start: Option<Instant>) {
if let Some(start) = start {
+32 -11
View File
@@ -15,6 +15,7 @@
//! the plain-data [`DecryptSlot`] mirror before it reaches [`Fsp::classify_epoch`].
use super::limits::FSP_CUTOVER_DELAY_MS;
use super::quorum::QuorumVerdict;
use crate::proto::stp::TreeCoordinate;
use crate::{FipsAddress, NodeAddr};
@@ -95,12 +96,17 @@ pub(crate) enum FspAction {
/// Invalidate the shared cached coordinates for `addr`
/// (`coord_cache.remove`).
InvalidateCoords { addr: NodeAddr },
/// Mark the shared cached coordinates for `addr` as an unverified hint,
/// keeping the value (`coord_cache.demote`). The entry goes on routing,
/// but no longer refuses a hint that replaces it.
DemoteCoords { addr: NodeAddr },
/// Write `mtu` into the shared `FipsAddress`-keyed path-MTU lookup, keeping
/// the tighter of existing-or-new (the shell applies the write under the
/// `path_mtu_lookup` guard).
TightenPathMtuLookup { fips_addr: FipsAddress, mtu: u16 },
/// Trigger discovery toward `dest` (`maybe_initiate_lookup`); emitted only
/// when the target's identity is cached.
/// Trigger discovery toward `dest` (`maybe_initiate_lookup`). The
/// `CoordsRequired` decision emits it only when the target's identity is
/// cached; the `PathBroken` decision emits it always.
InitiateLookup { dest: NodeAddr },
}
@@ -445,20 +451,35 @@ impl Fsp {
}
}
/// Decide the reaction to a `PathBroken` signal: unconditionally invalidate
/// the stale cached coordinates for `dest`, then (only when the identity is
/// cached) trigger re-discovery. Order is invalidate-then-lookup, matching
/// the pre-refactor handler. The warmup send and counter reset stay shell.
/// Decide the reaction to an admitted `PathBroken` signal for `dest`.
///
/// The signal is unauthenticated, so it may not discard coordinates a
/// lookup verified on its own say-so. `verified` is whether the cached
/// entry for `dest` is verified and still within its verification window;
/// `quorum` is what this report did to the destination's link quorum
/// (ignored when the entry is not verified).
///
/// - Not verified: remove the entry, as a hint can be replaced by any warm
/// anyway, then look it up.
/// - Verified, quorum reached: demote the entry to a hint, keeping the
/// value, then look it up.
/// - Verified, below quorum: leave the entry alone and look it up. A
/// successful lookup replaces the value whatever it was.
///
/// The lookup is emitted in every case. The warmup send, path-MTU release
/// and warmup-counter reset stay shell-side.
pub(crate) fn plan_path_broken(
&self,
dest: NodeAddr,
has_cached_identity: bool,
verified: bool,
quorum: QuorumVerdict,
) -> Vec<FspAction> {
let mut actions = vec![FspAction::InvalidateCoords { addr: dest }];
if has_cached_identity {
actions.push(FspAction::InitiateLookup { dest });
let lookup = FspAction::InitiateLookup { dest };
match (verified, quorum) {
(false, _) => vec![FspAction::InvalidateCoords { addr: dest }, lookup],
(true, QuorumVerdict::Reached) => vec![FspAction::DemoteCoords { addr: dest }, lookup],
(true, QuorumVerdict::Below { .. }) => vec![lookup],
}
actions
}
/// Decide whether a path-MTU update should tighten the shared lookup: emit
+3
View File
@@ -17,11 +17,14 @@
//! `classify_epoch`, the initiation tie-break, and the pure MTU-clamp /
//! bounded-queue / ECN transforms. No clock/crypto/I/O/tracing.
//! - `limits.rs` — the session-rekey timing constants.
//! - `quorum.rs` — the distinct-link quorum that gates demoting verified
//! coordinates on `PathBroken`. Clock injected.
//! - `wire.rs` — the FSP session wire codec and message types. Clock-free,
//! crypto-free.
pub(crate) mod core;
pub(crate) mod limits;
pub(crate) mod quorum;
pub(crate) mod wire;
#[cfg(test)]
+126
View File
@@ -0,0 +1,126 @@
//! Distinct-link quorum for `PathBroken` signals.
//!
//! A `PathBroken` carries no end-to-end authentication, so one report is not
//! enough evidence to discard coordinates a lookup verified. This module
//! counts, per destination, the distinct links that delivered a report naming
//! it within a window, and says when enough of them have.
//!
//! The vote is keyed on the authenticated link peer the datagram arrived over,
//! not on the reporter the body names: the reporter is plaintext the sender
//! chooses, so a sender on one link could invent as many reporters as the
//! quorum needs. Every datagram from one neighbour, whether its own or relayed
//! through it, arrives over the same link and is one vote.
//!
//! Sans-IO: the caller passes the time in, and nothing here logs or counts.
use std::collections::BTreeMap;
use crate::NodeAddr;
/// Distinct links that must deliver a report naming a destination within
/// [`QUORUM_WINDOW_MS`] before its verified coordinates are demoted.
///
/// Two is the smallest value that one neighbour cannot reach alone, whether it
/// forges the reports or relays a reflection. No measurement has counted how
/// many links a genuine failure's reports arrive over; a larger value only
/// keeps a genuinely stale verified entry verified for longer, while the
/// lookup every report starts replaces it anyway.
///
/// A node whose reports all arrive over one link, such as a leaf with a
/// single peer, never reaches this. A stale verified entry there lasts until
/// a lookup that a report started answers and replaces it, or at most until
/// its verification ages out after [`crate::cache::VERIFIED_TTL_MS`]; from
/// then on it no longer refuses a hint, and the next report removes it.
pub(crate) const QUORUM_LINKS: usize = 2;
/// Window within which distinct links count toward one quorum, in
/// milliseconds.
///
/// The sum of the default lookup attempt schedule (1 + 2 + 4 + 8 s), so the
/// reports that arrive during one re-validation cycle count together. An
/// anchor, not a measurement.
pub(crate) const QUORUM_WINDOW_MS: u64 = 15_000;
/// What one report did to its destination's quorum.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum QuorumVerdict {
/// Not enough distinct links yet; `distinct` counts this one.
Below { distinct: usize },
/// This report completed the quorum. The destination's record is cleared,
/// so the next quorum starts from nothing.
Reached,
}
/// Distinct links seen per destination, each with the time it was first seen
/// inside the current window.
#[derive(Debug, Default)]
pub(crate) struct LinkQuorum {
seen: BTreeMap<NodeAddr, Vec<(NodeAddr, u64)>>,
last_sweep_ms: u64,
}
impl LinkQuorum {
/// An empty quorum tracker.
pub(crate) fn new() -> Self {
Self::default()
}
/// Record that a report naming `dest` arrived over the link to
/// `link_peer` at `now_ms`.
///
/// A link already counted for `dest` inside the window does not count
/// again, and its first-seen time is not refreshed, so one neighbour
/// repeating itself can neither reach the quorum nor keep a record alive.
pub(crate) fn record(
&mut self,
dest: NodeAddr,
link_peer: NodeAddr,
now_ms: u64,
) -> QuorumVerdict {
self.sweep(now_ms);
let seen = self.seen.entry(dest).or_default();
seen.retain(|&(_, at)| live(at, now_ms));
if !seen.iter().any(|&(l, _)| l == link_peer) {
seen.push((link_peer, now_ms));
}
let distinct = seen.len();
if distinct >= QUORUM_LINKS {
self.seen.remove(&dest);
QuorumVerdict::Reached
} else {
QuorumVerdict::Below { distinct }
}
}
/// Forget every report about `dest`.
///
/// Called when a lookup verifies `dest` again: reports about the path the
/// fresh value replaced are not evidence against it.
pub(crate) fn clear(&mut self, dest: &NodeAddr) {
self.seen.remove(dest);
}
/// Number of destinations with a record.
#[cfg(test)]
pub(crate) fn len(&self) -> usize {
self.seen.len()
}
/// Drop destinations with no live link, at most once per window, so
/// the map holds only destinations reported within the last window or so.
fn sweep(&mut self, now_ms: u64) {
if now_ms.saturating_sub(self.last_sweep_ms) < QUORUM_WINDOW_MS {
return;
}
self.last_sweep_ms = now_ms;
self.seen.retain(|_, seen| {
seen.retain(|&(_, at)| live(at, now_ms));
!seen.is_empty()
});
}
}
/// Whether a report first seen at `at` still counts at `now_ms`.
fn live(at: u64, now_ms: u64) -> bool {
now_ms.saturating_sub(at) <= QUORUM_WINDOW_MS
}
+31 -10
View File
@@ -737,21 +737,42 @@ fn plan_coords_required_lookup_gated_on_identity() {
}
#[test]
fn plan_path_broken_invalidates_then_lookups() {
fn plan_path_broken_removes_an_unverified_entry_then_looks_it_up() {
use crate::proto::fsp::quorum::QuorumVerdict;
let fsp = Fsp::new();
let dest = make_node_addr(6);
// Cached identity: invalidate, then lookup — in that order.
assert_eq!(
fsp.plan_path_broken(dest, true),
vec![
let expected = vec![
FspAction::InvalidateCoords { addr: dest },
FspAction::InitiateLookup { dest },
]
);
// No cached identity: invalidate only (still unconditional).
];
// The quorum is irrelevant to a hint: removed either way.
for quorum in [QuorumVerdict::Below { distinct: 0 }, QuorumVerdict::Reached] {
assert_eq!(fsp.plan_path_broken(dest, false, quorum), expected);
}
}
#[test]
fn plan_path_broken_keeps_a_verified_entry_below_quorum_and_looks_it_up() {
use crate::proto::fsp::quorum::QuorumVerdict;
let fsp = Fsp::new();
let dest = make_node_addr(6);
assert_eq!(
fsp.plan_path_broken(dest, false),
vec![FspAction::InvalidateCoords { addr: dest }]
fsp.plan_path_broken(dest, true, QuorumVerdict::Below { distinct: 1 }),
vec![FspAction::InitiateLookup { dest }]
);
}
#[test]
fn plan_path_broken_demotes_a_verified_entry_at_quorum_then_looks_it_up() {
use crate::proto::fsp::quorum::QuorumVerdict;
let fsp = Fsp::new();
let dest = make_node_addr(6);
assert_eq!(
fsp.plan_path_broken(dest, true, QuorumVerdict::Reached),
vec![
FspAction::DemoteCoords { addr: dest },
FspAction::InitiateLookup { dest },
]
);
}
+1
View File
@@ -1,2 +1,3 @@
mod core;
mod quorum;
mod wire;
+118
View File
@@ -0,0 +1,118 @@
//! Unit tests for the `PathBroken` link quorum.
use crate::NodeAddr;
use crate::proto::fsp::quorum::{LinkQuorum, QUORUM_LINKS, QUORUM_WINDOW_MS, QuorumVerdict};
fn addr(v: u8) -> NodeAddr {
let mut bytes = [0u8; 16];
bytes[0] = v;
NodeAddr::from_bytes(bytes)
}
const T0: u64 = 1_000_000;
#[test]
fn the_quorum_needs_two_links() {
// The tests below are written for this value; a change to it should be a
// deliberate edit here too.
assert_eq!(QUORUM_LINKS, 2);
}
#[test]
fn one_report_stays_below_the_quorum() {
let mut q = LinkQuorum::new();
assert_eq!(
q.record(addr(1), addr(0xA), T0),
QuorumVerdict::Below { distinct: 1 }
);
}
#[test]
fn two_distinct_links_inside_the_window_reach_the_quorum() {
let mut q = LinkQuorum::new();
let _ = q.record(addr(1), addr(0xA), T0);
assert_eq!(
q.record(addr(1), addr(0xB), T0 + QUORUM_WINDOW_MS),
QuorumVerdict::Reached
);
}
#[test]
fn the_same_link_repeated_never_reaches_the_quorum() {
let mut q = LinkQuorum::new();
for i in 0..10 {
assert_eq!(
q.record(addr(1), addr(0xA), T0 + i),
QuorumVerdict::Below { distinct: 1 },
"repeat {i} counted again"
);
}
}
#[test]
fn links_further_apart_than_the_window_do_not_combine() {
let mut q = LinkQuorum::new();
let _ = q.record(addr(1), addr(0xA), T0);
assert_eq!(
q.record(addr(1), addr(0xB), T0 + QUORUM_WINDOW_MS + 1),
QuorumVerdict::Below { distinct: 1 }
);
}
#[test]
fn a_repeat_does_not_refresh_its_links_first_seen_time() {
let mut q = LinkQuorum::new();
let _ = q.record(addr(1), addr(0xA), T0);
let _ = q.record(addr(1), addr(0xA), T0 + QUORUM_WINDOW_MS);
// Had the repeat refreshed A's time, this would combine with it.
assert_eq!(
q.record(addr(1), addr(0xB), T0 + QUORUM_WINDOW_MS + 1),
QuorumVerdict::Below { distinct: 1 }
);
}
#[test]
fn reaching_the_quorum_clears_the_destination() {
let mut q = LinkQuorum::new();
let _ = q.record(addr(1), addr(0xA), T0);
assert_eq!(q.record(addr(1), addr(0xB), T0 + 1), QuorumVerdict::Reached);
assert_eq!(q.len(), 0);
assert_eq!(
q.record(addr(1), addr(0xA), T0 + 2),
QuorumVerdict::Below { distinct: 1 },
"the next demotion must need a fresh quorum"
);
}
#[test]
fn clear_forgets_the_reports_about_a_destination() {
let mut q = LinkQuorum::new();
let _ = q.record(addr(1), addr(0xA), T0);
q.clear(&addr(1));
assert_eq!(
q.record(addr(1), addr(0xB), T0 + 1),
QuorumVerdict::Below { distinct: 1 }
);
}
#[test]
fn destinations_are_counted_separately() {
let mut q = LinkQuorum::new();
let _ = q.record(addr(1), addr(0xA), T0);
assert_eq!(
q.record(addr(2), addr(0xB), T0),
QuorumVerdict::Below { distinct: 1 }
);
}
#[test]
fn the_sweep_drops_destinations_with_no_live_link() {
let mut q = LinkQuorum::new();
for d in 0..100 {
let _ = q.record(addr(d), addr(0xA), T0);
}
assert_eq!(q.len(), 100);
// One report a window and a millisecond later sweeps the stale records.
let _ = q.record(addr(200), addr(0xA), T0 + QUORUM_WINDOW_MS + 1);
assert_eq!(q.len(), 1);
}
+11 -5
View File
@@ -146,13 +146,19 @@ pub(crate) fn plan_initiate(request: &LookupRequest, rv: &impl RoutingView) -> V
/// Classification of an inbound LookupRequest, decided from Lookup state.
pub(crate) enum RequestOutcome {
/// request_id already in the dedup cache — drop.
/// request_id already in the dedup cache — drop. The test is on the id
/// alone, so one flood reaching this node through two neighbours lands
/// here just as a request sent twice does; it does not identify the peer
/// that delivered the copy.
Duplicate,
/// A request this node originated, looped back to it — drop without
/// recording. Kept separate from `Duplicate`, which means another node
/// resent a request, so the two do not share a rejection counter: this
/// one has a nonzero floor in healthy operation and says nothing about
/// the peer that delivered it.
/// recording. Kept apart from `Duplicate` by cause: this is the node's
/// own fan-out returning, while `Duplicate` is a request id the node has
/// already recorded, seen again. Neither identifies the delivering peer.
/// Recognised only while the lookup is outstanding and the id is among
/// the last `MAX_RECORDED_IDS` it issued; an own copy outside that reach
/// is recorded and forwarded as transit, and a later copy of it is
/// dropped as a duplicate.
OwnRequestLooped,
/// We are the lookup target — the shell generates + sends the response.
RespondAsTarget,
+89
View File
@@ -520,6 +520,95 @@ fn classify_request_drops_our_own_request_looped_back_to_us() {
));
}
#[test]
fn classify_request_records_our_own_id_as_transit_once_its_lookup_has_timed_out() {
// The own-request guard reaches only as far as the pending lookup. Once
// the ladder times out and the entry is dropped, a late copy of our own
// request is indistinguishable from transit: it is recorded and forwarded,
// and a later copy of it is a duplicate.
let mut lookup = empty_lookup();
let looping_peer = make_node_addr(0x01);
let my_addr = make_node_addr(0x99);
let target = make_node_addr(0xAA);
let t0 = 1000u64;
let mut pending = PendingLookup::new(t0);
pending.record(77);
lookup.pending_lookups.insert(target, pending);
// A one-rung ladder of 1s: the entry times out at t0 + 1000.
let now = t0 + 1000;
let outcome = poll_pending(&mut lookup, now, &[1]);
assert_eq!(outcome.timeouts.len(), 1, "the lookup must time out");
let request = make_request_id(77, target, 3);
let first = classify_request(
&mut lookup,
&request,
&looping_peer,
&my_addr,
now,
5000,
4096,
1,
);
assert!(matches!(first.outcome, RequestOutcome::Forward));
assert!(lookup.recent_requests.contains_key(&77));
let second = classify_request(
&mut lookup,
&request,
&looping_peer,
&my_addr,
now,
5000,
4096,
1,
);
assert!(matches!(second.outcome, RequestOutcome::Duplicate));
}
#[test]
fn classify_request_records_an_own_id_older_than_the_recorded_window_as_transit() {
// A pending lookup keeps only the last MAX_RECORDED_IDS (eight) ids its
// ladder issued. A returning copy of an earlier attempt is outside the
// guard's reach and goes through as transit; a recent one is still caught.
let mut lookup = empty_lookup();
let looping_peer = make_node_addr(0x01);
let my_addr = make_node_addr(0x99);
let target = make_node_addr(0xAA);
let mut pending = PendingLookup::new(1000);
for id in 1..=9 {
pending.record(id);
}
lookup.pending_lookups.insert(target, pending);
let oldest = classify_request(
&mut lookup,
&make_request_id(1, target, 3),
&looping_peer,
&my_addr,
1000,
5000,
4096,
1,
);
assert!(matches!(oldest.outcome, RequestOutcome::Forward));
assert!(lookup.recent_requests.contains_key(&1));
let newest = classify_request(
&mut lookup,
&make_request_id(9, target, 3),
&looping_peer,
&my_addr,
1000,
5000,
4096,
1,
);
assert!(matches!(newest.outcome, RequestOutcome::OwnRequestLooped));
assert!(!lookup.recent_requests.contains_key(&9));
}
#[test]
fn classify_request_still_transits_a_foreign_id_for_a_target_we_are_looking_up() {
// The guard keys on the id, not the target: another node's lookup for the
+8 -8
View File
@@ -692,14 +692,14 @@ fn parse_target_addr(addr: &TransportAddr) -> Result<SocksTarget, TransportError
/// "receive loop stopped" debug (without a `direction` field) that the
/// shared loop deliberately leaves to each transport.
///
/// The first-frame deadline is `None` on every nym connection. Nym is
/// outbound-only (`accept_connections()` is `false` and no listener is ever
/// bound), so no nym connection is admitted before a byte is read and none
/// occupies a capped slot: the pool carries `()` metadata and the teardown
/// hook decrements nothing. There is no resource for a silent remote to
/// exhaust, and the mixnet's Sphinx routing makes a first frame legitimately
/// slow, so a TCP-scale deadline here would drop good connections to defend
/// a cap that does not exist.
/// The inbound deadline, both first-frame and idle, is `None` on every nym
/// connection. Nym is outbound-only (`accept_connections()` is `false` and
/// no listener is ever bound), so no nym connection is admitted before a
/// byte is read and none occupies a capped slot: the pool carries `()`
/// metadata and the teardown hook decrements nothing. There is no resource
/// for a silent remote to exhaust, and the mixnet's Sphinx routing makes a
/// frame legitimately slow, so a TCP-scale deadline here would drop good
/// connections to defend a cap that does not exist.
#[allow(clippy::too_many_arguments)]
async fn nym_receive_loop(
reader: tokio::net::tcp::OwnedReadHalf,
+17 -12
View File
@@ -8,7 +8,6 @@
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
use futures::FutureExt;
use tokio::net::TcpStream;
@@ -22,6 +21,7 @@ use tokio::io::AsyncWriteExt;
use crate::transport::framing::read_fmp_packet;
use crate::transport::stream::{ConnId, PooledConn, remove_own};
use crate::transport::tcp::InboundDeadline;
use crate::transport::{
ConnectionState, PacketTx, ReceivedPacket, TransportAddr, TransportError, TransportId,
};
@@ -238,8 +238,9 @@ pub(crate) async fn proxied_send_loop<S: ProxiedStats, M>(
/// hoisted into each per-transport wrapper (tor carries a `direction` field
/// nym lacks), so this loop is silent on exit.
///
/// `first_frame_timeout` bounds the wait for the *first* complete frame only.
/// It is `Some` for a connection that takes a capped inbound slot from accept
/// `deadline` bounds the wait for every complete frame: the first-frame
/// deadline until one arrives, the idle deadline for each one after. It is
/// `Some` for a connection that takes a capped inbound slot from accept
/// — today only tor's onion listener — and `None` everywhere else, which
/// covers every outbound connection and the whole of the nym transport (nym
/// is outbound-only and keeps no counted slots). A deadline expiry is not a
@@ -268,7 +269,7 @@ pub(crate) async fn proxied_receive_loop<S: ProxiedStats, M>(
mtu: u16,
stats: Arc<S>,
label: &'static str,
first_frame_timeout: Option<Duration>,
deadline: Option<InboundDeadline>,
ready_rx: Option<tokio::sync::oneshot::Receiver<()>>,
on_remove: impl Fn(&S, &M),
) {
@@ -290,11 +291,13 @@ pub(crate) async fn proxied_receive_loop<S: ProxiedStats, M>(
if admitted {
let mut first = true;
loop {
let read = match first_frame_timeout {
// Bound the first read only. A silent remote otherwise holds its
// inbound slot for as long as it keeps the socket open.
Some(d) if first => {
match tokio::time::timeout(d, read_fmp_packet(&mut reader, mtu)).await {
let read = match deadline {
// Bound every read. A remote that goes silent, before or after
// its first frame, otherwise holds its inbound slot for as long
// as it keeps the socket open.
Some(d) => {
let limit = d.for_read(first);
match tokio::time::timeout(limit, read_fmp_packet(&mut reader, mtu)).await {
Ok(result) => result,
Err(_) => {
// Not a recv error: `record_recv_error` means framing
@@ -303,15 +306,16 @@ pub(crate) async fn proxied_receive_loop<S: ProxiedStats, M>(
debug!(
transport_id = %transport_id,
remote_addr = %remote_addr,
timeout_secs = d.as_secs_f64(),
"No complete frame within the first-frame deadline, dropping inbound {} connection",
deadline = InboundDeadline::phase(first),
timeout_secs = limit.as_secs_f64(),
"No complete frame within the inbound deadline, dropping inbound {} connection",
label
);
break;
}
}
}
_ => read_fmp_packet(&mut reader, mtu).await,
None => read_fmp_packet(&mut reader, mtu).await,
};
first = false;
@@ -372,6 +376,7 @@ mod tests {
use crate::transport::packet_channel;
use crate::transport::stream::{next_conn_id, park_writer};
use portable_atomic::{AtomicU64, Ordering};
use std::time::Duration;
use tokio::io::AsyncReadExt;
use tokio::net::TcpListener;
use tokio::time::timeout;
+295 -26
View File
@@ -89,6 +89,9 @@ pub struct TcpTransport {
/// Deadline from accept to the first complete inbound frame. Defaults to
/// `INBOUND_FIRST_FRAME_TIMEOUT`; overridable only from tests.
first_frame_timeout: Duration,
/// Longest wait for each later complete inbound frame. Defaults to
/// `INBOUND_IDLE_TIMEOUT`; the node sets it from its liveness timers.
idle_timeout: Duration,
/// Transport statistics.
stats: Arc<TcpStats>,
}
@@ -113,6 +116,7 @@ impl TcpTransport {
local_addr: None,
node_max_connections: None,
first_frame_timeout: INBOUND_FIRST_FRAME_TIMEOUT,
idle_timeout: INBOUND_IDLE_TIMEOUT,
stats: Arc::new(TcpStats::new()),
}
}
@@ -128,6 +132,22 @@ impl TcpTransport {
self.first_frame_timeout = d;
}
/// Set the inbound idle deadline: the longest an accepted connection may
/// go, after its first frame, without delivering another complete frame.
///
/// The node derives it from its own link-liveness timers, so it never
/// drops a connection carrying a link the node would keep. Takes effect
/// at the next `start_async()`.
pub fn set_inbound_idle_timeout(&mut self, d: Duration) {
self.idle_timeout = d;
}
/// The inbound idle deadline the accept loop will use.
#[cfg(test)]
pub(crate) fn inbound_idle_timeout(&self) -> Duration {
self.idle_timeout
}
/// Set the node-wide `node.limits.max_connections` value.
///
/// Used as the inbound-cap fallback when this transport instance has no
@@ -216,7 +236,10 @@ impl TcpTransport {
keepalive_secs: self.config.keepalive_secs(),
recv_buf: self.config.recv_buf_size(),
send_buf: self.config.send_buf_size(),
first_frame_timeout: self.first_frame_timeout,
deadline: InboundDeadline {
first_frame: self.first_frame_timeout,
idle: self.idle_timeout,
},
};
let accept_task = tokio::spawn(async move {
@@ -822,6 +845,45 @@ impl Transport for TcpTransport {
/// TOML surface.
pub(crate) const INBOUND_FIRST_FRAME_TIMEOUT: Duration = Duration::from_secs(30);
/// Default deadline for each complete inbound frame after the first.
///
/// Without it, a remote that sends one well-formed frame and then goes
/// silent holds its inbound slot for as long as it keeps the socket open:
/// a frame that names no session is dropped by the node without closing
/// the transport. The node replaces this with the bound derived from its
/// own liveness timers (`link_silence_ms`); 64 s is that bound at stock
/// settings, kept here so a transport built outside the node still has a
/// deadline.
pub(crate) const INBOUND_IDLE_TIMEOUT: Duration = Duration::from_secs(64);
/// Read deadlines for an inbound connection, which holds a capped pool
/// slot from accept.
///
/// Each deadline covers one complete frame, not a byte: a remote that
/// drips a frame slower than the deadline is dropped. The idle deadline
/// re-arms on every frame, so a connection carrying a live link, which
/// receives at least a heartbeat per interval, is never dropped by it.
#[derive(Clone, Copy, Debug)]
pub(crate) struct InboundDeadline {
/// Deadline from accept to the first complete frame.
pub(crate) first_frame: Duration,
/// Deadline for each later complete frame, from the end of the last.
pub(crate) idle: Duration,
}
impl InboundDeadline {
/// The deadline for the next read: `first` is true until a frame has
/// been received.
pub(crate) fn for_read(&self, first: bool) -> Duration {
if first { self.first_frame } else { self.idle }
}
/// The log word for an expiry of the deadline `for_read(first)` gave.
pub(crate) fn phase(first: bool) -> &'static str {
if first { "first-frame" } else { "idle" }
}
}
/// Socket configuration parameters passed to the accept loop.
struct AcceptConfig {
mtu: u16,
@@ -830,7 +892,7 @@ struct AcceptConfig {
keepalive_secs: u64,
recv_buf: usize,
send_buf: usize,
first_frame_timeout: Duration,
deadline: InboundDeadline,
}
/// TCP accept loop — runs as a spawned task when bind_addr is configured.
@@ -850,7 +912,7 @@ async fn accept_loop(
keepalive_secs,
recv_buf,
send_buf,
first_frame_timeout,
deadline,
} = cfg;
debug!(transport_id = %transport_id, "TCP accept loop starting");
@@ -965,7 +1027,7 @@ async fn accept_loop(
conn_mtu,
recv_stats,
Direction::Inbound,
Some(first_frame_timeout),
Some(deadline),
Some(ready_rx),
)
.await;
@@ -1114,9 +1176,10 @@ async fn tcp_send_loop(
/// `pool_outbound` counter regardless of whether the matching pool
/// entry survived to be removed.
///
/// `first_frame_timeout` bounds the wait for the *first* complete frame
/// only, and is `Some` for inbound connections (which hold a capped pool
/// slot from accept) and `None` for outbound ones. `ready_rx`, when
/// `deadline` bounds the wait for every complete frame: the first-frame
/// deadline until one arrives, the idle deadline for each one after. It is
/// `Some` for inbound connections (which hold a capped pool slot from
/// accept) and `None` for outbound ones. `ready_rx`, when
/// present, is the accept loop's readiness barrier: the loop must not run
/// its cleanup before the accept loop has inserted the pool entry.
///
@@ -1138,7 +1201,7 @@ async fn tcp_receive_loop(
mtu: u16,
stats: Arc<TcpStats>,
direction: Direction,
first_frame_timeout: Option<Duration>,
deadline: Option<InboundDeadline>,
ready_rx: Option<tokio::sync::oneshot::Receiver<()>>,
) {
let remote_addr = &key.remote;
@@ -1159,11 +1222,13 @@ async fn tcp_receive_loop(
if admitted {
let mut first = true;
loop {
let read = match first_frame_timeout {
// Bound the first read only. A silent remote otherwise holds
// its inbound slot for as long as it keeps the socket open.
Some(d) if first => {
match tokio::time::timeout(d, read_fmp_packet(&mut reader, mtu)).await {
let read = match deadline {
// Bound every read. A remote that goes silent, before or after
// its first frame, otherwise holds its inbound slot for as
// long as it keeps the socket open.
Some(d) => {
let limit = d.for_read(first);
match tokio::time::timeout(limit, read_fmp_packet(&mut reader, mtu)).await {
Ok(result) => result,
Err(_) => {
// Not a recv error: `record_recv_error` means
@@ -1172,14 +1237,15 @@ async fn tcp_receive_loop(
debug!(
transport_id = %transport_id,
remote_addr = %remote_addr,
timeout_secs = d.as_secs_f64(),
"No complete frame within the first-frame deadline, dropping inbound connection"
deadline = InboundDeadline::phase(first),
timeout_secs = limit.as_secs_f64(),
"No complete frame within the inbound deadline, dropping inbound connection"
);
break;
}
}
}
_ => read_fmp_packet(&mut reader, mtu).await,
None => read_fmp_packet(&mut reader, mtu).await,
};
first = false;
@@ -1390,7 +1456,7 @@ fn read_mss_mtu(stream: &std::net::TcpStream, default_mtu: u16) -> u16 {
mod tests {
use super::pool::PoolMap;
use super::*;
use crate::transport::framing::build_msg1_frame;
use crate::transport::framing::{build_established_frame, build_msg1_frame};
use crate::transport::packet_channel;
use crate::transport::stream::park_writer;
use tokio::time::{Duration, timeout};
@@ -2322,18 +2388,170 @@ mod tests {
transport.stop_async().await.unwrap();
}
/// Regression guard, not evidence that the fix works.
/// The smallest frame the reader accepts as established: a 16-byte
/// header, no payload, a 16-byte tag. The node drops one naming no
/// session without closing the transport, so it is what a squatter
/// sends to get past the first-frame deadline.
fn squatter_frame() -> Vec<u8> {
let frame = build_established_frame(0);
assert_eq!(frame.len(), crate::proto::fmp::wire::ENCRYPTED_MIN_SIZE);
frame
}
/// A remote that sends one well-formed frame and then goes silent must
/// lose its inbound slot at the idle deadline, without disconnecting.
///
/// The deadline is scoped to the first iteration, so an established
/// connection that then goes quiet cannot be dropped by it: this test
/// passes by construction under the current design. It is kept so that a
/// future general (every-read) idle deadline cannot silently start
/// reaping quiet links without a test going red.
/// The first-frame deadline is set far above the idle one, so the
/// release can only be the idle deadline's doing. Break-check: scope the
/// deadline back to the first read only and the count stays at 1.
#[tokio::test]
async fn established_connection_survives_long_idle() {
async fn inbound_connection_that_goes_silent_after_one_established_frame_releases_its_slot() {
let (tx, mut rx) = packet_channel(100);
let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx);
transport.set_first_frame_timeout(Duration::from_secs(5));
transport.set_inbound_idle_timeout(Duration::from_millis(300));
transport.start_async().await.unwrap();
let listen = transport.local_addr().unwrap();
let mut squatter = TcpStream::connect(listen).await.unwrap();
squatter.write_all(&squatter_frame()).await.unwrap();
let packet = timeout(Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for the squatter's frame")
.expect("packet channel closed");
assert_eq!(packet.data, squatter_frame());
assert_eq!(
transport.stats().pool_inbound_count(),
1,
"the connection should hold its slot once its first frame is in"
);
assert!(
wait_until(
|| transport.stats().pool_inbound_count() == 0,
Duration::from_secs(2)
)
.await,
"a connection silent after its first frame should lose its slot at the idle deadline"
);
assert!(
transport.pool.lock().await.is_empty(),
"the pool entry should go with the slot"
);
drop(squatter);
transport.stop_async().await.unwrap();
}
/// With the cap filled by a one-frame squatter, a genuine peer is refused
/// until the idle deadline frees the slot, and admitted afterwards.
///
/// Break-check: without the idle deadline the squatter never releases,
/// so the genuine peer's frame is never delivered.
#[tokio::test]
async fn inbound_cap_filled_by_one_frame_squatters_admits_a_genuine_peer_after_the_idle_deadline()
{
let (tx, mut rx) = packet_channel(100);
let mut transport = TcpTransport::new(TransportId::new(1), None, capped_config(1), tx);
transport.set_first_frame_timeout(Duration::from_secs(5));
transport.set_inbound_idle_timeout(Duration::from_secs(1));
transport.start_async().await.unwrap();
let listen = transport.local_addr().unwrap();
let mut squatter = TcpStream::connect(listen).await.unwrap();
squatter.write_all(&squatter_frame()).await.unwrap();
timeout(Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for the squatter's frame")
.expect("packet channel closed");
assert_eq!(transport.stats().pool_inbound_count(), 1);
// While the cap is full a genuine peer is rejected outright. The
// 250 ms window ends well inside the squatter's 1 s idle deadline.
let mut early = TcpStream::connect(listen).await.unwrap();
let _ = early.write_all(&build_msg1_frame()).await;
assert!(
timeout(Duration::from_millis(250), rx.recv())
.await
.is_err(),
"a peer arriving while the cap is full must not be admitted"
);
drop(early);
assert!(
wait_until(
|| transport.stats().pool_inbound_count() == 0,
Duration::from_secs(3)
)
.await,
"the idle deadline should free the slot the squatter took"
);
let mut genuine = TcpStream::connect(listen).await.unwrap();
genuine.write_all(&build_msg1_frame()).await.unwrap();
let packet = timeout(Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for the genuine peer's frame")
.expect("packet channel closed");
assert_eq!(packet.data, build_msg1_frame());
drop(squatter);
drop(genuine);
transport.stop_async().await.unwrap();
}
/// The healthy path: a connection that delivers a frame more often than
/// the idle deadline keeps its slot across many deadlines, and every
/// frame is delivered. This is the shape of a link kept alive only by
/// heartbeats.
///
/// Break-check: a deadline that does not re-arm on each frame (a single
/// deadline from accept) drops the connection after the first second.
#[tokio::test]
async fn inbound_connection_sending_a_frame_every_interval_below_the_idle_deadline_is_kept() {
let (tx, mut rx) = packet_channel(100);
let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx);
transport.set_first_frame_timeout(Duration::from_secs(1));
transport.set_inbound_idle_timeout(Duration::from_secs(1));
transport.start_async().await.unwrap();
let listen = transport.local_addr().unwrap();
let mut peer = TcpStream::connect(listen).await.unwrap();
// Twelve frames 250 ms apart: 3 s, three idle deadlines, with 750 ms
// of slack between each frame and the deadline it re-arms.
for i in 0..12 {
peer.write_all(&squatter_frame()).await.unwrap();
let packet = timeout(Duration::from_secs(2), rx.recv())
.await
.unwrap_or_else(|_| panic!("timeout waiting for frame {i}"))
.expect("packet channel closed");
assert_eq!(packet.data, squatter_frame());
assert_eq!(
transport.stats().pool_inbound_count(),
1,
"a connection delivering frames inside the idle deadline must keep its slot (frame {i})"
);
tokio::time::sleep(Duration::from_millis(250)).await;
}
assert_eq!(transport.stats().pool_inbound_count(), 1);
assert!(!transport.pool.lock().await.is_empty());
drop(peer);
transport.stop_async().await.unwrap();
}
/// An established connection quiet for longer than the first-frame
/// deadline, but not the idle deadline, keeps its slot: once a frame is
/// in, the first-frame deadline no longer applies.
///
/// Break-check: apply the first-frame deadline to every read and the
/// connection is dropped after 200 ms of quiet.
#[tokio::test]
async fn established_connection_quiet_past_the_first_frame_deadline_is_kept() {
let (tx, mut rx) = packet_channel(100);
let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx);
transport.set_first_frame_timeout(Duration::from_millis(200));
transport.set_inbound_idle_timeout(Duration::from_secs(5));
transport.start_async().await.unwrap();
let listen = transport.local_addr().unwrap();
@@ -2345,7 +2563,7 @@ mod tests {
.expect("packet channel closed");
assert_eq!(packet.data, build_msg1_frame());
// Four deadlines' worth of silence after the first frame.
// Four first-frame deadlines' worth of silence after the first frame.
tokio::time::sleep(Duration::from_millis(800)).await;
assert_eq!(
@@ -2359,6 +2577,54 @@ mod tests {
transport.stop_async().await.unwrap();
}
/// The idle deadline covers a complete frame, not its first bytes: a
/// frame after the first whose prefix arrives inside the deadline and
/// whose remainder arrives after it is not delivered, and the slot is
/// released.
///
/// Break-check: bound only the prefix read and the body read waits
/// forever, so the late frame is delivered.
#[tokio::test]
async fn inbound_connection_dripping_a_frame_slower_than_the_idle_deadline_is_dropped() {
let (tx, mut rx) = packet_channel(100);
let mut transport = TcpTransport::new(TransportId::new(1), None, make_config(), tx);
transport.set_first_frame_timeout(Duration::from_secs(5));
transport.set_inbound_idle_timeout(Duration::from_millis(300));
transport.start_async().await.unwrap();
let listen = transport.local_addr().unwrap();
let mut peer = TcpStream::connect(listen).await.unwrap();
peer.write_all(&squatter_frame()).await.unwrap();
timeout(Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for the first frame")
.expect("packet channel closed");
let frame = squatter_frame();
// Prefix inside the idle deadline, remainder well past it.
peer.write_all(&frame[..4]).await.unwrap();
tokio::time::sleep(Duration::from_millis(600)).await;
let _ = peer.write_all(&frame[4..]).await;
assert!(
timeout(Duration::from_millis(500), rx.recv())
.await
.is_err(),
"a frame completing after the idle deadline must not be delivered"
);
assert!(
wait_until(
|| transport.stats().pool_inbound_count() == 0,
Duration::from_secs(2)
)
.await,
"the dripped connection should have released its slot"
);
drop(peer);
transport.stop_async().await.unwrap();
}
/// A genuine peer that is slow to start, but finishes its first frame
/// inside the deadline, is admitted.
#[tokio::test]
@@ -2471,7 +2737,10 @@ mod tests {
1400,
stats.clone(),
Direction::Inbound,
Some(Duration::from_millis(50)),
Some(InboundDeadline {
first_frame: Duration::from_millis(50),
idle: Duration::from_millis(50),
}),
Some(ready_rx),
)
.await;
+162 -33
View File
@@ -35,7 +35,7 @@ use crate::transport::socks5::{
proxied_send_loop,
};
use crate::transport::stream::{ConnId, WRITER_DRAIN_TIMEOUT, drain_writer, next_conn_id};
use crate::transport::tcp::INBOUND_FIRST_FRAME_TIMEOUT;
use crate::transport::tcp::{INBOUND_FIRST_FRAME_TIMEOUT, INBOUND_IDLE_TIMEOUT, InboundDeadline};
use control::{ControlAuth, TorControlClient, TorMonitoringInfo};
use stats::TorStats;
@@ -151,6 +151,10 @@ pub struct TorTransport {
cached_monitoring: Arc<std::sync::RwLock<Option<TorMonitoringInfo>>>,
/// Background monitoring task handle.
monitoring_task: Option<JoinHandle<()>>,
/// Longest wait for each complete inbound onion frame after the first.
/// Defaults to `INBOUND_IDLE_TIMEOUT`; the node sets it from its
/// liveness timers.
idle_timeout: Duration,
}
impl TorTransport {
@@ -175,9 +179,27 @@ impl TorTransport {
control_client: None,
cached_monitoring: Arc::new(std::sync::RwLock::new(None)),
monitoring_task: None,
idle_timeout: INBOUND_IDLE_TIMEOUT,
}
}
/// Set the inbound idle deadline for the onion listener: the longest an
/// accepted connection may go, after its first frame, without delivering
/// another complete frame.
///
/// The node derives it from its own link-liveness timers, so it never
/// drops a connection carrying a link the node would keep. Takes effect
/// at the next `start_async()`.
pub fn set_inbound_idle_timeout(&mut self, d: Duration) {
self.idle_timeout = d;
}
/// The inbound idle deadline the onion accept loop will use.
#[cfg(test)]
pub(crate) fn inbound_idle_timeout(&self) -> Duration {
self.idle_timeout
}
/// Get the instance name (if configured as a named instance).
pub fn name(&self) -> Option<&str> {
self.name.as_deref()
@@ -359,6 +381,10 @@ impl TorTransport {
let mtu = self.config.mtu();
let max_inbound = self.config.max_inbound_connections();
let stats = self.stats.clone();
let deadline = InboundDeadline {
first_frame: INBOUND_FIRST_FRAME_TIMEOUT,
idle: self.idle_timeout,
};
let accept_handle = tokio::spawn(async move {
tor_accept_loop(
@@ -368,7 +394,7 @@ impl TorTransport {
pool,
mtu,
max_inbound,
INBOUND_FIRST_FRAME_TIMEOUT,
deadline,
stats,
)
.await;
@@ -1107,9 +1133,9 @@ impl Transport for TorTransport {
/// below zero). `direction` is retained for the terminal "receive loop
/// stopped" debug field the shared loop deliberately leaves to each transport.
///
/// `first_frame_timeout` is `Some` for an inbound connection, which holds a
/// capped pool slot from the moment it is accepted, and `None` for an
/// outbound one, which holds no such slot. `ready_rx`, when present, is the
/// `deadline` is `Some` for an inbound connection, which holds a capped pool
/// slot from the moment it is accepted, and `None` for an outbound one,
/// which holds no such slot. `ready_rx`, when present, is the
/// accept loop's readiness barrier: the loop must not run its cleanup before
/// the accept loop has inserted the pool entry and bumped its counter.
#[allow(clippy::too_many_arguments)]
@@ -1123,7 +1149,7 @@ async fn tor_receive_loop(
mtu: u16,
stats: Arc<TorStats>,
direction: Direction,
first_frame_timeout: Option<Duration>,
deadline: Option<InboundDeadline>,
ready_rx: Option<tokio::sync::oneshot::Receiver<()>>,
) {
proxied_receive_loop(
@@ -1136,7 +1162,7 @@ async fn tor_receive_loop(
mtu,
stats,
"Tor",
first_frame_timeout,
deadline,
ready_rx,
|stats, meta| match meta {
Direction::Inbound => stats.record_pool_inbound_removed(),
@@ -1164,11 +1190,11 @@ async fn tor_receive_loop(
/// socket options, split the stream, and spawn a per-connection
/// receive task.
///
/// `first_frame_timeout` is the deadline from accept to the first complete
/// inbound frame, handed to each spawned receive loop. An accepted socket
/// takes an inbound slot against `max_inbound` before any byte is read, so
/// without it a remote that connects and stays silent holds that slot for as
/// long as it keeps the socket open.
/// `deadline` holds the first-frame and idle deadlines handed to each spawned
/// receive loop. An accepted socket takes an inbound slot against
/// `max_inbound` before any byte is read, so without them a remote that
/// connects and stays silent, or sends one frame and then goes silent, holds
/// that slot for as long as it keeps the socket open.
#[allow(clippy::too_many_arguments)]
async fn tor_accept_loop(
listener: TcpListener,
@@ -1177,7 +1203,7 @@ async fn tor_accept_loop(
pool: ProxiedPool<Direction>,
mtu: u16,
max_inbound: usize,
first_frame_timeout: Duration,
deadline: InboundDeadline,
stats: Arc<TorStats>,
) {
debug!(
@@ -1272,7 +1298,7 @@ async fn tor_accept_loop(
mtu,
recv_stats,
Direction::Inbound,
Some(first_frame_timeout),
Some(deadline),
Some(ready_rx),
)
.await;
@@ -2044,7 +2070,8 @@ mod tests {
fn spawn_onion_accept_loop(
listener: TcpListener,
packet_tx: PacketTx,
first_frame_timeout: Duration,
first_frame: Duration,
idle: Duration,
) -> (ProxiedPool<Direction>, Arc<TorStats>, JoinHandle<()>) {
let pool: ProxiedPool<Direction> = Arc::new(Mutex::new(HashMap::new()));
let stats = Arc::new(TorStats::new());
@@ -2055,7 +2082,7 @@ mod tests {
pool.clone(),
1400,
64,
first_frame_timeout,
InboundDeadline { first_frame, idle },
stats.clone(),
));
(pool, stats, handle)
@@ -2070,8 +2097,12 @@ mod tests {
let (tx, _rx) = packet_channel(32);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let listen = listener.local_addr().unwrap();
let (pool, stats, accept) =
spawn_onion_accept_loop(listener, tx, Duration::from_millis(200));
let (pool, stats, accept) = spawn_onion_accept_loop(
listener,
tx,
Duration::from_millis(200),
Duration::from_secs(5),
);
// Held open for the whole test: any release is the deadline's doing.
let squatter = TcpStream::connect(listen).await.unwrap();
@@ -2099,8 +2130,12 @@ mod tests {
let (tx, mut rx) = packet_channel(32);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let listen = listener.local_addr().unwrap();
let (_pool, stats, accept) =
spawn_onion_accept_loop(listener, tx, Duration::from_millis(300));
let (_pool, stats, accept) = spawn_onion_accept_loop(
listener,
tx,
Duration::from_millis(300),
Duration::from_secs(5),
);
let frame = build_msg1_frame();
let mut peer = TcpStream::connect(listen).await.unwrap();
@@ -2124,18 +2159,99 @@ mod tests {
accept.abort();
}
/// The healthy path, and a regression guard as for TCP: the deadline is
/// scoped to the first iteration, so an established onion connection that
/// then goes quiet keeps its slot. It exists so a future general idle
/// deadline cannot start reaping quiet onion links without a test going
/// red.
/// The smallest frame the reader accepts as established, which the node
/// drops without closing the transport when it names no session.
fn squatter_frame() -> Vec<u8> {
let frame = crate::transport::framing::build_established_frame(0);
assert_eq!(frame.len(), crate::proto::fmp::wire::ENCRYPTED_MIN_SIZE);
frame
}
/// Mirror of the TCP case: an onion-side remote that sends one frame and
/// then goes silent must lose its slot at the idle deadline. The
/// first-frame deadline is far above the idle one, so the release is the
/// idle deadline's doing. Break-check: scope the deadline in the shared
/// loop back to the first read and the count stays at 1.
#[tokio::test]
async fn established_onion_connection_survives_long_idle() {
async fn onion_connection_that_goes_silent_after_one_frame_releases_its_slot() {
let (tx, mut rx) = packet_channel(32);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let listen = listener.local_addr().unwrap();
let (pool, stats, accept) = spawn_onion_accept_loop(
listener,
tx,
Duration::from_secs(5),
Duration::from_millis(300),
);
let mut squatter = TcpStream::connect(listen).await.unwrap();
squatter.write_all(&squatter_frame()).await.unwrap();
let packet = tokio::time::timeout(Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for the squatter's frame")
.expect("packet channel closed");
assert_eq!(packet.data, squatter_frame());
assert_eq!(stats.pool_inbound_count(), 1);
assert!(
wait_until(|| stats.pool_inbound_count() == 0, Duration::from_secs(2)).await,
"an onion connection silent after its first frame should lose its slot at the idle deadline"
);
assert!(pool.lock().await.is_empty());
drop(squatter);
accept.abort();
}
/// The healthy path: an onion connection delivering a frame more often
/// than the idle deadline keeps its slot across many deadlines.
///
/// Break-check: a deadline that does not re-arm on each frame (a single
/// deadline from accept) drops the connection after the first second.
#[tokio::test]
async fn onion_connection_sending_a_frame_every_interval_below_the_idle_deadline_is_kept() {
let (tx, mut rx) = packet_channel(32);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let listen = listener.local_addr().unwrap();
let (pool, stats, accept) =
spawn_onion_accept_loop(listener, tx, Duration::from_millis(200));
spawn_onion_accept_loop(listener, tx, Duration::from_secs(1), Duration::from_secs(1));
let mut peer = TcpStream::connect(listen).await.unwrap();
// Twelve frames 250 ms apart: 3 s, three idle deadlines, with 750 ms
// of slack between each frame and the deadline it re-arms.
for i in 0..12 {
peer.write_all(&squatter_frame()).await.unwrap();
let packet = tokio::time::timeout(Duration::from_secs(2), rx.recv())
.await
.unwrap_or_else(|_| panic!("timeout waiting for frame {i}"))
.expect("packet channel closed");
assert_eq!(packet.data, squatter_frame());
assert_eq!(
stats.pool_inbound_count(),
1,
"an onion connection delivering frames inside the idle deadline must keep its slot (frame {i})"
);
tokio::time::sleep(Duration::from_millis(250)).await;
}
assert!(!pool.lock().await.is_empty());
drop(peer);
accept.abort();
}
/// An established onion connection quiet for longer than the first-frame
/// deadline, but not the idle deadline, keeps its slot.
#[tokio::test]
async fn established_onion_connection_quiet_past_the_first_frame_deadline_is_kept() {
let (tx, mut rx) = packet_channel(32);
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let listen = listener.local_addr().unwrap();
let (pool, stats, accept) = spawn_onion_accept_loop(
listener,
tx,
Duration::from_millis(200),
Duration::from_secs(5),
);
let mut peer = TcpStream::connect(listen).await.unwrap();
peer.write_all(&build_msg1_frame()).await.unwrap();
@@ -2145,7 +2261,7 @@ mod tests {
.expect("packet channel closed");
assert_eq!(packet.data, build_msg1_frame());
// Four deadlines' worth of silence after the first frame.
// Four first-frame deadlines' worth of silence after the first frame.
tokio::time::sleep(Duration::from_millis(800)).await;
assert_eq!(
@@ -2210,7 +2326,10 @@ mod tests {
1400,
stats.clone(),
Direction::Inbound,
Some(Duration::from_millis(50)),
Some(InboundDeadline {
first_frame: Duration::from_millis(50),
idle: Duration::from_millis(50),
}),
Some(ready_rx),
)
.await;
@@ -2266,7 +2385,10 @@ mod tests {
1400,
recv_stats,
Direction::Inbound,
Some(Duration::from_secs(5)),
Some(InboundDeadline {
first_frame: Duration::from_secs(5),
idle: Duration::from_secs(5),
}),
Some(ready_rx),
)
.await;
@@ -2359,7 +2481,10 @@ mod tests {
pool.clone(),
1400,
64,
Duration::from_secs(5),
InboundDeadline {
first_frame: Duration::from_secs(5),
idle: Duration::from_secs(5),
},
stats.clone(),
));
@@ -2467,7 +2592,8 @@ mod tests {
let client_addr = sock.local_addr().unwrap().as_socket().unwrap();
let remote = TransportAddr::from_string(&client_addr.to_string());
let (pool, stats, accept) = spawn_onion_accept_loop(listener, tx, Duration::from_secs(5));
let (pool, stats, accept) =
spawn_onion_accept_loop(listener, tx, Duration::from_secs(5), Duration::from_secs(5));
sock.connect(&listen.into()).unwrap();
let std_stream: std::net::TcpStream = sock.into();
@@ -2543,7 +2669,10 @@ mod tests {
pool.clone(),
1400,
64,
Duration::from_secs(5),
InboundDeadline {
first_frame: Duration::from_secs(5),
idle: Duration::from_secs(5),
},
stats.clone(),
));
+10 -10
View File
@@ -30,7 +30,7 @@ actual assignment.
A spanning-tree update that changes only an internal path edge — no
root change, no depth change — must not produce a sustained bloom
announce storm at downstream nodes. The original regression
(rolled-back `0caef2a`, fixed in master `4cdf382`) had this property:
(since rolled back, and fixed in master `4cdf382`) had this property:
in the field, a single mid-chain ancestor swap on an upstream node
caused every downstream node in its subtree to issue a bloom
announce on every parent re-evaluation tick of the upstream node,
@@ -74,7 +74,7 @@ including a regressed one.
## Threshold derivation
The original `issues/2026-0019-repro/` reproduction harness measured
The original reproduction harness, kept outside this repository, measured
(90s flap window, ~21 induced parent switches at the mid-chain
node):
@@ -93,8 +93,8 @@ n01=5 n02=5 n03=4 n04=12 n05=6 n06=0
n04 (the flapping node) is the highest because it is legitimately
re-sending its filter on its own parent changes. n06 (the depth-4
"tail") sees 0, matching the calm post-fix behavior recorded in
`issues/2026-0019-repro/RESULTS.md` for the `fix2` variant.
"tail") sees 0, matching the calm post-fix behavior that harness recorded
for its `fix2` variant.
In the field, the regression's mesh-wide rate scaled ~480x above
steady state. A `30 / 30s / node` ceiling sits ~2.5x above the
@@ -132,7 +132,7 @@ applied to every chaos-spawned container.
Rationale for ceiling = 40: lab max 30 + ~2σ headroom (≈ 39.4) rounds to
40, giving 33 % margin over the observed lab maximum while still firing
loud on a regression-class storm (the original `0caef2a` regression
loud on a regression-class storm (the original regression
scaled mesh-wide bloom traffic ~480× above steady state, far above any
plausible jitter band).
@@ -140,11 +140,11 @@ plausible jitter band).
- The bloom-storm regression has not been confirmed-failing here
on a regressed binary in this harness directly; the threshold is
inferred from the values measured in the dedicated
`issues/2026-0019-repro/` post-mortem harness against
`0caef2a`. To gain that confirmation, check out `0caef2a`
(or the `backup-broadcast-gate-bloom-storm` branch if still
retained), build, copy binaries into `testing/docker/`, and rerun
inferred from the values measured in the dedicated post-mortem
harness against a regressed build whose commit is not in this
repository. To gain that confirmation, build a binary that
carries the regression and predates the `4cdf382` fix, copy
binaries into `testing/docker/`, and rerun
this scenario; the bloom-rate assertion is expected to fail loud
with n05/n06 deltas well above 40.
+9 -26
View File
@@ -14,8 +14,7 @@
#
# What counts as a violation: any `uses:` reference that is not
# * `owner/repo@<40 hex> # <tag>` — the required form, comment mandatory; or
# * a local action, `./path` or `docker://...`; or
# * one of the individually justified references listed below.
# * a local action, `./path` or `docker://...`.
#
# WHAT THIS GUARD DOES NOT COVER, so a green run is not read as "the workflows
# fetch nothing unverified":
@@ -43,19 +42,13 @@ REPO_ROOT="$SCRIPT_DIR/.."
# it cannot describe, and a pin nobody can read is a pin nobody updates.
PINNED_RE='^[^@]+@[0-9a-f]{40} +#.*$'
# Individually justified unpinned references. Each entry is the exact ref text.
#
# Both of these actions read the tool they install from the ref name itself
# (`github.action_ref`), so replacing the ref with a SHA hands them a 40-hex
# string where a toolchain or tool name belongs and the step fails outright.
# They are not pinnable without also moving the selection into `with:`, which
# changes which toolchain resolves, and that is a separate decision from
# pinning. Note what stays exposed: both remain repointable by their upstream
# owners.
ALLOWED_REFS=(
'dtolnay/rust-toolchain@nightly'
'taiki-e/install-action@nextest'
)
# There are no exceptions. Some actions select what they install from the ref
# they are called at: a per-tool `taiki-e/install-action` tag, or a
# `dtolnay/rust-toolchain` channel branch, sets the selection input's default
# in that ref's action.yml. Pin such an action by SHA and pass the selection as
# an explicit `with:` input (`tool:`, `toolchain:`). That form works at a SHA
# from any of the action's refs; a bare SHA does not, because their `v2` and
# `master` trees declare the input required with no default.
if ! command -v git >/dev/null 2>&1; then
echo "check-action-pins: git not available, cannot sweep" >&2
@@ -83,15 +76,6 @@ if [[ ${#files[@]} -eq 0 ]]; then
exit 2
fi
# True when this ref is one of the justified references above.
allowed_ref() {
local ref="$1" entry
for entry in "${ALLOWED_REFS[@]}"; do
[[ "$ref" == "$entry" ]] && return 0
done
return 1
}
violations=0
checked=0
@@ -117,7 +101,6 @@ for f in "${files[@]}"; do
[[ "$ref" == ./* ]] && continue
[[ "$ref" == docker://* ]] && continue
[[ "$ref" =~ $PINNED_RE ]] && continue
allowed_ref "$ref" && continue
echo "$f:$n: $ref"
violations=$((violations + 1))
@@ -141,5 +124,5 @@ if [[ $violations -gt 0 ]]; then
exit 1
fi
echo "check-action-pins: all $checked action reference(s) pinned or justified"
echo "check-action-pins: all $checked action reference(s) pinned"
exit 0
+56 -24
View File
@@ -1,7 +1,7 @@
#!/bin/bash
# ── Source comment reference guard ──────────────────────────────────────────
# Every reference a source comment makes must resolve for a reader who has
# only this repository.
# ── Comment and document reference guard ────────────────────────────────────
# Every reference a source comment or committed document makes must resolve
# for a reader who has only this repository.
#
# A comment that names a private planning artifact — by identifier, by file
# path, or in programme vocabulary that has no in-repo referent — is dead text
@@ -22,17 +22,42 @@
# documents nobody has thought of. Scoped to src/ because resolving the
# whole tree's relative documentation links against the repository root
# is a different checker with its own population to triage first.
# 3. programme vocabulary, scoped to src/. Phase, rung, milestone and step
# labels that name a plan a reader cannot open. The pattern is shared
# verbatim with the sweep that produced today's clean tree, so the two
# cannot drift apart.
# 3. programme vocabulary, repo-wide in two scopes. Phase, rung,
# milestone and step labels, and phrases such as "architectural plan",
# that name a plan a reader cannot open. VOCAB_PAT runs over the whole
# tree except this file, which would match itself. SRC_VOCAB_PAT adds
# two alternatives that run over src/ only, because outside src/ they
# are legitimate prose: "in Step N" is the documentation's own step
# cross-referencing, and "this PR" is how CONTRIBUTING.md and
# PR-REVIEW.md talk about the PR under review. This file is the only
# copy of both patterns.
#
# The tree-wide alternatives had no hit outside src/ when the scope was
# widened. The one most likely to catch legitimate prose later is
# \b[RQ][0-9]\b, which matches "Q4" or "R2" as words. If it does,
# rephrase the text or move that alternative to SRC_VOCAB_PAT; do not
# exclude the file or directory, which drops every other alternative
# with it.
#
# Known coverage gaps, recorded rather than discovered:
# - a bare "milestone" in some other phrasing (the narrow alternatives keep
# the Tor bootstrap loop in src/transport/tor/mod.rs out of check 3);
# - the CATEGORY_D / CATEGORY_E code identifiers and test names, which
# check 3's case-sensitive Category-[A-Z] deliberately does not match;
# - programme vocabulary, or a document path, outside src/;
# - a document path outside src/. Check 2 resolves a token against the
# repository root, but documentation links outside src/ are relative to
# the citing file, so widening it means a different resolver and a
# triage of the links that fail it;
# - a private-workspace directory cited in lowercase (an issues/... or
# tasks/... path with no .md token). Check 1's identifier prefix is
# case-sensitive and check 2 only resolves *.md tokens;
# - programme phrasing too common to deny, such as "a later step" or
# "the plan": an alternative for it would red legitimate text, so it
# passes check 3 by design;
# - bare commit hashes. Whether a hash resolves depends on the clone's
# object database, and GitHub CI clones are shallow, so a check would
# read differently on different hosts. The rule applied by hand is that
# a comment cites a commit only if a trunk branch contains it;
# - a reference to a private artifact made in free prose with no marker at
# all, at any scope: no check here matches it;
# - a pathspec typo introduced after this file lands. git grep returns 1
@@ -67,23 +92,23 @@ cd "$ROOT" || exit 2
# fires the assertion. This covers checks 2 and 3 only; check 1's pathspec is
# "." and stays non-empty from anywhere, so its wrong-directory case is covered
# by the --show-prefix assertion above and its mistyped-pathspec case by
# nothing.
# nothing. Check 3's tree-wide half uses "." with exclusions and is in the
# same position as check 1.
n=$(git ls-tree -r --name-only HEAD -- src/ | wc -l)
(( n > 0 )) || { echo "check-comment-refs: pathspec src/ matched no files" >&2; exit 2; }
# Deliberately a superset of the sweep's own acceptance regex: the year group
# is optional, so the three-digit form is caught as well, and the separator is
# optional so underscores and spaces are caught alongside hyphens. Narrowing
# this is how a whole class goes unguarded while every break-check still
# passes.
# Deliberately broad: the year group is optional, so the three-digit form is
# caught as well, and the separator is optional so underscores and spaces are
# caught alongside hyphens. Narrowing this is how a whole class goes unguarded
# while every break-check still passes.
ID_PAT='(TASK|ISSUE|IDEA|QUICK|RECUR)[-_ ]?(20[0-9]{2}[-_ ])?[0-9]{3,4}'
# Verbatim from the sweep's enumerating pattern. Do not edit one without the
# other. If check 3's scope is ever widened beyond src/, this text matches
# itself through six of its alternatives and the widening must add
# ':(exclude)testing/check-comment-refs.sh' — never an exclusion of testing/,
# which holds real check-1 hits a directory-wide exclusion would drop.
VOCAB_PAT='\b[RQ][0-9]\b|Category-[A-Z]|\bumbrella\b|refactor steps?|\bpre-scopes\b|R0 stub|read-isolation|cut over yet|Cutover begins|in Step [0-9]|(the|this) milestone|Milestone-'
# VOCAB_PAT runs tree-wide. This text matches itself, so the tree-wide run
# excludes this one file by name; never exclude testing/, which holds real
# hits a directory-wide exclusion would drop. SRC_VOCAB_PAT is VOCAB_PAT
# plus the alternatives that are legitimate prose outside src/.
VOCAB_PAT='\b[RQ][0-9]\b|Category-[A-Z]|\bumbrella\b|refactor steps?|\bpre-scopes\b|R0 stub|read-isolation|cut over yet|Cutover begins|(the|this) milestone|Milestone-|[Aa]rchitectural plan|[Ff]ollow-up wiring'
SRC_VOCAB_PAT="$VOCAB_PAT"'|in Step [0-9]|\b[Tt]his PR\b'
MD_PAT='[A-Za-z0-9_./-]+\.md\b'
@@ -157,9 +182,12 @@ if (( ${#CITES[@]} > 0 )); then
done < <(printf '%s\n' "${!CITES[@]}" | sort)
fi
# ── Check 3: programme vocabulary under src/ ────────────────────────────────
rc=0
out=$(git grep -nEI "$VOCAB_PAT" HEAD -- src/) || rc=$?
# ── Check 3: programme vocabulary, src/ and the rest of the tree ────────────
vocab_check() {
local pat=$1
shift
local rc=0 out
out=$(git grep -nEI "$pat" HEAD -- "$@") || rc=$?
if (( rc > 1 )); then
echo "check-comment-refs: check 3 grep failed (rc=$rc)" >&2
exit 2
@@ -169,10 +197,14 @@ if [[ -n "$out" ]]; then
| sed -E 's|^HEAD:([^:]*):([0-9]+):|\1:\2: names a plan this repository does not carry: |'
FAILED=1
fi
return 0
}
vocab_check "$SRC_VOCAB_PAT" src/
vocab_check "$VOCAB_PAT" . ':(exclude)src/' ':(exclude)testing/check-comment-refs.sh'
if (( FAILED != 0 )); then
echo "" >&2
echo "check-comment-refs: a source comment references something a reader holding" >&2
echo "check-comment-refs: a comment or document references something a reader holding" >&2
echo "only this repository cannot resolve. Rewrite the comment to say what the" >&2
echo "code does, citing nothing outside the tree." >&2
exit 1
-175
View File
@@ -1,175 +0,0 @@
#!/bin/bash
# ── Debian package version derivation check ─────────────────────────────────
# A release candidate is tagged vX.Y.Z-rcN, because git refuses '~' in a ref
# name. dpkg reads X.Y.Z-rcN as revision rcN of X.Y.Z and sorts it ABOVE the
# release, so a host that installed the candidate is never upgraded by the
# release. package-linux.yml's "Derive Linux package version" step therefore
# maps the tag's pre-release suffix to '~' for the .deb, which dpkg sorts below.
#
# This runs that step's own text, taken from the workflow, rather than a copy,
# so the check and the workflow cannot drift apart. Cases:
# refs/tags/v0.5.3 both versions 0.5.3
# refs/tags/v0.5.3-rc1 deb 0.5.3~rc1, tarball and artifact 0.5.3-rc1, and
# dpkg orders 0.5.2 < 0.5.3~rc1 < 0.5.3
# refs/heads/maint deb equals the tarball version, which is
# <Cargo version>+maint.<height>.<hash>
#
# It also checks the wiring the derivation needs to have any effect: the job
# declares the deb_package_version output, the .deb build passes it as
# --version, and that step renames a '~' in the package file name to '-' (a
# GitHub release renames an asset with special characters in its name, which
# would leave checksums-linux.txt naming a file the release does not have).
# Those three are read from the text, not executed.
#
# Exit 0 = clean. Exit 1 = a case or a wiring check failed. Exit 2 = the check
# could not run (no dpkg, no PyYAML, the step not found, or an output missing);
# never treated as a pass.
# ─────────────────────────────────────────────────────────────────────────────
set -uo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
WORKFLOW="$PROJECT_ROOT/.github/workflows/package-linux.yml"
cant() {
echo "check-deb-version: $*; cannot verify the Debian version derivation" >&2
exit 2
}
[ -f "$WORKFLOW" ] || cant "missing $WORKFLOW"
command -v dpkg >/dev/null 2>&1 || cant "dpkg not found"
command -v python3 >/dev/null 2>&1 || cant "python3 not found"
python3 -c "import yaml" >/dev/null 2>&1 || cant "python3 module 'yaml' not found"
WORK=$(mktemp -d) || cant "mktemp failed"
trap 'rm -rf "$WORK"' EXIT
# Pull the derivation step's run text, the job's declared outputs and the
# .deb build step's run text out of the workflow.
if ! python3 - "$WORKFLOW" "$WORK" <<'PY'
import sys
from pathlib import Path
import yaml
workflow, work = sys.argv[1], Path(sys.argv[2])
doc = yaml.safe_load(Path(workflow).read_text(encoding="utf-8"))
jobs = doc.get("jobs") or {}
def step_run(job, name):
for step in (jobs.get(job) or {}).get("steps") or []:
if step.get("name") == name:
return step.get("run")
return None
derive = step_run("determine-versioning", "Derive Linux package version")
build = step_run("build", "Build Debian package in the pinned container")
if not derive or not build:
missing = "derivation" if not derive else "Debian build"
print(f"check-deb-version: the {missing} step was not found in {workflow}",
file=sys.stderr)
sys.exit(2)
(work / "derive.sh").write_text(derive, encoding="utf-8")
(work / "build.sh").write_text(build, encoding="utf-8")
outputs = (jobs.get("determine-versioning") or {}).get("outputs") or {}
(work / "outputs").write_text(
"".join(f"{k}={v}\n" for k, v in outputs.items()), encoding="utf-8")
PY
then
exit 2
fi
FAILED=0
ok() { echo " PASS $*"; }
bad() { echo " FAIL $*"; FAILED=$((FAILED + 1)); }
# Run the derivation for one ref and load its outputs into LINUX and DEB.
# Exits 2 when the step fails or does not write both outputs: no version was
# established, which is not a failed case.
derive() {
local ref="$1" out="$WORK/github_output"
: > "$out"
if ! (cd "$PROJECT_ROOT" && GITHUB_OUTPUT="$out" GITHUB_REF="$ref" \
GITHUB_REF_NAME="${ref#refs/*/}" bash -eo pipefail "$WORK/derive.sh") >"$WORK/derive.log" 2>&1; then
echo "check-deb-version: the derivation failed for $ref:" >&2
cat "$WORK/derive.log" >&2
exit 2
fi
LINUX=$(sed -n 's/^linux_package_version=//p' "$out")
DEB=$(sed -n 's/^deb_package_version=//p' "$out")
if [ -z "$LINUX" ] || [ -z "$DEB" ]; then
cant "the derivation for $ref did not write both outputs (linux '$LINUX', deb '$DEB')"
fi
return 0
}
# Record whether dpkg orders $1 $2 $3 (lt, gt, ...).
expect_order() {
if dpkg --compare-versions "$1" "$2" "$3"; then
ok "dpkg: $1 $2 $3"
else
bad "dpkg: $1 $2 $3 does not hold"
fi
return 0
}
# Record whether a derived version equals the expected one.
expect_eq() {
local label="$1" got="$2" want="$3"
if [ "$got" = "$want" ]; then
ok "$label: $got"
else
bad "$label: $got (want $want)"
fi
return 0
}
echo "Release tag refs/tags/v0.5.3"
derive refs/tags/v0.5.3
expect_eq "linux_package_version" "$LINUX" "0.5.3"
expect_eq "deb_package_version" "$DEB" "0.5.3"
echo "Candidate tag refs/tags/v0.5.3-rc1"
derive refs/tags/v0.5.3-rc1
expect_eq "linux_package_version" "$LINUX" "0.5.3-rc1"
expect_eq "deb_package_version" "$DEB" "0.5.3~rc1"
expect_order "$DEB" lt 0.5.3
expect_order "$DEB" gt 0.5.2
echo "Branch refs/heads/maint"
derive refs/heads/maint
CRATE=$(awk -F'"' '/^version = /{print $2; exit}' "$PROJECT_ROOT/Cargo.toml")
HEIGHT=$(git -C "$PROJECT_ROOT" rev-list --count HEAD) || cant "git rev-list failed"
HASH=$(git -C "$PROJECT_ROOT" rev-parse --short HEAD) || cant "git rev-parse failed"
[ -n "$CRATE" ] || cant "no version in Cargo.toml"
expect_eq "linux_package_version" "$LINUX" "${CRATE}+maint.${HEIGHT}.${HASH}"
expect_eq "deb_package_version" "$DEB" "$LINUX"
echo "Wiring"
# shellcheck disable=SC2016 # the ${{ }} expressions are workflow text, not shell
if grep -qxF 'deb_package_version=${{ steps.linux_version.outputs.deb_package_version }}' "$WORK/outputs"; then
ok "determine-versioning declares the deb_package_version output"
else
bad "determine-versioning does not declare deb_package_version from the derivation step"
fi
# shellcheck disable=SC2016
if grep -qF -- '--version "${{ needs.determine-versioning.outputs.deb_package_version }}"' "$WORK/build.sh"; then
ok "the .deb build passes deb_package_version as --version"
else
bad "the .deb build does not pass deb_package_version as --version"
fi
if grep -qF "tr '~' '-'" "$WORK/build.sh"; then
ok "the .deb build renames a '~' in the package file name"
else
bad "the .deb build does not rename a '~' in the package file name"
fi
echo
if [ "$FAILED" -eq 0 ]; then
echo "check-deb-version: all checks passed"
exit 0
fi
echo "check-deb-version: $FAILED check(s) failed"
exit 1
+86 -22
View File
@@ -16,17 +16,37 @@
# Anything else is treated as a single ELF binary.
#
# Reads the floor from packaging/build-floor.env unless FIPS_GLIBC_FLOOR is set.
#
# Exit 0 = every input was examined and none is above the floor. Exit 1 = a
# binary needs a newer glibc than the floor. Exit 2 = an input could not be
# examined (missing, not an ELF object, a .deb that would not unpack or holds
# no binaries), or the check could not run at all; never treated as a pass,
# and never reported as a binary above the floor.
set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
# A missing or empty floor means the check cannot run, so it exits 2 here
# rather than letting `set -e` or a `:?` expansion end the run with status 1,
# which callers would read as a binary above the floor.
FLOOR_ENV="$REPO_ROOT/packaging/build-floor.env"
if [ -z "${FIPS_GLIBC_FLOOR:-}" ]; then
# shellcheck source=../packaging/build-floor.env
. "$REPO_ROOT/packaging/build-floor.env"
[ -r "$FLOOR_ENV" ] || {
echo "check-glibc-floor: cannot read $FLOOR_ENV and FIPS_GLIBC_FLOOR is not set;" >&2
echo " there is no floor to check against." >&2
exit 2
}
# shellcheck source-path=SCRIPTDIR source=../packaging/build-floor.env
. "$FLOOR_ENV"
fi
FLOOR="${FIPS_GLIBC_FLOOR:?no floor declared}"
if [ -z "${FIPS_GLIBC_FLOOR:-}" ]; then
echo "check-glibc-floor: $FLOOR_ENV declares no FIPS_GLIBC_FLOOR;" >&2
echo " there is no floor to check against." >&2
exit 2
fi
FLOOR="$FIPS_GLIBC_FLOOR"
for tool in readelf dpkg dpkg-deb; do
command -v "$tool" >/dev/null 2>&1 || {
@@ -65,6 +85,18 @@ max_glibc_need() {
FAILED=0
CHECKED=0
UNCHECKED=0
UNCHECKED_LIST=()
# unchecked <label> <reason>: records an input this run could not examine. It
# is kept apart from FAILED because the remedy differs: an input that was never
# read says nothing about the floor, and the above-the-floor advice would send
# the reader to rebuild a package that may be fine.
unchecked() {
echo " UNCHECKED $1: could not check ($2)" >&2
UNCHECKED_LIST+=("$1: $2")
UNCHECKED=$((UNCHECKED + 1))
}
is_elf() { readelf -hW "$1" >/dev/null 2>&1; }
@@ -78,8 +110,12 @@ check_binary() {
CHECKED=$((CHECKED + 1))
return
fi
echo " ERROR $label is not a readable ELF object" >&2
FAILED=$((FAILED + 1))
case "$label" in
*.tar.gz|*.tgz|*.tar|*.tar.*)
unchecked "$label" "not a readable ELF object; unpack it and pass its binaries" ;;
*)
unchecked "$label" "not a readable ELF object" ;;
esac
return
fi
CHECKED=$((CHECKED + 1))
@@ -96,11 +132,14 @@ check_deb() {
tmp=$(mktemp -d)
# shellcheck disable=SC2064
trap "rm -rf '$tmp'" RETURN
dpkg-deb -x "$deb" "$tmp"
if ! dpkg-deb -x "$deb" "$tmp"; then
unchecked "$(basename "$deb")" "dpkg-deb could not unpack it"
return
fi
# A package legitimately ships executable shell scripts alongside its
# binaries -- fips-dns-setup and its teardown are two -- so filter to ELF
# objects rather than treating a script as an unreadable binary. A .deb
# with no ELF object at all is still an error: it means the glob or the
# with no ELF object at all is still not a pass: it means the glob or the
# layout moved and this check examined nothing.
local found=0 f
while IFS= read -r -d '' f; do
@@ -109,8 +148,7 @@ check_deb() {
check_binary "$f" "$(basename "$deb"):$(basename "$f")"
done < <(find "$tmp" -type f -perm -u+x -print0)
if [ "$found" -eq 0 ]; then
echo " ERROR $(basename "$deb") contains no ELF executables" >&2
FAILED=$((FAILED + 1))
unchecked "$(basename "$deb")" "contains no ELF executables"
fi
}
@@ -121,27 +159,53 @@ check_deb() {
echo "=== glibc floor check (declared floor: $FLOOR) ==="
for arg in "$@"; do
[ -e "$arg" ] || { echo " ERROR $arg does not exist" >&2; FAILED=$((FAILED + 1)); continue; }
[ -e "$arg" ] || { unchecked "$arg" "does not exist"; continue; }
case "$arg" in
*.deb) check_deb "$arg" ;;
*) check_binary "$arg" "$(basename "$arg")" ;;
esac
done
# Nothing examined is a failure, not a pass. An argument list that matched no
# binary means the caller's glob went stale, and reporting that as green is how
# a guard quietly stops guarding.
if [ "$CHECKED" -eq 0 ] && [ "$FAILED" -eq 0 ]; then
# print_unchecked: lists the inputs this run could not examine, if any.
print_unchecked() {
[ "$UNCHECKED" -gt 0 ] || return 0
echo " Not checked:" >&2
local entry
for entry in "${UNCHECKED_LIST[@]}"; do
echo " $entry" >&2
done
}
# A real floor failure is the stronger signal, so it decides the status even
# when other inputs went unchecked; those are still listed.
if [ "$FAILED" -ne 0 ]; then
echo "check-glibc-floor: $FAILED binary(ies) above the floor across $CHECKED checked, $UNCHECKED input(s) not checked." >&2
echo " A binary above the floor installs cleanly and then fails to start." >&2
echo " Build through packaging/debian/build-deb-container.sh, which pins" >&2
echo " the build image to the oldest supported distribution." >&2
print_unchecked
exit 1
fi
# An input that could not be examined is not a pass. This is also where a
# stale caller glob lands: an unexpanded pattern such as "$UNPACK"/*/fips
# arrives as a path that does not exist, and reporting that as green is how a
# guard quietly stops guarding.
if [ "$UNCHECKED" -ne 0 ]; then
if [ "$CHECKED" -eq 0 ]; then
echo "check-glibc-floor: could not check $UNCHECKED input(s) and examined no binaries." >&2
else
echo "check-glibc-floor: could not check $UNCHECKED input(s); nothing was found above the floor in the $CHECKED binaries checked." >&2
fi
print_unchecked
exit 2
fi
# Backstop: every argument raises CHECKED, FAILED or UNCHECKED, and an empty
# argument list is refused above, so this is not expected to be reached.
if [ "$CHECKED" -eq 0 ]; then
echo "check-glibc-floor: examined no binaries; refusing to report a pass." >&2
exit 2
fi
if [ "$FAILED" -ne 0 ]; then
echo "check-glibc-floor: $FAILED problem(s) across $CHECKED binaries." >&2
echo " A binary above the floor installs cleanly and then fails to start." >&2
echo " Build through packaging/debian/build-deb-container.sh, which pins" >&2
echo " the build image to the oldest supported distribution." >&2
exit 1
fi
echo "=== glibc floor check passed ($CHECKED binaries, all at or below $FLOOR) ==="
+269
View File
@@ -0,0 +1,269 @@
#!/bin/bash
# ── Package version derivation check ────────────────────────────────────────
# A release candidate is tagged vX.Y.Z-rcN, because git refuses '~' in a ref
# name. dpkg reads X.Y.Z-rcN as revision rcN of X.Y.Z and sorts it ABOVE the
# release, so a host that installed the candidate is never upgraded by the
# release. package-linux.yml's "Derive Linux package version" step therefore
# maps the tag's pre-release suffix to '~' for the .deb, which dpkg sorts below.
#
# This runs that step's own text, taken from the workflow, rather than a copy,
# so the check and the workflow cannot drift apart.
#
# Debian cases:
# refs/tags/v0.5.3 both versions 0.5.3
# refs/tags/v0.5.3-rc1 deb 0.5.3~rc1, tarball and artifact 0.5.3-rc1, and
# dpkg orders 0.5.2 < 0.5.3~rc1 < 0.5.3
# refs/heads/maint deb equals the tarball version, which is
# <Cargo version>+maint.<height>.<hash>
#
# It also checks the wiring the derivation needs to have any effect: the job
# declares the deb_package_version output, the .deb build passes it as
# --version, and that step renames a '~' in the package file name to '-' (a
# GitHub release renames an asset with special characters in its name, which
# would leave checksums-linux.txt naming a file the release does not have).
# Those three are read from the text, not executed.
#
# OpenWrt's opkg compares versions with dpkg's algorithm (libopkg/pkg.c
# verrevcmp in openwrt/opkg-lede, read at 80503d94) and splits a revision at
# the last '-', so the .ipk has the same defect. package-openwrt.yml's "Derive
# package version" step maps the tag for the .ipk control Version, keeping the
# leading 'v' every released .ipk carries: under opkg, 0.5.4 sorts below
# v0.5.3, so dropping it would stop releases upgrading. dpkg stands in for
# opkg as the ordering oracle; it warns about the 'v' and still compares.
#
# ipk cases:
# refs/tags/v0.5.3 package_version and ipk_version v0.5.3
# refs/tags/v0.5.3-rc1 ipk_version v0.5.3~rc1, package_version (the file
# label) v0.5.3-rc1, and v0.5.2 < v0.5.3~rc1 < v0.5.3
# < v0.5.4
# refs/tags/v0.5.3-beta1 ipk_version v0.5.3~beta1, which sorts between
# v0.5.2 and v0.5.3~rc1
# refs/heads/maint ipk_version equals package_version
# and the wiring: the job declares ipk_version, and the "Build .ipk" step
# passes it as IPK_VERSION. build-ipk.sh's use of IPK_VERSION is executed by
# testing/openwrt/package-test.sh.
#
# Exit 0 = clean. Exit 1 = a case or a wiring check failed, including a
# derivation that ran but did not write a declared output. Exit 2 = the check
# could not run (no dpkg, no PyYAML, a step not found, the derivation failed,
# or dpkg could not compare); never treated as a pass.
# ─────────────────────────────────────────────────────────────────────────────
set -uo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
WORKFLOWS="$PROJECT_ROOT/.github/workflows"
cant() {
echo "check-package-versions: $*; cannot verify the package version derivation" >&2
exit 2
}
command -v dpkg >/dev/null 2>&1 || cant "dpkg not found"
command -v python3 >/dev/null 2>&1 || cant "python3 not found"
python3 -c "import yaml" >/dev/null 2>&1 || cant "python3 module 'yaml' not found"
WORK=$(mktemp -d) || cant "mktemp failed"
trap 'rm -rf "$WORK"' EXIT
# Pull a derivation step's run text, its job's declared outputs, and a build
# step's env block and run text out of a workflow, into
# $WORK/<prefix>.derive.sh, <prefix>.outputs and <prefix>.build.sh.
# Usage: extract <prefix> <workflow file> <job> <derive step> <build job> <build step>
extract() {
local prefix="$1" workflow="$WORKFLOWS/$2"
[ -f "$workflow" ] || cant "missing $workflow"
python3 - "$workflow" "$WORK" "$prefix" "$3" "$4" "$5" "$6" <<'PY' || exit 2
import sys
from pathlib import Path
import yaml
workflow, work, prefix, job, derive_name, build_job, build_name = sys.argv[1:]
work = Path(work)
doc = yaml.safe_load(Path(workflow).read_text(encoding="utf-8"))
jobs = doc.get("jobs") or {}
def find_step(job, name):
for step in (jobs.get(job) or {}).get("steps") or []:
if step.get("name") == name:
return step
return None
derive = find_step(job, derive_name)
build = find_step(build_job, build_name)
for label, step in (("derivation", derive), ("build", build)):
if not step or not step.get("run"):
print(f"check-package-versions: the {label} step was not found in {workflow}",
file=sys.stderr)
sys.exit(2)
(work / f"{prefix}.derive.sh").write_text(derive["run"], encoding="utf-8")
env = build.get("env") or {}
(work / f"{prefix}.build.sh").write_text(
"".join(f"{k}: {v}\n" for k, v in env.items()) + build["run"],
encoding="utf-8")
outputs = (jobs.get(job) or {}).get("outputs") or {}
(work / f"{prefix}.outputs").write_text(
"".join(f"{k}={v}\n" for k, v in outputs.items()), encoding="utf-8")
PY
}
FAILED=0
ok() { echo " PASS $*"; }
bad() { echo " FAIL $*"; FAILED=$((FAILED + 1)); }
# Run a derivation step for one ref and load the named outputs into OUT.
# Exits 2 when the step itself fails: no version was established. A step that
# runs but does not write a declared output is a failed check rather than a
# harness failure, so it is recorded and the output is left empty.
# Usage: derive <prefix> <ref> <output name>...
declare -A OUT
derive() {
local prefix="$1" ref="$2" out="$WORK/github_output" name
shift 2
: > "$out"
if ! (cd "$PROJECT_ROOT" && GITHUB_OUTPUT="$out" GITHUB_REF="$ref" \
GITHUB_REF_NAME="${ref#refs/*/}" bash -eo pipefail "$WORK/$prefix.derive.sh") >"$WORK/derive.log" 2>&1; then
echo "check-package-versions: the $prefix derivation failed for $ref:" >&2
cat "$WORK/derive.log" >&2
exit 2
fi
for name in "$@"; do
OUT[$name]=$(sed -n "s/^$name=//p" "$out")
if [ -z "${OUT[$name]}" ]; then
bad "the $prefix derivation for $ref did not write $name"
fi
done
return 0
}
# Record whether dpkg orders $1 $2 $3 (lt, gt, ...). An empty version is a
# failed case, because dpkg would compare it and sort it below everything.
expect_order() {
local rc=0
if [ -z "$1" ] || [ -z "$3" ]; then
bad "dpkg: no version to compare ('$1' $2 '$3')"
return 0
fi
dpkg --compare-versions "$1" "$2" "$3" 2>"$WORK/dpkg.err" || rc=$?
case $rc in
0) ok "dpkg: $1 $2 $3" ;;
1) bad "dpkg: $1 $2 $3 does not hold" ;;
*) cat "$WORK/dpkg.err" >&2
cant "dpkg could not compare $1 $2 $3 (exit $rc)" ;;
esac
return 0
}
# Record whether a derived version equals the expected one.
expect_eq() {
local label="$1" got="$2" want="$3"
if [ "$got" = "$want" ]; then
ok "$label: $got"
else
bad "$label: '$got' (want $want)"
fi
return 0
}
# Record whether extracted workflow text contains a fixed string. With -x the
# string must be a whole line.
# Usage: expect_text [-x] <file> <text> <description>
expect_text() {
local flags=-qF
if [ "$1" = -x ]; then flags=-qxF; shift; fi
if grep "$flags" -- "$2" "$1"; then
ok "$3"
else
bad "not so: $3"
fi
return 0
}
CRATE=$(awk -F'"' '/^version = /{print $2; exit}' "$PROJECT_ROOT/Cargo.toml")
HEIGHT=$(git -C "$PROJECT_ROOT" rev-list --count HEAD) || cant "git rev-list failed"
HASH=$(git -C "$PROJECT_ROOT" rev-parse --short HEAD) || cant "git rev-parse failed"
[ -n "$CRATE" ] || cant "no version in Cargo.toml"
# ── Debian .deb, package-linux.yml ──────────────────────────────────────────
extract deb package-linux.yml determine-versioning "Derive Linux package version" \
build "Build Debian package in the pinned container"
echo "Debian: release tag refs/tags/v0.5.3"
derive deb refs/tags/v0.5.3 linux_package_version deb_package_version
expect_eq "linux_package_version" "${OUT[linux_package_version]}" "0.5.3"
expect_eq "deb_package_version" "${OUT[deb_package_version]}" "0.5.3"
echo "Debian: candidate tag refs/tags/v0.5.3-rc1"
derive deb refs/tags/v0.5.3-rc1 linux_package_version deb_package_version
expect_eq "linux_package_version" "${OUT[linux_package_version]}" "0.5.3-rc1"
expect_eq "deb_package_version" "${OUT[deb_package_version]}" "0.5.3~rc1"
expect_order "${OUT[deb_package_version]}" lt 0.5.3
expect_order "${OUT[deb_package_version]}" gt 0.5.2
echo "Debian: branch refs/heads/maint"
derive deb refs/heads/maint linux_package_version deb_package_version
expect_eq "linux_package_version" "${OUT[linux_package_version]}" "${CRATE}+maint.${HEIGHT}.${HASH}"
expect_eq "deb_package_version" "${OUT[deb_package_version]}" "${OUT[linux_package_version]}"
echo "Debian: wiring"
# shellcheck disable=SC2016 # the ${{ }} expressions are workflow text, not shell
expect_text -x "$WORK/deb.outputs" \
'deb_package_version=${{ steps.linux_version.outputs.deb_package_version }}' \
"determine-versioning declares deb_package_version from the derivation step"
# shellcheck disable=SC2016
expect_text "$WORK/deb.build.sh" \
'--version "${{ needs.determine-versioning.outputs.deb_package_version }}"' \
"the .deb build passes deb_package_version as --version"
expect_text "$WORK/deb.build.sh" "tr '~' '-'" \
"the .deb build renames a '~' in the package file name"
# ── OpenWrt .ipk, package-openwrt.yml ─────────────────────────────────────
extract ipk package-openwrt.yml determine-versioning "Derive package version" \
build "Build .ipk"
echo "ipk: release tag refs/tags/v0.5.3"
derive ipk refs/tags/v0.5.3 package_version ipk_version
expect_eq "package_version" "${OUT[package_version]}" "v0.5.3"
expect_eq "ipk_version" "${OUT[ipk_version]}" "v0.5.3"
echo "ipk: candidate tag refs/tags/v0.5.3-rc1"
derive ipk refs/tags/v0.5.3-rc1 package_version ipk_version
expect_eq "package_version" "${OUT[package_version]}" "v0.5.3-rc1"
expect_eq "ipk_version" "${OUT[ipk_version]}" "v0.5.3~rc1"
expect_order "${OUT[ipk_version]}" lt v0.5.3
expect_order "${OUT[ipk_version]}" gt v0.5.2
expect_order v0.5.4 gt "${OUT[ipk_version]}"
RC1_IPK="${OUT[ipk_version]}"
echo "ipk: candidate tag refs/tags/v0.5.3-beta1"
derive ipk refs/tags/v0.5.3-beta1 package_version ipk_version
expect_eq "package_version" "${OUT[package_version]}" "v0.5.3-beta1"
expect_eq "ipk_version" "${OUT[ipk_version]}" "v0.5.3~beta1"
expect_order "${OUT[ipk_version]}" lt "$RC1_IPK"
expect_order "${OUT[ipk_version]}" gt v0.5.2
echo "ipk: branch refs/heads/maint"
derive ipk refs/heads/maint package_version ipk_version
expect_eq "package_version" "${OUT[package_version]}" "maint.${HEIGHT}.${HASH}"
expect_eq "ipk_version" "${OUT[ipk_version]}" "${OUT[package_version]}"
echo "ipk: wiring"
# shellcheck disable=SC2016 # the ${{ }} expressions are workflow text, not shell
expect_text -x "$WORK/ipk.outputs" \
'ipk_version=${{ steps.version.outputs.ipk_version }}' \
"determine-versioning declares ipk_version from the derivation step"
# shellcheck disable=SC2016
expect_text -x "$WORK/ipk.build.sh" \
'IPK_VERSION: ${{ needs.determine-versioning.outputs.ipk_version }}' \
"the .ipk build passes ipk_version as IPK_VERSION"
echo
if [ "$FAILED" -eq 0 ]; then
echo "check-package-versions: all checks passed"
exit 0
fi
echo "check-package-versions: $FAILED check(s) failed"
exit 1
+144
View File
@@ -0,0 +1,144 @@
#!/bin/bash
# ── OpenWrt shell-script lint guard ─────────────────────────────────────────
# Runs shellcheck over the shell scripts the OpenWrt packages ship, and over
# .github/scripts/install-nak.sh, which the OpenWrt Package workflow runs to
# fetch its publishing tool.
#
# This is the one copy of that lint. The OpenWrt Package workflow calls it
# after building each .ipk, and ci.yml and ci-local.sh call it too, because
# that workflow runs only on trunk pushes, tags and pull requests: without the
# other two, a finding in a script edited on a topic branch first shows up
# after the branch has reached a trunk.
#
# What is checked, and how:
# * Every script under packaging/openwrt-ipk/files/ and the maintainer
# scripts under packaging/openwrt-ipk/scripts/, as POSIX sh. Both the .ipk
# and the .apk package take their payload and maintainer scripts from these
# two directories (build-apk.sh wraps the maintainer scripts with one
# header line, which is not linted separately); the SDK feed Makefile ships
# preinst as well. On a router they run under busybox ash.
# * install-nak.sh as bash, with no exclusions. It is a CI script, not a
# shipped one, and the sh exclusion set below misfires on bash.
#
# The sh exclusions, with reason:
# SC1008 the init scripts' `#!/bin/sh /etc/rc.common` shebang, an
# interpreter line the linter does not recognise.
# SC2317 rc.common's start_service/stop_service/reload_service hooks,
# which nothing in the file itself calls.
# SC2034 the init scripts' USE_PROCD, START, STOP, EXTRA_COMMANDS and
# EXTRA_HELP, which rc.common reads rather than the script.
# SC3043 `local`, which POSIX leaves undefined and ash supports.
# SC2086, SC2089, SC2090 firewall.sh builds an nft match, quotes included,
# in one variable and relies on word splitting to pass it as
# separate arguments; nft parses the quotes itself.
#
# Every sh-family script in those two directories must be on the list below. A
# new one that is not fails the guard, so a script added to the package is not
# silently left unlinted.
#
# Exit 0 = clean. Exit 1 = a finding, a listed script missing, or a shipped
# script not on the list. Exit 2 = the guard could not run (shellcheck or git
# missing, or shellcheck could not process a file); never treated as a pass.
# ─────────────────────────────────────────────────────────────────────────────
set -uo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
cd "$PROJECT_ROOT" || { echo "check-shellcheck: cannot cd to $PROJECT_ROOT" >&2; exit 2; }
IPK=packaging/openwrt-ipk
SH_EXCLUDE=SC1008,SC2317,SC2034,SC3043,SC2086,SC2089,SC2090
SH_TARGETS=(
"$IPK/files/etc/init.d/fips"
"$IPK/files/etc/init.d/fips-gateway"
"$IPK/files/etc/fips/firewall.sh"
"$IPK/files/etc/hotplug.d/net/99-fips"
"$IPK/files/etc/uci-defaults/90-fips-setup"
"$IPK/files/usr/bin/fips-mesh-setup"
"$IPK/files/usr/bin/fips-ap-setup"
"$IPK/scripts/preinst"
"$IPK/scripts/postinst"
"$IPK/scripts/prerm"
)
BASH_TARGETS=(
".github/scripts/install-nak.sh"
)
if ! command -v shellcheck >/dev/null 2>&1; then
echo "check-shellcheck: shellcheck not found; cannot lint the shell scripts" >&2
echo "check-shellcheck: install it with 'apt-get install shellcheck'" >&2
exit 2
fi
if ! command -v git >/dev/null 2>&1; then
echo "check-shellcheck: git not found; cannot list the shipped scripts" >&2
exit 2
fi
shellcheck --version | sed -n 's/^version: /check-shellcheck: shellcheck /p'
findings=0
broken=0
# ── Completeness: every shipped sh-family script is on the list ─────────────
shipped=$(git ls-files -- "$IPK/files" "$IPK/scripts") || {
echo "check-shellcheck: git ls-files failed, refusing to pass" >&2
exit 2
}
if [[ -z "$shipped" ]]; then
echo "check-shellcheck: git ls-files found nothing under $IPK, refusing to pass" >&2
exit 2
fi
while IFS= read -r f; do
# A tracked file deleted from the working tree: if listed, the lint below
# reports it missing; if not, there is nothing to ship.
[[ -f "$f" ]] || continue
head -n 1 "$f" | grep -qE '^#![[:space:]]*[^[:space:]]*/(env[[:space:]]+)?(ba|a|da)?sh([[:space:]]|$)' || continue
listed=0
for t in "${SH_TARGETS[@]}"; do
[[ "$t" == "$f" ]] && { listed=1; break; }
done
if [[ $listed -eq 0 ]]; then
echo "FAIL: $f is a shipped shell script missing from SH_TARGETS in $0"
findings=1
fi
done <<< "$shipped"
# ── Lint ─────────────────────────────────────────────────────────────────────
lint() {
# lint <file> <shellcheck args...>: one file; sets findings or broken.
local f="$1" rc=0
shift
if [[ ! -f "$f" ]]; then
echo "FAIL: missing $f"
findings=1
return 0
fi
echo "==> shellcheck $* $f"
shellcheck "$@" "$f" || rc=$?
case $rc in
0) echo " PASS" ;;
1) echo " FAIL"; findings=1 ;;
*) echo " shellcheck exited $rc: could not check $f"; broken=1 ;;
esac
return 0
}
for f in "${SH_TARGETS[@]}"; do
lint "$f" --shell=sh --exclude="$SH_EXCLUDE"
done
for f in "${BASH_TARGETS[@]}"; do
lint "$f" --shell=bash
done
total=$(( ${#SH_TARGETS[@]} + ${#BASH_TARGETS[@]} ))
if [[ $broken -ne 0 ]]; then
echo "shellcheck could not check every script; refusing to pass"
exit 2
fi
if [[ $findings -ne 0 ]]; then
echo "shellcheck FAILED"
exit 1
fi
echo "shellcheck PASS ($total scripts: ${#SH_TARGETS[@]} as sh, ${#BASH_TARGETS[@]} as bash)"
exit 0
+34 -9
View File
@@ -1759,6 +1759,17 @@ run_portable_atomics() {
record "portable-atomics" $rc
}
# The shell scripts the OpenWrt packages ship, and the nak installer. The
# OpenWrt Package workflow lints them on GitHub, but only for trunk pushes,
# tags and pull requests, so this is where a branch first sees a finding.
# Mirrored in ci.yml's ci-parity job by hand. Static, about a second.
run_shellcheck() {
local rc=0
info "[shellcheck] Linting the OpenWrt package's shell scripts"
bash "$SCRIPT_DIR/check-shellcheck.sh" || rc=$?
record "shellcheck" $rc
}
# Every daemon log string a test matches on must still be emitted by src/.
# A stale one does not fail — it stops observing, and an expect-zero assertion
# built on it then passes for the wrong reason.
@@ -1798,15 +1809,16 @@ run_wait_converge() {
record "wait-converge" $rc
}
# The .deb version package-linux.yml derives for a release tag, a candidate
# tag and a branch. A candidate must sort below its release under dpkg, which
# the tag's -rcN does not, so the workflow maps it to ~rcN; this runs the
# workflow's own step text. Static, about a second, and needs nothing built.
run_deb_version() {
# The package versions the packaging workflows derive for a release tag, a
# candidate tag and a branch. A candidate must sort below its release under the
# package manager, which the tag's -rcN does not, so the workflows map it to
# ~rcN; this runs the workflows' own step text. Static, a few seconds, and
# needs nothing built.
run_package_versions() {
local rc=0
info "[deb-version] Checking the Debian version derived for tags and branches"
bash "$SCRIPT_DIR/check-deb-version.sh" || rc=$?
record "deb-version" $rc
info "[package-versions] Checking the package versions derived for tags and branches"
bash "$SCRIPT_DIR/check-package-versions.sh" || rc=$?
record "package-versions" $rc
}
# The GitHub unit-test jobs run check-nextest-flaky.sh after nextest to
@@ -1821,6 +1833,17 @@ run_nextest_flaky() {
record "nextest-flaky" $rc
}
# check-glibc-floor.sh's cases: an input it cannot examine reports "could not
# check" with exit 2, and only a binary above the floor gets exit 1 and the
# rebuild advice. Static, a few seconds, and builds its inputs from the host's
# own true executable.
run_glibc_floor() {
local rc=0
info "[glibc-floor] Checking the glibc floor check against its cases"
bash "$SCRIPT_DIR/glibc-floor/test.sh" || rc=$?
record "glibc-floor" $rc
}
# ── Main ───────────────────────────────────────────────────────────────────
main() {
@@ -1842,9 +1865,11 @@ main() {
run_action_pins
run_comment_refs
run_portable_atomics
run_shellcheck
run_wait_converge
run_deb_version
run_package_versions
run_nextest_flaky
run_glibc_floor
if [[ "$TEST_ONLY" == true ]]; then
run_tests
+8 -8
View File
@@ -15,7 +15,8 @@
#
# This is the most thorough test surface — it exercises:
# - cargo deb packaging (binary stripping, dependency declaration)
# - dpkg conffile placement (/etc/fips/fips.yaml)
# - dpkg conffile placement (/etc/fips/fips.nft) and postinst seeding
# of /etc/fips/fips.yaml
# - postinst maintainer scripts (systemd unit enablement,
# fips-dns.service running fips-dns-setup)
# - postrm purge (removing the DNS routing fips-dns-setup wrote)
@@ -141,8 +142,6 @@ wait_for_systemd() {
return 1
}
# Start a unit without waiting for its start job to finish.
#
# Start a unit and wait for its start job, under a bound.
#
# Blocking is the right default and the call returning is what synchronises the
@@ -194,8 +193,9 @@ container_systemd_version() {
}
# ─────────────────────────────────────────────────────────────────────
# Build the .deb once in a Debian 12 cargo-deb builder image (cached
# between runs). Output cached at testing/deb-install/.cache/deb/.
# Build the .deb once through packaging/debian/build-deb-container.sh,
# whose image is set in packaging/build-floor.env (cached between
# runs). Output cached at testing/deb-install/.cache/deb/.
# Rebuilt if any source/Cargo/packaging file is newer than the cached
# .deb, or if the .deb is missing.
# ─────────────────────────────────────────────────────────────────────
@@ -542,9 +542,9 @@ DOCKERFILE
fail "/usr/bin/fips-gateway missing"
fi
if docker exec "$name" test -f /etc/fips/fips.yaml; then
pass "/etc/fips/fips.yaml conffile installed"
pass "/etc/fips/fips.yaml seeded by postinst"
else
fail "/etc/fips/fips.yaml conffile missing"
fail "/etc/fips/fips.yaml missing"
fi
# Verify fips.service is enabled (postinst enables but does not
@@ -670,7 +670,7 @@ DOCKERFILE
fi
# Get the daemon's npub via fipsctl. Works for both ephemeral
# and persistent identity (no need to override the conffile).
# and persistent identity (no need to override /etc/fips/fips.yaml).
local npub
npub=$(docker exec "$name" fipsctl show status 2>/dev/null \
| grep -oE 'npub1[a-z0-9]+' | head -1)
+195
View File
@@ -0,0 +1,195 @@
#!/bin/bash
# ── Cases for check-glibc-floor.sh ──────────────────────────────────────────
# The floor check runs at release time, on artifacts nobody re-reads, so how it
# reports matters as much as what it finds. An input it cannot examine (a
# missing path, a file that is not an ELF object such as an unextracted
# tarball, a corrupt or binary-less .deb) must come out as "could not check"
# with exit 2, never as a binary above the floor: that advice sends the reader
# to rebuild a package that was never examined. A binary that really is above
# the floor must still exit 1 with the advice, even beside unchecked inputs. A
# floor that is missing or empty means the check cannot run at all, which is
# exit 2 as well.
#
# Inputs are built at run time from the host's own true executable (the first
# one on PATH). Every case either sets FIPS_GLIBC_FLOOR or runs a scratch copy
# of the check beside a build-floor.env of its own, so nothing depends on the
# host's glibc or on the repository's packaging/build-floor.env.
#
# Exit 0 = every case behaved. Exit 1 = a case did not. Exit 2 = the cases
# could not run (no readelf, no dpkg-deb, no dynamic true); never a pass.
# ─────────────────────────────────────────────────────────────────────────────
set -uo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
CHECK="$SCRIPT_DIR/../check-glibc-floor.sh"
WORK="$(mktemp -d)"
trap 'chmod -R u+rwx "$WORK" 2>/dev/null; rm -rf "$WORK"' EXIT
cannot() { echo "glibc-floor cases: $*; cannot run." >&2; exit 2; }
for tool in readelf dpkg dpkg-deb tar; do
command -v "$tool" >/dev/null 2>&1 || cannot "$tool is not installed"
done
# `true` is a shell builtin, so `command -v` would print the bare word.
HOSTELF="$(type -P true)" || cannot "no true executable on PATH"
readelf -hW "$HOSTELF" >/dev/null 2>&1 || cannot "$HOSTELF is not an ELF object"
NEED="$(readelf -VW "$HOSTELF" 2>/dev/null | awk '/Version needs section/,0' \
| grep -oE 'GLIBC_[0-9.]+' | sed 's/GLIBC_//' | sort -V | tail -1)"
[ -n "$NEED" ] || cannot "$HOSTELF has no glibc version requirement"
dpkg --compare-versions "$NEED" gt 2.0 || cannot "$HOSTELF needs glibc $NEED, not above 2.0"
FAILED=0
ok() { echo " ok $*"; }
bad() { echo " FAIL $*"; FAILED=$((FAILED + 1)); }
ADVICE='installs cleanly and then fails to start'
# run_script <script> <name> <floor> <arg...>: runs the given copy of the
# check, leaving combined output in $WORK/out and the status in RC.
run_script() {
local script="$1"
CASE="$2"
local floor="$3"
shift 3
FIPS_GLIBC_FLOOR="$floor" bash "$script" "$@" > "$WORK/out" 2>&1
RC=$?
return 0
}
# run_case <name> <floor> <arg...>: runs the check under test.
run_case() { run_script "$CHECK" "$@"; }
# floor_tree <name> [env contents]: a scratch copy of the check at
# $WORK/<name>/testing, so it reads its floor from $WORK/<name>/packaging. With
# no contents there is no packaging/build-floor.env at all. Prints the copy.
floor_tree() {
local root="$WORK/$1"
mkdir -p "$root/testing"
cp "$CHECK" "$root/testing/check-glibc-floor.sh"
if [ $# -gt 1 ]; then
mkdir -p "$root/packaging"
printf '%s\n' "$2" > "$root/packaging/build-floor.env"
fi
printf '%s\n' "$root/testing/check-glibc-floor.sh"
}
# Assertions on the last run.
exit_is() {
if [ "$RC" -eq "$1" ]; then ok "$CASE: exit $1"
else bad "$CASE: exit $RC, expected $1"; sed 's/^/ | /' "$WORK/out"; fi
}
has() {
if grep -qF -- "$1" "$WORK/out"; then ok "$CASE: says '$1'"
else bad "$CASE: does not say '$1'"; fi
}
lacks() {
if grep -qF -- "$1" "$WORK/out"; then bad "$CASE: says '$1'"
else ok "$CASE: does not say '$1'"; fi
}
# unchecked_lists <name>: the name appears on an UNCHECKED line where it was
# met, and again in the "Not checked:" list at the end of the run, which is the
# part a reader of a long run sees.
unchecked_lists() {
if grep -E '^\s*UNCHECKED ' "$WORK/out" | grep -qF -- "$1"; then
ok "$CASE: reports $1 as not checked"
else
bad "$CASE: does not report $1 as not checked"
fi
if sed -n '/^ Not checked:$/,$p' "$WORK/out" | grep -qF -- "$1"; then
ok "$CASE: lists $1 at the end of the run"
else
bad "$CASE: does not list $1 at the end of the run"
fi
}
# no_advice: the above-the-floor advice is absent.
no_advice() { lacks "$ADVICE"; }
# build_deb <name> <file to place in usr/bin>
build_deb() {
local name="$1" payload="$2" root="$WORK/pkg-$1"
mkdir -p "$root/DEBIAN" "$root/usr/bin"
cp "$payload" "$root/usr/bin/"
chmod 755 "$root/usr/bin/"*
printf 'Package: %s\nVersion: 1.0\nArchitecture: all\nMaintainer: t <t@t>\nDescription: t\n' \
"$name" > "$root/DEBIAN/control"
dpkg-deb --root-owner-group -b "$root" "$WORK/$name.deb" >/dev/null 2>&1 \
|| cannot "dpkg-deb could not build a test package"
}
echo "check-glibc-floor cases ($HOSTELF needs glibc $NEED)"
printf 'not an ELF object\n' > "$WORK/notes.txt"
mkdir -p "$WORK/tarsrc" && cp "$HOSTELF" "$WORK/tarsrc/true"
tar -czf "$WORK/fips.tar.gz" -C "$WORK/tarsrc" true
printf 'not a package\n' > "$WORK/x.deb"
printf '#!/bin/sh\nexit 0\n' > "$WORK/setup-script"
build_deb scripts-only "$WORK/setup-script"
build_deb with-elf "$HOSTELF"
run_case "text file" "$NEED" "$WORK/notes.txt"
exit_is 2; has "could not check"; unchecked_lists notes.txt; no_advice
run_case "tarball" "$NEED" "$WORK/fips.tar.gz"
exit_is 2; has "unpack it"; unchecked_lists fips.tar.gz; no_advice
run_case "nonexistent path" "$NEED" "$WORK/missing-binary"
exit_is 2; has "does not exist"; unchecked_lists missing-binary; no_advice
if [ "$(id -u)" -eq 0 ]; then
echo " SKIP unreadable file: running as root, so permissions do not stop a read"
else
cp "$HOSTELF" "$WORK/locked" && chmod 000 "$WORK/locked"
run_case "unreadable file" "$NEED" "$WORK/locked"
exit_is 2; unchecked_lists locked; no_advice
fi
run_case "corrupt .deb then a good binary" "$NEED" "$WORK/x.deb" "$HOSTELF"
exit_is 2; has "could not check"; unchecked_lists x.deb; no_advice
has "ok $(basename "$HOSTELF") needs glibc $NEED"
run_case ".deb with only a script" "$NEED" "$WORK/scripts-only.deb"
exit_is 2; has "no ELF executables"; unchecked_lists scripts-only.deb; no_advice
run_case "binary at the floor" "$NEED" "$HOSTELF"
exit_is 0; has "glibc floor check passed"; lacks "could not check"; no_advice
run_case ".deb with a binary at the floor" "$NEED" "$WORK/with-elf.deb"
exit_is 0; has "glibc floor check passed"; no_advice
run_case "binary above the floor" 2.0 "$HOSTELF"
exit_is 1; has "above the declared floor 2.0"; has "$ADVICE"
run_case "binary above the floor beside a text file" 2.0 "$HOSTELF" "$WORK/notes.txt"
exit_is 1; has "$ADVICE"; unchecked_lists notes.txt
run_case "binary at the floor beside a text file" "$NEED" "$HOSTELF" "$WORK/notes.txt"
exit_is 2; has "ok $(basename "$HOSTELF") needs glibc $NEED"; unchecked_lists notes.txt
no_advice
# With FIPS_GLIBC_FLOOR empty the floor comes from packaging/build-floor.env.
# When that cannot supply one, there is nothing to check against: that is
# "could not run" (exit 2), never a pass and never a binary above the floor.
NOENV="$(floor_tree no-env)"
run_script "$NOENV" "no build-floor.env and no floor set" "" "$HOSTELF"
exit_is 2; has "no floor to check against"; lacks "glibc floor check passed"
no_advice
EMPTYENV="$(floor_tree empty-env 'FIPS_GLIBC_FLOOR=""')"
run_script "$EMPTYENV" "build-floor.env with an empty floor" "" "$HOSTELF"
exit_is 2; has "no floor to check against"; lacks "glibc floor check passed"
no_advice
NOFLOOR="$(floor_tree no-floor 'FIPS_BUILD_IMAGE="ubuntu:22.04"')"
run_script "$NOFLOOR" "build-floor.env declaring no floor" "" "$HOSTELF"
exit_is 2; has "no floor to check against"; lacks "glibc floor check passed"
no_advice
GOODENV="$(floor_tree good-env "FIPS_GLIBC_FLOOR=\"$NEED\"")"
run_script "$GOODENV" "floor read from build-floor.env" "" "$HOSTELF"
exit_is 0; has "declared floor: $NEED"; has "glibc floor check passed"
if [ "$FAILED" -ne 0 ]; then
echo "check-glibc-floor cases: $FAILED assertion(s) failed"
exit 1
fi
echo "check-glibc-floor cases: all passed"
+3 -1
View File
@@ -7,7 +7,9 @@ connection owns nothing — a flow lives until its own descriptor is closed, and
listener until its own is — so the single connection is a convenience for the
checks rather than a lifetime the daemon respects. Descriptors are what keep
things alive, and this tool holds them until the step that closes them or until
it exits.
it exits. The daemon does keep its own copy of the descriptor in its last reply
until the next command arrives, so a check that closes one must send a command
before it expects the close to have taken effect.
Kinds of step:
+20 -6
View File
@@ -695,12 +695,20 @@ check_backlog_is_not_the_clients_bound() {
}
check_refusing_a_flow_frees_it() {
log "Refusing an accepted flow is closing its descriptor, and that frees it"
log "Refusing an accepted flow is closing its descriptor, and its listener's close frees it"
# There is no reject command: Berkeley has exactly one way to refuse a
# connection and so does this. Keeping a second would let a client refuse a
# flow two indistinguishable ways.
#
# Two things have to follow the close, and the second is what makes this
# The daemon keeps its own copy of an accepted flow's descriptor until the
# client writes on the flow or closes the listener, because on macOS the
# kernel can destroy a socket whose descriptor is still in an unread
# arrival. So a flow refused without a write outlives its close, and goes
# when the listener does. Both halves are asserted: a flow freed at the
# close would mean the daemon let its copy go early, and one that outlived
# the listener would be a leak.
#
# Two things have to follow the release, and the second is what makes this
# more than a repeat of the connected-flow close: the node forgets the flow,
# and the registry entry and its key go with it, so the very same key
# announces a new flow afterwards rather than delivering into the dead one.
@@ -713,16 +721,22 @@ check_refusing_a_flow_frees_it() {
{"command":"stats","params":{"flow_id":"@a"},"expect":{"status":"ok"}},
{"fd":"a","close":true},
{"sleep":1},
{"command":"stats","params":{"flow_id":"@a"},"expect":{"status":"error"}},
{"command":"stats","params":{"flow_id":"@a"},
"expect":{"status":"ok","data.closed":false}},
{"fd":"L","close":true},
{"command":"stats","params":{"flow_id":"@a"},"settle":true,
"expect":{"status":"error"}},
{"command":"listen","params":{"local_port":4304},"keep_listener":"M",
"expect":{"status":"ok"}},
{"command":"arrive","params":{"peer":"'"$PEER"'","src_port":5000,"dst_port":4304,"data":"bb"},
"expect":{"status":"ok","data.outcome":"announced"}},
{"accept":"L","keep_fd":"b","expect":{"local_port":4304,"remote_port":5000}},
{"accept":"M","keep_fd":"b","expect":{"local_port":4304,"remote_port":5000}},
{"fd":"b","read":1,"expect_bytes":"bb"}
]'
if run_client "$script"; then
pass "a refused flow is gone and its key is free to arrive again"
pass "a refused flow lasts until its listener closes, then is gone and its key is free to arrive again"
else
fail "closing a refused flow did not release it"
fail "a refused flow did not last until its listener closed, or was not released then"
fi
}
+42 -2
View File
@@ -47,7 +47,7 @@ fi
PKG_VERSION="pkgtest.$$"
TMP="$(mktemp -d)" || { echo "package-test: mktemp failed" >&2; exit 2; }
trap 'rm -rf "$TMP"; rm -f "$PROJECT_ROOT/dist/fips_${PKG_VERSION}_"*' EXIT
trap 'rm -rf "$TMP"; rm -f "$PROJECT_ROOT/dist/fips_${PKG_VERSION}"[_~]*' EXIT
harness_fail() {
echo "package-test: $*" >&2
@@ -227,7 +227,9 @@ done
# ── Build the .ipk ──────────────────────────────────────────────────────────
echo "==> build-ipk.sh"
if ! PKG_VERSION="$PKG_VERSION" \
# The first build is V1's "IPK_VERSION unset" case, so it must not inherit an
# IPK_VERSION from the caller's environment.
if ! env -u IPK_VERSION PKG_VERSION="$PKG_VERSION" \
bash "$PROJECT_ROOT/packaging/openwrt-ipk/build-ipk.sh" --arch x86_64 --bin-dir "$BINS" \
> "$TMP/build-ipk.log" 2>&1; then
cat "$TMP/build-ipk.log" >&2
@@ -240,6 +242,44 @@ tar -xzf "$IPK" -O ./data.tar.gz | tar -tzf - > "$TMP/ipk-data" \
tar -xzf "$IPK" -O ./control.tar.gz | tar -tzf - > "$TMP/ipk-control" \
|| harness_fail "cannot list control.tar.gz in $IPK"
# Print the Version field of an .ipk's control file.
ipk_version() {
tar -xzf "$1" -O ./control.tar.gz | tar -xzOf - ./control \
| sed -n 's/^Version: //p'
}
# ── V1 and V2. The control Version comes from IPK_VERSION ───────────────────
# The workflow passes a candidate's opkg-sortable version (vX.Y.Z~rcN) as
# IPK_VERSION and keeps the tag's form in PKG_VERSION for the file name, since
# a GitHub release renames an asset with '~' in its name. Unset, IPK_VERSION
# falls back to PKG_VERSION, as a local build expects.
got="$(ipk_version "$IPK")" || harness_fail "cannot read the control file in $IPK"
if [[ "$got" == "$PKG_VERSION" ]]; then
ok "V1 with IPK_VERSION unset the control Version is PKG_VERSION ($got)"
else
bad "V1 with IPK_VERSION unset the control Version is '$got', want $PKG_VERSION"
fi
# The first build's package is removed so a second build that names its file
# from anything but PKG_VERSION cannot be passed by reading the stale one.
RC_VERSION="${PKG_VERSION}~rc1"
rm -f "$IPK" || harness_fail "cannot remove $IPK before the second build"
if ! PKG_VERSION="$PKG_VERSION" IPK_VERSION="$RC_VERSION" \
bash "$PROJECT_ROOT/packaging/openwrt-ipk/build-ipk.sh" --arch x86_64 --bin-dir "$BINS" \
> "$TMP/build-ipk-rc.log" 2>&1; then
cat "$TMP/build-ipk-rc.log" >&2
harness_fail "build-ipk.sh failed with IPK_VERSION set"
fi
if [[ ! -f "$IPK" ]]; then
bad "V2 with IPK_VERSION set build-ipk.sh wrote no $IPK; the file name must come from PKG_VERSION"
else
got="$(ipk_version "$IPK")" || harness_fail "cannot read the control file in $IPK"
if [[ "$got" == "$RC_VERSION" ]]; then
ok "V2 with IPK_VERSION set the control Version is IPK_VERSION ($got), file named from PKG_VERSION"
else
bad "V2 with IPK_VERSION set the control Version is '$got', want $RC_VERSION"
fi
fi
# ── P1 and P2. Neither package ships the dnsmasq drop-in ────────────────────
# OpenWrt's dnsmasq builds its config from UCI and reads no directory under
# /etc, so .fips forwarding comes from the UCI entry 90-fips-setup adds. The