From 5cccab689907eb7243a82e2ec25d64daebc0212a Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Fri, 9 Oct 2026 16:39:19 +0200 Subject: [PATCH] mind-body: the binary restricts its umask; the writer refuses unsafe installs Soul s2: under a permissive umask the store was group/world-writable between CultCache's rename and the chmod, and an fd opened there survived it. The mind-control binary now sets umask 077 first (the library never touches the umask). The writer refuses to write through an existing directory, lock or store that is not root's or is group/world-writable, with the typed UnsafeStore error naming path, mode and owner. Tests: a non-root open-for-write probe against the real binary under umask 000; refusals for 0777/0775 directories, 0666/0664 locks and stores, and a non-root-owned directory; accepted root installs. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + crates/mind-body/Cargo.toml | 3 + crates/mind-body/src/cli.rs | 2 +- crates/mind-body/src/control.rs | 59 ++++++++++- crates/mind-body/src/main.rs | 18 +++- crates/mind-body/tests/control.rs | 160 +++++++++++++++++++++++++++++- 6 files changed, 231 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 01c68ba..a46de8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -832,6 +832,7 @@ dependencies = [ "chrono", "cultcache-rs", "eureka-pipeline", + "libc", "rust_decimal", "serde", "tempfile", diff --git a/crates/mind-body/Cargo.toml b/crates/mind-body/Cargo.toml index 7d43e39..eab1e63 100644 --- a/crates/mind-body/Cargo.toml +++ b/crates/mind-body/Cargo.toml @@ -22,6 +22,9 @@ eureka-pipeline = { path = "../eureka-pipeline" } rust_decimal = { version = "1", default-features = false, features = ["std", "serde"] } serde = { version = "1", features = ["derive"] } +[target.'cfg(unix)'.dependencies] +libc = "0.2" + [dev-dependencies] tempfile = "3" diff --git a/crates/mind-body/src/cli.rs b/crates/mind-body/src/cli.rs index e9b543e..e7aa841 100644 --- a/crates/mind-body/src/cli.rs +++ b/crates/mind-body/src/cli.rs @@ -1,5 +1,5 @@ //! The `mind-control` command line: the operator's hand on one instance's -//! control store. It performs no permission check of its own: the file is +//! control store. It checks no caller identity: the file is //! root-owned and the filesystem is the authority. //! //! ```text diff --git a/crates/mind-body/src/control.rs b/crates/mind-body/src/control.rs index d0b0282..f42dfc4 100644 --- a/crates/mind-body/src/control.rs +++ b/crates/mind-body/src/control.rs @@ -14,7 +14,9 @@ //! The reader opens the file read-only through CultCache's read-only snapshot //! door: it takes no lock and creates no sibling file, so the units she runs as //! need no write authority anywhere under `/etc`. Writing is the operator's; -//! the filesystem is the authority and nothing here checks permission. +//! the filesystem is the authority. The writer only refuses to write through an +//! existing directory, lock or store that is not root's or that group or world +//! can write (`UnsafeStore`); it checks no caller identity. use std::path::{Path, PathBuf}; use std::time::Duration; @@ -215,6 +217,45 @@ fn set_mode(path: &Path, mode: u32) -> Result<()> { .with_context(|| format!("failed to set the mode of {}", path.display())) } +/// A control path the writer will not write through: it is not root's, or +/// group or world can write it. Names the path, its mode and its owner. +#[derive(Debug)] +pub struct UnsafeStore { + pub path: PathBuf, + pub mode: u32, + pub uid: u32, +} + +impl std::fmt::Display for UnsafeStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "refusing to write: {} is mode {:04o} owned by uid {}; it must be owned by root (uid 0) and not group- or world-writable", + self.path.display(), + self.mode, + self.uid + ) + } +} + +impl std::error::Error for UnsafeStore {} + +#[cfg(unix)] +fn require_root_only_writable(path: &Path) -> Result<()> { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::metadata(path).with_context(|| format!("failed to inspect {}", path.display()))?; + let (mode, uid) = (meta.mode() & 0o7777, meta.uid()); + if uid != 0 || mode & 0o022 != 0 { + return Err(UnsafeStore { path: path.to_path_buf(), mode, uid }.into()); + } + Ok(()) +} + +#[cfg(not(unix))] +fn require_root_only_writable(_path: &Path) -> Result<()> { + Ok(()) +} + #[cfg(not(unix))] fn set_mode(_path: &Path, _mode: u32) -> Result<()> { Ok(()) @@ -231,16 +272,24 @@ impl ControlWriter { /// Before the first write: the control directory exists at 0755 and the /// lock file at 0644, set explicitly because creation takes the umask. - /// Called after a setter's own refusals, so a refused write creates nothing. + /// A directory, lock or store that already exists must be root's and not + /// group- or world-writable, else the write is refused (`UnsafeStore`): the + /// operator fixes the install. Called after a setter's own refusals, so a + /// refused write creates nothing. fn prepare(&self) -> Result<()> { let directory = self.path.parent().context("control path has no directory")?; + let mut lock_name = self.path.file_name().context("control path has no file name")?.to_os_string(); + lock_name.push(".lock"); + let lock = self.path.with_file_name(lock_name); + for existing in [directory, lock.as_path(), self.path.as_path()] { + if existing.exists() { + require_root_only_writable(existing)?; + } + } if !directory.exists() { std::fs::create_dir_all(directory).with_context(|| format!("failed to create {}", directory.display()))?; set_mode(directory, DIR_MODE)?; } - let mut lock_name = self.path.file_name().context("control path has no file name")?.to_os_string(); - lock_name.push(".lock"); - let lock = self.path.with_file_name(lock_name); if !lock.exists() { std::fs::OpenOptions::new() .create(true) diff --git a/crates/mind-body/src/main.rs b/crates/mind-body/src/main.rs index f8e771f..2b620fe 100644 --- a/crates/mind-body/src/main.rs +++ b/crates/mind-body/src/main.rs @@ -1,10 +1,26 @@ //! `mind-control`: the operator's hand on a Mind's brake and burn-rate dial. //! All behaviour is `mind_body::cli`; this reads the process's environment and -//! clock, the two things the library never reads. +//! clock, the two things the library never reads, and owns the process umask. use std::path::Path; +/// CultCache stages the store file with the process umask and offers no mode, +/// so the umask is what keeps a staged file from being group- or world-writable +/// for the instant between its rename and the writer's chmod. The binary owns +/// its process state; the library never touches the umask. +#[cfg(unix)] +fn restrict_umask() { + // SAFETY: umask only sets this process's file-creation mask. + unsafe { + libc::umask(0o077); + } +} + +#[cfg(not(unix))] +fn restrict_umask() {} + fn main() { + restrict_umask(); let args: Vec = std::env::args().skip(1).collect(); let set_by = mind_body::cli::operator_name(std::env::var("SUDO_USER").ok(), std::env::var("USER").ok()); let result = mind_body::cli::run( diff --git a/crates/mind-body/tests/control.rs b/crates/mind-body/tests/control.rs index 97b8101..9ce3a56 100644 --- a/crates/mind-body/tests/control.rs +++ b/crates/mind-body/tests/control.rs @@ -1,10 +1,13 @@ //! Behavioural tests for the brake, the dial and the one reader. Every test //! works in a temporary directory through the same file door the units use; -//! nothing touches `/etc`. +//! nothing touches `/etc` except the window probe, which writes a uniquely named +//! instance there as root and removes it. //! -//! The permission boundary (no user she runs as can write the file) belongs to -//! the filesystem, not to this crate, and is not tested here: the verify -//! container runs as root, where a permission failure cannot be provoked. +//! The permission boundary is the filesystem's, but the writer refuses to write +//! through a directory, lock or store that is not root's or that group or world +//! can write, and creates its own files at fixed modes. The suite therefore runs +//! as root (the verify container does); the tests that need a second user become +//! uid 65534 with setpriv and skip with a message when they cannot. use std::path::{Path, PathBuf}; use std::time::Duration; @@ -346,7 +349,8 @@ mod modes { #[test] fn mode_child() { let Ok(role) = std::env::var(CHILD) else { return }; - let path = Path::new(&std::env::var(ROOT).unwrap()).join("eureka").join("control.cc"); + let instance = std::env::var("MIND_BODY_INSTANCE").unwrap_or_else(|_| "eureka".into()); + let path = Path::new(&std::env::var(ROOT).unwrap()).join(instance).join("control.cc"); match role.as_str() { "write" => { let mut writer = ControlWriter::open(&path).unwrap(); @@ -356,6 +360,18 @@ mod modes { "brake" => { ControlWriter::open(&path).unwrap().set_brake(true, at(0), "op").unwrap(); } + "attacker" => { + // Spins on an open-for-write of the live store; any success is a hit. + let stop = std::env::var("MIND_BODY_STOP").unwrap(); + let started = std::time::Instant::now(); + while !Path::new(&stop).exists() && started.elapsed() < Duration::from_secs(120) { + for _ in 0..2000 { + if std::fs::OpenOptions::new().write(true).open(&path).is_ok() { + std::process::exit(3); + } + } + } + } "intruder" => { assert!(ControlWriter::open(&path).and_then(|mut writer| writer.set_brake(false, at(1), "intruder")).is_err()); assert!(ControlWriter::open(Path::new(&std::env::var(ROOT).unwrap()).join("ghost").join("control.cc")) @@ -419,3 +435,137 @@ mod modes { assert!(status.success(), "the intruder child asserted a refusal that did not happen"); } } + +#[cfg(unix)] +mod unsafe_installs { + use super::*; + use mind_body::control::UnsafeStore; + use std::os::unix::fs::PermissionsExt; + use std::process::Command; + + fn running_as_root() -> bool { + Command::new("id").arg("-u").output().map(|out| String::from_utf8_lossy(&out.stdout).trim() == "0").unwrap_or(false) + } + + fn chmod(path: &Path, mode: u32) { + std::fs::set_permissions(path, std::fs::Permissions::from_mode(mode)).unwrap(); + } + + /// An instance directory at `dir_mode`, optionally with a lock and a store + /// at the given modes, all root's (the suite runs as root). + fn install(dir_mode: u32, lock_mode: Option, store_mode: Option) -> (tempfile::TempDir, PathBuf) { + let root = tempfile::tempdir().unwrap(); + let instance = root.path().join("eureka"); + std::fs::create_dir(&instance).unwrap(); + for (name, mode) in [("control.cc.lock", lock_mode), ("control.cc", store_mode)] { + if let Some(mode) = mode { + std::fs::write(instance.join(name), b"").unwrap(); + chmod(&instance.join(name), mode); + } + } + chmod(&instance, dir_mode); + (root, instance) + } + + fn refusal(instance: &Path) -> UnsafeStore { + let path = instance.join("control.cc"); + let error = ControlWriter::open(&path).unwrap().set_brake(true, at(0), "op").expect_err("an unsafe install must be refused"); + error.downcast::().expect("the refusal is the typed UnsafeStore error") + } + + #[test] + fn a_group_or_world_writable_directory_lock_or_store_is_refused_and_nothing_is_written() { + if !running_as_root() { + eprintln!("SKIPPED: needs root to own the fixtures"); + return; + } + let cases: [(&str, u32, Option, Option, &str); 5] = [ + ("world-writable directory", 0o777, None, None, ""), + ("group-writable directory", 0o775, None, None, ""), + ("world-writable lock", 0o755, Some(0o666), None, "control.cc.lock"), + ("group-writable store", 0o755, Some(0o644), Some(0o664), "control.cc"), + ("world-writable store", 0o755, None, Some(0o666), "control.cc"), + ]; + for (what, dir_mode, lock_mode, store_mode, culprit) in cases { + let (_root, instance) = install(dir_mode, lock_mode, store_mode); + let before: Vec<_> = std::fs::read_dir(&instance).unwrap().map(|entry| entry.unwrap().file_name()).collect(); + let unsafe_store = refusal(&instance); + let expected = if culprit.is_empty() { instance.clone() } else { instance.join(culprit) }; + assert_eq!(unsafe_store.path, expected, "{what}: names the offending path"); + assert!(unsafe_store.mode & 0o022 != 0, "{what}: names the mode"); + assert!(unsafe_store.to_string().contains(&expected.display().to_string()), "{what}: the message names the path"); + let mut after: Vec<_> = std::fs::read_dir(&instance).unwrap().map(|entry| entry.unwrap().file_name()).collect(); + let mut before = before; + before.sort(); + after.sort(); + assert_eq!(before, after, "{what}: a refused write creates nothing"); + } + } + + #[test] + fn a_directory_not_owned_by_root_is_refused() { + if !running_as_root() { + eprintln!("SKIPPED: needs root to chown"); + return; + } + let (_root, instance) = install(0o755, None, None); + std::os::unix::fs::chown(&instance, Some(65534), Some(65534)).unwrap(); + let unsafe_store = refusal(&instance); + assert_eq!((unsafe_store.path, unsafe_store.uid), (instance, 65534)); + } + + #[test] + fn a_root_owned_install_without_group_or_world_write_is_accepted() { + if !running_as_root() { + eprintln!("SKIPPED: needs root to own the fixtures"); + return; + } + for dir_mode in [0o755, 0o750, 0o700] { + let (_root, instance) = install(dir_mode, Some(0o644), Some(0o644)); + ControlWriter::open(instance.join("control.cc")).unwrap().set_brake(true, at(0), "op").unwrap(); + } + } + + /// The window between CultCache's rename and the writer's chmod, probed by + /// a non-root process spinning open-for-write on the live store while the + /// real binary writes it under a starting umask of 000. The binary sets + /// its own umask, so the staged file is never writable by anyone else. + #[test] + fn no_non_root_open_for_write_succeeds_while_the_binary_writes_under_umask_000() { + if !running_as_root() || Command::new("setpriv").arg("--version").output().is_err() { + eprintln!("SKIPPED: needs root and setpriv; it writes under /etc/gamecult/minds in the verify container"); + return; + } + let instance = format!("window-probe-{}", std::process::id()); + let live = Path::new(mind_body::control::CONTROL_ROOT).join(&instance); + let work = tempfile::tempdir().unwrap(); + chmod(work.path(), 0o755); + let attacker_bin = work.path().join("attacker"); + std::fs::copy(std::env::current_exe().unwrap(), &attacker_bin).unwrap(); + chmod(&attacker_bin, 0o755); + let stop = work.path().join("stop"); + let mut attacker = Command::new("setpriv") + .args(["--reuid=65534", "--regid=65534", "--clear-groups"]) + .arg(&attacker_bin) + .args(["--exact", "modes::mode_child", "--nocapture"]) + .env("MIND_BODY_MODE_CHILD", "attacker") + .env("MIND_BODY_MODE_ROOT", live.parent().unwrap()) + .env("MIND_BODY_STOP", &stop) + .env("MIND_BODY_INSTANCE", &instance) + .spawn() + .unwrap(); + let bin = env!("CARGO_BIN_EXE_mind-control"); + for round in 0..60 { + let verb = if round % 2 == 0 { "release" } else { "hold" }; + let status = Command::new("sh") + .args(["-c", "umask 000; exec \"$@\"", "sh", bin, "--instance", &instance, "brake", verb]) + .status() + .unwrap(); + assert!(status.success()); + } + std::fs::write(&stop, b"").unwrap(); + let attacker_status = attacker.wait().unwrap(); + std::fs::remove_dir_all(&live).unwrap(); + assert_eq!(attacker_status.code(), Some(0), "a non-root process opened the live store for write"); + } +}