Skip to content

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

Open
dhoehna wants to merge 5 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-net-model2-deny-all-except-proxy
Open

[LXC] Address network policy gaps - model 2 (deny-all-except-proxy)#632
dhoehna wants to merge 5 commits into
microsoft:mainfrom
dhoehna:user/dahoehna/lxc-net-model2-deny-all-except-proxy

Conversation

@dhoehna

@dhoehna dhoehna commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Linked work item: AB#62830341 — [LXC] Address network policy gaps - model 2 (deny-all-except-proxy)

Summary

Implements the deny-all-except-proxy network model for LXC.

  • Fail-fast: when firewall mode is active but no veth interface is available, enforcement now returns an error instead of silently skipping the FORWARD hook (which previously left rules built but unenforced).
  • Deny-wins precedence: deny rules are now emitted before allow rules; rule construction refactored into a pure, ordered builder with a unit test asserting the order.
  • Proxy enforcement + env: when a proxy is configured, injects HTTP_PROXY / HTTPS_PROXY (+ lowercase) into the container and restricts egress to the proxy address:port (DROP other outbound).
  • Proxy env hygiene: scrubs inherited HTTP_PROXY / HTTPS_PROXY / ALL_PROXY / FTP_PROXY / NO_PROXY (+ lowercase) before setting configured values, via a pure proxy_env helper with unit tests.

Validation

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

Independent of the sibling LXC branches.

Microsoft Reviewers: Open in CodeFlow

- Fail-fast when no veth in firewall mode (no more silent skip of the FORWARD hook).
- Emit deny rules before allow rules (deny-wins); pure ordered rule-builder + tests.
- Inject HTTP(S)_PROXY + iptables proxy-only egress; scrub all inherited proxy env vars first.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3b78bec0-e139-4cfd-9c10-092ef986d4f4
Copilot AI review requested due to automatic review settings July 10, 2026 22:56
@dhoehna
dhoehna requested a review from a team as a code owner July 10, 2026 22:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a “deny-all-except-proxy” network model for the LXC backend, aiming to close enforcement gaps by failing fast when the FORWARD hook can’t be safely scoped, enforcing deny-wins rule ordering, and adding proxy-based egress restrictions plus proxy environment-variable hygiene for LXC executions.

Changes:

  • Refactors LXC iptables rule construction into an ordered rule builder (deny before allow) and fails fast when a veth interface is unavailable for scoping.
  • Adds LXC proxy enforcement that allows egress only to the configured proxy endpoint(s) and injects/scrubs proxy env vars for the container process.
  • Extends LXC attach-run environment handling with a “force clear env” control to prevent inherited proxy variables from leaking when the caller env becomes empty after scrubbing.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/core/wxc_common/src/proxy_env.rs New helper + unit tests for scrubbing and applying proxy-related env vars.
src/core/wxc_common/src/lib.rs Exposes the new proxy_env module.
src/core/wxc_common/src/config_parser.rs Allows network.proxy for lxc, adds LXC-specific validation and updates/extends tests.
src/backends/lxc/common/src/network_iptables.rs Implements proxy-only egress rules, deny-wins ordering, and fail-fast veth scoping; adds tests.
src/backends/lxc/common/src/lxc_runner.rs Uses apply_proxy_env and passes a force-clear-env flag into attach_run; includes proxy in “needs network” detection.
src/backends/lxc/common/src/lxc_bindings.rs Adds force_clear_env support to attach_run argument building and tests it.

Comment thread src/core/wxc_common/src/config_parser.rs
Comment thread src/core/wxc_common/src/config_parser.rs
Comment thread src/backends/lxc/common/src/network_iptables.rs
Comment thread src/backends/lxc/common/src/lxc_bindings.rs Outdated
…NS in proxy mode

Addresses Copilot reviewer feedback on the deny-all-except-proxy model:

- Reject network.proxy.localhost for LXC (127.0.0.1 is the container
  loopback, unreachable from the container netns); require network.proxy.url
  with a routable host. Update the LXC proxy test to use a url and add a
  rejection test.
- In proxy mode, only open outbound DNS when the proxy is addressed by
  hostname; when it is an IP literal keep the sandbox fully closed. Fix the
  misleading log line and add a host_is_ip_literal helper + unit test.
- Correct the build_attach_args doc comment: it is #[cfg(test)]-only; move
  the 'Linux + test builds' note onto build_attach_args_with_env_control.
- Note in docs/schema.md that LXC requires a url proxy (not loopback).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
logger,
)?;

self.rules_applied = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ACCEPT".to_string(),
]);
}
rules.push(vec![

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deny-all-except-proxy is IPv4-only; IPv6 egress is unfiltered?

This DROP- the "deny all other outbound" that the whole model-2 posture rests on, is only ever applied to the v4 table: every rule here is run through the iptables binary (run_iptables / run_iptables_args), and there's no ip6tables path anywhere in this file. So, on a container with IPv6 connectivity, all IPv6 egress bypasses this chain entirely and the "except-proxy" guarantee doesn't hold.

To be clear this is pre-existing v4-only behavior (resolve_host at :100-119 already drops v6, and run_iptables predates this PR), but this PR is what makes it load-bearing: before, v4-only filtering was a best-effort allow/block list; now it's the enforcement boundary of a deny-all-except-proxy security model, so the IPv6 gap becomes a real bypass rather than a minor omission.

Suggested fix: mirror the chain to ip6tables with a v6 DROP-all?


// Hook the chain into FORWARD for the container's traffic
if let Some(ref iface) = self.veth_interface {
if allow_dns {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DNS opened to the whole internet in proxy+hostname mode?

ACCEPTs udp/tcp 53 with no -d, so under "deny-all-except-proxy" the container still has an unrestricted DNS channel to any resolver- a standing DNS-tunnel exfil path. Since the proxy is already resolved host-side, consider injecting the resolved IP into HTTP_PROXY and keeping DNS closed, or scope the DNS ACCEPT to the configured resolver(s).

return Err("Network proxy port must be between 1 and 65535".to_string());
}

let ips = Self::resolve_host(address.host());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ACCEPT rule targets the host-side resolution of the proxy hostname, but the container is handed HTTP_PROXY=http://proxy.example.com:8080 (hostname, via to_url()) and resolves it itself. Under round-robin/split-horizon DNS the container may pick a different IP than the one in the ACCEPT rule → proxy unreachable while everything else is dropped. Can we pin the injected proxy to the resolved IP fixes both this and the DNS-hole above?

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

Copy link
Copy Markdown
Contributor

Shall we add LXC proxy.url test config json?

dhoehna and others added 2 commits July 15, 2026 11:38
…json)

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

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

dhoehna commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

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

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

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

  • cargo test -p wxc_common399 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_common44 passed, 0 failed — the deny-all-except-proxy iptables 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.

@microsoft-github-policy-service 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 17, 2026
…et-model2-deny-all-except-proxy

# Conflicts:
#	src/core/wxc_common/src/config_parser.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs-Attention Issue needs attention from Microsoft

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants