diff --git a/config/src/external/overlay/vpc.rs b/config/src/external/overlay/vpc.rs index a8665e8c0a..daac16ec88 100644 --- a/config/src/external/overlay/vpc.rs +++ b/config/src/external/overlay/vpc.rs @@ -151,8 +151,9 @@ 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. + // If stateful NAT is set up on one side of the peering, we don't support stateful NAT on + // the other side. Static NAT (and no NAT) opposite stateful is fine -- see the table + // below, which is what this actually enforces. let mut local_has_masquerading = false; let mut local_has_port_forwarding = false; for expose in self.local.valexp() { diff --git a/flow-filter/Cargo.toml b/flow-filter/Cargo.toml index 00c9063fec..7ce1bb62d8 100644 --- a/flow-filter/Cargo.toml +++ b/flow-filter/Cargo.toml @@ -32,4 +32,6 @@ 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"] } +# "bolero" exposes net's fuzz header-stack generator (net::headers::builder::ChainBase), which +# drives the adversarial-header suite against the NF's parser-facing edge. +net = { workspace = true, features = ["builder", "bolero"] } diff --git a/flow-filter/src/context/fuzz.rs b/flow-filter/src/context/fuzz.rs index 8d790713ae..d022990430 100644 --- a/flow-filter/src/context/fuzz.rs +++ b/flow-filter/src/context/fuzz.rs @@ -17,6 +17,7 @@ use crate::fuzz_gen::{OverlaySpec, Probe, ProbeSpec, bogus_vpcd}; use concurrency::sync::LazyLock; use concurrency::sync::atomic::{AtomicU64, Ordering}; use config::external::overlay::ValidatedOverlay; +use config::external::overlay::vpcpeering::ValidatedExpose; use lpm::prefix::{IpPrefix, L4Protocol, Prefix, PrefixWithOptionalPorts}; use net::ip::NextHeader; use net::packet::VpcDiscriminant; @@ -54,20 +55,51 @@ fn prefix_allows(prefix: &PrefixWithOptionalPorts, ip: IpAddr, port: u16) -> boo /// equal-length ties. Mirrors `rule_priority` without sharing its encoding. type Precedence = (u8, bool); -/// Keep the strictly-better candidate; equal precedence between candidates that can match the -/// same packet is a generator invariant violation, so fail loudly rather than pick one. -fn consider(best: &mut Option<(Precedence, T)>, precedence: Precedence, value: T) { +/// Keep the strictly-better candidate. +/// +/// Two candidates at equal precedence are only acceptable when they agree on the answer. Config +/// permits exactly one such tie: two peers masquerading behind the same public range, which +/// `VpcRoute::can_overlap` exempts. Both then say "masquerade destination", and since a masquerade +/// destination is verified against the flow rather than resolved from the address (see +/// [`StageOne`]), they say the *same* thing and the tie is harmless. +/// +/// A tie whose candidates disagree is a real ambiguity -- the tables would have to pick one, and +/// nothing says which -- so fail loudly rather than let a test pass on a coin flip. +fn consider( + best: &mut Option<(Precedence, T)>, + precedence: Precedence, + value: T, +) { match best { - Some((current, _)) if *current == precedence => { - panic!("ambiguous match at precedence {precedence:?}: generator invariant violated") + Some((current, existing)) if *current == precedence => { + assert!( + *existing == value, + "ambiguous match at precedence {precedence:?}: {existing:?} vs {value:?}", + ); } Some((current, _)) if *current > precedence => {} _ => *best = Some((precedence, value)), } } +/// What a stage-1 match says about a destination. +/// +/// [`StageOne::Masquerade`] deliberately carries no VPC. A masquerade public address does not +/// identify one -- config lets two peers share a range -- so the destination is verified against +/// the flow's candidate instead. Dropping the VPC here is what makes two overlapping masquerade +/// exposes compare equal, and so what makes the overlap a benign tie rather than an ambiguity. +#[derive(Debug, PartialEq, Eq)] +enum StageOne { + Masquerade, + Resolved(VpcDiscriminant, Option), +} + /// Answer a route lookup directly from the validated overlay. -fn oracle_lookup(overlay: &ValidatedOverlay, probe: &Probe) -> LookupResult { +/// +/// Exposed to the crate because the NF-level property test +/// (`tests::nf_metadata_matches_config_oracle`) predicts a packet's stamped metadata from this same +/// answer: the config's meaning of a route should be stated in exactly one place. +pub(crate) fn oracle_lookup(overlay: &ValidatedOverlay, probe: &Probe) -> LookupResult { let Some(src_vpc) = overlay .vpc_table() .values() @@ -82,7 +114,7 @@ fn oracle_lookup(overlay: &ValidatedOverlay, probe: &Probe) -> LookupResult { // Stage 1: the destination against every peer's public prefixes. Masquerade exposes are // included (marker rules); a default expose acts as a /0 of the peering's IP version. - let mut verdict: Option<(Precedence, (VpcDiscriminant, Option))> = None; + let mut verdict: Option<(Precedence, StageOne)> = None; for peering in src_vpc.peerings() { let dst_vpcd = VpcDiscriminant::from_vni(peering.remote_vni()); for expose in peering.remote().valexp() { @@ -91,22 +123,48 @@ fn oracle_lookup(overlay: &ValidatedOverlay, probe: &Probe) -> LookupResult { } for prefix in expose.public_ips() { if prefix_allows(prefix, probe.dst_ip, dport) { + let matched = if expose.has_masquerade() { + StageOne::Masquerade + } else { + StageOne::Resolved(dst_vpcd, NatRequirement::from_expose(expose)) + }; consider( &mut verdict, (prefix.prefix().length(), expose.has_port_forwarding()), - (dst_vpcd, NatRequirement::from_expose(expose)), + matched, ); } } } if peering.remote().has_default_expose() && probe.dst_ip.is_ipv4() == peering.is_v4() { - consider(&mut verdict, (0, false), (dst_vpcd, None)); + consider(&mut verdict, (0, false), StageOne::Resolved(dst_vpcd, None)); } } - let Some((_, (dst_vpcd, dst_nat))) = verdict else { + let Some((_, matched)) = verdict else { return LookupResult::DestinationMiss; }; + // A masquerade destination is verified against the flow's candidate, not resolved from the + // address. Two peers may masquerade behind one range -- config permits it and models the + // destination as a *set* of routes -- so "which VPC owns this address" has no unique answer. + // "Does this VPC masquerade this address for this source" does, and it is asked directly of + // the config here: find the peering to the flow's claimed VPC and check whether one of its + // masquerade exposes covers the destination. + let StageOne::Resolved(dst_vpcd, dst_nat) = matched else { + let verified = probe.flow_dst_vpcd.filter(|candidate| { + src_vpc + .peerings() + .iter() + .filter(|p| VpcDiscriminant::from_vni(p.remote_vni()) == *candidate) + .flat_map(|p| p.remote().valexp()) + .filter(|expose| expose.has_masquerade()) + .filter(|expose| proto_allows(expose.nat_proto(), probe.proto)) + .flat_map(ValidatedExpose::public_ips) + .any(|prefix| prefix_allows(prefix, probe.dst_ip, dport)) + }); + return LookupResult::MasqueradeDestination(verified); + }; + // Stage 2: the source against that peering's private prefixes. Port-forwarding sources are // excluded (they cannot initiate); a default expose acts as a /0 of the peering's version. let peering = src_vpc @@ -161,6 +219,7 @@ fn dpdk_backend_matches_reference_on_generated_overlays() { static ROUTES: LazyLock = LazyLock::new(|| AtomicU64::new(0)); static SOURCE_MISSES: LazyLock = LazyLock::new(|| AtomicU64::new(0)); static DESTINATION_MISSES: LazyLock = LazyLock::new(|| AtomicU64::new(0)); + static MASQUERADE_DESTINATIONS: LazyLock = LazyLock::new(|| AtomicU64::new(0)); bolero::check!() .with_type::<(OverlaySpec, [ProbeSpec; 40])>() @@ -175,21 +234,9 @@ fn dpdk_backend_matches_reference_on_generated_overlays() { // may not get enough of them during a time-boxed bolero run on a loaded CI runner to // get meaningful coverage. for probe in &built.routing_probes { - let want = reference.lookup( - probe.src_vpcd, - probe.src_ip, - probe.dst_ip, - probe.proto, - probe.ports, - ); + let want = reference.lookup(&probe.input()); assert_eq!( - dpdk.lookup( - probe.src_vpcd, - probe.src_ip, - probe.dst_ip, - probe.proto, - probe.ports - ), + dpdk.lookup(&probe.input()), want, "backends disagree on derived routing probe {probe:?}\nspec: {overlay_spec:?}", ); @@ -200,43 +247,46 @@ fn dpdk_backend_matches_reference_on_generated_overlays() { ROUTES.fetch_add(1, Ordering::Relaxed); } + // Derived masquerade probes: each names the peer that really masquerades its + // destination, so stage 3 must confirm it. This is the hit path -- a verification table + // that matched nothing would still pass every "must drop" assertion in this suite. + for probe in &built.masquerade_probes { + let want = reference.lookup(&probe.input()); + assert_eq!( + dpdk.lookup(&probe.input()), + want, + "backends disagree on derived masquerade probe {probe:?}\nspec: {overlay_spec:?}", + ); + assert_eq!( + want, + LookupResult::MasqueradeDestination(probe.flow_dst_vpcd), + "a masquerade destination did not verify against its own peer: {probe:?}\nspec: {overlay_spec:?}", + ); + MASQUERADE_DESTINATIONS.fetch_add(1, Ordering::Relaxed); + } + let probes: Vec = probe_specs .iter() - .map(|p| p.resolve(built.blocks)) + .map(|p| p.resolve(&built)) .collect(); let inputs: Vec = probes .iter() - .map(|p| LookupInput { - src_vpcd: p.src_vpcd, - src_ip: p.src_ip, - dst_ip: p.dst_ip, - proto: p.proto, - ports: p.ports, - }) + .map(|p| p.input()) .collect(); let mut expected = Vec::with_capacity(probes.len()); for probe in &probes { - let want = reference.lookup( - probe.src_vpcd, - probe.src_ip, - probe.dst_ip, - probe.proto, - probe.ports, - ); + let want = reference.lookup(&probe.input()); assert_eq!( - dpdk.lookup( - probe.src_vpcd, - probe.src_ip, - probe.dst_ip, - probe.proto, - probe.ports - ), + dpdk.lookup(&probe.input()), want, "backends disagree on single lookup of {probe:?}\nspec: {overlay_spec:?}", ); match want { LookupResult::Route(_) => ROUTES.fetch_add(1, Ordering::Relaxed), + LookupResult::MasqueradeDestination(_) => { + MASQUERADE_DESTINATIONS.fetch_add(1, Ordering::Relaxed) + } LookupResult::SourceMiss(_) => SOURCE_MISSES.fetch_add(1, Ordering::Relaxed), LookupResult::DestinationMiss => { DESTINATION_MISSES.fetch_add(1, Ordering::Relaxed) @@ -259,6 +309,7 @@ fn dpdk_backend_matches_reference_on_generated_overlays() { // 33 inputs of one version make the first chunk a full MAX_BATCH. let all_miss: Vec = (0..33u8) .map(|i| LookupInput { + flow_dst_vpcd: None, src_vpcd: bogus_vpcd(), src_ip: format!("10.0.0.{i}").parse().unwrap(), dst_ip: "10.0.0.99".parse().unwrap(), @@ -275,11 +326,16 @@ fn dpdk_backend_matches_reference_on_generated_overlays() { }); eprintln!( - "coverage: {} routes, {} source misses, {} destination misses", + "coverage: {} routes, {} masquerade destinations, {} source misses, {} destination misses", ROUTES.load(Ordering::Relaxed), + MASQUERADE_DESTINATIONS.load(Ordering::Relaxed), SOURCE_MISSES.load(Ordering::Relaxed), DESTINATION_MISSES.load(Ordering::Relaxed), ); + assert!( + MASQUERADE_DESTINATIONS.load(Ordering::Relaxed) >= 1, + "no masquerade destination was ever reached, so stage 3 went untested", + ); assert!(ROUTES.load(Ordering::Relaxed) >= 1, "no full routes at all"); assert!( SOURCE_MISSES.load(Ordering::Relaxed) >= 4, @@ -305,33 +361,15 @@ fn batched_lookup_matches_single_lookup() { let tables = FlowFilterContext::build(&built.overlay, Backend::Reference) .expect("reference build"); - let probes: Vec = probe_specs - .iter() - .map(|p| p.resolve(built.blocks)) - .collect(); - let inputs: Vec = probes - .iter() - .map(|p| LookupInput { - src_vpcd: p.src_vpcd, - src_ip: p.src_ip, - dst_ip: p.dst_ip, - proto: p.proto, - ports: p.ports, - }) - .collect(); + let probes: Vec = probe_specs.iter().map(|p| p.resolve(&built)).collect(); + let inputs: Vec = probes.iter().map(|p| p.input()).collect(); let mut out = vec![LookupResult::DestinationMiss; inputs.len()]; tables.lookup_batch(&inputs, &mut out); for (i, probe) in probes.iter().enumerate() { assert_eq!( out[i], - tables.lookup( - probe.src_vpcd, - probe.src_ip, - probe.dst_ip, - probe.proto, - probe.ports - ), + tables.lookup(&probe.input()), "batch slot {i} != single lookup for {probe:?}\nspec: {overlay_spec:?}", ); } @@ -349,18 +387,119 @@ fn reference_lookup_matches_config_oracle() { let tables = FlowFilterContext::build(&built.overlay, Backend::Reference).expect("reference build"); for probe_spec in probe_specs { - let probe = probe_spec.resolve(built.blocks); + let probe = probe_spec.resolve(&built); assert_eq!( - tables.lookup( - probe.src_vpcd, - probe.src_ip, - probe.dst_ip, - probe.proto, - probe.ports - ), + tables.lookup(&probe.input()), oracle_lookup(&built.overlay, &probe), "reference tables disagree with the config oracle on {probe:?}\nspec: {overlay_spec:?}", ); } }); } + +/// The exclusion prefixes the generator writes survive into the built configuration as a fan of +/// disjoint prefixes with many distinct lengths. +/// +/// Asserted rather than assumed, for the same reason the coverage counters elsewhere are: an +/// exclusion that validation collapsed back into the original block, or that the generator never +/// actually emitted, would leave every test above it passing on the single-prefix exposes it had +/// before -- exactly the failure mode where a generator looks like it covers a shape and does not. +/// +/// Punching one host out of a `/24` yields prefixes of every length from `/25` to `/32`, so a run +/// that ever picks [`ExcludeSel::UpperHost`] must reach a spread of 8. That spread is also what +/// stresses `rule_priority` hardest: it orders purely by prefix length, and its correctness rests +/// on config never letting two rules that can match the same packet share one. The config oracle's +/// `consider` panics on exactly that ambiguity, so the suites above are the ones proving it holds +/// -- this test only proves they are being handed the hard case. +#[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", + ); +} + +/// Generated overlays really do contain cross-peering masquerade overlaps. +/// +/// The shape this whole verification stage exists for: one VPC with two peers masquerading behind +/// the same public range, so a destination address maps to two VPCs at once. Config permits it and +/// models it as a set of routes per destination; stage 1 cannot represent that, which is why the +/// destination is verified against the flow rather than resolved. +/// +/// Asserted because the generator used to exclude this deliberately. Without it every property +/// above passes on overlay shapes where a destination happens to be unambiguous, and the ambiguity +/// -- the only thing stage 3 exists to handle -- is never built. +#[test] +fn cross_peering_masquerade_overlaps_are_generated() { + // Lazily initialized so this compiles under the loom backend, whose AtomicU64::new is not const. + static OVERLAPS: 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() { + // Every masquerade destination this VPC can address, with the peer advertising it. + let mut destinations: Vec<(Prefix, VpcDiscriminant)> = Vec::new(); + for peering in vpc.peerings() { + let dst_vpcd = VpcDiscriminant::from_vni(peering.remote_vni()); + for expose in peering.remote().valexp() { + if !expose.has_masquerade() { + continue; + } + for prefix in expose.public_ips() { + destinations.push((prefix.prefix(), dst_vpcd)); + } + } + } + // An overlap is the same destination claimed by two different peers -- exactly the + // case where the address alone cannot name the destination VPC. + let overlapping = destinations.iter().enumerate().any(|(i, (prefix, vpcd))| { + destinations[i + 1..] + .iter() + .any(|(other, other_vpcd)| other == prefix && other_vpcd != vpcd) + }); + if overlapping { + OVERLAPS.fetch_add(1, Ordering::Relaxed); + } + } + }); + + let overlaps = OVERLAPS.load(Ordering::Relaxed); + eprintln!("coverage: {overlaps} VPCs facing an ambiguous masquerade destination"); + assert!( + overlaps >= 1, + "no cross-peering masquerade overlap was ever generated; \ + the verification stage is untested by the property suite", + ); +} diff --git a/flow-filter/src/context/mod.rs b/flow-filter/src/context/mod.rs index 88cc7c8300..a2bd241d81 100644 --- a/flow-filter/src/context/mod.rs +++ b/flow-filter/src/context/mod.rs @@ -9,8 +9,10 @@ use config::ConfigError; use config::external::overlay::ValidatedOverlay; mod display; +// Not private: the NF-level property test in `crate::tests` predicts a packet's stamped metadata +// from this module's config oracle, so that the config's meaning of a route is stated once. #[cfg(test)] -mod fuzz; +pub(crate) mod fuzz; mod tables; #[cfg(test)] mod tests; diff --git a/flow-filter/src/context/tables.rs b/flow-filter/src/context/tables.rs index d8e98fabf5..208edd7327 100644 --- a/flow-filter/src/context/tables.rs +++ b/flow-filter/src/context/tables.rs @@ -66,6 +66,19 @@ type Route = (VpcDiscriminant, NatMode, NatMode); pub(crate) enum LookupResult { /// Both stages matched. Route(Route), + /// The destination is a masquerade public address. + /// + /// Carries a destination VPC only when the packet's flow named one and stage 3 confirmed that + /// this VPC masquerades this destination for this source. `None` means nothing vouched for the + /// packet -- no flow, or a flow whose VPC the configuration does not agree owns this address. + /// + /// This is separate from [`LookupResult::Route`] because a masquerade public address does not + /// identify a VPC on its own: config permits two peers to masquerade behind one range and + /// models the result as a set of routes per destination, which stage 1 cannot represent. The + /// destination here is therefore *verified*, never *resolved* -- see [`MasqueradeKey`]. A + /// masquerade destination also cannot accept a new connection, so there is no source NAT to + /// report: the flow carries the state such a packet needs. + MasqueradeDestination(Option), /// Stage 1 resolved the destination VPC, but the source matched nothing. SourceMiss(VpcDiscriminant), /// Stage 1 matched nothing: no peering covers this destination (also used for IP-version @@ -82,6 +95,12 @@ pub(crate) struct LookupInput { pub(crate) dst_ip: IpAddr, pub(crate) proto: NextHeader, pub(crate) ports: Option<(u16, u16)>, + /// The destination VPC recorded on the packet's flow, if it has one. + /// + /// An input to the lookup, not an output: it is the candidate stage 3 verifies when stage 1 + /// reports a masquerade destination. Carried here rather than consulted in the NF so the + /// verification stays on the batched path with the other two stages. + pub(crate) flow_dst_vpcd: Option, } /// Result of a stage-1 (remote/destination) match. @@ -100,6 +119,8 @@ struct Query { dst_ip: I, src_port: u16, dst_port: u16, + /// The candidate destination VPC from the packet's flow (see [`LookupInput::flow_dst_vpcd`]). + flow_dst_vni: Option, } /// Lower a config L4 protocol to a bitmask predicate: a specific protocol matches exactly (every @@ -162,6 +183,32 @@ pub(super) struct LocalKey { src_port: u16, } +/// Stage-3 key: "does `dst_vni` masquerade this destination for `src_vni`?" +/// +/// The question stage 1 cannot answer. A masquerade public address does not identify a VPC: config +/// lets two peers masquerade behind the same range (`VpcRoute::can_overlap` exempts +/// masquerade/masquerade), and models the result as a *set* of routes per destination +/// (`VpcRouteSet` is a `Vec`). Stage 1's `Verdict` holds one `dst_vpcd`, so lowering that set into +/// stage 1 collapses it and rte_acl's highest-priority-wins picks arbitrarily. +/// +/// So this key carries `dst_vni` as an *input* rather than producing it. "Which VPC owns this +/// address?" has no unique answer; "does this VPC own this address for me?" does. The candidate +/// comes from the packet's flow -- the only thing that can distinguish two connections to the same +/// masquerade address -- and this table says whether the configuration agrees. +#[derive(Debug, MatchKey, Clone, PartialEq, Eq)] +pub(super) struct MasqueradeKey { + #[mask] + proto: NextHeader, + #[exact] + src_vni: Vni, + #[exact] + dst_vni: Vni, + #[prefix] + dst_ip: I, + #[range] + dst_port: u16, +} + // ------------------------------------------------------------------------------------------------- // Backend selection. // @@ -287,12 +334,37 @@ impl fmt::Debug for AnyTable { // is a thin wrapper over an otherwise-const atomic. static TABLE_SEQ: LazyLock = LazyLock::new(|| AtomicU64::new(0)); -/// A process-unique rte_acl context name (rte_acl rejects duplicate names). +/// The longest name rte_acl accepts (`RTE_ACL_NAMESIZE` less its NUL). +const MAX_TABLE_NAME: usize = 31; +/// The longest `base` [`table_name`] may be given, so that the name fits for *every* counter value. +const MAX_TABLE_BASE: usize = 9; + +/// A process-unique rte_acl context name (rte_acl rejects duplicate names, and names over +/// [`MAX_TABLE_NAME`] bytes). +/// +/// The name has to be both unique and short, and the bound must hold for every value the counter +/// can take -- not merely the values a given run is expected to reach. A short prefix, a `base` of +/// at most [`MAX_TABLE_BASE`] bytes, and a hexadecimal counter (16 bytes at most for a `u64`) give +/// `3 + 9 + 1 + 16 = 29`, which fits with room to spare. +/// +/// This is not hypothetical. With the previous `flow_filter_` prefix, a 13-byte base and a decimal +/// counter, names passed 31 bytes once the counter reached 100_000 -- which a long fuzz run +/// reached, and which a long-lived process rebuilding its tables on every configuration change +/// would reach eventually. The failure mode was that every subsequent table build failed, so +/// reconfiguration stopped working. fn table_name(base: &str) -> String { - format!( - "flow_filter_{base}_{}", - TABLE_SEQ.fetch_add(1, Ordering::Relaxed) - ) + debug_assert!( + base.len() <= MAX_TABLE_BASE, + "table base {base:?} is {} bytes, over the {MAX_TABLE_BASE}-byte budget", + base.len(), + ); + let name = format!("ff_{base}_{:x}", TABLE_SEQ.fetch_add(1, Ordering::Relaxed)); + debug_assert!( + name.len() <= MAX_TABLE_NAME, + "table name {name:?} is {} bytes, over rte_acl's {MAX_TABLE_NAME}-byte limit", + name.len(), + ); + name } /// Build one table from backend-neutral rules using the selected backend. @@ -483,12 +555,62 @@ fn emit_local( } } +/// Lower a stage-3 (masquerade-verification) rule into the v4 or v6 bucket according to its prefix. +fn emit_masquerade( + v4: &mut Vec, ()>>, + v6: &mut Vec, ()>>, + src_vni: Vni, + dst_vni: Vni, + ip_range: Prefix, + port_range: RangeSpec, + proto: MaskSpec, +) { + // Every rule here answers the same yes/no question, so nothing competes: the key pins both + // VNIs exactly, and within one peering a destination is covered by at most one masquerade + // expose. The priority is pure prefix length, as elsewhere. + let priority = rule_priority(ip_range, false); + match ip_range { + Prefix::IPV4(prefix) => { + let rule = MasqueradeKeyRule:: { + proto, + src_vni: ExactSpec::new(src_vni), + dst_vni: ExactSpec::new(dst_vni), + dst_ip: PrefixSpec::from(prefix), + dst_port: port_range, + }; + v4.push(NeutralRule { + priority, + fields: rule.into_backend_fields::(), + rule, + action: (), + }); + } + Prefix::IPV6(prefix) => { + let rule = MasqueradeKeyRule:: { + proto, + src_vni: ExactSpec::new(src_vni), + dst_vni: ExactSpec::new(dst_vni), + dst_ip: PrefixSpec::from(prefix), + dst_port: port_range, + }; + v6.push(NeutralRule { + priority, + fields: rule.into_backend_fields::(), + rule, + action: (), + }); + } + } +} + #[derive(Default)] struct RuleSet { remote_v4: Vec, Verdict>>, remote_v6: Vec, Verdict>>, local_v4: Vec, NatMode>>, local_v6: Vec, NatMode>>, + masquerade_v4: Vec, ()>>, + masquerade_v6: Vec, ()>>, } impl RuleSet { @@ -528,6 +650,20 @@ impl RuleSet { proto, action, ); + // Stage 3: the same prefix, keyed additionally on the peer's VNI, so a + // masquerade destination can be *verified* against a candidate VPC rather + // than resolved from the address (which two peers may share). + if action.nat_mode == Some(NatRequirement::Masquerade) { + emit_masquerade( + &mut rules.masquerade_v4, + &mut rules.masquerade_v6, + src_vni, + remote_vni, + prefix.prefix(), + prefix.into(), + proto, + ); + } } } if peering.remote().has_default_expose() { @@ -594,6 +730,8 @@ pub struct FlowFilterContext { pub(super) local_v4: AnyTable, NatMode>, pub(super) remote_v6: AnyTable, Verdict>, pub(super) local_v6: AnyTable, NatMode>, + pub(super) masquerade_v4: AnyTable, ()>, + pub(super) masquerade_v6: AnyTable, ()>, } impl Default for FlowFilterContext { @@ -603,6 +741,8 @@ impl Default for FlowFilterContext { local_v4: AnyTable::empty(), remote_v6: AnyTable::empty(), local_v6: AnyTable::empty(), + masquerade_v4: AnyTable::empty(), + masquerade_v6: AnyTable::empty(), } } } @@ -614,6 +754,8 @@ impl fmt::Debug for FlowFilterContext { .field("local_v4", &self.local_v4) .field("remote_v6", &self.remote_v6) .field("local_v6", &self.local_v6) + .field("masquerade_v4", &self.masquerade_v4) + .field("masquerade_v6", &self.masquerade_v6) .finish() } } @@ -631,73 +773,54 @@ impl FlowFilterContext { local_v4: build_table(backend, "local_v4", rules.local_v4)?, remote_v6: build_table(backend, "remote_v6", rules.remote_v6)?, local_v6: build_table(backend, "local_v6", rules.local_v6)?, + masquerade_v4: build_table(backend, "masq_v4", rules.masquerade_v4)?, + masquerade_v6: build_table(backend, "masq_v6", rules.masquerade_v6)?, }) } - // Single-key lookup: the readable per-packet oracle used by tests; production runs - // lookup_batch. The differential test cross-checks the two against each other. + /// Single-key lookup: the readable per-packet oracle used by tests; production runs + /// [`lookup_batch`](Self::lookup_batch). The differential test cross-checks the two against + /// each other, which is why both take the same [`LookupInput`] and share [`lookup_one`]'s + /// stage logic rather than restating it. #[cfg(test)] - pub(super) fn lookup( - &self, - src_vpcd: VpcDiscriminant, - src_ip: IpAddr, - dst_ip: IpAddr, - proto: NextHeader, - ports: Option<(u16, u16)>, - ) -> LookupResult { - let src_vni = key_vni(src_vpcd); - let (src_port, dst_port) = ports.unzip(); - let src_port = src_port.unwrap_or(0); - let dst_port = dst_port.unwrap_or(0); - - match (src_ip, dst_ip) { - (IpAddr::V4(src_ip), IpAddr::V4(dst_ip)) => { - let Some(verdict) = self.remote_v4.lookup(&RemoteKey { - proto, - src_vni, - dst_ip, - dst_port, - }) else { - return LookupResult::DestinationMiss; - }; - match self.local_v4.lookup(&LocalKey { - proto, + pub(super) fn lookup(&self, input: &LookupInput) -> LookupResult { + let src_vni = key_vni(input.src_vpcd); + let (src_port, dst_port) = input.ports.unwrap_or((0, 0)); + let flow_dst_vni = input.flow_dst_vpcd.map(key_vni); + + match (input.src_ip, input.dst_ip) { + (IpAddr::V4(src_ip), IpAddr::V4(dst_ip)) => lookup_one( + &self.remote_v4, + &self.local_v4, + &self.masquerade_v4, + &Query { src_vni, - dst_vni: key_vni(verdict.dst_vpcd), + proto: input.proto, src_ip, - src_port, - }) { - Some(nat_mode) => { - LookupResult::Route((verdict.dst_vpcd, verdict.nat_mode, *nat_mode)) - } - None => LookupResult::SourceMiss(verdict.dst_vpcd), - } - } - (IpAddr::V6(src_ip), IpAddr::V6(dst_ip)) => { - let Some(verdict) = self.remote_v6.lookup(&RemoteKey { - proto, - src_vni, dst_ip, + src_port, dst_port, - }) else { - return LookupResult::DestinationMiss; - }; - match self.local_v6.lookup(&LocalKey { - proto, + flow_dst_vni, + }, + ), + (IpAddr::V6(src_ip), IpAddr::V6(dst_ip)) => lookup_one( + &self.remote_v6, + &self.local_v6, + &self.masquerade_v6, + &Query { src_vni, - dst_vni: key_vni(verdict.dst_vpcd), + proto: input.proto, src_ip, + dst_ip, src_port, - }) { - Some(nat_mode) => { - LookupResult::Route((verdict.dst_vpcd, verdict.nat_mode, *nat_mode)) - } - None => LookupResult::SourceMiss(verdict.dst_vpcd), - } - } + dst_port, + flow_dst_vni, + }, + ), _ => { debug!( - "Source and destination IP versions do not match: src_ip={src_ip:?}, dst_ip={dst_ip:?}", + "Source and destination IP versions do not match: src_ip={:?}, dst_ip={:?}", + input.src_ip, input.dst_ip, ); LookupResult::DestinationMiss } @@ -722,6 +845,7 @@ impl FlowFilterContext { let proto = input.proto; let src_vni = key_vni(input.src_vpcd); let (src_port, dst_port) = input.ports.unwrap_or((0, 0)); + let flow_dst_vni = input.flow_dst_vpcd.map(key_vni); match (input.src_ip, input.dst_ip) { (IpAddr::V4(src_ip), IpAddr::V4(dst_ip)) => { v4_idx.push(i); @@ -732,6 +856,7 @@ impl FlowFilterContext { dst_ip, src_port, dst_port, + flow_dst_vni, }); } (IpAddr::V6(src_ip), IpAddr::V6(dst_ip)) => { @@ -743,29 +868,114 @@ impl FlowFilterContext { dst_ip, src_port, dst_port, + flow_dst_vni, }); } _ => { /* version mismatch: leave "out[i] = DestinationMiss" */ } } } - lookup_versioned(&self.remote_v4, &self.local_v4, &v4_q, &v4_idx, out); - lookup_versioned(&self.remote_v6, &self.local_v6, &v6_q, &v6_idx, out); + lookup_versioned( + &self.remote_v4, + &self.local_v4, + &self.masquerade_v4, + &v4_q, + &v4_idx, + out, + ); + lookup_versioned( + &self.remote_v6, + &self.local_v6, + &self.masquerade_v6, + &v6_q, + &v6_idx, + out, + ); } } -/// The two-pass batched lookup for one IP version. `queries[k]` corresponds to output slot -/// `out[idx[k]]`. Runs in `MAX_BATCH`-sized rte_acl calls: stage 1 (destination -> [`Verdict`]), -/// then stage 2 (source -> source NAT) over the stage-1 hits only. +/// One query's stages, for a single IP version: the unbatched twin of [`lookup_versioned`]. +/// +/// Kept as its own function so the stage *logic* -- in particular which of stage 2 and stage 3 a +/// stage-1 hit needs -- is written once. The batched path differs only in gathering keys across a +/// burst before each rte_acl call; if the two ever disagree about the stages themselves, the +/// differential property test is comparing two different questions. +#[cfg(test)] +fn lookup_one( + remote: &AnyTable, Verdict>, + local: &AnyTable, NatMode>, + masquerade: &AnyTable, ()>, + q: &Query, +) -> LookupResult +where + RemoteKey: MatchKey, + LocalKey: MatchKey, + MasqueradeKey: MatchKey, +{ + let Some(verdict) = remote.lookup(&RemoteKey { + proto: q.proto, + src_vni: q.src_vni, + dst_ip: q.dst_ip, + dst_port: q.dst_port, + }) else { + return LookupResult::DestinationMiss; + }; + + // A masquerade destination is verified against the flow's candidate, never resolved from the + // address: see `MasqueradeKey`. No candidate, or one the configuration does not agree with, + // means nothing vouches for the packet. + if verdict.nat_mode == Some(NatRequirement::Masquerade) { + let verified = q.flow_dst_vni.filter(|dst_vni| { + masquerade + .lookup(&MasqueradeKey { + proto: q.proto, + src_vni: q.src_vni, + dst_vni: *dst_vni, + dst_ip: q.dst_ip, + dst_port: q.dst_port, + }) + .is_some() + }); + return LookupResult::MasqueradeDestination(verified.map(VpcDiscriminant::from_vni)); + } + + match local.lookup(&LocalKey { + proto: q.proto, + src_vni: q.src_vni, + dst_vni: key_vni(verdict.dst_vpcd), + src_ip: q.src_ip, + src_port: q.src_port, + }) { + Some(nat_mode) => LookupResult::Route((verdict.dst_vpcd, verdict.nat_mode, *nat_mode)), + None => LookupResult::SourceMiss(verdict.dst_vpcd), + } +} + +/// The batched lookup for one IP version. `queries[k]` corresponds to output slot `out[idx[k]]`. +/// Runs in `MAX_BATCH`-sized rte_acl calls: +/// +/// - stage 1 (destination -> [`Verdict`]) for every query; +/// - then, over the stage-1 hits, exactly one of: +/// - stage 3 (masquerade verification) when the verdict is a masquerade destination, because such +/// an address does not identify a VPC and the one stage 1 reports is an arbitrary pick among +/// the peers sharing it -- see [`MasqueradeKey`]; +/// - stage 2 (source -> source NAT) otherwise. +/// +/// The two are mutually exclusive: a masquerade destination cannot accept a new connection, so the +/// only traffic reaching it rides a flow that already carries its own NAT state, and no source NAT +/// needs resolving. Splitting them keeps both on the batched path and keeps each rte_acl call +/// asking a single question. fn lookup_versioned( remote: &AnyTable, Verdict>, local: &AnyTable, NatMode>, + masquerade: &AnyTable, ()>, queries: &[Query], idx: &[usize], out: &mut [LookupResult], ) where RemoteKey: MatchKey, LocalKey: MatchKey, + MasqueradeKey: MatchKey, { for (q_chunk, i_chunk) in queries.chunks(MAX_BATCH).zip(idx.chunks(MAX_BATCH)) { // Stage 1: destination -> Verdict. @@ -781,12 +991,31 @@ fn lookup_versioned( let mut verdicts: Vec> = vec![None; q_chunk.len()]; remote.lookup_batch(&remote_keys, &mut verdicts); - // Stage 2: for the hits only, source -> source NAT. + // Partition the stage-1 hits by which question they still need answered. let mut local_keys: Vec> = Vec::new(); let mut hit_pos: Vec = Vec::new(); + let mut masquerade_keys: Vec> = Vec::new(); + let mut masquerade_pos: Vec = Vec::new(); for (pos, verdict) in verdicts.iter().enumerate() { - if let Some(verdict) = verdict { - let q = &q_chunk[pos]; + let Some(verdict) = verdict else { continue }; + let q = &q_chunk[pos]; + if verdict.nat_mode == Some(NatRequirement::Masquerade) { + // Verify the flow's candidate rather than trusting the verdict's dst_vpcd. With no + // candidate there is nothing to verify, and nothing may pass: settle it here rather + // than sending a key the table cannot answer. + let Some(flow_dst_vni) = q.flow_dst_vni else { + out[i_chunk[pos]] = LookupResult::MasqueradeDestination(None); + continue; + }; + masquerade_keys.push(MasqueradeKey { + proto: q.proto, + src_vni: q.src_vni, + dst_vni: flow_dst_vni, + dst_ip: q.dst_ip, + dst_port: q.dst_port, + }); + masquerade_pos.push(pos); + } else { local_keys.push(LocalKey { proto: q.proto, src_vni: q.src_vni, @@ -797,6 +1026,22 @@ fn lookup_versioned( hit_pos.push(pos); } } + + // Stage 3: does the flow's VPC masquerade this destination for this source? + let mut verified: Vec> = vec![None; masquerade_keys.len()]; + masquerade.lookup_batch(&masquerade_keys, &mut verified); + for (hit, &pos) in masquerade_pos.iter().enumerate() { + let q = &q_chunk[pos]; + // A hit means the configuration agrees the flow's VPC owns this address; only then is + // the candidate promoted to the answer. + out[i_chunk[pos]] = LookupResult::MasqueradeDestination( + verified[hit] + .and(q.flow_dst_vni) + .map(VpcDiscriminant::from_vni), + ); + } + + // Stage 2: for the remaining hits, source -> source NAT. let mut nat_modes: Vec> = vec![None; local_keys.len()]; local.lookup_batch(&local_keys, &mut nat_modes); @@ -837,6 +1082,34 @@ mod unit_tests { assert_eq!(LocalKey::::N, 5); } + /// Every name `table_name` can produce fits rte_acl's limit, for every counter value. + /// + /// Asserted over `u64::MAX` rather than over the counter's current value: the point is that no + /// amount of running can push a name over the limit, which is exactly the property the previous + /// decimal scheme lacked. + #[test] + fn table_names_fit_the_rte_acl_limit_for_every_counter_value() { + for base in [ + "remote_v4", + "local_v4", + "remote_v6", + "local_v6", + "masq_v4", + "masq_v6", + ] { + assert!( + base.len() <= MAX_TABLE_BASE, + "base {base:?} is over the {MAX_TABLE_BASE}-byte budget", + ); + let longest = format!("ff_{base}_{:x}", u64::MAX); + assert!( + longest.len() <= MAX_TABLE_NAME, + "{longest:?} is {} bytes, over the {MAX_TABLE_NAME}-byte limit", + longest.len(), + ); + } + } + #[test] fn default_tables_are_empty() { let tables = FlowFilterContext::default(); diff --git a/flow-filter/src/context/tests.rs b/flow-filter/src/context/tests.rs index c94dcd49bb..cbaa5aefcf 100644 --- a/flow-filter/src/context/tests.rs +++ b/flow-filter/src/context/tests.rs @@ -5,8 +5,8 @@ #![cfg(test)] -use super::LookupResult; use super::tables::RuleRow; +use super::{LookupInput, LookupResult}; use crate::test_utils::*; use crate::{FlowFilterContext, NatMode, NatRequirement}; use lpm::prefix::L4Protocol; @@ -38,13 +38,25 @@ fn route( .map(NonZero::get) .zip(t.dst_port().map(NonZero::get)) }); - match context.lookup(src_vpcd, src_ip, dst_ip, proto, ports) { + let input = LookupInput { + src_vpcd, + src_ip, + dst_ip, + proto, + ports, + flow_dst_vpcd: None, + }; + match context.lookup(&input) { LookupResult::Route((dst_vpcd, dst_nat, src_nat)) => Some(Route { dst_vpcd, dst_nat, src_nat, }), - LookupResult::SourceMiss(_) | LookupResult::DestinationMiss => None, + // This helper probes without a flow, so a masquerade destination has no candidate to + // verify and nothing may pass -- the same "no route" answer as a miss. + LookupResult::MasqueradeDestination(_) + | LookupResult::SourceMiss(_) + | LookupResult::DestinationMiss => None, } } @@ -285,16 +297,31 @@ fn dst_side_overlay() -> FlowFilterContext { fn dst_side_nat_modes() { let ctx = dst_side_overlay(); - // Masquerade destination: resolves at table level as a marker (the NF only lets it through - // for reply traffic on an established masquerade flow; see crate::tests) - let masq = route( - &ctx, - vpcd(100), - &build_tcp_packet(v4("10.0.0.5"), v4("70.0.0.10"), 1234, 5678), - ) - .expect("masquerade destination resolves as a marker"); - assert_eq!(masq.dst_vpcd, vpcd(200)); - assert_eq!(masq.dst_nat, Some(NatRequirement::Masquerade)); + // Masquerade destination: never a route. The address alone cannot name a VPC (two peers may + // masquerade behind one range), so the tables verify a candidate the packet's flow supplies. + let masq_input = |flow_dst_vpcd| LookupInput { + src_vpcd: vpcd(100), + src_ip: std::net::IpAddr::V4(v4("10.0.0.5")), + dst_ip: std::net::IpAddr::V4(v4("70.0.0.10")), + proto: net::ip::NextHeader::TCP, + ports: Some((1234, 5678)), + flow_dst_vpcd, + }; + // No candidate: nothing vouches for the packet. + assert_eq!( + ctx.lookup(&masq_input(None)), + LookupResult::MasqueradeDestination(None), + ); + // The peer that really masquerades this destination: confirmed. + assert_eq!( + ctx.lookup(&masq_input(Some(vpcd(200)))), + LookupResult::MasqueradeDestination(Some(vpcd(200))), + ); + // Any other VPC: refused, whether or not it is peered at all. + assert_eq!( + ctx.lookup(&masq_input(Some(vpcd(300)))), + LookupResult::MasqueradeDestination(None), + ); // Port-forwarding destination (matching proto + port): returned let pf = route( @@ -714,10 +741,17 @@ fn reference_and_dpdk_backends_agree() { ]; for &(vni, src_ip, dst_ip, proto, ports) in probes { - let src_vpcd = vpcd(vni); + let input = LookupInput { + src_vpcd: vpcd(vni), + src_ip, + dst_ip, + proto, + ports, + flow_dst_vpcd: None, + }; assert_eq!( - reference.lookup(src_vpcd, src_ip, dst_ip, proto, ports), - dpdk.lookup(src_vpcd, src_ip, dst_ip, proto, ports), + reference.lookup(&input), + dpdk.lookup(&input), "backends disagree on {src_ip} -> {dst_ip} ({proto:?}) from vni {vni}", ); } @@ -728,6 +762,7 @@ fn reference_and_dpdk_backends_agree() { let inputs: Vec = std::iter::repeat_n(probes, 5) .flatten() .map(|&(vni, src_ip, dst_ip, proto, ports)| LookupInput { + flow_dst_vpcd: None, src_vpcd: vpcd(vni), src_ip, dst_ip, @@ -744,13 +779,7 @@ fn reference_and_dpdk_backends_agree() { assert_eq!(ref_out, dpdk_out, "batched backends disagree"); for (i, input) in inputs.iter().enumerate() { - let single = reference.lookup( - input.src_vpcd, - input.src_ip, - input.dst_ip, - input.proto, - input.ports, - ); + let single = reference.lookup(input); assert_eq!(ref_out[i], single, "batched != single at index {i}"); } } diff --git a/flow-filter/src/fuzz_gen.rs b/flow-filter/src/fuzz_gen.rs index 819ad985f3..5dd2e61e17 100644 --- a/flow-filter/src/fuzz_gen.rs +++ b/flow-filter/src/fuzz_gen.rs @@ -25,6 +25,7 @@ #![cfg(test)] +use crate::context::LookupInput; use bolero::TypeGenerator; use config::external::overlay::vpc::{Vpc, VpcTable}; use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest, VpcPeering, VpcPeeringTable}; @@ -73,7 +74,7 @@ impl FwProto { } } -#[derive(Debug, Clone, Copy, TypeGenerator)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, TypeGenerator)] pub(crate) enum ExposeSpec { Plain, StaticNat, @@ -95,10 +96,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,6 +107,18 @@ impl ExposeSpec { ) } + /// Whether a destination this expose matches is a *masquerade* destination: one the tables can + /// only verify against a flow's candidate, never resolve from the address. Such a destination + /// never yields a route, so it needs its own derived probes. + fn dest_is_masquerade(self) -> bool { + matches!( + self, + ExposeSpec::Masquerade + | ExposeSpec::MasqueradeNestingPortFw(_) + | ExposeSpec::MasqueradeSameLenPortFw(_) + ) + } + /// 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 @@ -126,19 +135,87 @@ impl ExposeSpec { } } +/// An exclusion (`not` / `not_as`) punched out of an expose's block. +/// +/// Exclusions are collapsed away during validation: `VpcExpose::validate` subtracts them and +/// normalizes, so a `ValidatedExpose` carries a *fan* of disjoint prefixes of differing lengths +/// rather than the one block that was written. That fan is the input shape the rest of the crate +/// most depends on and least often sees -- `rule_priority` orders purely by prefix length, and its +/// correctness rests on config guaranteeing that rules which can both match a packet never share +/// one. A block with a single host punched out expands to prefixes of every length from /25 to /32 +/// at once, which is as hard as that ordering gets. +/// +/// Every variant stays inside the block's upper half. Host `.1` of every block must survive, +/// because [`derive_routing_probes`] aims its guaranteed-routing probes there, and an exclusion +/// that swallowed it would turn those probes into misses. +#[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"), + // Force the host into the upper half so that host .1 (and the nested port-forwarding + // host, FW_HOST) are never the one removed. + (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, + /// Punched out of this expose's block(s). Ignored for port forwarding, which config forbids + /// exclusions on outright. + 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 } @@ -147,14 +224,18 @@ 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. + /// + /// Config forbids stateful NAT on *both* sides of a peering, but static NAT opposite stateful + /// is explicitly permitted (see `ValidatedPeering::validate_nat_combinations`: "no NAT or + /// static NAT only is compatible with all other modes on the other side"). Downgrading rather + /// than flattening to plain is what keeps masquerade-opposite-static-NAT peerings in the + /// generated population -- and those are the only ones that produce a route requiring both + /// stateful and static NAT, which is the sole case in which the NF retains a flow key. + fn strip_stateful_nat(&mut self) { for slot in self.exposes.iter_mut().flatten() { - if slot.has_nat() { - *slot = ExposeSpec::Plain; + if slot.spec.is_stateful() { + slot.spec = ExposeSpec::StaticNat; } } } @@ -162,7 +243,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()); } } } @@ -178,6 +259,14 @@ pub(crate) struct PeeringSpec { #[derive(Debug, Clone, Copy, TypeGenerator)] pub(crate) struct OverlaySpec { peerings: [Option; 4], + /// Put every plain-masquerade expose's public side in one shared range, so peers of the same + /// VPC advertise overlapping masquerade destinations. + /// + /// Overlay-level rather than per-peering because the overlap only exists between *two* + /// peerings of one VPC. Config permits it (`VpcRoute::can_overlap` exempts + /// masquerade/masquerade) and it is the one destination the tables cannot resolve from the + /// address -- only verify against a flow's candidate. + shared_masquerade_pool: bool, } /// A materialized overlay plus the number of allocated prefix blocks (probe specs map their @@ -185,7 +274,14 @@ pub(crate) struct OverlaySpec { pub(crate) struct BuiltOverlay { pub(crate) overlay: ValidatedOverlay, pub(crate) blocks: u8, + /// Whether masquerade destinations live in the shared pool, so probes know the block is worth + /// addressing at all. + pub(crate) shared_masquerade_pool: bool, pub(crate) routing_probes: Vec, + /// Probes at a masquerade destination, each carrying the *correct* candidate on its flow. + /// Every one must verify, which is what keeps stage 3's hit path exercised: a verification + /// table that matched nothing would still satisfy every "this must drop" assertion elsewhere. + pub(crate) masquerade_probes: Vec, } impl OverlaySpec { @@ -199,11 +295,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, }, }); @@ -211,18 +307,49 @@ 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()); + } + } + // Shared-pool mode. The overlap only exists when *two* peers of one VPC masquerade + // behind the same range, and left to chance that is well under 1% of generated + // overlays -- far too rare to catch a regression. So within this mode it is arranged + // rather than hoped for: every peering's remote side leads with a plain masquerade + // expose, which puts every peer of a given VPC on the shared range. + // + // The local side is stripped of stateful NAT because config forbids it on both sides of + // a peering, and the normalization below would otherwise undo the masquerade just + // installed. The second expose slot is left alone, so manifests keep some variety, and + // the other half of generated overlays are untouched by any of this. + if spec.shared_masquerade_pool { + peering.local.strip_stateful_nat(); + peering.remote.exposes[0] = Some(ExposeEntry { + spec: ExposeSpec::Masquerade, + exclude: peering.remote.exposes[0] + .map_or(ExcludeSel::None, |entry| entry.exclude), + }); + // Two shared-pool masquerade exposes in one manifest would claim the same public + // range, which `check_public_prefixes_dont_overlap` rejects. The overlap we want is + // across peerings, so keep at most one per manifest. + for manifest in [&mut peering.local, &mut peering.remote] { + let mut seen = false; + for entry in manifest.exposes.iter_mut().flatten() { + if entry.spec == ExposeSpec::Masquerade { + if seen { + entry.spec = ExposeSpec::StaticNat; + } + seen = true; + } + } } } // A default expose cannot face another default expose within one peering. 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(); + // Stateful NAT on one side of a peering forbids stateful NAT on the other. Static NAT + // opposite stateful is legal, so only the stateful side is downgraded. + 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 @@ -257,19 +384,35 @@ impl OverlaySpec { let mut peering_table = VpcPeeringTable::new(); let mut blocks: u8 = 0; let mut routing_probes = Vec::new(); + let mut masquerade_probes = Vec::new(); for (slot, peering) in spec.peerings.iter().enumerate() { let Some(peering) = peering else { continue }; let (a, b) = PEERING_PAIRS[slot]; // build_manifest assigns one block per expose spec, in order, so the block index of // expose spec `i` is `base + i`. Record each side's base before it advances. let local_base = blocks; - let local = build_manifest(&vpc_name(a), &peering.local, peering.v6, &mut blocks); + let local = build_manifest( + &vpc_name(a), + &peering.local, + peering.v6, + spec.shared_masquerade_pool, + &mut blocks, + ); let remote_base = blocks; - let remote = build_manifest(&vpc_name(b), &peering.remote, peering.v6, &mut blocks); + let remote = build_manifest( + &vpc_name(b), + &peering.remote, + peering.v6, + spec.shared_masquerade_pool, + &mut blocks, + ); derive_routing_probes( &mut routing_probes, + &mut masquerade_probes, VNIS[a], + VNIS[b], peering.v6, + spec.shared_masquerade_pool, (local_base, &peering.local), (remote_base, &peering.remote), ); @@ -291,7 +434,9 @@ impl OverlaySpec { BuiltOverlay { overlay, blocks, + shared_masquerade_pool: spec.shared_masquerade_pool, routing_probes, + masquerade_probes, } } } @@ -299,14 +444,19 @@ 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 +#[allow(clippy::too_many_arguments)] // internal builder; grouping the fields would not aid clarity fn derive_routing_probes( out: &mut Vec, + masquerade_out: &mut Vec, src_vni: u32, + remote_vni: u32, v6: bool, + shared_masquerade_pool: bool, (local_base, local): (u8, &ManifestSpec), (remote_base, remote): (u8, &ManifestSpec), ) { let src_vpcd = VpcDiscriminant::from_vni(Vni::new_checked(src_vni).unwrap()); + let remote_vpcd = VpcDiscriminant::from_vni(Vni::new_checked(remote_vni).unwrap()); for (li, lspec) in local.expose_specs().enumerate() { if !lspec.source_capable() { continue; @@ -316,12 +466,34 @@ fn derive_routing_probes( let Some(dst_public) = rspec.dest_public_space() else { continue; }; + let dst_block = if rspec == ExposeSpec::Masquerade && shared_masquerade_pool { + SHARED_MASQUERADE_BLOCK + } else { + remote_base + ri as u8 + }; + let dst_ip = block_addr(dst_block, 1, dst_public, v6); + if rspec.dest_is_masquerade() { + // A masquerade destination never routes. It is reachable only by reply traffic + // whose flow names this very peer, so the probe carries that candidate and must + // come back verified. + masquerade_out.push(Probe { + src_vpcd, + src_ip, + dst_ip, + proto: NextHeader::TCP, + ports: Some((1, 1)), + flow_dst_vpcd: Some(remote_vpcd), + }); + continue; + } out.push(Probe { src_vpcd, src_ip, - dst_ip: block_addr(remote_base + ri as u8, 1, dst_public, v6), + dst_ip, proto: NextHeader::TCP, ports: Some((1, 1)), + // No flow: these probes assert a route resolves from the tables alone. + flow_dst_vpcd: None, }); } } @@ -331,21 +503,44 @@ fn vpc_name(index: usize) -> String { format!("vpc{}", index + 1) } -fn build_manifest(vpc_name: &str, spec: &ManifestSpec, v6: bool, blocks: &mut u8) -> VpcManifest { +fn build_manifest( + vpc_name: &str, + spec: &ManifestSpec, + v6: bool, + shared_masquerade_pool: 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)); + } + // Only the plain variant joins the shared pool. The two combined variants exist to + // pin the nested and equal-length overlaps against their *own* block, which moving + // the public side would dissolve. + ExposeSpec::Masquerade if shared_masquerade_pool => { + // The public side lives in the shared block, so an exclusion keyed on `n` + // would not overlap it. Masquerade has no size-equality requirement between + // its two sides, so excluding from the private side alone is legal. + exposes.push(excluding(masquerade(n, v6, true), n, v6, exclude)); + } + ExposeSpec::Masquerade => { + exposes.push(excluding_both(masquerade(n, v6, false), n, v6, exclude)); + } + // Port forwarding forbids exclusion prefixes outright, so only the masquerade half of + // these pairs takes one -- which is also what keeps the nested overlap intact, since + // every exclusion stays in the block's upper half and FW_HOST is in the lower. ExposeSpec::MasqueradeNestingPortFw(proto) => { - exposes.push(masquerade(n, v6)); + exposes.push(excluding_both(masquerade(n, v6, false), 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, false), n, v6, exclude)); exposes.push(portfw_block(n, v6, proto)); } ExposeSpec::PortForwarding(proto) => exposes.push(portfw_host(n, v6, proto)), @@ -402,6 +597,33 @@ pub(crate) fn block_addr(n: u8, host: u8, public: bool, v6: bool) -> IpAddr { } } +/// Punch `exclude` out of an expose's private prefixes only. For a plain expose there is no public +/// side to keep in step; validation collapses the exclusion away, leaving a fan of 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, + } +} + +/// Punch the same-shaped `exclude` out of both the private and the public prefixes. +/// +/// Symmetry is not cosmetic: static NAT requires an equal address count on the two sides +/// (`ConfigError::MismatchedPrefixSizes`), so an exclusion applied to one side only would fail +/// validation for every static-NAT expose the generator emits. +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()) } @@ -415,12 +637,31 @@ fn static_nat(n: u8, v6: bool) -> VpcExpose { .unwrap() } -fn masquerade(n: u8, v6: bool) -> VpcExpose { +/// The public block every shared-pool masquerade expose translates into. +/// +/// Deliberately outside the per-expose pool (`blocks` never reaches 200), so it collides with +/// nothing except other shared-pool masquerade exposes -- which is the entire point. +const SHARED_MASQUERADE_BLOCK: u8 = 200; + +/// A masquerade expose. `shared_pool` puts its *public* side in a range every other shared-pool +/// masquerade expose also uses, while leaving its private side in its own block. +/// +/// That is the cross-peering overlap config permits and `VpcRoute::can_overlap` exempts: two peers +/// masquerading behind one public range. It is generated on purpose because the destination it +/// produces cannot be resolved from the address -- only verified against a flow's candidate -- and +/// a generator that never built one left the whole verification path untested by the property +/// suite. +fn masquerade(n: u8, v6: bool, shared_pool: bool) -> VpcExpose { + let public = if shared_pool { + SHARED_MASQUERADE_BLOCK + } else { + n + }; VpcExpose::empty() .make_masquerade(None) .unwrap() .ip(private_block(n, v6).as_str().into()) - .as_range(public_block(n, v6).as_str().into()) + .as_range(public_block(public, v6).as_str().into()) .unwrap() } @@ -502,11 +743,19 @@ pub(crate) struct ProbeSpec { src_public: bool, src_host: u8, dst_block: u8, + /// Aim the destination at the shared masquerade pool instead of a per-expose block. + /// + /// Without this no generated probe ever reaches a shared-pool masquerade destination -- the + /// pool sits outside the `0..blocks` range the selectors are reduced into -- so the ambiguous + /// destinations the overlay generator works to build would never actually be looked up. + dst_shared_masquerade: bool, dst_public: bool, dst_host: u8, proto: ProbeProto, sport: PortSel, dport: PortSel, + /// Which VPC the packet's flow claims as its destination, if any. + flow_dst_sel: Option, } /// A resolved probe: the arguments of one route lookup. @@ -517,11 +766,33 @@ pub(crate) struct Probe { pub(crate) dst_ip: IpAddr, pub(crate) proto: NextHeader, pub(crate) ports: Option<(u16, u16)>, + /// The destination VPC the packet's flow claims, if any -- the candidate the tables verify for + /// a masquerade destination. Generated independently of the rest of the probe, so it covers + /// having no candidate, the right one, a wrong-but-real one, and one no VPC uses. + pub(crate) flow_dst_vpcd: Option, +} + +impl Probe { + pub(crate) fn input(&self) -> LookupInput { + LookupInput { + src_vpcd: self.src_vpcd, + src_ip: self.src_ip, + dst_ip: self.dst_ip, + proto: self.proto, + ports: self.ports, + flow_dst_vpcd: self.flow_dst_vpcd, + } + } } impl ProbeSpec { - pub(crate) fn resolve(&self, blocks: u8) -> Probe { - let nblocks = blocks.max(1); + pub(crate) fn resolve(&self, built: &BuiltOverlay) -> Probe { + let nblocks = built.blocks.max(1); + let dst_block = if self.dst_shared_masquerade && built.shared_masquerade_pool { + SHARED_MASQUERADE_BLOCK + } else { + self.dst_block % nblocks + }; let vni = match self.vni_sel as usize % (VNIS.len() + 1) { i if i < VNIS.len() => VNIS[i], _ => BOGUS_VNI, @@ -535,17 +806,19 @@ impl ProbeSpec { self.src_public, self.v6, ), - dst_ip: block_addr( - self.dst_block % nblocks, - self.dst_host, - self.dst_public, - dst_v6, - ), + dst_ip: block_addr(dst_block, self.dst_host, self.dst_public, dst_v6), proto: self.proto.next_header(), ports: match self.proto { ProbeProto::Icmp => None, _ => Some((self.sport.resolve(), self.dport.resolve())), }, + flow_dst_vpcd: self.flow_dst_sel.map(|sel| { + let vni = match sel as usize % (VNIS.len() + 1) { + i if i < VNIS.len() => VNIS[i], + _ => BOGUS_VNI, + }; + VpcDiscriminant::from_vni(Vni::new_checked(vni).unwrap()) + }), } } } diff --git a/flow-filter/src/lib.rs b/flow-filter/src/lib.rs index c2250bde67..3fa78111b4 100644 --- a/flow-filter/src/lib.rs +++ b/flow-filter/src/lib.rs @@ -148,6 +148,11 @@ impl FlowFilter { .map(NonZero::get) .zip(t.dst_port().map(NonZero::get)) }), + // The candidate destination for a masquerade destination, which the tables verify + // rather than resolve. Passed in even when the flow is stale or inactive: the tables + // only answer whether the configuration permits it, and this function re-checks the + // flow's own state before acting on the answer. + flow_dst_vpcd: attached_flow.as_ref().and_then(|flow| flow.dst_vpcd), }; Classification::Lookup { input, @@ -174,6 +179,30 @@ impl FlowFilter { let nfi = &self.name; let (dst_vpcd, dst_nat_mode, src_nat_mode) = match result { LookupResult::Route(route) => route, + // A masquerade destination cannot accept a new connection, so only reply traffic on an + // established flow may pass -- and only towards the VPC the tables confirmed + // masquerades this destination for this source. The tables cannot name that VPC from + // the address alone (two peers may masquerade behind one range), so they verified the + // candidate the flow supplied; `None` means no candidate, or one the configuration + // does not agree with. Either way nothing vouches for the packet. + LookupResult::MasqueradeDestination(verified) => { + if let Some(dst_vpcd) = verified + && let Some(flow) = + active_stateful_flow(flow_summary, dst_vpcd, |f| f.needs_masquerade) + { + debug!( + "{nfi}: Masquerade destination {dst_vpcd} confirmed by established flow" + ); + Self::tag_for_bypass(packet.meta_mut(), dst_vpcd, flow); + return; + } + debug!( + "{nfi}: Masquerade destination with no flow vouching for it, dropping packet (cannot initiate a connection towards a masquerade expose)" + ); + packet.invalidate_flows(); + packet.done(DoneReason::Filtered); + return; + } LookupResult::SourceMiss(dst_vpcd) => { // Port-forwarding sources are deliberately absent from the local tables; reply // traffic from one rides its established flow. @@ -197,24 +226,6 @@ impl FlowFilter { } }; - // A masquerade destination cannot accept new connections; its rule is in the table only - // so that reply traffic on an established masquerade flow is distinguishable from a - // destination no peering covers. - if dst_nat_mode == Some(NatRequirement::Masquerade) { - if let Some(flow) = active_stateful_flow(flow_summary, dst_vpcd, |f| f.needs_masquerade) - { - debug!("{nfi}: Masquerade destination allowed by established flow"); - Self::tag_for_bypass(packet.meta_mut(), dst_vpcd, flow); - return; - } - debug!( - "{nfi}: Masquerade destination with no established flow, dropping packet (cannot initiate a connection towards a masquerade expose)" - ); - packet.invalidate_flows(); - packet.done(DoneReason::Filtered); - return; - } - debug!( "{nfi}: Packet matches peering configuration, found VPC {dst_vpcd} and NAT modes {src_nat_mode:?} (src), {dst_nat_mode:?} (dst)" ); diff --git a/flow-filter/src/test_utils.rs b/flow-filter/src/test_utils.rs index c1e6513483..0ae5ea64c2 100644 --- a/flow-filter/src/test_utils.rs +++ b/flow-filter/src/test_utils.rs @@ -200,6 +200,35 @@ 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() +} + +/// An ICMPv6 packet. Note the next header this carries is `NextHeader::ICMP6`, not the `ICMP` of +/// the v4 builder: a lookup key built for one will not match a packet built by the other. +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..c6251f08c1 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,852 @@ fn burst_processing_upholds_structural_invariants() { } }); } + +// ------------------------------------------------------------------------------------------------- +// End-to-end config oracle: from a generated configuration all the way to a packet's metadata. +// +// The context property tests stop at a `LookupResult`. The step after it -- turning that result +// into the destination and NAT flags the downstream NFs act on -- was covered only by hand-written +// examples, so the mapping was never checked against a configuration nobody chose. This closes +// that last hop: the route is predicted by the *same* config oracle the context suite uses (see +// `context::fuzz::oracle_lookup`), and only the `LookupResult` -> metadata step is restated here. +// +// Scope: packets with no attached flow. Everything a flow contributes -- bypass, invalidation, and +// the reply paths that let an established flow overrule a miss -- is deliberately excluded, since +// that logic is not a function of the configuration and is covered by +// `invalidation_decision_matches_spec` and the flow tests above. + +/// 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, + }, +} + +/// The outcome the configuration predicts, given the route it resolves to. +/// +/// Without a flow to vouch for it, every miss drops: a destination miss because no peering covers +/// the packet, a source miss because only an established port-forwarding flow could excuse one. A +/// masquerade destination drops for the same reason -- it cannot accept a new connection. +fn expected_outcome(result: LookupResult) -> NfOutcome { + let (dst_vpcd, dst_nat, src_nat) = match result { + LookupResult::Route(route) => route, + // A masquerade destination needs a flow to vouch for it, and these suites run flowless + // packets, so it always drops -- as do both misses. + LookupResult::MasqueradeDestination(_) + | LookupResult::SourceMiss(_) + | LookupResult::DestinationMiss => { + 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, + // Stateful NAT combined with static NAT is the one case that needs the original addresses + // kept, so the right flow-table entries can be built later. + flow_key: (masquerade || port_forwarding) && (static_nat_src || static_nat_dst), + } +} + +/// The lookup key a packet presents, as a [`Probe`] the config oracle can answer. +/// +/// Deliberately the same extraction `FlowFilter::classify` performs, through the same public +/// accessors: this is how a test asks "what did the NF see", not an independent re-derivation of +/// it. Returns `None` for a packet with no IP layer, which never reaches the tables at all. +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)) + }), + // These suites run flowless packets, so there is no candidate to verify. + flow_dst_vpcd: None, + }) +} + +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(), + } +} + +/// Realize a probe as a real packet, together with the probe as that packet actually carries it. +/// +/// Not every probe is a packet. Source and destination must agree on IP version (the lookup treats +/// a mismatch as a destination miss, but no such packet exists on the wire), ports on the wire are +/// non-zero (0 is the wildcard the lookup substitutes for a portless packet), and ICMP over v6 is a +/// different next header than over v4. Where the packet cannot carry the probe verbatim, the probe +/// is adjusted to match -- so the oracle is always asked about the packet that was really built. +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)) +} + +/// The metadata the NF stamps on a flowless packet is exactly what the configuration predicts, for +/// every probe of every generated overlay. +/// +/// Coverage is asserted, not assumed: the counters fail the test if a time-boxed run never routed a +/// packet, never applied a NAT requirement, or never reached the flow-key case (which needs a route +/// requiring both stateful and static NAT, and so exists only on masquerade-opposite-static-NAT +/// peerings). +#[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 NATTED: 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)| { + // Built inside the closure: the filter holds a classifier, which is not unwind-safe and + // so cannot be captured across bolero's catch_unwind boundary. + let built = overlay_spec.build(); + let (mut flow_filter, _writer) = + make_flow_filter(FlowFilterContext::for_test(&built.overlay)); + + // The overlay's own derived probes route by construction, which is where the NAT flags + // actually get exercised; the generated probes supply the misses and the edges. + let derived = built.routing_probes.iter().copied(); + let generated = probe_specs.iter().map(|spec| spec.resolve(&built)); + 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); + if masquerade || static_nat_src || static_nat_dst || port_forwarding { + NATTED.fetch_add(1, Ordering::Relaxed); + } + if flow_key { + FLOW_KEYED.fetch_add(1, Ordering::Relaxed); + } + } + } + } + }); + + let (routed, natted) = ( + ROUTED.load(Ordering::Relaxed), + NATTED.load(Ordering::Relaxed), + ); + let (flow_keyed, dropped) = ( + FLOW_KEYED.load(Ordering::Relaxed), + DROPPED.load(Ordering::Relaxed), + ); + eprintln!( + "coverage: {routed} routed ({natted} with a NAT requirement, \ + {flow_keyed} retaining a flow key), {dropped} dropped" + ); + assert!(routed >= 1, "no packet was ever routed"); + assert!(natted >= 1, "no route ever carried a NAT requirement"); + assert!(flow_keyed >= 1, "the flow-key case was never reached"); + assert!(dropped >= 1, "no packet was ever dropped"); +} + +// ------------------------------------------------------------------------------------------------- +// Adversarial header stacks. +// +// `classify` is this NF's parser-facing edge -- it is the only place that reaches into a packet's +// headers -- and every test above it feeds exactly four hand-built shapes (v4 TCP/UDP/ICMP and a +// bare Ethernet frame). Nothing exercised a VLAN tag, an IPv6 extension-header chain, a fragment, +// an authentication header, or the fuzzed remainder of any header's fields. +// +// `net` already ships a bolero header-stack generator; this points it at the NF. What the test can +// honestly claim is bounded: `probe_from_packet` extracts the lookup key with the same public +// accessors `classify` uses, so this does not independently verify that extraction. Its value is +// that (a) no header stack the generator can build makes the NF panic, (b) the fail-closed and +// burst-structural invariants hold on stacks nobody hand-picked, and (c) the route -> metadata +// mapping still holds for keys that *only* exotic stacks produce -- an extension-header protocol +// number, a portless IP packet, a fragment. Those keys are unreachable from the four hand-built +// shapes, and they are what the coverage counters below insist on reaching. + +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}; + + /// The source VPC every generated packet claims. + fn src_vpcd() -> VpcDiscriminant { + vpcd(100) + } + + /// A two-peering overlay whose prefixes the generated stacks aim at: a v4 peering carrying one + /// expose of each source NAT mode, and a v6 peering so the v6 stacks have somewhere to land. + /// Both manifests of a peering are single-version, as validation requires. + 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")]), + ), + ], + ) + } + + // Address pinning. Every other field of every header stays fuzzed; only the addresses are + // steered, because a uniformly random address never lands in a configured prefix and the whole + // run would collapse into destination misses. The fuzzed low byte is kept, so host selection + // (and with it the port-range and prefix-length edges) still comes from the driver. + + fn pin_v4(ip: &mut net::ipv4::Ipv4) { + let src = ip.source().inner().octets(); + // 1/2/3 are the plain, static-NAT and masquerade source exposes; 4 is covered by none. + 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(); + // 5 is the peer's expose; 9 is nowhere. + 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(); + // Half the destinations land in the peer's expose, half nowhere near it. + 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]), + )); + } + + /// One header stack per shape. Each is its own concrete generator type (`ValueGenerator` has a + /// generic method and so is not object-safe), which is why this dispatches through a `match` + /// rather than a table of boxed generators. + struct AnyStack; + + impl ValueGenerator for AnyStack { + type Output = Headers; + + fn generate(&self, driver: &mut D) -> Option { + match driver.produce::()? % 10 { + // No IP layer at all: the NotIp path. + 0 => ChainBase::new().eth(|_| {}).generate(driver), + 1 => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .tcp(|_| {}) + .generate(driver), + 2 => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .udp(|_| {}) + .generate(driver), + 3 => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .icmp4(|_| {}) + .generate(driver), + // A VLAN tag between the Ethernet and IP layers. + 4 => ChainBase::new() + .eth(|_| {}) + .vlan(|_| {}) + .ipv4(pin_v4) + .tcp(|_| {}) + .generate(driver), + // An IPv4 authentication header ahead of the transport. + 5 => ChainBase::new() + .eth(|_| {}) + .ipv4(pin_v4) + .ipv4_auth(|_| {}) + .tcp(|_| {}) + .generate(driver), + 6 => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .tcp(|_| {}) + .generate(driver), + 7 => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .udp(|_| {}) + .generate(driver), + // IPv6 extension-header chains, ahead of a transport header. + 8 => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .hop_by_hop(|_| {}) + .tcp(|_| {}) + .generate(driver), + 9 => ChainBase::new() + .eth(|_| {}) + .ipv6(pin_v6) + .fragment(|_| {}) + .udp(|_| {}) + .generate(driver), + _ => unreachable!("modulo 10"), + } + } + } + + /// Serialize generated headers into a parseable overlay packet. Returns `None` when the stack + /// cannot round-trip through a `TestBuffer` (too large, or not re-parseable) -- those are + /// counted, so a run that silently discarded everything cannot pass. + 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) + } + + /// However exotic the headers, a flowless packet leaves the NF with exactly the metadata the + /// configuration predicts for the key that packet presents -- and a packet with no IP layer is + /// dropped as `NotIp` without ever reaching the tables. + #[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)); + + let overlay = wire_overlay(); + + bolero::check!() + .with_generator(AnyStack) + .for_each(|headers: &Headers| { + // Built inside the closure: the filter holds a classifier, which is not unwind-safe + // and so cannot be captured across bolero's catch_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; + }; + + // 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 {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(", ") + ); + + // The point of the suite is the keys the hand-built shapes cannot produce. If the generator + // stops reaching them -- or starts failing to round-trip everything -- this must fail + // rather than pass on a diet of ordinary v4 TCP. + for (label, count) in counts { + if label == "unparseable" { + continue; + } + assert!(count >= 1, "no {label} packet was ever generated"); + } + } +} + +// ------------------------------------------------------------------------------------------------- +// Documented behaviour: IPv6 extension headers hide the transport protocol. +// +// Found by the adversarial-header suite above, which reaches these keys in the thousands per run. +// Pinned here as its own test because a fuzz run that merely agrees with the config oracle does not +// say *what* the configuration means for these packets, and the answer is surprising enough that a +// future change to it should be deliberate. + +/// `Net::next_header()` reports the IPv6 header's own next-header field. When any extension header +/// is present that field names the *extension*, not the transport -- while `try_transport()` still +/// walks the chain and finds the real ports. +/// +/// So an IPv6 packet carrying TCP behind a hop-by-hop header presents the lookup key +/// `(proto = HOPOPT, ports = the TCP ports)`. A protocol-restricted expose lowers to an exact match +/// on the protocol byte, so it cannot match such a packet, and the traffic is dropped rather than +/// port-forwarded. That is fail-closed, but it is a functional gap: legitimate TCP over IPv6 with +/// extension headers does not reach a TCP-restricted port-forwarding destination. +#[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 mechanism: the protocol byte names the extension header, but the ports are still found. + 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", + ); + + // The consequence: a TCP-restricted destination expose cannot match it. + 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", + ); + + // The same packet routes when the covering expose is not protocol-restricted, which confirms + // the protocol byte -- not the address or the port -- is what excluded it above. + 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))); +} + +// ------------------------------------------------------------------------------------------------- +// Documented behaviour: two more edges the adversarial-header and burst suites reach constantly +// but never pin, because a property test can only say "the NF agreed with the config", not "and +// here is what the config means". + +/// A packet with no usable transport ports looks up with port `0`, and config forbids port `0` in +/// an expose's ranges -- so a port-restricted expose can never match one. +/// +/// This is not an exotic case: an ICMP packet, a non-first fragment, and (per +/// [`ipv6_extension_header_masks_the_transport_protocol`]) anything behind an IPv6 extension header +/// all present portless keys. The adversarial-header suite generates thousands per run. The effect +/// is fail-closed, but it means port-forwarded destinations are unreachable by such traffic -- +/// including the ICMP errors that path MTU discovery depends on. +#[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); + + // The same address and protocol route once a covering expose carries no port constraint, which + // confirms the port -- not the address or the protocol -- is what excluded it. + 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 whose generation is *newer* than the pipeline's is trusted for bypass. +/// +/// `dst_vpcd_from_valid_flow` rejects only `flow_genid < genid`, so a flow tagged with a generation +/// the filter has not yet observed short-circuits the tables entirely. That is deliberate -- it is +/// the "small transient period" the function's comment describes, where a reconfiguring control +/// plane has already stamped flows with the new generation but this worker still reads the old one, +/// and treating those as stale would tear down every flow the new config just blessed. +/// +/// Pinned because it is the one direction of the genid comparison nothing else covers, and because +/// it is asymmetric in a way that reads like a bug until the transient is explained: the bypass +/// here wins over what the tables say, exactly as an equal-generation flow would. +#[test] +fn flow_from_a_newer_generation_is_honored_for_bypass() { + let (mut flow_filter, _writer) = make_flow_filter(source_nat_context()); + + // A destination the current tables do not cover at all: without the flow this would be + // Filtered, so honouring the flow is observable rather than incidental. + 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", + ); +} + +// ------------------------------------------------------------------------------------------------- +// Cross-peering masquerade overlap. +// +// Config lets two peers masquerade behind the same public range (`VpcRoute::can_overlap` exempts +// masquerade/masquerade) and models the result as a *set* of routes per destination +// (`VpcRouteSet` is a `Vec`). Stage 1 holds one destination VPC per match, so lowering that set +// into it collapses the set and rte_acl's highest-priority-wins picks arbitrarily -- which used to +// mean every masquerade reply toward the losing peer was dropped and its flow torn down. +// +// The destination for such traffic is now verified against the candidate the packet's flow names, +// never resolved from the address. + +/// vpc1 peers with vpc2 and vpc3; both peers masquerade behind the *same* public range. +fn shared_masquerade_range_context() -> FlowFilterContext { + context( + &[("vpc1", 100), ("vpc2", 200), ("vpc3", 300)], + vec![ + peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose("10.0.0.0/24")]), + ( + "vpc2", + vec![expose_masquerade("192.168.0.0/24", "20.0.0.0/24")], + ), + ), + peering( + "vpc1-to-vpc3", + ("vpc1", vec![expose("10.0.0.0/24")]), + ( + "vpc3", + vec![expose_masquerade("192.168.1.0/24", "20.0.0.0/24")], + ), + ), + ], + ) +} + +/// Reply traffic reaches *whichever* peer the flow names, not whichever rule happens to sort first. +/// +/// Both directions must work. Before the verification stage one of these two was always dropped and +/// its flow cancelled -- deterministically, but arbitrarily, and differently under rte_acl than +/// under the reference backend, since equal-priority ties are unspecified there. +#[test] +fn masquerade_replies_reach_both_peers_sharing_a_public_range() { + for peer in [200u32, 300u32] { + let (mut flow_filter, _writer) = make_flow_filter(shared_masquerade_range_context()); + // Advance the generation so the flow is outdated and the bypass in `classify` is refused: + // this must be answered by the tables, which is where the ambiguity lived. + set_genid(&mut flow_filter, 5); + + let mut p = packet( + Some(vpcd(100)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + let flow = attach_flow(&mut p, Some(vpcd(peer)), true, true, false); + let out = run(&mut flow_filter, p); + + assert!( + !out.is_done(), + "reply on an established masquerade flow to VNI {peer} was dropped: {:?}", + out.get_done(), + ); + assert_eq!( + out.meta().dst_vpcd, + Some(vpcd(peer)), + "reply was sent to the wrong peer", + ); + assert!(out.meta().requires_masquerade()); + assert_ne!( + flow.status(), + FlowStatus::Cancelled, + "a valid flow to VNI {peer} was torn down", + ); + } +} + +/// A flow naming a VPC that does not masquerade the destination is still refused. +/// +/// This is the staleness check the old destination-equality comparison was also providing, and it +/// must survive: verification asks the configuration, so a candidate the configuration does not +/// agree with is rejected exactly as an unrelated one is. vpc3 masquerades `20.0.0.0/24` but not +/// `40.0.0.0/24`, so a flow claiming vpc3 cannot carry traffic to the latter. +#[test] +fn masquerade_reply_is_refused_when_the_flow_names_the_wrong_peer() { + let ctx = context( + &[("vpc1", 100), ("vpc2", 200), ("vpc3", 300)], + vec![ + peering( + "vpc1-to-vpc2", + ("vpc1", vec![expose("10.0.0.0/24")]), + ( + "vpc2", + vec![expose_masquerade("192.168.0.0/24", "40.0.0.0/24")], + ), + ), + peering( + "vpc1-to-vpc3", + ("vpc1", vec![expose("10.0.0.0/24")]), + ( + "vpc3", + vec![expose_masquerade("192.168.1.0/24", "20.0.0.0/24")], + ), + ), + ], + ); + let (mut flow_filter, _writer) = make_flow_filter(ctx); + set_genid(&mut flow_filter, 5); + + let mut p = packet( + Some(vpcd(100)), + build_tcp_packet(v4("10.0.0.5"), v4("40.0.0.5"), 1234, 80), + ); + // The destination belongs to vpc2's masquerade range; the flow claims vpc3. + let flow = attach_flow(&mut p, Some(vpcd(300)), true, true, false); + let out = run(&mut flow_filter, p); + + assert_eq!(out.get_done(), Some(DoneReason::Filtered)); + assert_eq!(flow.status(), FlowStatus::Cancelled); +}