network: allow combining net allow and deny policies by bferanmi806-sketch · Pull Request #228 · multikernel/sandlock

bferanmi806-sketch · 2026-09-14T21:14:43Z

Fixes #135.

Outbound policy is now allow-only, deny-only, or combined allow-minus-deny: with both lists present a destination must match an allow rule and must not match a deny rule. The same precedence applies to TCP bind allow/deny, where denied ports win.

Architecture (per congwang-mk review 2026-09-18): Sandbox.net_allow holds only explicit user rules. HTTP reachability is generated at resolution time via http_net_allow_rules() from http_allow/http_deny/http_ports and merged only at consumption sites (sandbox spawn resolution including virtual /etc/hosts, Landlock connect-port gates, resolved feature flags). The mode is derived from the separated configuration with no net_allow_explicit flag, no custom trailing deserializer, no checkpoint/meta.json persistence or backfill, and no profile/CLI special cases: explicit allow always activates the layer; HTTP-derived reachability activates it only when no deny is present (HTTP-only stays a restrictive allowlist, deny-only+HTTP stays default-allow).

Checkpoint: IMAGE_VERSION bumped 2 -> 3. Upstream v2 images may store HTTP-derived rules inside net_allow, which the separated representation would misread as explicit rules (deny-only+HTTP loading as combined), so v2 is rejected outright with no compatibility fallback (pre-1.0 policy). Regression test: image_rejects_v2_http_mixed_net_allow.

Static deny stays effective after dynamic allow resolution: the effective allow resolves per-PID override > live policy > static allowlist, then the immutable static deny is checked and wins, including under Unrestricted allow.

Combined finite TCP policies keep the Landlock allow-side port gate; deny-only and all-ports/wildcard allow drop CONNECT_TCP so the on-behalf path enforces.

Regression coverage: combined finite overlap/non-overlap, wildcard and Unrestricted allow with deny carve-outs, hostname resolving into a denied CIDR, deny-only with HTTP, HTTP-only, combined+HTTP, profile/CLI rebuild round-trips, checkpoint round-trips plus v2 rejection, and Landlock mode selection (deny-only, combined finite/wildcard, HTTP-only, deny-only+HTTP, combined+HTTP) including ephemeral bind(0).

Validation (WSL Ubuntu, Rust 1.98.1): cargo test -p sandlock-core --lib => 802 passed, 0 failed. Sandbox-execution CLI/integration tests cannot run in this WSL host (Landlock ABI v3, NetTcp requires v4); expected in CI. Go/Python sources untouched by this change.

congwang-mk · 2026-09-17T20:30:29Z

@bferanmi806-sketch Thanks for the PR! Please fix the failing tests and rebase to solve the conflicts. I will look into it tomorrow.

bferanmi806-sketch · 2026-09-18T10:11:04Z

Thanks! I’ve rebased this onto the latest main and resolved the conflicts with the recent bind enforcement changes.

The learn-mode bind regression is covered by preserving the upstream wildcard bind allowance, and I added regression coverage around the combined bind policy and the NetTcp protection guard as well.

Local build/core validation is passing as far as this host supports. The new Actions run is currently waiting for approval to start on the fork, so there aren’t any CI jobs/results yet.

Should be ready for another look when you get a chance.

congwang-mk · 2026-09-18T22:16:12Z

Thanks for the rebase. Before a line-level pass I want to settle two design points.

1. The net_allow_explicit flag

This flag exists only because extend_net_allow_for_http appends generated reachability rules into the user's net_allow, so the code can no longer tell "the user gave allow rules" from "HTTP generated them". The PR patches that by recording an origin bit and threading it through the builder, Clone, the checkpoint blob, meta.json, profile output, the CLI profile re-feed, resolved.rs, and landlock.rs, with a tri-state whose None means different things depending on whether net_deny is present.

Please fix this at the root instead: keep HTTP-derived rules out of net_allow (a separate field, or generate them at resolve time from http_allow/http_deny/http_ports) and merge them only where the rules are consumed (resolve_net_allow in sandbox.rs and the feature flags in resolved.rs). Then the mode is derivable from the two lists alone, and the flag, the custom deserializer, and every special case in profile.rs and main.rs go away.

2. The compatibility machinery

Sandlock does not keep backward compatibility before 1.0, so please drop all of it:

deserialize_trailing_net_allow_explicit decides "legacy blob" by string-matching serde error text ("eof", "failed to fill", ...). That is fragile and not something we want in the policy loader.
The meta.json backfill is dead code: a legacy image has neither the trailing blob field nor the meta key, so there is nothing to backfill from.
If a new field must land in the checkpoint policy, bump IMAGE_VERSION and let old images be rejected; the loader already does that.

With point 1 done, none of this is needed anyway.

I will also approve the CI run once these are addressed, and then review the rest.

bferanmi806-sketch · 2026-09-19T09:58:31Z

Thanks for the detailed feedback! I've addressed both design points.

HTTP-derived reachability is now generated separately at resolution time, keeping Sandbox.net_allow explicit-only. I've also removed the net_allow_explicit flag and the associated compatibility machinery.

I bumped IMAGE_VERSION to 3 to prevent older checkpoints from being misinterpreted under the new policy semantics.

The Rust library tests pass locally (802 passed, 0 failed). The Linux integration tests still need CI validation, as my WSL host only supports Landlock ABI v3.

The changes are pushed, and I've updated the PR description accordingly. Could you approve the pending CI run when you get a chance?

Also, if there are any other issues or areas of Sandlock where you could use an extra pair of hands, I'd be happy to help. I'm keen to keep contributing to the project!

Thanks again!

congwang-mk

Thanks, both design points are addressed and the separation looks right. Remaining items are inline. One that has no line to anchor to:

Commit history. Please squash into 2 or 3 logical commits. Right now there are fixup commits, one with the bare subject docs:, early commits that add net_allow_explicit only for later ones to remove it, and the body of 884356f says "IMAGE_VERSION stays 2" which the next commit contradicts. Every commit also needs a Signed-off-by (git commit -s).

congwang-mk · 2026-09-19T21:56:53Z

 /// could OOM/abort, taking down every sandbox. The child sends with a bogus 4 GiB
-/// `addr_len` via raw `libc.sendto`. The supervisor must (a) survive — the whole
-/// run completes — and (b) reject the oversized length with `EINVAL` (22),
+/// `addr_len` via raw `libc.sendto`. The supervisor must (a) survive â€” the whole


Encoding damage: 24 comment lines in this file that the PR does not otherwise touch had their dashes turned into mojibake (looks like a Windows editor re-save). Please revert all of those hunks.

congwang-mk · 2026-09-19T21:56:53Z

        let pid_overrides = Arc::new(RwLock::new(HashMap::new()));
        let denied = Arc::new(crate::seccomp::state::DeniedSet::default());
-        let mut ctx = PolicyContext::new(live.clone(), ceiling, pid_overrides, denied);
+        let mut ctx = PolicyContext::new(


Unrelated churn: these six hunks only re-wrap PolicyContext::new(...), left over from a removed parameter. Please revert, along with the same re-wrap of spawn_policy_fn in sandbox.rs, the let nr move in seccomp/notif.rs, and the docstring deletions in test_policy_fn.rs and python/tests/test_policy_fn.py.

congwang-mk · 2026-09-19T21:56:53Z

+                let sup = Arc::clone(&__sup);
+                let notif_fd = cx.notif_fd;
+                async move {
+                    crate::port_remap::handle_bind(&notif, &sup, notif_fd).await


This block is now indented one level too deep. Only the &sup.network to &sup argument change is needed here.

congwang-mk · 2026-09-19T21:56:53Z

+                .effective_network_policy(
+                    notif.pid,
+                    crate::network::Protocol::Udp,
+                    live_policy.as_ref(),


Behavior change unrelated to #135: main passes None here. With a non-empty policy_fn live allowlist the UDP layer becomes an IP-only allow, denies_everything() turns false, and UDP binds start succeeding under TCP-only rule sets. Please revert to None, or split it out with a test and a justification.

congwang-mk · 2026-09-19T21:56:53Z

-            let per_ip = ips.iter().map(|&ip| (ip, PortAllow::Any)).collect();
+            let per_ip = ips
+                .iter()
+                .map(|ip| (ip.to_canonical(), PortAllow::Any))


to_canonical() is also unrelated to this PR and untested. Please drop it or send it separately with a test.

congwang-mk · 2026-09-19T21:56:53Z

+    legacy priority (per-PID override > live policy > static allowlist) and
+    the static denylist is applied after the allow verdict and always wins,
+    so a dynamic override can never erase it.
+3. **Seccomp COW with `workdir`.** When `workdir` is set, the


The list now has two items numbered 3 (this one and "HTTP host auto-expansion"), and the continuation lines of the new item 2 are indented 4 spaces instead of 3.

congwang-mk · 2026-09-19T21:56:53Z

 prefixes opt other protocols in (`"udp://1.1.1.1:53"`, `"udp://*"`,
 `"icmp://host"`, `"icmp://*"`). `NetDeny` is the inverse (default-allow
-denylist, IP/CIDR targets only, mutually exclusive with `NetAllow`).
+ denylist, IP/CIDR targets only); when both are set, denied destinations win.


Stray leading space here and on the two NetDenyBind lines below.

congwang-mk · 2026-09-19T21:56:53Z

        for p in &base.fs_readable { b = b.fs_read(p); }
        for p in &base.fs_writable { b = b.fs_write(p); }
        for p in &base.fs_denied { b = b.fs_deny(p); }
+        // `Sandbox.net_allow` holds only explicit rules; HTTP reachability is


Nit: this "net_allow holds only explicit rules" note is repeated in main.rs, profile.rs, builder.rs, http.rs, image.rs and twice in sandbox.rs. The field doc on Sandbox.net_allow is enough; please remove the rest.

congwang-mk · 2026-09-19T21:56:53Z

+    /// always activate the layer; HTTP-derived reachability activates it only
+    /// for non-deny policies (HTTP-only stays a restrictive allowlist, while
+    /// deny-only+HTTP stays default-allow with the HTTP proxy on top).
+    pub fn net_allow_is_active(&self) -> bool {


Nit: this is pub only so the integration tests can assert on it. Prefer pub(crate) and let those tests assert through behavior (they already do), or mark it #[doc(hidden)].

congwang-mk · 2026-09-19T21:56:53Z

-                || ns.bind_deny_ports.contains(&port)
-        }
-        None => false,
+    // Non-IP families: bind verbatim — there is no TCP port policy to apply.


Nit: please avoid em dashes in new comments (here and at line 202); use a colon or a comma.

Allow outbound net_allow and net_deny together with allow-minus-deny semantics: a destination must match the allow layer and must not match the deny layer. Same precedence for net_allow_bind and net_deny_bind, and the static deny layer runs after the allow verdict so dynamic policy_fn overrides can never erase it. Keep HTTP-derived reachability out of Sandbox.net_allow; generate at resolution time from http_allow, http_deny and http_ports and merge only where rules are consumed, with mode derived from separated config. Address review: restore None for UDP live-policy query in port_remap, drop unrelated to_canonical, formatting churn, mojibake, dead code and duplicate test, fix dispatch indent, helper visibility, docs, numbering and whitespace, and remove em dashes from new comments and docs. Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>

v3 stores only explicit net_allow rules; v2 images may contain HTTP-derived reachability inside net_allow, which v3 would misread as explicit rules (for example deny-only plus HTTP loading as combined), so v2 is rejected outright with no compatibility fallback (pre-1.0 policy). Add round-trip coverage for combined, deny-only plus HTTP and HTTP-only policies. Signed-off-by: Balogun Feranmi <bferanmi806@gmail.com>

bferanmi806-sketch · 2026-09-20T06:42:50Z

Thx i addedd the fix if there are any road map items or issues u need help with feel free to assign them to me

bferanmi806-sketch force-pushed the issue-135-combined-net-policy branch from e8b8e4a to 7eb6fa8 Compare September 18, 2026 09:40

congwang-mk reviewed Sep 19, 2026

View reviewed changes

bferanmi806-sketch added 2 commits September 19, 2026 23:31

bferanmi806-sketch force-pushed the issue-135-combined-net-policy branch from e88b61b to 5e57081 Compare September 19, 2026 22:32

Conversation

bferanmi806-sketch commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

congwang-mk commented Sep 17, 2026

Uh oh!

bferanmi806-sketch commented Sep 18, 2026

Uh oh!

congwang-mk commented Sep 18, 2026

Uh oh!

bferanmi806-sketch commented Sep 19, 2026

Uh oh!

congwang-mk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bferanmi806-sketch commented Sep 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bferanmi806-sketch commented Sep 14, 2026 •

edited

Loading