Refuse AF_PACKET by dhoehna · Pull Request #1457 · microsoft/mxc

Darren Hoehna (dhoehna) · 2026-10-09T18:03:36Z

📖 Description

Removing the CAP_NET_RAW capability allows a program to pass packets directly to the kernel and bypass the sandbox's networking rules. I decided to keep CAP_NET_RAW to allow LXC access to the ICMP protocol.

I have now found a way to deny a program the use of RAW and PACKAGE sockets while keeping ICMP. SecComp.

I made two changes

Refactor src/tools/lxc/src. The code is now way easier to read and understand. The flow is also easier to see.
Added refuse_packet_socket to lxc_bindings.rs. This gives the kernel our custom filter to block AF_PACKET and stop programs from bypassing the network sandbox rules.

🔗 References

https://task.ms/64150389

🔍 Validation

Added automatic tests. Further, I manually tested the change. Because lxc effects bubblewrap and lxc I also ran the bubble wrap tests too.

✅ Checklist

[x ] Signed the Contributor License Agreement
[ x] Linked to an issue
[ x] Updated documentation (if applicable)
[ x] Updated Copilot instructions (if build, architecture, or conventions changed)
[ x] If this PR changes Cargo.lock, the dependency-feed-check check passes (see pull request builds)

📋 Issue Type

[ x] Bug fix
Feature
Task

GitHub Actions runs the PR validation build automatically. The ADO pipeline
(MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHub
Actions build; it runs on merge to main, and Microsoft reviewers with write access can trigger it
on a PR with /azp run. See pull request builds.

If the dependency-feed-check check fails on a new dependency, the crate must be added to
the feed before the PR can pass. See pull request builds
for the steps.

Microsoft Reviewers: Open in CodeFlow

CAP_NET_RAW stays, because an explicit protocol: "icmp" allow needs a raw socket, and it also opens AF_PACKET, which reaches the interface below the hook the chains hang on. A syscall entering under numbering the filter was not built for kills the process rather than passing unchecked. AB#64150389 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9df5d37d-b3a2-487c-95df-ded398109b1d

azure-pipelines · 2026-10-09T18:03:46Z

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

Copilot

🟡 Changes recommended

The filter remains bypassable through io_uring, Windows Hyperlight builds fail, and the CLI refactor introduces diagnostic regressions.

4 open findings

What changed in this PR

Adds LXC network confinement intended to block AF_PACKET while retaining CAP_NET_RAW for ICMP, alongside an executor CLI refactor.

Changes:

Adds a seccomp socket filter and capability assertions.
Refactors LXC CLI argument and mode handling.
Updates ICMP tests and LXC limitations documentation.

File	Description
`tests/scripts/run_lxc_network_ga_egress_test.sh`	Removes the host ICMP preflight.
`tests/configs/lxc_network_ga_egress_icmp_allowed.json`	Exposes ping diagnostics.
`src/tools/lxc/src/main.rs`	Delegates CLI operations to the new argument module.
`src/tools/lxc/src/linux_executor_arguments.rs`	Encapsulates CLI parsing and operations.
`src/mxc-sdk/tests/wxc_e2e_tests_e2e_lxc_network_capability.rs`	Checks retained capabilities and seccomp mode.
`src/mxc-sdk/src/backends/lxc/common/lxc_runner.rs`	Updates confinement test naming.
`src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs`	Adds the seccomp `AF_PACKET` filter.
`docs/backends/lxc/lxc-backend.md`	Removes the raw-socket limitation.

🧠 Review effort: Balanced

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

+        libc::sock_filter { code: LOAD_WORD,        jt: 0, jf: 0, k: SYSCALL_NUMBER },
+        libc::sock_filter { code: JUMP_IF_AT_LEAST, jt: 5, jf: 0, k: USES_X32_ABI_FLAG }, // Test 32 bit
+        libc::sock_filter { code: JUMP_IF_EQUAL,    jt: 0, jf: 2, k: SOCKET_SYSCALL }, // Only process a socket syscall
+        libc::sock_filter { code: LOAD_WORD,        jt: 0, jf: 0, k: FIRST_ARGUMENT },
+        libc::sock_filter { code: JUMP_IF_EQUAL,    jt: 1, jf: 0, k: libc::AF_PACKET as u32 }, // return error if using AF_PACKET


+    assert_eq!(
+        status_number(&status, "Seccomp:"),
+        2,
+        "the workload runs with no seccomp filter; AF_PACKET reaches the interface below the chains\n{status}"
+    );


+    pub fn should_display_avalible_backends(&self) -> bool {
+        self.available_backends
+    }


Gudge (MGudgin)

Review summary

Requesting changes on the current PR head c6a95211a. The Windows x64 Hyperlight feature build fails (inline), and the security-sensitive filter has no behavioral test of the socket it is meant to refuse. Please address both before merge. The backend's broader packet-socket guarantee also needs either complete enforcement or an explicit limitation.

Scope and clean checks: The eight-file, +430/-288 local diff exactly matches gh pr diff 1457. The PR is behind the current base (a0b66e63c3); the reviewed three-dot range uses merge-base 7cd00d1ee6. The existing CAP_NET_ADMIN bounding-set drop is still present, and the E2E test still checks all three capability masks while retaining CAP_NET_RAW. Its test count remains one before and after the PR. The performance pass found no material new hot-path cost: the filter is fixed-size and installed once. These checks do not establish effective packet filtering.

Already raised on this PR (not duplicated inline)

High — behavioral packet-socket test missing (introduced_by_change): the E2E assertion reads Seccomp: 2, which any inherited filter can provide; it never attempts socket(AF_PACKET, ...). Existing discussion r4233182128 covers this. Require EPERM for the denied socket and positive ICMP/ordinary-socket controls; ideally prove the test fails with this filter removed.
Medium — optional --log-file failure aborts all modes (introduced_by_change): main.rs:29-37 now exits instead of warning, even for --available-backends. Existing discussion r4233182199 covers this.
Medium — io_uring socket path is not covered (claim_mismatch): the added filter examines SYS_socket only, while Linux io_socket calls __sys_socket_file without a userspace socket(2) call. Existing discussion r4233182073 identifies this route. It predates this PR and is not, by itself, a new exploit attributed to the change or an independent merge condition; whether it is usable depends on host policy. It does contradict the PR's claim to deny packet-capable sockets.

Claim gaps and additional regression coverage

Medium — legacy packet socket (claim_mismatch, PR description): the claim that this denies RAW/PACKAGE sockets is wider than the AF_PACKET-family check. Linux __sock_create translates PF_INET + SOCK_PACKET into PF_PACKET after seccomp sees the original family; CAP_NET_RAW remains available. Reject the legacy family/type pair or retain the limitation and test it.
Medium — AF_XDP (claim_mismatch, PR description): a kernel with usable AF_XDP can create AF_XDP / SOCK_RAW with CAP_NET_RAW and transmit link-layer frames; this family is allowed by the added filter. Actual bind/transmit availability depends on the host and other policy, so verify it in the supported environment before claiming this route is closed. Restrict it if the guarantee is meant to include all link-layer transmission.
Medium — BPF table regression seam (introduced_by_change, lxc_bindings.rs:223-232): positional branch offsets determine allow/deny/kill. This is part of the behavioral-test blocker above rather than a separate blocking issue. A pure builder or isolated child-process test for native/foreign ABI, x32, AF_PACKET, legacy packet type, and ordinary sockets would make changes to that table reviewable.
Medium — CLI contract tests (introduced_by_change, linux_executor_arguments.rs:94-115, main.rs:19-69): the refactor changes log failures, no-config errors and delete output with no targeted CLI tests. Add subprocess checks for exit code, stdout/stderr, mode precedence and invalid configuration. The separate parser logger and output regressions are also noted inline.
Low — ICMP harness diagnostic (introduced_by_change, tests/scripts/run_lxc_network_ga_egress_test.sh:283-289): the deleted peer ping preflight previously distinguished an unreachable/echo-silent peer from firewall denial. The allowed ICMP test still fails, but now reports a less precise cause; MXC_PING_MISSING is emitted without a harness check. Reinstate a reliable peer check or explicitly fail on that marker.

Verified pre-existing, not charged as introduced defects: Linux's PF_INET/SOCK_PACKET, io_uring socket creation and AF_XDP support predate this PR. They are listed above only as qualified mismatches with the PR's newly asserted security guarantee, capped at Medium. No finding is filed merely because unchanged code exists elsewhere. The signal-watchdog call also moved, but follow-up inspection found no Linux thread-spawning work before it; I am not filing a present-day signal bug on that basis.

The inline comments cover the newly introduced build failure, compat-ABI termination, seccomp-install prerequisite, parsing diagnostics and changed CLI outputs. The Windows feature-build failure was reproduced by cargo check -p lxc --features hyperlight; Linux LXC socket probes were not run on this Windows host.

Gudge (MGudgin) · 2026-10-09T20:33:58Z

+            }
+
+            //KVM is checked before anything is pulled.
+            #[cfg(target_os = "linux")]


High (maintainability, cross-platform parity) — Windows x64 Hyperlight builds fail.

Attribution: introduced_by_change — this extracted helper now returns Result, but the setup/result expression is inside the Linux-only block. With hyperlight on Windows x64, the WHP check is the only remaining expression; its success path evaluates to (), producing E0317. cargo check -p lxc --features hyperlight reproduced this on Windows.

Fix: Keep OS-specific WHP/KVM preconditions separate, then run the shared parse_runtimes/setup code on both platforms (or provide a Windows branch returning Result).

Gudge (MGudgin) · 2026-10-09T20:33:58Z

+        libc::sock_filter { code: JUMP_IF_EQUAL,    jt: 1, jf: 0, k: libc::AF_PACKET as u32 }, // return error if using AF_PACKET
+        libc::sock_filter { code: RETURN,           jt: 0, jf: 0, k: libc::SECCOMP_RET_ALLOW },
+        libc::sock_filter { code: RETURN,           jt: 0, jf: 0, k: REFUSE_WITH_EPERM },
+        libc::sock_filter { code: RETURN,           jt: 0, jf: 0, k: libc::SECCOMP_RET_KILL_PROCESS },


Medium (reliability) — compat-ABI workloads are killed on their first syscall.

Attribution: introduced_by_change — this new SECCOMP_RET_KILL_PROCESS branch handles foreign audit architectures and x32 syscall numbers. A 32-bit program under an installed firewall can now die with SIGSYS before it opens any network socket, rather than receiving a meaningful sandbox-policy error. This may be a deliberate fail-closed policy, but it is neither documented nor exercised by the new test.

Fix: Document and test the supported ABI restriction with a useful diagnostic, or add a compat-ABI policy that still refuses packet sockets.

Gudge (MGudgin) · 2026-10-09T20:33:58Z

+    let taken = unsafe {
+        libc::syscall(
+            libc::SYS_seccomp,
+            libc::SECCOMP_SET_MODE_FILTER as libc::c_ulong,


Medium (reliability) — filter installation adds an opaque privilege prerequisite.

Attribution: introduced_by_change — SECCOMP_SET_MODE_FILTER is new here. It requires CAP_SYS_ADMIN in the caller's user namespace or an already-set no_new_privs; the code does not set the latter. A caller with enough privilege for the existing PR_CAPBSET_DROP but not this new step now fails a firewalled run. The returned bare OS error does not identify which pre_exec step failed.

Fix: Document the new prerequisite, distinguish the seccomp failure without allocating unsafely in pre_exec, and test the failing-install path. Do not set no_new_privs indiscriminately because it changes privileged-exec behavior.

Gudge (MGudgin) · 2026-10-09T20:33:58Z

+            (String::new(), false)
+        };
+
+        let mut logger = Logger::new(if self.debug {


Medium (correctness) — request diagnostics no longer reach --log-file.

Attribution: introduced_by_change — get_request constructs a second logger with no file sink, then passes it to load_one_shot_request. Previously parsing used the logger configured in main, so validation warnings and parse errors were recorded in the requested log. The new logger is discarded, even when the CLI configured a file sink successfully.

Fix: Pass the already-configured logger into the request parser and preserve its diagnostics through the error path.

Gudge (MGudgin) · 2026-10-09T20:33:58Z

+        } else if let Some(ref path) = self.config_path {
+            (path.clone(), false)
+        } else {
+            (String::new(), false)


Low (reliability) — no-argument invocation loses its actionable error.

Attribution: introduced_by_change — the old main explicitly exited with Error: No config provided. Use a positional path, --config, or --config-base64. This new fallback passes an empty string into the parser and emits a generic Request error instead.

Fix: Restore the explicit missing-config check and its usage guidance before calling load_one_shot_request.

Gudge (MGudgin) · 2026-10-09T20:33:58Z

+    } else if arguments.does_user_want_to_delete_a_container() {
+        match arguments.delete_container() {
+            Ok(message) => {
+                eprintln!("{message}");


Low (reliability) — delete-mode output moved from stdout/file log to stderr.

Attribution: introduced_by_change — the old delete path logged its result through the CLI logger and printed the buffer on stdout. This new success path prints only to stderr; the --log-file sink also misses the result. Automation reading the previous output stream sees no success message.

Fix: Preserve the old output/logging contract, or document and test an intentional CLI-breaking change.

The setup body moved inside a target_os = "linux" block, which broke the lxc-exec build on Windows and macOS x86_64 and left --setup-hyperlight with nothing to do on either. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1654c073-27eb-43cf-82f3-417614ce845b

Copilot

🔵 Needs a closer look

The AF_PACKET restriction remains bypassable through io_uring, and the CLI refactor introduces diagnostic regressions.

4 open findings

Previously missed (2)

In code that hasn't changed since last review

Reuse configured logger for parser diagnostics

src/tools/lxc/src/linux_executor_arguments.rs:109

This second logger is not the configured logger from main, so successful parsing no longer sends parser diagnostics to --log-file, and retained parser warnings are discarded with this local value. Accept &mut Logger in get_request and pass the existing logger through to load_one_shot_request to preserve the prior diagnostic stream.

Clarify retained CAP_NET_RAW in security comment

src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs:142

This security comment says both capabilities are removed, but the implementation intentionally retains CAP_NET_RAW and restricts only AF_PACKET via seccomp. Describe the actual split so future changes do not treat the retained capability as an accidental omission.

🧠 Review effort: Balanced

The container check only asserted that a filter was attached, which a permissive filter passes identically. A paired unit test now opens a packet socket with and without the filter: allowed unfiltered, EPERM filtered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1654c073-27eb-43cf-82f3-417614ce845b

A missing config exited silently on an empty path, runtime parse failures lost their Error: prefix, the signal handler installed after work had begun, and an unreadable backend list exited 1 instead of warning and continuing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1654c073-27eb-43cf-82f3-417614ce845b

The later cases read an unanswered echo as a firewall verdict. A peer that never answers ICMP produces that same silence, which passes the run without anything having been enforced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1654c073-27eb-43cf-82f3-417614ce845b

Copilot

🟡 Changes recommended

The filter remains bypassable through io_uring, introduces kernel compatibility concerns, and the CLI refactor regresses logging behavior.

4 open findings

1 resolved since last review

Diagnostic sink failure incorrectly aborts all modes

Previously missed (2)

In code that hasn't changed since last review

Use configured logger for request parsing diagnostics

src/tools/lxc/src/linux_executor_arguments.rs:112

The new private logger disconnects config parsing from the logger configured in main. Consequently, successful normalization warnings are discarded in buffered mode, and neither warnings nor parse errors reach --log-file; load_one_shot_request emits both through the logger it receives. Pass main's &mut Logger into get_request instead of constructing another one.

Delete results bypass logger and redirect success output

src/tools/lxc/src/main.rs:78

This refactor bypasses the configured logger for delete results, so --delete --log-file ... no longer records either success or failure. It also moves the success message from stdout to stderr. Write both outcomes to the logger's diagnostic sink and keep successful user output on stdout.

🧠 Review effort: Balanced

+        libc::syscall(
+            libc::SYS_seccomp,
+            libc::SECCOMP_SET_MODE_FILTER as libc::c_ulong,
+            NO_FLAGS,
+            &header as *const libc::sock_fprog,


The watchdog destroys a running container on a fatal signal, and nothing earlier in main creates one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1654c073-27eb-43cf-82f3-417614ce845b

Copilot

🔵 Needs a closer look

The filter remains bypassable through io_uring, and the CLI refactor introduces diagnostic and output regressions.

4 open findings

Previously missed (4)

In code that hasn't changed since last review

Check CapEff instead of effective UID

src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs:1667

EUID 0 does not imply that the process has CAP_NET_RAW (for example, a hardened container can run root with that capability dropped), and a non-root process can also be granted the capability. This makes the control fail based on the test host's capability configuration rather than filter behavior. Compare the probe result with bit 13 of /proc/self/status's CapEff mask instead.

Use the configured logger for request parsing

src/tools/lxc/src/linux_executor_arguments.rs:112

This creates a second logger after main has configured --log-file, so parsing no longer uses the configured sink. Parse failures are absent from the requested log file, and successful normalization diagnostics buffered here are dropped with this local logger. Pass the already-configured logger into get_request instead of constructing another one.

Preserve logger routing for deletion results

src/tools/lxc/src/main.rs:67

This branch previously routed the deletion result through logger, which both preserved the CLI's stdout behavior and wrote it to --log-file. Sending both outcomes directly to stderr silently changes the command contract and leaves the configured file sink empty.

Clarify the actual capability and socket-filtering split

src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs:142

The comment says both capabilities are removed, but the implementation intentionally retains CAP_NET_RAW. The parenthetical leaves the security contract contradictory; state the actual split between the capability drop and socket filtering directly.

🧠 Review effort: Balanced

Darren Hoehna (dhoehna) and others added 4 commits October 7, 2026 17:08

simplifying main

f2aa888

Finishing up seccomp

a7b725e

Fixing up some AI comments

c6a9521

Darren Hoehna (dhoehna) requested a review from a team as a code owner October 9, 2026 18:03

Copilot AI balanced review requested due to automatic review settings October 9, 2026 18:03

Copilot started reviewing on behalf of Darren Hoehna (dhoehna) October 9, 2026 18:04 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Gudge (MGudgin) requested changes Oct 9, 2026

View reviewed changes

microsoft-github-policy-service Bot assigned Darren Hoehna (dhoehna) Oct 9, 2026

microsoft-github-policy-service Bot added the Needs-Author-Feedback Waiting for additional information or action from the issue or pull-request author. label Oct 9, 2026

Copilot AI balanced review requested due to automatic review settings October 9, 2026 22:36

Copilot started reviewing on behalf of Darren Hoehna (dhoehna) October 9, 2026 22:37 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Darren Hoehna (dhoehna) and others added 3 commits October 9, 2026 15:53

Copilot AI balanced review requested due to automatic review settings October 9, 2026 22:53

Copilot started reviewing on behalf of Darren Hoehna (dhoehna) October 9, 2026 22:54 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread src/mxc-sdk/src/backends/lxc/common/lxc_bindings.rs

Comment on lines +240 to +244

libc::syscall(

libc::SYS_seccomp,

libc::SECCOMP_SET_MODE_FILTER as libc::c_ulong,

NO_FLAGS,

&header as *const libc::sock_fprog,

Install the signal watchdog just before the container run

7c9b02f

The watchdog destroys a running container on a fatal signal, and nothing earlier in main creates one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1654c073-27eb-43cf-82f3-417614ce845b

Copilot AI balanced review requested due to automatic review settings October 9, 2026 23:28

Copilot started reviewing on behalf of Darren Hoehna (dhoehna) October 9, 2026 23:28 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Navigation Menu

Repository navigation

Refuse AF_PACKET - #1457

Refuse AF_PACKET#1457
Darren Hoehna (dhoehna) wants to merge 9 commits into
mainfrom
user/dahoehna/lxc-afpacket-seccomp

Darren Hoehna (dhoehna) commented Oct 9, 2026 •

edited by microsoft-github-policy-service Bot

Loading

Uh oh!

azure-pipelines Bot commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Gudge (MGudgin) left a comment

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Darren Hoehna (dhoehna) commented Oct 9, 2026 • edited by microsoft-github-policy-service Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Gudge (MGudgin) left a comment

Choose a reason for hiding this comment

Review summary

Already raised on this PR (not duplicated inline)

Claim gaps and additional regression coverage

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Gudge (MGudgin) Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Darren Hoehna (dhoehna) commented Oct 9, 2026 •

edited by microsoft-github-policy-service Bot

Loading