From 3c415ea1d736101cabf876ebc43ecc4fa468383d Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sun, 23 Aug 2026 13:36:46 -0600 Subject: [PATCH 01/19] fix(hardware): bind unclaimed NICs to vfio-pci A missing driver symlink means a NIC is unbound, which is valid when its kernel module was never loaded or it was already detached. Treating that as an error made BindToVfioPci refuse the device before its override-and-bind path. Continue binding unclaimed NICs, recognize e1000 and e1000e, and link the matching DPDK PMD for QEMU and bare-metal hosts. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dpdk-sys/build.rs | 1 + hardware/src/nic/mod.rs | 22 +++++++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/dpdk-sys/build.rs b/dpdk-sys/build.rs index 4101419add..9fdc9a71e6 100644 --- a/dpdk-sys/build.rs +++ b/dpdk-sys/build.rs @@ -76,6 +76,7 @@ fn main() { "rte_net_virtio", "rte_net_vhost", "rte_net_i40e", + "rte_net_e1000", "rte_vhost", "rte_net_mlx5", "rte_common_mlx5", diff --git a/hardware/src/nic/mod.rs b/hardware/src/nic/mod.rs index 69213f9bcc..9ccc8a4789 100644 --- a/hardware/src/nic/mod.rs +++ b/hardware/src/nic/mod.rs @@ -73,7 +73,14 @@ impl GetDriver for PciNic { fn driver(&self) -> Result, DriverErr> { let device_path = self.device_path().map_err(DriverErr::Sysfs)?; info!("found device {self} under device path {:?}", device_path); - let driver_path = device_path.relative("driver").map_err(DriverErr::Sysfs)?; + let driver_path = match device_path.relative("driver") { + Ok(p) => p, + Err(SysfsErr::IoError(e)) if e.kind() == ErrorKind::NotFound => { + info!("no driver symlink for {self} (no driver bound)"); + return Ok(None); + } + Err(e) => return Err(DriverErr::Sysfs(e)), + }; info!("{self} is using driver path {driver_path:?}"); match driver_path.inner().file_name() { Some(os_str) => match os_str.to_str() { @@ -99,6 +106,10 @@ impl std::fmt::Display for PciNic { /// Enum describing supported PCI drivers. #[derive(Debug, Copy, Clone, PartialEq, Eq, strum::EnumString, strum::IntoStaticStr)] pub enum PciDriver { + #[strum(serialize = "e1000")] + E1000, + #[strum(serialize = "e1000e")] + E1000E, /// Intel's i40e driver. #[strum(serialize = "i40e")] I40e, @@ -318,14 +329,7 @@ impl BindToVfioPci for PciNic { } } Ok(None) => { - let msg = format!( - "device {self} is unknown to the operating system. You may need to load (modprobe) a driver" - ); - error!("{msg}"); - return Err(DriverErr::Sysfs(SysfsErr::IoError(std::io::Error::new( - ErrorKind::Unsupported, - msg, - )))); + info!("device {self} has no driver bound; proceeding to vfio-pci bind"); } Err(err) => { error!("failed to get device driver: {:?}", err); From a0c7c83259db3b4d7045c3b8f5849a7f62972a8f Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 13:22:09 -0600 Subject: [PATCH 02/19] test(dataplane): build replies from delivered addresses Conversation replied from the address a sender dialed instead of the address that arrived after NAT. Those differ when an expose both translates and accepts connections, making the far side send from an address it does not own and causing a misleading flow-filter drop. Build replies from the delivered packet so the existing oracle genuinely checks reverse translation. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 2e691cc2cc..f95796cf12 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -3549,7 +3549,10 @@ mod routed { enum State { Opening, AwaitingRequest, - Replying { public: (IpAddr, u16) }, + Replying { + public: (IpAddr, u16), + landed: (IpAddr, u16), + }, AwaitingReply, Closed, Abandoned, @@ -3609,10 +3612,24 @@ mod routed { self.state = State::Abandoned; return; }; + let (Some(landed_dst), Some(landed_port)) = + (carried.ip_destination(), carried.transport_dst_port()) + else { + self.note("the delivered request had no destination tuple to answer from"); + self.state = State::Abandoned; + return; + }; self.note(&format!("request left as {public_src}:{}", port.get())); + if landed_dst != self.dst { + self.note(&format!( + "request landed on {landed_dst}:{}", + landed_port.get() + )); + } self.public = Some((public_src, port.get())); self.state = State::Replying { public: (public_src, port.get()), + landed: (landed_dst, landed_port.get()), }; } @@ -3667,8 +3684,11 @@ mod routed { self.state = State::AwaitingRequest; Some(tunnelled_from(self.path.from, &request)) } - State::Replying { public: (ip, port) } => { - let reply = udp(self.dst, ip, self.dport, port)?; + State::Replying { + public: (ip, port), + landed: (from_ip, from_port), + } => { + let reply = udp(from_ip, ip, from_port, port)?; self.state = State::AwaitingReply; Some(tunnelled_from(self.path.reversed().from, &reply)) } From a6e01389cdb94a6dac90d1fe06d782b4ceb883bb Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 13:22:57 -0600 Subject: [PATCH 03/19] fix(nat): retry burst flow lookup with the original key Static NAT rewrites a destination before masquerade, so the packet's current key differs from the initial key where FlowLookup filed its forward flow. The intra-burst fallback searched only the rewritten key and allocated a new public tuple for every packet. Keep the current-key lookup first for replies, then retry the original key on a miss and cover the combined translation. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/vpcpeering.rs | 35 ++++++-- dataplane/src/packet_processor/fuzz.rs | 102 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index ee1cfc5c86..a81d036363 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -1607,19 +1607,40 @@ pub mod contract { _ => "3.3.3.0/24", }; + let remote = vec![ + VpcExpose::empty().ip(remote_prefix + .parse::() + .unwrap_or_else(|_| unreachable!()) + .into()), + ]; + + overlay_between(exposes, remote, gwgroup) + } + + /// Build the fixed two-vpc overlay from both sides' exposes. + /// + /// `gwgroup` names the gateway group that owns the peering, which is what decides + /// whether a given gateway renders it. + /// + /// # Errors + /// + /// Returns an error if either vpc is rejected by the [`VpcTable`], or if the + /// peering between them is rejected by the [`VpcPeeringTable`]. + pub fn overlay_between( + local: Vec, + remote: Vec, + gwgroup: &str, + ) -> Result { let mut vpc_table = VpcTable::new(); vpc_table.add(Vpc::new("VPC-1", "AAAAA", LOCAL_VNI)?)?; vpc_table.add(Vpc::new("VPC-2", "BBBBB", REMOTE_VNI)?)?; - let local = exposes + let local = local .into_iter() .fold(VpcManifest::new("VPC-1"), VpcManifest::exposing); - let remote = VpcManifest::new("VPC-2").exposing( - VpcExpose::empty().ip(remote_prefix - .parse::() - .unwrap_or_else(|_| unreachable!()) - .into()), - ); + let remote = remote + .into_iter() + .fold(VpcManifest::new("VPC-2"), VpcManifest::exposing); let mut peerings = VpcPeeringTable::new(); peerings.add(VpcPeering::new( "VPC-1--VPC-2", diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index f95796cf12..c55a110fde 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -3031,6 +3031,108 @@ mod burst { super::assert_covered(checked > 0, "no burst of a single flow was ever delivered"); } + #[tokio::test] + #[dpdk::with_eal] + async fn a_burst_of_one_translated_flow_allocates_once() { + static CHECKED: AtomicU64 = AtomicU64::new(0); + + fn two_sided() -> Option { + let local = VpcExpose::empty() + .make_masquerade(None) + .ok()? + .ip("1.1.0.0/16".parse::().ok()?.into()) + .as_range("2.2.0.0/16".parse::().ok()?.into()) + .ok()?; + let remote = VpcExpose::empty() + .make_static_nat() + .ok()? + .ip("3.3.0.0/16".parse::().ok()?.into()) + .as_range("4.4.0.0/16".parse::().ok()?.into()) + .ok()?; + config::external::overlay::vpcpeering::contract::overlay_between( + vec![local], + vec![remote], + "default", + ) + .ok() + } + + let overlay = two_sided().expect("a valid two-sided configuration"); + + bolero::check!() + .with_max_len(MAX_INPUT_LEN) + .with_generator(Burst) + .for_each(|members| { + let m = members[0]; + let src: IpAddr = format!("1.1.0.{}", m.host) + .parse() + .unwrap_or_else(|_| unreachable!()); + let dst: IpAddr = "4.4.0.1".parse().unwrap_or_else(|_| unreachable!()); + let packet = || udp(src, dst, 4000, m.dport).map(|p| tunnelled(&p)); + + let tables = || topology(&[vni(LOCAL_VNI), vni(REMOTE_VNI)]); + let (Some(mut alone), Some(mut burst)) = ( + Fabric::routed_over(&overlay, tables()), + Fabric::routed_over(&overlay, tables()), + ) else { + return; + }; + let Some(one) = packet() else { return }; + let single = treatment(&alone.send(one)); + if !matches!(single.verdict, Verdict::Delivered { .. }) { + return; + } + let cost_of_one = alone.flows(); + + assert_eq!( + single.inner_dst, + Some("3.3.0.1".parse().unwrap_or_else(|_| unreachable!())), + "the far side's static nat did not translate the destination, so this \ + configuration does not reach the case this test is for" + ); + + let Some(together) = (0..BURST).map(|_| packet()).collect::>>() + else { + return; + }; + let out = burst.send_batch(together); + + for (i, packet) in out.iter().enumerate() { + let t = treatment(packet); + assert_eq!( + t.inner_sport, single.inner_sport, + "packet {i} of a burst of one masqueraded-and-translated flow was given \ + a different public port from the same packet sent alone: the burst \ + allocated more than once" + ); + assert_eq!( + t.inner_src, single.inner_src, + "packet {i} of a burst of one masqueraded-and-translated flow left under \ + a different public address" + ); + assert_eq!( + t.verdict, single.verdict, + "packet {i} of a burst of one masqueraded-and-translated flow reached a \ + different verdict" + ); + } + assert_eq!( + burst.flows(), + cost_of_one, + "a burst of {BURST} packets of one masqueraded-and-translated flow cost more \ + flow-table entries than one packet of it did" + ); + CHECKED.fetch_add(1, Ordering::Relaxed); + }); + + let checked = CHECKED.load(Ordering::Relaxed); + eprintln!("translated-single-flow-bursts={checked}"); + super::assert_covered( + checked > 0, + "no burst of a single masqueraded-and-translated flow was ever delivered", + ); + } + #[tokio::test] #[dpdk::with_eal] async fn a_burst_is_treated_the_same_as_one_packet_at_a_time() { From 250541fc1e323234bb231fa0b28f649269927a65 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 13:23:31 -0600 Subject: [PATCH 04/19] feat(config): generate static NAT exposes The operation algebra could not produce static NAT, leaving two schema degrees unreachable and making the config-apply StaticNat step store only empty tables. Add the missing expose flavour using the algebra's disjoint private and public address blocks, and update the completeness census. Mixed static NAT and masquerade configurations now reach their shared pipeline path. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 34 +++++++++++++++------ config/src/external/overlay/completeness.rs | 15 ++++----- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index ce3a36d0a9..1ac90509f4 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -46,6 +46,7 @@ impl Side { pub enum Flavour { Forward, Masquerade, + StaticNat, } pub const MAX_EXPOSES: u8 = 4; @@ -117,7 +118,9 @@ impl ExposeSpec { pub fn public(self, peering: PeeringHandle, side: Side) -> Prefix { match self.flavour { Flavour::Forward => self.private(peering, side), - Flavour::Masquerade => public_prefix(peering.block(side, self.slot)), + Flavour::Masquerade | Flavour::StaticNat => { + public_prefix(peering.block(side, self.slot)) + } } } @@ -131,6 +134,12 @@ impl ExposeSpec { .ip(private.into()) .as_range(self.public(peering, side).into()) .unwrap_or_else(|_| unreachable!("a masquerade expose accepts a public range")), + Flavour::StaticNat => VpcExpose::empty() + .make_static_nat() + .unwrap_or_else(|_| unreachable!("an empty expose accepts static nat")) + .ip(private.into()) + .as_range(self.public(peering, side).into()) + .unwrap_or_else(|_| unreachable!("a static nat expose accepts a public range")), } } } @@ -942,11 +951,13 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { } fn draw_flavour(driver: &mut D, spec: &PeeringSpec, side: Side) -> Option { - if driver.produce::()? && !spec.has_stateful(side.other()) { - Some(Flavour::Masquerade) + const ORDERED: [Flavour; 3] = [Flavour::Forward, Flavour::StaticNat, Flavour::Masquerade]; + let legal: &[Flavour] = if spec.has_stateful(side.other()) { + &ORDERED[..2] } else { - Some(Flavour::Forward) - } + &ORDERED + }; + pick(driver, legal) } fn pick(driver: &mut D, items: &[T]) -> Option { @@ -1067,6 +1078,7 @@ mod tests { fn every_sequence_builds_a_valid_configuration() { let masquerading = AtomicUsize::new(0); let forwarding = AtomicUsize::new(0); + let static_nat = AtomicUsize::new(0); check!() .with_generator(Sequence::default()) @@ -1090,6 +1102,7 @@ mod tests { match expose.flavour() { Flavour::Masquerade => &masquerading, Flavour::Forward => &forwarding, + Flavour::StaticNat => &static_nat, } .fetch_add(1, Relaxed); } @@ -1106,11 +1119,14 @@ mod tests { assert_every_kind_drawn(); assert!( - masquerading.load(Relaxed) > 0 && forwarding.load(Relaxed) > 0, - "only one flavour of expose was ever built (masquerade {}, forward {}), so the \ - combination rules were not exercised", + masquerading.load(Relaxed) > 0 + && forwarding.load(Relaxed) > 0 + && static_nat.load(Relaxed) > 0, + "not every flavour of expose was built (masquerade {}, forward {}, static nat {}), \ + so the combination rules were not exercised", masquerading.load(Relaxed), - forwarding.load(Relaxed) + forwarding.load(Relaxed), + static_nat.load(Relaxed) ); } diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index cd4dc1b29b..ce25304e77 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -98,7 +98,7 @@ const REACH: &[(&str, Reach)] = &[ ("VpcExpose.nat", Reach::Spans(&["absent", "present"])), ( "VpcExposeNat.as_range", - Reach::Determined("one prefix in the masquerade pool, from peering, side and slot"), + Reach::Determined("one prefix in the translated pool, from peering, side and slot"), ), ( "VpcExposeNat.as_range.ports", @@ -110,7 +110,7 @@ const REACH: &[(&str, Reach)] = &[ ), ( "VpcExposeNat.config", - Reach::Fixed("masquerade; the algebra has no static NAT or port-forwarding operations."), + Reach::Spans(&["masquerade", "static"]), ), ( "VpcExposeNat.proto", @@ -123,13 +123,14 @@ const REACH: &[(&str, Reach)] = &[ timeouts untested.", ), ), - ( - "VpcExposeStaticNat", - Reach::Fixed("never constructed; see `VpcExposeNat.config`."), - ), + ("VpcExposeStaticNat", Reach::Spans(&["constructed"])), ( "VpcExposePortForwarding.idle_timeout", - Reach::Fixed("never constructed; see `VpcExposeNat.config`."), + Reach::Fixed( + "never constructed. `Flavour` has no port-forwarding member, so this flavour of \ + nat -- and every question about the idle timeout it carries -- stays out of \ + reach, which the design note names as missing vocabulary.", + ), ), ]; From 0cb37530ffb36b2710349ab2e6c6fd7b6f6ea516 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 13:41:14 -0600 Subject: [PATCH 05/19] test(dataplane): use endpoint-owned addresses for inbound traffic Inbound loads used a peer's public translation range as the sender address and answered to the dialed address rather than the request's delivered source. Both values happened to agree before translated inbound exposes were generated; afterward the flow filter correctly rejected them as unowned. Separate dial destinations from peer source addresses and reply to the observed sender. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/fuzz.rs | 43 +++++++++++++++++++++----- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index c55a110fde..807e24d3f9 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -745,7 +745,7 @@ pub(crate) mod derive { use super::*; use config::external::overlay::ValidatedOverlay; use config::external::overlay::vpcpeering::ValidatedExpose; - use lpm::prefix::{Prefix, PrefixWithOptionalPorts}; + use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; #[derive(Debug, Clone, Copy)] pub(crate) struct Vary { @@ -793,13 +793,30 @@ pub(crate) mod derive { peering: &config::external::overlay::vpc::ValidatedPeering, n: u8, usable: fn(&ValidatedExpose) -> bool, + ) -> Option { + peer_address(peering, n, usable, ValidatedExpose::public_ips) + } + + fn peer_source_of( + peering: &config::external::overlay::vpc::ValidatedPeering, + n: u8, + usable: fn(&ValidatedExpose) -> bool, + ) -> Option { + peer_address(peering, n, usable, ValidatedExpose::ips) + } + + fn peer_address( + peering: &config::external::overlay::vpc::ValidatedPeering, + n: u8, + usable: fn(&ValidatedExpose) -> bool, + which: for<'a> fn(&'a ValidatedExpose) -> &'a PrefixPortsSet, ) -> Option { peering .remote() .valexp() .iter() .filter(|expose| usable(expose)) - .flat_map(|expose| expose.public_ips().into_iter()) + .flat_map(|expose| which(expose).into_iter()) .find_map(|entry| host_in(entry.prefix(), n)) } @@ -839,7 +856,8 @@ pub(crate) mod derive { let outward = peer_of(peering, v.host, |expose| { expose.can_receive_connection() && !expose.has_port_forwarding() }); - let inward = peer_of(peering, v.host, ValidatedExpose::can_init_connection); + let inward = + peer_source_of(peering, v.host, ValidatedExpose::can_init_connection); if expose.has_port_forwarding() { let (Some(outside), Some(inside_entry)) = ( @@ -3964,7 +3982,7 @@ mod routed { enum InboundState { Reaching, AwaitingArrival, - Answering, + Answering { reply_to: (IpAddr, u16) }, AwaitingAnswer, Closed, Abandoned, @@ -4012,8 +4030,17 @@ mod routed { "reached the right host on the wrong port. {}", self.describe() ); + let (Some(src), Some(sport)) = (arrived.ip_source(), arrived.transport_src_port()) + else { + self.log + .push("the arrived request had no source tuple to answer".to_owned()); + self.state = InboundState::Abandoned; + return; + }; self.log.push("arrived inside".to_owned()); - self.state = InboundState::Answering; + self.state = InboundState::Answering { + reply_to: (src, sport.get()), + }; } fn judge_answer(&mut self, got: &Packet) { @@ -4056,8 +4083,10 @@ mod routed { self.state = InboundState::AwaitingArrival; Some(tunnelled_from(self.path.to, &request)) } - InboundState::Answering => { - let answer = udp(self.internal, self.from, self.internal_port, self.sport)?; + InboundState::Answering { + reply_to: (to_ip, to_port), + } => { + let answer = udp(self.internal, to_ip, self.internal_port, to_port)?; self.state = InboundState::AwaitingAnswer; Some(tunnelled_from(self.path.from, &answer)) } From 6622a4c97322f5f3d40d76823be9377ef640b882 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 13:41:44 -0600 Subject: [PATCH 06/19] feat(config): generate port-forwarding exposes Port forwarding was the last missing translated expose flavour and the only source of generated port ranges, leaving several configuration fields untested. Add equal-width but distinct public and private ranges, forbid conflicting directional translations on the opposite side, and require evidence that inbound loads exercise the new flavour. Leave protocol and timeout variation to separate changes. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 63 +++++++++++++++++---- config/src/external/overlay/completeness.rs | 15 ++--- dataplane/src/packet_processor/fuzz.rs | 16 +++++- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 1ac90509f4..720e2f0f7a 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -6,7 +6,7 @@ use std::net::Ipv4Addr; use std::ops::Bound::Included; use bolero::{Driver, ValueGenerator}; -use lpm::prefix::{IpPrefix, Ipv4Prefix, Prefix}; +use lpm::prefix::{IpPrefix, Ipv4Prefix, PortRange, Prefix, PrefixWithOptionalPorts}; use crate::ConfigError; use crate::external::overlay::Overlay; @@ -47,6 +47,14 @@ pub enum Flavour { Forward, Masquerade, StaticNat, + PortForward, +} + +impl Flavour { + #[must_use] + pub const fn is_directional(self) -> bool { + matches!(self, Self::Masquerade | Self::PortForward) + } } pub const MAX_EXPOSES: u8 = 4; @@ -82,6 +90,14 @@ impl PeeringHandle { } } +pub(crate) const FORWARDED_PRIVATE_PORTS: (u16, u16) = (1000, 1004); + +pub(crate) const FORWARDED_PUBLIC_PORTS: (u16, u16) = (2000, 2004); + +fn port_range((start, end): (u16, u16)) -> PortRange { + PortRange::new(start, end).unwrap_or_else(|_| unreachable!("a well-formed port range")) +} + fn private_prefix(index: u32) -> Prefix { prefix_v4(0x0A00_0000 | (index << 8), 24) } @@ -118,7 +134,7 @@ impl ExposeSpec { pub fn public(self, peering: PeeringHandle, side: Side) -> Prefix { match self.flavour { Flavour::Forward => self.private(peering, side), - Flavour::Masquerade | Flavour::StaticNat => { + Flavour::Masquerade | Flavour::StaticNat | Flavour::PortForward => { public_prefix(peering.block(side, self.slot)) } } @@ -140,6 +156,20 @@ impl ExposeSpec { .ip(private.into()) .as_range(self.public(peering, side).into()) .unwrap_or_else(|_| unreachable!("a static nat expose accepts a public range")), + Flavour::PortForward => VpcExpose::empty() + .make_port_forwarding(None, None) + .unwrap_or_else(|_| unreachable!("an empty expose accepts port forwarding")) + .ip(PrefixWithOptionalPorts::new( + private, + Some(port_range(FORWARDED_PRIVATE_PORTS)), + )) + .as_range(PrefixWithOptionalPorts::new( + self.public(peering, side), + Some(port_range(FORWARDED_PUBLIC_PORTS)), + )) + .unwrap_or_else(|_| { + unreachable!("a port forwarding expose accepts a public range") + }), } } } @@ -183,10 +213,10 @@ impl PeeringSpec { } } - fn has_stateful(&self, side: Side) -> bool { + fn has_directional(&self, side: Side) -> bool { self.exposes(side) .iter() - .any(|expose| expose.flavour == Flavour::Masquerade) + .any(|expose| expose.flavour.is_directional()) } fn manifest(&self, peering: PeeringHandle, side: Side) -> VpcManifest { @@ -546,7 +576,7 @@ fn add_expose( if spec.exposes(side).iter().any(|e| e.slot == slot) { return None; } - if flavour == Flavour::Masquerade && spec.has_stateful(side.other()) { + if flavour.is_directional() && spec.has_directional(side.other()) { return None; } draft @@ -570,7 +600,7 @@ fn set_flavour( flavour: Flavour, ) -> Option { let spec = draft.peerings.get(&peering)?; - if flavour == Flavour::Masquerade && spec.has_stateful(side.other()) { + if flavour.is_directional() && spec.has_directional(side.other()) { return None; } let index = spec.exposes(side).iter().position(|e| e.slot == slot)?; @@ -951,8 +981,13 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { } fn draw_flavour(driver: &mut D, spec: &PeeringSpec, side: Side) -> Option { - const ORDERED: [Flavour; 3] = [Flavour::Forward, Flavour::StaticNat, Flavour::Masquerade]; - let legal: &[Flavour] = if spec.has_stateful(side.other()) { + const ORDERED: [Flavour; 4] = [ + Flavour::Forward, + Flavour::StaticNat, + Flavour::PortForward, + Flavour::Masquerade, + ]; + let legal: &[Flavour] = if spec.has_directional(side.other()) { &ORDERED[..2] } else { &ORDERED @@ -1079,6 +1114,7 @@ mod tests { let masquerading = AtomicUsize::new(0); let forwarding = AtomicUsize::new(0); let static_nat = AtomicUsize::new(0); + let port_forward = AtomicUsize::new(0); check!() .with_generator(Sequence::default()) @@ -1103,6 +1139,7 @@ mod tests { Flavour::Masquerade => &masquerading, Flavour::Forward => &forwarding, Flavour::StaticNat => &static_nat, + Flavour::PortForward => &port_forward, } .fetch_add(1, Relaxed); } @@ -1121,12 +1158,14 @@ mod tests { assert!( masquerading.load(Relaxed) > 0 && forwarding.load(Relaxed) > 0 - && static_nat.load(Relaxed) > 0, - "not every flavour of expose was built (masquerade {}, forward {}, static nat {}), \ - so the combination rules were not exercised", + && static_nat.load(Relaxed) > 0 + && port_forward.load(Relaxed) > 0, + "not every flavour of expose was built (masquerade {}, forward {}, static nat {}, \ + port forward {}), so the combination rules were not exercised", masquerading.load(Relaxed), forwarding.load(Relaxed), - static_nat.load(Relaxed) + static_nat.load(Relaxed), + port_forward.load(Relaxed) ); } diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index ce25304e77..0ec7fffeeb 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -87,10 +87,7 @@ const REACH: &[(&str, Reach)] = &[ "VpcExpose.ips", Reach::Determined("one prefix, from the expose's peering, side and slot"), ), - ( - "VpcExpose.ips.ports", - Reach::Fixed("unset; the algebra exposes whole prefixes without port restrictions."), - ), + ("VpcExpose.ips.ports", Reach::Spans(&["set", "unset"])), ( "VpcExpose.nots", Reach::Fixed("empty; the algebra never excludes addresses from an expose."), @@ -102,7 +99,7 @@ const REACH: &[(&str, Reach)] = &[ ), ( "VpcExposeNat.as_range.ports", - Reach::Fixed("unset, for the same reason as `VpcExpose.ips.ports`."), + Reach::Spans(&["set", "unset"]), ), ( "VpcExposeNat.not_as", @@ -110,7 +107,7 @@ const REACH: &[(&str, Reach)] = &[ ), ( "VpcExposeNat.config", - Reach::Spans(&["masquerade", "static"]), + Reach::Spans(&["masquerade", "port-forwarding", "static"]), ), ( "VpcExposeNat.proto", @@ -127,9 +124,9 @@ const REACH: &[(&str, Reach)] = &[ ( "VpcExposePortForwarding.idle_timeout", Reach::Fixed( - "never constructed. `Flavour` has no port-forwarding member, so this flavour of \ - nat -- and every question about the idle timeout it carries -- stays out of \ - reach, which the design note names as missing vocabulary.", + "absent. `make_port_forwarding(None, ..)` is the only call, for the same reason \ + `VpcExposeMasquerade.idle_timeout` is absent: the flavour is reachable now, but \ + nothing asks for a timeout, so no flow ages out under a configuration that set one.", ), ), ]; diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 807e24d3f9..0b0ca38539 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -2840,6 +2840,7 @@ mod generated { static MIXED: AtomicU64 = AtomicU64::new(0); static PEERED: AtomicU64 = AtomicU64::new(0); static MULTI: AtomicU64 = AtomicU64::new(0); + static INBOUND: AtomicU64 = AtomicU64::new(0); bolero::check!() .with_max_len(MAX_INPUT_LEN) @@ -2872,6 +2873,11 @@ mod generated { let mut loads = loads_for(&validated, vary); DERIVED.fetch_add(loads.len() as u64, Ordering::Relaxed); + for load in &loads { + if load.describe().starts_with("[inbound") { + INBOUND.fetch_add(1, Ordering::Relaxed); + } + } for burst in run_schedule(fabric.worker(), &mut loads, schedule) { let mut seen = burst.clone(); @@ -2902,8 +2908,9 @@ mod generated { MULTI.load(Ordering::Relaxed), ); eprintln!( - "checked={checked} derived={derived} peered-configs={peered} \ - configs-past-two-vpcs={multi} mixed-bursts={mixed}" + "checked={checked} derived={derived} inbound={} peered-configs={peered} \ + configs-past-two-vpcs={multi} mixed-bursts={mixed}", + INBOUND.load(Ordering::Relaxed) ); super::assert_covered(peered > 0, "no generated configuration ever had a peering"); super::assert_covered( @@ -2916,6 +2923,11 @@ mod generated { "no generated configuration ever implied any traffic", ); super::assert_covered(checked > 0, "no derived sender ever completed its business"); + super::assert_covered( + INBOUND.load(Ordering::Relaxed) > 0, + "no generated configuration ever produced an inbound load, so a port-forwarding \ + expose was drawn into configurations and then carried no traffic at all", + ); super::assert_covered( mixed > 0, "no burst ever carried more than one sender's traffic, so nothing was interleaved", From 04b1e20aa0342c5a00f4fbca0e654ea6b55ae7c2 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 14:08:29 -0600 Subject: [PATCH 07/19] feat(config): generate peering ACLs Every generated peering permitted all traffic, so pipeline properties never exercised a configuration intended to deny a packet. Add coarse ACL shapes whose verdicts remain predictable without copying the production evaluator. Update traffic derivation to omit denied loads and retain guards proving ACL decisions are actually reached. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 159 +++++++++++++- config/src/external/overlay/completeness.rs | 5 +- dataplane/src/packet_processor/fuzz.rs | 232 +++++++++++++++----- 3 files changed, 330 insertions(+), 66 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 720e2f0f7a..9b13211a1a 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -6,10 +6,13 @@ use std::net::Ipv4Addr; use std::ops::Bound::Included; use bolero::{Driver, ValueGenerator}; -use lpm::prefix::{IpPrefix, Ipv4Prefix, PortRange, Prefix, PrefixWithOptionalPorts}; +use lpm::prefix::{ + IpPrefix, Ipv4Prefix, PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, +}; use crate::ConfigError; use crate::external::overlay::Overlay; +use crate::external::overlay::acl::{Acl, AclAction, AclPattern, AclProtoMatch, AclRule, AclScope}; use crate::external::overlay::vpc::{Vpc, VpcTable}; use crate::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeering, VpcPeeringTable}; @@ -57,6 +60,43 @@ impl Flavour { } } +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Default)] +pub enum Guard { + #[default] + Open, + Permit, + Deny, +} + +impl Guard { + fn acl(self, spec: &PeeringSpec) -> Option { + let (default, action) = match self { + Guard::Open => return None, + Guard::Permit => (AclAction::Deny, AclAction::Allow), + Guard::Deny => (AclAction::Allow, AclAction::Deny), + }; + let rule = |side: Side| { + let (from, to) = (spec.vpc(side).name(), spec.vpc(side.other()).name()); + AclRule { + name: format!("{from}-to-{to}"), + from, + to, + action, + pattern: AclPattern { + src: PrefixPortsSet::new(), + dst: PrefixPortsSet::new(), + src_any_ports: Vec::new(), + dst_any_ports: Vec::new(), + proto: AclProtoMatch::Any, + }, + scope: AclScope::Packet, + log: false, + } + }; + Some(Acl::new(default, vec![rule(Side::Left), rule(Side::Right)])) + } +} + pub const MAX_EXPOSES: u8 = 4; pub const MAX_VPCS: u8 = 6; @@ -179,6 +219,7 @@ pub struct PeeringSpec { left: VpcHandle, right: VpcHandle, exposes: [Vec; 2], + guard: Guard, } impl PeeringSpec { @@ -190,6 +231,11 @@ impl PeeringSpec { } } + #[must_use] + pub fn guard(&self) -> Guard { + self.guard + } + #[must_use] pub fn exposes(&self, side: Side) -> &[ExposeSpec] { &self.exposes[side.index()] @@ -257,6 +303,14 @@ impl Draft { .map(|(handle, _)| *handle) } + #[must_use] + pub fn guard_named(&self, name: &str) -> Option { + self.peerings + .iter() + .find(|(handle, _)| handle.name() == name) + .map(|(_, spec)| spec.guard) + } + #[must_use] pub fn components(&self) -> Vec> { let mut unvisited: BTreeSet = self.vpcs.clone(); @@ -296,11 +350,13 @@ impl Draft { let mut peerings = VpcPeeringTable::new(); for (handle, spec) in self.peerings() { - peerings.add(VpcPeering::with_default_group( + let mut peering = VpcPeering::with_default_group( &handle.name(), spec.manifest(handle, Side::Left), spec.manifest(handle, Side::Right), - ))?; + ); + peering.acl = spec.guard.acl(spec); + peerings.add(peering)?; } Ok(Overlay::new(vpc_table, peerings)) @@ -373,6 +429,10 @@ pub enum Op { slot: u8, flavour: Flavour, }, + SetGuard { + peering: PeeringHandle, + guard: Guard, + }, } #[derive(Clone, PartialEq, Eq, Debug)] @@ -401,6 +461,10 @@ pub enum Undo { slot: u8, flavour: Flavour, }, + SetGuard { + peering: PeeringHandle, + guard: Guard, + }, } impl Op { @@ -411,7 +475,8 @@ impl Op { Op::AddPeering { left, right, .. } => Footprint::of([*left, *right], []), Op::AddExpose { peering, .. } | Op::RemoveExpose { peering, .. } - | Op::SetFlavour { peering, .. } => Footprint::of([], [*peering]), + | Op::SetFlavour { peering, .. } + | Op::SetGuard { peering, .. } => Footprint::of([], [*peering]), } } @@ -449,7 +514,8 @@ impl Op { } Op::AddExpose { peering, .. } | Op::RemoveExpose { peering, .. } - | Op::SetFlavour { peering, .. } => Footprint::of([], [*peering]), + | Op::SetFlavour { peering, .. } + | Op::SetGuard { peering, .. } => Footprint::of([], [*peering]), } } @@ -493,6 +559,7 @@ impl Op { left, right, exposes: [vec![first(0)], vec![first(0)]], + guard: Guard::Open, }, ); Some(Undo::RemovePeering(handle)) @@ -535,6 +602,15 @@ impl Op { slot, flavour, } => set_flavour(draft, peering, side, slot, flavour), + + Op::SetGuard { peering, guard } => { + let spec = draft.peerings.get_mut(&peering)?; + let previous = std::mem::replace(&mut spec.guard, guard); + Some(Undo::SetGuard { + peering, + guard: previous, + }) + } } } } @@ -695,6 +771,13 @@ impl Undo { .unwrap_or_else(|| unreachable!("no expose in slot {slot}")); spec.exposes_mut(*side)[index].flavour = *flavour; } + Undo::SetGuard { peering, guard } => { + draft + .peerings + .get_mut(peering) + .unwrap_or_else(|| unreachable!("no peering {peering:?}")) + .guard = *guard; + } } } } @@ -759,7 +842,7 @@ impl ValueGenerator for Sequence { } } -const MENU: [(Kind, u8); 7] = [ +const MENU: [(Kind, u8); 8] = [ (Kind::AddVpc, 4), (Kind::RemoveVpc, 1), (Kind::AddPeering, 4), @@ -767,6 +850,7 @@ const MENU: [(Kind, u8); 7] = [ (Kind::AddExpose, 2), (Kind::RemoveExpose, 1), (Kind::SetFlavour, 2), + (Kind::SetGuard, 2), ]; #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -778,6 +862,7 @@ enum Kind { AddExpose, RemoveExpose, SetFlavour, + SetGuard, } impl Kind { @@ -794,7 +879,7 @@ impl Kind { let vpcs = draft.vpcs.len(); next_peering < u8::MAX && vpcs >= 2 && draft.peerings.len() < vpcs * (vpcs - 1) / 2 } - Kind::RemovePeering | Kind::SetFlavour => !draft.peerings.is_empty(), + Kind::RemovePeering | Kind::SetFlavour | Kind::SetGuard => !draft.peerings.is_empty(), Kind::AddExpose => sides().any(|exposes| exposes.len() < usize::from(MAX_EXPOSES)), Kind::RemoveExpose => sides().any(|exposes| exposes.len() > 1), } @@ -824,6 +909,7 @@ fn draw( Kind::AddExpose => draw_add_expose(driver, draft), Kind::RemoveExpose => draw_remove_expose(driver, draft), Kind::SetFlavour => draw_set_flavour(driver, draft), + Kind::SetGuard => draw_set_guard(driver, draft), }; let Some(op) = built else { @@ -980,6 +1066,16 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { }) } +fn draw_set_guard(driver: &mut D, draft: &Draft) -> Option { + const ORDERED: [Guard; 3] = [Guard::Open, Guard::Permit, Guard::Deny]; + + let peerings: Vec = draft.peerings().map(|(handle, _)| handle).collect(); + Some(Op::SetGuard { + peering: pick(driver, &peerings)?, + guard: pick(driver, &ORDERED)?, + }) +} + fn draw_flavour(driver: &mut D, spec: &PeeringSpec, side: Side) -> Option { const ORDERED: [Flavour; 4] = [ Flavour::Forward, @@ -1067,7 +1163,8 @@ mod tests { ); } - static DRAWN: [AtomicUsize; 7] = [ + static DRAWN: [AtomicUsize; 8] = [ + AtomicUsize::new(0), AtomicUsize::new(0), AtomicUsize::new(0), AtomicUsize::new(0), @@ -1077,7 +1174,7 @@ mod tests { AtomicUsize::new(0), ]; - const KINDS: [&str; 7] = [ + const KINDS: [&str; 8] = [ "AddVpc", "RemoveVpc", "AddPeering", @@ -1085,6 +1182,7 @@ mod tests { "AddExpose", "RemoveExpose", "SetFlavour", + "SetGuard", ]; fn record(op: Op) { @@ -1096,6 +1194,7 @@ mod tests { Op::AddExpose { .. } => 4, Op::RemoveExpose { .. } => 5, Op::SetFlavour { .. } => 6, + Op::SetGuard { .. } => 7, }; DRAWN[index].fetch_add(1, Relaxed); } @@ -1115,6 +1214,11 @@ mod tests { let forwarding = AtomicUsize::new(0); let static_nat = AtomicUsize::new(0); let port_forward = AtomicUsize::new(0); + let guards = [ + AtomicUsize::new(0), + AtomicUsize::new(0), + AtomicUsize::new(0), + ]; check!() .with_generator(Sequence::default()) @@ -1149,6 +1253,33 @@ mod tests { let overlay = draft .overlay() .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")); + + for (handle, spec) in draft.peerings() { + let acl = overlay + .peering_table + .values() + .find(|peering| peering.name == handle.name()) + .and_then(|peering| peering.acl.as_ref()); + let observed = match acl.map(Acl::default_action) { + None => Guard::Open, + Some(AclAction::Deny) => Guard::Permit, + Some(AclAction::Allow) => Guard::Deny, + }; + assert_eq!( + observed, + spec.guard(), + "{:?} is guarded {:?} and assembled an acl reading {observed:?}", + handle, + spec.guard() + ); + guards[match observed { + Guard::Open => 0, + Guard::Permit => 1, + Guard::Deny => 2, + }] + .fetch_add(1, Relaxed); + } + if let Err(e) = overlay.validate() { panic!("{ops:?} builds a configuration the validator refuses: {e}"); } @@ -1167,6 +1298,16 @@ mod tests { static_nat.load(Relaxed), port_forward.load(Relaxed) ); + for (name, count) in ["open", "permit", "deny"].iter().zip(&guards) { + assert!( + count.load(Relaxed) > 0, + "no peering was ever left {name} (open {}, permit {}, deny {}), so the acl \ + vocabulary was not exercised", + guards[0].load(Relaxed), + guards[1].load(Relaxed), + guards[2].load(Relaxed) + ); + } } #[test] diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index 0ec7fffeeb..093a56408e 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -67,10 +67,7 @@ const REACH: &[(&str, Reach)] = &[ "VpcPeering.gwgroup", Reach::Fixed("the default group; the algebra only calls `with_default_group`."), ), - ( - "VpcPeering.acl", - Reach::Fixed("absent; the algebra has no operation that adds peering ACLs."), - ), + ("VpcPeering.acl", Reach::Spans(&["absent", "present"])), ( "VpcManifest.name", Reach::Determined("the side's vpc handle"), diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 0b0ca38539..e7354c2def 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -744,6 +744,7 @@ pub(crate) mod derive { use super::routed::{Blast, Conversation, Inbound}; use super::*; use config::external::overlay::ValidatedOverlay; + use config::external::overlay::algebra::{Draft, Guard}; use config::external::overlay::vpcpeering::ValidatedExpose; use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; @@ -824,6 +825,18 @@ pub(crate) mod derive { loads_where(overlay, vary, &|_| true) } + pub(crate) fn carried_by(draft: &Draft) -> impl Fn(Named<'_>) -> bool + '_ { + move |named| draft.guard_named(named.peering) != Some(Guard::Deny) + } + + pub(crate) fn loads_carried( + overlay: &ValidatedOverlay, + vary: &[Vary], + draft: &Draft, + ) -> Vec> { + loads_where(overlay, vary, &carried_by(draft)) + } + #[derive(Debug, Clone, Copy)] pub(crate) struct Named<'a> { pub(crate) local: &'a str, @@ -2778,16 +2791,70 @@ mod offers { #[cfg(test)] mod generated { - use super::derive::{Vary, loads_for}; + use super::derive::{Named, Vary, loads_where}; use super::*; use bolero::ValueGenerator; use concurrency::process_global::atomic::{AtomicU64, Ordering}; - use config::external::overlay::algebra::{Op, Sequence}; + use config::external::overlay::algebra::{Draft, Guard, Op, Sequence}; + use std::cell::Cell; use std::ops::Bound::Included; const SENDERS: usize = 6; const POLLS: usize = 8; + static CHECKED: AtomicU64 = AtomicU64::new(0); + static DERIVED: AtomicU64 = AtomicU64::new(0); + static MIXED: AtomicU64 = AtomicU64::new(0); + static PEERED: AtomicU64 = AtomicU64::new(0); + static MULTI: AtomicU64 = AtomicU64::new(0); + static INBOUND: AtomicU64 = AtomicU64::new(0); + static PERMITTING: AtomicU64 = AtomicU64::new(0); + + fn report_and_assert_coverage() { + let (checked, derived, mixed) = ( + CHECKED.load(Ordering::Relaxed), + DERIVED.load(Ordering::Relaxed), + MIXED.load(Ordering::Relaxed), + ); + let (peered, multi) = ( + PEERED.load(Ordering::Relaxed), + MULTI.load(Ordering::Relaxed), + ); + eprintln!( + "checked={checked} derived={derived} inbound={} \ + permitting-peerings={} peered-configs={peered} \ + configs-past-two-vpcs={multi} mixed-bursts={mixed}", + INBOUND.load(Ordering::Relaxed), + PERMITTING.load(Ordering::Relaxed) + ); + super::assert_covered(peered > 0, "no generated configuration ever had a peering"); + super::assert_covered( + multi > 0, + "no generated configuration ever had more than two vpcs, so this reached nothing the \ + two-vpc fixtures do not", + ); + super::assert_covered( + derived > 0, + "no generated configuration ever implied any traffic", + ); + super::assert_covered(checked > 0, "no derived sender ever completed its business"); + super::assert_covered( + INBOUND.load(Ordering::Relaxed) > 0, + "no generated configuration ever produced an inbound load, so a port-forwarding \ + expose was drawn into configurations and then carried no traffic at all", + ); + super::assert_covered( + PERMITTING.load(Ordering::Relaxed) > 0, + "no traffic was ever derived across a peering whose acl permits it, so every load here \ + ran with no acl in the way and a rule set that lowered to nothing would have gone \ + unnoticed", + ); + super::assert_covered( + mixed > 0, + "no burst ever carried more than one sender's traffic, so nothing was interleaved", + ); + } + pub(super) struct Generated; impl ValueGenerator for Generated { @@ -2832,16 +2899,23 @@ mod generated { super::assert_within_budget("generated::Generated", &Generated); } + fn carried_counting<'a>( + draft: &'a Draft, + permitting: &'a Cell, + ) -> impl Fn(Named<'_>) -> bool + 'a { + move |named| match draft.guard_named(named.peering) { + Some(Guard::Deny) => false, + Some(Guard::Permit) => { + permitting.set(permitting.get() + 1); + true + } + Some(Guard::Open) | None => true, + } + } + #[tokio::test] #[dpdk::with_eal] async fn a_generated_configuration_carries_its_own_traffic() { - static CHECKED: AtomicU64 = AtomicU64::new(0); - static DERIVED: AtomicU64 = AtomicU64::new(0); - static MIXED: AtomicU64 = AtomicU64::new(0); - static PEERED: AtomicU64 = AtomicU64::new(0); - static MULTI: AtomicU64 = AtomicU64::new(0); - static INBOUND: AtomicU64 = AtomicU64::new(0); - bolero::check!() .with_max_len(MAX_INPUT_LEN) .with_generator(Generated) @@ -2871,7 +2945,10 @@ mod generated { let mut fabric = Fabric::routed_over_validated(&validated, topology(&vnis)); - let mut loads = loads_for(&validated, vary); + let permitting = Cell::new(0); + let mut loads = + loads_where(&validated, vary, &carried_counting(&draft, &permitting)); + PERMITTING.fetch_add(permitting.get(), Ordering::Relaxed); DERIVED.fetch_add(loads.len() as u64, Ordering::Relaxed); for load in &loads { if load.describe().starts_with("[inbound") { @@ -2898,39 +2975,75 @@ mod generated { } }); - let (checked, derived, mixed) = ( - CHECKED.load(Ordering::Relaxed), - DERIVED.load(Ordering::Relaxed), - MIXED.load(Ordering::Relaxed), - ); - let (peered, multi) = ( - PEERED.load(Ordering::Relaxed), - MULTI.load(Ordering::Relaxed), - ); - eprintln!( - "checked={checked} derived={derived} inbound={} peered-configs={peered} \ - configs-past-two-vpcs={multi} mixed-bursts={mixed}", - INBOUND.load(Ordering::Relaxed) - ); - super::assert_covered(peered > 0, "no generated configuration ever had a peering"); - super::assert_covered( - multi > 0, - "no generated configuration ever had more than two vpcs, so this reached nothing the \ - two-vpc fixtures do not", - ); - super::assert_covered( - derived > 0, - "no generated configuration ever implied any traffic", + report_and_assert_coverage(); + } + + #[tokio::test] + #[dpdk::with_eal] + async fn a_denied_peering_carries_nothing() { + static SENT: AtomicU64 = AtomicU64::new(0); + static BY_ACL: AtomicU64 = AtomicU64::new(0); + static CONFIGS: AtomicU64 = AtomicU64::new(0); + + bolero::check!() + .with_max_len(MAX_INPUT_LEN) + .with_generator(Generated) + .for_each(|(ops, vary, _schedule)| { + let draft = Sequence::fold(ops); + let validated = draft + .overlay() + .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")) + .validate() + .unwrap_or_else(|e| panic!("{ops:?} does not validate: {e}")); + + let vnis: Vec = validated + .vpc_table() + .values() + .map(config::external::overlay::vpc::ValidatedVpc::vni) + .collect(); + if vnis.is_empty() { + return; + } + let carried = super::derive::carried_by(&draft); + let mut loads = loads_where(&validated, vary, &|named| !carried(named)); + if loads.is_empty() { + return; + } + CONFIGS.fetch_add(1, Ordering::Relaxed); + + let mut fabric = Fabric::routed_over_validated(&validated, topology(&vnis)); + for load in &mut loads { + let Some(packet) = load.next() else { + continue; + }; + let seen = verdict(&fabric.worker().send(packet)); + SENT.fetch_add(1, Ordering::Relaxed); + assert!( + matches!(seen, Verdict::Dropped(_)), + "a peering whose acl denies everything it carries produced {seen:?} for {}", + load.describe() + ); + if seen == Verdict::Dropped(DoneReason::AclDropped) { + BY_ACL.fetch_add(1, Ordering::Relaxed); + } + } + }); + + let (configs, sent, by_acl) = ( + CONFIGS.load(Ordering::Relaxed), + SENT.load(Ordering::Relaxed), + BY_ACL.load(Ordering::Relaxed), ); - super::assert_covered(checked > 0, "no derived sender ever completed its business"); + eprintln!("denied-configs={configs} sent={sent} (dropped by the acl {by_acl})"); super::assert_covered( - INBOUND.load(Ordering::Relaxed) > 0, - "no generated configuration ever produced an inbound load, so a port-forwarding \ - expose was drawn into configurations and then carried no traffic at all", + sent > 0, + "no denied peering ever had traffic derived for it, so this asserted nothing about \ + any packet", ); super::assert_covered( - mixed > 0, - "no burst ever carried more than one sender's traffic, so nothing was interleaved", + by_acl > 0, + "no packet was ever dropped by the acl: the claim is being satisfied by stages ahead \ + of it and would hold with the acl removed", ); } } @@ -4410,7 +4523,7 @@ mod icmp_error { #[cfg(test)] mod model { - use super::derive::loads_for; + use super::derive::loads_carried; use super::routed::{Conversation, exposes, inner, inside, tunnelled}; use super::*; use concurrency::process_global::OnceLock; @@ -4419,7 +4532,7 @@ mod model { use concurrency::thread; #[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] use concurrency::thread::BuilderExt; - use config::external::overlay::algebra::{Footprint, Sequence}; + use config::external::overlay::algebra::{Draft, Footprint, Sequence}; use net::packet::test_utils::build_test_udp_ipv4_packet; type Tuple = (Option, Option); @@ -4655,7 +4768,8 @@ mod model { .with_generator(generated::Generated) .with_iterations(CASES) .for_each(|(ops, vary, schedule)| { - let validated = Sequence::fold(ops) + let draft = Sequence::fold(ops); + let validated = draft .overlay() .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")) .validate() @@ -4666,18 +4780,23 @@ mod model { .values() .map(config::external::overlay::vpc::ValidatedVpc::vni) .collect(); - if vnis.is_empty() || loads_for(&validated, vary).len() < 2 { + if vnis.is_empty() || loads_carried(&validated, vary, &draft).len() < 2 { THIN.fetch_add(1, Ordering::Relaxed); return; } SPLIT.fetch_add(1, Ordering::Relaxed); - let drawn = - concurrency::sync::Arc::new((validated, vnis, vary.clone(), schedule.clone())); + let drawn = concurrency::sync::Arc::new(( + validated, + vnis, + vary.clone(), + schedule.clone(), + draft, + )); let entering = handle.clone(); concurrency::stress(move || { - let (validated, vnis, vary, schedule) = &*drawn; + let (validated, vnis, vary, schedule, draft) = &*drawn; let tables = topology(vnis); let fleet = Fleet::lowering(validated, Some(&tables), Arc::new(FlowTable::default())); @@ -4696,7 +4815,7 @@ mod model { tracectl::evidence::capture(format!("worker-{which}")); let mut worker = blueprint.worker(); let mut mine: Vec> = - loads_for(validated, vary) + loads_carried(validated, vary, draft) .into_iter() .enumerate() .filter(|(nth, _)| nth % 2 == which) @@ -5528,9 +5647,14 @@ mod model { within + u16::try_from(which).unwrap_or_else(|_| unreachable!()) * 32_768 } - fn outside(footprint: &Footprint) -> impl Fn(derive::Named<'_>) -> bool + '_ { + fn outside<'a>( + footprint: &'a Footprint, + draft: &'a Draft, + ) -> impl Fn(derive::Named<'_>) -> bool + 'a { + let carried = derive::carried_by(draft); move |named| { - !footprint.touches_peering_named(named.peering) + carried(named) + && !footprint.touches_peering_named(named.peering) && !footprint.touches_vpc_named(named.local) && !footprint.touches_vpc_named(named.remote) } @@ -5571,8 +5695,9 @@ mod model { let running = assemble(&before); let enacted = assemble(&Sequence::fold(ops)); - let framed = derive::loads_where(&running, vary, &outside(&footprint)).len(); - let total = derive::loads_for(&running, vary).len(); + let framed = + derive::loads_where(&running, vary, &outside(&footprint, &before)).len(); + let total = derive::loads_carried(&running, vary, &before).len(); FRAMED_OUT.fetch_add( u64::try_from(total - framed).unwrap_or_else(|_| unreachable!()), Ordering::Relaxed, @@ -5600,11 +5725,12 @@ mod model { vary.clone(), footprint, *change, + before, )); let entering = handle.clone(); concurrency::stress(move || { - let (running, enacted, vnis, vary, footprint, change) = &*drawn; + let (running, enacted, vnis, vary, footprint, change, before) = &*drawn; let tables = topology(vnis); let fleet = Fleet::lowering(running, Some(&tables), Arc::new(FlowTable::default())); @@ -5625,7 +5751,7 @@ mod model { let _evidence = tracectl::evidence::capture(format!("framed-{which}")); let mut worker = blueprint.worker(); - let outside = outside(footprint); + let outside = outside(footprint, before); gate.wait(); let mut seen = Vec::new(); for round in 1..=ROUNDS { From 1d39b2e24e61f2598a06deccaddb3501f413a7b0 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 14:08:39 -0600 Subject: [PATCH 08/19] test(config): measure ACL schema coverage The completeness survey stopped at whether a peering had an ACL, hiding fixed rule, pattern, and scope fields behind a single present value. Extend the census through the entire ACL schema. This reports eight pre-existing fixed degrees of freedom without claiming the algebra gained new reach. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/acl.rs | 4 +- config/src/external/overlay/completeness.rs | 126 ++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/config/src/external/overlay/acl.rs b/config/src/external/overlay/acl.rs index ad4b057d78..aa546489c3 100644 --- a/config/src/external/overlay/acl.rs +++ b/config/src/external/overlay/acl.rs @@ -424,8 +424,8 @@ impl ValidatedAclRule { #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct Acl { - default: AclAction, - rules: Vec, + pub(crate) default: AclAction, + pub(crate) rules: Vec, } impl Acl { diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index 093a56408e..aad7840b6b 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -7,6 +7,7 @@ use std::collections::{BTreeMap, BTreeSet}; use lpm::prefix::with_ports::{L4Protocol, PrefixPortsSet}; use super::Overlay; +use super::acl::{Acl, AclAction, AclPattern, AclProtoMatch, AclRule, AclScope}; use super::algebra::Sequence; use super::vpc::Vpc; use super::vpcpeering::{ @@ -68,6 +69,71 @@ const REACH: &[(&str, Reach)] = &[ Reach::Fixed("the default group; the algebra only calls `with_default_group`."), ), ("VpcPeering.acl", Reach::Spans(&["absent", "present"])), + ("Acl.default", Reach::Spans(&["allow", "deny"])), + ( + "Acl.rules", + Reach::Fixed( + "two, one per direction of the peering. So rule *precedence* is unreachable: a lookup \ + returns the first rule that matches, and with one rule per direction no packet ever \ + matches two. An ACL whose rules overlap is where an ordering defect would live.", + ), + ), + ( + "AclRule.name", + Reach::Determined("the two vpc handles, as `-to-`"), + ), + ( + "AclRule.from", + Reach::Determined("the vpc handle on the rule's side"), + ), + ( + "AclRule.to", + Reach::Determined("the vpc handle on the other side"), + ), + ("AclRule.action", Reach::Spans(&["allow", "deny"])), + ( + "AclRule.scope", + Reach::Fixed( + "`Packet`. A flow-scoped rule authorises a reply by its membership of the flow the \ + request opened, which is a whole mechanism -- `reverse_summary`, and the reverse \ + lookup in `AclFilter::lookup` -- that no generated configuration reaches. Drawing it \ + means drawing `validate_scope`'s condition too; see `algebra::Guard::acl`.", + ), + ), + ( + "AclRule.log", + Reach::Fixed("false. Nothing generated asks for a rule's verdict to be logged."), + ), + ( + "AclPattern.src", + Reach::Fixed( + "empty, which `AclRule::validate` fills in from the `from` manifest -- so every \ + generated rule covers its whole side. A rule matching *part* of what a peering \ + carries, which is what an ACL is normally for, is unreachable.", + ), + ), + ( + "AclPattern.dst", + Reach::Fixed("empty, for the same reason as `AclPattern.src`."), + ), + ( + "AclPattern.src_any_ports", + Reach::Fixed( + "empty -- the survey renders it as a count of zero. A `match` naming ports but no \ + address is a shape the k8s converter produces and nothing generated does.", + ), + ), + ( + "AclPattern.dst_any_ports", + Reach::Fixed("empty, for the same reason as `AclPattern.src_any_ports`."), + ), + ( + "AclPattern.proto", + Reach::Fixed( + "`Any`. Narrowing a rule to a protocol is what `acl_filter`'s own generator is aimed \ + at, and a rule that discriminates is one a property here would have to evaluate.", + ), + ), ( "VpcManifest.name", Reach::Determined("the side's vpc handle"), @@ -199,6 +265,9 @@ fn survey(overlay: &Overlay, seen: &mut Observed) { if acl.is_some() { "present" } else { "absent" }, ); seen.note("VpcPeering.no_multipath", no_multipath.to_string()); + if let Some(acl) = acl { + survey_acl(acl, seen); + } for (side, manifest) in [("VpcPeering.left", left), ("VpcPeering.right", right)] { seen.note(side, manifest.name.clone()); survey_manifest(manifest, seen); @@ -206,6 +275,63 @@ fn survey(overlay: &Overlay, seen: &mut Observed) { } } +fn survey_acl(acl: &Acl, seen: &mut Observed) { + let Acl { default, rules } = acl; + seen.note("Acl.default", action(*default)); + seen.count("Acl.rules", rules.len()); + for rule in rules { + let AclRule { + name, + from, + to, + action: verdict, + pattern, + scope, + log, + } = rule; + seen.note("AclRule.name", name.clone()); + seen.note("AclRule.from", from.clone()); + seen.note("AclRule.to", to.clone()); + seen.note("AclRule.action", action(*verdict)); + seen.note( + "AclRule.scope", + match scope { + AclScope::Flow => "flow", + AclScope::Packet => "packet", + }, + ); + seen.note("AclRule.log", log.to_string()); + + let AclPattern { + src, + dst, + src_any_ports, + dst_any_ports, + proto, + } = pattern; + seen.prefixes("AclPattern.src", src); + seen.prefixes("AclPattern.dst", dst); + seen.count("AclPattern.src_any_ports", src_any_ports.len()); + seen.count("AclPattern.dst_any_ports", dst_any_ports.len()); + seen.note( + "AclPattern.proto", + match proto { + AclProtoMatch::Tcp => "tcp".to_owned(), + AclProtoMatch::Udp => "udp".to_owned(), + AclProtoMatch::Other(number) => format!("other({number})"), + AclProtoMatch::Any => "any".to_owned(), + }, + ); + } +} + +fn action(action: AclAction) -> &'static str { + match action { + AclAction::Allow => "allow", + AclAction::Deny => "deny", + } +} + fn survey_manifest(manifest: &VpcManifest, seen: &mut Observed) { let VpcManifest { name, exposes } = manifest; seen.note("VpcManifest.name", name.clone()); From 877bf75493f2845e751d34d8b227e361c4a3d097 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 14:40:01 -0600 Subject: [PATCH 09/19] feat(config): generate stateful peering ACLs Packet-level ACL rules never exercise reverse-flow permission, where the verdict depends on NAT state. Generate permit-by-flow peerings and verify their replies through AclFilter's reverse lookup. The first run exposes and pins a separate defect: port-forwarded flows retain generation one and lose permission after any configuration change. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 182 ++++++++++++++------ config/src/external/overlay/completeness.rs | 19 +- dataplane/src/packet_processor/fuzz.rs | 31 +++- 3 files changed, 154 insertions(+), 78 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 9b13211a1a..a0b8b86829 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -65,15 +65,17 @@ pub enum Guard { #[default] Open, Permit, + PermitFlow, Deny, } impl Guard { fn acl(self, spec: &PeeringSpec) -> Option { - let (default, action) = match self { + let (default, action, scope) = match self { Guard::Open => return None, - Guard::Permit => (AclAction::Deny, AclAction::Allow), - Guard::Deny => (AclAction::Allow, AclAction::Deny), + Guard::Permit => (AclAction::Deny, AclAction::Allow, AclScope::Packet), + Guard::PermitFlow => (AclAction::Deny, AclAction::Allow, AclScope::Flow), + Guard::Deny => (AclAction::Allow, AclAction::Deny, AclScope::Packet), }; let rule = |side: Side| { let (from, to) = (spec.vpc(side).name(), spec.vpc(side.other()).name()); @@ -89,11 +91,32 @@ impl Guard { dst_any_ports: Vec::new(), proto: AclProtoMatch::Any, }, - scope: AclScope::Packet, + scope, log: false, } }; - Some(Acl::new(default, vec![rule(Side::Left), rule(Side::Right)])) + let rules = match self.opening_side(spec) { + Some(side) => vec![rule(side)], + None => vec![rule(Side::Left), rule(Side::Right)], + }; + Some(Acl::new(default, rules)) + } + + fn opening_side(self, spec: &PeeringSpec) -> Option { + match self { + Guard::PermitFlow => Some( + spec.sole_opener() + .unwrap_or_else(|| unreachable!("`legal_on` refused a guard with no side")), + ), + Guard::Open | Guard::Permit | Guard::Deny => None, + } + } + + fn legal_on(self, spec: &PeeringSpec) -> bool { + match self { + Guard::Open | Guard::Permit | Guard::Deny => true, + Guard::PermitFlow => spec.sole_opener().is_some(), + } } } @@ -259,6 +282,23 @@ impl PeeringSpec { } } + fn sole_opener(&self) -> Option { + [Side::Left, Side::Right].into_iter().find_map(|side| { + let exposes = self.exposes(side); + if exposes.is_empty() { + return None; + } + let all = |flavour| exposes.iter().all(|expose| expose.flavour == flavour); + if all(Flavour::Masquerade) { + Some(side) + } else if all(Flavour::PortForward) { + Some(side.other()) + } else { + None + } + }) + } + fn has_directional(&self, side: Side) -> bool { self.exposes(side) .iter() @@ -605,6 +645,9 @@ impl Op { Op::SetGuard { peering, guard } => { let spec = draft.peerings.get_mut(&peering)?; + if !guard.legal_on(spec) { + return None; + } let previous = std::mem::replace(&mut spec.guard, guard); Some(Undo::SetGuard { peering, @@ -661,11 +704,24 @@ fn add_expose( .unwrap_or_else(|| unreachable!("just found it")) .exposes_mut(side) .push(ExposeSpec { slot, flavour }); - Some(Undo::RemoveExpose { + respecting_guard( + draft, peering, - side, - slot, - }) + Undo::RemoveExpose { + peering, + side, + slot, + }, + ) +} + +fn respecting_guard(draft: &mut Draft, peering: PeeringHandle, undo: Undo) -> Option { + let spec = draft.peerings.get(&peering)?; + if spec.guard.legal_on(spec) { + return Some(undo); + } + undo.apply(draft); + None } fn set_flavour( @@ -687,12 +743,16 @@ fn set_flavour( .exposes_mut(side); let previous = exposes[index].flavour; exposes[index].flavour = flavour; - Some(Undo::SetFlavour { + respecting_guard( + draft, peering, - side, - slot, - flavour: previous, - }) + Undo::SetFlavour { + peering, + side, + slot, + flavour: previous, + }, + ) } impl Undo { @@ -912,7 +972,7 @@ fn draw( Kind::SetGuard => draw_set_guard(driver, draft), }; - let Some(op) = built else { + let Some(op) = built.filter(|op| op.applicable(draft)) else { menu.retain(|other| *other != kind); continue; }; @@ -1067,12 +1127,18 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { } fn draw_set_guard(driver: &mut D, draft: &Draft) -> Option { - const ORDERED: [Guard; 3] = [Guard::Open, Guard::Permit, Guard::Deny]; + const ORDERED: [Guard; 4] = [Guard::Open, Guard::Permit, Guard::PermitFlow, Guard::Deny]; + + let guard = pick(driver, &ORDERED)?; + let willing: Vec = draft + .peerings() + .filter(|(_, spec)| guard.legal_on(spec)) + .map(|(handle, _)| handle) + .collect(); - let peerings: Vec = draft.peerings().map(|(handle, _)| handle).collect(); Some(Op::SetGuard { - peering: pick(driver, &peerings)?, - guard: pick(driver, &ORDERED)?, + peering: pick(driver, &willing)?, + guard, }) } @@ -1210,15 +1276,8 @@ mod tests { #[test] fn every_sequence_builds_a_valid_configuration() { - let masquerading = AtomicUsize::new(0); - let forwarding = AtomicUsize::new(0); - let static_nat = AtomicUsize::new(0); - let port_forward = AtomicUsize::new(0); - let guards = [ - AtomicUsize::new(0), - AtomicUsize::new(0), - AtomicUsize::new(0), - ]; + let flavours = [const { AtomicUsize::new(0) }; 4]; + let guards = [const { AtomicUsize::new(0) }; 4]; check!() .with_generator(Sequence::default()) @@ -1239,12 +1298,12 @@ mod tests { for (_, spec) in draft.peerings() { for side in [Side::Left, Side::Right] { for expose in spec.exposes(side) { - match expose.flavour() { - Flavour::Masquerade => &masquerading, - Flavour::Forward => &forwarding, - Flavour::StaticNat => &static_nat, - Flavour::PortForward => &port_forward, - } + flavours[match expose.flavour() { + Flavour::Forward => 0, + Flavour::Masquerade => 1, + Flavour::StaticNat => 2, + Flavour::PortForward => 3, + }] .fetch_add(1, Relaxed); } } @@ -1260,10 +1319,11 @@ mod tests { .values() .find(|peering| peering.name == handle.name()) .and_then(|peering| peering.acl.as_ref()); - let observed = match acl.map(Acl::default_action) { + let observed = match acl.map(|acl| (acl.default_action(), acl.rules().len())) { None => Guard::Open, - Some(AclAction::Deny) => Guard::Permit, - Some(AclAction::Allow) => Guard::Deny, + Some((AclAction::Deny, 1)) => Guard::PermitFlow, + Some((AclAction::Deny, _)) => Guard::Permit, + Some((AclAction::Allow, _)) => Guard::Deny, }; assert_eq!( observed, @@ -1275,7 +1335,8 @@ mod tests { guards[match observed { Guard::Open => 0, Guard::Permit => 1, - Guard::Deny => 2, + Guard::PermitFlow => 2, + Guard::Deny => 3, }] .fetch_add(1, Relaxed); } @@ -1286,26 +1347,35 @@ mod tests { }); assert_every_kind_drawn(); - assert!( - masquerading.load(Relaxed) > 0 - && forwarding.load(Relaxed) > 0 - && static_nat.load(Relaxed) > 0 - && port_forward.load(Relaxed) > 0, - "not every flavour of expose was built (masquerade {}, forward {}, static nat {}, \ - port forward {}), so the combination rules were not exercised", - masquerading.load(Relaxed), - forwarding.load(Relaxed), - static_nat.load(Relaxed), - port_forward.load(Relaxed) - ); - for (name, count) in ["open", "permit", "deny"].iter().zip(&guards) { + assert_every_shape_built(&flavours, &guards); + } + + const FLAVOURS: [&str; 4] = ["forward", "masquerade", "static-nat", "port-forward"]; + const GUARDS: [&str; 4] = ["open", "permit", "permit-by-flow", "deny"]; + + fn assert_every_shape_built(flavours: &[AtomicUsize; 4], guards: &[AtomicUsize; 4]) { + let show = |names: [&str; 4], counts: &[AtomicUsize; 4]| { + names + .iter() + .zip(counts) + .map(|(name, count)| format!("{name}={}", count.load(Relaxed))) + .collect::>() + .join(" ") + }; + let (built, set) = (show(FLAVOURS, flavours), show(GUARDS, guards)); + eprintln!("exposes: {built}\nguards: {set}"); + + for (name, count) in FLAVOURS.iter().zip(flavours) { + assert!( + count.load(Relaxed) > 0, + "no expose was ever {name} ({built}), so the nat combination rules were not \ + exercised" + ); + } + for (name, count) in GUARDS.iter().zip(guards) { assert!( count.load(Relaxed) > 0, - "no peering was ever left {name} (open {}, permit {}, deny {}), so the acl \ - vocabulary was not exercised", - guards[0].load(Relaxed), - guards[1].load(Relaxed), - guards[2].load(Relaxed) + "no peering was ever left {name} ({set}), so the acl vocabulary was not exercised" ); } } diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index aad7840b6b..739351841d 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -70,14 +70,7 @@ const REACH: &[(&str, Reach)] = &[ ), ("VpcPeering.acl", Reach::Spans(&["absent", "present"])), ("Acl.default", Reach::Spans(&["allow", "deny"])), - ( - "Acl.rules", - Reach::Fixed( - "two, one per direction of the peering. So rule *precedence* is unreachable: a lookup \ - returns the first rule that matches, and with one rule per direction no packet ever \ - matches two. An ACL whose rules overlap is where an ordering defect would live.", - ), - ), + ("Acl.rules", Reach::Spans(&["1", "2"])), ( "AclRule.name", Reach::Determined("the two vpc handles, as `-to-`"), @@ -91,15 +84,7 @@ const REACH: &[(&str, Reach)] = &[ Reach::Determined("the vpc handle on the other side"), ), ("AclRule.action", Reach::Spans(&["allow", "deny"])), - ( - "AclRule.scope", - Reach::Fixed( - "`Packet`. A flow-scoped rule authorises a reply by its membership of the flow the \ - request opened, which is a whole mechanism -- `reverse_summary`, and the reverse \ - lookup in `AclFilter::lookup` -- that no generated configuration reaches. Drawing it \ - means drawing `validate_scope`'s condition too; see `algebra::Guard::acl`.", - ), - ), + ("AclRule.scope", Reach::Spans(&["flow", "packet"])), ( "AclRule.log", Reach::Fixed("false. Nothing generated asks for a rule's verdict to be logged."), diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index e7354c2def..692b73fbbd 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -410,6 +410,10 @@ impl Fabric { &mut self.worker } + pub(crate) fn fleet(&self) -> &Fleet { + &self.fleet + } + pub(crate) fn flows(&self) -> Option { self.fleet.blueprint().flow_table.len() } @@ -2809,6 +2813,7 @@ mod generated { static MULTI: AtomicU64 = AtomicU64::new(0); static INBOUND: AtomicU64 = AtomicU64::new(0); static PERMITTING: AtomicU64 = AtomicU64::new(0); + static BY_FLOW: AtomicU64 = AtomicU64::new(0); fn report_and_assert_coverage() { let (checked, derived, mixed) = ( @@ -2822,10 +2827,11 @@ mod generated { ); eprintln!( "checked={checked} derived={derived} inbound={} \ - permitting-peerings={} peered-configs={peered} \ + permitting-peerings={} (by flow {}) peered-configs={peered} \ configs-past-two-vpcs={multi} mixed-bursts={mixed}", INBOUND.load(Ordering::Relaxed), - PERMITTING.load(Ordering::Relaxed) + PERMITTING.load(Ordering::Relaxed), + BY_FLOW.load(Ordering::Relaxed) ); super::assert_covered(peered > 0, "no generated configuration ever had a peering"); super::assert_covered( @@ -2849,6 +2855,12 @@ mod generated { ran with no acl in the way and a rule set that lowered to nothing would have gone \ unnoticed", ); + super::assert_covered( + BY_FLOW.load(Ordering::Relaxed) > 0, + "no traffic was ever derived across a peering permitting only one direction, so no \ + reply here was authorised by the flow it belongs to and the reverse lookup in \ + `AclFilter::lookup` was never entered", + ); super::assert_covered( mixed > 0, "no burst ever carried more than one sender's traffic, so nothing was interleaved", @@ -2902,6 +2914,7 @@ mod generated { fn carried_counting<'a>( draft: &'a Draft, permitting: &'a Cell, + by_flow: &'a Cell, ) -> impl Fn(Named<'_>) -> bool + 'a { move |named| match draft.guard_named(named.peering) { Some(Guard::Deny) => false, @@ -2909,6 +2922,10 @@ mod generated { permitting.set(permitting.get() + 1); true } + Some(Guard::PermitFlow) => { + by_flow.set(by_flow.get() + 1); + true + } Some(Guard::Open) | None => true, } } @@ -2945,10 +2962,14 @@ mod generated { let mut fabric = Fabric::routed_over_validated(&validated, topology(&vnis)); - let permitting = Cell::new(0); - let mut loads = - loads_where(&validated, vary, &carried_counting(&draft, &permitting)); + let (permitting, by_flow) = (Cell::new(0), Cell::new(0)); + let mut loads = loads_where( + &validated, + vary, + &carried_counting(&draft, &permitting, &by_flow), + ); PERMITTING.fetch_add(permitting.get(), Ordering::Relaxed); + BY_FLOW.fetch_add(by_flow.get(), Ordering::Relaxed); DERIVED.fetch_add(loads.len() as u64, Ordering::Relaxed); for load in &loads { if load.describe().starts_with("[inbound") { From 0fc18e088515a6a6aa0c7cd575b8dbce3b321d25 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 14:52:55 -0600 Subject: [PATCH 10/19] feat(config): generate overlapping ACL rules With at most one rule per direction, generated ACLs could not make precedence affect a verdict. Add two predictable overlapping rules that partition a peering around one masquerade expose; reversing them now breaks the denial property. Track the named expose position and assign traffic variation before filtering so independent derivations remain comparable. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 119 ++++++++++++++++---- config/src/external/overlay/completeness.rs | 19 ++-- dataplane/src/packet_processor/fuzz.rs | 93 ++++++++++----- 3 files changed, 174 insertions(+), 57 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index a0b8b86829..701117d7c8 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -66,14 +66,17 @@ pub enum Guard { Open, Permit, PermitFlow, + PermitExcept, Deny, } impl Guard { - fn acl(self, spec: &PeeringSpec) -> Option { + fn acl(self, peering: PeeringHandle, spec: &PeeringSpec) -> Option { let (default, action, scope) = match self { Guard::Open => return None, - Guard::Permit => (AclAction::Deny, AclAction::Allow, AclScope::Packet), + Guard::Permit | Guard::PermitExcept => { + (AclAction::Deny, AclAction::Allow, AclScope::Packet) + } Guard::PermitFlow => (AclAction::Deny, AclAction::Allow, AclScope::Flow), Guard::Deny => (AclAction::Allow, AclAction::Deny, AclScope::Packet), }; @@ -95,9 +98,25 @@ impl Guard { log: false, } }; - let rules = match self.opening_side(spec) { - Some(side) => vec![rule(side)], - None => vec![rule(Side::Left), rule(Side::Right)], + let rules = match self { + Guard::PermitFlow => { + vec![rule(self.opening_side(spec).unwrap_or_else(|| { + unreachable!("`legal_on` refused a guard with no side") + }))] + } + Guard::PermitExcept => { + let (side, expose) = spec + .exception(peering) + .unwrap_or_else(|| unreachable!("`legal_on` refused a guard with no expose")); + let mut denial = rule(side); + denial.name = format!("{}-except", denial.name); + denial.action = AclAction::Deny; + denial.pattern.src = PrefixPortsSet::from([PrefixWithOptionalPorts::from(expose)]); + vec![denial, rule(side), rule(side.other())] + } + Guard::Open | Guard::Permit | Guard::Deny => { + vec![rule(Side::Left), rule(Side::Right)] + } }; Some(Acl::new(default, rules)) } @@ -108,7 +127,7 @@ impl Guard { spec.sole_opener() .unwrap_or_else(|| unreachable!("`legal_on` refused a guard with no side")), ), - Guard::Open | Guard::Permit | Guard::Deny => None, + Guard::Open | Guard::Permit | Guard::PermitExcept | Guard::Deny => None, } } @@ -116,6 +135,15 @@ impl Guard { match self { Guard::Open | Guard::Permit | Guard::Deny => true, Guard::PermitFlow => spec.sole_opener().is_some(), + Guard::PermitExcept => spec.exception_slot().is_some(), + } + } + + fn silences(self, spec: &PeeringSpec, side: Side, nth: usize) -> bool { + match self { + Guard::Open | Guard::Permit | Guard::PermitFlow => false, + Guard::Deny => true, + Guard::PermitExcept => spec.exception_slot() == Some((side, nth)), } } } @@ -299,6 +327,21 @@ impl PeeringSpec { }) } + fn exception_slot(&self) -> Option<(Side, usize)> { + [Side::Left, Side::Right].into_iter().find_map(|side| { + let nth = self + .exposes(side) + .iter() + .position(|expose| expose.flavour == Flavour::Masquerade)?; + Some((side, nth)) + }) + } + + fn exception(&self, peering: PeeringHandle) -> Option<(Side, Prefix)> { + let (side, nth) = self.exception_slot()?; + Some((side, self.exposes(side)[nth].private(peering, side))) + } + fn has_directional(&self, side: Side) -> bool { self.exposes(side) .iter() @@ -343,6 +386,24 @@ impl Draft { .map(|(handle, _)| *handle) } + #[must_use] + pub fn carries(&self, peering: &str, local: &str, nth: usize) -> bool { + let Some((_, spec)) = self + .peerings + .iter() + .find(|(handle, _)| handle.name() == peering) + else { + return true; + }; + let Some(side) = [Side::Left, Side::Right] + .into_iter() + .find(|side| spec.vpc(*side).name() == local) + else { + return true; + }; + !spec.guard.silences(spec, side, nth) + } + #[must_use] pub fn guard_named(&self, name: &str) -> Option { self.peerings @@ -395,7 +456,7 @@ impl Draft { spec.manifest(handle, Side::Left), spec.manifest(handle, Side::Right), ); - peering.acl = spec.guard.acl(spec); + peering.acl = spec.guard.acl(handle, spec); peerings.add(peering)?; } @@ -628,12 +689,16 @@ impl Op { } let index = spec.exposes(side).iter().position(|e| e.slot == slot)?; let removed = spec.exposes_mut(side).remove(index); - Some(Undo::RestoreExpose { + respecting_guard( + draft, peering, - side, - index, - spec: removed, - }) + Undo::RestoreExpose { + peering, + side, + index, + spec: removed, + }, + ) } Op::SetFlavour { @@ -1127,7 +1192,13 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { } fn draw_set_guard(driver: &mut D, draft: &Draft) -> Option { - const ORDERED: [Guard; 4] = [Guard::Open, Guard::Permit, Guard::PermitFlow, Guard::Deny]; + const ORDERED: [Guard; 5] = [ + Guard::Open, + Guard::Permit, + Guard::PermitExcept, + Guard::PermitFlow, + Guard::Deny, + ]; let guard = pick(driver, &ORDERED)?; let willing: Vec = draft @@ -1277,7 +1348,7 @@ mod tests { #[test] fn every_sequence_builds_a_valid_configuration() { let flavours = [const { AtomicUsize::new(0) }; 4]; - let guards = [const { AtomicUsize::new(0) }; 4]; + let guards = [const { AtomicUsize::new(0) }; 5]; check!() .with_generator(Sequence::default()) @@ -1322,6 +1393,7 @@ mod tests { let observed = match acl.map(|acl| (acl.default_action(), acl.rules().len())) { None => Guard::Open, Some((AclAction::Deny, 1)) => Guard::PermitFlow, + Some((AclAction::Deny, 3)) => Guard::PermitExcept, Some((AclAction::Deny, _)) => Guard::Permit, Some((AclAction::Allow, _)) => Guard::Deny, }; @@ -1335,8 +1407,9 @@ mod tests { guards[match observed { Guard::Open => 0, Guard::Permit => 1, - Guard::PermitFlow => 2, - Guard::Deny => 3, + Guard::PermitExcept => 2, + Guard::PermitFlow => 3, + Guard::Deny => 4, }] .fetch_add(1, Relaxed); } @@ -1351,10 +1424,16 @@ mod tests { } const FLAVOURS: [&str; 4] = ["forward", "masquerade", "static-nat", "port-forward"]; - const GUARDS: [&str; 4] = ["open", "permit", "permit-by-flow", "deny"]; + const GUARDS: [&str; 5] = [ + "open", + "permit", + "permit-except-one", + "permit-by-flow", + "deny", + ]; - fn assert_every_shape_built(flavours: &[AtomicUsize; 4], guards: &[AtomicUsize; 4]) { - let show = |names: [&str; 4], counts: &[AtomicUsize; 4]| { + fn assert_every_shape_built(flavours: &[AtomicUsize; 4], guards: &[AtomicUsize; 5]) { + let show = |names: &[&str], counts: &[AtomicUsize]| { names .iter() .zip(counts) @@ -1362,7 +1441,7 @@ mod tests { .collect::>() .join(" ") }; - let (built, set) = (show(FLAVOURS, flavours), show(GUARDS, guards)); + let (built, set) = (show(&FLAVOURS, flavours), show(&GUARDS, guards)); eprintln!("exposes: {built}\nguards: {set}"); for (name, count) in FLAVOURS.iter().zip(flavours) { diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index 739351841d..f0fd797e70 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -70,10 +70,10 @@ const REACH: &[(&str, Reach)] = &[ ), ("VpcPeering.acl", Reach::Spans(&["absent", "present"])), ("Acl.default", Reach::Spans(&["allow", "deny"])), - ("Acl.rules", Reach::Spans(&["1", "2"])), + ("Acl.rules", Reach::Spans(&["1", "2", "3"])), ( "AclRule.name", - Reach::Determined("the two vpc handles, as `-to-`"), + Reach::Determined("the two vpc handles, as `-to-`, and `-except` for a denial"), ), ( "AclRule.from", @@ -91,15 +91,20 @@ const REACH: &[(&str, Reach)] = &[ ), ( "AclPattern.src", - Reach::Fixed( - "empty, which `AclRule::validate` fills in from the `from` manifest -- so every \ - generated rule covers its whole side. A rule matching *part* of what a peering \ - carries, which is what an ACL is normally for, is unreachable.", + Reach::Determined( + "empty, or the excepted expose's private prefix from its peering, side and slot", ), ), ( "AclPattern.dst", - Reach::Fixed("empty, for the same reason as `AclPattern.src`."), + Reach::Fixed( + "empty, so `AclRule::validate` fills it in from the `to` manifest and every generated \ + rule reaches all of what its far side advertises. `AclPattern.src` is narrowed by \ + `Guard::PermitExcept` and this is not, and the asymmetry is deliberate: a source \ + prefix names the expose whose traffic it is, and a destination prefix names whichever \ + of the peer's exposes a load happens to aim at -- which is `peer_of`'s choice, so \ + predicting the effect would mean keeping a copy of it.", + ), ), ( "AclPattern.src_any_ports", diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 692b73fbbd..dce8d07d41 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -748,7 +748,7 @@ pub(crate) mod derive { use super::routed::{Blast, Conversation, Inbound}; use super::*; use config::external::overlay::ValidatedOverlay; - use config::external::overlay::algebra::{Draft, Guard}; + use config::external::overlay::algebra::Draft; use config::external::overlay::vpcpeering::ValidatedExpose; use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; @@ -830,7 +830,7 @@ pub(crate) mod derive { } pub(crate) fn carried_by(draft: &Draft) -> impl Fn(Named<'_>) -> bool + '_ { - move |named| draft.guard_named(named.peering) != Some(Guard::Deny) + move |named| draft.carries(named.peering, named.local, named.nth) } pub(crate) fn loads_carried( @@ -846,6 +846,7 @@ pub(crate) mod derive { pub(crate) local: &'a str, pub(crate) remote: &'a str, pub(crate) peering: &'a str, + pub(crate) nth: usize, } pub(crate) fn loads_where( @@ -857,19 +858,20 @@ pub(crate) mod derive { let mut nth = 0usize; for vpc in overlay.vpc_table().values() { for peering in vpc.peerings() { - if !keep(Named { - local: vpc.name(), - remote: peering.remote().name(), - peering: peering.name(), - }) { - continue; - } let path = super::routed::Path::new(vpc.vni(), peering.remote_vni()); - for expose in peering.local().valexp() { + for (which, expose) in peering.local().valexp().iter().enumerate() { let Some(v) = vary.get(nth % vary.len().max(1)).copied() else { continue; }; nth += 1; + if !keep(Named { + local: vpc.name(), + remote: peering.remote().name(), + peering: peering.name(), + nth: which, + }) { + continue; + } let outward = peer_of(peering, v.host, |expose| { expose.can_receive_connection() && !expose.has_port_forwarding() }); @@ -2814,6 +2816,7 @@ mod generated { static INBOUND: AtomicU64 = AtomicU64::new(0); static PERMITTING: AtomicU64 = AtomicU64::new(0); static BY_FLOW: AtomicU64 = AtomicU64::new(0); + static EXCEPTING: AtomicU64 = AtomicU64::new(0); fn report_and_assert_coverage() { let (checked, derived, mixed) = ( @@ -2827,11 +2830,12 @@ mod generated { ); eprintln!( "checked={checked} derived={derived} inbound={} \ - permitting-peerings={} (by flow {}) peered-configs={peered} \ + permitted-loads={} (by flow {}, past an exception {}) peered-configs={peered} \ configs-past-two-vpcs={multi} mixed-bursts={mixed}", INBOUND.load(Ordering::Relaxed), PERMITTING.load(Ordering::Relaxed), - BY_FLOW.load(Ordering::Relaxed) + BY_FLOW.load(Ordering::Relaxed), + EXCEPTING.load(Ordering::Relaxed) ); super::assert_covered(peered > 0, "no generated configuration ever had a peering"); super::assert_covered( @@ -2855,6 +2859,12 @@ mod generated { ran with no acl in the way and a rule set that lowered to nothing would have gone \ unnoticed", ); + super::assert_covered( + EXCEPTING.load(Ordering::Relaxed) > 0, + "no traffic was ever derived across a peering whose acl excepts one expose from an \ + otherwise permitting rule, so nothing here depended on a lookup returning the \ + *first* rule that matches", + ); super::assert_covered( BY_FLOW.load(Ordering::Relaxed) > 0, "no traffic was ever derived across a peering permitting only one direction, so no \ @@ -2915,18 +2925,21 @@ mod generated { draft: &'a Draft, permitting: &'a Cell, by_flow: &'a Cell, + excepting: &'a Cell, ) -> impl Fn(Named<'_>) -> bool + 'a { - move |named| match draft.guard_named(named.peering) { - Some(Guard::Deny) => false, - Some(Guard::Permit) => { - permitting.set(permitting.get() + 1); - true - } - Some(Guard::PermitFlow) => { - by_flow.set(by_flow.get() + 1); - true + let carried = super::derive::carried_by(draft); + move |named| { + let counter = match draft.guard_named(named.peering) { + Some(Guard::Permit) => permitting, + Some(Guard::PermitFlow) => by_flow, + Some(Guard::PermitExcept) => excepting, + Some(Guard::Open | Guard::Deny) | None => return carried(named), + }; + let kept = carried(named); + if kept { + counter.set(counter.get() + 1); } - Some(Guard::Open) | None => true, + kept } } @@ -2962,14 +2975,15 @@ mod generated { let mut fabric = Fabric::routed_over_validated(&validated, topology(&vnis)); - let (permitting, by_flow) = (Cell::new(0), Cell::new(0)); + let (permitting, by_flow, excepting) = (Cell::new(0), Cell::new(0), Cell::new(0)); let mut loads = loads_where( &validated, vary, - &carried_counting(&draft, &permitting, &by_flow), + &carried_counting(&draft, &permitting, &by_flow, &excepting), ); PERMITTING.fetch_add(permitting.get(), Ordering::Relaxed); BY_FLOW.fetch_add(by_flow.get(), Ordering::Relaxed); + EXCEPTING.fetch_add(excepting.get(), Ordering::Relaxed); DERIVED.fetch_add(loads.len() as u64, Ordering::Relaxed); for load in &loads { if load.describe().starts_with("[inbound") { @@ -3001,9 +3015,10 @@ mod generated { #[tokio::test] #[dpdk::with_eal] - async fn a_denied_peering_carries_nothing() { + async fn a_configuration_carries_nothing_it_denies() { static SENT: AtomicU64 = AtomicU64::new(0); static BY_ACL: AtomicU64 = AtomicU64::new(0); + static NARROWED: AtomicU64 = AtomicU64::new(0); static CONFIGS: AtomicU64 = AtomicU64::new(0); bolero::check!() @@ -3026,10 +3041,20 @@ mod generated { return; } let carried = super::derive::carried_by(&draft); - let mut loads = loads_where(&validated, vary, &|named| !carried(named)); + let narrowed = Cell::new(0); + let mut loads = loads_where(&validated, vary, &|named| { + if carried(named) { + return false; + } + if draft.guard_named(named.peering) != Some(Guard::Deny) { + narrowed.set(narrowed.get() + 1); + } + true + }); if loads.is_empty() { return; } + NARROWED.fetch_add(narrowed.get(), Ordering::Relaxed); CONFIGS.fetch_add(1, Ordering::Relaxed); let mut fabric = Fabric::routed_over_validated(&validated, topology(&vnis)); @@ -3041,7 +3066,7 @@ mod generated { SENT.fetch_add(1, Ordering::Relaxed); assert!( matches!(seen, Verdict::Dropped(_)), - "a peering whose acl denies everything it carries produced {seen:?} for {}", + "an acl that refuses this traffic produced {seen:?} for {}", load.describe() ); if seen == Verdict::Dropped(DoneReason::AclDropped) { @@ -3055,11 +3080,19 @@ mod generated { SENT.load(Ordering::Relaxed), BY_ACL.load(Ordering::Relaxed), ); - eprintln!("denied-configs={configs} sent={sent} (dropped by the acl {by_acl})"); + eprintln!( + "refusing-configs={configs} sent={sent} (dropped by the acl {by_acl}, \ + refused by a narrowed rule {})", + NARROWED.load(Ordering::Relaxed) + ); super::assert_covered( sent > 0, - "no denied peering ever had traffic derived for it, so this asserted nothing about \ - any packet", + "no refused traffic was ever derived, so this asserted nothing about any packet", + ); + super::assert_covered( + NARROWED.load(Ordering::Relaxed) > 0, + "every refusal came from a peering denied outright, so nothing here was refused by a \ + rule naming part of a peering and the first-match order was not under test", ); super::assert_covered( by_acl > 0, From 1d96e08eb646f928f447cfe53a3eb339a993e300 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 15:13:37 -0600 Subject: [PATCH 11/19] feat(config): vary gateway groups, ACL logging, and timeouts Gateway-group names, ACL logging, and idle timeouts remained fixed solely because the algebra never selected them. Vary all three and update the census. Record the current limits: disjoint exposes do not exercise gateway grouping semantics, and generated timeouts outlast these properties, so the fields lower successfully without testing expiration. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 20 ++++++++++++++++---- config/src/external/overlay/completeness.rs | 18 ++++-------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 701117d7c8..a53e3b3d20 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::net::Ipv4Addr; use std::ops::Bound::Included; +use std::time::Duration; use bolero::{Driver, ValueGenerator}; use lpm::prefix::{ @@ -95,7 +96,7 @@ impl Guard { proto: AclProtoMatch::Any, }, scope, - log: false, + log: action == AclAction::Deny, } }; let rules = match self { @@ -168,11 +169,17 @@ impl VpcHandle { } } +const LONG_IDLE_TIMEOUT: Duration = Duration::from_hours(1); + impl PeeringHandle { fn name(self) -> String { format!("PEERING-{:03}", self.0) } + fn group(self) -> String { + format!("group-{}", self.0 % 3) + } + fn block(self, side: Side, slot: u8) -> u32 { u32::from(self.0) * SLOTS_PER_PEERING + u32::try_from(side.index()).unwrap_or_else(|_| unreachable!()) @@ -216,6 +223,10 @@ impl ExposeSpec { self.flavour } + fn idle_timeout(self) -> Option { + (self.slot % 2 == 1).then_some(LONG_IDLE_TIMEOUT) + } + #[must_use] pub fn private(self, peering: PeeringHandle, side: Side) -> Prefix { private_prefix(peering.block(side, self.slot)) @@ -236,7 +247,7 @@ impl ExposeSpec { match self.flavour { Flavour::Forward => VpcExpose::empty().ip(private.into()), Flavour::Masquerade => VpcExpose::empty() - .make_masquerade(None) + .make_masquerade(self.idle_timeout()) .unwrap_or_else(|_| unreachable!("an empty expose accepts masquerade")) .ip(private.into()) .as_range(self.public(peering, side).into()) @@ -248,7 +259,7 @@ impl ExposeSpec { .as_range(self.public(peering, side).into()) .unwrap_or_else(|_| unreachable!("a static nat expose accepts a public range")), Flavour::PortForward => VpcExpose::empty() - .make_port_forwarding(None, None) + .make_port_forwarding(self.idle_timeout(), None) .unwrap_or_else(|_| unreachable!("an empty expose accepts port forwarding")) .ip(PrefixWithOptionalPorts::new( private, @@ -451,10 +462,11 @@ impl Draft { let mut peerings = VpcPeeringTable::new(); for (handle, spec) in self.peerings() { - let mut peering = VpcPeering::with_default_group( + let mut peering = VpcPeering::new( &handle.name(), spec.manifest(handle, Side::Left), spec.manifest(handle, Side::Right), + handle.group(), ); peering.acl = spec.guard.acl(handle, spec); peerings.add(peering)?; diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index f0fd797e70..bf582c765d 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -66,7 +66,7 @@ const REACH: &[(&str, Reach)] = &[ ), ( "VpcPeering.gwgroup", - Reach::Fixed("the default group; the algebra only calls `with_default_group`."), + Reach::Determined("the peering handle, over three groups"), ), ("VpcPeering.acl", Reach::Spans(&["absent", "present"])), ("Acl.default", Reach::Spans(&["allow", "deny"])), @@ -85,10 +85,7 @@ const REACH: &[(&str, Reach)] = &[ ), ("AclRule.action", Reach::Spans(&["allow", "deny"])), ("AclRule.scope", Reach::Spans(&["flow", "packet"])), - ( - "AclRule.log", - Reach::Fixed("false. Nothing generated asks for a rule's verdict to be logged."), - ), + ("AclRule.log", Reach::Spans(&["false", "true"])), ( "AclPattern.src", Reach::Determined( @@ -168,19 +165,12 @@ const REACH: &[(&str, Reach)] = &[ ), ( "VpcExposeMasquerade.idle_timeout", - Reach::Fixed( - "absent; the algebra always calls `make_masquerade(None)`, leaving custom \ - timeouts untested.", - ), + Reach::Spans(&["absent", "present"]), ), ("VpcExposeStaticNat", Reach::Spans(&["constructed"])), ( "VpcExposePortForwarding.idle_timeout", - Reach::Fixed( - "absent. `make_port_forwarding(None, ..)` is the only call, for the same reason \ - `VpcExposeMasquerade.idle_timeout` is absent: the flavour is reachable now, but \ - nothing asks for a timeout, so no flow ages out under a configuration that set one.", - ), + Reach::Spans(&["absent", "present"]), ), ]; From f404c5d07b0cea7c6e4d570a0aea62b35476eda6 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 15:18:29 -0600 Subject: [PATCH 12/19] feat(config): narrow ACL rules by protocol, port, and destination Generated ACLs still left protocol, port, and destination matching fixed, so dropping a field or lowering it as a wildcard could pass. Add predictable narrow rules whose target prefixes appear in exactly one traffic role, avoiding a duplicate ACL evaluator. Include a protocol guard that must not fire so both omission and wildcarding become observable. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 122 ++++++++++++++------ config/src/external/overlay/completeness.rs | 30 +---- dataplane/src/packet_processor/fuzz.rs | 2 +- 3 files changed, 94 insertions(+), 60 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index a53e3b3d20..8ee0dbaaa7 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -68,6 +68,7 @@ pub enum Guard { Permit, PermitFlow, PermitExcept, + PermitByProtocol, Deny, } @@ -75,12 +76,17 @@ impl Guard { fn acl(self, peering: PeeringHandle, spec: &PeeringSpec) -> Option { let (default, action, scope) = match self { Guard::Open => return None, - Guard::Permit | Guard::PermitExcept => { + Guard::Permit | Guard::PermitExcept | Guard::PermitByProtocol => { (AclAction::Deny, AclAction::Allow, AclScope::Packet) } Guard::PermitFlow => (AclAction::Deny, AclAction::Allow, AclScope::Flow), Guard::Deny => (AclAction::Allow, AclAction::Deny, AclScope::Packet), }; + let (proto, any_ports) = if self == Guard::PermitByProtocol { + (AclProtoMatch::Udp, vec![port_range(EVERY_PORT)]) + } else { + (AclProtoMatch::Any, Vec::new()) + }; let rule = |side: Side| { let (from, to) = (spec.vpc(side).name(), spec.vpc(side.other()).name()); AclRule { @@ -91,9 +97,9 @@ impl Guard { pattern: AclPattern { src: PrefixPortsSet::new(), dst: PrefixPortsSet::new(), - src_any_ports: Vec::new(), - dst_any_ports: Vec::new(), - proto: AclProtoMatch::Any, + src_any_ports: any_ports.clone(), + dst_any_ports: any_ports.clone(), + proto, }, scope, log: action == AclAction::Deny, @@ -105,15 +111,26 @@ impl Guard { unreachable!("`legal_on` refused a guard with no side") }))] } - Guard::PermitExcept => { - let (side, expose) = spec + Guard::PermitExcept | Guard::PermitByProtocol => { + let (side, which, prefix) = spec .exception(peering) .unwrap_or_else(|| unreachable!("`legal_on` refused a guard with no expose")); - let mut denial = rule(side); + let denied_from = match which { + Narrowing::Source => side, + Narrowing::Destination => side.other(), + }; + let mut denial = rule(denied_from); denial.name = format!("{}-except", denial.name); denial.action = AclAction::Deny; - denial.pattern.src = PrefixPortsSet::from([PrefixWithOptionalPorts::from(expose)]); - vec![denial, rule(side), rule(side.other())] + let named = PrefixPortsSet::from([PrefixWithOptionalPorts::from(prefix)]); + match which { + Narrowing::Source => denial.pattern.src = named, + Narrowing::Destination => denial.pattern.dst = named, + } + if self == Guard::PermitByProtocol { + denial.pattern.proto = AclProtoMatch::Tcp; + } + vec![denial, rule(denied_from), rule(denied_from.other())] } Guard::Open | Guard::Permit | Guard::Deny => { vec![rule(Side::Left), rule(Side::Right)] @@ -128,7 +145,11 @@ impl Guard { spec.sole_opener() .unwrap_or_else(|| unreachable!("`legal_on` refused a guard with no side")), ), - Guard::Open | Guard::Permit | Guard::PermitExcept | Guard::Deny => None, + Guard::Open + | Guard::Permit + | Guard::PermitExcept + | Guard::PermitByProtocol + | Guard::Deny => None, } } @@ -136,15 +157,17 @@ impl Guard { match self { Guard::Open | Guard::Permit | Guard::Deny => true, Guard::PermitFlow => spec.sole_opener().is_some(), - Guard::PermitExcept => spec.exception_slot().is_some(), + Guard::PermitExcept | Guard::PermitByProtocol => spec.exception_slot().is_some(), } } fn silences(self, spec: &PeeringSpec, side: Side, nth: usize) -> bool { match self { - Guard::Open | Guard::Permit | Guard::PermitFlow => false, + Guard::Open | Guard::Permit | Guard::PermitFlow | Guard::PermitByProtocol => false, Guard::Deny => true, - Guard::PermitExcept => spec.exception_slot() == Some((side, nth)), + Guard::PermitExcept => spec + .exception_slot() + .is_some_and(|(at, which, _)| (at, which) == (side, nth)), } } } @@ -192,6 +215,8 @@ pub(crate) const FORWARDED_PRIVATE_PORTS: (u16, u16) = (1000, 1004); pub(crate) const FORWARDED_PUBLIC_PORTS: (u16, u16) = (2000, 2004); +const EVERY_PORT: (u16, u16) = (1, u16::MAX); + fn port_range((start, end): (u16, u16)) -> PortRange { PortRange::new(start, end).unwrap_or_else(|_| unreachable!("a well-formed port range")) } @@ -276,6 +301,22 @@ impl ExposeSpec { } } +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum Narrowing { + Source, + Destination, +} + +impl Narrowing { + fn of(flavour: Flavour) -> Option { + match flavour { + Flavour::Masquerade => Some(Self::Source), + Flavour::PortForward => Some(Self::Destination), + Flavour::Forward | Flavour::StaticNat => None, + } + } +} + #[derive(Clone, PartialEq, Eq, Debug)] pub struct PeeringSpec { left: VpcHandle, @@ -338,19 +379,23 @@ impl PeeringSpec { }) } - fn exception_slot(&self) -> Option<(Side, usize)> { + fn exception_slot(&self) -> Option<(Side, usize, Narrowing)> { [Side::Left, Side::Right].into_iter().find_map(|side| { - let nth = self - .exposes(side) + self.exposes(side) .iter() - .position(|expose| expose.flavour == Flavour::Masquerade)?; - Some((side, nth)) + .enumerate() + .find_map(|(nth, expose)| Some((side, nth, Narrowing::of(expose.flavour)?))) }) } - fn exception(&self, peering: PeeringHandle) -> Option<(Side, Prefix)> { - let (side, nth) = self.exception_slot()?; - Some((side, self.exposes(side)[nth].private(peering, side))) + fn exception(&self, peering: PeeringHandle) -> Option<(Side, Narrowing, Prefix)> { + let (side, nth, which) = self.exception_slot()?; + let expose = self.exposes(side)[nth]; + let prefix = match which { + Narrowing::Source => expose.private(peering, side), + Narrowing::Destination => expose.public(peering, side), + }; + Some((side, which, prefix)) } fn has_directional(&self, side: Side) -> bool { @@ -1204,10 +1249,11 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { } fn draw_set_guard(driver: &mut D, draft: &Draft) -> Option { - const ORDERED: [Guard; 5] = [ + const ORDERED: [Guard; 6] = [ Guard::Open, Guard::Permit, Guard::PermitExcept, + Guard::PermitByProtocol, Guard::PermitFlow, Guard::Deny, ]; @@ -1360,7 +1406,7 @@ mod tests { #[test] fn every_sequence_builds_a_valid_configuration() { let flavours = [const { AtomicUsize::new(0) }; 4]; - let guards = [const { AtomicUsize::new(0) }; 5]; + let guards = [const { AtomicUsize::new(0) }; 6]; check!() .with_generator(Sequence::default()) @@ -1402,13 +1448,19 @@ mod tests { .values() .find(|peering| peering.name == handle.name()) .and_then(|peering| peering.acl.as_ref()); - let observed = match acl.map(|acl| (acl.default_action(), acl.rules().len())) { - None => Guard::Open, - Some((AclAction::Deny, 1)) => Guard::PermitFlow, - Some((AclAction::Deny, 3)) => Guard::PermitExcept, - Some((AclAction::Deny, _)) => Guard::Permit, - Some((AclAction::Allow, _)) => Guard::Deny, - }; + let observed = acl.map_or(Guard::Open, |acl| { + let inert = acl + .rules() + .first() + .is_some_and(|rule| rule.pattern.proto == AclProtoMatch::Tcp); + match (acl.default_action(), acl.rules().len()) { + (AclAction::Deny, 1) => Guard::PermitFlow, + (AclAction::Deny, 3) if inert => Guard::PermitByProtocol, + (AclAction::Deny, 3) => Guard::PermitExcept, + (AclAction::Deny, _) => Guard::Permit, + (AclAction::Allow, _) => Guard::Deny, + } + }); assert_eq!( observed, spec.guard(), @@ -1420,8 +1472,9 @@ mod tests { Guard::Open => 0, Guard::Permit => 1, Guard::PermitExcept => 2, - Guard::PermitFlow => 3, - Guard::Deny => 4, + Guard::PermitByProtocol => 3, + Guard::PermitFlow => 4, + Guard::Deny => 5, }] .fetch_add(1, Relaxed); } @@ -1436,15 +1489,16 @@ mod tests { } const FLAVOURS: [&str; 4] = ["forward", "masquerade", "static-nat", "port-forward"]; - const GUARDS: [&str; 5] = [ + const GUARDS: [&str; 6] = [ "open", "permit", "permit-except-one", + "permit-by-protocol", "permit-by-flow", "deny", ]; - fn assert_every_shape_built(flavours: &[AtomicUsize; 4], guards: &[AtomicUsize; 5]) { + fn assert_every_shape_built(flavours: &[AtomicUsize; 4], guards: &[AtomicUsize; 6]) { let show = |names: &[&str], counts: &[AtomicUsize]| { names .iter() diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index bf582c765d..bf60a95519 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -94,33 +94,13 @@ const REACH: &[(&str, Reach)] = &[ ), ( "AclPattern.dst", - Reach::Fixed( - "empty, so `AclRule::validate` fills it in from the `to` manifest and every generated \ - rule reaches all of what its far side advertises. `AclPattern.src` is narrowed by \ - `Guard::PermitExcept` and this is not, and the asymmetry is deliberate: a source \ - prefix names the expose whose traffic it is, and a destination prefix names whichever \ - of the peer's exposes a load happens to aim at -- which is `peer_of`'s choice, so \ - predicting the effect would mean keeping a copy of it.", - ), - ), - ( - "AclPattern.src_any_ports", - Reach::Fixed( - "empty -- the survey renders it as a count of zero. A `match` naming ports but no \ - address is a shape the k8s converter produces and nothing generated does.", - ), - ), - ( - "AclPattern.dst_any_ports", - Reach::Fixed("empty, for the same reason as `AclPattern.src_any_ports`."), - ), - ( - "AclPattern.proto", - Reach::Fixed( - "`Any`. Narrowing a rule to a protocol is what `acl_filter`'s own generator is aimed \ - at, and a rule that discriminates is one a property here would have to evaluate.", + Reach::Determined( + "empty, or the excepted expose's public prefix from its peering, side and slot", ), ), + ("AclPattern.src_any_ports", Reach::Spans(&["0", "1"])), + ("AclPattern.dst_any_ports", Reach::Spans(&["0", "1"])), + ("AclPattern.proto", Reach::Spans(&["any", "tcp", "udp"])), ( "VpcManifest.name", Reach::Determined("the side's vpc handle"), diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index dce8d07d41..2ad84ecf18 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -2932,7 +2932,7 @@ mod generated { let counter = match draft.guard_named(named.peering) { Some(Guard::Permit) => permitting, Some(Guard::PermitFlow) => by_flow, - Some(Guard::PermitExcept) => excepting, + Some(Guard::PermitExcept | Guard::PermitByProtocol) => excepting, Some(Guard::Open | Guard::Deny) | None => return carried(named), }; let kept = carried(named); From 7daa36826d94885bac6d55bcc1fd1a5453fded00 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 15:20:03 -0600 Subject: [PATCH 13/19] feat(config): vary port-forwarding protocols Every generated port-forwarding expose accepted any protocol, leaving its protocol key and collision behavior untested. Cycle protocols across expose slots so one manifest can contain several. Because derived traffic is UDP, exclude TCP-only exposes from offered loads and make Draft::carries account for both expose protocol and peering ACL. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 24 +++++++++++++++++++-- config/src/external/overlay/completeness.rs | 5 +---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 8ee0dbaaa7..9ede0666b6 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -7,6 +7,7 @@ use std::ops::Bound::Included; use std::time::Duration; use bolero::{Driver, ValueGenerator}; +use lpm::prefix::with_ports::L4Protocol; use lpm::prefix::{ IpPrefix, Ipv4Prefix, PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, }; @@ -252,6 +253,21 @@ impl ExposeSpec { (self.slot % 2 == 1).then_some(LONG_IDLE_TIMEOUT) } + fn nat_proto(self) -> Option { + match self.flavour { + Flavour::PortForward => Some(match self.slot % 3 { + 0 => L4Protocol::Any, + 1 => L4Protocol::Udp, + _ => L4Protocol::Tcp, + }), + Flavour::Forward | Flavour::Masquerade | Flavour::StaticNat => None, + } + } + + fn carries_udp(self) -> bool { + !matches!(self.nat_proto(), Some(L4Protocol::Tcp)) + } + #[must_use] pub fn private(self, peering: PeeringHandle, side: Side) -> Prefix { private_prefix(peering.block(side, self.slot)) @@ -284,7 +300,7 @@ impl ExposeSpec { .as_range(self.public(peering, side).into()) .unwrap_or_else(|_| unreachable!("a static nat expose accepts a public range")), Flavour::PortForward => VpcExpose::empty() - .make_port_forwarding(self.idle_timeout(), None) + .make_port_forwarding(self.idle_timeout(), self.nat_proto()) .unwrap_or_else(|_| unreachable!("an empty expose accepts port forwarding")) .ip(PrefixWithOptionalPorts::new( private, @@ -457,7 +473,11 @@ impl Draft { else { return true; }; - !spec.guard.silences(spec, side, nth) + let carried_by_the_expose = spec + .exposes(side) + .get(nth) + .is_none_or(|expose| expose.carries_udp()); + carried_by_the_expose && !spec.guard.silences(spec, side, nth) } #[must_use] diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index bf60a95519..a6499f9e75 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -139,10 +139,7 @@ const REACH: &[(&str, Reach)] = &[ "VpcExposeNat.config", Reach::Spans(&["masquerade", "port-forwarding", "static"]), ), - ( - "VpcExposeNat.proto", - Reach::Fixed("`Any`; no operation restricts the protocol to TCP or UDP."), - ), + ("VpcExposeNat.proto", Reach::Spans(&["any", "tcp", "udp"])), ( "VpcExposeMasquerade.idle_timeout", Reach::Spans(&["absent", "present"]), From 96f7a38491b40a1700a134693ec677f14c96e630 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 15:22:43 -0600 Subject: [PATCH 14/19] feat(config): generate address exclusions Generated exposes had no exclusions, so matchers, LPM tables, and RangeBuilder saw only contiguous prefixes. Remove a middle slice rather than one half, forcing the effective set into two ranges. This change measures reach and shape only; traffic derivation still selects from the effective set and therefore does not yet test the excluded hole. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 64 ++++++++++++++++----- config/src/external/overlay/completeness.rs | 4 +- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 9ede0666b6..6b653552ba 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -222,12 +222,20 @@ fn port_range((start, end): (u16, u16)) -> PortRange { PortRange::new(start, end).unwrap_or_else(|_| unreachable!("a well-formed port range")) } +const PRIVATE_BASE: u32 = 0x0A00_0000; + +const PUBLIC_BASE: u32 = 0xAC10_0000; + fn private_prefix(index: u32) -> Prefix { - prefix_v4(0x0A00_0000 | (index << 8), 24) + prefix_v4(PRIVATE_BASE | (index << 8), 24) } fn public_prefix(index: u32) -> Prefix { - prefix_v4(0xAC10_0000 | (index << 8), 24) + prefix_v4(PUBLIC_BASE | (index << 8), 24) +} + +fn excluded_slice(index: u32, base: u32) -> Prefix { + prefix_v4(base | (index << 8) | 0x40, 26) } fn prefix_v4(bits: u32, len: u8) -> Prefix { @@ -264,6 +272,10 @@ impl ExposeSpec { } } + fn excludes(self) -> bool { + self.slot < 2 && self.flavour != Flavour::PortForward + } + fn carries_udp(self) -> bool { !matches!(self.nat_proto(), Some(L4Protocol::Tcp)) } @@ -283,22 +295,44 @@ impl ExposeSpec { } } + fn carve(self, expose: VpcExpose, peering: PeeringHandle, side: Side) -> VpcExpose { + if !self.excludes() { + return expose; + } + let block = peering.block(side, self.slot); + let expose = expose.not(excluded_slice(block, PRIVATE_BASE).into()); + if expose.nat.is_none() { + return expose; + } + expose + .not_as(excluded_slice(block, PUBLIC_BASE).into()) + .unwrap_or_else(|_| unreachable!("a translating expose accepts an excluded range")) + } + fn expose(self, peering: PeeringHandle, side: Side) -> VpcExpose { let private = self.private(peering, side); match self.flavour { - Flavour::Forward => VpcExpose::empty().ip(private.into()), - Flavour::Masquerade => VpcExpose::empty() - .make_masquerade(self.idle_timeout()) - .unwrap_or_else(|_| unreachable!("an empty expose accepts masquerade")) - .ip(private.into()) - .as_range(self.public(peering, side).into()) - .unwrap_or_else(|_| unreachable!("a masquerade expose accepts a public range")), - Flavour::StaticNat => VpcExpose::empty() - .make_static_nat() - .unwrap_or_else(|_| unreachable!("an empty expose accepts static nat")) - .ip(private.into()) - .as_range(self.public(peering, side).into()) - .unwrap_or_else(|_| unreachable!("a static nat expose accepts a public range")), + Flavour::Forward => self.carve(VpcExpose::empty().ip(private.into()), peering, side), + Flavour::Masquerade => self.carve( + VpcExpose::empty() + .make_masquerade(self.idle_timeout()) + .unwrap_or_else(|_| unreachable!("an empty expose accepts masquerade")) + .ip(private.into()) + .as_range(self.public(peering, side).into()) + .unwrap_or_else(|_| unreachable!("a masquerade expose accepts a public range")), + peering, + side, + ), + Flavour::StaticNat => self.carve( + VpcExpose::empty() + .make_static_nat() + .unwrap_or_else(|_| unreachable!("an empty expose accepts static nat")) + .ip(private.into()) + .as_range(self.public(peering, side).into()) + .unwrap_or_else(|_| unreachable!("a static nat expose accepts a public range")), + peering, + side, + ), Flavour::PortForward => VpcExpose::empty() .make_port_forwarding(self.idle_timeout(), self.nat_proto()) .unwrap_or_else(|_| unreachable!("an empty expose accepts port forwarding")) diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index a6499f9e75..12db197b12 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -120,7 +120,7 @@ const REACH: &[(&str, Reach)] = &[ ("VpcExpose.ips.ports", Reach::Spans(&["set", "unset"])), ( "VpcExpose.nots", - Reach::Fixed("empty; the algebra never excludes addresses from an expose."), + Reach::Determined("a `/26` in the middle of the expose's block, on the low slots"), ), ("VpcExpose.nat", Reach::Spans(&["absent", "present"])), ( @@ -133,7 +133,7 @@ const REACH: &[(&str, Reach)] = &[ ), ( "VpcExposeNat.not_as", - Reach::Fixed("empty, for the same reason as `VpcExpose.nots`."), + Reach::Determined("a `/26` in the middle of the expose's translated block"), ), ( "VpcExposeNat.config", From 4fac88b01bc93acfe3fa736fc0a8ae5c9d942fdd Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 15:29:09 -0600 Subject: [PATCH 15/19] feat(config): generate default-route exposes The algebra could not express default-route exposes. Default routes overlap every neighboring route, so their validity depends on more than the operation's own arguments. Restrict them to otherwise empty peerings and let Op::reads inspect the draft, keeping applicability local and restoring the independence property. Skip loads whose expose protocol cannot carry their traffic. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 152 ++++++++++++++++---- config/src/external/overlay/completeness.rs | 5 +- dataplane/src/packet_processor/fuzz.rs | 8 ++ 3 files changed, 131 insertions(+), 34 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 6b653552ba..e651711afb 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -53,6 +53,7 @@ pub enum Flavour { Masquerade, StaticNat, PortForward, + Everything, } impl Flavour { @@ -268,16 +269,18 @@ impl ExposeSpec { 1 => L4Protocol::Udp, _ => L4Protocol::Tcp, }), - Flavour::Forward | Flavour::Masquerade | Flavour::StaticNat => None, + Flavour::Forward | Flavour::Masquerade | Flavour::StaticNat | Flavour::Everything => { + None + } } } fn excludes(self) -> bool { - self.slot < 2 && self.flavour != Flavour::PortForward - } - - fn carries_udp(self) -> bool { - !matches!(self.nat_proto(), Some(L4Protocol::Tcp)) + self.slot < 2 + && matches!( + self.flavour, + Flavour::Forward | Flavour::Masquerade | Flavour::StaticNat + ) } #[must_use] @@ -288,7 +291,7 @@ impl ExposeSpec { #[must_use] pub fn public(self, peering: PeeringHandle, side: Side) -> Prefix { match self.flavour { - Flavour::Forward => self.private(peering, side), + Flavour::Forward | Flavour::Everything => self.private(peering, side), Flavour::Masquerade | Flavour::StaticNat | Flavour::PortForward => { public_prefix(peering.block(side, self.slot)) } @@ -312,6 +315,7 @@ impl ExposeSpec { fn expose(self, peering: PeeringHandle, side: Side) -> VpcExpose { let private = self.private(peering, side); match self.flavour { + Flavour::Everything => VpcExpose::empty().set_default(), Flavour::Forward => self.carve(VpcExpose::empty().ip(private.into()), peering, side), Flavour::Masquerade => self.carve( VpcExpose::empty() @@ -362,7 +366,7 @@ impl Narrowing { match flavour { Flavour::Masquerade => Some(Self::Source), Flavour::PortForward => Some(Self::Destination), - Flavour::Forward | Flavour::StaticNat => None, + Flavour::Forward | Flavour::StaticNat | Flavour::Everything => None, } } } @@ -448,6 +452,12 @@ impl PeeringSpec { Some((side, which, prefix)) } + fn has_everything(&self, side: Side) -> bool { + self.exposes(side) + .iter() + .any(|expose| expose.flavour == Flavour::Everything) + } + fn has_directional(&self, side: Side) -> bool { self.exposes(side) .iter() @@ -507,11 +517,7 @@ impl Draft { else { return true; }; - let carried_by_the_expose = spec - .exposes(side) - .get(nth) - .is_none_or(|expose| expose.carries_udp()); - carried_by_the_expose && !spec.guard.silences(spec, side, nth) + !spec.guard.silences(spec, side, nth) } #[must_use] @@ -522,6 +528,22 @@ impl Draft { .map(|(_, spec)| spec.guard) } + fn peerings_of(&self, vpc: VpcHandle) -> usize { + self.peerings + .values() + .filter(|spec| spec.touches(vpc)) + .count() + } + + fn beside_everything(&self, vpc: VpcHandle) -> bool { + self.peerings.values().any(|spec| { + spec.touches(vpc) + && [Side::Left, Side::Right] + .into_iter() + .any(|side| spec.has_everything(side)) + }) + } + #[must_use] pub fn components(&self) -> Vec> { let mut unvisited: BTreeSet = self.vpcs.clone(); @@ -681,14 +703,34 @@ pub enum Undo { impl Op { #[must_use] - pub fn reads(&self) -> Footprint { + pub fn reads(&self, draft: &Draft) -> Footprint { match self { Op::AddVpc(_) | Op::RemoveVpc(_) | Op::RemovePeering(_) => Footprint::default(), - Op::AddPeering { left, right, .. } => Footprint::of([*left, *right], []), - Op::AddExpose { peering, .. } - | Op::RemoveExpose { peering, .. } - | Op::SetFlavour { peering, .. } - | Op::SetGuard { peering, .. } => Footprint::of([], [*peering]), + Op::AddPeering { left, right, .. } => Footprint::of( + [*left, *right], + draft + .peerings() + .filter(|(_, spec)| spec.touches(*left) || spec.touches(*right)) + .map(|(handle, _)| handle), + ), + Op::RemoveExpose { peering, .. } | Op::SetGuard { peering, .. } => { + Footprint::of([], [*peering]) + } + Op::AddExpose { + peering, flavour, .. + } + | Op::SetFlavour { + peering, flavour, .. + } => { + let mut footprint = Footprint::of([], [*peering]); + if *flavour == Flavour::Everything + && let Some(spec) = draft.peerings.get(peering) + { + footprint.vpcs.insert(spec.left); + footprint.vpcs.insert(spec.right); + } + footprint + } } } @@ -758,6 +800,8 @@ impl Op { || !draft.vpcs.contains(&left) || !draft.vpcs.contains(&right) || draft.peering_between(left, right).is_some() + || draft.beside_everything(left) + || draft.beside_everything(right) { return None; } @@ -874,6 +918,9 @@ fn add_expose( if flavour.is_directional() && spec.has_directional(side.other()) { return None; } + if flavour == Flavour::Everything && !may_expose_everything(draft, peering, side) { + return None; + } draft .peerings .get_mut(&peering) @@ -900,6 +947,17 @@ fn respecting_guard(draft: &mut Draft, peering: PeeringHandle, undo: Undo) -> Op None } +fn may_expose_everything(draft: &Draft, peering: PeeringHandle, side: Side) -> bool { + let Some(spec) = draft.peerings.get(&peering) else { + return false; + }; + !spec.has_everything(side) + && !spec.has_everything(side.other()) + && [Side::Left, Side::Right] + .into_iter() + .all(|which| draft.peerings_of(spec.vpc(which)) == 1) +} + fn set_flavour( draft: &mut Draft, peering: PeeringHandle, @@ -912,6 +970,12 @@ fn set_flavour( return None; } let index = spec.exposes(side).iter().position(|e| e.slot == slot)?; + if flavour == Flavour::Everything + && spec.exposes(side)[index].flavour != Flavour::Everything + && !may_expose_everything(draft, peering, side) + { + return None; + } let exposes = draft .peerings .get_mut(&peering) @@ -1253,7 +1317,12 @@ fn draw_add_expose(driver: &mut D, draft: &Draft) -> Option { peering, side, slot, - flavour: draw_flavour(driver, spec, side)?, + flavour: draw_flavour( + driver, + spec, + side, + may_expose_everything(draft, peering, side), + )?, }) } @@ -1294,11 +1363,20 @@ fn draw_set_flavour(driver: &mut D, draft: &Draft) -> Option { let (peering, side, slot) = pick(driver, &exposes)?; let spec = draft.peerings.get(&peering)?; + let already = spec + .exposes(side) + .iter() + .any(|expose| expose.slot == slot && expose.flavour == Flavour::Everything); Some(Op::SetFlavour { peering, side, slot, - flavour: draw_flavour(driver, spec, side)?, + flavour: draw_flavour( + driver, + spec, + side, + already || may_expose_everything(draft, peering, side), + )?, }) } @@ -1325,17 +1403,24 @@ fn draw_set_guard(driver: &mut D, draft: &Draft) -> Option { }) } -fn draw_flavour(driver: &mut D, spec: &PeeringSpec, side: Side) -> Option { - const ORDERED: [Flavour; 4] = [ +fn draw_flavour( + driver: &mut D, + spec: &PeeringSpec, + side: Side, + everything: bool, +) -> Option { + const ORDERED: [Flavour; 5] = [ Flavour::Forward, Flavour::StaticNat, Flavour::PortForward, Flavour::Masquerade, + Flavour::Everything, ]; - let legal: &[Flavour] = if spec.has_directional(side.other()) { + let end = if everything { 5 } else { 4 }; + let legal = if spec.has_directional(side.other()) { &ORDERED[..2] } else { - &ORDERED + &ORDERED[..end] }; pick(driver, legal) } @@ -1459,7 +1544,7 @@ mod tests { #[test] fn every_sequence_builds_a_valid_configuration() { - let flavours = [const { AtomicUsize::new(0) }; 4]; + let flavours = [const { AtomicUsize::new(0) }; 5]; let guards = [const { AtomicUsize::new(0) }; 6]; check!() @@ -1486,6 +1571,7 @@ mod tests { Flavour::Masquerade => 1, Flavour::StaticNat => 2, Flavour::PortForward => 3, + Flavour::Everything => 4, }] .fetch_add(1, Relaxed); } @@ -1542,7 +1628,13 @@ mod tests { assert_every_shape_built(&flavours, &guards); } - const FLAVOURS: [&str; 4] = ["forward", "masquerade", "static-nat", "port-forward"]; + const FLAVOURS: [&str; 5] = [ + "forward", + "masquerade", + "static-nat", + "port-forward", + "everything", + ]; const GUARDS: [&str; 6] = [ "open", "permit", @@ -1552,7 +1644,7 @@ mod tests { "deny", ]; - fn assert_every_shape_built(flavours: &[AtomicUsize; 4], guards: &[AtomicUsize; 6]) { + fn assert_every_shape_built(flavours: &[AtomicUsize; 5], guards: &[AtomicUsize; 6]) { let show = |names: &[&str], counts: &[AtomicUsize]| { names .iter() @@ -1677,8 +1769,8 @@ mod tests { } fn conflict(draft: &Draft, first: Op, second: Op) -> bool { - let (rw1, ww1) = (first.reads(), first.writes(draft)); - let (rw2, ww2) = (second.reads(), second.writes(draft)); + let (rw1, ww1) = (first.reads(draft), first.writes(draft)); + let (rw2, ww2) = (second.reads(draft), second.writes(draft)); ww1.intersects(&ww2) || ww1.intersects(&rw2) || rw1.intersects(&ww2) } diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index 12db197b12..cc34079d3b 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -109,10 +109,7 @@ const REACH: &[(&str, Reach)] = &[ "VpcManifest.exposes", Reach::Determined("one per `AddExpose`, in slot order"), ), - ( - "VpcExpose.default", - Reach::Fixed("false; neither `VpcExpose::empty` nor any operation sets it."), - ), + ("VpcExpose.default", Reach::Spans(&["false", "true"])), ( "VpcExpose.ips", Reach::Determined("one prefix, from the expose's peering, side and slot"), diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 2ad84ecf18..71b4027a9b 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -750,6 +750,7 @@ pub(crate) mod derive { use config::external::overlay::ValidatedOverlay; use config::external::overlay::algebra::Draft; use config::external::overlay::vpcpeering::ValidatedExpose; + use lpm::prefix::with_ports::L4Protocol; use lpm::prefix::{Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; #[derive(Debug, Clone, Copy)] @@ -878,6 +879,13 @@ pub(crate) mod derive { let inward = peer_source_of(peering, v.host, ValidatedExpose::can_init_connection); + if expose + .nat_proto() + .is_some_and(|proto| proto == L4Protocol::Tcp) + { + continue; + } + if expose.has_port_forwarding() { let (Some(outside), Some(inside_entry)) = ( expose.public_ips().into_iter().next(), From 02d3b8a50f3b9faaac186a53b797e0840717b55c Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 24 Aug 2026 15:35:49 -0600 Subject: [PATCH 16/19] test(dataplane): reject traffic to excluded addresses Existing exclusion coverage drew traffic only from the effective address set, so a matcher that ignored every exclusion still passed. Aim packets directly at excluded addresses and require them to be unreachable. Exempt a manifest with a default expose because its default route legitimately covers the hole, and mark the census row as behaviorally enforced. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 26 +++- config/src/external/overlay/completeness.rs | 12 +- dataplane/src/packet_processor/fuzz.rs | 124 ++++++++++++++++++++ 3 files changed, 160 insertions(+), 2 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index e651711afb..0d1932828e 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -2,7 +2,7 @@ // Copyright Open Network Fabric Authors use std::collections::{BTreeMap, BTreeSet}; -use std::net::Ipv4Addr; +use std::net::{IpAddr, Ipv4Addr}; use std::ops::Bound::Included; use std::time::Duration; @@ -520,6 +520,30 @@ impl Draft { !spec.guard.silences(spec, side, nth) } + #[must_use] + pub fn unexposed_address(&self, peering: &str, local: &str, nth: usize) -> Option { + let (handle, spec) = self + .peerings + .iter() + .find(|(handle, _)| handle.name() == peering)?; + let side = [Side::Left, Side::Right] + .into_iter() + .find(|side| spec.vpc(*side).name() == local)?; + let expose = *spec.exposes(side).get(nth)?; + if !expose.excludes() || expose.flavour == Flavour::Masquerade { + return None; + } + if spec.has_everything(side) { + return None; + } + let base = match expose.flavour { + Flavour::Forward => PRIVATE_BASE, + Flavour::StaticNat => PUBLIC_BASE, + Flavour::Masquerade | Flavour::PortForward | Flavour::Everything => return None, + }; + Some(excluded_slice(handle.block(side, expose.slot), base).as_address()) + } + #[must_use] pub fn guard_named(&self, name: &str) -> Option { self.peerings diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index cc34079d3b..7f00fec5b4 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -37,7 +37,17 @@ const REACH: &[(&str, Reach)] = &[ ("Vpc.vni", Reach::Determined("`VpcHandle::vni`")), ( "Vpc.interfaces", - Reach::Fixed("empty; the algebra has no operation that attaches VPC interfaces."), + Reach::Fixed( + "empty, and left that way on purpose. An operation attaching one is easy and would be \ + the wrong thing: nothing reads the field. `Vpc::validate` clones it into \ + `ValidatedVpc` without checking anything about it, `ValidatedVpc::interfaces` has no \ + callers, and the interfaces that reach the kernel and FRR come from the *internal* \ + config's vrf tables instead -- see `mgmt::vpc_manager` and \ + `converters::k8s::config::underlay`. Filling this in would move the row and cover \ + nothing, which is the one failure mode this whole record exists to prevent. The thing \ + worth doing is upstream of here: either the field has a consumer and this record \ + should follow it there, or it does not and it should go.", + ), ), ( "Vpc.peerings", diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 71b4027a9b..61326805d8 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -834,6 +834,61 @@ pub(crate) mod derive { move |named| draft.carries(named.peering, named.local, named.nth) } + #[derive(Debug)] + pub(crate) struct Probe { + path: super::routed::Path, + from: IpAddr, + at: IpAddr, + sport: u16, + dport: u16, + } + + impl Probe { + pub(crate) fn packet(&self) -> Option> { + let inner = super::round_trip::udp(self.from, self.at, self.sport, self.dport)?; + Some(super::routed::tunnelled_from(self.path.from(), &inner)) + } + } + + pub(crate) fn probes_for( + overlay: &ValidatedOverlay, + vary: &[Vary], + draft: &Draft, + ) -> Vec { + let mut probes = Vec::new(); + let mut nth = 0usize; + for vpc in overlay.vpc_table().values() { + for peering in vpc.peerings() { + for (which, _) in peering.local().valexp().iter().enumerate() { + let Some(v) = vary.get(nth % vary.len().max(1)).copied() else { + continue; + }; + nth += 1; + let Some(at) = draft.unexposed_address(peering.name(), vpc.name(), which) + else { + continue; + }; + let Some(from) = + peer_source_of(peering, v.host, ValidatedExpose::can_init_connection) + else { + continue; + }; + if from.is_ipv4() != at.is_ipv4() { + continue; + } + probes.push(Probe { + path: super::routed::Path::new(peering.remote_vni(), vpc.vni()), + from, + at, + sport: v.sport, + dport: v.dport, + }); + } + } + } + probes + } + pub(crate) fn loads_carried( overlay: &ValidatedOverlay, vary: &[Vary], @@ -3108,6 +3163,71 @@ mod generated { of it and would hold with the acl removed", ); } + + #[tokio::test] + #[dpdk::with_eal] + async fn an_excluded_address_is_not_reachable() { + static AIMED: AtomicU64 = AtomicU64::new(0); + static REFUSED: AtomicU64 = AtomicU64::new(0); + + bolero::check!() + .with_max_len(MAX_INPUT_LEN) + .with_generator(Generated) + .for_each(|(ops, vary, _schedule)| { + let draft = Sequence::fold(ops); + let validated = draft + .overlay() + .unwrap_or_else(|e| panic!("{ops:?} does not assemble: {e}")) + .validate() + .unwrap_or_else(|e| panic!("{ops:?} does not validate: {e}")); + + let vnis: Vec = validated + .vpc_table() + .values() + .map(config::external::overlay::vpc::ValidatedVpc::vni) + .collect(); + if vnis.is_empty() { + return; + } + let probes = super::derive::probes_for(&validated, vary, &draft); + if probes.is_empty() { + return; + } + + let mut fabric = Fabric::routed_over_validated(&validated, topology(&vnis)); + for probe in probes { + let Some(packet) = probe.packet() else { + continue; + }; + let seen = verdict(&fabric.worker().send(packet)); + AIMED.fetch_add(1, Ordering::Relaxed); + assert!( + matches!(seen, Verdict::Dropped(_)), + "an address the configuration excludes was reached: {seen:?} for {probe:?}" + ); + if seen == Verdict::Dropped(DoneReason::Filtered) { + REFUSED.fetch_add(1, Ordering::Relaxed); + } + } + }); + + let (aimed, refused) = ( + AIMED.load(Ordering::Relaxed), + REFUSED.load(Ordering::Relaxed), + ); + eprintln!("excluded-addresses-aimed-at={aimed} (refused as unplaceable {refused})"); + super::assert_covered( + aimed > 0, + "no packet was ever aimed at an excluded address, so this asserted nothing -- either \ + no generated expose carves a slice out of its advertised range, or none of those was \ + reachable from a peer", + ); + super::assert_covered( + refused > 0, + "every excluded address was refused for some reason other than being unplaceable, so \ + the exclusion may not be what refused any of them", + ); + } } #[cfg(test)] @@ -3619,6 +3739,10 @@ mod routed { Self::new(vni(LOCAL_VNI), vni(REMOTE_VNI)) } + pub(crate) fn from(self) -> Vni { + self.from + } + fn reversed(self) -> Self { Self::new(self.to, self.from) } From dc341eb51ef8f04e482658a75ca1ee1496d3d049 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 14 Sep 2026 21:03:59 -0600 Subject: [PATCH 17/19] fix(config): make PermitByProtocol's exception narrow to the prefix it names The `-except` denial inherited `src_any_ports`/`dst_any_ports` = `[1..65535]` from the `rule` closure and only narrowed `pattern.src`/`dst`. Those ranges mean "any address within the peering, restricted to these ports", and `AclRule::validate_patterns_coverage` materializes them back into `src`/`dst`, so the narrowing was undone: the rule denied *all TCP across the peering* rather than the one prefix it was built to except. `PermitExcept` carries empty vectors already, so only the protocol-narrowed guard was affected -- and it was invisible because every derived load is UDP, so no property ever sent traffic the broken denial could match. It also contradicted the `AclPattern.src`/`dst` rows in the completeness census, which record the exception as address-scoped. Clear the matching `*_any_ports` when narrowing, and assert it over generated configurations: a `-except` denial that names a prefix on a side must not also carry any-ports on that side. Break-tested by restoring the old two lines -- `every_sequence_builds_a_valid_configuration` fails with the prefix and the ranges that undo it. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/algebra.rs | 48 ++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/config/src/external/overlay/algebra.rs b/config/src/external/overlay/algebra.rs index 0d1932828e..1a6007e5d9 100644 --- a/config/src/external/overlay/algebra.rs +++ b/config/src/external/overlay/algebra.rs @@ -125,9 +125,24 @@ impl Guard { denial.name = format!("{}-except", denial.name); denial.action = AclAction::Deny; let named = PrefixPortsSet::from([PrefixWithOptionalPorts::from(prefix)]); + // Clearing the matching `*_any_ports` is what makes the narrowing narrow. + // Those ranges mean "any address within the peering, restricted to these + // ports" and `AclRule::validate_patterns_coverage` materializes them back + // into `src`/`dst`. `PermitByProtocol` inherits `EVERY_PORT` on both sides + // from `rule`, so leaving them set unions the whole manifest back over the + // excepted prefix and the denial matches every TCP packet across the + // peering rather than the one prefix it names. `PermitExcept` carries empty + // vectors already, so this only bites the protocol-narrowed guard -- and it + // was invisible because every derived load is UDP. match which { - Narrowing::Source => denial.pattern.src = named, - Narrowing::Destination => denial.pattern.dst = named, + Narrowing::Source => { + denial.pattern.src = named; + denial.pattern.src_any_ports = Vec::new(); + } + Narrowing::Destination => { + denial.pattern.dst = named; + denial.pattern.dst_any_ports = Vec::new(); + } } if self == Guard::PermitByProtocol { denial.pattern.proto = AclProtoMatch::Tcp; @@ -1632,6 +1647,35 @@ mod tests { handle, spec.guard() ); + // A `-except` denial that names a prefix must not also carry + // `*_any_ports` on that side. Those mean "any address within the + // peering, restricted to these ports" and `validate_patterns_coverage` + // materializes them back into `src`/`dst`, so keeping them unions the + // whole manifest over the prefix and the rule denies its entire protocol + // across the peering rather than the exception it was built to express. + for rule in acl.iter().flat_map(|acl| acl.rules()) { + if rule.action != AclAction::Deny || !rule.name.ends_with("-except") { + continue; + } + assert!( + rule.pattern.src.is_empty() || rule.pattern.src_any_ports.is_empty(), + "{:?}: the -except denial narrows its source to {:?} but keeps \ + src_any_ports {:?}, which validation unions back to the whole \ + peering", + handle, + rule.pattern.src, + rule.pattern.src_any_ports + ); + assert!( + rule.pattern.dst.is_empty() || rule.pattern.dst_any_ports.is_empty(), + "{:?}: the -except denial narrows its destination to {:?} but keeps \ + dst_any_ports {:?}, which validation unions back to the whole \ + peering", + handle, + rule.pattern.dst, + rule.pattern.dst_any_ports + ); + } guards[match observed { Guard::Open => 0, Guard::Permit => 1, From ec2337cdd59a67ed8dc251dc9a395d6ff1c9948f Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 21 Sep 2026 00:02:25 -0600 Subject: [PATCH 18/19] test(dataplane): use the shared fleet accessor in race regressions Signed-off-by: Daniel Noland --- dataplane/src/packet_processor/fuzz.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dataplane/src/packet_processor/fuzz.rs b/dataplane/src/packet_processor/fuzz.rs index 61326805d8..f81c05e171 100644 --- a/dataplane/src/packet_processor/fuzz.rs +++ b/dataplane/src/packet_processor/fuzz.rs @@ -2230,7 +2230,7 @@ mod acl { ACL reverse lookup" ); - fabric.fleet.enact(&overlay, Enact::Everything); + fabric.fleet().enact(&overlay, Enact::Everything); assert_eq!(fabric.fleet.blueprint.pipeline.genid(), FIRST_GENID + 1); assert_eq!( fabric.fleet.blueprint.pipeline.staging_genid(), @@ -2265,7 +2265,7 @@ mod acl { Enact::StaticNat, Enact::OpenGeneration, ] { - fabric.fleet.enact(&overlay, step); + fabric.fleet().enact(&overlay, step); } assert_eq!(fabric.fleet.blueprint.pipeline.genid(), FIRST_GENID); assert_eq!( @@ -2305,7 +2305,7 @@ mod acl { .expect("a forwarded request has a destination port"); // Publish the new generation. - fabric.fleet.enact(&overlay, Enact::PublishGeneration); + fabric.fleet().enact(&overlay, Enact::PublishGeneration); assert_eq!(fabric.fleet.blueprint.pipeline.genid(), FIRST_GENID + 1); let mut answer = super::round_trip::udp(inside, outside, inside_port.get(), 40000) From 0f77ac84bdc33c2d1276931f82975a1496ec044a Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 21 Sep 2026 23:07:06 -0600 Subject: [PATCH 19/19] test(config): draw enough configurations to reach a one-rule ACL The census asserts what the algebra reaches, and `Acl.rules` records that it reaches one-, two- and three-rule ACLs. A one-rule ACL turns up in roughly one draw in a hundred, so 512 draws miss it every few hundred runs. That is rare enough never to reproduce locally -- 60 consecutive runs here found it every time -- and common enough to fail CI, which it did, reporting only ["2", "3"] observed in 512 draws. Raise the draw count to 2048. This is a floor on the rarest value any `Reach::Spans` entry names rather than a runtime budget: the survey draws 2048 cases in about 0.8s at the rate the CI runner manages, well inside the budget `just` exports, and `enough_draws` still refuses to assert coverage on a sample cut short. The alternative was to record `Acl.rules` as reaching only two and three, which would have dropped a value the algebra genuinely produces -- the opposite of what this census is for. Validation: the three census tests pass, drawing 2048 cases with no skip. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- config/src/external/overlay/completeness.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/config/src/external/overlay/completeness.rs b/config/src/external/overlay/completeness.rs index 7f00fec5b4..b33e91a06a 100644 --- a/config/src/external/overlay/completeness.rs +++ b/config/src/external/overlay/completeness.rs @@ -369,7 +369,14 @@ fn survey_nat(nat: &VpcExposeNat, seen: &mut Observed) { } } -const CASES: usize = 512; +/// How many configurations the census draws before it judges what the algebra reaches. +/// +/// This is a floor on the rarest value any `Reach::Spans` entry names, not a runtime budget: +/// the survey finishes well inside Bolero's time limit either way. `Acl.rules` is the binding +/// entry -- a one-rule ACL turns up in roughly one draw in a hundred, so 512 draws miss it +/// about once in a few hundred runs, which is often enough to fail CI and never often enough +/// to reproduce locally. 2048 puts that below the rate at which anything else here fails. +const CASES: usize = 2048; /// Survey up to `CASES` configurations and count the draws. ///