diff --git a/crates/sandlock-core/src/chroot/dispatch.rs b/crates/sandlock-core/src/chroot/dispatch.rs index 55bef5b1..993aca36 100644 --- a/crates/sandlock-core/src/chroot/dispatch.rs +++ b/crates/sandlock-core/src/chroot/dispatch.rs @@ -49,6 +49,8 @@ use std::ffi::CString; use std::io::{Read, Seek, SeekFrom, Write}; +use std::collections::VecDeque; +use std::os::unix::ffi::{OsStrExt, OsStringExt}; use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd, RawFd}; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -60,6 +62,7 @@ use crate::chroot::resolve::{ }; use crate::sys::fs::{openat2_in_root, openat2_in_root_with_resolve}; use crate::seccomp::notif::{decode_open_args, read_child_mem, write_child_mem, NotifAction, NotifPolicy}; +use crate::seccomp::ctx::SupervisorCtx; use crate::seccomp::state::{ChrootState, CowState, ProcessIndex}; use crate::sys::structs::{SeccompNotif, SeccompNotifAddfd, SECCOMP_IOCTL_NOTIF_ADDFD}; @@ -169,6 +172,35 @@ impl ChrootCtx<'_> { Some((mount_hp, sub_str)) } + /// Return the physical root and root-relative spelling used to resolve a + /// virtual path, honoring the most-specific bind mount. + fn root_and_subpath(&self, virtual_path: &Path) -> (PathBuf, PathBuf) { + let mut best: Option<(&Path, &Path)> = None; + for (virtual_mount, host_mount) in self.mounts { + if virtual_path.starts_with(virtual_mount) + && best.map_or(true, |(old, _)| { + virtual_mount.as_os_str().len() > old.as_os_str().len() + }) + { + best = Some((virtual_mount, host_mount)); + } + } + match best { + Some((virtual_mount, host_mount)) => { + let relative = virtual_path + .strip_prefix(virtual_mount) + .expect("the most-specific mount prefix matched"); + let subpath = if relative.as_os_str().is_empty() { + PathBuf::from("/") + } else { + Path::new("/").join(relative) + }; + (host_mount.to_path_buf(), subpath) + } + None => (self.root.to_path_buf(), virtual_path.to_path_buf()), + } + } + /// Resolve a virtual path against mounts, using `resolver` for the part /// below the mount point. Returns (host_path, virtual_path); the virtual /// path is the confined form of what the child asked for, since that is @@ -463,6 +495,243 @@ fn resolve_chroot_path_existing( resolve_existing_in_root(ctx.root, &full_path) } +/// Resolve and pin a named AF_UNIX target in the same virtual chroot/mount/COW +/// view used by path syscalls. The returned descriptor names the exact socket +/// inode that was authorized; callers must use `/proc/self/fd/` to perform +/// the operation rather than returning a pathname to the kernel. +pub(crate) async fn pin_named_unix_target( + notif: &SeccompNotif, + sun_path: &Path, + supervisor: &Arc, +) -> Result { + let chroot = ChrootCtx::new(&supervisor.policy, &supervisor.processes); + let joined = if sun_path.is_absolute() { + sun_path.to_path_buf() + } else { + virtual_cwd_of(notif, &chroot) + .ok_or(NotifAction::Errno(libc::EACCES))? + .join(sun_path) + }; + let virtual_path = canon_proc_self_path(&joined, notif.pid); + + if chroot.is_denied(&crate::chroot::resolve::confine_path(&virtual_path)) { + return Err(NotifAction::Errno(libc::EACCES)); + } + + let cow = supervisor.cow.lock().await; + let (pinned, resolved_virtual) = + resolve_chroot_unix_target(&chroot, &virtual_path, cow.branch.as_ref()) + .map_err(NotifAction::Errno)?; + + let mut stat: libc::stat = unsafe { std::mem::zeroed() }; + if unsafe { libc::fstat(pinned.as_raw_fd(), &mut stat) } != 0 { + return Err(NotifAction::Errno(libc::EACCES)); + } + if stat.st_mode & libc::S_IFMT != libc::S_IFSOCK { + return Err(NotifAction::Errno(libc::ENOTSOCK)); + } + + let actual = std::fs::read_link(format!("/proc/self/fd/{}", pinned.as_raw_fd())) + .map_err(|_| NotifAction::Errno(libc::EACCES))?; + let logical = match cow.branch.as_ref() { + Some(branch) if actual.starts_with(branch.upper_dir()) => branch.workdir().join( + actual.strip_prefix(branch.upper_dir()).expect("upper path prefix matched"), + ), + _ => actual, + }; + let actual_virtual = chroot + .host_to_virtual(&logical) + .ok_or(NotifAction::Errno(libc::EACCES))?; + if !chroot.can_write(&resolved_virtual) || !chroot.can_write(&actual_virtual) { + return Err(NotifAction::Errno(libc::EACCES)); + } + + Ok(pinned) +} + +/// Rewrite procfs self aliases to the task that issued the notification. +/// The resolver runs in the supervisor, where `/proc/self` would name the +/// supervisor itself. +fn canon_proc_self_path(path: &Path, pid: u32) -> PathBuf { + let bytes = path.as_os_str().as_bytes(); + for magic in [b"/proc/self".as_slice(), b"/proc/thread-self".as_slice()] { + if bytes == magic || bytes.strip_prefix(magic).is_some_and(|tail| tail.starts_with(b"/")) { + let tail = &bytes[magic.len()..]; + let mut rewritten = format!("/proc/{pid}").into_bytes(); + rewritten.extend_from_slice(tail); + return PathBuf::from(std::ffi::OsString::from_vec(rewritten)); + } + } + path.to_path_buf() +} + +/// Resolve a pathname component by component so symlink targets can cross +/// virtual mount points and COW layers exactly where the child-visible path +/// does. Every component is opened without following symlinks; symlink text is +/// expanded as virtual path bytes and resolution restarts at the virtual root. +fn resolve_chroot_unix_target( + chroot: &ChrootCtx<'_>, + path: &Path, + cow: Option<&crate::cow::seccomp::SeccompCowBranch>, +) -> Result<(OwnedFd, PathBuf), i32> { + if requires_directory(path) { + return Err(libc::ENOTDIR); + } + let mut pending = path + .components() + .filter_map(|component| match component { + std::path::Component::Normal(_) | std::path::Component::ParentDir => { + Some(part_or_parent(component)) + } + _ => None, + }) + .collect::>(); + let mut virtual_prefix = PathBuf::from("/"); + let mut symlink_count = 0usize; + + while let Some(component) = pending.pop_front() { + if component == ".." { + virtual_prefix.pop(); + continue; + } + if component == "." || component.is_empty() { + continue; + } + + virtual_prefix.push(&component); + if virtual_prefix.as_os_str().len() > 4096 { + return Err(libc::ENAMETOOLONG); + } + let fd = open_chroot_unix_component(chroot, &virtual_prefix, cow)?; + let mut stat: libc::stat = unsafe { std::mem::zeroed() }; + if unsafe { libc::fstat(fd.as_raw_fd(), &mut stat) } != 0 { + return Err(last_errno(libc::EACCES)); + } + if stat.st_mode & libc::S_IFMT != libc::S_IFLNK { + if pending.is_empty() { + // Keep the descriptor we inspected instead of resolving the + // final pathname again after another task can replace it. + return Ok((fd, virtual_prefix)); + } + if stat.st_mode & libc::S_IFMT != libc::S_IFDIR { + return Err(libc::ENOTDIR); + } + continue; + } + + symlink_count += 1; + if symlink_count > 40 { + return Err(libc::ELOOP); + } + let empty = CString::new(Vec::::new()).expect("empty CString"); + let mut target = vec![0u8; 4096]; + let length = unsafe { + libc::readlinkat( + fd.as_raw_fd(), + empty.as_ptr(), + target.as_mut_ptr() as *mut libc::c_char, + target.len(), + ) + }; + if length < 0 { + return Err(last_errno(libc::EACCES)); + } + if length as usize == target.len() { + return Err(libc::ENAMETOOLONG); + } + target.truncate(length as usize); + let target_path = PathBuf::from(std::ffi::OsString::from_vec(target)); + if pending.is_empty() && requires_directory(&target_path) { + return Err(libc::ENOTDIR); + } + if target_path.is_absolute() { + virtual_prefix = PathBuf::from("/"); + } else { + virtual_prefix.pop(); + } + let pending_bytes = pending + .iter() + .map(|part| part.as_os_str().len().saturating_add(1)) + .sum::(); + if virtual_prefix + .as_os_str() + .len() + .saturating_add(target_path.as_os_str().len()) + .saturating_add(pending_bytes) + > 4096 + { + return Err(libc::ENAMETOOLONG); + } + let mut replacement = target_path + .components() + .filter_map(|component| match component { + std::path::Component::Normal(part) => Some(part.to_os_string()), + std::path::Component::ParentDir => Some(std::ffi::OsString::from("..")), + _ => None, + }) + .collect::>(); + replacement.extend(pending); + pending = replacement.into(); + } + + let pinned = open_chroot_unix_component(chroot, &virtual_prefix, cow)?; + Ok((pinned, virtual_prefix)) +} + +fn requires_directory(path: &Path) -> bool { + let bytes = path.as_os_str().as_bytes(); + bytes.ends_with(b"/") || bytes.ends_with(b"/.") || bytes == b"." +} + +fn part_or_parent(component: std::path::Component<'_>) -> std::ffi::OsString { + match component { + std::path::Component::Normal(part) => part.to_os_string(), + std::path::Component::ParentDir => std::ffi::OsString::from(".."), + _ => unreachable!("only normal and parent components are retained"), + } +} + +fn open_chroot_unix_component( + chroot: &ChrootCtx<'_>, + virtual_path: &Path, + cow: Option<&crate::cow::seccomp::SeccompCowBranch>, +) -> Result { + let (root, subpath) = chroot.root_and_subpath(virtual_path); + let candidate = root.join(subpath.strip_prefix("/").unwrap_or(&subpath)); + let (root, subpath): (PathBuf, PathBuf) = if let Some(branch) = cow { + if let Some(candidate_utf8) = candidate.to_str().filter(|p| branch.matches(p)) { + let effective = branch.handle_stat(candidate_utf8).ok_or(libc::ENOENT)?; + if effective.starts_with(branch.upper_dir()) { + ( + branch.upper_dir().to_path_buf(), + PathBuf::from("/").join(effective.strip_prefix(branch.upper_dir()).map_err(|_| libc::EACCES)?), + ) + } else if effective.starts_with(branch.workdir()) { + ( + branch.workdir().to_path_buf(), + PathBuf::from("/").join(effective.strip_prefix(branch.workdir()).map_err(|_| libc::EACCES)?), + ) + } else { + return Err(libc::EACCES); + } + } else if candidate.starts_with(branch.workdir()) && candidate.to_str().is_none() { + return Err(libc::EACCES); + } else { + (root, subpath) + } + } else { + (root, subpath) + }; + let fd = crate::sys::fs::openat2_in_root_path_with_resolve( + &root, + &subpath, + libc::O_PATH | libc::O_NOFOLLOW | libc::O_CLOEXEC, + 0, + RESOLVE_NO_SYMLINKS | RESOLVE_NO_MAGICLINKS, + )?; + Ok(unsafe { OwnedFd::from_raw_fd(fd) }) +} + /// Convert a Path to CString, returning Errno on failure. fn path_cstr(path: &Path, err: i32) -> Result { CString::new(path.to_str().unwrap_or("")).map_err(|_| NotifAction::Errno(err)) @@ -2657,7 +2926,11 @@ mod self_rewrite_tests { #[cfg(test)] mod mount_ro_tests { - use super::{ChrootCtx, ProcessIndex}; + use super::{resolve_chroot_unix_target, ChrootCtx, ProcessIndex}; + use std::fs; + use std::os::unix::ffi::{OsStrExt, OsStringExt}; + use std::os::unix::io::AsRawFd; + use std::os::unix::net::UnixListener; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -2702,4 +2975,129 @@ mod mount_ro_tests { assert!(c.can_read(Path::new("/data/file"))); assert!(c.can_write(Path::new("/data/file"))); } + + #[test] + fn target_resolution_uses_the_most_specific_mount_and_keeps_raw_bytes() { + use std::os::unix::ffi::{OsStrExt, OsStringExt}; + + let mounts = vec![ + (PathBuf::from("/data"), PathBuf::from("/host/data")), + ( + PathBuf::from("/data/nested"), + PathBuf::from("/host/special"), + ), + ]; + let processes = Arc::new(ProcessIndex::new()); + let c = ctx(&mounts, &[], &[], &processes); + let raw_name = PathBuf::from(std::ffi::OsString::from_vec( + [b"/data/nested/", &[0xff][..]].concat(), + )); + + let (root, subpath) = c.root_and_subpath(&raw_name); + assert_eq!(root, Path::new("/host/special")); + assert_eq!(subpath.as_os_str().as_bytes(), b"/\xff"); + } + + #[test] + fn unix_target_resolution_follows_virtual_symlink_across_mount() { + let temp = std::env::temp_dir().join(format!( + "sandlock-chroot-unix-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let root = temp.join("root"); + let mounted = temp.join("mounted"); + fs::create_dir_all(&root).unwrap(); + fs::create_dir_all(&mounted).unwrap(); + let socket_path = mounted.join("endpoint.sock"); + let listener = UnixListener::bind(&socket_path).unwrap(); + std::os::unix::fs::symlink("/mnt/endpoint.sock", root.join("alias.sock")).unwrap(); + + let mounts = vec![(PathBuf::from("/mnt"), mounted)]; + let writable = vec![PathBuf::from("/mnt")]; + let processes = Arc::new(ProcessIndex::new()); + let context = ChrootCtx { + root: &root, + readable: &[], + writable: &writable, + denied: &[], + mounts: &mounts, + mount_ro: &[], + processes: &processes, + }; + + let (pinned, resolved) = resolve_chroot_unix_target( + &context, + Path::new("/alias.sock"), + None, + ) + .unwrap(); + let mut stat: libc::stat = unsafe { std::mem::zeroed() }; + assert_eq!(unsafe { libc::fstat(pinned.as_raw_fd(), &mut stat) }, 0); + assert_eq!(stat.st_mode & libc::S_IFMT, libc::S_IFSOCK); + assert_eq!(resolved, Path::new("/mnt/endpoint.sock")); + assert!(context.can_write(&resolved)); + assert!(listener.local_addr().is_ok()); + + // Replace the pathname after resolution. An on-behalf connection + // through the pin must still reach the original listener. + fs::rename(&socket_path, socket_path.with_extension("old")).unwrap(); + let replacement = UnixListener::bind(&socket_path).unwrap(); + listener.set_nonblocking(true).unwrap(); + replacement.set_nonblocking(true).unwrap(); + let stream = std::os::unix::net::UnixStream::connect(format!( + "/proc/self/fd/{}", pinned.as_raw_fd() + )).unwrap(); + assert!(listener.accept().is_ok(), "pinned inode must receive the connection"); + assert_eq!(replacement.accept().unwrap_err().kind(), std::io::ErrorKind::WouldBlock); + drop(stream); + drop(replacement); + drop(listener); + drop(pinned); + fs::remove_dir_all(temp).unwrap(); + } + + #[test] + fn unix_target_resolution_preserves_non_utf8_socket_name() { + let temp = std::env::temp_dir().join(format!( + "sandlock-chroot-unix-byte-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&temp).unwrap(); + let name = std::ffi::OsString::from_vec(b"socket-\xff".to_vec()); + let path = temp.join(name); + let listener = UnixListener::bind(&path).unwrap(); + let processes = Arc::new(ProcessIndex::new()); + let writable = vec![PathBuf::from("/")]; + let context = ChrootCtx { + root: &temp, + readable: &[], + writable: &writable, + denied: &[], + mounts: &[], + mount_ro: &[], + processes: &processes, + }; + + let raw_virtual = PathBuf::from(std::ffi::OsString::from_vec( + [b"/socket-", &[0xff][..]].concat(), + )); + let (pinned, resolved) = + resolve_chroot_unix_target(&context, &raw_virtual, None).unwrap(); + assert_eq!( + resolved.as_os_str().as_bytes(), + [b"/socket-", &[0xff][..]].concat() + ); + assert!(context.can_write(&resolved)); + drop(listener); + drop(pinned); + fs::remove_dir_all(temp).unwrap(); + } } diff --git a/crates/sandlock-core/src/chroot/resolve.rs b/crates/sandlock-core/src/chroot/resolve.rs index f796767d..943e6459 100644 --- a/crates/sandlock-core/src/chroot/resolve.rs +++ b/crates/sandlock-core/src/chroot/resolve.rs @@ -27,6 +27,25 @@ pub fn confine(virtual_path: &str) -> PathBuf { result } +/// Byte-preserving variant of [`confine`] for Linux filesystem names that do +/// not originate in UTF-8 APIs, such as pathname AF_UNIX addresses. +pub fn confine_path(virtual_path: &Path) -> PathBuf { + use std::path::Component; + + let mut result = PathBuf::from("/"); + for component in virtual_path.components() { + match component { + Component::RootDir | Component::CurDir => {} + Component::ParentDir => { + result.pop(); + } + Component::Normal(part) => result.push(part), + Component::Prefix(prefix) => result.push(prefix.as_os_str()), + } + } + result +} + /// Strip chroot root prefix from host path. /// Returns None if host path is not under chroot root. pub fn to_virtual_path(chroot_root: &Path, host_path: &Path) -> Option { @@ -197,9 +216,27 @@ pub fn resolve_chroot_mounts(mounts: &[(PathBuf, PathBuf)]) -> Vec<(PathBuf, Pat #[cfg(test)] mod tests { use super::*; + use std::os::unix::ffi::OsStringExt; use std::os::unix::fs::symlink; use tempfile::TempDir; + #[test] + fn confine_path_preserves_non_utf8_components_and_clamps_parent() { + let raw = PathBuf::from(std::ffi::OsString::from_vec( + b"/allowed/\xff/../service/../../../../etc".to_vec(), + )); + assert_eq!( + confine_path(&raw).as_os_str().as_encoded_bytes(), + b"/etc" + ); + + let raw = PathBuf::from(std::ffi::OsString::from_vec(b"/allowed/\xff/service".to_vec())); + assert_eq!( + confine_path(&raw).as_os_str().as_encoded_bytes(), + b"/allowed/\xff/service" + ); + } + #[test] fn resolve_chroot_root_none_is_ok_none() { assert!(resolve_chroot_root(None).unwrap().is_none()); diff --git a/crates/sandlock-core/src/freeze.rs b/crates/sandlock-core/src/freeze.rs index d1d57649..2655fcdb 100644 --- a/crates/sandlock-core/src/freeze.rs +++ b/crates/sandlock-core/src/freeze.rs @@ -18,7 +18,11 @@ //! that share the calling task's `mm_struct` via //! `clone(CLONE_VM)` without `CLONE_THREAD`. //! -//! `freeze_sandbox_for_execve` closes both classes. When `policy_fn` +//! `freeze_sandbox_for_execve` covers tracked sandbox tasks in both classes. +//! It cannot discover a process outside `ProcessIndex` that independently +//! maps the same `MAP_SHARED` backing object; that writer class remains an +//! explicit limitation unless the backing object is otherwise controlled. +//! When `policy_fn` //! is active, every fork-like syscall is traced for one ptrace //! fork/clone/vfork event and the child is registered in //! `ProcessIndex` before it can run user code. The exec freeze can @@ -36,9 +40,12 @@ //! siblings and peers because `de_thread` will not run. //! //! Peer threads (different TGID) survive execve. The supervisor must -//! `PTRACE_DETACH` them after `NOTIF_SEND` so they can resume normal -//! execution. The freeze function returns the peer TID list for that -//! purpose; siblings are not returned because they need no follow-up. +//! eventually `PTRACE_DETACH` them so they can resume normal execution. +//! The current implementation detaches them after `NOTIF_SEND` succeeds, +//! but notification-send success is not documented here as proof that the +//! kernel has consumed execve's user-memory arguments. That ordering needs +//! a kernel-observable completion boundary before this can be claimed as a +//! complete TOCTOU guarantee. //! //! # Failure modes (strict) //! @@ -192,10 +199,7 @@ fn list_threads_of_tgid(tgid: i32) -> io::Result> { let dir = fs::read_dir(format!("/proc/{}/task", tgid))?; let mut tids = Vec::new(); for entry in dir { - let entry = match entry { - Ok(e) => e, - Err(_) => continue, - }; + let entry = entry?; let name = entry.file_name(); let name_str = match name.to_str() { Some(s) => s, @@ -208,6 +212,13 @@ fn list_threads_of_tgid(tgid: i32) -> io::Result> { Ok(tids) } +/// `ESRCH`/`ENOENT` mean the process or task directory disappeared during +/// enumeration. Other errors mean enumeration was incomplete and must not +/// be treated as a successful freeze. +fn task_directory_disappeared(error: &io::Error) -> bool { + matches!(error.raw_os_error(), Some(libc::ESRCH | libc::ENOENT)) +} + /// Read the TGID containing `tid`, as an `io::Result` so a missing or /// unparseable value aborts the freeze instead of silently narrowing it /// to one task. @@ -255,9 +266,13 @@ impl std::fmt::Display for FreezeError { } } -/// Freeze every sandbox thread that could mutate execve argv before -/// the supervisor reads it for `policy_fn` and before the kernel -/// re-reads it. +/// Freeze every enumerated, tracked sandbox thread that could mutate +/// execve argv before the supervisor reads it for `policy_fn`. +/// +/// This cannot discover external processes outside `ProcessIndex` that +/// independently map the same shared backing object. The caller also must +/// not treat `NOTIF_SEND` success as proof that the kernel has consumed the +/// execve user-memory arguments. /// /// Walks every TGID in `processes`, enumerates each TGID's threads via /// `/proc//task/`, and `PTRACE_SEIZE` + `PTRACE_INTERRUPT`s @@ -289,10 +304,23 @@ pub(crate) fn freeze_sandbox_for_execve( for tgid in &tgids { // /proc//task may disappear if the TGID exited between - // snapshot and walk — that's fine, no threads to freeze. + // snapshot and walk — that's fine. Any other enumeration error + // means the freeze would be incomplete, so roll back and fail. let tids = match list_threads_of_tgid(*tgid) { Ok(t) => t, - Err(_) => continue, + Err(e) if task_directory_disappeared(&e) => continue, + Err(e) => { + for t in &sibling_tids { + detach(*t); + } + for t in &peer_tids { + detach(*t); + } + return Err(FreezeError { + error: e, + pending_tids, + }); + } }; for tid in tids { if tid == caller_tid { @@ -420,6 +448,22 @@ mod tests { assert!(!requires_freeze_on_continue(libc::SYS_connect)); } + #[test] + fn only_disappeared_task_directories_are_ignored() { + assert!(task_directory_disappeared(&io::Error::from_raw_os_error( + libc::ESRCH + ))); + assert!(task_directory_disappeared(&io::Error::from_raw_os_error( + libc::ENOENT + ))); + assert!(!task_directory_disappeared(&io::Error::from_raw_os_error( + libc::EACCES + ))); + assert!(!task_directory_disappeared(&io::Error::from_raw_os_error( + libc::EIO + ))); + } + /// Regression test for the cross-process TOCTOU concern raised on /// issue #27 (Changaco): a peer process in the sandbox — different /// TGID, possibly aliasing argv pages via shared memory — must also diff --git a/crates/sandlock-core/src/network/connect.rs b/crates/sandlock-core/src/network/connect.rs index eee182a0..2fb7b61d 100644 --- a/crates/sandlock-core/src/network/connect.rs +++ b/crates/sandlock-core/src/network/connect.rs @@ -12,11 +12,11 @@ use crate::seccomp::notif::NotifAction; use crate::sys::structs::{SeccompNotif, ECONNREFUSED}; use super::materialize::{ - named_unix_socket_path, parse_ip_from_sockaddr, parse_port_from_sockaddr, + classify_unix_addr, parse_ip_from_sockaddr, parse_port_from_sockaddr, UnixAddr, set_port_in_sockaddr, sockaddr_is_ipv6, }; -use super::unix::connect_named_unix_on_behalf; -use super::verdict::{layered_destination_verdict, path_under_any}; +use super::unix::{connect_named_unix_on_behalf, connect_pinned_unix_on_behalf}; +use super::verdict::layered_destination_verdict; use super::query_socket_protocol; // ============================================================ @@ -128,17 +128,16 @@ pub(super) async fn connect_on_behalf( // EACCES. The decision is made on `addr_bytes` (our immune copy) and we // never return Continue on the deny path, so it is TOCTOU-safe. // Abstract sockets (no path) are handled by the Landlock abstract scope. - match named_unix_socket_path(&addr_bytes) { - Some(path) if ctx.policy.has_unix_fs_gate => { + match classify_unix_addr(&addr_bytes) { + UnixAddr::Named(path) if ctx.policy.has_unix_fs_gate => { if ctx.policy.chroot_root.is_some() { - // Chroot mode: the child's paths are virtual, so a lexical - // check against the (virtual) write grants is consistent, - // and host socket paths are absent from the chroot view - // anyway. Deny unless under a write grant. - if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) + 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)), + }; + match crate::chroot::dispatch::pin_named_unix_target(notif, &path, ctx).await { + Ok(pinned) => connect_pinned_unix_on_behalf(dup_fd, pinned), + Err(action) => action, } } else { // Non-chroot: resolve the symlink-followed real target and @@ -152,6 +151,9 @@ pub(super) async fn connect_on_behalf( ) } } + UnixAddr::Malformed(errno) if ctx.policy.has_unix_fs_gate => { + NotifAction::Errno(errno) + } // Abstract/unnamed socket, non-AF_UNIX family, or gate disabled. _ => NotifAction::Continue, } @@ -463,4 +465,3 @@ mod tests { } } - diff --git a/crates/sandlock-core/src/network/materialize.rs b/crates/sandlock-core/src/network/materialize.rs index f8b815ba..27d145a9 100644 --- a/crates/sandlock-core/src/network/materialize.rs +++ b/crates/sandlock-core/src/network/materialize.rs @@ -6,6 +6,7 @@ // never re-read child memory. use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::os::unix::ffi::OsStringExt; use std::os::unix::io::{AsRawFd, OwnedFd, RawFd}; use crate::seccomp::notif::read_child_mem; @@ -198,28 +199,50 @@ fn materialize_control( /// Extract the filesystem path of a NAMED `AF_UNIX` connect target from a raw /// `sockaddr`. Returns `None` for abstract sockets (`sun_path[0] == 0`), -/// unnamed sockets, or any non-`AF_UNIX` family (none of which the fs gate -/// applies to). +/// unnamed sockets, malformed addresses, or any non-`AF_UNIX` family. Callers +/// enforcing a filesystem gate must use [`classify_unix_addr`] to distinguish +/// malformed input from a genuine pathless address. pub(crate) fn named_unix_socket_path(addr_bytes: &[u8]) -> Option { - // sockaddr_un layout: u16 sun_family, then sun_path. Need the family plus - // at least one path byte. - if addr_bytes.len() < 3 { - return None; + match classify_unix_addr(addr_bytes) { + UnixAddr::Named(path) => Some(path), + _ => None, + } +} + +#[derive(Debug, PartialEq, Eq)] +pub(crate) enum UnixAddr { + NotUnix, + Pathless, + Named(std::path::PathBuf), + Malformed(i32), +} + +/// Parse a copied sockaddr while distinguishing pathless AF_UNIX addresses +/// from truncated or oversized AF_UNIX structures. +pub(crate) fn classify_unix_addr(addr_bytes: &[u8]) -> UnixAddr { + if addr_bytes.len() < std::mem::size_of::() { + return UnixAddr::Malformed(libc::EINVAL); } let family = u16::from_ne_bytes([addr_bytes[0], addr_bytes[1]]); if family != libc::AF_UNIX as u16 { - return None; + return UnixAddr::NotUnix; + } + // An address containing only sa_family_t is a valid unnamed AF_UNIX + // address (sun_path has length zero); it is not a truncated pathname. + if addr_bytes.len() == std::mem::size_of::() { + return UnixAddr::Pathless; + } + if addr_bytes.len() > std::mem::size_of::() { + return UnixAddr::Malformed(libc::EINVAL); } let sun_path = &addr_bytes[2..]; if sun_path[0] == 0 { - return None; // abstract namespace (Landlock scope handles it) + return UnixAddr::Pathless; } let end = sun_path.iter().position(|&b| b == 0).unwrap_or(sun_path.len()); - let raw = &sun_path[..end]; - if raw.is_empty() { - return None; - } - std::str::from_utf8(raw).ok().map(std::path::PathBuf::from) + UnixAddr::Named(std::path::PathBuf::from(std::ffi::OsString::from_vec( + sun_path[..end].to_vec(), + ))) } /// Classify a raw destination `sockaddr` into the [`DestShape`] the non-IP send @@ -231,18 +254,11 @@ pub(crate) fn named_unix_socket_path(addr_bytes: &[u8]) -> Option 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, + match classify_unix_addr(addr_bytes) { + UnixAddr::NotUnix => DestShape::NotUnix, + UnixAddr::Pathless => DestShape::UnixNoPath, + UnixAddr::Named(path) => DestShape::UnixNamed(path), + UnixAddr::Malformed(errno) => DestShape::UnixMalformed(errno), } } @@ -299,7 +315,7 @@ impl ChildMsghdr { /// `struct mmsghdr` on LP64: the 56-byte msghdr + 4-byte `msg_len` result + /// 4 bytes tail padding = 64 bytes, `msg_len` at offset 56. -const MMSGHDR_SIZE: usize = 64; +pub(crate) const MMSGHDR_SIZE: usize = 64; const MSG_LEN_OFFSET: usize = 56; /// Address of `sendmmsg` entry `i` in the child's `msgvec` array. The entry's @@ -309,6 +325,40 @@ pub(crate) fn mmsg_entry_ptr(msgvec_ptr: u64, i: usize) -> u64 { msgvec_ptr + (i * MMSGHDR_SIZE) as u64 } +/// Snapshot the complete bounded mmsghdr vector in one child-memory read. +/// The syscall handler uses these owned headers for both destination checks +/// and execution, so another thread cannot replace entries between the gate +/// and the on-behalf sends. +pub(crate) fn snapshot_mmsg_headers( + notif: &SeccompNotif, + notif_fd: RawFd, + msgvec_ptr: u64, + vlen: usize, +) -> Result, i32> { + let bytes_len = vlen.checked_mul(MMSGHDR_SIZE).ok_or(libc::EINVAL)?; + let bytes = read_child_mem(notif_fd, notif.id, notif.pid, msgvec_ptr, bytes_len) + .map_err(|_| libc::EFAULT)?; + if bytes.len() != bytes_len { + return Err(libc::EFAULT); + } + parse_mmsg_headers(msgvec_ptr, &bytes) +} + +fn parse_mmsg_headers(msgvec_ptr: u64, bytes: &[u8]) -> Result, i32> { + if bytes.len() % MMSGHDR_SIZE != 0 { + return Err(libc::EFAULT); + } + bytes + .chunks_exact(MMSGHDR_SIZE) + .enumerate() + .map(|(i, entry)| { + ChildMsghdr::parse(entry) + .map(|hdr| (mmsg_entry_ptr(msgvec_ptr, i), hdr)) + .ok_or(libc::EFAULT) + }) + .collect() +} + /// Address of the `msg_len` result field inside the entry at `entry_ptr`. pub(crate) fn mmsg_msglen_addr(entry_ptr: u64) -> u64 { entry_ptr + MSG_LEN_OFFSET as u64 @@ -489,6 +539,20 @@ mod tests { assert_eq!(mmsg_msglen_addr(0x1000), 0x1000 + MSG_LEN_OFFSET as u64); } + #[test] + fn mmsg_header_snapshot_parser_keeps_entry_addresses_and_fields() { + let mut bytes = vec![0u8; 2 * MMSGHDR_SIZE]; + bytes[0..8].copy_from_slice(&0x1234u64.to_ne_bytes()); + bytes[MMSGHDR_SIZE..MMSGHDR_SIZE + 8].copy_from_slice(&0x5678u64.to_ne_bytes()); + let parsed = parse_mmsg_headers(0x1000, &bytes).unwrap(); + assert_eq!(parsed.len(), 2); + assert_eq!(parsed[0].0, 0x1000); + assert_eq!(parsed[0].1.name_ptr, 0x1234); + assert_eq!(parsed[1].0, 0x1000 + MMSGHDR_SIZE as u64); + assert_eq!(parsed[1].1.name_ptr, 0x5678); + assert_eq!(parse_mmsg_headers(0x1000, &bytes[..bytes.len() - 1]), Err(libc::EFAULT)); + } + // --- parse_ip_from_sockaddr tests (sockaddr bytes, child-controlled) --- fn v6_sockaddr_bytes(ip: Ipv6Addr, port: u16) -> Vec { @@ -551,9 +615,7 @@ mod tests { #[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. + // Abstract and unnamed addresses have no filesystem pathname. assert_eq!( classify_dest_shape(&unix_sockaddr_bytes(b"\0abstract-name")), DestShape::UnixNoPath @@ -562,12 +624,41 @@ mod tests { classify_dest_shape(&unix_sockaddr_bytes(b"")), DestShape::UnixNoPath ); + let raw_path = + std::path::PathBuf::from(std::ffi::OsString::from_vec(b"/run/\xff\xfe".to_vec())); assert_eq!( classify_dest_shape(&unix_sockaddr_bytes(b"/run/\xff\xfe\0")), - DestShape::UnixNoPath + DestShape::UnixNamed(raw_path.clone()) + ); + assert_eq!( + named_unix_socket_path(&unix_sockaddr_bytes(b"/run/\xff\xfe\0")), + Some(raw_path) + ); + // Abstract is a genuine pathless address; truncation is distinct. + assert_eq!(classify_dest_shape(&[1]), DestShape::UnixMalformed(libc::EINVAL)); + assert_eq!( + classify_unix_addr(&(libc::AF_UNIX as u16).to_ne_bytes()), + UnixAddr::Pathless + ); + assert_eq!( + classify_unix_addr(&unix_sockaddr_bytes(b"\0abstract-name")), + UnixAddr::Pathless ); - // Too short to even carry a family: fail closed, do not guess. - assert_eq!(classify_dest_shape(&[1]), DestShape::UnixNoPath); + } + + #[test] + fn unix_address_length_boundaries_fail_closed() { + let family = (libc::AF_UNIX as u16).to_ne_bytes(); + for len in 0..family.len() { + assert_eq!(classify_unix_addr(&family[..len]), UnixAddr::Malformed(libc::EINVAL)); + } + let mut address = family.to_vec(); + address.resize(std::mem::size_of::(), b'x'); + assert!(matches!(classify_unix_addr(&address), UnixAddr::Named(_))); + address.push(0); + assert_eq!(classify_unix_addr(&address), UnixAddr::Malformed(libc::EINVAL)); + address[family.len()] = 0; + assert_eq!(classify_unix_addr(&address), UnixAddr::Malformed(libc::EINVAL)); } #[test] diff --git a/crates/sandlock-core/src/network/send.rs b/crates/sandlock-core/src/network/send.rs index a770cf36..6075eac8 100644 --- a/crates/sandlock-core/src/network/send.rs +++ b/crates/sandlock-core/src/network/send.rs @@ -13,16 +13,17 @@ use crate::sys::structs::{SeccompNotif, ECONNREFUSED}; use super::materialize::{ 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, + classify_unix_addr, parse_ip_from_sockaddr, parse_port_from_sockaddr, + snapshot_mmsg_headers, ChildMsghdr, MaterializedMsg, UnixAddr, 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, + materialize_named_unix_msghdr_pinned, sendmmsg_named_unix_on_behalf, + sendto_named_unix_on_behalf, sendto_pinned_target_on_behalf, sendto_pinned_unix_on_behalf, unix_sendmsg_gate, }; use super::verdict::{ - check_ip_destination, classify_send_path, path_under_any, DestShape, SendPath, + check_ip_destination, classify_send_path, DestShape, SendPath, }; use super::{query_socket_protocol, socket_is_unix, Protocol}; @@ -108,13 +109,18 @@ pub(super) async fn sendto_on_behalf( // Non-IP family. Gate a NAMED AF_UNIX datagram the same way as connect: // sendto to a named socket is a WRITE on its inode, so deny unless the // resolved real target is under an fs-write grant. - match named_unix_socket_path(&addr_bytes) { - Some(path) if ctx.policy.has_unix_fs_gate => { + match classify_unix_addr(&addr_bytes) { + UnixAddr::Named(path) if ctx.policy.has_unix_fs_gate => { if ctx.policy.chroot_root.is_some() { - if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) + 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)), + }; + match crate::chroot::dispatch::pin_named_unix_target(notif, &path, ctx).await { + Ok(pinned) => sendto_pinned_target_on_behalf( + notif, notif_fd, dup_fd, buf_ptr, buf_len, flags, pinned, + ), + Err(action) => action, } } else { sendto_named_unix_on_behalf( @@ -129,6 +135,9 @@ pub(super) async fn sendto_on_behalf( ) } } + UnixAddr::Malformed(errno) if ctx.policy.has_unix_fs_gate => { + NotifAction::Errno(errno) + } _ => { // Non-IP destination with no fs-path gate: an abstract unix // address, a named unix address with the fs-gate off, a non-unix @@ -141,7 +150,7 @@ pub(super) async fn sendto_on_behalf( // 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`) + // the child's context (abstract / empty `sun_path`) // under a destination policy — see the PR description. if !ctx.policy.has_net_destination_policy { return NotifAction::Continue; @@ -246,7 +255,9 @@ pub(super) async fn sendmsg_on_behalf( // IP path below only covers AF_INET/AF_INET6, and would pass a unix target // straight through. if ctx.policy.has_unix_fs_gate { - if let Some(action) = unix_sendmsg_gate(notif, ctx, notif_fd, sockfd, msghdr_ptr, flags) { + if let Some(action) = + unix_sendmsg_gate(notif, ctx, notif_fd, sockfd, msghdr_ptr, flags).await + { return action; } } @@ -423,6 +434,117 @@ async fn send_msghdr_on_behalf( /// hops + one sendmsg). const MAX_MMSGHDR_ENTRIES: usize = 256; +async fn sendmmsg_chroot_named_unix_on_behalf( + notif: &SeccompNotif, + ctx: &Arc, + notif_fd: RawFd, + sockfd: i32, + flags: i32, + headers: Vec<(u64, ChildMsghdr)>, + addresses: Vec>, + destinations: Vec>, +) -> NotifAction { + let mut sent = 0usize; + let mut first_errno = None; + 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)), + }; + let protocol = query_socket_protocol(dup_fd.as_raw_fd()); + for (((entry_ptr, hdr), addr_bytes), destination) in + headers.into_iter().zip(addresses).zip(destinations) + { + let message = if let Some(path) = destination { + let pinned = match crate::chroot::dispatch::pin_named_unix_target(notif, &path, ctx).await { + Ok(pinned) => pinned, + Err(NotifAction::Errno(errno)) => { + first_errno = Some(errno); + break; + } + Err(_) => { + first_errno = Some(libc::EACCES); + break; + } + }; + match materialize_named_unix_msghdr_pinned(notif, notif_fd, hdr, pinned) { + Ok(message) => message, + Err(errno) => { + first_errno = Some(errno); + break; + } + } + } else { + match send_mmsg_other_entry(notif, ctx, notif_fd, &dup_fd, protocol, hdr, addr_bytes).await { + Ok(message) => message, + Err(errno) => { + first_errno = Some(errno); + break; + } + } + }; + match batch_send_step( + &dup_fd, + message, + flags, + notif_fd, + notif.id, + notif.pid, + mmsg_msglen_addr(entry_ptr), + sent, + ) { + BatchStep::Sent => sent += 1, + BatchStep::Done(action) => return action, + BatchStep::Stop(errno) => { + if sent == 0 { + first_errno = Some(errno); + } + break; + } + } + } + if sent > 0 { + NotifAction::ReturnValue(sent as i64) + } else { + NotifAction::Errno(first_errno.unwrap_or(libc::EACCES)) + } +} + +async fn send_mmsg_other_entry( + notif: &SeccompNotif, + ctx: &Arc, + notif_fd: RawFd, + dup_fd: &std::os::unix::io::OwnedFd, + protocol: Option, + hdr: ChildMsghdr, + addr_bytes: Vec, +) -> Result { + if matches!(classify_unix_addr(&addr_bytes), UnixAddr::Pathless) { + // A named-unix batch is executed by the supervisor, outside the + // child's Landlock abstract-socket scope. Stop before sending an + // abstract entry rather than widening that scope. + return Err(libc::EAFNOSUPPORT); + } + if !hdr.connected() { + if let Some(ip) = parse_ip_from_sockaddr(&addr_bytes) { + if ctx.policy.has_net_destination_policy { + let port = parse_port_from_sockaddr(&addr_bytes); + let protocol = protocol.ok_or(ECONNREFUSED)?; + check_ip_destination(ctx, notif.pid, protocol, ip, port).await?; + } + } else if ctx.policy.has_net_destination_policy { + return Err(libc::EAFNOSUPPORT); + } + } + materialize_msg( + notif, + notif_fd, + &hdr, + addr_bytes, + socket_is_unix(dup_fd.as_raw_fd()), + None, + ) +} + /// Perform `sendmmsg()` on behalf of the child. Pre-scans every entry /// for Continue cases (NULL `msg_name` or non-IP family) — if any /// entry would Continue, we Continue the whole syscall to match @@ -455,27 +577,46 @@ pub(super) async fn sendmmsg_on_behalf( // call on the first non-IP entry, which would let a unix entry bypass the // gate. if ctx.policy.has_unix_fs_gate { - let mut named_unix = false; - for i in 0..vlen { - let entry_ptr = mmsg_entry_ptr(msgvec_ptr, i); - if mmsg_entry_named_unix_path(notif, notif_fd, entry_ptr).is_some() { - named_unix = true; - break; + let headers = match snapshot_mmsg_headers(notif, notif_fd, msgvec_ptr, vlen) { + Ok(headers) => headers, + Err(errno) => return NotifAction::Errno(errno), + }; + let mut addresses = Vec::with_capacity(headers.len()); + for (_, hdr) in &headers { + if hdr.connected() { + addresses.push(Vec::new()); + } else { + match super::read_sockaddr( + notif_fd, + notif.id, + notif.pid, + hdr.name_ptr, + hdr.namelen as usize, + ) { + Ok(bytes) => addresses.push(bytes), + Err(errno) => return NotifAction::Errno(errno), + } } } + let mut destinations = Vec::with_capacity(addresses.len()); + for ((_, hdr), bytes) in headers.iter().zip(&addresses) { + if hdr.connected() { + destinations.push(None); + continue; + } + match classify_unix_addr(bytes) { + UnixAddr::Named(path) => destinations.push(Some(path)), + UnixAddr::Malformed(errno) => return NotifAction::Errno(errno), + UnixAddr::NotUnix | UnixAddr::Pathless => destinations.push(None), + } + } + let named_unix = destinations.iter().any(Option::is_some); if named_unix { if ctx.policy.chroot_root.is_some() { - // Chroot: lexical check; deny the whole call if any named-unix - // entry is outside the (virtual) write grants. - for i in 0..vlen { - let entry_ptr = mmsg_entry_ptr(msgvec_ptr, i); - if let Some(path) = mmsg_entry_named_unix_path(notif, notif_fd, entry_ptr) { - if !path_under_any(&path, &ctx.policy.chroot_writable) { - return NotifAction::Errno(libc::EACCES); - } - } - } - // All granted: fall through to the existing path. + return sendmmsg_chroot_named_unix_on_behalf( + notif, ctx, notif_fd, sockfd, flags, headers, addresses, destinations, + ) + .await; } else { return sendmmsg_named_unix_on_behalf( notif, diff --git a/crates/sandlock-core/src/network/unix.rs b/crates/sandlock-core/src/network/unix.rs index e5909992..28e7a8e6 100644 --- a/crates/sandlock-core/src/network/unix.rs +++ b/crates/sandlock-core/src/network/unix.rs @@ -6,6 +6,7 @@ // and the syscall runs on-behalf against `/proc/self/fd/` so the // checked inode is the one acted on (TOCTOU- and symlink-safe). +use std::os::unix::ffi::{OsStrExt, OsStringExt}; use std::os::unix::io::{AsRawFd, FromRawFd, OwnedFd, RawFd}; use std::sync::Arc; @@ -14,11 +15,11 @@ 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, ChildMsghdr, - MaterializedMsg, + classify_unix_addr, materialize_msg, mmsg_entry_ptr, mmsg_msglen_addr, + named_unix_socket_path, ChildMsghdr, MaterializedMsg, UnixAddr, }; use super::send_engine::{batch_send_step, resolve_send, wants_blocking, BatchStep}; -use super::verdict::{path_under_any, real_path_under_any}; +use super::verdict::real_path_under_any; /// Render the supervisor-side path that addresses `sun_path` **in the child's /// root view**, i.e. `/proc//root` + the child's absolute `sun_path`. @@ -34,11 +35,13 @@ use super::verdict::{path_under_any, real_path_under_any}; /// (`/proc/42/rootsvc.dgram`); the explicit check makes the fail-closed /// property intentional rather than incidental, and keeps it under a refactor /// to `Path::join`, which would silently drop the prefix. -fn child_root_path(child_pid: u32, sun_path: &std::path::Path) -> Option { +fn child_root_path(child_pid: u32, sun_path: &std::path::Path) -> Option { if !sun_path.is_absolute() { return None; } - Some(format!("/proc/{}/root{}", child_pid, sun_path.display())) + let mut bytes = format!("/proc/{}/root", child_pid).into_bytes(); + bytes.extend_from_slice(sun_path.as_os_str().as_bytes()); + Some(std::ffi::OsString::from_vec(bytes)) } /// Pin the inode a named unix socket `sun_path` resolves to **in the child's @@ -60,7 +63,7 @@ fn pin_child_unix_target( }; // `O_PATH` follows symlinks to the real socket inode and pins it without // performing any I/O on the socket. - let c_proc = std::ffi::CString::new(proc_path) + let c_proc = std::ffi::CString::new(proc_path.as_os_str().as_bytes()) .map_err(|_| NotifAction::Errno(libc::EACCES))?; let pinned_raw = unsafe { libc::open(c_proc.as_ptr(), libc::O_PATH | libc::O_CLOEXEC) }; if pinned_raw < 0 { @@ -131,14 +134,21 @@ pub(super) fn connect_named_unix_on_behalf( Ok(fd) => fd, Err(action) => return action, }; - let (sun, len) = match proc_self_fd_sockaddr(pinned.as_raw_fd()) { - Some(s) => s, - None => return NotifAction::Errno(libc::ENAMETOOLONG), - }; let dup_fd = match crate::seccomp::notif::dup_fd_from_pid(child_pid, sockfd) { Ok(fd) => fd, Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)), }; + connect_pinned_unix_on_behalf(dup_fd, pinned) +} + +pub(super) fn connect_pinned_unix_on_behalf( + dup_fd: OwnedFd, + pinned: OwnedFd, +) -> NotifAction { + let (sun, len) = match proc_self_fd_sockaddr(pinned.as_raw_fd()) { + Some(s) => s, + None => return NotifAction::Errno(libc::ENAMETOOLONG), + }; let ret = unsafe { libc::connect( dup_fd.as_raw_fd(), @@ -166,10 +176,28 @@ pub(super) fn sendto_named_unix_on_behalf( sun_path: &std::path::Path, writable: &[std::path::PathBuf], ) -> NotifAction { + 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)), + }; let pinned = match resolve_named_unix_target(notif.pid, sun_path, writable) { Ok(fd) => fd, Err(action) => return action, }; + sendto_pinned_target_on_behalf( + notif, notif_fd, dup_fd, buf_ptr, buf_len, flags, pinned, + ) +} + +pub(super) fn sendto_pinned_target_on_behalf( + notif: &SeccompNotif, + notif_fd: RawFd, + dup_fd: OwnedFd, + buf_ptr: u64, + buf_len: usize, + flags: i32, + pinned: OwnedFd, +) -> NotifAction { let (sun, len) = match proc_self_fd_sockaddr(pinned.as_raw_fd()) { Some(s) => s, None => return NotifAction::Errno(libc::ENAMETOOLONG), @@ -178,10 +206,6 @@ pub(super) fn sendto_named_unix_on_behalf( Ok(b) => b, Err(_) => return NotifAction::Errno(libc::EIO), }; - 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)), - }; // Route through resolve_send like the sendmsg path instead of an inline // blocking sendto: the dup shares the child's blocking mode, so an inline // send on the notification loop wedges the whole loop when a child fills a @@ -262,7 +286,7 @@ pub(super) fn sendto_pinned_unix_on_behalf( /// unix socket. Returns `Some(action)` when the target is a named `AF_UNIX` /// socket (handled here), or `None` to fall through to the IP path (connected /// socket, IP family, abstract socket, or an unreadable header). -pub(super) fn unix_sendmsg_gate( +pub(super) async fn unix_sendmsg_gate( notif: &SeccompNotif, ctx: &Arc, notif_fd: RawFd, @@ -270,20 +294,43 @@ pub(super) fn unix_sendmsg_gate( msghdr_ptr: u64, flags: i32, ) -> Option { - let hdr = ChildMsghdr::read(notif, notif_fd, msghdr_ptr).ok()?; + let hdr = match ChildMsghdr::read(notif, notif_fd, msghdr_ptr) { + Ok(hdr) => hdr, + Err(errno) => return Some(NotifAction::Errno(errno)), + }; if hdr.connected() { return None; // connected socket: no address to gate } - let addr_bytes = - super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?; - // None unless this is a NAMED AF_UNIX target; IP/abstract fall through. - let path = named_unix_socket_path(&addr_bytes)?; + let addr_bytes = match super::read_sockaddr( + notif_fd, + notif.id, + notif.pid, + hdr.name_ptr, + hdr.namelen as usize, + ) { + Ok(bytes) => bytes, + Err(errno) => return Some(NotifAction::Errno(errno)), + }; + let path = match classify_unix_addr(&addr_bytes) { + UnixAddr::Named(path) => path, + UnixAddr::Malformed(errno) => return Some(NotifAction::Errno(errno)), + UnixAddr::NotUnix | UnixAddr::Pathless => return None, + }; if ctx.policy.chroot_root.is_some() { - return Some(if path_under_any(&path, &ctx.policy.chroot_writable) { - NotifAction::Continue - } else { - NotifAction::Errno(libc::EACCES) + let dup_fd = match crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd) { + Ok(fd) => fd, + Err(e) => return Some(NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF))), + }; + return Some(match crate::chroot::dispatch::pin_named_unix_target(notif, &path, ctx).await { + Err(action) => action, + Ok(pinned) => match materialize_named_unix_msghdr_pinned(notif, notif_fd, hdr, pinned) { + Ok(message) => { + let blocking = wants_blocking(dup_fd.as_raw_fd(), flags); + resolve_send(dup_fd, message, flags, blocking) + } + Err(errno) => NotifAction::Errno(errno), + }, }); } Some(sendmsg_named_unix_on_behalf( @@ -337,9 +384,30 @@ fn send_named_unix_msghdr( Err(NotifAction::Errno(e)) => return Err(e), Err(_) => return Err(libc::EACCES), }; - let (sun, sun_len) = proc_self_fd_sockaddr(pinned.as_raw_fd()).ok_or(libc::ENAMETOOLONG)?; - let hdr = ChildMsghdr::read(notif, notif_fd, msghdr_ptr)?; + send_named_unix_msghdr_pinned(notif, notif_fd, sockfd, hdr, pinned) +} + +pub(super) fn send_named_unix_msghdr_pinned( + notif: &SeccompNotif, + notif_fd: RawFd, + sockfd: i32, + hdr: ChildMsghdr, + pinned: OwnedFd, +) -> Result<(OwnedFd, MaterializedMsg), i32> { + let message = materialize_named_unix_msghdr_pinned(notif, notif_fd, hdr, pinned)?; + let dup_fd = crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd) + .map_err(|e| e.raw_os_error().unwrap_or(libc::EBADF))?; + Ok((dup_fd, message)) +} + +pub(super) fn materialize_named_unix_msghdr_pinned( + notif: &SeccompNotif, + notif_fd: RawFd, + hdr: ChildMsghdr, + pinned: OwnedFd, +) -> Result { + let (sun, sun_len) = proc_self_fd_sockaddr(pinned.as_raw_fd()).ok_or(libc::ENAMETOOLONG)?; // The destination is the `/proc/self/fd/` sockaddr; `pinned` must // stay open (and at the same fd number) for that path to resolve, so the @@ -347,12 +415,7 @@ fn send_named_unix_msghdr( let addr = sockaddr_un_bytes(&sun, sun_len); // Named target is always AF_UNIX, so translate SCM_RIGHTS / reject creds. - let m = materialize_msg(notif, notif_fd, &hdr, addr, true, Some(pinned))?; - - let dup_fd = crate::seccomp::notif::dup_fd_from_pid(notif.pid, sockfd) - .map_err(|e| e.raw_os_error().unwrap_or(libc::EBADF))?; - - Ok((dup_fd, m)) + materialize_msg(notif, notif_fd, &hdr, addr, true, Some(pinned)) } /// Read a `sendmmsg` entry's `msg_name` and return its NAMED `AF_UNIX` path, or @@ -364,6 +427,14 @@ pub(super) fn mmsg_entry_named_unix_path( entry_ptr: u64, ) -> Option { let hdr = ChildMsghdr::read(notif, notif_fd, entry_ptr).ok()?; + mmsg_header_named_unix_path(notif, notif_fd, &hdr) +} + +pub(super) fn mmsg_header_named_unix_path( + notif: &SeccompNotif, + notif_fd: RawFd, + hdr: &ChildMsghdr, +) -> Option { if hdr.connected() { return None; } @@ -438,8 +509,8 @@ mod tests { #[test] fn child_root_path_resolves_in_the_childs_root_view() { assert_eq!( - child_root_path(4242, std::path::Path::new("/run/svc.dgram")).as_deref(), - Some("/proc/4242/root/run/svc.dgram") + child_root_path(4242, std::path::Path::new("/run/svc.dgram")), + Some(std::ffi::OsString::from("/proc/4242/root/run/svc.dgram")) ); } @@ -460,6 +531,17 @@ mod tests { ); } + #[test] + fn child_root_path_preserves_non_utf8_components() { + let path = + std::path::PathBuf::from(std::ffi::OsString::from_vec(b"/run/\xff/socket".to_vec())); + let rendered = child_root_path(4242, &path).unwrap(); + assert_eq!( + rendered.as_os_str().as_bytes(), + b"/proc/4242/root/run/\xff/socket" + ); + } + /// End-to-end control for the two assertions above: the builder's output is /// actually what gets opened and pinned. Uses our own pid, whose root view /// is the test process's own root, so the pin is deterministic. diff --git a/crates/sandlock-core/src/network/verdict.rs b/crates/sandlock-core/src/network/verdict.rs index 978663f5..daa0273c 100644 --- a/crates/sandlock-core/src/network/verdict.rs +++ b/crates/sandlock-core/src/network/verdict.rs @@ -94,8 +94,10 @@ pub(crate) fn real_path_under_any(real: &std::path::Path, prefixes: &[std::path: /// filesystem), is at or under any of the granted `prefixes`. Mirrors the /// prefix matching the chroot fs enforcement uses. pub(crate) fn path_under_any(path: &std::path::Path, prefixes: &[std::path::PathBuf]) -> bool { - let norm = crate::chroot::resolve::confine(&path.to_string_lossy()); - prefixes.iter().any(|p| norm.starts_with(p)) + let norm = crate::chroot::resolve::confine_path(path); + prefixes + .iter() + .any(|prefix| norm.starts_with(&crate::chroot::resolve::confine_path(prefix))) } /// The shape of a non-IP destination sockaddr, keyed on its ADDRESS FAMILY @@ -113,9 +115,10 @@ pub(crate) enum DestShape { /// re-resolve in the child's root view. UnixNamed(std::path::PathBuf), /// `sa_family == AF_UNIX` with no usable pathname: an ABSTRACT address - /// (`sun_path[0] == 0`), an unnamed one, a non-UTF-8 `sun_path`, or a - /// buffer too short to carry a family at all. + /// (`sun_path[0] == 0`) or an unnamed one. UnixNoPath, + /// AF_UNIX sockaddr malformed before the kernel could interpret a target. + UnixMalformed(i32), /// `sa_family != AF_UNIX`. Reached here only for families /// `parse_ip_from_sockaddr` does not handle: `AF_NETLINK`, `AF_PACKET`, /// `AF_VSOCK`, ... @@ -163,7 +166,7 @@ pub(crate) enum SendPath { /// Everything else, failed closed with `EAFNOSUPPORT`: /// - a non-IP address on a non-unix socket — the address-family-swap shape; /// - an `AF_UNIX` destination we cannot pin in the child's context: an - /// ABSTRACT address, an empty one, or a non-UTF-8 `sun_path`. + /// ABSTRACT or empty AF_UNIX address. /// /// Abstract addresses fail closed because an on-behalf send is executed by /// the supervisor, which carries no Landlock domain, and @@ -200,7 +203,7 @@ pub(crate) fn classify_send_path( None if !is_unix_socket => SendPath::Reject, None => match dest { DestShape::UnixNamed(_) => SendPath::NamedUnixOnBehalf, - DestShape::UnixNoPath => SendPath::Reject, + DestShape::UnixNoPath | DestShape::UnixMalformed(_) => SendPath::Reject, DestShape::NotUnix => SendPath::RawDestOnBehalf, }, } @@ -351,9 +354,26 @@ mod tests { ); } + #[test] + fn chroot_unix_grant_prefix_matches_raw_bytes_exactly() { + use std::os::unix::ffi::OsStringExt; + let granted = std::path::PathBuf::from(std::ffi::OsString::from_vec(b"/run/\xff".to_vec())); + let allowed = + std::path::PathBuf::from(std::ffi::OsString::from_vec(b"/run/\xff/service".to_vec())); + let different = + std::path::PathBuf::from(std::ffi::OsString::from_vec(b"/run/\xfe/service".to_vec())); + + assert!(path_under_any(&allowed, std::slice::from_ref(&granted))); + assert!(!path_under_any(&different, std::slice::from_ref(&granted))); + assert!(path_under_any( + std::path::Path::new("/run/../run/service"), + &[std::path::PathBuf::from("/run")] + )); + } + #[test] fn send_path_abstract_unix_destination_is_rejected() { - // An abstract (or empty, or non-UTF-8) AF_UNIX address has no pathname + // An abstract or empty AF_UNIX address has no pathname // to pin. It must fail closed rather than be sent on-behalf — the // supervisor carries no Landlock domain, so an on-behalf abstract send // would escape the child's LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET. diff --git a/crates/sandlock-core/src/seccomp/notif.rs b/crates/sandlock-core/src/seccomp/notif.rs index e3cdd9e4..49a61721 100644 --- a/crates/sandlock-core/src/seccomp/notif.rs +++ b/crates/sandlock-core/src/seccomp/notif.rs @@ -2547,16 +2547,20 @@ async fn handle_notification( let fork_counted = matches!(action, NotifAction::Continue) && crate::resource::fork_counted_on_continue(¬if); - // TOCTOU-close for execve (issue #27): freeze every sandbox task - // that could mutate argv before policy_fn reads argv and before the - // kernel re-reads it after Continue. This covers two writer classes: + // Exec argv protection (issue #27): freeze every tracked sandbox task + // that could mutate argv before policy_fn reads it. The kernel's + // post-Continue consumption boundary is not observable here, and an + // outside process with a shared mapping is not enumerable via + // ProcessIndex; do not claim this freeze closes those remaining races. + // For tracked tasks, this covers two writer classes: // 1. Sibling threads of the calling tid (same TGID, share mm). // 2. Peer processes in other TGIDs that alias argv pages via // MAP_SHARED mappings or share mm via clone(CLONE_VM). // - // The freeze enumerates ProcessIndex. With policy_fn active, that - // index is complete: fork-like syscalls are traced at creation time - // below, before new children can run user code. + // The freeze enumerates ProcessIndex. With policy_fn active, sandbox + // fork-like syscalls are traced at creation time below, before new + // children can run user code. This does not include unrelated external + // processes mapping the same MAP_SHARED backing object. // // Strict on failure: if we cannot establish the freeze, we cannot // safely expose argv or allow execve, so we deny with EPERM. @@ -2592,8 +2596,9 @@ async fn handle_notification( } // Emit event to policy_fn callback if active. For execve, argv is - // only populated after `exec_freeze` has stopped every possible - // writer, and those tasks stay stopped until after NOTIF_SEND. + // populated after tracked tasks have been stopped. Peer tasks currently + // stay stopped until NOTIF_SEND returns; that return is not proof that + // the kernel has consumed all execve user-memory arguments. if let Some(verdict) = emit_policy_event(¬if, &action, &ctx.policy_fn, fd).await { use crate::policy_fn::Verdict; match verdict { diff --git a/crates/sandlock-core/src/sys/fs.rs b/crates/sandlock-core/src/sys/fs.rs index c9985eaf..2a8f1534 100644 --- a/crates/sandlock-core/src/sys/fs.rs +++ b/crates/sandlock-core/src/sys/fs.rs @@ -6,6 +6,7 @@ //! path opens through it (see issue #112). use std::ffi::CString; +use std::os::unix::ffi::OsStrExt; use std::os::unix::io::RawFd; use std::path::Path; @@ -40,7 +41,7 @@ pub(crate) fn openat2_in_root( flags: i32, mode: u32, ) -> Result { - openat2_in_root_with_resolve(root, path, flags, mode, 0) + openat2_in_root_path_with_resolve(root, Path::new(path), flags, mode, 0) } /// As [`openat2_in_root`], plus the caller's own `RESOLVE_*` flags. @@ -57,7 +58,21 @@ pub(crate) fn openat2_in_root_with_resolve( mode: u32, extra_resolve: u64, ) -> Result { - let c_root = CString::new(root.to_str().unwrap_or("")).map_err(|_| libc::EINVAL)?; + openat2_in_root_path_with_resolve(root, Path::new(path), flags, mode, extra_resolve) +} + +/// Byte-preserving variant of [`openat2_in_root_with_resolve`]. Filesystem +/// names on Linux are arbitrary non-NUL byte strings; this is needed for +/// pathname AF_UNIX targets and other kernel names that do not pass through a +/// UTF-8 API. +pub(crate) fn openat2_in_root_path_with_resolve( + root: &Path, + path: &Path, + flags: i32, + mode: u32, + extra_resolve: u64, +) -> Result { + let c_root = CString::new(root.as_os_str().as_bytes()).map_err(|_| libc::EINVAL)?; let root_fd = unsafe { libc::open( c_root.as_ptr(), @@ -68,9 +83,13 @@ pub(crate) fn openat2_in_root_with_resolve( return Err(last_errno(libc::EIO)); } - let rel_path = path.strip_prefix('/').unwrap_or(path); - let rel_path = if rel_path.is_empty() { "." } else { rel_path }; - let c_path = CString::new(rel_path).map_err(|_| { + let rel_path = path.strip_prefix("/").unwrap_or(path); + let rel_path = if rel_path.as_os_str().is_empty() { + Path::new(".") + } else { + rel_path + }; + let c_path = CString::new(rel_path.as_os_str().as_bytes()).map_err(|_| { unsafe { libc::close(root_fd) }; libc::EINVAL })?; @@ -606,6 +625,32 @@ mod tests { use std::os::unix::fs::symlink; use tempfile::TempDir; + #[test] + fn openat2_in_root_preserves_non_utf8_path_components() { + use std::os::unix::ffi::OsStringExt; + use std::os::unix::io::FromRawFd; + + let tmp = TempDir::new().unwrap(); + let path = std::ffi::OsString::from_vec(b"entry-\xff".to_vec()); + std::fs::write(tmp.path().join(&path), b"pinned").unwrap(); + + let fd = match openat2_in_root_path_with_resolve( + tmp.path(), + Path::new(&path), + libc::O_RDONLY | libc::O_CLOEXEC, + 0, + 0, + ) { + Ok(fd) => fd, + Err(libc::ENOSYS) => return, + Err(errno) => panic!("openat2 failed: {errno}"), + }; + let mut file = unsafe { std::fs::File::from_raw_fd(fd) }; + let mut contents = Vec::new(); + std::io::Read::read_to_end(&mut file, &mut contents).unwrap(); + assert_eq!(contents, b"pinned"); + } + #[test] fn openat2_in_root_confines_absolute_symlink() { let tmp = TempDir::new().unwrap(); diff --git a/docs/policy-fn.md b/docs/policy-fn.md index f94ffda6..7e4aeb22 100644 --- a/docs/policy-fn.md +++ b/docs/policy-fn.md @@ -57,15 +57,20 @@ positive int = deny with errno, `"audit"`/`-2` = allow + flag. > belongs in static Landlock rules (`fs_readable` / `fs_writable` / > `fs_denied`), kernel-enforced and TOCTOU-immune. Use > `ctx.deny_path()` for runtime additions. -> - **`event.argv` is exposed and TOCTOU-safe.** Before exposing -> `argv` to `policy_fn` or returning `Continue` for an -> `execve`, the supervisor freezes every task in `ProcessIndex`, -> including peer processes that may alias argv through shared memory. +> - **`event.argv` is read after a tracked-task freeze, with a remaining +> completion-boundary limitation.** Before exposing `argv` to +> `policy_fn`, the supervisor attempts to freeze every task in +> `ProcessIndex`, including tracked peers that may alias argv through +> shared memory. This cannot discover an outside process that independently +> maps the same `MAP_SHARED` backing object. Also, successful +> `SECCOMP_IOCTL_NOTIF_SEND` is not proof that the kernel has finished +> consuming execve's user-memory arguments; the current peer-release point +> therefore does not establish a complete TOCTOU guarantee. > With `policy_fn` active, fork-like syscalls are traced for one > ptrace creation event, so children are registered in `ProcessIndex` -> before they can run user code. If the freeze or creation tracking -> cannot be established (e.g., YAMA blocks ptrace), the syscall is -> denied with `EPERM`; the safety invariant is never silently relaxed. +> before they can run user code. If thread enumeration, the freeze, or +> creation tracking cannot be established (e.g., YAMA blocks ptrace), the +> syscall is denied with `EPERM`. **Context methods:** - `ctx.restrict_network(ips)` / `ctx.grant_network(ips)`: network control