From 3bfb0f866b0c8338096242297e2a9daee42704a0 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 10 Aug 2026 17:30:43 -0700 Subject: [PATCH] Fix connected UDP learning Allow TCP-only network profiles to create UDP sockets while keeping actual UDP traffic gated at connect/send/bind time. This lets libc DNS/address-selection probes work without opening UDP egress. Teach sandlock learn to promote a pending UDP connect when an address-less UDP send is observed, including when connect and send occur on sibling threads sharing the same fd table. Add learn-to-run connected UDP regression coverage and runtime tests proving TCP-only rules still deny UDP send, connect, and bind operations. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 33 ++++++- .../sandlock-cli/tests/learn_integration.rs | 92 +++++++++++++++++++ crates/sandlock-core/src/context.rs | 2 +- crates/sandlock-core/src/context/tests.rs | 35 +++++-- crates/sandlock-core/src/port_remap.rs | 18 ++++ crates/sandlock-core/src/resolved.rs | 9 +- crates/sandlock-core/src/sandbox.rs | 15 +-- crates/sandlock-core/src/seccomp/notif.rs | 12 +++ crates/sandlock-core/src/seccomp_plan.rs | 22 +++-- .../tests/integration/test_seccomp_enforce.rs | 60 ++++++++++++ 10 files changed, 265 insertions(+), 33 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index b4c56a95..ad9ec4fd 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -285,8 +285,12 @@ struct LearnObserver { /// /proc//exe. Pins [program].exec to an absolute path; argv[0] may /// be relative (resolved through $PATH by execvp) and useless to `run`. first_exe: Arc>>, - /// UDP connect() calls pending confirmation by a subsequent send(). - /// Key: (pid, fd). Value: formatted "udp://host:port" entry. + /// UDP connect() calls pending confirmation by a subsequent send. + /// Keyed by (TGID, fd) so a connect observed on one thread can be + /// confirmed by traffic from a sibling thread using the same fd table. + /// Connected UDP write(2) is deliberately not observed: trapping write + /// would catch the child bootstrap pipe writes before the parent receives + /// the seccomp listener fd. pending_udp_connects: Arc>>, } @@ -417,6 +421,23 @@ impl LearnObserver { } } "connect" | "sendto" | "sendmsg" | "sendmmsg" => { + // An address-less send on a connected UDP socket is how the + // glibc resolver talks to its nameserver (connect, then + // send with no sockaddr): real traffic, so it promotes the + // parked connect() the same way an addressed send does. + if event.host.is_none() + && event.syscall != "connect" + && event.protocol.as_deref() == Some("udp") + { + if let Some(fd) = event.fd { + let key = (tgid_of(event.pid), fd); + if let Some(pending) = self.pending_udp_connects + .lock().unwrap().remove(&key) + { + self.connects.lock().unwrap().insert(pending); + } + } + } if let (Some(ip), Some(port), Some(proto)) = (event.host, event.port, event.protocol) { if proto != "icmp" { let host = if ip.is_ipv6() { format!("[{ip}]") } else { ip.to_string() }; @@ -427,16 +448,18 @@ impl LearnObserver { // address-sorting probe (e.g. glibc getaddrinfo) and is // discarded when the workload exits. if let Some(fd) = event.fd { + let key = (tgid_of(event.pid), fd); self.pending_udp_connects.lock().unwrap() - .insert((event.pid, fd), entry); + .insert(key, entry); } } else { // For UDP send*, promote any pending connect() on the - // same (pid, fd) to confirmed traffic. + // same process fd to confirmed traffic. if proto == "udp" { if let Some(fd) = event.fd { + let key = (tgid_of(event.pid), fd); if let Some(pending) = self.pending_udp_connects - .lock().unwrap().remove(&(event.pid, fd)) + .lock().unwrap().remove(&key) { self.connects.lock().unwrap().insert(pending); } diff --git a/crates/sandlock-cli/tests/learn_integration.rs b/crates/sandlock-cli/tests/learn_integration.rs index 1918ac33..94b6cec4 100644 --- a/crates/sandlock-cli/tests/learn_integration.rs +++ b/crates/sandlock-cli/tests/learn_integration.rs @@ -4,11 +4,53 @@ // cargo test -p sandlock-cli --test learn_integration -- --test-threads=4 use std::process::Command; +use std::time::Duration; fn sandlock_bin() -> Command { Command::new(env!("CARGO_BIN_EXE_sandlock")) } +fn udp_sink(expected_datagrams: usize) -> (u16, std::thread::JoinHandle) { + let sock = std::net::UdpSocket::bind("127.0.0.1:0").expect("udp sink bind"); + sock.set_read_timeout(Some(Duration::from_secs(5))) + .expect("udp sink timeout"); + let port = sock.local_addr().expect("udp sink addr").port(); + let handle = std::thread::spawn(move || { + let mut buf = [0_u8; 64]; + let mut received = 0; + while received < expected_datagrams { + match sock.recv_from(&mut buf) { + Ok(_) => received += 1, + Err(_) => break, + } + } + received + }); + (port, handle) +} + +fn learn_then_run_script(profile_path: &str, script: &str) { + let learn = sandlock_bin() + .args(["learn", "-o", profile_path, "--", "python3", "-c", script]) + .output() + .expect("failed to run sandlock learn"); + assert!( + learn.status.success(), + "learn failed: {}", + String::from_utf8_lossy(&learn.stderr) + ); + + let run = sandlock_bin() + .args(["run", "--profile-file", profile_path, "--", "python3", "-c", script]) + .output() + .expect("failed to run sandlock run"); + assert!( + run.status.success(), + "run failed: {}", + String::from_utf8_lossy(&run.stderr) + ); +} + /// Learn → run with a read-only workload. #[test] fn test_learn_then_run() { @@ -32,6 +74,56 @@ fn test_learn_then_run() { "expected output from cat /etc/hostname"); } +/// Connected UDP send: an address-less send confirms the earlier UDP connect +/// as real traffic, so the generated profile must replay. +#[test] +fn test_learn_then_run_connected_udp_send() { + let (port, sink) = udp_sink(2); + let profile = tempfile::NamedTempFile::new().expect("tempfile"); + let profile_path = profile.path().to_str().unwrap().to_owned(); + let script = format!( + "import socket\n\ + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)\n\ + s.connect(('127.0.0.1', {port}))\n\ + s.send(b'x')\n\ + s.close()\n" + ); + + learn_then_run_script(&profile_path, &script); + let profile_toml = std::fs::read_to_string(&profile_path).expect("profile"); + assert!( + profile_toml.contains(&format!("udp://127.0.0.1:{port}")), + "expected connected UDP endpoint in profile:\n{profile_toml}" + ); + assert_eq!(sink.join().unwrap(), 2, "learn and run should each send one UDP datagram"); +} + +/// A connect observed on one thread and a send observed on a sibling thread +/// still refer to the same process fd table. +#[test] +fn test_learn_then_run_connected_udp_send_from_thread() { + let (port, sink) = udp_sink(2); + let profile = tempfile::NamedTempFile::new().expect("tempfile"); + let profile_path = profile.path().to_str().unwrap().to_owned(); + let script = format!( + "import socket, threading\n\ + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)\n\ + s.connect(('127.0.0.1', {port}))\n\ + t = threading.Thread(target=lambda: s.send(b'x'))\n\ + t.start()\n\ + t.join()\n\ + s.close()\n" + ); + + learn_then_run_script(&profile_path, &script); + let profile_toml = std::fs::read_to_string(&profile_path).expect("profile"); + assert!( + profile_toml.contains(&format!("udp://127.0.0.1:{port}")), + "expected connected UDP endpoint in profile:\n{profile_toml}" + ); + assert_eq!(sink.join().unwrap(), 2, "learn and run should each send one UDP datagram"); +} + /// Write path: COW isolates during learn; run creates the file for real. #[test] fn test_learn_then_run_write() { diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index a051b6c0..1ee25133 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -14,7 +14,7 @@ use crate::arch; #[cfg(test)] use crate::sys::structs::{ AF_INET, AF_INET6, CLONE_NS_FLAGS, DEFAULT_BLOCKLIST_SYSCALLS, PR_SET_DUMPABLE, - SIOCGIFCONF, SIOCETHTOOL, SOCK_DGRAM, SOCK_RAW, SOCK_TYPE_MASK, TIOCLINUX, TIOCSTI, + SIOCGIFCONF, SIOCETHTOOL, SOCK_RAW, SOCK_TYPE_MASK, TIOCLINUX, TIOCSTI, }; // ============================================================ diff --git a/crates/sandlock-core/src/context/tests.rs b/crates/sandlock-core/src/context/tests.rs index 2cfa1222..91e6cece 100644 --- a/crates/sandlock-core/src/context/tests.rs +++ b/crates/sandlock-core/src/context/tests.rs @@ -332,13 +332,36 @@ fn test_arg_filters_raw_sockets() { #[test] fn test_arg_filters_udp_denied_by_default() { - use crate::sys::structs::{BPF_JEQ, BPF_JMP, BPF_K}; - // UDP is denied by default: no `udp://...` rule in net_allow. + // SOCK_DGRAM creation is denied when no net rule exists: nothing traps + // sends, so socket() is the only enforcement point. let policy = Sandbox::builder().build().unwrap(); - let filters = arg_filters(&policy); - // Should have JEQ SOCK_DGRAM - assert!(filters.iter().any(|f| f.code == (BPF_JMP | BPF_JEQ | BPF_K) - && f.k == SOCK_DGRAM)); + assert_eq!(count_jeq_2(&arg_filters(&policy)), 2); +} + +/// AF_INET and SOCK_DGRAM share the constant 2, so presence of the +/// SOCK_DGRAM JEQ is observed as the JEQ-with-k==2 count: the AF_INET +/// domain check contributes one in every socket filter, the type check +/// contributes the second only when SOCK_DGRAM is in the blocked set. +fn count_jeq_2(filters: &[crate::sys::structs::SockFilter]) -> usize { + use crate::sys::structs::{BPF_JEQ, BPF_JMP, BPF_K}; + filters.iter() + .filter(|f| f.code == (BPF_JMP | BPF_JEQ | BPF_K) && f.k == 2) + .count() +} + +#[test] +fn test_arg_filters_udp_creatable_with_tcp_only_rules() { + // Any net rule moves UDP enforcement to send time: the on-behalf + // handlers deny destinations for rule-less protocols, so socket() + // must succeed (glibc getaddrinfo probes create UDP sockets). + let policy = Sandbox::builder().net_allow("1.1.1.1:443").build().unwrap(); + assert_eq!(count_jeq_2(&arg_filters(&policy)), 1); +} + +#[test] +fn test_arg_filters_udp_creatable_with_net_deny() { + let policy = Sandbox::builder().net_deny("10.0.0.0/8").build().unwrap(); + assert_eq!(count_jeq_2(&arg_filters(&policy)), 1); } #[test] diff --git a/crates/sandlock-core/src/port_remap.rs b/crates/sandlock-core/src/port_remap.rs index 1289c4f0..212de355 100644 --- a/crates/sandlock-core/src/port_remap.rs +++ b/crates/sandlock-core/src/port_remap.rs @@ -151,6 +151,24 @@ pub(crate) async fn handle_bind( Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)), }; + // A UDP bind is an inbound grant that Landlock cannot express + // (BIND_TCP is TCP-only). With no UDP rule the protocol is + // send-time deny-all, and a bound socket would still be a + // receive-only channel — so the bind is refused outright. Checked + // before the port extraction so ephemeral (port 0) binds are + // covered too. + let udp_deny_all = { + let ns = network.lock().await; + ns.effective_network_policy(notif.pid, crate::network::Protocol::Udp, None) + .denies_everything() + }; + if udp_deny_all + && crate::network::query_socket_protocol(dup_fd.as_raw_fd()) + == Some(crate::network::Protocol::Udp) + { + return NotifAction::Errno(libc::EACCES); + } + // Non-IP family or ephemeral (port == 0): bind verbatim — nothing to // track or remap. extract_port returns None for non-IP families and // for truncated buffers; in both cases the kernel will validate. diff --git a/crates/sandlock-core/src/resolved.rs b/crates/sandlock-core/src/resolved.rs index 78d0c08b..634755e6 100644 --- a/crates/sandlock-core/src/resolved.rs +++ b/crates/sandlock-core/src/resolved.rs @@ -1,4 +1,4 @@ -use crate::sandbox::{Protocol, Sandbox}; +use crate::sandbox::Sandbox; /// Internal normalized view of a sandbox configuration. /// @@ -46,7 +46,7 @@ pub(crate) struct SandboxFeatures { pub(crate) http_acl: bool, pub(crate) argv_safety_required: bool, pub(crate) sysv_ipc_allowed: bool, - pub(crate) udp_or_icmp_allowed: bool, + pub(crate) net_allow_present: bool, pub(crate) net_deny: bool, } @@ -84,10 +84,7 @@ impl SandboxFeatures { http_acl, argv_safety_required: sandbox.policy_fn.is_some() || exec_handler, sysv_ipc_allowed: sandbox.allows_sysv_ipc(), - udp_or_icmp_allowed: sandbox - .net_allow - .iter() - .any(|r| matches!(r.protocol, Protocol::Udp | Protocol::Icmp)), + net_allow_present: !sandbox.net_allow.is_empty(), net_deny: !sandbox.net_deny.is_empty(), } } diff --git a/crates/sandlock-core/src/sandbox.rs b/crates/sandlock-core/src/sandbox.rs index 3b4f8109..5c7b2cd0 100644 --- a/crates/sandlock-core/src/sandbox.rs +++ b/crates/sandlock-core/src/sandbox.rs @@ -397,12 +397,15 @@ pub struct Sandbox { /// concrete host or "any IP." TCP and UDP rules carry ports; ICMP /// rules have none. /// - /// **Protocol gating falls out of rule presence.** Sandlock denies - /// UDP and ICMP socket creation by default; opting in is "list at - /// least one rule for that protocol". Scheme-less specs expand to a - /// TCP + UDP rule pair at parse time, so any of them opts UDP in; - /// ICMP always needs an explicit rule (`icmp://*` for any ICMP - /// echo). TCP is always permitted. + /// **Protocol gating falls out of rule presence.** With no network + /// rules at all, Sandlock denies UDP and ICMP socket creation. Once + /// any network destination policy is active, datagram sockets may be + /// created so libc DNS/address-selection probes can run, but actual + /// UDP/ICMP destinations are still denied unless a matching rule for + /// that protocol exists. Scheme-less specs expand to a TCP + UDP rule + /// pair at parse time, so any of them opts UDP traffic in; ICMP always + /// needs an explicit rule (`icmp://*` for any ICMP echo). TCP is + /// always permitted. /// /// Empty `net_allow` and empty `http_allow`/`http_deny` together /// mean "deny all outbound" (Landlock direct path denies, no diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index 8664273c..1973d7e1 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -275,6 +275,18 @@ pub enum NetworkPolicy { } impl NetworkPolicy { + /// True iff no destination can ever match: the allowlist for a protocol + /// nothing was granted to. Distinguishes "deny all" from `Unrestricted` + /// and from a `DenyList` (both default-allow). + pub fn denies_everything(&self) -> bool { + match self { + NetworkPolicy::AllowList { per_ip, cidrs, any_ip_ports } => { + per_ip.is_empty() && cidrs.is_empty() && any_ip_ports.is_empty() + } + _ => false, + } + } + /// True iff a connection to (ip, port) should be permitted. pub fn allows(&self, ip: IpAddr, port: u16) -> bool { // `::ffff:a.b.c.d` is the same destination as `a.b.c.d` (a diff --git a/crates/sandlock-core/src/seccomp_plan.rs b/crates/sandlock-core/src/seccomp_plan.rs index 94bf0e7c..03fc4560 100644 --- a/crates/sandlock-core/src/seccomp_plan.rs +++ b/crates/sandlock-core/src/seccomp_plan.rs @@ -570,17 +570,21 @@ pub(crate) fn arg_filters_resolved(resolved: &ResolvedSandbox) -> Vec = Vec::new(); blocked_types.push(SOCK_RAW); - if !features.udp_or_icmp_allowed && !features.net_deny { + if !features.net_allow_present && !features.net_deny { blocked_types.push(SOCK_DGRAM); } diff --git a/crates/sandlock-core/tests/integration/test_seccomp_enforce.rs b/crates/sandlock-core/tests/integration/test_seccomp_enforce.rs index 578d4dd9..ae1b79e2 100644 --- a/crates/sandlock-core/tests/integration/test_seccomp_enforce.rs +++ b/crates/sandlock-core/tests/integration/test_seccomp_enforce.rs @@ -302,6 +302,66 @@ async fn test_udp_denied_by_default() { assert!(result.success()); } +// ------------------------------------------------------------------ +// 6b. With a TCP-only rule set, UDP moves to send-time gating: the +// socket is creatable (glibc getaddrinfo needs that for its +// address-sorting probes, or name resolution breaks on TCP-only +// profiles), but every send, connect, and bind on it is denied — +// no UDP rule means the protocol's allowlist is empty. +// ------------------------------------------------------------------ +#[tokio::test] +async fn test_udp_send_time_gating_with_tcp_only_rules() { + let out = temp_out("udp-send-gated"); + let script = format!(concat!( + "import socket, json\n", + "res = {{}}\n", + "try:\n", + " s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)\n", + " res['create'] = 'ok'\n", + "except OSError as e:\n", + " res['create'] = 'err:%d' % e.errno\n", + " s = None\n", + "if s is not None:\n", + " try:\n", + " s.sendto(b'x', ('127.0.0.1', 53))\n", + " res['sendto'] = 'ok'\n", + " except OSError as e:\n", + " res['sendto'] = 'err:%d' % e.errno\n", + " try:\n", + " s.connect(('127.0.0.1', 53))\n", + " res['connect'] = 'ok'\n", + " except OSError as e:\n", + " res['connect'] = 'err:%d' % e.errno\n", + " try:\n", + " s.bind(('127.0.0.1', 0))\n", + " res['bind'] = 'ok'\n", + " except OSError as e:\n", + " res['bind'] = 'err:%d' % e.errno\n", + " s.close()\n", + "open('{out}', 'w').write(json.dumps(res))\n", + ), out = out.display()); + + let policy = base_policy() + .net_allow("tcp://127.0.0.1:9") + .build() + .unwrap(); + let result = policy.clone().run_interactive(&["python3", "-c", &script]) + .await + .unwrap(); + + let contents = std::fs::read_to_string(&out).unwrap_or_default(); + let _ = std::fs::remove_file(&out); + assert!(contents.contains("\"create\": \"ok\""), + "UDP socket creation must succeed with net rules present; got: {contents}"); + assert!(contents.contains("\"sendto\": \"err:111\""), + "UDP sendto must be denied with ECONNREFUSED; got: {contents}"); + assert!(contents.contains("\"connect\": \"err:111\""), + "UDP connect must be denied with ECONNREFUSED; got: {contents}"); + assert!(contents.contains("\"bind\": \"err:13\""), + "UDP bind must be denied with EACCES; got: {contents}"); + assert!(result.success()); +} + // ------------------------------------------------------------------ // 7. SysV IPC (shmget) denied by default — sandlock has no IPC // namespace, so the deny is the only thing isolating shm