From e192d5d53e035c62333c0ae10151104b145f4ac6 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 18 Sep 2026 12:20:37 -0700 Subject: [PATCH 1/5] [LXC] Subtract an egress 'except' from its own rule so a carve-out cannot shadow a later deny Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../0.8.0/networking/networking.md | 2 +- .../lxc/common/src/network_iptables.rs | 220 +++++++++++++-- .../src/network_iptables_ga_egress_spec.rs | 264 +++++++++++++++++- src/core/wxc_common/src/config_parser.rs | 2 +- .../lxc_network_ga_egress_except_shadow.json | 30 ++ ...twork_ga_egress_except_shadow_control.json | 26 ++ .../scripts/run_lxc_network_ga_egress_test.sh | 18 +- 7 files changed, 515 insertions(+), 47 deletions(-) create mode 100644 tests/configs/lxc_network_ga_egress_except_shadow.json create mode 100644 tests/configs/lxc_network_ga_egress_except_shadow_control.json diff --git a/docs/sandbox-policy/0.8.0/networking/networking.md b/docs/sandbox-policy/0.8.0/networking/networking.md index 7fc3aaa94..d8bcdcfaa 100644 --- a/docs/sandbox-policy/0.8.0/networking/networking.md +++ b/docs/sandbox-policy/0.8.0/networking/networking.md @@ -226,7 +226,7 @@ Egress peer and port fields (used in `egress.allow[]` / `egress.deny[]`; not sho | Field | Type | Notes | |---|---|---| | `to[].cidr` | IPv4 / IPv6 CIDR, or 0.0.0.0/0 / ::/0 for any | Single CIDR string (CNI/Kubernetes style), replacing separate address + prefix length. | -| `to[].except` | list of CIDRs, optional | Exclusions within the peer's CIDR (Kubernetes `ipBlock.except` style). Expressible on Windows process containers (WFP) and the Linux backends (iptables) as additional deny rules; not supported on Seatbelt (no destination filtering). | +| `to[].except` | list of CIDRs, optional | Exclusions within the peer's CIDR (Kubernetes `ipBlock.except` style). An exclusion narrows the rule that carries it and nothing else: it never states a verdict of its own, so an address it removes is decided by the remaining rules and the direction default. Windows process containers pass the exclusion to the platform (WFP); the Linux backends subtract it from the peer and program the covering blocks that remain, because an `iptables` rule cannot carry an exclusion and a separate rule would leak into later ones. Not supported on Seatbelt (no destination filtering). | | `ports[].protocol` | tcp / udp / icmp / any | `any` matches at minimum TCP, UDP, and ICMPv4/6; a backend may match more. Enforced on Windows process containers (WFP) and the Linux backends (iptables); not supported on Seatbelt. | | `ports[].port` | uint16, optional | Destination port. Omit `ports` to match all ports/protocols. | | `ports[].endPort` | uint16, optional | End of a port range (Kubernetes `endPort` style); requires numeric port. Supported on Windows process containers (WFP) and the Linux backends (iptables); not supported on Seatbelt. | diff --git a/src/backends/lxc/common/src/network_iptables.rs b/src/backends/lxc/common/src/network_iptables.rs index 2b697e09d..c845d97a8 100644 --- a/src/backends/lxc/common/src/network_iptables.rs +++ b/src/backends/lxc/common/src/network_iptables.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::net::{IpAddr, Ipv6Addr, ToSocketAddrs}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, ToSocketAddrs}; use std::process::Command; use sha2::{Digest, Sha256}; @@ -101,6 +101,26 @@ enum IpFamily { V6, } +/// A destination CIDR as a number, with `base` masked to the block's first +/// address so comparisons are integer ordering. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct DestinationBlock { + base: u128, + prefix: u8, +} + +/// Ceiling on the blocks one peer may expand into. +/// +/// Subtracting an exclusion splits the surrounding block once per prefix level, +/// so a `/32` taken out of a `/8` is 24 blocks and several exclusions add up. +const MAX_BLOCKS_PER_PEER: usize = 256; + +/// Ceiling on the entries one egress policy may lower into. +/// +/// An entry is a cross product of blocks and ports, and the wire contract +/// bounds neither the peer list, the port list, nor the rule list. +const MAX_EGRESS_ENTRIES: usize = 65_536; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum RuleAction { Allow, @@ -845,10 +865,13 @@ impl NetworkIptablesManager { } } - fn lower_egress(policy: &ContainerPolicy, uses_directional_keys: bool) -> Vec { + fn lower_egress( + policy: &ContainerPolicy, + uses_directional_keys: bool, + ) -> Result, String> { match Self::stated_egress(policy, uses_directional_keys) { Some(egress) => Self::lower_directional_egress(egress), - None => Self::lower_legacy_hosts(policy), + None => Ok(Self::lower_legacy_hosts(policy)), } } @@ -876,27 +899,24 @@ impl NetworkIptablesManager { // Deny rules precede allow rules for the same first-match-wins reason // as the legacy lowering. - fn lower_directional_egress(egress: &NetworkEgressPolicy) -> Vec { + fn lower_directional_egress(egress: &NetworkEgressPolicy) -> Result, String> { let mut entries = Vec::new(); - let default_action = match egress.default { - NetworkAction::Allow => RuleAction::Allow, - NetworkAction::Deny => RuleAction::Deny, - }; + let mut remaining = MAX_EGRESS_ENTRIES; for rule in &egress.deny { - Self::lower_rule(rule, RuleAction::Deny, default_action, &mut entries); + Self::lower_rule(rule, RuleAction::Deny, &mut entries, &mut remaining)?; } for rule in &egress.allow { - Self::lower_rule(rule, RuleAction::Allow, default_action, &mut entries); + Self::lower_rule(rule, RuleAction::Allow, &mut entries, &mut remaining)?; } - entries + Ok(entries) } fn lower_rule( rule: &NetworkRule, action: RuleAction, - default_action: RuleAction, entries: &mut Vec, - ) { + remaining: &mut usize, + ) -> Result<(), String> { let matches = Self::lower_port_selectors(&rule.ports); let wildcard_peers; let peers = if rule.to.is_empty() { @@ -907,25 +927,167 @@ impl NetworkIptablesManager { }; for peer in peers { - for matching in &matches { - // `except` excludes the range from the rule rather than reversing it. - // Agreeing verdicts push nothing. - if default_action != action { - for excluded in &peer.except { - entries.push(EgressEntry { - destination: Self::cidr_destination(excluded), - action: default_action, - matching: *matching, - }); + for destination in Self::peer_destinations(peer)? { + for matching in &matches { + if *remaining == 0 { + return Err(format!( + "network.egress expands into more than {MAX_EGRESS_ENTRIES} firewall \ + rules. A rule becomes every destination block it resolves to in \ + every port it names, so narrow the peers, the exclusions, or the \ + ports." + )); } + *remaining -= 1; + entries.push(EgressEntry { + destination: destination.clone(), + action, + matching: *matching, + }); } - entries.push(EgressEntry { - destination: Self::cidr_destination(&peer.cidr), - action, - matching: *matching, - }); } } + Ok(()) + } + + /// The destinations a peer covers once its exclusions are removed. + /// + /// An `iptables` rule matches one destination block and cannot carry an + /// exclusion, so the exclusion is subtracted from the peer instead. + fn peer_destinations(peer: &NetworkPeer) -> Result, String> { + let family = match peer.cidr.address { + IpAddr::V4(_) => IpFamily::V4, + IpAddr::V6(_) => IpFamily::V6, + }; + let width = Self::family_width(family); + + // Passing an out-of-range prefix through unchanged keeps it on the path + // that reports a destination resolving to no address. + let Some(block) = Self::to_block(&peer.cidr, width) else { + return Ok(vec![Self::cidr_destination(&peer.cidr)]); + }; + + // Blocks carry no family, so an exclusion from the other one would be + // compared against the peer as a meaningless integer. + let exclusions: Vec = peer + .except + .iter() + .filter(|excluded| excluded.address.is_ipv4() == peer.cidr.address.is_ipv4()) + .filter_map(|excluded| Self::to_block(excluded, width)) + .collect(); + + let mut blocks = Vec::new(); + let mut remaining = MAX_BLOCKS_PER_PEER; + Self::subtract_blocks(block, &exclusions, width, &mut blocks, &mut remaining)?; + Ok(blocks + .into_iter() + .map(|block| Self::block_destination(block, family)) + .collect()) + } + + /// `block` minus `exclusions`, as the smallest set of whole CIDR blocks. + fn subtract_blocks( + block: DestinationBlock, + exclusions: &[DestinationBlock], + width: u8, + out: &mut Vec, + remaining: &mut usize, + ) -> Result<(), String> { + if exclusions + .iter() + .any(|excluded| Self::block_contains(*excluded, block, width)) + { + return Ok(()); + } + + if !exclusions + .iter() + .any(|excluded| Self::blocks_intersect(*excluded, block, width)) + { + if *remaining == 0 { + return Err(format!( + "a network.egress peer expands into more than {MAX_BLOCKS_PER_PEER} address \ + blocks once its 'except' entries are removed. Narrow the peer's CIDR or use \ + fewer exclusions." + )); + } + *remaining -= 1; + out.push(block); + return Ok(()); + } + + // Part of the block survives, so split it and test each half. A single + // address never reaches here: it is contained or disjoint. + if block.prefix >= width { + return Ok(()); + } + + let child_prefix = block.prefix + 1; + let step = Self::host_mask(width - child_prefix) + 1; + for base in [block.base, block.base + step] { + Self::subtract_blocks( + DestinationBlock { + base, + prefix: child_prefix, + }, + exclusions, + width, + out, + remaining, + )?; + } + Ok(()) + } + + fn family_width(family: IpFamily) -> u8 { + match family { + IpFamily::V4 => 32, + IpFamily::V6 => 128, + } + } + + /// Mask covering the low `bits` bits, saturating at the full width. + fn host_mask(bits: u8) -> u128 { + if bits >= 128 { + u128::MAX + } else { + (1u128 << bits) - 1 + } + } + + fn block_last(block: DestinationBlock, width: u8) -> u128 { + block.base | Self::host_mask(width - block.prefix) + } + + fn block_contains(outer: DestinationBlock, inner: DestinationBlock, width: u8) -> bool { + outer.base <= inner.base && Self::block_last(inner, width) <= Self::block_last(outer, width) + } + + fn blocks_intersect(left: DestinationBlock, right: DestinationBlock, width: u8) -> bool { + left.base <= Self::block_last(right, width) && right.base <= Self::block_last(left, width) + } + + /// `NetworkCidr` has public fields and validates only through `FromStr`, so + /// a programmatic request reaches here with a prefix the parser refuses. + fn to_block(cidr: &NetworkCidr, width: u8) -> Option { + if cidr.prefix_length > width { + return None; + } + let raw = match cidr.address { + IpAddr::V4(ip) => u128::from(u32::from(ip)), + IpAddr::V6(ip) => u128::from(ip), + }; + Some(DestinationBlock { + base: raw & !Self::host_mask(width - cidr.prefix_length), + prefix: cidr.prefix_length, + }) + } + + fn block_destination(block: DestinationBlock, family: IpFamily) -> String { + let address = match family { + IpFamily::V4 => IpAddr::V4(Ipv4Addr::from(block.base as u32)), + IpFamily::V6 => IpAddr::V6(Ipv6Addr::from(block.base)), + }; + format!("{}/{}", address, block.prefix) } // A v4 chain and a v6 chain are programmed separately. Neither wildcard @@ -1010,7 +1172,7 @@ impl NetworkIptablesManager { let mut args = FirewallRuleArgs::default(); let mut unresolved_denies: Vec<&str> = Vec::new(); let mut catch_all_allows: Vec<&str> = Vec::new(); - let entries = Self::lower_egress(policy, uses_directional_keys); + let entries = Self::lower_egress(policy, uses_directional_keys)?; for entry in &entries { let host = entry.destination.as_str(); let action = entry.action; diff --git a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs index b7a307490..465029e9a 100644 --- a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs +++ b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs @@ -152,6 +152,26 @@ fn new_connection_action<'a>( }) } +/// The verdict the whole chain gives a packet, including the closing default a +/// destination no rule names falls through to. +fn chain_verdict( + rules: &[Vec], + default: NetworkAction, + destination: std::net::IpAddr, + protocol: &str, + port: Option, +) -> String { + match matching_emitted_rule(rules, destination, protocol, port) + .and_then(|rule| argument_after(rule, "-j")) + { + Some(verdict) => verdict.to_string(), + None => match default { + NetworkAction::Allow => "ACCEPT".to_string(), + NetworkAction::Deny => "DROP".to_string(), + }, + } +} + #[test] fn explicit_deny_precedes_an_overlapping_allow_in_both_families() { let ipv4 = "198.51.100.0/24"; @@ -179,7 +199,7 @@ fn explicit_deny_precedes_an_overlapping_allow_in_both_families() { } #[test] -fn an_allow_peer_exclusion_is_denied_before_its_parent_cidr_is_allowed() { +fn an_allow_peer_exclusion_is_not_accepted_alongside_its_parent() { let parent = "10.0.0.0/8"; let exclusion = "10.10.0.0/16"; let policy = directional_policy( @@ -188,16 +208,29 @@ fn an_allow_peer_exclusion_is_denied_before_its_parent_cidr_is_allowed() { Vec::new(), ); let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); - let exclusion_position = rule_position(&rules.ipv4, exclusion, "DROP"); - let parent_position = rule_position(&rules.ipv4, parent, "ACCEPT"); - assert!( - matches!( - (exclusion_position, parent_position), - (Some(exclusion_position), Some(parent_position)) - if exclusion_position < parent_position + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), + "DROP", + "input=default deny, allow.to=[{{cidr:{parent}, except:[{exclusion}]}}], packet=10.10.1.1/tcp/443; the exclusion is outside the allow, so the chain's closing deny covers it; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.20.1.1"), + "tcp", + Some(443) ), - "input=default deny, allow.to=[{{cidr:{parent}, except:[{exclusion}]}}]; expected exclusion DROP before parent ACCEPT; positions={exclusion_position:?}/{parent_position:?}; output={:?}", + "ACCEPT", + "input=default deny, allow.to=[{{cidr:{parent}, except:[{exclusion}]}}], packet=10.20.1.1/tcp/443; the rest of the parent is still allowed; output={:?}", rules.ipv4 ); } @@ -214,8 +247,9 @@ fn a_deny_peer_exclusion_remains_outside_the_deny() { let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); assert_eq!( - new_connection_action( + chain_verdict( &rules.ipv4, + NetworkAction::Allow, packet_address("10.10.1.1"), "tcp", Some(443) @@ -225,8 +259,9 @@ fn a_deny_peer_exclusion_remains_outside_the_deny() { rules.ipv4 ); assert_eq!( - new_connection_action( + chain_verdict( &rules.ipv4, + NetworkAction::Allow, packet_address("10.20.1.1"), "tcp", Some(443) @@ -249,7 +284,13 @@ fn an_exclusion_inside_an_allow_rule_under_an_allow_default_stays_reachable() { let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); assert_eq!( - new_connection_action(&rules.ipv4, packet_address("10.10.1.1"), "tcp", Some(443)), + chain_verdict( + &rules.ipv4, + NetworkAction::Allow, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), "ACCEPT", "input=default allow, allow.to=[{{cidr:{parent}, except:[{exclusion}]}}], packet=10.10.1.1/tcp/443; an exclusion narrows its own rule and never reverses the direction default; output={:?}", rules.ipv4 @@ -274,7 +315,13 @@ fn an_exclusion_inside_a_deny_rule_under_a_deny_default_stays_blocked() { let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); assert_eq!( - new_connection_action(&rules.ipv4, packet_address("10.10.1.1"), "tcp", Some(443)), + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), "DROP", "input=default deny, deny.to=[{{cidr:{parent}, except:[{exclusion}]}}], packet=10.10.1.1/tcp/443; an exclusion narrows its own rule and never reverses the direction default; output={:?}", rules.ipv4 @@ -836,3 +883,194 @@ fn a_parsed_v08_request_without_a_network_section_drops_the_dns_exemption() { "input=0.8 with no network section; expected no chain at all, so no rule can open DNS" ); } +#[test] +fn an_exclusion_does_not_shadow_a_later_rule_that_names_it() { + let policy = directional_policy( + NetworkAction::Allow, + Vec::new(), + vec![ + rule(vec![peer("10.0.0.0/8", &["10.10.0.0/16"])], Vec::new()), + rule(vec![peer("10.10.1.0/24", &[])], Vec::new()), + ], + ); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Allow, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), + "DROP", + "input=default allow, deny.to=[{{cidr:10.0.0.0/8, except:[10.10.0.0/16]}}] then deny.to=[{{cidr:10.10.1.0/24}}], packet=10.10.1.1/tcp/443; the second rule denies this address outright, so the first rule's exclusion must not accept it first; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Allow, + packet_address("10.10.2.1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=as above, packet=10.10.2.1/tcp/443; this address is excluded from the deny and named by no later rule, so the default still reaches it; output={:?}", + rules.ipv4 + ); +} + +#[test] +fn the_emitted_blocks_cover_the_peer_without_its_exclusion() { + let policy = directional_policy( + NetworkAction::Deny, + vec![rule( + vec![peer("10.0.0.0/8", &["10.10.0.0/16"])], + Vec::new(), + )], + Vec::new(), + ); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + for reachable in ["10.0.0.1", "10.9.255.255", "10.11.0.0", "10.255.255.255"] { + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address(reachable), + "tcp", + Some(443) + ), + "ACCEPT", + "input=allow.to=[{{cidr:10.0.0.0/8, except:[10.10.0.0/16]}}], packet={reachable}; inside the peer and outside the exclusion; output={:?}", + rules.ipv4 + ); + } + + for excluded in ["10.10.0.0", "10.10.255.255"] { + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address(excluded), + "tcp", + Some(443) + ), + "DROP", + "input=allow.to=[{{cidr:10.0.0.0/8, except:[10.10.0.0/16]}}], packet={excluded}; inside the exclusion; output={:?}", + rules.ipv4 + ); + } +} + +#[test] +fn an_ipv6_exclusion_is_subtracted_within_its_own_family() { + let policy = directional_policy( + NetworkAction::Deny, + vec![rule( + vec![peer("2001:db8::/32", &["2001:db8:1::/48"])], + Vec::new(), + )], + Vec::new(), + ); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert_eq!( + chain_verdict( + &rules.ipv6, + NetworkAction::Deny, + packet_address("2001:db8:2::1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=allow.to=[{{cidr:2001:db8::/32, except:[2001:db8:1::/48]}}], packet=2001:db8:2::1; output={:?}", + rules.ipv6 + ); + assert_eq!( + chain_verdict( + &rules.ipv6, + NetworkAction::Deny, + packet_address("2001:db8:1::1"), + "tcp", + Some(443) + ), + "DROP", + "input=allow.to=[{{cidr:2001:db8::/32, except:[2001:db8:1::/48]}}], packet=2001:db8:1::1; output={:?}", + rules.ipv6 + ); + assert!( + rules.ipv4.is_empty(), + "an IPv6 peer must program no IPv4 rule; output={:?}", + rules.ipv4 + ); +} + +#[test] +fn a_peer_that_expands_past_the_block_ceiling_is_refused() { + // Each /32 removed from a /8 splits the surrounding space 24 times, so a + // handful of scattered exclusions outgrows the per-peer ceiling. + let exclusions: Vec = (0..32).map(|n| format!("10.{n}.0.1/32")).collect(); + let borrowed: Vec<&str> = exclusions.iter().map(String::as_str).collect(); + let policy = directional_policy( + NetworkAction::Deny, + vec![rule(vec![peer("10.0.0.0/8", &borrowed)], Vec::new())], + Vec::new(), + ); + let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + + let error = + NetworkIptablesManager::build_policy_rules_logged("MXC-test", &policy, true, &mut logger) + .expect_err( + "a peer expanding past the ceiling must be refused rather than partly installed", + ); + + assert!( + error.contains("except"), + "expected the refusal to name the exclusions that caused the expansion, got: {error}" + ); +} + +#[test] +fn an_ipv4_mapped_peer_still_reaches_the_ipv4_chain_after_subtraction() { + let policy = directional_policy( + NetworkAction::Deny, + vec![rule( + vec![peer("::ffff:10.0.0.0/104", &["::ffff:10.10.0.0/112"])], + Vec::new(), + )], + Vec::new(), + ); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.11.0.1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=allow.to=[{{cidr:::ffff:10.0.0.0/104, except:[::ffff:10.10.0.0/112]}}], packet=10.11.0.1; a mapped peer is programmed on the IPv4 chain; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.0.1"), + "tcp", + Some(443) + ), + "DROP", + "input=as above, packet=10.10.0.1; inside the exclusion; output={:?}", + rules.ipv4 + ); + assert!( + rules.ipv6.is_empty(), + "a mapped peer must program no IPv6 rule; output={:?}", + rules.ipv6 + ); +} diff --git a/src/core/wxc_common/src/config_parser.rs b/src/core/wxc_common/src/config_parser.rs index 7b09d9982..2c3dbddb5 100644 --- a/src/core/wxc_common/src/config_parser.rs +++ b/src/core/wxc_common/src/config_parser.rs @@ -4756,7 +4756,7 @@ mod tests { observed_counts, expected_counts, "explicit divergence inventory and observed category totals differ" ); - let expected_inventory = (368, 344, 14); + let expected_inventory = (370, 346, 14); assert_eq!( (files.len(), equivalent_accepts, shared_rejections), expected_inventory, diff --git a/tests/configs/lxc_network_ga_egress_except_shadow.json b/tests/configs/lxc_network_ga_egress_except_shadow.json new file mode 100644 index 000000000..97513fdb7 --- /dev/null +++ b/tests/configs/lxc_network_ga_egress_except_shadow.json @@ -0,0 +1,30 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-LXC-GA-Egress-Except-Shadow", + "containment": "lxc", + "process": { + "commandLine": "sh -c \"wget -qO- --timeout=10 http://203.0.113.2:443/ >/dev/null 2>&1 && echo MXC_NET_ALLOWED || echo MXC_NET_BLOCKED\"" + }, + "lifecycle": { + "destroyOnExit": true + }, + "lxc": { + "distribution": "alpine", + "release": "3.23" + }, + "network": { + "egress": { + "default": "allow", + "deny": [ + { + "to": [{ "cidr": "203.0.113.0/24", "except": ["203.0.113.2/32"] }], + "ports": [{ "protocol": "tcp", "port": 443 }] + }, + { + "to": [{ "cidr": "203.0.113.2/32" }], + "ports": [{ "protocol": "tcp", "port": 443 }] + } + ] + } + } +} diff --git a/tests/configs/lxc_network_ga_egress_except_shadow_control.json b/tests/configs/lxc_network_ga_egress_except_shadow_control.json new file mode 100644 index 000000000..16a2211c5 --- /dev/null +++ b/tests/configs/lxc_network_ga_egress_except_shadow_control.json @@ -0,0 +1,26 @@ +{ + "version": "0.8.0-alpha", + "containerId": "CLI-LXC-GA-Egress-Except-Shadow-Control", + "containment": "lxc", + "process": { + "commandLine": "sh -c \"wget -qO- --timeout=10 http://203.0.113.2:443/ >/dev/null 2>&1 && echo MXC_NET_ALLOWED || echo MXC_NET_BLOCKED\"" + }, + "lifecycle": { + "destroyOnExit": true + }, + "lxc": { + "distribution": "alpine", + "release": "3.23" + }, + "network": { + "egress": { + "default": "allow", + "deny": [ + { + "to": [{ "cidr": "203.0.113.0/24", "except": ["203.0.113.2/32"] }], + "ports": [{ "protocol": "tcp", "port": 443 }] + } + ] + } + } +} diff --git a/tests/scripts/run_lxc_network_ga_egress_test.sh b/tests/scripts/run_lxc_network_ga_egress_test.sh index 67d1c3425..ada59feeb 100644 --- a/tests/scripts/run_lxc_network_ga_egress_test.sh +++ b/tests/scripts/run_lxc_network_ga_egress_test.sh @@ -42,6 +42,8 @@ DNS_ALLOWED_CONFIG="$REPO_DIR/tests/configs/lxc_network_ga_egress_dns_allowed.js DENY_RULE_CONFIG="$REPO_DIR/tests/configs/lxc_network_ga_egress_deny_rule.json" EXCEPT_EXCLUDED_CONFIG="$REPO_DIR/tests/configs/lxc_network_ga_egress_except_excluded.json" EXCEPT_SIBLING_CONFIG="$REPO_DIR/tests/configs/lxc_network_ga_egress_except_sibling.json" +EXCEPT_SHADOW_CONFIG="$REPO_DIR/tests/configs/lxc_network_ga_egress_except_shadow.json" +EXCEPT_SHADOW_CONTROL_CONFIG="$REPO_DIR/tests/configs/lxc_network_ga_egress_except_shadow_control.json" fail() { echo "FAIL: $1" @@ -223,11 +225,13 @@ PY # Drift guard: the tcp/443 fixtures must target this peer, or the run would # probe a stale address and prove nothing. -for cfg in "$DENY_CONFIG" "$ALLOW_CONFIG" "$WRONG_PORT_CONFIG"; do +for cfg in "$DENY_CONFIG" "$ALLOW_CONFIG" "$WRONG_PORT_CONFIG" "$EXCEPT_SHADOW_CONFIG" \ + "$EXCEPT_SHADOW_CONTROL_CONFIG"; do grep -Fq "$PEER_IP" "$cfg" \ || fail "fixture ${cfg##*/} no longer targets the peer $PEER_IP; script and fixture drifted." done -for cfg in "$ALLOW_CONFIG" "$WRONG_PORT_CONFIG"; do +for cfg in "$ALLOW_CONFIG" "$WRONG_PORT_CONFIG" "$EXCEPT_SHADOW_CONFIG" \ + "$EXCEPT_SHADOW_CONTROL_CONFIG"; do grep -Fq "$PEER_CIDR" "$cfg" \ || fail "fixture ${cfg##*/} no longer allows $PEER_CIDR; script and fixture drifted." done @@ -261,5 +265,13 @@ assert_blocked "an address named in except was reachable through the rule that e run_case "except case: same policy, probe an address the exclusion does not cover" "$EXCEPT_SIBLING_CONFIG" assert_allowed "an address inside the allowed range but outside except was unreachable. The exclusion is over-blocking, so the case above proves only that the whole rule failed to install." -echo "PASS: schema 0.8 egress rules filtered by destination, by port, by resolver, by deny rule, and by exclusion." +# The chain is first-match-wins, so a carve-out programmed as its own accept +# rule would answer for the peer before the second rule's deny is reached. +run_case "shadow case: deny the peer's range except the peer, then deny the peer outright" "$EXCEPT_SHADOW_CONFIG" +assert_blocked "a destination denied by its own rule was reachable because an earlier rule excluded it. An except carve-out is escaping the rule that declared it and accepting traffic a later deny names, which turns a deny into an allow." + +run_case "shadow-control case: the same first rule with no second deny" "$EXCEPT_SHADOW_CONTROL_CONFIG" +assert_allowed "an address excluded from a deny was unreachable under egress.default allow. The exclusion is not narrowing its own rule, so the shadow case above proves only that everything was blocked." + +echo "PASS: schema 0.8 egress rules filtered by destination, by port, by resolver, by deny rule, and by exclusion, and no exclusion answered for a destination a later rule denied." echo "LXC schema 0.8 egress enforcement test complete." From 2c68844a431a17011b31eb8c83a143192cd96b87 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 18 Sep 2026 14:11:42 -0700 Subject: [PATCH 2/5] [LXC] [Docs] State the egress expansion limits the Linux backends enforce Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/sandbox-policy/0.8.0/networking/networking.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/sandbox-policy/0.8.0/networking/networking.md b/docs/sandbox-policy/0.8.0/networking/networking.md index d8bcdcfaa..0d606cd19 100644 --- a/docs/sandbox-policy/0.8.0/networking/networking.md +++ b/docs/sandbox-policy/0.8.0/networking/networking.md @@ -226,7 +226,7 @@ Egress peer and port fields (used in `egress.allow[]` / `egress.deny[]`; not sho | Field | Type | Notes | |---|---|---| | `to[].cidr` | IPv4 / IPv6 CIDR, or 0.0.0.0/0 / ::/0 for any | Single CIDR string (CNI/Kubernetes style), replacing separate address + prefix length. | -| `to[].except` | list of CIDRs, optional | Exclusions within the peer's CIDR (Kubernetes `ipBlock.except` style). An exclusion narrows the rule that carries it and nothing else: it never states a verdict of its own, so an address it removes is decided by the remaining rules and the direction default. Windows process containers pass the exclusion to the platform (WFP); the Linux backends subtract it from the peer and program the covering blocks that remain, because an `iptables` rule cannot carry an exclusion and a separate rule would leak into later ones. Not supported on Seatbelt (no destination filtering). | +| `to[].except` | list of CIDRs, optional | Exclusions within the peer's CIDR (Kubernetes `ipBlock.except` style). An exclusion narrows the rule that carries it and nothing else: it never states a verdict of its own, so an address it removes is decided by the remaining rules and the direction default. Windows process containers pass the exclusion to the platform (WFP); the Linux backends subtract it from the peer and program the covering blocks that remain, because an `iptables` rule cannot carry an exclusion and a separate rule would leak into later ones. The Linux backends bound the resulting expansion; the GA Scope by Backend section states the limits. Not supported on Seatbelt (no destination filtering). | | `ports[].protocol` | tcp / udp / icmp / any | `any` matches at minimum TCP, UDP, and ICMPv4/6; a backend may match more. Enforced on Windows process containers (WFP) and the Linux backends (iptables); not supported on Seatbelt. | | `ports[].port` | uint16, optional | Destination port. Omit `ports` to match all ports/protocols. | | `ports[].endPort` | uint16, optional | End of a port range (Kubernetes `endPort` style); requires numeric port. Supported on Windows process containers (WFP) and the Linux backends (iptables); not supported on Seatbelt. | @@ -474,6 +474,12 @@ LXC and Bubblewrap use iptables/nftables on the container network path. Their IN and the host-to-container half of `ingress.hostLoopback`; routing and output policy enforce its container-to-host half. Model 2 permits only the proxy endpoint. +Both backends subtract `to[].except` from the peer that carries it and program the covering blocks that remain, so one +peer can expand into many rules. Two ceilings bound that expansion: a single peer may expand into at most **256 address +blocks** once its exclusions are removed, and one `egress` policy may lower into at most **65,536 rules** in total, +counting every destination block in every port each rule names. A policy exceeding either ceiling is rejected with the +limit it hit; neither is silently truncated, and no partial policy is installed. + > **Implementation status (Bubblewrap).** Egress is enforced from schema 0.8+, > but not on the path described above. Unprivileged Bubblewrap has no host-side > veth, so no chain can be hooked into `FORWARD`. Instead the sandbox gets its From f8fb80f99bf22a4dfa8540bafd7c9133df5c0c85 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 18 Sep 2026 15:39:45 -0700 Subject: [PATCH 3/5] [LXC] Refuse egress exclusions that cross address families or cannot be lowered, before a container exists Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/backends/lxc/common/src/lxc_runner.rs | 61 +++++++++ .../lxc/common/src/network_iptables.rs | 55 ++++++++ .../src/network_iptables_ga_egress_spec.rs | 128 ++++++++++++++++++ 3 files changed, 244 insertions(+) diff --git a/src/backends/lxc/common/src/lxc_runner.rs b/src/backends/lxc/common/src/lxc_runner.rs index 4e7ad354b..a21ccbc1f 100644 --- a/src/backends/lxc/common/src/lxc_runner.rs +++ b/src/backends/lxc/common/src/lxc_runner.rs @@ -298,6 +298,16 @@ impl LxcScriptRunner { } } + // The policy lowers to the same rules with or without a container, so + // one that cannot be programmed is refused before a container exists + // rather than after it has been created, started, and torn down again. + if let Err(msg) = NetworkIptablesManager::validate_egress_lowering( + &request.policy, + uses_directional_keys(&request.policy), + ) { + return ScriptResponse::error(&msg); + } + if self.destroy_on_exit { signal_cleanup::set_active(&container_name); } @@ -1423,6 +1433,57 @@ mod tests { "a reused container must be destroyed on readiness timeout when destroyOnExit=true" ); } + + /// A policy whose `except` entry is malformed, so lowering refuses it. + fn request_with_unlowerable_egress() -> ExecutionRequest { + use wxc_common::models::{NetworkAction, NetworkCidr, NetworkPeer, NetworkRule}; + + let mut request = ExecutionRequest::default(); + request.policy.network_egress = Some(NetworkEgressPolicy { + default: NetworkAction::Deny, + allow: vec![NetworkRule { + to: vec![NetworkPeer { + cidr: NetworkCidr { + address: "10.0.0.0".parse().expect("literal"), + prefix_length: 8, + }, + except: vec![NetworkCidr { + address: "10.10.0.0".parse().expect("literal"), + prefix_length: 40, + }], + }], + ports: Vec::new(), + }], + deny: Vec::new(), + }); + request + } + + #[test] + fn an_egress_policy_that_cannot_be_lowered_is_refused_before_a_container_exists() { + // Reaching LXC would fail on a host without it, so an error carrying + // the lowering's own words is what shows the refusal came first. + let mut logger = Logger::new(Mode::Buffer); + + let response = + runner_for_guard_tests().run_internal(&request_with_unlowerable_egress(), &mut logger); + + assert_ne!( + response.exit_code, 0, + "input=allow.to=[{{cidr:10.0.0.0/8, except:[10.10.0.0/40]}}]; expected a refusal; output={response:?}" + ); + assert!( + response + .error_message + .contains("wider than its address family"), + "expected the lowering's refusal rather than a container failure, got: {response:?}" + ); + assert!( + !logger.get_buffer().contains("Creating LXC container"), + "the refusal must land before the container is created; log={}", + logger.get_buffer() + ); + } } #[cfg(all(test, unix))] diff --git a/src/backends/lxc/common/src/network_iptables.rs b/src/backends/lxc/common/src/network_iptables.rs index c845d97a8..8d39a2213 100644 --- a/src/backends/lxc/common/src/network_iptables.rs +++ b/src/backends/lxc/common/src/network_iptables.rs @@ -109,6 +109,13 @@ struct DestinationBlock { prefix: u8, } +/// `::ffff:0:0/96`, the range a destination is rewritten out of and programmed +/// as IPv4 in. +const IPV4_MAPPED_BLOCK: DestinationBlock = DestinationBlock { + base: 0xffff_0000_0000, + prefix: 96, +}; + /// Ceiling on the blocks one peer may expand into. /// /// Subtracting an exclusion splits the surrounding block once per prefix level, @@ -865,6 +872,14 @@ impl NetworkIptablesManager { } } + /// Lower the egress policy for its refusals alone, discarding the rules. + pub(crate) fn validate_egress_lowering( + policy: &ContainerPolicy, + uses_directional_keys: bool, + ) -> Result<(), String> { + Self::lower_egress(policy, uses_directional_keys).map(|_| ()) + } + fn lower_egress( policy: &ContainerPolicy, uses_directional_keys: bool, @@ -966,6 +981,25 @@ impl NetworkIptablesManager { return Ok(vec![Self::cidr_destination(&peer.cidr)]); }; + // Checked against each entry's own family, before the peer's family + // selects which ones apply: dropping a malformed exclusion would widen + // the rule it was written to narrow. + for excluded in &peer.except { + let excluded_width = Self::family_width(match excluded.address { + IpAddr::V4(_) => IpFamily::V4, + IpAddr::V6(_) => IpFamily::V6, + }); + if excluded.prefix_length > excluded_width { + return Err(format!( + "network.egress peer '{}' carries the 'except' entry '{}/{}', whose prefix \ + is wider than its address family (max /{excluded_width}).", + Self::cidr_destination(&peer.cidr), + excluded.address, + excluded.prefix_length + )); + } + } + // Blocks carry no family, so an exclusion from the other one would be // compared against the peer as a meaningless integer. let exclusions: Vec = peer @@ -978,6 +1012,27 @@ impl NetworkIptablesManager { let mut blocks = Vec::new(); let mut remaining = MAX_BLOCKS_PER_PEER; Self::subtract_blocks(block, &exclusions, width, &mut blocks, &mut remaining)?; + + // A destination inside `::ffff:0:0/96` is programmed as IPv4. A peer + // inside that range resolves to IPv4 as a whole, which is the caller's + // own writing; a peer outside it must not have subtraction hand back a + // piece that crosses over and opens IPv4 the peer never named. + if family == IpFamily::V6 && !Self::block_contains(IPV4_MAPPED_BLOCK, block, width) { + if let Some(crossing) = blocks + .iter() + .find(|candidate| Self::block_contains(IPV4_MAPPED_BLOCK, **candidate, width)) + { + return Err(format!( + "network.egress peer '{}' cannot carry an 'except' that splits the \ + IPv4-mapped range: removing it leaves '{}', which is programmed as IPv4 \ + and would open addresses this IPv6 peer never named. State the IPv4 \ + range as its own peer instead.", + Self::cidr_destination(&peer.cidr), + Self::block_destination(*crossing, family) + )); + } + } + Ok(blocks .into_iter() .map(|block| Self::block_destination(block, family)) diff --git a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs index 004a73976..3e7e65105 100644 --- a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs +++ b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs @@ -1137,3 +1137,131 @@ fn an_unresolvable_directional_deny_is_tolerated_under_a_deny_egress_default() { "input=egress.default=deny with an unresolvable deny, legacy default_network_policy=allow; expected no refusal, since the closing DROP already covers what the deny could not program", ); } + +fn lowering_error(policy: &ContainerPolicy) -> String { + let mut logger = Logger::new(wxc_common::logger::Mode::Buffer); + NetworkIptablesManager::build_policy_rules_logged("MXC-test", policy, true, &mut logger) + .expect_err("expected the policy to be refused") +} + +fn allow_peer_policy(peers: Vec) -> ContainerPolicy { + directional_policy( + NetworkAction::Deny, + vec![rule(peers, Vec::new())], + Vec::new(), + ) +} + +#[test] +fn an_ipv6_peer_whose_exclusion_splits_the_mapped_range_is_refused() { + let policy = allow_peer_policy(vec![peer("::/0", &["::ffff:10.0.0.0/104"])]); + let error = lowering_error(&policy); + + assert!( + error.contains("IPv4-mapped"), + "input=allow.to=[{{cidr:::/0, except:[::ffff:10.0.0.0/104]}}]; subtracting inside the mapped range leaves blocks that are programmed as IPv4, so the IPv6 peer would open IPv4 it never named; got: {error}" + ); +} + +#[test] +fn an_ipv6_peer_whose_exclusion_neighbours_the_mapped_range_is_refused() { + // The exclusion is outside the mapped range, but removing it still splits + // `::/0` down to a `::ffff:0:0/96` sibling, which renders as `0.0.0.0/0`. + let policy = allow_peer_policy(vec![peer("::/0", &["::fffe:0:0/96"])]); + let error = lowering_error(&policy); + + assert!( + error.contains("IPv4-mapped"), + "input=allow.to=[{{cidr:::/0, except:[::fffe:0:0/96]}}]; an exclusion beside the mapped range still yields the whole mapped block, which is programmed as 0.0.0.0/0; got: {error}" + ); +} + +#[test] +fn an_ipv6_peer_far_from_the_mapped_range_still_subtracts() { + let policy = allow_peer_policy(vec![peer("2001:db8::/32", &["2001:db8:1::/48"])]); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert!( + rules.ipv4.is_empty(), + "input=allow.to=[{{cidr:2001:db8::/32, except:[2001:db8:1::/48]}}]; no block is inside the mapped range, so nothing may reach the IPv4 chain; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv6, + NetworkAction::Deny, + packet_address("2001:db8:2::1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=as above, packet=2001:db8:2::1; the guard must not refuse an ordinary IPv6 subtraction; output={:?}", + rules.ipv6 + ); +} + +#[test] +fn an_ipv6_catch_all_without_an_exclusion_is_untouched_by_the_guard() { + let policy = allow_peer_policy(vec![peer("::/0", &[])]); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert!( + rules.ipv4.is_empty(), + "input=allow.to=[{{cidr:::/0}}]; a wildcard with no exclusion emits one IPv6 block and must not reach the IPv4 chain; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv6, + NetworkAction::Deny, + packet_address("2001:db8::1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=as above; the guard must not refuse a bare IPv6 wildcard; output={:?}", + rules.ipv6 + ); +} + +#[test] +fn an_exclusion_with_a_prefix_past_its_family_is_refused() { + let policy = allow_peer_policy(vec![NetworkPeer { + cidr: NetworkCidr { + address: packet_address("10.0.0.0"), + prefix_length: 8, + }, + except: vec![NetworkCidr { + address: packet_address("10.10.0.0"), + prefix_length: 40, + }], + }]); + let error = lowering_error(&policy); + + assert!( + error.contains("wider than its address family"), + "input=allow.to=[{{cidr:10.0.0.0/8, except:[10.10.0.0/40]}}]; discarding the malformed exclusion would accept the whole /8 it was written to narrow; got: {error}" + ); +} + +#[test] +fn an_other_family_exclusion_is_validated_before_it_is_filtered_out() { + // The peer's family drops this entry, so an unvalidated prefix would never + // be seen at all. + let policy = allow_peer_policy(vec![NetworkPeer { + cidr: NetworkCidr { + address: packet_address("10.0.0.0"), + prefix_length: 8, + }, + except: vec![NetworkCidr { + address: packet_address("2001:db8::"), + prefix_length: 200, + }], + }]); + let error = lowering_error(&policy); + + assert!( + error.contains("wider than its address family"), + "input=allow.to=[{{cidr:10.0.0.0/8, except:[2001:db8::/200]}}]; got: {error}" + ); +} From 6f71578effa93422842e26ae3bc83f4b8a49ca29 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Mon, 21 Sep 2026 14:29:18 -0700 Subject: [PATCH 4/5] [LXC] Match an egress exclusion to its peer by programmed family, so IPv4-mapped notation still narrows the rule Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../lxc/common/src/network_iptables.rs | 72 ++++++++----- .../src/network_iptables_ga_egress_spec.rs | 102 +++++++++++++++++- 2 files changed, 142 insertions(+), 32 deletions(-) diff --git a/src/backends/lxc/common/src/network_iptables.rs b/src/backends/lxc/common/src/network_iptables.rs index 8d39a2213..fc4fdb2d7 100644 --- a/src/backends/lxc/common/src/network_iptables.rs +++ b/src/backends/lxc/common/src/network_iptables.rs @@ -969,18 +969,6 @@ impl NetworkIptablesManager { /// An `iptables` rule matches one destination block and cannot carry an /// exclusion, so the exclusion is subtracted from the peer instead. fn peer_destinations(peer: &NetworkPeer) -> Result, String> { - let family = match peer.cidr.address { - IpAddr::V4(_) => IpFamily::V4, - IpAddr::V6(_) => IpFamily::V6, - }; - let width = Self::family_width(family); - - // Passing an out-of-range prefix through unchanged keeps it on the path - // that reports a destination resolving to no address. - let Some(block) = Self::to_block(&peer.cidr, width) else { - return Ok(vec![Self::cidr_destination(&peer.cidr)]); - }; - // Checked against each entry's own family, before the peer's family // selects which ones apply: dropping a malformed exclusion would widen // the rule it was written to narrow. @@ -1000,13 +988,23 @@ impl NetworkIptablesManager { } } - // Blocks carry no family, so an exclusion from the other one would be - // compared against the peer as a meaningless integer. + // Passing an out-of-range prefix through unchanged keeps it on the path + // that reports a destination resolving to no address. + let Some((family, block)) = Self::resolve_block(&peer.cidr) else { + return Ok(vec![Self::cidr_destination(&peer.cidr)]); + }; + let width = Self::family_width(family); + + // Compared after the mapped rewriting, so an exclusion written in the + // other notation for the same addresses still narrows its peer. An + // exclusion in the other family cannot overlap the peer at all, so it + // is dropped rather than compared as a meaningless integer. let exclusions: Vec = peer .except .iter() - .filter(|excluded| excluded.address.is_ipv4() == peer.cidr.address.is_ipv4()) - .filter_map(|excluded| Self::to_block(excluded, width)) + .filter_map(Self::resolve_block) + .filter(|(excluded_family, _)| *excluded_family == family) + .map(|(_, block)| block) .collect(); let mut blocks = Vec::new(); @@ -1121,20 +1119,38 @@ impl NetworkIptablesManager { left.base <= Self::block_last(right, width) && right.base <= Self::block_last(left, width) } - /// `NetworkCidr` has public fields and validates only through `FromStr`, so - /// a programmatic request reaches here with a prefix the parser refuses. - fn to_block(cidr: &NetworkCidr, width: u8) -> Option { - if cidr.prefix_length > width { + /// A CIDR as the family it is programmed in and the block it covers there. + /// + /// An IPv4-mapped address is programmed as IPv4, because Linux puts a + /// genuine IPv4 packet on the wire for one. `NetworkCidr` has public fields + /// and validates only through `FromStr`, so a programmatic request reaches + /// here with a prefix the parser refuses. + fn resolve_block(cidr: &NetworkCidr) -> Option<(IpFamily, DestinationBlock)> { + let (family, raw, prefix) = match cidr.address { + IpAddr::V4(ip) => (IpFamily::V4, u128::from(u32::from(ip)), cidr.prefix_length), + IpAddr::V6(ip) => match ip.to_ipv4_mapped() { + // Below /96 the block reaches outside the mapped range, so it + // stays IPv6 and keeps covering what it actually names. + Some(v4) if cidr.prefix_length >= 96 => ( + IpFamily::V4, + u128::from(u32::from(v4)), + cidr.prefix_length - 96, + ), + _ => (IpFamily::V6, u128::from(ip), cidr.prefix_length), + }, + }; + + let width = Self::family_width(family); + if prefix > width { return None; } - let raw = match cidr.address { - IpAddr::V4(ip) => u128::from(u32::from(ip)), - IpAddr::V6(ip) => u128::from(ip), - }; - Some(DestinationBlock { - base: raw & !Self::host_mask(width - cidr.prefix_length), - prefix: cidr.prefix_length, - }) + Some(( + family, + DestinationBlock { + base: raw & !Self::host_mask(width - prefix), + prefix, + }, + )) } fn block_destination(block: DestinationBlock, family: IpFamily) -> String { diff --git a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs index 3e7e65105..5c1a71b50 100644 --- a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs +++ b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs @@ -1153,13 +1153,107 @@ fn allow_peer_policy(peers: Vec) -> ContainerPolicy { } #[test] -fn an_ipv6_peer_whose_exclusion_splits_the_mapped_range_is_refused() { +fn an_ipv6_peer_whose_exclusion_splits_the_mapped_range_opens_no_ipv4() { let policy = allow_peer_policy(vec![peer("::/0", &["::ffff:10.0.0.0/104"])]); - let error = lowering_error(&policy); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); assert!( - error.contains("IPv4-mapped"), - "input=allow.to=[{{cidr:::/0, except:[::ffff:10.0.0.0/104]}}]; subtracting inside the mapped range leaves blocks that are programmed as IPv4, so the IPv6 peer would open IPv4 it never named; got: {error}" + rules.ipv4.is_empty(), + "input=allow.to=[{{cidr:::/0, except:[::ffff:10.0.0.0/104]}}]; the exclusion names mapped addresses, which are programmed as IPv4, so this IPv6 allow must reach the IPv4 chain with nothing; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), + "DROP", + "input=as above, packet=10.10.1.1; the direction default still covers IPv4; output={:?}", + rules.ipv4 + ); +} + +#[test] +fn an_ipv4_peer_subtracts_an_exclusion_written_in_mapped_notation() { + let policy = allow_peer_policy(vec![peer("10.0.0.0/8", &["::ffff:10.10.0.0/112"])]); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), + "DROP", + "input=allow.to=[{{cidr:10.0.0.0/8, except:[::ffff:10.10.0.0/112]}}], packet=10.10.1.1; the exclusion names 10.10.0.0/16 in mapped notation, so it must narrow the peer rather than be discarded; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.11.1.1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=as above, packet=10.11.1.1; the rest of the peer is still allowed; output={:?}", + rules.ipv4 + ); +} + +#[test] +fn a_mapped_peer_subtracts_an_exclusion_written_in_plain_ipv4() { + let policy = allow_peer_policy(vec![peer("::ffff:10.0.0.0/104", &["10.10.0.0/16"])]); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), + "DROP", + "input=allow.to=[{{cidr:::ffff:10.0.0.0/104, except:[10.10.0.0/16]}}], packet=10.10.1.1; the peer is programmed as IPv4, so a plain IPv4 exclusion must narrow it; output={:?}", + rules.ipv4 + ); + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.11.1.1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=as above, packet=10.11.1.1; the rest of the peer is still allowed; output={:?}", + rules.ipv4 + ); +} + +#[test] +fn an_exclusion_that_cannot_overlap_its_peer_is_dropped_rather_than_applied() { + let policy = allow_peer_policy(vec![peer("10.0.0.0/8", &["2001:db8::/32"])]); + let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + + assert_eq!( + chain_verdict( + &rules.ipv4, + NetworkAction::Deny, + packet_address("10.10.1.1"), + "tcp", + Some(443) + ), + "ACCEPT", + "input=allow.to=[{{cidr:10.0.0.0/8, except:[2001:db8::/32]}}], packet=10.10.1.1; an IPv6 range cannot lie inside an IPv4 peer, so the peer is unnarrowed rather than refused; output={:?}", + rules.ipv4 ); } From 800aa76b82760fb17ab5f2fa7cc4e47b7940b5a0 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 22 Sep 2026 09:37:42 -0700 Subject: [PATCH 5/5] Addressed pr comment Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/backends/lxc/common/src/lxc_runner.rs | 5 +- .../lxc/common/src/network_iptables.rs | 46 ++++++++--------- .../src/network_iptables_ga_egress_spec.rs | 50 ++++++++----------- 3 files changed, 44 insertions(+), 57 deletions(-) diff --git a/src/backends/lxc/common/src/lxc_runner.rs b/src/backends/lxc/common/src/lxc_runner.rs index a21ccbc1f..c21221b43 100644 --- a/src/backends/lxc/common/src/lxc_runner.rs +++ b/src/backends/lxc/common/src/lxc_runner.rs @@ -299,8 +299,7 @@ impl LxcScriptRunner { } // The policy lowers to the same rules with or without a container, so - // one that cannot be programmed is refused before a container exists - // rather than after it has been created, started, and torn down again. + // one that cannot be programmed is refused before a container exists. if let Err(msg) = NetworkIptablesManager::validate_egress_lowering( &request.policy, uses_directional_keys(&request.policy), @@ -1461,8 +1460,6 @@ mod tests { #[test] fn an_egress_policy_that_cannot_be_lowered_is_refused_before_a_container_exists() { - // Reaching LXC would fail on a host without it, so an error carrying - // the lowering's own words is what shows the refusal came first. let mut logger = Logger::new(Mode::Buffer); let response = diff --git a/src/backends/lxc/common/src/network_iptables.rs b/src/backends/lxc/common/src/network_iptables.rs index fc4fdb2d7..99d67bfe9 100644 --- a/src/backends/lxc/common/src/network_iptables.rs +++ b/src/backends/lxc/common/src/network_iptables.rs @@ -969,9 +969,8 @@ impl NetworkIptablesManager { /// An `iptables` rule matches one destination block and cannot carry an /// exclusion, so the exclusion is subtracted from the peer instead. fn peer_destinations(peer: &NetworkPeer) -> Result, String> { - // Checked against each entry's own family, before the peer's family - // selects which ones apply: dropping a malformed exclusion would widen - // the rule it was written to narrow. + // Run before the peer resolves, so a malformed exclusion is refused + // even when its peer names no address. for excluded in &peer.except { let excluded_width = Self::family_width(match excluded.address { IpAddr::V4(_) => IpFamily::V4, @@ -995,26 +994,32 @@ impl NetworkIptablesManager { }; let width = Self::family_width(family); - // Compared after the mapped rewriting, so an exclusion written in the - // other notation for the same addresses still narrows its peer. An - // exclusion in the other family cannot overlap the peer at all, so it - // is dropped rather than compared as a meaningless integer. - let exclusions: Vec = peer - .except - .iter() - .filter_map(Self::resolve_block) - .filter(|(excluded_family, _)| *excluded_family == family) - .map(|(_, block)| block) - .collect(); + let mut exclusions = Vec::new(); + for excluded in &peer.except { + // An out-of-range prefix is refused above, so every entry resolves. + let Some((excluded_family, excluded_block)) = Self::resolve_block(excluded) else { + continue; + }; + if excluded_family != family { + return Err(format!( + "network.egress peer '{}' carries the 'except' entry '{}/{}', which is \ + programmed for the other address family and so names no address the peer \ + covers. State an exclusion inside the peer's own range.", + Self::cidr_destination(&peer.cidr), + excluded.address, + excluded.prefix_length + )); + } + exclusions.push(excluded_block); + } let mut blocks = Vec::new(); let mut remaining = MAX_BLOCKS_PER_PEER; Self::subtract_blocks(block, &exclusions, width, &mut blocks, &mut remaining)?; - // A destination inside `::ffff:0:0/96` is programmed as IPv4. A peer - // inside that range resolves to IPv4 as a whole, which is the caller's - // own writing; a peer outside it must not have subtraction hand back a - // piece that crosses over and opens IPv4 the peer never named. + // A generated block inside `::ffff:0:0/96` is programmed as IPv4, so + // subtraction must not hand back a piece that opens addresses an IPv6 + // peer never named. if family == IpFamily::V6 && !Self::block_contains(IPV4_MAPPED_BLOCK, block, width) { if let Some(crossing) = blocks .iter() @@ -1120,11 +1125,6 @@ impl NetworkIptablesManager { } /// A CIDR as the family it is programmed in and the block it covers there. - /// - /// An IPv4-mapped address is programmed as IPv4, because Linux puts a - /// genuine IPv4 packet on the wire for one. `NetworkCidr` has public fields - /// and validates only through `FromStr`, so a programmatic request reaches - /// here with a prefix the parser refuses. fn resolve_block(cidr: &NetworkCidr) -> Option<(IpFamily, DestinationBlock)> { let (family, raw, prefix) = match cidr.address { IpAddr::V4(ip) => (IpFamily::V4, u128::from(u32::from(ip)), cidr.prefix_length), diff --git a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs index 5c1a71b50..e9eb0953e 100644 --- a/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs +++ b/src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs @@ -1153,26 +1153,13 @@ fn allow_peer_policy(peers: Vec) -> ContainerPolicy { } #[test] -fn an_ipv6_peer_whose_exclusion_splits_the_mapped_range_opens_no_ipv4() { +fn an_ipv6_peer_whose_exclusion_names_the_mapped_range_is_refused() { let policy = allow_peer_policy(vec![peer("::/0", &["::ffff:10.0.0.0/104"])]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let error = lowering_error(&policy); assert!( - rules.ipv4.is_empty(), - "input=allow.to=[{{cidr:::/0, except:[::ffff:10.0.0.0/104]}}]; the exclusion names mapped addresses, which are programmed as IPv4, so this IPv6 allow must reach the IPv4 chain with nothing; output={:?}", - rules.ipv4 - ); - assert_eq!( - chain_verdict( - &rules.ipv4, - NetworkAction::Deny, - packet_address("10.10.1.1"), - "tcp", - Some(443) - ), - "DROP", - "input=as above, packet=10.10.1.1; the direction default still covers IPv4; output={:?}", - rules.ipv4 + error.contains("other address family"), + "input=allow.to=[{{cidr:::/0, except:[::ffff:10.0.0.0/104]}}]; the exclusion is programmed as IPv4 while the peer is programmed as IPv6, so it narrows nothing and must be refused rather than ignored; got: {error}" ); } @@ -1239,21 +1226,24 @@ fn a_mapped_peer_subtracts_an_exclusion_written_in_plain_ipv4() { } #[test] -fn an_exclusion_that_cannot_overlap_its_peer_is_dropped_rather_than_applied() { +fn an_exclusion_in_the_other_family_is_refused_rather_than_dropped() { let policy = allow_peer_policy(vec![peer("10.0.0.0/8", &["2001:db8::/32"])]); - let rules = NetworkIptablesManager::build_policy_rule_args("MXC-test", &policy, true); + let error = lowering_error(&policy); - assert_eq!( - chain_verdict( - &rules.ipv4, - NetworkAction::Deny, - packet_address("10.10.1.1"), - "tcp", - Some(443) - ), - "ACCEPT", - "input=allow.to=[{{cidr:10.0.0.0/8, except:[2001:db8::/32]}}], packet=10.10.1.1; an IPv6 range cannot lie inside an IPv4 peer, so the peer is unnarrowed rather than refused; output={:?}", - rules.ipv4 + assert!( + error.contains("other address family"), + "input=allow.to=[{{cidr:10.0.0.0/8, except:[2001:db8::/32]}}]; discarding the exclusion would allow the whole parent under a deny default, and the parser refuses this shape too; got: {error}" + ); +} + +#[test] +fn an_ipv4_exclusion_on_an_ipv6_peer_is_refused_in_the_same_way() { + let policy = allow_peer_policy(vec![peer("2001:db8::/32", &["10.10.0.0/16"])]); + let error = lowering_error(&policy); + + assert!( + error.contains("other address family"), + "input=allow.to=[{{cidr:2001:db8::/32, except:[10.10.0.0/16]}}]; the refusal must not depend on which family the peer is in; got: {error}" ); }