fix: require chroot write grants for open side effects

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

@@ -143,6 +143,25 @@ impl ChrootCtx<'_> {
         self.writable.iter().any(|p| virtual_path.starts_with(p))
     }
 
+    /// The access mode governs the returned fd; creation and truncation are
+    /// separate effects, even when the requested fd is O_RDONLY. The
+    /// supervisor opens on behalf of the child, so Landlock cannot enforce
+    /// these checks on the supervisor's open.
+    fn can_open(&self, virtual_path: &Path, flags: i32) -> bool {
+        if flags & libc::O_PATH != 0 {
+            // O_PATH does not mutate or open file contents. Invalid flag
+            // combinations are still rejected by the openat2 path below.
+            return self.can_read(virtual_path);
+        }
+        let mode = flags & libc::O_ACCMODE;
+        let reads = mode != libc::O_WRONLY;
+        let writes = mode == libc::O_WRONLY
+            || mode == libc::O_RDWR
+            || flags & (libc::O_CREAT | libc::O_TRUNC) != 0
+            || flags & libc::O_TMPFILE == libc::O_TMPFILE;
+        (!reads || self.can_read(virtual_path)) && (!writes || self.can_write(virtual_path))
+    }
+
     /// Check if a virtual path falls under any mount point.
     fn is_mounted(&self, virtual_path: &Path) -> bool {
         self.mounts.iter().any(|(vp, _)| virtual_path.starts_with(vp))
@@ -594,13 +613,8 @@ pub(crate) async fn handle_chroot_open(
         None => return NotifAction::Errno(libc::EACCES),
     };
 
-    // Access check: writes need can_write, reads need can_read
-    let is_write = (flags as i32 & (libc::O_WRONLY | libc::O_RDWR)) != 0;
-    if is_write {
-        if !ctx.can_write(&virtual_path) {
-            return NotifAction::Errno(libc::EACCES);
-        }
-    } else if !ctx.can_read(&virtual_path) {
+    // Check both descriptor access and effects before any COW/open work.
+    if !ctx.can_open(&virtual_path, flags as i32) {
         return NotifAction::Errno(libc::EACCES);
     }
 
@@ -2702,4 +2716,24 @@ mod mount_ro_tests {
         assert!(c.can_read(Path::new("/data/file")));
         assert!(c.can_write(Path::new("/data/file")));
     }
+
+    #[test]
+    fn open_effects_require_write_permission_independently_of_fd_mode() {
+        let mounts = vec![(PathBuf::from("/ro"), PathBuf::from("/host"))];
+        let read_only = vec![PathBuf::from("/ro")];
+        let writable = vec![PathBuf::from("/")];
+        let processes = Arc::new(ProcessIndex::new());
+        let c = ctx(&mounts, &read_only, &writable, &processes);
+        for flags in [libc::O_RDONLY, libc::O_RDONLY | libc::O_DIRECTORY, libc::O_PATH] {
+            assert!(c.can_open(Path::new("/ro/file"), flags), "{flags:#x}");
+        }
+        for flags in [
+            libc::O_WRONLY, libc::O_RDWR, libc::O_RDONLY | libc::O_CREAT,
+            libc::O_RDONLY | libc::O_TRUNC, libc::O_RDONLY | libc::O_CREAT | libc::O_EXCL,
+            libc::O_RDWR | libc::O_TMPFILE,
+        ] {
+            assert!(!c.can_open(Path::new("/ro/file"), flags), "{flags:#x}");
+            assert!(c.can_open(Path::new("/rw/file"), flags), "{flags:#x}");
+        }
+    }
 }

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

@@ -2465,3 +2465,78 @@ async fn test_chroot_shebang_loop_is_eloop() {
     );
     cleanup_rootfs(&rootfs);
 }
+
+/// O_RDONLY describes the returned fd, not all effects of open(2): O_TRUNC
+/// and O_CREAT still mutate the filesystem in the supervisor's context.
+#[tokio::test]
+async fn test_chroot_readonly_open_cannot_mutate_files() {
+    check_readonly_open_effects(false).await;
+}
+
+#[tokio::test]
+async fn test_chroot_cow_readonly_open_cannot_mutate_files() {
+    check_readonly_open_effects(true).await;
+}
+
+async fn check_readonly_open_effects(cow: bool) {
+    let rootfs = build_test_rootfs(&format!("readonly-open-mutation-{cow}"));
+    let mounted = temp_dir(&format!("readonly-open-mutation-mount-{cow}"));
+    fs::create_dir_all(rootfs.join("ro")).unwrap();
+    fs::create_dir_all(rootfs.join("rw")).unwrap();
+    fs::create_dir_all(rootfs.join("mnt")).unwrap();
+    let mut builder = minimal_exec_policy(&rootfs)
+        .fs_read("/ro")
+        .fs_write("/rw")
+        .fs_mount_ro("/mnt", &mounted);
+    if cow { builder = builder.workdir(&rootfs).on_exit(BranchAction::Commit); }
+    let policy = builder.build().unwrap();
+    let mut failures = Vec::new();
+    for spelling in ["open", "openat", "openat2"] {
+        for (virtual_dir, host_dir, allowed) in [
+            ("/ro", rootfs.join("ro"), false),
+            ("/mnt", mounted.clone(), false),
+            ("/rw", rootfs.join("rw"), true),
+        ] {
+            for (name, flags, exists) in [
+                ("read", libc::O_RDONLY, true),
+                ("truncate", libc::O_RDONLY | libc::O_TRUNC, true),
+                ("create", libc::O_RDONLY | libc::O_CREAT, false),
+                ("create-exclusive", libc::O_RDONLY | libc::O_CREAT | libc::O_EXCL, false),
+                ("path-only", libc::O_PATH | libc::O_TRUNC, true),
+            ] {
+                let basename = format!("{spelling}-{name}");
+                let host = host_dir.join(&basename);
+                if exists { fs::write(&host, b"KEEP").unwrap(); }
+                let path = format!("{virtual_dir}/{basename}");
+                let flags_text = flags.to_string();
+                let result = policy.clone().run(&[
+                    "rootfs-helper", "open-flags", spelling, &path, &flags_text,
+                ]).await.expect("sandbox must run: no skip on setup errors");
+                assert!(result.success(), "helper failed: {:?}", result.stderr_str());
+                let mutates = name != "read" && name != "path-only";
+                let denied = mutates && !allowed;
+                let expected = if name == "path-only" {
+                    format!("ERR:{}", libc::EINVAL)
+                } else if denied { format!("ERR:{}", libc::EACCES) } else { "OPENED".into() };
+                let output = result.stdout_str().unwrap().trim();
+                if output != expected {
+                    failures.push(format!("{path}: expected {expected}, got {output}"));
+                }
+                if denied || !mutates {
+                    if exists {
+                        if fs::read(&host).unwrap() != b"KEEP" {
+                            failures.push(format!("{path}: original bytes changed"));
+                        }
+                    } else if host.exists() {
+                        failures.push(format!("{path}: forbidden file was created"));
+                    }
+                } else if !host.exists() || !fs::read(&host).unwrap().is_empty() {
+                    failures.push(format!("{path}: allowed operation did not take effect"));
+                }
+            }
+        }
+    }
+    cleanup_rootfs(&rootfs);
+    fs::remove_dir_all(mounted).unwrap();
+    assert!(failures.is_empty(), "{}", failures.join("\n"));
+}

tests/rootfs-helper.c

@@ -731,6 +731,31 @@ struct helper_open_how {
     unsigned long long resolve;
 };
 
+/* Report the raw errno without hiding failed syscall execution behind a skip. */
+static int cmd_open_flags(int argc, char **argv) {
+    if (argc != 3) return 2;
+    int flags = (int)strtol(argv[2], NULL, 0);
+    unsigned int mode = (flags & O_CREAT) ? 0600 : 0;
+    long fd;
+    if (strcmp(argv[0], "openat2") == 0) {
+        struct helper_open_how how = { .flags = (unsigned int)flags, .mode = mode };
+        fd = syscall(__NR_openat2, AT_FDCWD, argv[1], &how, sizeof(how));
+    } else if (strcmp(argv[0], "openat") == 0) {
+        fd = syscall(SYS_openat, AT_FDCWD, argv[1], flags, mode);
+    } else if (strcmp(argv[0], "open") == 0) {
+#ifdef SYS_open
+        fd = syscall(SYS_open, argv[1], flags, mode);
+#else
+        fd = syscall(SYS_openat, AT_FDCWD, argv[1], flags, mode);
+#endif
+    } else {
+        return 2;
+    }
+    if (fd < 0) printf("ERR:%d\n", errno);
+    else { close((int)fd); puts("OPENED"); }
+    return 0;
+}
+
 static int cmd_openat2(int argc, char **argv) {
     if (argc < 1) { fprintf(stderr, "openat2: missing operand\n"); return 1; }
     /* Second operand, when present, is a RESOLVE_* mask (decimal). */
@@ -880,6 +905,7 @@ static int cmd_fexecve(int argc, char **argv) {
 /* ── dispatch ───────────────────────────────────────────────── */
 
 static int dispatch(const char *cmd, int argc, char **argv) {
+    if (strcmp(cmd, "open-flags") == 0)    return cmd_open_flags(argc, argv);
     if (strcmp(cmd, "chdir") == 0)          return cmd_chdir(argc, argv);
     if (strcmp(cmd, "fchdir") == 0)         return cmd_fchdir(argc, argv);
     if (strcmp(cmd, "openat2") == 0)        return cmd_openat2(argc, argv);