From 26be2b7283f2a6f648c2eb3f4e49ac963f79613d Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 25 Sep 2026 03:52:25 +0000 Subject: [PATCH 01/14] Track whether each bloom filter announce reached its peer A filter announce is recorded as sent once the transport accepts the frame, which on UDP and Ethernet says nothing about delivery. The bloom state now keeps each peer's last announce outstanding, with its link counter and session, and decides from the peer's ordinary receiver reports whether it arrived, by comparing the report that covers the announce with the report taken before the send, its base. A loss is concluded only when fewer frames arrived between the two reports than the window holds. Delivery is confirmed when the frames that arrived in counter order cover the window, or when all arrivals do and the base had no holes: nothing reported yet in the peer's first session, or a first-session report that counted every counter up to its highest. A frame counted as a reorder may be one of the window's frames or a late frame from before the base, so any other pair is no evidence and the announce stays outstanding. A report from another session, recognisable because its highest counter is at or above the next counter the current session will use, is ignored, and an inconsistent pair of reports proves nothing. An announce the reports cannot check is resent once, as soon as a usable report arrives or after 30 s; a usable report that does not yet cover an announce sent in the current session becomes its base instead. After a rekey the peer's cumulative count includes earlier sessions, so no base can be shown to have no holes, and an in-window reorder there costs one fallback resend. Resends are bounded per announce and per peer: at most one unchecked and three loss resends per announce per session, and a per-peer backoff of 1, 2, 4 ... s up to 60 s that resets only after 120 s without a resend. MMP metrics gain a read-only accessor for the last accepted report's cumulative counters. --- src/proto/bloom/mod.rs | 2 +- src/proto/bloom/state.rs | 375 ++++++++++++++++++++++ src/proto/bloom/tests/state.rs | 571 +++++++++++++++++++++++++++++++++ src/proto/mmp/metrics.rs | 11 + 4 files changed, 958 insertions(+), 1 deletion(-) diff --git a/src/proto/bloom/mod.rs b/src/proto/bloom/mod.rs index cf4332e0..b58ad251 100644 --- a/src/proto/bloom/mod.rs +++ b/src/proto/bloom/mod.rs @@ -28,7 +28,7 @@ mod tests; pub use core::BloomFilter; pub use limits::{DEFAULT_FILTER_SIZE_BITS, DEFAULT_HASH_COUNT, V1_SIZE_CLASS}; -pub use state::BloomState; +pub use state::{BloomState, LinkEvidence, RrCounters}; pub use wire::FilterAnnounce; /// Errors related to Bloom filter operations. diff --git a/src/proto/bloom/state.rs b/src/proto/bloom/state.rs index 034dd2fe..e827c4f9 100644 --- a/src/proto/bloom/state.rs +++ b/src/proto/bloom/state.rs @@ -5,6 +5,225 @@ use alloc::collections::{BTreeMap, BTreeSet}; use super::BloomFilter; use crate::NodeAddr; +/// How long an announce the receiver reports cannot check waits before its +/// one unchecked resend, in milliseconds. Equal to the default link dead +/// timeout, so an outage that did not remove the peer has ended by then. +pub const FALLBACK_MS: u64 = 30_000; + +/// The largest gap the per-peer resend backoff imposes, in milliseconds. +pub const MAXGAP_MS: u64 = 60_000; + +/// A run of resends with no resend for this long, in milliseconds, resets the +/// backoff. It must exceed [`MAXGAP_MS`], or a sustained trigger resending at +/// the largest gap would reset its own backoff every time. +pub const QUIET_MS: u64 = 120_000; + +/// Unchecked resends (`Unverified`, `SessionChanged` or `Timeout`) allowed per +/// announce lineage per session. +pub const UNVERIFIED_BUDGET: u8 = 1; + +/// Resends on reported loss allowed per announce lineage per session. +pub const LOSS_BUDGET: u8 = 3; + +/// Highest backoff level. `gap` at this level is already capped at +/// [`MAXGAP_MS`], so a higher level would add nothing; the cap keeps the +/// shift in range. +const MAX_LEVEL: u8 = 7; + +/// The cumulative counters of one ReceiverReport the peer sent about our +/// frames on a link. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct RrCounters { + /// Highest link counter the peer had received from us. + pub highest: u64, + /// Link frames from us the peer had counted, cumulative. + pub received: u64, + /// Of those, frames that arrived below the highest counter, cumulative. + pub reordered: u32, +} + +/// What the shell reads from one peer's link at one moment, for deciding +/// whether an announce reached that peer. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct LinkEvidence { + /// Identity of the current link session (from its handshake hash). The + /// send counter restarts at 0 in every session, so counters are compared + /// only within one epoch. + pub epoch: u64, + /// The next send counter the current session will use. + pub next_counter: u64, + /// The last ReceiverReport accepted in the current session, if any. + pub rr: Option, +} + +impl LinkEvidence { + /// The report, when it can describe the current session. + /// + /// The peer cannot have received a counter this session has not used yet, + /// so a report whose highest counter is at or above `next_counter` + /// describes another session. That happens briefly around a rekey, when a + /// report or frame of the old session is counted against the new one, and + /// such a report is no evidence either way. + pub fn usable_rr(&self) -> Option { + self.rr.filter(|rr| rr.highest < self.next_counter) + } +} + +/// Why an announce is being resent, for the shell's log line. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ResendReason { + /// A report covering the announce shows fewer frames arrived since its + /// base than were sent. + Loss, + /// The first usable report already covers an announce it cannot check. + Unverified, + /// The announce was sent on an earlier session and cannot be checked. + SessionChanged, + /// No usable report checked the announce within the fallback interval. + Timeout, +} + +/// What an announce's delivery is measured from. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Base { + /// The last usable report before the announce was sent, and whether it + /// has no holes: every counter up to its highest had arrived. + Counted(RrCounters, bool), + /// Nothing received yet in the peer's first session: every counter from 0 + /// must arrive. + Zero, + /// No base: a later report can become one if it does not yet cover the + /// announce. + Unknown, +} + +/// The one announce to a peer still awaiting confirmation. +#[derive(Clone, Copy, Debug)] +struct SentAnnounce { + /// Link counter the announce was sent with. + counter: u64, + /// Session the counter belongs to (or, once orphaned, the session that + /// orphaned it). + epoch: u64, + /// What delivery is measured from. + base: Base, + /// Sent on an earlier session, so it can never be checked. + orphan: bool, + /// When it was sent or orphaned, for the fallback. + at_ms: u64, +} + +/// Per-peer delivery tracking for filter announces. +#[derive(Clone, Debug)] +struct AckState { + /// Session in which this entry was created; only there does a missing + /// report mean the peer has received nothing yet. + first_epoch: u64, + /// The outstanding announce, if one is unconfirmed. + sent: Option, + /// Backoff level: the number of resends in the current run, capped. + level: u8, + /// When the last resend was triggered. + resent_ms: Option, + /// Session the budgets were last refilled for. + budget_epoch: u64, + /// Unchecked resends left for the current lineage in this session. + unverified_left: u8, + /// Loss resends left for the current lineage in this session. + loss_left: u8, +} + +impl AckState { + /// A fresh entry for a peer first sent to in session `epoch`. + fn new(epoch: u64) -> Self { + Self { + first_epoch: epoch, + sent: None, + level: 0, + resent_ms: None, + budget_epoch: epoch, + unverified_left: UNVERIFIED_BUDGET, + loss_left: LOSS_BUDGET, + } + } + + /// Refill both budgets for session `epoch`. + fn refill(&mut self, epoch: u64) { + self.budget_epoch = epoch; + self.unverified_left = UNVERIFIED_BUDGET; + self.loss_left = LOSS_BUDGET; + } + + /// Whether report `rr`, taken in session `epoch`, shows no holes: every + /// counter up to its highest had arrived. Only in the peer's first session + /// does its cumulative count start at 0, so a later session's report is + /// never known to be whole. + fn whole(&self, epoch: u64, rr: RrCounters) -> bool { + epoch == self.first_epoch && rr.highest.checked_add(1) == Some(rr.received) + } + + /// Whether the backoff allows a resend at `now_ms`, resetting the level + /// after a quiet period. + fn backoff_allows(&mut self, now_ms: u64) -> bool { + let Some(last) = self.resent_ms else { + return true; + }; + if now_ms >= last.saturating_add(QUIET_MS) { + self.level = 0; + } + now_ms >= last.saturating_add(gap(self.level)) + } +} + +/// Minimum time after a resend before the next one, at backoff `level`. +fn gap(level: u8) -> u64 { + match level { + 0 => 0, + n => (1000u64 << (n.min(MAX_LEVEL) - 1)).min(MAXGAP_MS), + } +} + +/// Whether every counter in the report's range since `base` arrived. +/// +/// Within one receiver epoch every frame counted between two reports is a +/// distinct counter at or below `h1`. Those in `(h0, h1]` number at most all +/// receipts, `got`, and at least the non-reorder receipts, `sure`: a frame +/// that arrived after a higher counter is a reorder whether its counter lies +/// inside the window or at or below `h0`. +/// +/// - `got` below the span is a loss: fewer frames arrived than the window +/// holds. +/// - `sure` equal to the span is delivery. +/// - `got` equal to the span is delivery when the base has no holes, since +/// then no counter at or below `h0` is left to arrive late. +/// +/// Anything else is ambiguous and proves nothing, as is an inconsistent pair +/// (a counter went backwards), which means the two reports straddle a +/// receiver reset or another session's frame. Frames reserved but never +/// sent and frames dropped before counting only lower the counts, so a lost +/// frame is confirmed only if the peer overcounts. +fn delivered(base: Base, rr: RrCounters) -> Option { + let (r0, o0, span, complete) = match base { + Base::Counted(b, complete) => ( + b.received, + b.reordered, + rr.highest.checked_sub(b.highest)?, + complete, + ), + Base::Zero => (0, 0, rr.highest.checked_add(1)?, true), + Base::Unknown => return None, + }; + let got = rr.received.checked_sub(r0)?; + let sure = got.checked_sub(u64::from(rr.reordered.checked_sub(o0)?))?; + if got < span { + Some(false) + } else if sure == span || (complete && got == span) { + Some(true) + } else { + None + } +} + /// State for managing Bloom filter announcements. /// /// Tracks local filter state and what needs to be sent to peers. @@ -26,6 +245,10 @@ pub struct BloomState { sequence: u64, /// Last outgoing filter sent to each peer (for change detection). last_sent_filters: BTreeMap, + /// How long an unchecked announce waits for its fallback resend (ms). + fallback_ms: u64, + /// Per-peer delivery tracking for sent announces. + acks: BTreeMap, } impl BloomState { @@ -40,6 +263,8 @@ impl BloomState { pending_updates: BTreeSet::new(), sequence: 0, last_sent_filters: BTreeMap::new(), + fallback_ms: FALLBACK_MS, + acks: BTreeMap::new(), } } @@ -81,6 +306,12 @@ impl BloomState { self.update_debounce_ms = ms; } + /// Set how long an announce the receiver reports cannot check waits + /// before its fallback resend. Defaults to [`FALLBACK_MS`]. + pub fn set_fallback(&mut self, ms: u64) { + self.fallback_ms = ms; + } + /// Add a leaf dependent that we'll include in our filter. pub fn add_leaf_dependent(&mut self, node_addr: NodeAddr) { self.leaf_dependents.insert(node_addr); @@ -159,6 +390,150 @@ impl BloomState { self.last_sent_filters.remove(peer_id); self.last_update_sent.remove(peer_id); self.pending_updates.remove(peer_id); + self.acks.remove(peer_id); + } + + /// Record an announce the transport accepted for `peer`, sent with link + /// counter `counter`, so it stays outstanding until the peer's receiver + /// reports show it arrived. + /// + /// The transport accepting a frame is not delivery: a datagram can still + /// be lost, and announces are sent only when a filter changes, so a lost + /// one would otherwise leave the peer's filter stale indefinitely. + /// + /// Call this before [`record_sent_filter`](Self::record_sent_filter) for + /// the same send: it compares `filter` with the last one sent, and new + /// content starts a new lineage with fresh resend budgets. The budgets + /// also refill when the session changes, and never otherwise, so a resend + /// of the same content spends from its lineage's budget. + pub fn record_announce( + &mut self, + peer: NodeAddr, + filter: &BloomFilter, + counter: u64, + link: &LinkEvidence, + now_ms: u64, + ) { + let new_lineage = self.last_sent_filters.get(&peer) != Some(filter); + let ack = self + .acks + .entry(peer) + .or_insert_with(|| AckState::new(link.epoch)); + if new_lineage || link.epoch != ack.budget_epoch { + ack.refill(link.epoch); + } + // With no report yet, the zero baseline holds only in the peer's first + // session: a later session's cumulative count includes earlier ones. + let base = match link.usable_rr() { + Some(rr) if rr.highest < counter => Base::Counted(rr, ack.whole(link.epoch, rr)), + _ if link.rr.is_none() && link.epoch == ack.first_epoch => Base::Zero, + _ => Base::Unknown, + }; + ack.sent = Some(SentAnnounce { + counter, + epoch: link.epoch, + base, + orphan: false, + at_ms: now_ms, + }); + } + + /// Decide whether the outstanding announce to `peer` must be resent. + /// + /// Confirms the announce when a usable report covering its counter shows + /// every counter since its base arrived. Resends on a covering report that + /// shows a loss; once, as soon as a usable report arrives, for an announce + /// the reports cannot check; and once after the fallback interval when no + /// usable report checks it. A usable report that does not yet cover an + /// announce sent in the current session becomes its base instead of + /// triggering a resend. A report from another session, or a pair of + /// reports that is inconsistent or cannot tell a late frame from before + /// the base from one inside the window, is no evidence. Each announce lineage + /// gets [`UNVERIFIED_BUDGET`] unchecked and [`LOSS_BUDGET`] loss resends + /// per session, and a per-peer backoff spaces all resends by 1, 2, 4 ... + /// up to 60 s until [`QUIET_MS`] passes with none. + /// + /// On `Some`, the peer has been marked for an update; the ordinary send + /// path delivers the resend. + pub fn check_announce( + &mut self, + peer: &NodeAddr, + link: &LinkEvidence, + now_ms: u64, + ) -> Option { + let fallback_ms = self.fallback_ms; + let ack = self.acks.get_mut(peer)?; + let mut sent = ack.sent?; + + if link.epoch != ack.budget_epoch { + ack.refill(link.epoch); + } + if sent.epoch != link.epoch { + sent.orphan = true; + sent.epoch = link.epoch; + sent.base = Base::Unknown; + sent.at_ms = now_ms; + } + ack.sent = Some(sent); + + let rr = link.usable_rr(); + let due = now_ms >= sent.at_ms.saturating_add(fallback_ms); + let candidate = match (sent.base, rr) { + (Base::Unknown, Some(rr)) => { + if !sent.orphan && rr.highest < sent.counter { + sent.base = Base::Counted(rr, ack.whole(link.epoch, rr)); + ack.sent = Some(sent); + return None; + } + Some(if sent.orphan { + ResendReason::SessionChanged + } else { + ResendReason::Unverified + }) + } + (Base::Unknown, None) => due.then_some(ResendReason::Timeout), + (base, rr) => { + let covering = rr.filter(|rr| rr.highest >= sent.counter); + let loss = match covering.and_then(|rr| delivered(base, rr)) { + Some(true) => { + ack.sent = None; + return None; + } + Some(false) if ack.loss_left > 0 => Some(ResendReason::Loss), + _ => None, + }; + loss.or(due.then_some(ResendReason::Timeout)) + } + }?; + + let loss = candidate == ResendReason::Loss; + let left = if loss { + ack.loss_left + } else { + ack.unverified_left + }; + if left == 0 || !ack.backoff_allows(now_ms) { + return None; + } + if loss { + ack.loss_left -= 1; + } else { + ack.unverified_left -= 1; + } + ack.level = (ack.level + 1).min(MAX_LEVEL); + ack.resent_ms = Some(now_ms); + self.mark_update_needed(*peer); + Some(candidate) + } + + /// Whether an announce to `peer` is still awaiting confirmation. + pub fn announce_outstanding(&self, peer: &NodeAddr) -> bool { + self.outstanding_counter(peer).is_some() + } + + /// The link counter of the announce to `peer` awaiting confirmation. + pub fn outstanding_counter(&self, peer: &NodeAddr) -> Option { + self.acks.get(peer)?.sent.map(|sent| sent.counter) } /// Mark only peers whose outgoing filter has actually changed. diff --git a/src/proto/bloom/tests/state.rs b/src/proto/bloom/tests/state.rs index 159b2767..c5e66e74 100644 --- a/src/proto/bloom/tests/state.rs +++ b/src/proto/bloom/tests/state.rs @@ -313,3 +313,574 @@ fn test_bloom_state_mark_changed_peers_excludes_source() { assert!(!state.needs_update(&peer1)); } + +// ===== Delivery tracking for sent announces ===== +// +// Synthetic milliseconds, counters and receiver reports, no I/O. A report is +// written `(highest, received, reordered)`. Unless a test says otherwise, a +// send is recorded with `next_counter = counter + 1`, as the shell reads it +// straight after the send. + +use crate::NodeAddr; +use crate::proto::bloom::state::{ + FALLBACK_MS, LOSS_BUDGET, LinkEvidence, QUIET_MS, ResendReason, RrCounters, UNVERIFIED_BUDGET, +}; + +/// First session. +const E1: u64 = 0x0e01; +/// Second session. +const E2: u64 = 0x0e02; +/// Third session. +const E3: u64 = 0x0e03; + +/// A report's cumulative counters. +fn rr(highest: u64, received: u64, reordered: u32) -> Option { + Some(RrCounters { + highest, + received, + reordered, + }) +} + +/// Link evidence for session `epoch`. +fn link(epoch: u64, next_counter: u64, rr: Option) -> LinkEvidence { + LinkEvidence { + epoch, + next_counter, + rr, + } +} + +/// Filter content number `n`: distinct numbers give distinct filters. +fn content(n: u8) -> BloomFilter { + let mut filter = BloomFilter::new(); + filter.insert(&make_node_addr(100u8.wrapping_add(n))); + filter +} + +/// One peer's announces, driven as the shell drives them. +struct Track { + state: BloomState, + peer: NodeAddr, + content: u8, + counter: u64, +} + +impl Track { + /// A tracker with nothing sent yet, sending content 1. + fn new() -> Self { + Self { + state: BloomState::new(make_node_addr(0)), + peer: make_node_addr(1), + content: 1, + counter: 0, + } + } + + /// Record a send of the current content at `counter`, then the sent + /// filter, in the shell's order. + fn send(&mut self, counter: u64, link: LinkEvidence, now_ms: u64) { + let filter = content(self.content); + self.state + .record_announce(self.peer, &filter, counter, &link, now_ms); + self.state.record_sent_filter(self.peer, filter); + self.counter = counter; + } + + /// Send new content at `counter`. + fn send_new(&mut self, counter: u64, link: LinkEvidence, now_ms: u64) { + self.content += 1; + self.send(counter, link, now_ms); + } + + /// One tick of the tracker. + fn check(&mut self, link: LinkEvidence, now_ms: u64) -> Option { + self.state.check_announce(&self.peer, &link, now_ms) + } + + /// Whether the announce is still unconfirmed. + fn outstanding(&self) -> bool { + self.state.announce_outstanding(&self.peer) + } + + /// Check every 1,000 ms from `from_ms` to `to_ms` inclusive. `model` gives + /// the evidence at a time, from the outstanding counter: for a check with + /// `false`, and for recording a resend with `true`, where the resend takes + /// the counter `next_counter - 1` of that evidence. Each resend is + /// recorded, with new content when `renew` is set. Returns the resends. + fn hold( + &mut self, + from_ms: u64, + to_ms: u64, + renew: bool, + model: impl Fn(u64, u64, bool) -> LinkEvidence, + ) -> Vec<(u64, ResendReason)> { + let mut resends = Vec::new(); + let mut now = from_ms; + while now <= to_ms { + if let Some(reason) = self.check(model(now, self.counter, false), now) { + resends.push((now, reason)); + let ev = model(now, self.counter, true); + if renew { + self.content += 1; + } + self.send(ev.next_counter - 1, ev, now); + } + now += 1_000; + } + resends + } +} + +/// Loss on every check: each send is based on a report just below it, and +/// each check sees a report two counters on with one frame missing. +fn lossy(_now: u64, counter: u64, recording: bool) -> LinkEvidence { + if recording { + let n = counter + 1; + link(E1, n + 1, rr(n - 1, n, 0)) + } else { + link(E1, counter + 2, rr(counter + 1, counter + 1, 0)) + } +} + +/// The resend times of `resends`, in ms. +fn times(resends: &[(u64, ResendReason)]) -> Vec { + resends.iter().map(|(t, _)| *t).collect() +} + +/// A covering report with a frame missing since the base is a loss. +#[test] +fn test_bloom_ack_covering_report_with_a_missing_frame_resends_on_loss() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + // Four of 10..=14 arrived; 12 is the missing one. + assert_eq!( + t.check(link(E1, 15, rr(14, 14, 0)), 1_000), + Some(ResendReason::Loss) + ); + assert!(t.state.needs_update(&t.peer), "the peer must be marked"); +} + +/// A covering report with every frame since the base confirms. +#[test] +fn test_bloom_ack_covering_report_with_every_frame_confirms() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + assert_eq!(t.check(link(E1, 15, rr(14, 15, 0)), 1_000), None); + assert!(!t.outstanding(), "the announce must be confirmed"); + assert!( + !t.state.needs_update(&t.peer), + "the peer must not be marked" + ); +} + +/// A late frame from before the base cannot stand in for the lost +/// announce. The base has a hole, so the pair cannot tell that late frame +/// from an in-window reorder: no evidence, and the fallback resends. +#[test] +fn test_bloom_ack_reordered_frame_does_not_mask_a_lost_announce() { + let mut t = Track::new(); + // Base (9, 9, 0): frame 5 missing at the time. + t.send(12, link(E1, 13, rr(9, 9, 0)), 0); + // Late frame 5 plus 10, 11, 13, 14; 12 lost. A naive count gives 5 == 5. + let late = |_, c, recording| link(E1, c + if recording { 2 } else { 3 }, rr(14, 14, 1)); + assert_eq!(t.check(late(1_000, 12, false), 1_000), None); + assert!(t.outstanding(), "a lost announce must not confirm"); + let resends = t.hold(2_000, 120_000, false, late); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} + +/// A report that does not yet cover the announce decides nothing, and the +/// next one is measured against the base taken at the send. +#[test] +fn test_bloom_ack_report_below_the_announce_waits_for_a_covering_one() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + assert_eq!(t.check(link(E1, 13, rr(11, 12, 0)), 1_000), None); + assert!(t.outstanding(), "an uncovered announce stays outstanding"); + assert_eq!(t.check(link(E1, 15, rr(14, 15, 0)), 2_000), None); + assert!(!t.outstanding(), "the covering report must confirm"); +} + +/// An announce from an earlier session is resent once the new session can +/// check the resend, or once after the fallback if it never can. +#[test] +fn test_bloom_ack_announce_from_an_earlier_session_is_resent_once() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + assert_eq!(t.check(link(E2, 1, None), 1_000), None); + assert_eq!( + t.check(link(E2, 5, rr(3, 4, 0)), 2_000), + Some(ResendReason::SessionChanged) + ); + t.send(5, link(E2, 6, rr(3, 4, 0)), 2_000); + assert_eq!(t.check(link(E2, 7, rr(6, 7, 0)), 3_000), None); + assert!(!t.outstanding(), "the resend must be confirmed"); + + // No usable report in the new session: one Timeout, 30 s after the change + // was seen, and no second. + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + assert_eq!(t.check(link(E2, 1, None), 1_000), None); + let resends = t.hold(2_000, 121_000, false, |_, c, recording| { + link(E2, c + if recording { 2 } else { 1 }, None) + }); + assert_eq!(resends, vec![(31_000, ResendReason::Timeout)]); +} + +/// In the peer's first session, with no report before the send, every +/// counter from 0 must arrive. +#[test] +fn test_bloom_ack_first_session_measures_from_counter_zero() { + let mut t = Track::new(); + t.send(12, link(E1, 13, None), 0); + assert_eq!(t.check(link(E1, 13, rr(12, 13, 0)), 1_000), None); + assert!(!t.outstanding(), "13 of 0..=12 must confirm"); + + let mut t = Track::new(); + t.send(12, link(E1, 13, None), 0); + assert_eq!( + t.check(link(E1, 13, rr(12, 12, 0)), 1_000), + Some(ResendReason::Loss) + ); +} + +/// In a later session, the peer's cumulative count includes earlier +/// sessions, so no report means no base: a first report below the announce +/// becomes the base, and one already covering it cannot check it. +#[test] +fn test_bloom_ack_later_session_without_a_report_has_no_base() { + // Case A: re-based, then confirmed with no resend. + let mut t = Track::new(); + t.send(3, link(E1, 4, None), 0); + t.send(12, link(E2, 13, None), 1_000); + assert_eq!(t.check(link(E2, 13, rr(8, 509, 0)), 2_000), None); + assert!(t.outstanding()); + assert_eq!(t.check(link(E2, 15, rr(14, 515, 0)), 3_000), None); + assert!(!t.outstanding(), "the re-based announce must confirm"); + + // Case B: the first usable report already covers the announce. + let mut t = Track::new(); + t.send(3, link(E1, 4, None), 0); + t.send(12, link(E2, 13, None), 1_000); + assert_eq!( + t.check(link(E2, 15, rr(14, 515, 0)), 2_000), + Some(ResendReason::Unverified) + ); +} + +/// A report from another session at send time is no evidence, and the +/// announce gets exactly one fallback resend. +#[test] +fn test_bloom_ack_report_from_another_session_at_send_gets_one_timeout() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(20, 30, 0)), 0); + let resends = t.hold(1_000, 120_000, false, |_, c, recording| { + link(E1, c + if recording { 2 } else { 1 }, rr(20, 30, 0)) + }); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} + +/// With no report ever, one fallback resend per session. +#[test] +fn test_bloom_ack_no_report_ever_resends_once_per_session() { + let mut t = Track::new(); + t.send(12, link(E1, 13, None), 0); + assert_eq!(t.check(link(E1, 13, None), FALLBACK_MS - 1), None); + let quiet = |_, c, recording| link(E1, c + if recording { 2 } else { 1 }, None); + let resends = t.hold(FALLBACK_MS, 120_000, false, quiet); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); + + assert_eq!(t.check(link(E2, 1, None), 121_000), None); + let later = |_, c, recording| link(E2, c + if recording { 2 } else { 1 }, None); + let resends = t.hold(122_000, 240_000, false, later); + assert_eq!(resends, vec![(151_000, ResendReason::Timeout)]); +} + +/// A trigger that never stops is spaced 1, 2, 4 ... s apart up to 60 s. +#[test] +fn test_bloom_ack_sustained_trigger_backs_off_to_one_resend_a_minute() { + let mut t = Track::new(); + t.send(12, lossy(0, 11, true), 0); + let resends = t.hold(0, 600_000, true, lossy); + let expected: Vec = [0, 1, 3, 7, 15, 31, 63] + .iter() + .map(|s| s * 1_000) + .chain((123_000..=600_000).step_by(60_000)) + .collect(); + assert_eq!(times(&resends), expected); + for (i, &(start, _)) in resends.iter().enumerate() { + let in_window = resends[i..] + .iter() + .take_while(|(t, _)| *t < start + 60_000) + .count(); + assert!(in_window <= 6, "{in_window} resends in 60 s from {start}"); + } +} + +/// 120 s with no resend resets the backoff; 119 s does not. +#[test] +fn test_bloom_ack_backoff_resets_only_after_the_quiet_period() { + let run = |next_ms: u64| { + let mut t = Track::new(); + t.send(12, lossy(0, 11, true), 0); + let first = t.hold(0, 31_000, true, lossy); + assert_eq!(times(&first), vec![0, 1_000, 3_000, 7_000, 15_000, 31_000]); + t.hold(next_ms, next_ms + 70_000, true, lossy) + }; + // Case A: 120 s after the last resend, the level resets to 0. + let a = run(31_000 + QUIET_MS); + assert_eq!(times(&a[..2]), vec![151_000, 152_000]); + // Case B: 119 s after, level 6 still applies and reaches level 7. + let b = run(31_000 + QUIET_MS - 1_000); + assert_eq!(times(&b[..2]), vec![150_000, 210_000]); +} + +/// A report that went backwards within a session is no evidence +/// (defensive: the MMP layer never stores one). +#[test] +fn test_bloom_ack_regressed_report_is_no_evidence() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + let regressed = |_, c, recording| link(E1, c + if recording { 2 } else { 3 }, rr(14, 8, 0)); + assert_eq!(t.check(regressed(1_000, 12, false), 1_000), None); + assert!(t.outstanding()); + let resends = t.hold(2_000, FALLBACK_MS, false, regressed); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} + +/// Removing the peer forgets its announce, and the next send starts a +/// fresh entry whose first session measures from counter zero. +#[test] +fn test_bloom_ack_removed_peer_starts_fresh() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + t.state.remove_peer_state(&t.peer); + assert_eq!(t.check(link(E1, 15, rr(14, 14, 0)), 1_000), None); + assert!(!t.outstanding()); + + t.send(3, link(E2, 4, None), 2_000); + assert_eq!(t.check(link(E2, 4, rr(3, 4, 0)), 3_000), None); + assert!(!t.outstanding(), "the new entry must measure from zero"); +} + +/// A confirmation does not reset the backoff. +#[test] +fn test_bloom_ack_confirmation_keeps_the_backoff() { + let mut t = Track::new(); + t.send(12, lossy(0, 11, true), 0); + assert_eq!(t.check(lossy(0, 12, false), 0), Some(ResendReason::Loss)); + t.send(13, lossy(0, 12, true), 0); + assert_eq!(t.check(link(E1, 15, rr(14, 15, 0)), 100), None); + assert!(!t.outstanding(), "setup: the resend is confirmed"); + + t.send_new(15, lossy(200, 14, true), 200); + assert_eq!(t.check(lossy(500, 15, false), 500), None); + assert_eq!( + t.check(lossy(1_000, 15, false), 1_000), + Some(ResendReason::Loss) + ); +} + +/// The initiator holds a report from the responder's view of the old +/// session, frozen until the new session's counter passes it. It never +/// triggers more than each announce's one unchecked resend. +#[test] +fn test_bloom_ack_frozen_report_after_a_rekey_spends_only_the_unchecked_budget() { + // The session sends 100 frames a second from counter 1. + let next = |now: u64| 1 + now / 10; + let frozen = rr(5_000, 90_000, 40); + let accepted = rr(6_100, 96_101, 45); + let report = move |now: u64| if now < 62_000 { frozen } else { accepted }; + let model = move |now: u64, _c: u64, recording: bool| { + link(E2, next(now) + u64::from(recording), report(now)) + }; + + let mut t = Track::new(); + t.send(3, link(E1, 4, None), 0); + t.send(3, link(E2, 4, frozen), 0); + let first = t.hold(1_000, 59_000, false, model); + assert_eq!(first, vec![(FALLBACK_MS, ResendReason::Timeout)]); + + // A new-content announce based on the now-usable frozen report. + t.send_new(6_000, link(E2, 6_001, frozen), 60_000); + let second = t.hold(61_000, 120_000, false, model); + assert_eq!(second, vec![(90_000, ResendReason::Timeout)]); + + let third = t.hold(121_000, 140_000, false, |now, c, recording| { + let n = 10 + (now - 121_000) / 1_000 + u64::from(recording); + link(E3, n.max(c + 1), rr(5, 6, 0)) + }); + assert_eq!(third, vec![(121_000, ResendReason::SessionChanged)]); +} + +/// The responder receives reports whose highest counter comes from its +/// own previous session. They change on every report and are never usable, +/// so they trigger nothing beyond the one fallback resend. +#[test] +fn test_bloom_ack_polluted_report_after_a_rekey_spends_only_the_unchecked_budget() { + let model = |now: u64, c: u64, recording: bool| { + let secs = now / 1_000; + let n = (1 + 10 * secs).max(c + 1) + u64::from(recording); + link(E2, n, rr(9_000, 100 + 5 * secs, 5 * secs as u32)) + }; + let mut t = Track::new(); + t.send(3, link(E1, 4, None), 0); + t.send(5, model(0, 4, true), 0); + let resends = t.hold(1_000, 120_000, false, model); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} + +/// More receipts than counters means the reports straddle a reset or +/// another session's frame, which is no evidence. +#[test] +fn test_bloom_ack_surplus_receipts_are_no_evidence() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + let surplus = |_, c, recording| link(E1, c + if recording { 2 } else { 3 }, rr(14, 20, 0)); + assert_eq!(t.check(surplus(1_000, 12, false), 1_000), None); + assert!(t.outstanding()); + let resends = t.hold(2_000, FALLBACK_MS, false, surplus); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} + +/// One lineage in one session gets three loss resends and one unchecked +/// resend; new content or a new session refills both. +#[test] +fn test_bloom_ack_budgets_bound_resends_per_lineage_per_session() { + let spend = || { + let mut t = Track::new(); + t.send(12, lossy(0, 11, true), 0); + let resends = t.hold(0, 120_000, false, lossy); + assert_eq!( + resends, + vec![ + (0, ResendReason::Loss), + (1_000, ResendReason::Loss), + (3_000, ResendReason::Loss), + (33_000, ResendReason::Timeout), + ] + ); + assert_eq!(LOSS_BUDGET, 3); + assert_eq!(UNVERIFIED_BUDGET, 1); + t + }; + + let mut t = spend(); + let c = t.counter; + t.send_new(c + 1, lossy(121_000, c, true), 121_000); + assert_eq!( + t.check(lossy(122_000, t.counter, false), 122_000), + Some(ResendReason::Loss), + "new content must refill the loss budget" + ); + + let mut t = spend(); + let c = t.counter; + let rekeyed = |ev: LinkEvidence| link(E2, ev.next_counter, ev.rr); + t.send(c + 1, rekeyed(lossy(121_000, c, true)), 121_000); + assert_eq!( + t.check(rekeyed(lossy(122_000, t.counter, false)), 122_000), + Some(ResendReason::Loss), + "a new session must refill the loss budget" + ); +} + +/// A resend carries the content it repeats, so it spends from the same +/// lineage's budget instead of starting a new one. +#[test] +fn test_bloom_ack_resend_of_the_same_content_is_not_a_new_lineage() { + let mut t = Track::new(); + t.send(12, lossy(0, 11, true), 0); + let resends = t.hold(0, 7_000, false, lossy); + assert_eq!(times(&resends), vec![0, 1_000, 3_000]); + assert_eq!( + t.check(lossy(8_000, t.counter, false), 8_000), + None, + "a fourth loss resend must not be allowed" + ); +} + +/// A report polluted after the base was taken is unusable, not a loss. +#[test] +fn test_bloom_ack_report_polluted_after_the_base_is_not_a_loss() { + let mut t = Track::new(); + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + assert_eq!(t.check(link(E1, 20, rr(9_000, 16, 0)), 1_000), None); + assert!(t.outstanding()); +} + +/// A frame inside the checked window that arrives after a higher counter is +/// counted as a reorder, but it did arrive. On a base with no holes (a +/// first-session report that counted every frame up to its highest), every +/// counter in the window arriving confirms the announce. +#[test] +fn test_bloom_ack_in_window_reorder_on_a_complete_base_confirms() { + let mut t = Track::new(); + // Base (9, 10, 0): all of 0..=9 counted. + t.send(12, link(E1, 13, rr(9, 10, 0)), 0); + // 10..=14 all arrived, 12 after 13. + assert_eq!(t.check(link(E1, 15, rr(14, 15, 1)), 1_000), None); + assert!( + !t.outstanding(), + "every counter arrived, so it must confirm" + ); + assert!( + !t.state.needs_update(&t.peer), + "the peer must not be marked" + ); +} + +/// In the peer's first session with no report before the send, frames that +/// arrive out of order are still every counter from 0, so the announce +/// confirms. +#[test] +fn test_bloom_ack_in_window_reorder_on_the_zero_base_confirms() { + let mut t = Track::new(); + t.send(4, link(E1, 5, None), 0); + // 0..=4 all arrived, as 0, 1, 4, 3, 2. + assert_eq!(t.check(link(E1, 5, rr(4, 5, 2)), 1_000), None); + assert!( + !t.outstanding(), + "every counter arrived, so it must confirm" + ); + assert!( + !t.state.needs_update(&t.peer), + "the peer must not be marked" + ); +} + +/// In a later session the base cannot be shown to have no holes, so a +/// covering report with an in-window reorder cannot tell a late frame from +/// before the base from one inside the window. That is no evidence: no loss +/// resend, and the fallback covers the announce. +#[test] +fn test_bloom_ack_ambiguous_pair_in_a_later_session_waits_for_the_fallback() { + let mut t = Track::new(); + t.send(3, link(E1, 4, None), 0); + // Cumulative counts include 500 frames of the earlier session. + t.send(12, link(E2, 13, rr(9, 510, 0)), 0); + let ambiguous = |_, c, recording| link(E2, c + if recording { 2 } else { 3 }, rr(14, 515, 1)); + assert_eq!(t.check(ambiguous(1_000, 12, false), 1_000), None); + assert!(t.outstanding(), "an ambiguous pair must not confirm"); + let resends = t.hold(2_000, 120_000, false, ambiguous); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} + +/// A later-session base whose counts happen to read as having no holes is +/// still not trusted: the cumulative count includes earlier sessions. A late +/// frame from before the base arriving with the announce lost must not +/// confirm it. +#[test] +fn test_bloom_ack_later_session_base_is_never_complete() { + let mut t = Track::new(); + t.send(3, link(E1, 4, None), 0); + // Base (9, 10, 0) in E2: 3 frames of E1 plus 7 of 0..=9, with 5 missing. + t.send(12, link(E2, 13, rr(9, 10, 0)), 0); + // Late frame 5 plus 10, 11, 13, 14; 12 lost. Received rose by 5 == span. + let late = |_, c, recording| link(E2, c + if recording { 2 } else { 3 }, rr(14, 15, 1)); + assert_eq!(t.check(late(1_000, 12, false), 1_000), None); + assert!(t.outstanding(), "a lost announce must not confirm"); + let resends = t.hold(2_000, 120_000, false, late); + assert_eq!(resends, vec![(FALLBACK_MS, ResendReason::Timeout)]); +} diff --git a/src/proto/mmp/metrics.rs b/src/proto/mmp/metrics.rs index 0803ce2e..a7b7146f 100644 --- a/src/proto/mmp/metrics.rs +++ b/src/proto/mmp/metrics.rs @@ -360,6 +360,17 @@ impl MmpMetrics { self.prev_rr_ecn_ce } + /// Cumulative counters of the last accepted ReceiverReport: highest counter, + /// packets received and reorder count. `None` until a report is accepted in + /// the current session. + pub fn rr_counters(&self) -> Option<(u64, u64, u32)> { + self.has_prev_rr.then_some(( + self.prev_rr_highest_counter, + self.prev_rr_cum_packets, + self.prev_rr_reorder, + )) + } + /// ReceiverReports processed, including stale and duplicate ones. pub fn reports_seen(&self) -> u64 { self.reports_seen From 60ab868636150ba5e2c4fb9150af2d477d6a6898 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Fri, 25 Sep 2026 03:52:25 +0000 Subject: [PATCH 02/14] Resend a bloom filter announce the peer did not receive A node counted a FilterAnnounce as delivered as soon as the transport accepted it, and announces go out only when a filter changes. A dropped datagram, or a link outage shorter than the dead timeout, therefore left the peer holding the old filter until something else changed, so destinations could stay missing from its discovery indefinitely. Each tick now checks every outstanding announce against the link's existing receiver reports before sending pending announces, and marks the peer again when a report covering the announce shows fewer frames arrived than were sent, or when the announce cannot be confirmed: once, as soon as a usable report arrives, or after 30 s. A report that shows neither delivery nor a loss, as when a reordered frame could be a late one from before the announce, waits for the 30 s fallback. The resend goes out through the ordinary debounced send path in the same tick. Reports from another session are ignored, at most one unchecked and three loss resends are sent per announce per session, and a peer connection sees at most six resends a minute, and one a minute while losses persist. Nothing on the wire changes. The design notes describe the resend and its limits. --- docs/design/fips-bloom-filters.md | 46 ++- src/node/bloom.rs | 68 +++- src/node/tests/bloom.rs | 586 ++++++++++++++++++++++++++++++ 3 files changed, 697 insertions(+), 3 deletions(-) diff --git a/docs/design/fips-bloom-filters.md b/docs/design/fips-bloom-filters.md index d0fce166..7b394367 100644 --- a/docs/design/fips-bloom-filters.md +++ b/docs/design/fips-bloom-filters.md @@ -201,7 +201,7 @@ there is no incremental update. ### Update Triggers -Filter updates are event-driven, not periodic: +New filter content is sent only on these events, never periodically: - Peer connects (new filter includes the new peer's reachability) - Peer disconnects (filter must exclude the departed peer's entries) @@ -209,6 +209,46 @@ Filter updates are event-driven, not periodic: be recomputed) - Local state changes (new identity, leaf-only dependent changes) +The one timed send is the resend of an announce that was not confirmed +delivered. The transport accepting a FilterAnnounce does not mean the peer +received it: a datagram can be lost, or a link can go down for less than +the dead timeout without the peer being removed. Each announce therefore +stays outstanding until the link's ordinary MMP ReceiverReports show that +every link frame up to and including the announce's counter arrived. No +new message or field is involved. + +A report covering the announce is compared with the report the announce +was sent after, its base. Between them the peer counted `got` frames, of +which `sure` arrived in counter order. A frame that arrived after a higher +counter is a reorder, and it can be one of the window's frames or a late +frame from before the base, so the frames of the window number between +`sure` and `got`. The announce is lost when `got` is below the number of +counters the window holds, and delivered when `sure` equals it, or when +`got` equals it and the base has no holes. A base has no holes when +nothing had been reported yet in the peer's first session, or when it is a +first-session report that counted exactly its highest counter plus one +frame. After a rekey the peer's cumulative count includes earlier +sessions, so no base can be shown to have no holes. Any other pair is no +evidence, and the 30 s fallback below covers the announce. + +An unconfirmed announce is resent: + +- on a report that covers it and shows fewer frames arrived than were + sent; +- once, when the reports cannot check it (sent before the session's first + usable report, carried over a rekey, or a report left over from the + previous session), as soon as a usable report arrives, or after 30 s if + none does. A usable report that does not yet cover an announce sent in + the current session becomes the base it is checked against instead. + +A report whose highest counter is at or above the next counter the current +session will use describes another session and is ignored. Each announce +gets at most one unchecked resend and three loss resends per session, and +a per-peer backoff spaces resends 1, 2, 4 ... s apart up to 60 s until +120 s pass with none, so a peer connection sees at most six resends in a +minute and one a minute while losses persist. A resend of content the +peer already holds changes nothing there, so it propagates no further. + ### Rate Limiting Updates are rate-limited at a 500ms minimum interval per peer @@ -241,7 +281,9 @@ property of the data structure). Entries are expired through: - **Implicit timeout**: If a peer becomes unresponsive, the MMP link liveness detector eventually declares the link dead and removes the peer, which triggers filter cleanup as a side effect of peer removal. - There is no independent filter staleness timer. + There is no independent filter staleness timer; the only timer is the + 30 s fallback that resends an announce the receiver reports cannot + confirm (see Update Triggers). ## Membership Test diff --git a/src/node/bloom.rs b/src/node/bloom.rs index 5c6586af..e85ea4c9 100644 --- a/src/node/bloom.rs +++ b/src/node/bloom.rs @@ -6,6 +6,7 @@ use crate::NodeAddr; use crate::proto::bloom::BloomFilter; use crate::proto::bloom::FilterAnnounce; +use crate::proto::bloom::{LinkEvidence, RrCounters}; use super::reject::BloomReject; use super::{Node, NodeError}; @@ -71,6 +72,20 @@ impl Node { self.metrics().bloom.sent.inc(); + // Read after the send: anything else taking a counter in between only + // makes the recorded counter higher, which delays confirmation rather + // than confirming a frame that was never covered. + if let Some(link) = self.link_evidence(peer_addr) { + let counter = link.next_counter.saturating_sub(1); + self.bloom_state.record_announce( + *peer_addr, + &sent_filter, + counter, + &link, + crate::time::mono_ms(), + ); + } + // Self-plausibility check: WARN if our own outgoing filter is // above the antipoison cap. Independent detection signal if // aggregation drift or an ingress-check bypass pushes us over @@ -271,10 +286,61 @@ impl Node { .mark_changed_peers(from, &peer_addrs, &peer_filters); } + /// Read what `peer_addr`'s link shows about delivery of our frames: the + /// session's identity and next send counter, and the last ReceiverReport + /// accepted on it. `None` when the peer has no session. + fn link_evidence(&self, peer_addr: &NodeAddr) -> Option { + let peer = self.peers.get(peer_addr)?; + let session = peer.noise_session()?; + let mut epoch = [0u8; 8]; + epoch.copy_from_slice(&session.handshake_hash()[..8]); + let rr = peer.mmp().and_then(|mmp| mmp.metrics.rr_counters()).map( + |(highest, received, reordered)| RrCounters { + highest, + received, + reordered, + }, + ); + Some(LinkEvidence { + epoch: u64::from_le_bytes(epoch), + next_counter: session.current_send_counter(), + rr, + }) + } + + /// Mark for resend every peer whose outstanding announce the receiver + /// reports show was lost, or could not confirm in time. + fn check_announces(&mut self) { + let now_ms = crate::time::mono_ms(); + let waiting: Vec = self + .peers + .keys() + .filter(|addr| self.bloom_state.announce_outstanding(addr)) + .copied() + .collect(); + for addr in waiting { + let Some(link) = self.link_evidence(&addr) else { + continue; + }; + let counter = self.bloom_state.outstanding_counter(&addr); + if let Some(reason) = self.bloom_state.check_announce(&addr, &link, now_ms) { + debug!( + peer = %self.peer_display_name(&addr), + reason = ?reason, + counter = ?counter, + "Resending unconfirmed FilterAnnounce" + ); + } + } + } + /// Check bloom filter state on tick (called from event loop). /// - /// Sends any pending debounced filter announces. + /// Marks peers whose last announce was not confirmed delivered, then sends + /// any pending debounced filter announces, so a resend goes out in the + /// same tick through the ordinary send path. pub(super) async fn check_bloom_state(&mut self) { + self.check_announces(); self.send_pending_filter_announces().await; } } diff --git a/src/node/tests/bloom.rs b/src/node/tests/bloom.rs index 0542b0a2..978b57cd 100644 --- a/src/node/tests/bloom.rs +++ b/src/node/tests/bloom.rs @@ -988,3 +988,589 @@ async fn test_bloom_tree_announce_without_tree_peer_flip_marks_no_peer() { assert!(!bloom.needs_update(&c), "C must not be marked"); cleanup_nodes(&mut fx.nodes).await; } + +// ===== Resend of a filter announce the peer did not receive ===== +// +// A lost datagram is made by taking M's frame out of P's receive channel +// without processing it: the transport returned `Ok` and the receiver never +// saw the frame, which is the shape of a real loss on UDP or Ethernet. Final +// assertions read the filter P stores for M, never what M believes it sent. + +/// Index of P in `FlipFixture::nodes`. +const P: usize = 0; + +/// Set every link report interval on `node` to zero, so the next +/// `check_mmp_reports` sends each report that has interval data. +/// +/// Processing a ReceiverReport re-derives the intervals from SRTT, so callers +/// re-apply this before every `check_mmp_reports`. +fn zero_intervals(node: &mut Node) { + for peer in node.peers.values_mut() { + if let Some(mmp) = peer.mmp_mut() { + mmp.sender.update_report_interval_with_bounds(1_000, 0, 0); + mmp.receiver.update_report_interval_with_bounds(1_000, 0, 0); + } + } +} + +/// One MMP exchange between M and P: M reports, P processes, P reports, M +/// processes. C is never asked to report. +async fn mmp_round(nodes: &mut [TestNode]) { + zero_intervals(&mut nodes[M].node); + zero_intervals(&mut nodes[P].node); + nodes[M].node.check_mmp_reports().await; + process_available_packets(nodes).await; + zero_intervals(&mut nodes[M].node); + zero_intervals(&mut nodes[P].node); + nodes[P].node.check_mmp_reports().await; + process_available_packets(nodes).await; +} + +/// Process packets on every node until a pass handles none, at most 50 passes. +async fn drain_quiet(nodes: &mut [TestNode]) { + for _ in 0..50 { + if process_available_packets(nodes).await == 0 { + return; + } + } + panic!("setup: packets still flowing after 50 passes"); +} + +/// Wait at most 1 s for `tn` to hold a queued frame, then take every queued +/// frame without processing it. Returns how many were taken. +async fn drop_queued(tn: &mut TestNode) -> usize { + let deadline = std::time::Instant::now() + Duration::from_secs(1); + while tn.packet_rx.is_empty() && std::time::Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(5)).await; + } + let mut dropped = 0; + while tn.packet_rx.try_recv().is_ok() { + dropped += 1; + } + dropped +} + +/// ReceiverReports M has seen from P, including stale and duplicate ones. +fn reports_seen(fx: &FlipFixture) -> u64 { + fx.nodes[M] + .node + .get_peer(&fx.p) + .and_then(|peer| peer.mmp()) + .map_or(0, |mmp| mmp.metrics.reports_seen()) +} + +/// Whether the filter P stores for M contains the marker. +fn holds_marker(fx: &FlipFixture) -> bool { + fx.nodes[P] + .node + .get_peer(&fx.m) + .and_then(|peer| peer.inbound_filter()) + .is_some_and(|filter| filter.contains(&marker())) +} + +/// Parent-switch counts at M and P before a run of MMP rounds. +/// +/// A first RTT sample can re-evaluate the parent, and a switch marks every +/// peer, which would pass a resend test for a reason unrelated to the resend. +struct SwitchGuard { + m: u64, + p: u64, +} + +/// Snapshot M's and P's parent-switch counters. +fn switch_guard(fx: &FlipFixture) -> SwitchGuard { + SwitchGuard { + m: fx.nodes[M].node.metrics().tree.parent_switches.get(), + p: fx.nodes[P].node.metrics().tree.parent_switches.get(), + } +} + +/// Assert neither M nor P switched parent since `guard`, and M's parent is P. +fn assert_unswitched(fx: &FlipFixture, guard: &SwitchGuard) { + assert_eq!( + fx.nodes[M].node.metrics().tree.parent_switches.get(), + guard.m, + "setup: M must not switch parent during the MMP rounds" + ); + assert_eq!( + fx.nodes[P].node.metrics().tree.parent_switches.get(), + guard.p, + "setup: P must not switch parent during the MMP rounds" + ); + assert_eq!( + fx.nodes[M].node.tree_state().my_declaration().parent_id(), + &fx.p, + "setup: M's parent must still be P" + ); +} + +/// FilterAnnounces M has sent. +fn sent_count(fx: &FlipFixture) -> u64 { + fx.nodes[M].node.metrics().bloom.sent.get() +} + +/// Drain the fixture, then run MMP rounds until M has seen a report from P. +async fn start_reports(fx: &mut FlipFixture) { + drain_quiet(&mut fx.nodes).await; + for _ in 0..10 { + if reports_seen(fx) >= 1 { + break; + } + mmp_round(&mut fx.nodes).await; + } + assert!(reports_seen(fx) >= 1, "setup: P must report to M"); +} + +/// Deliver C's filter carrying the marker and send M's announce of it to P. +/// Returns M's sent count after the send. +async fn send_marker(fx: &mut FlipFixture) -> u64 { + let c = fx.c; + deliver_filter(fx, &[c, marker()]).await; + let before = sent_count(fx); + fx.nodes[M].node.send_pending_filter_announces().await; + let after = sent_count(fx); + assert_eq!(after, before + 1, "setup: M must send exactly one announce"); + after +} + +/// Lose the announce just sent to P, and check the loss took. +async fn lose_announce(fx: &mut FlipFixture) { + assert_eq!( + drop_queued(&mut fx.nodes[P]).await, + 1, + "setup: exactly the one announce frame must be lost" + ); + assert!( + !holds_marker(fx), + "control: P must not hold the lost announce's content" + ); + assert!( + sent_to_parent(fx).contains(&marker()), + "control: M must record the lost announce as sent" + ); +} + +/// Get reports flowing from P to M, then send M's announce carrying the +/// marker and lose it on the way to P. Returns M's sent count after the send. +async fn lose_marker(fx: &mut FlipFixture) -> u64 { + start_reports(fx).await; + let sent = send_marker(fx).await; + lose_announce(fx).await; + sent +} + +/// The first eight bytes of the handshake hash of M's current session with P. +fn link_epoch(fx: &FlipFixture) -> [u8; 8] { + let hash = fx.nodes[M] + .node + .get_peer(&fx.p) + .and_then(|peer| peer.noise_session()) + .expect("M has a session with P") + .handshake_hash(); + let mut epoch = [0u8; 8]; + epoch.copy_from_slice(&hash[..8]); + epoch +} + +/// A FilterAnnounce lost in transit is resent once a receiver report shows +/// the loss, so the peer ends up holding the filter. +#[tokio::test] +async fn test_bloom_filter_announce_lost_in_transit_reaches_the_peer_after_a_receiver_report() { + let mut fx = flip_fixture(true).await; + lose_marker(&mut fx).await; + + let seen = reports_seen(&fx); + let guard = switch_guard(&fx); + for _ in 0..3 { + mmp_round(&mut fx.nodes).await; + fx.nodes[M].node.check_bloom_state().await; + process_available_packets(&mut fx.nodes).await; + } + assert!( + reports_seen(&fx) > seen, + "setup: a receiver report must arrive after the loss" + ); + assert_unswitched(&fx, &guard); + + assert!( + holds_marker(&fx), + "P must hold the filter whose announce was lost" + ); + cleanup_nodes(&mut fx.nodes).await; +} + +/// Receiving the same filter again under a newer sequence changes no outgoing +/// filter, so it marks no peer and cannot cascade. +#[tokio::test] +async fn test_bloom_unchanged_filter_with_newer_sequence_marks_no_peer() { + let mut fx = flip_fixture(true).await; + let (c, p) = (fx.c, fx.p); + + deliver_filter(&mut fx, &[c, marker()]).await; + fx.nodes[M].node.send_pending_filter_announces().await; + let bloom = &fx.nodes[M].node.bloom_state; + assert!( + !bloom.needs_update(&p) && !bloom.needs_update(&c), + "control: the first delivery must be fully sent" + ); + + deliver_filter(&mut fx, &[c, marker()]).await; + let bloom = &fx.nodes[M].node.bloom_state; + assert!(!bloom.needs_update(&p), "P must not be marked"); + assert!(!bloom.needs_update(&c), "C must not be marked"); + cleanup_nodes(&mut fx.nodes).await; +} + +/// Make M rekey on its next check: one message on a session is enough, time +/// never triggers it, and both ends of M's links are aged past the +/// responder's rekey-acceptance gate so both rekeys are ordinary ones. +fn arm_rekey(fx: &mut FlipFixture) { + fx.nodes[M].node.replace_context(|ctx| { + let mut cfg = (*ctx.config).clone(); + cfg.node.rekey.enabled = true; + cfg.node.rekey.after_messages = 1; + cfg.node.rekey.after_secs = u64::MAX; + ctx.config = std::sync::Arc::new(cfg); + }); + let (m, p, c) = (fx.m, fx.p, fx.c); + let age = Duration::from_secs(31); + for (i, remote) in [(M, p), (P, m), (M, c), (C, m)] { + fx.nodes[i] + .node + .get_peer_mut(&remote) + .expect("setup: link peer present") + .test_backdate_session_established(age); + } +} + +/// Drive the real rekey handshake until M's session with P is cut over. +async fn rekey_cutover(fx: &mut FlipFixture) { + let before = link_epoch(fx); + for _ in 0..6 { + fx.nodes[M].node.check_rekey().await; + fx.nodes[P].node.check_rekey().await; + for _ in 0..3 { + tokio::time::sleep(Duration::from_millis(5)).await; + process_available_packets(&mut fx.nodes).await; + } + if link_epoch(fx) != before { + break; + } + } + assert_ne!(link_epoch(fx), before, "setup: M's link to P must rekey"); + let (m, p) = (fx.m, fx.p); + assert!( + !fx.nodes[M].node.get_peer(&p).unwrap().rekey_in_progress(), + "setup: M's rekey with P must be complete" + ); + assert!( + !fx.nodes[P].node.get_peer(&m).unwrap().rekey_in_progress(), + "setup: P's rekey with M must be complete" + ); +} + +/// An announce lost just before a link rekey is resent on the new session. +#[tokio::test] +async fn test_bloom_announce_lost_before_a_link_rekey_reaches_the_peer_after_the_cutover() { + let mut fx = flip_fixture(true).await; + let sent = lose_marker(&mut fx).await; + + arm_rekey(&mut fx); + rekey_cutover(&mut fx).await; + + let guard = switch_guard(&fx); + for _ in 0..5 { + mmp_round(&mut fx.nodes).await; + fx.nodes[M].node.check_bloom_state().await; + process_available_packets(&mut fx.nodes).await; + } + assert_unswitched(&fx, &guard); + + assert_eq!( + sent_count(&fx), + sent + 1, + "M must resend to P exactly once after the loss" + ); + assert!( + holds_marker(&fx), + "P must hold the filter whose announce was lost before the rekey" + ); + assert!( + !fx.nodes[M].node.bloom_state.announce_outstanding(&fx.p), + "M's resend on the new session must be confirmed" + ); + cleanup_nodes(&mut fx.nodes).await; +} + +/// An announce that arrives is confirmed from the receiver reports and never +/// resent. +#[tokio::test] +async fn test_bloom_delivered_announce_is_confirmed_without_a_resend() { + let mut fx = flip_fixture(true).await; + let p = fx.p; + start_reports(&mut fx).await; + let sent = send_marker(&mut fx).await; + process_available_packets(&mut fx.nodes).await; + assert!(holds_marker(&fx), "control: P must hold the announce"); + + let guard = switch_guard(&fx); + for _ in 0..3 { + mmp_round(&mut fx.nodes).await; + fx.nodes[M].node.check_bloom_state().await; + process_available_packets(&mut fx.nodes).await; + } + assert_unswitched(&fx, &guard); + + let bloom = &fx.nodes[M].node.bloom_state; + assert_eq!( + sent_count(&fx), + sent, + "M must not resend a delivered announce" + ); + assert!(!bloom.needs_update(&p), "P must not be marked"); + assert!( + !bloom.announce_outstanding(&p), + "the delivered announce must be confirmed" + ); + cleanup_nodes(&mut fx.nodes).await; +} + +/// On a converged mesh with clean links, every announce is confirmed from the +/// receiver reports and none is resent. The convergence announces go out +/// before any report, so this checks the zero baseline on real counters. +#[tokio::test] +async fn test_bloom_clean_links_confirm_every_announce_without_a_resend() { + let mut nodes = run_tree_test(3, &[(0, 1), (1, 2)], false).await; + drain_quiet(&mut nodes).await; + let snapshot = |nodes: &[TestNode]| -> Vec<(u64, u64, NodeAddr)> { + nodes + .iter() + .map(|tn| { + ( + tn.node.metrics().bloom.sent.get(), + tn.node.metrics().tree.parent_switches.get(), + *tn.node.tree_state().my_declaration().parent_id(), + ) + }) + .collect() + }; + let before = snapshot(&nodes); + + for _ in 0..5 { + for i in 0..nodes.len() { + zero_intervals(&mut nodes[i].node); + nodes[i].node.check_mmp_reports().await; + process_available_packets(&mut nodes).await; + } + for tn in nodes.iter_mut() { + tn.node.check_bloom_state().await; + } + process_available_packets(&mut nodes).await; + } + + assert_eq!( + snapshot(&nodes), + before, + "no node may send an announce, switch parent or change parent" + ); + for (i, tn) in nodes.iter().enumerate() { + for peer in tn.node.peers.keys() { + assert!( + !tn.node.bloom_state.announce_outstanding(peer), + "node {i} must have confirmed its announce to every peer" + ); + } + } + cleanup_nodes(&mut nodes).await; +} + +/// With no receiver report at all, a lost announce is still resent once the +/// fallback interval passes. +#[tokio::test] +async fn test_bloom_lost_announce_is_resent_after_the_fallback_when_no_receiver_report_arrives() { + let mut fx = flip_fixture(true).await; + drain_quiet(&mut fx.nodes).await; + fx.nodes[M].node.bloom_state.set_fallback(0); + let seen = reports_seen(&fx); + send_marker(&mut fx).await; + lose_announce(&mut fx).await; + + fx.nodes[M].node.check_bloom_state().await; + process_available_packets(&mut fx.nodes).await; + + assert_eq!(reports_seen(&fx), seen, "setup: P must send no report"); + assert!( + holds_marker(&fx), + "P must hold the filter once the fallback resends it" + ); + cleanup_nodes(&mut fx.nodes).await; +} + +/// The cumulative counters of the last report `node` accepted from `peer`. +fn rr_counters(node: &Node, peer: &NodeAddr) -> Option<(u64, u64, u32)> { + node.get_peer(peer)?.mmp()?.metrics.rr_counters() +} + +/// The next send counter of `node`'s current session with `peer`. +fn next_counter(node: &Node, peer: &NodeAddr) -> u64 { + node.get_peer(peer) + .and_then(|p| p.noise_session()) + .expect("setup: session present") + .current_send_counter() +} + +/// Around a rekey, reports that describe the previous session reach both +/// ends of the link: the initiator accepts one the responder built before it +/// switched, and the responder's frames from the old session pollute the +/// initiator's receiver. Neither kind of report may trigger a resend. +#[tokio::test] +async fn test_bloom_reports_from_the_previous_session_do_not_trigger_resends() { + let mut fx = flip_fixture(true).await; + let (m, p) = (fx.m, fx.p); + fx.nodes[P].node.bloom_state.set_update_debounce_ms(0); + start_reports(&mut fx).await; + arm_rekey(&mut fx); + + // A session reaching its rekey has carried many frames. Reserve counters + // on both old sessions so their counters stay above the new sessions' + // for the whole test, as they do in the field; with a short history the + // new counters pass them within a few rounds, the reports become usable, + // and each announce spends its one unchecked resend. + for (i, remote) in [(M, p), (P, m)] { + let session = fx.nodes[i] + .node + .get_peer_mut(&remote) + .and_then(|peer| peer.noise_session_mut()) + .expect("setup: session present"); + for _ in 0..1000 { + session + .take_send_counter() + .expect("setup: counter available"); + } + } + + // Reports both ways, then M reports alone so P holds interval data. + mmp_round(&mut fx.nodes).await; + zero_intervals(&mut fx.nodes[M].node); + zero_intervals(&mut fx.nodes[P].node); + fx.nodes[M].node.check_mmp_reports().await; + process_available_packets(&mut fx.nodes).await; + + // M starts the rekey and holds the new session, not yet cut over. + let before = link_epoch(&fx); + fx.nodes[M].node.check_rekey().await; + for _ in 0..10 { + if fx.nodes[M] + .node + .get_peer(&p) + .is_some_and(|peer| peer.pending_new_session().is_some()) + { + break; + } + process_available_packets(&mut fx.nodes).await; + } + assert!( + fx.nodes[M] + .node + .get_peer(&p) + .is_some_and(|peer| peer.pending_new_session().is_some()), + "setup: M must hold P's new session" + ); + assert_eq!(link_epoch(&fx), before, "setup: M must not have cut over"); + + // P reports on the old session; hold its frames back from M. + assert!( + fx.nodes[M].packet_rx.is_empty(), + "setup: M's queue is empty" + ); + zero_intervals(&mut fx.nodes[P].node); + fx.nodes[P].node.check_mmp_reports().await; + let deadline = std::time::Instant::now() + Duration::from_secs(1); + while fx.nodes[M].packet_rx.is_empty() && std::time::Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(5)).await; + } + let mut held = Vec::new(); + while let Ok(packet) = fx.nodes[M].packet_rx.try_recv() { + held.push(packet); + } + assert!(!held.is_empty(), "setup: P must queue its reports for M"); + + // M cuts over, then receives P's old-session frames. + fx.nodes[M].node.check_rekey().await; + assert_ne!(link_epoch(&fx), before, "setup: M must cut over"); + for packet in held { + fx.nodes[M].node.handle_encrypted_frame(packet).await; + } + mmp_round(&mut fx.nodes).await; + + let m_rr = rr_counters(&fx.nodes[M].node, &p).expect("setup: M holds a report"); + let p_rr = rr_counters(&fx.nodes[P].node, &m).expect("setup: P holds a report"); + assert!( + m_rr.0 >= next_counter(&fx.nodes[M].node, &p), + "setup: M's report must describe M's previous session" + ); + assert!( + p_rr.0 >= next_counter(&fx.nodes[P].node, &m), + "setup: P's report must carry P's previous-session counter" + ); + + fx.nodes[M].node.bloom_state.mark_update_needed(p); + fx.nodes[P].node.bloom_state.mark_update_needed(m); + fx.nodes[M].node.send_pending_filter_announces().await; + fx.nodes[P].node.send_pending_filter_announces().await; + process_available_packets(&mut fx.nodes).await; + assert!( + fx.nodes[M].node.bloom_state.announce_outstanding(&p) + && fx.nodes[P].node.bloom_state.announce_outstanding(&m), + "setup: both ends must have an announce outstanding" + ); + + let sent_m = fx.nodes[M].node.metrics().bloom.sent.get(); + let sent_p = fx.nodes[P].node.metrics().bloom.sent.get(); + let guard = switch_guard(&fx); + let mut last = rr_counters(&fx.nodes[P].node, &m); + let mut changes = 0; + for _ in 0..5 { + mmp_round(&mut fx.nodes).await; + fx.nodes[M].node.check_bloom_state().await; + fx.nodes[P].node.check_bloom_state().await; + process_available_packets(&mut fx.nodes).await; + let now = rr_counters(&fx.nodes[P].node, &m); + if now != last { + changes += 1; + } + last = now; + } + assert!( + changes >= 2, + "setup: P must accept at least two reports from M, saw {changes}" + ); + assert_unswitched(&fx, &guard); + let m_rr = rr_counters(&fx.nodes[M].node, &p).expect("setup: M holds a report"); + let p_rr = rr_counters(&fx.nodes[P].node, &m).expect("setup: P holds a report"); + assert!( + m_rr.0 >= next_counter(&fx.nodes[M].node, &p) + && p_rr.0 >= next_counter(&fx.nodes[P].node, &m), + "setup: both reports must still describe a previous session" + ); + + assert_eq!( + fx.nodes[M].node.metrics().bloom.sent.get(), + sent_m, + "M must not resend on reports from the previous session" + ); + assert_eq!( + fx.nodes[P].node.metrics().bloom.sent.get(), + sent_p, + "P must not resend on reports carrying its previous-session counter" + ); + assert!( + fx.nodes[M].node.bloom_state.announce_outstanding(&p), + "M must still hold its announce to P" + ); + assert!( + fx.nodes[P].node.bloom_state.announce_outstanding(&m), + "P must still hold its announce to M" + ); + cleanup_nodes(&mut fx.nodes).await; +} From 57a21b3ab87ae2e475917e5cc2b65b12748b6bcc Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 15:24:09 +0000 Subject: [PATCH 03/14] Assert datagram delivery over Ethernet in the ethernet-mesh chaos scenario No chaos scenario sent a datagram across an Ethernet link: both Ethernet scenarios asserted only that the tree formed, so framing, AEAD over AF_PACKET and delivery through the Ethernet transport were untested. A new delivery assertion pings node pairs at each configured payload size at teardown, after flapped links and stopped nodes are restored, one packet at a time until min_replies replies arrive or deadline_secs passes. The verdict is whether that happened, not a loss ratio, so a busy host slows the probe without failing it. With require_transport and transport_node it also reads that node's peers before and after the probe and fails if the node has no peers or any peer on another transport, so traversal is checked rather than assumed. A probe that never ran fails as a harness failure. ethernet-mesh asserts delivery to and from n04, whose only edges are Ethernet, at payloads 0 and 1200: n04 to n06 and back cross Ethernet then UDP, and n04 to n05 is one Ethernet hop. Break-checks: dropping echo requests on n06's TUN fails n04->n06 with zero replies while baseline passes; adding a UDP edge to n04 fails the traversal check. The payload sizes cannot exercise the receiver's padding trim on veth: an empty echo is a 226-byte frame and a 1200-byte echo 1342 bytes, and the veth does not pad shorter frames (54-byte control frames and 48-byte beacons arrived unpadded; no received data frame carried bytes past its length field). The trim is covered by the unit tests on the receive loop's parse. --- testing/chaos/README.md | 22 +++- testing/chaos/scenarios/ethernet-mesh.yaml | 45 +++++-- testing/chaos/scenarios/ethernet-only.yaml | 11 +- testing/chaos/sim/assertions.py | 96 ++++++++++++++ testing/chaos/sim/runner.py | 95 ++++++++++++++ testing/chaos/sim/scenario.py | 139 +++++++++++++++++++++ 6 files changed, 393 insertions(+), 15 deletions(-) diff --git a/testing/chaos/README.md b/testing/chaos/README.md index b86d08e2..900fa2db 100644 --- a/testing/chaos/README.md +++ b/testing/chaos/README.md @@ -105,7 +105,10 @@ Explicit topologies exercising non-UDP transports. - **ethernet-only**: 4-node ring on raw Ethernet (AF_PACKET). Peers discovered via beacons, not static config. Minimal netem (1-5ms delay). - **ethernet-mesh**: Mirrors `tcp-mesh` topology but with Ethernet instead of - TCP. UDP edges use static config; Ethernet edges use beacon discovery. + TCP. UDP edges use static config; Ethernet edges use beacon discovery. The + only scenario that asserts datagram delivery over Ethernet: its `delivery` + assertion pings to and from n04, whose only edges are Ethernet, at two + payload sizes, and checks that n04 has no peer on another transport. - **tcp-mesh**: 6-node mesh with 4 UDP and 3 TCP edges. Both transports use static peer config. Netem mutation (30% fraction, every 20-40s) and link flaps (1 link max, 10-20s down). @@ -258,6 +261,23 @@ The assertion thresholds in the shipped file are calibrated against recorded runs at the invocation CI uses, and the file's own comments say what they were derived from. Read those before retuning them. +A `delivery` assertion checks the data plane rather than the tree. It runs +at teardown, after flapped links and stopped nodes are restored, and pings +each pair at each payload size until `min_replies` replies arrive or +`deadline_secs` passes: + +```yaml +assertions: + delivery: + pairs: [[n04, n06], [n06, n04]] # [src, dst] node ids + payload_bytes: [0, 1200] # ICMPv6 echo payload sizes, 0-1400 + min_replies: 3 # default 3 + deadline_secs: 60 # default 60, per pair and size + require_transport: ethernet # optional, with transport_node: + transport_node: n04 # fail if n04 has no peers, or any + # peer on another transport +``` + ## Topology Algorithms | Algorithm | Parameters | Description | diff --git a/testing/chaos/scenarios/ethernet-mesh.yaml b/testing/chaos/scenarios/ethernet-mesh.yaml index d1ceb44f..a980856f 100644 --- a/testing/chaos/scenarios/ethernet-mesh.yaml +++ b/testing/chaos/scenarios/ethernet-mesh.yaml @@ -3,7 +3,8 @@ # Exercises both transports in a single mesh. UDP edges use static # peer config; Ethernet edges use beacon discovery. Tests that the # spanning tree converges across heterogeneous transports with netem -# and link flaps active. +# and link flaps active, and that datagrams are delivered over the +# Ethernet links (the delivery assertion below). # # Topology: # @@ -62,20 +63,46 @@ link_flaps: traffic: enabled: false -# Baseline: the mesh came up, agreed on a root, and took parents. This -# asserts nothing about Ethernet link behaviour under flaps; it exists -# so that a run in which the mesh never formed cannot report success, -# which until now it could, because this scenario carried no assertions -# at all. +# Baseline: the mesh came up, agreed on a root, and took parents. It +# says nothing about the data plane; it exists so that a run in which +# the mesh never formed cannot report success. Six nodes, one root, five +# parented in all six provably-completed archived runs. # -# Six nodes, one root, five parented in all six provably-completed -# archived runs. The other assertion covering Ethernet transport in -# CI; see ethernet-only for why that matters. +# Delivery: the one assertion anywhere in CI that a datagram crossed an +# Ethernet link. n04's only edges are Ethernet (n01-n04, n04-n05), so a +# probe to or from n04 must cross one, provided n04 has no peer on +# another transport; its container also sits on the docker network, so +# that is checked, not assumed: n04's peers are read before and after +# the probe, and the assertion fails if it has none or any is not +# Ethernet. n04 -> n06 and n06 -> n04 cross Ethernet and then UDP; +# n04 -> n05 is a direct Ethernet hop. +# +# Load-robust by shape: each pair and size is pinged one packet at a +# time until 3 replies or 60 s, and the verdict is whether that +# happened, not a loss ratio, so a busy host slows it without failing +# it. It runs at teardown, after flapped links are restored. +# +# Sizes: payload 0 is the smallest ICMPv6 echo and 1200 is near the +# 1280-byte TUN MTU. Neither can reach the receiver's padding trim on a +# veth link. Measured on a live run of this scenario (2026-09-19): an +# empty echo is a 226-byte Ethernet frame and a 1200-byte one 1342 +# bytes, far above the 60-byte minimum; and the veth does not pad the +# frames that are shorter (54-byte control frames and 48-byte beacons +# arrived as sent; none of 250 received data frames carried bytes past +# its length field). That trim is covered instead by the unit tests on the receive loop's +# data-frame parse (data_payload in src/transport/ethernet/mod.rs). assertions: baseline: min_nodes_reporting: 6 max_roots: 1 min_nodes_parented: 5 + delivery: + pairs: [[n04, n06], [n06, n04], [n04, n05]] + payload_bytes: [0, 1200] + min_replies: 3 + deadline_secs: 60 + require_transport: ethernet + transport_node: n04 logging: rust_log: "info" diff --git a/testing/chaos/scenarios/ethernet-only.yaml b/testing/chaos/scenarios/ethernet-only.yaml index 1b11f2f0..a0c1bea1 100644 --- a/testing/chaos/scenarios/ethernet-only.yaml +++ b/testing/chaos/scenarios/ethernet-only.yaml @@ -50,11 +50,12 @@ traffic: # A 4-node mesh forms a spanning tree: one root and three nodes # with a parent. All six provably-completed archived runs show exactly # that, so these are the shape of a converged mesh rather than a -# tolerance fitted to observations. This is the only assertion covering -# Ethernet transport anywhere in CI, and it covers the control plane -# only: traffic is disabled above, so no datagram crosses an Ethernet -# link in any test. Framing, the length field that trims NIC minimum- -# frame padding, and AEAD over Ethernet are all unexercised as a result. +# tolerance fitted to observations. This scenario covers the Ethernet +# control plane only (beacon discovery, peering, the tree): traffic is +# disabled above and no datagram is probed here. Data-plane delivery +# over Ethernet (framing, AEAD over AF_PACKET) is asserted in +# ethernet-mesh's delivery assertion, and the length field that trims +# minimum-frame padding by unit tests on the receive loop's parse. assertions: baseline: min_nodes_reporting: 4 diff --git a/testing/chaos/sim/assertions.py b/testing/chaos/sim/assertions.py index 1fc636aa..4c39f1a6 100644 --- a/testing/chaos/sim/assertions.py +++ b/testing/chaos/sim/assertions.py @@ -10,6 +10,15 @@ Currently supported assertions: - ``bloom_send_rate``: per-node trailing-window ceiling on ``stats.bloom.sent`` delta. Calibrated for the bloom-storm regression scenario but generally usable. +- ``min_parent_switches`` / ``max_parent_switches``: bounds on the + parent switches counted from the node logs. +- ``max_errors``: ceiling on ERROR-level log lines (applied by default). +- ``baseline``: the mesh formed, agreed on a root and took parents. +- ``tree_parents``: named nodes ended with the named parents. +- ``congestion_signals``: floors on how many nodes saw each congestion + counter. +- ``delivery``: datagrams of each configured size were delivered + between node pairs, optionally proven to cross one transport. """ from __future__ import annotations @@ -22,6 +31,7 @@ from .scenario import ( BaselineAssertion, BloomSendRateAssertion, CongestionSignalsAssertion, + DeliveryAssertion, MaxErrorsAssertion, MaxParentSwitchesAssertion, MinParentSwitchesAssertion, @@ -370,6 +380,92 @@ def evaluate_congestion_signals( ) +_TRANSPORT_LABEL = {"ethernet": "Ethernet", "udp": "UDP", "tcp": "TCP"} + + +def _transport_failure(transport: dict) -> str | None: + """Return why the transport proof failed, or None when it holds.""" + node, want = transport["node"], transport["want"] + for read in transport["reads"]: + when, peers = read["when"], read["peers"] + if peers is None: + return ( + f"traversal unproven: show_peers on {node} failed {when} the " + f"probe" + ) + if not peers: + return ( + f"traversal unproven: {node} reported zero peers {when} the " + f"probe, after waiting {read['waited_s']:.0f}s" + ) + other = [ + f"{str(p.get('npub', '?'))[:16]} via " + f"{p.get('transport_type', 'no transport_type')}" + for p in peers if p.get("transport_type") != want + ] + if other: + return ( + f"{node} has a non-{_TRANSPORT_LABEL.get(want, want)} peer " + f"{when} the probe ({'; '.join(other)}), so a probe to or " + f"from it may not have crossed {want}" + ) + return None + + +def evaluate_delivery( + cfg: DeliveryAssertion, + probe: dict | None, +) -> AssertionOutcome: + """Every configured pair and size got ``min_replies`` replies in time. + + ``probe`` is the runner's delivery probe result. None means the probe + never ran, which fails as a harness failure: no probe and no delivery + would otherwise look alike. + """ + if probe is None: + return AssertionOutcome( + name="delivery", + passed=False, + detail=( + "FAIL delivery: the delivery probe never ran, so nothing was " + "observed. This is a harness failure, not a statement about " + "delivery." + ), + ) + + failures = [] + transport = probe.get("transport") + if transport is not None: + why = _transport_failure(transport) + if why: + failures.append(why) + + parts = [] + for r in probe["results"]: + label = f"{r['src']}->{r['dst']} {r['size']}B" + if r["replies"] >= cfg.min_replies: + parts.append(f"{label} {r['replies']}/{r['attempts']} in {r['elapsed_s']:.1f}s") + else: + failures.append( + f"{label} got {r['replies']} of {cfg.min_replies} replies in " + f"{r['elapsed_s']:.0f}s ({r['attempts']} attempts); last ping: " + f"{r['last_output']!r}" + ) + + if failures: + return AssertionOutcome( + name="delivery", + passed=False, + detail=f"FAIL delivery: {'; '.join(failures)}", + ) + via = f" via {transport['want']} ({transport['node']})" if transport else "" + return AssertionOutcome( + name="delivery", + passed=True, + detail=f"PASS delivery{via}: {'; '.join(parts)}", + ) + + def evaluate_max_errors( cfg: MaxErrorsAssertion, errors: list[tuple[str, str]], diff --git a/testing/chaos/sim/runner.py b/testing/chaos/sim/runner.py index b97a1653..44c4261a 100644 --- a/testing/chaos/sim/runner.py +++ b/testing/chaos/sim/runner.py @@ -17,6 +17,7 @@ from .assertions import ( BloomSendRateMonitor, evaluate_baseline, evaluate_congestion_signals, + evaluate_delivery, evaluate_max_errors, evaluate_max_parent_switches, evaluate_min_parent_switches, @@ -25,6 +26,7 @@ from .assertions import ( from .compose import generate_compose from .config_gen import write_configs from .control import ( + query_peers, query_status, snapshot_all_congestion, snapshot_all_mmp, @@ -96,6 +98,9 @@ class SimRunner: # than as an absence of congestion. self.final_congestion: dict | None = None self.final_tree: dict | None = None + # Set by _probe_delivery. None means the probe never ran, which the + # delivery assertion reports as a harness failure. + self.delivery_probe: dict | None = None def _evaluate_max_parent_switches( self, cfg, parent_switches: list[tuple[str, str]] @@ -712,6 +717,17 @@ class SimRunner: # it as absent. Wait for every node to answer first. self._wait_control_sockets(CONTROL_WAIT_SECS) + # Every flapped link and stopped node is back, so the delivery + # probe measures the restored mesh. + if self.scenario.assertions.delivery is not None: + try: + self._probe_delivery() + except Exception: + # Leaves delivery_probe None, which the assertion + # reports as a harness failure; the rest of teardown + # still has to run. + log.exception("Delivery probe failed") + # Collect iperf3 throughput results before containers stop if self.traffic_mgr: iperf_results = self.traffic_mgr.collect_results() @@ -797,6 +813,15 @@ class SimRunner: else: log.error("%s", outcome.detail) + dl_cfg = self.scenario.assertions.delivery + if dl_cfg is not None: + outcome = evaluate_delivery(dl_cfg, self.delivery_probe) + self.assertion_outcomes.append(outcome) + if outcome.passed: + log.info("%s", outcome.detail) + else: + log.error("%s", outcome.detail) + cong_cfg = self.scenario.assertions.congestion_signals if cong_cfg is not None: outcome = evaluate_congestion_signals( @@ -878,6 +903,76 @@ class SimRunner: else: log.info("Control socket wait: all nodes answered after %.1fs", elapsed) + def _read_peers(self, node_id: str, when: str, wait_secs: float) -> dict: + """Read a node's peers, waiting up to wait_secs for at least one. + + A node restored at teardown may not have re-peered yet, which is + not the failure the transport check is for, so an empty list is + re-read until the wait runs out. A failed read is retried the same + way and reported as None if it never succeeds. + """ + container = self.topology.container_name(node_id) + start = time.monotonic() + while True: + data = query_peers(container) + peers = None if data is None else data.get("peers", []) + waited = time.monotonic() - start + if peers or waited >= wait_secs: + return {"when": when, "peers": peers, "waited_s": waited} + time.sleep(2) + + def _ping_until(self, src: str, dst: str, size: int, cfg) -> dict: + """Ping dst from src until cfg.min_replies replies or the deadline.""" + container = self.topology.container_name(src) + target = f"{self.topology.nodes[dst].npub}.fips" + cmd = ["docker", "exec", container, "ping6", "-c", "1", "-W", "2", + "-s", str(size), target] + start = time.monotonic() + replies = attempts = 0 + last = "" + while replies < cfg.min_replies and time.monotonic() - start < cfg.deadline_secs: + attempts += 1 + try: + res = subprocess.run(cmd, capture_output=True, text=True, timeout=15) + lines = (res.stdout + res.stderr).strip().splitlines() + last = lines[-1] if lines else "" + if res.returncode == 0: + replies += 1 + continue + except subprocess.TimeoutExpired: + last = "docker exec ping6 timed out" + time.sleep(1) + return { + "src": src, "dst": dst, "size": size, "replies": replies, + "attempts": attempts, "elapsed_s": time.monotonic() - start, + "last_output": last, + } + + def _probe_delivery(self): + """Run the delivery assertion's probes and keep the result.""" + cfg = self.scenario.assertions.delivery + log.info("Probing delivery: %d pair(s) x %d size(s)", + len(cfg.pairs), len(cfg.payload_bytes)) + transport = None + if cfg.require_transport: + transport = { + "node": cfg.transport_node, "want": cfg.require_transport, + "reads": [self._read_peers(cfg.transport_node, "before", + cfg.deadline_secs)], + } + results = [ + self._ping_until(src, dst, size, cfg) + for src, dst in cfg.pairs + for size in cfg.payload_bytes + ] + if transport is not None: + transport["reads"].append( + self._read_peers(cfg.transport_node, "after", 0) + ) + self.delivery_probe = {"transport": transport, "results": results} + with open(os.path.join(self.output_dir, "delivery-probe.json"), "w") as f: + json.dump(self.delivery_probe, f, indent=2) + def _take_snapshot(self, label: str): """Query all nodes via control socket and save tree/MMP/congestion snapshots.""" if not self.topology: diff --git a/testing/chaos/sim/scenario.py b/testing/chaos/sim/scenario.py index 56c27947..a2a6a023 100644 --- a/testing/chaos/sim/scenario.py +++ b/testing/chaos/sim/scenario.py @@ -301,6 +301,32 @@ class CongestionSignalsAssertion: min_nodes_ce_received: int | None = None +@dataclass +class DeliveryAssertion: + """Datagrams of each size must be delivered between each node pair. + + The probe runs at teardown, after every flapped link and stopped node + has been restored. For each pair and payload size it repeats a + one-packet ping6 over the mesh until ``min_replies`` replies have come + back or ``deadline_secs`` has passed. The verdict is binary per pair + and size, not a loss ratio, so a busy host slows the probe without + failing it. + + ``require_transport`` with ``transport_node`` proves the probes + crossed that transport: the node's peers are read before and after + the probe, and the assertion fails if the node has no peers or any + peer on another transport. Choose a node whose only edges use that + transport and put it in every pair. + """ + + pairs: list[tuple[str, str]] = field(default_factory=list) + payload_bytes: list[int] = field(default_factory=list) + min_replies: int = 3 + deadline_secs: int = 60 + require_transport: str | None = None + transport_node: str | None = None + + @dataclass class MaxErrorsAssertion: """Ceiling on ERROR-level lines across every node's log. @@ -334,6 +360,7 @@ class AssertionsConfig: congestion_signals: CongestionSignalsAssertion | None = None tree_parents: TreeParentsAssertion | None = None baseline: BaselineAssertion | None = None + delivery: DeliveryAssertion | None = None @dataclass @@ -414,6 +441,7 @@ _SECTION_KEYS = { "assertions": { "bloom_send_rate", "min_parent_switches", "max_parent_switches", "max_errors", "congestion_signals", "tree_parents", "baseline", + "delivery", }, "logging": {"rust_log", "output_dir"}, } @@ -428,7 +456,14 @@ _ASSERTION_KEYS = { "baseline": { "min_nodes_reporting", "max_roots", "min_nodes_parented", "min_sessions", }, + "delivery": { + "pairs", "payload_bytes", "min_replies", "deadline_secs", + "require_transport", "transport_node", + }, } +# Largest delivery probe payload: an ICMPv6 echo of this size fits the +# 1280-byte TUN MTU with FIPS overhead to spare. +_DELIVERY_MAX_PAYLOAD = 1400 _NETEM_POLICY_KEYS = { "delay_ms", "jitter_ms", "loss_pct", "duplicate_pct", "reorder_pct", "corrupt_pct", @@ -776,6 +811,10 @@ def load_scenario(path: str) -> Scenario: "min_nodes_ce_received); a block with none asserts nothing" ) s.assertions.congestion_signals = CongestionSignalsAssertion(**floors) + if "delivery" in asrt: + s.assertions.delivery = _parse_delivery( + asrt["delivery"], s.topology.num_nodes + ) if "tree_parents" in asrt: tp = asrt["tree_parents"] if not isinstance(tp, dict) or not tp: @@ -861,6 +900,106 @@ def load_scenario(path: str) -> Scenario: return s +def _delivery_node(val, num_nodes: int, where: str) -> str: + """Validate one node id named by the delivery assertion.""" + if not isinstance(val, str) or not _NODE_ID_RE.fullmatch(val): + raise ValueError( + f"assertions.delivery.{where}: {val!r} is not a node id of the " + f"form 'n04'" + ) + idx = int(val[1:]) + if idx < 1 or idx > num_nodes: + raise ValueError( + f"assertions.delivery.{where}: '{val}' is outside this " + f"scenario's {num_nodes} nodes" + ) + return val + + +def _delivery_int(dl: dict, key: str, default: int) -> int: + """Read a positive integer setting of the delivery assertion.""" + val = dl.get(key, default) + if isinstance(val, bool) or not isinstance(val, int) or val < 1: + raise ValueError( + f"assertions.delivery.{key}: must be a positive integer, got {val!r}" + ) + return val + + +def _parse_delivery(dl, num_nodes: int) -> DeliveryAssertion: + """Parse and validate the delivery assertion block.""" + if not isinstance(dl, dict): + raise ValueError("assertions.delivery: must be a mapping") + _reject_unknown(dl, _ASSERTION_KEYS["delivery"], "assertions.delivery") + + raw_pairs = dl.get("pairs") + if not isinstance(raw_pairs, list) or not raw_pairs: + raise ValueError( + "assertions.delivery.pairs: give at least one [src, dst] pair; " + "an empty list asserts nothing" + ) + pairs = [] + for pair in raw_pairs: + if not isinstance(pair, list) or len(pair) != 2: + raise ValueError( + f"assertions.delivery.pairs: each entry must be [src, dst], " + f"got {pair!r}" + ) + src = _delivery_node(pair[0], num_nodes, "pairs") + dst = _delivery_node(pair[1], num_nodes, "pairs") + if src == dst: + raise ValueError( + f"assertions.delivery.pairs: [{src}, {dst}] pings a node from " + f"itself, which crosses no link" + ) + pairs.append((src, dst)) + + sizes = dl.get("payload_bytes") + if not isinstance(sizes, list) or not sizes: + raise ValueError( + "assertions.delivery.payload_bytes: give at least one size; an " + "empty list asserts nothing" + ) + for size in sizes: + if (isinstance(size, bool) or not isinstance(size, int) + or not 0 <= size <= _DELIVERY_MAX_PAYLOAD): + raise ValueError( + f"assertions.delivery.payload_bytes: each size must be an " + f"integer from 0 to {_DELIVERY_MAX_PAYLOAD}, got {size!r}" + ) + + transport = dl.get("require_transport") + node = dl.get("transport_node") + if (transport is None) != (node is None): + raise ValueError( + "assertions.delivery: require_transport and transport_node go " + "together; one without the other proves nothing" + ) + if transport is not None: + if transport not in VALID_TRANSPORTS: + raise ValueError( + f"assertions.delivery.require_transport: {transport!r} is not " + f"one of {', '.join(VALID_TRANSPORTS)}" + ) + node = _delivery_node(node, num_nodes, "transport_node") + missing = [p for p in pairs if node not in p] + if missing: + raise ValueError( + f"assertions.delivery: pairs {missing} do not include " + f"transport_node '{node}', so the transport check says " + f"nothing about them" + ) + + return DeliveryAssertion( + pairs=pairs, + payload_bytes=list(sizes), + min_replies=_delivery_int(dl, "min_replies", 3), + deadline_secs=_delivery_int(dl, "deadline_secs", 60), + require_transport=transport, + transport_node=node, + ) + + _SUPPRESSING_LOG_LEVELS = ("off", "error", "warn") From d246f84da5ca9ced1d0812c96dabb58c2c146ba5 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 18:09:44 +0000 Subject: [PATCH 04/14] Rebuild alias name resolution and ACL alias entries when the peer list is replaced at runtime Node::update_peers replaced the configured peer list but left every map that reads peer aliases as it was at startup. Each hosts map (the display map, the peer ACL's alias resolution and the .fips DNS responder's map) kept the startup aliases as a fixed base and only re-merged the hosts file over it. After an update, a new peer's alias did not resolve as a .fips name or match an ACL entry naming it, a removed or moved alias kept resolving to the old npub, and a deny entry written as an alias that moved to another key kept denying the old key while admitting the new one. A kept peer whose alias was removed also kept showing the old alias as its display name. update_peers now rebuilds the alias base from the new peer list and hands it to all three maps: the display map directly, the peer ACL through a forced rebuild that runs before any added peer is dialed, and the running DNS responder through a watch channel it checks before each answer. Each map keeps the hosts file as last read, so the file stays merged on top and still wins, including edits picked up at runtime. The kept peer's display name falls back to its short npub when its alias is removed. run_dns_responder keeps its signature. This reaches only embedders that call Node::update_peers. --- src/control/snapshot.rs | 11 +- src/node/acl.rs | 65 ++++- src/node/lifecycle/mod.rs | 30 ++- src/node/lifecycle/supervisor.rs | 5 + src/node/mod.rs | 25 +- src/node/reloadable.rs | 90 ++++++- src/node/tests/mod.rs | 1 + src/node/tests/update_peers.rs | 397 +++++++++++++++++++++++++++++++ src/upper/dns.rs | 83 ++++++- src/upper/hosts.rs | 98 +++++++- 10 files changed, 769 insertions(+), 36 deletions(-) create mode 100644 src/node/tests/update_peers.rs diff --git a/src/control/snapshot.rs b/src/control/snapshot.rs index bea9d8d7..c7263b07 100644 --- a/src/control/snapshot.rs +++ b/src/control/snapshot.rs @@ -10,7 +10,8 @@ //! - the cheap scalar gauges `show_status` needs (`estimated_mesh_size`, //! node `state`, `tun_state`, `tun_name`, `effective_ipv6_mtu`, and the //! peer / session / link / connection / transport counts), plus -//! `peer_aliases` (effectively immutable after construction). +//! `peer_aliases` (copied from the node's display-name map on each tick; +//! it changes when `update_peers` replaces the peer list). //! //! The snapshot holds *data*, not rendered `Response` envelopes: //! rendering happens in the control task off the rx_loop. Staleness is bounded @@ -61,11 +62,13 @@ pub(crate) struct StatsSnapshot { /// transport type. Configured-but-idle types appear with a zero count. /// Keyed by the transport type name (`"udp"`, `"tcp"`, `"tor"`, ...). pub transport_peer_counts: std::collections::BTreeMap, - /// Configured peer aliases, keyed by `NodeAddr`. Effectively immutable - /// after construction; shared to avoid a per-tick map clone. + /// Configured peer aliases, keyed by `NodeAddr`. Copied from the node's + /// display-name map on each stats tick; it changes when `update_peers` + /// replaces the peer list. pub peer_aliases: Arc>, /// Loaded peer-ACL status (`show_acl`). The ACL itself is an - /// `arc_swap::ArcSwap` mutated only by the tick's `reload_peer_acl`; + /// `arc_swap::ArcSwap` mutated only by the tick's `reload_peer_acl` + /// and by `update_peers`; /// the human-readable status is a cheap projection of it. pub acl_status: PeerAclStatus, /// Per-stats-history-peer metadata resolved against the live peer/session diff --git a/src/node/acl.rs b/src/node/acl.rs index d079cf3a..3ba32472 100644 --- a/src/node/acl.rs +++ b/src/node/acl.rs @@ -529,7 +529,8 @@ impl PeerAcl { /// [`arc_swap::ArcSwap`] so the authorization hot path reads it without /// locking, while the reloader's change-detection state (file mtimes, the /// embedded hosts reloader) is touched only by [`Reloadable::reload`] on the -/// single node tick task. +/// node tick task and by [`PeerAclReloader::rebase`], which `update_peers` +/// reaches through `&mut Node`, so there is still one writer at a time. pub struct PeerAclReloader { /// Reader-facing effective ACL snapshot. acl: arc_swap::ArcSwap, @@ -551,6 +552,10 @@ pub struct PeerAclReloader { retry_pending: bool, /// Consecutive reloads held back by the empty-snapshot guard. empty_holds: u32, + /// Set when the alias base changed, forcing the next reload to rebuild + /// although no file changed. Cleared only when a rebuilt ACL is + /// published, so a held reload retries the rebuild on later ticks. + rebased: bool, } impl PeerAclReloader { @@ -646,9 +651,22 @@ impl PeerAclReloader { last_deny_mtime, retry_pending, empty_holds: 0, + rebased: false, } } + /// Replace the peer-alias base the ACL's alias entries resolve through, + /// and rebuild the ACL from it now. + /// + /// Goes through [`Reloadable::reload`], so an unreadable input holds the + /// last good ACL and the empty-ACL guard applies exactly as on a tick. + /// Returns `true` if a rebuilt ACL was published. + pub(crate) async fn rebase(&mut self, base: HostMap) -> bool { + self.hosts.set_base(base); + self.rebased = true; + self.reload().await + } + /// Keep the published snapshot after a reload input failed to read. /// /// Leaves the recorded mtimes and the ACL in force untouched, arms the @@ -714,6 +732,7 @@ impl Reloadable for PeerAclReloader { && deny_mtime == self.last_deny_mtime && !hosts_changed && !self.retry_pending + && !self.rebased && !allow_moved && !deny_moved { @@ -764,6 +783,7 @@ impl Reloadable for PeerAclReloader { ); } self.retry_pending = false; + self.rebased = false; self.empty_holds = 0; self.last_allow_mtime = allow_mtime; self.last_deny_mtime = deny_mtime; @@ -1800,4 +1820,47 @@ mod tests { PeerAclDecision::DefaultAllow ); } + + /// Rebasing the alias map rebuilds and publishes the ACL although no ACL + /// or hosts file changed, and the forced rebuild does not repeat on the + /// next reload. + #[tokio::test] + async fn rebase_republishes_alias_entries_without_any_file_change() { + let dir = tempfile::tempdir().unwrap(); + let allow = dir.path().join("peers.allow"); + let deny = dir.path().join("peers.deny"); + let hosts = dir.path().join("hosts"); + let (x, y) = (test_npub(), test_npub()); + write_file(&allow, "node-a\n"); + + let mut base = HostMap::new(); + base.insert("node-a", &x).unwrap(); + let mut reloader = PeerAclReloader::with_alias_sources(allow, deny, base, hosts); + assert_eq!( + reloader.acl().check(&test_peer(&x)), + PeerAclDecision::AllowList, + "alias resolves to X at startup" + ); + + let mut moved = HostMap::new(); + moved.insert("node-a", &y).unwrap(); + assert!( + reloader.rebase(moved).await, + "rebase publishes a rebuilt ACL" + ); + assert_eq!( + reloader.acl().check(&test_peer(&y)), + PeerAclDecision::AllowList, + "alias entry follows the new base to Y" + ); + assert_eq!( + reloader.acl().check(&test_peer(&x)), + PeerAclDecision::DefaultAllow, + "X is no longer on the allow list" + ); + assert!( + !reloader.reload().await, + "a reload with nothing changed does not rebuild again" + ); + } } diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index 6d16427f..cca08fd2 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -89,6 +89,11 @@ impl Node { /// is already connected and a new concrete candidate appears, FIPS starts /// an alternate handshake in parallel; promotion switches only after that /// handshake authenticates. + /// + /// Peer aliases follow the new list: `.fips` names, display names and + /// peer ACL entries written as an alias are rebuilt from it, with the + /// hosts file still taking precedence. Rebuilding the ACL re-reads the + /// ACL files and checks the hosts file before any added peer is dialed. pub async fn update_peers( &mut self, new_peers: Vec, @@ -164,8 +169,12 @@ impl Node { state.peer_config = new_peer.clone(); state.retry_after_ms = Self::now_ms(); } - if let Some(alias) = new_peer.alias.clone() { - self.peer_aliases.insert(*node_addr, alias); + if let Ok(identity) = PeerIdentity::from_npub(&new_peer.npub) { + let name = new_peer + .alias + .clone() + .unwrap_or_else(|| identity.short_npub()); + self.peer_aliases.insert(*node_addr, name); } } else { outcome.unchanged += 1; @@ -185,6 +194,7 @@ impl Node { let mut new_config = (*self.context.config).clone(); new_config.peers = new_by_addr.into_values().collect(); self.replace_context(|ctx| ctx.config = std::sync::Arc::new(new_config)); + self.rebase_aliases().await; for peer_config in added_configs { outcome.added += 1; @@ -1843,6 +1853,8 @@ impl Node { let hosts_path = std::path::PathBuf::from( crate::upper::hosts::DEFAULT_HOSTS_PATH, ); + let (aliases_tx, aliases_rx) = + tokio::sync::watch::channel(base_hosts.clone()); let reloader = crate::upper::hosts::HostMapReloader::new( base_hosts, hosts_path, ); @@ -1880,17 +1892,19 @@ impl Node { let dns_child_tx = self.child_exit_tx.clone(); let handle = tokio::spawn(report_exit( Child::Dns, - crate::upper::dns::run_dns_responder( + crate::upper::dns::run_responder( socket, identity_tx, dns_ttl, reloader, + Some(aliases_rx), mesh_ifindex, ), dns_child_tx, )); self.supervisor.dns_identity_rx = Some(identity_rx); self.supervisor.dns_task = Some(handle); + self.supervisor.dns_aliases = Some(aliases_tx); self.supervisor.dns_local_addr = Some(local_addr); Event::SubstrateUp { child } } @@ -2161,6 +2175,7 @@ impl Node { handle.abort(); debug!("DNS responder stopped"); } + self.supervisor.dns_aliases.take(); // Retract the published address in the same step that kills // the listener, so an embedder polling `dns_local_addr()` // never dials a socket that is already gone. @@ -2276,10 +2291,11 @@ impl Node { /// reads it to rebuild the teardown set, and aborting an already-finished /// handle there is harmless. /// - /// For `Dns` the event comes only from a panic. `run_dns_responder` is an - /// unconditional loop whose every failure arm continues, so it has no - /// ordinary exit; [`report_exit`] catches a panic in it and reports - /// `Child::Dns`, which is what reaches this. + /// For `Dns` the event comes only from a panic. `run_responder`, the loop + /// the node spawns and `run_dns_responder` wraps, is an unconditional + /// loop whose every failure arm continues, so it has no ordinary exit; + /// [`report_exit`] catches a panic in it and reports `Child::Dns`, which + /// is what reaches this. pub(in crate::node) fn retract_child_publications(&mut self, child: Child) { if matches!(child, Child::Dns) { self.supervisor.dns_local_addr.take(); diff --git a/src/node/lifecycle/supervisor.rs b/src/node/lifecycle/supervisor.rs index af5197da..e62a399b 100644 --- a/src/node/lifecycle/supervisor.rs +++ b/src/node/lifecycle/supervisor.rs @@ -697,6 +697,10 @@ pub(crate) struct Supervisor { /// only while the responder is up; published to embedders through /// [`Node::dns_local_addr`](crate::Node::dns_local_addr). pub(in crate::node) dns_local_addr: Option, + /// Sends a new peer-alias base to the running DNS responder; `Some` only + /// while it runs. + pub(in crate::node) dns_aliases: + Option>, /// Sender for each UDP listen socket the transport spawn binds — its raw /// fd and the instance name it was configured under — armed by @@ -751,6 +755,7 @@ impl Supervisor { dns_identity_rx: None, dns_task: None, dns_local_addr: None, + dns_aliases: None, #[cfg(unix)] udp_fd_tx: None, nostr_rendezvous: crate::nostr::RendezvousDriver::default(), diff --git a/src/node/mod.rs b/src/node/mod.rs index 8debb4e6..588c7c16 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -637,7 +637,7 @@ pub struct Node { // === Display Names === /// Human-readable names for configured peers (alias or short npub). - /// Populated at startup from peer config. + /// Populated at startup from peer config and updated by `update_peers`. peer_aliases: HashMap, /// Reloadable peer ACL state from standard allow/deny files. @@ -645,8 +645,9 @@ pub struct Node { // === Host Map === /// Static hostname → npub mapping for DNS resolution. - /// Built at construction from peer aliases and /etc/fips/hosts, and - /// published through a lock-free snapshot for the display path. + /// Built at construction from peer aliases and /etc/fips/hosts, with the + /// peer aliases replaced by `update_peers`, and published through a + /// lock-free snapshot for the display path. host_map: reloadable::HostMapReloadable, /// Sessions whose recv cipher + replay window have been handed @@ -1386,11 +1387,27 @@ impl Node { self.host_map.reload().await } + /// Rebuild every peer-alias map from the current peer list. + /// + /// Replaces the alias base under the display host map, the peer ACL's + /// alias resolution (rebuilding the ACL now) and the running DNS + /// responder's map. The hosts file stays merged over each and still wins. + pub(crate) async fn rebase_aliases(&mut self) { + let base = HostMap::from_peer_configs(self.config().peers()); + tracing::debug!(entries = base.len(), "Rebuilding peer alias maps"); + self.host_map.set_base(base.clone()); + self.peer_acl.rebase(base.clone()).await; + if let Some(tx) = &self.supervisor.dns_aliases { + tx.send_replace(base); + } + } + /// Return a human-readable display name for a NodeAddr. /// /// Lookup order: /// 1. Host map hostname (from peer aliases + /etc/fips/hosts) - /// 2. Configured peer alias or short npub (from startup map) + /// 2. Configured peer alias or short npub (from startup map, updated by + /// `update_peers`) /// 3. Active peer's short npub (e.g., inbound peer not in config) /// 4. Session endpoint's short npub (end-to-end, may not be direct peer) /// 5. Truncated NodeAddr hex (unknown address) diff --git a/src/node/reloadable.rs b/src/node/reloadable.rs index 3fc58b2c..c05c4e0e 100644 --- a/src/node/reloadable.rs +++ b/src/node/reloadable.rs @@ -8,17 +8,19 @@ //! //! # Canonical Arc-wrapper template //! -//! These resources follow a single-writer / many-reader pattern: the node -//! tick task is the only writer, while the hot path reads the current value -//! frequently and must never block. +//! These resources follow a single-writer / many-reader pattern: every write +//! goes through `&mut Node`, from the node tick task or from +//! `Node::update_peers`, so there is one writer at a time, while the hot path +//! reads the current value frequently and must never block. //! //! - The reader-facing immutable snapshot lives in an //! [`arc_swap::ArcSwap`]. Readers call [`Reloadable::load`], which yields //! a lock-free [`arc_swap::Guard>`] that derefs straight to the //! snapshot — no mutex, no clone on the read path. //! - The owning struct also holds the change-detection state (file mtime, -//! immutable base data, source path). That state is touched only by -//! [`Reloadable::reload`], which runs on the single writer task. +//! base data, source path). That state is touched only by +//! [`Reloadable::reload`] and, for the host map's peer-alias base, by +//! `HostMapReloadable::set_base`, both reached only through `&mut Node`. //! - `reload` builds a brand-new `T` and then stores `Arc::new(new)` into the //! `ArcSwap`, so a reader either sees the entire old snapshot or the entire //! new one — never a partial update. @@ -92,16 +94,20 @@ pub trait Reloadable: Send { /// Reloadable hostname → npub map (base peer aliases merged with the operator /// hosts file). /// -/// Holds the immutable base map (from peer-config aliases) plus the -/// change-detection state for the hosts file. The effective map (base merged +/// Holds the base map (from peer-config aliases, replaced when the peer list +/// is replaced at runtime) plus the change-detection state for the hosts +/// file. The effective map (base merged /// with the hosts file) is published through an [`arc_swap::ArcSwap`] so the /// display path can read it without locking. pub struct HostMapReloadable { /// Reader-facing effective snapshot (base merged with hosts file). snapshot: arc_swap::ArcSwap, - /// Base map from peer-config aliases (never changes). Read only by - /// `reload` on the tick task. + /// Base map from peer-config aliases. Read by `reload` and replaced by + /// `set_base`, both reached only through `&mut Node`. base: HostMap, + /// The hosts file as last read, kept so a new base can be merged under + /// it without reading the file again. Written by `new` and `reload`. + file: HostMap, /// Path to the operator hosts file. Read only by `reload`. path: std::path::PathBuf, /// Last observed modification time of the hosts file (`None` if absent). @@ -118,15 +124,29 @@ impl HostMapReloadable { let last_mtime = file_mtime(&path); let hosts_file = HostMap::load_hosts_file(&path); let mut effective = base.clone(); - effective.merge(hosts_file); + effective.merge(hosts_file.clone()); Self { snapshot: arc_swap::ArcSwap::from(Arc::new(effective)), base, + file: hosts_file, path, last_mtime, } } + + /// Replace the peer-alias base and publish it merged with the hosts file + /// as last read, which still wins on conflicts. + /// + /// Reads no file and leaves the recorded mtime alone. Like `reload`, it is + /// reached only through `&mut Node`, which keeps one writer at a time; + /// readers see either the whole old snapshot or the whole new one. + pub(crate) fn set_base(&mut self, base: HostMap) { + let mut effective = base.clone(); + effective.merge(self.file.clone()); + self.base = base; + self.snapshot.store(Arc::new(effective)); + } } impl Reloadable for HostMapReloadable { @@ -143,7 +163,8 @@ impl Reloadable for HostMapReloadable { self.last_mtime = current_mtime; let hosts_file = HostMap::load_hosts_file(&self.path); let mut new_effective = self.base.clone(); - new_effective.merge(hosts_file); + new_effective.merge(hosts_file.clone()); + self.file = hosts_file; let count = new_effective.len(); self.snapshot.store(Arc::new(new_effective)); @@ -329,4 +350,51 @@ mod tests { assert_eq!(snapshot.lookup_npub(key), expected.lookup_npub(key)); } } + + /// Build a one-entry host map. + fn one_entry(name: &str, id: &Identity) -> HostMap { + let mut map = HostMap::new(); + map.insert(name, &id.npub()).unwrap(); + map + } + + /// Replacing the base swaps the peer aliases while the hosts file, as it + /// was last re-read at runtime, stays merged on top and still wins. + #[tokio::test] + async fn set_base_replaces_peer_aliases_and_keeps_the_last_reloaded_hosts_file_on_top() { + let [x, y, z, v, w] = std::array::from_fn(|_| Identity::generate()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + std::fs::write(&path, format!("f {}\na {}\n", y.npub(), z.npub())).unwrap(); + + let mut reloadable = HostMapReloadable::new(one_entry("a", &x), path.clone()); + let npub = |r: &HostMapReloadable, name: &str| r.load().lookup_npub(name).map(String::from); + assert_eq!(npub(&reloadable, "f"), Some(y.npub()), "startup file entry"); + + std::thread::sleep(std::time::Duration::from_millis(50)); + std::fs::write(&path, format!("g {}\na {}\n", v.npub(), z.npub())).unwrap(); + assert!(reloadable.reload().await, "reload sees the rewrite"); + reloadable.set_base(one_entry("b", &w)); + + assert_eq!(npub(&reloadable, "b"), Some(w.npub()), "new base alias"); + assert_eq!( + npub(&reloadable, "g"), + Some(v.npub()), + "file entry added at runtime survives set_base" + ); + assert_eq!(npub(&reloadable, "a"), Some(z.npub()), "file still wins"); + assert_eq!( + npub(&reloadable, "f"), + None, + "file entry removed at runtime stays removed" + ); + let x_addr = *crate::PeerIdentity::from_npub(&x.npub()) + .unwrap() + .node_addr(); + assert_eq!( + reloadable.load().lookup_hostname(&x_addr), + None, + "old base npub no longer reverse-resolves" + ); + } } diff --git a/src/node/tests/mod.rs b/src/node/tests/mod.rs index 9b793f78..b31c75aa 100644 --- a/src/node/tests/mod.rs +++ b/src/node/tests/mod.rs @@ -27,6 +27,7 @@ mod session; mod spanning_tree; mod tcp; mod unit; +mod update_peers; pub(super) fn make_node() -> Node { make_node_with(Config::new()) diff --git a/src/node/tests/update_peers.rs b/src/node/tests/update_peers.rs new file mode 100644 index 00000000..7a195a29 --- /dev/null +++ b/src/node/tests/update_peers.rs @@ -0,0 +1,397 @@ +//! Replacing the peer list at runtime must carry peer aliases into every map +//! that reads them: the display host map, the peer ACL's alias entries and +//! the running `.fips` DNS responder. + +use super::*; +use crate::config::{ConnectPolicy, PeerAddress, PeerConfig}; +use crate::node::acl::{PeerAclDecision, PeerAclReloader}; +use crate::node::reloadable::HostMapReloadable; +use crate::upper::hosts::HostMap; +use std::net::Ipv6Addr; + +/// A suffix taken from a fresh npub's data part, so alias names cannot +/// collide with a hosts file on the build host. +fn suffix() -> String { + Identity::generate().npub()[5..17].to_string() +} + +/// A peer entry with an explicit address and connect policy. +fn peer_at(id: &Identity, alias: Option<&str>, addr: &str, policy: ConnectPolicy) -> PeerConfig { + PeerConfig { + npub: id.npub(), + alias: alias.map(String::from), + addresses: vec![PeerAddress::new("udp", addr)], + connect_policy: policy, + auto_reconnect: false, + via_nostr: false, + } +} + +/// An on-demand peer entry on the placeholder address, which nothing dials. +fn peer(id: &Identity, alias: Option<&str>) -> PeerConfig { + peer_at(id, alias, "127.0.0.1:9", ConnectPolicy::OnDemand) +} + +/// The identity form of a test identity, as the ACL and display paths see it. +fn ident(id: &Identity) -> PeerIdentity { + PeerIdentity::from_npub(&id.npub()).unwrap() +} + +/// The node address of a test identity. +fn addr(id: &Identity) -> NodeAddr { + *ident(id).node_addr() +} + +/// Build a node from `peers`, with its host map and peer ACL rebuilt exactly +/// as construction builds them but on temp paths, and its display-name map +/// seeded as `start()` seeds it (alias or else short npub). +/// +/// `allow`, `deny` and `hosts` are file contents; `None` leaves the file +/// absent. The returned directory must outlive the node. +fn alias_node( + peers: Vec, + allow: Option<&str>, + deny: Option<&str>, + hosts: Option<&str>, +) -> (tempfile::TempDir, Node) { + let dir = tempfile::tempdir().unwrap(); + let allow_path = dir.path().join("peers.allow"); + let deny_path = dir.path().join("peers.deny"); + let hosts_path = dir.path().join("hosts"); + for (path, contents) in [ + (&allow_path, allow), + (&deny_path, deny), + (&hosts_path, hosts), + ] { + if let Some(contents) = contents { + std::fs::write(path, contents).unwrap(); + } + } + + let mut config = Config::new(); + config.peers = peers; + let mut node = make_node_with(config); + let base = || HostMap::from_peer_configs(node.config().peers()); + let host_map = HostMapReloadable::new(base(), hosts_path.clone()); + let peer_acl = PeerAclReloader::with_alias_sources(allow_path, deny_path, base(), hosts_path); + node.host_map = host_map; + node.peer_acl = peer_acl; + + let seeded: Vec<_> = node + .config() + .peers() + .iter() + .map(|pc| { + let id = PeerIdentity::from_npub(&pc.npub).unwrap(); + let name = pc.alias.clone().unwrap_or_else(|| id.short_npub()); + (*id.node_addr(), name) + }) + .collect(); + node.peer_aliases.extend(seeded); + (dir, node) +} + +/// The npub a name resolves to in the node's display host map. +fn resolved(node: &Node, name: &str) -> Option { + node.host_map.load().lookup_npub(name).map(String::from) +} + +/// The ACL decision the node's authorization path would make for `id`. +fn decision(node: &Node, id: &Identity) -> PeerAclDecision { + node.peer_acl.acl().check(&ident(id)) +} + +/// Aliases renamed, removed and moved by `update_peers` are reflected in the +/// display host map, in display names and in the stats snapshot, with the +/// hosts file still overlaid and winning as at startup. +#[tokio::test] +async fn update_peers_rebuilds_host_map_display_names_and_the_stats_snapshot_from_the_new_aliases() +{ + let s = suffix(); + let [a, b, c, d, e, f, g] = std::array::from_fn(|_| Identity::generate()); + let (alpha, alpha2, bravo) = ( + format!("alpha-{s}"), + format!("alpha2-{s}"), + format!("bravo-{s}"), + ); + let (charlie, delta, echo) = ( + format!("charlie-{s}"), + format!("delta-{s}"), + format!("echo-{s}"), + ); + let hosts = format!("{echo} {}\n{charlie} {}\n", f.npub(), g.npub()); + let (_dir, mut node) = alias_node( + vec![ + peer(&a, Some(&alpha)), + peer(&b, Some(&bravo)), + peer(&d, Some(&delta)), + ], + None, + None, + Some(&hosts), + ); + + assert_eq!(resolved(&node, &alpha), Some(a.npub()), "before: alpha"); + assert_eq!(resolved(&node, &bravo), Some(b.npub()), "before: bravo"); + assert_eq!( + resolved(&node, &echo), + Some(f.npub()), + "before: echo from file" + ); + assert_eq!(node.peer_display_name(&addr(&a)), alpha, "before: A's name"); + assert_eq!( + node.peer_aliases.get(&addr(&d)), + Some(&delta), + "before: D's seeded display entry" + ); + assert_eq!(node.peer_display_name(&addr(&d)), delta, "before: D's name"); + + node.update_peers(vec![ + peer(&a, Some(&alpha2)), + peer(&c, Some(&charlie)), + peer(&d, None), + peer(&e, Some(&delta)), + ]) + .await + .unwrap(); + + assert_eq!( + resolved(&node, &alpha2), + Some(a.npub()), + "renamed alias resolves" + ); + assert_eq!(resolved(&node, &alpha), None, "old name of a renamed alias"); + assert_eq!(resolved(&node, &bravo), None, "alias of a removed peer"); + assert_eq!(resolved(&node, &delta), Some(e.npub()), "moved alias"); + assert_eq!(resolved(&node, &echo), Some(f.npub()), "file entry kept"); + assert_eq!( + resolved(&node, &charlie), + Some(g.npub()), + "file entry still wins over a peer alias" + ); + + let d_short = ident(&d).short_npub(); + assert_eq!(node.peer_display_name(&addr(&a)), alpha2, "renamed display"); + assert_eq!(node.peer_display_name(&addr(&e)), delta, "moved display"); + assert_eq!( + node.peer_display_name(&addr(&d)), + d_short, + "a peer whose alias was removed shows its short npub" + ); + + node.record_stats_history(); + let snapshot = node.stats_snapshot.load(); + assert_eq!( + snapshot.peer_aliases.get(&addr(&d)), + Some(&d_short), + "snapshot: removed alias" + ); + assert_eq!( + snapshot.peer_aliases.get(&addr(&a)), + Some(&alpha2), + "snapshot: renamed alias" + ); +} + +/// A deny entry written as an alias follows the alias to its new npub: the +/// new key is refused and the old one is no longer denied. +#[tokio::test] +async fn update_peers_moves_a_deny_entry_written_as_an_alias_onto_the_new_npub() { + let blocked = format!("blocked-{}", suffix()); + let [x, y] = std::array::from_fn(|_| Identity::generate()); + let (_dir, mut node) = alias_node( + vec![peer(&x, Some(&blocked))], + None, + Some(&format!("{blocked}\n")), + None, + ); + assert_eq!(decision(&node, &x), PeerAclDecision::DenyList, "before: X"); + assert_eq!( + decision(&node, &y), + PeerAclDecision::DefaultAllow, + "before: Y" + ); + + node.update_peers(vec![peer(&x, None), peer(&y, Some(&blocked))]) + .await + .unwrap(); + + assert_eq!(decision(&node, &y), PeerAclDecision::DenyList, "after: Y"); + assert_eq!( + decision(&node, &x), + PeerAclDecision::DefaultAllow, + "after: X" + ); +} + +/// Under deny `ALL`, an allow entry written as an alias follows the alias: +/// the new key is admitted and the old key falls to the deny. +#[tokio::test] +async fn update_peers_moves_an_allow_entry_written_as_an_alias_under_deny_all() { + let trusted = format!("trusted-{}", suffix()); + let [a, b] = std::array::from_fn(|_| Identity::generate()); + let (_dir, mut node) = alias_node( + vec![peer(&a, Some(&trusted))], + Some(&format!("{trusted}\n")), + Some("ALL\n"), + None, + ); + assert_eq!(decision(&node, &a), PeerAclDecision::AllowList, "before: A"); + assert_eq!(decision(&node, &b), PeerAclDecision::DenyList, "before: B"); + + node.update_peers(vec![peer(&a, None), peer(&b, Some(&trusted))]) + .await + .unwrap(); + + assert_eq!(decision(&node, &b), PeerAclDecision::AllowList, "after: B"); + assert_eq!(decision(&node, &a), PeerAclDecision::DenyList, "after: A"); +} + +/// The rebuilt ACL is in force before `update_peers` dials an added peer: a +/// deny entry moved onto the added peer Y stops its dial, while a control +/// peer Z added in the same call is dialed. +#[tokio::test] +async fn update_peers_applies_the_rebuilt_acl_before_dialing_an_added_peer() { + let blocked = format!("blocked-{}", suffix()); + let [x, y, z] = std::array::from_fn(|_| Identity::generate()); + let x_at = |alias| peer_at(&x, alias, "127.0.0.1:29", ConnectPolicy::OnDemand); + let (_dir, mut node) = alias_node( + vec![x_at(Some(&blocked))], + None, + Some(&format!("{blocked}\n")), + None, + ); + let (packet_tx, packet_rx) = packet_channel(64); + node.supervisor.packet_tx = Some(packet_tx.clone()); + node.packet_rx = Some(packet_rx); + let transport_id = TransportId::new(1); + let mut udp = UdpTransport::new( + transport_id, + Some("main".to_string()), + crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }, + packet_tx, + ); + udp.start_async().await.unwrap(); + node.transports + .insert(transport_id, TransportHandle::Udp(udp)); + + node.update_peers(vec![ + x_at(None), + peer_at( + &y, + Some(&blocked), + "127.0.0.1:9", + ConnectPolicy::AutoConnect, + ), + peer_at(&z, None, "127.0.0.1:19", ConnectPolicy::AutoConnect), + ]) + .await + .unwrap(); + + let dialed: Vec<_> = node + .connections() + .filter_map(|(_, machine)| machine.conn_expected_identity().copied()) + .map(|id| *id.node_addr()) + .collect(); + assert_eq!( + dialed.len(), + 1, + "only the control peer is dialed; Y's dial must meet the moved deny entry" + ); + assert_eq!(dialed[0], addr(&z), "the one dial is the control peer Z"); + + for transport in node.transports.values_mut() { + transport.stop().await.ok(); + } +} + +/// Send an AAAA query for `name` to the responder at `dns` and return the +/// address answered, or `None` for a reply without an answer. Fails if no +/// reply arrives within two seconds. +async fn query_aaaa(dns: std::net::SocketAddr, name: &str) -> Option { + use simple_dns::{CLASS, Name, Packet, QCLASS, QTYPE, Question, TYPE}; + let mut packet = Packet::new_query(0x4242); + packet.questions.push(Question::new( + Name::new_unchecked(name).into_owned(), + QTYPE::TYPE(TYPE::AAAA), + QCLASS::CLASS(CLASS::IN), + false, + )); + let query = packet.build_bytes_vec().unwrap(); + let client = tokio::net::UdpSocket::bind("[::1]:0").await.unwrap(); + client.send_to(&query, dns).await.unwrap(); + + let mut buf = [0u8; 512]; + let (len, _) = tokio::time::timeout(Duration::from_secs(2), client.recv_from(&mut buf)) + .await + .unwrap_or_else(|_| panic!("no DNS reply for {name} within the timeout")) + .unwrap(); + let reply = Packet::parse(&buf[..len]).expect("well-formed DNS response"); + reply.answers.first().map(|answer| match &answer.rdata { + simple_dns::rdata::RData::AAAA(aaaa) => Ipv6Addr::from(aaaa.address), + other => panic!("expected an AAAA record for {name}, got {other:?}"), + }) +} + +/// A running DNS responder answers `.fips` alias names from the new peer +/// list after `update_peers`, through the node's real DNS start path. +#[tokio::test] +async fn update_peers_republishes_aliases_to_the_running_dns_responder() { + let s = suffix(); + let (alpha, charlie, delta) = ( + format!("alpha-{s}"), + format!("charlie-{s}"), + format!("delta-{s}"), + ); + let [a, b, c, d] = std::array::from_fn(|_| Identity::generate()); + let mut config = Config::new(); + config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }); + config.dns.enabled = true; + config.dns.bind_addr = Some("::1".to_string()); + config.dns.port = Some(0); + config.peers = vec![peer(&a, Some(&alpha)), peer(&d, Some(&delta))]; + let mut node = make_node_with(config); + node.start().await.unwrap(); + let dns = node.dns_local_addr().expect("responder is up"); + let fips = |name: &str| format!("{name}.fips"); + let v6 = |id: &Identity| id.address().to_ipv6(); + + assert_eq!( + query_aaaa(dns, &fips(&alpha)).await, + Some(v6(&a)), + "before: alpha" + ); + assert_eq!( + query_aaaa(dns, &fips(&charlie)).await, + None, + "before: charlie" + ); + + node.update_peers(vec![peer(&b, Some(&alpha)), peer(&c, Some(&charlie))]) + .await + .unwrap(); + + assert_eq!( + query_aaaa(dns, &fips(&alpha)).await, + Some(v6(&b)), + "after: alpha moved to B" + ); + assert_eq!( + query_aaaa(dns, &fips(&charlie)).await, + Some(v6(&c)), + "after: charlie added" + ); + assert_eq!( + query_aaaa(dns, &fips(&delta)).await, + None, + "after: delta removed" + ); + + node.stop().await.unwrap(); +} diff --git a/src/upper/dns.rs b/src/upper/dns.rs index 2fca5e70..69500c21 100644 --- a/src/upper/dns.rs +++ b/src/upper/dns.rs @@ -167,10 +167,27 @@ fn is_mesh_interface_query(arrival_ifindex: Option, mesh_ifindex: Option, +) { + run_responder(socket, identity_tx, ttl, reloader, None, mesh_ifindex).await +} + +/// Run the DNS responder UDP server loop, taking peer-alias base updates. +/// +/// Behaves as [`run_dns_responder`], and in addition, when `aliases` is +/// `Some`, applies the newest peer-alias base sent on it before answering +/// each query, so aliases follow the node's peer list when it is replaced +/// at runtime. The hosts file stays merged over the new base and still wins. +pub(crate) async fn run_responder( socket: tokio::net::UdpSocket, identity_tx: DnsIdentityTx, ttl: u32, mut reloader: HostMapReloader, + mut aliases: Option>, mesh_ifindex: Option, ) { let mut buf = [0u8; 512]; // Standard DNS UDP max @@ -195,8 +212,9 @@ pub async fn run_dns_responder( let query_bytes = &buf[..len]; - // Check for hosts file changes on each request (cheap stat call) - reloader.check_reload(); + // Apply any new peer-alias base, then check for hosts file changes + // (cheap stat call). + refresh_hosts(&mut reloader, aliases.as_mut()); match handle_dns_packet(query_bytes, ttl, reloader.hosts()) { Some((response_bytes, identity)) => { @@ -219,6 +237,29 @@ pub async fn run_dns_responder( } } +/// Bring the responder's host map up to date before answering a query. +/// +/// Applies the newest peer-alias base from `aliases` if it has not been seen +/// yet, then re-reads the hosts file if its mtime changed. The change test is +/// made on the borrowed value rather than the receiver, so a value sent just +/// before the sender closed is still applied. +fn refresh_hosts( + reloader: &mut HostMapReloader, + aliases: Option<&mut tokio::sync::watch::Receiver>, +) { + if let Some(rx) = aliases { + // Take the value out so the watch lock is released before the merge. + let next = { + let seen = rx.borrow_and_update(); + seen.has_changed().then(|| seen.clone()) + }; + if let Some(base) = next { + reloader.set_base(base); + } + } + reloader.check_reload(); +} + /// Receive a UDP datagram with arrival-interface info via `IPV6_PKTINFO`. /// /// Returns `(len, src, arrival_ifindex)`. The ifindex is `Some` when the @@ -940,4 +981,42 @@ mod tests { packet.questions.push(question); packet.build_bytes_vec().unwrap() } + + /// Build a one-entry host map. + fn one_entry(name: &str, id: &Identity) -> HostMap { + let mut map = HostMap::new(); + map.insert(name, &id.npub()).unwrap(); + map + } + + /// A base sent on the alias channel is applied before the next answer, + /// and is kept once the sender is gone. + #[test] + fn refresh_hosts_applies_the_latest_base_before_answering() { + let (x, y) = (Identity::generate(), Identity::generate()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("absent-hosts"); + let npub = |r: &HostMapReloader| r.hosts().lookup_npub("a").map(String::from); + + let mut reloader = HostMapReloader::new(one_entry("a", &x), path.clone()); + let (tx, mut rx) = tokio::sync::watch::channel(one_entry("a", &x)); + tx.send_replace(one_entry("a", &y)); + refresh_hosts(&mut reloader, Some(&mut rx)); + assert_eq!(npub(&reloader), Some(y.npub()), "new base applied"); + drop(tx); + refresh_hosts(&mut reloader, Some(&mut rx)); + assert_eq!(npub(&reloader), Some(y.npub()), "base kept after close"); + + // A value sent just before the sender closed is still applied. + let mut reloader = HostMapReloader::new(one_entry("a", &x), path); + let (tx, mut rx) = tokio::sync::watch::channel(one_entry("a", &x)); + tx.send_replace(one_entry("a", &y)); + drop(tx); + refresh_hosts(&mut reloader, Some(&mut rx)); + assert_eq!( + npub(&reloader), + Some(y.npub()), + "pending base applied although the sender is closed" + ); + } } diff --git a/src/upper/hosts.rs b/src/upper/hosts.rs index f9e9f3ff..c41034ba 100644 --- a/src/upper/hosts.rs +++ b/src/upper/hosts.rs @@ -222,12 +222,17 @@ pub fn file_mtime(path: &Path) -> Option { /// Tracks a hosts file and reloads it when the modification time changes. /// -/// Holds the base host map (from peer config aliases) and the current -/// effective map (base + hosts file). On each `check_reload()`, stats the -/// hosts file and rebuilds the effective map if the mtime has changed. +/// Holds the base host map (from peer config aliases), the hosts file as last +/// read, and the current effective map (base + hosts file). On each +/// `check_reload()`, stats the hosts file and rebuilds the effective map if +/// the mtime has changed. The base is replaced when the node's peer list is +/// replaced at runtime. pub struct HostMapReloader { - /// Base map from peer config aliases (never changes). + /// Base map from peer config aliases, replaced by `set_base`. base: HostMap, + /// The hosts file as last applied, kept so a new base can be merged under + /// it without reading the file again. + file: HostMap, /// Current effective map (base merged with hosts file). effective: HostMap, /// Path to the hosts file. @@ -252,10 +257,11 @@ impl HostMapReloader { } }; let mut effective = base.clone(); - effective.merge(hosts_file); + effective.merge(hosts_file.clone()); Self { base, + file: hosts_file, effective, path, last_mtime, @@ -307,11 +313,24 @@ impl HostMapReloader { Ok(true) } + /// Replace the peer-alias base and rebuild the effective map over the + /// hosts file as last read, which still wins on conflicts. + /// + /// Reads no file and leaves the recorded mtime alone, so a hosts-file + /// change is still picked up by the next reload check. + pub(crate) fn set_base(&mut self, base: HostMap) { + let mut effective = base.clone(); + effective.merge(self.file.clone()); + self.base = base; + self.effective = effective; + } + /// Replace the effective map with the base merged with a freshly read - /// hosts file. + /// hosts file, and keep that file as the one last applied. fn apply(&mut self, hosts_file: HostMap) { let mut new_effective = self.base.clone(); - new_effective.merge(hosts_file); + new_effective.merge(hosts_file.clone()); + self.file = hosts_file; let count = new_effective.len(); self.effective = new_effective; @@ -763,4 +782,69 @@ mod tests { assert!(reloader.hosts().lookup_npub("core").is_some()); assert!(reloader.hosts().lookup_npub("gateway").is_none()); } + + /// Build a one-entry host map. + fn one_entry(name: &str, id: &Identity) -> HostMap { + let mut map = HostMap::new(); + map.insert(name, &id.npub()).unwrap(); + map + } + + /// Replacing the base swaps the peer aliases while the hosts file, as it + /// was last re-read at runtime through either reload path, stays merged on + /// top and still wins on conflicts. + #[test] + fn set_base_replaces_peer_aliases_and_keeps_the_last_reloaded_hosts_file_on_top() { + let [x, y, z, v, w, u, t] = std::array::from_fn(|_| Identity::generate()); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hosts"); + std::fs::write(&path, format!("f {}\na {}\n", y.npub(), z.npub())).unwrap(); + + let mut reloader = HostMapReloader::new(one_entry("a", &x), path.clone()); + let npub = |r: &HostMapReloader, name: &str| r.hosts().lookup_npub(name).map(String::from); + assert_eq!(npub(&reloader, "f"), Some(y.npub()), "startup file entry"); + + // Step 1: the file changes at runtime and is re-read by check_reload. + std::thread::sleep(std::time::Duration::from_millis(50)); + std::fs::write(&path, format!("g {}\na {}\n", v.npub(), z.npub())).unwrap(); + assert!(reloader.check_reload(), "check_reload sees the rewrite"); + reloader.set_base(one_entry("b", &w)); + assert_eq!(npub(&reloader, "b"), Some(w.npub()), "new base alias"); + assert_eq!( + npub(&reloader, "g"), + Some(v.npub()), + "file entry added at runtime survives set_base" + ); + assert_eq!(npub(&reloader, "a"), Some(z.npub()), "file still wins"); + assert_eq!( + npub(&reloader, "f"), + None, + "file entry removed at runtime stays removed" + ); + let x_addr = *PeerIdentity::from_npub(&x.npub()).unwrap().node_addr(); + assert_eq!( + reloader.hosts().lookup_hostname(&x_addr), + None, + "old base npub no longer reverse-resolves" + ); + + // Step 2: the file changes again and is re-read by try_check_reload. + std::thread::sleep(std::time::Duration::from_millis(50)); + std::fs::write(&path, format!("h {}\na {}\n", u.npub(), z.npub())).unwrap(); + assert!( + reloader.try_check_reload().unwrap(), + "try_check_reload sees the rewrite" + ); + reloader.set_base(one_entry("c", &t)); + assert_eq!(npub(&reloader, "c"), Some(t.npub()), "second base alias"); + assert_eq!( + npub(&reloader, "h"), + Some(u.npub()), + "file entry re-read by try_check_reload survives set_base" + ); + assert_eq!(npub(&reloader, "a"), Some(z.npub()), "file still wins"); + for gone in ["g", "f", "b"] { + assert_eq!(npub(&reloader, gone), None, "{gone} no longer resolves"); + } + } } From 65b92ed77728c6c2d8a2ec3118ff12749a82ed9b Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 18:59:06 +0000 Subject: [PATCH 05/14] Derive the gateway's LAN interface in its container at every start The gateway test wrote lan_interface: eth1 into the gateway config before any container existed, on the assumption that Docker attaches the LAN network as eth1. Docker does not promise that order, at the first start or at later ones, and a wrong name that exists passes the gateway's startup check. The LAN masquerade for inbound port forwards and the proxy NDP entries for virtual IPs then went on the wrong interface, and nothing in the suite noticed. The gateway container's entrypoint now finds the interface holding the gateway's LAN address and writes the gateway's config from it before fips-gateway starts, so every docker start and restart re-derives it. The config the suite writes carries only a placeholder. The suite checks the running gateway's interface against its own derivation after the first start and after each later start, checks that the LAN masquerade and the proxy NDP entry are on it, and checks that a restarted gateway's config carries the ttl and grace settings the later phases rely on. inject-config passes its values to Python as arguments rather than splicing them into the program, and a failed config write now fails the writer instead of reporting success. The output parsers are checked against canned tool output by a new selftest subcommand, which the suite also runs first as Phase 0. --- testing/docker/entrypoint.sh | 102 +++++++- testing/static/docker-compose.yml | 8 +- testing/static/scripts/gateway-test.sh | 322 +++++++++++++++++++++++-- 3 files changed, 405 insertions(+), 27 deletions(-) diff --git a/testing/docker/entrypoint.sh b/testing/docker/entrypoint.sh index 644b308b..3c6e960b 100644 --- a/testing/docker/entrypoint.sh +++ b/testing/docker/entrypoint.sh @@ -173,6 +173,80 @@ start_tor_directory() { fi } +# ── Gateway: LAN interface and config copy ────────────────────────────── + +# Print the one interface holding the IPv6 address $1. Docker may attach the +# LAN network after the container starts, so poll for up to 15 s. The rule is +# the one lan_iface uses in testing/static/scripts/gateway-test.sh: addresses +# compare as addresses, and an @ifN suffix is dropped from the name. +lanif_find() { + local name + for _ in $(seq 1 30); do + if name=$(ip -6 -o addr show | python3 -c ' +import ipaddress, sys +want = ipaddress.ip_address(sys.argv[1]) +holders = set() +for line in sys.stdin: + f = line.split() + if len(f) < 4 or f[2] != "inet6": + continue + try: + addr = ipaddress.ip_interface(f[3]).ip + except ValueError: + continue + if addr == want: + holders.add(f[1].split("@")[0]) +if len(holders) != 1: + sys.exit(1) +print(holders.pop()) +' "$1"); then + echo "$name" + return 0 + fi + sleep 0.5 + done + echo "FATAL: no single interface holds $1" >&2 + ip -6 -o addr show >&2 + return 1 +} + +# Write config $2 to $3 with its lan_interface set to $1. The source is a +# read-only bind mount, so fips-gateway reads this copy instead. The image has +# no YAML parser, so the edit is line-based, and it is refused unless the +# source has exactly one lan_interface line and the result names $1 on +# exactly one. A failure leaves $3 as it was. +gwconf_write() { + local iface="$1" src="$2" dest="$3" + local key='^[[:space:]]*lan_interface:' + local n + if ! [[ "$iface" =~ ^[A-Za-z0-9_.-]{1,15}$ ]]; then + rm -f "$dest.tmp" + echo "FATAL: invalid interface name $iface" >&2 + return 1 + fi + n=$(grep -cE "$key" "$src") || n=0 + if [ "$n" -ne 1 ]; then + rm -f "$dest.tmp" + echo "FATAL: $src has $n lan_interface lines" >&2 + return 1 + fi + # The file carries the node's nsec. The umask is set in a subshell so it + # does not reach the daemons this script starts next. + if ! ( umask 077 && sed -E "s/^([[:space:]]*lan_interface:).*/\1 $iface/" "$src" > "$dest.tmp" ); then + rm -f "$dest.tmp" + echo "FATAL: could not write $dest.tmp" >&2 + return 1 + fi + n=$(grep -cE "$key" "$dest.tmp") || n=0 + if [ "$n" -ne 1 ] || ! grep -qE "^[[:space:]]*lan_interface: ${iface//./\\.}\$" "$dest.tmp"; then + rm -f "$dest.tmp" + echo "FATAL: $dest.tmp does not hold exactly one lan_interface: $iface line" >&2 + return 1 + fi + mv -f "$dest.tmp" "$dest" || { rm -f "$dest.tmp"; return 1; } + return 0 +} + # ── Mode dispatch ──────────────────────────────────────────────────────── case "$MODE" in @@ -209,17 +283,23 @@ case "$MODE" in # No dnsmasq — gateway DNS replaces it on port 53 start_services - # Extract LAN interface from config (gateway.lan_interface) - LAN_IF=$(grep 'lan_interface:' "$CONFIG" | head -1 | sed 's/.*: *//' | tr -d '"' | tr -d "'") - LAN_IF="${LAN_IF:-eth0}" + # The LAN interface is the one holding the gateway's LAN address, + # derived at every start: Docker does not promise which ethN the LAN + # network gets, and a restart can change it. The config's own + # lan_interface is a placeholder; fips-gateway reads a copy that + # names the derived interface. + if [ -z "${FIPS_GW_LAN_ADDR:-}" ]; then + echo "FATAL: FIPS_GW_LAN_ADDR is not set" + exit 1 + fi + LAN_IF=$(lanif_find "$FIPS_GW_LAN_ADDR") || exit 1 + echo "LAN interface: $LAN_IF holds $FIPS_GW_LAN_ADDR" + gwconf_write "$LAN_IF" "$CONFIG" /etc/fips/gateway.yaml || exit 1 - # Wait for LAN interface (Docker attaches second network after start) - for i in $(seq 1 15); do - [ -e "/sys/class/net/$LAN_IF" ] && break - sleep 0.5 - done - - # Ensure IPv6 is enabled on the LAN interface (may inherit host default) + # Ensure IPv6 is enabled on the LAN interface (may inherit host + # default). The address was found on it before this runs only + # because compose sets net.ipv6.conf.default.disable_ipv6=0 for this + # service, so keep that sysctl if this one moves. sysctl -w "net.ipv6.conf.${LAN_IF}.disable_ipv6=0" >/dev/null 2>&1 || true sysctl -w net.ipv6.conf.all.forwarding=1 >/dev/null 2>&1 || true sysctl -w net.ipv6.conf.all.proxy_ndp=1 >/dev/null 2>&1 || true @@ -255,7 +335,7 @@ case "$MODE" in done echo "fips0 ready, starting gateway" - exec fips-gateway --config "$CONFIG" --log-level debug + exec fips-gateway --config /etc/fips/gateway.yaml --log-level debug ;; *) echo "Unknown FIPS_TEST_MODE: $MODE" diff --git a/testing/static/docker-compose.yml b/testing/static/docker-compose.yml index eb025aee..94fdf1d9 100644 --- a/testing/static/docker-compose.yml +++ b/testing/static/docker-compose.yml @@ -371,14 +371,18 @@ services: profiles: ["gateway"] container_name: fips-gw-gateway${FIPS_CI_NAME_SUFFIX:-} hostname: gw-gateway - # Privileged required: gateway must enable IPv6 on eth1 (second network, - # attached after container start) and manage nftables NAT rules. + # Privileged required: gateway must enable IPv6 on the LAN interface + # (Docker may attach it after container start) and manage nftables NAT + # rules. privileged: true environment: # Debug for the NAT rebuild and pool tick timing lines only; the # gateway's --log-level is ignored while RUST_LOG is set. - RUST_LOG=info,fips::gateway::nat=debug,fips_gateway=debug - FIPS_TEST_MODE=gateway + # The entrypoint names the interface holding this address as the + # gateway's LAN interface; it must match ipv6_address below. + - FIPS_GW_LAN_ADDR=${FIPS_GW_LAN6_PREFIX:-fd02}::10 sysctls: - net.ipv6.conf.all.disable_ipv6=0 - net.ipv6.conf.default.disable_ipv6=0 diff --git a/testing/static/scripts/gateway-test.sh b/testing/static/scripts/gateway-test.sh index dd441b91..18216fe0 100755 --- a/testing/static/scripts/gateway-test.sh +++ b/testing/static/scripts/gateway-test.sh @@ -5,10 +5,11 @@ # gw-client (non-FIPS) → gw-gateway (fips + fips-gateway) → gw-server (fips + http) # # Usage: -# ./scripts/gateway-test.sh [inject-config] +# ./scripts/gateway-test.sh [inject-config | selftest] # # Subcommands: -# inject-config — post-process generated configs to add gateway section +# inject-config — post-process generated configs to add the gateway section +# selftest — check the output readers against canned input # (no args) — run the test (containers must be running) set -e @@ -43,21 +44,26 @@ inject_gateway_config() { if [ ! -f "$config_file" ]; then echo "Error: $config_file not found. Run generate-configs.sh gateway first." >&2 - exit 1 + return 1 fi echo "Injecting gateway config into $config_file" - python3 -c " -import yaml + # Opening with 'w' truncates in place and keeps the inode, which the + # container's single-file bind mount of this file needs. + python3 - "$config_file" "$GW_CLIENT_LAN" <<'PYEOF' || return 1 +import sys, yaml +path, client = sys.argv[1:3] -with open('$config_file') as f: +with open(path) as f: cfg = yaml.safe_load(f) cfg['gateway'] = { 'enabled': True, 'pool': 'fd01::/112', - # Docker assigns gateway-lan to eth1 (fips-net is eth0). The - # LAN-side masquerade for inbound port forwards gates on this. + # A placeholder. Docker does not promise which interface the LAN + # network gets, so the gateway container's entrypoint replaces this with + # the interface holding the gateway's LAN address before fips-gateway + # starts. The LAN-side masquerade and the proxy NDP entries use it. 'lan_interface': 'eth1', 'dns': { 'listen': '[::]:53', @@ -68,33 +74,221 @@ cfg['gateway'] = { { 'listen_port': 18080, 'proto': 'tcp', - 'target': '[${GW_CLIENT_LAN}]:8080', + 'target': f'[{client}]:8080', }, # 6B: second TCP forward — exercises multiple simultaneous TCP # rules sharing the same LAN backend on a different listen port. { 'listen_port': 18082, 'proto': 'tcp', - 'target': '[${GW_CLIENT_LAN}]:8081', + 'target': f'[{client}]:8081', }, # 6A: UDP forward — exercises the runtime UDP DNAT path (rule # shape + conntrack handling) end-to-end. { 'listen_port': 18081, 'proto': 'udp', - 'target': '[${GW_CLIENT_LAN}]:8081', + 'target': f'[{client}]:8081', }, ], } -with open('$config_file', 'w') as f: +with open(path, 'w') as f: yaml.dump(cfg, f, default_flow_style=False, sort_keys=False) -" +PYEOF echo " ✓ Gateway config injected" + return 0 } +# ── Readers ────────────────────────────────────────────────────────────── +# +# Each reader parses one tool's output on stdin and prints a single answer. +# A reader that cannot answer exits non-zero and prints nothing, rather than +# printing a default a check could mistake for an answer. Addresses are +# compared as addresses, never as strings: ip prints fd02:0:0:0::10 as +# fd02::10. Arguments reach Python through sys.argv. + +# The interface holding address $1, from `ip -6 -o addr show`. Fails when no +# interface holds it or more than one does. +lan_iface() { + python3 -c ' +import ipaddress, sys +want = ipaddress.ip_address(sys.argv[1]) +holders = set() +for line in sys.stdin: + f = line.split() + if len(f) < 4 or f[2] != "inet6": + continue + try: + addr = ipaddress.ip_interface(f[3]).ip + except ValueError: + continue + if addr == want: + holders.add(f[1].split("@")[0]) +if len(holders) != 1: + sys.exit(1) +print(holders.pop()) +' "$@" +} + +# The gateway's lan_interface, from a show_gateway response. +gw_iface() { + python3 -c ' +import json, sys +try: + r = json.load(sys.stdin) +except ValueError: + sys.exit(1) +if not isinstance(r, dict) or r.get("status") != "ok": + sys.exit(1) +data = r.get("data") +if not isinstance(data, dict): + sys.exit(1) +name = data.get("lan_interface") +if not isinstance(name, str) or not name: + sys.exit(1) +print(name) +' +} + +# The output interface of the LAN masquerade (the one rule matching +# iifname "fips0" and masquerading), from `nft list table inet fips_gateway`. +masq_iface() { + python3 -c ' +import re, sys +found = [] +for line in sys.stdin: + if "iifname \"fips0\"" not in line or not re.search(r"\bmasquerade\b", line): + continue + m = re.search(r"\boifname \"([^\"]+)\"", line) + found.append(m.group(1) if m else "") +if len(found) != 1 or not found[0]: + sys.exit(1) +print(found[0]) +' +} + +# The device of the proxy neighbour entry for address $1, from +# `ip -6 neigh show proxy`, whose lines read `ADDR dev DEV proxy`. Fails when +# no entry matches or matching entries name different devices. +proxy_dev() { + python3 -c ' +import ipaddress, sys +want = ipaddress.ip_address(sys.argv[1]) +devs = set() +for line in sys.stdin: + f = line.split() + if len(f) < 3 or "dev" not in f[1:-1]: + continue + try: + addr = ipaddress.ip_address(f[0]) + except ValueError: + continue + if addr == want: + devs.add(f[f.index("dev", 1) + 1]) +if len(devs) != 1: + sys.exit(1) +print(devs.pop()) +' "$@" +} + +# ── Reader self-test ───────────────────────────────────────────────────── + +# Run one reader on a canned input and compare its status and output with +# the expected ones. A case expecting failure also requires empty output. +# Usage: gw_case LABEL WANT_RC WANT_OUT INPUT READER [ARGS...] +gw_case() { + local label="$1" want_rc="$2" want_out="$3" input="$4" + shift 4 + local out rc + if out=$("$@" <<< "$input" 2>/dev/null); then rc=0; else rc=$?; fi + if [ "$rc" -eq "$want_rc" ] && [ "$out" = "$want_out" ]; then + echo " selftest $label ... OK" + return 0 + fi + echo " selftest $label ... FAIL (rc $rc, output '$out'; expected rc $want_rc, output '$want_out')" + return 1 +} + +# Feed every reader canned tool output and check its answers. The inputs +# follow each tool's printed format; the ip -o addr lines follow a capture, +# the others are written from the tools' documented formats. +gw_selftest() { + local fails=0 + local addr_eth0 addr_eth1 addr_claimed addr_at nft_lan nft_nolan + + addr_eth0='1: lo inet6 ::1/128 scope host \ valid_lft forever preferred_lft forever +2: fips0 inet6 fd3c:9a51:7e02:4b18::1/8 scope global \ valid_lft forever preferred_lft forever +2: fips0 inet6 fe80::5c2a:91ff:fe3b:1d7e/64 scope link \ valid_lft forever preferred_lft forever +40: eth0 inet6 fd02::10/64 scope global nodad \ valid_lft forever preferred_lft forever +40: eth0 inet6 fe80::42:acff:fe13:3/64 scope link \ valid_lft forever preferred_lft forever +42: eth1 inet6 fe80::42:acff:fe12:2/64 scope link \ valid_lft forever preferred_lft forever' + addr_eth1='1: lo inet6 ::1/128 scope host \ valid_lft forever preferred_lft forever +2: fips0 inet6 fd3c:9a51:7e02:4b18::1/8 scope global \ valid_lft forever preferred_lft forever +2: fips0 inet6 fe80::5c2a:91ff:fe3b:1d7e/64 scope link \ valid_lft forever preferred_lft forever +40: eth0 inet6 fe80::42:acff:fe12:2/64 scope link \ valid_lft forever preferred_lft forever +42: eth1 inet6 fd02::10/64 scope global nodad \ valid_lft forever preferred_lft forever +42: eth1 inet6 fe80::42:acff:fe13:3/64 scope link \ valid_lft forever preferred_lft forever' + addr_claimed='1: lo inet6 ::1/128 scope host \ valid_lft forever preferred_lft forever +40: eth0 inet6 fe80::42:acff:fe12:2/64 scope link \ valid_lft forever preferred_lft forever +42: eth1 inet6 fd02:0:0:5::10/64 scope global nodad \ valid_lft forever preferred_lft forever' + addr_at='1: lo inet6 ::1/128 scope host \ valid_lft forever preferred_lft forever +6: eth0@if5 inet6 fe80::42:acff:fe12:2/64 scope link \ valid_lft forever preferred_lft forever +8: eth1@if7 inet6 fd02::10/64 scope global nodad \ valid_lft forever preferred_lft forever' + + gw_case "lan_iface: LAN on eth0" 0 eth0 "$addr_eth0" lan_iface fd02::10 || fails=$((fails + 1)) + gw_case "lan_iface: LAN on eth1" 0 eth1 "$addr_eth1" lan_iface fd02::10 || fails=$((fails + 1)) + gw_case "lan_iface: claimed prefix" 0 eth1 "$addr_claimed" lan_iface fd02:0:0:5::10 || fails=$((fails + 1)) + gw_case "lan_iface: first claimed /64 printed short" 0 eth0 "$addr_eth0" lan_iface fd02:0:0:0::10 || fails=$((fails + 1)) + gw_case "lan_iface: name with @ifN suffix" 0 eth1 "$addr_at" lan_iface fd02::10 || fails=$((fails + 1)) + gw_case "lan_iface: no holder" 1 "" "$addr_claimed" lan_iface fd02::10 || fails=$((fails + 1)) + gw_case "lan_iface: empty input" 1 "" "" lan_iface fd02::10 || fails=$((fails + 1)) + + gw_case "gw_iface: ok response" 0 eth0 \ + '{"status":"ok","data":{"pool_cidr":"fd01::/112","lan_interface":"eth0"}}' gw_iface || fails=$((fails + 1)) + gw_case "gw_iface: error response" 1 "" \ + '{"status":"error","message":"gateway not yet initialized"}' gw_iface || fails=$((fails + 1)) + gw_case "gw_iface: empty input" 1 "" "" gw_iface || fails=$((fails + 1)) + + nft_lan='table inet fips_gateway { + chain prerouting { + type nat hook prerouting priority dstnat; policy accept; + meta nfproto ipv6 ip6 daddr fd01::1 dnat ip6 to fd3c:9a51:7e02:4b18::2 + iifname "fips0" meta nfproto ipv6 meta l4proto tcp tcp dport 18080 dnat ip6 to [fd02::20]:8080 + } + + chain postrouting { + type nat hook postrouting priority srcnat; policy accept; + oifname "fips0" masquerade + meta nfproto ipv6 ip6 saddr fd3c:9a51:7e02:4b18::2 snat ip6 to fd01::1 + iifname "fips0" oifname "eth0" meta nfproto ipv6 masquerade + } +}' + nft_nolan=$(grep -v 'iifname "fips0" oifname' <<< "$nft_lan") + gw_case "masq_iface: LAN masquerade on eth0" 0 eth0 "$nft_lan" masq_iface || fails=$((fails + 1)) + gw_case "masq_iface: no LAN masquerade" 1 "" "$nft_nolan" masq_iface || fails=$((fails + 1)) + gw_case "masq_iface: empty input" 1 "" "" masq_iface || fails=$((fails + 1)) + + gw_case "proxy_dev: entry on eth0 after another on eth1" 0 eth0 \ + $'fd01::2 dev eth1 proxy\nfd01::1 dev eth0 proxy' proxy_dev fd01::1 || fails=$((fails + 1)) + gw_case "proxy_dev: no entry" 1 "" 'fd01::2 dev eth1 proxy' proxy_dev fd01::1 || fails=$((fails + 1)) + gw_case "proxy_dev: empty input" 1 "" "" proxy_dev fd01::1 || fails=$((fails + 1)) + gw_case "proxy_dev: entry on two devices" 1 "" \ + $'fd01::1 dev eth0 proxy\nfd01::1 dev eth1 proxy' proxy_dev fd01::1 || fails=$((fails + 1)) + + echo " selftest: $fails case(s) failed" + if [ "$fails" -eq 0 ]; then + return 0 + fi + return 1 +} + +if [ "${1:-}" = "selftest" ]; then + if gw_selftest; then exit 0; else exit 1; fi +fi + if [ "${1:-}" = "inject-config" ]; then - inject_gateway_config + inject_gateway_config || exit 1 exit 0 fi @@ -123,15 +317,71 @@ check() { fi } +# Record one check that the running gateway's lan_interface is the interface +# holding its LAN address, derived here from the container's addresses +# independently of the entrypoint. Sets LAN_IF to the derived name, or to +# empty when no single interface holds the address. Needs no set -e: callers +# may run it where set -e is suspended. +lan_agree() { + local label="$1" reported="" derived="" + for _ in $(seq 1 30); do + if reported=$(docker exec "$GATEWAY" bash -c \ + 'echo "{\"command\":\"show_gateway\"}" | nc -U -w1 /run/fips/gateway.sock 2>/dev/null' \ + | gw_iface); then + break + fi + reported="" + sleep 1 + done + if derived=$(docker exec "$GATEWAY" ip -6 -o addr show 2>/dev/null | lan_iface "$GW_DNS"); then + : + else + derived="" + fi + LAN_IF="$derived" + if [ -z "$derived" ]; then + check "$label: no single interface holds $GW_DNS" 1 + elif [ -z "$reported" ]; then + check "$label: gateway did not report its lan_interface (derived $derived)" 1 + elif [ "$derived" != "$reported" ]; then + check "$label: derived $derived, gateway $reported" 1 + else + check "$label: gateway uses $derived, which holds $GW_DNS" 0 + fi + return 0 +} + echo "=== FIPS Gateway Integration Test ===" echo "" +# Phase 0: the readers the later phases rely on, against canned input. +echo "Phase 0: Reader self-test" +if gw_selftest; then + check "Reader self-test" 0 +else + check "Reader self-test" 1 +fi +echo "" + # Phase 1: Wait for mesh convergence (gateway ↔ server, gateway ↔ server-2) echo "Phase 1: Mesh convergence" wait_for_peers "$GATEWAY" 2 30 || true wait_for_peers "$SERVER" 1 30 || true wait_for_peers "$SERVER2" 1 30 || true +# Phase 1b: LAN interface +# +# A wrong lan_interface that exists passes the gateway's startup check, and +# the LAN masquerade and the proxy NDP entries then go on the wrong interface. +# The gateway container's entrypoint derives the interface holding the LAN +# address at every container start and writes it into the gateway's config. +# This checks the result against an independent derivation from outside. An +# empty LAN_IF afterwards makes every later interface check fail. +echo "" +echo "Phase 1b: LAN interface" +LAN_IF="" +lan_agree "LAN interface" + # Phase 2: Wait for gateway DNS to respond echo "" echo "Phase 2: Gateway DNS readiness" @@ -340,6 +590,20 @@ else check "Gateway counts a session (skipped — no virtual IP)" 1 fi +# The mapping's proxy neighbour entry must be on the LAN interface, or LAN +# hosts without a static route cannot reach the virtual IP. Phase 3's static +# routes bypass neighbour resolution, so nothing else would notice. +if [ -n "$VIRTUAL_IP" ] && [ -n "$LAN_IF" ] \ + && PROXY_IF=$(docker exec "$GATEWAY" ip -6 neigh show proxy 2>/dev/null | proxy_dev "$VIRTUAL_IP"); then + if [ "$PROXY_IF" = "$LAN_IF" ]; then + check "Proxy NDP entry for $VIRTUAL_IP on $PROXY_IF, the LAN interface" 0 + else + check "Proxy NDP entry for $VIRTUAL_IP on $PROXY_IF, but the LAN interface is $LAN_IF" 1 + fi +else + check "Proxy NDP entry for '$VIRTUAL_IP' on the LAN interface '$LAN_IF' (none found once)" 1 +fi + # Phase 7: Inbound port forwarding — UDP and a second simultaneous TCP forward. # # Three forwards exercised: @@ -371,6 +635,18 @@ if echo "$NFT_RULES" | grep -q "18081"; then else check "nftables port-forward DNAT rule (udp 18081)" 1 fi +# The LAN masquerade must name the interface Phase 1b derived. An empty +# LAN_IF or a reader that found no single rule fails, so a failed listing +# cannot pass as two empty strings. +if [ -n "$LAN_IF" ] && MASQ_IF=$(masq_iface <<< "$NFT_RULES"); then + if [ "$MASQ_IF" = "$LAN_IF" ]; then + check "LAN masquerade on $MASQ_IF, the LAN interface $LAN_IF" 0 + else + check "LAN masquerade on $MASQ_IF, but the LAN interface is $LAN_IF" 1 + fi +else + check "LAN masquerade on the LAN interface '$LAN_IF' (no single LAN masquerade rule)" 1 +fi # Start marker HTTP servers on the LAN-side client. # :8080 → "inbound-forward-ok" (target of tcp 18080) @@ -608,6 +884,24 @@ PYEOF return 1 fi + # The entrypoint re-derives the LAN interface at this start; a mismatch + # reds this check alone, since nothing below depends on the interface. + lan_agree "$prefix: LAN interface after restart" + + # fips-gateway reads the entrypoint's copy, not the mounted file checked + # above, and the checks below need its ttl and grace. + local copy_ttl copy_grace + copy_ttl=$(docker exec "$GATEWAY" grep -c "ttl: 1800" /etc/fips/gateway.yaml 2>/dev/null || true) + copy_grace=$(docker exec "$GATEWAY" grep -c "pool_grace_period: 1800" /etc/fips/gateway.yaml 2>/dev/null || true) + copy_ttl=${copy_ttl:-0} + copy_grace=${copy_grace:-0} + if [ "$copy_ttl" -ge 1 ] && [ "$copy_grace" -ge 1 ]; then + check "$prefix: gateway config copy has ttl 1800 and grace 1800" 0 + else + check "$prefix: gateway config copy (ttl: $copy_ttl, grace: $copy_grace)" 1 + return 1 + fi + sleep 1 local started_log rev_lines started_log=$(docker logs --timestamps --since "$GW_STARTED" "$GATEWAY" 2>&1) From 364fed7c076d2017c90c9d045d4b903fe5fcb699 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 18:59:06 +0000 Subject: [PATCH 06/14] Probe the gateway's inbound forwards with no mapping and assert the rewrite The inbound port-forward probes ran while the DNS mapping to gw-server from Phase 4 was still live. Each mapping installs an SNAT rule matching only the mesh address, ahead of the LAN masquerade, so that rule took gw-server's inbound flows and the probes passed whether or not the LAN masquerade worked. The probes also passed on any response, so a flow rewritten by the LAN masquerade could not be told from one rewritten by a mapping's SNAT rule. Phase 7 now checks only the port-forward rules, and the probes move to a new Phase 8b, after Phase 8 has reclaimed the mapping. Phase 8b first checks that the control socket reports no mapping to gw-server and that the kernel's table holds no SNAT rule. If that gate fails, the probes are recorded as failed rather than skipped silently, so a reclamation failure cannot leave the forwards passing through the SNAT rule. After the probes, Phase 8b reads the gateway's conntrack table and checks, for each of the three forwards, that the reply goes to the gateway's LAN address. A mapping's SNAT sends it to a pool address instead, and no rewrite at all to gw-server's own mesh address, so each case reds with the address it found. The self-test's conntrack and proxy neighbour inputs are now taken verbatim from real gateway suite runs: a healthy run's table after the inbound probes, a mapping's SNAT entry, and a wrong-interface run's unreplied entries and neighbour entries. Inputs that need two situations at once join lines from different runs. --- testing/static/scripts/gateway-test.sh | 402 ++++++++++++++++++------- 1 file changed, 292 insertions(+), 110 deletions(-) diff --git a/testing/static/scripts/gateway-test.sh b/testing/static/scripts/gateway-test.sh index 18216fe0..c116821d 100755 --- a/testing/static/scripts/gateway-test.sh +++ b/testing/static/scripts/gateway-test.sh @@ -192,6 +192,71 @@ print(devs.pop()) ' "$@" } +# How many mappings have mesh_addr $1, from a show_mappings response. Fails +# on an error response or one without a mappings list, so a failed query +# cannot read as zero mappings. +server_mapped() { + python3 -c ' +import ipaddress, json, sys +want = ipaddress.ip_address(sys.argv[1]) +try: + r = json.load(sys.stdin) +except ValueError: + sys.exit(1) +if not isinstance(r, dict) or r.get("status") != "ok": + sys.exit(1) +data = r.get("data") +if not isinstance(data, dict) or not isinstance(data.get("mappings"), list): + sys.exit(1) +hits = 0 +for m in data["mappings"]: + try: + if ipaddress.ip_address(m["mesh_addr"]) == want: + hits += 1 + except (KeyError, TypeError, ValueError): + sys.exit(1) +print(hits) +' "$@" +} + +# The reply destination of the conntrack entries for PROTO $1 to port $2, from +# `conntrack -L -f ipv6`. Of each line's two tuples the first is the original +# direction and the second the reply, so the reply destination is the second +# dst=. It shows which rule rewrote the flow's source: the gateway's LAN +# address for the LAN masquerade, a pool address for a mapping's SNAT, or the +# sender's own address for no rewrite. Fails when no entry matches or the +# matching entries disagree. +reply_dst() { + python3 -c ' +import ipaddress, sys +proto, dport = sys.argv[1], sys.argv[2] +found = set() +for line in sys.stdin: + f = line.split() + if not f or f[0] != proto: + continue + dports = [t[6:] for t in f if t.startswith("dport=")] + dsts = [t[4:] for t in f if t.startswith("dst=")] + if not dports or dports[0] != dport: + continue + try: + found.add(ipaddress.ip_address(dsts[1])) + except (IndexError, ValueError): + sys.exit(1) +if len(found) != 1: + sys.exit(1) +print(found.pop()) +' "$@" +} + +# Succeeds when $1 and $2 are the same IPv6 address in any written form. +same_addr() { + python3 -c ' +import ipaddress, sys +sys.exit(0 if ipaddress.ip_address(sys.argv[1]) == ipaddress.ip_address(sys.argv[2]) else 1) +' "$@" +} + # ── Reader self-test ───────────────────────────────────────────────────── # Run one reader on a canned input and compare its status and output with @@ -210,9 +275,12 @@ gw_case() { return 1 } -# Feed every reader canned tool output and check its answers. The inputs -# follow each tool's printed format; the ip -o addr lines follow a capture, -# the others are written from the tools' documented formats. +# Feed every reader canned tool output and check its answers. The ip -o addr +# lines follow a capture. The ip -6 neigh show proxy and conntrack -L lines +# are verbatim output of those tools from gateway suite runs on 2026-09-23 +# and 2026-09-26; an input that needs two situations at once joins lines +# from different runs. The nft, show_gateway and show_mappings inputs are +# written from the documented formats. gw_selftest() { local fails=0 local addr_eth0 addr_eth1 addr_claimed addr_at nft_lan nft_nolan @@ -269,12 +337,56 @@ gw_selftest() { gw_case "masq_iface: no LAN masquerade" 1 "" "$nft_nolan" masq_iface || fails=$((fails + 1)) gw_case "masq_iface: empty input" 1 "" "" masq_iface || fails=$((fails + 1)) + # Captured lines: one run's entries were on eth0 and a wrong-interface + # run's on eth1. The mixed inputs join lines from the two captures, since + # no single run holds entries on both devices. + local nd_one0='fd01::1 dev eth0 proxy ' nd_one1='fd01::1 dev eth1 proxy ' + local nd_two1='fd01::2 dev eth1 proxy ' + gw_case "proxy_dev: captured entries on eth0" 0 eth0 \ + $'fd01::1 dev eth0 proxy \nfd01::2 dev eth0 proxy ' proxy_dev fd01::1 || fails=$((fails + 1)) gw_case "proxy_dev: entry on eth0 after another on eth1" 0 eth0 \ - $'fd01::2 dev eth1 proxy\nfd01::1 dev eth0 proxy' proxy_dev fd01::1 || fails=$((fails + 1)) - gw_case "proxy_dev: no entry" 1 "" 'fd01::2 dev eth1 proxy' proxy_dev fd01::1 || fails=$((fails + 1)) + "$nd_two1"$'\n'"$nd_one0" proxy_dev fd01::1 || fails=$((fails + 1)) + gw_case "proxy_dev: no entry" 1 "" "$nd_two1" proxy_dev fd01::1 || fails=$((fails + 1)) gw_case "proxy_dev: empty input" 1 "" "" proxy_dev fd01::1 || fails=$((fails + 1)) gw_case "proxy_dev: entry on two devices" 1 "" \ - $'fd01::1 dev eth0 proxy\nfd01::1 dev eth1 proxy' proxy_dev fd01::1 || fails=$((fails + 1)) + "$nd_one0"$'\n'"$nd_one1" proxy_dev fd01::1 || fails=$((fails + 1)) + + local maps_one + maps_one='{"status":"ok","data":{"mappings":[{"virtual_ip":"fd01::1","mesh_addr":"fd3c:9a51:7e02:4b18::2","node_addr":"0a1b2c3d4e5f60718293a4b5c6d7e8f9","dns_name":"npub1example.fips","state":"active","sessions":0,"age_secs":3,"last_ref_secs":3}]}}' + gw_case "server_mapped: one mapping to the server" 0 1 "$maps_one" \ + server_mapped fd3c:9a51:7e02:4b18:0:0:0:2 || fails=$((fails + 1)) + gw_case "server_mapped: a mapping to another node" 0 0 "$maps_one" \ + server_mapped fd3c:9a51:7e02:4b18::3 || fails=$((fails + 1)) + gw_case "server_mapped: no mappings" 0 0 '{"status":"ok","data":{"mappings":[]}}' \ + server_mapped fd3c:9a51:7e02:4b18::2 || fails=$((fails + 1)) + gw_case "server_mapped: error response" 1 "" '{"status":"error","message":"gateway not yet initialized"}' \ + server_mapped fd3c:9a51:7e02:4b18::2 || fails=$((fails + 1)) + gw_case "server_mapped: ok response without data" 1 "" '{"status":"ok"}' \ + server_mapped fd3c:9a51:7e02:4b18::2 || fails=$((fails + 1)) + gw_case "server_mapped: empty input" 1 "" "" server_mapped fd3c:9a51:7e02:4b18::2 || fails=$((fails + 1)) + + # Captured lines. ct_masq is one healthy run's whole table after the + # probes; ct_snat is a mapping's SNAT entry and ct_unreplied a + # wrong-interface run's entry, whose reply tuple is not rewritten. + # ct_other is one line of ct_masq, and ct_split joins the SNAT line with + # a masquerade line, since no single run holds both for one port. + local srv=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c + local ct_masq ct_snat ct_unreplied ct_other ct_split ct_masq80 + ct_masq80='tcp 6 119 TIME_WAIT src=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c dst=fd8d:4f49:3df7:6e1d:171e:c08d:f45f:97f3 sport=48192 dport=18080 src=fd02::20 dst=fd02::10 sport=8080 dport=48192 [ASSURED] mark=0 use=1' + ct_other='tcp 6 119 TIME_WAIT src=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c dst=fd8d:4f49:3df7:6e1d:171e:c08d:f45f:97f3 sport=48486 dport=18082 src=fd02::20 dst=fd02::10 sport=8081 dport=48486 [ASSURED] mark=0 use=1' + ct_masq='udp 17 29 src=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c dst=fd8d:4f49:3df7:6e1d:171e:c08d:f45f:97f3 sport=57965 dport=18081 src=fd02::20 dst=fd02::10 sport=8081 dport=57965 mark=0 use=1'$'\n'"$ct_masq80"$'\n'"$ct_other" + ct_snat='tcp 6 119 TIME_WAIT src=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c dst=fd8d:4f49:3df7:6e1d:171e:c08d:f45f:97f3 sport=48130 dport=18080 src=fd02::20 dst=fd01::1 sport=8080 dport=48130 [ASSURED] mark=0 use=1' + ct_unreplied='udp 17 24 src=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c dst=fd8d:4f49:3df7:6e1d:171e:c08d:f45f:97f3 sport=57286 dport=18081 [UNREPLIED] src=fd02:0:0:1::20 dst=fda3:bc52:6504:aa72:71ca:376a:9249:ef0c sport=8081 dport=57286 mark=0 use=1' + ct_split="$ct_snat"$'\n'"$ct_masq80" + gw_case "reply_dst: masquerade" 0 fd02::10 "$ct_masq" reply_dst tcp 18080 || fails=$((fails + 1)) + gw_case "reply_dst: mapping SNAT" 0 fd01::1 "$ct_snat" reply_dst tcp 18080 || fails=$((fails + 1)) + gw_case "reply_dst: udp, replied" 0 fd02::10 "$ct_masq" reply_dst udp 18081 || fails=$((fails + 1)) + gw_case "reply_dst: udp, unreplied, no rewrite" 0 "$srv" "$ct_unreplied" reply_dst udp 18081 || fails=$((fails + 1)) + gw_case "reply_dst: only another port's entry" 1 "" "$ct_other" reply_dst tcp 18080 || fails=$((fails + 1)) + gw_case "reply_dst: empty input" 1 "" "" reply_dst tcp 18080 || fails=$((fails + 1)) + gw_case "reply_dst: entries disagree" 1 "" "$ct_split" reply_dst tcp 18080 || fails=$((fails + 1)) + gw_case "same_addr: two forms of one address" 0 "" "" same_addr fd02:0:0:0::10 fd02::10 || fails=$((fails + 1)) + gw_case "same_addr: different addresses" 1 "" "" same_addr fd01::1 fd02::10 || fails=$((fails + 1)) echo " selftest: $fails case(s) failed" if [ "$fails" -eq 0 ]; then @@ -604,18 +716,21 @@ else check "Proxy NDP entry for '$VIRTUAL_IP' on the LAN interface '$LAN_IF' (none found once)" 1 fi -# Phase 7: Inbound port forwarding — UDP and a second simultaneous TCP forward. +# Phase 7: Inbound port-forward rules — UDP and a second simultaneous TCP +# forward. # -# Three forwards exercised: +# Three forwards configured: # tcp 18080 → [fd02::20]:8080 (original — single TCP rule) # tcp 18082 → [fd02::20]:8081 (6B — second TCP rule, multiple forwards) # udp 18081 → [fd02::20]:8081 (6A — UDP DNAT runtime path) # -# Mesh peer (gw-server) hits each gw-gateway fips0: rule, which -# DNATs into the LAN-side gw-client. Exercises the DNAT rules + LAN-side -# masquerade installed by set_port_forwards(). +# Checks the DNAT rules and the LAN-side masquerade that set_port_forwards() +# installs. The traffic through them is Phase 8b's: while the Phase 4 +# mapping to gw-server is live, its SNAT rule matches gw-server's inbound +# flows before the LAN masquerade does, so probes sent here would pass +# without the masquerade. echo "" -echo "Phase 7: Inbound port forwards" +echo "Phase 7: Inbound port-forward rules" # Confirm all three port-forward DNAT rules are present on the gateway. # The distinctive listen ports identify our rules regardless of how nft @@ -648,104 +763,6 @@ else check "LAN masquerade on the LAN interface '$LAN_IF' (no single LAN masquerade rule)" 1 fi -# Start marker HTTP servers on the LAN-side client. -# :8080 → "inbound-forward-ok" (target of tcp 18080) -# :8081 → "inbound-forward-ok-2" (target of tcp 18082) -# `docker exec -d` is required; `docker exec bash -c 'cmd &'` doesn't -# keep the child alive past the exec session, even with nohup. -docker exec "$CLIENT" sh -c ' - mkdir -p /tmp/inbound /tmp/inbound2 - echo "inbound-forward-ok" > /tmp/inbound/index.html - echo "inbound-forward-ok-2" > /tmp/inbound2/index.html - pkill -f "http.server 8080" 2>/dev/null || true - pkill -f "http.server 8081" 2>/dev/null || true - pkill -f "udp_echo.py" 2>/dev/null || true -' >/dev/null 2>&1 || true -docker exec -d "$CLIENT" python3 -m http.server 8080 --bind :: --directory /tmp/inbound \ - >/dev/null 2>&1 || true -docker exec -d "$CLIENT" python3 -m http.server 8081 --bind :: --directory /tmp/inbound2 \ - >/dev/null 2>&1 || true - -# Start a UDP echo server on the LAN-side client at [::]:8081/udp. -# This is the target of the udp 18081 forward. Stash the script as a -# named file (`udp_echo.py`) so the cleanup pkill above can find it. -docker exec "$CLIENT" sh -c 'cat > /tmp/udp_echo.py <<'\''PYEOF'\'' -import socket, sys -s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) -s.bind(("::", 8081)) -while True: - data, addr = s.recvfrom(2048) - s.sendto(b"udp-forward-ok:" + data, addr) -PYEOF' >/dev/null 2>&1 || true -docker exec -d "$CLIENT" python3 /tmp/udp_echo.py >/dev/null 2>&1 || true - -# Give the servers a moment to bind. -for _ in 1 2 3 4 5; do - TCP_READY=$(docker exec "$CLIENT" ss -6lnt 2>/dev/null | grep -cE ':8080|:8081' || true) - UDP_READY=$(docker exec "$CLIENT" ss -6lnu 2>/dev/null | grep -c ':8081' || true) - if [ "$TCP_READY" -ge 2 ] && [ "$UDP_READY" -ge 1 ]; then - break - fi - sleep 1 -done - -# Derive the gateway's mesh IPv6 (fd00::/8 address assigned to fips0). -GW_MESH_IP=$(docker exec "$GATEWAY" bash -c \ - "ip -6 -o addr show fips0 | awk '/inet6 fd/ {print \$4}' | cut -d/ -f1 | head -1" \ - 2>/dev/null || echo "") - -if [ -z "$GW_MESH_IP" ]; then - check "Gateway fips0 IPv6 address" 1 -else - echo " Gateway mesh IPv6: $GW_MESH_IP" - - # From the mesh side (gw-server), fetch through each TCP forward. - FWD_RESPONSE=$(docker exec "$SERVER" curl -6 -s --max-time 10 \ - "http://[${GW_MESH_IP}]:18080/" 2>&1) || true - # 8080 backend serves "inbound-forward-ok" (no -2 suffix) — distinct - # from the 8081 backend so a misrouted response would be detectable. - if echo "$FWD_RESPONSE" | grep -qE '^inbound-forward-ok$'; then - check "Inbound HTTP via TCP forward 18080 → [${GW_CLIENT_LAN}]:8080" 0 - else - check "Inbound HTTP via TCP forward 18080 (response: '${FWD_RESPONSE:0:80}')" 1 - fi - - FWD_RESPONSE_2=$(docker exec "$SERVER" curl -6 -s --max-time 10 \ - "http://[${GW_MESH_IP}]:18082/" 2>&1) || true - if echo "$FWD_RESPONSE_2" | grep -q "inbound-forward-ok-2"; then - check "Inbound HTTP via TCP forward 18082 → [${GW_CLIENT_LAN}]:8081 (6B)" 0 - else - check "Inbound HTTP via TCP forward 18082 (response: '${FWD_RESPONSE_2:0:80}')" 1 - fi - - # 6A: UDP forward. Send a probe via a one-shot Python client on - # gw-server; the LAN-side echo server prepends "udp-forward-ok:". - UDP_RESPONSE=$(docker exec "$SERVER" python3 -c " -import socket, sys -s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) -s.settimeout(5) -s.sendto(b'ping-via-udp-fwd', ('${GW_MESH_IP}', 18081)) -try: - data, _ = s.recvfrom(2048) - sys.stdout.write(data.decode('utf-8', 'replace')) -except Exception as e: - sys.stdout.write('ERR: ' + str(e)) -" 2>&1) || true - if echo "$UDP_RESPONSE" | grep -q "udp-forward-ok:ping-via-udp-fwd"; then - check "Inbound UDP via forward 18081 → [${GW_CLIENT_LAN}]:8081 (6A)" 0 - else - check "Inbound UDP via forward 18081 (response: '${UDP_RESPONSE:0:80}')" 1 - fi -fi - -# Cleanup: stop the LAN-side responders so Phase 8's pool-reclamation -# wait isn't interfered with by lingering sessions. -docker exec "$CLIENT" sh -c ' - pkill -f "http.server 8080" 2>/dev/null || true - pkill -f "http.server 8081" 2>/dev/null || true - pkill -f "udp_echo.py" 2>/dev/null || true -' >/dev/null 2>&1 || true - # Phase 8: TTL expiration and pool reclamation echo "" echo "Phase 8: TTL expiration and pool reclamation" @@ -784,6 +801,171 @@ else check "Mapping reclaimed (count: $MAPPING_COUNT)" 1 fi +# Phase 8b: Inbound port forwards through the LAN masquerade +# +# Mesh peer (gw-server) hits each gw-gateway fips0: rule, which DNATs +# into the LAN-side gw-client, and the LAN masquerade rewrites the source to +# the gateway's LAN address. Runs after Phase 8 has reclaimed the mapping to +# gw-server, because a live mapping's SNAT rule matches the same flows first +# and would do the rewrite instead. Runs before Phase 9 kills the daemon. +# +# The gate reads both the control socket's mappings, a snapshot refreshed +# on the pool tick, and the kernel's table, which is what decides the rule +# that matches. A zero SNAT count needs a successful listing; Phase 11 reads +# the same pattern expecting one rule per mapping. +echo "" +echo "Phase 8b: Inbound port forwards through the LAN masquerade" +SERVER_MESH=$(docker exec "$SERVER" bash -c \ + "ip -6 -o addr show fips0 | awk '/inet6 fd/ {print \$4}' | cut -d/ -f1 | head -1" \ + 2>/dev/null || echo "") +if [ -n "$SERVER_MESH" ] && SERVER_MAPS=$(docker exec "$GATEWAY" bash -c \ + 'echo "{\"command\":\"show_mappings\"}" | nc -U -w1 /run/fips/gateway.sock 2>/dev/null' \ + | server_mapped "$SERVER_MESH"); then + : +else + SERVER_MAPS=error +fi +GATE_NFT_RC=0 +GATE_NFT=$(docker exec "$GATEWAY" nft list table inet fips_gateway 2>&1) || GATE_NFT_RC=$? +GATE_SNAT=$(grep -cE "saddr [0-9a-f:]+ .*snat" <<< "$GATE_NFT" || true) +GATE_VALUES="server mesh '$SERVER_MESH', mappings to it $SERVER_MAPS, nft rc $GATE_NFT_RC, SNAT rules $GATE_SNAT" +if [ -n "$SERVER_MESH" ] && [ "$SERVER_MAPS" = "0" ] && [ "$GATE_NFT_RC" -eq 0 ] && [ "$GATE_SNAT" -eq 0 ]; then + check "No mapping or SNAT rule to $SERVER before the probes ($GATE_VALUES)" 0 + GATE_OK=true +else + check "No mapping or SNAT rule to $SERVER before the probes ($GATE_VALUES)" 1 + GATE_OK=false +fi + +if [ "$GATE_OK" = true ]; then + # Start marker HTTP servers on the LAN-side client. + # :8080 → "inbound-forward-ok" (target of tcp 18080) + # :8081 → "inbound-forward-ok-2" (target of tcp 18082) + # `docker exec -d` is required; `docker exec bash -c 'cmd &'` doesn't + # keep the child alive past the exec session, even with nohup. + docker exec "$CLIENT" sh -c ' + mkdir -p /tmp/inbound /tmp/inbound2 + echo "inbound-forward-ok" > /tmp/inbound/index.html + echo "inbound-forward-ok-2" > /tmp/inbound2/index.html + pkill -f "http.server 8080" 2>/dev/null || true + pkill -f "http.server 8081" 2>/dev/null || true + pkill -f "udp_echo.py" 2>/dev/null || true + ' >/dev/null 2>&1 || true + docker exec -d "$CLIENT" python3 -m http.server 8080 --bind :: --directory /tmp/inbound \ + >/dev/null 2>&1 || true + docker exec -d "$CLIENT" python3 -m http.server 8081 --bind :: --directory /tmp/inbound2 \ + >/dev/null 2>&1 || true + + # Start a UDP echo server on the LAN-side client at [::]:8081/udp. + # This is the target of the udp 18081 forward. Stash the script as a + # named file (`udp_echo.py`) so the cleanup pkill above can find it. + docker exec "$CLIENT" sh -c 'cat > /tmp/udp_echo.py <<'\''PYEOF'\'' +import socket, sys +s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) +s.bind(("::", 8081)) +while True: + data, addr = s.recvfrom(2048) + s.sendto(b"udp-forward-ok:" + data, addr) +PYEOF' >/dev/null 2>&1 || true + docker exec -d "$CLIENT" python3 /tmp/udp_echo.py >/dev/null 2>&1 || true + + # Give the servers a moment to bind. + for _ in 1 2 3 4 5; do + TCP_READY=$(docker exec "$CLIENT" ss -6lnt 2>/dev/null | grep -cE ':8080|:8081' || true) + UDP_READY=$(docker exec "$CLIENT" ss -6lnu 2>/dev/null | grep -c ':8081' || true) + if [ "$TCP_READY" -ge 2 ] && [ "$UDP_READY" -ge 1 ]; then + break + fi + sleep 1 + done + + # Derive the gateway's mesh IPv6 (fd00::/8 address assigned to fips0). + GW_MESH_IP=$(docker exec "$GATEWAY" bash -c \ + "ip -6 -o addr show fips0 | awk '/inet6 fd/ {print \$4}' | cut -d/ -f1 | head -1" \ + 2>/dev/null || echo "") + + if [ -z "$GW_MESH_IP" ]; then + check "Gateway fips0 IPv6 address" 1 + else + echo " Gateway mesh IPv6: $GW_MESH_IP" + + # From the mesh side (gw-server), fetch through each TCP forward. + FWD_RESPONSE=$(docker exec "$SERVER" curl -6 -s --max-time 10 \ + "http://[${GW_MESH_IP}]:18080/" 2>&1) || true + # 8080 backend serves "inbound-forward-ok" (no -2 suffix) — distinct + # from the 8081 backend so a misrouted response would be detectable. + if echo "$FWD_RESPONSE" | grep -qE '^inbound-forward-ok$'; then + check "Inbound HTTP via TCP forward 18080 → [${GW_CLIENT_LAN}]:8080" 0 + else + check "Inbound HTTP via TCP forward 18080 (response: '${FWD_RESPONSE:0:80}')" 1 + fi + + FWD_RESPONSE_2=$(docker exec "$SERVER" curl -6 -s --max-time 10 \ + "http://[${GW_MESH_IP}]:18082/" 2>&1) || true + if echo "$FWD_RESPONSE_2" | grep -q "inbound-forward-ok-2"; then + check "Inbound HTTP via TCP forward 18082 → [${GW_CLIENT_LAN}]:8081 (6B)" 0 + else + check "Inbound HTTP via TCP forward 18082 (response: '${FWD_RESPONSE_2:0:80}')" 1 + fi + + # 6A: UDP forward. Send a probe via a one-shot Python client on + # gw-server; the LAN-side echo server prepends "udp-forward-ok:". + UDP_RESPONSE=$(docker exec "$SERVER" python3 -c " +import socket, sys +s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) +s.settimeout(5) +s.sendto(b'ping-via-udp-fwd', ('${GW_MESH_IP}', 18081)) +try: + data, _ = s.recvfrom(2048) + sys.stdout.write(data.decode('utf-8', 'replace')) +except Exception as e: + sys.stdout.write('ERR: ' + str(e)) +" 2>&1) || true + if echo "$UDP_RESPONSE" | grep -q "udp-forward-ok:ping-via-udp-fwd"; then + check "Inbound UDP via forward 18081 → [${GW_CLIENT_LAN}]:8081 (6A)" 0 + else + check "Inbound UDP via forward 18081 (response: '${UDP_RESPONSE:0:80}')" 1 + fi + fi + + # A response shows only that some rule rewrote the flow. The reply + # destination in the gateway's conntrack entry shows which: the LAN + # masquerade sends the reply to the gateway's LAN address, a mapping's + # SNAT to a pool address, and no rewrite to gw-server's mesh address. + # conntrack lists the reply tuple of an unreplied entry too. + if CT_TABLE=$(docker exec "$GATEWAY" conntrack -L -f ipv6 2>/dev/null); then + CT_OK=true + else + CT_OK=false + CT_TABLE="" + fi + for fwd in tcp:18080 tcp:18082 udp:18081; do + fwd_proto="${fwd%%:*}" + fwd_port="${fwd#*:}" + found="" + if [ "$CT_OK" = true ] && found=$(reply_dst "$fwd_proto" "$fwd_port" <<< "$CT_TABLE") \ + && same_addr "$found" "$GW_DNS"; then + check "Reply to $fwd_proto $fwd_port goes to $found, the gateway's LAN address $GW_DNS" 0 + else + check "Reply to $fwd_proto $fwd_port goes to '$found' (conntrack read $CT_OK), expected the gateway's LAN address $GW_DNS" 1 + fi + done + + # Stop the LAN-side responders; no later phase uses them. + docker exec "$CLIENT" sh -c ' + pkill -f "http.server 8080" 2>/dev/null || true + pkill -f "http.server 8081" 2>/dev/null || true + pkill -f "udp_echo.py" 2>/dev/null || true + ' >/dev/null 2>&1 || true +else + check "Inbound HTTP via TCP forward 18080 (skipped: gate)" 1 + check "Inbound HTTP via TCP forward 18082 (skipped: gate)" 1 + check "Inbound UDP via forward 18081 (skipped: gate)" 1 + check "Reply to tcp 18080 goes to the gateway's LAN address (skipped: gate)" 1 + check "Reply to tcp 18082 goes to the gateway's LAN address (skipped: gate)" 1 + check "Reply to udp 18081 goes to the gateway's LAN address (skipped: gate)" 1 +fi + # Phase 9: SERVFAIL when daemon DNS is down echo "" echo "Phase 9: SERVFAIL when daemon DNS is down" From 6c1fc4e83a06688048ff3ce3f9add68064f42de9 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 18:00:18 +0000 Subject: [PATCH 07/14] Fail the AUR build job on namcap error-level findings The AUR build job ran namcap on the PKGBUILD and the built package and took its exit status as the verdict. namcap exits 0 when it reports error-level findings, and also exits 0 when it cannot read its input at all: a missing file, an unexpanded glob, or a file that is not a package. The lint could therefore never fail the job, and a package with error-level findings could still be published. namcap-gate.sh runs namcap with informational lines and tag names on each PKGBUILD or built package and fails a file when namcap reports an E: finding, exits nonzero, prints a line that is not a tagged finding, or, for a built package, omits the line that shows it analysed dependencies. The last three stop an unreadable input from passing as a clean one. Warnings stay advisory. Every file is examined before the verdict, and namcap runs with /usr/bin first on PATH so an undeclared script interpreter is reported the same way whether or not it runs as root. The build job now lints through the gate. Because the publish job needs the build job, a package with error-level findings is no longer published. test-namcap-gate.sh checks the gate against namcap output captured from the real package and from toy packages; the build job runs it. With --live it builds three toy packages and runs the gate with the real namcap, and the build job runs that too, so a namcap update that stops reporting missing dependencies as errors turns the job red. --- .github/workflows/aur-publish.yml | 22 +- packaging/aur/README.md | 2 + packaging/aur/namcap-gate.sh | 135 +++++++++ packaging/aur/test-namcap-gate.sh | 445 ++++++++++++++++++++++++++++++ 4 files changed, 598 insertions(+), 6 deletions(-) create mode 100755 packaging/aur/namcap-gate.sh create mode 100755 packaging/aur/test-namcap-gate.sh diff --git a/.github/workflows/aur-publish.yml b/.github/workflows/aur-publish.yml index d82f8192..022b5d62 100644 --- a/.github/workflows/aur-publish.yml +++ b/.github/workflows/aur-publish.yml @@ -28,7 +28,8 @@ jobs: # namcap in an Arch container (neither tool exists on ubuntu-latest) and builds # the *checked-out tree* from a local git-archive tarball, so it works for # branch/PR builds and unreleased rc tags whose GitHub source archive does not - # exist yet. This job never publishes. + # exist yet. The lint fails the job on namcap error-level (E:) findings; + # warnings (W:) are advisory. This job never publishes. # ─────────────────────────────────────────────────────────────────────────── aur-build: name: Build and lint fips AUR package @@ -49,6 +50,11 @@ jobs: # before a release tag depends on it. run: bash packaging/aur/test-await-package-runs.sh + - name: Test the namcap gate + # Fixture tests, with canned namcap output, for the script the lint + # below runs namcap through. + run: bash packaging/aur/test-namcap-gate.sh + - name: Resolve package version id: ver env: @@ -88,6 +94,13 @@ jobs: # The checkout is owned by root; hand it to the build user. chown -R builder:builder "$GITHUB_WORKSPACE" + - name: Prove the namcap gate against real namcap + # Builds toy packages, one declared correctly and two missing a + # dependency, and checks the gate passes the first and fails the others + # with this run's namcap. Runs as the build user because makepkg + # refuses root and because that is how the lint below runs. + run: sudo -u builder bash packaging/aur/test-namcap-gate.sh --live + - name: Build a local source tarball of the checkout env: VERSION: ${{ steps.ver.outputs.version }} @@ -122,7 +135,7 @@ jobs: sudo -u builder bash -euo pipefail -c ' cd packaging/aur echo "::group::namcap PKGBUILD" - namcap PKGBUILD + bash namcap-gate.sh PKGBUILD echo "::endgroup::" echo "::group::makepkg build" # --nocheck: skip the PKGBUILD check() (cargo test --lib); the test @@ -130,10 +143,7 @@ jobs: makepkg -s --noconfirm --nocheck echo "::endgroup::" echo "::group::namcap built package" - for pkg in *.pkg.tar.*; do - echo "namcap $pkg" - namcap "$pkg" - done + bash namcap-gate.sh ./*.pkg.tar.* echo "::endgroup::" ' diff --git a/packaging/aur/README.md b/packaging/aur/README.md index 8017349f..0593c462 100644 --- a/packaging/aur/README.md +++ b/packaging/aur/README.md @@ -22,6 +22,8 @@ This directory contains Arch Linux packaging files for two AUR packages: | `patch-pkgbuild.sh` | Rewrites `pkgver`, `pkgrel`, `conflicts`, `options`, and `b2sums` in the PKGBUILD at publish time | | `await-package-runs.sh` | Holds the AUR publish until every `package-*.yml` run for the release tag has succeeded | | `test-await-package-runs.sh` | Fixture tests for `await-package-runs.sh`, run by the `aur-build` job | +| `namcap-gate.sh` | Fails the `aur-build` job on namcap error-level findings; warnings are advisory | +| `test-namcap-gate.sh` | Fixture tests for `namcap-gate.sh` with canned namcap output, plus `--live` against real namcap; both run by the `aur-build` job | Both PKGBUILDs reference files from `packaging/debian/` (service files) and `packaging/common/` (config files) at build time. These are pulled from the diff --git a/packaging/aur/namcap-gate.sh b/packaging/aur/namcap-gate.sh new file mode 100755 index 00000000..4a982b1c --- /dev/null +++ b/packaging/aur/namcap-gate.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# Run namcap on PKGBUILDs and built packages and fail on error-level findings. +# +# The AUR build job lints the release PKGBUILD and the package it builds. The +# rule is that namcap error-level (E:) findings fail the job and warnings (W:) +# stay advisory. namcap's own exit status cannot carry that verdict: namcap +# 3.6 exits 0 when it reports E: findings, and also exits 0 when it could not +# read its input at all (a missing file, an unexpanded glob, a file that is not +# a package). So this script reads namcap's output instead. +# +# For each FILE it requires all of: +# - the file exists, and its name is a PKGBUILD (PKGBUILD*) or a built +# package (*.pkg.tar.*); +# - namcap exits 0; +# - every non-blank output line is a tagged finding (" X: ..." or +# "PKGBUILD () X: ..." with X one of E, W, I); +# - for a built package, the output includes the "depends-by-namcap-sight" +# informational line, which namcap prints only after it has analysed the +# package's dependencies; +# - no E: findings. +# The last three close the case in which namcap examined nothing: without them +# an unreadable input would show the same zero E: count as a clean package. +# +# A red from the second, third or fourth rule is a failure of the gate to read +# namcap, not a packaging finding. It can follow a namcap update (a Python +# warning on stdout, a renamed tag). Fix the gate for it; do not edit depends. +# +# namcap resolves script interpreters through PATH, and on Arch /usr/sbin is a +# symlink to bin that pacman does not record. Run as root, where /usr/sbin comes +# first, namcap reports an undeclared script dependency such as nftables as a +# warning instead of an error. So namcap runs with /usr/bin first on PATH. The +# AUR job runs namcap under sudo, whose secure_path already puts /usr/bin +# first, so GitHub CI does not exercise this pin; only a run as root does. +# +# Known limit: a PKGBUILD has no such dependency-analysis line, so an empty +# namcap output for a PKGBUILD passes as a clean one does. The built package +# carries dependency detection. +# +# namcap is not pinned, so a namcap or Arch repository update can red the gate +# with no fips change, and that is intended. One known case: the fips package +# does not declare bash, which namcap accepts only because dbus pulls it in; if +# Arch's dbus stops depending on bash, namcap reports bash as an error. +# +# Every file is examined before the verdict, so a failure in one is reported +# even when a later one is clean. +# +# Exit status: 0 all files passed, 1 at least one file failed, 2 usage error +# or namcap not found. +# +# Usage: bash namcap-gate.sh FILE... + +set -euo pipefail + +if [ "$#" -eq 0 ]; then + echo "usage: namcap-gate.sh FILE..." >&2 + exit 2 +fi + +# Resolve namcap before PATH is changed, so a namcap earlier on the caller's +# PATH is the one that runs. +namcap_bin=$(command -v namcap) || { + echo "namcap gate: namcap not found on PATH" >&2 + exit 2 +} + +tagged='^[^ ]+( \([^)]*\))? [EWI]:[[:space:]]' +errpat='^[^ ]+( \([^)]*\))? E:[[:space:]]' +marker='^[^ ]+ I: depends-by-namcap-sight[[:space:]]' + +fails=0 + +for f in "$@"; do + case "$(basename -- "$f")" in + PKGBUILD*) mode=pkgbuild ;; + *.pkg.tar.*) mode=package ;; + *) + echo "namcap gate: $f: not a PKGBUILD or package" + fails=$((fails + 1)) + continue + ;; + esac + + if [ ! -f "$f" ]; then + echo "namcap gate: $f: not found" + fails=$((fails + 1)) + continue + fi + + rc=0 + out=$(PATH="/usr/bin:$PATH" "$namcap_bin" -i -m "$f" 2>&1) || rc=$? + echo "namcap: $f" + printf '%s\n' "$out" + + errs=0 + seen=0 + bad="" + while IFS= read -r line; do + [[ $line =~ ^[[:space:]]*$ ]] && continue + if ! [[ $line =~ $tagged ]]; then + [ -n "$bad" ] || bad=$line + continue + fi + if [[ $line =~ $errpat ]]; then + errs=$((errs + 1)) + echo "::error title=namcap::$line" + fi + if [[ $line =~ $marker ]]; then + seen=1 + fi + done <<< "$out" + + failed=0 + if [ "$rc" -ne 0 ]; then + echo "namcap gate: $f: namcap exited $rc" + failed=1 + fi + if [ -n "$bad" ]; then + echo "namcap gate: $f: unrecognised namcap output: $bad" + failed=1 + fi + if [ "$mode" = package ] && [ "$seen" -eq 0 ]; then + echo "namcap gate: $f: no dependency analysis in namcap output" + failed=1 + fi + echo "namcap gate: $f: $errs error-level finding(s)" + [ "$errs" -eq 0 ] || failed=1 + fails=$((fails + failed)) +done + +if [ "$fails" -eq 0 ]; then + echo "namcap gate: passed ($# file(s), W: findings are advisory)" + exit 0 +fi +echo "namcap gate: FAILED ($fails of $# file(s) failed)" +exit 1 diff --git a/packaging/aur/test-namcap-gate.sh b/packaging/aur/test-namcap-gate.sh new file mode 100755 index 00000000..e309fdd5 --- /dev/null +++ b/packaging/aur/test-namcap-gate.sh @@ -0,0 +1,445 @@ +#!/usr/bin/env bash +# Tests for namcap-gate.sh, the check that fails the AUR build job on namcap +# error-level findings. +# +# Default mode runs the gate against canned namcap output. A stub `namcap` +# first on PATH records its arguments, prints /.out for the +# file it is given (nothing if absent), and exits with .rc (0 if +# absent). It exits 2 on an argument shape namcap does not accept. The canned +# output is copied verbatim from namcap 3.6.0-3 runs on the real fips package +# and on toy packages. These cases prove the parsing: which lines fail the +# gate, and that an unreadable input cannot pass as a clean one. +# +# --live builds three toy packages with makepkg and runs the gate on each with +# the real namcap: one declared correctly, one missing a library dependency, +# one missing a script interpreter's package. It proves that the installed +# namcap still reports those as error-level findings, which canned output +# cannot. It needs makepkg, gcc, namcap and the dbus and nftables packages, +# and refuses to run as root, because makepkg does. +# +# GATE= runs the cases against another script in place of the gate. +# Exits nonzero if any case fails or if fewer cases ran than are defined. +# +# Usage: bash packaging/aur/test-namcap-gate.sh [--live] + +set -euo pipefail + +HERE=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +GATE="${GATE:-$HERE/namcap-gate.sh}" + +case "${1:-}" in + '') MODE=canned ;; + --live) MODE=live ;; + *) echo "usage: test-namcap-gate.sh [--live]" >&2; exit 2 ;; +esac + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT + +CASES_DEFINED=0 +CASES_RAN=0 +FAILED=0 +STUBDIR="" + +# Run the gate on the given files and check its exit status and that its +# output states the expected reason, so a red caused by a crash in the gate +# does not pass as the intended red. The stub namcap is first on PATH in +# canned mode only. Sets LAST_OUT to the gate's combined output. +# Args: name fixture-dir expect(zero|nonzero) pattern [file...] +check() { + local name=$1 fixture=$2 expect=$3 pattern=$4 rc=0 + shift 4 + CASES_RAN=$((CASES_RAN + 1)) + : > "$WORK/calls" + LAST_OUT=$(PATH="$STUBDIR$PATH" STUB_FIXTURE="$fixture" STUB_CALLS="$WORK/calls" \ + bash "$GATE" "$@" 2>&1) || rc=$? + if { [ "$expect" = zero ] && [ "$rc" -eq 0 ]; } || + { [ "$expect" = nonzero ] && [ "$rc" -ne 0 ]; }; then + if printf '%s\n' "$LAST_OUT" | grep -qE -- "$pattern"; then + echo "PASS $name (exit $rc)" + return 0 + fi + echo "FAIL $name: exit $rc as expected, but output lacks /$pattern/" + else + echo "FAIL $name: expected $expect exit, got $rc" + fi + printf '%s\n' "$LAST_OUT" | sed 's/^/ /' + FAILED=$((FAILED + 1)) + return 1 +} + +# Record an extra assertion's failure against the case that just ran. +fail() { + echo "FAIL $1" + FAILED=$((FAILED + 1)) +} + +# Count the stub namcap's invocations in the case that just ran. +calls() { + grep -c '' "$WORK/calls" || true +} + +# Make a fresh fixture directory for a case and print its path. +fixture() { + local dir="$WORK/fx/$1" + mkdir -p "$dir/files" + echo "$dir" +} + +# Create the file the gate is given, in the fixture's files directory, and +# print its path. The content is irrelevant: the stub serves the output. +placeholder() { + : > "$1/files/$2" + echo "$1/files/$2" +} + +# --- canned namcap output ---------------------------------------------------- + +# The real fips 0.5.1 package built from maint, as declared. +REAL_DECLARED=$(cat <<'EOF' +fips W: file-not-world-readable etc/fips/fips.yaml +fips I: script-link-detected nft in ['etc/fips/fips.nft'] +fips I: script-link-detected bash in ['usr/lib/fips/fips-dns-setup', 'usr/lib/fips/fips-dns-teardown'] +fips I: libdepends-missing-provides ld-linux-x86-64.so=2-64 glibc (['usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop', 'usr/bin/fips-gateway']) +fips I: libdepends-missing-provides libc.so=6-64 glibc (['usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop', 'usr/bin/fips-gateway']) +fips I: libdepends-missing-provides libm.so=6-64 glibc (['usr/bin/fips', 'usr/bin/fipstop', 'usr/bin/fips-gateway']) +fips I: link-level-dependence dbus in ['usr/lib/libdbus-1.so.3'] +fips I: link-level-dependence glibc in ['usr/lib/ld-linux-x86-64.so.2', 'usr/lib/libc.so.6', 'usr/lib/libm.so.6'] +fips I: link-level-dependence libgcc in ['usr/lib/libgcc_s.so.1'] +fips I: libdepends-detected-not-included libdbus-1.so=3-64 dbus (['usr/bin/fips']) +fips I: libdepends-detected-not-included libgcc_s.so=1-64 libgcc (['usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop', 'usr/bin/fips-gateway']) +fips I: libdepends-by-namcap-sight depends=(glibc libdbus-1.so=3-64 libgcc_s.so=1-64) +fips I: libprovides-by-namcap-sight provides=() +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fips +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fips-gateway +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fipsctl +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fipstop +fips W: dependency-detected-but-optional nftables (programs-needed ['nft'] ['etc/fips/fips.nft']) +fips W: dependency-implicitly-satisfied libgcc (libraries-needed ['usr/lib/libgcc_s.so.1'] ['usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop', 'usr/bin/fips-gateway']) +fips W: dependency-implicitly-satisfied bash (programs-needed ['bash'] ['usr/lib/fips/fips-dns-setup', 'usr/lib/fips/fips-dns-teardown']) +fips W: dependency-not-needed gcc-libs +fips I: dependency-detected-satisfied glibc (libraries-needed ['usr/lib/ld-linux-x86-64.so.2', 'usr/lib/libc.so.6', 'usr/lib/libm.so.6'] ['usr/bin/fipsctl', 'usr/bin/fips-gateway', 'usr/bin/fipstop', 'usr/bin/fips']) +fips I: dependency-detected-satisfied dbus (libraries-needed ['usr/lib/libdbus-1.so.3'] ['usr/bin/fips']) +fips I: depends-by-namcap-sight depends=(nftables libgcc glibc dbus bash) +EOF +) + +# The same package rebuilt with dbus dropped from depends. +REAL_NODBUS=$(cat <<'EOF' +fips W: file-not-world-readable etc/fips/fips.yaml +fips I: script-link-detected nft in ['etc/fips/fips.nft'] +fips I: script-link-detected bash in ['usr/lib/fips/fips-dns-teardown', 'usr/lib/fips/fips-dns-setup'] +fips I: libdepends-missing-provides ld-linux-x86-64.so=2-64 glibc (['usr/bin/fips-gateway', 'usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop']) +fips I: libdepends-missing-provides libc.so=6-64 glibc (['usr/bin/fips-gateway', 'usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop']) +fips I: libdepends-missing-provides libm.so=6-64 glibc (['usr/bin/fips-gateway', 'usr/bin/fips', 'usr/bin/fipstop']) +fips I: link-level-dependence dbus in ['usr/lib/libdbus-1.so.3'] +fips I: link-level-dependence glibc in ['usr/lib/libc.so.6', 'usr/lib/ld-linux-x86-64.so.2', 'usr/lib/libm.so.6'] +fips I: link-level-dependence libgcc in ['usr/lib/libgcc_s.so.1'] +fips I: libdepends-detected-not-included libdbus-1.so=3-64 dbus (['usr/bin/fips']) +fips I: libdepends-detected-not-included libgcc_s.so=1-64 libgcc (['usr/bin/fips-gateway', 'usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop']) +fips I: libdepends-by-namcap-sight depends=(glibc libdbus-1.so=3-64 libgcc_s.so=1-64) +fips I: libprovides-by-namcap-sight provides=() +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fips +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fips-gateway +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fipsctl +fips W: unused-sodepend /usr/lib64/ld-linux-x86-64.so.2 usr/bin/fipstop +fips E: dependency-detected-not-included dbus (libraries-needed ['usr/lib/libdbus-1.so.3'] ['usr/bin/fips']) +fips E: dependency-detected-not-included bash (programs-needed ['bash'] ['usr/lib/fips/fips-dns-teardown', 'usr/lib/fips/fips-dns-setup']) +fips W: dependency-implicitly-satisfied libgcc (libraries-needed ['usr/lib/libgcc_s.so.1'] ['usr/bin/fips-gateway', 'usr/bin/fipsctl', 'usr/bin/fips', 'usr/bin/fipstop']) +fips W: dependency-detected-but-optional nftables (programs-needed ['nft'] ['etc/fips/fips.nft']) +fips W: dependency-not-needed gcc-libs +fips I: dependency-detected-satisfied glibc (libraries-needed ['usr/lib/libc.so.6', 'usr/lib/ld-linux-x86-64.so.2', 'usr/lib/libm.so.6'] ['usr/bin/fipsctl', 'usr/bin/fipstop', 'usr/bin/fips-gateway', 'usr/bin/fips']) +fips I: depends-by-namcap-sight depends=(dbus glibc libgcc nftables bash) +EOF +) + +# A toy package with nftables in neither depends nor optdepends and an nft +# script under etc/. +TOY_NONFT=$(cat <<'EOF' +fips W: elffile-without-relro usr/bin/fips +fips I: script-link-detected nft in ['etc/fips/fips.nft'] +fips I: libdepends-missing-provides libc.so=6-64 glibc (['usr/bin/fips']) +fips I: link-level-dependence dbus in ['usr/lib/libdbus-1.so.3'] +fips I: link-level-dependence glibc in ['usr/lib/libc.so.6'] +fips I: libdepends-detected-not-included libdbus-1.so=3-64 dbus (['usr/bin/fips']) +fips I: libdepends-by-namcap-sight depends=(glibc libdbus-1.so=3-64) +fips I: libprovides-by-namcap-sight provides=() +fips E: dependency-detected-not-included nftables (programs-needed ['nft'] ['etc/fips/fips.nft']) +fips I: dependency-detected-satisfied glibc (libraries-needed ['usr/lib/libc.so.6'] ['usr/bin/fips']) +fips I: dependency-detected-satisfied dbus (libraries-needed ['usr/lib/libdbus-1.so.3'] ['usr/bin/fips']) +fips I: depends-by-namcap-sight depends=(glibc nftables dbus) +EOF +) + +# Warning-level findings only, with the dependency-analysis line. +WARN_ONLY=$(cat <<'EOF' +fips W: dependency-detected-but-optional nftables (programs-needed ['nft'] ['etc/fips/fips.nft']) +fips I: depends-by-namcap-sight depends=(dbus glibc nftables) +EOF +) + +# A PKGBUILD without url or maintainer. +PKGBUILD_BAD=$(cat <<'EOF' +PKGBUILD (fips) W: missing-maintainer +PKGBUILD (fips) E: missing-url +PKGBUILD (fips) W: pkgname-in-description +EOF +) + +# The maint release PKGBUILD. +PKGBUILD_CLEAN=$(cat <<'EOF' +PKGBUILD (fips) I: missing-contributor +EOF +) + +# namcap given a file that does not exist; it exits 0. +UNREADABLE=$(cat <<'EOF' +Error: Problem reading nosuch.pkg.tar.zst +usage: python3 -m namcap [-h] [-L] [-i] [-m] [-t TAGS] [-e RULELIST | + -r RULELIST] [-v] + [packages ...] +Error: nosuch.pkg.tar.zst not package or PKGBUILD +EOF +) + +# --- canned cases ------------------------------------------------------------ + +# Run the cases against canned namcap output through the stub. +canned() { + local fx f a b + + mkdir -p "$WORK/bin" + cat > "$WORK/bin/namcap" <<'STUB' +#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$*" >> "$STUB_CALLS" +file="" +for a in "$@"; do + case "$a" in + -i|-m) ;; + -*) echo "stub namcap: unexpected option: $a" >&2; exit 2 ;; + *) + [ -z "$file" ] || { echo "stub namcap: more than one file: $*" >&2; exit 2; } + file=$a + ;; + esac +done +[ -n "$file" ] || { echo "stub namcap: no file given" >&2; exit 2; } +b=$(basename -- "$file") +if [ -f "$STUB_FIXTURE/$b.out" ]; then cat "$STUB_FIXTURE/$b.out"; fi +exit "$(cat "$STUB_FIXTURE/$b.rc" 2>/dev/null || echo 0)" +STUB + chmod +x "$WORK/bin/namcap" + STUBDIR="$WORK/bin:" + + local pkg=fips-0.5.1-1-x86_64.pkg.tar.zst + + # C1: the real package as declared has warnings only, and the gate asks + # namcap for informational lines and tag names. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C1); f=$(placeholder "$fx" "$pkg") + printf '%s\n' "$REAL_DECLARED" > "$fx/$pkg.out" + if check "C1 real package as declared passes" "$fx" zero '^namcap gate: passed' "$f"; then + grep -q -- "^-i -m $f\$" "$WORK/calls" || + fail "C1 real package as declared passes: namcap not called with -i -m: $(cat "$WORK/calls")" + fi + + # C2: the real package with dbus dropped from depends. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C2); f=$(placeholder "$fx" "$pkg") + printf '%s\n' "$REAL_NODBUS" > "$fx/$pkg.out" + check "C2 real package missing dbus fails" "$fx" nonzero \ + '^::error title=namcap::fips E: dependency-detected-not-included dbus ' "$f" || true + + # C3: a script interpreter's package declared nowhere. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C3); f=$(placeholder "$fx" "$pkg") + printf '%s\n' "$TOY_NONFT" > "$fx/$pkg.out" + check "C3 undeclared script dependency fails" "$fx" nonzero \ + '^::error title=namcap::fips E: dependency-detected-not-included nftables ' "$f" || true + + # C4: warnings are advisory. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C4); f=$(placeholder "$fx" "$pkg") + printf '%s\n' "$WARN_ONLY" > "$fx/$pkg.out" + check "C4 warnings only pass" "$fx" zero '^namcap gate: passed' "$f" || true + + # C5: an error-level finding on a PKGBUILD. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C5); f=$(placeholder "$fx" PKGBUILD) + printf '%s\n' "$PKGBUILD_BAD" > "$fx/PKGBUILD.out" + check "C5 PKGBUILD error fails" "$fx" nonzero \ + '^::error title=namcap::PKGBUILD \(fips\) E: missing-url' "$f" || true + + # C6: a clean PKGBUILD needs no dependency-analysis line. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C6); f=$(placeholder "$fx" PKGBUILD) + printf '%s\n' "$PKGBUILD_CLEAN" > "$fx/PKGBUILD.out" + check "C6 clean PKGBUILD passes" "$fx" zero '^namcap gate: passed' "$f" || true + + # C7: namcap could not read its input, printed an error and usage, and + # exited 0. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C7); f=$(placeholder "$fx" "$pkg") + printf '%s\n' "$UNREADABLE" > "$fx/$pkg.out" + check "C7 unreadable input fails" "$fx" nonzero \ + ': unrecognised namcap output: Error: Problem reading' "$f" || true + + # C8: a package for which namcap printed nothing. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C8); f=$(placeholder "$fx" "$pkg") + check "C8 empty output for a package fails" "$fx" nonzero \ + ': no dependency analysis in namcap output$' "$f" || true + + # C9: namcap exits nonzero on otherwise clean output. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C9); f=$(placeholder "$fx" "$pkg") + printf '%s\n' "$REAL_DECLARED" > "$fx/$pkg.out" + echo 1 > "$fx/$pkg.rc" + check "C9 namcap nonzero exit fails" "$fx" nonzero ': namcap exited 1$' "$f" || true + + # C10: the named file does not exist, as when a glob matched nothing. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C10) + if check "C10 missing file fails" "$fx" nonzero ': not found$' "$fx/files/*.pkg.tar.*"; then + [ "$(calls)" -eq 0 ] || + fail "C10 missing file fails: namcap was called $(calls) time(s), expected 0" + fi + + # C11: no files at all. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C11) + check "C11 no arguments fails" "$fx" nonzero '^usage: ' || true + + # C12: two packages, the error only in the second. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C12) + a=$(placeholder "$fx" a-1-1-x86_64.pkg.tar.zst); b=$(placeholder "$fx" b-1-1-x86_64.pkg.tar.zst) + printf '%s\n' "$REAL_DECLARED" > "$fx/a-1-1-x86_64.pkg.tar.zst.out" + printf '%s\n' "$REAL_NODBUS" > "$fx/b-1-1-x86_64.pkg.tar.zst.out" + if check "C12 error in the second of two packages fails" "$fx" nonzero \ + "^namcap gate: $b: 2 error-level finding" "$a" "$b"; then + [ "$(calls)" -eq 2 ] || + fail "C12 error in the second of two packages fails: namcap called $(calls) time(s), expected 2" + fi + + # C13: two packages, the error only in the first; the clean second must not + # overwrite the verdict. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C13) + a=$(placeholder "$fx" a-1-1-x86_64.pkg.tar.zst); b=$(placeholder "$fx" b-1-1-x86_64.pkg.tar.zst) + printf '%s\n' "$REAL_NODBUS" > "$fx/a-1-1-x86_64.pkg.tar.zst.out" + printf '%s\n' "$REAL_DECLARED" > "$fx/b-1-1-x86_64.pkg.tar.zst.out" + if check "C13 error in the first of two packages fails" "$fx" nonzero \ + "^namcap gate: $a: 2 error-level finding" "$a" "$b"; then + [ "$(calls)" -eq 2 ] || + fail "C13 error in the first of two packages fails: namcap called $(calls) time(s), expected 2" + fi + + # C14: " E: " inside a warning's text is not an error-level finding. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C14); f=$(placeholder "$fx" "$pkg") + { printf '%s\n' "$REAL_DECLARED"; echo "fips W: some-tag text ' E: ' inside"; } > "$fx/$pkg.out" + check "C14 E: inside a warning's text passes" "$fx" zero '^namcap gate: passed' "$f" || true + + # C15: a file that is neither a PKGBUILD nor a package. + CASES_DEFINED=$((CASES_DEFINED + 1)) + fx=$(fixture C15); f=$(placeholder "$fx" fips.tar.gz) + if check "C15 unknown file shape fails" "$fx" nonzero ': not a PKGBUILD or package$' "$f"; then + [ "$(calls)" -eq 0 ] || + fail "C15 unknown file shape fails: namcap was called $(calls) time(s), expected 0" + fi +} + +# --- live cases -------------------------------------------------------------- + +# Write a toy package's PKGBUILD and sources into a directory, build it with +# makepkg, and print the built package's path. The binary links libdbus; the +# nft script under etc/ needs nftables' interpreter. +# Args: dir depends optdepends +toypkg() { + local dir=$1 pkgs + mkdir -p "$dir" + cat > "$dir/probe.c" <<'EOF' +/* Calls one libdbus symbol so the binary carries NEEDED libdbus-1.so.3. */ +extern void *dbus_message_new(int message_type); +int main(void) { return dbus_message_new(1) == 0; } +EOF + printf '#!/usr/sbin/nft -f\nflush ruleset\n' > "$dir/probe.nft" + cat > "$dir/PKGBUILD" < +pkgname=namcap-probe +pkgver=1 +pkgrel=1 +pkgdesc="Toy package for the namcap gate test" +url="https://example.invalid" +license=('MIT') +arch=('x86_64') +depends=($2) +optdepends=($3) +options=('!debug') +source=("probe.c" "probe.nft") +b2sums=('SKIP' 'SKIP') +build() { gcc -O2 -o namcap-probe probe.c -Wl,--no-as-needed -ldbus-1; } +package() { + install -Dm0755 namcap-probe "\$pkgdir/usr/bin/namcap-probe" + install -Dm0644 /dev/null "\$pkgdir/usr/share/licenses/namcap-probe/LICENSE" + install -Dm0644 probe.nft "\$pkgdir/etc/namcap-probe/probe.nft" +} +EOF + (cd "$dir" && makepkg -f -d --noconfirm) > "$dir/makepkg.log" 2>&1 || return 1 + pkgs=("$dir"/*.pkg.tar.*) + [ -f "${pkgs[0]}" ] || return 1 + echo "${pkgs[0]}" +} + +# Build one toy package and run the gate on it with the real namcap. A build +# failure fails the case and prints the build log; it is never a skip. +# Args: name expect pattern depends optdepends +livecase() { + local name=$1 expect=$2 pattern=$3 dir pkg + dir="$WORK/live/${name%% *}" + CASES_DEFINED=$((CASES_DEFINED + 1)) + if ! pkg=$(toypkg "$dir" "$4" "$5"); then + CASES_RAN=$((CASES_RAN + 1)) + fail "$name: toy package did not build" + sed 's/^/ /' "$dir/makepkg.log" 2>/dev/null || true + return 0 + fi + check "$name" "$dir" "$expect" "$pattern" "$pkg" || true +} + +# Run the cases that build toy packages and lint them with the real namcap. +live() { + local missing + if [ "$(id -u)" -eq 0 ]; then + echo "test-namcap-gate.sh --live: run as a non-root user; makepkg refuses root" >&2 + exit 2 + fi + if ! missing=$(pacman -Q dbus nftables 2>&1); then + echo "FAIL live cases need dbus and nftables installed, as namcap looks up" + echo " script and library owners in the local package database:" + printf '%s\n' "$missing" | sed 's/^/ /' + exit 1 + fi + + livecase "L1 correctly declared toy package passes" zero '^namcap gate: passed' \ + "'dbus' 'glibc'" "'nftables: ruleset'" + livecase "L2 toy package missing dbus fails" nonzero \ + '^::error title=namcap::namcap-probe E: dependency-detected-not-included dbus ' \ + "'glibc'" "'nftables: ruleset'" + livecase "L3 toy package missing nftables fails" nonzero \ + '^::error title=namcap::namcap-probe E: dependency-detected-not-included nftables ' \ + "'dbus' 'glibc'" "" +} + +# ----------------------------------------------------------------------------- + +if [ "$MODE" = live ]; then live; else canned; fi + +echo "cases defined: $CASES_DEFINED, ran: $CASES_RAN, failed: $FAILED" +if [ "$CASES_RAN" -ne "$CASES_DEFINED" ]; then + echo "FAIL: not every defined case ran" + exit 1 +fi +[ "$FAILED" -eq 0 ] From 2ca7bd22f4859446e57a05e41044cbd027484f40 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 19:00:04 +0000 Subject: [PATCH 08/14] Update the changelog for the bloom filter announce resend and the runtime peer list alias fix --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6e6a245..9d169546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 cleanly, and a peer whose send failed is retried after a shorter fixed interval instead. That retry interval gates only a peer whose last attempt failed, so it cannot clamp a `heartbeat_interval_secs` configured below it. +- Replacing the peer list at runtime with `Node::update_peers` now updates + everything that reads peer aliases. `.fips` names, peer ACL entries written as + an alias, and peer display names kept following the aliases the node started + with, so a new peer's alias did not resolve, a removed one still did, and a + deny entry naming an alias moved to another key kept denying the old key and + admitted the new one. They now follow the new peer list, with the hosts file + still taking precedence as it does at startup. #### Routing and discovery @@ -129,6 +136,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 child's stayed advertised, until some unrelated change. The re-announce fires only when that relation flips and only to peers whose filter actually changed, so ordinary tree churn does not multiply announce traffic. +- A bloom filter announce lost on the link is now resent. A node counted an + announce as delivered once the transport accepted it, and announces go out + only when a filter changes, so a dropped datagram or a link outage shorter + than the dead timeout left the peer holding the old filter until something + else changed, and destinations could stay missing from discovery. The node + now confirms each announce from the link's existing receiver reports, + resends when they show a loss, and resends once after 30 seconds when the + reports cannot confirm it. Resends over one peer connection are limited to + six a minute, and to one a minute while losses persist. #### Session setup From 9462d5d1254b2be29d2d18b52cdc8c265267bee7 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 23 Sep 2026 04:03:32 +0000 Subject: [PATCH 09/14] Restart fips and the gateway when an apk upgrade replaces them apk-tools v3 runs only the incoming package's pre-upgrade and post-upgrade scripts on an upgrade, and the .apk registered neither. An upgrade replaced the binaries and init scripts on disk but left procd running the old fips and fips-gateway processes until a reboot or a manual restart. The .apk now registers pre-upgrade and post-upgrade as thin wrappers around the same prerm and postinst bodies the .ipk ships. One header line gives each body the opkg upgrade contract it already handles: pre-upgrade rewrites apk's " " arguments to "upgrade ", so prerm stops both services without disabling them and leaves its marker, and post-upgrade exports PKG_UPGRADE=1, as OpenWrt's own package-pack.mk does, so postinst starts fips and starts the gateway only if it was enabled. Registering post-upgrade alone would not have been enough: procd ignores a start of a running instance whose command line is unchanged, so the services have to be stopped first. testing/openwrt/package-test.sh runs the real build-apk.sh on the host against a stub apk and checks the registered phases, the #! lines, that the install and removal scripts are the shipped bodies, and, by executing each wrapper's header with apk's argv and environment, that the upgrade pair hands the bodies the right arguments. The ash harness runs it first and then runs the captured scripts in three new apk scenarios, and the packaging workflow's apk structural check now requires all four scripts in the adbdump. An upgrade onto a package built this way was run on OpenWrt 25.12.2 with its apk-tools 3.0.5: apk ran both pre-upgrade and post-upgrade, and both came from the incoming package. The adbdump key format the workflow check matches, each script as a ":" key under scripts:, was read from the apk-tools v3.0.5 source (src/serialize_yaml.c), the tag the packaging workflow builds from source. It matches the dump that source-built 3.0.5 printed for this change in the packaging workflow, where all four scripts appeared under scripts: and passed the check on both architectures. OpenWrt's own apk-tools 3.0.5 is built without mkpkg, and on a router adbdump cannot read the installed database and info has no --scripts, so which scripts an installed package registered cannot be read back on a device. The check covers the built package only. --- .github/workflows/package-openwrt.yml | 11 + packaging/openwrt-apk/README.md | 15 ++ packaging/openwrt-apk/build-apk.sh | 51 ++++- packaging/openwrt-ipk/scripts/postinst | 8 +- packaging/openwrt-ipk/scripts/prerm | 3 +- testing/openwrt/maintainer-scripts-test.sh | 14 ++ testing/openwrt/package-test.sh | 240 +++++++++++++++++++++ testing/openwrt/scenarios.sh | 117 ++++++++++ 8 files changed, 449 insertions(+), 10 deletions(-) create mode 100755 testing/openwrt/package-test.sh diff --git a/.github/workflows/package-openwrt.yml b/.github/workflows/package-openwrt.yml index d8d79fe0..dfd80513 100644 --- a/.github/workflows/package-openwrt.yml +++ b/.github/workflows/package-openwrt.yml @@ -841,6 +841,17 @@ jobs: fi done + # Maintainer scripts. adbdump prints each registered script as a + # ":" key under scripts:. An upgrade runs only pre-upgrade and + # post-upgrade, so a package missing either restarts nothing. + for s in post-install pre-upgrade post-upgrade pre-deinstall; do + if grep -qE "^[[:space:]]*${s}:" "$DUMP"; then + echo " PASS script: $s" + else + echo " FAIL script: missing $s"; fail=1 + fi + done + if [ "$fail" -ne 0 ]; then echo "apk structural verification FAILED" exit 1 diff --git a/packaging/openwrt-apk/README.md b/packaging/openwrt-apk/README.md index d7eec73e..26383661 100644 --- a/packaging/openwrt-apk/README.md +++ b/packaging/openwrt-apk/README.md @@ -94,6 +94,21 @@ key; a single `--allow-untrusted` package install does not. If we ever publish a apk feed, add ECDSA (prime256v1) signing via `apk mkpkg --sign` and distribute the public key to `/etc/apk/keys/`. +## Upgrading + +Upgrade with the same command, pointed at the new package: + +```bash +ssh root@192.168.1.1 apk add --allow-untrusted /tmp/fips__.apk +``` + +The new package's upgrade scripts stop `fips` and `fips-gateway` before the +files are replaced, then start `fips` again and start `fips-gateway` only if it +was enabled, so the upgrade keeps the gateway's enabled state. apk runs +the incoming package's upgrade scripts, not the installed one's, so this holds +from the first upgrade onto a package that carries them, whatever version is +installed. + `/etc/fips/fips.yaml` is marked as a config file (via `/lib/apk/packages/fips.conffiles`), so apk preserves local edits across upgrades, and `/lib/upgrade/keep.d/fips` preserves `/etc/fips/` across `sysupgrade` — the diff --git a/packaging/openwrt-apk/build-apk.sh b/packaging/openwrt-apk/build-apk.sh index cfe18a60..89063850 100755 --- a/packaging/openwrt-apk/build-apk.sh +++ b/packaging/openwrt-apk/build-apk.sh @@ -228,16 +228,53 @@ cat > "$STAGE_DIR/lib/apk/packages/${PKG_NAME}.conffiles" <<'EOF' EOF # ---- maintainer scripts ---- -# Map our opkg maintainer scripts onto apk's lifecycle phases: -# opkg postinst -> apk post-install (enable + start the daemon) -# opkg prerm -> apk pre-deinstall (stop + disable services) - # Both bodies come from packaging/openwrt-ipk/scripts/, the same files the # .ipk ships, so the two packagers cannot drift apart and testing/openwrt/ -# exercises what both install. apk runs post-install only on a fresh install, -# so the postinst's upgrade branch is unreachable here. +# exercises what both install. apk-tools v3 runs a different script on each +# path, and only ever the incoming package's: +# +# fresh install post-install = postinst as shipped (enable + start fips; +# the gateway stays off) +# upgrade pre-upgrade = prerm, told it is an opkg-style upgrade; +# runs before any file is replaced +# post-upgrade = postinst with PKG_UPGRADE=1; runs after +# removal pre-deinstall = prerm as shipped (stop + disable both) +# +# On an upgrade apk passes " " and a PATH-only +# environment, which is not the contract the bodies were written for: prerm +# would read the new version as "not an upgrade" and disable the gateway, and +# postinst would see no PKG_UPGRADE. The upgrade pair therefore gets one header +# line that restores opkg's contract, "upgrade " for prerm and +# PKG_UPGRADE=1 for postinst (OpenWrt's own package-pack.mk builds its +# post-upgrade scripts the same way). +# +# Registering post-upgrade alone would not restart anything: procd treats a +# start of a running instance with an unchanged command line as a no-op, so +# the old binaries would keep running until a reboot. pre-upgrade stops both +# services first, which also means a failed extraction leaves them stopped, +# as an opkg upgrade already does. + +wrap_script() { + # wrap_script + # Writes to with inserted after its #! line. + local header="$1" src="$2" dst="$3" + if [ "$(head -n 1 "$src")" != "#!/bin/sh" ]; then + echo "Error: $src does not start with #!/bin/sh; cannot wrap it." >&2 + exit 1 + fi + { + echo "#!/bin/sh" + echo "$header" + tail -n +2 "$src" + } > "$dst" + chmod 0755 "$dst" +} + install -m 0755 "$SCRIPTS_SRC/postinst" "$SCRIPTS_DIR/post-install" install -m 0755 "$SCRIPTS_SRC/prerm" "$SCRIPTS_DIR/pre-deinstall" +# shellcheck disable=SC2016 # $1 is expanded by the script at run time +wrap_script 'set -- upgrade "$1"' "$SCRIPTS_SRC/prerm" "$SCRIPTS_DIR/pre-upgrade" +wrap_script 'export PKG_UPGRADE=1' "$SCRIPTS_SRC/postinst" "$SCRIPTS_DIR/post-upgrade" # --------------------------------------------------------------------------- # 3. Assemble the .apk via apk mkpkg @@ -268,6 +305,8 @@ $FAKEROOT "$APK_BIN" mkpkg \ --info "maintainer:FIPS Network" \ --info "depends:$DEPENDS" \ --script "post-install:$SCRIPTS_DIR/post-install" \ + --script "pre-upgrade:$SCRIPTS_DIR/pre-upgrade" \ + --script "post-upgrade:$SCRIPTS_DIR/post-upgrade" \ --script "pre-deinstall:$SCRIPTS_DIR/pre-deinstall" \ --files "$STAGE_DIR" \ --output "$DIST_DIR/$PKG_FILENAME" diff --git a/packaging/openwrt-ipk/scripts/postinst b/packaging/openwrt-ipk/scripts/postinst index 3e505e6d..18361729 100755 --- a/packaging/openwrt-ipk/scripts/postinst +++ b/packaging/openwrt-ipk/scripts/postinst @@ -2,7 +2,8 @@ # Maintainer script run after the FIPS package is unpacked. # # Installed as the .ipk CONTROL/postinst and registered as the .apk -# post-install script, so one body serves both packagers. +# post-install script, and as the .apk post-upgrade script with PKG_UPGRADE=1 +# exported ahead of this body, so one body serves both packagers. # # The fips daemon is enabled and started on every install. The gateway is not: # the package ships that service disabled, and the README and the deployment @@ -19,8 +20,9 @@ # re-enabled, which also re-enables one an operator had # disabled by hand. # -# Under apk this script runs only on a fresh install, so it takes the first -# branch and the gateway stays off. +# Under apk, a fresh install runs this as post-install and the gateway stays +# off. An upgrade runs it as post-upgrade, after the .apk pre-upgrade script +# (the prerm body) has stopped the services and left the marker. UPGRADE_MARKER=/tmp/fips-prerm-upgrade diff --git a/packaging/openwrt-ipk/scripts/prerm b/packaging/openwrt-ipk/scripts/prerm index f1db5153..09dcf365 100755 --- a/packaging/openwrt-ipk/scripts/prerm +++ b/packaging/openwrt-ipk/scripts/prerm @@ -2,7 +2,8 @@ # Maintainer script run before the FIPS package is removed or replaced. # # Installed as the .ipk CONTROL/prerm and registered as the .apk pre-deinstall -# script, so one body serves both packagers. +# script, and as the .apk pre-upgrade script with its arguments rewritten to +# "upgrade ", so one body serves both packagers. # # opkg calls this with "upgrade " when the package is being # replaced. Disabling the services there would erase the operator's choice, diff --git a/testing/openwrt/maintainer-scripts-test.sh b/testing/openwrt/maintainer-scripts-test.sh index 8a8d4eea..e31b8d22 100755 --- a/testing/openwrt/maintainer-scripts-test.sh +++ b/testing/openwrt/maintainer-scripts-test.sh @@ -31,9 +31,23 @@ if [[ ! -f "$SCRIPT_DIR/scenarios.sh" ]]; then exit 2 fi +# The .apk wraps the shared bodies for its upgrade path. package-test.sh builds +# the package on the host with the real build-apk.sh, checks what it registers, +# and leaves the four scripts here so the scenarios run exactly what ships. +APK_DIR="$(mktemp -d)" || { echo "openwrt-scripts: mktemp failed" >&2; exit 2; } +trap 'rm -rf "$APK_DIR"' EXIT +bash "$SCRIPT_DIR/package-test.sh" --keep "$APK_DIR" +rc=$? +if [[ $rc -ne 0 ]]; then + echo "openwrt-scripts: package-test.sh exited $rc" >&2 + exit $rc +fi + docker run --rm --network none \ -v "$PROJECT_ROOT:/src:ro" \ + -v "$APK_DIR:/apk:ro" \ -e REPO=/src \ + -e APK_SCRIPTS=/apk \ -e "POSTINST=${POSTINST:-}" \ -e "PRERM=${PRERM:-}" \ "$IMAGE" sh /src/testing/openwrt/scenarios.sh diff --git a/testing/openwrt/package-test.sh b/testing/openwrt/package-test.sh new file mode 100755 index 00000000..c15ecdfa --- /dev/null +++ b/testing/openwrt/package-test.sh @@ -0,0 +1,240 @@ +#!/bin/bash +# ── OpenWrt package contents, checked on the host ─────────────────────────── +# Runs the real packaging/openwrt-apk/build-apk.sh against placeholder +# binaries and a stub `apk` that records what `apk mkpkg` was asked to +# package. No docker, no apk-tools and no FIPS build are needed. +# +# What it checks is which maintainer scripts the .apk registers and what each +# one does with apk's upgrade arguments and environment (apk-tools v3 passes +# " " and a PATH-only environment to pre-upgrade and +# post-upgrade). Whether a real `apk mkpkg` accepts the result is left to the +# GitHub packaging workflow, which builds with the real tool. +# +# Usage: package-test.sh [--keep ] +# --keep copy the captured apk scripts into as post-install, +# pre-upgrade, post-upgrade and pre-deinstall, so +# scenarios.sh can run them under ash. +# +# Exit 0 = every check passed. Exit 1 = at least one failed. Exit 2 = the +# harness could not run; never treated as a pass. +# ───────────────────────────────────────────────────────────────────────────── +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +SCRIPTS_SRC="$PROJECT_ROOT/packaging/openwrt-ipk/scripts" + +KEEP="" +while [[ $# -gt 0 ]]; do + case "$1" in + --keep) + [[ $# -ge 2 ]] || { echo "package-test: --keep needs a directory" >&2; exit 2; } + KEEP="$2" + shift 2 + ;; + *) echo "package-test: unknown argument: $1" >&2; exit 2 ;; + esac +done +if [[ -n "$KEEP" && ! -d "$KEEP" ]]; then + echo "package-test: --keep needs an existing directory" >&2 + exit 2 +fi + +# The version is unique to this run so the cleanup below removes only what +# this run's builders wrote into dist/. +PKG_VERSION="pkgtest.$$" +TMP="$(mktemp -d)" || { echo "package-test: mktemp failed" >&2; exit 2; } + +trap 'rm -rf "$TMP"; rm -f "$PROJECT_ROOT/dist/fips_${PKG_VERSION}_"*' EXIT + +harness_fail() { + echo "package-test: $*" >&2 + exit 2 +} + +FAILURES=0 +CASES=0 + +ok() { + CASES=$((CASES + 1)) + echo " ok $*" + return 0 +} + +bad() { + CASES=$((CASES + 1)) + FAILURES=$((FAILURES + 1)) + echo " FAIL $*" + return 0 +} + +# ── Build the .apk against a stub apk ─────────────────────────────────────── + +BINS="$TMP/bins" +CAPTURE="$TMP/capture" +mkdir -p "$BINS" "$CAPTURE" || harness_fail "cannot create $TMP subdirectories" +for bin in fips fipsctl fipstop fips-gateway; do + printf 'x' > "$BINS/$bin" || harness_fail "cannot write placeholder $bin" +done + +# The stub knows only the arguments build-apk.sh passes today. Anything else +# exits 64, so a new mkpkg argument fails the build rather than going unseen. +cat > "$TMP/apk" <&2; exit 64; } +shift +files="" +out="" +while [ \$# -gt 0 ]; do + [ \$# -ge 2 ] || { echo "stub apk: \$1 has no value" >&2; exit 64; } + case "\$1" in + --info) ;; + --script) + phase="\${2%%:*}" + cp "\${2#*:}" "\$cap/script.\$phase" + echo "\$phase" >> "\$cap/phases" + ;; + --files) files="\$2" ;; + --output) out="\$2" ;; + *) echo "stub apk: unknown argument \$1" >&2; exit 64 ;; + esac + shift 2 +done +[ -n "\$files" ] && [ -n "\$out" ] || { echo "stub apk: --files and --output are required" >&2; exit 64; } +(cd "\$files" && find . -mindepth 1 | LC_ALL=C sort) > "\$cap/payload" +: > "\$out" +: > "\$cap/called" +STUB +chmod 0755 "$TMP/apk" || harness_fail "cannot make the stub apk executable" + +echo "OpenWrt package checks" +echo "==> build-apk.sh with a stub apk" +if ! PKG_VERSION="$PKG_VERSION" APK_VERSION=0.0.0-r0 APK_BIN="$TMP/apk" \ + bash "$PROJECT_ROOT/packaging/openwrt-apk/build-apk.sh" --arch x86_64 --bin-dir "$BINS" \ + > "$TMP/build-apk.log" 2>&1; then + cat "$TMP/build-apk.log" >&2 + harness_fail "build-apk.sh failed, so nothing was checked" +fi +[[ -f "$CAPTURE/called" ]] || harness_fail "build-apk.sh exited 0 but never ran apk mkpkg" +[[ -f "$CAPTURE/phases" ]] || harness_fail "apk mkpkg ran but no script phase was recorded" + +# ── A1. The .apk registers exactly the four lifecycle scripts ─────────────── +# apk runs only post-install on a fresh install, only pre-upgrade and +# post-upgrade on an upgrade, and only pre-deinstall on a removal. +WANT_PHASES="post-install post-upgrade pre-deinstall pre-upgrade" +got_phases="$(LC_ALL=C sort "$CAPTURE/phases" | tr '\n' ' ')" +got_phases="${got_phases% }" +if [[ "$got_phases" == "$WANT_PHASES" ]]; then + ok "A1 the .apk registers $WANT_PHASES" +else + missing="" + for phase in $WANT_PHASES; do + grep -qxF "$phase" "$CAPTURE/phases" || missing="$missing $phase" + done + bad "A1 registered phases are '$got_phases', want '$WANT_PHASES'; missing:${missing:- none}" +fi + +# ── A2. Every registered script starts with a working #! line ─────────────── +# apk execs the script directly, so the kernel reads line 1. +for phase in $WANT_PHASES; do + script="$CAPTURE/script.$phase" + if [[ ! -f "$script" ]]; then + bad "A2 $phase is not registered, so it has no #! line" + elif [[ "$(head -n 1 "$script")" == "#!/bin/sh" ]]; then + ok "A2 $phase starts with #!/bin/sh" + else + bad "A2 $phase starts with '$(head -n 1 "$script")', not #!/bin/sh" + fi +done + +# ── A3. Install and removal ship the shared bodies unchanged ──────────────── +for pair in post-install:postinst pre-deinstall:prerm; do + phase="${pair%%:*}" + src="$SCRIPTS_SRC/${pair#*:}" + if cmp -s "$CAPTURE/script.$phase" "$src"; then + ok "A3 $phase is the shipped ${pair#*:}, byte for byte" + else + bad "A3 $phase differs from the shipped ${pair#*:}" + fi +done + +# ── A4 and A5. The upgrade pair wraps the shared bodies ───────────────────── +# A4 reads the structure: the script ends with the body after its #! line, and +# at least one header line sits between the two. A5 runs the header with apk's +# upgrade argv and environment, so what is judged is what the shell does with +# it, not how it reads. +for pair in pre-upgrade:prerm post-upgrade:postinst; do + phase="${pair%%:*}" + src="$SCRIPTS_SRC/${pair#*:}" + script="$CAPTURE/script.$phase" + if [[ ! -f "$script" ]]; then + bad "A4 $phase is not registered" + bad "A5 $phase is not registered, so its header cannot run" + continue + fi + + tail -n +2 "$src" > "$TMP/body" || harness_fail "cannot read $src" + body_lines=$(wc -l < "$TMP/body") + script_lines=$(wc -l < "$script") + header_lines=$((script_lines - body_lines)) + if [[ $header_lines -lt 2 ]]; then + bad "A4 $phase has $header_lines header line(s) before the ${pair#*:} body, want at least 2" + elif ! tail -n "$body_lines" "$script" | cmp -s - "$TMP/body"; then + bad "A4 $phase does not end with the ${pair#*:} body" + else + ok "A4 $phase is a $header_lines-line header and the ${pair#*:} body" + fi + + if [[ $header_lines -lt 1 ]]; then + bad "A5 $phase has no header to run" + continue + fi + probe="$TMP/probe.$phase" + { + head -n "$header_lines" "$script" + # shellcheck disable=SC2016 + printf '%s\n' 'printf '\''%s|%s|%s\n'\'' "${1-}" "${2-}" "${PKG_UPGRADE-}"' + } > "$probe" + chmod 0755 "$probe" || harness_fail "cannot make the $phase probe executable" + got="$(env -i PATH=/usr/sbin:/usr/bin:/sbin:/bin "$probe" 0.6.0-r1 0.5.2-r1)" + rc=$? + if [[ $rc -eq 126 || $rc -eq 127 ]]; then + harness_fail "the $phase probe could not be executed (exit $rc)" + fi + IFS='|' read -r f1 f2 f3 <<< "$got" + case "$phase" in + pre-upgrade) + if [[ $rc -eq 0 && "$f1|$f2" == "upgrade|0.6.0-r1" ]]; then + ok "A5 pre-upgrade hands the body 'upgrade 0.6.0-r1'" + else + bad "A5 pre-upgrade hands the body '$f1 $f2' (exit $rc), want 'upgrade 0.6.0-r1'" + fi + ;; + post-upgrade) + if [[ $rc -eq 0 && "$f3" == "1" ]]; then + ok "A5 post-upgrade runs the body with PKG_UPGRADE=1" + else + bad "A5 post-upgrade runs the body with PKG_UPGRADE='$f3' (exit $rc), want 1" + fi + ;; + esac +done + +# ── Hand the scripts to the ash scenarios ─────────────────────────────────── +if [[ -n "$KEEP" ]]; then + for phase in $WANT_PHASES; do + [[ -f "$CAPTURE/script.$phase" ]] || continue + install -m 0755 "$CAPTURE/script.$phase" "$KEEP/$phase" \ + || harness_fail "cannot copy $phase into $KEEP" + done +fi + +echo "" +if [[ $FAILURES -eq 0 ]]; then + echo "package-test: all $CASES checks passed" + exit 0 +fi +echo "package-test: $FAILURES of $CASES checks failed" +exit 1 diff --git a/testing/openwrt/scenarios.sh b/testing/openwrt/scenarios.sh index b060d0b1..6c7f4007 100755 --- a/testing/openwrt/scenarios.sh +++ b/testing/openwrt/scenarios.sh @@ -11,6 +11,13 @@ # POSTINST and PRERM may be pointed at other files. That is the seam used to # see a scenario red against the previously released scripts, and to re-break # the fixed ones during a break-check. +# +# APK_SCRIPTS names a directory holding the four scripts the .apk registers +# (post-install, pre-upgrade, post-upgrade, pre-deinstall), as captured from +# the real build-apk.sh by package-test.sh --keep. Scenarios 8 to 10 execute +# them directly with apk-tools v3's argv and PATH-only environment, so the +# kernel reads their #! line and ash runs them. A missing directory or script +# fails those scenarios; it is never a skip. set -u @@ -19,6 +26,7 @@ POSTINST="${POSTINST:-$REPO/packaging/openwrt-ipk/scripts/postinst}" PRERM="${PRERM:-$REPO/packaging/openwrt-ipk/scripts/prerm}" RELEASED_PRERM="$REPO/testing/openwrt/fixtures/released-prerm" INIT_GATEWAY="$REPO/packaging/openwrt-ipk/files/etc/init.d/fips-gateway" +APK_SCRIPTS="${APK_SCRIPTS:-}" SHIPPED_YAML="$REPO/packaging/openwrt-ipk/files/etc/fips/fips.yaml" WORK=/tmp/fips-openwrt-scenarios @@ -146,6 +154,52 @@ assert_absent() { return 0 } +assert_present() { + if [ -e "$1" ]; then + ok "$2" + else + bad "$2 — $1 does not exist" + fi + return 0 +} + +first_call_line() { + grep -nxF "$1" "$CALLS" | head -n 1 | cut -d: -f1 + return 0 +} + +assert_order() { + # assert_order + first="$(first_call_line "$1")" + second="$(first_call_line "$2")" + if [ -z "$first" ] || [ -z "$second" ]; then + bad "$3 — '$1' and '$2' were not both called: $(calls_oneline)" + elif [ "$first" -lt "$second" ]; then + ok "$3" + else + bad "$3 — '$2' came before '$1': $(calls_oneline)" + fi + return 0 +} + +run_apk_script() { + # run_apk_script + # Runs one captured .apk script the way apk-tools v3 does: executed + # directly, with only PATH in the environment. The stubs' own state + # variables are passed through so they can record the calls. + phase="$1" + shift + script="$APK_SCRIPTS/$phase" + if [ -z "$APK_SCRIPTS" ] || [ ! -x "$script" ]; then + bad "the .apk $phase script is not available at '$script'" + return 1 + fi + env -i PATH=/usr/sbin:/usr/bin:/sbin:/bin \ + CALLS="$CALLS" GW_STATE="$GW_STATE" FIPS_STATE="$FIPS_STATE" \ + "$script" "$@" >/dev/null 2>&1 + return 0 +} + # ── 1. Fresh install ──────────────────────────────────────────────────────── # opkg runs the postinst with "configure"; PKG_UPGRADE is set only on upgrades, # so both its absence and an explicit 0 must leave the gateway alone. @@ -321,9 +375,69 @@ YAML return 0 } +# ── 8. apk fresh install ──────────────────────────────────────────────────── +# apk-tools v3 runs only post-install, with the new version as its argument. +scenario_apk_fresh_install() { + note "scenario 8: apk fresh install" + reset_state + + run_apk_script post-install 0.6.0-r1 || return 0 + + assert_called "fips enable" "an apk install enables the daemon" + assert_called "fips start" "an apk install starts the daemon" + assert_not_called "fips-gateway enable" "an apk install does not enable the gateway" + assert_not_called "fips-gateway start" "an apk install does not start the gateway" + assert_file_is "$GW_STATE" "0" "an apk install leaves the gateway disabled" + return 0 +} + +# ── 9. apk upgrade, gateway enabled ───────────────────────────────────────── +# apk-tools v3 runs only the new package's pre-upgrade and post-upgrade, with +# " "; the old package runs nothing. +scenario_apk_upgrade_enabled() { + note "scenario 9: apk upgrade, gateway enabled" + reset_state + echo 1 > "$GW_STATE" + echo 1 > "$FIPS_STATE" + + run_apk_script pre-upgrade 0.6.0-r1 0.5.2-r1 || return 0 + assert_called "fips-gateway stop" "pre-upgrade stops the gateway" + assert_called "fips stop" "pre-upgrade stops the daemon" + assert_not_called "fips-gateway disable" "pre-upgrade does not disable the gateway" + assert_not_called "fips disable" "pre-upgrade does not disable the daemon" + assert_file_is "$GW_STATE" "1" "the gateway is still enabled after pre-upgrade" + assert_present "$UPGRADE_MARKER" "pre-upgrade leaves the upgrade marker" + + run_apk_script post-upgrade 0.6.0-r1 0.5.2-r1 || return 0 + assert_order "fips stop" "fips start" "the daemon is started again after it was stopped" + assert_order "fips-gateway stop" "fips-gateway start" "the gateway is started again after it was stopped" + assert_not_called "fips-gateway enable" "an enabled gateway does not need re-enabling" + assert_file_is "$GW_STATE" "1" "the gateway stays enabled across the apk upgrade" + assert_absent "$UPGRADE_MARKER" "post-upgrade removes the upgrade marker" + return 0 +} + +# ── 10. apk upgrade, gateway disabled ─────────────────────────────────────── +scenario_apk_upgrade_disabled() { + note "scenario 10: apk upgrade, gateway disabled" + reset_state + echo 1 > "$FIPS_STATE" + + run_apk_script pre-upgrade 0.6.0-r1 0.5.2-r1 || return 0 + run_apk_script post-upgrade 0.6.0-r1 0.5.2-r1 || return 0 + + assert_order "fips stop" "fips start" "the daemon is started again after it was stopped" + assert_file_is "$GW_STATE" "0" "a disabled gateway stays disabled across the apk upgrade" + assert_not_called "fips-gateway enable" "a disabled gateway is not enabled by the apk upgrade" + assert_not_called "fips-gateway start" "a disabled gateway is not started by the apk upgrade" + assert_absent "$UPGRADE_MARKER" "post-upgrade removes the upgrade marker" + return 0 +} + echo "OpenWrt maintainer-script scenarios (shell: $(readlink -f /proc/$$/exe 2>/dev/null || echo sh))" echo " postinst: $POSTINST" echo " prerm: $PRERM" +echo " apk: ${APK_SCRIPTS:-(not set)}" scenario_fresh_install scenario_upgrade_from_released @@ -332,6 +446,9 @@ scenario_upgrade_disabled scenario_removal scenario_config_reader scenario_start_service_guard +scenario_apk_fresh_install +scenario_apk_upgrade_enabled +scenario_apk_upgrade_disabled echo "" if [ "$FAILURES" -eq 0 ]; then From 72159a911d0e40457c83480bfdb70a895a4a36ff Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 23 Sep 2026 04:07:36 +0000 Subject: [PATCH 10/14] Stop shipping the OpenWrt dnsmasq drop-in nothing reads Both OpenWrt packages and the SDK feed Makefile installed /etc/dnsmasq.d/fips.conf. As shipped it forwarded .fips to 127.0.0.1#5354, an address the daemon's DNS responder does not listen on, since it binds ::1. On a device that has ever run the gateway the file says something else: the gateway init script rewrote its server line to ::1#5353 on every start and left the comments above it naming 127.0.0.1:5354. Neither version was ever read, because OpenWrt's dnsmasq init script builds its config from UCI and never loads that directory. What forwards .fips is the UCI server entry. 90-fips-setup adds it pointing at the daemon on ::1#5354, and the gateway init script points it at the gateway on ::1#5353 when the gateway starts and back at the daemon when it stops. This change leaves both writers of that entry alone; which port it should name while the gateway runs is a separate question. The drop-in is removed from the payload, the builders, the Makefile, the README and the packaging workflow's path lists. fips-gateway no longer rewrites it on every start and stop. The 90-fips-setup comment now says that the UCI entry is the forwarding path and that the gateway repoints it, and the shipped fips.yaml comment on the gateway, which said the fips init script configures that forwarding, now names the fips-gateway init script. An opkg upgrade removes the old file; an apk upgrade keeps it only if it was modified, as it is on a device that ran the gateway. Either way nothing reads it. package-test.sh now also builds the .ipk and checks that neither package installs anything under /etc/dnsmasq.d, with a positive control on files both packages must ship, and that no OpenWrt packaging file still names the drop-in, which is the only check covering the SDK Makefile. --- .github/workflows/package-openwrt.yml | 3 +- packaging/openwrt-apk/build-apk.sh | 3 - packaging/openwrt-ipk/Makefile | 4 - packaging/openwrt-ipk/README.md | 3 +- packaging/openwrt-ipk/build-ipk.sh | 3 - .../openwrt-ipk/files/etc/dnsmasq.d/fips.conf | 11 --- .../openwrt-ipk/files/etc/fips/fips.yaml | 3 +- .../openwrt-ipk/files/etc/init.d/fips-gateway | 5 -- .../files/etc/uci-defaults/90-fips-setup | 8 +- testing/openwrt/package-test.sh | 83 ++++++++++++++++++- 10 files changed, 90 insertions(+), 36 deletions(-) delete mode 100644 packaging/openwrt-ipk/files/etc/dnsmasq.d/fips.conf diff --git a/.github/workflows/package-openwrt.yml b/.github/workflows/package-openwrt.yml index dfd80513..e54e8597 100644 --- a/.github/workflows/package-openwrt.yml +++ b/.github/workflows/package-openwrt.yml @@ -489,7 +489,6 @@ jobs: ./etc/init.d/fips-gateway ./etc/fips/fips.yaml ./etc/fips/firewall.sh - ./etc/dnsmasq.d/fips.conf ./etc/sysctl.d/fips-gateway.conf ./etc/sysctl.d/fips-bridge.conf ./etc/hotplug.d/net/99-fips @@ -830,7 +829,7 @@ jobs: usr/bin/fips usr/bin/fipsctl usr/bin/fipstop usr/bin/fips-gateway \ usr/bin/fips-mesh-setup usr/bin/fips-ap-setup \ etc/init.d/fips etc/init.d/fips-gateway \ - etc/fips/fips.yaml etc/fips/firewall.sh etc/dnsmasq.d/fips.conf \ + etc/fips/fips.yaml etc/fips/firewall.sh \ etc/sysctl.d/fips-gateway.conf etc/sysctl.d/fips-bridge.conf \ etc/hotplug.d/net/99-fips etc/uci-defaults/90-fips-setup \ lib/upgrade/keep.d/fips; do diff --git a/packaging/openwrt-apk/build-apk.sh b/packaging/openwrt-apk/build-apk.sh index 89063850..399ac556 100755 --- a/packaging/openwrt-apk/build-apk.sh +++ b/packaging/openwrt-apk/build-apk.sh @@ -201,9 +201,6 @@ install -m 0755 "$FILES_DIR/etc/fips/firewall.sh" "$STAGE_DIR/etc/fips/firewall. # of the file; operators can still edit /etc/fips/fips.yaml for non-standard boards. sed -i 's|interface: "eth0"|interface: "wan"|' "$STAGE_DIR/etc/fips/fips.yaml" -install -d "$STAGE_DIR/etc/dnsmasq.d" -install -m 0644 "$FILES_DIR/etc/dnsmasq.d/fips.conf" "$STAGE_DIR/etc/dnsmasq.d/fips.conf" - install -d "$STAGE_DIR/etc/sysctl.d" install -m 0644 "$FILES_DIR/etc/sysctl.d/fips-bridge.conf" "$STAGE_DIR/etc/sysctl.d/fips-bridge.conf" install -m 0644 "$FILES_DIR/etc/sysctl.d/fips-gateway.conf" "$STAGE_DIR/etc/sysctl.d/fips-gateway.conf" diff --git a/packaging/openwrt-ipk/Makefile b/packaging/openwrt-ipk/Makefile index 6f354272..cabd0380 100644 --- a/packaging/openwrt-ipk/Makefile +++ b/packaging/openwrt-ipk/Makefile @@ -114,10 +114,6 @@ define Package/fips/install # Firewall helper script (called by UCI include and hotplug) $(INSTALL_BIN) $(CURDIR)/files/etc/fips/firewall.sh $(1)/etc/fips/firewall.sh - # dnsmasq drop-in: forward .fips queries to the FIPS DNS responder - $(INSTALL_DIR) $(1)/etc/dnsmasq.d - $(INSTALL_DATA) $(CURDIR)/files/etc/dnsmasq.d/fips.conf $(1)/etc/dnsmasq.d/fips.conf - # sysctl: enable br_netfilter so AF_PACKET sees frames on bridge member ports $(INSTALL_DIR) $(1)/etc/sysctl.d $(INSTALL_DATA) $(CURDIR)/files/etc/sysctl.d/fips-bridge.conf $(1)/etc/sysctl.d/fips-bridge.conf diff --git a/packaging/openwrt-ipk/README.md b/packaging/openwrt-ipk/README.md index 7971168e..bc8cb577 100644 --- a/packaging/openwrt-ipk/README.md +++ b/packaging/openwrt-ipk/README.md @@ -17,11 +17,10 @@ OpenWrt 22.03+ router via the standard `opkg` package system. | `/etc/init.d/fips-gateway` | procd service for the gateway (disabled by default) | | `/etc/fips/fips.yaml` | Node configuration (edit before first start) | | `/etc/fips/firewall.sh` | Firewall helper — accepts traffic on `fips0` | -| `/etc/dnsmasq.d/fips.conf` | Forwards `.fips` DNS queries to the daemon | | `/etc/sysctl.d/fips-bridge.conf` | `br_netfilter` settings for Ethernet transport | | `/etc/sysctl.d/fips-gateway.conf` | `proxy_ndp` and IPv6 forwarding for the gateway | | `/etc/hotplug.d/net/99-fips` | Applies firewall rules when `fips0` comes up | -| `/etc/uci-defaults/90-fips-setup` | First-boot kernel module and firewall setup | +| `/etc/uci-defaults/90-fips-setup` | First-boot kernel module, firewall and dnsmasq `.fips` forwarding setup | | `/lib/upgrade/keep.d/fips` | Preserves `/etc/fips/` across `sysupgrade` | ## Requirements diff --git a/packaging/openwrt-ipk/build-ipk.sh b/packaging/openwrt-ipk/build-ipk.sh index 1b2c200e..cdd82eec 100755 --- a/packaging/openwrt-ipk/build-ipk.sh +++ b/packaging/openwrt-ipk/build-ipk.sh @@ -173,9 +173,6 @@ install -d "$DATA_DIR/etc/fips" install -m 0600 "$FILES_DIR/etc/fips/fips.yaml" "$DATA_DIR/etc/fips/fips.yaml" install -m 0755 "$FILES_DIR/etc/fips/firewall.sh" "$DATA_DIR/etc/fips/firewall.sh" -install -d "$DATA_DIR/etc/dnsmasq.d" -install -m 0644 "$FILES_DIR/etc/dnsmasq.d/fips.conf" "$DATA_DIR/etc/dnsmasq.d/fips.conf" - install -d "$DATA_DIR/etc/sysctl.d" install -m 0644 "$FILES_DIR/etc/sysctl.d/fips-bridge.conf" "$DATA_DIR/etc/sysctl.d/fips-bridge.conf" install -m 0644 "$FILES_DIR/etc/sysctl.d/fips-gateway.conf" "$DATA_DIR/etc/sysctl.d/fips-gateway.conf" diff --git a/packaging/openwrt-ipk/files/etc/dnsmasq.d/fips.conf b/packaging/openwrt-ipk/files/etc/dnsmasq.d/fips.conf deleted file mode 100644 index d0680d9d..00000000 --- a/packaging/openwrt-ipk/files/etc/dnsmasq.d/fips.conf +++ /dev/null @@ -1,11 +0,0 @@ -# FIPS mesh DNS — forward .fips queries to the local FIPS DNS responder. -# -# server= forward all .fips queries to 127.0.0.1:5354 -# rebind-domain-ok= disable DNS-rebind protection for .fips; FIPS node -# addresses live in fd00::/8 (ULA), which dnsmasq blocks by -# default as a rebind-attack countermeasure. -# -# If your fips.yaml sets dns.port to something other than 5354, update the -# port number below to match. -server=/fips/127.0.0.1#5354 -rebind-domain-ok=/fips/ diff --git a/packaging/openwrt-ipk/files/etc/fips/fips.yaml b/packaging/openwrt-ipk/files/etc/fips/fips.yaml index 6bb09aee..a12db9b8 100644 --- a/packaging/openwrt-ipk/files/etc/fips/fips.yaml +++ b/packaging/openwrt-ipk/files/etc/fips/fips.yaml @@ -172,7 +172,8 @@ transports: # accept_connections: true # Outbound LAN gateway. dnsmasq forwards .fips queries to listen=[::1]:5353 -# (configured by the fips init script). Requires IPv6 forwarding enabled. +# while it runs (configured by the fips-gateway init script). Requires IPv6 +# forwarding enabled. gateway: enabled: true pool: "fd01::/112" diff --git a/packaging/openwrt-ipk/files/etc/init.d/fips-gateway b/packaging/openwrt-ipk/files/etc/init.d/fips-gateway index 4b9c7041..e9e16879 100755 --- a/packaging/openwrt-ipk/files/etc/init.d/fips-gateway +++ b/packaging/openwrt-ipk/files/etc/init.d/fips-gateway @@ -189,11 +189,6 @@ dnsmasq_swap_fips_upstream() { uci add_list dhcp.@dnsmasq[0].server="/fips/::1#${port}" uci commit dhcp - # Update the drop-in config file as well (belt-and-suspenders). - if [ -f /etc/dnsmasq.d/fips.conf ]; then - sed -i "s|^server=/fips/.*|server=/fips/::1#${port}|" /etc/dnsmasq.d/fips.conf - fi - # Restart dnsmasq to pick up the change. /etc/init.d/dnsmasq restart 2>/dev/null || true } diff --git a/packaging/openwrt-ipk/files/etc/uci-defaults/90-fips-setup b/packaging/openwrt-ipk/files/etc/uci-defaults/90-fips-setup index 654b77c2..a89c27b5 100644 --- a/packaging/openwrt-ipk/files/etc/uci-defaults/90-fips-setup +++ b/packaging/openwrt-ipk/files/etc/uci-defaults/90-fips-setup @@ -59,9 +59,11 @@ uci commit firewall # --------------------------------------------------------------------------- # 3. dnsmasq UCI registration # --------------------------------------------------------------------------- -# /etc/dnsmasq.d/fips.conf already handles runtime forwarding. -# Register via UCI as well so the settings survive a full dnsmasq config -# regeneration (e.g. after a firmware upgrade that rebuilds dnsmasq.conf). +# This UCI entry is what forwards .fips queries to the daemon: OpenWrt's +# dnsmasq init script builds its config from UCI and loads no directory under +# /etc. The daemon's DNS responder binds ::1. The 127.0.0.1 del_list removes +# the entry older packages added. While fips-gateway runs, its init script +# points this entry at the gateway's DNS port instead. uci -q del_list dhcp.@dnsmasq[0].server="/fips/127.0.0.1#5354" 2>/dev/null || true uci -q del_list dhcp.@dnsmasq[0].server="/fips/::1#5354" 2>/dev/null || true diff --git a/testing/openwrt/package-test.sh b/testing/openwrt/package-test.sh index c15ecdfa..e6f8056c 100755 --- a/testing/openwrt/package-test.sh +++ b/testing/openwrt/package-test.sh @@ -7,8 +7,10 @@ # What it checks is which maintainer scripts the .apk registers and what each # one does with apk's upgrade arguments and environment (apk-tools v3 passes # " " and a PATH-only environment to pre-upgrade and -# post-upgrade). Whether a real `apk mkpkg` accepts the result is left to the -# GitHub packaging workflow, which builds with the real tool. +# post-upgrade), and which files the .apk and the .ipk install. The .ipk is +# built for real by build-ipk.sh, which needs only tar. Whether a real +# `apk mkpkg` accepts the result is left to the GitHub packaging workflow, +# which builds with the real tool. # # Usage: package-test.sh [--keep ] # --keep copy the captured apk scripts into as post-install, @@ -222,6 +224,83 @@ for pair in pre-upgrade:prerm post-upgrade:postinst; do esac done +# ── Build the .ipk ────────────────────────────────────────────────────────── + +echo "==> build-ipk.sh" +if ! PKG_VERSION="$PKG_VERSION" \ + bash "$PROJECT_ROOT/packaging/openwrt-ipk/build-ipk.sh" --arch x86_64 --bin-dir "$BINS" \ + > "$TMP/build-ipk.log" 2>&1; then + cat "$TMP/build-ipk.log" >&2 + harness_fail "build-ipk.sh failed, so the .ipk was not checked" +fi +IPK="$PROJECT_ROOT/dist/fips_${PKG_VERSION}_x86_64.ipk" +[[ -f "$IPK" ]] || harness_fail "build-ipk.sh exited 0 but wrote no $IPK" +tar -xzf "$IPK" -O ./data.tar.gz | tar -tzf - > "$TMP/ipk-data" \ + || harness_fail "cannot list data.tar.gz in $IPK" +tar -xzf "$IPK" -O ./control.tar.gz | tar -tzf - > "$TMP/ipk-control" \ + || harness_fail "cannot list control.tar.gz in $IPK" + +# ── P1 and P2. Neither package ships the dnsmasq drop-in ──────────────────── +# OpenWrt's dnsmasq builds its config from UCI and reads no directory under +# /etc, so .fips forwarding comes from the UCI entry 90-fips-setup adds. The +# match is on the directory prefix: tar lists the directory with a trailing +# slash and the stub's find without one. +for pair in "P1:apk:$CAPTURE/payload" "P2:ipk:$TMP/ipk-data"; do + id="${pair%%:*}" + rest="${pair#*:}" + kind="${rest%%:*}" + listing="${rest#*:}" + hits="$(grep -F './etc/dnsmasq.d' "$listing" | tr '\n' ' ')" + if [[ -z "$hits" ]]; then + ok "$id the .$kind installs nothing under /etc/dnsmasq.d" + else + bad "$id the .$kind still installs: $hits" + fi +done + +# ── P3. Positive control for P1 and P2 ────────────────────────────────────── +# An empty or unreadable listing would pass P1 and P2, so each listing must +# show files that are known to ship. +for pair in "apk:$CAPTURE/payload" "ipk:$TMP/ipk-data"; do + kind="${pair%%:*}" + listing="${pair#*:}" + for path in ./etc/init.d/fips-gateway ./etc/uci-defaults/90-fips-setup; do + if grep -qxF "$path" "$listing"; then + ok "P3 the .$kind payload lists $path" + else + bad "P3 the .$kind payload does not list $path, so P1/P2 saw no real listing" + fi + done +done +for path in ./postinst ./prerm; do + if grep -qxF "$path" "$TMP/ipk-control"; then + ok "P3 the .ipk control archive lists $path" + else + bad "P3 the .ipk control archive does not list $path" + fi +done + +# ── P4. No source still names the drop-in ─────────────────────────────────── +# This is the only check on the SDK feed Makefile, which nothing here builds. +# grep exits 1 when nothing matches and 2 when it could not read a path; only +# the first is a pass. +(cd "$PROJECT_ROOT" && grep -rlF 'dnsmasq.d/fips.conf' \ + packaging/openwrt-ipk packaging/openwrt-apk .github/workflows/package-openwrt.yml) \ + > "$TMP/refs" +rc=$? +[[ $rc -le 1 ]] || harness_fail "the drop-in reference search failed (grep exit $rc)" +refs="$(tr '\n' ' ' < "$TMP/refs")" +if [[ -z "$refs" ]]; then + ok "P4 no OpenWrt packaging file names the dnsmasq drop-in" +else + bad "P4 the dnsmasq drop-in is still named in: $refs" +fi +if [[ -e "$PROJECT_ROOT/packaging/openwrt-ipk/files/etc/dnsmasq.d/fips.conf" ]]; then + bad "P4 packaging/openwrt-ipk/files/etc/dnsmasq.d/fips.conf still exists" +else + ok "P4 the drop-in source file is gone" +fi + # ── Hand the scripts to the ash scenarios ─────────────────────────────────── if [[ -n "$KEEP" ]]; then for phase in $WANT_PHASES; do From 8b9b0ed5c1c7df3a2dc01813fc33e45c09e366c9 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 23 Sep 2026 04:10:16 +0000 Subject: [PATCH 11/14] Correct the OpenWrt README's upgrade commands and default settings The README told operators to upgrade with opkg install --force-reinstall. opkg runs that as a removal followed by a fresh install, so the removal disabled fips-gateway and the fresh install left it off. A plain opkg install takes the upgrade path, which stops both services and starts the gateway again only if it was enabled. The Upgrading section now says so for OpenWrt 24.10 and earlier, notes that the first upgrade from 0.5.1 or earlier re-enables the gateway because those packages' prerm disabled it without recording its state, and explains when --force-downgrade is needed. OpenWrt 25 has no opkg, so neither opkg command runs there. The section now opens with the apk add --allow-untrusted command that upgrades the .apk package on OpenWrt 25 and later, and points to the .apk README. The list of what the default config enables was also out of date. The shipped config generates an ephemeral identity on each start unless persistent is uncommented, the DNS responder binds [::1]:5354, UDP binds [::]:2121, TCP listens on 0.0.0.0:8443, and the ethernet: section ships uncommented, so the text now says to edit its interface names rather than uncomment it. --- packaging/openwrt-ipk/README.md | 64 +++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/packaging/openwrt-ipk/README.md b/packaging/openwrt-ipk/README.md index bc8cb577..6b0d170d 100644 --- a/packaging/openwrt-ipk/README.md +++ b/packaging/openwrt-ipk/README.md @@ -123,13 +123,18 @@ vi /etc/fips/fips.yaml ``` The default config enables: -- Persistent identity (key generated on first start, saved to `/etc/fips/fips.key`) -- TUN interface `fips0` -- DNS responder on `127.0.0.1:5354` -- UDP transport on `0.0.0.0:2121` -For Ethernet transport, uncomment the `ethernet:` section and set the correct -physical interface names for your router. **Always use physical port names +- An ephemeral identity, generated on each start. Uncomment + `node.identity.persistent: true` to keep one; the key is then saved next to + the config, as `/etc/fips/fips.key`. +- TUN interface `fips0` +- DNS responder on `[::1]:5354` +- UDP transport on `[::]:2121` +- TCP transport on `0.0.0.0:8443` +- Ethernet transport, including the `wan`, `wwan` and `lan` entries + +For Ethernet transport, edit the interface names in the `ethernet:` section to +match your router. **Always use physical port names (`eth0`, `eth1`, or DSA port names like `wan`/`lan1`), never bridge names (`br-lan`).** The shipped default WAN port is `eth0` (OpenWrt 24); on OpenWrt 25 (DSA) boards the WAN port is named `wan` — the `.apk` package ships that @@ -187,12 +192,51 @@ for the full subcommand list. ## Upgrading -Install the new `.ipk` over the existing one: +OpenWrt 25 and later have no opkg. Upgrade there with the `.apk` package, +using the same command that installs it: ```bash -opkg install --force-reinstall fips__.ipk +apk add --allow-untrusted /tmp/fips__.apk +``` + +The `.apk` package's upgrade scripts stop `fips` and `fips-gateway`, start +`fips` again, and start `fips-gateway` only if it was enabled; see +[`../openwrt-apk/README.md`](../openwrt-apk/README.md). + +On OpenWrt 24.10 and earlier, install the new `.ipk` with a plain +`opkg install`: + +```bash +opkg install /tmp/fips__.ipk +``` + +opkg runs this as an upgrade. The installed package's `prerm` stops `fips` and +`fips-gateway` without disabling them, and the new package's `postinst` starts +`fips` and starts `fips-gateway` again if it was enabled. + +An upgrade from 0.5.1 or earlier is the exception. The `prerm` in those +packages disables `fips-gateway` and records nothing about whether it was +enabled, so the new `postinst` enables it again. If you had the gateway +disabled, disable it again after that first upgrade: + +```bash +/etc/init.d/fips-gateway stop +/etc/init.d/fips-gateway disable +``` + +If opkg refuses because the new file's version sorts lower than the installed +one, as it can between development builds, add `--force-downgrade`. opkg then +takes the same upgrade path. + +Do not use `--force-reinstall`. opkg runs it as a removal followed by a fresh +install, so `fips-gateway` ends up disabled. To turn it back on: + +```bash +/etc/init.d/fips-gateway enable +/etc/init.d/fips-gateway start ``` The config in `/etc/fips/fips.yaml` and the identity key `/etc/fips/fips.key` -are preserved by `opkg` (the yaml is installed as a conffile; the key is not a -package file). Both survive `sysupgrade` via `/lib/upgrade/keep.d/fips`. +(when persistent identity is on) are preserved by `opkg` (the yaml is installed +as a conffile; the key is not a package file). Both survive `sysupgrade` via +`/lib/upgrade/keep.d/fips`. From 877d31246f316e486179c3122572cb7d7ea020ae Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 23 Sep 2026 04:11:52 +0000 Subject: [PATCH 12/14] Correct the BLE comments in the shipped configs The example configs said BLE needs "the 'ble' feature", but no such cargo feature exists: the BLE transport is compiled for glibc Linux and Android by build.rs and nowhere else. Say which builds have it in the common config, and drop the BLE example from the OpenWrt config, whose musl builds never include the transport. Correct the same claim in the BleConfig doc comment. Add lib tests that the shipped configs name only cargo features that exist and that the OpenWrt config offers no BLE block. --- packaging/common/fips.yaml | 3 +- .../openwrt-ipk/files/etc/fips/fips.yaml | 9 +- src/config/transport.rs | 2 +- src/packaging_tests.rs | 101 ++++++++++++++++++ 4 files changed, 105 insertions(+), 10 deletions(-) diff --git a/packaging/common/fips.yaml b/packaging/common/fips.yaml index c8ea17ce..e23b4bf1 100644 --- a/packaging/common/fips.yaml +++ b/packaging/common/fips.yaml @@ -103,7 +103,8 @@ transports: # auto_connect: true # accept_connections: true - # Bluetooth Low Energy transport — requires BlueZ and the 'ble' feature. + # Bluetooth Low Energy transport: Linux (glibc) builds only, with BlueZ + # (bluetoothd) running. Not available on macOS, FreeBSD, Windows or OpenWrt. # ble: # adapter: "hci0" # mtu: 2048 diff --git a/packaging/openwrt-ipk/files/etc/fips/fips.yaml b/packaging/openwrt-ipk/files/etc/fips/fips.yaml index a12db9b8..2f15160d 100644 --- a/packaging/openwrt-ipk/files/etc/fips/fips.yaml +++ b/packaging/openwrt-ipk/files/etc/fips/fips.yaml @@ -162,14 +162,7 @@ transports: # auto_connect: true # accept_connections: true - # Bluetooth Low Energy transport — requires BlueZ and the 'ble' feature. - # ble: - # adapter: "hci0" - # mtu: 2048 - # advertise: true - # scan: true - # auto_connect: true - # accept_connections: true + # No BLE transport: OpenWrt builds target musl, which has no BlueZ backend. # Outbound LAN gateway. dnsmasq forwards .fips queries to listen=[::1]:5353 # while it runs (configured by the fips-gateway init script). Requires IPv6 diff --git a/src/config/transport.rs b/src/config/transport.rs index 6bc3d380..b035b302 100644 --- a/src/config/transport.rs +++ b/src/config/transport.rs @@ -702,7 +702,7 @@ const DEFAULT_BLE_PROBE_COOLDOWN_SECS: u64 = 30; /// BLE transport instance configuration. /// /// BleConfig is always compiled (for config parsing on any platform), -/// but the transport runtime requires Linux and the `ble` feature. +/// but the transport runtime is compiled only for glibc Linux and Android. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct BleConfig { diff --git a/src/packaging_tests.rs b/src/packaging_tests.rs index 6716cac1..7812c7d1 100644 --- a/src/packaging_tests.rs +++ b/src/packaging_tests.rs @@ -188,6 +188,53 @@ fn case_branch(sh: &str, label: &str) -> Vec { lines[start..start + len].to_vec() } +/// Returns the feature names declared in the `[features]` table of a +/// Cargo.toml. +fn cargo_features(cargo_toml: &str) -> Vec { + toml_section(cargo_toml, "[features]") + .into_iter() + .map(str::trim) + .filter(|l| !l.is_empty() && !l.starts_with('#')) + .filter_map(|l| l.split_once('=').map(|(key, _)| key.trim().to_string())) + .collect() +} + +/// Returns the cargo feature names a config file's comments mention: on each +/// `#` comment line, the token before the word `feature` or `features` when +/// that token is wrapped in `'`, `"` or `` ` ``. +fn feature_mentions(text: &str) -> Vec { + let mut found = Vec::new(); + for line in text.lines().map(str::trim) { + if !line.starts_with('#') { + continue; + } + let words: Vec<&str> = line.split_whitespace().collect(); + for pair in words.windows(2) { + let word = pair[1].trim_end_matches(|c: char| c.is_ascii_punctuation()); + if word != "feature" && word != "features" { + continue; + } + let quoted = ['\'', '"', '`'].iter().find_map(|q| { + pair[0] + .strip_prefix(*q) + .and_then(|rest| rest.strip_suffix(*q)) + }); + if let Some(name) = quoted { + found.push(name.to_string()); + } + } + } + found +} + +/// Whether a config line, commented out or not, starts a `ble:` block. +fn is_ble_key(line: &str) -> bool { + line.trim() + .trim_start_matches('#') + .trim_start() + .starts_with("ble:") +} + #[test] fn deb_and_aur_packages_declare_nftables_for_the_firewall_units_nft() { let unit = repo_file("packaging/debian/fips-firewall.service"); @@ -758,3 +805,57 @@ fn windows_installer_icacls_calls_act_on_links_and_check_exit_codes() { ); } } + +const COMMON_CONFIG: &str = "packaging/common/fips.yaml"; +const OPENWRT_CONFIG: &str = "packaging/openwrt-ipk/files/etc/fips/fips.yaml"; + +#[test] +fn shipped_configs_name_only_cargo_features_that_exist() { + assert_eq!( + feature_mentions( + " # Bluetooth Low Energy transport — requires BlueZ and the 'ble' feature." + ), + ["ble"], + "control: the feature-mention scanner no longer finds a quoted feature name" + ); + let features = cargo_features(&repo_file("Cargo.toml")); + assert!( + features.iter().any(|f| f == "profiling"), + "control: expected the profiling feature in Cargo.toml [features], read {features:?}" + ); + + let mut unknown = Vec::new(); + for rel in [COMMON_CONFIG, OPENWRT_CONFIG] { + for name in feature_mentions(&repo_file(rel)) { + if !features.contains(&name) { + unknown.push(format!("{rel}: '{name}'")); + } + } + } + assert!( + unknown.is_empty(), + "shipped configs name cargo features that Cargo.toml does not define \ + (it defines {features:?}):\n {}", + unknown.join("\n ") + ); +} + +#[test] +fn openwrt_config_offers_no_ble_block_because_musl_builds_have_no_ble() { + assert!( + repo_file(COMMON_CONFIG).lines().any(is_ble_key), + "control: expected the ble: example in {COMMON_CONFIG}" + ); + let text = repo_file(OPENWRT_CONFIG); + let found: Vec<(usize, &str)> = text + .lines() + .enumerate() + .filter(|(_, l)| is_ble_key(l)) + .map(|(i, l)| (i + 1, l.trim_end())) + .collect(); + assert!( + found.is_empty(), + "{OPENWRT_CONFIG} offers a ble: block, but OpenWrt builds target musl, \ + where the BLE transport is not compiled: {found:?}" + ); +} From c560ba0f07ae594e2b02c3beb17a4f16a493b156 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Wed, 23 Sep 2026 04:12:13 +0000 Subject: [PATCH 13/14] Correct the reason given for disabling the MIPS OpenWrt builds The comment said fips's own atomics already use portable_atomic, so only the Nostr relay pool stood in the way. That is true only of the transport stats modules; the node metrics, the profiler and other modules still use std's AtomicU64, which 32-bit MIPS lacks. Say so, and note that the nightly matrix entries would also need -Zbuild-std, which the build step does not pass. --- .github/workflows/package-openwrt.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/package-openwrt.yml b/.github/workflows/package-openwrt.yml index e54e8597..c9945dd6 100644 --- a/.github/workflows/package-openwrt.yml +++ b/.github/workflows/package-openwrt.yml @@ -86,9 +86,11 @@ jobs: rust_target: aarch64-unknown-linux-musl rust_channel: stable # MT3000, MT6000, Flint 2, RPi 3/4/5 - # MIPS disabled: nostr-relay-pool 0.44 uses std::sync::atomic::AtomicU64 - # directly (fips's own atomics already use portable_atomic). Re-enable - # once an upstream portable-atomic patch lands (or via [patch.crates-io]). + # MIPS disabled: 32-bit MIPS has no 64-bit atomics, and both + # nostr-relay-pool 0.44 and fips itself (outside the transport stats + # modules) use std::sync::atomic::AtomicU64. Re-enabling needs both on + # portable_atomic, and the nightly entries below also need -Zbuild-std, + # which the build step does not pass. # - build_arch: mipsel # openwrt_arch: mipsel_24kc # rust_target: mipsel-unknown-linux-musl From 99e51c89a5415bb8448cf623719f5ff03ba0aa52 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 26 Sep 2026 20:42:02 +0000 Subject: [PATCH 14/14] Update the changelog for the OpenWrt upgrade restart, dnsmasq drop-in and README fixes --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d169546..e1f45d14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -360,6 +360,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 four script bodies live in `packaging/openwrt-ipk/scripts/` instead of inside heredocs in the two build scripts, so the scenarios in `testing/openwrt/` run what ships. +- An `apk` upgrade on OpenWrt 25 now restarts `fips`, and restarts + `fips-gateway` if it was enabled, so the new binaries run without a reboot. + apk-tools v3 runs only the incoming package's pre-upgrade and post-upgrade + scripts, and the `.apk` registered neither, so an upgrade replaced the files + on disk and left the old processes running until a reboot or a manual + restart. +- The packages no longer ship `/etc/dnsmasq.d/fips.conf`. OpenWrt's dnsmasq + builds its config from UCI and never reads that directory; `.fips` + forwarding has always come from the UCI server entry, which is unchanged. An + opkg upgrade removes the old file, and an apk upgrade keeps it only if it + was modified. Either way nothing reads it. +- The package README's upgrade commands and default settings are corrected. + It now gives the `apk add` command for OpenWrt 25, where there is no opkg, + and for OpenWrt 24.10 and earlier a plain `opkg install` in place of + `--force-reinstall`, which removed and reinstalled the package and so left + `fips-gateway` disabled. Its description of the default config now matches + the shipped `fips.yaml`. #### Packaging (Debian)