From a02089e4eeed63d2bf5af8570df442e77ee6dcb9 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 31 Jul 2026 16:20:09 -0600 Subject: [PATCH 1/7] test(flow-filter): verify metadata against config Compare flowless packet metadata with the config routing oracle. Extend generated probes and packet builders to cover all NAT modes and IPv6 traffic. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- config/src/external/overlay/vpc.rs | 3 +- flow-filter/src/context/fuzz.rs | 3 +- flow-filter/src/context/mod.rs | 2 +- flow-filter/src/fuzz_gen.rs | 86 ++++++++----- flow-filter/src/test_utils.rs | 28 ++++ flow-filter/src/tests.rs | 197 ++++++++++++++++++++++++++++- 6 files changed, 281 insertions(+), 38 deletions(-) diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index a8665e8c0a..e05c31267a 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -151,8 +151,7 @@ impl ValidatedPeering { } fn validate_nat_combinations(&self) -> ConfigResult { - // If stateful NAT is set up on one side of the peering, we don't support NAT (static or - // stateful) on the other side. + // Stateful NAT cannot appear on both sides; static NAT is compatible with every mode. let mut local_has_masquerading = false; let mut local_has_port_forwarding = false; for expose in self.local.valexp() { diff --git a/flow-filter/src/context/fuzz.rs b/flow-filter/src/context/fuzz.rs index 8d790713ae..aa4c0e7eff 100644 --- a/flow-filter/src/context/fuzz.rs +++ b/flow-filter/src/context/fuzz.rs @@ -67,7 +67,8 @@ fn consider(best: &mut Option<(Precedence, T)>, precedence: Precedence, value } /// Answer a route lookup directly from the validated overlay. -fn oracle_lookup(overlay: &ValidatedOverlay, probe: &Probe) -> LookupResult { +/// Shared by the context and NF metadata property tests. +pub(crate) fn oracle_lookup(overlay: &ValidatedOverlay, probe: &Probe) -> LookupResult { let Some(src_vpc) = overlay .vpc_table() .values() diff --git a/flow-filter/src/context/mod.rs b/flow-filter/src/context/mod.rs index 88cc7c8300..cd703a8403 100644 --- a/flow-filter/src/context/mod.rs +++ b/flow-filter/src/context/mod.rs @@ -10,7 +10,7 @@ use config::external::overlay::ValidatedOverlay; mod display; #[cfg(test)] -mod fuzz; +pub(crate) mod fuzz; mod tables; #[cfg(test)] mod tests; diff --git a/flow-filter/src/fuzz_gen.rs b/flow-filter/src/fuzz_gen.rs index 819ad985f3..73e2388042 100644 --- a/flow-filter/src/fuzz_gen.rs +++ b/flow-filter/src/fuzz_gen.rs @@ -71,6 +71,14 @@ impl FwProto { FwProto::Udp => Some(L4Protocol::Udp), } } + + /// A protocol accepted by this expose. Generated `Any` probes use TCP. + fn probe_next_header(self) -> NextHeader { + match self { + FwProto::Any | FwProto::Tcp => NextHeader::TCP, + FwProto::Udp => NextHeader::UDP, + } + } } #[derive(Debug, Clone, Copy, TypeGenerator)] @@ -95,10 +103,6 @@ impl ExposeSpec { !matches!(self, ExposeSpec::Plain | ExposeSpec::StaticNat) } - fn has_nat(self) -> bool { - !matches!(self, ExposeSpec::Plain) - } - /// Whether this expose gives the source side of a route an unconstrained, connection-initiating /// match: a plain / static-nat / masquerade private block (a `/24` or `/120` with no port /// constraint, `can_init_connection`). Port forwarding cannot initiate, so a pure @@ -110,10 +114,19 @@ impl ExposeSpec { ) } - /// Where a destination address this expose matches lives, or `None` if it only matches - /// port-forwarded destinations (skipped -- those need a specific public port). `Some(true)` - /// means the public block (NAT exposes translate destinations into it); `Some(false)` means the - /// private block (a plain expose's public IPs are its private IPs). + /// Protocols for probes targeting this expose's port-forwarded destination. + fn portfw_protos(self) -> Vec { + match self { + ExposeSpec::Plain | ExposeSpec::StaticNat | ExposeSpec::Masquerade => Vec::new(), + ExposeSpec::PortForwarding(proto) + | ExposeSpec::MasqueradeNestingPortFw(proto) + | ExposeSpec::MasqueradeSameLenPortFw(proto) => vec![proto], + ExposeSpec::PortFwProtoPair => vec![FwProto::Tcp, FwProto::Udp], + } + } + + /// Whether a matching destination uses the public or private block. + /// Returns `None` for exposes that require a port-forwarding probe. fn dest_public_space(self) -> Option { match self { ExposeSpec::Plain => Some(false), @@ -147,14 +160,13 @@ impl ManifestSpec { self.expose_specs().any(ExposeSpec::is_stateful) } - fn has_nat(&self) -> bool { - self.expose_specs().any(ExposeSpec::has_nat) - } - - fn strip_nat(&mut self) { + /// Replace every stateful-NAT expose with a static-NAT one. + /// + /// This makes both peering sides compatible while retaining static-NAT combinations. + fn strip_stateful_nat(&mut self) { for slot in self.exposes.iter_mut().flatten() { - if slot.has_nat() { - *slot = ExposeSpec::Plain; + if slot.is_stateful() { + *slot = ExposeSpec::StaticNat; } } } @@ -218,11 +230,9 @@ impl OverlaySpec { if peering.local.default && peering.remote.default { peering.remote.drop_default(); } - // Stateful NAT on one side of a peering forbids any NAT on the other side. - if peering.local.has_stateful() && peering.remote.has_nat() { - peering.remote.strip_nat(); - } else if peering.remote.has_stateful() && peering.local.has_nat() { - peering.local.strip_nat(); + // At most one side of a peering may use stateful NAT. + if peering.local.has_stateful() && peering.remote.has_stateful() { + peering.remote.strip_stateful_nat(); } } // Each VPC may see at most one default destination across all of its peerings. A default @@ -296,9 +306,9 @@ impl OverlaySpec { } } -/// Append one guaranteed-routing probe for each (source-capable local expose, matchable remote -/// expose) pair of a peering. The source lands at host `.1` of a can-init private block and the -/// destination at host `.1` of the peer's matching block +/// Append a routing probe for each compatible pair of local and remote exposes. +/// +/// Port-forwarding probes target [`FW_HOST`] and [`FW_PUBLIC_PORTS`]. Other probes use host `.1`. fn derive_routing_probes( out: &mut Vec, src_vni: u32, @@ -313,16 +323,26 @@ fn derive_routing_probes( } let src_ip = block_addr(local_base + li as u8, 1, false, v6); for (ri, rspec) in remote.expose_specs().enumerate() { - let Some(dst_public) = rspec.dest_public_space() else { - continue; - }; - out.push(Probe { - src_vpcd, - src_ip, - dst_ip: block_addr(remote_base + ri as u8, 1, dst_public, v6), - proto: NextHeader::TCP, - ports: Some((1, 1)), - }); + let dst_block = remote_base + ri as u8; + if let Some(dst_public) = rspec.dest_public_space() { + out.push(Probe { + src_vpcd, + src_ip, + dst_ip: block_addr(dst_block, 1, dst_public, v6), + proto: NextHeader::TCP, + ports: Some((1, 1)), + }); + } + for proto in rspec.portfw_protos() { + out.push(Probe { + src_vpcd, + src_ip, + dst_ip: block_addr(dst_block, FW_HOST, true, v6), + proto: proto.probe_next_header(), + // Source exposes do not constrain ports. + ports: Some((1, FW_PUBLIC_PORTS.0)), + }); + } } } } diff --git a/flow-filter/src/test_utils.rs b/flow-filter/src/test_utils.rs index c1e6513483..3c919b6038 100644 --- a/flow-filter/src/test_utils.rs +++ b/flow-filter/src/test_utils.rs @@ -200,6 +200,34 @@ pub(crate) fn build_icmp_packet(src: Ipv4Addr, dst: Ipv4Addr) -> Headers { .unwrap() } +pub(crate) fn build_udp_packet_v6(src: Ipv6Addr, dst: Ipv6Addr, sport: u16, dport: u16) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv6(|ip| { + ip.set_source(UnicastIpv6Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .udp(|udp| { + udp.set_source(UdpPort::try_from(sport).unwrap()); + udp.set_destination(UdpPort::try_from(dport).unwrap()); + }) + .build_headers() + .unwrap() +} + +/// Build an ICMPv6 packet with an `ICMP6` next header. +pub(crate) fn build_icmp_packet_v6(src: Ipv6Addr, dst: Ipv6Addr) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv6(|ip| { + ip.set_source(UnicastIpv6Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .icmp6(|_| {}) + .build_headers() + .unwrap() +} + pub(crate) fn build_tcp_packet_v6(src: Ipv6Addr, dst: Ipv6Addr, sport: u16, dport: u16) -> Headers { HeaderStack::new() .eth(|_| {}) diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index 2e2c84107b..a650202671 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -5,13 +5,14 @@ #![cfg(test)] -use crate::FlowFilter; use crate::context::{FlowFilterContext, FlowFilterContextWriter}; +use crate::fuzz_gen::Probe; use crate::test_utils::{ build_icmp_packet, build_nonip_packet, build_tcp_packet, build_tcp_packet_v6, build_udp_packet, context, expose, expose_masquerade, expose_port_forwarding, expose_static, peering, v4, v6, vpcd, }; +use crate::{FlowFilter, LookupResult, NatRequirement}; use concurrency::sync::Arc; use lpm::prefix::L4Protocol; use net::FlowKey; @@ -1061,3 +1062,197 @@ fn burst_processing_upholds_structural_invariants() { } }); } + +// Generated-overlay metadata oracle for packets without attached flows. + +/// What the NF must leave on a flowless packet. +#[derive(Debug, PartialEq, Eq)] +enum NfOutcome { + /// Dropped for this reason, with no destination stamped. + Dropped(Option), + Routed { + dst_vpcd: Option, + masquerade: bool, + static_nat_src: bool, + static_nat_dst: bool, + port_forwarding: bool, + /// Whether the pre-translation flow key was retained. + flow_key: bool, + }, +} + +/// Convert a config lookup into the expected flowless-packet outcome. +fn expected_outcome(result: LookupResult) -> NfOutcome { + let (dst_vpcd, dst_nat, src_nat) = match result { + LookupResult::Route(route) => route, + LookupResult::SourceMiss(_) | LookupResult::DestinationMiss => { + return NfOutcome::Dropped(Some(DoneReason::Filtered)); + } + }; + if dst_nat == Some(NatRequirement::Masquerade) { + return NfOutcome::Dropped(Some(DoneReason::Filtered)); + } + + let masquerade = src_nat == Some(NatRequirement::Masquerade); + let static_nat_src = src_nat == Some(NatRequirement::Static); + let static_nat_dst = dst_nat == Some(NatRequirement::Static); + let port_forwarding = src_nat == Some(NatRequirement::PortForwarding) + || dst_nat == Some(NatRequirement::PortForwarding); + NfOutcome::Routed { + dst_vpcd: Some(dst_vpcd), + masquerade, + static_nat_src, + static_nat_dst, + port_forwarding, + // Combined stateful and static NAT retains the pre-translation addresses. + flow_key: (masquerade || port_forwarding) && (static_nat_src || static_nat_dst), + } +} + +fn observed_outcome(pkt: &Packet) -> NfOutcome { + if pkt.is_done() { + return NfOutcome::Dropped(pkt.get_done()); + } + let meta = pkt.meta(); + NfOutcome::Routed { + dst_vpcd: meta.dst_vpcd, + masquerade: meta.requires_masquerade(), + static_nat_src: meta.requires_static_nat_src(), + static_nat_dst: meta.requires_static_nat_dst(), + port_forwarding: meta.requires_port_forwarding(), + flow_key: meta.flow_key.is_some(), + } +} + +/// Build a packet and normalize its probe to the values carried on the wire. +/// Returns `None` when the source and destination use different IP versions. +fn probe_packet(probe: &Probe) -> Option<(Packet, Probe)> { + use crate::test_utils::{build_icmp_packet_v6, build_udp_packet_v6}; + use net::ip::NextHeader; + + let mut probe = *probe; + if let Some((sport, dport)) = probe.ports.as_mut() { + *sport = (*sport).max(1); + *dport = (*dport).max(1); + } + + let headers = match (probe.src_ip, probe.dst_ip) { + (std::net::IpAddr::V4(src), std::net::IpAddr::V4(dst)) => match probe.ports { + Some((sp, dp)) if probe.proto == NextHeader::TCP => build_tcp_packet(src, dst, sp, dp), + Some((sp, dp)) if probe.proto == NextHeader::UDP => build_udp_packet(src, dst, sp, dp), + _ => { + probe.proto = NextHeader::ICMP; + probe.ports = None; + build_icmp_packet(src, dst) + } + }, + (std::net::IpAddr::V6(src), std::net::IpAddr::V6(dst)) => match probe.ports { + Some((sp, dp)) if probe.proto == NextHeader::TCP => { + build_tcp_packet_v6(src, dst, sp, dp) + } + Some((sp, dp)) if probe.proto == NextHeader::UDP => { + build_udp_packet_v6(src, dst, sp, dp) + } + _ => { + probe.proto = NextHeader::ICMP6; + probe.ports = None; + build_icmp_packet_v6(src, dst) + } + }, + _ => return None, + }; + Some((packet(Some(probe.src_vpcd), headers), probe)) +} + +/// Check flowless-packet metadata against the config oracle for generated overlays. +/// Each routing, drop, NAT, and flow-key outcome must be reached. +#[test] +fn nf_metadata_matches_config_oracle() { + use crate::context::fuzz::oracle_lookup; + use crate::fuzz_gen::{OverlaySpec, ProbeSpec}; + use concurrency::sync::LazyLock; + use concurrency::sync::atomic::{AtomicU64, Ordering}; + + // Lazily initialized so this compiles under the loom backend, whose AtomicU64::new is not const. + static ROUTED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static MASQUERADE: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static STATIC_SRC: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static STATIC_DST: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static PORT_FORWARDING: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static FLOW_KEYED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static DROPPED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + bolero::check!() + .with_type::<(OverlaySpec, [ProbeSpec; 8])>() + .for_each(|(overlay_spec, probe_specs)| { + // The classifier is not safe to capture across Bolero's unwind boundary. + let built = overlay_spec.build(); + let (mut flow_filter, _writer) = + make_flow_filter(FlowFilterContext::for_test(&built.overlay)); + + // Derived probes cover routes; generated probes add misses and edge cases. + let derived = built.routing_probes.iter().copied(); + let generated = probe_specs.iter().map(|spec| spec.resolve(built.blocks)); + for probe in derived.chain(generated) { + let Some((pkt, probe)) = probe_packet(&probe) else { + continue; + }; + let expected = expected_outcome(oracle_lookup(&built.overlay, &probe)); + assert_eq!( + observed_outcome(&run(&mut flow_filter, pkt)), + expected, + "stamped metadata diverges from the configuration for {probe:?}\n\ + spec: {overlay_spec:?}", + ); + + match expected { + NfOutcome::Dropped(_) => { + DROPPED.fetch_add(1, Ordering::Relaxed); + } + NfOutcome::Routed { + masquerade, + static_nat_src, + static_nat_dst, + port_forwarding, + flow_key, + .. + } => { + ROUTED.fetch_add(1, Ordering::Relaxed); + for (hit, counter) in [ + (masquerade, &MASQUERADE), + (static_nat_src, &STATIC_SRC), + (static_nat_dst, &STATIC_DST), + (port_forwarding, &PORT_FORWARDING), + (flow_key, &FLOW_KEYED), + ] { + if hit { + counter.fetch_add(1, Ordering::Relaxed); + } + } + } + } + } + }); + + let counts = [ + ("routed", &ROUTED), + ("dropped", &DROPPED), + ("masquerade", &MASQUERADE), + ("static NAT (src)", &STATIC_SRC), + ("static NAT (dst)", &STATIC_DST), + ("port forwarding", &PORT_FORWARDING), + ("retaining a flow key", &FLOW_KEYED), + ] + .map(|(label, counter)| (label, counter.load(Ordering::Relaxed))); + eprintln!( + "coverage: {}", + counts + .iter() + .map(|(label, n)| format!("{n} {label}")) + .collect::>() + .join(", ") + ); + for (label, count) in counts { + assert!(count >= 1, "the {label} case was never reached"); + } +} From 40302d093ac7ee56bd54f027efe8437b564221b4 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 31 Jul 2026 16:50:48 -0600 Subject: [PATCH 2/7] test(flow-filter): fuzz packet header stacks Generate VLAN, IPv4, IPv6, extension, fragment, and transport headers. Check each packet with the config oracle and require every shape. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- flow-filter/Cargo.toml | 3 +- flow-filter/src/tests.rs | 351 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 353 insertions(+), 1 deletion(-) diff --git a/flow-filter/Cargo.toml b/flow-filter/Cargo.toml index 00c9063fec..627c93494c 100644 --- a/flow-filter/Cargo.toml +++ b/flow-filter/Cargo.toml @@ -32,4 +32,5 @@ acl = { workspace = true, features = ["reference"] } bolero = { workspace = true, features = ["std"] } dpdk = { workspace = true, features = ["test"] } lpm = { workspace = true, features = ["testing"] } -net = { workspace = true, features = ["builder"] } +# Enable generated header stacks for classifier tests. +net = { workspace = true, features = ["builder", "bolero"] } diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index a650202671..629a534a94 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -1109,6 +1109,26 @@ fn expected_outcome(result: LookupResult) -> NfOutcome { } } +/// Extract the lookup key seen by `FlowFilter::classify`. +/// Returns `None` for packets without an IP layer. +fn probe_from_packet(pkt: &Packet, src_vpcd: VpcDiscriminant) -> Option { + use net::headers::{TryIp, TryTransport}; + use std::num::NonZero; + + let net = pkt.try_ip()?; + Some(Probe { + src_vpcd, + src_ip: net.src_addr(), + dst_ip: net.dst_addr(), + proto: net.next_header(), + ports: pkt.try_transport().and_then(|t| { + t.src_port() + .map(NonZero::get) + .zip(t.dst_port().map(NonZero::get)) + }), + }) +} + fn observed_outcome(pkt: &Packet) -> NfOutcome { if pkt.is_done() { return NfOutcome::Dropped(pkt.get_done()); @@ -1256,3 +1276,334 @@ fn nf_metadata_matches_config_oracle() { assert!(count >= 1, "the {label} case was never reached"); } } + +// Generated header-stack coverage for packet classification. + +mod adversarial_headers { + use super::{ + NfOutcome, expected_outcome, make_flow_filter, observed_outcome, probe_from_packet, + }; + use crate::context::FlowFilterContext; + use crate::context::fuzz::oracle_lookup; + use crate::test_utils::{expose, expose_masquerade, expose_static, overlay, peering, vpcd}; + use bolero::{Driver, ValueGenerator}; + use concurrency::sync::LazyLock; + use concurrency::sync::atomic::{AtomicU64, Ordering}; + use config::external::overlay::ValidatedOverlay; + use net::buffer::TestBuffer; + use net::headers::Headers; + use net::headers::builder::ChainBase; + use net::ip::NextHeader; + use net::ipv4::UnicastIpv4Addr; + use net::ipv6::UnicastIpv6Addr; + use net::packet::{DoneReason, Packet, VpcDiscriminant}; + use net::parse::DeParse; + use pipeline::NetworkFunction; + use std::net::{Ipv4Addr, Ipv6Addr}; + + /// Source VPC for generated packets. + fn src_vpcd() -> VpcDiscriminant { + vpcd(100) + } + + /// Overlay with IPv4 NAT modes and an IPv6 route for generated packets. + fn wire_overlay() -> ValidatedOverlay { + overlay( + &[("vpc1", 100), ("vpc2", 200), ("vpc3", 300)], + vec![ + peering( + "vpc1-to-vpc2", + ( + "vpc1", + vec![ + expose("1.0.0.0/24"), + expose_static("2.0.0.0/24", "20.0.0.0/24"), + expose_masquerade("3.0.0.0/24", "30.0.0.0/24"), + ], + ), + ("vpc2", vec![expose("5.0.0.0/24")]), + ), + peering( + "vpc1-to-vpc3", + ("vpc1", vec![expose("2001:db8::/32")]), + ("vpc3", vec![expose("2001:db9::/32")]), + ), + ], + ) + } + + // Pin network prefixes while leaving host bits and all other fields generated. + + fn pin_v4(ip: &mut net::ipv4::Ipv4) { + let src = ip.source().inner().octets(); + // Networks 1-3 select each source NAT mode; network 4 misses. + let src_net = src[0] % 4 + 1; + ip.set_source( + UnicastIpv4Addr::new(Ipv4Addr::new(src_net, 0, 0, src[3])) + .unwrap_or_else(|e| unreachable!("pinned v4 source is unicast: {e:?}")), + ); + let dst = ip.destination().octets(); + // Network 5 routes; network 9 misses. + let dst_net = if dst[0].is_multiple_of(4) { 9 } else { 5 }; + ip.set_destination(Ipv4Addr::new(dst_net, 0, 0, dst[3])); + } + + fn pin_v6(ip: &mut net::ipv6::Ipv6) { + let src = ip.source().inner().octets(); + ip.set_source( + UnicastIpv6Addr::new(Ipv6Addr::new( + 0x2001, + 0x0db8, + 0, + 0, + 0, + 0, + 0, + u16::from(src[15]), + )) + .unwrap_or_else(|e| unreachable!("pinned v6 source is unicast: {e:?}")), + ); + let dst = ip.destination().octets(); + // Route three quarters of destinations and miss the rest. + let net = if dst[0].is_multiple_of(4) { + 0x0dbf + } else { + 0x0db9 + }; + ip.set_destination(Ipv6Addr::new( + 0x2001, + net, + 0, + 0, + 0, + 0, + 0, + u16::from(dst[15]), + )); + } + + /// Header shapes, named so coverage can be required for each one. + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum Shape { + /// No IP layer at all: the `NotIp` path. + NoIp, + V4Tcp, + V4Udp, + V4Icmp, + /// A VLAN tag between the Ethernet and IP layers. + VlanV4Tcp, + /// An IPv4 authentication header ahead of the transport. + V4AuthTcp, + V6Tcp, + V6Udp, + /// IPv6 extension-header chains, ahead of a transport header. + V6HopByHopTcp, + V6FragmentUdp, + } + + impl Shape { + /// Every shape, in selector and counter order. + const ALL: [Shape; 10] = [ + Shape::NoIp, + Shape::V4Tcp, + Shape::V4Udp, + Shape::V4Icmp, + Shape::VlanV4Tcp, + Shape::V4AuthTcp, + Shape::V6Tcp, + Shape::V6Udp, + Shape::V6HopByHopTcp, + Shape::V6FragmentUdp, + ]; + } + + /// Generate one concrete header stack for each shape. + /// `ValueGenerator` is not object-safe, so generation dispatches with a `match`. + struct AnyStack; + + impl ValueGenerator for AnyStack { + type Output = (Shape, Headers); + + fn generate(&self, driver: &mut D) -> Option<(Shape, Headers)> { + let selector = usize::from(driver.produce::()?) % Shape::ALL.len(); + let shape = Shape::ALL[selector]; + let headers = match shape { + Shape::NoIp => ChainBase::new().eth(|_| {}).generate(driver), + Shape::V4Tcp => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .tcp(|_| {}) + .generate(driver), + Shape::V4Udp => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .udp(|_| {}) + .generate(driver), + Shape::V4Icmp => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .icmp4(|_| {}) + .generate(driver), + Shape::VlanV4Tcp => ChainBase::new() + .eth(|_| {}) + .vlan(|_| {}) + .ipv4(pin_v4) + .tcp(|_| {}) + .generate(driver), + Shape::V4AuthTcp => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .ipv4_auth(|_| {}) + .tcp(|_| {}) + .generate(driver), + Shape::V6Tcp => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .tcp(|_| {}) + .generate(driver), + Shape::V6Udp => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .udp(|_| {}) + .generate(driver), + Shape::V6HopByHopTcp => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .hop_by_hop(|_| {}) + .tcp(|_| {}) + .generate(driver), + Shape::V6FragmentUdp => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .fragment(|_| {}) + .udp(|_| {}) + .generate(driver), + }?; + Some((shape, headers)) + } + } + + /// Serialize and parse a generated overlay packet. + fn wire_packet(headers: &Headers) -> Option> { + let mut buffer = TestBuffer::new(); + headers.deparse(buffer.as_mut()).ok()?; + let mut packet = Packet::new(buffer).ok()?; + packet.meta_mut().set_overlay(true); + packet.meta_mut().src_vpcd = Some(src_vpcd()); + Some(packet) + } + + /// Check generated header stacks against the config oracle. + #[test] + fn arbitrary_header_stacks_uphold_the_config_contract() { + // Lazily initialized so this compiles under the loom backend, whose AtomicU64::new is not + // const. + static UNPARSEABLE: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static NOT_IP: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static PORTLESS: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static EXOTIC_PROTO: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static ROUTED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static DROPPED: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + /// Packets reaching the NF, indexed by [`Shape`]. + static BY_SHAPE: LazyLock<[AtomicU64; Shape::ALL.len()]> = + LazyLock::new(|| std::array::from_fn(|_| AtomicU64::new(0))); + + let overlay = wire_overlay(); + + bolero::check!().with_generator(AnyStack).for_each( + |(shape, headers): &(Shape, Headers)| { + // The classifier is not safe to capture across Bolero's unwind boundary. + let (mut flow_filter, _writer) = + make_flow_filter(FlowFilterContext::for_test(&overlay)); + + let Some(packet) = wire_packet(headers) else { + UNPARSEABLE.fetch_add(1, Ordering::Relaxed); + return; + }; + BY_SHAPE[*shape as usize].fetch_add(1, Ordering::Relaxed); + + // Extract the key before the NF consumes the packet. + let probe = probe_from_packet(&packet, src_vpcd()); + if let Some(probe) = probe.as_ref() { + if probe.ports.is_none() { + PORTLESS.fetch_add(1, Ordering::Relaxed); + } + if !matches!( + probe.proto, + NextHeader::TCP | NextHeader::UDP | NextHeader::ICMP | NextHeader::ICMP6 + ) { + EXOTIC_PROTO.fetch_add(1, Ordering::Relaxed); + } + } else { + NOT_IP.fetch_add(1, Ordering::Relaxed); + } + + let out = flow_filter + .process([packet].into_iter()) + .next() + .unwrap_or_else(|| unreachable!("enforce keeps Filtered and NotIp packets")); + + let expected = match probe.as_ref() { + // No IP layer: dropped before any table is consulted. + None => NfOutcome::Dropped(Some(DoneReason::NotIp)), + Some(probe) => expected_outcome(oracle_lookup(&overlay, probe)), + }; + assert_eq!( + observed_outcome(&out), + expected, + "NF diverged from the configuration on {shape:?} stack {headers:?}", + ); + + match expected { + NfOutcome::Routed { .. } => ROUTED.fetch_add(1, Ordering::Relaxed), + NfOutcome::Dropped(_) => DROPPED.fetch_add(1, Ordering::Relaxed), + }; + }, + ); + + let counts = [ + ("unparseable", &UNPARSEABLE), + ("not-IP", &NOT_IP), + ("portless IP", &PORTLESS), + ("non-transport proto", &EXOTIC_PROTO), + ("routed", &ROUTED), + ("dropped", &DROPPED), + ] + .map(|(label, counter)| (label, counter.load(Ordering::Relaxed))); + eprintln!( + "coverage: {}", + counts + .iter() + .map(|(label, n)| format!("{n} {label}")) + .collect::>() + .join(", ") + ); + + let by_shape = + Shape::ALL.map(|shape| (shape, BY_SHAPE[shape as usize].load(Ordering::Relaxed))); + eprintln!( + "coverage by shape: {}", + by_shape + .iter() + .map(|(shape, n)| format!("{n} {shape:?}")) + .collect::>() + .join(", ") + ); + + // Require every outcome class produced by generated stacks. + for (label, count) in counts { + if label == "unparseable" { + continue; + } + assert!(count >= 1, "no {label} packet was ever generated"); + } + // Require every shape to round-trip through `wire_packet` and reach the NF. + for (shape, count) in by_shape { + assert!( + count >= 1, + "no {shape:?} packet ever reached the NF: the shape is no longer generated, or no \ + longer round-trips through a TestBuffer", + ); + } + } +} From 2e94476f33deabb30abc579eed17f80f741c8d8d Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 31 Jul 2026 16:55:29 -0600 Subject: [PATCH 3/7] test(flow-filter): generate expose exclusions Generate not/not_as exclusions and require multi-length prefix fans. Preserve static NAT address counts and port-forwarding probe hosts. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- flow-filter/src/context/fuzz.rs | 40 +++++++++++ flow-filter/src/fuzz_gen.rs | 113 +++++++++++++++++++++++++++----- 2 files changed, 138 insertions(+), 15 deletions(-) diff --git a/flow-filter/src/context/fuzz.rs b/flow-filter/src/context/fuzz.rs index aa4c0e7eff..27eee723a2 100644 --- a/flow-filter/src/context/fuzz.rs +++ b/flow-filter/src/context/fuzz.rs @@ -365,3 +365,43 @@ fn reference_lookup_matches_config_oracle() { } }); } + +/// Require generated exclusions to produce multi-length prefix fans. +/// Removing one host from a `/24` must produce lengths `/25` through `/32`. +#[test] +fn exclusions_reach_the_config_as_multi_length_prefix_fans() { + use std::collections::BTreeSet; + + // Lazily initialized so this compiles under the loom backend, whose AtomicU64::new is not const. + static WIDEST_SPREAD: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + + bolero::check!() + .with_type::() + .for_each(|overlay_spec| { + let built = overlay_spec.build(); + for vpc in built.overlay.vpc_table().values() { + for peering in vpc.peerings() { + let exposes = peering + .local() + .valexp() + .iter() + .chain(peering.remote().valexp()); + for expose in exposes { + for set in [expose.ips(), expose.public_ips()] { + let lengths: BTreeSet = + set.iter().map(|p| p.prefix().length()).collect(); + WIDEST_SPREAD.fetch_max(lengths.len() as u64, Ordering::Relaxed); + } + } + } + } + }); + + let spread = WIDEST_SPREAD.load(Ordering::Relaxed); + eprintln!("coverage: widest prefix-length spread in a single expose: {spread}"); + assert!( + spread >= 8, + "exclusions never produced a full prefix-length fan (widest spread was {spread}); \ + the generator is emitting single-block exposes and the priority ordering is untested", + ); +} diff --git a/flow-filter/src/fuzz_gen.rs b/flow-filter/src/fuzz_gen.rs index 73e2388042..b0138ed0fc 100644 --- a/flow-filter/src/fuzz_gen.rs +++ b/flow-filter/src/fuzz_gen.rs @@ -139,19 +139,74 @@ impl ExposeSpec { } } +/// An exclusion applied to an expose's private and public blocks. +/// All selections preserve probe host `.1` and port-forwarding host [`FW_HOST`]. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum ExcludeSel { + None, + /// The block's upper half (a `/25`, or `/121` for v6). + UpperHalf, + /// The block's second quarter, leaving runs on both sides of the hole. + SecondQuarter, + /// A single host in the upper half: the widest fan of prefix lengths a single exclusion can + /// produce. + UpperHost(u8), +} + +impl ExcludeSel { + /// The prefix to exclude from block `n`, or `None` to leave the block whole. + fn resolve(self, n: u8, public: bool, v6: bool) -> Option { + let (net4, net6) = if public { (20, "db9") } else { (10, "db8") }; + Some(match (self, v6) { + (ExcludeSel::None, _) => return None, + (ExcludeSel::UpperHalf, false) => format!("{net4}.{n}.0.128/25"), + (ExcludeSel::UpperHalf, true) => format!("2001:{net6}:0:{n:x}::80/121"), + (ExcludeSel::SecondQuarter, false) => format!("{net4}.{n}.0.64/26"), + (ExcludeSel::SecondQuarter, true) => format!("2001:{net6}:0:{n:x}::40/122"), + // Keep probe and port-forwarding hosts below the exclusion. + (ExcludeSel::UpperHost(h), false) => { + format!("{net4}.{n}.0.{}/32", h | 0x80) + } + (ExcludeSel::UpperHost(h), true) => { + format!("2001:{net6}:0:{n:x}::{:x}/128", h | 0x80) + } + }) + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct ExposeEntry { + spec: ExposeSpec, + /// Exclusion for this expose. Port forwarding ignores it because config forbids exclusions. + exclude: ExcludeSel, +} + +impl ExposeEntry { + const fn plain() -> Self { + Self { + spec: ExposeSpec::Plain, + exclude: ExcludeSel::None, + } + } +} + #[derive(Debug, Clone, Copy, TypeGenerator)] pub(crate) struct ManifestSpec { /// Up to two expose specs (some expand to two actual exposes). - exposes: [Option; 2], + exposes: [Option; 2], /// Whether the manifest carries a default (catch-all) expose. default: bool, } impl ManifestSpec { - fn expose_specs(&self) -> impl Iterator + '_ { + fn entries(&self) -> impl Iterator + '_ { self.exposes.iter().flatten().copied() } + fn expose_specs(&self) -> impl Iterator + '_ { + self.entries().map(|entry| entry.spec) + } + fn is_empty(&self) -> bool { self.exposes.iter().all(Option::is_none) && !self.default } @@ -165,8 +220,8 @@ impl ManifestSpec { /// This makes both peering sides compatible while retaining static-NAT combinations. fn strip_stateful_nat(&mut self) { for slot in self.exposes.iter_mut().flatten() { - if slot.is_stateful() { - *slot = ExposeSpec::StaticNat; + if slot.spec.is_stateful() { + slot.spec = ExposeSpec::StaticNat; } } } @@ -174,7 +229,7 @@ impl ManifestSpec { fn drop_default(&mut self) { self.default = false; if self.is_empty() { - self.exposes[0] = Some(ExposeSpec::Plain); + self.exposes[0] = Some(ExposeEntry::plain()); } } } @@ -211,11 +266,11 @@ impl OverlaySpec { spec.peerings[0] = Some(PeeringSpec { v6: false, local: ManifestSpec { - exposes: [Some(ExposeSpec::Plain), None], + exposes: [Some(ExposeEntry::plain()), None], default: false, }, remote: ManifestSpec { - exposes: [Some(ExposeSpec::Plain), None], + exposes: [Some(ExposeEntry::plain()), None], default: false, }, }); @@ -223,7 +278,7 @@ impl OverlaySpec { for peering in spec.peerings.iter_mut().flatten() { for manifest in [&mut peering.local, &mut peering.remote] { if manifest.is_empty() { - manifest.exposes[0] = Some(ExposeSpec::Plain); + manifest.exposes[0] = Some(ExposeEntry::plain()); } } // A default expose cannot face another default expose within one peering. @@ -353,19 +408,25 @@ fn vpc_name(index: usize) -> String { fn build_manifest(vpc_name: &str, spec: &ManifestSpec, v6: bool, blocks: &mut u8) -> VpcManifest { let mut exposes = Vec::new(); - for expose_spec in spec.expose_specs() { + for entry in spec.entries() { let n = *blocks; *blocks += 1; - match expose_spec { - ExposeSpec::Plain => exposes.push(plain(n, v6)), - ExposeSpec::StaticNat => exposes.push(static_nat(n, v6)), - ExposeSpec::Masquerade => exposes.push(masquerade(n, v6)), + let exclude = entry.exclude; + match entry.spec { + ExposeSpec::Plain => exposes.push(excluding(plain(n, v6), n, v6, exclude)), + ExposeSpec::StaticNat => { + exposes.push(excluding_both(static_nat(n, v6), n, v6, exclude)); + } + ExposeSpec::Masquerade => { + exposes.push(excluding_both(masquerade(n, v6), n, v6, exclude)); + } + // Only the masquerade half may carry an exclusion. ExposeSpec::MasqueradeNestingPortFw(proto) => { - exposes.push(masquerade(n, v6)); + exposes.push(excluding_both(masquerade(n, v6), n, v6, exclude)); exposes.push(portfw_host(n, v6, proto)); } ExposeSpec::MasqueradeSameLenPortFw(proto) => { - exposes.push(masquerade(n, v6)); + exposes.push(excluding_both(masquerade(n, v6), n, v6, exclude)); exposes.push(portfw_block(n, v6, proto)); } ExposeSpec::PortForwarding(proto) => exposes.push(portfw_host(n, v6, proto)), @@ -422,6 +483,28 @@ pub(crate) fn block_addr(n: u8, host: u8, public: bool, v6: bool) -> IpAddr { } } +/// Remove `exclude` from an expose's private prefixes. +fn excluding(expose: VpcExpose, n: u8, v6: bool, exclude: ExcludeSel) -> VpcExpose { + match exclude.resolve(n, false, v6) { + Some(prefix) => expose.not(prefix.as_str().into()), + None => expose, + } +} + +/// Remove matching exclusions from both sides to preserve static NAT address counts. +fn excluding_both(expose: VpcExpose, n: u8, v6: bool, exclude: ExcludeSel) -> VpcExpose { + let Some(private) = exclude.resolve(n, false, v6) else { + return expose; + }; + let Some(public) = exclude.resolve(n, true, v6) else { + unreachable!("resolve is None only for ExcludeSel::None, handled above"); + }; + expose + .not(private.as_str().into()) + .not_as(public.as_str().into()) + .unwrap_or_else(|e| unreachable!("exclusion on an expose with a public range: {e}")) +} + fn plain(n: u8, v6: bool) -> VpcExpose { VpcExpose::empty().ip(private_block(n, v6).as_str().into()) } From ab42e834ea143edfff0455df0030bc38c78cfae7 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 31 Jul 2026 16:56:36 -0600 Subject: [PATCH 4/7] test(flow-filter): cover protocol and flow edge cases Cover IPv6 extension headers, portless packets, and newer flows. Clarify which flow generations bypass the filter. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- flow-filter/src/lib.rs | 4 +- flow-filter/src/tests.rs | 153 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 2 deletions(-) diff --git a/flow-filter/src/lib.rs b/flow-filter/src/lib.rs index c2250bde67..7aa534d111 100644 --- a/flow-filter/src/lib.rs +++ b/flow-filter/src/lib.rs @@ -350,8 +350,8 @@ impl FlowFilter { return None; }; - // The flow has the same generation id as the current config. Small transient period aside, - // this means that the flow is up-to-date and we can bypass the filter + // Current and newer-generation flows bypass the filter. Workers may observe a new config + // generation after flows have already been stamped with it. debug!("{nfi}: Packet can bypass flow filter thanks to flow information"); Some(dst_vpcd) } diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index 629a534a94..cb9222387b 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -1607,3 +1607,156 @@ mod adversarial_headers { } } } + +// Protocol, port, and flow-generation edge cases. + +/// IPv6 extension headers occupy `Net::next_header()`, while `try_transport()` still finds the TCP +/// ports. Protocol-restricted exposes therefore do not match TCP behind an extension header. +#[test] +fn ipv6_extension_header_masks_the_transport_protocol() { + use net::headers::builder::HeaderStack; + use net::headers::{TryIp, TryTransport}; + use net::ipv6::UnicastIpv6Addr; + use net::tcp::TcpPort; + + let with_hop_by_hop = || { + HeaderStack::new() + .eth(|_| {}) + .ipv6(|ip| { + ip.set_source(UnicastIpv6Addr::new(v6("2001:db8::1")).unwrap()); + ip.set_destination(v6("2001:db9::5")); + }) + .hop_by_hop(|_| {}) + .tcp(|tcp| { + tcp.set_source(TcpPort::try_from(1234u16).unwrap()); + tcp.set_destination(TcpPort::try_from(80u16).unwrap()); + }) + .build_headers() + .unwrap() + }; + + // The protocol is hop-by-hop while transport parsing still reaches TCP. + let probe_packet = packet(Some(vpcd(100)), with_hop_by_hop()); + let net = probe_packet.try_ip().unwrap(); + assert_eq!( + net.next_header(), + net::ip::NextHeader::new(0), + "an extension header should occupy the next-header field", + ); + assert_eq!( + probe_packet + .try_transport() + .and_then(|t| t.dst_port()) + .map(std::num::NonZero::get), + Some(80), + "the transport header is still reachable behind the extension header", + ); + + // A TCP-restricted expose sees the hop-by-hop protocol. + let tcp_only = context( + &[("vpc1", 100), ("vpc2", 200)], + vec![peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose("2001:db8::/32")]), + ( + "vpc2", + vec![expose_port_forwarding( + "2001:db9::5/128", + (22, 22), + "2001:db9::5/128", + (80, 80), + Some(L4Protocol::Tcp), + )], + ), + )], + ); + let (mut flow_filter, _writer) = make_flow_filter(tcp_only); + let out = run(&mut flow_filter, packet(Some(vpcd(100)), with_hop_by_hop())); + assert_eq!( + out.get_done(), + Some(DoneReason::Filtered), + "a TCP-restricted expose does not see this packet as TCP, so nothing covers it", + ); + + // An unrestricted expose confirms that the address remains routable. + let any_proto = context( + &[("vpc1", 100), ("vpc2", 200)], + vec![peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose("2001:db8::/32")]), + ("vpc2", vec![expose("2001:db9::/32")]), + )], + ); + let (mut flow_filter, _writer) = make_flow_filter(any_proto); + let out = run(&mut flow_filter, packet(Some(vpcd(100)), with_hop_by_hop())); + assert!(!out.is_done(), "{:?}", out.get_done()); + assert_eq!(out.meta().dst_vpcd, Some(vpcd(200))); +} + +/// Portless packets use port `0`, which cannot match a configured port range. +/// This prevents ICMP and ICMPv6 from reaching port-forwarded destinations. +#[test] +fn portless_packet_cannot_match_a_port_restricted_expose() { + // vpc2 publishes 80.0.0.5 only as a port-forwarding destination on public port 2222. + let (mut flow_filter, _) = make_flow_filter(dst_port_forwarding_context()); + let out = run( + &mut flow_filter, + packet( + Some(vpcd(100)), + build_icmp_packet(v4("10.0.0.5"), v4("80.0.0.5")), + ), + ); + assert_eq!( + out.get_done(), + Some(DoneReason::Filtered), + "a portless packet cannot match the port-restricted expose that publishes this address", + ); + assert_eq!(out.meta().dst_vpcd, None); + + // An unrestricted expose confirms that only the port constraint prevents routing. + let unrestricted = context( + &[("vpc1", 100), ("vpc2", 200)], + vec![peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose("10.0.0.0/24")]), + ("vpc2", vec![expose("80.0.0.0/24")]), + )], + ); + let (mut flow_filter, _writer) = make_flow_filter(unrestricted); + let out = run( + &mut flow_filter, + packet( + Some(vpcd(100)), + build_icmp_packet(v4("10.0.0.5"), v4("80.0.0.5")), + ), + ); + assert!(!out.is_done(), "{:?}", out.get_done()); + assert_eq!(out.meta().dst_vpcd, Some(vpcd(200))); +} + +/// A flow newer than the worker's config generation bypasses the tables during config propagation. +#[test] +fn flow_from_a_newer_generation_is_honored_for_bypass() { + let (mut flow_filter, _writer) = make_flow_filter(source_nat_context()); + + // Use an uncovered destination so bypass is observable. + let mut p = packet( + Some(vpcd(100)), + build_tcp_packet(v4("1.0.0.5"), v4("9.9.9.9"), 1234, 5678), + ); + let flow = attach_flow(&mut p, Some(vpcd(200)), true, false, false); + flow.set_genid(9); + + let out = run(&mut flow_filter, p); + assert!( + !out.is_done(), + "a newer-generation flow must bypass the tables: {:?}", + out.get_done(), + ); + assert_eq!(out.meta().dst_vpcd, Some(vpcd(200))); + assert_eq!( + flow.status(), + FlowStatus::Active, + "the bypass path must not invalidate the flow it just honoured", + ); +} From 8869fe36c9a69d3dc82b0452d85e5e2813428954 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Fri, 31 Jul 2026 16:06:52 -0600 Subject: [PATCH 5/7] test(acl-filter): compare lowering with config semantics Generate valid ACL overlays and compare reference-table lookups with an independent oracle over the validated config. Cover ordering, direction, prefix cross-products, protocols, metadata, IP versions, and defaults. Compare the same cases with rte_acl to cover backend encoding and priority. Coverage counters prevent vacuous short runs. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- Cargo.lock | 1 + acl-filter/Cargo.toml | 1 + acl-filter/src/fuzz.rs | 376 +++++++++++++++++++++++++ acl-filter/src/fuzz_gen.rs | 562 +++++++++++++++++++++++++++++++++++++ acl-filter/src/lib.rs | 4 + 5 files changed, 944 insertions(+) create mode 100644 acl-filter/src/fuzz.rs create mode 100644 acl-filter/src/fuzz_gen.rs diff --git a/Cargo.lock b/Cargo.lock index 630c9f6dd8..59f514d47e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1243,6 +1243,7 @@ dependencies = [ name = "dataplane-acl-filter" version = "0.24.0" dependencies = [ + "bolero", "dataplane-acl", "dataplane-common", "dataplane-concurrency", diff --git a/acl-filter/Cargo.toml b/acl-filter/Cargo.toml index 2dfc98aa11..232547ea03 100644 --- a/acl-filter/Cargo.toml +++ b/acl-filter/Cargo.toml @@ -26,6 +26,7 @@ tracing = { workspace = true } # EAL-free semantic suite. It is `cfg(test)`-gated in the source, so it is never part of a # production build; this dev-dep just makes `acl::reference` available to test builds. acl = { workspace = true, features = ["reference"] } +bolero = { workspace = true, features = ["std"] } dpdk = { workspace = true, features = ["test"] } flow-entry = { workspace = true } flow-filter = { workspace = true } diff --git a/acl-filter/src/fuzz.rs b/acl-filter/src/fuzz.rs new file mode 100644 index 0000000000..c52ffd97a2 --- /dev/null +++ b/acl-filter/src/fuzz.rs @@ -0,0 +1,376 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Property tests for ACL lowering and lookup. +//! +//! The oracle evaluates the validated config directly, without using lowered rules or a +//! classifier. This keeps lowering mistakes independent from the expected result. + +#![cfg(test)] + +use crate::PacketSummary; +use crate::context::{AclTables, Backend, LookupResult}; +use crate::fuzz_gen::{OverlaySpec, ProbeSpec, vni}; +use concurrency::sync::LazyLock; +use concurrency::sync::atomic::{AtomicU64, Ordering}; +use config::external::overlay::ValidatedOverlay; +use config::external::overlay::acl::{AclAction, AclProtoMatch, AclScope, ValidatedAclRule}; +use config::external::overlay::vpc::ValidatedPeering; +use lpm::prefix::{IpPrefix, Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; +use net::ip::NextHeader; +use net::vxlan::Vni; +use std::net::IpAddr; + +// ------------------------------------------------------------------------------------------------- +// The config-semantics oracle. + +/// The outcome of the first matching config rule. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct OracleVerdict { + action: AclAction, + log: bool, + scope: AclScope, +} + +impl From<&LookupResult> for OracleVerdict { + fn from(result: &LookupResult) -> Self { + Self { + action: result.action, + log: result.log, + scope: result.scope, + } + } +} + +fn proto_allows(rule: AclProtoMatch, packet: NextHeader) -> bool { + match rule { + AclProtoMatch::Any => true, + AclProtoMatch::Tcp => packet == NextHeader::TCP, + AclProtoMatch::Udp => packet == NextHeader::UDP, + AclProtoMatch::Other(p) => packet == NextHeader::new(p), + } +} + +fn entry_allows(entry: &PrefixWithOptionalPorts, ip: IpAddr, port: u16) -> bool { + let covers = match (entry.prefix(), ip) { + (Prefix::IPV4(p), IpAddr::V4(a)) => p.covers_addr(&a), + (Prefix::IPV6(p), IpAddr::V6(a)) => p.covers_addr(&a), + _ => false, + }; + covers + && entry + .ports() + .is_none_or(|r| r.start() <= port && port <= r.end()) +} + +fn side_allows(set: &PrefixPortsSet, ip: IpAddr, port: u16) -> bool { + set.iter().any(|entry| entry_allows(entry, ip, port)) +} + +fn rule_matches(rule: &ValidatedAclRule, packet: &PacketSummary) -> bool { + // Match `AclTables::lookup`: cross-version packets consult neither table. + if packet.src_ip.is_ipv4() != packet.dst_ip.is_ipv4() { + return false; + } + let pattern = rule.pattern(); + let (sport, dport) = packet.ports.unwrap_or((0, 0)); + proto_allows(pattern.proto(), packet.proto) + && side_allows(pattern.src(), packet.src_ip, sport) + && side_allows(pattern.dst(), packet.dst_ip, dport) +} + +/// Find a peering in the packet's direction. +fn directed_peering( + overlay: &ValidatedOverlay, + src_vni: Vni, + dst_vni: Vni, +) -> Option<&ValidatedPeering> { + overlay + .vpc_table() + .values() + .find(|vpc| vpc.vni() == src_vni)? + .peerings() + .iter() + .find(|peering| peering.remote_vni() == dst_vni) +} + +/// Return the first matching rule from the directed peering. +/// Exact VNI fields prevent rules from other peerings or directions from competing. +fn oracle_lookup(overlay: &ValidatedOverlay, packet: &PacketSummary) -> Option { + let peering = directed_peering(overlay, packet.src_vni, packet.dst_vni)?; + let acl = peering.acl().as_ref()?; + acl.rules() + .iter() + .filter(|rule| rule.from() == peering.local().name()) + .find(|rule| rule_matches(rule, packet)) + .map(|rule| OracleVerdict { + action: rule.action(), + log: rule.log(), + scope: rule.scope(), + }) +} + +fn oracle_default_action( + overlay: &ValidatedOverlay, + src_vni: Vni, + dst_vni: Vni, +) -> Option { + Some( + directed_peering(overlay, src_vni, dst_vni)? + .acl() + .as_ref()? + .default_action(), + ) +} + +fn resolved_action(rule: Option, default: Option) -> AclAction { + rule.map_or_else(|| default.unwrap_or(AclAction::Allow), |v| v.action) +} + +// ------------------------------------------------------------------------------------------------- +// Properties. + +/// Ensure short property-test runs exercise each outcome. +struct Coverage { + rule_allows: LazyLock, + rule_denies: LazyLock, + default_falls: LazyLock, + unconfigured: LazyLock, +} + +impl Coverage { + const fn new() -> Self { + Self { + rule_allows: LazyLock::new(|| AtomicU64::new(0)), + rule_denies: LazyLock::new(|| AtomicU64::new(0)), + default_falls: LazyLock::new(|| AtomicU64::new(0)), + unconfigured: LazyLock::new(|| AtomicU64::new(0)), + } + } + + fn record(&self, verdict: Option, has_default: bool) { + let counter = match (verdict, has_default) { + (Some(v), _) if v.action == AclAction::Allow => &self.rule_allows, + (Some(_), _) => &self.rule_denies, + (None, true) => &self.default_falls, + (None, false) => &self.unconfigured, + }; + counter.fetch_add(1, Ordering::Relaxed); + } + + fn assert_reached(&self, label: &str) { + let (allows, denies) = ( + self.rule_allows.load(Ordering::Relaxed), + self.rule_denies.load(Ordering::Relaxed), + ); + let (defaults, unconfigured) = ( + self.default_falls.load(Ordering::Relaxed), + self.unconfigured.load(Ordering::Relaxed), + ); + eprintln!( + "{label} coverage: {allows} rule allows, {denies} rule denies, \ + {defaults} default fallbacks, {unconfigured} unconfigured pairs" + ); + assert!(allows >= 1, "{label}: no rule ever allowed a packet"); + assert!(denies >= 1, "{label}: no rule ever denied a packet"); + assert!(defaults >= 1, "{label}: never fell through to a default"); + assert!( + unconfigured >= 1, + "{label}: never probed a pair with no ACL at all" + ); + } +} + +/// Lowered reference tables must match the validated config. +#[test] +fn reference_lookup_matches_config_oracle() { + static COVERAGE: Coverage = Coverage::new(); + + bolero::check!() + .with_type::<(OverlaySpec, [ProbeSpec; 8])>() + .for_each(|(overlay_spec, probe_specs)| { + let built = overlay_spec.build(); + let tables = AclTables::build(&built.overlay, Backend::Reference) + .expect("reference backend build is infallible"); + + for probe_spec in probe_specs { + let probe = probe_spec.resolve(&built); + let want = oracle_lookup(&built.overlay, &probe); + let got = tables.lookup(&probe).map(OracleVerdict::from); + assert_eq!( + got, want, + "tables disagree with the config oracle on {probe:?}\nspec: {overlay_spec:?}", + ); + + // Defaults are lowered separately from rules, so compare them separately. + let want_default = + oracle_default_action(&built.overlay, probe.src_vni, probe.dst_vni); + let got_default = tables.find_default_action(probe.src_vni, probe.dst_vni); + assert_eq!( + got_default, want_default, + "default action disagrees for {} -> {}\nspec: {overlay_spec:?}", + probe.src_vni, probe.dst_vni, + ); + + assert_eq!( + resolved_action(got, got_default), + resolved_action(want, want_default), + "resolved action disagrees on {probe:?}\nspec: {overlay_spec:?}", + ); + + COVERAGE.record(want, want_default.is_some()); + } + }); + + COVERAGE.assert_reached("reference vs oracle"); +} + +/// Earlier matching rules must take precedence. +/// The DPDK differential test covers the priority encoding; the reference backend is first-match. +#[test] +fn earlier_rules_win_over_later_matching_rules() { + bolero::check!() + .with_type::<(OverlaySpec, [ProbeSpec; 8])>() + .for_each(|(overlay_spec, probe_specs)| { + let built = overlay_spec.build(); + let tables = AclTables::build(&built.overlay, Backend::Reference) + .expect("reference backend build is infallible"); + + for probe_spec in probe_specs { + let probe = probe_spec.resolve(&built); + let Some(peering) = directed_peering(&built.overlay, probe.src_vni, probe.dst_vni) + else { + continue; + }; + let Some(acl) = peering.acl().as_ref() else { + continue; + }; + + let matching: Vec<&ValidatedAclRule> = acl + .rules() + .iter() + .filter(|rule| rule.from() == peering.local().name()) + .filter(|rule| rule_matches(rule, &probe)) + .collect(); + + let got = tables.lookup(&probe).map(OracleVerdict::from); + match matching.first() { + Some(first) => assert_eq!( + got, + Some(OracleVerdict { + action: first.action(), + log: first.log(), + scope: first.scope(), + }), + "a later rule beat the first of {} matching rules on {probe:?}\n\ + spec: {overlay_spec:?}", + matching.len(), + ), + None => assert_eq!( + got, None, + "table matched a rule the config says cannot match {probe:?}\n\ + spec: {overlay_spec:?}", + ), + } + } + }); +} + +/// Generated cases must behave identically under rte_acl and the reference backend. +#[test] +#[dpdk::with_eal] +fn dpdk_backend_matches_reference_on_generated_overlays() { + static COVERAGE: Coverage = Coverage::new(); + + bolero::check!() + .with_type::<(OverlaySpec, [ProbeSpec; 8])>() + .for_each(|(overlay_spec, probe_specs)| { + let built = overlay_spec.build(); + let reference = AclTables::build(&built.overlay, Backend::Reference) + .expect("reference backend build is infallible"); + let dpdk = + AclTables::build(&built.overlay, Backend::Dpdk).expect("rte_acl backend build"); + + for probe_spec in probe_specs { + let probe = probe_spec.resolve(&built); + let want = reference.lookup(&probe).map(OracleVerdict::from); + assert_eq!( + dpdk.lookup(&probe).map(OracleVerdict::from), + want, + "backends disagree on {probe:?}\nspec: {overlay_spec:?}", + ); + + let want_default = reference.find_default_action(probe.src_vni, probe.dst_vni); + assert_eq!( + dpdk.find_default_action(probe.src_vni, probe.dst_vni), + want_default, + "backends disagree on the default action for {} -> {}\nspec: {overlay_spec:?}", + probe.src_vni, + probe.dst_vni, + ); + + COVERAGE.record(want, want_default.is_some()); + } + }); + + COVERAGE.assert_reached("dpdk vs reference"); +} + +/// A default exists exactly when the directed peering has an ACL. +#[test] +fn absent_acl_and_absent_peering_have_no_default_action() { + bolero::check!() + .with_type::<(OverlaySpec, [ProbeSpec; 4])>() + .for_each(|(overlay_spec, probe_specs)| { + let built = overlay_spec.build(); + let tables = AclTables::build(&built.overlay, Backend::Reference) + .expect("reference backend build is infallible"); + + for probe_spec in probe_specs { + let probe = probe_spec.resolve(&built); + let peering = directed_peering(&built.overlay, probe.src_vni, probe.dst_vni); + let configured = peering.is_some_and(|p| p.acl().is_some()); + assert_eq!( + tables + .find_default_action(probe.src_vni, probe.dst_vni) + .is_some(), + configured, + "a default action exists exactly when the pair is peered and has an ACL: \ + {} -> {}\nspec: {overlay_spec:?}", + probe.src_vni, + probe.dst_vni, + ); + if !configured { + assert_eq!( + tables.lookup(&probe).map(OracleVerdict::from), + None, + "an unconfigured pair must match no rule: {probe:?}\n\ + spec: {overlay_spec:?}", + ); + } + } + }); +} + +/// Keep the bogus probe VNI distinct from generated VPCs. +#[test] +fn generated_overlays_use_the_declared_vnis() { + use crate::fuzz_gen::VNIS; + + bolero::check!() + .with_type::() + .for_each(|overlay_spec| { + let built = overlay_spec.build(); + let vnis: Vec = built + .overlay + .vpc_table() + .values() + .map(|vpc| vpc.vni()) + .collect(); + assert_eq!( + vnis, + VNIS.iter().copied().map(vni).collect::>(), + "generated VPC table drifted from the declared VNIs", + ); + }); +} diff --git a/acl-filter/src/fuzz_gen.rs b/acl-filter/src/fuzz_gen.rs new file mode 100644 index 0000000000..56e9db5d20 --- /dev/null +++ b/acl-filter/src/fuzz_gen.rs @@ -0,0 +1,562 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Generators for ACL property tests. +//! +//! [`OverlaySpec::build`] normalizes a compact spec and validates the resulting config. Probes are +//! biased toward generated peerings so short runs exercise rule matches as well as misses. +//! +//! This remains separate from `flow-filter`'s generator: flow-filter needs disjoint prefix blocks, +//! while ACL ordering requires overlapping patterns. Invalid cross-version rules are out of scope +//! because config validation rejects them before lowering. + +#![cfg(test)] + +use crate::PacketSummary; +use bolero::TypeGenerator; +use config::external::overlay::acl::{ + Acl, AclAction, AclPattern, AclProtoMatch, AclRule, AclScope, +}; +use config::external::overlay::vpc::{Vpc, VpcTable}; +use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeering, VpcPeeringTable}; +use config::external::overlay::{Overlay, ValidatedOverlay}; +use lpm::prefix::{PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts}; +use net::ip::NextHeader; +use net::vxlan::Vni; +use std::net::IpAddr; + +pub(crate) const VNIS: [u32; 3] = [100, 200, 300]; +/// A VNI outside [`VNIS`]. +const BOGUS_VNI: u32 = 999; + +/// The unique VPC pair assigned to each peering slot. +const PEERING_PAIRS: [(usize, usize); 3] = [(0, 1), (0, 2), (1, 2)]; + +pub(crate) fn vni(id: u32) -> Vni { + Vni::new_checked(id).unwrap_or_else(|e| unreachable!("{id} is a valid VNI: {e:?}")) +} + +// ------------------------------------------------------------------------------------------------- +// Prefix pool. +// +// Each peering side owns a distinct block; rule patterns overlap within it. + +fn private_block(n: u8, v6: bool) -> String { + if v6 { + format!("2001:db8:0:{n:x}::/120") + } else { + format!("10.{n}.0.0/24") + } +} + +fn public_block(n: u8, v6: bool) -> String { + if v6 { + format!("2001:db9:0:{n:x}::/120") + } else { + format!("20.{n}.0.0/24") + } +} + +/// An address inside block `n`. +pub(crate) fn block_addr(n: u8, host: u8, public: bool, v6: bool) -> IpAddr { + if v6 { + let net = if public { "db9" } else { "db8" }; + format!("2001:{net}:0:{n:x}::{host:x}") + .parse() + .unwrap_or_else(|e| unreachable!("generated v6 address must parse: {e}")) + } else { + let net = if public { 20 } else { 10 }; + format!("{net}.{n}.0.{host}") + .parse() + .unwrap_or_else(|e| unreachable!("generated v4 address must parse: {e}")) + } +} + +// ------------------------------------------------------------------------------------------------- +// Rule pattern selectors. + +/// An overlapping region within a generated block. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum PrefixSel { + Block, + LowerHalf, + UpperHalf, + Host(u8), +} + +impl PrefixSel { + fn resolve(self, n: u8, public: bool, v6: bool) -> Prefix { + let block = if public { + public_block(n, v6) + } else { + private_block(n, v6) + }; + let text = match (self, v6) { + (PrefixSel::Block, _) => block, + (PrefixSel::LowerHalf, false) => block.replace("/24", "/25"), + (PrefixSel::UpperHalf, false) => block.replace(".0.0/24", ".0.128/25"), + (PrefixSel::LowerHalf, true) => block.replace("/120", "/121"), + (PrefixSel::UpperHalf, true) => block.replace("::/120", "::80/121"), + (PrefixSel::Host(h), false) => block.replace(".0.0/24", &format!(".0.{h}/32")), + (PrefixSel::Host(h), true) => block.replace("::/120", &format!("::{h:x}/128")), + }; + Prefix::from(text.as_str()) + } +} + +/// One side of a rule pattern. Two entries make lowering's cross-product observable. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum SideSel { + /// Validation expands this to the manifest's coverage set. + All, + One(PrefixSel), + Two(PrefixSel, PrefixSel), +} + +impl SideSel { + fn resolve(self, n: u8, public: bool, v6: bool) -> Vec { + match self { + SideSel::All => Vec::new(), + SideSel::One(a) => vec![a.resolve(n, public, v6)], + SideSel::Two(a, b) => { + vec![a.resolve(n, public, v6), b.resolve(n, public, v6)] + } + } + } +} + +/// A generated TCP or UDP port constraint. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum PortSel { + None, + Low, + High, + /// A port nested inside `Low`. + Single(u8), +} + +impl PortSel { + fn resolve(self) -> Option { + let range = |lo: u16, hi: u16| { + PortRange::new(lo, hi).unwrap_or_else(|e| unreachable!("valid port range: {e:?}")) + }; + match self { + PortSel::None => None, + PortSel::Low => Some(range(1, 1023)), + PortSel::High => Some(range(1024, u16::MAX)), + PortSel::Single(k) => { + let port = 500 + u16::from(k); + Some(range(port, port)) + } + } + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum ProtoSel { + Any, + Tcp, + Udp, + /// A protocol number, including aliases for TCP and UDP. + Other(u8), +} + +impl ProtoSel { + fn to_config(self) -> AclProtoMatch { + match self { + ProtoSel::Any => AclProtoMatch::Any, + ProtoSel::Tcp => AclProtoMatch::Tcp, + ProtoSel::Udp => AclProtoMatch::Udp, + ProtoSel::Other(p) => AclProtoMatch::Other(p), + } + } + + fn allows_ports(self) -> bool { + matches!(self, ProtoSel::Tcp | ProtoSel::Udp) + } +} + +// ------------------------------------------------------------------------------------------------- +// Specs. + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct RuleSpec { + /// Apply the rule from remote to local. + reverse: bool, + allow: bool, + /// Request flow scope; normalization adds the required NAT. + flow_scope: bool, + log: bool, + proto: ProtoSel, + src: SideSel, + dst: SideSel, + src_ports: PortSel, + dst_ports: PortSel, +} + +/// Ensures a generated ACL is non-empty. +const FALLBACK_RULE: RuleSpec = RuleSpec { + reverse: false, + allow: true, + flow_scope: false, + log: false, + proto: ProtoSel::Any, + src: SideSel::All, + dst: SideSel::All, + src_ports: PortSel::None, + dst_ports: PortSel::None, +}; + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct AclSpec { + default_allow: bool, + /// Up to four rules in precedence order. + rules: [Option; 4], +} + +impl AclSpec { + fn rule_specs(&self) -> impl Iterator + '_ { + self.rules.iter().flatten().copied() + } +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct PeeringSpec { + /// IP version for both manifests. + v6: bool, + /// Masquerade changes a side's public prefix. + local_masq: bool, + remote_masq: bool, + /// `None` leaves the peering without an ACL. + acl: Option, +} + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct OverlaySpec { + peerings: [Option; 3], +} + +/// One direction of a generated peering, used to bias probes toward matches. +#[derive(Debug, Clone, Copy)] +pub(crate) struct Anchor { + src_vni: u32, + dst_vni: u32, + /// The source side's private block. + src_block: u8, + /// The destination side's advertised block. + dst_block: u8, + dst_public: bool, + v6: bool, +} + +/// A validated overlay and its probe metadata. +pub(crate) struct BuiltOverlay { + pub(crate) overlay: ValidatedOverlay, + pub(crate) blocks: u8, + pub(crate) anchors: Vec, +} + +impl OverlaySpec { + /// Normalize and validate the generated config. + pub(crate) fn build(&self) -> BuiltOverlay { + let mut spec = *self; + + // Keep at least one peering to probe. + if spec.peerings.iter().all(Option::is_none) { + spec.peerings[0] = Some(PeeringSpec { + v6: false, + local_masq: false, + remote_masq: false, + acl: Some(AclSpec { + default_allow: false, + rules: [Some(FALLBACK_RULE), None, None, None], + }), + }); + } + for peering in spec.peerings.iter_mut().flatten() { + if let Some(acl) = peering.acl.as_mut() + && acl.rules.iter().all(Option::is_none) + { + acl.rules[0] = Some(FALLBACK_RULE); + } + // Flow scope requires stateful NAT on one side. Stateful NAT cannot be on both sides. + let wants_flow_scope = peering + .acl + .iter() + .flat_map(AclSpec::rule_specs) + .any(|rule| rule.flow_scope); + if wants_flow_scope { + peering.local_masq = true; + } + if peering.local_masq { + peering.remote_masq = false; + } + } + + // Materialize. + let mut vpc_table = VpcTable::new(); + for (i, id) in VNIS.iter().enumerate() { + vpc_table + .add( + Vpc::new(&vpc_name(i), &format!("VPC{:02}", i + 1), *id) + .unwrap_or_else(|e| unreachable!("valid VPC: {e}")), + ) + .unwrap_or_else(|e| unreachable!("distinct VPCs: {e}")); + } + let mut peering_table = VpcPeeringTable::new(); + let mut blocks: u8 = 0; + let mut anchors = Vec::new(); + for (slot, peering) in spec.peerings.iter().enumerate() { + let Some(peering) = peering else { continue }; + let (a, b) = PEERING_PAIRS[slot]; + let local_block = blocks; + let remote_block = blocks + 1; + blocks += 2; + + // Anchor both rule directions. + anchors.push(Anchor { + src_vni: VNIS[a], + dst_vni: VNIS[b], + src_block: local_block, + dst_block: remote_block, + dst_public: peering.remote_masq, + v6: peering.v6, + }); + anchors.push(Anchor { + src_vni: VNIS[b], + dst_vni: VNIS[a], + src_block: remote_block, + dst_block: local_block, + dst_public: peering.local_masq, + v6: peering.v6, + }); + + let local = manifest(&vpc_name(a), local_block, peering.local_masq, peering.v6); + let remote = manifest(&vpc_name(b), remote_block, peering.remote_masq, peering.v6); + let mut built = VpcPeering::with_default_group( + &format!("{}-to-{}", vpc_name(a), vpc_name(b)), + local, + remote, + ); + built.acl = peering.acl.map(|acl| { + build_acl( + &acl, + (&vpc_name(a), local_block, peering.local_masq), + (&vpc_name(b), remote_block, peering.remote_masq), + peering.v6, + ) + }); + peering_table + .add(built) + .unwrap_or_else(|e| unreachable!("distinct peerings: {e}")); + } + + let overlay = Overlay::new(vpc_table, peering_table) + .validate() + .unwrap_or_else(|e| { + panic!( + "generated overlay must validate (generator/config drift): {e}\nspec: {spec:?}" + ) + }); + BuiltOverlay { + overlay, + blocks, + anchors, + } + } +} + +fn vpc_name(index: usize) -> String { + format!("vpc{}", index + 1) +} + +/// Build one plain or masqueraded expose over block `n`. +fn manifest(name: &str, n: u8, masquerade: bool, v6: bool) -> VpcManifest { + let expose = if masquerade { + VpcExpose::empty() + .make_masquerade(None) + .unwrap_or_else(|e| unreachable!("masquerade on an empty expose: {e}")) + .ip(private_block(n, v6).as_str().into()) + .as_range(public_block(n, v6).as_str().into()) + .unwrap_or_else(|e| unreachable!("equal-size public range: {e}")) + } else { + VpcExpose::empty().ip(private_block(n, v6).as_str().into()) + }; + VpcManifest::with_exposes(name, vec![expose]) +} + +fn build_acl(spec: &AclSpec, local: (&str, u8, bool), remote: (&str, u8, bool), v6: bool) -> Acl { + let (local_name, local_block, local_masq) = local; + let (remote_name, remote_block, remote_masq) = remote; + + let rules = spec + .rule_specs() + .enumerate() + .map(|(i, rule)| { + // Rules match the source's private space and destination's public space. + let ((from, src_block), (to, dst_block, dst_masq)) = if rule.reverse { + ( + (remote_name, remote_block), + (local_name, local_block, local_masq), + ) + } else { + ( + (local_name, local_block), + (remote_name, remote_block, remote_masq), + ) + }; + + let (src_ports, dst_ports) = if rule.proto.allows_ports() { + (rule.src_ports.resolve(), rule.dst_ports.resolve()) + } else { + (None, None) + }; + + AclRule { + name: format!("rule{i}"), + from: from.to_owned(), + to: to.to_owned(), + action: if rule.allow { + AclAction::Allow + } else { + AclAction::Deny + }, + pattern: AclPattern { + src: prefix_set(rule.src.resolve(src_block, false, v6), src_ports), + dst: prefix_set(rule.dst.resolve(dst_block, dst_masq, v6), dst_ports), + src_any_ports: Vec::new(), + dst_any_ports: Vec::new(), + proto: rule.proto.to_config(), + }, + scope: if rule.flow_scope { + AclScope::Flow + } else { + AclScope::Packet + }, + log: rule.log, + } + }) + .collect(); + + Acl::new( + if spec.default_allow { + AclAction::Allow + } else { + AclAction::Deny + }, + rules, + ) +} + +fn prefix_set(prefixes: Vec, ports: Option) -> PrefixPortsSet { + prefixes + .into_iter() + .map(|prefix| PrefixWithOptionalPorts::new(prefix, ports)) + .collect() +} + +// ------------------------------------------------------------------------------------------------- +// Probes. + +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum ProbeProto { + Tcp, + Udp, + Icmp, + Other(u8), +} + +impl ProbeProto { + fn next_header(self) -> NextHeader { + match self { + ProbeProto::Tcp => NextHeader::TCP, + ProbeProto::Udp => NextHeader::UDP, + ProbeProto::Icmp => NextHeader::ICMP, + ProbeProto::Other(p) => NextHeader::new(p), + } + } + + fn has_ports(self) -> bool { + matches!(self, ProbeProto::Tcp | ProbeProto::Udp) + } +} + +/// A port biased toward generated range boundaries. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum ProbePort { + Exact(u16), + WellKnown(u8), + Nested(u8), +} + +impl ProbePort { + fn resolve(self) -> u16 { + match self { + ProbePort::Exact(p) => p, + ProbePort::WellKnown(k) => 1 + u16::from(k) * 4, + ProbePort::Nested(k) => 499 + u16::from(k), + } + } +} + +/// One deliberate departure from a peering anchor. +/// `Option` keeps half of probes on the match path. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) enum Stray { + SrcVni, + DstVni, + /// Use the reverse VNI pair without reversing addresses. + SwapVnis, + SrcBlock(u8), + DstBlock(u8), + SrcPublic, + CrossVersion, +} + +/// A generated packet relative to a peering anchor. +#[derive(Debug, Clone, Copy, TypeGenerator)] +pub(crate) struct ProbeSpec { + anchor_sel: u8, + stray: Option, + src_host: u8, + dst_host: u8, + proto: ProbeProto, + sport: ProbePort, + dport: ProbePort, +} + +impl ProbeSpec { + pub(crate) fn resolve(&self, built: &BuiltOverlay) -> PacketSummary { + let anchor = built.anchors[self.anchor_sel as usize % built.anchors.len()]; + let nblocks = built.blocks.max(1); + + let mut src_vni = anchor.src_vni; + let mut dst_vni = anchor.dst_vni; + let mut src_block = anchor.src_block; + let mut dst_block = anchor.dst_block; + let mut src_public = false; + let mut dst_v6 = anchor.v6; + match self.stray { + None => {} + Some(Stray::SrcVni) => src_vni = BOGUS_VNI, + Some(Stray::DstVni) => dst_vni = BOGUS_VNI, + Some(Stray::SwapVnis) => std::mem::swap(&mut src_vni, &mut dst_vni), + Some(Stray::SrcBlock(b)) => src_block = b % nblocks, + Some(Stray::DstBlock(b)) => dst_block = b % nblocks, + Some(Stray::SrcPublic) => src_public = true, + Some(Stray::CrossVersion) => dst_v6 = !anchor.v6, + } + + PacketSummary { + src_vni: vni(src_vni), + dst_vni: vni(dst_vni), + src_ip: block_addr(src_block, self.src_host, src_public, anchor.v6), + dst_ip: block_addr(dst_block, self.dst_host, anchor.dst_public, dst_v6), + proto: self.proto.next_header(), + ports: self + .proto + .has_ports() + .then(|| (self.sport.resolve(), self.dport.resolve())), + } + } +} diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index aaafa896ff..3e17f71453 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -21,6 +21,10 @@ mod access; mod context; mod display; +#[cfg(test)] +mod fuzz; +#[cfg(test)] +mod fuzz_gen; #[cfg(test)] mod tests; From b9811a2321f2075c0c3bdcb39db80d3ad7061e4d Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 3 Aug 2026 16:33:58 -0600 Subject: [PATCH 6/7] fix(config): reject mixed-IP manifests Expose validation checks one expose at a time, so a manifest could still combine IPv4 and IPv6. Filters choose one table version per peering and could omit rules for the other version. Require one IP version across a manifest's non-default exposes. Add tests for both expose orders, default exposes, and valid single-version manifests. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- .../src/external/overlay/validation_tests.rs | 49 +++++++++++++++++++ config/src/external/overlay/vpcpeering.rs | 26 ++++++++++ flow-filter/src/fuzz_gen.rs | 6 +-- 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/config/src/external/overlay/validation_tests.rs b/config/src/external/overlay/validation_tests.rs index f33ba4fc7a..571811ebc8 100644 --- a/config/src/external/overlay/validation_tests.rs +++ b/config/src/external/overlay/validation_tests.rs @@ -741,6 +741,55 @@ mod test { // VpcManifest validation, overlap and NAT checks // ================================================================================== + // Reject mixed versions across exposes, not only within one expose. + #[test] + fn test_manifest_mixing_ip_versions_rejected() { + let mut manifest = VpcManifest::new("VPC-1"); + manifest.add_expose(VpcExpose::empty().ip("10.0.0.0/24".into())); + manifest.add_expose(VpcExpose::empty().ip("2001:db8::/32".into())); + let result = manifest.validate(); + assert!( + matches!(result, Err(ConfigError::Forbidden(_))), + "a manifest mixing IPv4 and IPv6 exposes must be rejected: {result:?}", + ); + } + + // The result must not depend on expose order. + #[test] + fn test_manifest_mixing_ip_versions_rejected_either_order() { + let mut manifest = VpcManifest::new("VPC-1"); + manifest.add_expose(VpcExpose::empty().ip("2001:db8::/32".into())); + manifest.add_expose(VpcExpose::empty().ip("10.0.0.0/24".into())); + assert!(matches!( + manifest.validate(), + Err(ConfigError::Forbidden(_)) + )); + } + + // Default exposes do not constrain the manifest's IP version. + #[test] + fn test_manifest_default_expose_does_not_constrain_ip_version() { + for ip in ["10.0.0.0/24", "2001:db8::/32"] { + let mut manifest = VpcManifest::new("VPC-1"); + manifest.add_expose(VpcExpose::empty().set_default()); + manifest.add_expose(VpcExpose::empty().ip(ip.into())); + let result = manifest.validate(); + assert!( + result.is_ok(), + "a default expose alongside {ip} must be accepted: {result:?}", + ); + } + } + + // IPv6-only manifests remain valid. + #[test] + fn test_manifest_single_ip_version_accepted() { + let mut v6 = VpcManifest::new("VPC-1"); + v6.add_expose(VpcExpose::empty().ip("2001:db8::/32".into())); + v6.add_expose(VpcExpose::empty().ip("2001:db9::/32".into())); + assert!(v6.validate().is_ok()); + } + // Two no-NAT exposes with disjoint ips passes #[test] fn test_no_nat_disjoint_ips_passes() { diff --git a/config/src/external/overlay/vpcpeering.rs b/config/src/external/overlay/vpcpeering.rs index fce928055e..d5f641a26c 100644 --- a/config/src/external/overlay/vpcpeering.rs +++ b/config/src/external/overlay/vpcpeering.rs @@ -692,6 +692,7 @@ impl VpcManifest { valid_manifest_candidate.valexp.push(expose.validate()?); } + valid_manifest_candidate.validate_single_ip_version()?; valid_manifest_candidate.validate_expose_collisions()?; Ok(valid_manifest_candidate) } @@ -776,6 +777,31 @@ impl ValidatedManifest { self.valexp.len() == 1 && self.valexp.first().is_some_and(ValidatedExpose::is_default) } + /// Reject manifests containing both IPv4 and IPv6 exposes. + /// Default exposes are version-neutral. + fn validate_single_ip_version(&self) -> ConfigResult { + let mut version: Option = None; + for expose in &self.valexp { + // A default expose has no IP version. + let is_v4 = if expose.is_v4() { + true + } else if expose.is_v6() { + false + } else { + continue; + }; + match version { + Some(seen) if seen != is_v4 => { + return Err(ConfigError::Forbidden( + "A manifest cannot mix IPv4 and IPv6 expose blocks", + )); + } + _ => version = Some(is_v4), + } + } + Ok(()) + } + fn validate_expose_collisions(&self) -> ConfigResult { // Check that prefixes in each expose don't overlap with prefixes in other exposes for (index, expose_left) in self.valexp.iter().enumerate() { diff --git a/flow-filter/src/fuzz_gen.rs b/flow-filter/src/fuzz_gen.rs index b0138ed0fc..43b4bffdd5 100644 --- a/flow-filter/src/fuzz_gen.rs +++ b/flow-filter/src/fuzz_gen.rs @@ -18,10 +18,8 @@ //! tie that the port-forwarding tie-break bit resolves), //! - two port-forwarding exposes sharing prefixes and ports, distinguished only by L4 protocol. //! -//! Deliberately out of scope: cross-peering masquerade/masquerade destination overlaps (legal, -//! but the winning destination VPC of the resulting equal-priority marker rules is unspecified, -//! and benign only because the NF gates masquerade verdicts on the flow's destination), and -//! mixed-IP-version manifests (they currently pass validation but yield one-sided tables). +//! Cross-peering masquerade destination overlaps are out of scope because their equal-priority +//! winner is unspecified. Mixed-IP manifests are rejected by config validation. #![cfg(test)] From 1ab134538841cf397554242563b60b50c71116df Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Tue, 4 Aug 2026 00:04:34 -0600 Subject: [PATCH 7/7] fix(acl-filter): reject rules with the wrong IP version lower_rules previously dropped rules that did not match the selected table's IP version. Return FailureApply instead so an invariant violation rejects reconfiguration rather than silently omitting a rule. Validated manifests prevent this case; the check is defense in depth. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- acl-filter/src/access.rs | 11 +++++--- acl-filter/src/context.rs | 58 +++++++++++++++++++++++++++------------ acl-filter/src/fuzz.rs | 8 +++--- 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/acl-filter/src/access.rs b/acl-filter/src/access.rs index 72554bf7d9..e861241656 100644 --- a/acl-filter/src/access.rs +++ b/acl-filter/src/access.rs @@ -25,12 +25,15 @@ impl TryFrom<&ValidatedOverlay> for AclFilterContext { #[cfg(test)] impl AclFilterContext { - /// Build a context using the reference backend, for tests that want the fast, EAL-free oracle. - /// Production goes through [`TryFrom`], which uses the rte_acl backend. + /// Build an EAL-free reference context for tests. + /// + /// # Panics + /// + /// Panics if the validated overlay cannot be lowered. pub(crate) fn for_test(overlay: &ValidatedOverlay) -> Self { use crate::context::Backend; - let acls = AclTables::build(overlay, Backend::Reference) - .expect("reference backend build is infallible"); + let acls = + AclTables::build(overlay, Backend::Reference).expect("validated overlay must lower"); Self { acls } } diff --git a/acl-filter/src/context.rs b/acl-filter/src/context.rs index ac3c2768fd..20f1bab76e 100644 --- a/acl-filter/src/context.rs +++ b/acl-filter/src/context.rs @@ -212,9 +212,8 @@ impl AclKey { } } -/// Lower a single rule to backend-neutral field predicates for the concrete IP version of its -/// prefixes. Returns `None` if the source and destination prefixes disagree on IP version, which -/// the config validation already rules out for a well-formed peering. +/// Lower one rule to backend-neutral predicates. +/// Returns `None` if either prefix does not match `T`. fn rule_predicates( proto: AclProtoMatch, src_vni: Vni, @@ -270,14 +269,20 @@ impl IpVersion for Ipv6Addr { } } -/// Lower all rules for one IP version into `(predicates, action)` pairs, preserving order (which is -/// the precedence). A missing prefix or port range becomes the wildcard for that field. -fn lower_rules( - rules: &[PeeringAclRule], -) -> Vec<(AclKeyRule, Vec, LookupResult)> { +/// A typed rule, its backend predicates, and its action. +type LoweredRule = (AclKeyRule, Vec, LookupResult); + +/// Lower rules for `T` in first-match order. +/// Missing prefixes and port ranges become wildcards. +/// +/// # Errors +/// +/// Returns an error if a prefix has the wrong IP version. +/// Validation should make this unreachable; returning an error avoids silently dropping the rule. +fn lower_rules(rules: &[PeeringAclRule]) -> Result>, ConfigError> { rules .iter() - .filter_map(|rule| { + .map(|rule| { let (key_rule, fields) = rule_predicates( rule.proto, rule.src_vni, @@ -288,8 +293,21 @@ fn lower_rules( .unwrap_or(lpm::prefix::with_ports::PORT_RANGE_WILDCARD), rule.dst_port_range .unwrap_or(lpm::prefix::with_ports::PORT_RANGE_WILDCARD), - )?; - Some(( + ) + .ok_or_else(|| { + let (src, dst) = (rule.src_ip_range, rule.dst_ip_range); + error!( + "ACL rule for {} -> {} does not match the IP version of the table it was \ + filed under (src {src:?}, dst {dst:?}); refusing the configuration", + rule.src_vni, rule.dst_vni, + ); + ConfigError::FailureApply(format!( + "ACL rule for VNI {} -> {} has prefixes (src {src:?}, dst {dst:?}) that do \ + not match the table's IP version", + rule.src_vni, rule.dst_vni, + )) + })?; + Ok(( key_rule, fields, LookupResult { @@ -494,12 +512,18 @@ impl Default for AclTables { impl AclTables { pub(super) fn build(overlay: &ValidatedOverlay, backend: Backend) -> Result { let ruleset = PeeringAclRuleSet::from(overlay); - let v4 = - build_table::, _>(backend, "v4", lower_rules::(&ruleset.v4)) - .map_err(ConfigError::FailureApply)?; - let v6 = - build_table::, _>(backend, "v6", lower_rules::(&ruleset.v6)) - .map_err(ConfigError::FailureApply)?; + let v4 = build_table::, _>( + backend, + "v4", + lower_rules::(&ruleset.v4)?, + ) + .map_err(ConfigError::FailureApply)?; + let v6 = build_table::, _>( + backend, + "v6", + lower_rules::(&ruleset.v6)?, + ) + .map_err(ConfigError::FailureApply)?; Ok(Self { v4, v6, diff --git a/acl-filter/src/fuzz.rs b/acl-filter/src/fuzz.rs index c52ffd97a2..156fbe82c9 100644 --- a/acl-filter/src/fuzz.rs +++ b/acl-filter/src/fuzz.rs @@ -191,7 +191,7 @@ fn reference_lookup_matches_config_oracle() { .for_each(|(overlay_spec, probe_specs)| { let built = overlay_spec.build(); let tables = AclTables::build(&built.overlay, Backend::Reference) - .expect("reference backend build is infallible"); + .expect("validated overlay must lower"); for probe_spec in probe_specs { let probe = probe_spec.resolve(&built); @@ -234,7 +234,7 @@ fn earlier_rules_win_over_later_matching_rules() { .for_each(|(overlay_spec, probe_specs)| { let built = overlay_spec.build(); let tables = AclTables::build(&built.overlay, Backend::Reference) - .expect("reference backend build is infallible"); + .expect("validated overlay must lower"); for probe_spec in probe_specs { let probe = probe_spec.resolve(&built); @@ -287,7 +287,7 @@ fn dpdk_backend_matches_reference_on_generated_overlays() { .for_each(|(overlay_spec, probe_specs)| { let built = overlay_spec.build(); let reference = AclTables::build(&built.overlay, Backend::Reference) - .expect("reference backend build is infallible"); + .expect("validated overlay must lower"); let dpdk = AclTables::build(&built.overlay, Backend::Dpdk).expect("rte_acl backend build"); @@ -324,7 +324,7 @@ fn absent_acl_and_absent_peering_have_no_default_action() { .for_each(|(overlay_spec, probe_specs)| { let built = overlay_spec.build(); let tables = AclTables::build(&built.overlay, Backend::Reference) - .expect("reference backend build is infallible"); + .expect("validated overlay must lower"); for probe_spec in probe_specs { let probe = probe_spec.resolve(&built);