From ce3cc8794510886bb83e569e0b1a7180cc244648 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Mon, 20 Jul 2026 18:42:41 -0700 Subject: [PATCH] fix(supervisor): tailor Landlock rights by inode type Closes #2218 Signed-off-by: Drew Newberry --- architecture/security-policy.md | 7 + .../src/sandbox/linux/landlock.rs | 120 +++++++++++++++++- e2e/rust/tests/landlock.rs | 74 +++++++++++ 3 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 e2e/rust/tests/landlock.rs diff --git a/architecture/security-policy.md b/architecture/security-policy.md index a4d20b9b0f..c91ba445eb 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -28,6 +28,13 @@ enrichment adds existing GPU device nodes as read-write paths and promotes `/proc` to read-write because CUDA workloads write thread metadata under `/proc//task//comm`. +Landlock rules are tailored to the inode type reported by the already-opened +path descriptor. Directories retain the requested directory and file rights; +regular files, device nodes, sockets, and other non-directories retain only +file-compatible rights. This avoids rejecting valid mixed-path policies without +weakening `hard_requirement`: genuine unsupported ABI capabilities and +preparation failures still fail sandbox startup. + ## Network Decisions Ordinary network traffic follows this order: diff --git a/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs b/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs index fb82cf02fd..bf42faede8 100644 --- a/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs +++ b/crates/openshell-supervisor-process/src/sandbox/linux/landlock.rs @@ -4,11 +4,12 @@ //! Landlock filesystem sandboxing. use landlock::{ - ABI, Access, AccessFs, CompatLevel, Compatible, PathBeneath, PathFd, PathFdError, Ruleset, - RulesetAttr, RulesetCreatedAttr, + ABI, Access, AccessFs, BitFlags, CompatLevel, Compatible, PathBeneath, PathFd, PathFdError, + Ruleset, RulesetAttr, RulesetCreatedAttr, }; use miette::{IntoDiagnostic, Result}; use openshell_core::policy::{LandlockCompatibility, SandboxPolicy}; +use std::os::fd::AsFd; use std::path::{Path, PathBuf}; use tracing::debug; @@ -216,9 +217,10 @@ fn prepare_with_path_open_mode( for path in &read_only { if let Some(path_fd) = try_open_path(path, compatibility, path_open_mode)? { + let allowed_access = access_for_path_fd(&path_fd, access_read, abi)?; debug!(path = %path.display(), "Landlock allow read-only"); ruleset = ruleset - .add_rule(PathBeneath::new(path_fd, access_read)) + .add_rule(PathBeneath::new(path_fd, allowed_access)) .into_diagnostic()?; rules_applied += 1; } @@ -226,9 +228,10 @@ fn prepare_with_path_open_mode( for path in &read_write { if let Some(path_fd) = try_open_path(path, compatibility, path_open_mode)? { + let allowed_access = access_for_path_fd(&path_fd, access_all, abi)?; debug!(path = %path.display(), "Landlock allow read-write"); ruleset = ruleset - .add_rule(PathBeneath::new(path_fd, access_all)) + .add_rule(PathBeneath::new(path_fd, allowed_access)) .into_diagnostic()?; rules_applied += 1; } @@ -344,6 +347,23 @@ pub fn apply(policy: &SandboxPolicy, workdir: Option<&str>) -> Result<()> { Ok(()) } +/// Tailor a rule's access mask to the inode referenced by its already-open FD. +/// +/// Landlock directory-only rights such as `ReadDir` are invalid for regular +/// files and device nodes in hard-requirement mode. Classifying through the +/// same `PathFd` used by the rule avoids a pathname TOCTOU race. +fn access_for_path_fd( + path_fd: &PathFd, + requested_access: BitFlags, + abi: ABI, +) -> Result> { + let stat = rustix::fs::fstat(path_fd.as_fd()).into_diagnostic()?; + Ok(match rustix::fs::FileType::from_raw_mode(stat.st_mode) { + rustix::fs::FileType::Directory => requested_access, + _ => requested_access & AccessFs::from_file(abi), + }) +} + /// Attempt to open a path for Landlock rule creation. /// /// In `BestEffort` mode, inaccessible paths (missing, permission denied, symlink @@ -468,6 +488,98 @@ fn compat_level(level: &LandlockCompatibility) -> CompatLevel { #[cfg(test)] mod tests { use super::*; + use openshell_core::policy::{FilesystemPolicy, LandlockPolicy, NetworkPolicy, ProcessPolicy}; + + fn hard_requirement_policy(read_only: Vec, read_write: Vec) -> SandboxPolicy { + SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy { + read_only, + read_write, + include_workdir: false, + }, + network: NetworkPolicy::default(), + landlock: LandlockPolicy { + compatibility: LandlockCompatibility::HardRequirement, + }, + process: ProcessPolicy::default(), + } + } + + #[test] + fn prepare_hard_requirement_accepts_device_paths() { + if !matches!(probe_availability(), LandlockAvailability::Available { .. }) { + return; + } + + let policy = hard_requirement_policy( + vec![PathBuf::from("/tmp"), PathBuf::from("/dev/urandom")], + vec![PathBuf::from("/dev/null")], + ); + + let result = prepare(&policy, None); + if let Err(err) = result { + panic!("hard_requirement should accept mixed directory and device paths: {err}"); + } + } + fn tailored_access(path: &Path, requested_access: BitFlags) -> BitFlags { + let path_fd = PathFd::new(path).unwrap(); + access_for_path_fd(&path_fd, requested_access, ABI::V2).unwrap() + } + + #[test] + fn access_for_path_fd_preserves_directory_access() { + let dir = tempfile::tempdir().unwrap(); + let requested_access = AccessFs::from_all(ABI::V2); + + assert_eq!( + tailored_access(dir.path(), requested_access), + requested_access + ); + } + + #[test] + fn access_for_path_fd_limits_regular_file_access() { + let file = tempfile::NamedTempFile::new().unwrap(); + let requested_access = AccessFs::from_all(ABI::V2); + + assert_eq!( + tailored_access(file.path(), requested_access), + requested_access & AccessFs::from_file(ABI::V2) + ); + } + + #[test] + fn access_for_path_fd_limits_character_device_access() { + let requested_read = AccessFs::from_read(ABI::V2); + let requested_write = AccessFs::from_all(ABI::V2); + + assert_eq!( + tailored_access(Path::new("/dev/urandom"), requested_read), + requested_read & AccessFs::from_file(ABI::V2) + ); + assert_eq!( + tailored_access(Path::new("/dev/null"), requested_write), + requested_write & AccessFs::from_file(ABI::V2) + ); + } + + #[test] + fn access_for_path_fd_classifies_symlink_target() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("target"); + let link = dir.path().join("link"); + std::fs::File::create(&target).unwrap(); + symlink(&target, &link).unwrap(); + + let requested_access = AccessFs::from_all(ABI::V2); + assert_eq!( + tailored_access(&link, requested_access), + requested_access & AccessFs::from_file(ABI::V2) + ); + } #[test] fn try_open_path_best_effort_returns_none_for_missing_path() { diff --git a/e2e/rust/tests/landlock.rs b/e2e/rust/tests/landlock.rs new file mode 100644 index 0000000000..ae1e16601f --- /dev/null +++ b/e2e/rust/tests/landlock.rs @@ -0,0 +1,74 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e")] + +//! End-to-end coverage for Landlock filesystem enforcement. + +use std::io::Write; + +use openshell_e2e::harness::sandbox::SandboxGuard; +use tempfile::NamedTempFile; + +const SUCCESS_MARKER: &str = "landlock-hard-requirement-ok"; + +fn write_hard_requirement_policy() -> Result { + let mut file = + NamedTempFile::new().map_err(|error| format!("create temporary policy: {error}"))?; + let policy = r#"version: 1 + +filesystem_policy: + include_workdir: true + read_only: + - /usr + - /lib + - /etc + - /proc + read_write: + - /sandbox + - /tmp + +landlock: + compatibility: hard_requirement + +process: + run_as_user: sandbox + run_as_group: sandbox + +network_policies: + landlock_regression: + name: landlock_regression + endpoints: + - host: example.com + port: 443 + binaries: + - path: "/**" +"#; + + file.write_all(policy.as_bytes()) + .map_err(|error| format!("write temporary policy: {error}"))?; + file.flush() + .map_err(|error| format!("flush temporary policy: {error}"))?; + Ok(file) +} + +#[tokio::test] +async fn hard_requirement_accepts_enriched_device_path() { + let policy = write_hard_requirement_policy().expect("write hard-requirement policy"); + let policy_path = policy.path().to_string_lossy().into_owned(); + let script = concat!( + "set -eu; ", + "bytes=$(head -c 16 /dev/urandom | wc -c); ", + "test \"$bytes\" -eq 16; ", + "printf landlock-ok > /tmp/landlock-check; ", + "test \"$(cat /tmp/landlock-check)\" = landlock-ok; ", + "echo landlock-hard-requirement-ok", + ); + + let mut sandbox = SandboxGuard::create(&["--policy", &policy_path, "--", "sh", "-lc", script]) + .await + .expect("hard_requirement should accept the enriched /dev/urandom path"); + + assert!(sandbox.create_output.contains(SUCCESS_MARKER)); + sandbox.cleanup().await; +}