[Bubblewrap/LXC] Address common network policy gaps - model 1 by dhoehna · Pull Request #631 · microsoft/mxc

dhoehna · 2026-07-10T22:56:10Z

Linked work item: AB#62830559 — [Bubblewrap/LXC] Address common network policy gaps - model 1

Summary

Adds IPv6, CIDR, port, and protocol filtering to the shared iptables/ip6tables enforcement path. This applies to both the LXC and Bubblewrap backends, since Bubblewrap reuses lxc_common::network_iptables::NetworkIptablesManager for host-level network filtering.

resolve_host now returns dual-stack (IPv4 + IPv6); IPv6 literals / AAAA results are retained instead of dropped.
New run_ip6tables helper + a parallel v6 chain mirroring loopback / ESTABLISHED,RELATED / DNS / default-policy, with an ip6tables -I FORWARD hook and matching cleanup.
CIDR passthrough for v4 and v6 (validated via std::net), routed to the correct table by address family.
Per-rule port (--dport) and protocol (-p tcp/udp/icmp) enforcement via a new internal egress_rules model field; the legacy allowed_hosts / blocked_hosts path is preserved.
Rule construction refactored into pure helpers returning arg vectors, with unit tests; the old IPv6-drop tests and the resolve_host doc comment were updated to the new dual-stack behavior.

Model

Adds ContainerPolicy.egress_rules: Vec<EgressRule> (+ EgressRule, Protocol, RuleAction), default empty.

Validation

cargo fmt --all -- --check; cargo clippy --target x86_64-unknown-linux-gnu -p lxc_common -D warnings — clean
cargo test -p wxc_common — 395 passed
cargo check --target x86_64-unknown-linux-gnu -p lxc_common --tests — pass

Coupling

EgressRule / Protocol / RuleAction are also introduced independently by the schema PR (AB#62830582) and net-model-2 (AB#62830341); merge-time reconciliation is expected. The JSON parser for egress_rules is intentionally owned by the schema PR.

Review follow-ups (round 2)

ip6tables probe: ip6tables is probed once per apply; on IPv4-only hosts (binary absent or IPv6 disabled) the v6 chain is skipped with a warning instead of failing the whole firewall setup.
Partial-apply rollback: a failed apply_firewall_rules now tears down any partially-created chains / FORWARD hooks (shared teardown with remove_firewall_rules), so a retry no longer trips over "chain already exists".
Fail-closed egress default: EgressRule now defaults to action: deny so an under-specified rule cannot silently widen access.
Ordering: documented the interim allow-before-deny ordering in build_policy_rule_args; deny-precedence reconciliation stays owned by AB#62830341.
Test configs: added tests/configs/lxc_network_ipv6_cidr.json and tests/configs/bubblewrap_network_ipv6_cidr.json exercising IPv6 + CIDR host filtering.

Microsoft Reviewers: Open in CodeFlow

…(AB#62830559) - resolve_host returns dual-stack; add ip6tables v6 chain mirroring the v4 chain. - CIDR (v4/v6) passthrough; per-rule --dport and -p tcp/udp/icmp via new EgressRule model field. - Pure rule-builder helpers with unit tests; update legacy IPv6-drop tests to dual-stack. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b78bec0-e139-4cfd-9c10-092ef986d4f4

Copilot

Pull request overview

This PR extends the LXC firewall enforcement path to support a richer, dual-stack egress network policy model (IPv4 + IPv6), including CIDR destinations and per-rule protocol/port filtering, while preserving the legacy allowed/blocked host lists.

Changes:

Added ContainerPolicy.egress_rules with supporting EgressRule, Protocol, and RuleAction types in wxc_common.
Updated the LXC iptables enforcement implementation to build parallel iptables + ip6tables chains and to preserve IPv6 literals/AAAA resolution results.
Refactored rule construction into helper functions returning argument vectors, with unit tests for the new rule-building behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
src/core/wxc_common/src/models.rs	Introduces the `egress_rules` policy model and supporting enums/structs on `ContainerPolicy`.
src/backends/lxc/common/src/network_iptables.rs	Implements dual-stack (iptables/ip6tables) rule application, CIDR/protocol/port handling, and adds unit tests for rule argument construction.

Resolves the five Copilot reviewer comments: - resolve_host doc comment now accurately describes CIDR validation (address parses + prefix in range; host bits are not required to be zero since iptables/ip6tables apply the mask). - protocol_arg is address-family aware: IPv6 ICMP rules use `ipv6-icmp` (ip6tables rejects `icmp`). - ICMP rules never emit `--dport`; the port dimension is collapsed for portless protocols so no invalid or duplicate rules are generated. - FORWARD hook and its cleanup now match container egress by input interface (`-i <veth>`) instead of `-o <veth>`, and the delete matches the insert. Adds unit tests for IPv4/IPv6 ICMP protocol naming and ICMP dport suppression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

SohamDas2021 · 2026-07-14T22:02:02Z

PR description only calls out LXC- "Adds IPv6, CIDR, port, and protocol filtering to the LXC iptables enforcement path.", but this is for Bwrap as well, please update?

SohamDas2021 · 2026-07-14T21:54:02Z

+            vec![
+                "-A", chain_name, "-p", "udp", "--dport", "53", "-j", "ACCEPT",
+            ],
+            vec![
+                "-A", chain_name, "-p", "tcp", "--dport", "53", "-j", "ACCEPT",


These ACCEPT udp/tcp port 53 to any destination, applied to both chains. Under default: Block, this is a standing DNS-tunneling exfiltration path- exactly the threat the GA doc’s default-deny targets and the PR now extends it to IPv6. Pre-existing for v4, but this is the natural place to tighten.
Suggest: scope the DNS ACCEPT to the container’s configured resolver(s), not 0.0.0.0/0 / ::/0.

Agreed this is worth tightening, and thanks for flagging it. Two constraints on doing it inside this PR: (1) it's pre-existing v4 behavior, and (2) there is no resolver/nameserver field on ContainerPolicy today, so "the container's configured resolver(s)" isn't expressible yet — scoping the port-53 ACCEPT to specific resolvers needs a new config surface. The default-deny still drops all non-DNS egress and the ACCEPT is limited to udp/tcp dport 53, but I agree it remains a residual DNS-exfil channel.

I'd rather land resolver-scoping as a dedicated follow-up that (a) adds a dnsServers/resolver policy field and (b) scopes both the v4 and v6 port-53 ACCEPTs to it, instead of hard-coding a resolver here. Leaving this open to track it — I can file it as its own work item if you'd like.

SohamDas2021 · 2026-07-14T22:01:15Z

-                .map(|ip| ip.to_string())
-                .collect(),
-            Err(_) => Vec::new(),
+    fn build_policy_rule_args(chain_name: &str, policy: &ContainerPolicy) -> FirewallRuleArgs {


allow-before-deny ordering has no deny-precedence. build_policy_rule_args appends allowed before blocked, then egress_rules in author order. With
iptables first-match-wins, an IP in both lists is ACCEPTed, and egress_rules carry no deny priority, contrary to GA D4 (deny-wins). This is captured in #62830341, but since this PR introduces the mixed allow/deny model, flag it so the ordering is reconciled rather than assumed correct in the interim.

Flagged in 96af8f9: added a NOTE on build_policy_rule_args documenting that rules are emitted allow-list -> block-list -> egress_rules (author order) and applied first-match-wins, so this model-1 change does not implement deny-precedence — a destination present in both lists is ACCEPTed, and egress_rules carry no deny priority. Reconciling to the GA "deny-wins" ordering across the combined allow/deny model stays owned by net-model-2 (AB#62830341), as noted in the PR's Coupling section. Leaving this thread open so the interim behavior stays tracked until that reconciliation lands.

SohamDas2021 · 2026-07-14T22:38:17Z

Lets add a test config jsons for bwrap and lxc that exercises ipv6 filtering and CIDR ranges.

…-proto Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e5d2aa5b-7f04-4e4d-83d3-a02efe7020ab

… fail-closed egress default - network_iptables: probe ip6tables once and skip the parallel v6 chain (with a warning) when the binary is absent or IPv6 is disabled, so IPv4-only hosts no longer fail the whole firewall setup. - network_iptables: roll back partially-created chains/FORWARD hooks on any apply error so a retry does not hit "chain already exists" and leak state; share the teardown path with remove_firewall_rules. - models: EgressRule default action is now Deny (fail-closed) so an under-specified egress rule cannot silently widen access. - network_iptables: document the interim allow-before-deny ordering (deny-precedence reconciliation tracked by AB#62830341). - tests: add lxc + bubblewrap network configs exercising IPv6 and CIDR host filtering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

dhoehna · 2026-07-16T18:28:06Z

@SohamDas2021 thanks for the review — addressed the two non-inline items in 96af8f9:

Re: "PR description only calls out LXC ... but this is for Bwrap as well"
Updated the PR description. The summary now states the change applies to the shared iptables/ip6tables enforcement path used by both the LXC and Bubblewrap backends (Bubblewrap reuses lxc_common::network_iptables::NetworkIptablesManager), and I added a "Review follow-ups" section documenting this round's fixes.

Re: "add test config jsons for bwrap and lxc that exercises ipv6 filtering and CIDR ranges"
Added two configs that exercise IPv6 literals + IPv4/IPv6 CIDR ranges in both the allow and block lists:

tests/configs/lxc_network_ipv6_cidr.json
tests/configs/bubblewrap_network_ipv6_cidr.json

Both validate against the dev schema (node scripts/versioning/validate-configs.js passes). Note they drive IPv6/CIDR through allowedHosts / blockedHosts (which this PR extends to retain IPv6 literals and pass CIDRs through by family) rather than the richer egressRules block — the JSON parser for egressRules is intentionally owned by the schema PR (AB#62830582), so it isn't wired into config parsing on this branch yet.

Also pushed the three inline fixes (ip6tables probe, partial-apply rollback, fail-closed EgressRule default) and flagged the allow-before-deny ordering in code; replies are on the individual threads.

dhoehna · 2026-07-17T17:09:48Z

🧪 Local test re-verification — 2026-07-17

Re-ran the test suites locally at branch tip 96af8f9 on a dev workstation. All green (0 failures).

Windows host (x86_64-pc-windows-msvc, cargo 1.96.1):

cargo test -p wxc_common → 396 passed, 0 failed

Linux (WSL2 Ubuntu-24.04, x86_64-unknown-linux-gnu, cargo 1.97.0, isolated CARGO_TARGET_DIR):

cargo test -p lxc_common → 50 passed, 0 failed — the iptables/ip6tables rule-builder enforcement ran on Linux (the original Validation only cargo check-compiled it for the Linux target).

Note: the wxc_common count differs slightly from the original Validation because the branch advanced since it was written.

Copilot AI review requested due to automatic review settings July 10, 2026 22:56

dhoehna requested a review from a team as a code owner July 10, 2026 22:56

Copilot started reviewing on behalf of dhoehna July 10, 2026 22:56 View session

Copilot AI reviewed Jul 10, 2026

View reviewed changes

SohamDas2021 requested changes Jul 14, 2026

View reviewed changes

microsoft-github-policy-service Bot assigned dhoehna Jul 14, 2026

microsoft-github-policy-service Bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Jul 14, 2026

SohamDas2021 mentioned this pull request Jul 14, 2026

[LXC] Address network policy gaps - model 2 (deny-all-except-proxy) #632

Open

dhoehna and others added 2 commits July 15, 2026 13:10

Merge microsoft/mxc main (47a172c) into lxc-net-model1-ipv6-cidr-port…

30384c8

…-proto Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e5d2aa5b-7f04-4e4d-83d3-a02efe7020ab

microsoft-github-policy-service Bot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Jul 16, 2026

Uh oh!

Conversation

dhoehna commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Model

Validation

Coupling

Review follow-ups (round 2)

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SohamDas2021 commented Jul 14, 2026

Uh oh!

Uh oh!

Uh oh!

SohamDas2021 Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

dhoehna Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SohamDas2021 Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

dhoehna Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

SohamDas2021 commented Jul 14, 2026

Uh oh!

dhoehna commented Jul 16, 2026

Uh oh!

dhoehna commented Jul 17, 2026

🧪 Local test re-verification — 2026-07-17

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dhoehna commented Jul 10, 2026 •

edited

Loading