diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ebfb3a6..68f5d798 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,6 +84,11 @@ 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 fmt: name: Format check @@ -636,6 +641,36 @@ jobs: timeout-minutes: 5 run: bash testing/openwrt/maintainer-scripts-test.sh +# ───────────────────────────────────────────────────────────────────────────── +# Job 2f – systemd tarball upgrade scenarios +# +# Runs the tarball's install.sh twice in a systemd container, the second time +# as an upgrade, with fips, fips-dns and fips-gateway in a known state, and +# checks that the units running before the upgrade are running after it. The +# binaries are stubs, so like the OpenWrt job it needs no FIPS build and no +# shared test image, and has a job of its own. +# +# The leg keeps `suite:` because testing/check-ci-parity.sh matches it against +# TARBALL_INSTALL_SUITES in ci-local.sh; the step below does not read it. +# ───────────────────────────────────────────────────────────────────────────── + tarball-install: + name: systemd tarball (${{ matrix.suite }}) + runs-on: ubuntu-latest + if: ${{ !inputs.skip_integration }} + + strategy: + fail-fast: false + matrix: + include: + - suite: tarball-install + + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + + - name: Run the systemd tarball upgrade scenarios + timeout-minutes: 10 + run: bash testing/tarball-install/test.sh + # ───────────────────────────────────────────────────────────────────────────── # Job 3 – Integration tests (static mesh + chaos simulation) # diff --git a/.github/workflows/package-linux.yml b/.github/workflows/package-linux.yml index 7a7ff22d..d9f20e17 100644 --- a/.github/workflows/package-linux.yml +++ b/.github/workflows/package-linux.yml @@ -18,6 +18,7 @@ jobs: runs-on: ubuntu-latest outputs: linux_package_version: ${{ steps.linux_version.outputs.linux_package_version }} + deb_package_version: ${{ steps.linux_version.outputs.deb_package_version }} steps: - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: @@ -42,7 +43,18 @@ jobs: VERSION="${BASE_VERSION}+${BRANCH}.${HEIGHT}.${HASH}" fi + # 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. + DEB_VERSION="$VERSION" + if [[ "$GITHUB_REF" == refs/tags/* ]] \ + && [[ "$VERSION" =~ ^([0-9]+\.[0-9]+\.[0-9]+)-((alpha|beta|pre|rc)[0-9]*)$ ]]; then + DEB_VERSION="${BASH_REMATCH[1]}~${BASH_REMATCH[2]}" + fi + echo "linux_package_version=${VERSION}" >> "$GITHUB_OUTPUT" + echo "deb_package_version=${DEB_VERSION}" >> "$GITHUB_OUTPUT" build: name: Build Linux artifacts (${{ matrix.artifact_arch }}) @@ -128,7 +140,7 @@ jobs: : ${GITHUB_OUTPUT:=/tmp/github_output} packaging/debian/build-deb-container.sh \ - --version "${{ needs.determine-versioning.outputs.linux_package_version }}" \ + --version "${{ needs.determine-versioning.outputs.deb_package_version }}" \ --output-dir deploy \ --image-archive "$RUNNER_TEMP/deb-builder-image.tar" \ | tee /tmp/build-deb-container.log @@ -149,6 +161,19 @@ jobs: ;; esac + # A candidate's package Version carries '~' (see determine-versioning), + # and cargo-deb puts it in the file name. GitHub renames a release + # asset whose name has special characters, which would leave + # checksums-linux.txt naming a file the release does not have. The + # file takes the tag's '-' instead; the Version inside is unchanged. + case "$DEB_FILE" in + *~*) + RENAMED="$(dirname "$DEB_FILE")/$(basename "$DEB_FILE" | tr '~' '-')" + mv "$DEB_FILE" "$RENAMED" + DEB_FILE="$RENAMED" + ;; + esac + # Record it relative to the checkout: upload-artifact derives the # archive layout from the common ancestor of its paths, and an # absolute path here would nest the package under directories the diff --git a/CHANGELOG.md b/CHANGELOG.md index 3434e374..7c3024dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -324,6 +324,10 @@ with v0.5.x or earlier peers. ### Changed +- Transport address clones share immutable bytes instead of allocating a copy. + Socket addresses are formatted on the stack so construction still needs only + one allocation, including scoped IPv6 addresses. + - `Degraded` is now a level rather than a latch. The supervisor's reason set was monotonic, which was correct while no child could recover; with recovery it would have meant "something broke at some point since boot" rather than @@ -502,6 +506,19 @@ with v0.5.x or earlier peers. reaching it means a second build image and a second floor. `FIPS_BUILD_IMAGE` is still the oldest Debian-family distribution, which is no longer the oldest distribution outright. +- A release-candidate `.deb` is versioned `X.Y.Z~rcN` instead of + `X.Y.Z-rcN`. dpkg read the old form as a revision of the release and sorted + it above `X.Y.Z`, so a host that installed a candidate was not upgraded by + the release. The tarball, artifact and `.deb` file names keep the tag's + `-rcN`. + +### Deprecated + +#### Sessions and rekey + +- Setting `node.rekey.enabled: false`. The disabled path is less tested and is + not supported, and the option is removed in v2. It still works as before; + the configuration reference and the mesh-layer design now say so. ### Removed @@ -643,6 +660,24 @@ with v0.5.x or earlier peers. the Noise session, tree position and routes survive it. Linux and macOS (the platforms with the connected-socket fast path); elsewhere the heartbeat alone carries the new address. +- The Ethernet transport on Linux no longer leaks a socket when the interface + is missing or its name is invalid at the moment the socket is opened, as when + the interface goes away between the presence check and the bind. +- TCP connections try the remaining addresses for a hostname after a + connection fails, within the existing overall connection timeout. + +#### Gateway + +- The gateway retries failed firewall rebuilds every ten seconds without + waiting for another mapping change. Retries apply the latest desired + mappings and port forwards, preserving changes across transient failures. + +#### Packaging + +- The systemd tarball's `install.sh` restarts fips-dns and fips-gateway after + an upgrade when they were running. It stopped fips first, which stopped both + through `Requires=fips.service`, then restarted only fips, so `.fips` + resolution stayed down and the gateway stayed stopped until started by hand. - A node no longer relays away the answer to its own lookup. A request is flooded to every tree peer whose bloom filter claims the target, so a false diff --git a/Cargo.toml b/Cargo.toml index 70a5d296..8994c2de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,3 +144,8 @@ path = "src/bin/fipstop/main.rs" name = "routing_next_hop" path = "benches/routing_next_hop.rs" harness = false + +[[bench]] +name = "transport_addr_clone" +path = "benches/transport_addr_clone.rs" +harness = false diff --git a/benches/transport_addr_clone.rs b/benches/transport_addr_clone.rs new file mode 100644 index 00000000..603e0bcf --- /dev/null +++ b/benches/transport_addr_clone.rs @@ -0,0 +1,107 @@ +//! Compare shared addresses with the former Vec backing, including the receive +//! constructor and a producer cloning while a consumer drops on another thread. +//! The cross-thread case includes bounded-channel overhead in both measurements; +//! it is not an isolated measurement of the atomic reference count. + +use std::hint::black_box; +use std::io::Write; +use std::net::SocketAddr; +use std::sync::{Barrier, mpsc}; +use std::time::{Duration, Instant}; + +use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main}; +use fips::TransportAddr; + +const ADDRESS_BYTES: [usize; 3] = [6, 18, 58]; + +fn former_from_socket_addr(addr: SocketAddr) -> Vec { + let mut buf = Vec::with_capacity(56); + write!(&mut buf, "{addr}").expect("Vec::write_fmt is infallible"); + buf +} + +fn bench_clone(c: &mut Criterion) { + let mut group = c.benchmark_group("transport_addr_clone"); + for len in ADDRESS_BYTES { + let former = vec![0x5a; len]; + let shared = TransportAddr::from_bytes(&former); + group.bench_with_input(BenchmarkId::new("former_vec", len), &former, |b, addr| { + b.iter(|| black_box(addr.clone())) + }); + group.bench_with_input(BenchmarkId::new("shared", len), &shared, |b, addr| { + b.iter(|| black_box(addr.clone())) + }); + } + group.finish(); +} + +fn bench_socket_addr_and_clone(c: &mut Criterion) { + let mut group = c.benchmark_group("transport_addr_socket_and_clone"); + for (name, text) in [ + ("ipv4", "192.168.1.1:2121"), + ("ipv6", "[2001:db8::1]:2121"), + ( + "scoped_ipv6", + "[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff%4294967295]:65535", + ), + ] { + let socket: SocketAddr = text.parse().unwrap(); + group.bench_with_input(BenchmarkId::new("former_vec", name), &socket, |b, addr| { + b.iter(|| { + let addr = former_from_socket_addr(black_box(*addr)); + black_box((addr.clone(), addr)) + }) + }); + group.bench_with_input(BenchmarkId::new("shared", name), &socket, |b, addr| { + b.iter(|| { + let addr = TransportAddr::from_socket_addr(black_box(*addr)); + black_box((addr.clone(), addr)) + }) + }); + } + group.finish(); +} + +fn cross_thread_clone_drop(addr: &T, iterations: u64) -> Duration { + let (tx, rx) = mpsc::sync_channel::(256); + let ready = Barrier::new(2); + std::thread::scope(|scope| { + let consumer = scope.spawn(|| { + ready.wait(); + for value in rx { + drop(black_box(value)); + } + }); + // Thread startup is outside the timed interval; completion is included. + ready.wait(); + let start = Instant::now(); + for _ in 0..iterations { + tx.send(black_box(addr.clone())).unwrap(); + } + drop(tx); + consumer.join().unwrap(); + start.elapsed() + }) +} + +fn bench_cross_thread(c: &mut Criterion) { + let mut group = c.benchmark_group("transport_addr_cross_thread"); + for len in ADDRESS_BYTES { + let former = vec![0x5a; len]; + let shared = TransportAddr::from_bytes(&former); + group.bench_with_input(BenchmarkId::new("former_vec", len), &former, |b, addr| { + b.iter_custom(|iterations| cross_thread_clone_drop(addr, iterations)) + }); + group.bench_with_input(BenchmarkId::new("shared", len), &shared, |b, addr| { + b.iter_custom(|iterations| cross_thread_clone_drop(addr, iterations)) + }); + } + group.finish(); +} + +criterion_group! { + name = benches; + config = Criterion::default().sample_size(50); + targets = bench_clone, bench_socket_addr_and_clone, bench_cross_thread +} +criterion_main!(benches); diff --git a/docs/design/fips-gateway.md b/docs/design/fips-gateway.md index d3530135..34d15f70 100644 --- a/docs/design/fips-gateway.md +++ b/docs/design/fips-gateway.md @@ -458,8 +458,10 @@ sequence is: batch, and a batch too large for any send buffer is refused before it is sent. A batch the kernel refuses leaves the previous table in place; the failure is logged, and the gateway keeps its - record of the change, so the next rebuild that succeeds applies - it. + record of the change. Pending changes are retried every ten seconds, + even when no new mapping event arrives. Each retry rebuilds the latest + desired state, so a newer change supersedes an earlier failed one. + Successful rebuilds clear the pending flag; clean tables are not retried. The rustables crate does not expose rule-handle tracking, so incremental update of individual rules is not available. Atomic diff --git a/docs/design/fips-mesh-layer.md b/docs/design/fips-mesh-layer.md index 0ac4bf16..9d016c77 100644 --- a/docs/design/fips-mesh-layer.md +++ b/docs/design/fips-mesh-layer.md @@ -384,8 +384,9 @@ A rekey is initiated when either threshold is reached on the link's current session: `node.rekey.after_secs` (default 120) elapsed since the link came up or last rekeyed, or `node.rekey.after_messages` (default 65536) frames sent. Either side can be the initiator independently. Rekey is on by -default and can be disabled via `node.rekey.enabled: false` (the -configuration tree is documented in +default. `node.rekey.enabled: false` stops this node initiating rekey, but +that configuration is unsupported: it is a less-tested path, and the option +is removed in v2 (the configuration tree is documented in [../reference/configuration.md](../reference/configuration.md)). ### Mechanism diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 5fd33b2d..b5b91fa9 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -498,7 +498,7 @@ cutover. | Parameter | Type | Default | Description | |-----------|------|---------|-------------| -| `node.rekey.enabled` | bool | `true` | Initiate periodic Noise rekey on links and sessions. A peer-driven session rekey is still answered when this is off, so session keys can still rotate | +| `node.rekey.enabled` | bool | `true` | Initiate periodic Noise rekey on links and sessions. A peer-driven session rekey is still answered when this is off, so session keys can still rotate. Disabling rekey is unsupported: it runs the node on a less-tested path, and the option is removed in v2. | | `node.rekey.after_secs` | u64 | `120` | Initiate rekey after this many seconds on a session | | `node.rekey.after_messages` | u64 | `65536` | Initiate rekey after this many messages sent on a session | @@ -1366,6 +1366,7 @@ node: loss_threshold: 0.05 # MMP loss rate threshold for CE marking (5%) etx_threshold: 3.0 # MMP ETX threshold for CE marking rekey: + # enabled: false is unsupported, less tested, and removed in v2 enabled: true # periodic Noise rekey for forward secrecy after_secs: 120 # rekey interval (seconds) after_messages: 65536 # rekey after N messages sent diff --git a/packaging/README.md b/packaging/README.md index ae948cae..a989409c 100644 --- a/packaging/README.md +++ b/packaging/README.md @@ -393,9 +393,11 @@ pkg add ./fips--pfsense-ce2.8-amd64.pkg /usr/local/libexec/fips/fips-dns-setup # edits config.xml; run deliberately ``` -Not a Netgate-supported package, and a pfSense firmware upgrade removes -it. See [pfsense/README.md](pfsense/README.md) for the "Allow IPv6" -prerequisite the mesh depends on, firewall-rule notes, and removal +Not a Netgate-supported package. A pfSense firmware upgrade keeps it (it +is a plain pkg, not a `pfSense-pkg-*`); after a major base change, +reinstall the package built for the new base. See +[pfsense/README.md](pfsense/README.md) for the "Allow IPv6" prerequisite +the mesh depends on, firewall-rule notes, and upgrade and removal behaviour. ### Windows (`.zip`) diff --git a/packaging/pfsense/build-pkg.sh b/packaging/pfsense/build-pkg.sh index 37efbbda..4840767e 100755 --- a/packaging/pfsense/build-pkg.sh +++ b/packaging/pfsense/build-pkg.sh @@ -24,8 +24,10 @@ # This package integrates through the DNS Resolver custom options. # - The responder's bind address, for the reason recorded in # fips.yaml.dns. -# - Lifetime. A pfSense firmware upgrade reinstalls the base image and -# takes third-party packages with it, so post-install says so. +# - Lifetime. A firmware upgrade keeps the package (pfSense-upgrade +# reinstalls only pfSense-pkg-* packages), but a major upgrade changes +# the FreeBSD base, so post-install says to reinstall the package +# built for the new base. # # Ships fips, fipsctl and fipstop. fips-gateway is excluded: its NAT # backend is nftables (Linux-only), and pfSense has pf for that anyway. diff --git a/packaging/pfsense/fips-dns-setup b/packaging/pfsense/fips-dns-setup index 41b33640..04ed4a42 100755 --- a/packaging/pfsense/fips-dns-setup +++ b/packaging/pfsense/fips-dns-setup @@ -13,10 +13,11 @@ # options" box, which unbound.inc splices into the generated config # verbatim. It is stored base64-encoded in config.xml, which is the part # that makes it the right home: config.xml is what survives a reboot, a -# firmware upgrade and a config restore, whereas everything this package -# installs under /usr/local does not. So the .fips zone keeps resolving -# across an upgrade that removes the daemon, which is a loud failure -# (SERVFAIL on .fips) rather than a quiet one. +# firmware upgrade and a config restore. What this package installs +# under /usr/local survives a firmware upgrade too (pfSense-upgrade +# reinstalls only pfSense-pkg-* packages), but not a removal of the +# package. So the .fips zone can outlive the daemon, which is a loud +# failure (SERVFAIL on .fips) rather than a quiet one. # # This edits the firewall's live configuration, so it is deliberately # NOT run from the package's post-install: installing a package should diff --git a/packaging/systemd/install.sh b/packaging/systemd/install.sh index f71513b7..89e78111 100755 --- a/packaging/systemd/install.sh +++ b/packaging/systemd/install.sh @@ -112,19 +112,20 @@ fi # --- Install systemd units --- -was_active=false -if systemctl is-active --quiet fips.service 2>/dev/null; then - was_active=true - echo "Stopping running fips service..." - systemctl stop fips.service -fi - -dns_was_active=false -if systemctl is-active --quiet fips-dns.service 2>/dev/null; then - dns_was_active=true - echo "Stopping running fips-dns service..." - systemctl stop fips-dns.service -fi +# fips-dns and fips-gateway carry Requires=fips.service, so stopping fips +# stops them too. Record all three before stopping any of them. +declare -A was_active=() +for unit in fips fips-dns fips-gateway; do + if systemctl is-active --quiet "${unit}.service" 2>/dev/null; then + was_active[$unit]=true + fi +done +for unit in fips-gateway fips-dns fips; do + if [ "${was_active[$unit]:-}" = true ]; then + echo "Stopping running ${unit} service..." + systemctl stop "${unit}.service" + fi +done install -m 0644 "${SCRIPT_DIR}/fips.service" "${SYSTEMD_DIR}/fips.service" install -m 0644 "${SCRIPT_DIR}/fips-dns.service" "${SYSTEMD_DIR}/fips-dns.service" @@ -164,15 +165,13 @@ systemctl enable fips.service systemctl enable fips-dns.service echo "Services enabled (will start on boot)." -# Restart if they were running before -if $was_active; then - echo "Restarting fips service..." - systemctl start fips.service -fi -if $dns_was_active; then - echo "Restarting fips-dns service..." - systemctl start fips-dns.service -fi +# Restart each unit that was running before +for unit in fips fips-dns fips-gateway; do + if [ "${was_active[$unit]:-}" = true ]; then + echo "Restarting ${unit} service..." + systemctl start "${unit}.service" + fi +done echo "" echo "=== Installation complete ===" diff --git a/src/bin/fips-gateway.rs b/src/bin/fips-gateway.rs index 3d2b784d..4fa248c7 100644 --- a/src/bin/fips-gateway.rs +++ b/src/bin/fips-gateway.rs @@ -107,6 +107,30 @@ fn report_unreadable_conntrack( } } +/// Log a pending NAT rebuild retry once per change of outcome. +/// +/// A recovery is reported whether the retry applied the rules itself or a +/// mapping change applied them in between and left nothing pending. +#[cfg(target_os = "linux")] +fn report_nat_retry(log: &mut nat::RetryLog, result: &Result) { + match (log.observe(result.as_ref().err()), result) { + (nat::RetryReport::Failed, Err(e)) => { + warn!(error = %e, "Failed to retry pending NAT rules") + } + (nat::RetryReport::Repeated, Err(e)) => { + debug!(error = %e, "Pending NAT rules still failing to apply") + } + (nat::RetryReport::Recovered, Ok(true)) => { + info!("Applied pending NAT rules; retries recovered") + } + (nat::RetryReport::Recovered, _) => { + info!("Pending NAT rules were applied by a later update; retries recovered") + } + (nat::RetryReport::Clean, Ok(true)) => info!("Applied pending NAT rules"), + _ => {} + } +} + /// Check once at startup which conntrack source the tick will read, and say so. /// /// Without this, an operator on a kernel with no readable source learns that @@ -545,8 +569,14 @@ async fn main() { info!("fips-gateway running"); let mut exit_code = 0; + let mut nat_retry = tokio::time::interval(std::time::Duration::from_secs(10)); + nat_retry.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + let mut nat_retry_log = nat::RetryLog::default(); loop { tokio::select! { + _ = nat_retry.tick() => { + report_nat_retry(&mut nat_retry_log, &nat_mgr.retry_pending()); + } Some(event) = event_rx.recv() => { match event { pool::PoolEvent::MappingCreated { virtual_ip, mesh_addr } => { diff --git a/src/gateway/nat.rs b/src/gateway/nat.rs index 7a0d2770..72de3a89 100644 --- a/src/gateway/nat.rs +++ b/src/gateway/nat.rs @@ -174,6 +174,8 @@ pub struct NatManager { mappings: HashMap, /// Inbound port-forward rules. port_forwards: Vec, + /// Desired state has not yet been acknowledged by the kernel. + rebuild_pending: bool, } impl NatManager { @@ -199,6 +201,7 @@ impl NatManager { lan_interface, mappings: HashMap::new(), port_forwards: Vec::new(), + rebuild_pending: false, } } @@ -222,7 +225,7 @@ impl NatManager { /// the nftables table atomically. Pass an empty slice to clear. pub fn set_port_forwards(&mut self, forwards: &[PortForward]) -> Result<(), NatError> { self.port_forwards = forwards.to_vec(); - self.rebuild()?; + self.rebuild_desired()?; info!( count = self.port_forwards.len(), "Applied inbound port forwards" @@ -307,6 +310,18 @@ impl NatManager { self.mappings.len() } + /// Retry a failed rebuild using the latest desired state. + /// + /// Returns whether a pending rebuild was applied. A clean manager does + /// not open a socket or rebuild the table. + pub fn retry_pending(&mut self) -> Result { + if !self.rebuild_pending { + return Ok(false); + } + self.rebuild_desired()?; + Ok(true) + } + /// The objects a rebuild sends, grouped into the batches that carry them. /// /// One batch, always. The kernel applies a batch as a single transaction, @@ -497,12 +512,85 @@ impl NatManager { /// Rebuild, returning the outcome with the time the rebuild took in /// microseconds, so a mapping change can log its cost on either path. - fn timed_rebuild(&self) -> (Result<(), NatError>, u64) { + fn timed_rebuild(&mut self) -> (Result<(), NatError>, u64) { let started = Instant::now(); - let result = self.rebuild(); + let result = self.rebuild_desired(); let elapsed_us = u64::try_from(started.elapsed().as_micros()).unwrap_or(u64::MAX); (result, elapsed_us) } + + /// Rebuild the desired state, leaving it marked pending unless the kernel + /// accepts it. + fn rebuild_desired(&mut self) -> Result<(), NatError> { + self.track_rebuild(Self::rebuild) + } + + /// Run `apply` with the desired state marked pending, and clear the mark + /// only if it succeeds. + /// + /// Split from `rebuild_desired` so a test can drive both outcomes without + /// a netlink socket. + fn track_rebuild( + &mut self, + apply: impl FnOnce(&Self) -> Result<(), NatError>, + ) -> Result<(), NatError> { + self.rebuild_pending = true; + apply(self)?; + self.rebuild_pending = false; + Ok(()) + } +} + +/// How a pending-rebuild retry compares with the retry before it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RetryReport { + /// A failure that differs from the previous retry's outcome, or the first. + Failed, + /// The same failure as the previous retry. + Repeated, + /// The first success after a failed retry. + Recovered, + /// A success with no failed retry before it. + Clean, +} + +/// Remembers the last failed retry of a pending rebuild. +/// +/// A failure that does not clear on its own, for example the kernel refusing +/// the batch while a module or capability is missing, fails every retry the +/// same way, and a per-retry warning would repeat for the life of the +/// process. Warning on a change of outcome, and reporting the recovery once, +/// keeps the log readable. A retry that finds nothing pending after a failure +/// counts as recovery, since a mapping change rebuilt the table in between. +#[derive(Debug, Default)] +pub struct RetryLog { + last_failure: Option, +} + +impl RetryLog { + /// Record a retry outcome and say how it compares with the last one. + /// + /// `None` is a success, whether or not anything was pending; `Some` is + /// the retry's error. Two failures are the same when their messages are. + pub fn observe(&mut self, failure: Option<&NatError>) -> RetryReport { + match (failure.map(ToString::to_string), self.last_failure.take()) { + (Some(now), Some(before)) => { + let report = if now == before { + RetryReport::Repeated + } else { + RetryReport::Failed + }; + self.last_failure = Some(now); + report + } + (Some(now), None) => { + self.last_failure = Some(now); + RetryReport::Failed + } + (None, Some(_)) => RetryReport::Recovered, + (None, None) => RetryReport::Clean, + } + } } /// Largest batch, in bytes, that the kernel can admit in one send. @@ -855,6 +943,13 @@ fn send_batch(bytes: &[u8]) -> Result<(), NatError> { // `SO_SNDBUF` fallback after `SO_SNDBUFFORCE` returns `EPERM` needs a process // without CAP_NET_ADMIN, and the gateway suite's container is privileged, so // nothing runs that either. The gateway suite covers only the success path. +// +// The pending-rebuild flag is covered here on both sides through +// `track_rebuild`, with a stand-in for the rebuild: a failure leaves it set and +// `retry_pending` still retries, and a success clears it so `retry_pending` +// finds nothing to do. What stays only in the ignored kernel test, which no +// suite runs, is `retry_pending` itself applying a pending rebuild and +// returning `Ok(true)`, and a kernel rejection leaving the old table in place. #[cfg(test)] mod tests { use super::*; @@ -883,6 +978,185 @@ mod tests { mgr } + #[test] + fn failed_rebuild_retains_latest_desired_state_for_retry() { + let mut mgr = manager_with_mappings(2); + // Make encoding fail before opening a socket. + mgr.pre_chain = Chain::new(&mgr.table); + assert!(!mgr.retry_pending().unwrap(), "clean retry must not encode"); + + assert!(mgr.add_mapping(vip(3), mesh(3)).is_err()); + assert!(mgr.rebuild_pending); + assert!(mgr.retry_pending().is_err()); + assert!(mgr.rebuild_pending); + assert!(mgr.remove_mapping(vip(3)).is_err()); + assert!(!mgr.mappings.contains_key(&vip(3))); + assert!(mgr.add_mapping(vip(2), mesh(99)).is_err()); + assert_eq!(mgr.mappings[&vip(2)].mesh_addr, mesh(99)); + assert!(mgr.remove_mapping(vip(4)).is_err()); + assert!( + mgr.rebuild_pending, + "an absent mapping must not clear pending work" + ); + } + + #[test] + fn failed_tracked_rebuild_stays_pending_and_retry_still_rebuilds() { + let mut mgr = manager_with_mappings(1); + let refused = || NatError::Nftables("refused".into()); + + assert!(mgr.track_rebuild(|_| Err(refused())).is_err()); + assert!(mgr.rebuild_pending, "a failed rebuild must stay pending"); + + // With encoding made to fail, a retry that is still pending attempts + // the rebuild, and fails, instead of reporting nothing to do. + mgr.pre_chain = Chain::new(&mgr.table); + assert!(mgr.retry_pending().is_err()); + assert!(mgr.rebuild_pending); + } + + #[test] + fn successful_tracked_rebuild_clears_pending_and_retry_does_nothing() { + let mut mgr = manager_with_mappings(1); + assert!( + mgr.track_rebuild(|_| Err(NatError::Nftables("refused".into()))) + .is_err() + ); + assert!(mgr.rebuild_pending); + + let mut applied = 0; + mgr.track_rebuild(|m| { + assert!(m.rebuild_pending, "the rebuild runs with the mark set"); + applied += 1; + Ok(()) + }) + .unwrap(); + assert_eq!(applied, 1); + assert!( + !mgr.rebuild_pending, + "a successful rebuild must clear pending" + ); + + // Encoding would fail, so `Ok(false)` shows the retry did not rebuild. + mgr.pre_chain = Chain::new(&mgr.table); + assert!(!mgr.retry_pending().unwrap()); + } + + #[test] + fn retry_log_warns_on_a_new_failure_and_reports_recovery_once() { + let kernel = |errno| NatError::Kernel { + errno: Errno(errno), + seq: 3, + }; + let mut log = RetryLog::default(); + + assert_eq!(log.observe(None), RetryReport::Clean); + assert_eq!(log.observe(Some(&kernel(22))), RetryReport::Failed); + assert_eq!(log.observe(Some(&kernel(22))), RetryReport::Repeated); + assert_eq!(log.observe(Some(&kernel(22))), RetryReport::Repeated); + assert_eq!( + log.observe(Some(&kernel(1))), + RetryReport::Failed, + "a different failure is a different outcome and is worth a line" + ); + assert_eq!(log.observe(None), RetryReport::Recovered); + assert_eq!(log.observe(None), RetryReport::Clean); + assert_eq!( + log.observe(Some(&kernel(1))), + RetryReport::Failed, + "a failure after recovery warns again" + ); + } + + #[test] + fn failed_port_forward_rebuild_remains_pending() { + let mut mgr = manager_with_mappings(1); + mgr.pre_chain = Chain::new(&mgr.table); + let forward = PortForward { + proto: Proto::Tcp, + listen_port: 8080, + target: SocketAddrV6::new(Ipv6Addr::LOCALHOST, 80, 0, 0), + }; + assert!(mgr.set_port_forwards(&[forward]).is_err()); + assert_eq!(mgr.port_forwards.len(), 1); + assert!(mgr.rebuild_pending); + assert!(mgr.set_port_forwards(&[]).is_err()); + assert!(mgr.port_forwards.is_empty()); + assert!(mgr.rebuild_pending); + } + + #[test] + #[ignore = "requires CAP_NET_ADMIN and nft in an isolated network namespace"] + fn kernel_rejection_retries_latest_state_without_another_mapping_event() { + let mut mgr = manager_with_mappings(2); + let table_name = format!("{TABLE_NAME}_retry_test_{}", std::process::id()); + mgr.table = Table::new(ProtocolFamily::Inet).with_name(&table_name); + mgr.pre_chain = Chain::new(&mgr.table) + .with_name(PREROUTING_CHAIN) + .with_type(ChainType::Nat) + .with_hook(Hook::new(HookClass::PreRouting, DSTNAT_PRIORITY)); + mgr.post_chain = Chain::new(&mgr.table) + .with_name(POSTROUTING_CHAIN) + .with_type(ChainType::Nat) + .with_hook(Hook::new(HookClass::PostRouting, SRCNAT_PRIORITY)); + mgr.rebuild().unwrap(); + let listing = || { + let output = std::process::Command::new("nft") + .args(["-j", "list", "table", "inet", &table_name]) + .output() + .unwrap(); + assert!(output.status.success()); + output.stdout + }; + let before = listing(); + // Encoding succeeds, but the kernel rejects an overlong chain name. + let invalid = Chain::new(&mgr.table).with_name("x".repeat(300)); + let original = std::mem::replace(&mut mgr.pre_chain, invalid); + assert!(matches!( + mgr.add_mapping(vip(3), mesh(3)), + Err(NatError::Kernel { .. }) + )); + assert!(mgr.remove_mapping(vip(1)).is_err()); + assert!(mgr.remove_mapping(vip(3)).is_err()); + assert!(mgr.add_mapping(vip(2), mesh(99)).is_err()); + assert!(mgr.retry_pending().is_err()); + assert_eq!( + listing(), + before, + "rejection leaves the old kernel table intact" + ); + + mgr.pre_chain = original; + assert!(mgr.retry_pending().unwrap()); + assert!(!mgr.retry_pending().unwrap()); + assert_eq!(mgr.mapping_count(), 1); + assert_eq!(mgr.mappings[&vip(2)].mesh_addr, mesh(99)); + let after = listing(); + assert_ne!(after, before); + let table: serde_json::Value = serde_json::from_slice(&after).unwrap(); + assert_eq!( + table["nftables"] + .as_array() + .unwrap() + .iter() + .filter(|entry| entry.get("rule").is_some()) + .count(), + 3 + ); + assert!(String::from_utf8(after).unwrap().contains("fd02::63")); + + // A successful normal update also clears earlier pending work. + mgr.pre_chain = Chain::new(&mgr.table); + assert!(mgr.add_mapping(vip(4), mesh(4)).is_err()); + mgr.pre_chain = Chain::new(&mgr.table) + .with_name(PREROUTING_CHAIN) + .with_type(ChainType::Nat) + .with_hook(Hook::new(HookClass::PreRouting, DSTNAT_PRIORITY)); + mgr.remove_mapping(vip(4)).unwrap(); + assert!(!mgr.retry_pending().unwrap()); + mgr.cleanup().unwrap(); + } + #[test] fn rebuild_deletes_and_recreates_the_table_inside_one_batch() { let batches = manager_with_mappings(3).rebuild_batches(); diff --git a/src/testutil.rs b/src/testutil.rs index 65874819..f55e42e8 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -19,6 +19,11 @@ pub(crate) fn make_node_addr(val: u8) -> NodeAddr { pub(crate) struct LogCapture(std::sync::Arc>>); impl LogCapture { + /// Every captured line, each prefixed with its level. + pub(crate) fn lines(&self) -> Vec { + self.0.lock().unwrap().clone() + } + /// Only the captured lines emitted at WARN. pub(crate) fn warnings(&self) -> Vec { self.0 diff --git a/src/transport/ethernet/io_linux.rs b/src/transport/ethernet/io_linux.rs index cc0bae0c..5c26be69 100644 --- a/src/transport/ethernet/io_linux.rs +++ b/src/transport/ethernet/io_linux.rs @@ -1,7 +1,7 @@ //! AF_PACKET socket creation, binding, and ioctl helpers (Linux). use crate::transport::TransportError; -use std::os::unix::io::{AsRawFd, RawFd}; +use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; /// Wrapper around an AF_PACKET SOCK_DGRAM file descriptor. /// @@ -39,6 +39,9 @@ impl PacketSocket { err ))); } + // SAFETY: socket returned a new descriptor, owned only by this guard + // until construction succeeds. Every setup error closes it on drop. + let owned_fd = unsafe { OwnedFd::from_raw_fd(fd) }; // Look up interface index let if_index = get_if_index(fd, interface)?; @@ -58,7 +61,6 @@ impl PacketSocket { }; if ret < 0 { let err = std::io::Error::last_os_error(); - unsafe { libc::close(fd) }; // The interface can disappear between the index lookup and the // bind. That is absence arriving a few microseconds late, not a // configuration fault, so it reports as absence. @@ -77,7 +79,6 @@ impl PacketSocket { let flags = unsafe { libc::fcntl(fd, libc::F_GETFL) }; if flags < 0 { let err = std::io::Error::last_os_error(); - unsafe { libc::close(fd) }; return Err(TransportError::StartFailed(format!( "fcntl(F_GETFL) failed: {}", err @@ -86,7 +87,6 @@ impl PacketSocket { let ret = unsafe { libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) }; if ret < 0 { let err = std::io::Error::last_os_error(); - unsafe { libc::close(fd) }; return Err(TransportError::StartFailed(format!( "fcntl(F_SETFL, O_NONBLOCK) failed: {}", err @@ -94,7 +94,7 @@ impl PacketSocket { } Ok(Self { - fd, + fd: owned_fd.into_raw_fd(), if_index, ethertype, }) @@ -354,3 +354,69 @@ fn get_if_mtu(fd: RawFd, if_index: i32) -> Result { let mtu = unsafe { ifr.ifr_ifru.ifru_mtu } as u16; Ok(mtu) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn invalid_interface_does_not_leak_socket() { + const CHILD: &str = "FIPS_TEST_PACKET_SOCKET_CHILD"; + if std::env::var_os(CHILD).is_none() { + if PacketSocket::open("lo", 0x88b5).is_err() { + // Unprivileged: every open fails at socket() before the paths + // under test. Loud on a runner that claims privilege, so this + // cannot go quiet. + assert!( + std::env::var_os("FIPS_TEST_PRIVILEGED").is_none(), + "this runner declares FIPS_TEST_PRIVILEGED but cannot open a \ + raw socket on lo: the descriptor leak check went unexercised" + ); + return; + } + // Count descriptors in a separate process so concurrently running + // tests cannot affect the result. + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "transport::ethernet::io::platform::tests::invalid_interface_does_not_leak_socket", + "--include-ignored", + "--nocapture", + ]) + .env(CHILD, "1") + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "{stdout}\n{stderr}"); + // libtest exits 0 when the filter selects nothing, so require + // that the child actually ran this test. + assert!( + stdout.contains("1 passed"), + "child did not run the test:\n{stdout}\n{stderr}" + ); + return; + } + + let open_fds = || std::fs::read_dir("/proc/self/fd").unwrap().count(); + let before = open_fds(); + for interface in ["fips-missing-interface", "invalid\0interface"] { + for _ in 0..8 { + let result = PacketSocket::open(interface, 0x88b5); + let expected = match &result { + Err(TransportError::InterfaceUnavailable { .. }) => true, + Err(TransportError::StartFailed(message)) => { + message.starts_with("invalid interface name:") + } + _ => false, + }; + assert!( + expected, + "unexpected result for {interface:?}: {:?}", + result.as_ref().map(|_| "PacketSocket") + ); + } + } + assert_eq!(open_fds(), before, "failed setup leaked a socket"); + } +} diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 81a25fee..585508ac 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -483,9 +483,12 @@ impl TransportAddr { /// Create a UDP/TCP transport address directly from a socket address. pub fn from_socket_addr(addr: std::net::SocketAddr) -> Self { use std::io::Write; - let mut buf = Vec::with_capacity(56); - write!(&mut buf, "{addr}").expect("Vec::write_fmt is infallible"); - Self::new(buf) + // The longest form is 58 bytes: [39-byte IPv6%10-digit scope]:65535. + let mut buf = [0u8; 64]; + let mut cursor = &mut buf[..]; + write!(&mut cursor, "{addr}").expect("64 bytes is enough for any SocketAddr"); + let len = 64 - cursor.len(); + Self::from_bytes(&buf[..len]) } } @@ -1269,28 +1272,23 @@ impl TransportHandle { pub(crate) async fn resolve_socket_addr( addr: &TransportAddr, ) -> Result { + resolve_socket_addrs(addr).await?.next().ok_or_else(|| { + TransportError::InvalidAddress(format!("DNS resolution returned no addresses for {}", addr)) + }) +} + +/// Resolve every socket address in resolver order, bypassing DNS for numeric IPs. +pub(crate) async fn resolve_socket_addrs( + addr: &TransportAddr, +) -> Result, TransportError> { let s = addr .as_str() .ok_or_else(|| TransportError::InvalidAddress("not valid UTF-8".into()))?; - // Fast path: numeric IP address — no DNS lookup - if let Ok(sock_addr) = s.parse::() { - return Ok(sock_addr); - } - - // Slow path: DNS resolution - tokio::net::lookup_host(s) - .await - .map_err(|e| { - TransportError::InvalidAddress(format!("DNS resolution failed for {}: {}", s, e)) - })? - .next() - .ok_or_else(|| { - TransportError::InvalidAddress(format!( - "DNS resolution returned no addresses for {}", - s - )) - }) + // lookup_host handles numeric addresses without allocating or querying DNS. + tokio::net::lookup_host(s).await.map_err(|e| { + TransportError::InvalidAddress(format!("DNS resolution failed for {}: {}", s, e)) + }) } // ============================================================================ @@ -1400,11 +1398,28 @@ mod tests { #[test] fn test_transport_addr_from_socket_addr() { - let addr = TransportAddr::from_socket_addr("127.0.0.1:2121".parse().unwrap()); - assert_eq!(addr.as_str(), Some("127.0.0.1:2121")); - - let addr = TransportAddr::from_socket_addr("[::1]:2121".parse().unwrap()); - assert_eq!(addr.as_str(), Some("[::1]:2121")); + for text in [ + "0.0.0.0:0", + "127.0.0.1:2121", + "255.255.255.255:65535", + "[::1]:2121", + "[2001:db8::1]:2121", + "[::ffff:255.255.255.255]:65535", + "[fe80::1%4294967295]:65535", + "[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff%4294967295]:65535", + ] { + let socket: std::net::SocketAddr = text.parse().unwrap(); + let addr = TransportAddr::from_socket_addr(socket); + assert_eq!(addr.as_str(), Some(text)); + assert_eq!(addr.as_bytes(), socket.to_string().as_bytes()); + assert_eq!( + addr.as_str() + .unwrap() + .parse::() + .unwrap(), + socket + ); + } } #[test] diff --git a/src/transport/tcp/mod.rs b/src/transport/tcp/mod.rs index 4b6c67bd..82450626 100644 --- a/src/transport/tcp/mod.rs +++ b/src/transport/tcp/mod.rs @@ -25,7 +25,7 @@ mod pool; pub mod stats; -use super::resolve_socket_addr; +use super::resolve_socket_addrs; use super::{ ConnectionState, DiscoveredPeer, PacketTx, ReceivedPacket, Transport, TransportAddr, TransportError, TransportId, TransportState, TransportType, @@ -388,25 +388,22 @@ impl TcpTransport { /// Configures socket options, reads TCP_MAXSEG for MTU, splits the /// stream, spawns a receive task, and stores in the pool. async fn connect(&self, addr: &TransportAddr) -> Result>, TransportError> { - let socket_addr = resolve_socket_addr(addr).await?; + let socket_addrs: Vec<_> = resolve_socket_addrs(addr).await?.collect(); let timeout_ms = self.config.connect_timeout_ms(); - // Connect with timeout - let stream = match tokio::time::timeout( - Duration::from_millis(timeout_ms), - TcpStream::connect(socket_addr), - ) - .await - { - Ok(Ok(stream)) => stream, - Ok(Err(_)) => { + let connected = + connect_to_any_addr(self.transport_id, addr, &socket_addrs, timeout_ms).await; + let stream = match connected { + Ok(stream) => stream, + Err(error @ TransportError::ConnectionRefused) => { self.stats.record_connect_refused(); - return Err(TransportError::ConnectionRefused); + return Err(error); } - Err(_) => { + Err(error @ TransportError::Timeout) => { self.stats.record_connect_timeout(); - return Err(TransportError::Timeout); + return Err(error); } + Err(error) => return Err(error), }; // Configure socket options via socket2 @@ -557,10 +554,8 @@ impl TcpTransport { } // Validate address is UTF-8 before spawning (fail fast on bad input) - let addr_string = addr - .as_str() - .ok_or_else(|| TransportError::InvalidAddress("not valid UTF-8".into()))? - .to_string(); + addr.as_str() + .ok_or_else(|| TransportError::InvalidAddress("not valid UTF-8".into()))?; let timeout_ms = self.config.connect_timeout_ms(); let config = self.config.clone(); let transport_id = self.transport_id; @@ -575,51 +570,22 @@ impl TcpTransport { let task = tokio::spawn(async move { // Resolve address (may involve DNS for hostnames) - let socket_addr: SocketAddr = if let Ok(sa) = addr_string.parse() { - sa - } else { - tokio::net::lookup_host(&addr_string) - .await - .map_err(|e| { - TransportError::InvalidAddress(format!( - "DNS resolution failed for {}: {}", - addr_string, e - )) - })? - .next() - .ok_or_else(|| { - TransportError::InvalidAddress(format!( - "DNS resolution returned no addresses for {}", - addr_string - )) - })? - }; + let socket_addrs: Vec<_> = resolve_socket_addrs(&remote_addr).await?.collect(); - // Connect with timeout - let stream = match tokio::time::timeout( - Duration::from_millis(timeout_ms), - TcpStream::connect(socket_addr), - ) - .await - { - Ok(Ok(stream)) => stream, - Ok(Err(e)) => { - debug!( - transport_id = %transport_id, - remote_addr = %remote_addr, - error = %e, - "Background TCP connect refused" - ); - return Err(TransportError::ConnectionRefused); - } - Err(_) => { + // A refusal is logged with its OS error by connect_to_any_addr. + let connected = + connect_to_any_addr(transport_id, &remote_addr, &socket_addrs, timeout_ms).await; + let stream = match connected { + Ok(stream) => stream, + Err(error @ TransportError::Timeout) => { debug!( transport_id = %transport_id, remote_addr = %remote_addr, "Background TCP connect timed out" ); - return Err(TransportError::Timeout); + return Err(error); } + Err(error) => return Err(error), }; // Configure socket options via socket2 @@ -1280,6 +1246,43 @@ async fn tcp_receive_loop( // Socket Configuration Helpers // ============================================================================ +/// Connect to the first reachable of a peer's resolved addresses within one timeout. +async fn connect_to_any_addr( + transport_id: TransportId, + remote_addr: &TransportAddr, + socket_addrs: &[SocketAddr], + timeout_ms: u64, +) -> Result { + if socket_addrs.is_empty() { + return Err(TransportError::InvalidAddress(format!( + "DNS resolution returned no addresses for {}", + remote_addr + ))); + } + + // Try candidates in resolver order within one overall connection timeout. + match tokio::time::timeout( + Duration::from_millis(timeout_ms), + TcpStream::connect(socket_addrs), + ) + .await + { + Ok(Ok(stream)) => Ok(stream), + Ok(Err(error)) => { + // Tokio returns the last candidate's error; keep it in the log, + // since the returned variant does not carry it. + debug!( + transport_id = %transport_id, + remote_addr = %remote_addr, + error = %error, + "TCP connect failed" + ); + Err(TransportError::ConnectionRefused) + } + Err(_) => Err(TransportError::Timeout), + } +} + /// Configure a TCP socket with the transport's settings. fn configure_socket( stream: &std::net::TcpStream, @@ -1432,6 +1435,130 @@ mod tests { } } + #[tokio::test] + async fn test_connect_tries_later_candidates() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let good_addr = listener.local_addr().unwrap(); + + // No listener can bind port zero: binding it allocates an ephemeral port. + let bad_addr = "127.0.0.1:0".parse().unwrap(); + + let remote = TransportAddr::from_string(&format!("localhost:{}", good_addr.port())); + let stream = + connect_to_any_addr(TransportId::new(1), &remote, &[bad_addr, good_addr], 1_000) + .await + .expect("second TCP candidate should connect"); + let (accepted, _) = timeout(Duration::from_secs(1), listener.accept()) + .await + .unwrap() + .unwrap(); + assert_eq!(stream.peer_addr().unwrap(), good_addr); + assert_eq!(accepted.peer_addr().unwrap(), stream.local_addr().unwrap()); + } + + #[tokio::test] + async fn test_connect_candidates_fail() { + let id = TransportId::new(1); + let remote = TransportAddr::from_string("localhost:0"); + assert!(matches!( + connect_to_any_addr(id, &remote, &[], 1_000).await, + Err(TransportError::InvalidAddress(ref message)) if message.ends_with("localhost:0") + )); + assert!(matches!( + connect_to_any_addr(id, &remote, &["127.0.0.1:0".parse().unwrap()], 1_000).await, + Err(TransportError::ConnectionRefused) + )); + } + + #[tokio::test] + async fn test_connect_failure_logs_os_error_with_peer() { + let remote = TransportAddr::from_string("localhost:0"); + let refused = ["127.0.0.1:0".parse().unwrap()]; + // Register the log callsite before installing the capture. Other + // tests reach it in parallel, and one that registers it first while + // this capture is the only subscriber caches its own thread's "never" + // interest; installing the capture rebuilds already-registered + // callsites, so registering first makes the capture see the event. + let _ = connect_to_any_addr(TransportId::new(7), &remote, &refused, 1_000).await; + let (logs, guard) = crate::testutil::capture_logs_scoped(); + let result = connect_to_any_addr(TransportId::new(7), &remote, &refused, 1_000).await; + drop(guard); + + assert!(matches!(result, Err(TransportError::ConnectionRefused))); + // The returned variant always reads "connection refused"; the log + // line must carry the OS error and the peer instead. + let lines = logs.lines(); + assert!( + lines.iter().any(|line| line.starts_with("DEBUG") + && line.contains("TCP connect failed") + && line.contains("transport_id=transport:7") + && line.contains("remote_addr=localhost:0") + && line.contains("os error")), + "expected a debug line with the OS error and peer, got {lines:?}", + ); + } + + async fn send_to_hostname(background: bool) { + // Prefer the last localhost address so dual-stack hosts also + // exercise fallback through the full transport connection path. + let mut addresses: Vec<_> = tokio::net::lookup_host("localhost:0") + .await + .unwrap() + .collect(); + addresses.reverse(); + let listener = TcpListener::bind(addresses.as_slice()).await.unwrap(); + let (tx, _rx) = packet_channel(10); + let config = make_outbound_config(); + // A refused localhost connection can take over two seconds on Windows. + // Allow the configured connection timeout, plus DNS/scheduling margin. + let connect_deadline = + Duration::from_millis(config.connect_timeout_ms()) + Duration::from_secs(2); + let mut sender = TcpTransport::new(TransportId::new(1), None, config, tx); + sender.start_async().await.unwrap(); + let remote = TransportAddr::from_string(&format!( + "localhost:{}", + listener.local_addr().unwrap().port() + )); + + if background { + sender.connect_async(&remote).await.unwrap(); + assert!( + wait_until( + || sender.connection_state_sync(&remote) == ConnectionState::Connected, + connect_deadline, + ) + .await + ); + } + timeout( + connect_deadline, + sender.send_async(&remote, &build_msg1_frame()), + ) + .await + .expect("hostname send exceeded connection deadline") + .unwrap(); + let (mut stream, _) = timeout(Duration::from_secs(2), listener.accept()) + .await + .unwrap() + .unwrap(); + let packet = timeout(Duration::from_secs(2), read_fmp_packet(&mut stream, 1400)) + .await + .unwrap() + .unwrap(); + assert_eq!(packet, build_msg1_frame()); + sender.stop_async().await.unwrap(); + } + + #[tokio::test] + async fn test_connect_on_send_hostname() { + send_to_hostname(false).await; + } + + #[tokio::test] + async fn test_connect_async_hostname() { + send_to_hostname(true).await; + } + fn make_outbound_config() -> TcpConfig { TcpConfig { bind_addr: None, diff --git a/src/transport/types.rs b/src/transport/types.rs index 4e95f4ce..7557279c 100644 --- a/src/transport/types.rs +++ b/src/transport/types.rs @@ -8,6 +8,7 @@ //! via `pub use types::*`, so existing `crate::transport::{LinkId, ...}` //! imports are unaffected. +use alloc::{string::String, sync::Arc, vec::Vec}; use core::fmt; use core::time::Duration; @@ -91,23 +92,28 @@ impl fmt::Display for LinkDirection { /// Each transport type interprets this differently: /// - UDP/TCP: "host:port" (IP address or DNS hostname) /// - Ethernet: MAC address (6 bytes) +/// +/// The immutable bytes are shared across clones. #[derive(Clone, PartialEq, Eq, Hash)] -pub struct TransportAddr(Vec); +pub struct TransportAddr(Arc<[u8]>); impl TransportAddr { /// Create a transport address from raw bytes. + /// + /// Copies the bytes into shared storage; the vector's allocation is not + /// reused. Prefer [`Self::from_bytes`] when a byte slice is already available. pub fn new(bytes: Vec) -> Self { - Self(bytes) + Self(bytes.into()) } /// Create a transport address from a byte slice. pub fn from_bytes(bytes: &[u8]) -> Self { - Self(bytes.to_vec()) + Self(Arc::from(bytes)) } /// Create a transport address from a string. pub fn from_string(s: &str) -> Self { - Self(s.as_bytes().to_vec()) + Self::from_bytes(s.as_bytes()) } /// Get the raw bytes. @@ -158,7 +164,7 @@ impl fmt::Display for TransportAddr { Ok(()) } None => { - for byte in &self.0 { + for byte in self.0.iter() { write!(f, "{:02x}", byte)?; } Ok(()) @@ -175,7 +181,36 @@ impl From<&str> for TransportAddr { impl From for TransportAddr { fn from(s: String) -> Self { - Self(s.into_bytes()) + Self::from_string(&s) + } +} + +#[cfg(test)] +mod tests { + use super::TransportAddr; + + #[test] + fn transport_addr_clone_shares_immutable_bytes() { + let addr = TransportAddr::from_string("192.168.1.1:2121"); + let cloned = addr.clone(); + + assert_eq!(addr, cloned); + assert_eq!(addr.as_bytes().as_ptr(), cloned.as_bytes().as_ptr()); + } + + #[test] + fn transport_addr_equality_and_hash_use_byte_values() { + // Only tests use std; the transport primitives remain alloc-only. + use std::collections::HashSet; + + let original = TransportAddr::from_string("192.168.1.1:2121"); + let same_value = TransportAddr::from_bytes(b"192.168.1.1:2121"); + let mut addrs = HashSet::new(); + + assert_ne!(original.as_bytes().as_ptr(), same_value.as_bytes().as_ptr()); + assert_eq!(original, same_value); + addrs.insert(original); + assert!(addrs.contains(&same_value)); } } diff --git a/testing/README.md b/testing/README.md index 779d0d67..f1d96569 100644 --- a/testing/README.md +++ b/testing/README.md @@ -110,6 +110,14 @@ what the real `build-apk.sh` and `build-ipk.sh` package. No router, opkg or apk-tools is involved. Part of both CI runners as `openwrt-scripts`. +### [tarball-install/](tarball-install/) -- systemd Tarball Upgrade + +Runs the systemd tarball's `install.sh` twice in a Debian 12 systemd +container, the second time as an upgrade, with stub binaries. Checks +that the units running before the upgrade (fips, fips-dns and +fips-gateway, in three combinations) are the ones running after it. +Part of both CI runners as `tarball-install`. + ### [native-api/](native-api/) -- Native Datagram API Checks the experimental native datagram API: a client process opens a diff --git a/testing/check-deb-version.sh b/testing/check-deb-version.sh new file mode 100755 index 00000000..a18b747b --- /dev/null +++ b/testing/check-deb-version.sh @@ -0,0 +1,175 @@ +#!/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 +# +maint.. +# +# 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 diff --git a/testing/ci-local.sh b/testing/ci-local.sh index d985b144..07b573ae 100755 --- a/testing/ci-local.sh +++ b/testing/ci-local.sh @@ -224,6 +224,7 @@ CHAOS_SUITES=( # testing/chaos/scripts/chaos.sh bloom-storm. GATEWAY_SUITES=(gateway) OPENWRT_SUITES=(openwrt-scripts) +TARBALL_INSTALL_SUITES=(tarball-install) SIDECAR_SUITES=(sidecar) FIREWALL_SUITES=(firewall) IFACE_BINDING_SUITES=(iface-binding) @@ -266,6 +267,9 @@ list_suites() { echo " OpenWrt packaging:" for s in "${OPENWRT_SUITES[@]}"; do echo " $s"; done echo "" + echo " systemd tarball:" + for s in "${TARBALL_INSTALL_SUITES[@]}"; do echo " $s"; done + echo "" echo " Firewall baseline:" for s in "${FIREWALL_SUITES[@]}"; do echo " $s"; done echo "" @@ -1402,6 +1406,15 @@ run_integration() { return fi + # Also ahead of the build context, for the same reason: the tarball + # install scenarios use stub binaries and their own systemd image. + if [[ -z "$ONLY_SUITE" ]]; then + run_tarball_install + elif [[ "$ONLY_SUITE" == "tarball-install" ]]; then + run_tarball_install + return + fi + # Populate THIS run's build context, then install the binaries into it. # Everything but the binaries is copied from the tracked context directory; # the binaries are installed fresh, and a previous run's are deliberately @@ -1576,6 +1589,8 @@ run_suite() { run_gateway ;; openwrt-scripts) run_openwrt_scripts ;; + tarball-install) + run_tarball_install ;; firewall) run_firewall ;; iface-binding) @@ -1676,6 +1691,18 @@ run_openwrt_scripts() { return $rc } +# The systemd tarball's install.sh, run as an upgrade under real systemd with +# stub binaries: the units that were running before the upgrade must be the +# ones running after it, including fips-dns and fips-gateway, which stop with +# fips through Requires=. +run_tarball_install() { + local rc=0 + info "[tarball-install] Running the systemd tarball upgrade scenarios" + bash "$SCRIPT_DIR/tarball-install/test.sh" || rc=$? + record "tarball-install" $rc + return $rc +} + run_ci_parity() { local rc=0 info "[ci-parity] Comparing the local suite set against the GitHub matrix" @@ -1755,6 +1782,17 @@ 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() { + 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 +} + # ── Main ─────────────────────────────────────────────────────────────────── main() { @@ -1776,6 +1814,7 @@ main() { run_action_pins run_comment_refs run_wait_converge + run_deb_version if [[ "$TEST_ONLY" == true ]]; then run_tests diff --git a/testing/interop/interop-test.sh b/testing/interop/interop-test.sh index 913518fe..857e4b59 100755 --- a/testing/interop/interop-test.sh +++ b/testing/interop/interop-test.sh @@ -574,14 +574,19 @@ count_node_pattern() { docker logs "${CONTAINER[$node]}" 2>&1 | grep -cE "$pattern" || true } +# Wait until the count of a pattern across all node logs reaches +# min_count. Returns 0 when it does, 1 on timeout, and 2 at once when a +# node's logs cannot be read, since no count is established then; the +# caller reports that when it takes its own count. wait_for_log_pattern_count() { local pattern="$1" min_count="$2" timeout="$3" - local start=$SECONDS - while (( SECONDS - start < timeout )); do - [ "$(count_log_pattern "$pattern")" -ge "$min_count" ] && return 0 + local start=$SECONDS count + while :; do + count="$(count_log_pattern "$pattern")" || return 2 + [ "$count" -ge "$min_count" ] && return 0 + (( SECONDS - start < timeout )) || return 1 sleep "$LOG_POLL_INTERVAL" done - [ "$(count_log_pattern "$pattern")" -ge "$min_count" ] } # One node's bloom-union mesh-size estimate, or "null" when the daemon @@ -866,8 +871,11 @@ PASSED=0; FAILED=0 wait_for_log_pattern_count \ "Rekey cutover complete \(initiator\), K-bit flipped" 1 \ "$FIRST_REKEY_TIMEOUT" || true -fmp_cutovers="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit flipped')" -if [ "$fmp_cutovers" -ge 1 ]; then +if ! fmp_cutovers="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit flipped')"; then + echo " FAIL FMP rekey cutovers: node logs unreadable ($fmp_cutovers), not established" + FAILED=$((FAILED + 1)) + INTEROP_FAILURES+=("[log] FMP rekey cutovers: node logs unreadable ($fmp_cutovers)") +elif [ "$fmp_cutovers" -ge 1 ]; then echo " PASS FMP rekey initiator cutovers: $fmp_cutovers" PASSED=$((PASSED + 1)) else @@ -897,10 +905,17 @@ echo "Phase 4: Second rekey cycle (waiting up to ${SECOND_REKEY_WAIT}s for the n # the same pre/post cutover-count delta convention as the control window # (Phase 1b), instead of a blind sleep. Bounded by SECOND_REKEY_WAIT so a # stalled rekey falls through to the strict Phase 5/6 assertions. -fmp_cutovers_before="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit flipped')" -wait_for_log_pattern_count \ - "Rekey cutover complete \(initiator\), K-bit flipped" \ - "$((fmp_cutovers_before + 1))" "$SECOND_REKEY_WAIT" || true +# An unreadable baseline gives nothing to wait beyond, so do not wait. +# Phase 4 keeps no PASSED/FAILED of its own; the INTEROP_FAILURES entry +# is what turns the run red. +if fmp_cutovers_before="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit flipped')"; then + wait_for_log_pattern_count \ + "Rekey cutover complete \(initiator\), K-bit flipped" \ + "$((fmp_cutovers_before + 1))" "$SECOND_REKEY_WAIT" || true +else + echo " FAIL second rekey cycle: node logs unreadable ($fmp_cutovers_before), baseline not established" + INTEROP_FAILURES+=("[log] second rekey cycle: node logs unreadable ($fmp_cutovers_before)") +fi echo "" echo "Phase 5: Post-second-rekey connectivity (reconverge within ${POST_REKEY_TIMEOUT}s)" @@ -1015,16 +1030,22 @@ done # Positive checks — the rekey machinery actually exercised both layers. echo "" echo " -- Rekey machinery exercised --" -fmp_total="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit flipped')" -fsp_total="$(count_log_pattern 'FSP rekey cutover complete')" -if [ "$fmp_total" -ge 1 ]; then +# An unreadable log means the harness observed nothing, so it is a FAIL +# on either layer; the FSP WARN below is for a readable zero only. +if ! fmp_total="$(count_log_pattern 'Rekey cutover complete \(initiator\), K-bit flipped')"; then + echo " FAIL FMP rekey cutovers: node logs unreadable ($fmp_total), not established" + FAILED=$((FAILED + 1)) +elif [ "$fmp_total" -ge 1 ]; then echo " PASS FMP rekey cutovers across mesh: $fmp_total" PASSED=$((PASSED + 1)) else echo " FAIL FMP rekey cutovers: $fmp_total (expected >= 1)" FAILED=$((FAILED + 1)) fi -if [ "$fsp_total" -ge 1 ]; then +if ! fsp_total="$(count_log_pattern 'FSP rekey cutover complete')"; then + echo " FAIL FSP rekey cutovers: node logs unreadable ($fsp_total), not established" + FAILED=$((FAILED + 1)) +elif [ "$fsp_total" -ge 1 ]; then echo " PASS FSP rekey cutovers across mesh: $fsp_total" PASSED=$((PASSED + 1)) else diff --git a/testing/tarball-install/test.sh b/testing/tarball-install/test.sh new file mode 100755 index 00000000..e3474095 --- /dev/null +++ b/testing/tarball-install/test.sh @@ -0,0 +1,367 @@ +#!/bin/bash +# Test the systemd tarball's install.sh as an upgrade, under real systemd. +# +# fips-dns.service and fips-gateway.service carry Requires=fips.service, so +# stopping fips stops both. An upgrade that checks which units are running +# only after stopping fips sees the other two as already stopped, and starts +# fips alone again. This suite installs the tarball once, puts the units in a +# known state, runs install.sh a second time (the upgrade), and checks that +# exactly the units that were running before are running after. +# +# The tarball is staged from the real packaging/systemd/install.sh and unit +# files, and the real packaging/common configuration, the same file set +# build-tarball.sh ships. The binaries and DNS helpers are stubs: the fips and +# fips-gateway stubs sleep, so their units stay active, and the DNS helpers +# exit 0. What is under test is install.sh against real units and a real +# systemd, which carries the Requires= stop propagation; nothing here depends +# on what the binaries do. +# +# Scenarios, each on a fresh container: +# all-active fips, fips-dns and fips-gateway running before the upgrade. +# All three must be running after it, and fips must have been +# restarted (its InvocationID changes). +# fips-only only fips running. fips must be running after, and the other +# two must still be stopped. +# none-active nothing running. Nothing may be running after. +# +# Usage: ./test.sh [--install-sh PATH] [scenario ...] +# No scenarios = run all of them. +# --install-sh PATH runs another install.sh in place of the tree's, so the +# suite can be pointed at an older script without editing the tree. +# +# Requirements: Docker able to grant SYS_ADMIN and NET_ADMIN and an unconfined +# AppArmor profile (the container is not privileged; see +# testing/lib/systemd-container.sh), and /dev/net/tun on the host. +# +# Exit 0 only when every check passed. A container that cannot boot or be +# inspected is a FAIL, never a skip. + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# shellcheck source=SCRIPTDIR/../lib/systemd-container.sh +source "$SCRIPT_DIR/../lib/systemd-container.sh" +# shellcheck source=SCRIPTDIR/../lib/image-build.sh +source "$SCRIPT_DIR/../lib/image-build.sh" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" + +IMAGE="fips-tarball-test:debian12" +BOOT_TIMEOUT=60 +# Bounds one install.sh run. It starts at most three units, and the gateway's +# ExecStartPre returns at once because fips0 exists before the first start. +INSTALL_TIMEOUT=120 +UNIT_START_TIMEOUT=60 +TARBALL_DIR=/opt/fips-tarball + +PASS=0 +FAIL=0 + +INSTALL_SH="$REPO_ROOT/packaging/systemd/install.sh" +STAGE="" + +log() { echo "=== $*"; } +pass() { echo " PASS: $*"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $*"; FAIL=$((FAIL + 1)); } + +# Remove a container, ignoring one that does not exist. +cleanup_container() { + local name="$1" + docker rm -f "$name" >/dev/null 2>&1 || true +} + +# Remove every scenario's container and the staged tarball. +cleanup_all() { + local s + for s in $ALL_SCENARIOS; do + cleanup_container "$(container_name "$s")" + done + [ -n "$STAGE" ] && rm -rf "$STAGE" +} + +# The container name for a scenario, scoped to this CI run. +container_name() { + echo "fips-tarball-test-$1${FIPS_CI_NAME_SUFFIX:-}" + return 0 +} + +# Build the systemd image. The build context is an empty directory: the +# tarball is copied into the container after it starts, not baked in. +build_image() { + local ctx rc=0 + ctx=$(mktemp -d) + retry_build "docker build -t $IMAGE" build_inline "$IMAGE" "$(cat <<'DOCKERFILE' +FROM debian:12 +ENV DEBIAN_FRONTEND=noninteractive +RUN apt-get update && apt-get install -y --no-install-recommends \ + systemd iproute2 procps && \ + apt-get clean && rm -rf /var/lib/apt/lists/* +CMD ["/lib/systemd/systemd"] +DOCKERFILE + )" "$ctx" || rc=$? + rm -rf "$ctx" + return "$rc" +} + +# Stage a tarball directory: the real install script, units and configuration, +# with stub binaries and DNS helpers. +stage_tarball() { + local f + STAGE=$(mktemp -d) + cp "$INSTALL_SH" "$STAGE/install.sh" || return 1 + for f in fips.service fips-dns.service fips-gateway.service fips-firewall.service; do + cp "$REPO_ROOT/packaging/systemd/$f" "$STAGE/" || return 1 + done + for f in fips.yaml hosts fips.nft; do + cp "$REPO_ROOT/packaging/common/$f" "$STAGE/" || return 1 + done + printf '#!/bin/sh\nexec sleep infinity\n' > "$STAGE/fips" + printf '#!/bin/sh\nexec sleep infinity\n' > "$STAGE/fips-gateway" + printf '#!/bin/sh\nexit 0\n' > "$STAGE/fipsctl" + printf '#!/bin/sh\nexit 0\n' > "$STAGE/fips-dns-setup" + printf '#!/bin/sh\nexit 0\n' > "$STAGE/fips-dns-teardown" + chmod 0755 "$STAGE"/install.sh "$STAGE"/fips "$STAGE"/fips-gateway \ + "$STAGE"/fipsctl "$STAGE"/fips-dns-setup "$STAGE"/fips-dns-teardown + return 0 +} + +# Start the scenario's systemd container. Not privileged: see +# testing/lib/systemd-container.sh for the flags and why. The TUN device is +# passed so the scenario can create the fips0 that fips-gateway.service's +# ExecStartPre waits for. +start_container() { + local name="$1" + cleanup_container "$name" + run_quiet "docker run $name" \ + docker run -d --name "$name" \ + --label com.corganlabs.fips-ci=1 \ + "${SYSTEMD_CAPS[@]}" \ + --cgroupns=host \ + --device /dev/net/tun \ + -v /sys/fs/cgroup:/sys/fs/cgroup:rw \ + --tmpfs /run --tmpfs /run/lock \ + "$IMAGE" || return + check_isolation "$name" + return +} + +# Wait for systemd to finish booting. `degraded` counts as booted: units such +# as systemd-modules-load cannot succeed in a container, and the system still +# finished starting. The state string is tested directly rather than piped, so +# pipefail cannot turn a degraded boot into a failure. +wait_for_systemd() { + local name="$1" state + for _i in $(seq 1 "$BOOT_TIMEOUT"); do + state=$(docker exec "$name" systemctl is-system-running --wait 2>/dev/null || true) + case "$state" in + running | degraded) return 0 ;; + esac + sleep 1 + done + echo " ERROR: systemd did not reach running state in ${BOOT_TIMEOUT}s" >&2 + return 1 +} + +# Run the staged install.sh in the container, under a bound. +run_install() { + local name="$1" label="$2" out rc=0 + out=$(timeout "$INSTALL_TIMEOUT" docker exec "$name" "$TARBALL_DIR/install.sh" 2>&1) || rc=$? + if [ "$rc" -ne 0 ]; then + fail "$label: install.sh exited $rc" + echo "$out" | sed 's/^/ /' + return 1 + fi + pass "$label: install.sh exits 0" + return 0 +} + +# Print a unit's active state ("unknown" when the container cannot be asked). +unit_state() { + local name="$1" unit="$2" state + state=$(docker exec "$name" systemctl is-active "$unit" 2>/dev/null) + echo "${state:-unknown}" + return 0 +} + +# Print a unit's InvocationID, which changes on every start. +invocation_id() { + docker exec "$1" systemctl show -p InvocationID --value "$2" 2>/dev/null + return +} + +# Record a PASS when a unit is in the wanted state, a FAIL otherwise. +expect_state() { + local name="$1" label="$2" unit="$3" want="$4" got + got=$(unit_state "$name" "$unit") + if [ "$got" = "$want" ]; then + pass "$label: $unit is $got" + else + fail "$label: $unit is $got (want $want)" + fi + return 0 +} + +# Start a unit under a bound, recording a FAIL when it does not start. +start_unit() { + local name="$1" unit="$2" out + if ! out=$(timeout "$UNIT_START_TIMEOUT" docker exec "$name" systemctl start "$unit" 2>&1); then + fail "setup: could not start $unit: ${out:-no output}" + return 1 + fi + return 0 +} + +# Boot a fresh container, copy the tarball in and run the first install. +# Returns 1, having recorded a FAIL, when any of that does not happen. +prepare() { + local name="$1" label="$2" + start_container "$name" || { fail "$label: container did not start"; return 1; } + wait_for_systemd "$name" || { + fail "$label: systemd did not boot; the checks would read an unstarted system" + return 1 + } + if ! docker cp "$STAGE/." "$name:$TARBALL_DIR" >/dev/null; then + fail "$label: could not copy the tarball into $name" + return 1 + fi + if ! docker exec "$name" ip tuntap add fips0 mode tun >/dev/null 2>&1; then + fail "$label: could not create fips0 in $name" + return 1 + fi + run_install "$name" "$label: first install" || return 1 + return 0 +} + +scenario_all_active() { + local label="all-active" name before after + name=$(container_name "$label") + log "$label: fips, fips-dns and fips-gateway running before the upgrade" + if prepare "$name" "$label" \ + && start_unit "$name" fips.service \ + && start_unit "$name" fips-dns.service \ + && start_unit "$name" fips-gateway.service; then + expect_state "$name" "$label: before" fips.service active + expect_state "$name" "$label: before" fips-dns.service active + expect_state "$name" "$label: before" fips-gateway.service active + before=$(invocation_id "$name" fips.service) + if run_install "$name" "$label: upgrade"; then + expect_state "$name" "$label: after" fips.service active + expect_state "$name" "$label: after" fips-dns.service active + expect_state "$name" "$label: after" fips-gateway.service active + after=$(invocation_id "$name" fips.service) + if [ -z "$before" ] || [ -z "$after" ]; then + fail "$label: could not read fips.service's InvocationID (before '${before}', after '${after}')" + elif [ "$before" != "$after" ]; then + pass "$label: the upgrade restarted fips.service" + else + fail "$label: fips.service kept InvocationID $before, so the upgrade did not stop and restart it" + fi + fi + fi + cleanup_container "$name" + return 0 +} + +scenario_fips_only() { + local label="fips-only" name + name=$(container_name "$label") + log "$label: only fips running before the upgrade" + if prepare "$name" "$label" && start_unit "$name" fips.service; then + expect_state "$name" "$label: before" fips.service active + expect_state "$name" "$label: before" fips-dns.service inactive + expect_state "$name" "$label: before" fips-gateway.service inactive + if run_install "$name" "$label: upgrade"; then + expect_state "$name" "$label: after" fips.service active + expect_state "$name" "$label: after" fips-dns.service inactive + expect_state "$name" "$label: after" fips-gateway.service inactive + fi + fi + cleanup_container "$name" + return 0 +} + +scenario_none_active() { + local label="none-active" name + name=$(container_name "$label") + log "$label: nothing running before the upgrade" + if prepare "$name" "$label"; then + expect_state "$name" "$label: before" fips.service inactive + if run_install "$name" "$label: upgrade"; then + expect_state "$name" "$label: after" fips.service inactive + expect_state "$name" "$label: after" fips-dns.service inactive + expect_state "$name" "$label: after" fips-gateway.service inactive + fi + fi + cleanup_container "$name" + return 0 +} + +ALL_SCENARIOS="all-active fips-only none-active" + +_args=() +while [ $# -gt 0 ]; do + case "$1" in + --install-sh) + INSTALL_SH="${2:?--install-sh requires a path}" + shift 2 + ;; + -h|--help) + echo "usage: test.sh [--install-sh PATH] [scenario ...]" + echo "scenarios: $ALL_SCENARIOS" + exit 0 + ;; + -*) + echo "Unknown option: $1" >&2 + exit 1 + ;; + *) + _args+=("$1") + shift + ;; + esac +done +set -- ${_args[@]+"${_args[@]}"} + +if [ $# -eq 0 ]; then + scenarios="$ALL_SCENARIOS" +else + scenarios="$*" +fi + +for scenario in $scenarios; do + case " $ALL_SCENARIOS " in + *" $scenario "*) ;; + *) + echo "Unknown scenario: $scenario (available: $ALL_SCENARIOS)" >&2 + exit 1 + ;; + esac +done + +if [ ! -f "$INSTALL_SH" ]; then + echo "install script not found: $INSTALL_SH" >&2 + exit 1 +fi + +trap cleanup_all EXIT + +log "Building $IMAGE" +if ! build_image; then + fail "image build failed" +elif ! stage_tarball; then + fail "could not stage the tarball" +else + for scenario in $scenarios; do + case "$scenario" in + all-active) scenario_all_active ;; + fips-only) scenario_fips_only ;; + none-active) scenario_none_active ;; + esac + echo + done +fi + +echo "═══════════════════════════════════════" +echo "Results: $PASS passed, $FAIL failed" +echo "═══════════════════════════════════════" + +[ "$FAIL" -eq 0 ] && [ "$PASS" -gt 0 ]