Merge pull request #154 from dzerik/fix/net-sendto-toctou

fix(net): close a sendto non-IP destination-policy TOCTOU (sendto half of #130)

Author: congwang-mk

crates/sandlock-core/src/network/materialize.rs

@@ -11,6 +11,8 @@ use std::os::unix::io::{AsRawFd, OwnedFd, RawFd};
 use crate::seccomp::notif::read_child_mem;
 use crate::sys::structs::{SeccompNotif, AF_INET, AF_INET6};
 
+use super::verdict::DestShape;
+
 /// Maximum buffer size for sendto/sendmsg on-behalf operations (64 MiB).
 /// Prevents a sandboxed process from triggering OOM in the supervisor.
 pub(crate) const MAX_SEND_BUF: usize = 64 << 20;
@@ -220,6 +222,30 @@ pub(crate) fn named_unix_socket_path(addr_bytes: &[u8]) -> Option<std::path::Pat
     std::str::from_utf8(raw).ok().map(std::path::PathBuf::from)
 }
 
+/// Classify a raw destination `sockaddr` into the [`DestShape`] the non-IP send
+/// arms key off. Pure function of the already-copied bytes: no child state is
+/// re-read, so the shape cannot change after the decision.
+///
+/// The distinction that matters is the destination's ADDRESS FAMILY, not just
+/// "did a pathname come out of it": an `AF_NETLINK` sockaddr and an abstract
+/// `AF_UNIX` sockaddr both yield no pathname, but they must take opposite
+/// paths — see [`DestShape`].
+pub(crate) fn classify_dest_shape(addr_bytes: &[u8]) -> DestShape {
+    // Below two bytes there is no family to read; fail closed by reporting the
+    // shape the caller refuses.
+    if addr_bytes.len() < 2 {
+        return DestShape::UnixNoPath;
+    }
+    let family = u16::from_ne_bytes([addr_bytes[0], addr_bytes[1]]);
+    if family != libc::AF_UNIX as u16 {
+        return DestShape::NotUnix;
+    }
+    match named_unix_socket_path(addr_bytes) {
+        Some(path) => DestShape::UnixNamed(path),
+        None => DestShape::UnixNoPath,
+    }
+}
+
 /// `struct msghdr` size on LP64 Linux (x86_64 / aarch64 / riscv64): four
 /// 8-byte pointer/length fields, one (u32 + pad) `msg_namelen`, and the
 /// trailing (i32 + pad) `msg_flags` = 56 bytes.
@@ -506,4 +532,57 @@ mod tests {
         let v4_family = (AF_INET as u16).to_ne_bytes();
         assert!(!sockaddr_is_ipv6(&[v4_family[0], v4_family[1], 0, 0]));
     }
+
+    /// Build a raw `sockaddr_un` image for `sun_path` (a leading NUL byte in
+    /// `sun_path` makes it an abstract address, exactly as the kernel reads it).
+    fn unix_sockaddr_bytes(sun_path: &[u8]) -> Vec<u8> {
+        let mut out = (libc::AF_UNIX as u16).to_ne_bytes().to_vec();
+        out.extend_from_slice(sun_path);
+        out
+    }
+
+    #[test]
+    fn dest_shape_named_unix_carries_the_path() {
+        assert_eq!(
+            classify_dest_shape(&unix_sockaddr_bytes(b"/run/svc.dgram\0")),
+            DestShape::UnixNamed(std::path::PathBuf::from("/run/svc.dgram"))
+        );
+    }
+
+    #[test]
+    fn dest_shape_pathless_unix_addresses_are_unix_no_path() {
+        // Abstract (leading NUL), unnamed (no path at all), and non-UTF-8
+        // sun_path all land in the arm that fails closed: there is nothing the
+        // supervisor can re-resolve in the child's root view.
+        assert_eq!(
+            classify_dest_shape(&unix_sockaddr_bytes(b"\0abstract-name")),
+            DestShape::UnixNoPath
+        );
+        assert_eq!(
+            classify_dest_shape(&unix_sockaddr_bytes(b"")),
+            DestShape::UnixNoPath
+        );
+        assert_eq!(
+            classify_dest_shape(&unix_sockaddr_bytes(b"/run/\xff\xfe\0")),
+            DestShape::UnixNoPath
+        );
+        // Too short to even carry a family: fail closed, do not guess.
+        assert_eq!(classify_dest_shape(&[1]), DestShape::UnixNoPath);
+    }
+
+    #[test]
+    fn dest_shape_netlink_is_not_unix() {
+        // sandlock's NETLINK_ROUTE virtualization hands the child a
+        // socketpair(AF_UNIX, ...) end that glibc addresses with a
+        // `sockaddr_nl`. That address has no pathname either, but it is NOT
+        // the abstract-unix case: classifying on the family keeps them apart.
+        let mut nl = (libc::AF_NETLINK as u16).to_ne_bytes().to_vec();
+        nl.extend_from_slice(&[0u8; 10]); // pad, nl_pid, nl_groups
+        assert_eq!(classify_dest_shape(&nl), DestShape::NotUnix);
+
+        // Same for any other non-unix, non-IP family.
+        let mut vsock = (libc::AF_VSOCK as u16).to_ne_bytes().to_vec();
+        vsock.extend_from_slice(&[0u8; 14]);
+        assert_eq!(classify_dest_shape(&vsock), DestShape::NotUnix);
+    }
 }

crates/sandlock-core/src/network/mod.rs

@@ -149,7 +149,21 @@ fn socket_is_unix(fd: RawFd) -> bool {
 /// at all (they return ReturnValue/Errno after performing the syscall in
 /// the supervisor). The Continue cases in this module are:
 ///   1. Non-IP families (AF_UNIX etc.) — the IP allowlist doesn't apply;
-///      Landlock IPC scoping is the enforcement boundary.
+///      Landlock IPC scoping is the enforcement boundary. One sub-case is
+///      narrower: `sendto`'s non-IP fall-through does NOT Continue once a
+///      destination policy is active. That fall-through has no chroot check —
+///      it is whatever misses the named-path gate, i.e. a pathless AF_UNIX or a
+///      non-AF_UNIX destination — so it fails closed under `--chroot` too.
+///      There the send goes on-behalf on a
+///      pinned fd — a named unix target resolved in the child's root view, or a
+///      non-unix destination on a unix socket (the NETLINK_ROUTE
+///      virtualization) — or fails closed with EAFNOSUPPORT for an AF_UNIX
+///      address with no pathname to pin, notably an abstract one: the
+///      supervisor has no Landlock domain, so an on-behalf abstract send would
+///      escape the child's scope. The other non-IP arms still Continue under a
+///      destination policy — `connect`'s abstract/unnamed/non-AF_UNIX arm and
+///      both chroot named-unix arms (`connect`, `sendto`/`sendmsg`), which is
+///      the residue tracked by issues #27 / #143.
 ///   2. Connected sockets with addr_ptr == 0 — the address was already
 ///      validated at connect time, so the kernel re-read of (nothing) is
 ///      moot.

crates/sandlock-core/src/network/send.rs

@@ -12,16 +12,18 @@ use crate::seccomp::notif::{read_child_mem, NotifAction};
 use crate::sys::structs::{SeccompNotif, ECONNREFUSED};
 
 use super::materialize::{
-    materialize_msg, mmsg_entry_ptr, mmsg_msglen_addr, named_unix_socket_path,
-    parse_ip_from_sockaddr, parse_port_from_sockaddr, ChildMsghdr, MaterializedMsg,
-    MAX_SEND_BUF,
+    classify_dest_shape, materialize_msg, mmsg_entry_ptr, mmsg_msglen_addr,
+    named_unix_socket_path, parse_ip_from_sockaddr, parse_port_from_sockaddr, ChildMsghdr,
+    MaterializedMsg, MAX_SEND_BUF,
 };
 use super::send_engine::{batch_send_step, resolve_send, wants_blocking, BatchStep};
 use super::unix::{
     mmsg_entry_named_unix_path, sendmmsg_named_unix_on_behalf, sendto_named_unix_on_behalf,
-    unix_sendmsg_gate,
+    sendto_pinned_unix_on_behalf, unix_sendmsg_gate,
+};
+use super::verdict::{
+    check_ip_destination, classify_send_path, path_under_any, DestShape, SendPath,
 };
-use super::verdict::{check_ip_destination, path_under_any};
 use super::{query_socket_protocol, socket_is_unix, Protocol};
 
 // ============================================================
@@ -127,7 +129,95 @@ pub(super) async fn sendto_on_behalf(
                     )
                 }
             }
-            _ => NotifAction::Continue,
+            _ => {
+                // Non-IP destination with no fs-path gate: an abstract unix
+                // address, a named unix address with the fs-gate off, a non-unix
+                // family (AF_NETLINK/AF_PACKET/AF_VSOCK/...) on any socket. With
+                // NO destination policy there is nothing to bypass, so Continue
+                // (matching the connected fast path). Under a destination policy
+                // do NOT Continue: the kernel would re-read `sockfd` and the
+                // address after our decision, and a racing
+                // `dup2(inet_sock, sockfd)` + address swap could ride the Continue
+                // out to a denied IP. Pin the fd and gate on its STABLE socket
+                // domain instead. NOTE: this fails closed for non-IP sends on
+                // non-unix sockets and for AF_UNIX destinations we cannot pin in
+                // the child's context (abstract / empty / non-UTF-8 `sun_path`)
+                // under a destination policy — see the PR description.
+                if !ctx.policy.has_net_destination_policy {
+                    return NotifAction::Continue;
+                }
+                let dup_fd = match crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd) {
+                    Ok(fd) => fd,
+                    Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)),
+                };
+                // The address is non-IP here (parse_ip_from_sockaddr returned
+                // None), so classify on the pinned socket's stable domain plus the
+                // destination's ADDRESS FAMILY. Family first: an abstract AF_UNIX
+                // address and a sockaddr_nl both carry no pathname but are not the
+                // same case (see DestShape).
+                let dest = classify_dest_shape(&addr_bytes);
+                match classify_send_path(
+                    false,
+                    None,
+                    socket_is_unix(dup_fd.as_raw_fd()),
+                    &dest,
+                ) {
+                    SendPath::NamedUnixOnBehalf => match &dest {
+                        // Resolve the target in the CHILD's root view and send to
+                        // the pinned inode: the supervisor performs the send, so
+                        // passing the child's raw `sun_path` through would resolve
+                        // it against the supervisor's cwd/root. No fs-grant check
+                        // here — this arm is only reachable with
+                        // `has_unix_fs_gate == false` (the gated case is handled
+                        // above), i.e. the sandbox declares no fs grants to check
+                        // against.
+                        DestShape::UnixNamed(path) => sendto_pinned_unix_on_behalf(
+                            notif, notif_fd, dup_fd, buf_ptr, buf_len, flags, path,
+                        ),
+                        // Unreachable: the classifier returns NamedUnixOnBehalf
+                        // only for DestShape::UnixNamed. Fail closed rather than
+                        // unwrap — see the panic note below.
+                        _ => NotifAction::Errno(libc::EAFNOSUPPORT),
+                    },
+                    // A non-AF_UNIX destination on a unix socket. Send on-behalf
+                    // on the pinned fd with the child's address bytes verbatim:
+                    // this is what sandlock's own NETLINK_ROUTE virtualization
+                    // produces (child fd = one end of a socketpair(AF_UNIX,
+                    // SOCK_SEQPACKET), addressed with a sockaddr_nl), and that
+                    // socket is connected, so the kernel ignores `msg_name`
+                    // outright. Nothing is widened: the fd was pinned before its
+                    // domain was read, so no dup2 can redirect the send, and for
+                    // any other family the kernel rejects the mismatch itself.
+                    SendPath::RawDestOnBehalf => {
+                        let data =
+                            match read_child_mem(notif_fd, notif.id, notif.pid, buf_ptr, buf_len) {
+                                Ok(b) => b,
+                                Err(_) => return NotifAction::Errno(libc::EIO),
+                            };
+                        let m = MaterializedMsg {
+                            data,
+                            control: None,
+                            addr: addr_bytes,
+                            _scm_fds: Vec::new(),
+                            _pinned: None,
+                        };
+                        let blocking = wants_blocking(dup_fd.as_raw_fd(), flags);
+                        resolve_send(dup_fd, m, flags, blocking)
+                    }
+                    // Reject (a non-IP address on a non-unix socket — the
+                    // address-family-swap shape — or an AF_UNIX address with no
+                    // pathname to pin, notably an ABSTRACT one: the supervisor
+                    // carries no Landlock domain, so sending it on-behalf would
+                    // escape the child's LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET) and,
+                    // defensively, the connected/IP variants that
+                    // classify_send_path(false, None, ..) cannot return: fail
+                    // closed with an errno (parity with the sendmsg Reject arm)
+                    // rather than panic in the seccomp-notif supervisor — a panic
+                    // on this untrusted path would unwind the supervisor task and
+                    // DoS the whole sandbox.
+                    _ => NotifAction::Errno(libc::EAFNOSUPPORT),
+                }
+            }
         }
     }
 }