diff --git a/miri.just b/miri.just index 091da10355..db51d93875 100644 --- a/miri.just +++ b/miri.just @@ -25,7 +25,20 @@ export target := cpu + "-unknown-linux-gnu" export provenance := "permissive" export schedule_seed := choose('5', "0123456789") export seeds := "1" -export stacked_borrow_check := "disabled" +# Overridable from the environment because `just` will not override a *module's* variables from the +# command line: `just miri stacked_borrow_check=enabled test` is parsed as a recipe name, and +# `just --set stacked_borrow_check enabled miri test` is refused outright ("not present in justfile"). +# Without `env()` these knobs can only be changed by editing this file, which is how +# `stacked_borrow_check` came to be effectively unreachable. +export stacked_borrow_check := env("STACKED_BORROW_CHECK", "disabled") + +# How long each bolero property gets, in milliseconds. +# +# Needed because this recipe launches its own `nix-shell`, which does not inherit the caller's +# environment: exporting `BOLERO_RANDOM_TEST_TIME_MS` before `just miri test` has no effect, and every +# property silently stops at bolero's one-second default. Under miri that is about 25 cases -- enough to +# prove the harness runs and nothing else. +export bolero_test_time_ms := env("BOLERO_TEST_TIME_MS", "30000") export preemption_rate := "0.10" export weak_failure_rate := "0.05" export randomize_struct_layout := "enabled" @@ -63,6 +76,7 @@ test *args="": # Umbrella cfg shared with the qemu-user path in nix/profiles.nix. RUSTFLAGS+="--cfg=emulated" declare -rx RUSTFLAGS + declare -rx BOLERO_RANDOM_TEST_TIME_MS="${bolero_test_time_ms}" declare -a cmd=("nice" "-n" "19" "cargo" "miri" "nextest" "run" "--profile=miri" "--target=${target}") if [ "${cores}" != "0" ]; then # nextest defaults --test-threads to the core count; the miri profile diff --git a/net/src/headers/embedded_view.rs b/net/src/headers/embedded_view.rs index fe6d5eba0e..f5aeee0ad6 100644 --- a/net/src/headers/embedded_view.rs +++ b/net/src/headers/embedded_view.rs @@ -1285,6 +1285,52 @@ mod tests { assert!(matches::<(&Ipv4, &TruncatedTcp)>(e)); } + /// An ICMP error quoting an ICMP packet, which the builder had never been asked to assemble. + /// + /// `EmbeddedAssembler::icmp4` and `::icmp6` were the two inner-transport methods no test called, + /// and with them the `EmbeddedTransport::Icmp4 => NextHeader::ICMP` arm of the fixup that sets + /// the quoted packet's protocol field. A ping that draws a destination-unreachable is the + /// ordinary way to produce one, so the gap was in the tests rather than in the scenario. + /// + /// The inner protocol number is checked directly: `fixup_embedded` is what writes it, the shape + /// match below would pass without it, and a quoted packet whose IP header disagrees with the + /// transport it carries is exactly what a peer cannot parse. + #[test] + fn icmp4_quoted_inside_an_icmp4_error() { + use crate::headers::builder::Blank; + use crate::icmp4::Icmp4; + use crate::ip::NextHeader; + let h = icmp4_with_embedded(|a| a.ipv4(|_| {}).icmp4(Icmp4::blank())); + let e = h.embedded_ip().expect("embedded must be present"); + assert!(matches::<(&Ipv4, &TruncatedIcmp4)>(e)); + let Some(Net::Ipv4(ip)) = e.net() else { + unreachable!("the quoted packet carries an IPv4 header") + }; + assert_eq!( + ip.next_header(), + NextHeader::ICMP, + "the quoted IP header does not name the transport it carries" + ); + } + + #[test] + fn icmp6_quoted_inside_an_icmp6_error() { + use crate::headers::builder::Blank; + use crate::icmp6::Icmp6; + use crate::ip::NextHeader; + let h = icmp6_with_embedded(|a| a.ipv6(|_| {}).icmp6(Icmp6::blank())); + let e = h.embedded_ip().expect("embedded must be present"); + assert!(matches::<(&Ipv6, &TruncatedIcmp6)>(e)); + let Some(Net::Ipv6(ip)) = e.net() else { + unreachable!("the quoted packet carries an IPv6 header") + }; + assert_eq!( + ip.next_header(), + NextHeader::ICMP6, + "the quoted IP header does not name the transport it carries" + ); + } + #[test] fn ipv6_truncated_udp_matches_full_inner_packet() { use crate::udp::UdpPort; @@ -1790,3 +1836,655 @@ mod tests { } } } + +// =========================================================================== +// Differential properties +// =========================================================================== + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] // fine to unwrap in tests +mod embedded_view_properties { + use super::*; + use crate::eth::Eth; + use crate::headers::{Headers, ShapedIcmpError, ShapedQuote}; + use crate::icmp4::Icmp4; + use crate::icmp6::Icmp6; + use crate::vlan::Vlan; + + /// Both structural walks over a quoted packet, across the range of outer and inner shapes. + /// + /// The pairing this checks is the one soundness rests on, and it is not the obvious one. + /// `as_embedded`/`as_embedded_mut` *decide* with [`embedded_sealed::Sealed::matches`], which + /// walks [`EmbeddedHeaders`] through [`EmbeddedStep`]. [`EmbeddedLookMut::look_mut`] then + /// *delivers* through [`EmbeddedMatcherMut`], a completely separate implementation with its own + /// pre-split [`EmbeddedFields`](super::super::pat::EmbeddedFields) and its own gap check, and it + /// unwraps that chain with `unreachable_unchecked` on the strength of the first one's answer. + /// Two implementations, one of them licensing the other to skip its own `None` branch. If they + /// ever disagree the result is undefined behaviour, not a wrong answer. + /// + /// So the oracle here is `pat()`/`pat_mut()`'s `.embedded()` chain, which is the same machinery + /// `look_mut` uses -- making this differential test exactly the invariant `look_mut` assumes. + /// + /// Arity matters twice over, because `matches`, `look` and `look_mut` are generated per *inner* + /// arity while `as_embedded`/`as_embedded_mut` are generated per *outer* arity, each threading + /// the extension cursor by hand. Before this the suite reached one outer arity and one inner + /// arity, of eight and three. + macro_rules! embedded_agrees { + ( + $read:ident, $mutable:ident, + ($outer:ty, $($ol:ident),+), + ($inner:ty, $($il:ident),+) + ) => { + #[test] + fn $read() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static SEEN: AtomicUsize = AtomicUsize::new(0); + static HIT: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + SEEN.fetch_add(1, Ordering::Relaxed); + let licensed = h + .as_view::<$outer>() + .is_some_and(|w| w.as_embedded::<$inner>().is_some()); + if licensed { + HIT.fetch_add(1, Ordering::Relaxed); + } + let deliverable = + h.pat()$(.$ol())+.embedded()$(.$il())+.done().is_some(); + assert_eq!( + licensed, deliverable, + concat!( + "`matches` and the read walk disagree for ", + stringify!($inner), + " inside ", + stringify!($outer), + ", so `look` would reach `unreachable_unchecked`: {:?}" + ), + h + ); + }); + agreement_is_not_vacuous( + concat!(stringify!($inner), " in ", stringify!($outer)), + SEEN.load(Ordering::Relaxed), + HIT.load(Ordering::Relaxed), + ); + } + + #[test] + fn $mutable() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static SEEN: AtomicUsize = AtomicUsize::new(0); + static HIT: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + SEEN.fetch_add(1, Ordering::Relaxed); + // `as_embedded_mut`'s reference cannot outlive the `and_then` closure it + // would be produced in, so this asks the question without keeping it. + let licensed = match owned.as_view_mut::<$outer>() { + Some(w) => w.as_embedded_mut::<$inner>().is_some(), + None => false, + }; + if licensed { + HIT.fetch_add(1, Ordering::Relaxed); + } + let deliverable = + owned.pat_mut()$(.$ol())+.embedded()$(.$il())+.done().is_some(); + assert_eq!( + licensed, deliverable, + concat!( + "`matches` and the mutable walk disagree for ", + stringify!($inner), + " inside ", + stringify!($outer), + ", so `look_mut` would reach `unreachable_unchecked`: {:?}" + ), + h + ); + }); + agreement_is_not_vacuous( + concat!(stringify!($inner), " in ", stringify!($outer)), + SEEN.load(Ordering::Relaxed), + HIT.load(Ordering::Relaxed), + ); + } + }; + } + + /// A shape the generator never produces makes its property pass for the wrong reason. + /// + /// Two shapes compound here -- an outer arity and an inner one -- so a property can starve much + /// more quietly than in [`view`](super::super::view): demanding four VLAN tags *and* a + /// particular extension inside the quoted packet multiplies two small probabilities. Each pair + /// reports its own hit rate, and the pairs are chosen to vary one dimension at a time for + /// exactly that reason. + fn agreement_is_not_vacuous(shape: &str, seen: usize, hit: usize) { + println!("{shape}: matched {hit} of {seen}"); + // Twenty thousand rather than [`view`](super::super::view)'s five hundred, because + // compounding costs an order of magnitude: the widest pair here matches about one packet in + // twenty, and `(&Ipv6, &HopByHop, &TruncatedTcp)` inside an ICMPv6 error matches about one + // in a thousand. Five hundred cases at that rate would fail this check outright half the + // time it ran. A deliberately short run should say nothing rather than say something false. + // + // Ten thousand, not the twenty-five thousand a default run manages here. The thinnest shape + // in this module draws about eight hits per ten thousand, and coverage instrumentation costs + // about a fifth of the throughput -- measured at 17 hits in 20,206 cases against 21 in + // 25,857 without. A threshold set at the observed count would therefore switch itself off on + // any busier runner, silently. At ten thousand draws a shape that really is drawable misses + // entirely about three times in ten thousand runs, which is the trade this wants. + if seen > 10_000 { + assert!( + hit > 0, + "{shape} never matched in {seen} packets: the two walks agree only because the \ + generator cannot produce this shape" + ); + } + } + + type O4V4 = (&'static Eth, &'static Ipv4, &'static Icmp4); + type O4V6 = (&'static Eth, &'static Ipv6, &'static Icmp6); + + // ---- Inner shapes, at the one outer arity the builders could already express ---------- + // + // Varying the inner shape alone: network layer, both enum forms, every truncated transport, + // and the extension region. + embedded_agrees!( + read_inner_v4_tcp, + mutable_inner_v4_tcp, + (O4V4, eth, ipv4, icmp4), + ((&Ipv4, &TruncatedTcp), ipv4, tcp) + ); + embedded_agrees!( + read_inner_v6_udp, + mutable_inner_v6_udp, + (O4V6, eth, ipv6, icmp6), + ((&Ipv6, &TruncatedUdp), ipv6, udp) + ); + embedded_agrees!( + read_inner_net_only, + mutable_inner_net_only, + (O4V4, eth, ipv4, icmp4), + ((&Net,), net) + ); + embedded_agrees!( + read_inner_v6_only, + mutable_inner_v6_only, + (O4V6, eth, ipv6, icmp6), + ((&Ipv6,), ipv6) + ); + // An ICMP error quoting an ICMP packet -- a `TruncatedIcmp4` inside an `Icmp4`. Legitimate: + // an unreachable in response to a ping quotes the echo request. + embedded_agrees!( + read_inner_icmp_in_icmp4, + mutable_inner_icmp_in_icmp4, + (O4V4, eth, ipv4, icmp4), + ((&Ipv4, &TruncatedIcmp4), ipv4, icmp4) + ); + embedded_agrees!( + read_inner_icmp_in_icmp6, + mutable_inner_icmp_in_icmp6, + (O4V6, eth, ipv6, icmp6), + ((&Ipv6, &TruncatedIcmp6), ipv6, icmp6) + ); + + // ---- The embedded extension region --------------------------------------------------- + // + // `ext_gap_ok_embedded` versus `ext_gap_ok_mut_embedded`: naming an extension switches the + // quoted packet from skip-extensions-silently to consume-them-all, so these match only a quote + // carrying that extension and no other. The read pair shares `ext_gap_ok_embedded` between the + // two walks and so tests the by-hand `ec` threading; the mutable pair runs two genuinely + // different implementations against each other, one over `EmbeddedHeaders` and one over a + // pre-split `EmbeddedFields`. + embedded_agrees!( + read_inner_ext_v6, + mutable_inner_ext_v6, + (O4V6, eth, ipv6, icmp6), + ((&Ipv6, &HopByHop, &TruncatedTcp), ipv6, hop_by_hop, tcp) + ); + embedded_agrees!( + read_inner_ext_v4_auth, + mutable_inner_ext_v4_auth, + (O4V4, eth, ipv4, icmp4), + ((&Ipv4, &Ipv4Auth, &TruncatedTcp), ipv4, ipv4_auth, tcp) + ); + // Entering the region and stopping there: the gap check runs at the transport step, so with no + // transport named the remaining extensions are simply left unvisited. Telling this apart from + // the two above is the whole reason the check sits where it does. + embedded_agrees!( + read_inner_ext_no_transport, + mutable_inner_ext_no_transport, + (O4V6, eth, ipv6, icmp6), + ((&Ipv6, &HopByHop), ipv6, hop_by_hop) + ); + + // ---- Outer arities ------------------------------------------------------------------- + // + // `as_embedded` and `as_embedded_mut` are written out once per outer arity, eight times. Arities + // 1 and 2 cannot be instantiated: both require `EmbeddedHeaders: Within`, which holds + // only for `Icmp4` and `Icmp6`, and neither can appear that early. `Eth` is the only layer with + // `Within<()>`, so an arity-1 shape must be `(&Eth,)`; an arity-2 shape must then end in + // something `Within`, and no ICMP layer is. Asking for either is a compile error -- + // `Icmp4: Within<()> is not satisfied` and `Icmp4: Within is not satisfied` -- so those + // two macro arms are 44 lines that can never run and could be deleted. + // + // 3 through 8 is therefore all of them. The inner shape is held cheap on purpose here, so that a + // thin outer shape is the only improbable thing each property asks for. + embedded_agrees!( + read_outer_4, + mutable_outer_4, + ((&Eth, &Vlan, &Ipv4, &Icmp4), eth, vlan, ipv4, icmp4), + ((&Ipv4, &TruncatedTcp), ipv4, tcp) + ); + embedded_agrees!( + read_outer_5, + mutable_outer_5, + ( + (&Eth, &Vlan, &Vlan, &Ipv6, &Icmp6), + eth, + vlan, + vlan, + ipv6, + icmp6 + ), + ((&Ipv6, &TruncatedUdp), ipv6, udp) + ); + embedded_agrees!( + read_outer_6, + mutable_outer_6, + ( + (&Eth, &Vlan, &Vlan, &Vlan, &Ipv4, &Icmp4), + eth, + vlan, + vlan, + vlan, + ipv4, + icmp4 + ), + // An inner shape that can miss on a quote that *is* present, so the shape check's own + // rejection is reached rather than only the absent-quote early return above it. + ((&Ipv4, &TruncatedTcp), ipv4, tcp) + ); + embedded_agrees!( + read_outer_7, + mutable_outer_7, + ( + (&Eth, &Vlan, &Vlan, &Vlan, &Vlan, &Ipv4, &Icmp4), + eth, + vlan, + vlan, + vlan, + vlan, + ipv4, + icmp4 + ), + ((&Ipv4, &TruncatedTcp), ipv4, tcp) + ); + // Arity 8, and the only outer shape that also enters the *outer* extension region: `Icmp6` + // directly after a `HopByHop` makes the outer gap check strict too. + embedded_agrees!( + read_outer_8, + mutable_outer_8, + ( + (&Eth, &Vlan, &Vlan, &Vlan, &Vlan, &Ipv6, &HopByHop, &Icmp6), + eth, + vlan, + vlan, + vlan, + vlan, + ipv6, + hop_by_hop, + icmp6 + ), + // `Ipv6` rather than `Net`: a quote of the other family misses, which is the only way to + // reach this arity's shape-check rejection. + ((&Ipv6,), ipv6) + ); + + // The `EmbeddedTransport` enum has no read-side property above, and that is an asymmetry in + // `pat.rs` rather than a choice here: `EmbeddedMatcherMut::transport` exists but + // `EmbeddedMatcher::transport` does not, so the immutable oracle cannot name the enum even + // though every concrete variant and every other layer is paired between the two. The mutable + // side is checked below; adding the missing method would let the read side follow in one line. + #[test] + fn mutable_inner_transport_enum() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static SEEN: AtomicUsize = AtomicUsize::new(0); + static HIT: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + SEEN.fetch_add(1, Ordering::Relaxed); + let licensed = match owned.as_view_mut::() { + Some(w) => w.as_embedded_mut::<(&Net, &EmbeddedTransport)>().is_some(), + None => false, + }; + if licensed { + HIT.fetch_add(1, Ordering::Relaxed); + } + let deliverable = owned + .pat_mut() + .eth() + .ipv4() + .icmp4() + .embedded() + .net() + .transport() + .done() + .is_some(); + assert_eq!( + licensed, deliverable, + "`matches` and the mutable walk disagree for (&Net, &EmbeddedTransport): {h:?}" + ); + }); + agreement_is_not_vacuous( + "(&Net, &EmbeddedTransport)", + SEEN.load(Ordering::Relaxed), + HIT.load(Ordering::Relaxed), + ); + } + + // ---- Delivering, not merely deciding ------------------------------------------------- + + /// Write through every reference `look_mut` hands out, in the extension-region shape. + /// + /// The properties above compare *decisions*. This one takes the decision up on its offer: it + /// calls `look_mut`, which runs the `unreachable_unchecked` the decision licenses, and then + /// writes through all three references. The assertions are close to beside the point -- what + /// matters is that three `&mut` into one `EmbeddedHeaders` are created and written through, so + /// that miri with stacked borrows enabled can judge whether + /// [`EmbeddedFields`](super::super::pat::EmbeddedFields) handed out two paths to the same + /// layer. The arity-3 shape is the one that borrows from all three of its fields at once, + /// including a slice element, which is the hardest of them to split soundly. + fn exercise_the_embedded_split() { + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + let Some(outer) = owned.as_view_mut::() else { + return; + }; + let Some(ew) = outer.as_embedded_mut::<(&Ipv6, &HopByHop, &TruncatedTcp)>() else { + return; + }; + let (ip, ext, tcp) = ew.look_mut(); + + let want_hops = ip.hop_limit().wrapping_add(1); + ip.set_hop_limit(want_hops); + let seen_ext = ext.next_header(); + let seen_tcp = matches!(tcp, TruncatedTcp::FullHeader(_)); + + assert_eq!( + ip.hop_limit(), + want_hops, + "the write through ipv6 did not stick" + ); + assert_eq!( + ext.next_header(), + seen_ext, + "the extension header changed under a write to ipv6" + ); + assert_eq!( + matches!(tcp, TruncatedTcp::FullHeader(_)), + seen_tcp, + "the quoted transport changed under a write to ipv6" + ); + }); + } + + /// Shards of [`exercise_the_embedded_split`], so the machine can be used. + /// + /// Same reasoning as the shards in [`view`](super::super::view): under miri this is the + /// expensive property and the one that matters most, and its cost is wall-clock rather than + /// cores. Each shard seeds itself from the OS, so `N` shards explore `N` independent streams + /// and nextest runs them concurrently. Eight rather than sixteen -- `view`'s split is the wider + /// one and should keep the larger share of the cores. + macro_rules! split_shards { + ($($name:ident),* $(,)?) => { + $( + #[test] + fn $name() { + exercise_the_embedded_split(); + } + )* + }; + } + + split_shards!( + the_embedded_split_hands_out_distinct_layers, + embedded_split_shard_2, + embedded_split_shard_3, + embedded_split_shard_4, + embedded_split_shard_5, + embedded_split_shard_6, + embedded_split_shard_7, + embedded_split_shard_8, + ); + + /// `look` and `look_mut` pick the same slots, one inner shape per instantiation. + /// + /// Agreeing that a match exists is weaker than agreeing on *which* layers were selected, and + /// this is the only thing in the suite that asks the stronger question across the family. + /// + /// It is also the only thing that *calls* `look` and `look_mut` at more than a couple of shapes, + /// which matters more than it looks: the differential properties above go through + /// [`embedded_sealed::Sealed::matches`] and so exercise [`EmbeddedStep`], while `look_mut` is the + /// sole caller of [`EmbeddedStepMut`] -- a second, separate set of per-layer impls that delegate + /// into [`EmbeddedMatcherMut`]. Deciding a shape matches never runs a line of it. So there is one + /// instantiation per layer that has an `EmbeddedStepMut` impl, and the extension ones are the + /// point: one implementation indexes `net_ext` by a cursor, the other consumes a slice, and + /// nothing before this compared what they returned. + macro_rules! same_layers { + ($name:ident, $gen:expr, $outer:ty, $shape:ty, $($binding:ident),+) => { + #[test] + fn $name() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static SEEN: AtomicUsize = AtomicUsize::new(0); + static HIT: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator($gen) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + SEEN.fetch_add(1, Ordering::Relaxed); + // `h` and `&mut h` cannot be held at once, so the packet is pinned across + // both calls and the raw addresses compared afterwards. + let immutable = { + let Some(outer) = owned.as_view::<$outer>() else { + return; + }; + let Some(ew) = outer.as_embedded::<$shape>() else { + return; + }; + let ($($binding,)+) = ew.look(); + ($(std::ptr::from_ref($binding),)+) + }; + HIT.fetch_add(1, Ordering::Relaxed); + let mutable = { + let outer = owned.as_view_mut::<$outer>().unwrap_or_else(|| { + unreachable!("the same packet matched a moment ago") + }); + let ew = outer.as_embedded_mut::<$shape>().unwrap_or_else(|| { + unreachable!("the same shape matched a moment ago") + }); + let ($($binding,)+) = ew.look_mut(); + ($(std::ptr::from_ref(&*$binding),)+) + }; + assert_eq!( + immutable, mutable, + concat!( + "`look` and `look_mut` selected different layers of the quoted \ + packet for ", + stringify!($shape) + ) + ); + }); + agreement_is_not_vacuous( + concat!("look/look_mut ", stringify!($shape)), + SEEN.load(Ordering::Relaxed), + HIT.load(Ordering::Relaxed), + ); + } + }; + } + + /// The strict gap check applies to the transport *enum* too, not just concrete variants. + /// + /// Stated as a direct assertion rather than a differential, because the immutable oracle cannot + /// express it -- see the note above `mutable_inner_transport_enum`. That leaves the enum's own + /// gap check with nothing else reaching it, and it is worth reaching: naming an extension makes + /// the check strict, so a second, unnamed extension must turn a hit into a miss even though the + /// transport pattern is the permissive one. + /// + /// The second extension is a clone of the first, which keeps this free of any assumption about + /// what other extension types exist while still being two headers where the shape names one. + #[test] + fn the_transport_enum_still_refuses_an_unconsumed_extension() { + bolero::check!() + .with_generator(ShapedQuote { ext: 0, v4: false }) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + let quoted = owned + .embedded_ip_mut() + .unwrap_or_else(|| unreachable!("ShapedQuote always attaches a quote")); + let first = quoted + .net_ext + .first() + .unwrap_or_else(|| unreachable!("ShapedQuote always places one extension")) + .clone(); + quoted.net_ext.push(first); + + let outer = owned + .as_view::() + .unwrap_or_else(|| unreachable!("ShapedQuote draws an ICMPv6 error")); + assert!( + outer + .as_embedded::<(&Ipv6, &HopByHop, &EmbeddedTransport)>() + .is_none(), + "naming one extension of two matched anyway, so the gap check did not run for \ + the transport enum" + ); + // Not naming an extension skips the region silently, which is the other half of the + // same rule and shows the miss above is the gap check rather than a broken shape. + assert!( + outer.as_embedded::<(&Ipv6, &EmbeddedTransport)>().is_some(), + "a shape that never entered the extension region was refused" + ); + }); + } + + // Network layer, both concrete and as the enum, at inner arity 1. These shapes are common + // enough in the broad generator's output -- around one packet in twenty -- to use it. + same_layers!(same_layers_v4_only, ShapedIcmpError, O4V4, (&Ipv4,), ip); + same_layers!(same_layers_v6_only, ShapedIcmpError, O4V6, (&Ipv6,), ip); + same_layers!(same_layers_net_only, ShapedIcmpError, O4V4, (&Net,), net); + + // Every truncated transport, and the transport enum. + same_layers!( + same_layers_v4_tcp, + ShapedIcmpError, + O4V4, + (&Ipv4, &TruncatedTcp), + ip, + tcp + ); + same_layers!( + same_layers_v6_udp, + ShapedIcmpError, + O4V6, + (&Ipv6, &TruncatedUdp), + ip, + udp + ); + same_layers!( + same_layers_icmp_in_icmp4, + ShapedIcmpError, + O4V4, + (&Ipv4, &TruncatedIcmp4), + ip, + icmp + ); + same_layers!( + same_layers_icmp_in_icmp6, + ShapedIcmpError, + O4V6, + (&Ipv6, &TruncatedIcmp6), + ip, + icmp + ); + same_layers!( + same_layers_transport_enum, + ShapedIcmpError, + O4V4, + (&Net, &EmbeddedTransport), + net, + transport + ); + + // Every extension header, which is where the two walks diverge most: one indexes `net_ext` by a + // cursor, the other consumes a slice. These use the narrow generator, because on the broad one + // they matched six to twenty-three packets in twenty-four thousand -- see [`ShapedQuote`]. + // + // Each therefore sees a quoted packet with exactly one extension, and that is not a limitation + // of the generator but of what is expressible: embedded shapes stop at arity 3, so a shape can + // name at most one extension, and naming one makes the gap check strict -- a second, unconsumed + // extension is a miss. One is the only number of extensions a matching quote can carry. Cursor + // arithmetic past position zero is consequently unreachable from here, and is covered by the + // differential properties above, which do draw multi-extension quotes and compare the two walks + // on the misses. + same_layers!( + same_layers_ext_hop_by_hop, + ShapedQuote { ext: 0, v4: false }, + O4V6, + (&Ipv6, &HopByHop, &TruncatedTcp), + ip, + ext, + tcp + ); + same_layers!( + same_layers_ext_dest_opts, + ShapedQuote { ext: 1, v4: false }, + O4V6, + (&Ipv6, &DestOpts, &TruncatedTcp), + ip, + ext, + tcp + ); + same_layers!( + same_layers_ext_routing, + ShapedQuote { ext: 2, v4: false }, + O4V6, + (&Ipv6, &Routing, &TruncatedTcp), + ip, + ext, + tcp + ); + same_layers!( + same_layers_ext_fragment, + ShapedQuote { ext: 3, v4: false }, + O4V6, + (&Ipv6, &Fragment, &TruncatedTcp), + ip, + ext, + tcp + ); + same_layers!( + same_layers_ext_v6_auth, + ShapedQuote { ext: 4, v4: false }, + O4V6, + (&Ipv6, &Ipv6Auth, &TruncatedTcp), + ip, + ext, + tcp + ); + same_layers!( + same_layers_ext_v4_auth, + ShapedQuote { ext: 0, v4: true }, + O4V4, + (&Ipv4, &Ipv4Auth, &TruncatedTcp), + ip, + ext, + tcp + ); +} diff --git a/net/src/headers/mod.rs b/net/src/headers/mod.rs index 22fb5237ca..6514cfec17 100644 --- a/net/src/headers/mod.rs +++ b/net/src/headers/mod.rs @@ -1194,7 +1194,10 @@ where mod contract { use crate::eth::ethtype::CommonEthType; use crate::eth::{Eth, GenWithEthType}; - use crate::headers::{Headers, Net, Transport}; + use crate::headers::{ + EmbeddedHeaders, EmbeddedTransport, Headers, MAX_NET_EXTENSIONS, MAX_VLANS, Net, NetExt, + Transport, + }; use crate::icmp4::Icmp4; use crate::icmp6::Icmp6; use crate::ipv4; @@ -1205,6 +1208,7 @@ mod contract { use crate::vxlan::Vxlan; use arrayvec::ArrayVec; use bolero::{Driver, TypeGenerator, ValueGenerator}; + use std::ops::Bound; impl TypeGenerator for Headers { /// Generate a completely arbitrary value of [`Headers`]. @@ -1250,6 +1254,393 @@ mod contract { } } + /// Draws [`Headers`] whose **layer structure** varies: VLAN tags, IPv6 extension headers, and + /// every combination of the two. + /// + /// [`CommonHeaders`] deliberately does not. All six of its construction sites set + /// `vlan: ArrayVec::default()` and `net_ext: ArrayVec::default()`, so it never produces a VLAN tag + /// or an extension header at all -- reasonable for the "sunny-day" packet processing it was written + /// for, and useless for the code whose whole subject is those two things: + /// + /// * [`HeadersView`](crate::headers::view::HeadersView) exists to decide whether a packet's + /// structure matches a type-level shape, and its contract is stated in terms of VLAN tags that + /// are not mentioned in the shape and extension regions the shape may or may not enter; + /// * [`Matcher`](crate::headers::pat::Matcher) threads the same `ExtGapCheck`. + /// + /// Neither semantics could be reached by any existing generator, which is a better explanation of + /// `view.rs` sitting at 42% line coverage than "nobody wrote tests" -- the tests are there. + /// + /// This is structural on purpose. `ViewStep::step` walks the in-memory layers with VLAN and + /// extension cursors, so what matters here is which slots are populated and how many, not whether + /// `next_header` agrees with the layer that follows it. A generator that also kept the wire fields + /// consistent would be the right tool for a parse round-trip property, and the wrong one for this: + /// it would couple a structural test to a byte-level invariant and shrink the space it explores. + #[allow(dead_code)] // constructed through `.with_generator()` + #[repr(transparent)] + pub struct ShapedHeaders; + + impl ValueGenerator for ShapedHeaders { + type Output = Headers; + + fn generate(&self, driver: &mut D) -> Option { + // Start from a common stack so the base layers are realistic, then vary the structure the + // view and matcher semantics actually turn on. + let mut headers = CommonHeaders.generate(driver)?; + + let vlans = driver.gen_usize(Bound::Included(&0), Bound::Included(&MAX_VLANS))?; + for _ in 0..vlans { + headers.vlan.push(driver.produce()?); + } + + // Extension headers hang off the net layer, so there is nothing to attach them to without + // one. + if headers.net.is_some() { + headers.net_ext = ext_run(driver, matches!(headers.net, Some(Net::Ipv4(_))))?; + } + + Some(headers) + } + } + + /// Draw a run of 0..=[`MAX_NET_EXTENSIONS`] extension headers for the given family. + /// + /// The IPv4 authentication header is the only extension that belongs on a v4 packet, so `v4` + /// selects between one choice and five rather than merely reweighting them. + /// + /// A quarter of the v6 runs follow RFC 8200's recommended order instead of drawing each slot + /// independently. Without that bias a shape naming three specific extensions in sequence is + /// drawn about once in fifteen thousand packets, which is enough to keep a property honest and + /// nowhere near enough for it to find anything: measured at two hits in 36,000 cases. The + /// ordered runs are also the ones a real peer sends, so this makes the generator both more + /// useful and more realistic. + fn ext_run( + driver: &mut D, + v4: bool, + ) -> Option> { + let mut out = ArrayVec::default(); + let count = driver.gen_usize(Bound::Included(&0), Bound::Included(&MAX_NET_EXTENSIONS))?; + let ordered = driver.gen_u8(Bound::Included(&0), Bound::Included(&3))? == 0; + for slot in 0..count { + let pick = if ordered { + u8::try_from(slot).unwrap_or(u8::MAX) + } else { + driver.gen_u8(Bound::Included(&0), Bound::Included(&4))? + }; + out.push(one_ext(driver, v4, pick)?); + } + Some(out) + } + + /// Draws a packet with layers deliberately missing, which is what the optional matchers are for. + /// + /// [`CommonHeaders`] sets every layer it knows about on every path, so `net` and `transport` are + /// always `Some` in anything it or [`ShapedHeaders`] produces. That makes the absent-layer arm of + /// every `opt_*` method in [`pat`](crate::headers::pat) -- `None => Some(a.append(None))`, the + /// arm that distinguishes "the layer is not there, which is fine" from "the layer is there and is + /// the wrong one, which is a miss" -- unreachable by any generator in the crate. The whole point + /// of an optional matcher is the packet that stops early, and nothing could draw one. + /// + /// Truncation here is by *suffix*, because that is the only shape a real short packet takes: a + /// layer sits inside the one below it, so a packet cannot carry a transport header without a + /// network header to carry it. Dropping a suffix also keeps `net_ext` consistent, since + /// extensions hang off the network layer and have to go when it does. + #[allow(dead_code)] // constructed through `.with_generator()` + #[repr(transparent)] + pub struct ThinHeaders; + + impl ValueGenerator for ThinHeaders { + type Output = Headers; + + fn generate(&self, driver: &mut D) -> Option { + let mut headers = ShapedHeaders.generate(driver)?; + // Keep the full stack a fifth of the time, so a property reading this generator still + // sees the ordinary case alongside the truncated ones. + match driver.gen_u8(Bound::Included(&0), Bound::Included(&4))? { + 0 => {} + 1 => headers.udp_encap = None, + 2 => { + headers.udp_encap = None; + headers.transport = None; + } + 3 => { + headers.udp_encap = None; + headers.transport = None; + headers.net_ext.clear(); + headers.net = None; + } + _ => { + headers.udp_encap = None; + headers.transport = None; + headers.net_ext.clear(); + headers.net = None; + headers.vlan.clear(); + headers.eth = None; + } + } + Some(headers) + } + } + + /// [`ShapedHeaders`] with the Ethernet header taken away one packet in eight. + /// + /// Every `Shape` starts at `Eth`, and the branch where that first step refuses is generated + /// separately at each of the eight arities -- eight copies of `return false`, none of which any + /// generator could reach, because `CommonHeaders` sets `eth` on all six of its paths. + /// + /// [`ThinHeaders`] reaches them, but it truncates four packets in five, and the deep shapes pay + /// for that: arity seven already needs exactly four VLAN tags, so another factor of five would + /// leave it matching too rarely to find anything. One in eight covers the branch at every arity + /// while leaving seven eighths of the draw doing the work it was doing before. + #[allow(dead_code)] // constructed through `.with_generator()` + #[repr(transparent)] + pub struct SometimesHeadless; + + impl ValueGenerator for SometimesHeadless { + type Output = Headers; + + fn generate(&self, driver: &mut D) -> Option { + let mut headers = ShapedHeaders.generate(driver)?; + if driver.gen_u8(Bound::Included(&0), Bound::Included(&7))? == 0 { + // The tags go with it. A VLAN tag is carried by the Ethernet header, so a packet + // holding one without the other is not a short packet, it is an impossible one, and + // the properties reading this generator are about short packets. + headers.vlan.clear(); + headers.eth = None; + } + Some(headers) + } + } + + /// Draw one extension header, with fuzzed contents. + /// + /// `pick` indexes the IPv6 extension order RFC 8200 recommends -- 0 hop-by-hop, 1 destination + /// options, 2 routing, 3 fragment, anything else authentication -- and is ignored for `v4`, + /// where the IPv4 authentication header is the only extension there is. + fn one_ext(driver: &mut D, v4: bool, pick: u8) -> Option { + if v4 { + return Some(NetExt::Ipv4Auth(driver.produce()?)); + } + Some(match pick { + 0 => NetExt::HopByHop(driver.produce()?), + 1 => NetExt::DestOpts(driver.produce()?), + 2 => NetExt::Routing(driver.produce()?), + 3 => NetExt::Fragment(driver.produce()?), + _ => NetExt::Ipv6Auth(driver.produce()?), + }) + } + + /// Draws an ICMP error whose quoted packet carries exactly one chosen extension header, and + /// nothing else in the way. + /// + /// Companion to [`ShapedIcmpError`], and the division between them is deliberate. A property + /// comparing *hit against miss* needs both, so it wants the broad generator. A property that + /// only inspects matches -- does `look` pick the same layer `look_mut` does -- gets nothing from + /// a packet it skips, so for that one a narrow generator is not a weaker test but a stronger + /// one. + /// + /// The difference is not marginal. Naming a specific extension inside a quoted packet compounds + /// six independent conditions, one of which is `P(no VLAN tags) = 1/5`, since a tag the outer + /// shape does not name is a miss. Measured on [`ShapedIcmpError`], + /// `(&Ipv6, &DestOpts, &TruncatedTcp)` matched 6 packets in 23,910: a property doing real work + /// six times a second, and a hit rate low enough that asserting it is ever non-zero is itself a + /// coin flip. This generator produces that shape every time, and fuzzes the contents instead. + #[allow(dead_code)] // constructed through `.with_generator()` + pub struct ShapedQuote { + /// Which extension header to place, as the `pick` index [`one_ext`] uses. + pub ext: u8, + /// Family of both the quoting message and the packet it quotes. + pub v4: bool, + } + + impl ValueGenerator for ShapedQuote { + type Output = Headers; + + fn generate(&self, driver: &mut D) -> Option { + let eth_type = if self.v4 { + CommonEthType::Ipv4 + } else { + CommonEthType::Ipv6 + }; + let eth = GenWithEthType(eth_type.into()).generate(driver)?; + + let mut quoted_ext = ArrayVec::default(); + quoted_ext.push(one_ext(driver, self.v4, self.ext)?); + let quoted_transport = EmbeddedTransport::Tcp(driver.produce()?); + + let (net, transport, quoted_net) = if self.v4 { + ( + Net::Ipv4( + ipv4::GenWithNextHeader(ipv4::CommonNextHeader::Icmp4.into()) + .generate(driver)?, + ), + Transport::Icmp4(driver.produce()?), + Net::Ipv4( + ipv4::GenWithNextHeader(ipv4::CommonNextHeader::Tcp.into()) + .generate(driver)?, + ), + ) + } else { + ( + Net::Ipv6( + ipv6::GenWithNextHeader(ipv6::CommonNextHeader::Icmp6.into()) + .generate(driver)?, + ), + Transport::Icmp6(driver.produce()?), + Net::Ipv6( + ipv6::GenWithNextHeader(ipv6::CommonNextHeader::Tcp.into()) + .generate(driver)?, + ), + ) + }; + + Some(Headers { + eth: Some(eth), + // No VLAN tags and no outer extensions: a tag the outer shape does not name is a + // miss, and this generator exists to stop producing misses. + vlan: ArrayVec::default(), + net: Some(net), + net_ext: ArrayVec::default(), + transport: Some(transport), + udp_encap: None, + embedded_ip: Some(EmbeddedHeaders::new( + Some(quoted_net), + Some(quoted_transport), + quoted_ext, + None, + )), + }) + } + } + + /// Draws an ICMP error message quoting an inner packet, with the layer structure of *both* the + /// outer message and the quoted packet varying. + /// + /// [`CommonHeaders`] cannot produce one at all: all six of its construction sites set + /// `embedded_ip: None`, so [`ShapedHeaders`], which builds on it, cannot either. That is a + /// better explanation of [`embedded_view`](crate::headers::embedded_view) sitting at 34% line + /// coverage than any claim about missing tests -- its hand-written tests are thorough, but every + /// one of them builds its packet through `HeaderStack`/`header_chain`, which pin the shape at + /// `(Eth, Ipv4, Icmp4)` outside and `(Ipv4, Tcp)` inside and offer no way to attach an extension + /// header to either side. Six of the eight `as_embedded` arities and the whole embedded + /// extension region were unreachable by construction. + /// + /// What varies here is what `EmbeddedShape`, `EmbeddedStep` and + /// [`ExtGapCheck`](crate::headers::pat::ExtGapCheck) actually turn on: + /// + /// * outer VLAN tags and outer extension headers, so the outer arity spans the range of + /// `as_embedded` impls instead of only the one a builder can express; + /// * whether the embedded section is present at all -- an ICMP message that is not an error + /// quotes nothing, and every embedded shape must miss on it; + /// * the inner network layer, its extension headers, and its truncated transport, including a + /// quote that stops before the transport header and the ICMP-inside-ICMP case. + /// + /// The inner family follows the outer family most of the time, because that is what a real ICMP + /// error looks like: an `ICMPv4` message quotes an IPv4 packet. It deliberately does not always. + /// A family mismatch is where two independent structural walks are most likely to disagree, so + /// it wants drawing rather than assuming away. + /// + /// Structural on purpose, for the same reason as [`ShapedHeaders`]: `next_header` is set to + /// match the transport where one was drawn, but nothing keeps the extension chain's own + /// `next_header` fields consistent, because no code under test reads them. + #[allow(dead_code)] // constructed through `.with_generator()` + #[repr(transparent)] + pub struct ShapedIcmpError; + + impl ValueGenerator for ShapedIcmpError { + type Output = Headers; + + fn generate(&self, driver: &mut D) -> Option { + let outer_v4 = driver.produce::()?; + let eth_type = if outer_v4 { + CommonEthType::Ipv4 + } else { + CommonEthType::Ipv6 + }; + let eth = GenWithEthType(eth_type.into()).generate(driver)?; + + let mut vlan = ArrayVec::default(); + let vlans = driver.gen_usize(Bound::Included(&0), Bound::Included(&MAX_VLANS))?; + for _ in 0..vlans { + vlan.push(driver.produce()?); + } + + let (net, transport) = if outer_v4 { + let ip = ipv4::GenWithNextHeader(ipv4::CommonNextHeader::Icmp4.into()) + .generate(driver)?; + (Net::Ipv4(ip), Transport::Icmp4(driver.produce()?)) + } else { + let ip = ipv6::GenWithNextHeader(ipv6::CommonNextHeader::Icmp6.into()) + .generate(driver)?; + (Net::Ipv6(ip), Transport::Icmp6(driver.produce()?)) + }; + + // Echo requests and replies are not errors and quote nothing, so absence is a shape the + // API has to handle, not an edge case to skip. + let embedded_ip = if driver.produce::()? { + Some(quoted_packet(driver, outer_v4)?) + } else { + None + }; + + Some(Headers { + eth: Some(eth), + vlan, + net: Some(net), + net_ext: ext_run(driver, outer_v4)?, + transport: Some(transport), + udp_encap: None, + embedded_ip, + }) + } + } + + /// Draw the packet quoted inside an ICMP error. `outer_v4` is the family of the quoting + /// message; see [`ShapedIcmpError`] for why the quoted packet usually but not always shares it. + fn quoted_packet(driver: &mut D, outer_v4: bool) -> Option { + // A quoting host copies as much of the offending packet as it can, and RFC 792 asked for + // only the header plus eight bytes. A quote can therefore be too short to hold even the + // network header -- which is the one case where the optional embedded matchers' absent-layer + // arm is reachable, since a quote that *has* a network layer always has a version. + if driver.gen_u8(Bound::Included(&0), Bound::Included(&7))? == 0 { + return Some(EmbeddedHeaders::new(None, None, ArrayVec::default(), None)); + } + let mismatch = driver.gen_u8(Bound::Included(&0), Bound::Included(&7))? == 0; + let v4 = outer_v4 != mismatch; + + // Pick the transport first so the network layer's `next_header` can name it. A quote that + // stops before the transport header is the truncation the `Truncated*` types exist for. + let transport = match driver.gen_u8(Bound::Included(&0), Bound::Included(&3))? { + 0 => Some(EmbeddedTransport::Tcp(driver.produce()?)), + 1 => Some(EmbeddedTransport::Udp(driver.produce()?)), + 2 if v4 => Some(EmbeddedTransport::Icmp4(driver.produce()?)), + 2 => Some(EmbeddedTransport::Icmp6(driver.produce()?)), + _ => None, + }; + + let net = if v4 { + let next = match transport { + Some(EmbeddedTransport::Udp(_)) => ipv4::CommonNextHeader::Udp, + Some(EmbeddedTransport::Icmp4(_)) => ipv4::CommonNextHeader::Icmp4, + _ => ipv4::CommonNextHeader::Tcp, + }; + Net::Ipv4(ipv4::GenWithNextHeader(next.into()).generate(driver)?) + } else { + let next = match transport { + Some(EmbeddedTransport::Udp(_)) => ipv6::CommonNextHeader::Udp, + Some(EmbeddedTransport::Icmp6(_)) => ipv6::CommonNextHeader::Icmp6, + _ => ipv6::CommonNextHeader::Tcp, + }; + Net::Ipv6(ipv6::GenWithNextHeader(next.into()).generate(driver)?) + }; + + Some(EmbeddedHeaders::new( + Some(net), + transport, + ext_run(driver, v4)?, + None, + )) + } + #[allow(dead_code)] // rustc not able to infer we construct this through .with_generator() #[repr(transparent)] pub struct CommonHeaders; @@ -1390,7 +1781,7 @@ mod test { use crate::tcp::{TcpChecksum, TcpChecksumPayload, TcpPort}; use crate::udp::{UdpChecksum, UdpChecksumPayload, UdpPort}; - fn parse_back_test(headers: &Headers) { + pub(crate) fn parse_back_test(headers: &Headers) { let mut buffer = [0_u8; 1024]; let bytes_written = match headers.deparse(&mut buffer[..headers.size().into_non_zero_usize().get()]) { diff --git a/net/src/headers/pat.rs b/net/src/headers/pat.rs index a5f640119d..d38a1f0436 100644 --- a/net/src/headers/pat.rs +++ b/net/src/headers/pat.rs @@ -2955,3 +2955,742 @@ mod tests { }; } } + +// =========================================================================== +// Optional-layer and combinator properties +// =========================================================================== + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] // fine to unwrap in tests +mod opt_properties { + use super::*; + use crate::headers::{Headers, ShapedIcmpError, ThinHeaders}; + use std::cell::Cell; + + /// `opt_X` is never stricter than `X`, for every layer and on both the read and mutable paths. + /// + /// One invariant, universally true, across three families whose internals differ sharply. + /// `opt_eth` cannot miss at all. `opt_vlan` and the optional extension methods cannot miss + /// either, but advance their cursor only when they matched, so they skip rather than refuse. + /// `opt_net`, the optional transport methods and `opt_vxlan` are three-way: present and right is + /// a hit, absent is a hit carrying `None`, present and wrong is a miss. Whatever the family, + /// weakening a requirement cannot turn a match into a miss -- and the direction is the thing a + /// mis-wiring would invert, since `and_then` and `map` differ by exactly that. + /// + /// Every one of these methods was uncovered before this: 250 of `pat.rs`'s 296 unreached lines + /// were the `opt_*` family and the combinators below. + macro_rules! opt_is_weaker { + ($read:ident, $mutable:ident, [$($pre:ident),*], $strict:ident, $opt:ident) => { + #[test] + fn $read() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static STRICT: AtomicUsize = AtomicUsize::new(0); + static OPT_ONLY: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ThinHeaders) + .for_each(|h: &Headers| { + let strict = h.pat()$(.$pre())*.$strict().done().is_some(); + let opt = h.pat()$(.$pre())*.$opt().done().is_some(); + assert!( + !strict || opt, + concat!( + "`", stringify!($strict), "` matched where `", stringify!($opt), + "` did not, so the optional form is the stricter one: {:?}" + ), + h + ); + if strict { + STRICT.fetch_add(1, Ordering::Relaxed); + } else if opt { + OPT_ONLY.fetch_add(1, Ordering::Relaxed); + } + }); + both_outcomes_seen( + concat!(stringify!($opt), " (read)"), + STRICT.load(Ordering::Relaxed), + OPT_ONLY.load(Ordering::Relaxed), + ); + } + + #[test] + fn $mutable() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static STRICT: AtomicUsize = AtomicUsize::new(0); + static OPT_ONLY: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ThinHeaders) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + let strict = owned.pat_mut()$(.$pre())*.$strict().done().is_some(); + let opt = owned.pat_mut()$(.$pre())*.$opt().done().is_some(); + assert!( + !strict || opt, + concat!( + "`", stringify!($strict), "` matched where `", stringify!($opt), + "` did not on the mutable path: {:?}" + ), + h + ); + if strict { + STRICT.fetch_add(1, Ordering::Relaxed); + } else if opt { + OPT_ONLY.fetch_add(1, Ordering::Relaxed); + } + }); + both_outcomes_seen( + concat!(stringify!($opt), " (mut)"), + STRICT.load(Ordering::Relaxed), + OPT_ONLY.load(Ordering::Relaxed), + ); + } + }; + } + + /// An implication is satisfied for free when its antecedent never holds, and again when the two + /// sides never differ. + /// + /// `strict implies opt` would pass on a generator that produced nothing matching -- and it would + /// pass just as quietly on one where `opt` never accepted anything `strict` refused, which is the + /// more likely failure and the one that would make the optional method's whole reason for + /// existing untested. So both counts have to be non-zero: the requirement is sometimes met, and + /// relaxing it sometimes matters. + fn both_outcomes_seen(what: &str, strict: usize, opt_only: usize) { + println!("{what}: {strict} strict matches, {opt_only} matched only optionally"); + assert!( + strict > 0, + "{what}: the strict form never matched, so the implication held vacuously" + ); + assert!( + opt_only > 0, + "{what}: the optional form never accepted anything the strict form refused, so being \ + optional was never tested" + ); + } + + opt_is_weaker!(read_opt_eth, mut_opt_eth, [], eth, opt_eth); + opt_is_weaker!(read_opt_vlan, mut_opt_vlan, [eth], vlan, opt_vlan); + opt_is_weaker!(read_opt_net, mut_opt_net, [eth], net, opt_net); + opt_is_weaker!(read_opt_ipv4, mut_opt_ipv4, [eth], ipv4, opt_ipv4); + opt_is_weaker!(read_opt_ipv6, mut_opt_ipv6, [eth], ipv6, opt_ipv6); + opt_is_weaker!( + read_opt_hop_by_hop, + mut_opt_hop_by_hop, + [eth, ipv6], + hop_by_hop, + opt_hop_by_hop + ); + opt_is_weaker!( + read_opt_dest_opts, + mut_opt_dest_opts, + [eth, ipv6], + dest_opts, + opt_dest_opts + ); + opt_is_weaker!( + read_opt_routing, + mut_opt_routing, + [eth, ipv6], + routing, + opt_routing + ); + opt_is_weaker!( + read_opt_fragment, + mut_opt_fragment, + [eth, ipv6], + fragment, + opt_fragment + ); + opt_is_weaker!( + read_opt_ipv6_auth, + mut_opt_ipv6_auth, + [eth, ipv6], + ipv6_auth, + opt_ipv6_auth + ); + opt_is_weaker!( + read_opt_ipv4_auth, + mut_opt_ipv4_auth, + [eth, ipv4], + ipv4_auth, + opt_ipv4_auth + ); + opt_is_weaker!(read_opt_tcp, mut_opt_tcp, [eth, net], tcp, opt_tcp); + opt_is_weaker!(read_opt_udp, mut_opt_udp, [eth, net], udp, opt_udp); + opt_is_weaker!(read_opt_icmp4, mut_opt_icmp4, [eth, ipv4], icmp4, opt_icmp4); + opt_is_weaker!(read_opt_icmp6, mut_opt_icmp6, [eth, ipv6], icmp6, opt_icmp6); + opt_is_weaker!( + read_opt_transport, + mut_opt_transport, + [eth, net], + transport, + opt_transport + ); + opt_is_weaker!( + read_opt_vxlan, + mut_opt_vxlan, + [eth, net, udp], + vxlan, + opt_vxlan + ); + + /// The same invariant for the matchers over a quoted ICMP-error payload. + /// + /// Separate macro only because the chain has to pass through `.embedded()` partway, which is not + /// a layer name. The embedded families mirror the outer ones and are generated by their own set + /// of macros, so every arm needs reaching on its own -- covering `opt_hop_by_hop` says nothing + /// about the five sibling copies `embedded_ext!` emits. + macro_rules! embedded_opt_is_weaker { + ( + $read:ident, $mutable:ident, + [$($o:ident),*], [$($i:ident),*], + $strict:ident, $opt:ident + ) => { + #[test] + fn $read() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static STRICT: AtomicUsize = AtomicUsize::new(0); + static OPT_ONLY: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + let strict = h.pat()$(.$o())*.embedded()$(.$i())*.$strict() + .done().is_some(); + let opt = h.pat()$(.$o())*.embedded()$(.$i())*.$opt() + .done().is_some(); + assert!( + !strict || opt, + concat!( + "quoted `", stringify!($strict), "` matched where `", + stringify!($opt), "` did not: {:?}" + ), + h + ); + if strict { + STRICT.fetch_add(1, Ordering::Relaxed); + } else if opt { + OPT_ONLY.fetch_add(1, Ordering::Relaxed); + } + }); + both_outcomes_seen( + concat!("quoted ", stringify!($opt), " (read)"), + STRICT.load(Ordering::Relaxed), + OPT_ONLY.load(Ordering::Relaxed), + ); + } + + #[test] + fn $mutable() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static STRICT: AtomicUsize = AtomicUsize::new(0); + static OPT_ONLY: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + let strict = owned.pat_mut()$(.$o())*.embedded()$(.$i())*.$strict() + .done().is_some(); + let opt = owned.pat_mut()$(.$o())*.embedded()$(.$i())*.$opt() + .done().is_some(); + assert!( + !strict || opt, + concat!( + "quoted `", stringify!($strict), "` matched where `", + stringify!($opt), "` did not on the mutable path: {:?}" + ), + h + ); + if strict { + STRICT.fetch_add(1, Ordering::Relaxed); + } else if opt { + OPT_ONLY.fetch_add(1, Ordering::Relaxed); + } + }); + both_outcomes_seen( + concat!("quoted ", stringify!($opt), " (mut)"), + STRICT.load(Ordering::Relaxed), + OPT_ONLY.load(Ordering::Relaxed), + ); + } + }; + } + + embedded_opt_is_weaker!( + read_quoted_opt_ipv4, + mut_quoted_opt_ipv4, + [eth, ipv4, icmp4], + [], + ipv4, + opt_ipv4 + ); + embedded_opt_is_weaker!( + read_quoted_opt_ipv6, + mut_quoted_opt_ipv6, + [eth, ipv6, icmp6], + [], + ipv6, + opt_ipv6 + ); + // No `opt_net` or `opt_transport` pair here, and no `transport` on the read side, because the + // embedded matchers do not have them. The enum-level vocabulary is complete on the outer + // matchers and mostly absent on the embedded ones: + // + // | | `net` | `opt_net` | `transport` | `opt_transport` | + // |----------------------|-------|-----------|-------------|-----------------| + // | `Matcher` | yes | yes | yes | yes | + // | `MatcherMut` | yes | yes | yes | yes | + // | `EmbeddedMatcher` | yes | no | no | no | + // | `EmbeddedMatcherMut` | yes | no | yes | no | + // + // Five methods missing, all hand-written rather than macro-generated, which is the likely reason + // -- the per-variant methods come from `embedded_net!` and `embedded_transport!` and are all + // present. The consequence is not cosmetic: a shape naming `Net` or `EmbeddedTransport` inside a + // quoted packet cannot be written as a matcher chain, so `embedded_view`'s differential + // properties have no read-side oracle for the enum forms either. + embedded_opt_is_weaker!( + read_quoted_opt_hop_by_hop, + mut_quoted_opt_hop_by_hop, + [eth, ipv6, icmp6], + [ipv6], + hop_by_hop, + opt_hop_by_hop + ); + embedded_opt_is_weaker!( + read_quoted_opt_dest_opts, + mut_quoted_opt_dest_opts, + [eth, ipv6, icmp6], + [ipv6], + dest_opts, + opt_dest_opts + ); + embedded_opt_is_weaker!( + read_quoted_opt_routing, + mut_quoted_opt_routing, + [eth, ipv6, icmp6], + [ipv6], + routing, + opt_routing + ); + embedded_opt_is_weaker!( + read_quoted_opt_fragment, + mut_quoted_opt_fragment, + [eth, ipv6, icmp6], + [ipv6], + fragment, + opt_fragment + ); + embedded_opt_is_weaker!( + read_quoted_opt_ipv6_auth, + mut_quoted_opt_ipv6_auth, + [eth, ipv6, icmp6], + [ipv6], + ipv6_auth, + opt_ipv6_auth + ); + embedded_opt_is_weaker!( + read_quoted_opt_ipv4_auth, + mut_quoted_opt_ipv4_auth, + [eth, ipv4, icmp4], + [ipv4], + ipv4_auth, + opt_ipv4_auth + ); + embedded_opt_is_weaker!( + read_quoted_opt_tcp, + mut_quoted_opt_tcp, + [eth, ipv4, icmp4], + [ipv4], + tcp, + opt_tcp + ); + embedded_opt_is_weaker!( + read_quoted_opt_udp, + mut_quoted_opt_udp, + [eth, ipv6, icmp6], + [ipv6], + udp, + opt_udp + ); + embedded_opt_is_weaker!( + read_quoted_opt_icmp4, + mut_quoted_opt_icmp4, + [eth, ipv4, icmp4], + [ipv4], + icmp4, + opt_icmp4 + ); + embedded_opt_is_weaker!( + read_quoted_opt_icmp6, + mut_quoted_opt_icmp6, + [eth, ipv6, icmp6], + [ipv6], + icmp6, + opt_icmp6 + ); + + // ---- Exact semantics, per family ------------------------------------------------------ + + /// The three-way families accept a matching layer or an absent one, and refuse a wrong one. + /// + /// `opt_is_weaker` above only pins the direction. This states the whole rule, and the arm it + /// exists for is the middle one: a packet that stops before the layer is a *hit* carrying `None`, + /// while a packet carrying the wrong layer is a miss. Conflating those two is the mistake this + /// family invites, and no generator could produce the first case until `ThinHeaders`. + #[test] + fn the_three_way_families_separate_an_absent_layer_from_a_wrong_one() { + bolero::check!() + .with_generator(ThinHeaders) + .for_each(|h: &Headers| { + // Reachability, spelled out once: a strict `.eth()` needs an Ethernet header, and + // the network step's gap check needs every VLAN tag consumed -- none were named. + let reached_net = h.eth().is_some() && h.vlan().is_empty(); + + assert_eq!( + h.pat().eth().opt_net().done().is_some(), + reached_net, + "the network enum is never the wrong variant, so absent or present must both \ + match: {h:?}" + ); + assert_eq!( + h.pat().eth().opt_ipv4().done().is_some(), + reached_net && !matches!(h.net(), Some(Net::Ipv6(_))), + "opt_ipv4 must accept IPv4 and absence, and refuse IPv6: {h:?}" + ); + assert_eq!( + h.pat().eth().opt_ipv6().done().is_some(), + reached_net && !matches!(h.net(), Some(Net::Ipv4(_))), + "opt_ipv6 must accept IPv6 and absence, and refuse IPv4: {h:?}" + ); + + let reached_transport = reached_net && h.net().is_some(); + assert_eq!( + h.pat().eth().net().opt_transport().done().is_some(), + reached_transport, + "the transport enum is never the wrong variant either: {h:?}" + ); + assert_eq!( + h.pat().eth().net().opt_tcp().done().is_some(), + reached_transport && matches!(h.transport(), None | Some(Transport::Tcp(_))), + "opt_tcp must accept TCP and absence, and refuse every other transport: {h:?}" + ); + + // `opt_vxlan` sits behind a concrete `.udp()`, so its own absent arm is the packet + // that carries UDP and no encapsulation -- much the commoner case in real traffic. + let reached_vxlan = + reached_transport && matches!(h.transport(), Some(Transport::Udp(_))); + assert_eq!( + h.pat().eth().net().udp().opt_vxlan().done().is_some(), + reached_vxlan, + "a UDP packet with no encapsulation must match opt_vxlan: {h:?}" + ); + }); + } + + /// The cursor families skip rather than refuse, and advance only on a hit. + /// + /// This is the sharpest observable difference between the two designs, and it is entirely about + /// the cursor. `opt_vlan` cannot miss, so a chain of them followed by a *strict* network step + /// succeeds exactly when the packet has no more tags than the chain has optional slots -- the + /// strict step's gap check is what makes the cursor's behaviour visible from outside. + #[test] + fn optional_vlans_absorb_one_tag_each_and_only_when_they_match() { + bolero::check!() + .with_generator(ThinHeaders) + .for_each(|h: &Headers| { + let reachable = h.eth().is_some() && h.net().is_some(); + let tags = h.vlan().len(); + assert_eq!( + h.pat().eth().opt_vlan().net().done().is_some(), + reachable && tags <= 1, + "one optional tag absorbed the wrong number of tags: {h:?}" + ); + assert_eq!( + h.pat().eth().opt_vlan().opt_vlan().net().done().is_some(), + reachable && tags <= 2, + "two optional tags absorbed the wrong number of tags: {h:?}" + ); + // `MAX_VLANS` is four, so four optional slots absorb any packet there can be. If + // `opt_vlan` advanced its cursor on a miss this would fail on the untagged packets. + assert_eq!( + h.pat() + .eth() + .opt_vlan() + .opt_vlan() + .opt_vlan() + .opt_vlan() + .net() + .done() + .is_some(), + reachable, + "four optional tags failed to absorb a packet with at most four: {h:?}" + ); + }); + } + + /// Naming an extension optionally still enters the region, which makes the gap check strict. + /// + /// The subtlety worth a test: `opt_hop_by_hop` cannot itself fail, so it looks harmless, but it + /// moves `Pos` to `HopByHop` and that is what `ExtGapCheck` dispatches on. The transport step + /// afterwards therefore demands every extension be consumed -- so an *optional* extension can + /// turn a later, unrelated step into a miss. + #[test] + fn an_optional_extension_still_makes_the_transport_gap_check_strict() { + bolero::check!() + .with_generator(ThinHeaders) + .for_each(|h: &Headers| { + let consumed = + usize::from(matches!(h.net_ext().first(), Some(NetExt::HopByHop(_)))); + let want = h.eth().is_some() + && h.vlan().is_empty() + && matches!(h.net(), Some(Net::Ipv6(_))) + && h.transport().is_some() + && h.net_ext().len() == consumed; + assert_eq!( + h.pat() + .eth() + .ipv6() + .opt_hop_by_hop() + .transport() + .done() + .is_some(), + want, + "an optional extension left the transport gap check lenient: {h:?}" + ); + }); + } + + // ---- Combinators ---------------------------------------------------------------------- + + /// `when`, `inspect` and `otherwise` on all four matchers. + /// + /// Three small methods repeated four times, and all twelve copies were unreached. Two of them + /// are side-effecting, which makes "did it run" the entire contract rather than a detail: + /// `inspect` must run exactly on a match and `otherwise` exactly on a miss, and neither may + /// change the outcome. `when` must be able to destroy a match and must never manufacture one -- + /// on a chain that already failed, a `true` predicate has nothing to revive. + macro_rules! combinators_fire_exactly_once_and_only_when_due { + ($name:ident, $gen:expr, $subject:expr, $fires:expr, $($chain:tt)*) => { + // The `mut` binding is what `pat_mut()` needs and what `pat()` does not, and the same + // macro serves both, so half the instantiations declare a `mut` they never use. + #[allow(unused_mut)] + #[test] + fn $name() { + bolero::check!() + .with_generator($gen) + .for_each(|h: &Headers| { + // The chain is a token sequence rather than a closure because a closure + // returning a matcher borrowed from its own argument needs a higher-ranked + // lifetime, which closure inference will not produce. + let mut owned = h.clone(); + let base = owned $($chain)* .done().is_some(); + // What the combinators actually track, which is not always `base`: see + // `the_embedded_combinators_track_the_inner_match_only` below. + let fires: bool = $fires(h); + + let mut owned = h.clone(); + assert!( + owned $($chain)* .when(|_| false).done().is_none(), + concat!($subject, ": a false predicate left the match standing: {:?}"), + h + ); + let mut owned = h.clone(); + assert_eq!( + owned $($chain)* .when(|_| true).done().is_some(), + base, + concat!($subject, ": a true predicate was not a no-op: {:?}"), + h + ); + + let ran = Cell::new(false); + let mut owned = h.clone(); + let after = owned $($chain)* + .inspect(|_| ran.set(true)) + .done() + .is_some(); + assert_eq!( + ran.get(), fires, + concat!($subject, ": inspect ran on a miss or skipped a match: {:?}"), + h + ); + assert_eq!( + after, base, + concat!($subject, ": inspect changed the result: {:?}"), + h + ); + + let ran = Cell::new(false); + let mut owned = h.clone(); + let after = owned $($chain)* + .otherwise(|| ran.set(true)) + .done() + .is_some(); + assert_eq!( + ran.get(), !fires, + concat!($subject, ": otherwise ran on a match or skipped a miss: {:?}"), + h + ); + assert_eq!( + after, base, + concat!($subject, ": otherwise changed the result: {:?}"), + h + ); + }); + } + }; + } + + /// The outer matchers carry one accumulator, so their combinators fire exactly on the result. + fn whole_chain(h: &Headers) -> bool { + h.pat().eth().net().done().is_some() + } + + /// The embedded matchers carry two, and their combinators watch only the inner one. + /// + /// `.embedded().ipv4()` leaves the inner accumulator populated exactly when a quote is present + /// and its network layer is IPv4 -- a condition that says nothing about whether the outer chain + /// that led there succeeded. + fn quoted_ipv4_matched(h: &Headers) -> bool { + h.embedded_ip() + .is_some_and(|e| matches!(e.net(), Some(Net::Ipv4(_)))) + } + + combinators_fire_exactly_once_and_only_when_due!( + matcher_combinators, ThinHeaders, "Matcher", whole_chain, + .pat().eth().net() + ); + combinators_fire_exactly_once_and_only_when_due!( + matcher_mut_combinators, ThinHeaders, "MatcherMut", whole_chain, + .pat_mut().eth().net() + ); + combinators_fire_exactly_once_and_only_when_due!( + embedded_matcher_combinators, ShapedIcmpError, "EmbeddedMatcher", quoted_ipv4_matched, + .pat().eth().ipv4().icmp4().embedded().ipv4() + ); + combinators_fire_exactly_once_and_only_when_due!( + embedded_matcher_mut_combinators, ShapedIcmpError, "EmbeddedMatcherMut", + quoted_ipv4_matched, + .pat_mut().eth().ipv4().icmp4().embedded().ipv4() + ); + + /// `otherwise` does not run for every chain that returns `None`, and `inspect` runs for some. + /// + /// Found by the property above, which originally assumed the combinators tracked `.done()`. + /// They do not, and the difference is only observable on the embedded matchers, which carry two + /// accumulators: `done()` requires *both* to be populated, while `when`, `inspect` and + /// `otherwise` all read the inner one alone. + /// + /// So a packet whose outer chain fails -- an unconsumed VLAN tag will do it -- but whose quoted + /// packet matches will run `inspect`, skip `otherwise`, and then return `None`. The doc comments + /// are accurate ("apply a predicate to the inner accumulator", "run a closure if the inner match + /// has already failed"), so this is documented rather than broken. It still seems worth a + /// decision: `otherwise` is the error-handling hook, and there is a whole class of failure it + /// stays silent for. + /// + /// This test pins the behaviour as it stands. If the combinators are ever changed to track + /// `done()` it will fail, which is the point -- that should be a decision rather than a + /// discovery. + #[test] + fn the_embedded_combinators_track_the_inner_match_only() { + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static DIVERGED: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator(ShapedIcmpError) + .for_each(|h: &Headers| { + let whole = h + .pat() + .eth() + .ipv4() + .icmp4() + .embedded() + .ipv4() + .done() + .is_some(); + let inner = quoted_ipv4_matched(h); + if whole == inner { + return; + } + DIVERGED.fetch_add(1, Ordering::Relaxed); + // Divergence is one-directional: the inner match can succeed where the whole chain + // fails, never the reverse, since `done()` needs the inner accumulator too. + assert!( + inner && !whole, + "the whole chain matched while the inner one did not, which `done` forbids: \ + {h:?}" + ); + let ran = Cell::new(false); + let _ = h + .pat() + .eth() + .ipv4() + .icmp4() + .embedded() + .ipv4() + .otherwise(|| ran.set(true)) + .done(); + assert!( + !ran.get(), + "`otherwise` ran on a chain whose inner match succeeded; the divergence \ + documented here has been fixed, so this test should be deleted: {h:?}" + ); + }); + let diverged = DIVERGED.load(Ordering::Relaxed); + println!("outer failed while the quote matched: {diverged} packets"); + assert!( + diverged > 0, + "the two never diverged, so this test proved nothing about which one the combinators \ + follow" + ); + } + + /// The quoted transport enum refuses an unconsumed extension on the mutable path too. + /// + /// `EmbeddedMatcherMut::transport` is the one enum-level embedded method that exists, and its + /// gap-check rejection had nothing reaching it. Stated directly rather than differentially, + /// because the read-side counterpart it would be compared against is one of the five missing + /// methods listed above. + #[test] + fn the_quoted_transport_enum_refuses_an_unconsumed_extension() { + bolero::check!() + .with_generator(crate::headers::ShapedQuote { ext: 0, v4: false }) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + let quoted = owned + .embedded_ip_mut() + .unwrap_or_else(|| unreachable!("ShapedQuote always attaches a quote")); + let first = quoted + .net_ext + .first() + .unwrap_or_else(|| unreachable!("ShapedQuote always places one extension")) + .clone(); + quoted.net_ext.push(first); + + let mut two = owned.clone(); + assert!( + two.pat_mut() + .eth() + .ipv6() + .icmp6() + .embedded() + .ipv6() + .hop_by_hop() + .transport() + .done() + .is_none(), + "one extension named of two, yet the transport enum matched: {h:?}" + ); + let mut one = h.clone(); + assert!( + one.pat_mut() + .eth() + .ipv6() + .icmp6() + .embedded() + .ipv6() + .hop_by_hop() + .transport() + .done() + .is_some(), + "the sole extension was named and consumed, yet the chain missed: {h:?}" + ); + }); + } +} diff --git a/net/src/headers/view.rs b/net/src/headers/view.rs index 516f60ff28..9f14ee8135 100644 --- a/net/src/headers/view.rs +++ b/net/src/headers/view.rs @@ -2575,3 +2575,682 @@ mod tests { assert_eq!(v.vid(), vid_updated); } } + +/// The `unsafe` boundary, checked against the safe implementation of the same semantics. +/// +/// [`HeadersView`] earns its zero-cost extraction with `unwrap_unchecked`: [`Look::look`] repeats the +/// [`ViewStep::step`] chain that [`sealed::Sealed::matches`] already ran and tells the compiler the +/// `None` arms cannot happen. **The soundness of that rests entirely on the two chains agreeing**, and +/// they are written out separately for every arity by the macro above -- eight-odd hand-written pairs, +/// each threading the VLAN and extension cursors through by hand. A transposed cursor in one of them +/// is not a wrong answer, it is undefined behaviour. +/// +/// That is the same shape as every defect this campaign found in `routing`: an invariant enforced at a +/// distance by a different function from the one relying on it. The difference is the consequence. +/// +/// The oracle is [`Matcher`](super::pat::Matcher), which decides the same question safely and returns +/// an `Option`. Comparing the two is a differential test between two existing implementations rather +/// than against a third transcription of the rules -- if they disagree, either `matches` admits a +/// packet `look` cannot extract from (undefined behaviour) or `Matcher` mis-matches in the datapath. +/// Both are worth knowing. +/// +/// References are compared by **address**, not by value: two VLAN tags with identical contents are a +/// pass under `assert_eq!` and a bug if the two implementations picked different ones. +/// +/// # Two lines of defence, and where each one fires +/// +/// Both `matches` and `look` call the same [`ViewStep::step`], so a bug *inside* `step` is invisible +/// here -- it moves both sides together. What is checked is the hand-written *chaining* around it, +/// which is where the duplication is. +/// +/// Against a divergence in that chaining there are two independent guards, and both were demonstrated +/// by breaking the arity-3 arm on purpose: +/// +/// 1. **This differential, which fires first.** The `Matcher` comparison happens *before* `look` is +/// called, so a divergence in either direction fails with a shrunk counterexample rather than by +/// invoking undefined behaviour. Making `matches` stricter, or making it accept everything, both +/// fail here in under a second. +/// 2. **The standard library's own check, as a backstop.** `unwrap_unchecked` bottoms out in +/// `hint::unreachable_unchecked`, whose `assert_unsafe_precondition!` is gated on `ub_checks` -- +/// which follows `-Cdebug-assertions`, and `profile.fuzz` sets that **on**. So if a divergence ever +/// slipped past guard 1 -- if `Matcher` carried the same bug, say -- reaching `look` aborts: +/// +/// ```text +/// unsafe precondition(s) violated: hint::unreachable_unchecked must never be reached +/// thread caused non-unwinding panic. aborting. +/// ``` +/// +/// Verified by calling `look` on a deliberately over-permissive `matches` with the differential +/// removed: `SIGABRT`, in the fuzz profile, no miri required. Note it is a *non-unwinding* panic, so +/// bolero cannot catch it and the process dies -- which under libfuzzer is exactly right, a saved +/// `crash-*` artifact rather than a silent pass. +/// +/// The practical consequence is that **`just fuzz` on the fuzz profile already detects the unsound +/// direction**, and that is where the assurance mostly comes from. Miri remains useful for what +/// `ub_checks` does not model -- aliasing and provenance across the `as_ref_unchecked` boundary -- but +/// it is not the only thing standing between this and undefined behaviour. +/// +/// These do run clean under `just miri test -p dataplane-net view_properties`. Worth knowing what that +/// is worth, though: bolero manages 5 cases a second under miri against roughly 35,000 native, and the +/// miri recipe spawns its own `nix-shell`, so `BOLERO_RANDOM_TEST_TIME_MS` from the caller's +/// environment never arrives and the run stops at **25 cases per property**. A smoke test, not a proof. +/// Raising it means setting the budget inside `miri.just`. +#[cfg(test)] +mod view_properties { + use crate::eth::Eth; + use crate::headers::view::Look; + use crate::headers::{Headers, Net, ShapedHeaders, Transport}; + use crate::vlan::Vlan; + + /// `as_view` and `Matcher` must agree on whether the packet has the shape, and on which layers. + #[test] + fn eth_net_transport_agrees_with_the_matcher() { + bolero::check!() + .with_generator(ShapedHeaders) + .for_each(|h: &Headers| { + let matched = h.pat().eth().net().transport().done(); + match h.as_view::<(&Eth, &Net, &Transport)>() { + None => assert!( + matched.is_none(), + "the matcher accepted a shape as_view refused: {h:?}" + ), + Some(view) => { + let Some((m_eth, m_net, m_transport)) = matched else { + panic!("as_view accepted a shape the matcher refused: {h:?}"); + }; + let (v_eth, v_net, v_transport) = view.look(); + assert!(std::ptr::eq(v_eth, m_eth), "eth differs: {h:?}"); + assert!(std::ptr::eq(v_net, m_net), "net differs: {h:?}"); + assert!( + std::ptr::eq(v_transport, m_transport), + "transport differs: {h:?}" + ); + } + } + }); + } + + /// The same, for a shape that names a VLAN tag. + /// + /// This is where the interesting half of the contract lives: a tag the shape does not mention is a + /// miss, so the two implementations have to agree about *how many* tags were consumed, not merely + /// that some were. + #[test] + fn eth_vlan_net_transport_agrees_with_the_matcher() { + bolero::check!() + .with_generator(ShapedHeaders) + .for_each(|h: &Headers| { + let matched = h.pat().eth().vlan().net().transport().done(); + match h.as_view::<(&Eth, &Vlan, &Net, &Transport)>() { + None => assert!( + matched.is_none(), + "the matcher accepted a vlan shape as_view refused: {h:?}" + ), + Some(view) => { + let Some((m_eth, m_vlan, m_net, m_transport)) = matched else { + panic!("as_view accepted a vlan shape the matcher refused: {h:?}"); + }; + let (v_eth, v_vlan, v_net, v_transport) = view.look(); + assert!(std::ptr::eq(v_eth, m_eth), "eth differs: {h:?}"); + assert!( + std::ptr::eq(v_vlan, m_vlan), + "the two implementations consumed different vlan tags: {h:?}" + ); + assert!(std::ptr::eq(v_net, m_net), "net differs: {h:?}"); + assert!( + std::ptr::eq(v_transport, m_transport), + "transport differs: {h:?}" + ); + } + } + }); + } + + // A shape starting at `Net` rather than `Eth` was tried here and does not compile: `Net` does not + // satisfy `Within<()>`, so the adjacency graph forbids a shape that begins mid-stack. That half of + // the contract is enforced at compile time and needs no property. +} + +/// The **mutable** half of the unsafe boundary, which is the more delicate one. +/// +/// [`Look::look`] and [`sealed::Sealed::matches`] at least walk the stack the same way: both chain +/// [`ViewStep::step`]. [`LookMut::look_mut`] does not. It builds a [`MatcherMut`](super::pat::MatcherMut) +/// from [`Headers::pat_mut`] and chains [`ViewStepMut::chain`] over it, then calls +/// `unreachable_unchecked` if that comes back `None`. +/// +/// So the invariant is established by one traversal and consumed by a **different** one. Nothing makes +/// `ViewStep::step` and `ViewStepMut::chain` agree except that they were written to; where the read path +/// risks a mis-threaded cursor between two copies of the same walk, this risks two walks disagreeing +/// outright. +/// +/// Note what is *not* worth testing here: `look_mut` against `MatcherMut` directly. `look_mut` **is** +/// `MatcherMut` plus an `unreachable_unchecked`, so that comparison is the implementation against +/// itself. The question worth asking is whether `matches` -- the `ViewStep` walk that licensed the +/// unchecked call -- agrees with the `ViewStepMut` walk that has to deliver on it. +/// +/// # Aliasing, and why `ub_checks` cannot help +/// +/// `look_mut` hands back several `&mut` into one [`Headers`], pre-split through +/// [`Fields`](super::pat::Fields). If that split ever aliased, two of those references would point at +/// the same layer -- undefined behaviour of a kind the `ub_checks` backstop does **not** model. It +/// checks the `unreachable_unchecked` precondition, nothing about aliasing. +/// +/// Only miri sees that, and only with stacked borrows *on*, which this repo turns off by default: +/// +/// ```text +/// STACKED_BORROW_CHECK=enabled just miri test -p dataplane-net view_mut_properties +/// ``` +/// +/// Through the environment, not `just`'s command line: a *module's* variables cannot be overridden +/// there. `just miri stacked_borrow_check=enabled test` parses as a recipe name and +/// `just --set stacked_borrow_check enabled miri test` is refused, which is why `miri.just` reads both +/// knobs via `env()`. +#[cfg(test)] +mod view_mut_properties { + use crate::eth::Eth; + use crate::headers::view::{Look, LookMut}; + use crate::headers::{Headers, Net, ShapedHeaders, SometimesHeadless, Transport}; + + /// The type a `Matcher` method name selects. + /// + /// A shape and the chain that matches it are the same statement written twice -- + /// `(&Eth, &Ipv6, &HopByHop, &Transport)` and `.eth().ipv6().hop_by_hop().transport()` -- and + /// writing both by hand at every instantiation is a standing invitation to write two different + /// statements. The pair that disagrees still compiles and still passes; it just quietly tests + /// something other than what it says. So the chain is the input and the shape is derived. + /// + /// The table is the only place the correspondence lives, and it is the whole of it: every entry + /// below names a method the `matcher_net!`, `matcher_ext!` and `matcher_transport!` invocations + /// in [`pat`](super::pat) generate, plus the four written out by hand. + macro_rules! layer_ty { + (eth) => { + crate::eth::Eth + }; + (vlan) => { + crate::vlan::Vlan + }; + (net) => { + crate::headers::Net + }; + (ipv4) => { + crate::ipv4::Ipv4 + }; + (ipv6) => { + crate::ipv6::Ipv6 + }; + (hop_by_hop) => { + crate::ipv6::HopByHop + }; + (dest_opts) => { + crate::ipv6::DestOpts + }; + (routing) => { + crate::ipv6::Routing + }; + (fragment) => { + crate::ipv6::Fragment + }; + (ipv4_auth) => { + crate::ip_auth::Ipv4Auth + }; + (ipv6_auth) => { + crate::ip_auth::Ipv6Auth + }; + (transport) => { + crate::headers::Transport + }; + (tcp) => { + crate::tcp::Tcp + }; + (udp) => { + crate::udp::Udp + }; + (icmp4) => { + crate::icmp4::Icmp4 + }; + (icmp6) => { + crate::icmp6::Icmp6 + }; + (vxlan) => { + crate::vxlan::Vxlan + }; + } + + /// The `Shape` a chain of matcher method names denotes. + macro_rules! shape_of { + ($($layer:ident),+ $(,)?) => { ($(&'static layer_ty!($layer),)+) }; + } + + /// The address of each layer in a tuple of references, without naming the arity. + /// + /// Agreeing that a shape matches is the soundness question; handing back the *same* layers is the + /// correctness one, and it was previously checked only at arities three and four, because that is + /// where a tuple can be destructured by hand into a fixed number of bindings. Reducing the tuple + /// to its addresses removes the need to name the arity at all, so the check applies wherever the + /// agreement check does. + /// + /// Addresses rather than values: two layers can hold equal bytes without being the same layer, and + /// picking the wrong VLAN tag out of four identical ones is exactly the cursor bug this is looking + /// for. + trait Addrs { + /// One address per element. + type Out: PartialEq + core::fmt::Debug; + /// Where each element of this tuple lives. + fn addrs(&self) -> Self::Out; + } + + macro_rules! impl_addrs { + ($n:literal; $($T:ident $idx:tt),+) => { + impl<'a, $($T),+> Addrs for ($(&'a $T,)+) { + type Out = [usize; $n]; + fn addrs(&self) -> [usize; $n] { + [$(core::ptr::from_ref::<$T>(self.$idx) as usize),+] + } + } + + impl<'a, $($T),+> Addrs for ($(&'a mut $T,)+) { + type Out = [usize; $n]; + fn addrs(&self) -> [usize; $n] { + [$(core::ptr::from_ref::<$T>(&*self.$idx) as usize),+] + } + } + }; + } + + impl_addrs!(1; A 0); + impl_addrs!(2; A 0, B 1); + impl_addrs!(3; A 0, B 1, C 2); + impl_addrs!(4; A 0, B 1, C 2, D 3); + impl_addrs!(5; A 0, B 1, C 2, D 3, E 4); + impl_addrs!(6; A 0, B 1, C 2, D 3, E 4, F 5); + impl_addrs!(7; A 0, B 1, C 2, D 3, E 4, F 5, G 6); + impl_addrs!(8; A 0, B 1, C 2, D 3, E 4, F 5, G 6, H 7); + + /// Both walks, at every arity the oracle can express. + /// + /// The decision is what soundness turns on: if `matches` says yes where the walk that must deliver + /// says no, `look`/`look_mut` reach `unreachable_unchecked`. So this compares decisions across the + /// whole family, for the read path and the mutable path both. + /// + /// Arity matters because `matches`, `look` and `look_mut` are generated separately for each one, by + /// a macro, with the VLAN and extension cursors threaded through by hand every time. Testing two + /// arities tested two of eight copies. + /// + /// Each property asks two things of a shape, and the second is the reason `look` and `look_mut` + /// appear here rather than only in the split test: + /// + /// * **the decision**, `matches` against the matcher chain. This is what soundness turns on: if + /// `matches` says yes where the walk that must deliver says no, `look`/`look_mut` reach + /// `unreachable_unchecked`. Disagreement here is undefined behaviour, not a wrong answer. + /// * **the delivery**, `look`/`look_mut` against the same chain's tuple, compared by address. + /// Agreeing that a shape is present is not the same as picking the same layers out of it, and + /// the cursor arithmetic that decides *which* VLAN tag or *which* extension header comes back + /// is written out by hand once per arity. A shape naming four tags has four chances to be off + /// by one and no way for the decision check to notice. + /// + /// Delivery was previously checked at arities three and four only, because a tuple has to be + /// destructured into a fixed number of bindings to be compared -- which is why [`Addrs`] exists. + macro_rules! arity_agrees { + ($read:ident, $mutable:ident, $gen:expr, $($layer:ident),+) => { + #[test] + fn $read() { + type Shape = shape_of!($($layer),+); + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static SEEN: AtomicUsize = AtomicUsize::new(0); + static HIT: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator($gen) + .for_each(|h: &Headers| { + SEEN.fetch_add(1, Ordering::Relaxed); + let licensed = h.as_view::().is_some(); + if licensed { + HIT.fetch_add(1, Ordering::Relaxed); + } + let chain = h.pat()$(.$layer())+.done(); + assert_eq!( + licensed, chain.is_some(), + concat!( + "`matches` and the read walk disagree for ", + stringify!(($($layer),+)), + ", so `look` would reach `unreachable_unchecked`: {:?}" + ), + h + ); + if let (Some(view), Some(chain)) = (h.as_view::(), chain) { + assert_eq!( + view.look().addrs(), chain.addrs(), + concat!( + "`look` and the read walk agreed that ", + stringify!(($($layer),+)), + " is present and then handed back different layers: {:?}" + ), + h + ); + } + }); + agreement_is_not_vacuous( + stringify!(($($layer),+)), + SEEN.load(Ordering::Relaxed), + HIT.load(Ordering::Relaxed), + ); + } + + #[test] + fn $mutable() { + type Shape = shape_of!($($layer),+); + use concurrency::sync::atomic::{AtomicUsize, Ordering}; + static SEEN: AtomicUsize = AtomicUsize::new(0); + static HIT: AtomicUsize = AtomicUsize::new(0); + bolero::check!() + .with_generator($gen) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + SEEN.fetch_add(1, Ordering::Relaxed); + let licensed = owned.as_view_mut::().is_some(); + if licensed { + HIT.fetch_add(1, Ordering::Relaxed); + } + // The two walks each want the whole of `owned` mutably, so they take turns + // and hand back addresses rather than references. Nothing is written + // between the two, so the addresses stay comparable. + let chain = owned.pat_mut()$(.$layer())+.done().map(|t| t.addrs()); + assert_eq!( + licensed, chain.is_some(), + concat!( + "`matches` and the mutable walk disagree for ", + stringify!(($($layer),+)), + ", so `look_mut` would reach `unreachable_unchecked`: {:?}" + ), + h + ); + if let Some(view) = owned.as_view_mut::() { + assert_eq!( + Some(view.look_mut().addrs()), chain, + concat!( + "`look_mut` and the mutable walk agreed that ", + stringify!(($($layer),+)), + " is present and then handed back different layers: {:?}" + ), + h + ); + } + }); + agreement_is_not_vacuous( + stringify!(($($layer),+)), + SEEN.load(Ordering::Relaxed), + HIT.load(Ordering::Relaxed), + ); + } + }; + } + + /// A shape the generator never produces makes its property pass for the wrong reason. + /// + /// Two implementations agreeing that nothing matches is not agreement worth having, and the higher + /// arities are where it would happen quietly: arity 7 needs a packet carrying exactly four VLAN + /// tags, since a tag the shape does not name is a miss. So every arity reports its own hit rate and + /// fails if it never matched at all. + fn agreement_is_not_vacuous(shape: &str, seen: usize, hit: usize) { + println!("{shape}: matched {hit} of {seen}"); + // The default run is a second long, which is plenty here -- these properties manage tens of + // thousands of cases a second -- but a short sample should say so rather than fail. + if seen > 500 { + assert!( + hit > 0, + "{shape} never matched in {seen} packets: the two walks agree only because the \ + generator cannot produce this shape" + ); + } + } + + // Every arity from one to eight, which is all of them. + // + // `Matcher` names every layer the `Within` graph does -- `matcher_net!`, `matcher_ext!` and + // `matcher_transport!` give it `.ipv4()`, `.ipv6()`, `.hop_by_hop()`, `.dest_opts()`, + // `.routing()`, `.fragment()`, `.ipv4_auth()`, `.ipv6_auth()`, `.tcp()`, `.udp()`, `.icmp4()` and + // `.icmp6()` alongside the generic `.eth()` / `.vlan()` / `.net()` / `.transport()`. So the + // oracle reaches as far as the code does: `Eth` + four VLAN tags (`MAX_VLANS`) + `Ipv6` + + // `HopByHop` + `Transport` is eight, and the extension region is expressible. + // + // `SometimesHeadless` rather than `ShapedHeaders`: the first step of every one of these is `Eth`, + // and the branch where that step refuses is generated once per arity. Nothing that always sets + // `eth` can reach any of them. + arity_agrees!(read_1, mutable_1, SometimesHeadless, eth); + arity_agrees!(read_2, mutable_2, SometimesHeadless, eth, net); + arity_agrees!(read_3, mutable_3, SometimesHeadless, eth, net, transport); + arity_agrees!( + read_4, + mutable_4, + SometimesHeadless, + eth, + vlan, + net, + transport + ); + arity_agrees!( + read_5, + mutable_5, + SometimesHeadless, + eth, + vlan, + vlan, + net, + transport + ); + arity_agrees!( + read_6, + mutable_6, + SometimesHeadless, + eth, + vlan, + vlan, + vlan, + net, + transport + ); + arity_agrees!( + read_7, + mutable_7, + SometimesHeadless, + eth, + vlan, + vlan, + vlan, + vlan, + net, + transport + ); + arity_agrees!( + read_8, + mutable_8, + SometimesHeadless, + eth, + vlan, + vlan, + vlan, + vlan, + ipv6, + hop_by_hop, + transport + ); + + // Shapes that enter the IPv6 extension region, where `ExtGapCheck` switches from + // skip-extensions-silently to consume-them-all. + // + // This is the subtlest part of the contract and the part the module documentation spends most of + // its words on. It is also where the two walks are least alike: on the read path both call + // `ExtGapCheck::ext_gap_ok`, so what is under test is the by-hand threading of the `ec` cursor + // through each separately generated arity -- which is the plausible bug. On the mutable path + // they call genuinely different implementations, `ext_gap_ok` against a `Headers` versus + // `ext_gap_ok_mut` against a pre-split `Fields`, so those two are checked against each other + // as well. + // + // Each shape below is deliberately exact: naming `HopByHop` and then `Transport` matches only a + // packet carrying that extension and no other, since an unconsumed extension is a miss once the + // chain has entered the region. + arity_agrees!( + read_ext_v6_one, + mutable_ext_v6_one, + SometimesHeadless, + eth, + ipv6, + hop_by_hop, + transport + ); + arity_agrees!( + read_ext_v6_two, + mutable_ext_v6_two, + SometimesHeadless, + eth, + ipv6, + hop_by_hop, + dest_opts, + transport + ); + arity_agrees!( + read_ext_v6_three, + mutable_ext_v6_three, + SometimesHeadless, + eth, + ipv6, + hop_by_hop, + dest_opts, + routing, + transport + ); + // The IPv4 authentication header is the only extension that belongs on a v4 packet, and it is + // the only way to reach the strict branch without IPv6. + arity_agrees!( + read_ext_v4_auth, + mutable_ext_v4_auth, + SometimesHeadless, + eth, + ipv4, + ipv4_auth, + transport + ); + // Entering the region and then *not* naming a transport: the gap check never runs, so every + // extension after the named one is simply left unvisited. Distinguishing this from the strict + // case above is the whole point of running the check at the transport step rather than the + // extension step. + arity_agrees!( + read_ext_v6_no_transport, + mutable_ext_v6_no_transport, + SometimesHeadless, + eth, + ipv6, + hop_by_hop + ); + + // The UDP encapsulation layer, which no shape named until now. + // + // `Vxlan` is the one `ViewStep` that runs no gap check at all -- it reads `udp_encap` and ignores + // both cursors -- and the one layer that is not part of the linear header stack, so nothing about + // it follows from the arities above. `CommonHeaders` has been drawing VXLAN packets the whole + // time; the gap was that no property ever asked for one. + // + // `ShapedHeaders` rather than `SometimesHeadless` here, because this shape is already narrow: + // `Udp` is one of three next headers, the encapsulation is a coin flip on top of that, and the + // net step demands no VLAN tags. Removing the Ethernet header as well would spend the hit rate + // on a branch the eight arities above already cover. + arity_agrees!( + read_vxlan_v4, + mutable_vxlan_v4, + ShapedHeaders, + eth, + ipv4, + udp, + vxlan + ); + arity_agrees!( + read_vxlan_net, + mutable_vxlan_net, + ShapedHeaders, + eth, + net, + udp, + vxlan + ); + + /// Exercise the multi-`&mut` split itself: write through every reference and read the writes back. + /// + /// The assertions are almost beside the point. What matters is that the references are *created and + /// written through*, so that miri with stacked borrows enabled can judge whether + /// [`Fields`](super::pat::Fields) handed out two paths to the same layer. Nothing else in the suite + /// does that. + fn exercise_the_mutable_split() { + bolero::check!() + .with_generator(ShapedHeaders) + .for_each(|h: &Headers| { + let mut owned = h.clone(); + let Some(view) = owned.as_view_mut::<(&Eth, &Net, &Transport)>() else { + return; + }; + let (eth, net, transport) = view.look_mut(); + + // A write through each reference, then a read back through the same one. If two of + // these aliased, the writes would interfere and miri would object to the borrow stack + // long before the values did. + let want_src = + crate::eth::mac::SourceMac::try_from(crate::eth::mac::Mac([2, 0, 0, 0, 0, 1])) + .unwrap_or_else(|_| { + unreachable!("a locally-administered unicast mac is a valid source") + }); + eth.set_source(want_src); + let seen_net = net.dst_addr(); + let seen_transport = transport.dst_port(); + + assert_eq!( + eth.source(), + want_src, + "the write through eth did not stick" + ); + assert_eq!(net.dst_addr(), seen_net, "net changed under a write to eth"); + assert_eq!( + transport.dst_port(), + seen_transport, + "transport changed under a write to eth" + ); + }); + } + + /// Shards of [`exercise_the_mutable_split`], so the machine can be used. + /// + /// Under miri this property is the expensive one and the one that matters most, and its cost is + /// wall-clock: bolero manages about five cases a second. Raising the time budget buys cases + /// linearly, but only on one core. + /// + /// Each shard seeds itself from the OS, so `N` shards explore `N` independent streams and nextest + /// runs them concurrently -- turning a machine with cores to spare into more cases for the same + /// wall time, which is the only lever that does not cost patience. Sixteen rather than sixty: miri + /// carries a large memory footprint per process, and the other properties want cores too. + macro_rules! split_shards { + ($($name:ident),* $(,)?) => { + $( + #[test] + fn $name() { + exercise_the_mutable_split(); + } + )* + }; + } + + split_shards!( + the_mutable_split_hands_out_distinct_layers, + split_shard_02, + split_shard_03, + split_shard_04, + split_shard_05, + split_shard_06, + split_shard_07, + split_shard_08, + split_shard_09, + split_shard_10, + split_shard_11, + split_shard_12, + split_shard_13, + split_shard_14, + split_shard_15, + split_shard_16, + ); +} diff --git a/net/src/headers/within.rs b/net/src/headers/within.rs index 2fc8646360..c2e2240537 100644 --- a/net/src/headers/within.rs +++ b/net/src/headers/within.rs @@ -554,3 +554,357 @@ impl Within for Tcp { impl Within for Udp { fn conform(_parent: &mut Net) {} } + +/// What [`Within::conform`] is for, checked against the parser. +/// +/// `conform` writes the parent's protocol field so it names the child: `EthType::IPV6` on an +/// Ethernet header carrying IPv6, `NextHeader::ROUTING` on an IPv6 header carrying a routing +/// extension. It is the only reason this trait has a method at all -- the ordering half of the +/// contract is enforced by the compiler, by the presence or absence of an impl, and needs no test. +/// +/// Thirty-two of these bodies had never run. Every one of them was an IPv6 extension header +/// transition or `Vlan` inside `Vlan`, which is to say: the whole extension region, plus the second +/// tag of a double-tagged frame. The reason is that `conform` is reached only through +/// [`HeaderStack::stack`](crate::headers::builder::HeaderStack::stack), and while the builder has had +/// `.hop_by_hop()`, `.dest_opts()`, `.routing()`, `.fragment()`, `.ipv4_auth()` and `.ipv6_auth()` +/// all along, no test ever called one. The generators reach the extension region constantly, but +/// they assemble [`Headers`](crate::headers::Headers) field by field and never go through the +/// builder, so they never conform anything. +/// +/// # The oracle +/// +/// A test that builds a packet and then reads back the field `conform` just wrote would be checking +/// the implementation against itself. So the check is deparse-then-parse: the parser decides what +/// follows an IPv6 header by reading its next-header field, which is the field `conform` sets, so a +/// `conform` that names the wrong protocol produces bytes the parser reads as a different packet -- +/// or as no valid packet at all. +/// +/// # Scrambling +/// +/// Each layer's next-header field is set to a fuzzed byte *before* the next layer is stacked, so +/// `conform` is always overwriting a wrong value rather than filling in a blank one. +/// +/// That is not a precaution, it is load-bearing, and the case that proves it is `Ipv4` inside `Eth`. +/// [`Blank`] for `Eth` produces `EthType::IPV4`, so on a blank Ethernet header the conform that sets +/// `EthType::IPV4` has nothing to do: delete its body and every packet still round-trips. Scrambled, +/// the same deletion fails two chains. `Vlan` has the same blank and the same exposure. +/// +/// # What is left, and why it stays uncovered +/// +/// Nineteen no-op `conform` bodies remain unrun: the enum-level impls, the `EmbeddedStart` impls and +/// everything `impl_truncated_within!` generates. They are not merely untested, they are unreachable +/// through the builder, and the compiler says so twice -- writing +/// `HeaderStack::new().eth(..).stack::(..)` fails with both `Net: Blank is not satisfied` and +/// `Headers: Install is not satisfied`. Either bound alone would be enough. +/// +/// They exist to give the pattern matcher its `Within` edges, which need the trait but not the +/// method. `conform` is a public trait method, so they are callable in principle by anyone holding +/// the parent; nothing in the tree does. Whether nineteen uncallable bodies are worth keeping is a +/// question for whoever owns the trait, not something a test can settle. +/// +/// [`Blank`]: crate::headers::builder::Blank +#[cfg(test)] +mod conform_properties { + use crate::eth::Eth; + use crate::eth::ethtype::EthType; + use crate::headers::builder::HeaderStack; + use crate::headers::test::parse_back_test; + use crate::headers::{Headers, Transport}; + use crate::icmp4::{ + Icmp4, Icmp4DestUnreachable, Icmp4EchoReply, Icmp4EchoRequest, Icmp4ParamProblem, + Icmp4Redirect, Icmp4TimeExceeded, Icmp4Type, + }; + use crate::icmp6::{ + Icmp6, Icmp6DestUnreachable, Icmp6EchoReply, Icmp6EchoRequest, Icmp6PacketTooBig, + Icmp6ParamProblem, Icmp6TimeExceeded, Icmp6Type, + }; + use crate::ip::NextHeader; + use crate::ip_auth::{Ipv4Auth, Ipv6Auth}; + use crate::ipv4::Ipv4; + use crate::ipv6::{DestOpts, Fragment, HopByHop, Ipv6, Routing}; + use crate::tcp::Tcp; + use crate::udp::Udp; + use crate::vlan::Vlan; + + /// Put a wrong protocol number in the field `conform` is responsible for. + /// + /// Implemented for every layer the builder can stack. The transport types are the leaves of + /// every chain -- nothing is ever stacked on top of one, so nothing ever conforms one -- and + /// their impls are deliberately empty rather than absent, so that the chain macro does not have + /// to know which layers are interior. + trait Scramble { + /// Overwrite the protocol field with something derived from `seed`. + fn scramble(&mut self, seed: u8); + } + + macro_rules! scramble_next_header { + ($($T:ty),+ $(,)?) => {$( + impl Scramble for $T { + fn scramble(&mut self, seed: u8) { + self.set_next_header(NextHeader::new(seed)); + } + } + )+}; + } + + macro_rules! scramble_leaf { + ($($T:ty),+ $(,)?) => {$( + impl Scramble for $T { + fn scramble(&mut self, _seed: u8) {} + } + )+}; + } + + scramble_next_header!( + Ipv4, Ipv6, HopByHop, DestOpts, Routing, Fragment, Ipv4Auth, Ipv6Auth + ); + scramble_leaf!(Tcp, Udp); + // The ICMP subtypes are the only layers that can sit on top of an ICMP header, and nothing sits + // on top of them. + scramble_leaf!( + Icmp4DestUnreachable, + Icmp4Redirect, + Icmp4TimeExceeded, + Icmp4ParamProblem, + Icmp4EchoRequest, + Icmp4EchoReply, + Icmp6DestUnreachable, + Icmp6PacketTooBig, + Icmp6TimeExceeded, + Icmp6ParamProblem, + Icmp6EchoRequest, + Icmp6EchoReply, + ); + + // ICMP is the one place where `conform` writes a message type rather than a protocol number, so + // `Unknown` is the scramble: it is the one variant matching no subtype, which makes it wrong for + // every chain below rather than accidentally right for one of them. + // + // The type byte is fixed rather than fuzzed, and has to be. `Unknown` stores the raw byte, so + // `Unknown { type_u8: 3 }` deparses to the same three bytes a destination-unreachable message + // does and parses back as one -- a header the round-trip is right to reject, and nothing to do + // with `conform`. 253 for v4 and 200 for v6 are reserved for experimentation and belong to no + // variant, so they survive the round trip as themselves. The rest of the message stays fuzzed. + impl Scramble for Icmp4 { + fn scramble(&mut self, seed: u8) { + self.set_type(crate::icmp4::Icmp4Type::Unknown { + type_u8: 253, + code_u8: seed, + bytes5to8: [seed; 4], + }); + } + } + + impl Scramble for Icmp6 { + fn scramble(&mut self, seed: u8) { + self.set_type(crate::icmp6::Icmp6Type::Unknown { + type_u8: 200, + code_u8: seed, + bytes5to8: [seed; 4], + }); + } + } + + impl Scramble for Eth { + fn scramble(&mut self, seed: u8) { + self.set_ether_type(EthType::new(u16::from(seed))); + } + } + + impl Scramble for Vlan { + fn scramble(&mut self, seed: u8) { + self.set_inner_ethtype(EthType::new(u16::from(seed))); + } + } + + /// Build the named chain through the builder, scrambling as it goes, and round-trip it. + /// + /// The chain is a list of `HeaderStack` method names, and the closures are written here rather + /// than at the call site: a closure written at the call site could not name the `seed` this + /// macro binds, macro hygiene being what it is, and every call site would then have to repeat + /// the same closure once per layer. + macro_rules! conform_chain { + ($name:ident, $($layer:ident),+ $(,)?) => { + conform_chain!(@build $name, |_| {}, $($layer),+); + }; + (specialized $name:ident, $($layer:ident),+ $(,)?) => { + conform_chain!(@build $name, icmp_type_was_specialized, $($layer),+); + }; + (@build $name:ident, $check:expr, $($layer:ident),+) => { + #[test] + fn $name() { + bolero::check!().with_type().for_each(|seed: &u8| { + let seed = *seed; + let built = HeaderStack::new() + $(.$layer(|l| l.scramble(seed)))+ + .build_headers(); + let headers = built.unwrap_or_else(|e| { + unreachable!("a blank {} chain does not overflow: {e:?}", stringify!($name)) + }); + parse_back_test(&headers); + $check(&headers); + }); + } + }; + } + + /// The scrambled ICMP type did not survive into the built packet. + /// + /// Only for chains that end in a subtype layer. A chain ending at a bare `.icmp4()` has nothing + /// above it to conform it, so the scramble is *supposed* to survive there, and round-tripping is + /// the only thing to check. + /// + /// Worth stating why this is separate from the round trip rather than folded into it: the + /// scrambled type is a well-formed ICMP message, so a packet still carrying it deparses and + /// parses back perfectly. The round trip cannot tell that the specialization never happened. + fn icmp_type_was_specialized(headers: &Headers) { + match headers.transport() { + Some(Transport::Icmp4(icmp)) => assert!( + !matches!(icmp.icmp_type(), Icmp4Type::Unknown { type_u8: 253, .. }), + "the scrambled ICMPv4 type survived the build, so nothing specialized it" + ), + Some(Transport::Icmp6(icmp)) => assert!( + !matches!(icmp.icmp_type(), Icmp6Type::Unknown { type_u8: 200, .. }), + "the scrambled ICMPv6 type survived the build, so nothing specialized it" + ), + other => unreachable!("a subtype chain builds an ICMP transport, got {other:?}"), + } + } + + // Seventeen chains, chosen to cover all thirty-two unreached transitions between them. Three + // extension headers is the ceiling -- `MAX_NET_EXTENSIONS` -- so the deeper regions of the graph + // have to be reached by several chains rather than one long one. + conform_chain!( + double_tag_then_three_extensions, + eth, + vlan, + vlan, + ipv6, + dest_opts, + routing, + fragment, + tcp + ); + conform_chain!( + hop_by_hop_routing_dest_opts, + eth, + ipv6, + hop_by_hop, + routing, + dest_opts, + udp + ); + conform_chain!( + routing_fragment_auth, + eth, + ipv6, + routing, + fragment, + ipv6_auth, + tcp + ); + conform_chain!( + fragment_then_dest_opts, + eth, + ipv6, + fragment, + dest_opts, + icmp6 + ); + conform_chain!( + hop_by_hop_then_fragment, + eth, + ipv6, + hop_by_hop, + fragment, + udp + ); + conform_chain!( + dest_opts_then_fragment, + eth, + ipv6, + dest_opts, + fragment, + icmp6 + ); + conform_chain!(auth_then_dest_opts, eth, ipv6, ipv6_auth, dest_opts, udp); + conform_chain!( + hop_by_hop_then_auth, + eth, + ipv6, + hop_by_hop, + ipv6_auth, + icmp6 + ); + conform_chain!(dest_opts_then_auth, eth, ipv6, dest_opts, ipv6_auth, tcp); + conform_chain!(routing_then_auth, eth, ipv6, routing, ipv6_auth, udp); + conform_chain!(hop_by_hop_then_udp, eth, ipv6, hop_by_hop, udp); + conform_chain!(routing_then_udp, eth, ipv6, routing, udp); + conform_chain!( + hop_by_hop_then_routing_then_tcp, + eth, + ipv6, + hop_by_hop, + routing, + tcp + ); + conform_chain!(hop_by_hop_then_icmp6, eth, ipv6, hop_by_hop, icmp6); + conform_chain!(routing_then_icmp6, eth, ipv6, routing, icmp6); + // The IPv4 authentication header is the only extension that belongs on a v4 packet. + conform_chain!(v4_auth_then_udp, eth, ipv4, ipv4_auth, udp); + conform_chain!(v4_auth_then_icmp4, eth, ipv4, ipv4_auth, icmp4); + + // The ICMP message subtypes, which the builder can specialize into and which no test had ever + // asked for. Ten of the twelve `Blank` impls behind them had never been called either -- the two + // that had were what made the shared macro bodies look covered, since a `macro_rules!` line + // counts as run once any one of its expansions runs. Worth knowing generally: a table of twelve + // generated impls reports as covered when one of the twelve is exercised, and only the + // hand-written part of each -- here `blank()` -- shows the difference. + conform_chain!(specialized icmp4_dest_unreachable, eth, ipv4, icmp4, dest_unreachable); + conform_chain!(specialized icmp4_redirect, eth, ipv4, icmp4, redirect); + conform_chain!(specialized icmp4_time_exceeded, eth, ipv4, icmp4, time_exceeded); + conform_chain!(specialized icmp4_param_problem, eth, ipv4, icmp4, param_problem); + conform_chain!(specialized icmp4_echo_request, eth, ipv4, icmp4, echo_request); + conform_chain!(specialized icmp4_echo_reply, eth, ipv4, icmp4, echo_reply); + conform_chain!(specialized icmp6_dest_unreachable, eth, ipv6, icmp6, dest_unreachable6); + conform_chain!(specialized icmp6_packet_too_big, eth, ipv6, icmp6, packet_too_big6); + conform_chain!(specialized icmp6_time_exceeded, eth, ipv6, icmp6, time_exceeded6); + conform_chain!(specialized icmp6_param_problem, eth, ipv6, icmp6, param_problem6); + conform_chain!(specialized icmp6_echo_request, eth, ipv6, icmp6, echo_request6); + conform_chain!(specialized icmp6_echo_reply, eth, ipv6, icmp6, echo_reply6); + + /// A subtype the caller customized, which is what separates the two writers of the ICMP type. + /// + /// The chains above cannot tell `Within::conform` from `Install` for these layers, and no test + /// could, because both write the same value: `conform` sets `DestUnreachable(blank())` and + /// `Install` sets `DestUnreachable(value)`, and `value` is `blank()` whenever the caller does not + /// change it. Empty either one alone and the other still produces the right packet. + /// + /// Choosing a code other than the blank one separates them, and the answer is that `conform` is + /// the redundant half. `Install` runs unconditionally from `build_headers`, after `conform`, and + /// overwrites whatever `conform` wrote -- so all twelve of these `conform` bodies could be empty + /// with no observable change. Nothing can be stacked on a subtype, so there is no arrangement in + /// which `conform` gets the last word. + /// + /// Pinned here rather than acted on: emptying them is a call for whoever owns the builder. + #[test] + fn a_customized_subtype_survives_the_build() { + let headers = HeaderStack::new() + .eth(|l| l.scramble(0)) + .ipv4(|l| l.scramble(0)) + .icmp4(|l| l.scramble(0)) + .dest_unreachable(|d| *d = Icmp4DestUnreachable::Port) + .build_headers() + .unwrap_or_else(|e| unreachable!("a blank chain does not overflow: {e:?}")); + + let Some(Transport::Icmp4(icmp)) = headers.transport() else { + unreachable!("the chain builds an ICMPv4 transport") + }; + assert_eq!( + icmp.icmp_type(), + Icmp4Type::DestUnreachable(Icmp4DestUnreachable::Port), + "the code the caller chose did not reach the built packet" + ); + parse_back_test(&headers); + } +}