diff --git a/.gitignore b/.gitignore index 8450062587..2dbd36d349 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,8 @@ **.profraw **/__fuzz__/** +# libfuzzer writes one of these per worker into the working directory when +# `just fuzz` is given -j; the corpus itself lives under __fuzz__. +fuzz-*.log # qemu-user core dumps from SIGABRT under emulated tests. **/qemu_*.core result* diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs index 42af55b92f..5998aaecf2 100644 --- a/acl-filter/src/tests.rs +++ b/acl-filter/src/tests.rs @@ -935,6 +935,10 @@ mod end_to_end { static_nat_writer.get_reader(), )); + // Share the allocator between port forwarding and masquerade. + let mut allocator = NatAllocatorWriter::new(); + allocator.update_nat_allocator(MasqueradeConfig::new(overlay.vpc_table()), 1, &flow_table); + // Port forwarding let mut portfw_writer = PortFwTableWriter::new(); portfw_writer @@ -944,11 +948,10 @@ mod end_to_end { "port-forwarder", portfw_writer.reader(), flow_table.clone(), + allocator.get_reader(), )); // Masquerade (creates the related flow pair used by 'flow'-scoped replies) - let mut allocator = NatAllocatorWriter::new(); - allocator.update_nat_allocator(MasqueradeConfig::new(overlay.vpc_table()), 1, &flow_table); pipeline = pipeline.add_stage(Masquerade::new( "masquerade", flow_table.clone(), diff --git a/concurrency-macros/src/lib.rs b/concurrency-macros/src/lib.rs index 33d00bac31..d756beac66 100644 --- a/concurrency-macros/src/lib.rs +++ b/concurrency-macros/src/lib.rs @@ -11,19 +11,10 @@ use syn::{ parse_macro_input, }; -/// Resolve a path prefix for `dataplane-concurrency` in the consumer's -/// `Cargo.toml`. Returns a token stream that resolves to the crate root, -/// so callers can append `::stress` or `::with_loom` etc. +/// Resolve the consumer's name for `dataplane-concurrency`. /// -/// * Workspace consumer with `concurrency = { package = "dataplane-concurrency", ... }` -/// in its `Cargo.toml`: returns `::concurrency`. -/// * External consumer with `dataplane-concurrency = "..."` directly: -/// returns `::dataplane_concurrency`. -/// * `dataplane-concurrency`'s own integration tests: returns -/// `::dataplane_concurrency` (which requires the test file to do -/// `extern crate dataplane_concurrency;` -- cargo doesn't let a crate -/// list itself as a regular dev-dep, but `extern crate` works in the -/// integration test). +/// Workspace crates use `concurrency`; external users and this crate's integration tests use +/// `dataplane_concurrency`. fn concurrency_crate_path() -> TokenStream2 { match crate_name("dataplane-concurrency") { Ok(FoundCrate::Itself) => { @@ -103,9 +94,8 @@ pub fn concurrency_mode(attr: TokenStream, item: TokenStream) -> TokenStream { /// Mark a backend-routed concurrency test. /// -/// The default backend emits a flat `#[test]`; loom/shuttle emit a -/// nested `concurrency_model::{loom,shuttle}` leaf so nextest filters -/// can select one backend cleanly. +/// The default backend emits a flat test. Model-checker tests get a backend-named leaf so nextest +/// can select them safely. /// /// # Example /// @@ -116,10 +106,8 @@ pub fn concurrency_mode(attr: TokenStream, item: TokenStream) -> TokenStream { /// } /// ``` /// -/// The function must take no arguments and return `()`. The body is -/// captured as a closure, so it must be `Fn() + Send + Sync + 'static` -/// (no borrows of locals, no `FnOnce`-only constructs). This matches -/// what `loom::model` and `shuttle::check_*` require. +/// The function must take no arguments, return `()`, and work as an +/// `Fn() + Send + Sync + 'static` closure. /// /// # Limitations /// @@ -155,16 +143,7 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream { } let krate = concurrency_crate_path(); - // Default backend: flat `#[test] fn () { ... }`. No nested - // module wrapping -- the production code path runs the body once, - // and there is no second backend to disambiguate from. - // - // Model-checker backends: emit `mod { mod concurrency_model - // { #[test] fn () { ... } } }`. The leaf function name - // identifies the active backend, so a nextest report shows entries - // like `some_test::concurrency_model::loom` and a filter like - // `-E 'test(/concurrency_model::loom$/)'` picks them out - // unambiguously. + // Backend-named leaves let nextest isolate tests that use model-checker primitives. quote! { #[cfg(not(any(feature = "loom", feature = "shuttle")))] #[::core::prelude::v1::test] @@ -193,6 +172,72 @@ pub fn test(_attr: TokenStream, item: TokenStream) -> TokenStream { fn shuttle() { #krate::stress(|| #block); } + + } + } + } + .into() +} + +/// Give a test a backend-named leaf without wrapping its body in `stress`. +/// +/// Use this when a generator is the outer loop and invokes `stress` per generated case: +/// +/// ```ignore +/// bolero::check!().with_type().cloned().for_each(|scenario: Scenario| { +/// concurrency::stress(move || scenario.run()); // one exploration per generated shape +/// }); +/// ``` +/// +/// The leaf is named `plain`, `loom`, or `shuttle`, allowing the same nextest filters used by +/// [`macro@test`]. +#[proc_macro_attribute] +pub fn model_test(_attr: TokenStream, item: TokenStream) -> TokenStream { + let func = parse_macro_input!(item as ItemFn); + + let attrs = &func.attrs; + let sig = &func.sig; + let block = &func.block; + let fn_name = &sig.ident; + + if let Some(asyncness) = sig.asyncness { + return syn::Error::new_spanned( + asyncness, + "#[concurrency::model_test] does not support async functions yet", + ) + .to_compile_error() + .into(); + } + if !sig.inputs.is_empty() { + return syn::Error::new_spanned( + &sig.inputs, + "#[concurrency::model_test] functions must take no arguments", + ) + .to_compile_error() + .into(); + } + + quote! { + #[allow(non_snake_case)] + mod #fn_name { + use super::*; + mod concurrency_model { + use super::*; + + #[cfg(feature = "loom")] + #[::core::prelude::v1::test] + #(#attrs)* + fn loom() #block + + #[cfg(feature = "shuttle")] + #[::core::prelude::v1::test] + #(#attrs)* + fn shuttle() #block + + #[cfg(not(any(feature = "loom", feature = "shuttle")))] + #[::core::prelude::v1::test] + #(#attrs)* + fn plain() #block } } } diff --git a/concurrency/src/macros.rs b/concurrency/src/macros.rs index 63e88ef4f6..e290af389d 100644 --- a/concurrency/src/macros.rs +++ b/concurrency/src/macros.rs @@ -160,4 +160,4 @@ macro_rules! with_std { ($($item:item)*) => {}; } -pub use concurrency_macros::{concurrency_mode, test}; +pub use concurrency_macros::{concurrency_mode, model_test, test}; diff --git a/dataplane/src/packet_processor/mod.rs b/dataplane/src/packet_processor/mod.rs index 4e9190c5ce..6ca5d04d3c 100644 --- a/dataplane/src/packet_processor/mod.rs +++ b/dataplane/src/packet_processor/mod.rs @@ -122,6 +122,7 @@ pub(crate) fn start_router( "port-forwarder", portfw_factory.handle(), flow_table_clone.clone(), + natallocator_factory.handle(), ); let pkt_stats_nf = PacketStatsNF::new(pkt_stats.clone()); diff --git a/development/code/running-tests.md b/development/code/running-tests.md index 697aa1e099..f15e435f45 100644 --- a/development/code/running-tests.md +++ b/development/code/running-tests.md @@ -62,8 +62,75 @@ change this. The major downside is that these processes are very computationally intensive and can take a long time to run. In fact, the [afl] fuzzer runs until you terminate it. +## Running a real fuzzing campaign + +To run a target under [libfuzzer], which is coverage guided and explores far deeper than the random +driver the test suite uses, list the targets and pick one: + +```shell +just fuzz-list -p dataplane-nat +just fuzz 'masquerade::apalloc::region::bolero_tests::decompose_properties' 10min -p dataplane-nat +``` + +The duration defaults to `60s`; anything after it is forwarded to `cargo bolero test`. As a sense of +the difference, a property that manages a few thousand cases per second under `just test` reaches +several hundred thousand per minute here, because libfuzzer mutates towards inputs that reach new +code rather than sampling blindly. + +Findings are written to a `__fuzz__` directory beside the test. That directory is gitignored: the +corpus is a local artifact that seeds later runs on the same machine, not something to commit. + +Pass `-j` to spread the campaign over more cores, which is the cheapest way to reach deeper: + +```shell +just fuzz 'some::module::tests::some_property' 10min -p some-package -j 60 +``` + +Each worker then writes a `fuzz-.log` into the directory you ran from, rather than into +`__fuzz__`. Those are gitignored too, and are only worth reading when a run reports a crash. + +### Sanitizers + +`cargo bolero` builds with the `fuzz` profile and links [AddressSanitizer] unless told otherwise, so +a plain `just fuzz` is already an asan campaign. To swap sanitizers, set the same `sanitize` +variable the rest of the justfile uses: + +```shell +just sanitize=thread fuzz 'some::module::tests::some_property' 5min -p some-package +``` + +[ThreadSanitizer] only reports on a target that actually spawns threads, so it is worth the extra +cost on a concurrency suite and close to pointless on a single-threaded property. It also takes +much longer to get going, because thread instrumentation changes the ABI: `just` therefore adds +`--build-std` for it, since a std left uninstrumented fails the build on a mismatch against `core`. + +A sanitizer is not free. Instrumentation costs roughly a factor of four in executions per second, +so it is worth spending some of a campaign with none at all, reaching deeper into the input space +in exchange for only catching what the test's own assertions catch: + +```shell +just sanitize=NONE fuzz 'some::module::tests::some_property' 30min -p some-package +``` + +The two are complementary: asan for memory errors the assertions cannot see, `NONE` for depth. + +The suite as a whole can also be run under either sanitizer with the standard runner, which is what +CI's `sanitize/fuzz/*` jobs do: + +```shell +just profile=fuzz sanitize=thread test +just profile=fuzz sanitize=address test +``` + +That covers far more code than a single fuzz target, but only with the brief random driver rather +than a real campaign. The two are complementary. + > [!NOTE] -> Dedicated `just` recipes for running full fuzz campaigns (with libfuzzer/afl) are planned for a future PR. +> `just fuzz` passes `--rustc-bootstrap`, because libfuzzer wants a nightly compiler for its +> sanitizer coverage flags while the pinned toolchain is stable. An [afl] recipe is still to come. + +[AddressSanitizer]: https://clang.llvm.org/docs/AddressSanitizer.html +[ThreadSanitizer]: https://clang.llvm.org/docs/ThreadSanitizer.html [README.md]: ../../README.md [afl]: https://github.com/AFLplusplus/AFLplusplus diff --git a/flow-entry/src/flow_table/concurrent_fuzz.rs b/flow-entry/src/flow_table/concurrent_fuzz.rs index f5b649b47d..8403c5ca8d 100644 --- a/flow-entry/src/flow_table/concurrent_fuzz.rs +++ b/flow-entry/src/flow_table/concurrent_fuzz.rs @@ -296,7 +296,7 @@ impl Scenario { /// Drive one bolero shape per iteration through [`concurrency::stress`]: /// a single direct run on the std backend (real OS threads — build with /// `just test sanitize=thread`), or the full portfolio under shuttle. -#[test] +#[concurrency::model_test] fn stress_test_concurrency_model() { // Single-threaded runtime is enough: we never need the timer task to // run, only a context for `insert`'s `tokio::task::spawn` to succeed. diff --git a/justfile b/justfile index 06fbe9acd4..ec713f98e3 100644 --- a/justfile +++ b/justfile @@ -170,6 +170,32 @@ test package="tests.all" *args: (build (if package == "tests.all" { "tests.all" declare -r target="{{ if package == "tests.all" { "tests.all" } else { "tests.pkg." + package } }}" cargo nextest run --archive-file results/${target}/*.tar.zst --workspace-remap $(pwd) {{ filter }} +# List the bolero targets `just fuzz` can run. Args go to `cargo bolero list` +[script] +fuzz-list *args="": + {{ _just_debuggable_ }} + cargo bolero list {{ _cargo_feature_flags }} {{ args }} + +# Fuzz one bolero target under libfuzzer. See development/code/running-tests.md +[script] +fuzz target time="60s" *args="": + {{ _just_debuggable_ }} + # libfuzzer wants a nightly compiler for its sanitizer coverage flags, while the + # pinned toolchain is stable; --rustc-bootstrap bridges that. cargo-bolero already + # builds with the fuzz profile and links AddressSanitizer unless told otherwise, so + # a plain `just fuzz` is already an asan run. Findings land in a gitignored + # `__fuzz__` directory beside the test. + # + # `sanitize=thread` additionally rebuilds std: thread instrumentation changes the + # ABI, so a std left uninstrumented fails the build on a mismatch against `core`. + # asan does not need that, and skipping the std rebuild keeps it far quicker. + # `sanitize=NONE` drops instrumentation altogether, which buys roughly four times + # the executions per second in exchange for only catching what the test asserts. + cargo bolero test '{{ target }}' --rustc-bootstrap -T '{{ time }}' \ + {{ if sanitize != "" { "--sanitizer " + sanitize } else { "" } }} \ + {{ if sanitize == "thread" { "--build-std" } else { "" } }} \ + {{ _cargo_feature_flags }} {{ args }} + # Build and run the criterion benches. The rte_acl benches are gated behind the # `dpdk` feature, so run `just features=dpdk bench` to exercise them; a plain # `just bench` builds them as empty `main()` and only runs the reference benches. @@ -444,24 +470,20 @@ doctest *args: {{ _just_debuggable_ }} cargo test --doc {{ _cargo_feature_flags }} {{ _cargo_profile_flag }} {{ args }} -# Run tests with code coverage. Args will be forwarded to nextest +# Run instrumented tests and report coverage. Args are forwarded to nextest; for example, +# `just coverage -p dataplane-nat` scopes the run to this crate. [script] -coverage target="tests.all" *args: (build (if target == "tests.all" { "tests.all" } else { "tests.pkg." + target }) args) +coverage *args: {{ _just_debuggable_ }} - declare -r target="{{ if target == "tests.all" { "tests.all" } else { "tests.pkg." + target } }}" export LLVM_COV="$(pwd)/devroot/bin/llvm-cov" export LLVM_PROFDATA="$(pwd)/devroot/bin/llvm-profdata" - export CARGO_LLVM_COV_TARGET_DIR="$(pwd)/target/llvm-cov" - export CARGO_LLVM_COV_BUILD_DIR="$(pwd)" - cargo llvm-cov clean - cargo llvm-cov show-env - cargo llvm-cov --no-report --branch nextest --archive-file "./results/${target}/"*.tar.zst --workspace-remap . {{ args }} - # NOTE: --profile="" is intentional. When collecting coverage from a nextest archive, the - # profile path component that cargo-llvm-cov normally expects in the profdata directory is - # absent. Passing an empty profile string removes that component from the lookup path so - # the tool can find the profdata generated by the archive run above. - cargo llvm-cov report --html --profile="" --output-dir=./target/nextest/coverage - cargo llvm-cov --branch report --codecov --profile="" --output-path=./target/nextest/coverage/codecov.json + declare -r out="./target/nextest/coverage" + cargo llvm-cov clean --workspace + cargo llvm-cov --no-report --branch nextest {{ args }} + mkdir -p "${out}" + cargo llvm-cov report --branch --html --output-dir="${out}" + cargo llvm-cov report --branch --codecov --output-path="${out}/codecov.json" + cargo llvm-cov report --branch --summary-only # Regenerate the dependency graph for the project [script] diff --git a/miri.just b/miri.just index d56cd986d8..495bb1a790 100644 --- a/miri.just +++ b/miri.just @@ -39,8 +39,11 @@ test *args="": {{ _just_debuggable_ }} declare -ri START_SEED="$((10#$schedule_seed))" declare -ri END_SEED="$((START_SEED + ${seeds}))" + # Parallel Miri workers cannot share an incremental compilation session. + declare -rx CARGO_INCREMENTAL=0 declare MIRIFLAGS="" - declare RUSTFLAGS="" + # Environment RUSTFLAGS replace the cargo-configured flags. + declare RUSTFLAGS="--cfg=tokio_unstable --check-cfg=cfg(emulated) " MIRIFLAGS+="-Zmiri-compare-exchange-weak-failure-rate=${weak_failure_rate} " MIRIFLAGS+="-Zmiri-disable-isolation " MIRIFLAGS+="-Zmiri-many-seeds=${START_SEED}..${END_SEED} " diff --git a/nat/src/masquerade/allocation.rs b/nat/src/masquerade/allocation.rs index 4a5bed08d1..aace14e208 100644 --- a/nat/src/masquerade/allocation.rs +++ b/nat/src/masquerade/allocation.rs @@ -35,6 +35,27 @@ pub enum AllocatorError { NoPoolFound, } +impl AllocatorError { + /// Whether a caller may try another allocator because this one ran out of space. + /// + /// The exhaustive match must agree with the errors mapped to `NatOutOfResources`. + #[must_use] + pub fn is_exhaustion(&self) -> bool { + match self { + AllocatorError::NoFreeIp + | AllocatorError::NoPortBlock + | AllocatorError::NoFreePort(_) => true, + AllocatorError::PortAllocationFailed(_) + | AllocatorError::PortReservationFailed(_) + | AllocatorError::UnsupportedProtocol(_) + | AllocatorError::MissingDiscriminant + | AllocatorError::InternalIssue(_) + | AllocatorError::Denied + | AllocatorError::NoPoolFound => false, + } + } +} + impl From<&AllocatorError> for DoneReason { fn from(error: &AllocatorError) -> Self { match error { @@ -70,3 +91,106 @@ impl Display for AllocationResult { ) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::port::NatPortError; + + // Keep the exhaustive name match and test values in one table. + macro_rules! error_table { + ($($pattern:pat, $name:literal, $value:expr);+ $(;)?) => { + fn every_error() -> Vec { + vec![$($value),+] + } + + fn name(error: &AllocatorError) -> &'static str { + match error { + $($pattern => $name),+ + } + } + }; + } + + error_table! { + AllocatorError::NoFreeIp, "NoFreeIp", AllocatorError::NoFreeIp; + AllocatorError::NoPortBlock, "NoPortBlock", AllocatorError::NoPortBlock; + AllocatorError::NoFreePort(_), "NoFreePort", AllocatorError::NoFreePort(1024); + AllocatorError::PortAllocationFailed(_), "PortAllocationFailed", + AllocatorError::PortAllocationFailed(NatPortError::InvalidPort(0)); + AllocatorError::PortReservationFailed(_), "PortReservationFailed", + AllocatorError::PortReservationFailed(8080); + AllocatorError::UnsupportedProtocol(_), "UnsupportedProtocol", + AllocatorError::UnsupportedProtocol(NextHeader::TCP); + AllocatorError::MissingDiscriminant, "MissingDiscriminant", + AllocatorError::MissingDiscriminant; + AllocatorError::InternalIssue(_), "InternalIssue", + AllocatorError::InternalIssue("bookkeeping".to_string()); + AllocatorError::Denied, "Denied", AllocatorError::Denied; + AllocatorError::NoPoolFound, "NoPoolFound", AllocatorError::NoPoolFound; + } + + // Catch rows whose value does not match the named variant. + #[test] + fn every_error_appears_in_the_table_exactly_once() { + let mut names: Vec<&str> = every_error().iter().map(name).collect(); + let before = names.len(); + names.sort_unstable(); + names.dedup(); + assert_eq!( + names.len(), + before, + "a row's value does not match the variant it names, so an error is listed twice \ + and another not at all: {names:?}" + ); + } + + /// Only space exhaustion permits trying another allocator. + #[test] + fn exhaustion_is_exactly_running_out_of_space() { + let exhausting = ["NoFreeIp", "NoPortBlock", "NoFreePort"]; + for error in every_error() { + assert_eq!( + error.is_exhaustion(), + exhausting.contains(&name(&error)), + "{} is classified wrongly by is_exhaustion", + name(&error) + ); + } + } + + #[test] + fn each_error_reaches_the_packet_as_the_right_outcome() { + for error in every_error() { + let expected = match name(&error) { + "NoFreeIp" | "NoPortBlock" | "NoFreePort" => DoneReason::NatOutOfResources, + "UnsupportedProtocol" => DoneReason::NatUnsupportedProto, + "PortAllocationFailed" | "PortReservationFailed" | "MissingDiscriminant" => { + DoneReason::NatFailure + } + "InternalIssue" => DoneReason::InternalFailure, + "Denied" | "NoPoolFound" => DoneReason::Filtered, + other => unreachable!("{other} has no expected outcome"), + }; + assert_eq!( + DoneReason::from(&error), + expected, + "{} reaches the packet as the wrong outcome", + name(&error) + ); + } + } + + /// Exhaustion errors must map to `NatOutOfResources`, and only those errors may do so. + #[test] + fn the_two_classifications_agree() { + for error in every_error() { + assert_eq!( + error.is_exhaustion(), + DoneReason::from(&error) == DoneReason::NatOutOfResources, + "{} is exhaustion to one classification and not the other", + name(&error) + ); + } + } +} diff --git a/nat/src/masquerade/allocator_writer.rs b/nat/src/masquerade/allocator_writer.rs index fe8469f477..3a0e2789ed 100644 --- a/nat/src/masquerade/allocator_writer.rs +++ b/nat/src/masquerade/allocator_writer.rs @@ -11,8 +11,8 @@ use flow_entry::flow_table::FlowTable; use net::packet::VpcDiscriminant; use tracing::debug; -use crate::masquerade::flows::check_masquerading_flows; -use crate::masquerade::flows::invalidate_all_masquerading_flows; +use crate::masquerade::flows::reconcile_nat_flows; +use crate::masquerade::flows::remove_allocator_from_flows; use crate::masquerade::flows::upgrade_all_masquerading_flows; #[derive(Debug, PartialEq, Clone)] @@ -100,12 +100,10 @@ impl NatAllocatorWriter { self.get_reader().factory() } - /// Replace the nat allocator with a new one for the new config. If the config is such that - /// no masquerading is needed no allocator will be stored and the existing one, if any, be - /// removed. Flows using that allocator will be cancelled. If, instead, a new, distinct - /// masquerading config is provided, a new allocator will be installed and the flows using the - /// previous one be either invalidated or adapted to use the new allocator: their ports/ips - /// will be transferred (reserved) in the new allocator. + /// Install the allocator for a new NAT configuration. + /// + /// Removing masquerade invalidates NAT flows and clears port-forward leases. Replacement + /// carries compatible allocations forward. pub fn update_nat_allocator( &mut self, nat_config: MasqueradeConfig, @@ -127,25 +125,18 @@ impl NatAllocatorWriter { // if we transition to a config without masquerading, flush allocator and remove all flows if !nat_config.has_masquerading_peerings() { if curr_allocator.is_some() { - debug!("No masquerade is required anymore: will invalidate flows"); + debug!("Removing masquerade allocator and its flow state"); self.0.store(None); - invalidate_all_masquerading_flows(flow_table); + remove_allocator_from_flows(flow_table); } return; } - let mut allocator = NatAllocator::new(nat_config, genid); - if curr_allocator.is_some() { - let guard = check_masquerading_flows(flow_table, &mut allocator); - debug!("Replacing masquerade NAT allocator..."); - self.0.store(Some(Arc::new(allocator))); - debug!("NAT allocator has been replaced"); - drop(guard); - } else { - debug!("Installing new masquerade NAT allocator..."); - self.0.store(Some(Arc::new(allocator))); - debug!("NAT allocator is installed"); - } + let allocator = NatAllocator::new(nat_config, genid); + let guard = reconcile_nat_flows(flow_table, &allocator); + debug!("Installing masquerade NAT allocator..."); + self.0.store(Some(Arc::new(allocator))); + drop(guard); } } diff --git a/nat/src/masquerade/apalloc/alloc.rs b/nat/src/masquerade/apalloc/alloc.rs index ae7886c33f..fbfd8f5cdf 100644 --- a/nat/src/masquerade/apalloc/alloc.rs +++ b/nat/src/masquerade/apalloc/alloc.rs @@ -1,36 +1,26 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Open Network Fabric Authors -//! IP allocation components for the default allocator for masquerade. -//! -//! This submodule focuses on allocating IP addresses, and it gets an address, calls the methods -//! from its port allocator to allocate ports for this IP address. The [`IpAllocator`] is the main -//! entry point. -//! -//! See also the architecture diagram at the top of mod.rs. +//! Masquerade IP allocation. See the architecture diagram in `mod.rs`. +use super::region::AddrInterval; use super::{NatIpWithBitmap, port_alloc}; use crate::masquerade::allocation::AllocatorError; use crate::masquerade::natip::NatIp; use crate::port::NatPort; use crate::ranges::IpRange; use concurrency::sync::{Arc, RwLock, RwLockReadGuard, Weak}; -use lpm::prefix::range_map::DisjointRangesBTreeMap; -use lpm::prefix::{IpPrefix, PortRange, Prefix}; use roaring::RoaringBitmap; use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::net::{IpAddr, Ipv6Addr}; use std::time::Duration; -use tracing::debug; +use tracing::{debug, error}; /////////////////////////////////////////////////////////////////////////////// // IpAllocator /////////////////////////////////////////////////////////////////////////////// -/// [`IpAllocator`] is a thread-safe allocator for IP addresses. It wraps around a [`NatPool`] -/// object that contains IP availables for a given -/// [`VpcExpose`](config::external::overlay::vpcpeering::VpcExpose). It can allocate an IP and -/// (using this IP) a port. +/// Thread-safe allocation of addresses and their ports from a [`NatPool`]. #[derive(Debug, Clone)] pub(crate) struct IpAllocator { pool: Arc>>, @@ -49,10 +39,6 @@ impl IpAllocator { self.pool.read() } - pub(crate) fn idle_timeout(&self) -> Duration { - self.pool.read().idle_timeout() - } - fn deallocate_ip(&self, ip: I) { self.pool.write().deallocate_from_pool(ip); } @@ -61,25 +47,38 @@ impl IpAllocator { &self, allow_null: bool, ) -> Result, AllocatorError> { - let allocated_ips = self.pool.read(); - for ip_weak in allocated_ips.ips_in_use() { - let Some(ip) = ip_weak.upgrade() else { - continue; - }; - if !ip.has_free_ports() { - continue; - } - match ip.allocate_port_for_ip(allow_null) { - Ok(port) => { - debug!("Allocated port {port}"); - return Ok(port); + // Keep upgraded addresses alive until the read guard is gone. Their drop path takes this + // pool's write lock. + let mut examined: Vec>> = Vec::new(); + let outcome = { + let allocated_ips = self.pool.read(); + let mut outcome = Err(AllocatorError::NoFreeIp); + for ip_weak in allocated_ips.ips_in_use() { + let Some(ip) = ip_weak.upgrade() else { + continue; + }; + examined.push(ip.clone()); + if !ip.has_free_ports() { + continue; + } + match ip.allocate_port_for_ip(allow_null) { + Ok(port) => { + debug!("Allocated port {port}"); + outcome = Ok(port); + break; + } + // If there is no free port left, loop again to try another IP address + Err(AllocatorError::NoFreePort(_)) => {} + Err(e) => { + outcome = Err(e); + break; + } } - // If there is no free port left, loop again to try another IP address - Err(AllocatorError::NoFreePort(_)) => {} - Err(e) => return Err(e), } - } - Err(AllocatorError::NoFreeIp) + outcome + }; + drop(examined); + outcome } fn allocate_new_ip_from_pool(&self) -> Result>, AllocatorError> { @@ -100,8 +99,13 @@ impl IpAllocator { } fn cleanup_used_ips(&self) { - let mut allocated_ips = self.pool.write(); - allocated_ips.cleanup(); + // Release upgraded entries after the pool's write guard. See `reuse_allocated_ip`. + let mut released = Vec::new(); + { + let mut allocated_ips = self.pool.write(); + allocated_ips.cleanup(&mut released); + } + drop(released); } pub(crate) fn allocate( @@ -111,16 +115,24 @@ impl IpAllocator { // FIXME: Should we clean up every time?? self.cleanup_used_ips(); - if let Ok(port) = self.reuse_allocated_ip(allow_null) { - return Ok(port); + // Draw a fresh address only when the addresses already in use are exhausted. Other errors + // describe allocator failure and must be preserved. + match self.reuse_allocated_ip(allow_null) { + Ok(port) => Ok(port), + Err(e) if e.is_exhaustion() => self.allocate_from_new_ip(allow_null), + Err(e) => Err(e), } - self.allocate_from_new_ip(allow_null) } fn get_allocated_ip(&self, ip: I) -> Result>, AllocatorError> { - self.pool - .write() - .reserve_from_pool(ip, self.clone(), self.randomize) + // Keep upgrades alive past the pool guard. See `cleanup_used_ips`. + let mut examined = Vec::new(); + let outcome = + self.pool + .write() + .reserve_from_pool(ip, self.clone(), self.randomize, &mut examined); + drop(examined); + outcome } pub(crate) fn reserve( @@ -140,6 +152,91 @@ impl IpAllocator { } } +/////////////////////////////////////////////////////////////////////////////// +// PoolSet +/////////////////////////////////////////////////////////////////////////////// + +/// One region of the public address space, and the allocator that owns it. +#[derive(Debug, Clone)] +pub(crate) struct PoolRegion { + range: AddrInterval, + allocator: IpAllocator, +} + +impl PoolRegion { + pub(crate) fn range(&self) -> AddrInterval { + self.range + } + + pub(crate) fn allocator(&self) -> &IpAllocator { + &self.allocator + } +} + +/// The ordered regions and settings belonging to one expose. +/// +/// Shared regions share an allocator, keeping public tuples unique across exposes. +#[derive(Debug, Clone)] +pub(crate) struct PoolSet { + regions: Vec>, + idle_timeout: Duration, +} + +impl PoolSet { + pub(crate) fn new(idle_timeout: Duration) -> Self { + Self { + regions: Vec::new(), + idle_timeout, + } + } + + pub(crate) fn push_region(&mut self, range: AddrInterval, allocator: IpAllocator) { + self.regions.push(PoolRegion { range, allocator }); + } + + pub(crate) fn idle_timeout(&self) -> Duration { + self.idle_timeout + } + + pub(crate) fn regions(&self) -> impl Iterator> { + self.regions.iter() + } + + /// Allocate from the first region with room, preserving non-exhaustion errors. + pub(crate) fn allocate( + &self, + allow_null: bool, + ) -> Result, AllocatorError> { + let mut exhausted = None; + for region in &self.regions { + match region.allocator.allocate(allow_null) { + Ok(port) => return Ok(port), + Err(e) if e.is_exhaustion() => { + debug!("Region {:?} is out of space: {e}", region.range); + exhausted = Some(e); + } + Err(e) => return Err(e), + } + } + Err(exhausted.unwrap_or(AllocatorError::NoFreeIp)) + } + + /// Reserve a specific address and port, which has to come from the region owning that address. + pub(crate) fn reserve( + &self, + ip: I, + port: NatPort, + ) -> Result, AllocatorError> { + let bits = ip.to_addr_bits(); + let region = self + .regions + .iter() + .find(|region| region.range.contains(bits)) + .ok_or(AllocatorError::NoPoolFound)?; + region.allocator.reserve(ip, port) + } +} + /////////////////////////////////////////////////////////////////////////////// // AllocatedIp /////////////////////////////////////////////////////////////////////////////// @@ -159,17 +256,12 @@ impl AllocatedIp { fn new( ip: I, ip_allocator: IpAllocator, - reserved_port_range: Option, randomize: bool, exclude_wellknown_ports: bool, ) -> Self { Self { ip, - port_allocator: port_alloc::PortAllocator::new( - reserved_port_range, - randomize, - exclude_wellknown_ports, - ), + port_allocator: port_alloc::PortAllocator::new(randomize, exclude_wellknown_ports), ip_allocator, } } @@ -228,33 +320,38 @@ impl Drop for AllocatedIp { /// A [`NatPool`] is a pool of IP addresses that can be allocated from. It contains a bitmap of /// available IP addresses, and a list of weak references to [`AllocatedIp`] objects representing /// the allocated IPs potentially available for use (if they still have free ports) -#[derive(Debug, Clone)] +#[derive(Debug)] pub(crate) struct NatPool { bitmap: PoolBitmap, bitmap_mapping: BTreeMap, reverse_bitmap_mapping: BTreeMap, in_use: VecDeque>>, - reserved_prefixes_ports: Option>, - idle_timeout: Duration, exclude_wellknown_ports: bool, } impl NatPool { - pub(crate) fn new( - bitmap: PoolBitmap, - bitmap_mapping: BTreeMap, - reverse_bitmap_mapping: BTreeMap, - reserved_prefixes_ports: Option>, - idle_timeout: Duration, - exclude_wellknown_ports: bool, - ) -> Self { + /// Build a pool over one disjoint public region. + pub(crate) fn for_range(range: AddrInterval, exclude_wellknown_ports: bool) -> Self { + // IPv6 uses offsets from the region start because its addresses do not fit in the bitmap. + let bitmap_mapping = BTreeMap::from([(0u32, range.start)]); + let reverse_bitmap_mapping = BTreeMap::from([(range.start, 0u32)]); + + // A region holding more addresses than the u32 bitmap can index is truncated. We would run + // out of memory long before allocating four billion addresses. + let span = range.len().saturating_sub(1).min(u128::from(u32::MAX)); + let to_offset = |bits: u128| { + let address = I::try_from_bits(bits).unwrap_or_else(|()| unreachable!()); + I::try_to_offset(address, &reverse_bitmap_mapping).unwrap_or_else(|_| unreachable!()) + }; + Self { - bitmap, + bitmap: PoolBitmap::with_offset_range( + to_offset(range.start), + to_offset(range.start + span), + ), bitmap_mapping, reverse_bitmap_mapping, in_use: VecDeque::new(), - reserved_prefixes_ports, - idle_timeout, exclude_wellknown_ports, } } @@ -263,30 +360,22 @@ impl NatPool { self.in_use.push_back(Arc::downgrade(ip)); } - fn cleanup(&mut self) { - self.in_use.retain(|ip| ip.upgrade().is_some()); - } - - pub(crate) fn idle_timeout(&self) -> Duration { - self.idle_timeout + /// Drop the entries whose addresses are gone, handing the caller every address that is still + /// alive so it can release them once the pool lock is no longer held. See `cleanup_used_ips`. + fn cleanup(&mut self, keep_alive: &mut Vec>>) { + self.in_use.retain(|ip| match ip.upgrade() { + Some(alive) => { + keep_alive.push(alive); + true + } + None => false, + }); } pub(crate) fn ips_in_use(&self) -> impl Iterator>> { self.in_use.iter() } - // Used for Display - pub(crate) fn reserved_prefixes_ports( - &self, - ) -> Option> { - Some( - self.reserved_prefixes_ports - .as_ref()? - .iter() - .map(|(&r, &p)| (r, p)), - ) - } - fn use_new_ip( &mut self, ip_allocator: IpAllocator, @@ -297,16 +386,9 @@ impl NatPool { let ip = I::try_from_offset(offset, &self.bitmap_mapping)?; - // Check if the IP is in a reserved prefix, retrieve the reserved port range if any - let reserved_port_range = self - .reserved_prefixes_ports - .as_ref() - .and_then(|ranges| ranges.lookup(&ip.to_ip_addr()).map(|(_, range)| *range)); - Ok(AllocatedIp::new( ip, ip_allocator, - reserved_port_range, randomize, self.exclude_wellknown_ports, )) @@ -314,22 +396,34 @@ impl NatPool { fn deallocate_from_pool(&mut self, ip: I) { debug!("Address {ip} was deallocated"); - let offset = I::try_to_offset(ip, &self.reverse_bitmap_mapping).unwrap(); - self.bitmap.set_ip_free(offset); + // The address was handed out by this pool, so it maps back into it. This runs while an + // allocation is being dropped and has nowhere to report a failure, so say so and leave the + // address marked in use rather than panicking on the drop path. + match I::try_to_offset(ip, &self.reverse_bitmap_mapping) { + Ok(offset) => { + self.bitmap.set_ip_free(offset); + } + Err(e) => error!("Address {ip} does not map back into the pool it came from: {e}"), + } } + /// `keep_alive` collects every address upgraded here, for the caller to release once the pool + /// lock is gone. See `cleanup_used_ips`. fn reserve_from_pool( &mut self, ip: I, ip_allocator: IpAllocator, randomize: bool, + keep_alive: &mut Vec>>, ) -> Result>, AllocatorError> { let offset = I::try_to_offset(ip, &self.reverse_bitmap_mapping)?; for ip_weak in self.ips_in_use() { - if let Some(ip_arc) = ip_weak.upgrade() - && ip_arc.ip() == ip - { + let Some(ip_arc) = ip_weak.upgrade() else { + continue; + }; + keep_alive.push(ip_arc.clone()); + if ip_arc.ip() == ip { // We found the allocated IP in the list of IPs in use, return it debug!("Reserved ip {ip_arc}"); return Ok(ip_arc); @@ -347,10 +441,7 @@ impl NatPool { let arc_ip = Arc::new(AllocatedIp::new( ip, ip_allocator, - None, randomize, - // Keep the low-port exclusion policy for explicitly reserved IPs as well, so - // reserve() follows the same TCP/UDP allocation rules as allocate(). self.exclude_wellknown_ports, )); self.add_in_use(&arc_ip); @@ -418,8 +509,11 @@ impl NatPool { pub(crate) struct PoolBitmap(RoaringBitmap); impl PoolBitmap { - pub(crate) fn new() -> Self { - Self(RoaringBitmap::new()) + /// Mark every index in the inclusive range as free. + pub(crate) fn with_offset_range(start: u32, end: u32) -> Self { + let mut bitmap = RoaringBitmap::new(); + bitmap.insert_range(start..=end); + Self(bitmap) } fn pop_ip(&mut self) -> Result { @@ -435,21 +529,6 @@ impl PoolBitmap { fn set_ip_free(&mut self, index: u32) -> bool { self.0.insert(index) } - - pub(crate) fn add_prefix(&mut self, prefix: &Prefix, bitmap_mapping: &BTreeMap) { - match prefix { - Prefix::IPV4(p) => { - let start = p.network().to_bits(); - let end = p.last_address().to_bits(); - self.0.insert_range(start..=end); - } - Prefix::IPV6(p) => { - let start = map_address(p.network(), bitmap_mapping); - let end = map_address(p.last_address(), bitmap_mapping); - self.0.insert_range(start..=end); - } - } - } } /////////////////////////////////////////////////////////////////////////////// @@ -460,12 +539,7 @@ pub(crate) fn map_offset( offset: u32, bitmap_mapping: &BTreeMap, ) -> Result { - // Field bitmap_mapping is a BTreeMap that associates, to each given u32 offset, an IPv6 - // address, as a u128, corresponding to the network address of the corresponding prefix in - // the list. - // Here we lookup for the closest lower offset in the tree, which returns the network - // address for the prefix start address and its offset, and we deduce the IPv6 address we're - // looking for. + // Find the closest mapped prefix and apply the remaining offset. let (prefix_offset, prefix_start_bits) = bitmap_mapping .range(..=offset) @@ -480,11 +554,25 @@ pub(crate) fn map_offset( } // Reverse operation from map_offset() -pub(crate) fn map_address(address: Ipv6Addr, bitmap_mapping: &BTreeMap) -> u32 { +// +// Not every address of a region has an offset. A region may hold more addresses than a u32 can +// index, in which case the bitmap covers only the first 2^32 of them (see `NatPool::for_range`), +// and an address beyond that is simply not one this pool can serve. That is reachable from +// configuration rather than a bug: a flow carried across a config change presents the address it +// already holds, and the region it falls in may have grown downwards underneath it. Report it as +// the pool not serving the address, so the flow is dropped like any other that cannot be carried +// over, rather than panicking in the middle of applying a config. +pub(crate) fn map_address( + address: Ipv6Addr, + bitmap_mapping: &BTreeMap, +) -> Result { let (prefix_start_bits, prefix_offset) = bitmap_mapping .range(..=address.to_bits()) .next_back() - .expect("This should never fail"); + .ok_or(AllocatorError::NoPoolFound)?; - prefix_offset + u32::try_from(address.to_bits() - prefix_start_bits).unwrap() + u32::try_from(address.to_bits() - prefix_start_bits) + .ok() + .and_then(|offset| prefix_offset.checked_add(offset)) + .ok_or(AllocatorError::NoPoolFound) } diff --git a/nat/src/masquerade/apalloc/concurrent_fuzz.rs b/nat/src/masquerade/apalloc/concurrent_fuzz.rs new file mode 100644 index 0000000000..f4caa2ffd8 --- /dev/null +++ b/nat/src/masquerade/apalloc/concurrent_fuzz.rs @@ -0,0 +1,488 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Concurrent property tests for masquerade allocator replacement. +//! +//! Bolero generates ranges and per-thread operations; `concurrency::stress` explores each fixed +//! scenario. Packet threads allocate from the published generation while a writer re-reserves +//! surviving tuples in a replacement and publishes it. +//! +//! The suite checks that live and carried tuples remain unique and that contention never produces +//! [`AllocatorError::InternalIssue`]. Some scenarios retain every allocation for an exact +//! uniqueness oracle; others exercise release paths. +//! +//! Loom is excluded because its `Weak` shim keeps liveness entries alive. The suite uses +//! `model_test` because it invokes `stress` inside Bolero's outer loop. + +#![cfg(test)] +#![cfg(not(feature = "loom"))] + +use super::AllocatedPort; +use super::alloc::PoolSet; +use super::region::AddrInterval; +use super::setup::{PoolSpec, pool_sets_for_specs}; +use crate::masquerade::allocation::AllocatorError; +use crate::port::NatPort; +use concurrency::slot::SlotOption; +use concurrency::sync::{Arc, Mutex}; +use concurrency::thread; +// `spawn_scoped` is inherent on std's `Builder`, but supplied by `BuilderExt` under shuttle +#[cfg_attr(not(feature = "shuttle"), allow(unused_imports))] +use concurrency::thread::BuilderExt; +use net::ip::NextHeader; +use std::collections::BTreeSet; +use std::net::Ipv4Addr; +use std::time::Duration; + +// 10.1.0.0, over a window narrow enough that generated ranges overlap most of the time and the +// pools stay cheap to build once per published generation. +const BASE: u128 = 0x0A01_0000; +const WINDOW: u128 = 8; +const MAX_EXPOSES: u8 = 3; +const MAX_RANGE_LEN: u8 = 4; + +// Op streams are kept short: the backend explores interleavings of a fixed shape, so length costs +// schedule space without buying coverage. +const MAX_PACKET_OPS: usize = 6; +const MAX_CONFIG_OPS: usize = 3; +const PACKET_WORKERS: usize = 2; + +const IDLE_TIMEOUT: Duration = Duration::from_mins(2); + +/// What a packet thread does. Allocating and freeing model flows starting and ending; reserving +/// models a flow being carried over, and is the op that races reservation against allocation on +/// pools that are already published and in use. +#[derive(Clone, Copy, Debug, bolero::TypeGenerator)] +enum PacketOp { + Allocate, + FreeOldest, + ReserveExisting, +} + +/// What the config thread does. Production has a single writer, so only this thread republishes. +#[derive(Clone, Copy, Debug, bolero::TypeGenerator)] +enum ConfigOp { + Republish, + Idle, +} + +/// One generated shape: the public ranges each expose claims, and an op stream per thread. +#[derive(Clone, Debug)] +struct Scenario { + ranges: Vec>, + packet_ops: [Vec; PACKET_WORKERS], + config_ops: Vec, + /// Whether this scenario exercises release paths instead of an exact monotone oracle. + frees_allowed: bool, +} + +impl bolero::TypeGenerator for Scenario { + /// Ensure every generated shape has concurrent packet and configuration work. + fn generate(driver: &mut D) -> Option { + let expose_count = usize::from(driver.produce::()? % MAX_EXPOSES + 1); + let mut ranges = Vec::with_capacity(expose_count); + for _ in 0..expose_count { + let offset = driver.produce::()? % u8::try_from(WINDOW).ok()?; + let length = driver.produce::()? % MAX_RANGE_LEN + 1; + ranges.push(vec![(offset, length)]); + } + + let mut packet_ops: [Vec; PACKET_WORKERS] = driver.produce()?; + for ops in &mut packet_ops { + ops.truncate(MAX_PACKET_OPS); + if !ops.iter().any(|op| matches!(op, PacketOp::Allocate)) { + let at = driver.produce::()? % (ops.len() + 1); + ops.insert(at, PacketOp::Allocate); + } + } + + let mut config_ops: Vec = driver.produce()?; + config_ops.truncate(MAX_CONFIG_OPS); + if !config_ops + .iter() + .any(|op| matches!(op, ConfigOp::Republish)) + { + let at = driver.produce::()? % (config_ops.len() + 1); + config_ops.insert(at, ConfigOp::Republish); + } + + Some(Self { + ranges, + packet_ops, + config_ops, + frees_allowed: driver.produce()?, + }) + } +} + +/// A published generation and the reservations carried into it. +struct Published { + generation: u64, + pools: Vec>, + carried: BTreeSet<(Ipv4Addr, u16)>, + _reservations: Vec>, +} + +impl Published { + /// Build a generation and reserve its surviving tuples before publication. + fn build( + specs: &[PoolSpec], + generation: u64, + survivors: &[(usize, Ipv4Addr, NatPort)], + ) -> Self { + let pools = pool_sets_for_specs::(specs, NextHeader::TCP, false); + let mut carried = BTreeSet::new(); + let mut reservations = Vec::new(); + + for &(owner, ip, port) in survivors { + let Some(pool) = pools.get(owner) else { + continue; + }; + match pool.reserve(ip, port) { + Ok(reservation) => { + carried.insert((ip, port.as_u16())); + reservations.push(reservation); + } + // Same-spec replacement allocators must accept every survivor. + Err(e) => { + panic!("re-reserving {ip}:{port} for generation {generation} failed: {e}") + } + } + } + + Self { + generation, + pools, + carried, + _reservations: reservations, + } + } +} + +/// Live tuples, keyed by generation. +/// +/// Recording happens after allocation, so release scenarios cannot distinguish a hidden duplicate +/// from legitimate reuse. Scenarios that disable releases provide the exact oracle. +struct Live(Mutex>); + +impl Live { + fn new() -> Self { + Self(Mutex::new(BTreeSet::new())) + } + + /// Record a freshly allocated pair, failing if it is already held. + fn claim(&self, generation: u64, ip: Ipv4Addr, port: u16) { + let mut live = self.0.lock(); + assert!( + live.insert((generation, ip, port)), + "generation {generation} handed out {ip}:{port} to two flows at once" + ); + } + + /// Give a pair back, freeing it while the record is still locked so that no other thread can + /// claim it before the allocator has actually released it. + fn release(&self, generation: u64, allocation: AllocatedPort) { + let mut live = self.0.lock(); + live.remove(&(generation, allocation.ip(), allocation.port().as_u16())); + drop(allocation); + } +} + +impl Scenario { + fn specs(&self) -> Vec { + self.ranges + .iter() + .map(|ranges| PoolSpec { + public_ranges: ranges + .iter() + .map(|&(offset, length)| { + let start = u128::from(offset); + let end = (start + u128::from(length) - 1).min(WINDOW - 1); + AddrInterval::new(BASE + start, BASE + end) + }) + .collect(), + idle_timeout: IDLE_TIMEOUT, + }) + .collect() + } + + /// Race packet workers against allocator publication. + fn run(&self) { + let specs = self.specs(); + + // The flows that already exist when the config change arrives. + let initial = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + let mut existing = Vec::new(); + let mut survivors = Vec::new(); + for (owner, pool) in initial.iter().enumerate() { + if let Ok(allocation) = pool.allocate(false) { + survivors.push((owner, allocation.ip(), allocation.port())); + existing.push(allocation); + } + } + + let slot = Arc::new(SlotOption::new(Some(Arc::new(Published::build( + &specs, 0, &survivors, + ))))); + let live = Arc::new(Live::new()); + + thread::scope(|scope| { + let mut packet_handles = Vec::new(); + + for (index, ops) in self.packet_ops.iter().enumerate() { + let slot = slot.clone(); + let live = live.clone(); + let ops = ops.clone(); + let survivors = survivors.clone(); + let frees_allowed = self.frees_allowed; + packet_handles.push( + thread::Builder::new() + .name(format!("packet-{index}")) + .spawn_scoped(scope, move || { + packet_worker(&slot, &live, &ops, &survivors, frees_allowed) + }) + .expect("spawn packet worker"), + ); + } + + let config_handle = { + let slot = slot.clone(); + let ops = self.config_ops.clone(); + let specs = specs.clone(); + let survivors = survivors.clone(); + thread::Builder::new() + .name("config".to_string()) + .spawn_scoped(scope, move || { + let mut generation = 0u64; + for op in ops { + match op { + ConfigOp::Republish => { + generation += 1; + let next = Published::build(&specs, generation, &survivors); + slot.store(Some(Arc::new(next))); + } + ConfigOp::Idle => {} + } + thread::yield_now(); + } + }) + .expect("spawn config worker") + }; + + // Collect rather than drop: an allocation released while another thread is still + // allocating is one the record cannot reason about, so everything stays held until + // every thread has finished. + let leftovers: Vec<_> = packet_handles + .into_iter() + .map(|handle| handle.join().expect("packet worker panicked")) + .collect(); + config_handle.join().expect("config worker panicked"); + drop(leftovers); + }); + + drop(existing); + } +} + +/// Return remaining allocations so they stay live until all workers finish. +fn packet_worker( + slot: &SlotOption, + live: &Live, + ops: &[PacketOp], + survivors: &[(usize, Ipv4Addr, NatPort)], + frees_allowed: bool, +) -> Vec<(u64, AllocatedPort)> { + // What this thread is holding, tagged with the generation it was drawn from. + let mut held: Vec<(u64, AllocatedPort)> = Vec::new(); + + for (step, op) in ops.iter().enumerate() { + let published = slot.load_full().expect("pools are always published"); + + match op { + PacketOp::Allocate => { + if published.pools.is_empty() { + continue; + } + let owner = step % published.pools.len(); + match published.pools[owner].allocate(false) { + Ok(allocation) => { + let pair = (allocation.ip(), allocation.port().as_u16()); + + // The writer re-reserved every survivor before publishing, so a new flow + // must never be handed what a carried-over flow still holds. + assert!( + !published.carried.contains(&pair), + "generation {} handed out {}:{}, which a carried-over flow holds", + published.generation, + pair.0, + pair.1 + ); + + live.claim(published.generation, pair.0, pair.1); + held.push((published.generation, allocation)); + } + Err(AllocatorError::InternalIssue(message)) => { + panic!( + "allocating from generation {}: {message}", + published.generation + ) + } + // Running out of addresses or ports is a legitimate outcome. + Err(_) => {} + } + } + PacketOp::FreeOldest => { + if frees_allowed && !held.is_empty() { + let (generation, allocation) = held.remove(0); + live.release(generation, allocation); + } + } + PacketOp::ReserveExisting => { + // A carried tuple must stay unavailable. Hold unexpected successes so the model's + // live set remains accurate. + if let Some(&(owner, ip, port)) = survivors.get(step % survivors.len().max(1)) + && let Some(pool) = published.pools.get(owner) + { + let carried = published.carried.contains(&(ip, port.as_u16())); + match pool.reserve(ip, port) { + Ok(reservation) => { + assert!( + !carried, + "generation {} reserved {ip}:{port} a second time, although a \ + carried-over flow already holds it", + published.generation + ); + live.claim(published.generation, ip, port.as_u16()); + held.push((published.generation, reservation)); + } + Err(AllocatorError::InternalIssue(message)) => panic!( + "reserving {ip}:{port} in generation {}: {message}", + published.generation + ), + // Refused because it is held, which is the ordinary outcome here. + Err(_) => {} + } + } + } + } + + // Give the model checker a preemption point between ops; a cheap hint under std. + thread::yield_now(); + } + + held +} + +#[concurrency::model_test] +fn stress_test_config_change() { + bolero::check!() + .with_type() + .cloned() + .for_each(|scenario: Scenario| { + concurrency::stress(move || { + scenario.run(); + }); + }); +} + +/// Formatting must not drop the last address reference while holding the pool's read lock. +#[concurrency::model_test] +fn printing_the_pool_does_not_wedge_it_against_a_flow_ending() { + concurrency::stress(|| { + let specs = vec![PoolSpec { + // One address, so the flow that ends is the last holder of the one being printed. + public_ranges: vec![AddrInterval::new(BASE, BASE)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pools = Arc::new(pool_sets_for_specs::( + &specs, + NextHeader::TCP, + false, + )); + + let allocation = pools[0].allocate(false).expect("the pool can serve"); + + let releaser = thread::spawn(move || drop(allocation)); + let printer = { + let pools = pools.clone(); + thread::spawn(move || { + let _ = format!("{}", pools[0]); + }) + }; + + printer.join().expect("the printing thread panicked"); + releaser.join().expect("the releasing thread panicked"); + }); +} + +/// A reservation racing a block release is contention, not corrupt state. +#[concurrency::model_test] +fn reservation_racing_block_release_is_not_an_internal_error() { + concurrency::stress(|| { + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(BASE, BASE)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pools = Arc::new(pool_sets_for_specs::( + &specs, + NextHeader::TCP, + false, + )); + + let allocation = pools[0].allocate(false).expect("the pool can serve"); + let (ip, port) = (allocation.ip(), allocation.port()); + + let releaser = thread::spawn(move || drop(allocation)); + let reserver = { + let pools = pools.clone(); + thread::spawn(move || pools[0].reserve(ip, port)) + }; + + let outcome = reserver.join().expect("the reserving thread panicked"); + releaser.join().expect("the releasing thread panicked"); + + if let Err(AllocatorError::InternalIssue(message)) = &outcome { + panic!("a reservation racing the release of its block was called a bug: {message}"); + } + }); +} + +/// An expired weak entry must not erase a block inserted at the same index. +#[concurrency::model_test] +fn tidying_a_dead_block_entry_does_not_drop_a_live_one() { + concurrency::stress(|| { + let address = Ipv4Addr::from(u32::try_from(BASE).unwrap_or_else(|_| unreachable!())); + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(BASE, BASE)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pools = Arc::new(pool_sets_for_specs::( + &specs, + NextHeader::TCP, + false, + )); + + let keeper_port = NatPort::new_port_checked(1024).unwrap_or_else(|_| unreachable!()); + let _keeper = pools[0] + .reserve(address, keeper_port) + .expect("the keeper reservation"); + // This opens the next block and leaves a stale per-thread hint when dropped. + drop(pools[0].allocate(false).expect("the second block")); + + let holder = { + let pools = pools.clone(); + thread::spawn(move || pools[0].allocate(false).ok()) + }; + let mine = pools[0].allocate(false).ok(); + let theirs = holder.join().expect("the other task panicked"); + + if mine.is_some() || theirs.is_some() { + let port = NatPort::new_port_checked(1400).unwrap_or_else(|_| unreachable!()); + let outcome = pools[0].reserve(address, port); + assert!( + outcome.is_ok(), + "a block in use was dropped from the list: reserving into it gave {outcome:?}" + ); + } + }); +} diff --git a/nat/src/masquerade/apalloc/display.rs b/nat/src/masquerade/apalloc/display.rs index c92ae5a2cc..4298d3d056 100644 --- a/nat/src/masquerade/apalloc/display.rs +++ b/nat/src/masquerade/apalloc/display.rs @@ -3,10 +3,11 @@ //! Display implementations for allocator types -use super::alloc::{AllocatedIp, IpAllocator, NatPool}; +use super::alloc::{AllocatedIp, IpAllocator, NatPool, PoolSet}; use super::port_alloc::PortAllocator; use super::{NatAllocator, NatIp, NatIpWithBitmap, PoolTable, PoolTableKey}; use common::cliprovider::{CliSource, Heading}; +use concurrency::sync::{Arc, Weak}; use indenter::indented; use std::fmt::{Display, Error, Formatter, Result, Write}; @@ -57,19 +58,57 @@ where fn fmt(&self, f: &mut Formatter<'_>) -> Result { write!( f, - "{} | dest VPC: {}, for IPs: [ {} .. {} ]", - self.protocol, self.dst_vpcd, self.addr, self.addr_range_end + "{} | source VPC: {}, dest VPC: {}, for IPs: [ {} .. {} ]", + self.protocol, self.src_vpcd, self.dst_vpcd, self.addr, self.addr_range_end ) } } +impl Display for PoolSet +where + I: NatIpWithBitmap + Display, +{ + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + writeln!(f, "idle timeout: {:?}", self.idle_timeout())?; + let mut empty = true; + for region in self.regions() { + empty = false; + let range = region.range(); + let start = I::try_from_bits(range.start).map_err(|()| Error)?; + let end = I::try_from_bits(range.end).map_err(|()| Error)?; + writeln!(f, "region [ {start} .. {end} ]:")?; + write!(with_indent!(f), "{}", region.allocator())?; + } + if empty { + writeln!(f, "(no region)")?; + } + Ok(()) + } +} + impl Display for IpAllocator where I: NatIpWithBitmap + Display, { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - let pool = self.read(); - write!(f, "{pool}") + // The same hazard the allocation paths guard against, reached by printing the table. + // + // The pool holds weak references to the addresses in use; the strong ones belong to the + // blocks handed out from each. `NatPool`'s own `fmt` upgrades each weak reference to print + // it, and the guard below is held for all of that. Another thread ending the last flow on + // an address at that moment leaves one of those upgrades as the only strong reference, and + // letting it go runs `AllocatedIp::drop` here, which takes this same lock for writing. + // + // Holding an upgrade of every address across the guard keeps the ones taken while printing + // from ever being last. They are released below, once the guard is gone. + let mut examined: Vec>> = Vec::new(); + let outcome = { + let pool = self.read(); + examined.extend(pool.ips_in_use().filter_map(Weak::upgrade)); + write!(f, "{pool}") + }; + drop(examined); + outcome } } @@ -78,15 +117,6 @@ where I: NatIpWithBitmap + Display, { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - writeln!(f, "idle timeout: {:?}", self.idle_timeout())?; - - if let Some(reserved) = self.reserved_prefixes_ports() { - writeln!(f, "reserved ranges:")?; - for (ips, ports) in reserved { - writeln!(with_indent!(f), "{ips}:{ports}")?; - } - } - writeln!(f, "IP ranges in pool:")?; for range in self.ips_in_bitmap().map_err(|()| Error)? { writeln!(with_indent!(f), "{range}")?; @@ -127,10 +157,6 @@ where I: NatIpWithBitmap + Display, { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - if let Some(reserved) = self.reserved_port_range() { - writeln!(f, "reserved port range: {reserved}")?; - } - writeln!(f, "allocated ports:")?; if !self.has_free_ports() { return writeln!(with_indent!(f), "[all ports allocated]"); diff --git a/nat/src/masquerade/apalloc/mod.rs b/nat/src/masquerade/apalloc/mod.rs index 51338fccfc..67ee1460d6 100644 --- a/nat/src/masquerade/apalloc/mod.rs +++ b/nat/src/masquerade/apalloc/mod.rs @@ -8,31 +8,39 @@ //! Here is an attempt to visualize the allocator structure: //! //! ```text -//! ┌────────────┐ -//! │NatAllocator├────────────────────────────┬──────┬──────┐ -//! └────────┬───┘ │ │ │ -//! │ │ │ │ -//! ┌────────▼────────┐ ┌─────────────▼──────▼──────▼───┐ -//! │PoolTable (src44)│ │PoolTable (src66, dst44, dst66)│ -//! └───────┬─────────┘ └───────────────────────────────┘ -//! │ -//! ┌───────▼────┐ associates ┌───────────┐ -//! │PoolTableKey┼──────────────►IpAllocator◄────────────────┐ -//! └────────────┘ └────┬──────┘ │ -//! │ │ -//! ┌────▼──┐ │ -//! ┌─────────────────────┤NatPool├───┐ │ -//! │ └───────┘ │ │ -//! │ │ │ -//! ┌─────────────────┐ ┌─────────▼──────────┐ │ -//! │ │ │PoolBitmap │ │ -//! │(weak references)│ │(map free addresses)│ │ -//! └─────────────────┘ └────────────────────┘ │ -//! │ │ -//! ┌─────▼─────┐ │ -//! │AllocatedIp│────────────────────────────────────────────┘ -//! └─▲─────────┘ back-reference, for deallocation -//! │ │ +//! ┌────────────┐ +//! │NatAllocator│ +//! └──────┬─────┘ +//! ┌─────────────┴─────────────┐ +//! ┌─────────▼───────┐ ┌─────────▼───────┐ +//! │PoolTable (src44)│ │PoolTable (src66)│ one per address family +//! └─────────┬───────┘ └─────────────────┘ +//! │ keyed by +//! ┌─────────▼──┐ +//! │PoolTableKey│ protocol, source VPC, dest VPC, public range +//! └─────────┬──┘ +//! │ which an expose may draw from +//! ┌───────▼─┐ +//! │PoolSet │ the regions this expose's ranges cover, exclusive ones first +//! └───────┬─┘ +//! │ +//! ┌────────▼──┐ ┌───────────┐ +//! │PoolRegion ├────────►AddrInterval│ the slice of public space it owns +//! └────────┬──┘ └───────────┘ +//! │ one allocator per region, shared by every expose over it +//! ┌─────────▼─┐ +//! │IpAllocator│◄───────────────────────────────┐ +//! └─────────┬─┘ │ +//! ┌────▼──┐ │ +//! ┌───────┤NatPool├──────────┐ │ +//! │ └───────┘ │ │ +//! ┌──▼──────────────┐ ┌────────▼───────────┐ │ +//! │ │ │PoolBitmap │ │ +//! │(weak references)│ │(map free addresses)│ │ +//! └──┬──────────────┘ └────────────────────┘ │ +//! ┌──▼────────┐ │ +//! │AllocatedIp│──────────────────────────────────────┘ +//! └─▲───────┬─┘ back-reference, for deallocation //! │ ┌─────▼───────┐ //! │ │PortAllocator│ //! │ └─────┬───────┘ @@ -54,12 +62,11 @@ //! Returned object //! ``` //! -//! The [`AllocatedPort`](port_alloc::AllocatedPort) has a back-reference to -//! [`AllocatedPortBlock`](port_alloc::AllocatedPortBlock), to deallocate the ports when the -//! [`AllocatedPort`](port_alloc::AllocatedPort) is dropped; -//! [`AllocatedPortBlock`](port_alloc::AllocatedPortBlock) has a back reference to -//! [`AllocatedIp`](alloc::AllocatedIp), and then the [`IpAllocator`](alloc::IpAllocator), to -//! deallocate the IP address when they are dropped. +//! Overlapping public ranges are divided into disjoint [`PoolRegion`](alloc::PoolRegion)s. Exposes +//! share the allocator for each common region, ensuring a public tuple is leased only once. +//! +//! Allocation objects hold back-references to their owning block, address, and pool. Dropping the +//! outer allocation therefore releases the tuple. #![allow(rustdoc::private_intra_doc_links)] @@ -68,7 +75,8 @@ use crate::NatPort; use crate::masquerade::MasqueradeConfig; pub use crate::masquerade::apalloc::natip_with_bitmap::NatIpWithBitmap; use crate::masquerade::natip::NatIp; -use concurrency::sync::atomic::{AtomicI64, Ordering}; +use concurrency::sync::atomic::{AtomicI64, AtomicUsize, Ordering}; +use concurrency::sync::{Arc, RwLock, Weak}; use config::GenId; use net::ip::NextHeader; use net::packet::VpcDiscriminant; @@ -81,9 +89,12 @@ use tracectl::trace_target; trace_target!("nat-allocation", LevelFilter::ERROR, &["masquerade"]); mod alloc; +mod concurrent_fuzz; mod display; mod natip_with_bitmap; +mod pool_fuzz; mod port_alloc; +mod region; mod setup; mod test_alloc; @@ -93,18 +104,29 @@ pub use port_alloc::AllocatedPort; // PoolTableKey /////////////////////////////////////////////////////////////////////////////// +/// Identifies the pool serving a private source address. +/// +/// Both VPC discriminants precede the address so range lookup stays within one VPC pair. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] struct PoolTableKey { protocol: NextHeader, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, addr: I, addr_range_end: I, } impl PoolTableKey { - fn new(protocol: NextHeader, dst_vpcd: VpcDiscriminant, addr: I, addr_range_end: I) -> Self { + fn new( + protocol: NextHeader, + src_vpcd: VpcDiscriminant, + dst_vpcd: VpcDiscriminant, + addr: I, + addr_range_end: I, + ) -> Self { Self { protocol, + src_vpcd, dst_vpcd, addr, addr_range_end, @@ -118,7 +140,7 @@ impl PoolTableKey { #[derive(Debug)] struct PoolTable( - BTreeMap, alloc::IpAllocator>, + BTreeMap, alloc::PoolSet>, ); impl PoolTable { @@ -126,34 +148,70 @@ impl PoolTable { Self(BTreeMap::new()) } - fn get(&self, key: &PoolTableKey) -> Option<&alloc::IpAllocator> { - // We need to find the entry with the ID, and the prefix for the corresponding address. - // Get the range of "lower" entries, the one with the address before ours is the prefix we - // need, if the ID also matches. - match self.0.range(..=key).next_back() { - Some((k, v)) - if k.addr_range_end >= key.addr - && k.dst_vpcd == key.dst_vpcd - && k.protocol == key.protocol => + /// Find the longest matching private prefix within one protocol and VPC pair. + /// + /// Walking backwards must skip nested prefixes that start later but end before the queried + /// address. Once a match is found, only entries with the same start can be narrower. + fn get(&self, key: &PoolTableKey) -> Option<&alloc::PoolSet> { + let mut best: Option<(&PoolTableKey, &alloc::PoolSet)> = None; + for (candidate, pool_set) in self.0.range(..=key).rev() { + // The keys of one protocol and pair of VPCs are contiguous, so leaving that run means + // there is nothing further back to find. + if candidate.protocol != key.protocol + || candidate.src_vpcd != key.src_vpcd + || candidate.dst_vpcd != key.dst_vpcd + { + break; + } + // Walking back, prefixes start further from the address as we go. Once one covering it + // has been found, only another starting at the same address can be narrower. + if let Some((found, _)) = best + && candidate.addr < found.addr { - Some(v) + break; + } + if candidate.addr_range_end >= key.addr { + // Entries sharing a start address are visited widest first, so a later one is + // always the narrower match. + best = Some((candidate, pool_set)); } - _ => None, } + best.map(|(_, pool_set)| pool_set) } fn get_entry( &self, protocol: NextHeader, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, addr: I, - ) -> Option<&alloc::IpAllocator> { - let key = PoolTableKey::new(protocol, dst_vpcd, addr, max_range::()); + ) -> Option<&alloc::PoolSet> { + let key = PoolTableKey::new(protocol, src_vpcd, dst_vpcd, addr, max_range::()); self.get(&key) } - fn add_entry(&mut self, key: PoolTableKey, allocator: alloc::IpAllocator) { - self.0.insert(key, allocator); + fn add_entry(&mut self, key: PoolTableKey, pool_set: alloc::PoolSet) { + if self.0.contains_key(&key) { + warn!( + "Overwriting NAT pool entry {key:?}: within one VPC, the same private prefix is \ + masqueraded by more than one expose towards the same peer VPC" + ); + } + self.0.insert(key, pool_set); + } + + fn public_allocator( + &self, + protocol: NextHeader, + dst_vpcd: VpcDiscriminant, + addr: J, + ) -> Option<&alloc::IpAllocator> { + self.0 + .iter() + .filter(|(key, _)| key.protocol == protocol && key.dst_vpcd == dst_vpcd) + .flat_map(|(_, pools)| pools.regions()) + .find(|region| region.range().contains(addr.to_addr_bits())) + .map(alloc::PoolRegion::allocator) } } @@ -162,7 +220,7 @@ impl PoolTable { /////////////////////////////////////////////////////////////////////////////// /// [`Allocation`] is the non-generic object representing an allocation, be it IPv4 or IPv6 -#[derive(Debug, Clone)] +#[derive(Debug)] pub enum Allocation { V4(AllocatedPort), V6(AllocatedPort), @@ -195,6 +253,14 @@ impl Display for Allocation { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +struct PortForwardLeaseKey { + protocol: NextHeader, + peer_vpcd: VpcDiscriminant, + ip: IpAddr, + port: u16, +} + /// [`NatAllocator`] is the IP addresses and ports allocator for masquerade. /// /// Internally, it contains various bitmap-based IP pools, and each IP address allocated from these @@ -206,6 +272,8 @@ pub struct NatAllocator { genid: AtomicI64, pools_src44: PoolTable, pools_src66: PoolTable, + port_forward_leases: RwLock>>, + port_forward_lease_uses: AtomicUsize, randomize: bool, } @@ -218,11 +286,11 @@ impl NatAllocator { genid: AtomicI64::new(genid), pools_src44: PoolTable::new(), pools_src66: PoolTable::new(), + port_forward_leases: RwLock::new(BTreeMap::new()), + port_forward_lease_uses: AtomicUsize::new(0), randomize: config.randomize(), }; - for nat_peering in config.iter() { - allocator.add_peering_addresses(&nat_peering.peering, nat_peering.dst_vpcd); - } + allocator.build_pools(&config); allocator.config = config; allocator } @@ -242,46 +310,121 @@ impl NatAllocator { self.genid.store(genid, Ordering::Relaxed); } + /// Reserve a public tuple while port-forwarded flows use it. + /// + /// Several clients may use one forwarding rule, so they share one lease. A tuple outside the + /// masquerade pools, or in the well-known range masquerade already excludes, needs no lease. + pub(crate) fn reserve_port_forward( + &self, + protocol: NextHeader, + peer_vpcd: VpcDiscriminant, + ip: IpAddr, + port: std::num::NonZero, + ) -> Result>, AllocatorError> { + if !matches!(protocol, NextHeader::TCP | NextHeader::UDP) { + return Err(AllocatorError::UnsupportedProtocol(protocol)); + } + if port.get() < port_alloc::IANA_WELLKNOWN_PORT_LIMIT { + return Ok(None); + } + + let key = PortForwardLeaseKey { + protocol, + peer_vpcd, + ip, + port: port.get(), + }; + let mut leases = self.port_forward_leases.write(); + if self + .port_forward_lease_uses + .fetch_add(1, Ordering::Relaxed) + .is_multiple_of(256) + { + leases.retain(|_, lease| lease.upgrade().is_some()); + } + if let Some(existing) = leases.get(&key) { + match existing.upgrade() { + Some(lease) => return Ok(Some(lease)), + // Reservations may stop before the next stale-entry sweep. + None => { + leases.remove(&key); + } + } + } + + let nat_port = NatPort::new_port(port); + let allocation = match ip { + IpAddr::V4(ip) => { + let Some(pool) = self.pools_src44.public_allocator(protocol, peer_vpcd, ip) else { + return Ok(None); + }; + Allocation::V4(pool.reserve(ip, nat_port)?) + } + IpAddr::V6(ip) => { + let Some(pool) = self.pools_src66.public_allocator(protocol, peer_vpcd, ip) else { + return Ok(None); + }; + Allocation::V6(pool.reserve(ip, nat_port)?) + } + }; + let lease = Arc::new(allocation); + leases.insert(key, Arc::downgrade(&lease)); + Ok(Some(lease)) + } + fn allocate_v4( &self, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, src_ip: Ipv4Addr, next_header: NextHeader, ) -> Result>, AllocatorError> { - Self::allocate_from_tables(src_ip.into(), dst_vpcd, next_header, &self.pools_src44) + Self::allocate_from_tables( + src_ip.into(), + src_vpcd, + dst_vpcd, + next_header, + &self.pools_src44, + ) } fn allocate_v6( &self, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, src_ip: Ipv6Addr, next_header: NextHeader, ) -> Result>, AllocatorError> { - Self::allocate_from_tables(src_ip.into(), dst_vpcd, next_header, &self.pools_src66) + Self::allocate_from_tables( + src_ip.into(), + src_vpcd, + dst_vpcd, + next_header, + &self.pools_src66, + ) } /// Allocate an IP address and port for the given source IP, dispatching on IP version. pub(crate) fn allocate( &self, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, src_ip: IpAddr, next_header: NextHeader, ) -> Result, AllocatorError> { match src_ip { - IpAddr::V4(ip) => { - self.allocate_v4(dst_vpcd, ip, next_header) - .map(|r| AllocationResult { - allocation: Allocation::V4(r.allocation), - idle_timeout: r.idle_timeout, - }) - } - IpAddr::V6(ip) => { - self.allocate_v6(dst_vpcd, ip, next_header) - .map(|r| AllocationResult { - allocation: Allocation::V6(r.allocation), - idle_timeout: r.idle_timeout, - }) - } + IpAddr::V4(ip) => self + .allocate_v4(src_vpcd, dst_vpcd, ip, next_header) + .map(|r| AllocationResult { + allocation: Allocation::V4(r.allocation), + idle_timeout: r.idle_timeout, + }), + IpAddr::V6(ip) => self + .allocate_v6(src_vpcd, dst_vpcd, ip, next_header) + .map(|r| AllocationResult { + allocation: Allocation::V6(r.allocation), + idle_timeout: r.idle_timeout, + }), } } fn check_proto(next_header: NextHeader) -> Result<(), AllocatorError> { @@ -292,6 +435,7 @@ impl NatAllocator { } fn allocate_from_tables( src_ip: IpAddr, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, next_header: NextHeader, pools_src: &PoolTable, @@ -303,6 +447,7 @@ impl NatAllocator { let pool = pools_src .get_entry( next_header, + src_vpcd, dst_vpcd, NatIp::try_from_addr(src_ip).map_err(|()| { AllocatorError::InternalIssue("Failed to convert src IP address".to_string()) @@ -327,34 +472,46 @@ impl NatAllocator { fn reserve_ipv4_port( &self, protocol: NextHeader, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, src_ip: Ipv4Addr, ip: Ipv4Addr, port: NatPort, ) -> Result, AllocatorError> { - let Some(pool) = self.pools_src44.get_entry(protocol, dst_vpcd, src_ip) else { - warn!("No pool found for proto:{protocol} dst-vpcd:{dst_vpcd} and src:{src_ip}"); + let Some(pool) = self + .pools_src44 + .get_entry(protocol, src_vpcd, dst_vpcd, src_ip) + else { + warn!( + "No pool found for proto:{protocol} src-vpcd:{src_vpcd} dst-vpcd:{dst_vpcd} and src:{src_ip}" + ); return Err(AllocatorError::NoPoolFound); }; - debug!("Pool found for {protocol} {dst_vpcd} {src_ip}"); + debug!("Pool found for {protocol} {src_vpcd} {dst_vpcd} {src_ip}"); pool.reserve(ip, port) .inspect_err(|e| error!("Failed to reserve ip {ip} port {port}: {e}")) } fn reserve_ipv6_port( &self, protocol: NextHeader, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, src_ip: Ipv6Addr, ip: Ipv6Addr, port: NatPort, ) -> Result, AllocatorError> { - let Some(pool) = self.pools_src66.get_entry(protocol, dst_vpcd, src_ip) else { - warn!("No pool found for proto:{protocol} dst-vpcd:{dst_vpcd} and src:{src_ip}"); + let Some(pool) = self + .pools_src66 + .get_entry(protocol, src_vpcd, dst_vpcd, src_ip) + else { + warn!( + "No pool found for proto:{protocol} src-vpcd:{src_vpcd} dst-vpcd:{dst_vpcd} and src:{src_ip}" + ); return Err(AllocatorError::NoPoolFound); }; - debug!("Pool found for {protocol} {dst_vpcd} {src_ip}"); + debug!("Pool found for {protocol} {src_vpcd} {dst_vpcd} {src_ip}"); pool.reserve(ip, port) .inspect_err(|e| error!("Failed to reserve ip {ip} port {port}: {e}")) } @@ -363,18 +520,19 @@ impl NatAllocator { pub(crate) fn reserve_port( &self, protocol: NextHeader, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, src_ip: IpAddr, ip: IpAddr, port: NatPort, ) -> Result { - debug!("Re-reserving {ip} {protocol}:{port}, dst_vpcd:{dst_vpcd}"); + debug!("Re-reserving {ip} {protocol}:{port}, src_vpcd:{src_vpcd} dst_vpcd:{dst_vpcd}"); let allocation = match (src_ip, ip) { (IpAddr::V4(src), IpAddr::V4(allocated)) => self - .reserve_ipv4_port(protocol, dst_vpcd, src, allocated, port) + .reserve_ipv4_port(protocol, src_vpcd, dst_vpcd, src, allocated, port) .map(Allocation::V4)?, (IpAddr::V6(src), IpAddr::V6(allocated)) => self - .reserve_ipv6_port(protocol, dst_vpcd, src, allocated, port) + .reserve_ipv6_port(protocol, src_vpcd, dst_vpcd, src, allocated, port) .map(Allocation::V6)?, _ => { return Err(AllocatorError::InternalIssue(format!( @@ -401,6 +559,131 @@ fn max_range() -> I { // Tests /////////////////////////////////////////////////////////////////////////////// +#[cfg(test)] +mod bolero_tests { + use super::*; + use bolero::{Driver, TypeGenerator}; + use net::vxlan::Vni; + use std::time::Duration; + + // Prefixes are drawn from a narrow window so that nesting and overlap are the common case + // rather than astronomically unlikely, and so that every address in the window can be checked + // rather than a sampled few. + const BASE: u32 = 0x0A00_0000; // 10.0.0.0 + const WINDOW: u32 = 24; + const MAX_ENTRIES: u8 = 6; + const MAX_LEN: u8 = 12; + + // Which entry a lookup landed on, as a value a `PoolSet` can carry. + // + // Both bounds, because either alone loses entries. Marking by end made two entries ending + // together indistinguishable however far apart they start -- and "nearest start" is half of + // the rule under test, so a walk preferring the farther of the two passed. + const SPAN: u32 = WINDOW + MAX_LEN as u32 + 1; + fn marker(start: u32, end: u32) -> u32 { + (start - BASE) * SPAN + (end - start) + } + + fn vpcd(id: u32) -> VpcDiscriminant { + VpcDiscriminant::VNI(Vni::new_checked(id).unwrap()) + } + + /// A set of entries, each an offset into the window and a length, and each in one of two + /// groups so that the walk's refusal to cross between groups is exercised too. + /// + /// The foreign group sorts *before* the queried one, which is what makes that last part true: + /// keys order by destination VPC before address, so a group sorting after is cut off by + /// `range(..=key)` and never reaches the guard at all. + #[derive(Debug, Clone)] + struct Scenario { + entries: Vec<(u8, u8, bool)>, + } + + impl TypeGenerator for Scenario { + fn generate(driver: &mut D) -> Option { + let count = usize::from(driver.produce::()? % MAX_ENTRIES + 1); + let mut entries = Vec::with_capacity(count); + for _ in 0..count { + entries.push(( + driver.produce::()? % u8::try_from(WINDOW).ok()?, + driver.produce::()? % MAX_LEN + 1, + driver.produce::()?, + )); + } + Some(Self { entries }) + } + } + + impl Scenario { + // The entries of the group under test, as inclusive address bounds. + fn ranges(&self) -> Vec<(u32, u32)> { + self.entries + .iter() + .filter(|(_, _, other_group)| !other_group) + .map(|&(offset, length, _)| { + let start = BASE + u32::from(offset); + (start, start + u32::from(length) - 1) + }) + .collect() + } + + fn table(&self) -> PoolTable { + let mut table = PoolTable::new(); + for &(offset, length, other_group) in &self.entries { + let start = BASE + u32::from(offset); + let end = start + u32::from(length) - 1; + table.add_entry( + PoolTableKey::new( + NextHeader::TCP, + vpcd(1), + // Below the queried group, so the walk has to refuse to enter it. + if other_group { vpcd(2) } else { vpcd(3) }, + Ipv4Addr::from(start), + Ipv4Addr::from(end), + ), + alloc::PoolSet::new(Duration::from_secs(u64::from(marker(start, end)))), + ); + } + table + } + + // The oracle: among the entries covering the address, the one starting nearest to it, and + // of those the narrowest. Straight from the inputs, with no walking. + fn expected(&self, address: u32) -> Option { + self.ranges() + .into_iter() + .filter(|&(start, end)| start <= address && address <= end) + .min_by_key(|&(start, end)| (std::cmp::Reverse(start), end)) + .map(|(start, end)| marker(start, end)) + } + } + + /// Compare lookup with a nearest-start, then narrowest-range oracle. + #[test] + fn pool_table_lookup_matches_an_interval_oracle() { + bolero::check!() + .with_type() + .cloned() + .for_each(|scenario: Scenario| { + let table = scenario.table(); + // A margin either side, so addresses below and above every entry are covered. + for offset in 0..WINDOW + u32::from(MAX_LEN) + 2 { + let address = BASE + offset; + let found = table + .get_entry(NextHeader::TCP, vpcd(1), vpcd(3), Ipv4Addr::from(address)) + .map(|pool_set| u32::try_from(pool_set.idle_timeout().as_secs()).unwrap()); + assert_eq!( + found, + scenario.expected(address), + "lookup for {} disagreed with the oracle, entries {:?}", + Ipv4Addr::from(address), + scenario.ranges() + ); + } + }); + } +} + #[cfg(test)] mod tests { #![allow(clippy::ip_constant)] @@ -411,6 +694,9 @@ mod tests { fn vpcd(vpc_id: u32) -> VpcDiscriminant { VpcDiscriminant::VNI(Vni::new_checked(vpc_id).unwrap()) } + fn vpcd1() -> VpcDiscriminant { + vpcd(1) + } fn vpcd2() -> VpcDiscriminant { vpcd(2) } @@ -418,20 +704,141 @@ mod tests { vpcd(3) } - // Ensure that keys are sorted first by L4 protocol type, then by VPC IDs, and then by IP - // address. This is essential to make sure we can lookup for entries associated with prefixes - // for a given ID in the pool tables. + // A pool set carrying nothing but a distinguishable idle timeout, so a lookup can be told + // which entry it landed on. + fn marked_pool_set(marker: u64) -> alloc::PoolSet { + alloc::PoolSet::new(std::time::Duration::from_secs(marker)) + } + + // Not the lowest group, so boundary tests can place foreign entries before it. + fn queried_group() -> (NextHeader, VpcDiscriminant, VpcDiscriminant) { + (NextHeader::TCP, vpcd2(), vpcd3()) + } + + fn table_with(entries: &[(&str, &str, u64)]) -> PoolTable { + let (protocol, src, dst) = queried_group(); + let mut table = PoolTable::new(); + for &(start, end, marker) in entries { + table.add_entry( + PoolTableKey::new( + protocol, + src, + dst, + start.parse().unwrap(), + end.parse().unwrap(), + ), + marked_pool_set(marker), + ); + } + table + } + + fn lookup(table: &PoolTable, addr: &str) -> Option { + let (protocol, src, dst) = queried_group(); + table + .get_entry(protocol, src, dst, addr.parse().unwrap()) + .map(|pool_set| pool_set.idle_timeout().as_secs()) + } + + // A nested prefix must not hide later addresses covered by its parent. + #[test] + fn test_a_nested_prefix_does_not_hide_the_one_containing_it() { + let table = table_with(&[ + ("10.0.0.0", "10.0.255.255", 16), // 10.0.0.0/16 + ("10.0.1.0", "10.0.1.255", 24), // 10.0.1.0/24, nested inside it + ]); + + // Below the nested prefix, and inside it: unambiguous either way. + assert_eq!(lookup(&table, "10.0.0.5"), Some(16)); + // Past the nested prefix, but still inside the wider one. This is the case that failed. + assert_eq!( + lookup(&table, "10.0.2.5"), + Some(16), + "an address covered by the wider prefix was not served" + ); + // Well past both. + assert_eq!(lookup(&table, "10.1.0.1"), None); + } + + // Where both cover an address, the more specific prefix serves it, as it does everywhere else + // in routing. + #[test] + fn test_the_most_specific_prefix_wins() { + let table = table_with(&[ + ("10.0.0.0", "10.0.255.255", 16), + ("10.0.1.0", "10.0.1.255", 24), + ]); + assert_eq!(lookup(&table, "10.0.1.5"), Some(24)); + } + + // Several prefixes nested one inside the next, to check the walk does not stop early. + #[test] + fn test_deeply_nested_prefixes() { + let table = table_with(&[ + ("10.0.0.0", "10.255.255.255", 8), + ("10.0.0.0", "10.0.255.255", 16), + ("10.0.0.0", "10.0.0.255", 24), + ]); + assert_eq!(lookup(&table, "10.0.0.1"), Some(24)); + assert_eq!(lookup(&table, "10.0.1.1"), Some(16)); + assert_eq!(lookup(&table, "10.1.0.1"), Some(8)); + assert_eq!(lookup(&table, "11.0.0.1"), None); + } + + // Foreign groups sort before the queried group, forcing lookup to reach and reject them. + #[test] + fn test_the_walk_does_not_cross_into_another_group() { + let (protocol, src, dst) = queried_group(); + // One boundary at a time, each a group immediately below the queried one. + let foreign = [ + (NextHeader::ICMP, src, dst), + (protocol, vpcd1(), dst), + (protocol, src, vpcd2()), + ]; + + for (index, &(f_protocol, f_src, f_dst)) in foreign.iter().enumerate() { + let mut table = table_with(&[("10.0.1.0", "10.0.1.255", 24)]); + // A wider prefix, covering everything the queried group's entry does and more, but + // reached only by walking out of the queried group. + table.add_entry( + PoolTableKey::new( + f_protocol, + f_src, + f_dst, + "10.0.0.0".parse().unwrap(), + "10.0.255.255".parse().unwrap(), + ), + marked_pool_set(99), + ); + // An address only the foreign entry covers is not served at all... + assert_eq!( + lookup(&table, "10.0.2.5"), + None, + "foreign group {index} served an address of its own" + ); + // ...and one both cover is served by the queried group's entry. + assert_eq!( + lookup(&table, "10.0.1.5"), + Some(24), + "foreign group {index} displaced the entry that should serve" + ); + } + } + + // PoolTable lookup relies on protocol/VPC groups being contiguous before address ordering. #[allow(clippy::too_many_lines)] #[test] fn test_key_order() { let key1 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(1, 1, 1, 1), ); let key2 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(1, 1, 1, 1), @@ -440,12 +847,14 @@ mod tests { let key1 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(1, 1, 1, 1), ); let key2 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(255, 255, 255, 255), @@ -454,12 +863,14 @@ mod tests { let key1 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(1, 1, 1, 1), ); let key2 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 2), Ipv4Addr::new(255, 255, 255, 255), @@ -468,12 +879,14 @@ mod tests { let key1 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(2, 1, 1, 1), Ipv4Addr::new(1, 1, 1, 1), ); let key2 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 255, 255, 255), Ipv4Addr::new(255, 255, 255, 255), @@ -484,12 +897,14 @@ mod tests { let key1 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(255, 255, 255, 255), ); let key2 = PoolTableKey::new( NextHeader::UDP, + vpcd1(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(255, 255, 255, 255), @@ -498,16 +913,75 @@ mod tests { let key1 = PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd3(), Ipv4Addr::new(2, 2, 2, 2), Ipv4Addr::new(255, 255, 255, 255), ); let key2 = PoolTableKey::new( NextHeader::UDP, + vpcd1(), + vpcd2(), + Ipv4Addr::new(1, 1, 1, 1), + Ipv4Addr::new(1, 1, 1, 1), + ); + assert!(key1 < key2); + + // Mixing source VPCs. The source discriminant outranks both the destination and the + // address, so the entries of one source VPC never interleave with those of another. + + let key1 = PoolTableKey::new( + NextHeader::TCP, + vpcd1(), + vpcd3(), + Ipv4Addr::new(2, 2, 2, 2), + Ipv4Addr::new(255, 255, 255, 255), + ); + let key2 = PoolTableKey::new( + NextHeader::TCP, + vpcd2(), vpcd2(), Ipv4Addr::new(1, 1, 1, 1), Ipv4Addr::new(1, 1, 1, 1), ); assert!(key1 < key2); + + // ... but it does not outrank the protocol. + + let key1 = PoolTableKey::new( + NextHeader::TCP, + vpcd3(), + vpcd2(), + Ipv4Addr::new(1, 1, 1, 1), + Ipv4Addr::new(255, 255, 255, 255), + ); + let key2 = PoolTableKey::new( + NextHeader::UDP, + vpcd1(), + vpcd2(), + Ipv4Addr::new(1, 1, 1, 1), + Ipv4Addr::new(255, 255, 255, 255), + ); + assert!(key1 < key2); + + // Two source VPCs using the same private address are distinct keys, which is the whole + // point of carrying the source discriminant: a private address only means something + // within the VPC it belongs to. + + let key1 = PoolTableKey::new( + NextHeader::TCP, + vpcd1(), + vpcd3(), + Ipv4Addr::new(1, 1, 1, 1), + Ipv4Addr::new(1, 1, 255, 255), + ); + let key2 = PoolTableKey::new( + NextHeader::TCP, + vpcd2(), + vpcd3(), + Ipv4Addr::new(1, 1, 1, 1), + Ipv4Addr::new(1, 1, 255, 255), + ); + assert_ne!(key1, key2); } } diff --git a/nat/src/masquerade/apalloc/natip_with_bitmap.rs b/nat/src/masquerade/apalloc/natip_with_bitmap.rs index 52385055a7..73e8c04703 100644 --- a/nat/src/masquerade/apalloc/natip_with_bitmap.rs +++ b/nat/src/masquerade/apalloc/natip_with_bitmap.rs @@ -60,6 +60,6 @@ impl NatIpWithBitmap for Ipv6Addr { bitmap_mapping: &BTreeMap, ) -> Result { // Reverse operation of map_offset() - Ok(map_address(address, bitmap_mapping)) + map_address(address, bitmap_mapping) } } diff --git a/nat/src/masquerade/apalloc/pool_fuzz.rs b/nat/src/masquerade/apalloc/pool_fuzz.rs new file mode 100644 index 0000000000..888695c1f4 --- /dev/null +++ b/nat/src/masquerade/apalloc/pool_fuzz.rs @@ -0,0 +1,440 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Property tests for pools built over overlapping public ranges. +//! +//! Generated ranges come from a narrow window so overlap is common. The properties require unique +//! live tuples, allocations within each expose's ranges, and safe carry-over between generations. + +#![cfg(test)] + +use super::alloc::{PoolSet, map_address}; +use super::region::AddrInterval; +use super::setup::{PoolSpec, pool_sets_for_specs}; +use crate::masquerade::allocation::AllocatorError; +use crate::port::NatPort; +use bolero::{Driver, TypeGenerator}; +use net::ip::NextHeader; +use std::collections::{BTreeMap, BTreeSet}; +use std::net::{Ipv4Addr, Ipv6Addr}; +use std::time::Duration; + +// 10.1.0.0, with a window small enough that regions stay cheap to build. +const BASE: u128 = 0x0A01_0000; +const WINDOW: u128 = 16; +const MAX_OWNERS: u8 = 4; +const MAX_RANGES_PER_OWNER: u8 = 2; +const MAX_RANGE_LEN: u8 = 8; +// Enough allocations for several exposes to be served repeatedly out of any shared region. +const ALLOCATIONS: usize = 24; + +const IDLE_TIMEOUT: Duration = Duration::from_mins(2); + +/// One generated configuration: for each expose, the public ranges it masquerades onto. +#[derive(Debug, Clone)] +struct Config { + owners: Vec>, +} + +impl TypeGenerator for Config { + fn generate(driver: &mut D) -> Option { + let owner_count = usize::from(driver.produce::()? % MAX_OWNERS + 1); + let mut owners = Vec::with_capacity(owner_count); + for _ in 0..owner_count { + let range_count = usize::from(driver.produce::()? % MAX_RANGES_PER_OWNER + 1); + let mut ranges = Vec::with_capacity(range_count); + for _ in 0..range_count { + let offset = driver.produce::()? % u8::try_from(WINDOW).ok()?; + let length = driver.produce::()? % MAX_RANGE_LEN + 1; + ranges.push((offset, length)); + } + owners.push(ranges); + } + Some(Self { owners }) + } +} + +impl Config { + fn owner_ranges(&self) -> Vec> { + self.owners + .iter() + .map(|ranges| { + ranges + .iter() + .map(|&(offset, length)| { + let start = u128::from(offset); + let end = (start + u128::from(length) - 1).min(WINDOW - 1); + AddrInterval::new(BASE + start, BASE + end) + }) + .collect() + }) + .collect() + } + + fn specs(&self) -> Vec { + self.owner_ranges() + .into_iter() + .map(|public_ranges| PoolSpec { + public_ranges, + idle_timeout: IDLE_TIMEOUT, + }) + .collect() + } + + fn pool_sets(&self) -> Vec> { + // No randomization: a failure has to reproduce from its seed alone. + pool_sets_for_specs::(&self.specs(), NextHeader::TCP, false) + } + + fn owner_count(&self) -> usize { + self.owners.len() + } +} + +fn bits(ip: Ipv4Addr) -> u128 { + u128::from(ip.to_bits()) +} + +fn declares(ranges: &[AddrInterval], ip: Ipv4Addr) -> bool { + ranges.iter().any(|range| range.contains(bits(ip))) +} + +/// Allocate round-robin across the exposes, holding every allocation so nothing is freed and +/// reused mid-run. Returns which expose got what. +fn allocate_round_robin( + pool_sets: &[PoolSet], + count: usize, +) -> Vec<(usize, super::AllocatedPort)> { + let mut held = Vec::new(); + for step in 0..count { + let owner = step % pool_sets.len(); + if let Ok(allocation) = pool_sets[owner].allocate(false) { + held.push((owner, allocation)); + } + } + held +} + +#[test] +fn allocations_are_unique_and_within_the_declared_ranges() { + bolero::check!() + .with_type() + .cloned() + .for_each(|config: Config| { + let ranges = config.owner_ranges(); + let pool_sets = config.pool_sets(); + assert_eq!(pool_sets.len(), config.owner_count()); + + let mut seen = BTreeSet::new(); + for (owner, allocation) in allocate_round_robin(&pool_sets, ALLOCATIONS) { + let ip = allocation.ip(); + let port = allocation.port().as_u16(); + + // Two live flows may not share a public address and port, whichever exposes they + // belong to: their reverse flow keys would be identical and return traffic for one + // would be delivered to the other. + assert!( + seen.insert((ip, port)), + "{ip}:{port} was handed out twice, the second time to expose {owner}" + ); + + // An expose may only be given an address it declares, however the space was cut. + assert!( + declares(&ranges[owner], ip), + "expose {owner} was given {ip}, which is outside the ranges it declares" + ); + + // TCP pools keep off the IANA system range. + assert!(port >= 1024, "{ip}:{port} is in the well-known port range"); + } + }); +} + +#[test] +fn freed_allocations_become_available_again() { + bolero::check!() + .with_type() + .cloned() + .for_each(|config: Config| { + let pool_sets = config.pool_sets(); + + let first = allocate_round_robin(&pool_sets, ALLOCATIONS); + let taken: BTreeSet<_> = first + .iter() + .map(|(_, allocation)| (allocation.ip(), allocation.port().as_u16())) + .collect(); + assert!(!taken.is_empty(), "a config with ranges allocated nothing"); + drop(first); + + // Everything was released, so the same space must be servable again. Values need not + // repeat, but the pools must not have leaked capacity. + let second = allocate_round_robin(&pool_sets, ALLOCATIONS); + assert_eq!( + second.len(), + taken.len(), + "the pools served fewer allocations after everything was freed" + ); + }); +} + +#[test] +fn a_port_freed_while_neighbours_are_held_is_reused() { + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(BASE, BASE)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pool_sets = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + + let mut held: Vec<_> = (0..5) + .map(|_| pool_sets[0].allocate(false).expect("pool has room")) + .collect(); + + let returned = held.remove(2); + let tuple = (returned.ip(), returned.port().as_u16()); + drop(returned); + + let next = pool_sets[0].allocate(false).expect("pool has room"); + assert_eq!((next.ip(), next.port().as_u16()), tuple); +} + +/// A tuple accepted during carry-over must not be allocated to a new flow. +#[test] +fn re_reservation_after_a_config_change_is_honoured() { + bolero::check!() + .with_type() + .cloned() + .for_each(|(before, after): (Config, Config)| { + let before_pools = before.pool_sets(); + let held = allocate_round_robin(&before_pools, ALLOCATIONS); + + let after_ranges = after.owner_ranges(); + let after_pools = after.pool_sets(); + + // Carry each flow over to the new pools, where the new config still has an expose for + // it. Flows whose expose is gone would be invalidated instead. + let mut carried = Vec::new(); + let mut reserved = BTreeSet::new(); + for (owner, allocation) in &held { + let Some(pool_set) = after_pools.get(*owner) else { + continue; + }; + let ip = allocation.ip(); + let port = allocation.port(); + match pool_set.reserve(ip, port) { + Ok(reservation) => { + // Accepting an address means the expose really does declare it under the + // new config; nothing may be carried over into space it no longer holds. + assert!( + declares(&after_ranges[*owner], ip), + "expose {owner} kept {ip}, which its new config does not declare" + ); + assert!( + reserved.insert((ip, port.as_u16())), + "{ip}:{port} was reserved twice across exposes" + ); + carried.push(reservation); + } + Err(AllocatorError::NoPoolFound) => { + // The new config dropped that address, so the flow cannot be carried over. + assert!( + !declares(&after_ranges[*owner], ip), + "expose {owner} was refused {ip}, which its new config declares" + ); + } + Err(_) => {} + } + } + + // New flows arriving under the new config must not be given anything a carried-over + // flow is still using. + for (_, allocation) in allocate_round_robin(&after_pools, ALLOCATIONS) { + let pair = (allocation.ip(), allocation.port().as_u16()); + assert!( + !reserved.contains(&pair), + "{}:{} was allocated although a carried-over flow holds it", + pair.0, + pair.1 + ); + } + + drop(carried); + }); +} + +#[test] +#[cfg_attr(miri, ignore = "exhaustive allocator walk is too slow under miri")] +fn a_region_can_be_allocated_dry() { + const PORTS_PER_ADDRESS: usize = 65536 - 1024; + const ADDRESSES: usize = 2; + + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(BASE, BASE + ADDRESSES as u128 - 1)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pool_sets = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + + let first = Ipv4Addr::from(u32::try_from(BASE).unwrap_or_else(|_| unreachable!())); + let mut held = Vec::new(); + let mut seen = BTreeSet::new(); + let mut moved_at = None; + + while let Ok(allocation) = pool_sets[0].allocate(false) { + let tuple = (allocation.ip(), allocation.port().as_u16()); + assert!(tuple.1 >= 1024); + assert!( + seen.insert(tuple), + "{}:{} was handed out twice", + tuple.0, + tuple.1 + ); + if tuple.0 != first && moved_at.is_none() { + moved_at = Some(held.len()); + } + held.push(allocation); + assert!(held.len() <= ADDRESSES * PORTS_PER_ADDRESS); + } + + assert_eq!(moved_at, Some(PORTS_PER_ADDRESS)); + assert_eq!(held.len(), ADDRESSES * PORTS_PER_ADDRESS); + + drop(held); + assert!(pool_sets[0].allocate(false).is_ok()); +} + +#[test] +#[cfg_attr(miri, ignore = "exhaustive allocator walk is too slow under miri")] +fn a_freed_port_block_is_reused_while_its_address_is_held() { + const PORTS_PER_BLOCK: usize = 256; + const PORTS_PER_ADDRESS: usize = 65536 - 1024; + + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(BASE, BASE)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pool_sets = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + + let mut held = Vec::with_capacity(PORTS_PER_ADDRESS); + while let Ok(allocation) = pool_sets[0].allocate(false) { + held.push(allocation); + assert!(held.len() <= PORTS_PER_ADDRESS); + } + assert_eq!(held.len(), PORTS_PER_ADDRESS); + + let freed_block = 1024..=1279u16; + held.retain(|allocation| !freed_block.contains(&allocation.port().as_u16())); + assert_eq!(held.len(), PORTS_PER_ADDRESS - PORTS_PER_BLOCK); + + for _ in 0..PORTS_PER_BLOCK { + let allocation = pool_sets[0] + .allocate(false) + .expect("the freed block has room"); + assert!(freed_block.contains(&allocation.port().as_u16())); + held.push(allocation); + } + assert!(pool_sets[0].allocate(false).is_err()); +} + +/////////////////////////////////////////////////////////////////////////////// +// IPv6 +/////////////////////////////////////////////////////////////////////////////// + +#[test] +fn the_offset_mapping_refuses_an_address_it_cannot_index() { + let start = u128::from(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0)); + let mapping = BTreeMap::from([(start, 0u32)]); + + assert_eq!(map_address(Ipv6Addr::from(start + 1), &mapping), Ok(1)); + assert_eq!( + map_address(Ipv6Addr::from(start + u128::from(u32::MAX)), &mapping), + Ok(u32::MAX) + ); + for beyond in [u128::from(u32::MAX) + 1, 1u128 << 33] { + assert_eq!( + map_address(Ipv6Addr::from(start + beyond), &mapping), + Err(AllocatorError::NoPoolFound) + ); + } +} + +/// A region may hold more addresses than a `u32` can index, in which case the bitmap covers only +/// the first 2^32 of them. An address past that is inside the region but not servable, which a +/// flow carried across a config change can present, and which must be an error rather than a panic +/// part way through applying a config. +#[test] +#[cfg_attr(miri, ignore = "the 2^32-entry bitmap is too slow under miri")] +fn an_address_past_the_indexable_span_is_refused_rather_than_panicking() { + let start = u128::from(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0)); + // Far wider than the bitmap can index. + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(start, start + (1u128 << 40))], + idle_timeout: IDLE_TIMEOUT, + }]; + let pool_sets = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + let port = NatPort::new_port_checked(4096).unwrap_or_else(|_| unreachable!()); + + // Just inside the indexable span: an ordinary carry-over, which must still work. + let near = Ipv6Addr::from(start + 1); + assert!( + pool_sets[0].reserve(near, port).is_ok(), + "an address the pool can index was refused" + ); + + // Past it: refused, and specifically not a panic. + let far = Ipv6Addr::from(start + (1u128 << 33)); + assert_eq!( + pool_sets[0].reserve(far, port).unwrap_err(), + AllocatorError::NoPoolFound, + "an address the pool cannot index should be refused as unserved" + ); +} + +/// The pools are generic over the address family but every other test here is IPv4. Allocating +/// from an IPv6 pool goes through the offset mapping that IPv4 skips entirely, so cover it. +#[test] +fn ipv6_pools_allocate_within_their_range() { + let start = u128::from(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0)); + let end = start + 3; + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(start, end)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pool_sets = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + + let mut held = Vec::new(); + let mut seen = BTreeSet::new(); + for step in 0..8 { + let allocation = pool_sets[0].allocate(false).expect("pool has room"); + let bits = u128::from(allocation.ip()); + if step == 0 { + assert_eq!(bits, start, "the offset mapping skipped the range start"); + } + assert!( + (start..=end).contains(&bits), + "{} is outside the range the pool was built for", + allocation.ip() + ); + assert!( + seen.insert((allocation.ip(), allocation.port().as_u16())), + "{}:{} was handed out twice", + allocation.ip(), + allocation.port() + ); + held.push(allocation); + } +} + +#[test] +fn a_live_tuple_cannot_be_reserved_again() { + let specs = vec![PoolSpec { + public_ranges: vec![AddrInterval::new(BASE, BASE)], + idle_timeout: IDLE_TIMEOUT, + }]; + let pool_sets = pool_sets_for_specs::(&specs, NextHeader::TCP, false); + + let held = pool_sets[0].allocate(false).expect("pool has room"); + let tuple = (held.ip(), held.port()); + assert!(pool_sets[0].reserve(tuple.0, tuple.1).is_err()); + + drop(held); + assert!(pool_sets[0].reserve(tuple.0, tuple.1).is_ok()); +} diff --git a/nat/src/masquerade/apalloc/port_alloc.rs b/nat/src/masquerade/apalloc/port_alloc.rs index 2fa67d0ef6..1e7661f679 100644 --- a/nat/src/masquerade/apalloc/port_alloc.rs +++ b/nat/src/masquerade/apalloc/port_alloc.rs @@ -15,18 +15,21 @@ use crate::port::NatPort; use concurrency::concurrency_mode; use concurrency::sync::atomic::{AtomicBool, AtomicU16, AtomicUsize}; use concurrency::sync::{Arc, Mutex, RwLock, Weak}; -use concurrency::thread::ThreadId; +use concurrency::thread::{self, ThreadId}; use lpm::prefix::PortRange; use std::collections::{BTreeSet, HashMap}; use std::fmt::Display; -use tracing::debug; +use tracing::{debug, error}; #[concurrency_mode(std)] use rand::seq::SliceRandom; #[concurrency_mode(shuttle)] use shuttle::rand::{Rng, thread_rng}; +/// Bounds retries while a port block changes hands. +const BLOCK_LOOKUP_ATTEMPTS: usize = 4; + /////////////////////////////////////////////////////////////////////////////// // AllocatorPortBlock /////////////////////////////////////////////////////////////////////////////// @@ -84,23 +87,18 @@ pub(crate) struct PortAllocator { current_alloc_index: AtomicUsize, thread_blocks: ThreadPortMap, allocated_blocks: AllocatedPortBlockMap, - reserved_port_range: Option, exclude_wellknown_ports: bool, } /// Ports 0..=1023 cover the IANA system/well-known range and should not be /// allocated by masquerade NAT for TCP or UDP. -const IANA_WELLKNOWN_PORT_LIMIT: u16 = 1024; +pub(super) const IANA_WELLKNOWN_PORT_LIMIT: u16 = 1024; /// Number of 256-port blocks covering the IANA well-known port range (0-1023). const IANA_WELLKNOWN_BLOCKS: u16 = IANA_WELLKNOWN_PORT_LIMIT / 256; impl PortAllocator { - pub(crate) fn new( - reserved_port_range: Option, - randomize: bool, - exclude_wellknown_ports: bool, - ) -> Self { + pub(crate) fn new(randomize: bool, exclude_wellknown_ports: bool) -> Self { let mut base_ports = (0..=255).collect::>(); // Shuffle the list of port blocks for the port allocator. This way, we can pick blocks in a @@ -132,17 +130,13 @@ impl PortAllocator { current_alloc_index: AtomicUsize::new(0), thread_blocks: ThreadPortMap::new(), allocated_blocks: AllocatedPortBlockMap::new(), - reserved_port_range, exclude_wellknown_ports, } } #[cfg(test)] - pub(crate) fn new_no_randomness( - reserved_port_range: Option, - exclude_wellknown_ports: bool, - ) -> Self { - Self::new(reserved_port_range, false, exclude_wellknown_ports) + pub(crate) fn new_no_randomness(exclude_wellknown_ports: bool) -> Self { + Self::new(false, exclude_wellknown_ports) } #[concurrency_mode(std)] @@ -212,8 +206,7 @@ impl PortAllocator { let (index, block) = self .cycle_blocks() .find(|(_, block)| { - // Find the first block for which the atomic compare_exchange succeeds - if block + block .free .compare_exchange( true, @@ -221,30 +214,7 @@ impl PortAllocator { concurrency::sync::atomic::Ordering::Relaxed, concurrency::sync::atomic::Ordering::Relaxed, ) - .is_err() - { - return false; - } - - // Check if this block is fully contained in the reserved range - if let Some(reserved_range) = self.reserved_port_range - && reserved_range.len() >= 255 - { - // Corner case: reserved_range is 1-255+, but 0 cannot be allocated so - // reserved_range effectively renders the block unusable (except maybe for ICMP - // but never mind) - let adjusted_reserved_range = if reserved_range.start() == 1 { - PortRange::new(0, reserved_range.end()).unwrap_or_else(|_| unreachable!()) - } else { - reserved_range - }; - - let block_range = PortRange::from(*block); - if adjusted_reserved_range.covers(block_range) { - return false; - } - } - true + .is_ok() }) .ok_or(AllocatorError::NoPortBlock)?; Ok((index, block.to_port_number())) @@ -268,20 +238,7 @@ impl PortAllocator { self.usable_blocks .fetch_sub(1, concurrency::sync::atomic::Ordering::Relaxed); - let reserved_port_range_for_block = self.reserved_port_range.and_then(|range| { - range.intersection( - PortRange::new(base_port_index, base_port_index + 255) - .unwrap_or_else(|_| unreachable!()), - ) - }); - - AllocatedPortBlock::new( - ip, - index, - base_port_index, - reserved_port_range_for_block, - allow_null, - ) + AllocatedPortBlock::new(ip, index, base_port_index, allow_null) } pub(crate) fn allocate_port( @@ -344,7 +301,6 @@ impl PortAllocator { ip, index, (port.as_u16() / 256) * 256, // port block base index, discard offset within block - None, allow_null, )?); self.allocated_blocks @@ -357,21 +313,21 @@ impl PortAllocator { ip: Arc>, port: NatPort, ) -> Result>, AllocatorError> { - let (block_was_free, index) = self.try_to_reserve_block(port)?; let allow_null = matches!(port, NatPort::Identifier(_)); - if block_was_free { - return self.allocate_block_for_reservation(ip, index, port, allow_null); + for _ in 0..BLOCK_LOOKUP_ATTEMPTS { + let (block_was_free, index) = self.try_to_reserve_block(port)?; + if block_was_free { + return self.allocate_block_for_reservation(ip, index, port, allow_null); + } + if let Some(block) = self.allocated_blocks.search_for_block(port) { + return Ok(block); + } + // The free flag and map entry change separately. Retry between those updates. + debug!("Block holding port {port} changed mid-lookup; retrying"); + thread::yield_now(); } - self.allocated_blocks - .search_for_block(port) - // Block was not free but is not in the list of allocated blocks either?? - // - // FIXME: This can legitimately happen if the block was released just after we checked - // whether it was free? (Not observed in shuttle tests so far.) Do we need an additional - // lock around the PortAllocator? - .ok_or(AllocatorError::InternalIssue( - "Block not free, although absent from list of allocated blocks".to_string(), - )) + debug!("Block holding port {port} changed {BLOCK_LOOKUP_ATTEMPTS} times"); + Err(AllocatorError::PortReservationFailed(port.as_u16())) } pub(crate) fn reserve_port( @@ -390,10 +346,6 @@ impl PortAllocator { block.reserve_port_from_block(port) } - pub(crate) fn reserved_port_range(&self) -> Option { - self.reserved_port_range - } - // Used for Display pub(crate) fn allocated_port_ranges(&self) -> BTreeSet { self.allocated_blocks.allocated_port_ranges() @@ -426,7 +378,6 @@ impl AllocatedPortBlock { ip: Arc>, index: usize, base_port_idx: u16, - reserved_port_range: Option, allow_null: bool, ) -> Result { let block = Self { @@ -435,30 +386,11 @@ impl AllocatedPortBlock { index, usage_bitmap: Mutex::new(Bitmap256::new()), }; - // Port 0 may be reserved, in which case we don't want to use it, so we mark it as not free. - let reserve_zero = !allow_null && block.base_port_idx == 0; - let reserve_range = reserved_port_range.is_some(); - if reserve_zero || reserve_range { + if !allow_null && block.base_port_idx == 0 { let mut mutex_guard = block.usage_bitmap.lock(); - if reserve_zero { - mutex_guard.reserve_port_from_bitmap(0).map_err(|()| { - AllocatorError::InternalIssue( - "Failed to reserve port 0 from new block".to_string(), - ) - })?; - } - if reserve_range { - mutex_guard - .reserve_port_range_from_bitmap( - // We just check that reserved_port_range.is_some() - reserved_port_range.unwrap_or_else(|| unreachable!()), - ) - .map_err(|()| { - AllocatorError::InternalIssue( - "Failed to reserve port range from new block".to_string(), - ) - })?; - } + mutex_guard.reserve_port_from_bitmap(0).map_err(|()| { + AllocatorError::InternalIssue("Failed to reserve port 0 from new block".to_string()) + })?; } Ok(block) } @@ -575,7 +507,7 @@ impl Drop for AllocatedPortBlock { /// /// It contains a back reference to its parent [`AllocatedPortBlock`], to deallocate the port when /// the [`AllocatedPort`] is dropped. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct AllocatedPort { port: NatPort, // the actual allocated value block_allocator: Arc>, // block/IP the allocated value belongs to @@ -602,7 +534,14 @@ impl AllocatedPort { impl Drop for AllocatedPort { fn drop(&mut self) { debug!("Dropping allocated port {self}..."); - let _ = self.block_allocator.deallocate_port_from_block(self.port); + // Not panicking on a drop path is right; discarding the answer is not. A port that cannot + // be given back has either been given back already or was never recorded as taken, and + // both mean the bitmap no longer says what is in use -- the same accounting whose silence + // let a pair be handed out twice until the error above was made load-bearing. There is + // nothing to do about it here, but it should not pass unsaid. + if let Err(e) = self.block_allocator.deallocate_port_from_block(self.port) { + error!("Failed to give back {self}: {e}"); + } } } @@ -671,13 +610,19 @@ impl AllocatedPortBlockMap { self.0.read().get(&index).cloned() } - fn remove(&self, index: usize) { - self.0.write().remove(&index); + // A live block may replace the expired entry before this lock is acquired. + fn remove_if_still_dead(&self, index: usize) { + let mut blocks = self.0.write(); + if let Some(stored) = blocks.get(&index) + && stored.upgrade().is_none() + { + blocks.remove(&index); + } } fn get(&self, index: usize) -> Option>> { self.get_weak(index)?.upgrade().or_else(|| { - self.remove(index); + self.remove_if_still_dead(index); None }) } @@ -694,11 +639,10 @@ impl AllocatedPortBlockMap { } fn search_for_block(&self, port: NatPort) -> Option>> { - let blocks = self.0.read(); - blocks + self.0 + .read() .values() - .find(|block| block.upgrade().is_some_and(|block| block.covers(port)))? - .upgrade() + .find_map(|block| block.upgrade().filter(|block| block.covers(port))) } // Used for Display @@ -835,85 +779,36 @@ impl Bitmap256 { Err(()) } - fn set_bitmap_value(&mut self, port_in_block: u8, value: u128) -> Result<(), ()> { - if port_in_block < 128 { - if self.first_half & (1 << port_in_block) == value { - return Err(()); - } - self.first_half |= value << port_in_block; + /// Mark a port used or free, failing if it is already in that state. + /// + /// The error is load-bearing in both directions: it is how reserving a port that has already + /// been handed out is refused, rather than the pair going to two flows at once, and how giving + /// back a port that was never taken is reported as the bookkeeping mistake it is. + fn set_bitmap_value(&mut self, port_in_block: u8, used: bool) -> Result<(), ()> { + let (half, bit) = if port_in_block < 128 { + (&mut self.first_half, port_in_block) } else { - if self.second_half & (1 << (port_in_block - 128)) == value { - return Err(()); - } - self.second_half |= value << (port_in_block - 128); - } - Ok(()) - } - - fn deallocate_port_from_bitmap(&mut self, port_in_block: u8) -> Result<(), ()> { - self.set_bitmap_value(port_in_block, 0) - } - - fn reserve_port_from_bitmap(&mut self, port_in_block: u8) -> Result<(), ()> { - self.set_bitmap_value(port_in_block, 1) - } + (&mut self.second_half, port_in_block - 128) + }; + let mask = 1u128 << bit; - fn set_half_bitmap_range( - half: &mut u128, - start_offset: u8, - end_offset: u8, - value: u128, - ) -> Result<(), ()> { - if start_offset > 127 || end_offset > 127 || start_offset > end_offset { + if (*half & mask != 0) == used { return Err(()); } - let mask = if end_offset - start_offset == 127 { - u128::MAX + if used { + *half |= mask; } else { - ((1u128 << (end_offset - start_offset + 1)) - 1) << start_offset - }; - match value { - 0 => { - *half &= !mask; - } - 1 => { - *half |= mask; - } - _ => return Err(()), + *half &= !mask; } Ok(()) } - fn set_bitmap_range( - &mut self, - start_offset: u8, - end_offset: u8, - value: u128, - ) -> Result<(), ()> { - if start_offset < 128 { - Self::set_half_bitmap_range( - &mut self.first_half, - start_offset, - end_offset.min(127), - value, - )?; - } - if end_offset >= 128 { - Self::set_half_bitmap_range( - &mut self.second_half, - start_offset.max(128) - 128, - end_offset - 128, - value, - )?; - } - Ok(()) + fn deallocate_port_from_bitmap(&mut self, port_in_block: u8) -> Result<(), ()> { + self.set_bitmap_value(port_in_block, false) } - fn reserve_port_range_from_bitmap(&mut self, range: PortRange) -> Result<(), ()> { - let start = u8::try_from(range.start() % 256).unwrap_or_else(|_| unreachable!()); - let end = u8::try_from(range.end() % 256).unwrap_or_else(|_| unreachable!()); - self.set_bitmap_range(start, end, 1)?; - Ok(()) + fn reserve_port_from_bitmap(&mut self, port_in_block: u8) -> Result<(), ()> { + self.set_bitmap_value(port_in_block, true) } // Used for Display @@ -989,244 +884,99 @@ mod tests { use lpm::prefix::PortRange; use std::net::Ipv4Addr; - // set_half_bitmap_range() - - #[test] - fn set_half_bitmap_range_single_bit() { - let mut half = 0u128; - Bitmap256::set_half_bitmap_range(&mut half, 0, 0, 1).unwrap(); - assert_eq!(half, 1); - } - - #[test] - fn set_half_bitmap_range_first_few_bits() { - let mut half = 0u128; - Bitmap256::set_half_bitmap_range(&mut half, 0, 3, 1).unwrap(); - assert_eq!(half, 0b1111); - } - - #[test] - fn set_half_bitmap_range_middle_bits() { - let mut half = 0u128; - Bitmap256::set_half_bitmap_range(&mut half, 4, 7, 1).unwrap(); - assert_eq!(half, 0b1111_0000); - } - - #[test] - fn set_half_bitmap_range_full_range() { - let mut half = 0u128; - Bitmap256::set_half_bitmap_range(&mut half, 0, 127, 1).unwrap(); - assert_eq!(half, u128::MAX); - } - - #[test] - fn set_half_bitmap_range_clear_bits() { - let mut half = u128::MAX; - Bitmap256::set_half_bitmap_range(&mut half, 4, 7, 0).unwrap(); - assert_eq!(half, !0b1111_0000); - } + // set_bitmap_value(), through the two operations built on it - #[test] - fn set_half_bitmap_range_invalid_start_gt_end() { - let mut half = 0u128; - assert!(Bitmap256::set_half_bitmap_range(&mut half, 5, 3, 1).is_err()); - } - - #[test] - fn set_half_bitmap_range_invalid_start_too_large() { - let mut half = 0u128; - assert!(Bitmap256::set_half_bitmap_range(&mut half, 128, 128, 1).is_err()); - } - - #[test] - fn set_half_bitmap_range_invalid_end_too_large() { - let mut half = 0u128; - assert!(Bitmap256::set_half_bitmap_range(&mut half, 0, 128, 1).is_err()); - } - - #[test] - fn set_half_bitmap_range_invalid_value() { - let mut half = 0u128; - assert!(Bitmap256::set_half_bitmap_range(&mut half, 0, 3, 2).is_err()); - } - - #[test] - fn set_half_bitmap_range_high_bits() { - let mut half = 0u128; - Bitmap256::set_half_bitmap_range(&mut half, 120, 127, 1).unwrap(); - let expected = ((1u128 << 8) - 1) << 120; - assert_eq!(half, expected); - } - - // set_bitmap_range() - - #[test] - fn set_bitmap_range_first_half_only() { - let mut bitmap = Bitmap256::new(); - bitmap.set_bitmap_range(10, 20, 1).unwrap(); - let expected = ((1u128 << 11) - 1) << 10; - assert_eq!(bitmap.first_half, expected); - assert_eq!(bitmap.second_half, 0); + fn port_is_used(bitmap: &Bitmap256, port: u8) -> bool { + if port < 128 { + bitmap.first_half & (1u128 << port) != 0 + } else { + bitmap.second_half & (1u128 << (port - 128)) != 0 + } } + // A port given back is free again. Blocks outlive the ports drawn from them, so a port that + // stays marked used is one the block can never hand out again. #[test] - fn set_bitmap_range_second_half_only() { + fn a_deallocated_port_becomes_free_again() { let mut bitmap = Bitmap256::new(); - bitmap.set_bitmap_range(130, 140, 1).unwrap(); - let expected = ((1u128 << 11) - 1) << 2; // 130-128=2 - assert_eq!(bitmap.first_half, 0); - assert_eq!(bitmap.second_half, expected); - } + for port in [5u8, 200] { + bitmap.reserve_port_from_bitmap(port).unwrap(); + assert!( + port_is_used(&bitmap, port), + "port {port} was not marked used" + ); - #[test] - fn set_bitmap_range_spans_both_halves() { - let mut bitmap = Bitmap256::new(); - bitmap.set_bitmap_range(120, 135, 1).unwrap(); - // First half: bits 120..=127 - let first_expected = ((1u128 << 8) - 1) << 120; - // Second half: bits 0..=7 (135-128=7) - let second_expected = (1u128 << 8) - 1; - assert_eq!(bitmap.first_half, first_expected); - assert_eq!(bitmap.second_half, second_expected); + bitmap.deallocate_port_from_bitmap(port).unwrap(); + assert!( + !port_is_used(&bitmap, port), + "port {port} is still marked used after being given back" + ); + } } + // Allocation hands out the lowest free port, so a freed port is the next one out. #[test] - fn set_bitmap_range_full_range() { + fn a_deallocated_port_is_handed_out_again() { let mut bitmap = Bitmap256::new(); - bitmap.set_bitmap_range(0, 255, 1).unwrap(); - assert_eq!(bitmap.first_half, u128::MAX); - assert_eq!(bitmap.second_half, u128::MAX); - } + let first = bitmap.allocate_port_from_bitmap().unwrap(); + let second = bitmap.allocate_port_from_bitmap().unwrap(); + assert_eq!((first, second), (0, 1)); - #[test] - fn set_bitmap_range_clear_spanning() { - let mut bitmap = Bitmap256::new(); - bitmap.first_half = u128::MAX; - bitmap.second_half = u128::MAX; - bitmap.set_bitmap_range(120, 135, 0).unwrap(); - let first_expected = !(((1u128 << 8) - 1) << 120); - let second_expected = !((1u128 << 8) - 1); - assert_eq!(bitmap.first_half, first_expected); - assert_eq!(bitmap.second_half, second_expected); + bitmap + .deallocate_port_from_bitmap(u8::try_from(first).unwrap()) + .unwrap(); + assert_eq!( + bitmap.allocate_port_from_bitmap().unwrap(), + first, + "a freed port was not handed out again" + ); } + // Reserving a port already in use has to fail: that is what tells a flow being carried across + // a config change that its address and port have been taken, rather than handing the same pair + // to two flows. #[test] - fn set_bitmap_range_at_boundary_128() { - let mut bitmap = Bitmap256::new(); - bitmap.set_bitmap_range(127, 128, 1).unwrap(); - assert_eq!(bitmap.first_half, 1u128 << 127); - assert_eq!(bitmap.second_half, 1u128); + fn reserving_a_used_port_fails() { + for port in [0u8, 7, 128, 201] { + let mut bitmap = Bitmap256::new(); + bitmap.reserve_port_from_bitmap(port).unwrap(); + assert!( + bitmap.reserve_port_from_bitmap(port).is_err(), + "reserving port {port} twice was allowed" + ); + } } - // reserve_port_range_from_bitmap() - + // An allocated nonzero port must also be unavailable for reservation. #[test] - fn reserve_port_range_marks_bits() { + fn reserving_an_allocated_port_fails() { let mut bitmap = Bitmap256::new(); - let range = PortRange::new(10, 19).unwrap(); - bitmap.reserve_port_range_from_bitmap(range).unwrap(); - let expected = ((1u128 << 10) - 1) << 10; - assert_eq!(bitmap.first_half, expected); + let mut allocated = 0; + for _ in 0..4 { + allocated = u8::try_from(bitmap.allocate_port_from_bitmap().unwrap()).unwrap(); + } + assert_ne!(allocated, 0); + assert!( + bitmap.reserve_port_from_bitmap(allocated).is_err(), + "reserving port {allocated}, which is allocated, was allowed" + ); } + // Giving back a port that is already free is a bookkeeping error, and says so. #[test] - fn reserve_port_range_prevents_allocation() { + fn deallocating_a_free_port_fails() { let mut bitmap = Bitmap256::new(); - // Reserve ports 0..=9 - let range = PortRange::new(0, 9).unwrap(); - bitmap.reserve_port_range_from_bitmap(range).unwrap(); - // First allocation should skip reserved ports and return 10 - let port = bitmap.allocate_port_from_bitmap().unwrap(); - assert_eq!(port, 10); - } - - // pick_available_block() - - #[test] - fn pick_available_block_no_reserved_range() { - let allocator = PortAllocator::::new_no_randomness(None, false); - let (index, base_port) = allocator.pick_available_block().unwrap(); - assert_eq!(index, 0); - assert_eq!(base_port, 0); - } - - #[test] - fn pick_available_block_reserved_range_covers_first_block() { - // Reserve 0..=255 (entire first block) → should skip to block 1 (ports 256-511) - let reserved = PortRange::new(0, 255).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); - let (index, base_port) = allocator.pick_available_block().unwrap(); - assert_eq!(index, 1); - assert_eq!(base_port, 256); - } - - #[test] - fn pick_available_block_reserved_range_starting_at_one() { - // Corner case: reserved 1..=255 does not literally cover 0..=255, but port 0 cannot - // be allocated anyway, so the block is effectively unusable. The code adjusts the - // reserved range to start at 0, causing the block to be skipped. - let reserved = PortRange::new(1, 255).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); - let (index, base_port) = allocator.pick_available_block().unwrap(); - assert_eq!(index, 1); - assert_eq!(base_port, 256); - } - - #[test] - fn pick_available_block_reserved_range_covers_multiple_blocks() { - // Reserve 0..=511 (first two blocks) → should skip to block 2 (ports 512-767) - let reserved = PortRange::new(0, 511).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); - let (index, base_port) = allocator.pick_available_block().unwrap(); - assert_eq!(index, 2); - assert_eq!(base_port, 512); + assert!(bitmap.deallocate_port_from_bitmap(9).is_err()); } #[test] - fn pick_available_block_reserved_range_does_not_cover_other_blocks() { - // Reserve 0..=255 only covers block 0, block 1 is unaffected - let reserved = PortRange::new(0, 255).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); - // First pick skips block 0, gets block 1 - let (_, base_port1) = allocator.pick_available_block().unwrap(); - assert_eq!(base_port1, 256); - // Second pick gets block 2 - let (_, base_port2) = allocator.pick_available_block().unwrap(); - assert_eq!(base_port2, 512); - } - - #[test] - fn pick_available_block_partial_reserved_range_does_not_skip() { - // Reserve 1..=200 (len 200 < 255) → block is NOT skipped entirely, individual ports - // are reserved within the block instead - let reserved = PortRange::new(1, 200).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); + fn pick_available_block_starts_at_zero() { + let allocator = PortAllocator::::new_no_randomness(false); let (index, base_port) = allocator.pick_available_block().unwrap(); assert_eq!(index, 0); assert_eq!(base_port, 0); } - #[test] - fn pick_available_block_reserved_middle_block() { - // Reserve 256..=511 (block 1 only) → block 0 is fine, block 1 is skipped - let reserved = PortRange::new(256, 511).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); - // First pick: block 0 - let (_, base_port1) = allocator.pick_available_block().unwrap(); - assert_eq!(base_port1, 0); - // Second pick: block 1 is skipped, picks block 2 - let (_, base_port2) = allocator.pick_available_block().unwrap(); - assert_eq!(base_port2, 512); - } - - #[test] - fn pick_available_block_all_blocks_reserved() { - // Reserve 0..=65535 (all blocks) → NoPortBlock error - let reserved = PortRange::new(0, 65535).unwrap(); - let allocator = PortAllocator::::new_no_randomness(Some(reserved), false); - assert!(allocator.pick_available_block().is_err()); - } - fn port_range(start: u16, end: u16) -> PortRange { PortRange::new(start, end).unwrap() } @@ -1354,7 +1104,7 @@ mod tests { fn exclude_wellknown_ports_first_available_block_is_1024() { // With no randomness and IANA exclusion, blocks 0-3 (ports 0-1023) are pre-marked // non-free, so the first block handed out should start at port 1024. - let allocator = PortAllocator::::new_no_randomness(None, true); + let allocator = PortAllocator::::new_no_randomness(true); let (_, base_port) = allocator.pick_available_block().unwrap(); assert_eq!(base_port, 1024); } @@ -1363,7 +1113,7 @@ mod tests { fn exclude_wellknown_ports_all_252_blocks_are_above_1023() { // Exactly 252 blocks (256 - 4 IANA blocks) should be allocatable; every one should // start at port >= 1024. The 253rd attempt should fail with NoPortBlock. - let allocator = PortAllocator::::new_no_randomness(None, true); + let allocator = PortAllocator::::new_no_randomness(true); for _ in 0..252 { let (_, base_port) = allocator.pick_available_block().unwrap(); assert!( @@ -1377,29 +1127,8 @@ mod tests { #[test] fn exclude_wellknown_ports_disabled_starts_at_port_zero() { // Sanity check: without the flag, block 0 (port 0) is returned first. - let allocator = PortAllocator::::new_no_randomness(None, false); + let allocator = PortAllocator::::new_no_randomness(false); let (_, base_port) = allocator.pick_available_block().unwrap(); assert_eq!(base_port, 0); } - - #[test] - fn exclude_wellknown_ports_combined_with_reserved_range() { - let reserved = PortRange::new(2048, 2303).unwrap(); // entire block 8 - let allocator = PortAllocator::::new_no_randomness(Some(reserved), true); - - let (_, b0) = allocator.pick_available_block().unwrap(); - assert_eq!(b0, 1024); // block 4 - - let (_, b1) = allocator.pick_available_block().unwrap(); - assert_eq!(b1, 1280); // block 5 - - let (_, b2) = allocator.pick_available_block().unwrap(); - assert_eq!(b2, 1536); // block 6 - - let (_, b3) = allocator.pick_available_block().unwrap(); - assert_eq!(b3, 1792); // block 7 - - let (_, b4) = allocator.pick_available_block().unwrap(); - assert_eq!(b4, 2304); // block 9 — block 8 (2048-2303) was skipped - } } diff --git a/nat/src/masquerade/apalloc/region.rs b/nat/src/masquerade/apalloc/region.rs new file mode 100644 index 0000000000..ee1c8a3a8a --- /dev/null +++ b/nat/src/masquerade/apalloc/region.rs @@ -0,0 +1,581 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Open Network Fabric Authors + +//! Split overlapping public address ranges into disjoint regions with constant ownership. +//! +//! Each region has one allocator shared by its owners, preventing duplicate public tuples. Port +//! ranges are not part of this decomposition. + +use std::collections::{BTreeMap, BTreeSet}; + +/// An inclusive range of addresses, as raw bits, so that IPv4 and IPv6 can be cut the same way. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct AddrInterval { + pub(crate) start: u128, + pub(crate) end: u128, +} + +impl AddrInterval { + pub(crate) fn new(start: u128, end: u128) -> Self { + debug_assert!(start <= end, "an interval cannot end before it starts"); + Self { start, end } + } + + pub(crate) fn contains(&self, addr: u128) -> bool { + self.start <= addr && addr <= self.end + } + + /// Number of addresses covered, saturating at [`u128::MAX`] for the whole space. + pub(crate) fn len(&self) -> u128 { + (self.end - self.start).saturating_add(1) + } +} + +/// A stretch of public address space over which the set of exposes entitled to allocate does not +/// change. Owners are indices into the slice handed to [`decompose`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Region { + pub(crate) range: AddrInterval, + pub(crate) owners: BTreeSet, +} + +/// Cut the input ranges into ordered, maximal, disjoint regions labelled by owner index. +pub(crate) fn decompose(owner_ranges: &[Vec]) -> Vec { + // Coverage can only change at the start of a range, or just past the end of one. Cutting at + // every such point gives elementary intervals that each range either covers entirely or does + // not touch at all, so testing a single address per interval decides ownership. + let mut cuts = BTreeSet::new(); + for ranges in owner_ranges { + for range in ranges { + cuts.insert(range.start); + // A range ending at the top of the space has nothing past it to cut at. + if let Some(past_end) = range.end.checked_add(1) { + cuts.insert(past_end); + } + } + } + + let cuts: Vec = cuts.into_iter().collect(); + let mut regions: Vec = Vec::new(); + for (index, &start) in cuts.iter().enumerate() { + let end = match cuts.get(index + 1) { + Some(&next_cut) => next_cut - 1, + None => u128::MAX, + }; + let owners: BTreeSet = owner_ranges + .iter() + .enumerate() + .filter(|(_, ranges)| ranges.iter().any(|range| range.contains(start))) + .map(|(owner, _)| owner) + .collect(); + if owners.is_empty() { + // A gap between ranges, or the tail past the last one. + continue; + } + debug_assert!( + owner_ranges.iter().enumerate().all(|(owner, ranges)| { + owners.contains(&owner) == ranges.iter().any(|range| range.contains(end)) + }), + "an elementary interval must be covered as a whole or not at all" + ); + regions.push(Region { + range: AddrInterval::new(start, end), + owners, + }); + } + + merge_adjacent(regions) +} + +// Neighbouring elementary intervals with the same owners describe one region. This happens when an +// expose lists several adjacent prefixes, and keeps the number of allocators down. +fn merge_adjacent(regions: Vec) -> Vec { + let mut merged: Vec = Vec::with_capacity(regions.len()); + for region in regions { + match merged.last_mut() { + Some(previous) + if previous.range.end.checked_add(1) == Some(region.range.start) + && previous.owners == region.owners => + { + previous.range.end = region.range.end; + } + _ => merged.push(region), + } + } + merged +} + +/// The regions each owner may allocate from, in the order they should be tried. +/// +/// Regions an owner has to itself come first: allocating there cannot contend with another VPC, and +/// leaves the shared regions for the exposes that have nowhere else to go. +pub(crate) fn regions_by_owner(regions: &[Region]) -> BTreeMap> { + let mut by_owner: BTreeMap> = BTreeMap::new(); + for (index, region) in regions.iter().enumerate() { + for &owner in ®ion.owners { + by_owner.entry(owner).or_default().push(index); + } + } + for indices in by_owner.values_mut() { + indices.sort_by_key(|&index| { + // Fewest sharers first, then widest, then by address so the order is deterministic. + ( + regions[index].owners.len(), + std::cmp::Reverse(regions[index].range.len()), + regions[index].range.start, + ) + }); + } + by_owner +} + +#[cfg(test)] +mod bolero_tests { + use super::*; + use bolero::{Driver, TypeGenerator}; + + // A narrow window makes overlap common and permits checking every address. + const WINDOW: u128 = 48; + const MAX_OWNERS: u8 = 5; + const MAX_RANGES_PER_OWNER: u8 = 3; + const MAX_RANGE_LEN: u8 = 12; + + // Where the window sits. Includes both ends of the space, so the cut just past the end of a + // range is exercised where it can overflow and where the first address has nothing below it. + const BASES: [u128; 4] = [ + 0, + 1, + (u32::MAX as u128) - WINDOW + 1, + u128::MAX - WINDOW + 1, + ]; + + /// A generated set of owner ranges: a window position, and per owner a list of ranges given as + /// an offset into the window and a length. + #[derive(Debug, Clone)] + struct Scenario { + base: u128, + owners: Vec>, + } + + impl TypeGenerator for Scenario { + fn generate(driver: &mut D) -> Option { + let base = BASES[usize::from(driver.produce::()? % 4)]; + let owner_count = usize::from(driver.produce::()? % MAX_OWNERS + 1); + let mut owners = Vec::with_capacity(owner_count); + for _ in 0..owner_count { + let range_count = usize::from(driver.produce::()? % MAX_RANGES_PER_OWNER + 1); + let mut ranges = Vec::with_capacity(range_count); + for _ in 0..range_count { + let offset = driver.produce::()? % u8::try_from(WINDOW).ok()?; + let length = driver.produce::()? % MAX_RANGE_LEN + 1; + ranges.push((offset, length)); + } + owners.push(ranges); + } + Some(Self { base, owners }) + } + } + + impl Scenario { + // Materialize the ranges, clipped to the window so that every address a region can cover + // is one the oracle below walks. + fn owner_ranges(&self) -> Vec> { + self.owners + .iter() + .map(|ranges| { + ranges + .iter() + .map(|&(offset, length)| { + let start = u128::from(offset); + let end = (start + u128::from(length) - 1).min(WINDOW - 1); + AddrInterval::new(self.base + start, self.base + end) + }) + .collect() + }) + .collect() + } + + fn addresses(&self) -> impl Iterator + '_ { + (0..WINDOW).map(move |offset| self.base + offset) + } + + // The oracle: who covers this address, straight from the inputs. + fn owners_covering(ranges: &[Vec], address: u128) -> BTreeSet { + ranges + .iter() + .enumerate() + .filter(|(_, ranges)| ranges.iter().any(|range| range.contains(address))) + .map(|(owner, _)| owner) + .collect() + } + } + + #[test] + fn decompose_properties() { + bolero::check!() + .with_type() + .cloned() + .for_each(|scenario: Scenario| { + let owner_ranges = scenario.owner_ranges(); + let regions = decompose(&owner_ranges); + + for region in ®ions { + assert!( + region.range.start <= region.range.end, + "region {:?} ends before it starts", + region.range + ); + assert!( + !region.owners.is_empty(), + "region {:?} has no owner, so nothing may allocate from it", + region.range + ); + assert!( + region.owners.iter().all(|&o| o < owner_ranges.len()), + "region {:?} names an owner that does not exist", + region.range + ); + } + + // Regions are ordered and never overlap. Overlap is what would let two allocators + // hand out the same address. + for pair in regions.windows(2) { + assert!( + pair[0].range.end < pair[1].range.start, + "regions {:?} and {:?} overlap or are out of order", + pair[0].range, + pair[1].range + ); + } + + // Every address has exactly the owners declared by the inputs. + for address in scenario.addresses() { + let expected = Scenario::owners_covering(&owner_ranges, address); + match regions.iter().find(|region| region.range.contains(address)) { + Some(region) => assert_eq!( + region.owners, expected, + "address {address} is owned by {:?} but was claimed by {expected:?}", + region.owners + ), + None => assert!( + expected.is_empty(), + "address {address} was claimed by {expected:?} but is in no region" + ), + } + } + + // Regions are maximal: neighbours that touch must differ in their owners, or they + // should have been one region. + for pair in regions.windows(2) { + if pair[0].range.end.checked_add(1) == Some(pair[1].range.start) { + assert_ne!( + pair[0].owners, pair[1].owners, + "adjacent regions {:?} and {:?} have the same owners", + pair[0].range, pair[1].range + ); + } + } + + // Building the pools twice for one config must give the same answer. + assert_eq!( + decompose(&owner_ranges), + regions, + "decompose is not a function" + ); + }); + } + + #[test] + fn regions_by_owner_properties() { + bolero::check!() + .with_type() + .cloned() + .for_each(|scenario: Scenario| { + let owner_ranges = scenario.owner_ranges(); + let regions = decompose(&owner_ranges); + let by_owner = regions_by_owner(®ions); + + for (owner, _) in owner_ranges.iter().enumerate() { + let listed = by_owner.get(&owner).map_or(&[][..], Vec::as_slice); + + // An owner is offered exactly the regions it owns: nothing it may not use, and + // nothing it may use left out. + let expected: BTreeSet = regions + .iter() + .enumerate() + .filter(|(_, region)| region.owners.contains(&owner)) + .map(|(index, _)| index) + .collect(); + assert_eq!( + listed.iter().copied().collect::>(), + expected, + "owner {owner} was offered the wrong regions" + ); + + // No region is offered twice, or the same space would be tried repeatedly. + assert_eq!( + listed.len(), + expected.len(), + "owner {owner} was offered a region more than once" + ); + + // Space the owner has to itself comes first. + for pair in listed.windows(2) { + assert!( + regions[pair[0]].owners.len() <= regions[pair[1]].owners.len(), + "owner {owner} is offered shared space before exclusive space" + ); + } + + // Every address the owner claimed is in one of the regions it was offered. + for address in scenario.addresses() { + if owner_ranges[owner] + .iter() + .any(|range| range.contains(address)) + { + assert!( + listed + .iter() + .any(|&index| regions[index].range.contains(address)), + "owner {owner} claimed address {address} but was offered no region holding it" + ); + } + } + } + }); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn interval(start: u128, end: u128) -> AddrInterval { + AddrInterval::new(start, end) + } + + fn owners(indices: &[usize]) -> BTreeSet { + indices.iter().copied().collect() + } + + #[test] + fn decompose_single_range_is_left_whole() { + let regions = decompose(&[vec![interval(10, 20)]]); + assert_eq!( + regions, + vec![Region { + range: interval(10, 20), + owners: owners(&[0]) + }] + ); + } + + #[test] + fn decompose_disjoint_ranges_stay_separate() { + let regions = decompose(&[vec![interval(10, 20)], vec![interval(30, 40)]]); + assert_eq!( + regions, + vec![ + Region { + range: interval(10, 20), + owners: owners(&[0]) + }, + Region { + range: interval(30, 40), + owners: owners(&[1]) + }, + ] + ); + } + + #[test] + fn decompose_identical_ranges_share_one_region() { + let regions = decompose(&[vec![interval(10, 20)], vec![interval(10, 20)]]); + assert_eq!( + regions, + vec![Region { + range: interval(10, 20), + owners: owners(&[0, 1]) + }] + ); + } + + #[test] + fn decompose_partial_overlap_splits_into_three() { + // 0: [10, 20], 1: [15, 25] -> [10,14] {0}, [15,20] {0,1}, [21,25] {1} + let regions = decompose(&[vec![interval(10, 20)], vec![interval(15, 25)]]); + assert_eq!( + regions, + vec![ + Region { + range: interval(10, 14), + owners: owners(&[0]) + }, + Region { + range: interval(15, 20), + owners: owners(&[0, 1]) + }, + Region { + range: interval(21, 25), + owners: owners(&[1]) + }, + ] + ); + } + + #[test] + fn decompose_nested_range_splits_into_three() { + // 1 sits strictly inside 0. + let regions = decompose(&[vec![interval(10, 30)], vec![interval(15, 20)]]); + assert_eq!( + regions, + vec![ + Region { + range: interval(10, 14), + owners: owners(&[0]) + }, + Region { + range: interval(15, 20), + owners: owners(&[0, 1]) + }, + Region { + range: interval(21, 30), + owners: owners(&[0]) + }, + ] + ); + } + + #[test] + fn decompose_three_way_overlap() { + // A staircase: every combination of owners shows up. + let regions = decompose(&[ + vec![interval(0, 30)], + vec![interval(10, 40)], + vec![interval(20, 50)], + ]); + assert_eq!( + regions, + vec![ + Region { + range: interval(0, 9), + owners: owners(&[0]) + }, + Region { + range: interval(10, 19), + owners: owners(&[0, 1]) + }, + Region { + range: interval(20, 30), + owners: owners(&[0, 1, 2]) + }, + Region { + range: interval(31, 40), + owners: owners(&[1, 2]) + }, + Region { + range: interval(41, 50), + owners: owners(&[2]) + }, + ] + ); + } + + #[test] + fn decompose_adjacent_ranges_of_one_owner_are_merged() { + let regions = decompose(&[vec![interval(10, 19), interval(20, 29)]]); + assert_eq!( + regions, + vec![Region { + range: interval(10, 29), + owners: owners(&[0]) + }] + ); + } + + #[test] + fn decompose_gap_between_ranges_is_not_covered() { + let regions = decompose(&[vec![interval(10, 19), interval(30, 39)]]); + assert_eq!( + regions, + vec![ + Region { + range: interval(10, 19), + owners: owners(&[0]) + }, + Region { + range: interval(30, 39), + owners: owners(&[0]) + }, + ] + ); + } + + #[test] + fn decompose_range_reaching_the_top_of_the_space() { + let regions = decompose(&[vec![interval(u128::MAX - 1, u128::MAX)]]); + assert_eq!( + regions, + vec![Region { + range: interval(u128::MAX - 1, u128::MAX), + owners: owners(&[0]) + }] + ); + } + + #[test] + fn decompose_empty_input() { + assert_eq!(decompose(&[]), vec![]); + assert_eq!(decompose(&[vec![]]), vec![]); + } + + // The properties the allocator depends on: regions never overlap, and every owner's range is + // covered exactly by the regions it owns. + #[test] + fn decompose_regions_are_disjoint_and_cover_each_owner_exactly() { + let inputs = vec![ + vec![interval(0, 30), interval(60, 70)], + vec![interval(10, 40)], + vec![interval(20, 50), interval(65, 80)], + vec![interval(100, 100)], + ]; + let regions = decompose(&inputs); + + for pair in regions.windows(2) { + assert!( + pair[0].range.end < pair[1].range.start, + "regions {:?} and {:?} overlap", + pair[0].range, + pair[1].range + ); + } + + for (owner, ranges) in inputs.iter().enumerate() { + for address in 0..=120u128 { + let in_owner_range = ranges.iter().any(|range| range.contains(address)); + let in_owned_region = regions + .iter() + .any(|region| region.owners.contains(&owner) && region.range.contains(address)); + assert_eq!( + in_owner_range, in_owned_region, + "address {address} disagrees for owner {owner}" + ); + } + } + } + + #[test] + fn regions_by_owner_prefers_exclusive_regions() { + // 0 owns [0,9] alone and shares [10,19] with 1. + let regions = decompose(&[vec![interval(0, 19)], vec![interval(10, 19)]]); + let by_owner = regions_by_owner(®ions); + + let for_zero = &by_owner[&0]; + assert_eq!(regions[for_zero[0]].range, interval(0, 9)); + assert_eq!(regions[for_zero[1]].range, interval(10, 19)); + + // 1 only has the shared region. + assert_eq!(by_owner[&1].len(), 1); + assert_eq!(regions[by_owner[&1][0]].range, interval(10, 19)); + } +} diff --git a/nat/src/masquerade/apalloc/setup.rs b/nat/src/masquerade/apalloc/setup.rs index 7c587a7345..8465b7ae4e 100644 --- a/nat/src/masquerade/apalloc/setup.rs +++ b/nat/src/masquerade/apalloc/setup.rs @@ -1,46 +1,37 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright Open Network Fabric Authors -use super::NatIpWithBitmap; -use super::alloc::{IpAllocator, NatPool, PoolBitmap}; -use super::{NatAllocator, PoolTable, PoolTableKey}; +//! Build masquerade pools by grouping exposes per peer VPC and splitting overlapping public ranges +//! into disjoint, shared regions. + +use super::alloc::{IpAllocator, NatPool, PoolSet}; +use super::region::{AddrInterval, Region, decompose, regions_by_owner}; +use super::{NatAllocator, NatIpWithBitmap, PoolTable, PoolTableKey}; +use crate::masquerade::allocator_writer::MasqueradeConfig; use crate::masquerade::natip::NatIp; -use crate::ranges::IpRange; -use config::external::overlay::vpc::ValidatedPeering; use config::external::overlay::vpcpeering::{ValidatedExpose, ValidatedManifest}; -use lpm::prefix::range_map::DisjointRangesBTreeMap; -use lpm::prefix::{ - IpPrefix, L4Protocol, PortRange, Prefix, PrefixPortsSet, PrefixWithOptionalPorts, -}; +use lpm::prefix::{PrefixPortsSet, PrefixWithOptionalPorts}; use net::ip::NextHeader; use net::packet::VpcDiscriminant; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::time::Duration; -use tracing::error; +use tracing::debug; const DEFAULT_MASQUERADE_IDLE_TIMEOUT: Duration = Duration::from_mins(2); impl NatAllocator { - pub(crate) fn add_peering_addresses( - &mut self, - peering: &ValidatedPeering, - dst_vpc_id: VpcDiscriminant, - ) { - build_nat_pool_generic( - peering.local(), - dst_vpc_id, + pub(crate) fn build_pools(&mut self, config: &MasqueradeConfig) { + build_pools_generic( + config, ValidatedManifest::masquerade_exposes_44, - ValidatedManifest::port_forwarding_exposes_44, &mut self.pools_src44, NextHeader::ICMP, self.randomize, ); - build_nat_pool_generic( - peering.local(), - dst_vpc_id, + build_pools_generic( + config, ValidatedManifest::masquerade_exposes_66, - ValidatedManifest::port_forwarding_exposes_66, &mut self.pools_src66, NextHeader::ICMP6, self.randomize, @@ -48,207 +39,209 @@ impl NatAllocator { } } -#[allow(clippy::too_many_arguments)] -fn build_nat_pool_generic<'a, I: NatIpWithBitmap, J: NatIpWithBitmap, F, FIter, P, PIter>( - manifest: &'a ValidatedManifest, - dst_vpc_id: VpcDiscriminant, - // A filter to select relevant exposes: those with masquerade, for the relevant IP version +/////////////////////////////////////////////////////////////////////////////// +// Gathering +/////////////////////////////////////////////////////////////////////////////// + +/// One masquerade expose, with everything the pools need from it. +struct GatheredExpose<'a> { + src_vpc_id: VpcDiscriminant, + // The private prefixes this expose masquerades, which is what the pool table is keyed by. + private_prefixes: &'a PrefixPortsSet, + // The public range this expose allocates from, as raw address intervals. + public_ranges: Vec, + idle_timeout: Duration, +} + +// Exposes toward different peers may safely reuse the same public range. +fn gather_exposes<'a, J, F, FIter>( + config: &'a MasqueradeConfig, + exposes_filter: &F, +) -> BTreeMap>> +where + J: NatIp, + F: Fn(&'a ValidatedManifest) -> FIter, + FIter: Iterator, +{ + let mut groups: BTreeMap>> = BTreeMap::new(); + + for nat_peering in config.iter() { + let manifest = nat_peering.peering.local(); + for expose in exposes_filter(manifest) { + let public_ranges = public_intervals::(expose.as_range_or_empty()); + if public_ranges.is_empty() { + // A masquerade expose is validated to have a non-empty as_range, so this only + // happens if none of its prefixes are of the version we are building. + continue; + } + groups + .entry(nat_peering.dst_vpcd) + .or_default() + .push(GatheredExpose { + src_vpc_id: nat_peering.src_vpcd, + private_prefixes: expose.ips(), + public_ranges, + idle_timeout: expose + .idle_timeout() + .unwrap_or(DEFAULT_MASQUERADE_IDLE_TIMEOUT), + }); + } + } + + groups +} + +// Convert a set of public prefixes into raw address intervals, dropping any that are not of the +// version being built. +fn public_intervals(ranges: &PrefixPortsSet) -> Vec { + ranges + .iter() + .filter_map(|prefix| { + // FIXME: Account for port ranges. A public range may be restricted to a port range, + // which the pools do not model, so the whole port space of the address is used. + let start = J::try_from_addr(prefix.prefix().as_address()).ok()?; + let end = J::try_from_addr(prefix.prefix().last_address()).ok()?; + Some(AddrInterval::new(start.to_addr_bits(), end.to_addr_bits())) + }) + .collect() +} + +/////////////////////////////////////////////////////////////////////////////// +// Building +/////////////////////////////////////////////////////////////////////////////// + +fn build_pools_generic<'a, I, J, F, FIter>( + config: &'a MasqueradeConfig, exposes_filter: F, - // A filter to select other exposes with port forwarding, for the relevant IP version - port_forwarding_exposes_filter: P, table: &mut PoolTable, icmp_proto: NextHeader, randomize: bool, ) where - F: FnOnce(&'a ValidatedManifest) -> FIter, + I: NatIpWithBitmap, + J: NatIpWithBitmap, + F: Fn(&'a ValidatedManifest) -> FIter, FIter: Iterator, - P: FnOnce(&'a ValidatedManifest) -> PIter, - PIter: Iterator, { - let port_forwarding_exposes: Vec<&'a ValidatedExpose> = - port_forwarding_exposes_filter(manifest).collect(); - - exposes_filter(manifest).for_each(|expose| { - let prefixes_and_ports_to_exclude_from_pools = - find_masquerade_portfw_overlap(&port_forwarding_exposes, expose); + let groups = gather_exposes::(config, &exposes_filter); + + for (dst_vpc_id, exposes) in groups { + // Allocations for TCP, for example, do not affect allocations for UDP or for ICMP: the + // space made of addresses and L4 ports or identifiers is distinct for each protocol. So + // each region backs one allocator per protocol, over the same addresses. + for protocol in [NextHeader::TCP, NextHeader::UDP, icmp_proto] { + let specs: Vec = exposes + .iter() + .map(|expose| PoolSpec { + public_ranges: expose.public_ranges.clone(), + idle_timeout: expose.idle_timeout, + }) + .collect(); + + let pool_sets = pool_sets_for_specs::(&specs, protocol, randomize); + for (expose, pool_set) in exposes.iter().zip(pool_sets) { + add_pool_entries( + table, + expose.private_prefixes, + expose.src_vpc_id, + dst_vpc_id, + protocol, + &pool_set, + ); + } + } + } +} - let idle_timeout = expose - .idle_timeout() - .unwrap_or(DEFAULT_MASQUERADE_IDLE_TIMEOUT); +/// The config-independent inputs for one expose's pools. +#[derive(Clone)] +pub(crate) struct PoolSpec { + pub(crate) public_ranges: Vec, + pub(crate) idle_timeout: Duration, +} - // TCP/UDP masquerade allocators should avoid the IANA system/well-known range - // (0-1023). ICMP identifiers are allocated independently and are not subject to that - // TCP/UDP source-port policy. - let tcp_ip_allocator = ip_allocator_for_prefixes( - expose.as_range_or_empty(), - idle_timeout, - &prefixes_and_ports_to_exclude_from_pools.tcp, - randomize, - true, - ); - let udp_ip_allocator = ip_allocator_for_prefixes( - expose.as_range_or_empty(), - idle_timeout, - &prefixes_and_ports_to_exclude_from_pools.udp, - randomize, - true, - ); - let icmp_ip_allocator = ip_allocator_for_prefixes( - expose.as_range_or_empty(), - idle_timeout, - &PrefixPortsSet::default(), - randomize, - false, - ); +/// Cut the space the given exposes claim into disjoint regions, build one allocator per region, +/// and return the pools each expose may allocate from, in the same order as `specs`. +/// +/// This is where the guarantee lives: exposes sharing a region share its allocator, so a public +/// address and port cannot be handed out twice, and an expose is only ever offered regions its own +/// ranges cover. +pub(crate) fn pool_sets_for_specs( + specs: &[PoolSpec], + protocol: NextHeader, + randomize: bool, +) -> Vec> { + let owner_ranges: Vec> = specs + .iter() + .map(|spec| spec.public_ranges.clone()) + .collect(); + let regions = decompose(&owner_ranges); + debug!( + "Public space cut into {} region(s) for {} expose(s) ({protocol})", + regions.len(), + specs.len() + ); - add_pool_entries( - table, - expose.ips(), - dst_vpc_id, - &tcp_ip_allocator, - &udp_ip_allocator, - &icmp_ip_allocator, - icmp_proto, - ); - }); -} + let allocators = build_region_allocators::(®ions, protocol, randomize); + let by_owner = regions_by_owner(®ions); -#[derive(Debug, Default, Clone, PartialEq, Eq)] -struct ReserveSets { - tcp: PrefixPortsSet, - udp: PrefixPortsSet, + specs + .iter() + .enumerate() + .map(|(owner, spec)| { + let mut pool_set = PoolSet::new(spec.idle_timeout); + for ®ion_index in by_owner.get(&owner).map_or(&[][..], Vec::as_slice) { + pool_set.push_region( + regions[region_index].range, + allocators[region_index].clone(), + ); + } + pool_set + }) + .collect() } -fn find_masquerade_portfw_overlap<'a>( - port_forwarding_exposes: &Vec<&'a ValidatedExpose>, - expose: &'a ValidatedExpose, -) -> ReserveSets { - let expose_nat = expose.nat().unwrap_or_else(|| unreachable!()); - let mut reserve_sets = ReserveSets::default(); +// One allocator per region. Every expose owning a region shares that allocator, which is what +// keeps a public address and port from being handed out twice. +fn build_region_allocators( + regions: &[Region], + protocol: NextHeader, + randomize: bool, +) -> Vec> { + // TCP and UDP masquerade allocators should avoid the IANA system/well-known range (0-1023). + // ICMP identifiers are allocated independently and are not subject to that policy. + let exclude_wellknown_ports = matches!(protocol, NextHeader::TCP | NextHeader::UDP); - for pf_expose in port_forwarding_exposes { - let pf_nat = pf_expose.nat().unwrap_or_else(|| unreachable!()); - let Some(relevant_proto) = expose_nat.proto.intersection(&pf_nat.proto) else { - // No overlap on L4 protocols, so no overlap for prefixes and ports. - continue; - }; - let ranges_intersection = pf_expose - .ips() - .intersection_prefixes_and_ports(expose.ips()); - match relevant_proto { - L4Protocol::Tcp => reserve_sets.tcp.extend(ranges_intersection), - L4Protocol::Udp => reserve_sets.udp.extend(ranges_intersection), - L4Protocol::Any => { - reserve_sets.tcp.extend(ranges_intersection.clone()); - reserve_sets.udp.extend(ranges_intersection); - } - } - } - reserve_sets + regions + .iter() + .map(|region| { + let pool = NatPool::for_range(region.range, exclude_wellknown_ports); + IpAllocator::new(pool, randomize) + }) + .collect() } fn pool_table_key_for_expose( prefix: &PrefixWithOptionalPorts, protocol: NextHeader, + src_vpc_id: VpcDiscriminant, dst_vpc_id: VpcDiscriminant, ) -> PoolTableKey { let (addr, addr_range_end) = prefix_bounds(prefix); - PoolTableKey::new(protocol, dst_vpc_id, addr, addr_range_end) + PoolTableKey::new(protocol, src_vpc_id, dst_vpc_id, addr, addr_range_end) } -#[allow(clippy::too_many_arguments)] fn add_pool_entries( table: &mut PoolTable, prefixes: &PrefixPortsSet, + src_vpc_id: VpcDiscriminant, dst_vpc_id: VpcDiscriminant, - tcp_allocator: &IpAllocator, - udp_allocator: &IpAllocator, - icmp_allocator: &IpAllocator, - icmp_proto: NextHeader, + protocol: NextHeader, + pool_set: &PoolSet, ) { for prefix in prefixes { - // We insert three times the entry, once for TCP, once for UDP and once for ICMP (v4 or v6 - // depending on the case). Allocations for TCP, for example, do not affect allocations for UDP - // or for ICMP, the space defined by the combination of IP addresses and L4 ports/id is distinct - // for each protocol. - - let tcp_key = pool_table_key_for_expose(prefix, NextHeader::TCP, dst_vpc_id); - let udp_key = pool_table_key_for_expose(prefix, NextHeader::UDP, dst_vpc_id); - let icmp_key = pool_table_key_for_expose(prefix, icmp_proto, dst_vpc_id); - - table.add_entry(tcp_key, tcp_allocator.clone()); - table.add_entry(udp_key, udp_allocator.clone()); - table.add_entry(icmp_key, icmp_allocator.clone()); - } -} - -fn ip_allocator_for_prefixes( - prefixes: &PrefixPortsSet, - idle_timeout: Duration, - prefixes_and_ports_to_exclude_from_pools: &PrefixPortsSet, - randomize: bool, - exclude_wellknown_ports: bool, -) -> IpAllocator { - let pool = create_natpool( - prefixes, - prefixes_and_ports_to_exclude_from_pools, - idle_timeout, - exclude_wellknown_ports, - ); - IpAllocator::new(pool, randomize) -} - -fn create_natpool( - prefixes: &PrefixPortsSet, - prefixes_and_ports_to_exclude_from_pools: &PrefixPortsSet, - idle_timeout: Duration, - exclude_wellknown_ports: bool, -) -> NatPool { - // Build mappings for IPv6 <-> u32 bitmap translation - let (bitmap_mapping, reverse_bitmap_mapping) = create_ipv6_bitmap_mappings( - &prefixes - .iter() - // FIXME: Add port range, too - .map(PrefixWithOptionalPorts::prefix) - .collect::>(), - ); - - // Mark all addresses as available (free) in bitmap - let mut bitmap = PoolBitmap::new(); - prefixes - .iter() - // FIXME: Add port range, too - .for_each(|prefix| bitmap.add_prefix(&prefix.prefix(), &reverse_bitmap_mapping)); - - let reserved_prefixes_ports = - build_reserved_prefixes_ports(prefixes_and_ports_to_exclude_from_pools); - - NatPool::new( - bitmap, - bitmap_mapping, - reverse_bitmap_mapping, - reserved_prefixes_ports, - idle_timeout, - exclude_wellknown_ports, - ) -} - -fn build_reserved_prefixes_ports( - prefixes_and_ports_to_exclude_from_pools: &PrefixPortsSet, -) -> Option> { - if prefixes_and_ports_to_exclude_from_pools.is_empty() { - return None; - } - let mut reserved_prefixes_ports = DisjointRangesBTreeMap::new(); - for prefix in prefixes_and_ports_to_exclude_from_pools { - debug_assert!(prefix.ports().is_some()); - let Some(ports) = prefix.ports() else { - error!("Stepped on a port-forwarding prefix without ports. This is a bug"); - continue; - }; - reserved_prefixes_ports.insert(prefix.prefix().into(), ports); + let key = pool_table_key_for_expose(prefix, protocol, src_vpc_id, dst_vpc_id); + table.add_entry(key, pool_set.clone()); } - Some(reserved_prefixes_ports) } fn prefix_bounds(prefix: &PrefixWithOptionalPorts) -> (I, I) { @@ -258,191 +251,3 @@ fn prefix_bounds(prefix: &PrefixWithOptionalPorts) -> (I, I) { // FIXME: Account for port ranges (addr, addr_range_end) } - -// The allocator's bitmap contains u32 only. For IPv4, it maps well to the address space. For IPv6, -// we need some mapping to associate IPv6 addresses with u32 indices. This also means that we cannot -// use more than 2^32 addresses for one expose, for NAT. If the prefixes we get contain more, we'll -// just ignore the remaining addresses. Hardware limitations are such that working with 4 billion -// allocated addresses is unreallistic anyway. -#[allow(clippy::type_complexity)] -fn create_ipv6_bitmap_mappings( - prefixes: &BTreeSet, -) -> (BTreeMap, BTreeMap) { - let mut bitmap_mapping = BTreeMap::new(); - let mut reverse_bitmap_mapping = BTreeMap::new(); - let mut index = 0; - - for prefix in prefixes { - if let Prefix::IPV6(p) = prefix { - let start_address = p.network().to_bits(); - bitmap_mapping.insert(index, start_address); - reverse_bitmap_mapping.insert(start_address, index); - if p.size() + u128::from(index) >= 2_u128.pow(32) { - break; - } - let Ok(psize) = u128::try_from(p.size()) else { - error!("Failed to get u128 from prefix {:#?}", p.size()); - continue; - }; - let Ok(psize_u32) = u32::try_from(psize) else { - error!("Failed to convert {psize} to u32"); - continue; - }; - index += psize_u32; - } - } - (bitmap_mapping, reverse_bitmap_mapping) -} - -#[cfg(test)] -mod tests { - use super::{ReserveSets, find_masquerade_portfw_overlap}; - use config::external::overlay::vpcpeering::VpcExpose; - use lpm::prefix::{L4Protocol, PortRange, PrefixPortsSet, PrefixWithOptionalPorts}; - - fn prefix_with_ports(s: &str, start: u16, end: u16) -> PrefixWithOptionalPorts { - PrefixWithOptionalPorts::new(s.into(), Some(PortRange::new(start, end).unwrap())) - } - - // tests for find_masquerade_portfw_overlap() - - #[test] - fn find_masquerade_portfw_overlap_multiple_pf_exposes() { - let expose = VpcExpose::empty() - .make_masquerade(None) - .unwrap() - .ip("10.0.0.0/16".into()) - .ip("172.16.0.0/16".into()) - .as_range("192.168.0.0/16".into()) - .unwrap() - .validate() - .unwrap(); - let pf_expose1 = VpcExpose::empty() - .make_port_forwarding(None, None) - .unwrap() - .ip(prefix_with_ports("10.0.1.0/24", 8080, 8090)) - .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() - .unwrap(); - let pf_expose2 = VpcExpose::empty() - .make_port_forwarding(None, None) - .unwrap() - .ip(prefix_with_ports("172.16.5.0/24", 8080, 8090)) - .as_range(prefix_with_ports("192.168.2.0/24", 8080, 8090)) - .unwrap() - .validate() - .unwrap(); - let pf_exposes_vec = vec![&pf_expose1, &pf_expose2]; - let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); - assert_eq!( - result, - ReserveSets { - tcp: PrefixPortsSet::from([ - prefix_with_ports("10.0.1.0/24", 8080, 8090), - prefix_with_ports("172.16.5.0/24", 8080, 8090), - ]), - udp: PrefixPortsSet::from([ - prefix_with_ports("10.0.1.0/24", 8080, 8090), - prefix_with_ports("172.16.5.0/24", 8080, 8090), - ]), - } - ); - } - - #[test] - fn find_masquerade_portfw_overlap_with_ports() { - let expose = VpcExpose::empty() - .make_masquerade(None) - .unwrap() - .ip("10.0.0.0/24".into()) - .as_range("192.168.0.0/24".into()) - .unwrap() - .validate() - .unwrap(); - let pf_expose = VpcExpose::empty() - .make_port_forwarding(None, None) - .unwrap() - .ip(prefix_with_ports("10.0.0.0/24", 8080, 8090)) - .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() - .unwrap(); - let pf_exposes_vec = vec![&pf_expose]; - let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); - assert_eq!( - result, - ReserveSets { - tcp: PrefixPortsSet::from([prefix_with_ports("10.0.0.0/24", 8080, 8090)]), - udp: PrefixPortsSet::from([prefix_with_ports("10.0.0.0/24", 8080, 8090)]), - } - ); - } - - #[test] - fn find_masquerade_portfw_overlap_with_ports_tcp() { - let expose = VpcExpose::empty() - .make_masquerade(None) - .unwrap() - .ip("10.0.0.0/24".into()) - .as_range("192.168.0.0/24".into()) - .unwrap() - .validate() - .unwrap(); - let pf_expose = VpcExpose::empty() - .make_port_forwarding(None, Some(L4Protocol::Tcp)) // TCP only - .unwrap() - .ip(prefix_with_ports("10.0.0.0/24", 8080, 8090)) - .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() - .unwrap(); - let pf_exposes_vec = vec![&pf_expose]; - let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); - assert_eq!( - result, - ReserveSets { - tcp: PrefixPortsSet::from([prefix_with_ports("10.0.0.0/24", 8080, 8090)]), - udp: PrefixPortsSet::default() - } - ); - } - - #[test] - fn find_masquerade_portfw_overlap_duplicates_collapsed() { - // Two port-forwarding exposes with the same prefix should produce one entry - let expose = VpcExpose::empty() - .make_masquerade(None) - .unwrap() - .ip("10.0.0.0/16".into()) - .as_range("192.168.0.0/24".into()) - .unwrap() - .validate() - .unwrap(); - let pf_expose1 = VpcExpose::empty() - .make_port_forwarding(None, None) - .unwrap() - .ip(prefix_with_ports("10.0.1.0/24", 8080, 8090)) - .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() - .unwrap(); - let pf_expose2 = VpcExpose::empty() - .make_port_forwarding(None, None) - .unwrap() - .ip(prefix_with_ports("10.0.1.0/24", 8080, 8090)) - .as_range(prefix_with_ports("192.168.1.0/24", 8080, 8090)) - .unwrap() - .validate() - .unwrap(); - let pf_exposes_vec = vec![&pf_expose1, &pf_expose2]; - let result = find_masquerade_portfw_overlap(&pf_exposes_vec, &expose); - assert_eq!( - result, - ReserveSets { - tcp: PrefixPortsSet::from([prefix_with_ports("10.0.1.0/24", 8080, 8090)]), - udp: PrefixPortsSet::from([prefix_with_ports("10.0.1.0/24", 8080, 8090)]), - } - ); - } -} diff --git a/nat/src/masquerade/apalloc/test_alloc.rs b/nat/src/masquerade/apalloc/test_alloc.rs index a8116404bf..100a9bfc44 100644 --- a/nat/src/masquerade/apalloc/test_alloc.rs +++ b/nat/src/masquerade/apalloc/test_alloc.rs @@ -9,7 +9,7 @@ use concurrency::concurrency_mode; // by tests in other modules. These helpers are not to be used outside of tests. mod context { use crate::masquerade::allocator_writer::MasqueradeConfig; - use crate::masquerade::apalloc::alloc::IpAllocator; + use crate::masquerade::apalloc::alloc::{IpAllocator, PoolSet}; use crate::masquerade::apalloc::{NatAllocator, PoolTable, PoolTableKey}; use config::external::overlay::vpc::{Peering, ValidatedVpcTable, Vpc, VpcTable}; use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest}; @@ -18,7 +18,7 @@ mod context { use net::udp::UdpPort; use net::vxlan::Vni; use net::{IpProtoKey, UdpProtoKey}; - use std::net::{IpAddr, Ipv4Addr}; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::str::FromStr; #[allow(dead_code)] @@ -41,12 +41,19 @@ mod context { Vni::new_checked(200).unwrap() } #[allow(dead_code)] + pub fn vni3() -> Vni { + Vni::new_checked(300).unwrap() + } pub fn vpcd1() -> VpcDiscriminant { VpcDiscriminant::from_vni(vni1()) } pub fn vpcd2() -> VpcDiscriminant { VpcDiscriminant::from_vni(vni2()) } + #[allow(dead_code)] + pub fn vpcd3() -> VpcDiscriminant { + VpcDiscriminant::from_vni(vni3()) + } #[allow(unused)] pub fn udp_proto_key(src_port: u16, dst_port: u16) -> IpProtoKey { @@ -58,17 +65,33 @@ mod context { pub fn get_ip_allocator_v4( pool: &mut PoolTable, + src_vpcd: VpcDiscriminant, dst_vpcd: VpcDiscriminant, protocol: NextHeader, src_ip: Ipv4Addr, ) -> &IpAllocator { - pool.get(&PoolTableKey::new( - protocol, - dst_vpcd, - src_ip, - Ipv4Addr::from_str("255.255.255.255").unwrap(), - )) - .unwrap() + let pool_set = pool + .get(&PoolTableKey::new( + protocol, + src_vpcd, + dst_vpcd, + src_ip, + Ipv4Addr::from_str("255.255.255.255").unwrap(), + )) + .unwrap(); + sole_region(pool_set) + } + + // The public ranges in most of these fixtures do not overlap, so each expose owns exactly one + // region and the tests can look straight at its allocator. + pub fn sole_region(pool_set: &PoolSet) -> &IpAllocator { + let mut regions = pool_set.regions(); + let region = regions.next().expect("expose has no region"); + assert!( + regions.next().is_none(), + "expose was cut into more than one region" + ); + region.allocator() } fn build_context() -> ValidatedVpcTable { @@ -133,13 +156,254 @@ mod context { let config = MasqueradeConfig::new(&vpc_table); NatAllocator::new(config, 1) } + + // Two VPCs masquerade onto one public range toward the same peer. + #[allow(dead_code)] + fn build_context_shared_public_range() -> ValidatedVpcTable { + let masquerade_manifest = |name: &str, private: &str| { + VpcManifest::with_exposes( + name, + vec![ + VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip(private.into()) + .as_range("10.1.0.0/30".into()) + .unwrap(), + ], + ) + }; + let remote = + VpcManifest::with_exposes("VPC-3", vec![VpcExpose::empty().ip("3.0.0.0/24".into())]); + + let mut vpc1 = Vpc::new("VPC-1", "67890", vni1().as_u32()).unwrap(); + let mut vpc2 = Vpc::new("VPC-2", "12345", vni2().as_u32()).unwrap(); + let vpc3 = Vpc::new("VPC-3", "11111", vni3().as_u32()).unwrap(); + + vpc1.peerings.push(Peering { + name: "shared_peering1".into(), + local: masquerade_manifest("VPC-1", "1.1.0.0/16"), + remote: remote.clone(), + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc3.vni, + gwgroup: "default".into(), + acl: None, + }); + vpc2.peerings.push(Peering { + name: "shared_peering2".into(), + local: masquerade_manifest("VPC-2", "2.1.0.0/16"), + remote, + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc3.vni, + gwgroup: "default".into(), + acl: None, + }); + + let mut vpctable = VpcTable::new(); + vpctable.add(vpc1).unwrap(); + vpctable.add(vpc2).unwrap(); + vpctable.add(vpc3).unwrap(); + + vpctable.validate().unwrap() + } + + #[allow(dead_code)] + pub fn build_allocator_shared_public_range() -> NatAllocator { + let vpc_table = build_context_shared_public_range(); + // Without randomization the first port block picked is deterministic, so two pools that + // wrongly believe they each own the whole public range collide on their first allocation. + let config = MasqueradeConfig::new(&vpc_table).set_randomize(false); + NatAllocator::new(config, 1) + } + + // Two VPCs using the *same* private prefix, each peering with the same destination VPC and + // masquerading onto a public range of its own. Tenants reusing private address space is + // ordinary, and is much of what NAT is for, so both are entitled to their own pool. + #[allow(dead_code)] + fn build_context_overlapping_private_prefixes() -> ValidatedVpcTable { + let masquerade_manifest = |name: &str, public: &str| { + VpcManifest::with_exposes( + name, + vec![ + VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("192.168.0.0/16".into()) + .as_range(public.into()) + .unwrap(), + ], + ) + }; + let remote = + VpcManifest::with_exposes("VPC-3", vec![VpcExpose::empty().ip("3.0.0.0/24".into())]); + + let mut vpc1 = Vpc::new("VPC-1", "67890", vni1().as_u32()).unwrap(); + let mut vpc2 = Vpc::new("VPC-2", "12345", vni2().as_u32()).unwrap(); + let vpc3 = Vpc::new("VPC-3", "11111", vni3().as_u32()).unwrap(); + + vpc1.peerings.push(Peering { + name: "overlapping_peering1".into(), + local: masquerade_manifest("VPC-1", "10.1.0.0/30"), + remote: remote.clone(), + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc3.vni, + gwgroup: "default".into(), + acl: None, + }); + vpc2.peerings.push(Peering { + name: "overlapping_peering2".into(), + local: masquerade_manifest("VPC-2", "10.2.0.0/30"), + remote, + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc3.vni, + gwgroup: "default".into(), + acl: None, + }); + + let mut vpctable = VpcTable::new(); + vpctable.add(vpc1).unwrap(); + vpctable.add(vpc2).unwrap(); + vpctable.add(vpc3).unwrap(); + + vpctable.validate().unwrap() + } + + #[allow(dead_code)] + pub fn build_allocator_overlapping_private_prefixes() -> NatAllocator { + let vpc_table = build_context_overlapping_private_prefixes(); + let config = MasqueradeConfig::new(&vpc_table).set_randomize(false); + NatAllocator::new(config, 1) + } + + // Two VPCs peering with the same destination VPC, masquerading onto public ranges that overlap + // *partially*: VPC-1 takes 10.1.0.0/30 (.0 through .3) and VPC-2 takes 10.1.0.2/31 (.2 and + // .3). Neither range contains the other, so no single range identifies the shared space. + #[allow(dead_code)] + fn build_context_partial_overlap() -> ValidatedVpcTable { + let masquerade_manifest = |name: &str, private: &str, public: &str| { + VpcManifest::with_exposes( + name, + vec![ + VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip(private.into()) + .as_range(public.into()) + .unwrap(), + ], + ) + }; + let remote = + VpcManifest::with_exposes("VPC-3", vec![VpcExpose::empty().ip("3.0.0.0/24".into())]); + + let mut vpc1 = Vpc::new("VPC-1", "67890", vni1().as_u32()).unwrap(); + let mut vpc2 = Vpc::new("VPC-2", "12345", vni2().as_u32()).unwrap(); + let vpc3 = Vpc::new("VPC-3", "11111", vni3().as_u32()).unwrap(); + + vpc1.peerings.push(Peering { + name: "partial_peering1".into(), + local: masquerade_manifest("VPC-1", "1.1.0.0/16", "10.1.0.0/30"), + remote: remote.clone(), + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc3.vni, + gwgroup: "default".into(), + acl: None, + }); + vpc2.peerings.push(Peering { + name: "partial_peering2".into(), + local: masquerade_manifest("VPC-2", "2.1.0.0/16", "10.1.0.2/31"), + remote, + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc3.vni, + gwgroup: "default".into(), + acl: None, + }); + + let mut vpctable = VpcTable::new(); + vpctable.add(vpc1).unwrap(); + vpctable.add(vpc2).unwrap(); + vpctable.add(vpc3).unwrap(); + + vpctable.validate().unwrap() + } + + #[allow(dead_code)] + pub fn build_allocator_partial_overlap() -> NatAllocator { + let vpc_table = build_context_partial_overlap(); + let config = MasqueradeConfig::new(&vpc_table).set_randomize(false); + NatAllocator::new(config, 1) + } + + #[allow(dead_code)] + fn build_context_v6() -> ValidatedVpcTable { + let masquerade = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("2001:db8:1::/64".into()) + .as_range("2001:db8:ffff::/112".into()) + .unwrap(); + let remote = VpcManifest::with_exposes( + "VPC-2", + vec![VpcExpose::empty().ip("2001:db8:2::/64".into())], + ); + + let mut vpc1 = Vpc::new("VPC-1", "67890", vni1().as_u32()).unwrap(); + let vpc2 = Vpc::new("VPC-2", "12345", vni2().as_u32()).unwrap(); + vpc1.peerings.push(Peering { + name: "v6_peering".into(), + local: VpcManifest::with_exposes("VPC-1", vec![masquerade]), + remote, + remote_id: "12345".try_into().unwrap(), + remote_vni: vpc2.vni, + gwgroup: "default".into(), + acl: None, + }); + + let mut vpctable = VpcTable::new(); + vpctable.add(vpc1).unwrap(); + vpctable.add(vpc2).unwrap(); + vpctable.validate().unwrap() + } + + #[allow(dead_code)] + pub fn build_allocator_v6() -> NatAllocator { + let config = MasqueradeConfig::new(&build_context_v6()).set_randomize(false); + NatAllocator::new(config, 1) + } + + #[allow(dead_code)] + pub fn addr_v6(ip: &str) -> Ipv6Addr { + Ipv6Addr::from_str(ip).unwrap() + } + + #[allow(dead_code)] + pub fn get_pool_set_v4( + pool: &PoolTable, + src_vpcd: VpcDiscriminant, + dst_vpcd: VpcDiscriminant, + protocol: NextHeader, + src_ip: Ipv4Addr, + ) -> &PoolSet { + pool.get(&PoolTableKey::new( + protocol, + src_vpcd, + dst_vpcd, + src_ip, + Ipv4Addr::from_str("255.255.255.255").unwrap(), + )) + .unwrap() + } } mod tests { use super::context::*; + use crate::NatPort; + use crate::masquerade::allocation::AllocatorError; use concurrency::sync::Arc; use concurrency::thread; use net::ip::NextHeader; + use std::net::IpAddr; + use std::num::NonZero; #[allow(dead_code)] pub(super) fn concurrent_allocations() { @@ -153,17 +417,17 @@ mod tests { handles.push(thread::spawn(move || { let _allocation1 = allocator1 - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) .unwrap(); })); handles.push(thread::spawn(move || { let _allocation2 = allocator2 - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) .unwrap(); })); handles.push(thread::spawn(move || { let _allocation3 = allocator3 - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) .unwrap(); })); @@ -177,6 +441,7 @@ mod tests { let mut allocator_again = Arc::try_unwrap(allocator_arc).unwrap(); let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator_again.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -185,13 +450,75 @@ mod tests { assert_eq!(bitmap.len(), 3); // 3 IP addresses available to NAT 1.1.0.0 assert!(in_use.front().unwrap().upgrade().is_none()); // Weak references in list no longer resolve } + + #[test] + fn port_forward_flows_share_and_release_a_public_tuple() { + let allocator = build_allocator(); + let public_ip = IpAddr::V4(addr_v4("10.1.0.0")); + let public_port = NonZero::new(1024).unwrap(); + + let first = allocator + .reserve_port_forward(NextHeader::TCP, vpcd2(), public_ip, public_port) + .unwrap() + .expect("the tuple overlaps a masquerade pool"); + let second = allocator + .reserve_port_forward(NextHeader::TCP, vpcd2(), public_ip, public_port) + .unwrap() + .expect("the tuple overlaps a masquerade pool"); + assert!(Arc::ptr_eq(&first, &second)); + + let port = NatPort::new_port(public_port); + match allocator.reserve_port( + NextHeader::TCP, + vpcd1(), + vpcd2(), + ipaddr("1.1.0.1"), + public_ip, + port, + ) { + Err(AllocatorError::PortReservationFailed(blocked)) => { + assert_eq!(blocked, public_port.get()); + } + other => panic!("a live port-forward lease must block the reservation, got {other:?}"), + } + + drop((first, second)); + assert!( + allocator + .reserve_port( + NextHeader::TCP, + vpcd1(), + vpcd2(), + ipaddr("1.1.0.1"), + public_ip, + port, + ) + .is_ok() + ); + } + + #[test] + fn well_known_port_forwards_need_no_lease() { + let allocator = build_allocator(); + let lease = allocator + .reserve_port_forward( + NextHeader::TCP, + vpcd2(), + ipaddr("10.1.0.0"), + NonZero::new(80).unwrap(), + ) + .unwrap(); + assert!(lease.is_none()); + } } #[concurrency_mode(std)] mod std_tests { use super::context::*; use crate::masquerade::apalloc::PoolTableKey; + use crate::masquerade::apalloc::alloc::PoolRegion; use net::ip::NextHeader; + use std::net::IpAddr; #[test] fn test_build_allocator() { @@ -237,16 +564,17 @@ mod std_tests { assert_eq!(allocator.pools_src66.0.len(), 0); - let ip_allocator = allocator + let pool_set = allocator .pools_src44 .get(&PoolTableKey::new( NextHeader::TCP, + vpcd1(), vpcd2(), addr_v4("1.1.0.0"), addr_v4("255.255.255.255"), )) .unwrap(); - let (bitmap, in_use) = ip_allocator.get_pool_clone_for_tests(); + let (bitmap, in_use) = sole_region(pool_set).get_pool_clone_for_tests(); assert!(bitmap.contains_range(addr_v4_bits("10.1.0.0")..=addr_v4_bits("10.1.0.2"))); assert_eq!(bitmap.len(), 3); @@ -261,6 +589,7 @@ mod std_tests { let mut allocator = build_allocator(); let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -270,7 +599,7 @@ mod std_tests { assert_eq!(in_use.len(), 0); // None allocated yet let alloc_result = allocator - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) .unwrap(); println!("{alloc_result}"); @@ -278,6 +607,7 @@ mod std_tests { let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -291,6 +621,7 @@ mod std_tests { let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -307,6 +638,7 @@ mod std_tests { let mut allocator = build_allocator(); let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -317,6 +649,7 @@ mod std_tests { let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::UDP, addr_v4("1.1.0.0"), @@ -327,13 +660,14 @@ mod std_tests { // Allocate for TCP let tcp_allocation = allocator - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) .unwrap(); println!("{tcp_allocation}"); // Check number of allocated IPs for TCP after we have allocated for TCP let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -345,6 +679,7 @@ mod std_tests { // Check number of allocated IPs for UDP after we have allocated for TCP let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::UDP, addr_v4("1.1.0.0"), @@ -355,13 +690,14 @@ mod std_tests { // Allocate for UDP let udp_allocation = allocator - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::UDP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::UDP) .unwrap(); println!("{udp_allocation}"); // Check number of allocated IPs for TCP after we have allocated for UDP let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::TCP, addr_v4("1.1.0.0"), @@ -373,6 +709,7 @@ mod std_tests { // Check number of allocated IPs for UDP after we have allocated for UDP let (bitmap, in_use) = get_ip_allocator_v4( &mut allocator.pools_src44, + vpcd1(), vpcd2(), NextHeader::UDP, addr_v4("1.1.0.0"), @@ -381,6 +718,296 @@ mod std_tests { assert_eq!(bitmap.len(), 2); // 2 free IP addresses left to NAT 1.1.0.0 (UDP) assert_eq!(in_use.len(), 1); // 1 allocated, in use } + + // Two VPCs masquerading onto the same public range towards the same destination VPC describe + // one pool, not two, and must therefore share a single allocator. Allocating for a source in + // each VPC in turn may not yield the same public address and port twice. + #[test] + fn test_shared_public_range_is_allocated_once() { + let allocator = build_allocator_shared_public_range(); + + let alloc_a = allocator + .allocate_v4(vpcd1(), vpcd3(), addr_v4("1.1.0.1"), NextHeader::TCP) + .unwrap(); + let alloc_b = allocator + .allocate_v4(vpcd2(), vpcd3(), addr_v4("2.1.0.1"), NextHeader::TCP) + .unwrap(); + + assert_ne!( + (alloc_a.allocation.ip(), alloc_a.allocation.port().as_u16()), + (alloc_b.allocation.ip(), alloc_b.allocation.port().as_u16()), + "the same public address and port was handed out to two different flows" + ); + + // Sharing the pool means the second allocation reuses the address the first one took, + // and simply draws the next port from it. + assert_eq!(alloc_a.allocation.ip(), alloc_b.allocation.ip()); + } + + // Sharing a pool must not collapse the private-side lookups: each private prefix keeps its own + // entry, since that is how the first packet of a flow finds its pool. + #[test] + fn test_shared_public_range_keeps_both_private_entries() { + let allocator = build_allocator_shared_public_range(); + + let tcp_entries = allocator + .pools_src44 + .0 + .keys() + .filter(|k| k.protocol == NextHeader::TCP && k.dst_vpcd == vpcd3()) + .count(); + assert_eq!(tcp_entries, 2); + + for (src_vpcd, src) in [(vpcd1(), "1.1.0.1"), (vpcd2(), "2.1.0.1")] { + assert!( + allocator + .pools_src44 + .get(&PoolTableKey::new( + NextHeader::TCP, + src_vpcd, + vpcd3(), + addr_v4(src), + addr_v4("255.255.255.255"), + )) + .is_some(), + "no pool found for private source {src}" + ); + } + } + + // Two VPCs may use the same private address space, so a private prefix on its own does not + // identify a pool. Each VPC has to be masqueraded onto the public range its own expose + // declares, rather than whichever expose happened to be registered last. + #[test] + fn test_overlapping_private_prefixes_use_their_own_pool() { + let allocator = build_allocator_overlapping_private_prefixes(); + + let from_vpc1 = allocator + .allocate_v4(vpcd1(), vpcd3(), addr_v4("192.168.0.1"), NextHeader::TCP) + .unwrap(); + let from_vpc2 = allocator + .allocate_v4(vpcd2(), vpcd3(), addr_v4("192.168.0.1"), NextHeader::TCP) + .unwrap(); + + assert_eq!( + from_vpc1.allocation.ip(), + addr_v4("10.1.0.0"), + "traffic from VPC-1 was not masqueraded onto the range VPC-1 exposes" + ); + assert_eq!( + from_vpc2.allocation.ip(), + addr_v4("10.2.0.0"), + "traffic from VPC-2 was not masqueraded onto the range VPC-2 exposes" + ); + } + + // Partially overlapping public ranges are cut so that the shared part becomes a region of its + // own. VPC-1 keeps an exclusive region for the half only it claims, plus the shared one; + // VPC-2 only ever sees the shared one. + #[test] + fn test_partial_overlap_is_cut_into_regions() { + let allocator = build_allocator_partial_overlap(); + + let for_vpc1 = get_pool_set_v4( + &allocator.pools_src44, + vpcd1(), + vpcd3(), + NextHeader::TCP, + addr_v4("1.1.0.1"), + ); + let vpc1_ranges: Vec<_> = for_vpc1.regions().map(PoolRegion::range).collect(); + assert_eq!(vpc1_ranges.len(), 2, "VPC-1 should own two regions"); + // The exclusive region is offered first. + assert_eq!(vpc1_ranges[0].start, u128::from(addr_v4_bits("10.1.0.0"))); + assert_eq!(vpc1_ranges[0].end, u128::from(addr_v4_bits("10.1.0.1"))); + assert_eq!(vpc1_ranges[1].start, u128::from(addr_v4_bits("10.1.0.2"))); + assert_eq!(vpc1_ranges[1].end, u128::from(addr_v4_bits("10.1.0.3"))); + + let for_vpc2 = get_pool_set_v4( + &allocator.pools_src44, + vpcd2(), + vpcd3(), + NextHeader::TCP, + addr_v4("2.1.0.1"), + ); + let vpc2_ranges: Vec<_> = for_vpc2.regions().map(PoolRegion::range).collect(); + assert_eq!( + vpc2_ranges.len(), + 1, + "VPC-2 should only own the shared region" + ); + assert_eq!(vpc2_ranges[0].start, u128::from(addr_v4_bits("10.1.0.2"))); + assert_eq!(vpc2_ranges[0].end, u128::from(addr_v4_bits("10.1.0.3"))); + } + + // The shared region is one allocator, not a copy per VPC: an address VPC-2 takes from it is no + // longer free in the view VPC-1 has of that same region. This is what stops the two from + // handing out the same public address and port. + #[test] + fn test_partial_overlap_shares_one_allocator_for_the_shared_region() { + let allocator = build_allocator_partial_overlap(); + + let taken = allocator + .allocate_v4(vpcd2(), vpcd3(), addr_v4("2.1.0.1"), NextHeader::TCP) + .unwrap(); + // VPC-2 can only allocate from the shared region. + assert_eq!(taken.allocation.ip(), addr_v4("10.1.0.2")); + + let for_vpc1 = get_pool_set_v4( + &allocator.pools_src44, + vpcd1(), + vpcd3(), + NextHeader::TCP, + addr_v4("1.1.0.1"), + ); + let shared = for_vpc1.regions().nth(1).expect("no shared region"); + let (bitmap, in_use) = shared.allocator().get_pool_clone_for_tests(); + assert!( + !bitmap.contains(addr_v4_bits("10.1.0.2")), + "VPC-1 still sees an address VPC-2 has taken from the shared region" + ); + assert_eq!(in_use.len(), 1); + } + + // Allocation prefers space a VPC has to itself, so the shared region stays available for the + // VPC that has nowhere else to go. + #[test] + fn test_partial_overlap_prefers_exclusive_space() { + let allocator = build_allocator_partial_overlap(); + + let from_vpc1 = allocator + .allocate_v4(vpcd1(), vpcd3(), addr_v4("1.1.0.1"), NextHeader::TCP) + .unwrap(); + assert_eq!( + from_vpc1.allocation.ip(), + addr_v4("10.1.0.0"), + "VPC-1 should have drawn from the region it does not share" + ); + + let from_vpc2 = allocator + .allocate_v4(vpcd2(), vpcd3(), addr_v4("2.1.0.1"), NextHeader::TCP) + .unwrap(); + assert_ne!( + ( + from_vpc1.allocation.ip(), + from_vpc1.allocation.port().as_u16() + ), + ( + from_vpc2.allocation.ip(), + from_vpc2.allocation.port().as_u16() + ), + ); + } + + // Whichever region it draws from, a VPC may only ever be given an address its own expose + // declares. + #[test] + fn test_partial_overlap_never_allocates_outside_the_configured_range() { + let allocator = build_allocator_partial_overlap(); + + let mut held = Vec::new(); + for _ in 0..16 { + let allocation = allocator + .allocate_v4(vpcd2(), vpcd3(), addr_v4("2.1.0.1"), NextHeader::TCP) + .unwrap(); + let ip = allocation.allocation.ip(); + assert!( + ip == addr_v4("10.1.0.2") || ip == addr_v4("10.1.0.3"), + "VPC-2 was given {ip}, which is outside the range it exposes" + ); + held.push(allocation); + } + } + + #[test] + fn test_masquerade_v6_allocates_within_the_public_range() { + let allocator = build_allocator_v6(); + assert!(allocator.pools_src44.0.is_empty()); + assert!(!allocator.pools_src66.0.is_empty()); + + let mut held = Vec::new(); + let mut seen = std::collections::BTreeSet::new(); + for step in 0..8 { + let allocation = allocator + .allocate( + vpcd1(), + vpcd2(), + IpAddr::V6(addr_v6("2001:db8:1::1")), + NextHeader::TCP, + ) + .expect("the v6 pool has room"); + let IpAddr::V6(ip) = allocation.allocation.ip() else { + panic!("an IPv6 source received an IPv4 translation"); + }; + let port = allocation.allocation.port().as_u16(); + + assert_eq!( + ip.segments()[0..7], + addr_v6("2001:db8:ffff::").segments()[0..7] + ); + if step == 0 { + assert_eq!(ip, addr_v6("2001:db8:ffff::")); + } + assert!(port >= 1024); + assert!(seen.insert((ip, port)), "{ip}:{port} was handed out twice"); + held.push(allocation); + } + } + + #[test] + fn test_masquerade_v6_reserves_a_carried_address() { + let allocator = build_allocator_v6(); + let allocation = allocator + .allocate( + vpcd1(), + vpcd2(), + IpAddr::V6(addr_v6("2001:db8:1::1")), + NextHeader::TCP, + ) + .expect("the v6 pool has room"); + let held = allocation.allocation.ip(); + let port = allocation.allocation.port(); + + let next = build_allocator_v6(); + let carried = next + .reserve_port( + NextHeader::TCP, + vpcd1(), + vpcd2(), + IpAddr::V6(addr_v6("2001:db8:1::1")), + held, + port, + ) + .expect("the next config still serves the tuple"); + assert_eq!((carried.ip(), carried.port()), (held, port)); + + let fresh = next + .allocate( + vpcd1(), + vpcd2(), + IpAddr::V6(addr_v6("2001:db8:1::2")), + NextHeader::TCP, + ) + .expect("the v6 pool has room"); + assert_ne!( + (fresh.allocation.ip(), fresh.allocation.port()), + (held, port) + ); + } + + // Both VPCs keep their own entry, rather than the second overwriting the first. + #[test] + fn test_overlapping_private_prefixes_keep_separate_entries() { + let allocator = build_allocator_overlapping_private_prefixes(); + + let tcp_entries = allocator + .pools_src44 + .0 + .keys() + .filter(|k| k.protocol == NextHeader::TCP && k.dst_vpcd == vpcd3()) + .count(); + assert_eq!(tcp_entries, 2); + } } // Loom's Weak shim keeps allocator liveness entries alive forever. @@ -400,12 +1027,12 @@ mod concurrency_tests { let t1 = thread::spawn(move || { let _allocation1 = allocator1 - .allocate_v4(vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.1.0.0"), NextHeader::TCP) .unwrap(); }); let t2 = thread::spawn(move || { let _allocation2 = allocator2 - .allocate_v4(vpcd2(), addr_v4("1.2.0.0"), NextHeader::TCP) + .allocate_v4(vpcd1(), vpcd2(), addr_v4("1.2.0.0"), NextHeader::TCP) .unwrap(); }); t1.join().unwrap(); diff --git a/nat/src/masquerade/flows.rs b/nat/src/masquerade/flows.rs index 6fffeeb6f7..69a7408e9b 100644 --- a/nat/src/masquerade/flows.rs +++ b/nat/src/masquerade/flows.rs @@ -5,6 +5,7 @@ use crate::NatPort; use crate::common::NatAction; use crate::masquerade::apalloc::NatAllocator; use crate::masquerade::state::MasqueradeState; +use crate::portfw::update_port_forward_lease; use config::GenId; use flow_entry::flow_table::{FlowTable, FlowTableReadGuard}; @@ -15,14 +16,13 @@ use net::flows::FlowInfo; use std::net::IpAddr; use tracing::{debug, error}; -/// Invalidate all of the flows that have masquerading state -pub(crate) fn invalidate_all_masquerading_flows(flow_table: &FlowTable) { - debug!("INVALIDATING all masquerading flows..."); +/// Detach flows from an allocator before it is removed. +pub(crate) fn remove_allocator_from_flows(flow_table: &FlowTable) { flow_table.for_each_flow(|_key, flow_info| { - let locked = flow_info.locked.read(); - if locked.nat_state.as_ref().is_some() { + if flow_info.locked.read().nat_state.as_ref().is_some() { flow_info.invalidate_pair(); } + let _ = update_port_forward_lease(flow_info, None); }); } @@ -60,19 +60,23 @@ fn re_reserve_ip_and_port( ) -> Result<(), ()> { let flow_key = flow_info.flowkey(); let proto = flow_key.proto(); - let dst_vpcd = flow_info.get_dst_vpcd().unwrap_or_else(|| unreachable!()); + // A flow without both VPC identities cannot be carried into the replacement allocator. + let (Some(src_vpcd), Some(dst_vpcd)) = (flow_key.src_vpcd(), flow_info.get_dst_vpcd()) else { + error!( + "Flow {flow_key} has no VPC discriminant, so it cannot be carried over. This is a bug" + ); + return Err(()); + }; let src_ip = *flow_key.src_ip(); let port_u16 = port.as_u16(); debug!("Attempting to re-reserve {ip} {proto}:{port_u16} for flow {flow_key}"); - match new_allocator.reserve_port(proto, dst_vpcd, src_ip, ip, port) { + match new_allocator.reserve_port(proto, src_vpcd, dst_vpcd, src_ip, ip, port) { Ok(alloc) => { debug!("Successfully re-reserved ip {ip} port/Id {port_u16} ({proto})"); let mut guard = flow_info.locked.write(); let nat_state = guard.nat_state.as_mut().ok_or(())?; - let nat_state = nat_state - .extract_mut::() - .unwrap_or_else(|| unreachable!()); + let nat_state = nat_state.extract_mut::().ok_or(())?; debug_assert!(matches!(nat_state.action(), NatAction::SrcNat)); nat_state.set_allocation(alloc); debug!("Successfully associated ip {ip}, {proto}:{port_u16} to flow {flow_key}"); @@ -101,8 +105,12 @@ pub(crate) fn check_masquerading_flow( let Some((ip, port)) = get_flow_masquerading_allocation(flow_info) else { return; }; - let dst_vpcd = flow_info.get_dst_vpcd().unwrap_or_else(|| unreachable!()); - let src_vpcd = flow_key.src_vpcd().unwrap_or_else(|| unreachable!()); + // Flows without VPC identity cannot be validated against the replacement config. + let (Some(dst_vpcd), Some(src_vpcd)) = (flow_info.get_dst_vpcd(), flow_key.src_vpcd()) else { + error!("Flow {flow_key} has no VPC discriminant, so it cannot be checked. This is a bug"); + flow_info.invalidate_pair(); + return; + }; debug!("Checking flow {}", flow_info.logfmt()); let Some(nat_peering) = config.get_peering(src_vpcd, dst_vpcd) else { @@ -149,13 +157,7 @@ pub(crate) fn check_masquerading_flow( return; } - // Flow uses an ip address that is compatible with a masquerading expose in the current configuration and the source of - // the masqueraded flow is still allowed over the peering. - // So, we should continue serving the flow. To do so, we: - // 1) allocate the address and port in the new allocator to prevent it from using it for other flows and - // 2) link the flow to the new object representing the ip/port so that it gets released when the flow is terminated. - // - // If either of those fails, we invalidate the flow. On success, we upgrade the flow to the new gen id. + // Reserve the tuple in the replacement before advancing the flow generation. if re_reserve_ip_and_port(allocator, flow_info, ip, port).is_ok() { debug!("Upgrading flow {} to gen id {genid}...", flow_info.logfmt()); flow_info.set_genid_pair(genid); @@ -164,19 +166,26 @@ pub(crate) fn check_masquerading_flow( } } -/// Main function called to deal with flows when masquerade configuration changes. This function: -/// - locks the flow table -/// - examines all masquerade flows to determine if they should continue or be invalidated according to the new allocator config -/// - flows that continue get a new allocation with the same ip and port in the `new_allocator` -pub(crate) fn check_masquerading_flows<'a>( +/// Move live NAT flows to a replacement allocator while blocking flow insertion. +pub(crate) fn reconcile_nat_flows<'a>( flow_table: &'a FlowTable, - new_allocator: &mut NatAllocator, + new_allocator: &NatAllocator, ) -> FlowTableReadGuard<'a> { let genid = new_allocator.genid(); debug!("CHECKING flows against new masquerade configuration with genid {genid}..."); let guard = flow_table.for_each_flow_filtered( |_, f| f.is_active(), - |flow_key, flow_info| check_masquerading_flow(flow_key, flow_info, new_allocator), + |flow_key, flow_info| { + check_masquerading_flow(flow_key, flow_info, new_allocator); + // The check may invalidate a flow selected while it was still active. + if !flow_info.is_active() { + return; + } + if let Err(error) = update_port_forward_lease(flow_info, Some(new_allocator)) { + error!("Failed to reserve a live port-forward tuple: {error}"); + flow_info.invalidate_pair(); + } + }, ); debug!("CHECKING flows against new masquerade configuration COMPLETED"); guard diff --git a/nat/src/masquerade/icmp_handling.rs b/nat/src/masquerade/icmp_handling.rs index fcb6bed7ea..1f4af97246 100644 --- a/nat/src/masquerade/icmp_handling.rs +++ b/nat/src/masquerade/icmp_handling.rs @@ -17,15 +17,19 @@ pub(crate) fn handle_icmp_error_masquerading( packet: &mut Packet, flow_info: &FlowInfo, ) -> Result { - let src_vpcd = packet.meta().src_vpcd.unwrap_or_else(|| unreachable!()); let f = flow_info.logfmt(); - debug!("(masquerade): Processing ICMP error message from {src_vpcd} with flow {f}"); + if let Some(src_vpcd) = packet.meta().src_vpcd { + debug!("(masquerade): Processing ICMP error message from {src_vpcd} with flow {f}"); + } else { + // The missing source only affects this log line. + debug!("(masquerade): Processing ICMP error message with flow {f}"); + } let flow_info_locked = flow_info.locked.read(); - let state = flow_info_locked - .nat_state - .extract_ref::() - .unwrap_or_else(|| unreachable!()); + let Some(state) = flow_info_locked.nat_state.extract_ref::() else { + debug!("(masquerade): ICMP error hit a flow carrying no masquerade state"); + return Err(DoneReason::InternalFailure); + }; // translate inner packet fragment with the common API object `NatTranslationData` let nat_translation = state.reverse_translation_data(); diff --git a/nat/src/masquerade/mod.rs b/nat/src/masquerade/mod.rs index d7b2861dba..968e942e5a 100644 --- a/nat/src/masquerade/mod.rs +++ b/nat/src/masquerade/mod.rs @@ -15,6 +15,7 @@ mod test; // re exports pub use allocator_writer::MasqueradeConfig; +pub(crate) use allocator_writer::NatAllocatorReader; pub use allocator_writer::NatAllocatorWriter; pub use nf::Masquerade; diff --git a/nat/src/masquerade/natip.rs b/nat/src/masquerade/natip.rs index ccef2c21ae..a9879b00fb 100644 --- a/nat/src/masquerade/natip.rs +++ b/nat/src/masquerade/natip.rs @@ -4,7 +4,6 @@ //! NAT IP address trait: a sealed trait to represent either IPv4 or IPv6 in IP-version-generic //! code. -use net::headers::Net; use std::fmt::{Debug, Display}; use std::hash::Hash; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; @@ -22,15 +21,13 @@ pub trait NatIp: // Convert to `IpAddr` object fn to_ip_addr(&self) -> IpAddr; - // Extract the source IP address from a Net object as a `NatIp` - fn from_src_addr(net: &Net) -> Option; - - // Extract the destination IP address from a Net object as a `NatIp` - fn from_dst_addr(net: &Net) -> Option; - // Convert from a 128-bit integer to a `NatIp`, if possible fn try_from_bits(bits: u128) -> Result; + // Convert to a 128-bit integer. Named to avoid colliding with the inherent `to_bits` of + // `Ipv4Addr`, which yields a `u32` and would silently win method resolution. + fn to_addr_bits(&self) -> u128; + // Convert from an `IpAddr` object to a `NatIp`, if possible fn try_from_addr(addr: IpAddr) -> Result; } @@ -42,23 +39,12 @@ impl NatIp for Ipv4Addr { fn to_ip_addr(&self) -> IpAddr { IpAddr::V4(*self) } - fn from_src_addr(net: &Net) -> Option { - if let IpAddr::V4(addr) = net.src_addr() { - Some(addr) - } else { - None - } - } - fn from_dst_addr(net: &Net) -> Option { - if let IpAddr::V4(addr) = net.dst_addr() { - Some(addr) - } else { - None - } - } fn try_from_bits(bits: u128) -> Result { Ok(Self::from(u32::try_from(bits).map_err(|_| ())?)) } + fn to_addr_bits(&self) -> u128 { + u128::from(self.to_bits()) + } fn try_from_addr(addr: IpAddr) -> Result { if let IpAddr::V4(addr) = addr { Ok(addr) @@ -72,23 +58,12 @@ impl NatIp for Ipv6Addr { fn to_ip_addr(&self) -> IpAddr { IpAddr::V6(*self) } - fn from_src_addr(net: &Net) -> Option { - if let IpAddr::V6(addr) = net.src_addr() { - Some(addr) - } else { - None - } - } - fn from_dst_addr(net: &Net) -> Option { - if let IpAddr::V6(addr) = net.dst_addr() { - Some(addr) - } else { - None - } - } fn try_from_bits(bits: u128) -> Result { Ok(Self::from(bits)) } + fn to_addr_bits(&self) -> u128 { + self.to_bits() + } fn try_from_addr(addr: IpAddr) -> Result { if let IpAddr::V6(addr) = addr { Ok(addr) diff --git a/nat/src/masquerade/nf.rs b/nat/src/masquerade/nf.rs index f34b1d9a6a..c03274f0f5 100644 --- a/nat/src/masquerade/nf.rs +++ b/nat/src/masquerade/nf.rs @@ -41,6 +41,8 @@ pub(crate) enum MasqueradeError { FlowKeyError, #[error("no allocator available")] NoAllocator, + #[error("packet reached masquerade without a VPC discriminant")] + MissingDiscriminant, #[error("allocation failed: {0}")] AllocationFailure(AllocatorError), #[error("invalid port {0}")] @@ -70,10 +72,27 @@ pub struct Masquerade { } impl Masquerade { + // Slow emulated tests need more wall-clock time between packet-driven refreshes. + const TIMEOUT_SCALE: u64 = cfg_select! { + emulated => 100, + _ => 1, + }; + // Internal flow timeouts for masquerading - pub const MASQUERADE_ONEWAY_TIMEOUT: Duration = Duration::from_secs(5); - pub const MASQUERADE_TWOWAY_TIMEOUT: Duration = Duration::from_secs(3); - pub const MASQUERADE_CLOSING_TIMEOUT: Duration = Duration::from_secs(2); + pub const MASQUERADE_ONEWAY_TIMEOUT: Duration = Duration::from_secs(5 * Self::TIMEOUT_SCALE); + pub const MASQUERADE_TWOWAY_TIMEOUT: Duration = Duration::from_secs(3 * Self::TIMEOUT_SCALE); + pub const MASQUERADE_CLOSING_TIMEOUT: Duration = Duration::from_secs(2 * Self::TIMEOUT_SCALE); + + /// Return the VPC identities required for masquerading, or drop malformed input. + fn discriminants( + packet: &Packet, + ) -> Result<(VpcDiscriminant, VpcDiscriminant), MasqueradeError> { + let (Some(src), Some(dst)) = (packet.meta().src_vpcd, packet.meta().dst_vpcd) else { + error!("Masqueraded packet without a VPC discriminant. This is a bug"); + return Err(MasqueradeError::MissingDiscriminant); + }; + Ok((src, dst)) + } /// Creates a new [`Masquerade`] processor from provided parameters. #[must_use] @@ -250,8 +269,7 @@ impl Masquerade { let idle_timeout = alloc.idle_timeout; // src and dst vpc of this packet - let src_vpc_id = packet.meta().src_vpcd.unwrap_or_else(|| unreachable!()); - let dst_vpc_id = packet.meta().dst_vpcd.unwrap_or_else(|| unreachable!()); + let (src_vpc_id, dst_vpc_id) = Self::discriminants(packet)?; // build key for reverse flow, based on the current packet headers: if we use masquerading // with static NAT, we assume we've already been through static destination NAT and we'll @@ -374,7 +392,7 @@ impl Masquerade { return Err(MasqueradeError::IntendedDrop("TCP without SYN")); } - let dst_vpcd = packet.meta().dst_vpcd.unwrap_or_else(|| unreachable!()); + let (src_vpcd, dst_vpcd) = Self::discriminants(packet)?; // Extract flow key for the current packet let current_flow_key = @@ -393,7 +411,7 @@ impl Masquerade { // Create a new session and translate the address let src_ip = *initial_flow_key.src_ip(); let alloc = allocator - .allocate(dst_vpcd, src_ip, initial_flow_key.proto()) + .allocate(src_vpcd, dst_vpcd, src_ip, initial_flow_key.proto()) .map_err(MasqueradeError::AllocationFailure)?; // The generation the installed allocator serves @@ -511,6 +529,7 @@ impl From<&MasqueradeError> for DoneReason { DoneReason::Malformed } MasqueradeError::CapacityExceeded => DoneReason::FlowCapacityExceeded, + MasqueradeError::MissingDiscriminant => DoneReason::Unroutable, MasqueradeError::NoAllocator | MasqueradeError::UnexpectedKeyVariant | MasqueradeError::IcmpUnsupportedCategory diff --git a/nat/src/masquerade/state.rs b/nat/src/masquerade/state.rs index 17cc8bfbfc..43df798490 100644 --- a/nat/src/masquerade/state.rs +++ b/nat/src/masquerade/state.rs @@ -9,7 +9,7 @@ use std::fmt::Display; use std::net::IpAddr; use std::time::Duration; -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct MasqueradeState { pub(crate) status: AtomicNatFlowStatus, action: NatAction, diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index c87a2f4aad..6dc22440c5 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -329,6 +329,175 @@ fn build_overlay_2vpcs_modified() -> Overlay { Overlay::new(vpc_table, peering_table) } +// Two VPCs reuse one private prefix but masquerade onto separate public ranges. +fn build_overlay_shared_private_prefix() -> Overlay { + let mut vpc_table = VpcTable::new(); + let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-2", "BBBBB", 200).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-3", "CCCCC", 300).expect("Failed to add VPC")); + + let expose13 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.1.0.0/16".into()) + .as_range("2.2.0.0/16".into()) + .unwrap(); + // The same private space as VPC-1, onto a different public range. + let expose23 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.1.0.0/16".into()) + .as_range("4.4.0.0/16".into()) + .unwrap(); + + let peering13 = VpcPeering::with_default_group( + "VPC-1--VPC-3", + VpcManifest::new("VPC-1").exposing(expose13), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + let peering23 = VpcPeering::with_default_group( + "VPC-2--VPC-3", + VpcManifest::new("VPC-2").exposing(expose23), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + + let mut peering_table = VpcPeeringTable::new(); + peering_table.add(peering13).expect("Failed to add peering"); + peering_table.add(peering23).expect("Failed to add peering"); + + Overlay::new(vpc_table, peering_table) +} + +// Add a disjoint expose to force allocator replacement without invalidating existing flows. +fn build_overlay_shared_private_prefix_extended() -> Overlay { + let mut vpc_table = VpcTable::new(); + let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-2", "BBBBB", 200).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-3", "CCCCC", 300).expect("Failed to add VPC")); + + let expose13 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.1.0.0/16".into()) + .as_range("2.2.0.0/16".into()) + .unwrap(); + let expose13_extra = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.9.0.0/16".into()) + .as_range("9.9.0.0/16".into()) + .unwrap(); + let expose23 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.1.0.0/16".into()) + .as_range("4.4.0.0/16".into()) + .unwrap(); + + let peering13 = VpcPeering::with_default_group( + "VPC-1--VPC-3", + VpcManifest::new("VPC-1") + .exposing(expose13) + .exposing(expose13_extra), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + let peering23 = VpcPeering::with_default_group( + "VPC-2--VPC-3", + VpcManifest::new("VPC-2").exposing(expose23), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + + let mut peering_table = VpcPeeringTable::new(); + peering_table.add(peering13).expect("Failed to add peering"); + peering_table.add(peering23).expect("Failed to add peering"); + + Overlay::new(vpc_table, peering_table) +} + +// Stop exposing VPC-1's original private source while retaining its public range. +fn build_overlay_shared_private_prefix_narrowed() -> Overlay { + let mut vpc_table = VpcTable::new(); + let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-2", "BBBBB", 200).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-3", "CCCCC", 300).expect("Failed to add VPC")); + + let expose13 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.7.0.0/16".into()) // no longer covers 1.1.0.1 + .as_range("2.2.0.0/16".into()) + .unwrap(); + let expose23 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.1.0.0/16".into()) + .as_range("4.4.0.0/16".into()) + .unwrap(); + + let peering13 = VpcPeering::with_default_group( + "VPC-1--VPC-3", + VpcManifest::new("VPC-1").exposing(expose13), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + let peering23 = VpcPeering::with_default_group( + "VPC-2--VPC-3", + VpcManifest::new("VPC-2").exposing(expose23), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + + let mut peering_table = VpcPeeringTable::new(); + peering_table.add(peering13).expect("Failed to add peering"); + peering_table.add(peering23).expect("Failed to add peering"); + + Overlay::new(vpc_table, peering_table) +} + +// Remove masquerading while retaining the peering. +fn build_overlay_without_masquerade() -> Overlay { + let mut vpc_table = VpcTable::new(); + let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-3", "CCCCC", 300).expect("Failed to add VPC")); + + let peering13 = VpcPeering::with_default_group( + "VPC-1--VPC-3", + VpcManifest::new("VPC-1").exposing(VpcExpose::empty().ip("1.1.0.0/16".into())), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + ); + + let mut peering_table = VpcPeeringTable::new(); + peering_table.add(peering13).expect("Failed to add peering"); + + Overlay::new(vpc_table, peering_table) +} + +// Build a TCP packet towards VPC-3 from the given private endpoint. +fn tcp_from(src_vni_id: u32, src_ip: &str, sport: u16, syn: bool) -> Packet { + let mut packet = build_test_tcp_ipv4_packet(src_ip, "3.3.3.1", sport, 80); + { + let tcp = packet.try_tcp_mut().unwrap(); + tcp.set_syn(syn); + tcp.set_ack(false); + tcp.set_fin(false); + tcp.set_rst(false); + } + packet.meta_mut().set_overlay(true); + packet.meta_mut().src_vpcd = Some(vpcd(src_vni_id)); + packet.meta_mut().set_masquerade(true); + packet +} + +fn translated_source(packet: &Packet) -> Ipv4Addr { + packet.try_ipv4().unwrap().source().inner() +} + +// The public address and port a flow has been masqueraded onto. +fn translation(packet: &Packet) -> (Ipv4Addr, u16) { + ( + translated_source(packet), + packet.transport_src_port().unwrap().into(), + ) +} + fn check_packet( nat: &mut Masquerade, src_vni: Vni, @@ -1420,17 +1589,20 @@ fn nat_flow_status(packet: &Packet) -> Option { .map(|state| state.status.load()) } -fn masquerade_state(packet: &Packet) -> Option { - packet - .meta() - .flow_info - .as_ref()? - .locked - .read() +// Read something out of a flow's masquerade state, under the lock. +// +// Deliberately not a clone of the state: it owns the allocation, and a copy of it is a second +// owner of the same public address and port. Nothing in a test needs that. +fn with_masquerade_state( + packet: &Packet, + read: impl FnOnce(&MasqueradeState) -> T, +) -> Option { + let locked = packet.meta().flow_info.as_ref()?.locked.read(); + let state = locked .nat_state - .as_ref() - .and_then(|s| s.extract_ref::()) - .cloned() + .as_ref()? + .extract_ref::()?; + Some(read(state)) } fn build_reply(packet: &Packet) -> Packet { @@ -1502,7 +1674,7 @@ fn establish_tcp_connection(pipeline: &mut DynPipeline) { assert_eq!(nat_flow_status(&output), Some(NatFlowStatus::Established)); // configured timeout for the flow - let timeout = masquerade_state(&output).unwrap().idle_timeout(); + let timeout = with_masquerade_state(&output, MasqueradeState::idle_timeout).unwrap(); // check that flow timeouts "match" the ones configured, allowing for 5 second error (for the test) let flow_info_ack = output.meta().flow_info.as_ref().unwrap(); @@ -1539,8 +1711,9 @@ async fn test_masquerade_check() { let out = process_packet(&mut pipeline, packet); // packet hit flow with SRC nat rule - let state = masquerade_state(&out).expect("Must have flow info w/ masquerade state"); - assert_eq!(state.action(), NatAction::SrcNat); + let action = with_masquerade_state(&out, MasqueradeState::action) + .expect("Must have flow info w/ masquerade state"); + assert_eq!(action, NatAction::SrcNat); test_case("Process packet masquerade dest nat"); // process packet in dst nat direction @@ -1548,8 +1721,9 @@ async fn test_masquerade_check() { let out = process_packet(&mut pipeline, reply); // packet hit flow with dst nat rule - let state = masquerade_state(&out).expect("Must have flow info w/ masquerade state"); - assert_eq!(state.action(), NatAction::DstNat); + let action = with_masquerade_state(&out, MasqueradeState::action) + .expect("Must have flow info w/ masquerade state"); + assert_eq!(action, NatAction::DstNat); assert_eq!(nat_flow_status(&out).unwrap(), NatFlowStatus::Established); assert_eq!(flow_status(&out).unwrap(), FlowStatus::Active); @@ -1582,8 +1756,9 @@ async fn test_masquerade_tcp_reset() { let reply_out = process_packet(&mut pipeline, reply); // packet hits flow with dst nat rule. Nat flow status becomes reset and flow is cancelled - let state = masquerade_state(&reply_out).expect("Must have flow info w/ masquerade state"); - assert_eq!(state.action(), NatAction::DstNat); + let action = with_masquerade_state(&reply_out, MasqueradeState::action) + .expect("Must have flow info w/ masquerade state"); + assert_eq!(action, NatAction::DstNat); assert_eq!(nat_flow_status(&out).unwrap(), NatFlowStatus::Reset); assert_eq!(flow_status(&reply_out).unwrap(), FlowStatus::Cancelled); @@ -1598,6 +1773,105 @@ async fn test_masquerade_tcp_reset() { assert_eq!(flow_table.active_len(), Some(0)); } +// Walk the client-initiated graceful-close states. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_tcp_close_initiated_by_the_client() { + let (_flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs()); + establish_tcp_connection(&mut pipeline); + + // The client closes its side. + let mut fin = tcp_packet_to_masquerade(); + fin.try_tcp_mut().unwrap().set_fin(true); + let closing = process_packet(&mut pipeline, fin); + assert_eq!( + nat_flow_status(&closing), + Some(NatFlowStatus::CClosing), + "a FIN from the client should start the close" + ); + + // The server acknowledges it and keeps sending: half closed, not closed. + let mut ack = build_reply(&closing); + ack.try_tcp_mut().unwrap().set_fin(false); + let half = process_packet(&mut pipeline, ack); + assert_eq!( + nat_flow_status(&half), + Some(NatFlowStatus::CHalfClose), + "an ack of the client's FIN should leave the connection half closed" + ); + + // Then the server closes its own side. + let mut server_fin = build_reply(&closing); + server_fin.try_tcp_mut().unwrap().set_fin(true); + let last = process_packet(&mut pipeline, server_fin); + assert_eq!( + nat_flow_status(&last), + Some(NatFlowStatus::LastAck), + "the server's FIN should leave only the last ack outstanding" + ); + + // And the client acknowledges that. + let mut final_ack = tcp_packet_to_masquerade(); + final_ack.try_tcp_mut().unwrap().set_ack(true); + let closed = process_packet(&mut pipeline, final_ack); + assert_eq!( + nat_flow_status(&closed), + Some(NatFlowStatus::Closed), + "the last ack should close the connection" + ); +} + +// Walk the server-initiated graceful-close states. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_tcp_close_initiated_by_the_server() { + let (_flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs()); + establish_tcp_connection(&mut pipeline); + + // A packet out, so there is something to build the server's replies from. + let out = process_packet(&mut pipeline, tcp_packet_to_masquerade()); + + // The server closes first. + let mut server_fin = build_reply(&out); + server_fin.try_tcp_mut().unwrap().set_fin(true); + let closing = process_packet(&mut pipeline, server_fin); + assert_eq!( + nat_flow_status(&closing), + Some(NatFlowStatus::SClosing), + "a FIN from the server should start the close" + ); + + // The client acknowledges it and keeps sending. + let mut ack = tcp_packet_to_masquerade(); + ack.try_tcp_mut().unwrap().set_ack(true); + let half = process_packet(&mut pipeline, ack); + assert_eq!( + nat_flow_status(&half), + Some(NatFlowStatus::SHalfClose), + "an ack of the server's FIN should leave the connection half closed" + ); + + // Then closes its own side. + let mut client_fin = tcp_packet_to_masquerade(); + client_fin.try_tcp_mut().unwrap().set_fin(true); + let last = process_packet(&mut pipeline, client_fin); + assert_eq!( + nat_flow_status(&last), + Some(NatFlowStatus::LastAck), + "the client's FIN should leave only the last ack outstanding" + ); + + // And the server acknowledges that. + let mut final_ack = build_reply(&out); + final_ack.try_tcp_mut().unwrap().set_fin(false); + let closed = process_packet(&mut pipeline, final_ack); + assert_eq!( + nat_flow_status(&closed), + Some(NatFlowStatus::Closed), + "the last ack should close the connection" + ); +} + #[tokio::test] #[cfg_attr(not(emulated), traced_test)] async fn test_masquerade_reconfig_keep_flow() { @@ -1634,6 +1908,247 @@ async fn test_masquerade_reconfig_keep_flow() { assert_eq!(flow_table.active_len(), Some(2)); } +// Private addresses shared across VPCs must retain their VPC-specific translations. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_reconfig_two_vpcs_sharing_a_private_prefix() { + let genid = 1; + let (flow_table, mut pipeline, mut allocw) = + test_setup(genid, &build_overlay_shared_private_prefix()); + + // The SYN opens each flow; a follow-up packet exposes its state. + process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, true)); + process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, true)); + + let from_vpc1 = process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, false)); + let from_vpc2 = process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, false)); + + let public1 = translated_source(&from_vpc1); + let public2 = translated_source(&from_vpc2); + assert_eq!( + public1.octets()[0..2], + [2, 2], + "VPC-1 was not masqueraded onto the range its own expose declares, got {public1}" + ); + assert_eq!( + public2.octets()[0..2], + [4, 4], + "VPC-2 was not masqueraded onto the range its own expose declares, got {public2}" + ); + assert_eq!(flow_genid(&from_vpc1).unwrap(), genid); + assert_eq!(flow_genid(&from_vpc2).unwrap(), genid); + + // An identical config keeps the allocator and advances only the flow generation. + let overlay = build_overlay_shared_private_prefix().validate().unwrap(); + let nat_config = MasqueradeConfig::new(overlay.vpc_table()); + allocw.update_nat_allocator(nat_config, genid + 1, &flow_table); + + // Both survive, each still translated exactly as before. + let from_vpc1 = process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, false)); + let from_vpc2 = process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, false)); + + assert_eq!( + translated_source(&from_vpc1), + public1, + "VPC-1's flow did not keep its public address across the config change" + ); + assert_eq!( + translated_source(&from_vpc2), + public2, + "VPC-2's flow did not keep its public address across the config change" + ); + assert_eq!(flow_genid(&from_vpc1).unwrap(), genid + 1); + assert_eq!(flow_genid(&from_vpc2).unwrap(), genid + 1); + + tokio::time::sleep(Duration::from_secs(1)).await; + assert_eq!(flow_table.active_len(), Some(4)); +} + +// Replacement must reserve each surviving tuple in its source VPC's pool before publication. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_reconfig_carries_flows_into_a_new_allocator() { + let genid = 1; + let (flow_table, mut pipeline, mut allocw) = + test_setup(genid, &build_overlay_shared_private_prefix()); + + process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, true)); + process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, true)); + let before_vpc1 = translation(&process_packet( + &mut pipeline, + tcp_from(100, "1.1.0.1", 4321, false), + )); + let before_vpc2 = translation(&process_packet( + &mut pipeline, + tcp_from(200, "1.1.0.1", 4321, false), + )); + assert_eq!(before_vpc1.0.octets()[0..2], [2, 2]); + assert_eq!(before_vpc2.0.octets()[0..2], [4, 4]); + + // Rebuild the allocator without invalidating either flow. + let overlay = build_overlay_shared_private_prefix_extended() + .validate() + .unwrap(); + let nat_config = MasqueradeConfig::new(overlay.vpc_table()); + allocw.update_nat_allocator(nat_config, genid + 1, &flow_table); + + let after_vpc1 = translation(&process_packet( + &mut pipeline, + tcp_from(100, "1.1.0.1", 4321, false), + )); + let after_vpc2 = translation(&process_packet( + &mut pipeline, + tcp_from(200, "1.1.0.1", 4321, false), + )); + assert_eq!( + after_vpc1, before_vpc1, + "VPC-1's flow was not carried into the new allocator unchanged" + ); + assert_eq!( + after_vpc2, before_vpc2, + "VPC-2's flow was not carried into the new allocator unchanged" + ); + + // New flows must not reuse carried tuples. + process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 5555, true)); + let fresh = translation(&process_packet( + &mut pipeline, + tcp_from(100, "1.1.0.1", 5555, false), + )); + assert_ne!( + fresh, before_vpc1, + "a flow created after the change was given what a carried-over flow still holds" + ); + assert_ne!(fresh, before_vpc2); +} + +// Remove one peering without disturbing flows on another. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_reconfig_drops_a_flow_whose_peering_is_gone() { + let genid = 1; + let (flow_table, mut pipeline, mut allocw) = + test_setup(genid, &build_overlay_shared_private_prefix()); + + process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, true)); + process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, true)); + let vpc2_before = translation(&process_packet( + &mut pipeline, + tcp_from(200, "1.1.0.1", 4321, false), + )); + + // VPC-1 loses its peering with VPC-3; VPC-2 keeps both. + let mut vpc_table = VpcTable::new(); + let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-2", "BBBBB", 200).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-3", "CCCCC", 300).expect("Failed to add VPC")); + let mut peering_table = VpcPeeringTable::new(); + peering_table + .add(VpcPeering::with_default_group( + "VPC-2--VPC-3", + VpcManifest::new("VPC-2").exposing( + VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("1.1.0.0/16".into()) + .as_range("4.4.0.0/16".into()) + .unwrap(), + ), + VpcManifest::new("VPC-3").exposing(VpcExpose::empty().ip("3.3.3.0/24".into())), + )) + .expect("Failed to add peering"); + let overlay = Overlay::new(vpc_table, peering_table).validate().unwrap(); + allocw.update_nat_allocator( + MasqueradeConfig::new(overlay.vpc_table()), + genid + 1, + &flow_table, + ); + + let out = process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, false)); + assert_eq!( + out.get_done(), + Some(DoneReason::Filtered), + "traffic kept flowing over a peering that no longer exists" + ); + + // VPC-2's flow survives. + assert_eq!( + translation(&process_packet( + &mut pipeline, + tcp_from(200, "1.1.0.1", 4321, false) + )), + vpc2_before, + "a flow whose peering still exists was invalidated too" + ); +} + +// Stop exposing one flow's private source while retaining its public range. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_reconfig_drops_a_flow_whose_source_is_no_longer_exposed() { + let genid = 1; + let (flow_table, mut pipeline, mut allocw) = + test_setup(genid, &build_overlay_shared_private_prefix()); + + process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, true)); + process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, true)); + let vpc2_before = translation(&process_packet( + &mut pipeline, + tcp_from(200, "1.1.0.1", 4321, false), + )); + + let overlay = build_overlay_shared_private_prefix_narrowed() + .validate() + .unwrap(); + let nat_config = MasqueradeConfig::new(overlay.vpc_table()); + allocw.update_nat_allocator(nat_config, genid + 1, &flow_table); + + let out = process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, false)); + assert_eq!( + out.get_done(), + Some(DoneReason::Filtered), + "traffic from a source that is no longer masqueraded was let through" + ); + + // VPC-2's flow survives. + let vpc2_after = translation(&process_packet( + &mut pipeline, + tcp_from(200, "1.1.0.1", 4321, false), + )); + assert_eq!( + vpc2_after, vpc2_before, + "a flow of the VPC whose configuration did not change was disturbed" + ); +} + +// Taking masquerade out of the configuration entirely drops every masqueraded flow. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn test_masquerade_reconfig_without_masquerade_drops_every_flow() { + let genid = 1; + let (flow_table, mut pipeline, mut allocw) = + test_setup(genid, &build_overlay_shared_private_prefix()); + + process_packet(&mut pipeline, tcp_from(100, "1.1.0.1", 4321, true)); + process_packet(&mut pipeline, tcp_from(200, "1.1.0.1", 4321, true)); + + let overlay = build_overlay_without_masquerade().validate().unwrap(); + let nat_config = MasqueradeConfig::new(overlay.vpc_table()); + allocw.update_nat_allocator(nat_config, genid + 1, &flow_table); + + for vni in [100, 200] { + let out = process_packet(&mut pipeline, tcp_from(vni, "1.1.0.1", 4321, false)); + assert_eq!( + out.get_done(), + Some(DoneReason::NatFailure), + "traffic from VNI {vni} was let through after masquerade was removed" + ); + } + + tokio::time::sleep(Duration::from_secs(1)).await; + assert_eq!(flow_table.active_len(), Some(0)); +} + #[tokio::test] #[cfg_attr(not(emulated), traced_test)] async fn test_masquerade_reconfig_drop_flow() { diff --git a/nat/src/portfw/flow_state.rs b/nat/src/portfw/flow_state.rs index 0b5c8c5677..efef56ee01 100644 --- a/nat/src/portfw/flow_state.rs +++ b/nat/src/portfw/flow_state.rs @@ -6,7 +6,7 @@ #![allow(clippy::single_match_else)] use net::buffer::PacketBufferMut; -use net::flows::{ExtractRef, FlowStatus}; +use net::flows::{ExtractMut, ExtractRef, FlowStatus}; use net::ip::UnicastIpAddr; use net::packet::{Packet, VpcDiscriminant}; use net::{FlowKey, IpProtoKey}; @@ -19,40 +19,72 @@ use concurrency::sync::{Arc, Weak}; use flow_entry::flow_table::FlowInfo; use crate::common::{AtomicNatFlowStatus, NatAction, NatFlowStatus}; +use crate::masquerade::allocation::AllocatorError; +use crate::masquerade::apalloc::{Allocation, NatAllocator}; use crate::portfw::PortFwEntry; use crate::portfw::protocol::next_flow_status; #[allow(unused)] use tracing::{debug, error, warn}; +#[derive(Debug, Clone)] +pub(crate) struct PublicTuple { + ip: UnicastIpAddr, + port: NonZero, + peer_vpcd: VpcDiscriminant, + lease: Option>, +} + +impl PublicTuple { + pub(crate) fn new( + ip: UnicastIpAddr, + port: NonZero, + peer_vpcd: VpcDiscriminant, + lease: Option>, + ) -> Self { + Self { + ip, + port, + peer_vpcd, + lease, + } + } + + fn set_lease(&mut self, lease: Option>) { + self.lease = lease; + } +} + #[derive(Debug, Clone)] pub struct PortFwState { pub(crate) action: NatAction, pub(crate) status: AtomicNatFlowStatus, use_ip: UnicastIpAddr, use_port: NonZero, + public: PublicTuple, pub(crate) rule: Weak, } impl PortFwState { #[must_use] - pub fn new_snat( - use_ip: UnicastIpAddr, - use_port: NonZero, + pub(crate) fn new_snat( + public: PublicTuple, rule: Weak, status: AtomicNatFlowStatus, ) -> Self { Self { action: NatAction::SrcNat, status, - use_ip, - use_port, + use_ip: public.ip, + use_port: public.port, + public, rule, } } #[must_use] - pub fn new_dnat( + pub(crate) fn new_dnat( use_ip: UnicastIpAddr, use_port: NonZero, + public: PublicTuple, rule: Weak, status: AtomicNatFlowStatus, ) -> Self { @@ -61,6 +93,7 @@ impl PortFwState { status, use_ip, use_port, + public, rule, } } @@ -77,6 +110,22 @@ impl PortFwState { self.use_port } #[must_use] + pub(crate) fn public_ip(&self) -> UnicastIpAddr { + self.public.ip + } + #[must_use] + pub(crate) fn public_port(&self) -> NonZero { + self.public.port + } + pub(crate) fn set_lease(&mut self, lease: Option>) { + self.public.set_lease(lease); + } + #[cfg(test)] + #[must_use] + pub(crate) fn lease(&self) -> Option<&Arc> { + self.public.lease.as_ref() + } + #[must_use] pub fn rule(&self) -> &Weak { &self.rule } @@ -98,6 +147,52 @@ impl Display for PortFwState { } } +/// Update the public-tuple lease of a flow with port-forwarding state. +pub(crate) fn update_port_forward_lease( + flow_info: &FlowInfo, + allocator: Option<&NatAllocator>, +) -> Result<(), AllocatorError> { + let (public_ip, public_port, peer_vpcd) = { + let locked = flow_info.locked.read(); + let Some(state) = locked + .port_fw_state + .as_ref() + .and_then(|state| state.extract_ref::()) + else { + return Ok(()); + }; + ( + state.public_ip(), + state.public_port(), + state.public.peer_vpcd, + ) + }; + + let lease = allocator + .map(|allocator| { + allocator.reserve_port_forward( + flow_info.flowkey().proto(), + peer_vpcd, + public_ip.inner(), + public_port, + ) + }) + .transpose()? + .flatten(); + + let mut locked = flow_info.locked.write(); + let Some(state) = locked + .port_fw_state + .as_mut() + .and_then(|state| state.extract_mut::()) + else { + debug!("Port-forwarding state vanished while updating its lease"); + return Ok(()); + }; + state.set_lease(lease); + Ok(()) +} + // Build the flow keys for a port-forwarding flow pub(crate) fn build_portfw_flow_keys( packet: &mut Packet, // packet to be port-forwarded (in the forward path) @@ -136,12 +231,14 @@ pub(crate) fn setup_forward_flow( entry: &Arc, new_dst_ip: UnicastIpAddr, new_dst_port: NonZero, + public: PublicTuple, ) -> AtomicNatFlowStatus { // build port forwarding state for the forward flow let status = AtomicNatFlowStatus::new(); let port_fw_state = PortFwState::new_dnat( new_dst_ip, new_dst_port, + public, Arc::downgrade(entry), status.clone(), ); @@ -160,12 +257,11 @@ pub(crate) fn setup_reverse_flow( reverse_key: &FlowKey, reverse_flow: &Arc, entry: &Arc, - dst_ip: UnicastIpAddr, - dst_port: NonZero, + public: PublicTuple, status: AtomicNatFlowStatus, ) { // build port forwarding state for the REVERSE flow - let port_fw_state = PortFwState::new_snat(dst_ip, dst_port, Arc::downgrade(entry), status); + let port_fw_state = PortFwState::new_snat(public, Arc::downgrade(entry), status); // set the port forwarding state in the flow { diff --git a/nat/src/portfw/mod.rs b/nat/src/portfw/mod.rs index 20d367e319..3448499a54 100644 --- a/nat/src/portfw/mod.rs +++ b/nat/src/portfw/mod.rs @@ -13,6 +13,7 @@ mod test; // re-exports pub use flow_state::PortFwState; +pub(crate) use flow_state::update_port_forward_lease; pub use nf::PortForwarder; pub use portfwtable::PortFwTableError; pub use portfwtable::access::{PortFwTableReader, PortFwTableReaderFactory, PortFwTableWriter}; diff --git a/nat/src/portfw/nf.rs b/nat/src/portfw/nf.rs index d4bff9fab1..88781bf444 100644 --- a/nat/src/portfw/nf.rs +++ b/nat/src/portfw/nf.rs @@ -3,6 +3,7 @@ //! Port forwarding stage +use crate::masquerade::NatAllocatorReader; use crate::portfw::{PortFwEntry, PortFwKey, PortFwState, PortFwTable, PortFwTableReader}; use concurrency::sync::{Arc, Weak}; use flow_entry::flow_table::table::FlowTable; @@ -17,11 +18,13 @@ use std::num::NonZero; use std::time::Instant; use crate::common::NatAction; +use crate::portfw::flow_state::PublicTuple; use crate::portfw::flow_state::build_portfw_flow_keys; use crate::portfw::flow_state::get_packet_port_fw_state; use crate::portfw::flow_state::refresh_port_fw_entry; use crate::portfw::flow_state::setup_forward_flow; use crate::portfw::flow_state::setup_reverse_flow; +use crate::portfw::flow_state::update_port_forward_lease; use crate::portfw::packet::nat_packet; #[allow(unused)] @@ -32,17 +35,24 @@ pub struct PortForwarder { name: String, flow_table: Arc, fwtable: PortFwTableReader, + allocator: NatAllocatorReader, pipeline_data: Arc, } impl PortForwarder { /// Creates a new [`PortForwarder`] #[must_use] - pub fn new(name: &str, fwtable: PortFwTableReader, flow_table: Arc) -> Self { + pub fn new( + name: &str, + fwtable: PortFwTableReader, + flow_table: Arc, + allocator: NatAllocatorReader, + ) -> Self { Self { name: name.to_string(), flow_table, fwtable, + allocator, pipeline_data: Arc::from(PipelineData::default()), } } @@ -115,6 +125,23 @@ impl PortForwarder { return; }; + let lease = match self.allocator.get().map(|allocator| { + allocator.reserve_port_forward( + fw_key.proto(), + entry.key.src_vpcd(), + dst_ip.inner(), + dst_port, + ) + }) { + Some(Ok(lease)) => lease, + Some(Err(error)) => { + debug!("Unable to reserve {dst_ip}:{dst_port} for port forwarding: {error}"); + packet.done((&error).into()); + return; + } + None => None, + }; + // create a pair of related flow entries (outside the flow table). Timeout is set according to the rule matched let timeout = Instant::now() + entry.init_timeout(); let (fw_flow, rev_flow) = FlowInfo::related_pair( @@ -129,8 +156,16 @@ impl PortForwarder { fw_flow.set_genid_pair(self.pipeline_data.genid()); // set the flows in the FORWARD & REVERSE direction for subsequent packets - let status = setup_forward_flow(&fw_key, &fw_flow, entry, new_dst_ip, new_dst_port); - setup_reverse_flow(&rev_key, &rev_flow, entry, dst_ip, dst_port, status); + let public = PublicTuple::new(dst_ip, dst_port, entry.key.src_vpcd(), lease); + let status = setup_forward_flow( + &fw_key, + &fw_flow, + entry, + new_dst_ip, + new_dst_port, + public.clone(), + ); + setup_reverse_flow(&rev_key, &rev_flow, entry, public, status); // get the state we just created for the FORWARD direction let locked = fw_flow.locked.read(); @@ -145,6 +180,7 @@ impl PortForwarder { packet.done(DoneReason::InternalFailure); return; } + drop(locked); // insert the two related flows if let Err(e) = self.flow_table.insert_from_arc(&fw_flow) { @@ -164,6 +200,15 @@ impl PortForwarder { debug_assert!(false, "reverse port-forwarding flow insert failed: {e:?}"); return; } + + let allocator = self.allocator.get(); + for flow in [&fw_flow, &rev_flow] { + if let Err(error) = update_port_forward_lease(flow, allocator.as_deref()) { + flow.invalidate_pair(); + packet.done((&error).into()); + return; + } + } debug!("Inserted forward and reverse port-forwarding flow entries"); } diff --git a/nat/src/portfw/test.rs b/nat/src/portfw/test.rs index 95c60451b1..b2a62335eb 100644 --- a/nat/src/portfw/test.rs +++ b/nat/src/portfw/test.rs @@ -3,11 +3,17 @@ #[cfg(test)] mod nf_test { + use crate::NatPort; use crate::common::NatFlowStatus; + use crate::masquerade::allocation::AllocatorError; + use crate::masquerade::apalloc::Allocation; + use crate::masquerade::{MasqueradeConfig, NatAllocatorWriter}; use crate::portfw::{PortForwarder, PortFwEntry, PortFwKey, PortFwState, PortFwTableWriter}; - use concurrency::sync::Arc; - use flow_entry::flow_table::{FlowLookup, FlowTable}; + use concurrency::sync::{Arc, Weak}; + use config::external::overlay::vpc::{Peering, ValidatedVpcTable, Vpc, VpcTable}; + use config::external::overlay::vpcpeering::{VpcExpose, VpcManifest}; + use flow_entry::flow_table::{FlowInfo, FlowLookup, FlowTable}; use lpm::prefix::Prefix; use net::buffer::TestBuffer; use net::flows::FlowStatus; @@ -17,6 +23,8 @@ mod nf_test { use net::packet::test_utils::{build_test_tcp_ipv4_packet, build_test_udp_ipv4_packet}; use net::packet::{DoneReason, Packet, VpcDiscriminant}; use pipeline::{DynPipeline, NetworkFunction}; + use std::net::IpAddr; + use std::num::NonZero; use std::str::FromStr; use std::time::Duration; use tracing_test::traced_test; @@ -129,6 +137,51 @@ mod nf_test { ruleset } + // Overlap the forwarded address with VPC-2's masquerade pool. + fn build_masquerade_vpc_table() -> ValidatedVpcTable { + let local = VpcManifest::with_exposes( + "VPC-2", + vec![ + VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("192.168.0.0/16".into()) + .as_range("70.71.72.0/24".into()) + .unwrap(), + ], + ); + let remote = + VpcManifest::with_exposes("VPC-1", vec![VpcExpose::empty().ip("10.0.0.0/24".into())]); + + let vpc1 = Vpc::new("VPC-1", "11111", 2000).unwrap(); + let mut vpc2 = Vpc::new("VPC-2", "22222", 3000).unwrap(); + vpc2.peerings.push(Peering { + name: "portfw_masquerade".into(), + local, + remote, + remote_id: "11111".try_into().unwrap(), + remote_vni: vpc1.vni, + gwgroup: "default".into(), + acl: None, + }); + + let mut vpctable = VpcTable::new(); + vpctable.add(vpc1).unwrap(); + vpctable.add(vpc2).unwrap(); + vpctable.validate().unwrap() + } + + const RELEASE_POLLS: usize = 64; + + fn lease_of(flow: &FlowInfo) -> Option> { + flow.locked + .read() + .port_fw_state + .as_ref() + .and_then(|state| state.extract_ref::()) + .and_then(|state| state.lease().cloned()) + } + // build a UDP packet to be port forwarded according to the port-forwarding table fn udp_packet_to_port_forward() -> Packet { let mut packet: Packet = @@ -196,15 +249,20 @@ mod nf_test { } } - /// sets up a port-forwarding pipeline - fn setup_pipeline( + fn setup_pipeline_with_allocator( ruleset: &[PortFwEntry], + allocator: &NatAllocatorWriter, ) -> (Arc, DynPipeline, PortFwTableWriter) { // build a pipeline with flow lookup + port forwarder let mut writer = PortFwTableWriter::new(); let flow_table = Arc::new(FlowTable::default()); let flow_lookup_nf = FlowLookup::new("flow-lookup", flow_table.clone()); - let nf = PortForwarder::new("port-forwarder", writer.reader(), flow_table.clone()); + let nf = PortForwarder::new( + "port-forwarder", + writer.reader(), + flow_table.clone(), + allocator.get_reader(), + ); let pipeline: DynPipeline = DynPipeline::new() .add_stage(flow_lookup_nf) .add_stage(TestFlowFilter) @@ -218,6 +276,13 @@ mod nf_test { (flow_table, pipeline, writer) } + fn setup_pipeline( + ruleset: &[PortFwEntry], + ) -> (Arc, DynPipeline, PortFwTableWriter) { + let allocator = NatAllocatorWriter::new(); + setup_pipeline_with_allocator(ruleset, &allocator) + } + #[cfg_attr(not(emulated), traced_test)] #[tokio::test] async fn test_nf_port_forwarding_base() { @@ -318,6 +383,79 @@ mod nf_test { ); } + #[cfg_attr(not(emulated), traced_test)] + #[tokio::test] + async fn port_forwarded_tuple_holds_a_masquerade_lease() { + let ruleset = build_test_port_forwarding_ruleset(); + let mut allocator = NatAllocatorWriter::new(); + let (flow_table, mut pipeline, _writer) = + setup_pipeline_with_allocator(&ruleset, &allocator); + allocator.update_nat_allocator( + MasqueradeConfig::new(&build_masquerade_vpc_table()), + 1, + &flow_table, + ); + + let output = process_packet(&mut pipeline, udp_packet_to_port_forward()); + assert!(!output.is_done()); + let reply = process_packet(&mut pipeline, build_reply(&output)); + + let one = reply + .meta() + .flow_info + .as_ref() + .expect("the reply matches the flow pair") + .clone(); + let other = one + .related + .as_ref() + .and_then(Weak::upgrade) + .expect("the forwarded flow has a related flow"); + + let lease_one = lease_of(&one).expect("the forwarded tuple overlaps a masquerade pool"); + let lease_other = lease_of(&other).expect("the forwarded tuple overlaps a masquerade pool"); + assert!( + Arc::ptr_eq(&lease_one, &lease_other), + "both directions must share one lease" + ); + let released = Arc::downgrade(&lease_one); + drop((lease_one, lease_other)); + + let nat = allocator + .get_reader() + .get() + .expect("allocator is installed"); + let port = NatPort::new_port(NonZero::new(3053).unwrap()); + let public: IpAddr = "70.71.72.73".parse().unwrap(); + let private: IpAddr = "192.168.1.2".parse().unwrap(); + + match nat.reserve_port(NextHeader::UDP, vpcd2(), vpcd1(), private, public, port) { + Err(AllocatorError::PortReservationFailed(blocked)) => assert_eq!(blocked, 3053), + other => panic!("a live lease must block the reservation, got {other:?}"), + } + + let keys = [*one.flowkey(), *other.flowkey()]; + drop((reply, output, one, other, pipeline)); + for key in &keys { + flow_table.remove(key).expect("the flow was in the table"); + } + + // Timer tasks retain flow state until cancellation is polled. + for _ in 0..RELEASE_POLLS { + if released.upgrade().is_none() { + break; + } + tokio::task::yield_now().await; + } + assert!( + released.upgrade().is_none(), + "retiring both flows must release the lease", + ); + + nat.reserve_port(NextHeader::UDP, vpcd2(), vpcd1(), private, public, port) + .expect("the tuple is free once the forwarded flows are gone"); + } + #[cfg_attr(not(emulated), traced_test)] #[tokio::test] async fn test_nf_port_forwarding_tcp_establishment() { diff --git a/nat/src/test.rs b/nat/src/test.rs index 09d9b48596..c8178f640b 100644 --- a/nat/src/test.rs +++ b/nat/src/test.rs @@ -8,6 +8,7 @@ #![cfg(all(test, not(miri)))] use super::*; +use crate::masquerade::allocation::AllocatorError; use crate::masquerade::{MasqueradeConfig, NatAllocatorWriter}; use crate::portfw::{PortForwarder, PortFwTableWriter}; use crate::static_nat::NatTablesWriter; @@ -105,19 +106,25 @@ fn setup_masq_pipeline( let static_nat = StaticNat::with_reader("static-NAT-1", static_nat_writer.get_reader()); pipeline = pipeline.add_stage(static_nat); + let mut allocator = NatAllocatorWriter::new(); + // Port forwarding let mut portfw_writer = PortFwTableWriter::new(); portfw_writer .update_from_vpc_table(overlay.vpc_table()) .unwrap(); - let portfw = PortForwarder::new("port-forwarder", portfw_writer.reader(), flow_table.clone()); + let portfw = PortForwarder::new( + "port-forwarder", + portfw_writer.reader(), + flow_table.clone(), + allocator.get_reader(), + ); if let Some(table) = portfw_writer.enter() { println!("{}", table.as_ref()); } pipeline = pipeline.add_stage(portfw); // Masquerade - let mut allocator = NatAllocatorWriter::new(); let masquerade = Masquerade::new("masquerade", flow_table.clone(), allocator.get_reader()); let masquerade_config = MasqueradeConfig::new(overlay.vpc_table()); allocator.update_nat_allocator(masquerade_config, 1, &flow_table); @@ -136,6 +143,105 @@ fn setup_masq_pipeline( ) } +fn build_overlapping_masquerade_and_port_forward() -> ValidatedOverlay { + let mut vpc_table = VpcTable::new(); + vpc_table + .add(Vpc::new("external", "VPC01", 100).unwrap()) + .unwrap(); + vpc_table + .add(Vpc::new("internal", "VPC02", 200).unwrap()) + .unwrap(); + + let masquerade = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("192.168.0.0/24".into()) + .as_range("5.6.7.8/32".into()) + .unwrap(); + let port_forward = VpcExpose::empty() + .make_port_forwarding(None, None) + .unwrap() + .ip(pwp("192.168.0.8/32", 8000, 8000)) + .as_range(pwp("5.6.7.8/32", 1024, 1024)) + .unwrap(); + + let mut peerings = VpcPeeringTable::new(); + peerings + .add(VpcPeering::new( + "external--internal", + VpcManifest::with_exposes("external", vec![VpcExpose::empty().ip("1.2.3.0/24".into())]), + VpcManifest::with_exposes("internal", vec![masquerade, port_forward]), + "default".into(), + )) + .unwrap(); + + Overlay::new(vpc_table, peerings).validate().unwrap() +} + +#[tokio::test] +#[dpdk::with_eal] +async fn inactive_port_forward_does_not_reduce_masquerade_space() { + let overlay = build_overlapping_masquerade_and_port_forward(); + let (mut pipeline, flow_table, _flow_filter, _static_nat, _portfw, mut allocator) = + setup_masq_pipeline(&overlay); + allocator.update_nat_allocator( + MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), + 2, + &flow_table, + ); + + let packet = build_packet("192.168.0.9", "1.2.3.10", 4000, 9000, vni(200)); + let output: Vec<_> = pipeline.process(std::iter::once(packet)).collect(); + let output = output.first().expect("masquerade should accept the flow"); + assert_eq!(output.ip_source(), Some(addr("5.6.7.8"))); + assert_eq!(output.transport_src_port().unwrap().get(), 1024); +} + +#[tokio::test] +#[dpdk::with_eal] +async fn active_port_forward_reserves_its_public_tuple() { + let overlay = build_overlapping_masquerade_and_port_forward(); + let (mut pipeline, flow_table, _flow_filter, _static_nat, _portfw, mut allocator) = + setup_masq_pipeline(&overlay); + allocator.update_nat_allocator( + MasqueradeConfig::new(overlay.vpc_table()).set_randomize(false), + 2, + &flow_table, + ); + + for (client, port) in [("1.2.3.4", 5000), ("1.2.3.5", 5001)] { + let packet = build_packet(client, "5.6.7.8", port, 1024, vni(100)); + let output: Vec<_> = pipeline.process(std::iter::once(packet)).collect(); + let output = output + .first() + .expect("port forwarding should accept the flow"); + assert_eq!(output.ip_destination(), Some(addr("192.168.0.8"))); + assert_eq!(output.transport_dst_port().unwrap().get(), 8000); + } + + let tuple_is_reserved = |allocator: &NatAllocatorWriter| { + matches!( + allocator.get_reader().get().unwrap().reserve_port( + NextHeader::UDP, + vni(200).into(), + vni(100).into(), + addr("192.168.0.9"), + addr("5.6.7.8"), + NatPort::new_port_checked(1024).unwrap(), + ), + Err(AllocatorError::PortReservationFailed(1024)) + ) + }; + assert!(tuple_is_reserved(&allocator)); + + allocator.update_nat_allocator( + MasqueradeConfig::new(overlay.vpc_table()).set_randomize(true), + 3, + &flow_table, + ); + assert!(tuple_is_reserved(&allocator)); +} + #[tokio::test] #[dpdk::with_eal] async fn test_nat_combination_static_masquerade() {