[LXC] Subtract an egress 'except' from its own rule so a carve-out cannot shadow a later deny

[Soham Das (SohamDas2021)]

📖 Description

On the LXC backend, a CIDR except exclusion on a schema-0.8 directional egress rule was emitted as its own iptables rule ahead of its parent CIDR. The chain is first-match-wins, so that carve-out escaped the rule that declared it and answered for every later rule covering the same addresses.

A caller could write:

"egress": {
  "default": "allow",
  "deny": [
    { "to": [{ "cidr": "10.0.0.0/8", "except": ["10.10.0.0/16"] }] },
    { "to": [{ "cidr": "10.10.1.0/24" }] }
  ]
}

and the container still reached 10.10.1.1, despite a rule denying it outright. The exclusion only ever turned a deny into an accept, so the failure was silent and always in the permissive direction.

An exclusion now narrows only the rule that declares it and states no verdict of its own. The exclusion is subtracted from its peer and the covering blocks that remain are programmed, mirroring what the Bubblewrap backend already does in src/backends/bubblewrap/common/src/network_rules.rs.

Subtraction splits the surrounding block once per prefix level, so a peer can expand into many blocks. Two ceilings bound that: a peer may expand into at most 256 blocks, and one egress policy may lower into at most 65,536 entries. A policy exceeding either is rejected, the container is torn down, and the script never runs — no partial policy is installed. Both limits are now stated in the 0.8 networking contract, which had documented neither.

Correction. An earlier version of this description said that rejection happens before the sandbox is created. It does not. The check runs during firewall setup, which is after the container has started, because the surrounding setup needs the container's network namespace. The observable outcome above is unchanged — nothing runs unenforced — but making it a genuine preflight is worth a follow-up.

tests/configs/ gained two fixtures, so the corpus inventory pinned in config_parser.rs moves from (368, 344, 14) to (370, 346, 14) — two files, two equivalent accepts, no new divergence and no new rejection.

🔗 References

Resolves #1010

🔍 Validation

Every number below is from a local run.

Check	Result
cargo fmt --all -- --check	exit 0
cargo clippy -p lxc_common -p wxc_common --all-targets -- -D warnings	exit 0
cargo test -p lxc_common --all-targets	314 + 3 + 28, 0 failed
cargo test --workspace	101 suites, 4502 passed, 0 failed, 30 ignored
tests/scripts/run_lxc_all_tests.sh (WSL2 Ubuntu 24.04, root)	28 passed, 0 failed, 0 skipped, 1 disabled

The one disabled entry is the pre-existing quarantined "LXC Network" test. These counts match the baseline taken on main before the change.

The tests were checked against the bug. Reverting only network_iptables.rs and re-running:

• Unit: 312 passed, 2 failed — an_exclusion_does_not_shadow_a_later_rule_that_names_it and a_peer_that_expands_past_the_block_ceiling_is_refused.
• Integration: run_lxc_network_ga_egress_test.sh exits 1 on the new shadow case.

The egress script needed the new fixtures to see this at all. Its two existing except cases are both default: deny with an allow rule, a direction that was already correct, so the suite passed unchanged against a binary built without the fix. The shadow case supplies the missing direction, and the shadow-control case keeps it honest by requiring the same address to stay reachable once the second deny is removed — otherwise a rule set that blocked everything would pass.

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Updated documentation (if applicable)
• Updated Copilot instructions (if build, architecture, or conventions changed)
• If this PR changes Cargo.lock, the dependency-feed-check check passes (see docs/pull-requests.md)

📋 Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

🟡 Changes recommended

Malformed exclusions can fail open, and expansion validation occurs only after container startup.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes LXC egress exclusions so carve-outs do not override later rules.

Changes:

• Subtracts exclusions from parent CIDRs with bounded expansion.
• Adds unit and integration regression coverage.
• Corrects schema 0.8 networking documentation.

File summaries

File	Description
src/backends/lxc/common/src/network_iptables.rs	Implements bounded CIDR subtraction.
src/backends/lxc/common/src/network_iptables_ga_egress_spec.rs	Tests subtraction and shadowing behavior.
src/core/wxc_common/src/config_parser.rs	Updates fixture inventory counts.
tests/scripts/run_lxc_network_ga_egress_test.sh	Adds integration shadow checks.
tests/configs/lxc_network_ga_egress_except_shadow.json	Reproduces exclusion shadowing.
tests/configs/lxc_network_ga_egress_except_shadow_control.json	Verifies the exclusion remains reachable.
docs/sandbox-policy/0.8.0/networking/networking.md	Documents exclusion semantics.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 4
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Jeff Whiteside (jsidewhite)]

Does this logic really only apply to LXC? It seems generic to all backends.

[Soham Das (SohamDas2021)]

Does this logic really only apply to LXC? It seems generic to all backends.

The bug was LXC-only, Bubblewrap already subtracts, and WSLC/Seatbelt have no destination filtering.

[Copilot]

🟡 Changes recommended

CIDR subtraction can unexpectedly migrate broad IPv6 rules into the IPv4 chain and silently ignores malformed programmatic exclusions.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/backends/lxc/common/src/network_iptables.rs:976

• filter_map silently discards a malformed exclusion from a programmatically constructed NetworkPeer. Because NetworkCidr fields are public, an allow such as 10.0.0.0/8 except 10.1.0.0/40 then becomes an allow for the entire /8, including the intended carve-out. Validate every exclusion and return Err instead of dropping invalid or non-contained entries; Bubblewrap already rejects this same input.

        // Blocks carry no family, so an exclusion from the other one would be
        // compared against the peer as a meaningless integer.
        let exclusions: Vec<DestinationBlock> = peer
            .except
            .iter()
            .filter(|excluded| excluded.address.is_ipv4() == peer.cidr.address.is_ipv4())
            .filter_map(|excluded| Self::to_block(excluded, width))
            .collect();

src/backends/lxc/common/src/network_iptables.rs:904

• The new 65,536-entry policy ceiling is not exercised by the added tests; only the per-peer block ceiling is covered. Add boundary tests showing that exactly 65,536 lowered entries are accepted and the next entry is rejected, including accumulation across multiple rules and an any selector with a port (which lowers to both TCP and UDP). The analogous Bubblewrap coverage is in network_rules.rs:2126-2201.

        let mut entries = Vec::new();
        let mut remaining = MAX_EGRESS_ENTRIES;

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Malformed typed exclusions can be silently discarded, broadening allow rules, and the total-entry ceiling lacks direct tests.

Review details

Suppressed comments (2)

src/backends/lxc/common/src/network_iptables.rs:939

• The new total-policy ceiling is not exercised by the added tests; they cover only the per-peer block ceiling. Add cumulative/cross-product cases showing that more than 65,536 entries is rejected and exactly 65,536 is accepted, so a future per-rule budget reset or off-by-one cannot silently disable this advertised guard.

                    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."
                        ));
                    }

src/backends/lxc/common/src/network_iptables.rs:976

• filter_map silently discards a malformed same-family exclusion. NetworkCidr has public fields, and this module deliberately routes malformed parent CIDRs to validation; with a directly constructed IPv4 /40 exclusion on an allow rule, this instead removes the exclusion and accepts the whole parent. Return an error from to_block so malformed policies fail closed, matching the Bubblewrap path.

        let exclusions: Vec<DestinationBlock> = peer
            .except
            .iter()
            .filter(|excluded| excluded.address.is_ipv4() == peer.cidr.address.is_ipv4())
            .filter_map(|excluded| Self::to_block(excluded, width))
            .collect();

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Copilot review overview

🟡 Changes recommended

Unbounded exclusion traversal can make preflight prohibitively expensive, and the new policy-wide limit lacks boundary coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

• Update PR description to reflect preflight check timing · New
• The new policy-wide 65,536-entry ceiling is untested; the added ceiling test exercises only…

Resolved since last review (3)

• A valid broad IPv6 peer can change families after subtraction. For example, `allow ::/0 except…
• Malformed programmatic exclusions are silently discarded here. NetworkCidr has public fields, so…
• This is the first point that checks the new expansion ceilings, but it runs from…

Previously missed (2)

In code that hasn't changed since last review

Exclusion-heavy inputs can bypass output limits and cause quadratic work

src/​backends/​lxc/​common/​src/​network_iptables.rs:1052

The output ceilings do not bound the work here when exclusions cover the peer. For example, a valid 0.0.0.0/0 peer excluded by all 65,536 /16s produces zero output blocks, so remaining never decreases, while each recursive node scans the full unbounded exclusion list. That makes preflight quadratic and can stall policy processing; canonicalize/index the exclusions or impose a separate input/work budget before recursing.

Use neutral wording for mixed allow and deny fixtures

tests/​scripts/​run_lxc_network_ga_egress_test.sh:237

These newly added fixtures describe deny rules, but this loop's failure still says each fixture “no longer allows” the CIDR. A drift failure for either shadow fixture is therefore misleading; use neutral “targets” wording for this mixed allow/deny list.

[Sylvester Kaczmarek (sylvesterkaczmarek)]

Different-family except entries are still silently filtered out here. An IPv4 peer carrying an IPv6 exclusion, or vice versa, therefore applies the full parent rule instead of rejecting the malformed peer; for an allow rule under a deny default that widens access. Could this return an error for a family mismatch rather than dropping the exclusion, matching the fail-closed handling for an invalid prefix?