From 8824b6d75c4dd13cf0d68549356f6d6ac53c4017 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 4 May 2026 18:05:21 -0700 Subject: [PATCH 1/2] net-allow: support `*` in port position for any-port wildcard Issue #32 asked for a way to allow unrestricted outbound network access without enumerating every port. This adds a `*` wildcard in the port position of `--net-allow` specs, with three useful forms: *:* / :* any host, any port (fully unrestricted egress) host:* any port to host (other hosts still blocked) host:80,* rejected (no mixing wildcard with concrete ports) NetAllow gains an `all_ports: bool` field; the parser sets it when it sees the `*` token and rejects mixed specs. ResolvedNetAllow exposes `per_ip_all_ports` (IPs from `host:*` rules after DNS resolution) and `any_ip_all_ports` (true when any rule is `:*`). Signed-off-by: Cong Wang --- README.md | 35 +++-- crates/sandlock-core/src/landlock.rs | 46 +++++-- crates/sandlock-core/src/network.rs | 95 ++++++++++++- crates/sandlock-core/src/policy.rs | 128 +++++++++++++++++- crates/sandlock-core/src/sandbox.rs | 20 ++- .../tests/integration/test_network.rs | 89 ++++++++++++ 6 files changed, 377 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 4fc9789..a7a12ee 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,13 @@ sandlock run --net-allow api.openai.com:443 -r /usr -r /lib -r /etc -- python3 a sandlock run --net-allow github.com:22,443 --net-allow :8080 \ -r /usr -r /lib -r /etc -- python3 agent.py +# Wildcard port — `host:*` permits every port to the host +sandlock run --net-allow github.com:* -r /usr -r /lib -r /etc -- ssh user@github.com + +# Unrestricted outbound — `:*` opens any host and any port. UDP socket +# creation is still gated by --allow-udp; pair the two for full egress. +sandlock run --net-allow :* --allow-udp -r /usr -r /lib -r /etc -- ./client + # UDP — opt in to UDP and allowlist the destination (e.g. DNS) sandlock run --allow-udp --net-allow 1.1.1.1:53 --net-allow :443 \ -r /usr -r /lib -r /etc -- ./client @@ -517,23 +524,32 @@ one rule. The same allowlist applies to TCP `connect()` and to UDP ``` --net-allow repeatable; no rules = deny all outbound = host:port[,port,...] (IP-restricted) - | :port | *:port (any IP) + | :port | *:port (any IP, listed port) + | host:* (host, any port) + | :* | *:* (any IP, any port) ``` **Defaults.** With no `--net-allow` and no HTTP ACL flags, Landlock denies every TCP `connect()`, UDP and raw socket creation are denied -at the seccomp layer, and there is no on-behalf path active. There is -no "allow-all networking" mode — opt in with explicit endpoints. +at the seccomp layer, and there is no on-behalf path active. For +unrestricted egress, opt in explicitly with `--net-allow :*` (still +UDP-gated by `--allow-udp`). **Resolution.** Concrete hostnames are resolved once at sandbox start and pinned in a synthetic `/etc/hosts`. The synthetic file replaces the real one only when `--net-allow` includes at least one concrete host; pure `:port` rules leave the real `/etc/hosts` and DNS visible. -**Wildcards.** Hostnames are matched literally. `--net-allow -*.example.com:443` is **not** supported — list each domain you need. -The `*` form is only valid as the host part of a `*:port` rule (alias -for `:port`). +**Wildcards.** Hostnames are matched literally — `--net-allow +*.example.com:443` is **not** supported, list each domain you need. +The `*` token is allowed in two positions: as the host (alias for +empty: `*:port` ≡ `:port`) and as the port to mean "any port" +(`host:*`, `:*`, `*:*`). Mixing `*` with concrete ports +(`host:80,*`) is rejected — use either the wildcard or an explicit +list. When any rule uses the all-ports wildcard, Landlock no longer +filters TCP connect at the kernel level (it cannot express "every +port" without enumerating 65535 rules); the on-behalf path becomes +the sole enforcer, and for `:*` it short-circuits to allow-all. **Implementation.** Two enforcement paths: @@ -635,8 +651,9 @@ Policy( allow_syscalls=None, # Allowlist mode (stricter) # Network — see "Network Model" above. Each entry is `host:port[,port,...]`, - # `:port`, or `*:port`. Empty list = deny all outbound. Same allowlist - # gates UDP destinations when allow_udp=True (e.g. `:53` for DNS). + # `:port`, `*:port`, `host:*`, or `:*` / `*:*`. Empty list = deny all + # outbound; `:*` = unrestricted. Same allowlist gates UDP destinations + # when allow_udp=True (e.g. `:53` for DNS). net_allow=["api.example.com:443", "github.com:22,443", ":8080"], net_bind=[8080], # TCP bind ports (Landlock; ABI v4+) diff --git a/crates/sandlock-core/src/landlock.rs b/crates/sandlock-core/src/landlock.rs index 61a937a..80db874 100644 --- a/crates/sandlock-core/src/landlock.rs +++ b/crates/sandlock-core/src/landlock.rs @@ -195,9 +195,21 @@ pub fn confine(policy: &Policy) -> Result<(), SandlockError> { // Step 2 -- build handled_access_fs / handled_access_net / scoped. let handled_access_fs = base_fs_access(abi); - // Always restrict TCP bind/connect via Landlock. Empty net_bind/net_connect - // means deny all — same semantics as fs_readable/fs_writable. - let handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP | LANDLOCK_ACCESS_NET_CONNECT_TCP; + // Restrict TCP bind/connect via Landlock by default. When any + // `--net-allow` rule has the all-ports wildcard (`host:*` or + // `:*`), Landlock cannot express "every port" without enumerating + // 65535 rules, so we drop CONNECT_TCP from the handled set — + // unhandled access is unrestricted by Landlock. The on-behalf + // path (seccomp notif on connect/sendto/sendmsg) still enforces + // the per-rule IP allowlist when the rule is `host:*`. For `:*` + // the on-behalf path becomes `NetworkPolicy::Unrestricted` (no + // additional check). Bind enforcement is unaffected. + let net_wildcard = policy.net_allow.iter().any(|r| r.all_ports); + let handled_access_net = if net_wildcard { + LANDLOCK_ACCESS_NET_BIND_TCP + } else { + LANDLOCK_ACCESS_NET_BIND_TCP | LANDLOCK_ACCESS_NET_CONNECT_TCP + }; // IPC and signal isolation are always enabled. let scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET | LANDLOCK_SCOPE_SIGNAL; @@ -286,19 +298,25 @@ pub fn confine(policy: &Policy) -> Result<(), SandlockError> { // the kernel rejects before seccomp gets a chance to dispatch. Allow // every port that any --net-allow rule mentions, plus every HTTP // intercept port; the on-behalf check ensures the IP also matches. - let mut connect_ports: std::collections::HashSet = std::collections::HashSet::new(); - for rule in &policy.net_allow { - for &p in &rule.ports { + // + // When `net_wildcard` is set we already excluded CONNECT_TCP from + // `handled_access_net`, so adding rules here would fail with EINVAL. + // Skip — the on-behalf path is the sole enforcer. + if !net_wildcard { + let mut connect_ports: std::collections::HashSet = std::collections::HashSet::new(); + for rule in &policy.net_allow { + for &p in &rule.ports { + connect_ports.insert(p); + } + } + for &p in &policy.http_ports { connect_ports.insert(p); } - } - for &p in &policy.http_ports { - connect_ports.insert(p); - } - for port in connect_ports { - add_net_rule(&ruleset_fd, port, LANDLOCK_ACCESS_NET_CONNECT_TCP).map_err(|e| { - SandlockError::Sandbox(crate::error::SandboxError::Confinement(e)) - })?; + for port in connect_ports { + add_net_rule(&ruleset_fd, port, LANDLOCK_ACCESS_NET_CONNECT_TCP).map_err(|e| { + SandlockError::Sandbox(crate::error::SandboxError::Confinement(e)) + })?; + } } // Step 6 — enforce (irreversible). diff --git a/crates/sandlock-core/src/network.rs b/crates/sandlock-core/src/network.rs index cf3dc6e..02f6068 100644 --- a/crates/sandlock-core/src/network.rs +++ b/crates/sandlock-core/src/network.rs @@ -603,10 +603,20 @@ pub(crate) async fn handle_net( /// Resolved form of `Policy::net_allow`, ready for the on-behalf path. pub struct ResolvedNetAllow { /// Per-IP port rules (each concrete-host entry resolves to one or - /// more IPs). + /// more IPs). An IP appearing here with an empty port set means + /// "all ports for this IP" (from a `host:*` rule). pub per_ip: HashMap>, + /// IPs permitted on every port (from `host:*` rules after host + /// resolution). The on-behalf path treats these the same as + /// `PortAllow::Any` — the entry in `per_ip` is kept as a + /// placeholder for diagnostic / `/etc/hosts` purposes. + pub per_ip_all_ports: HashSet, /// Ports permitted to any IP (the `:port` form). pub any_ip_ports: HashSet, + /// Any-host any-port wildcard (`:*` / `*:*`). When true, the + /// sandbox is fully unrestricted on outbound TCP/UDP and the + /// on-behalf path is bypassed (`NetworkPolicy::Unrestricted`). + pub any_ip_all_ports: bool, /// Synthetic `/etc/hosts` content for any concrete hostnames. /// `None` when no concrete hostnames are present (real `/etc/hosts` /// stays visible). @@ -618,15 +628,21 @@ pub async fn resolve_net_allow( rules: &[crate::policy::NetAllow], ) -> io::Result { let mut per_ip: HashMap> = HashMap::new(); + let mut per_ip_all_ports: HashSet = HashSet::new(); let mut any_ip_ports: HashSet = HashSet::new(); + let mut any_ip_all_ports = false; let mut etc_hosts = String::from("127.0.0.1 localhost\n::1 localhost\n"); let mut has_concrete_host = false; for rule in rules { match &rule.host { None => { - for &p in &rule.ports { - any_ip_ports.insert(p); + if rule.all_ports { + any_ip_all_ports = true; + } else { + for &p in &rule.ports { + any_ip_ports.insert(p); + } } } Some(host) => { @@ -640,9 +656,17 @@ pub async fn resolve_net_allow( })?; for socket_addr in resolved { let ip = socket_addr.ip(); - let entry = per_ip.entry(ip).or_default(); - for &p in &rule.ports { - entry.insert(p); + if rule.all_ports { + per_ip_all_ports.insert(ip); + // Keep an entry in per_ip so callers iterating + // resolved hosts still see this IP. The runtime + // policy honors per_ip_all_ports first. + per_ip.entry(ip).or_default(); + } else { + let entry = per_ip.entry(ip).or_default(); + for &p in &rule.ports { + entry.insert(p); + } } etc_hosts.push_str(&format!("{} {}\n", ip, host)); } @@ -652,7 +676,9 @@ pub async fn resolve_net_allow( Ok(ResolvedNetAllow { per_ip, + per_ip_all_ports, any_ip_ports, + any_ip_all_ports, etc_hosts: if has_concrete_host { Some(etc_hosts) } else { None }, }) } @@ -679,6 +705,7 @@ mod tests { let rules = vec![NetAllow { host: Some("localhost".to_string()), ports: vec![80, 443], + all_ports: false, }]; let resolved = resolve_net_allow(&rules).await.unwrap(); // localhost should resolve to at least one loopback addr. @@ -692,10 +719,64 @@ mod tests { #[tokio::test] async fn test_resolve_net_allow_any_ip() { - let rules = vec![NetAllow { host: None, ports: vec![8080] }]; + let rules = vec![NetAllow { host: None, ports: vec![8080], all_ports: false }]; let resolved = resolve_net_allow(&rules).await.unwrap(); assert!(resolved.per_ip.is_empty()); assert!(resolved.any_ip_ports.contains(&8080)); + assert!(!resolved.any_ip_all_ports); assert!(resolved.etc_hosts.is_none()); } + + #[tokio::test] + async fn test_resolve_net_allow_any_ip_all_ports() { + // `:*` — fully unrestricted egress. + let rules = vec![NetAllow { host: None, ports: vec![], all_ports: true }]; + let resolved = resolve_net_allow(&rules).await.unwrap(); + assert!(resolved.any_ip_all_ports); + assert!(resolved.per_ip.is_empty()); + assert!(resolved.per_ip_all_ports.is_empty()); + assert!(resolved.any_ip_ports.is_empty()); + } + + #[tokio::test] + async fn test_resolve_net_allow_concrete_host_all_ports() { + // `localhost:*` — every port to localhost only. + let rules = vec![NetAllow { + host: Some("localhost".to_string()), + ports: vec![], + all_ports: true, + }]; + let resolved = resolve_net_allow(&rules).await.unwrap(); + assert!(!resolved.any_ip_all_ports); + assert!(!resolved.per_ip_all_ports.is_empty(), + "localhost should resolve to at least one IP marked as any-port"); + // per_ip has placeholder entries for the same IPs (so callers + // iterating per_ip still see them). + for ip in resolved.per_ip_all_ports.iter() { + assert!(resolved.per_ip.contains_key(ip)); + } + // /etc/hosts is synthesized for concrete hosts. + assert!(resolved.etc_hosts.is_some()); + } + + #[tokio::test] + async fn test_resolve_net_allow_mixed_wildcard_and_concrete() { + // Wildcard rule alongside concrete: wildcard sets the global + // any-host any-port flag; concrete rule still resolves into + // per_ip (the runtime layer chooses Unrestricted, ignoring the + // concrete entries — that's a runtime-policy concern, not a + // resolver concern). + let rules = vec![ + NetAllow { host: None, ports: vec![], all_ports: true }, + NetAllow { + host: Some("localhost".to_string()), + ports: vec![22], + all_ports: false, + }, + ]; + let resolved = resolve_net_allow(&rules).await.unwrap(); + assert!(resolved.any_ip_all_ports); + // Concrete entries still present in per_ip. + assert!(!resolved.per_ip.is_empty()); + } } diff --git a/crates/sandlock-core/src/policy.rs b/crates/sandlock-core/src/policy.rs index eb4fd52..b2da54c 100644 --- a/crates/sandlock-core/src/policy.rs +++ b/crates/sandlock-core/src/policy.rs @@ -84,12 +84,22 @@ pub enum BranchAction { pub struct NetAllow { /// Hostname; `None` means "any IP" (the `:port` form). pub host: Option, - /// Permitted ports. Must be non-empty. + /// Permitted ports. Must be non-empty unless `all_ports` is true, + /// in which case it must be empty. pub ports: Vec, + /// "Any port" wildcard from the `*` token in port position. When + /// true, `ports` is empty; the rule permits every TCP/UDP port to + /// the host (or to any IP, when `host` is `None`). + #[serde(default)] + pub all_ports: bool, } impl NetAllow { - /// Parse a `host:port[,port,...]` / `:port` / `*:port` spec. + /// Parse a `host:port[,port,...]` / `:port` / `*:port` / + /// `host:*` / `:*` / `*:*` spec. + /// + /// `*` in port position means "any port" (the all-ports wildcard). + /// Mixing `*` with concrete ports (e.g. `host:80,*`) is rejected. pub fn parse(s: &str) -> Result { let (host_part, port_part) = s.rsplit_once(':').ok_or_else(|| { PolicyError::Invalid(format!( @@ -101,9 +111,18 @@ impl NetAllow { "" | "*" => None, h => Some(h.to_string()), }; + + // Detect the wildcard token. We split on ',' first so a + // single `*` is a clean match — `*,80` is rejected explicitly + // below rather than letting `*` parse as port 0. let mut ports = Vec::new(); + let mut saw_wildcard = false; for p in port_part.split(',') { let p = p.trim(); + if p == "*" { + saw_wildcard = true; + continue; + } let n: u16 = p.parse().map_err(|_| { PolicyError::Invalid(format!("--net-allow: invalid port `{}` in `{}`", p, s)) })?; @@ -115,13 +134,19 @@ impl NetAllow { } ports.push(n); } - if ports.is_empty() { + if saw_wildcard && !ports.is_empty() { + return Err(PolicyError::Invalid(format!( + "--net-allow: cannot mix `*` with concrete ports in `{}`", + s + ))); + } + if !saw_wildcard && ports.is_empty() { return Err(PolicyError::Invalid(format!( "--net-allow: at least one port required in `{}`", s ))); } - Ok(NetAllow { host, ports }) + Ok(NetAllow { host, ports, all_ports: saw_wildcard }) } } @@ -795,12 +820,14 @@ impl PolicyBuilder { net_allow.push(NetAllow { host: None, ports: http_ports.clone(), + all_ports: false, }); } for h in concrete_hosts { net_allow.push(NetAllow { host: Some(h), ports: http_ports.clone(), + all_ports: false, }); } } @@ -1156,4 +1183,97 @@ mod http_rule_tests { let rule = HttpRule::parse("GET example.com/v1//models").unwrap(); assert_eq!(rule.path, "/v1/models"); } + + // --- NetAllow::parse tests --- + + #[test] + fn netallow_parse_concrete_host_port() { + let r = NetAllow::parse("example.com:443").unwrap(); + assert_eq!(r.host.as_deref(), Some("example.com")); + assert_eq!(r.ports, vec![443]); + assert!(!r.all_ports); + } + + #[test] + fn netallow_parse_any_host_port() { + let r = NetAllow::parse(":8080").unwrap(); + assert_eq!(r.host, None); + assert_eq!(r.ports, vec![8080]); + assert!(!r.all_ports); + + let r = NetAllow::parse("*:8080").unwrap(); + assert_eq!(r.host, None); + assert_eq!(r.ports, vec![8080]); + assert!(!r.all_ports); + } + + #[test] + fn netallow_parse_multiple_ports() { + let r = NetAllow::parse("github.com:22,80,443").unwrap(); + assert_eq!(r.host.as_deref(), Some("github.com")); + assert_eq!(r.ports, vec![22, 80, 443]); + assert!(!r.all_ports); + } + + #[test] + fn netallow_parse_wildcard_any_host_any_port_colon() { + let r = NetAllow::parse(":*").unwrap(); + assert_eq!(r.host, None); + assert!(r.ports.is_empty()); + assert!(r.all_ports); + } + + #[test] + fn netallow_parse_wildcard_any_host_any_port_star() { + let r = NetAllow::parse("*:*").unwrap(); + assert_eq!(r.host, None); + assert!(r.ports.is_empty()); + assert!(r.all_ports); + } + + #[test] + fn netallow_parse_wildcard_concrete_host_any_port() { + let r = NetAllow::parse("example.com:*").unwrap(); + assert_eq!(r.host.as_deref(), Some("example.com")); + assert!(r.ports.is_empty()); + assert!(r.all_ports); + } + + #[test] + fn netallow_parse_rejects_mixed_wildcard_and_concrete() { + // `host:80,*` and `host:*,80` are both ambiguous: the user + // either meant "any port" (wildcard wins) or "ports 80 plus + // some weird placeholder". Refuse and force a clean spec. + let err = NetAllow::parse("example.com:80,*").unwrap_err(); + assert!(format!("{}", err).contains("cannot mix")); + let err = NetAllow::parse("example.com:*,80").unwrap_err(); + assert!(format!("{}", err).contains("cannot mix")); + } + + #[test] + fn netallow_parse_rejects_port_zero() { + let err = NetAllow::parse("example.com:0").unwrap_err(); + assert!(format!("{}", err).contains("port 0")); + } + + #[test] + fn netallow_parse_rejects_empty_port() { + let err = NetAllow::parse("example.com:").unwrap_err(); + assert!(format!("{}", err).contains("invalid port")); + } + + #[test] + fn netallow_parse_rejects_no_colon() { + let err = NetAllow::parse("example.com").unwrap_err(); + assert!(format!("{}", err).contains("expected")); + } + + #[test] + fn netallow_parse_repeated_wildcard_is_idempotent() { + // `*,*` collapses to a single wildcard — neither token contributes + // a concrete port, so the rule remains "any port". + let r = NetAllow::parse(":*,*").unwrap(); + assert!(r.all_ports); + assert!(r.ports.is_empty()); + } } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 15c5480..7c3903a 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -1036,14 +1036,30 @@ impl Sandbox { // NetworkState let mut net_state = NetworkState::new(); - net_state.network_policy = if self.policy.net_allow.is_empty() { + net_state.network_policy = if self.policy.net_allow.is_empty() + || resolved_net_allow.any_ip_all_ports + { + // Empty net_allow leaves only Landlock to enforce + // (kernel-level deny-all-connect by default). The + // `:*` wildcard explicitly opens egress, so the + // on-behalf path becomes a no-op. crate::seccomp::notif::NetworkPolicy::Unrestricted } else { use crate::seccomp::notif::PortAllow; let per_ip = resolved_net_allow .per_ip .iter() - .map(|(ip, ports)| (*ip, PortAllow::Specific(ports.clone()))) + .map(|(ip, ports)| { + // `host:*` resolves into per_ip_all_ports; for those + // IPs we use PortAllow::Any regardless of the (empty) + // port set in per_ip. + let allow = if resolved_net_allow.per_ip_all_ports.contains(ip) { + PortAllow::Any + } else { + PortAllow::Specific(ports.clone()) + }; + (*ip, allow) + }) .collect(); crate::seccomp::notif::NetworkPolicy::AllowList { per_ip, diff --git a/crates/sandlock-core/tests/integration/test_network.rs b/crates/sandlock-core/tests/integration/test_network.rs index 6fd3dc5..ba26606 100644 --- a/crates/sandlock-core/tests/integration/test_network.rs +++ b/crates/sandlock-core/tests/integration/test_network.rs @@ -226,3 +226,92 @@ async fn test_grandchild_network_connect() { srv.join().unwrap(); let _ = std::fs::remove_file(&out); } + +/// `--net-allow :*` opens egress to every host and port. Verifies +/// that both the Landlock side (CONNECT_TCP unhandled) and the +/// on-behalf side (NetworkPolicy::Unrestricted) cooperate to allow a +/// connection to a port that no concrete rule mentions. Issue #32. +#[tokio::test] +async fn test_net_allow_wildcard_any_host_any_port() { + let out = temp_file("wildcard-any"); + + // Stand up a server on a port nothing else mentions. + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let srv = std::thread::spawn(move || { + let (mut conn, _) = listener.accept().unwrap(); + let _ = std::io::Write::write_all(&mut conn, b"ok"); + }); + + let policy = base_policy().net_allow(":*").build().unwrap(); + + let script = format!(concat!( + "import socket\n", + "s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)\n", + "s.settimeout(5)\n", + "s.connect(('127.0.0.1', {port}))\n", + "data = s.recv(16)\n", + "s.close()\n", + "open('{out}', 'w').write(data.decode())\n", + ), out = out.display(), port = port); + + let result = Sandbox::run_interactive(&policy, &["python3", "-c", &script]).await.unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert_eq!(content, "ok", "wildcard :* should permit arbitrary egress"); + + srv.join().unwrap(); + let _ = std::fs::remove_file(&out); +} + +/// `--net-allow host:*` permits every port to the host but still +/// blocks other hosts. Verifies the Landlock-unhandled + on-behalf +/// per_ip_all_ports path keeps the IP allowlist intact. +#[tokio::test] +async fn test_net_allow_wildcard_host_only() { + let out = temp_file("wildcard-host"); + + // Server on localhost; a non-localhost connect must still be blocked. + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let srv = std::thread::spawn(move || { + let (mut conn, _) = listener.accept().unwrap(); + let _ = std::io::Write::write_all(&mut conn, b"ok"); + }); + + let policy = base_policy().net_allow("localhost:*").build().unwrap(); + + let script = format!(concat!( + "import socket\n", + "results = []\n", + // localhost any port — should connect + "s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)\n", + "s.settimeout(5)\n", + "try:\n", + " s.connect(('127.0.0.1', {port}))\n", + " results.append('local:ok')\n", + " s.close()\n", + "except OSError as e:\n", + " results.append(f'local:err{{e.errno}}')\n", + // non-localhost — should be blocked + "s2 = socket.socket(socket.AF_INET, socket.SOCK_STREAM)\n", + "s2.settimeout(2)\n", + "try:\n", + " s2.connect(('1.1.1.1', 80))\n", + " results.append('remote:ALLOWED')\n", + " s2.close()\n", + "except (OSError, socket.timeout):\n", + " results.append('remote:blocked')\n", + "open('{out}', 'w').write(','.join(results))\n", + ), out = out.display(), port = port); + + let result = Sandbox::run_interactive(&policy, &["python3", "-c", &script]).await.unwrap(); + assert!(result.success(), "exit={:?}", result.code()); + let content = std::fs::read_to_string(&out).unwrap_or_default(); + assert!(content.contains("local:ok"), "localhost should connect; got: {}", content); + assert!(content.contains("remote:blocked"), + "remote host must remain blocked under host:* wildcard; got: {}", content); + + srv.join().unwrap(); + let _ = std::fs::remove_file(&out); +} From 8e89129623f205d8448ee2e024ad417afefc082c Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 4 May 2026 18:31:20 -0700 Subject: [PATCH 2/2] ffi: surface PolicyBuilder::build() errors to Python via err_msg out-param Signed-off-by: Cong Wang --- crates/sandlock-ffi/include/sandlock.h | 9 ++++++++- crates/sandlock-ffi/src/lib.rs | 28 ++++++++++++++++++++++---- python/src/sandlock/_sdk.py | 24 +++++++++++++++++++--- 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/crates/sandlock-ffi/include/sandlock.h b/crates/sandlock-ffi/include/sandlock.h index b8eba35..ce2f10d 100644 --- a/crates/sandlock-ffi/include/sandlock.h +++ b/crates/sandlock-ffi/include/sandlock.h @@ -65,8 +65,15 @@ sandlock_builder_t *sandlock_policy_builder_no_randomize_memory(sandlock_builder sandlock_builder_t *sandlock_policy_builder_no_huge_pages(sandlock_builder_t *b, bool v); /* Build & free */ -sandlock_policy_t *sandlock_policy_build(sandlock_builder_t *b, int *err); +/* On failure, *err is set to -1 and *err_msg (if non-null) is set to a + * heap-allocated C string with the error description. Caller frees it + * via sandlock_string_free. Pass NULL for err_msg to discard. */ +sandlock_policy_t *sandlock_policy_build(sandlock_builder_t *b, + int *err, + char **err_msg); void sandlock_policy_free(sandlock_policy_t *p); +/* sandlock_string_free is declared further down — used for any + * heap-allocated C string the FFI returns to the caller. */ /* ---------------------------------------------------------------- * Run diff --git a/crates/sandlock-ffi/src/lib.rs b/crates/sandlock-ffi/src/lib.rs index df03117..fac8ec3 100644 --- a/crates/sandlock-ffi/src/lib.rs +++ b/crates/sandlock-ffi/src/lib.rs @@ -601,15 +601,26 @@ pub unsafe extern "C" fn sandlock_policy_builder_hostname( /// Consume the builder and produce a policy. /// On success, `*err` is 0 and a non-null policy pointer is returned. -/// On failure, `*err` is -1 and null is returned. +/// On failure, `*err` is -1, null is returned, and `*err_msg` (if +/// non-null) is set to a heap-allocated C string describing the +/// error. The caller must release that string with +/// [`sandlock_string_free`]. Pass `null` for `err_msg` to discard. /// /// # Safety -/// `b` must be a valid builder pointer. `err` may be null. +/// `b` must be a valid builder pointer. `err` and `err_msg` may both +/// be null. When `err_msg` is non-null, it must point to writable +/// storage for one `*mut c_char`. #[no_mangle] pub unsafe extern "C" fn sandlock_policy_build( - b: *mut PolicyBuilder, err: *mut c_int, + b: *mut PolicyBuilder, err: *mut c_int, err_msg: *mut *mut c_char, ) -> *mut sandlock_policy_t { + if !err_msg.is_null() { *err_msg = ptr::null_mut(); } if b.is_null() { + // Null-builder is a programmer error in the binding layer, + // not a policy-validation failure. We surface the err code + // but deliberately leave err_msg null — there is no + // user-actionable message and inventing one here would + // require a hard-coded literal living in the wrong layer. if !err.is_null() { *err = -1; } return ptr::null_mut(); } @@ -619,8 +630,17 @@ pub unsafe extern "C" fn sandlock_policy_build( if !err.is_null() { *err = 0; } Box::into_raw(Box::new(sandlock_policy_t { _private: policy })) } - Err(_) => { + Err(e) => { if !err.is_null() { *err = -1; } + if !err_msg.is_null() { + // CString::new fails only on embedded NULs; thiserror + // Display impls don't produce NULs in this codebase, + // so on the impossible failure we leave err_msg null + // rather than substituting an invented string. + if let Ok(c) = CString::new(format!("{}", e)) { + *err_msg = c.into_raw(); + } + } ptr::null_mut() } } diff --git a/python/src/sandlock/_sdk.py b/python/src/sandlock/_sdk.py index c02a414..e0dc2bf 100644 --- a/python/src/sandlock/_sdk.py +++ b/python/src/sandlock/_sdk.py @@ -207,11 +207,21 @@ def confine(policy: "PolicyDataclass") -> None: _lib.sandlock_policy_build.restype = _c_policy_p -_lib.sandlock_policy_build.argtypes = [_c_builder_p, ctypes.POINTER(ctypes.c_int)] +_lib.sandlock_policy_build.argtypes = [ + _c_builder_p, + ctypes.POINTER(ctypes.c_int), + ctypes.POINTER(ctypes.c_char_p), +] _lib.sandlock_policy_free.restype = None _lib.sandlock_policy_free.argtypes = [_c_policy_p] +# String-out-param release. The FFI returns CString::into_raw pointers +# for error messages from sandlock_policy_build; we must free them via +# this function rather than ctypes' own deallocator. +_lib.sandlock_string_free.restype = None +_lib.sandlock_string_free.argtypes = [ctypes.c_char_p] + # Run _lib.sandlock_run.restype = _c_result_p _lib.sandlock_run.argtypes = [_c_policy_p, ctypes.POINTER(ctypes.c_char_p), ctypes.c_uint] @@ -947,9 +957,17 @@ def _c_callback(event_p, ctx_p): b = _lib.sandlock_policy_builder_policy_fn(b, c_callback) err = ctypes.c_int(0) - ptr = _lib.sandlock_policy_build(b, ctypes.byref(err)) + err_msg = ctypes.c_char_p() + ptr = _lib.sandlock_policy_build(b, ctypes.byref(err), ctypes.byref(err_msg)) if not ptr or err.value != 0: - raise RuntimeError("Failed to build policy") + # err_msg.value is a copy of the C string's bytes; the + # underlying allocation still needs releasing afterwards. + # When the FFI leaves err_msg null (e.g. internal binding + # bug), raise without a message rather than inventing one. + msg = err_msg.value.decode("utf-8", "replace") if err_msg.value else None + if err_msg.value: + _lib.sandlock_string_free(err_msg) + raise RuntimeError(msg) if msg else RuntimeError() native = _NativePolicy(ptr) native._c_callback = c_callback # prevent GC return native