diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..3c13b858 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,77 @@ +name: Release Object +on: + push: + tags: + - "v*" + workflow_dispatch: + inputs: + tag: + description: "Tag to create the Release object for" + required: true + +# Every packaging workflow's upload job runs a `Wait for tag release` step that +# polls `gh release view` 20 times at 15s intervals and then fails the job. Five +# waiters and no creator means a tag push fails all five unless a human runs +# `gh release create` inside that five-minute budget. This job is the creator. +# +# It deliberately does not live in a packaging workflow. The previous creator did +# (package-openwrt.yml, via an action with generate_release_notes), and it raced +# the operator's hand-created object for the right to decide the Release body; +# whichever lost left the written notes stranded. Here there is one creator, it +# publishes the notes file the release wrote, and it is idempotent, so running it +# alongside a hand-created object is a no-op rather than a race. + +permissions: + contents: read + +concurrency: + group: release-object-${{ github.ref }} + +jobs: + create-release: + name: Create the Release object + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + ref: ${{ inputs.tag || github.ref }} + + - name: Create the Release object if it does not exist + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ inputs.tag || github.ref_name }} + run: | + set -euo pipefail + + if gh release view "${TAG}" --repo "${GITHUB_REPOSITORY}" >/dev/null 2>&1; then + echo "Release ${TAG} already exists; leaving it untouched." + exit 0 + fi + + args=(--repo "${GITHUB_REPOSITORY}" --title "FIPS ${TAG}") + + # A tag carrying a pre-release suffix (v0.5.0-rc1) is an RC: it is + # marked as a pre-release and has no written notes of its own, since + # the notes are finalized for the stable tag. + notes="docs/releases/release-notes-${TAG}.md" + if [[ "${TAG}" == *-* ]]; then + args+=(--prerelease --notes "Release candidate - packaging validation only.") + elif [[ -f "${notes}" ]]; then + args+=(--notes-file "${notes}") + else + # Never --generate-notes: an auto-generated commit list published + # under a stable tag is exactly what stranded the written notes + # before. An empty body is visibly wrong and trivially editable. + echo "::warning::${notes} is missing; publishing ${TAG} with an empty body." + args+=(--notes "") + fi + + if ! gh release create "${TAG}" "${args[@]}"; then + # A concurrent creator - the operator by hand, or a re-run - may have + # won between the check above and here. An object that now exists is + # the outcome this job wanted. + gh release view "${TAG}" --repo "${GITHUB_REPOSITORY}" >/dev/null 2>&1 + echo "Release ${TAG} was created concurrently; nothing to do." + fi diff --git a/CHANGELOG.md b/CHANGELOG.md index b114365a..14be01ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -521,6 +521,26 @@ with v0.4.x or earlier peers. #### Packaging & deployment +- A NixOS module and an overlay are exposed from the flake, so a flake consumer + enables the daemon with one line rather than hand-rolling a systemd unit. + `overlays.default` adds `pkgs.fips`, and `nixosModules.default` provides + `services.fips.*`: `enable`, `package`, `configFile`, `openFirewall` (UDP 2121 + and TCP 8443) and `dns.enable`, which routes `.fips` to `[::1]:5354` through + systemd-resolved declaratively rather than with setup and teardown scripts. + `packaging/nixos/README.md` documents it with a full consumer `flake.nix`. + Contributed by Arjen. **Unexercised here**: no CI job builds the flake and no + Nix toolchain is present on the machine this release was assembled on, so the + module is untested outside its author's environment. + +- `fipsctl address [npub|hostname]` prints a node's `fd00::/8` mesh address and + nothing else, without contacting the daemon. With no argument it derives the + local node's address from `fips.key` in the default key directory, falling + back to the world-readable `fips.pub` beside it; `--key PATH` names a key or + public key file elsewhere. This lets an installer or image build write a mesh + address into a config file at a point where no node is running and none can + be, and keeps the derivation in one place rather than reimplemented by + whatever needs it. + - `packaging/debian/build-deb.sh --features ` builds the `.deb` with a Cargo feature list, which is how an instrumented package is produced for a measurement run. The auto-derived dev Version gains a matching `+` @@ -729,6 +749,36 @@ with v0.4.x or earlier peers. ### Fixed +#### Peer and link state + +- A peer that goes away no longer leaves its path-MTU state behind. The record + of which transport link-seeded a destination's path MTU had no removal site + on any lifecycle, so the map grew one entry per peer this node had ever + linked with and held them for the life of the process. Both the stored value + and its seeding record are now released when the path they describe goes, + together rather than separately: releasing the record alone would leave the + value with nothing recording where it came from, and a peer returning over a + wider transport would then be refused by the never-loosen rule indefinitely. + A peer whose link is still up has both written straight back by the reseed + that already follows. + +- Removing a link now clears every reverse-lookup key it was inserted under. + The removal rebuilt one key from the link's own remote address, so an entry + inserted under a different address form — which the cross-connection + promotion arms do, keying on the packet's source address — outlived the link + it named. An entry a newer link has since claimed is still left alone. + +- A rejected handshake no longer strands a session index or a retry schedule. + Two reject arms freed neither, which the `next`-line versions already do; + master now carries the same shape, so an ACL-rejected outbound dial is + rescheduled and an abandoned rekey index is returned rather than held. + +#### Transport + +- The UDP listen socket's address-reuse flags are now set before its bind + rather than after, where they had no effect on the socket they were meant to + configure. + #### Control socket - `disconnect` on the control socket now closes the transport connection diff --git a/docs/reference/cli-fipsctl.md b/docs/reference/cli-fipsctl.md index ffa26936..e90c0b9f 100644 --- a/docs/reference/cli-fipsctl.md +++ b/docs/reference/cli-fipsctl.md @@ -16,8 +16,8 @@ one JSON request, and pretty-prints the response. Exits with a non-zero status if the socket cannot be reached, the daemon returns an error, or the request times out. -`fipsctl keygen` is a special case: it does not contact the daemon and -operates purely on local files. +`fipsctl keygen` and `fipsctl address` are special cases: they do not +contact the daemon and operate purely on local files. For the line-delimited JSON wire protocol, see [control-socket.md](control-socket.md). For the YAML configuration @@ -98,6 +98,30 @@ daemon. on Unix. After running `keygen`, set `node.identity.persistent: true` in `fips.yaml` or the daemon will overwrite the keys on next start. +### `address [identity] [options]` + +Print a node's mesh address (`fd00::/8`) and nothing else, so it can be +captured in a shell substitution. Does not contact the daemon, which +makes it usable from an installer or image build where no node is +running. + +| Argument | Description | +| -------- | ----------- | +| `identity` | npub (bech32) or hostname from `/etc/fips/hosts`. Omit to use this node's own identity. | + +| Flag | Argument | Default | Description | +| ---- | -------- | ------- | ----------- | +| `-k`, `--key` | `PATH` | *(none)* | Derive from this key file (an `nsec`) or public key file (an `npub`). Conflicts with `identity`. | + +With neither an `identity` nor `--key`, the address comes from +`fips.key` in the default key directory (the same directory `keygen` +writes to), falling back to `fips.pub` beside it, since `fips.key` is +mode `0600` and an unprivileged run cannot read it. + +The address is derived from the public key exactly as the daemon +derives its own: `fd` followed by the first 15 bytes of +SHA-256(pubkey). + ### `connect
` Tell the daemon to dial a peer over a specific transport. diff --git a/src/bin/fipsctl.rs b/src/bin/fipsctl.rs index 8fb7fe3e..3159c910 100644 --- a/src/bin/fipsctl.rs +++ b/src/bin/fipsctl.rs @@ -7,10 +7,10 @@ //! On Windows, uses a TCP connection to localhost. use clap::{Parser, Subcommand}; -use fips::config::{write_key_file, write_pub_file}; +use fips::config::{read_key_file, write_key_file, write_pub_file}; use fips::upper::hosts::HostMap; use fips::version; -use fips::{Identity, encode_nsec}; +use fips::{ConfigError, Identity, PeerIdentity, encode_nsec}; use std::io::{BufRead, BufReader, IsTerminal, Write}; use std::net::{Ipv6Addr, SocketAddrV6}; use std::path::{Path, PathBuf}; @@ -58,6 +58,15 @@ enum Commands { #[arg(short = 's', long = "stdout")] stdout: bool, }, + /// Print a node's mesh address, without contacting the daemon + Address { + /// npub (bech32) or hostname from /etc/fips/hosts. Defaults to this + /// node's own identity, read from its key files. + identity: Option, + /// Derive from this key file (an nsec) or public key file (an npub) + #[arg(short = 'k', long = "key", conflicts_with = "identity")] + key: Option, + }, /// Connect to a peer Connect { /// Peer identifier: npub (bech32) or hostname from /etc/fips/hosts @@ -423,6 +432,63 @@ fn resolve_peer(peer: &str) -> String { } } +/// Derive the mesh address for whichever identity the arguments name. +/// +/// Precedence is the order the arguments are documented in: an explicit npub +/// or hostname, then an explicit key file, then this node's own key files in +/// the default key directory. Nothing here touches the control socket, so the +/// address is available to a maintainer script with no daemon running. +fn mesh_address(identity: Option<&str>, key: Option<&Path>) -> Result { + match (identity, key) { + (Some(peer), _) => address_from_npub(&resolve_peer(peer)), + (None, Some(path)) => address_from_file(path), + (None, None) => address_from_key_dir(&default_key_dir()), + } +} + +/// Derive a mesh address from a bech32 npub. +fn address_from_npub(npub: &str) -> Result { + let peer = PeerIdentity::from_npub(npub).map_err(|e| format!("invalid npub: {e}"))?; + Ok(peer.address().to_ipv6()) +} + +/// Derive a mesh address from a key file holding an nsec, or from a public +/// key file holding an npub. +/// +/// The file contents are the private key in the first case, so they are held +/// in a guard and cleared on every exit path. +fn address_from_file(path: &Path) -> Result { + let contents = Zeroizing::new(read_key_file(path).map_err(|e| match e { + // ReadFile's own text names the file a config file, which this is not. + ConfigError::ReadFile { source, .. } => { + format!("cannot read {}: {source}", path.display()) + } + other => other.to_string(), + })?); + + if contents.starts_with("npub1") { + return address_from_npub(contents.as_str()); + } + + let identity = Identity::from_secret_str(contents.as_str()) + .map_err(|e| format!("{} does not hold a usable key: {e}", path.display()))?; + Ok(identity.address().to_ipv6()) +} + +/// Derive this node's mesh address from the key files in `dir`. +/// +/// Tries `fips.key` first and falls back to `fips.pub`: the private key is +/// mode 0600, so an unprivileged run can still answer from the world-readable +/// public key beside it. When neither is readable both attempts are reported, +/// since either file would have answered. +fn address_from_key_dir(dir: &Path) -> Result { + match address_from_file(&dir.join("fips.key")) { + Ok(addr) => Ok(addr), + Err(key_err) => address_from_file(&dir.join("fips.pub")) + .map_err(|pub_err| format!("{key_err}\n{pub_err}")), + } +} + fn main() { let cli = Cli::parse(); @@ -492,6 +558,17 @@ fn main() { return; } + if let Commands::Address { identity, key } = &cli.command { + match mesh_address(identity.as_deref(), key.as_deref()) { + Ok(address) => println!("{address}"), + Err(e) => { + eprintln!("error: {e}"); + std::process::exit(1); + } + } + return; + } + let socket_path = cli.socket.unwrap_or_else(default_socket_path); let request = match &cli.command { @@ -566,7 +643,7 @@ fn main() { ProfileTickAction::Status => build_query("profile_tick_status"), }, }, - Commands::Keygen { .. } => unreachable!(), + Commands::Keygen { .. } | Commands::Address { .. } => unreachable!(), }; // For plot output we need to post-process the JSON response rather @@ -1513,6 +1590,85 @@ mod tests { assert_eq!(default_key_dir(), PathBuf::from("/etc/fips")); } + /// Build a key file for `identity` in `dir` and return its path. + fn write_identity_key(dir: &Path, identity: &Identity) -> PathBuf { + let mut keypair = identity.keypair(); + let mut secret_key = keypair.secret_key(); + let nsec = Zeroizing::new(encode_nsec(&secret_key)); + secret_key.non_secure_erase(); + keypair.non_secure_erase(); + let path = dir.join("fips.key"); + write_key_file(&path, &nsec).unwrap(); + path + } + + #[test] + fn the_address_derived_from_an_npub_is_the_one_its_owner_uses() { + let identity = Identity::generate(); + let derived = address_from_npub(&identity.npub()).unwrap(); + assert_eq!(derived, identity.address().to_ipv6()); + assert_eq!(derived.octets()[0], 0xfd); + } + + #[test] + fn a_malformed_npub_is_refused_rather_than_hashed() { + assert!(address_from_npub("npub1notarealkey").is_err()); + assert!(address_from_npub("").is_err()); + } + + #[test] + fn the_address_derived_from_a_key_file_matches_its_npub() { + let dir = tempfile::tempdir().unwrap(); + let identity = Identity::generate(); + let key_path = write_identity_key(dir.path(), &identity); + + let expected = identity.address().to_ipv6(); + assert_eq!(address_from_file(&key_path).unwrap(), expected); + assert_eq!(address_from_key_dir(dir.path()).unwrap(), expected); + assert_eq!( + mesh_address(None, Some(key_path.as_path())).unwrap(), + expected + ); + } + + #[test] + fn the_public_key_file_answers_when_the_private_one_is_absent() { + let dir = tempfile::tempdir().unwrap(); + let identity = Identity::generate(); + write_pub_file(&dir.path().join("fips.pub"), &identity.npub()).unwrap(); + + assert_eq!( + address_from_key_dir(dir.path()).unwrap(), + identity.address().to_ipv6() + ); + } + + #[test] + fn a_missing_key_file_reports_every_path_that_was_tried() { + let dir = tempfile::tempdir().unwrap(); + let err = address_from_key_dir(dir.path()).unwrap_err(); + assert!(err.contains("fips.key"), "{err}"); + assert!(err.contains("fips.pub"), "{err}"); + assert!(address_from_file(&dir.path().join("fips.key")).is_err()); + } + + #[test] + fn an_empty_or_unparsable_key_file_is_refused() { + let dir = tempfile::tempdir().unwrap(); + + let empty = dir.path().join("empty.key"); + std::fs::write(&empty, "\n").unwrap(); + assert!(address_from_file(&empty).unwrap_err().contains("empty")); + + let junk = dir.path().join("junk.key"); + std::fs::write(&junk, "not-a-key\n").unwrap(); + assert!( + address_from_file(&junk) + .unwrap_err() + .contains("does not hold a usable key") + ); + } + #[test] fn test_acl_show_command_name() { assert_eq!(AclCommands::Show.command_name(), "show_acl"); diff --git a/src/node/dataplane/dispatch.rs b/src/node/dataplane/dispatch.rs index f63676d1..fd8dc2e4 100644 --- a/src/node/dataplane/dispatch.rs +++ b/src/node/dataplane/dispatch.rs @@ -149,6 +149,16 @@ impl Node { } self.pending_tun_packets.remove(node_addr); + // Release the path MTU state this peer's promotion seeded: the link it + // measured has just gone. The other callers all fire on session state, + // so without this a peer that never opened an FSP session leaves both + // its `path_mtu_lookup` entry and its seeding-transport record behind + // for the life of the process. The peer is already out of `self.peers` + // above, so no reseed follows and the two go together, which is what + // keeps a peer returning over a wider link from being clamped to the + // one it left. + self.path_mtu_lookup_release(node_addr); + let link_id = peer.link_id(); let transport_id = peer.transport_id(); diff --git a/src/node/handlers/handshake.rs b/src/node/handlers/handshake.rs index 04280f77..80a3d74e 100644 --- a/src/node/handlers/handshake.rs +++ b/src/node/handlers/handshake.rs @@ -236,6 +236,12 @@ impl Node { } else { Msg1Class::Stranger }; + // The binding name matters. `_slot` holds the guard until this + // function returns, and that is what releases the pending slot on + // every exit path. Renaming it to a bare `_` drops the guard right + // here instead, releasing the slot at acquire time — silently, with + // no clippy lint catching the difference. Do not "tidy" this + // binding; the assertion below is what reds if it is tidied. let _slot = match self.msg1_rate_limiter.start_handshake(class) { Ok(slot) => slot, Err(reason) => { @@ -249,6 +255,19 @@ impl Node { } }; + // Test-build witness for the paragraph above, and the only thing that + // observes it. A guard released at acquire time leaves this msg1 + // in flight with its slot already back in the pool, which no counter, + // log line or lint reports: by the time any test can look, the count + // has returned to its baseline either way. Sampling it here, on the + // handler's own stack, is what tells the two apart — see + // `msg1_handler_holds_its_pending_slot_while_the_handler_runs`. + #[cfg(test)] + assert!( + self.msg1_rate_limiter.pending_count() > 0, + "the msg1 pending slot was released before the handler ran" + ); + // accept_connections gate. Rekey/restart msg1 on an existing link // is always admitted; the gate only filters truly-fresh connections // from strangers. Without this carve-out, the dual-init tie-breaker @@ -1000,6 +1019,19 @@ impl Node { if let Some(idx) = our_index { let _ = self.index_allocator.free(idx); } + // Put the dial back on the retry schedule. The disposal above takes + // this leg out of the stuck-leg sweep — `has_pending_leg` reads false + // for it from here on, so the reap that normally reaches + // `note_handshake_timeout` never runs — and that reflex is the only + // thing that seeds `retry_pending` for a configured peer. + // `close_connection` only drops the transport's pool entry; it + // schedules nothing. + // + // `peer_identity` is safe to reschedule against here because this is + // an IK dial: the initiator's expected identity is fixed at + // `start_handshake` and `complete_handshake` never overwrites it, so + // it is still the peer we meant to dial and not whoever answered. + self.note_handshake_timeout(*peer_identity.node_addr(), packet.timestamp_ms); self.stats_mut() .record_reject(RejectReason::Handshake(HandshakeReject::BadState)); return; diff --git a/src/node/handlers/timeout.rs b/src/node/handlers/timeout.rs index 21fadb25..273728d0 100644 --- a/src/node/handlers/timeout.rs +++ b/src/node/handlers/timeout.rs @@ -532,12 +532,12 @@ impl Node { /// Expire `path_mtu_lookup` entries that nothing else will ever release. /// - /// The three callers of `path_mtu_lookup_release` all fire on session + /// The callers of `path_mtu_lookup_release` all fire on session or peer /// state, so an entry written by the discovery `LookupResponse` carrier - /// for a destination this node never opens a session with has no release - /// path at all. Keep-tighter then makes one such response permanent: a - /// `path_mtu` of 256 pins that destination's SYN-time MSS clamp at 119 - /// bytes until the process restarts. Only those entries carry a + /// for a destination this node neither peers with nor opens a session + /// with has no release path at all. Keep-tighter then makes one such + /// response permanent: a `path_mtu` of 256 pins that destination's + /// SYN-time MSS clamp at 119 bytes until the process restarts. Only those entries carry a /// `learned_ms`, and only they are expired here. /// /// The deadline is the coordinate cache's own TTL, because the same diff --git a/src/node/mod.rs b/src/node/mod.rs index 124b5a8b..470ec87f 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -404,6 +404,11 @@ pub struct Node { /// value that still describes the current path from one that describes a /// path the peer has left. Absent for destinations reached over multiple /// hops: those are never link-seeded, so never-loosen applies unchanged. + /// + /// An entry lives exactly as long as the `path_mtu_lookup` entry it + /// describes: `path_mtu_lookup_release` drops both together, and peer + /// removal is one of its callers, so a peer that never comes back leaves + /// nothing behind. path_mtu_seeded_by: Arc>>, // === Transports & Links === @@ -2565,20 +2570,21 @@ impl Node { /// Remove a link. /// - /// Only removes the addr_to_link reverse lookup if it still points to this - /// link. In cross-connection scenarios, a newer link may have replaced the - /// entry for the same address. + /// Drops every `addr_to_link` entry that still maps to this link, rather + /// than only the key rebuilt from the link's own remote address. A link can + /// be registered under more than one address form: the cross-connection + /// arms key the winner on the *packet's* source address, which need not + /// equal the winner's own (a hostname against its resolved numeric form, or + /// a different source port on a connection-oriented transport). Rebuilding + /// a single key left those entries naming a link that no longer exists, for + /// as long as the node ran. + /// + /// Entries a newer link has already claimed are left alone, since they no + /// longer name this link. pub fn remove_link(&mut self, link_id: &LinkId) -> Option { - if let Some(link) = self.links.remove(link_id) { - // Clean up reverse lookup only if it still maps to this link - let key = (link.transport_id(), link.remote_addr().clone()); - if self.addr_to_link.get(&key) == Some(link_id) { - self.addr_to_link.remove(&key); - } - Some(link) - } else { - None - } + let link = self.links.remove(link_id)?; + self.addr_to_link.retain(|_, mapped| *mapped != *link_id); + Some(link) } /// Single choke-point for dropping a per-peer control machine. Also drops the @@ -3003,8 +3009,9 @@ impl Node { /// removal would silently drop a direct peer back to the conservative /// ceiling until its link re-handshakes. /// - /// Two stores describe the same dead path, so this releases both: the - /// `FipsAddress`-keyed map the TCP MSS clamp reads, and the session's own + /// Three stores describe the same dead path, so this releases all of + /// them: the `FipsAddress`-keyed map the TCP MSS clamp reads, the record + /// of which transport last link-seeded that map, and the session's own /// source-side path MTU estimate. fn path_mtu_lookup_release(&mut self, addr: &NodeAddr) { // The evidence that corroborates a reactive MtuExceeded described the @@ -3048,6 +3055,24 @@ impl Node { return; } } + // The seeding record describes the path just released, so it goes with + // it. Taken after the guard above is dropped, so + // `seed_path_mtu_for_link_peer` remains the only site holding both + // locks at once, and before the reseed below, so a peer whose link is + // still up writes its transport straight back in. + match self.path_mtu_seeded_by.write() { + Ok(mut seeded_by) => { + seeded_by.remove(&fips_addr); + } + Err(e) => { + tracing::warn!( + fips_addr = %fips_addr, + error = %e, + "path_mtu_seeded_by write lock poisoned; seeding record not released" + ); + } + } + // The write guard above must be dropped before the seed runs: it takes // the same lock, and `std::sync::RwLock` is not re-entrant. if let Some(peer) = self.peers.get(addr) diff --git a/src/node/tests/session.rs b/src/node/tests/session.rs index 3aa50d5e..b47ef144 100644 --- a/src/node/tests/session.rs +++ b/src/node/tests/session.rs @@ -4530,19 +4530,38 @@ async fn test_forged_setups_from_one_link_peer_stop_creating_session_entries_onc #[tokio::test] async fn test_a_drained_setup_bucket_refills_and_admits_the_next_legitimate_setup() { - // Fast refill: the point is that the denial is transient, and that the - // initiator's own msg1 resend schedule covers a window this short. - let mut nodes = make_setup_limited_pair(2, 50.0).await; + // The refill has to be slow enough that the draining loop below cannot be + // outrun by the refill it is draining against. At the 50/s this test used + // to run at, a token returned every 20 ms, so on a loaded runner the loop + // outlived its own window, the third setup was admitted, and the + // precondition failed on arrangement rather than on behaviour. At 2/s a + // delivery would have to take 500 ms to lose that race. + let mut nodes = make_setup_limited_pair(2, 2.0).await; - for _ in 0..3 { + // Deliver until one is actually refused, rather than assuming three is + // enough: a delivery the refill absorbs costs one more iteration and + // nothing else. The cap is what a runner slow enough to lose even this + // race trips, and it says so rather than reporting a drained bucket that + // was never drained. + let before = nodes[1].node.stats().session.setup_rate_limited; + let mut delivered = 0; + while nodes[1].node.stats().session.setup_rate_limited == before { + assert!( + delivered < 50, + "the bucket must actually be drained before the refill is tested; \ + 50 forged setups drew no refusal, so each delivery is outlasting \ + the 500 ms refill interval" + ); deliver_forged_setup_over_link(&mut nodes).await; + delivered += 1; } - assert!( - nodes[1].node.stats().session.setup_rate_limited > 0, - "the bucket must actually be drained before the refill is tested" - ); - tokio::time::sleep(Duration::from_millis(100)).await; + // A full burst back from empty at 2/s, so the legitimate setup below meets + // the same bucket however many tokens the drain left behind. The point + // being made is that the denial is transient and clears on its own; the + // length of the window is a function of the configured rate, not of the + // claim. + tokio::time::sleep(Duration::from_millis(1200)).await; establish_pair_session(&mut nodes).await; cleanup_nodes(&mut nodes).await; diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index b64d93ee..94e82466 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -303,6 +303,52 @@ fn test_node_link_management() { ); } +#[test] +fn remove_link_clears_a_reverse_lookup_entry_keyed_on_a_second_address_form() { + let mut node = make_node(); + let transport_id = TransportId::new(1); + + let link_id = node.allocate_link_id(); + node.add_link(Link::connectionless( + link_id, + transport_id, + TransportAddr::from_string("10.128.2.4:2121"), + LinkDirection::Inbound, + Duration::from_millis(50), + )) + .unwrap(); + + // The cross-connection arms key the surviving link on the *packet's* + // source address, which need not be the form the link itself carries. + node.addr_to_link.insert( + (transport_id, TransportAddr::from_string("node-b:2121")), + link_id, + ); + + // An entry another link has claimed is not this link's to remove. + let other_link_id = node.allocate_link_id(); + node.addr_to_link.insert( + (transport_id, TransportAddr::from_string("10.128.2.5:2121")), + other_link_id, + ); + + node.remove_link(&link_id); + + assert!( + node.find_link_by_addr(transport_id, &TransportAddr::from_string("10.128.2.4:2121")) + .is_none() + ); + assert!( + node.find_link_by_addr(transport_id, &TransportAddr::from_string("node-b:2121")) + .is_none(), + "the second address form outlived the link it named" + ); + assert_eq!( + node.find_link_by_addr(transport_id, &TransportAddr::from_string("10.128.2.5:2121")), + Some(other_link_id) + ); +} + #[test] fn test_node_link_limit() { let mut node = make_node_with_max_links(2); @@ -2144,6 +2190,113 @@ async fn test_seed_path_mtu_keeps_tighter_value_when_reseeding_same_transport() } } +/// The seeding record is bounded by the same lifecycle that writes it. +/// +/// Promotion seeds; release drops. Without the release the map keeps a row +/// per peer this node has ever linked with, for the life of the process, and +/// the two stores drift apart: `path_mtu_lookup` forgets the value while the +/// record still names the transport that supplied it. +#[tokio::test] +async fn test_releasing_a_path_drops_the_seeding_transport_record_with_the_value() { + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.supervisor.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1452).await; + node.transports.insert(TransportId::new(1), udp); + + let peer_addr = make_node_addr(0xE4); + let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr); + let transport_addr = TransportAddr::from_string("10.0.0.11:2121"); + + node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr); + assert_eq!( + node.path_mtu_seeded_by + .read() + .unwrap() + .get(&fips_addr) + .copied(), + Some(TransportId::new(1)), + "the seed records the transport it came from" + ); + + // No entry in `node.peers`, so nothing reseeds behind the release — the + // departed-peer case. + node.path_mtu_lookup_release(&peer_addr); + + assert!( + node.path_mtu_lookup + .read() + .unwrap() + .get(&fips_addr) + .is_none(), + "release drops the stored value" + ); + assert!( + node.path_mtu_seeded_by + .read() + .unwrap() + .get(&fips_addr) + .is_none(), + "release must drop the seeding record with it, or the map grows for \ + the life of the process" + ); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +/// The live-link case: release is immediately followed by a reseed, so both +/// stores come back rather than leaving a linked peer on the fallback ceiling. +#[tokio::test] +async fn test_releasing_a_path_for_a_still_linked_peer_reseeds_both_stores() { + let mut node = make_node(); + let (packet_tx, packet_rx) = packet_channel(64); + node.supervisor.packet_tx = Some(packet_tx); + node.packet_rx = Some(packet_rx); + + let udp = make_udp_transport_with_mtu(1, 1452).await; + node.transports.insert(TransportId::new(1), udp); + + let identity = make_peer_identity(); + let peer_addr = *identity.node_addr(); + let fips_addr = crate::FipsAddress::from_node_addr(&peer_addr); + let transport_addr = TransportAddr::from_string("10.0.0.12:2121"); + + let mut peer = crate::peer::ActivePeer::new(identity, LinkId::new(1), 0); + peer.set_current_addr(TransportId::new(1), transport_addr.clone()); + node.peers.insert(peer_addr, peer); + + node.seed_path_mtu_for_link_peer(&peer_addr, TransportId::new(1), &transport_addr); + node.path_mtu_lookup_release(&peer_addr); + + assert_eq!( + node.path_mtu_lookup + .read() + .unwrap() + .get(&fips_addr) + .map(|e| e.mtu), + Some(1452), + "a peer whose link is still up is reseeded from that link" + ); + assert_eq!( + node.path_mtu_seeded_by + .read() + .unwrap() + .get(&fips_addr) + .copied(), + Some(TransportId::new(1)), + "and the seeding record comes back with it, so a later move is still \ + detectable" + ); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + // === Outbound admission gate tests === /// Inject `count` synthetic active peers into `node.peers` so peer_count() @@ -2912,8 +3065,8 @@ async fn app_owned_udp_fd_seam_stays_silent_without_a_udp_transport() { } /// A UDP transport that never bound has no fd to hand out. The bind address is -/// deliberately unparseable — a busy port would not do it, since -/// `UdpRawSocket::open` sets `SO_REUSEADDR`/`SO_REUSEPORT` before binding. +/// deliberately unparseable, so the failure is in parsing and cannot depend on +/// what else happens to hold a port while the suite runs. #[cfg(unix)] #[tokio::test] async fn app_owned_udp_fd_seam_stays_silent_when_the_udp_transport_fails_to_start() { @@ -4160,3 +4313,74 @@ fn mesh_filter_resolves_the_live_tun_device_rather_than_the_configured_name() { node.tun_name = Some(loopback.to_string()); assert_eq!(node.mesh_ifindex(), Some(expected)); } + +/// The msg1 handler keeps its pending slot for as long as it is running. +/// +/// The complement of `msg1_reject_arms_do_not_release_another_handshakes_slot`: +/// that one covers releasing a slot the handler never took, this one covers +/// releasing its own slot too early. Rebinding `handle_msg1`'s `let _slot` to +/// a bare `_` drops the guard at acquire time, so the limiter's concurrency +/// limb stops bounding anything — and every counter this test could read +/// afterwards is identical either way, because the slot comes back at the end +/// of the handler in both worlds. The difference exists only while the handler +/// is on the stack, which is why the observation lives there: the +/// `#[cfg(test)]` assertion in `handle_msg1` immediately below the acquire +/// fires under the premature release and under nothing else. +/// +/// Two packets, so the handler is entered twice on different paths past the +/// acquire, and each arm asserts the reject counter it must bump. Without +/// that, a msg1 refused before the acquire (an empty bucket, say) would leave +/// this test passing while sampling nothing. +#[tokio::test] +async fn msg1_handler_holds_its_pending_slot_while_the_handler_runs() { + use crate::noise::HANDSHAKE_MSG1_SIZE; + use crate::proto::fmp::wire::build_msg1; + use crate::utils::index::SessionIndex; + + // No transport is registered: both arms reject before any send, and the + // absent transport admits past the `accept_connections` gate. + let mut node = make_node(); + let transport_id = TransportId::new(1); + let source = TransportAddr::from_string("198.51.100.9:4141"); + let packet = |data: Vec| ReceivedPacket { + transport_id, + remote_addr: source.clone(), + data, + timestamp_ms: 1000, + }; + + assert_eq!( + node.msg1_rate_limiter.pending_count(), + 0, + "baseline: no handshake in flight" + ); + + // Arm 1: rejected at the header parse, the shortest path past the acquire. + let before = node.stats().handshake.bad_state; + node.handle_msg1(packet(vec![0u8; 8])).await; + assert_eq!( + node.stats().handshake.bad_state, + before + 1, + "arm 1 must reach the invalid-header reject, not a rate-limit refusal" + ); + + // Arm 2: well-formed header, unusable Noise payload — rejected further in, + // after the duplicate short-circuit and the DH attempt. + let before = node.stats().handshake.bad_state; + node.handle_msg1(packet(build_msg1( + SessionIndex::new(0x4242), + &[0u8; HANDSHAKE_MSG1_SIZE], + ))) + .await; + assert_eq!( + node.stats().handshake.bad_state, + before + 1, + "arm 2 must reach the receive_handshake_init reject" + ); + + assert_eq!( + node.msg1_rate_limiter.pending_count(), + 0, + "each handler released its own slot exactly once on the way out" + ); +} diff --git a/src/noise/session.rs b/src/noise/session.rs index 92a8a059..99d17ac5 100644 --- a/src/noise/session.rs +++ b/src/noise/session.rs @@ -14,7 +14,17 @@ pub struct NoiseSession { send_cipher: CipherState, /// Cipher for receiving. recv_cipher: CipherState, - /// Handshake hash. + /// Handshake hash, copied out of the handshake's `SymmetricState`. + /// + /// Deliberately not cleared on drop, unlike the copy it came from. That + /// copy is cleared because it sits beside the chaining key in a type whose + /// contract is that it erases what it holds; this one is a transcript hash + /// that nothing derives a key from, and `handshake_hash()` hands it out to + /// any caller. A type cannot both publish a value and treat it as secret. + /// + /// Revisit if the hash ever becomes key-adjacent: feeding it to the AEAD as + /// associated data, or building channel binding or an exporter on it, would + /// make this session's copy the long-lived home of something worth erasing. handshake_hash: [u8; 32], /// Remote peer's static public key. remote_static: PublicKey, diff --git a/src/transport/udp/io/mod.rs b/src/transport/udp/io/mod.rs index e372afe7..8acbb630 100644 --- a/src/transport/udp/io/mod.rs +++ b/src/transport/udp/io/mod.rs @@ -76,6 +76,29 @@ mod tests { assert!(send_buf > 0, "send buffer should be non-zero"); } + /// The listen socket's reuse flags go on after its own bind, so they + /// never license the kernel to hand this socket a port someone else + /// holds. The visible consequence is that a second bind of an occupied + /// port fails loudly instead of silently sharing it and splitting the + /// inbound datagrams between the two recv loops. + #[cfg(unix)] + #[test] + fn a_second_open_of_an_occupied_port_fails_instead_of_sharing_it() { + let holder = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 65536, 65536) + .expect("failed to bind the holding socket"); + let addr = holder.local_addr(); + + // `UdpRawSocket` is not `Debug`, so this cannot be `expect_err`. + let Err(err) = UdpRawSocket::open(addr, 65536, 65536) else { + panic!("the port is already held, so the second bind must fail"); + }; + + assert!( + err.to_string().contains("bind failed"), + "the failure must come from bind, not from a later step: {err}", + ); + } + #[tokio::test] async fn test_async_udp_socket_send_recv() { let sock1 = UdpRawSocket::open("127.0.0.1:0".parse().unwrap(), 65536, 65536) diff --git a/src/transport/udp/io/unix.rs b/src/transport/udp/io/unix.rs index 66e8e5e3..60a4cb43 100644 --- a/src/transport/udp/io/unix.rs +++ b/src/transport/udp/io/unix.rs @@ -57,19 +57,27 @@ impl UdpRawSocket { sock.set_nonblocking(true) .map_err(|e| TransportError::StartFailed(format!("set nonblocking failed: {}", e)))?; - // SO_REUSEPORT lets per-peer `ConnectedPeerSocket`s bind - // to the same wildcard port the listen socket holds. Must - // be set BEFORE bind. Without this, the connected-UDP - // activation handler fails with EADDRINUSE on Linux and - // every outbound packet falls back to the wildcard listen - // socket — losing the kernel 5-tuple cache benefit and - // most of the multihop forwarding throughput gain. - let _ = sock.set_reuse_port(true); - let _ = sock.set_reuse_address(true); - sock.bind(&bind_addr.into()) .map_err(|e| TransportError::StartFailed(format!("bind failed: {}", e)))?; + // SO_REUSEPORT lets per-peer `ConnectedPeerSocket`s bind to the same + // port this listen socket holds; without it the connected-UDP + // activation handler fails with EADDRINUSE on Linux and every + // outbound packet falls back to the wildcard listen socket, losing + // the kernel 5-tuple cache benefit and most of the multihop + // forwarding throughput gain. + // + // Set AFTER bind, deliberately. The flags mean different things + // either side of it: before bind, "the kernel may give me a port + // another socket already holds", which for the `outbound_only` + // `0.0.0.0:0` bind means duplicate ephemeral ports, and for a + // configured port means a second daemon silently shares it instead + // of failing to start; after bind, "another socket may later join my + // port", which is the only half the fast path needs. A joiner still + // binds successfully against a holder flagged after its own bind. + let _ = sock.set_reuse_port(true); + let _ = sock.set_reuse_address(true); + // Set socket buffer sizes sock.set_recv_buffer_size(recv_buf_size) .map_err(|e| TransportError::StartFailed(format!("set recv buffer: {}", e)))?; diff --git a/src/transport/udp/mod.rs b/src/transport/udp/mod.rs index 9580ed47..0fc65feb 100644 --- a/src/transport/udp/mod.rs +++ b/src/transport/udp/mod.rs @@ -100,11 +100,41 @@ impl UdpTransport { /// Raw file descriptor of the bound listen socket, or `None` before /// `start_async` has bound one. /// + /// The socket stays owned by the transport, so the descriptor is a borrow + /// and not a handover. It is the wildcard listen socket only: the per-peer + /// `connect()`-ed sockets the fast path opens on Linux and macOS are not + /// reachable through here. + /// + /// What a holder may do with it: read it (`getsockname`, `getsockopt`), + /// query it (`SIOCGIFINDEX` and friends), and set the host-network options + /// the seam exists for — binding it to a device (`SO_BINDTODEVICE`), + /// marking it (`SO_MARK`), or attaching it to a routing table. `dup()` is + /// fine as long as the duplicate is closed by whoever made it. + /// + /// What a holder may not do: `close()` it, adopt it into an owning wrapper + /// (`OwnedFd::from_raw_fd`, `UdpSocket::from_raw_fd`) whose `Drop` will + /// close it, `shutdown()` it, re-`bind()` or `connect()` it, or clear + /// `O_NONBLOCK` on it. The transport registered this descriptor with the + /// tokio reactor through `AsyncFd`, and reads packets from it on its own + /// task; any of those actions either stalls the receive loop or, in the + /// case of a close, frees a number the kernel is free to hand to the next + /// socket or file this process opens, at which point every later use of it + /// silently addresses something else. + /// + /// The descriptor is invalidated by anything that drops the transport's + /// socket: `stop_async`, the node stop that calls it, a transport that + /// fails and is torn down, or dropping the transport itself. **Nothing + /// notifies the holder when that happens.** A holder that outlives a stop + /// has to treat the value it kept as stale on its own account — after a + /// restart the transport binds a fresh socket, and the descriptor names a + /// different socket even when the kernel hands back the same number. + /// Fetch it again after each `start_async` rather than caching it + /// across one; embedders that receive it through + /// [`Node::enable_app_owned_udp_fd`](crate::Node::enable_app_owned_udp_fd) + /// get exactly that, one message per successful bind. + /// /// Unix-only: `RawFd` is a unix concept and the Windows backend is built - /// on `tokio::net::UdpSocket` with no descriptor to hand out. The socket - /// stays owned by the transport, so the descriptor is a borrow with no - /// lifetime promise beyond "this is the transport's socket, and it is - /// open right now". + /// on `tokio::net::UdpSocket` with no descriptor to hand out. #[cfg(unix)] pub fn raw_fd(&self) -> Option { use std::os::unix::io::AsRawFd; diff --git a/testing/check-log-strings.py b/testing/check-log-strings.py index 7a597f9f..d230eea5 100755 --- a/testing/check-log-strings.py +++ b/testing/check-log-strings.py @@ -37,6 +37,14 @@ ALLOWED = { "ERROR": "tracing's own level token, produced by the subscriber's formatter", " ERROR ": "tracing's own level token, produced by the subscriber's formatter", " WARN ": "tracing's own level token, produced by the subscriber's formatter", + "caught a signal": ( + "read from the strfry relay container's log, not the fips daemon's — " + "strfry's own crash handler writes it" + ), + "terminate called": ( + "read from the strfry relay container's log, not the fips daemon's — " + "the C++ runtime writes it on an uncaught exception" + ), "Bootstrapped 100%": ( "read from the tor-daemon container's log, not the fips daemon's — " "Tor's own bootstrap progress line" diff --git a/testing/interop/interop-test.sh b/testing/interop/interop-test.sh index 83c1ac9f..3d8e41dd 100755 --- a/testing/interop/interop-test.sh +++ b/testing/interop/interop-test.sh @@ -51,9 +51,14 @@ # FIPS_INTEROP_STREAMS data-plane stream pairs (`nid-nid` tokens, both # directions streamed). Enables Phase 1b/5b. Set # by --topology; empty = streams off. -# STREAM_LOSS_MARGIN_PCT rekey-vs-control loss margin (default 1). +# STREAM_LOSS_MARGIN_PCT rekey-vs-control loss margin (default 5). # CONTROL_STREAM_SECS quiet control-window length (default 12). -# MESH_SIZE_TIMEOUT Phase 7 convergence poll budget (default 180). +# MESH_SIZE_WARMUP Phase 7 bloom warmup, from mesh start, before +# any estimate counts (default 300). +# MESH_SIZE_SETTLE Phase 7 unbroken in-band window a node must +# hold to be credited (default 60). +# MESH_SIZE_TIMEOUT Phase 7 poll budget beyond warmup+settle +# (default 180). # FIPS_INTEROP_KEEP_UP 1 = leave containers running after the test (debug). # REKEY_AFTER_SECS rekey interval to generate configs with (default # 35; multihop-3v-cycle defaults it to 50). @@ -174,15 +179,40 @@ LOG_POLL_INTERVAL=2 # Data-plane continuity stream (control-differential). Streams run a # sustained ping6 over the overlay across the rekey window vs a quiet # control window; loss is compared to prove rekey is hitless. -STREAM_RATE_HZ=5 # ping6 -i 0.2 +# The control window cannot be lengthened much: it has to fit inside one +# rekey interval (REKEY_AFTER_SECS, 35s by default) to stand a chance of +# being cutover-free, and it already gets contaminated sometimes at 12s. +# So the sample is raised by rate instead. At 20 Hz the control window is +# 240 packets and the rekey window ~3100, where at 5 Hz they were 60 and +# ~775 and a 1% margin came to 0.6 of a control packet — below the +# quantisation of its own measurement, so a one-packet difference decided +# the verdict and the netem arm failed on same-version pairs as readily +# as on mixed ones. +STREAM_RATE_HZ=20 CONTROL_STREAM_SECS="${CONTROL_STREAM_SECS:-12}" # quiet pre-rekey window -STREAM_LOSS_MARGIN_PCT="${STREAM_LOSS_MARGIN_PCT:-1}" +# 5% is 12 packets of the 240-packet control window and ~155 of the +# ~3100-packet rekey window. On a path losing up to 6% (the range the +# v0.4.2 netem runs baselined at, under `loss 2%` over several hops) the +# difference of the two windows has a standard deviation below 1.6%, so +# 5% is past three sigma and sampling spread can no longer produce a red. +# It still catches what this check exists for: a rekey that is not +# hitless blackholes until the session reconverges, which the harness +# budgets 45s for, against the ~8s of traffic 5% of the rekey window is. +STREAM_LOSS_MARGIN_PCT="${STREAM_LOSS_MARGIN_PCT:-5}" # The rekey-window stream must span Phases 2-5 (both cutovers + reconverge). REKEY_STREAM_SECS=$(( FIRST_REKEY_TIMEOUT + SECOND_REKEY_WAIT + POST_REKEY_TIMEOUT + 15 )) -# Mesh-size estimate convergence (strict ±25% of true N). Generous poll -# budget — the bloom-union estimate converges over minutes. +# Mesh-size estimate convergence (strict ±25% of true N). The archived +# design note puts bloom-filter warmup at ~5 minutes from node start and +# calls the estimate unreliable before that, so nothing sampled inside +# MESH_SIZE_WARMUP is evidence of convergence, and a node is credited +# only after holding the band unbroken for MESH_SIZE_SETTLE afterwards. +# MESH_SIZE_TIMEOUT is the extra budget for reaching that state, on top +# of the warmup and settle windows rather than covering them. +MESH_SIZE_WARMUP="${MESH_SIZE_WARMUP:-300}" +MESH_SIZE_SETTLE="${MESH_SIZE_SETTLE:-60}" MESH_SIZE_TIMEOUT="${MESH_SIZE_TIMEOUT:-180}" +MESH_SIZE_POLL=5 # ── Counters ───────────────────────────────────────────────────────── @@ -512,6 +542,14 @@ wait_for_log_pattern_count() { [ "$(count_log_pattern "$pattern")" -ge "$min_count" ] } +# One node's bloom-union mesh-size estimate, or "null" when the daemon +# has no estimate yet or cannot be reached. +mesh_estimate() { + docker exec "${CONTAINER[$1]}" fipsctl show status 2>/dev/null \ + | python3 -c "import sys,json; v=json.load(sys.stdin).get('estimated_mesh_size'); print(v if v is not None else 'null')" 2>/dev/null \ + || echo null +} + # ── Data-plane continuity streams ──────────────────────────────────── # # A sustained ping6 stream over the overlay (.fips) is data-plane @@ -530,8 +568,11 @@ _stream_one() { local from="$1" to="$2" dur="$3" outfile="$4" local from_ctr="${CONTAINER[$from]}" to_npub="${NPUB_OF[$to]}" local count=$(( dur * STREAM_RATE_HZ )) - local out tx rx - out=$(docker exec "$from_ctr" ping6 -i 0.2 -c "$count" -W "$PING_TIMEOUT" \ + local out tx rx interval + # Interval and count both derive from STREAM_RATE_HZ, so the packet + # count the margin is reasoned about cannot drift from the rate sent. + interval=$(awk -v hz="$STREAM_RATE_HZ" 'BEGIN{ printf "%.3f", 1/hz }') + out=$(docker exec "$from_ctr" ping6 -i "$interval" -c "$count" -W "$PING_TIMEOUT" \ "${to_npub}.fips" 2>&1) tx=$(echo "$out" | grep -oE '[0-9]+ packets transmitted' | grep -oE '^[0-9]+') rx=$(echo "$out" | grep -oE '[0-9]+ received' | grep -oE '^[0-9]+') @@ -661,6 +702,9 @@ if ! compose up -d; then echo " FAIL compose up failed" exit 1 fi +# Phase 7 measures bloom-filter warmup from node start, not from its own +# start, so the clock has to be taken here. +MESH_UP_AT=$SECONDS # Optional netem: applied via `docker exec ... tc qdisc` on each # container's eth0 — host bridge qdisc does NOT shape inter-container @@ -948,43 +992,80 @@ echo "" # .estimated_mesh_size) should converge to the true node count across # versions. A mixed-version bloom/tree-encoding divergence shows up as a # node that never produces an in-band estimate (or returns null). Strict -# band = [0.75N, 1.25N]; polled up to MESH_SIZE_TIMEOUT (the estimate -# converges over minutes and is transiently jittery). +# band = [0.75N, 1.25N]. +# +# The verdict is taken on a settled estimate rather than on a node's +# first tolerable reading. Two windows enforce that. Nothing sampled +# before MESH_SIZE_WARMUP has elapsed since the mesh came up counts at +# all, because the estimate is documented as unreliable during warmup; +# after it, a node is credited only once it has held the band unbroken +# for MESH_SIZE_SETTLE. Every node is re-polled every round and any +# out-of-band reading restarts that node's settle clock, so a node +# cannot be credited for a sample it has since contradicted. echo "Phase 7: Mesh-size estimate convergence (strict ±25% of true N=$NUM_NODES)" PASSED=0; FAILED=0 ms_lo="$(awk -v n="$NUM_NODES" 'BEGIN{printf "%.2f", 0.75*n}')" ms_hi="$(awk -v n="$NUM_NODES" 'BEGIN{printf "%.2f", 1.25*n}')" -echo " band [$ms_lo, $ms_hi], poll up to ${MESH_SIZE_TIMEOUT}s" -declare -A MS_EST MS_OK -ms_deadline=$(( SECONDS + MESH_SIZE_TIMEOUT )) +declare -A MS_EST MS_OK MS_INBAND_SINCE +ms_accept_after=$(( MESH_UP_AT + MESH_SIZE_WARMUP )) +echo " band [$ms_lo, $ms_hi], warmup ends $(( ms_accept_after - SECONDS ))s from now, then ${MESH_SIZE_SETTLE}s settled, poll up to ${MESH_SIZE_TIMEOUT}s beyond that" + +# Poll through the warmup as well. Nothing here is asserted on — it is +# the trajectory the phase has never recorded, and it is what an +# undercount that never recovers would show up in. +while [ "$SECONDS" -lt "$ms_accept_after" ]; do + ms_line="" + for n in "${NODES[@]}"; do + MS_EST[$n]="$(mesh_estimate "$n")" + ms_line+=" $n=${MS_EST[$n]}" + done + echo " warmup, $(( ms_accept_after - SECONDS ))s to go:$ms_line" + sleep 30 +done + +ms_deadline=$(( SECONDS + MESH_SIZE_SETTLE + MESH_SIZE_TIMEOUT )) while :; do all_ok=1 for n in "${NODES[@]}"; do - [ "${MS_OK[$n]:-0}" = "1" ] && continue - est="$(docker exec "${CONTAINER[$n]}" fipsctl show status 2>/dev/null \ - | python3 -c "import sys,json; v=json.load(sys.stdin).get('estimated_mesh_size'); print(v if v is not None else 'null')" 2>/dev/null || echo null)" + est="$(mesh_estimate "$n")" MS_EST[$n]="$est" if [ "$est" != "null" ] && awk "BEGIN{exit !($est>=$ms_lo && $est<=$ms_hi)}"; then + [ -z "${MS_INBAND_SINCE[$n]:-}" ] && MS_INBAND_SINCE[$n]="$SECONDS" + else + unset "MS_INBAND_SINCE[$n]" + fi + if [ -n "${MS_INBAND_SINCE[$n]:-}" ] \ + && [ $(( SECONDS - ${MS_INBAND_SINCE[$n]} )) -ge "$MESH_SIZE_SETTLE" ]; then MS_OK[$n]=1 else + MS_OK[$n]=0 all_ok=0 fi done [ "$all_ok" = "1" ] && break [ "$SECONDS" -ge "$ms_deadline" ] && break - sleep 3 + sleep "$MESH_SIZE_POLL" done for n in "${NODES[@]}"; do s="${SLOT_OF[$n]}"; u="$(echo "$s" | tr '[:lower:]' '[:upper:]')" if [ "${MS_OK[$n]:-0}" = "1" ]; then - echo " PASS $n [$u]: estimated_mesh_size=${MS_EST[$n]}" + echo " PASS $n [$u]: estimated_mesh_size=${MS_EST[$n]} (held band ${MESH_SIZE_SETTLE}s+ after warmup)" PASSED=$((PASSED + 1)) else - echo " FAIL $n [$u]: estimated_mesh_size=${MS_EST[$n]:-null} (outside [$ms_lo,$ms_hi] after ${MESH_SIZE_TIMEOUT}s)" + if [ -n "${MS_INBAND_SINCE[$n]:-}" ]; then + ms_why="held band only $(( SECONDS - ${MS_INBAND_SINCE[$n]} ))s of the ${MESH_SIZE_SETTLE}s settle window" + else + ms_why="outside [$ms_lo,$ms_hi]" + fi + echo " FAIL $n [$u]: estimated_mesh_size=${MS_EST[$n]:-null} ($ms_why)" FAILED=$((FAILED + 1)) - INTEROP_FAILURES+=("[mesh-size] node $n ($u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}): estimate=${MS_EST[$n]:-null} outside [$ms_lo,$ms_hi]") + INTEROP_FAILURES+=("[mesh-size] node $n ($u ${SLOT_REF[$s]}@${SLOT_SHA[$s]}): estimate=${MS_EST[$n]:-null} $ms_why") fi done +# The band is per-node, so a green says every node was plausible, not +# that the nodes agreed. Print the spread so nobody has to infer it. +ms_spread="$(printf '%s\n' "${MS_EST[@]}" | sort -n | awk '/^[0-9]+$/{ if (lo=="") lo=$1; hi=$1 } END{ if (lo=="") print "no numeric estimates"; else if (lo==hi) print "all nodes agree on " lo; else print "nodes disagree: " lo " to " hi }')" +echo " NOTE: final estimates — $ms_spread (agreement is reported, not asserted)" phase_result "Mesh-size estimate convergence" echo "" diff --git a/testing/nat/scripts/nostr-relay-test.sh b/testing/nat/scripts/nostr-relay-test.sh index 9847cb85..c9b0e1b5 100755 --- a/testing/nat/scripts/nostr-relay-test.sh +++ b/testing/nat/scripts/nostr-relay-test.sh @@ -85,7 +85,69 @@ require_test_image() { "$BUILD_SCRIPT" } +# State the relay's own condition, in its own words, before anything else. +# +# The relay is a third-party container and it has died mid-run before: strfry +# took a SIGSEGV twelve milliseconds after both nodes had connected, and every +# symptom under that was a correct report of a dead relay — subscriptions +# dropped, no advert consumed, empty peer lists, and a run that failed on the +# peer-count wait. That verdict is indistinguishable from a product failure +# unless the relay's state is stated, and the only evidence of the real cause +# was one container-log line ninety lines above the summary. This does not +# decide the run; it says whose failure it was, in a line a reader of the +# output can find. +# +# A relay whose state or log cannot be read has not been shown to be healthy, +# so that case is reported as unestablished rather than as "not the relay". +relay_verdict() { + local state="" status="" exit_code="" restarts="" logs="" + + state="$(docker inspect \ + -f '{{.State.Status}} {{.State.ExitCode}} {{.RestartCount}}' \ + "$RELAY_CONTAINER" 2>/dev/null)" || state="" + + if [ -z "$state" ]; then + echo "RELAY FAILURE: $RELAY_CONTAINER is gone; whatever this run" \ + "asserted about peering happened without a relay" >&2 + return 0 + fi + + read -r status exit_code restarts <<<"$state" + + if [ "$status" != "running" ]; then + echo "RELAY FAILURE: $RELAY_CONTAINER is $status (exit $exit_code);" \ + "the assertions in this run are downstream of that, not of the" \ + "nodes" >&2 + return 0 + fi + + if [ "${restarts:-0}" -gt 0 ]; then + echo "RELAY FAILURE: $RELAY_CONTAINER has restarted $restarts time(s)" \ + "during this run; the nodes lost their subscriptions with it" >&2 + return 0 + fi + + if ! logs="$(docker logs "$RELAY_CONTAINER" 2>&1)"; then + echo "RELAY HEALTH NOT ESTABLISHED: could not read" \ + "$RELAY_CONTAINER's logs, so a crash cannot be ruled in or out" >&2 + return 0 + fi + + if grep -Eq "caught a signal|SIGSEGV|SIGABRT|terminate called" <<<"$logs"; then + echo "RELAY FAILURE: $RELAY_CONTAINER faulted during this run:" >&2 + grep -E "caught a signal|SIGSEGV|SIGABRT|terminate called" <<<"$logs" >&2 + return 0 + fi + + echo "relay: $RELAY_CONTAINER running, no fault in its log — this run's" \ + "verdict is about the nodes" + return 1 +} + dump_diagnostics() { + echo "" + echo "=== relay verdict ===" + relay_verdict || true echo "" echo "=== nostr publish/consume diagnostics ===" for c in "$NODE_A" "$NODE_B" "$RELAY_CONTAINER"; do @@ -397,6 +459,13 @@ run_test() { return 1 fi + # A relay that faulted while the assertions still passed is a finding + # about the relay, not about this run, so it is reported and not made a + # failure: the suite proved what it set out to prove. + if relay_verdict; then + echo "NOTE: the assertions above passed despite that." >&2 + fi + cleanup echo "nostr-relay-test passed" } diff --git a/testing/repro-tarball-gate.sh b/testing/repro-tarball-gate.sh new file mode 100755 index 00000000..34f55844 --- /dev/null +++ b/testing/repro-tarball-gate.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# Gate 11: build the systemd install tarball twice and compare the two byte for byte. +# +# packaging/systemd/build-tarball.sh already carries the tar-level determinism work +# (SOURCE_DATE_EPOCH, --mtime, --sort=name, --numeric-owner --owner=0 --group=0). +# What has never existed at any FIPS release is a wrapper that actually builds twice +# and compares, so this supplies only that. +# +# Builds run in throwaway worktrees, never in a working checkout, so no existing +# target/ cache is destroyed and no checkout is left dirty. +# +# A and B same path, built twice -- this is the gate +# C a different path -- probe only, never gating: there is no +# [profile.release], no .cargo/config.toml and no --remap-path-prefix, +# so an absolute build path can reach the binaries +# +# Usage: testing/repro-tarball-gate.sh [src-repo] [out-dir] +# Exit: 0 if A and B are identical, 1 if they differ, 2 on a setup failure. + +set -euo pipefail + +REF="${1:?usage: repro-tarball-gate.sh [src-repo] [out-dir]}" +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +SRC_REPO="${2:-${REPO_ROOT}}" +# Under target/, which is gitignored, so a gate run never dirties the checkout +# it was launched from. +OUT_DIR="${3:-${REPO_ROOT}/target/repro-tarball}" + +# `--git-dir` rather than a test for a .git directory: a linked worktree carries +# a .git file, and the release runs this out of whatever checkout is to hand. +git -C "${SRC_REPO}" rev-parse --git-dir >/dev/null 2>&1 \ + || { echo "no source checkout at ${SRC_REPO}" >&2; exit 2; } + +SHA="$(git -C "${SRC_REPO}" rev-parse "${REF}")" +# Pin explicitly rather than letting build-tarball.sh derive it, so all three +# builds share one value even if they are cut from different worktrees. +SOURCE_DATE_EPOCH="$(git -C "${SRC_REPO}" log -1 --format=%ct "${SHA}")" +export SOURCE_DATE_EPOCH + +WORK="$(mktemp -d -t repro-gate-XXXXXX)" +mkdir -p "${OUT_DIR}" +trap 'for w in "${WORK}"/*; do [ -d "$w" ] && git -C "${SRC_REPO}" worktree remove --force "$w" 2>/dev/null || true; done; rm -rf "${WORK}"' EXIT + +echo "ref ${REF} (${SHA})" +echo "SOURCE_DATE_EPOCH ${SOURCE_DATE_EPOCH}" +echo "work ${WORK}" +echo + +# Build one tarball in a fresh worktree at $1, leaving it at $2. +build() { + local dir="$1" dest="$2" label="$3" + echo "=== ${label}: ${dir}" + git -C "${SRC_REPO}" worktree add --detach --quiet "${dir}" "${SHA}" + ( cd "${dir}" && ./packaging/systemd/build-tarball.sh ) >"${OUT_DIR}/${label}.log" 2>&1 || { + echo "${label}: build failed, see ${OUT_DIR}/${label}.log" >&2 + tail -20 "${OUT_DIR}/${label}.log" >&2 + exit 2 + } + local tb + tb="$(ls "${dir}"/deploy/*.tar.gz)" + cp "${tb}" "${dest}" + echo "${label}: $(sha256sum "${dest}" | cut -d' ' -f1) $(basename "${tb}")" + git -C "${SRC_REPO}" worktree remove --force "${dir}" +} + +# A and B share one path, so the second reuses nothing: the worktree is removed +# and recreated between them, which is what makes this a real rebuild. +build "${WORK}/same" "${OUT_DIR}/A.tar.gz" A +build "${WORK}/same" "${OUT_DIR}/B.tar.gz" B +build "${WORK}/other-path-for-the-probe" "${OUT_DIR}/C.tar.gz" C + +echo +rc=0 +if cmp -s "${OUT_DIR}/A.tar.gz" "${OUT_DIR}/B.tar.gz"; then + echo "GATE PASS: A and B are byte-identical" +else + echo "GATE FAIL: A and B differ" + rc=1 +fi + +if cmp -s "${OUT_DIR}/A.tar.gz" "${OUT_DIR}/C.tar.gz"; then + echo "PROBE: a different build path changes nothing" +else + echo "PROBE: a different build path changes the tarball (not gating)" +fi + +# Localize any difference to the file level, for both the gate and the probe. +for pair in A:B A:C; do + l="${pair%%:*}"; r="${pair##*:}" + cmp -s "${OUT_DIR}/${l}.tar.gz" "${OUT_DIR}/${r}.tar.gz" && continue + echo + echo "--- per-member digests, ${l} vs ${r}" + for s in "${l}" "${r}"; do + rm -rf "${WORK}/x-${s}"; mkdir -p "${WORK}/x-${s}" + tar -xzf "${OUT_DIR}/${s}.tar.gz" -C "${WORK}/x-${s}" + ( cd "${WORK}/x-${s}" && find . -type f | sort | xargs sha256sum ) >"${WORK}/d-${s}.txt" + done + diff "${WORK}/d-${l}.txt" "${WORK}/d-${r}.txt" || true +done + +exit "${rc}"