cow: gate reads on ownership, serve O_PATH opens, and fix legacy readlink

crates/sandlock-core/src/cow/dispatch.rs

@@ -198,6 +198,29 @@ pub(crate) fn open_confined(
     crate::sys::fs::openat2_in_root(root, &rel, flags, mode)
 }
 
+/// Reopen an `O_PATH` descriptor as `O_RDONLY`, for a regular file or a
+/// directory. `None` for anything else. TOCTOU-safe: the stat and the
+/// reopen both go through the held descriptor.
+fn reopen_o_path_readable(fd: RawFd) -> Option<RawFd> {
+    let mut st: libc::stat = unsafe { std::mem::zeroed() };
+    let empty = c"".as_ptr();
+    if unsafe { libc::fstatat(fd, empty, &mut st, libc::AT_EMPTY_PATH) } < 0 {
+        return None;
+    }
+    let flags = match st.st_mode & libc::S_IFMT {
+        libc::S_IFDIR => libc::O_RDONLY | libc::O_DIRECTORY | libc::O_CLOEXEC,
+        libc::S_IFREG => libc::O_RDONLY | libc::O_CLOEXEC,
+        _ => return None, // symlink, FIFO, socket, device
+    };
+    let link = std::ffi::CString::new(format!("/proc/self/fd/{}", fd)).ok()?;
+    let reopened = unsafe { libc::open(link.as_ptr(), flags) };
+    if reopened < 0 {
+        return None;
+    }
+    unsafe { libc::close(fd) };
+    Some(reopened)
+}
+
 /// The branch's directories, taken under the lock for work done without it.
 #[derive(Clone)]
 pub(crate) struct Layers {
@@ -433,6 +456,22 @@ impl CowOpen {
             // The kernel cannot see the upper layer, so its answer would be wrong.
             Err(errno) => return NotifAction::Errno(errno),
         };
+
+        // ADDFD refuses an O_PATH source fd, so hand over a readable handle to
+        // the same entry. Under COW the workdir governs access to upper-layer
+        // entries, so this grants nothing a plain O_RDONLY would not. The
+        // child's fd reports O_RDONLY via F_GETFL, not O_PATH, and read() on
+        // it succeeds. Entries without read permission still return
+        // EOPNOTSUPP.
+        let fd = if self.open_flags & libc::O_PATH != 0 {
+            match reopen_o_path_readable(fd) {
+                Some(reopened) => reopened,
+                None => fd,
+            }
+        } else {
+            fd
+        };
+
         // Preserve O_CLOEXEC from the original openat flags.
         let newfd_flags = if self.flags & libc::O_CLOEXEC as u64 != 0 {
             libc::O_CLOEXEC as u32
@@ -1122,7 +1161,7 @@ pub(crate) async fn handle_cow_stat(
         let upper_root = cow.upper_dir().to_path_buf();
         let workdir_root = cow.workdir().to_path_buf();
         let path = map_cow_upper_path(cow, &path);
-        if !cow.has_changes() || !cow.matches(&path) {
+        if !cow.has_changes() || !cow.matches(&path) || !cow.needs_read_intercept(&path) {
             return NotifAction::Continue;
         }
         let real = match cow.handle_stat(&path) {
@@ -1226,7 +1265,7 @@ pub(crate) async fn handle_cow_statx(
         let upper_root = cow.upper_dir().to_path_buf();
         let workdir_root = cow.workdir().to_path_buf();
         let path = map_cow_upper_path(cow, &path);
-        if !cow.has_changes() || !cow.matches(&path) {
+        if !cow.has_changes() || !cow.matches(&path) || !cow.needs_read_intercept(&path) {
             return NotifAction::Continue;
         }
         let real = match cow.handle_stat(&path) {
@@ -1311,7 +1350,7 @@ pub(crate) async fn handle_cow_exec(
         let upper_root = cow.upper_dir().to_path_buf();
         let workdir_root = cow.workdir().to_path_buf();
         let path = map_cow_upper_path(cow, &resolved);
-        if !cow.has_changes() || !cow.matches(&path) {
+        if !cow.has_changes() || !cow.matches(&path) || !cow.needs_read_intercept(&path) {
             return NotifAction::Continue;
         }
         let real = match cow.handle_stat(&path) {
@@ -1381,15 +1420,21 @@ pub(crate) async fn handle_cow_readlink(
     processes: &Arc<ProcessIndex>,
     notif_fd: RawFd,
 ) -> NotifAction {
-    // readlinkat(dirfd, pathname, buf, bufsiz)
-    let dirfd = notif.data.args[0] as i64;
+    // readlinkat(dirfd, pathname, buf, bufsiz), and legacy
+    // readlink(pathname, buf, bufsiz), which shifts every argument by one.
+    let a = notif.data.args;
+    let (dirfd, path_ptr, buf_addr, bufsiz) = if arch::sys_readlink() == Some(notif.data.nr as i64)
+    {
+        (libc::AT_FDCWD as i64, a[0], a[1], a[2])
+    } else {
+        (a[0] as i64, a[1], a[2], a[3])
+    };
+    let bufsiz = (bufsiz & 0xFFFFFFFF) as usize;
     let virtual_cwd = current_virtual_cwd(processes, notif.pid).await;
-    let path = match read_path(notif, notif.data.args[1], notif_fd) {
+    let path = match read_path(notif, path_ptr, notif_fd) {
         Some(p) => resolve_at_path_with_virtual(notif, dirfd, &p, virtual_cwd.as_deref()),
         None => return NotifAction::Continue,
     };
-    let buf_addr = notif.data.args[2];
-    let bufsiz = (notif.data.args[3] & 0xFFFFFFFF) as usize;
 
     let st = cow_state.lock().await;
     let cow = match st.branch.as_ref() {
@@ -1398,7 +1443,7 @@ pub(crate) async fn handle_cow_readlink(
     };
 
     let path = map_cow_upper_path(cow, &path);
-    if !cow.has_changes() || !cow.matches(&path) {
+    if !cow.has_changes() || !cow.matches(&path) || !cow.needs_read_intercept(&path) {
         return NotifAction::Continue;
     }
 

crates/sandlock-core/src/cow/seccomp.rs

@@ -1099,7 +1099,8 @@ impl SeccompCowBranch {
     /// read must be redirected there, never fall through to the lower file.
     pub fn needs_read_intercept(&self, path: &str) -> bool {
         if let Some(rel) = self.safe_rel(path) {
-            self.deleted.covers(&rel) || self.upper.join(&rel).exists()
+            let upper_entry = self.upper.join(&rel);
+            self.deleted.covers(&rel) || upper_entry.exists() || upper_entry.is_symlink()
         } else {
             false
         }
@@ -3971,6 +3972,20 @@ mod tests {
         );
     }
 
+    /// A dangling symlink staged in upper (target does not exist) must still
+    /// report as intercepted: `Path::exists()` follows symlinks, so a naive
+    /// check would miss it.
+    #[test]
+    fn needs_read_intercept_sees_a_dangling_symlink_in_upper() {
+        let (workdir, storage) = setup_workdir();
+        let mut branch = SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap();
+        let lnk = abs(&branch, "lnk");
+
+        assert!(!branch.needs_read_intercept(&lnk));
+        branch.handle_symlink("missing-target", &lnk).unwrap();
+        assert!(branch.needs_read_intercept(&lnk));
+    }
+
     /// Nothing outside the workdir may be mapped into the upper. `safe_rel` is
     /// the only thing standing between a host path and a COW copy of it, so it
     /// must reject both an ordinary escape and the string-prefix trap — a

crates/sandlock-core/tests/integration/test_cow.rs

@@ -2105,3 +2105,203 @@ async fn test_seccomp_cow_fchmodat2_is_virtualized() {
     assert!(!workdir.join("made").exists());
     let _ = fs::remove_dir_all(&workdir);
 }
+
+/// A read is intercepted only for a path the branch actually owns; anything
+/// else must reach the kernel, not the supervisor. /proc/self and a pipe
+/// made in the child (not inherited, so the supervisor has no fd of the
+/// same number pointing at it) make the unowned case observable, since
+/// intercepted they'd answer for the supervisor instead of the caller.
+#[tokio::test]
+async fn test_seccomp_cow_read_intercept_follows_ownership() {
+    let staged = std::env::temp_dir().join(format!("sandlock-proc-self-{}", std::process::id()));
+    let _ = fs::remove_dir_all(&staged);
+
+    let script = format!(
+        concat!(
+            "import os, sys\n",
+            "os.mkdir('{staged}')\n", // staged in upper, this one must be intercepted
+            "dfd = os.open('/etc', os.O_RDONLY | os.O_DIRECTORY)\n",
+            "exe = os.readlink('/proc/self/exe', dir_fd=dfd)\n",
+            "r, w = os.pipe()\n",
+            "own_pipe = os.stat('/proc/self/fd/%d' % r).st_ino == os.fstat(r).st_ino\n",
+            "staged_ok = os.path.isdir('{staged}')\n",
+            "sys.stdout.write(exe + '\\n' + str(own_pipe) + '\\n' + str(staged_ok))\n",
+        ),
+        staged = staged.display()
+    );
+
+    let mut policy = Sandbox::builder()
+        .fs_read("/usr")
+        .fs_read("/lib")
+        .fs_read_if_exists("/lib64")
+        .fs_read("/bin")
+        .fs_read("/etc")
+        .fs_read("/proc")
+        .fs_read("/dev")
+        .fs_write("/tmp")
+        .workdir("/")
+        .cwd("/tmp")
+        .on_exit(BranchAction::Abort)
+        .build()
+        .unwrap();
+
+    let result = policy.run(&["python3", "-c", &script]).await.unwrap();
+    assert!(
+        result.success(),
+        "exit={:?}, stderr={}",
+        result.code(),
+        result.stderr_str().unwrap_or("")
+    );
+
+    let out = result.stdout_str().unwrap_or("").to_string();
+    let mut lines = out.lines();
+    let exe = lines.next().unwrap_or("");
+    let own_pipe = lines.next().unwrap_or("");
+    let staged_ok = lines.next().unwrap_or("");
+    assert!(
+        exe.contains("python"),
+        "not intercepted: /proc/self/exe must name the child's interpreter, not the supervisor: {exe}"
+    );
+    assert_eq!(
+        own_pipe, "True",
+        "not intercepted: /proc/self/fd/N must resolve to the child's own fd, not the supervisor's"
+    );
+    assert_eq!(
+        staged_ok, "True",
+        "intercepted: the staged directory is owned by the branch and must still be visible"
+    );
+
+    let _ = fs::remove_dir_all(&staged);
+}
+
+/// readlink(path, buf, size) and readlinkat(dirfd, path, buf, size) do not
+/// share an argument layout. Read as readlinkat, a legacy readlink has its
+/// output buffer taken as the path, so the branch never recognises the call
+/// and a symlink created during the run cannot be read back.
+#[tokio::test]
+async fn test_seccomp_cow_legacy_readlink_reads_its_own_arguments() {
+    let workdir = temp_dir("seccomp-cow-legacy-readlink");
+
+    let script = concat!(
+        "import os, sys\n",
+        "os.symlink('target', 'lnk')\n", // exists only in the upper layer
+        "try:\n",
+        "    sys.stdout.write(os.readlink('lnk'))\n",
+        "except OSError as e:\n",
+        "    sys.stdout.write('errno=%d' % e.errno)\n",
+    );
+
+    let mut policy = Sandbox::builder()
+        .fs_read("/usr")
+        .fs_read("/lib")
+        .fs_read_if_exists("/lib64")
+        .fs_read("/bin")
+        .fs_read("/etc")
+        .fs_read("/proc")
+        .fs_read("/dev")
+        .fs_write(&workdir)
+        .fs_write("/tmp")
+        .workdir(&workdir)
+        .cwd(&workdir)
+        .on_exit(BranchAction::Abort)
+        .build()
+        .unwrap();
+
+    let result = policy.run(&["python3", "-c", script]).await.unwrap();
+    assert!(
+        result.success(),
+        "exit={:?}, stderr={}",
+        result.code(),
+        result.stderr_str().unwrap_or("")
+    );
+    assert_eq!(
+        result.stdout_str().unwrap_or("").trim(),
+        "target",
+        "a symlink created during the run must read back through legacy readlink"
+    );
+
+    let _ = fs::remove_dir_all(&workdir);
+}
+
+/// An O_PATH open of an entry that lives only in the upper layer must be
+/// served. ADDFD refuses an O_PATH source descriptor, so the supervisor hands
+/// over a read-only handle to the same entry instead, which grants nothing a
+/// plain O_RDONLY would not under COW.
+///
+/// Callers do not reliably pass O_DIRECTORY even when opening a directory, so
+/// the decision is made on what the entry is, not on the flags.
+#[tokio::test]
+async fn test_seccomp_cow_o_path_open_of_created_entries() {
+    let workdir = temp_dir("seccomp-cow-o-path-kinds");
+
+    let script = format!(
+        concat!(
+            "import os, sys, fcntl\n",
+            "os.mkdir('{wd}/d')\n",
+            "open('{wd}/f', 'w').write('x')\n",
+            "open('{wd}/noperm', 'w').write('x')\n",
+            "os.chmod('{wd}/noperm', 0)\n",
+            "def probe_dir(p):\n",
+            "    try:\n",
+            "        fd = os.open(p, os.O_PATH)\n",
+            "        os.close(fd)\n",
+            "        return 'ok'\n",
+            "    except OSError as e:\n",
+            "        return 'errno=%d' % e.errno\n",
+            "def probe_file(p):\n",
+            "    try:\n",
+            "        fd = os.open(p, os.O_PATH)\n",
+            "    except OSError as e:\n",
+            "        return 'errno=%d' % e.errno\n",
+            "    try:\n",
+            "        rdonly = (fcntl.fcntl(fd, fcntl.F_GETFL) & os.O_ACCMODE) == os.O_RDONLY\n",
+            "        return 'ok rdonly=%s data=%s' % (rdonly, os.read(fd, 1) == b'x')\n",
+            "    finally:\n",
+            "        os.close(fd)\n",
+            // no O_DIRECTORY on the directory: that is what callers actually do
+            "sys.stdout.write('dir=%s|file=%s|noperm=%s' % (\n",
+            "    probe_dir('{wd}/d'), probe_file('{wd}/f'), probe_file('{wd}/noperm')))\n",
+        ),
+        wd = workdir.display()
+    );
+
+    let mut policy = Sandbox::builder()
+        .fs_read("/usr")
+        .fs_read("/lib")
+        .fs_read_if_exists("/lib64")
+        .fs_read("/bin")
+        .fs_read("/etc")
+        .fs_read("/proc")
+        .fs_read("/dev")
+        .fs_write(&workdir)
+        .fs_write("/tmp")
+        .workdir(&workdir)
+        .cwd(&workdir)
+        .on_exit(BranchAction::Abort)
+        .build()
+        .unwrap();
+    let result = policy.run(&["python3", "-c", &script]).await.unwrap();
+    assert!(
+        result.success(),
+        "exit={:?}, stderr={}",
+        result.code(),
+        result.stderr_str().unwrap_or("")
+    );
+
+    let got = result.stdout_str().unwrap_or("").trim().to_string();
+    let file_ok = "file=ok rdonly=True data=True";
+    let noperm = if unsafe { libc::geteuid() } == 0 {
+        // A root supervisor's reopen bypasses the permission bit too.
+        "noperm=ok rdonly=True data=True".to_string()
+    } else {
+        format!("noperm=errno={}", libc::EOPNOTSUPP)
+    };
+    assert_eq!(
+        got,
+        format!("dir=ok|{file_ok}|{noperm}"),
+        "O_PATH of an upper-layer entry: served with a real O_RDONLY fd, \
+         not a bare O_PATH one"
+    );
+
+    let _ = fs::remove_dir_all(&workdir);
+}