From 53f131e34d20f9492cf91f870e96e1fb6570795c Mon Sep 17 00:00:00 2001 From: Alexey <247128645+axkurcom@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:24:29 +0300 Subject: [PATCH] Runtime Paths Checks as opt-in #935 Co-Authored-By: brekotis <93345790+brekotis@users.noreply.github.com> --- src/cli.rs | 25 +++-- src/cli/daemon_commands.rs | 173 +++++++++++++++++++++++++++++++++-- src/daemon/mod.rs | 3 + src/daemon/pid_file.rs | 55 ++++++----- src/daemon/pid_file/tests.rs | 155 ++++++++++++++++++++++++++----- src/logging.rs | 18 ++-- src/logging/file.rs | 14 ++- src/logging/file/tests.rs | 125 +++++++++++++++++++++++-- src/logging/tests.rs | 1 + src/maestro/bootstrap.rs | 14 +++ src/maestro/helpers.rs | 20 ++++ src/maestro/mod.rs | 28 ++++-- src/maestro/orchestrator.rs | 7 +- src/util/secure_fs/mod.rs | 5 +- src/util/secure_fs/path.rs | 41 ++++++++- src/util/secure_fs/tests.rs | 45 +++++++++ 16 files changed, 635 insertions(+), 94 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index d9a4e15..fc4a662 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -2,9 +2,9 @@ //! //! Subcommands: //! - `start [OPTIONS] [config.toml]` - Start the daemon -//! - `stop [--pid-file PATH]` - Stop a running daemon -//! - `reload [--pid-file PATH]` - Reload configuration (SIGHUP) -//! - `status [--pid-file PATH]` - Check daemon status +//! - `stop [--pid-file PATH] [--strict-runtime-paths]` - Stop a running daemon +//! - `reload [--pid-file PATH] [--strict-runtime-paths]` - Reload configuration (SIGHUP) +//! - `status [--pid-file PATH] [--strict-runtime-paths]` - Check daemon status //! - `run [OPTIONS] [config.toml]` - Run in foreground (default behavior) //! - `healthcheck [OPTIONS] [config.toml]` - Run control-plane health probe @@ -137,6 +137,10 @@ pub fn parse_command(args: &[String]) -> ParsedCommand { while i < args.len() { match args[i].as_str() { "start" | "stop" | "reload" | "status" | "run" | "healthcheck" => {} + #[cfg(unix)] + "--strict-runtime-paths" => { + cmd.daemon_opts.strict_runtime_paths = true; + } "--mode" => { i += 1; if i < args.len() { @@ -199,9 +203,18 @@ pub fn parse_command(args: &[String]) -> ParsedCommand { #[cfg(unix)] pub fn execute_subcommand(cmd: &ParsedCommand) -> Option { match cmd.subcommand { - Subcommand::Stop => Some(daemon_commands::stop(&cmd.pid_file)), - Subcommand::Reload => Some(daemon_commands::reload(&cmd.pid_file)), - Subcommand::Status => Some(daemon_commands::status(&cmd.pid_file)), + Subcommand::Stop => Some(daemon_commands::stop( + &cmd.pid_file, + cmd.daemon_opts.strict_runtime_paths, + )), + Subcommand::Reload => Some(daemon_commands::reload( + &cmd.pid_file, + cmd.daemon_opts.strict_runtime_paths, + )), + Subcommand::Status => Some(daemon_commands::status( + &cmd.pid_file, + cmd.daemon_opts.strict_runtime_paths, + )), Subcommand::Healthcheck => { if let Some(invalid_mode) = cmd.healthcheck_mode_invalid.as_ref() { if invalid_mode.is_empty() { diff --git a/src/cli/daemon_commands.rs b/src/cli/daemon_commands.rs index ee6fcca..eb95023 100644 --- a/src/cli/daemon_commands.rs +++ b/src/cli/daemon_commands.rs @@ -15,6 +15,9 @@ pub fn parse_daemon_args(args: &[String]) -> DaemonOptions { "--foreground" | "-f" => { opts.foreground = true; } + "--strict-runtime-paths" => { + opts.strict_runtime_paths = true; + } "--pid-file" => { i += 1; if i < args.len() { @@ -60,19 +63,21 @@ pub fn parse_daemon_args(args: &[String]) -> DaemonOptions { } /// Sends SIGTERM and waits briefly for graceful PID-file cleanup. -pub(super) fn stop(pid_file: &Path) -> i32 { +pub(super) fn stop(pid_file: &Path, strict_runtime_paths: bool) -> i32 { use nix::sys::signal::Signal; println!("Stopping telemt daemon..."); - match daemon::signal_pid_file(pid_file, Signal::SIGTERM) { + match daemon::signal_pid_file(pid_file, Signal::SIGTERM, strict_runtime_paths) { Ok(()) => { println!("Stop signal sent successfully"); // Wait for process to exit for up to ten seconds. for _ in 0..20 { std::thread::sleep(std::time::Duration::from_millis(500)); - if let daemon::DaemonStatus::NotRunning = daemon::check_status(pid_file) { + if let daemon::DaemonStatus::NotRunning = + daemon::check_status(pid_file, strict_runtime_paths) + { println!("Daemon stopped"); return 0; } @@ -88,12 +93,12 @@ pub(super) fn stop(pid_file: &Path) -> i32 { } /// Sends SIGHUP to trigger configuration reload. -pub(super) fn reload(pid_file: &Path) -> i32 { +pub(super) fn reload(pid_file: &Path, strict_runtime_paths: bool) -> i32 { use nix::sys::signal::Signal; println!("Reloading telemt configuration..."); - match daemon::signal_pid_file(pid_file, Signal::SIGHUP) { + match daemon::signal_pid_file(pid_file, Signal::SIGHUP, strict_runtime_paths) { Ok(()) => { println!("Reload signal sent successfully"); 0 @@ -106,8 +111,8 @@ pub(super) fn reload(pid_file: &Path) -> i32 { } /// Reports daemon status without mutating PID lifecycle state. -pub(super) fn status(pid_file: &Path) -> i32 { - match daemon::check_status(pid_file) { +pub(super) fn status(pid_file: &Path, strict_runtime_paths: bool) -> i32 { + match daemon::check_status(pid_file, strict_runtime_paths) { daemon::DaemonStatus::Running(pid) => { println!("telemt is running (pid {})", pid); 0 @@ -126,16 +131,168 @@ pub(super) fn status(pid_file: &Path) -> i32 { #[cfg(test)] mod tests { use std::fs; + use std::os::unix::fs::{PermissionsExt, symlink}; + use std::process::{Child, Command, Stdio}; + use std::time::{Duration, Instant}; use super::*; + const CONTROL_PID: &str = "TELEMT_RUNTIME_PATH_TEST_PID"; + const CONTROL_READY: &str = "TELEMT_RUNTIME_PATH_TEST_READY"; + const CONTROL_RELOADED: &str = "TELEMT_RUNTIME_PATH_TEST_RELOADED"; + + struct ControlChild(Child); + + impl Drop for ControlChild { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + fn wait_for_file(path: &Path) -> bool { + let deadline = Instant::now() + Duration::from_secs(5); + while Instant::now() < deadline { + if path.exists() { + return true; + } + std::thread::sleep(Duration::from_millis(10)); + } + false + } + + #[test] + fn runtime_path_policy_defaults_to_compatibility_for_all_commands() { + for command in ["run", "start", "stop", "reload", "status"] { + let args = vec![command.to_string(), "config.toml".to_string()]; + assert!( + !crate::cli::parse_command(&args) + .daemon_opts + .strict_runtime_paths + ); + assert!(!parse_daemon_args(&args).strict_runtime_paths); + + for position in [1, args.len()] { + let mut strict_args = args.clone(); + strict_args.insert(position, "--strict-runtime-paths".to_string()); + let parsed = crate::cli::parse_command(&strict_args); + assert!(parsed.daemon_opts.strict_runtime_paths); + assert!(parse_daemon_args(&strict_args).strict_runtime_paths); + assert_eq!(parsed.config_path, "config.toml"); + } + } + let args = vec![ + "--strict-runtime-paths".to_string(), + "config.toml".to_string(), + ]; + let parsed = crate::cli::parse_command(&args); + assert_eq!(parsed.subcommand, crate::cli::Subcommand::Run); + assert!(parsed.daemon_opts.strict_runtime_paths); + assert_eq!(parsed.config_path, "config.toml"); + } + + #[tokio::test(flavor = "current_thread")] + async fn daemon_control_subprocess() { + let Some(pid_path) = std::env::var_os(CONTROL_PID) else { + return; + }; + let ready = PathBuf::from(std::env::var_os(CONTROL_READY).unwrap()); + let reloaded = PathBuf::from(std::env::var_os(CONTROL_RELOADED).unwrap()); + let mut terminate = + tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()).unwrap(); + let mut reload = + tokio::signal::unix::signal(tokio::signal::unix::SignalKind::hangup()).unwrap(); + let mut owner = daemon::PidFile::new(PathBuf::from(pid_path), false); + owner.acquire().unwrap(); + fs::write(&ready, b"ready").unwrap(); + let deadline = tokio::time::sleep(Duration::from_secs(15)); + tokio::pin!(deadline); + loop { + tokio::select! { + _ = terminate.recv() => break, + _ = reload.recv() => fs::write(&reloaded, b"reloaded").unwrap(), + _ = &mut deadline => panic!("daemon control subprocess timed out"), + } + } + owner.release().unwrap(); + } + + #[test] + fn control_commands_follow_runtime_path_policy() { + for (mode, linked_parent, trusted_parent) in [ + (0o777, false, false), + (0o777, true, false), + (0o755, false, true), + ] { + let root = tempfile::tempdir().unwrap(); + let real = root.path().join("run"); + let linked = root.path().join("linked"); + fs::create_dir(&real).unwrap(); + fs::set_permissions(&real, fs::Permissions::from_mode(mode)).unwrap(); + symlink(&real, &linked).unwrap(); + let pid_path = if linked_parent { &linked } else { &real }.join("telemt.pid"); + let ready = root.path().join("ready"); + let reloaded = root.path().join("reloaded"); + let mut child = ControlChild( + Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "cli::daemon_commands::tests::daemon_control_subprocess", + "--nocapture", + ]) + .env(CONTROL_PID, &pid_path) + .env(CONTROL_READY, &ready) + .env(CONTROL_RELOADED, &reloaded) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(), + ); + assert!( + wait_for_file(&ready), + "daemon control subprocess did not become ready" + ); + let command = |name: &str, strict: bool| { + let mut args = vec![ + name.to_string(), + "--pid-file".to_string(), + pid_path.to_str().unwrap().to_string(), + ]; + if strict { + args.push("--strict-runtime-paths".to_string()); + } + crate::cli::execute_subcommand(&crate::cli::parse_command(&args)) + }; + + assert_eq!(command("status", false), Some(0)); + if trusted_parent { + assert_eq!(command("status", true), Some(0)); + assert_eq!(command("reload", true), Some(0)); + assert!(wait_for_file(&reloaded)); + assert_eq!(command("stop", true), Some(0)); + } else { + assert_eq!(command("status", true), Some(1)); + assert_eq!(command("reload", true), Some(1)); + assert!(!reloaded.exists()); + assert_eq!(command("stop", true), Some(1)); + assert_eq!(command("status", false), Some(0)); + assert_eq!(command("reload", false), Some(0)); + assert!(wait_for_file(&reloaded)); + assert_eq!(command("stop", false), Some(0)); + } + assert!(child.0.wait().unwrap().success()); + assert!(!pid_path.exists()); + assert_eq!(command("status", false), Some(1)); + } + } + #[test] fn status_does_not_remove_stale_pid_file() { let directory = tempfile::tempdir().unwrap(); let pid_file = directory.path().join("telemt.pid"); fs::write(&pid_file, b"2000000000\n").unwrap(); - assert_eq!(status(&pid_file), 1); + assert_eq!(status(&pid_file, false), 1); assert!(pid_file.exists()); } } diff --git a/src/daemon/mod.rs b/src/daemon/mod.rs index 15d274c..a3a8b80 100644 --- a/src/daemon/mod.rs +++ b/src/daemon/mod.rs @@ -29,6 +29,8 @@ pub struct DaemonOptions { pub daemonize: bool, /// Path to PID file. pub pid_file: Option, + /// Require trusted, symlink-free PID and log parents. Disabled by default for compatibility. + pub strict_runtime_paths: bool, /// User to run as after binding sockets. pub user: Option, /// Group to run as after binding sockets. @@ -332,6 +334,7 @@ mod tests { fn test_daemon_options_default() { let opts = DaemonOptions::default(); assert!(!opts.daemonize); + assert!(!opts.strict_runtime_paths); assert!(!opts.should_daemonize()); assert_eq!(opts.pid_file_path(), Path::new(DEFAULT_PID_FILE)); } diff --git a/src/daemon/pid_file.rs b/src/daemon/pid_file.rs index 0eb70f0..c9fd035 100644 --- a/src/daemon/pid_file.rs +++ b/src/daemon/pid_file.rs @@ -17,6 +17,7 @@ use crate::util::secure_fs::AnchoredPath; /// PID file manager backed by a persistent sibling lock file. pub struct PidFile { path: PathBuf, + strict_runtime_paths: bool, lock_path: PathBuf, pid_file: Option, pid_identity: Option, @@ -40,12 +41,13 @@ impl FileIdentity { } impl PidFile { - /// Creates a new PID file manager for the given path. - pub fn new>(path: P) -> Self { + /// Creates a PID manager with explicit parent-path policy; `false` allows legacy parents. + pub fn new>(path: P, strict_runtime_paths: bool) -> Self { let path = normalize_pid_path(path.as_ref()); let lock_path = sibling_lock_path(&path); Self { path, + strict_runtime_paths, lock_path, pid_file: None, pid_identity: None, @@ -56,7 +58,7 @@ impl PidFile { /// Checks whether the PID file names a running process without modifying either file. pub fn check_running(&self) -> Result, DaemonError> { - let Some(pid) = read_pid_file_if_exists(&self.path)? else { + let Some(pid) = read_pid_file_if_exists(&self.path, self.strict_runtime_paths)? else { return Ok(None); }; Ok(is_process_running(pid).then_some(pid)) @@ -67,9 +69,10 @@ impl PidFile { /// Fails if another owner holds the lock or the existing PID names a running process. pub fn acquire(&mut self) -> Result<(), DaemonError> { let anchor = - AnchoredPath::open_trusted_parent_or_create(&self.path, 0o755).map_err(|error| { + AnchoredPath::open_runtime_parent(&self.path, Some(0o755), self.strict_runtime_paths) + .map_err(|error| { DaemonError::PidFile(format!( - "cannot open trusted parent for {}: {}", + "cannot open PID parent for {}: {}", self.path.display(), error )) @@ -245,13 +248,16 @@ fn open_file_at(anchor: &AnchoredPath, name: &OsStr, flags: OFlag, mode: u32) -> Ok(File::from(descriptor)) } -fn read_pid_file_if_exists(path: &Path) -> Result, DaemonError> { - let anchor = match AnchoredPath::open_trusted_parent(path) { +fn read_pid_file_if_exists( + path: &Path, + strict_runtime_paths: bool, +) -> Result, DaemonError> { + let anchor = match AnchoredPath::open_runtime_parent(path, None, strict_runtime_paths) { Ok(anchor) => anchor, Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None), Err(error) => { return Err(DaemonError::PidFile(format!( - "cannot open trusted parent for {}: {}", + "cannot open PID parent for {}: {}", path.display(), error ))); @@ -352,11 +358,14 @@ fn validate_regular_single_link(file: &File, path: &Path) -> Result>(path: P) -> Result { +pub fn read_pid_file>( + path: P, + strict_runtime_paths: bool, +) -> Result { let path = normalize_pid_path(path.as_ref()); - read_pid_file_if_exists(&path)?.ok_or_else(|| { + read_pid_file_if_exists(&path, strict_runtime_paths)?.ok_or_else(|| { DaemonError::PidFile(format!( "cannot read {}: file does not exist", path.display() @@ -364,17 +373,18 @@ pub fn read_pid_file>(path: P) -> Result { }) } -/// Sends a signal to the process specified in a PID file. +/// Signals a lock-owning process using the same parent-path policy for PID and lock files. #[allow(dead_code)] pub fn signal_pid_file>( path: P, signal: nix::sys::signal::Signal, + strict_runtime_paths: bool, ) -> Result<(), DaemonError> { let path = normalize_pid_path(path.as_ref()); - let pid = read_pid_file(&path)?; + let pid = read_pid_file(&path, strict_runtime_paths)?; #[cfg(target_os = "linux")] let pidfd = open_pidfd(pid)?; - if !daemon_lock_is_held(&path)? { + if !daemon_lock_is_held(&path, strict_runtime_paths)? { return Err(DaemonError::PidFile(format!( "refusing to signal unlocked or stale PID file {}", path.display() @@ -399,12 +409,15 @@ pub enum DaemonStatus { NotRunning, } -/// Checks daemon status without modifying the PID or lock file. +/// Checks daemon status read-only, applying the selected policy to both parent lookups. #[allow(dead_code)] -pub fn check_status>(path: P) -> DaemonStatus { +pub fn check_status>(path: P, strict_runtime_paths: bool) -> DaemonStatus { let path = normalize_pid_path(path.as_ref()); - match read_pid_file_if_exists(&path) { - Ok(Some(pid)) if daemon_lock_is_held(&path).unwrap_or(false) && is_process_running(pid) => { + match read_pid_file_if_exists(&path, strict_runtime_paths) { + Ok(Some(pid)) + if daemon_lock_is_held(&path, strict_runtime_paths).unwrap_or(false) + && is_process_running(pid) => + { DaemonStatus::Running(pid) } Ok(Some(pid)) => DaemonStatus::Stale(pid), @@ -412,14 +425,14 @@ pub fn check_status>(path: P) -> DaemonStatus { } } -fn daemon_lock_is_held(path: &Path) -> Result { +fn daemon_lock_is_held(path: &Path, strict_runtime_paths: bool) -> Result { let lock_path = sibling_lock_path(path); - let anchor = match AnchoredPath::open_trusted_parent(path) { + let anchor = match AnchoredPath::open_runtime_parent(path, None, strict_runtime_paths) { Ok(anchor) => anchor, Err(error) if error.kind() == ErrorKind::NotFound => return Ok(false), Err(error) => { return Err(DaemonError::PidFile(format!( - "cannot open trusted parent for {}: {}", + "cannot open PID parent for {}: {}", path.display(), error ))); diff --git a/src/daemon/pid_file/tests.rs b/src/daemon/pid_file/tests.rs index d4cbed1..aff20b2 100644 --- a/src/daemon/pid_file/tests.rs +++ b/src/daemon/pid_file/tests.rs @@ -38,6 +38,107 @@ fn pid_file_remains_send_and_sync() { assert_send_sync::(); } +#[test] +fn compatibility_pid_lifecycle_accepts_writable_parent_directories() { + use std::os::unix::fs::PermissionsExt; + + for mode in [0o770, 0o777, 0o1777] { + let directory = tempfile::tempdir().unwrap(); + fs::set_permissions(directory.path(), fs::Permissions::from_mode(mode)).unwrap(); + let pid_path = directory.path().join("telemt.pid"); + let mut strict_owner = PidFile::new(&pid_path, true); + assert!(strict_owner.acquire().is_err()); + assert!(!pid_path.exists()); + assert!(!sibling_lock_path(&pid_path).exists()); + let mut owner = PidFile::new(&pid_path, false); + owner.acquire().unwrap(); + + assert_eq!( + read_pid_file(&pid_path, false).unwrap(), + std::process::id() as i32 + ); + assert_eq!( + owner.check_running().unwrap(), + Some(std::process::id() as i32) + ); + assert_eq!( + check_status(&pid_path, false), + DaemonStatus::Running(std::process::id() as i32) + ); + assert!(read_pid_file(&pid_path, true).is_err()); + assert!(strict_owner.check_running().is_err()); + assert_eq!(check_status(&pid_path, true), DaemonStatus::NotRunning); + + owner.release().unwrap(); + assert!(!pid_path.exists()); + assert!(sibling_lock_path(&pid_path).exists()); + assert_eq!(check_status(&pid_path, false), DaemonStatus::NotRunning); + } +} + +#[test] +fn compatibility_pid_lifecycle_follows_symlinked_parents() { + let directory = tempfile::tempdir().unwrap(); + let real = directory.path().join("tmp"); + let linked = directory.path().join("var"); + fs::create_dir(&real).unwrap(); + symlink("tmp", &linked).unwrap(); + let pid_path = linked.join("run/telemt.pid"); + let mut strict_owner = PidFile::new(&pid_path, true); + assert!(strict_owner.acquire().is_err()); + assert!(!real.join("run").exists()); + let mut owner = PidFile::new(&pid_path, false); + owner.acquire().unwrap(); + + assert_eq!( + read_pid_file(&pid_path, false).unwrap(), + std::process::id() as i32 + ); + assert_eq!( + check_status(&pid_path, false), + DaemonStatus::Running(std::process::id() as i32) + ); + assert!(real.join("run/telemt.pid.lock").exists()); + assert!(read_pid_file(&pid_path, true).is_err()); + assert_eq!(check_status(&pid_path, true), DaemonStatus::NotRunning); + + owner.release().unwrap(); + assert!(!real.join("run/telemt.pid").exists()); +} + +#[test] +fn private_pid_parent_supports_both_policies() { + for strict_runtime_paths in [false, true] { + let directory = tempfile::tempdir().unwrap(); + let pid_path = directory.path().join("nested/run/telemt.pid"); + let mut owner = PidFile::new(&pid_path, strict_runtime_paths); + owner.acquire().unwrap(); + + for read_strict in [false, true] { + assert_eq!( + read_pid_file(&pid_path, read_strict).unwrap(), + std::process::id() as i32 + ); + assert_eq!( + check_status(&pid_path, read_strict), + DaemonStatus::Running(std::process::id() as i32) + ); + } + assert_eq!( + fs::metadata(pid_path.parent().unwrap()).unwrap().mode() & 0o777, + 0o755 + ); + let mut contender = PidFile::new(&pid_path, strict_runtime_paths); + assert!(matches!( + contender.acquire(), + Err(DaemonError::AlreadyRunning(_)) + )); + owner.release().unwrap(); + contender.acquire().unwrap(); + contender.release().unwrap(); + } +} + #[test] fn system_var_run_alias_keeps_the_default_pid_path_usable() { let Ok(metadata) = fs::symlink_metadata("/var/run") else { @@ -52,7 +153,7 @@ fn system_var_run_alias_keeps_the_default_pid_path_usable() { return; } - let pid_file = PidFile::new("/var/run/telemt.pid"); + let pid_file = PidFile::new("/var/run/telemt.pid", false); assert_eq!(pid_file.path(), Path::new("/run/telemt.pid")); } @@ -64,7 +165,7 @@ fn lock_holder_subprocess() { }; let ready_path = PathBuf::from(std::env::var_os(HELPER_READY_PATH).unwrap()); let stop_path = PathBuf::from(std::env::var_os(HELPER_STOP_PATH).unwrap()); - let mut pid_file = PidFile::new(PathBuf::from(pid_path)); + let mut pid_file = PidFile::new(PathBuf::from(pid_path), false); pid_file.acquire().unwrap(); fs::write(&ready_path, b"ready").unwrap(); assert!(wait_for_path(&stop_path, Duration::from_secs(10))); @@ -100,7 +201,7 @@ fn persistent_sibling_lock_serializes_processes_after_pid_unlink() { let lock_inode = fs::metadata(&lock_path).unwrap().ino(); fs::remove_file(&pid_path).unwrap(); - let mut contender = PidFile::new(&pid_path); + let mut contender = PidFile::new(&pid_path, false); assert!(contender.acquire().is_err()); fs::write(&stop_path, b"stop").unwrap(); @@ -123,10 +224,13 @@ fn stale_pid_checks_are_read_only() { let directory = tempfile::tempdir().unwrap(); let pid_path = directory.path().join("telemt.pid"); fs::write(&pid_path, b"2000000000\n").unwrap(); - let pid_file = PidFile::new(&pid_path); + let pid_file = PidFile::new(&pid_path, false); assert_eq!(pid_file.check_running().unwrap(), None); - assert_eq!(check_status(&pid_path), DaemonStatus::Stale(2_000_000_000)); + assert_eq!( + check_status(&pid_path, false), + DaemonStatus::Stale(2_000_000_000) + ); assert!(pid_path.exists()); } @@ -136,15 +240,15 @@ fn status_requires_live_lock_ownership() { let pid_path = directory.path().join("telemt.pid"); fs::write(&pid_path, format!("{}\n", std::process::id())).unwrap(); assert_eq!( - check_status(&pid_path), + check_status(&pid_path, false), DaemonStatus::Stale(std::process::id() as i32) ); fs::remove_file(&pid_path).unwrap(); - let mut owner = PidFile::new(&pid_path); + let mut owner = PidFile::new(&pid_path, false); owner.acquire().unwrap(); assert_eq!( - check_status(&pid_path), + check_status(&pid_path, false), DaemonStatus::Running(std::process::id() as i32) ); owner.release().unwrap(); @@ -155,7 +259,7 @@ fn unowned_release_does_not_remove_pid_file() { let directory = tempfile::tempdir().unwrap(); let pid_path = directory.path().join("telemt.pid"); fs::write(&pid_path, b"2000000000\n").unwrap(); - let mut pid_file = PidFile::new(&pid_path); + let mut pid_file = PidFile::new(&pid_path, false); pid_file.release().unwrap(); @@ -169,10 +273,11 @@ fn acquire_rejects_pid_symlink_without_truncating_target() { let target_path = directory.path().join("target"); fs::write(&target_path, b"preserve\n").unwrap(); symlink(&target_path, &pid_path).unwrap(); - let mut pid_file = PidFile::new(&pid_path); - - assert!(pid_file.acquire().is_err()); - assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n"); + for strict_runtime_paths in [false, true] { + let mut pid_file = PidFile::new(&pid_path, strict_runtime_paths); + assert!(pid_file.acquire().is_err()); + assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n"); + } } #[test] @@ -182,10 +287,11 @@ fn acquire_rejects_pid_hard_link_without_truncating_target() { let target_path = directory.path().join("target"); fs::write(&target_path, b"preserve\n").unwrap(); fs::hard_link(&target_path, &pid_path).unwrap(); - let mut pid_file = PidFile::new(&pid_path); - - assert!(pid_file.acquire().is_err()); - assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n"); + for strict_runtime_paths in [false, true] { + let mut pid_file = PidFile::new(&pid_path, strict_runtime_paths); + assert!(pid_file.acquire().is_err()); + assert_eq!(fs::read(&target_path).unwrap(), b"preserve\n"); + } } #[test] @@ -196,7 +302,7 @@ fn acquire_rejects_symlinked_parent_without_publishing_outside() { fs::create_dir(&real_parent).unwrap(); symlink(&real_parent, &linked_parent).unwrap(); let pid_path = linked_parent.join("telemt.pid"); - let mut pid_file = PidFile::new(&pid_path); + let mut pid_file = PidFile::new(&pid_path, true); assert!(pid_file.acquire().is_err()); assert!(!real_parent.join("telemt.pid").exists()); @@ -210,7 +316,7 @@ fn release_remains_anchored_after_parent_path_replacement() { let moved_parent = directory.path().join("moved"); fs::create_dir(&active_parent).unwrap(); let pid_path = active_parent.join("telemt.pid"); - let mut pid_file = PidFile::new(&pid_path); + let mut pid_file = PidFile::new(&pid_path, false); pid_file.acquire().unwrap(); fs::rename(&active_parent, &moved_parent).unwrap(); @@ -231,7 +337,7 @@ fn release_does_not_remove_replacement_path() { let directory = tempfile::tempdir().unwrap(); let pid_path = directory.path().join("telemt.pid"); let owned_path = directory.path().join("owned.pid"); - let mut pid_file = PidFile::new(&pid_path); + let mut pid_file = PidFile::new(&pid_path, false); pid_file.acquire().unwrap(); fs::rename(&pid_path, &owned_path).unwrap(); fs::write(&pid_path, b"replacement\n").unwrap(); @@ -253,7 +359,7 @@ fn pid_parser_rejects_process_group_values() { for value in ["-1\n", "0\n", "1\n"] { fs::write(&pid_path, value).unwrap(); - assert!(read_pid_file(&pid_path).is_err()); + assert!(read_pid_file(&pid_path, false).is_err()); } } @@ -262,7 +368,7 @@ fn pid_file_release_keeps_lock_inode() { let directory = tempfile::tempdir().unwrap(); let pid_path = directory.path().join("telemt.pid"); let lock_path = sibling_lock_path(&pid_path); - let mut pid_file = PidFile::new(&pid_path); + let mut pid_file = PidFile::new(&pid_path, false); pid_file.acquire().unwrap(); assert!( @@ -271,7 +377,10 @@ fn pid_file_release_keeps_lock_inode() { .into_iter() .all(|file| file.is_some()) ); - assert_eq!(read_pid_file(&pid_path).unwrap(), std::process::id() as i32); + assert_eq!( + read_pid_file(&pid_path, false).unwrap(), + std::process::id() as i32 + ); let lock_inode = fs::metadata(&lock_path).unwrap().ino(); pid_file.release().unwrap(); diff --git a/src/logging.rs b/src/logging.rs index 9567fa1..cfd32a3 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -77,6 +77,8 @@ pub struct LoggingOptions { pub destination: LogDestination, /// Disable ANSI colors. pub disable_colors: bool, + /// Require trusted, symlink-free log parents on Unix. Disabled by default for compatibility. + pub strict_runtime_paths: bool, } /// Guard that must be held to keep file logging active. @@ -125,7 +127,6 @@ pub fn init_logging( #[cfg(unix)] LogDestination::Syslog => { - // Use a custom fmt layer that writes to syslog let fmt_layer = fmt::Layer::default() .with_ansi(false) .with_target(false) @@ -143,7 +144,8 @@ pub fn init_logging( LogDestination::File { options } => { let file_appender = - file::BoundedFileAppender::new(options.clone()).expect("Failed to open log file"); + file::BoundedFileAppender::new(options.clone(), opts.strict_runtime_paths) + .expect("Failed to open log file"); let (non_blocking, guard) = tracing_appender::non_blocking(file_appender); let fmt_layer = fmt::Layer::default() @@ -175,14 +177,10 @@ struct SyslogWriter { #[cfg(unix)] impl SyslogMakeWriter { fn new() -> Self { - // Open syslog connection on first use static INIT: std::sync::Once = std::sync::Once::new(); - INIT.call_once(|| { - unsafe { - // Open syslog with ident "telemt", LOG_PID, LOG_DAEMON facility - let ident = b"telemt\0".as_ptr() as *const libc::c_char; - libc::openlog(ident, libc::LOG_PID | libc::LOG_NDELAY, libc::LOG_DAEMON); - } + INIT.call_once(|| unsafe { + let ident = b"telemt\0".as_ptr() as *const libc::c_char; + libc::openlog(ident, libc::LOG_PID | libc::LOG_NDELAY, libc::LOG_DAEMON); }); Self } @@ -202,7 +200,6 @@ fn syslog_priority_for_level(level: &tracing::Level) -> libc::c_int { #[cfg(unix)] impl std::io::Write for SyslogWriter { fn write(&mut self, buf: &[u8]) -> std::io::Result { - // Convert to C string, stripping newlines let msg = String::from_utf8_lossy(buf); let msg = msg.trim_end(); @@ -210,7 +207,6 @@ impl std::io::Write for SyslogWriter { return Ok(buf.len()); } - // Write to syslog let c_msg = std::ffi::CString::new(msg.as_bytes()) .unwrap_or_else(|_| std::ffi::CString::new("(invalid utf8)").unwrap()); diff --git a/src/logging/file.rs b/src/logging/file.rs index f14c24f..8b1de52 100644 --- a/src/logging/file.rs +++ b/src/logging/file.rs @@ -44,13 +44,15 @@ pub(crate) struct BoundedFileAppender { } impl BoundedFileAppender { - pub(crate) fn new(options: FileLogOptions) -> io::Result { - Self::with_now(options, Box::new(Utc::now)) + /// Opens the appender using the process-level Unix parent-path policy. + pub(crate) fn new(options: FileLogOptions, strict_runtime_paths: bool) -> io::Result { + Self::with_now(options, Box::new(Utc::now), strict_runtime_paths) } fn with_now( options: FileLogOptions, now: Box DateTime + Send + Sync>, + strict_runtime_paths: bool, ) -> io::Result { let path = Path::new(&options.path); let dir = path @@ -67,7 +69,13 @@ impl BoundedFileAppender { let start = now(); let current_path = active_path_for(&dir, &base_name, options.rotation, &start); #[cfg(unix)] - let dir_fd = crate::util::secure_fs::open_trusted_dir_nofollow_or_create(&dir, 0o750)?; + let dir_fd = if strict_runtime_paths { + crate::util::secure_fs::open_trusted_dir_nofollow_or_create(&dir, 0o750)? + } else { + crate::util::secure_fs::open_compatible_dir(&dir, Some(0o750))? + }; + #[cfg(not(unix))] + let _ = strict_runtime_paths; #[cfg(unix)] let (file, current_size) = open_append_file(&dir_fd, ¤t_path)?; #[cfg(not(unix))] diff --git a/src/logging/file/tests.rs b/src/logging/file/tests.rs index bd2656c..28d2485 100644 --- a/src/logging/file/tests.rs +++ b/src/logging/file/tests.rs @@ -34,6 +34,115 @@ fn matching_logs(dir: &Path) -> Vec { files } +#[cfg(unix)] +#[test] +fn compatibility_appender_accepts_writable_log_directories() { + use std::os::unix::fs::PermissionsExt; + + for mode in [0o770, 0o777, 0o1777] { + let dir = tempdir().unwrap(); + fs::set_permissions(dir.path(), fs::Permissions::from_mode(mode)).unwrap(); + let path = dir.path().join("telemt.log"); + assert!( + BoundedFileAppender::with_now(options(path.clone()), Box::new(fixed_now), true) + .is_err() + ); + assert!(!path.exists()); + let mut appender = + BoundedFileAppender::with_now(options(path.clone()), Box::new(fixed_now), false) + .unwrap(); + appender.write_all(b"compatibility\n").unwrap(); + appender.flush().unwrap(); + + assert_eq!(fs::read(&path).unwrap(), b"compatibility\n"); + assert_eq!( + fs::metadata(&path).unwrap().permissions().mode() & 0o777, + 0o640 + ); + } +} + +#[cfg(unix)] +#[test] +fn compatibility_appender_follows_symlinked_parents() { + use std::os::unix::fs::symlink; + + let dir = tempdir().unwrap(); + let real = dir.path().join("real"); + let linked = dir.path().join("linked"); + fs::create_dir(&real).unwrap(); + symlink(&real, &linked).unwrap(); + let path = linked.join("nested/telemt.log"); + assert!( + BoundedFileAppender::with_now(options(path.clone()), Box::new(fixed_now), true).is_err() + ); + assert!(!real.join("nested").exists()); + let mut appender = + BoundedFileAppender::with_now(options(path), Box::new(fixed_now), false).unwrap(); + appender.write_all(b"compatibility\n").unwrap(); + appender.flush().unwrap(); + + assert_eq!( + fs::read(real.join("nested/telemt.log")).unwrap(), + b"compatibility\n" + ); +} + +#[cfg(unix)] +#[test] +fn appender_rejects_final_symlinks_and_hard_links_in_both_modes() { + use std::os::unix::fs::symlink; + + for strict_runtime_paths in [false, true] { + for hard_link in [false, true] { + let dir = tempdir().unwrap(); + let target = dir.path().join("sentinel"); + let path = dir.path().join("telemt.log"); + fs::write(&target, b"preserve\n").unwrap(); + if hard_link { + fs::hard_link(&target, &path).unwrap(); + } else { + symlink(&target, &path).unwrap(); + } + + assert!( + BoundedFileAppender::with_now( + options(path), + Box::new(fixed_now), + strict_runtime_paths + ) + .is_err() + ); + assert_eq!(fs::read(&target).unwrap(), b"preserve\n"); + } + } +} + +#[cfg(unix)] +#[test] +fn time_rotation_and_retention_work_through_compatible_parent_alias() { + use std::os::unix::fs::{PermissionsExt, symlink}; + + let root = tempdir().unwrap(); + let real = root.path().join("logs"); + let linked = root.path().join("linked"); + fs::create_dir(&real).unwrap(); + fs::set_permissions(&real, fs::Permissions::from_mode(0o777)).unwrap(); + symlink(&real, &linked).unwrap(); + let mut options = options(linked.join("telemt.log")); + options.rotation = LogRotation::Daily; + options.max_files = 1; + let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now), false).unwrap(); + appender.write_all(b"first\n").unwrap(); + appender.now = Box::new(|| fixed_now() + ChronoDuration::days(1)); + appender.write_all(b"second\n").unwrap(); + appender.flush().unwrap(); + + let remaining = matching_logs(&real); + assert_eq!(remaining.len(), 1); + assert_eq!(fs::read(&remaining[0]).unwrap(), b"second\n"); +} + #[test] fn size_rotation_keeps_latest_write_in_active_file() { let dir = tempdir().unwrap(); @@ -41,7 +150,7 @@ fn size_rotation_keeps_latest_write_in_active_file() { let mut options = options(path.clone()); options.max_size_bytes = 6; - let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now)).unwrap(); + let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now), false).unwrap(); appender.write_all(b"abc\n").unwrap(); appender.write_all(b"def\n").unwrap(); appender.flush().unwrap(); @@ -58,7 +167,7 @@ fn max_files_retention_removes_oldest_archives() { options.max_size_bytes = 4; options.max_files = 2; - let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now)).unwrap(); + let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now), false).unwrap(); for line in [b"aa\n", b"bb\n", b"cc\n", b"dd\n"] { appender.write_all(line).unwrap(); } @@ -94,7 +203,7 @@ fn max_age_retention_removes_old_archives() { let mut options = options(path); options.max_age_secs = 1; - let _appender = BoundedFileAppender::with_now(options, Box::new(fixed_now)).unwrap(); + let _appender = BoundedFileAppender::with_now(options, Box::new(fixed_now), false).unwrap(); assert!(!old_archive.exists()); } @@ -112,7 +221,7 @@ fn rotation_stays_bound_to_opened_directory_after_path_replacement() { fs::create_dir(&redirect).unwrap(); let mut options = options(original.join("telemt.log")); options.max_size_bytes = 4; - let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now)).unwrap(); + let mut appender = BoundedFileAppender::with_now(options, Box::new(fixed_now), false).unwrap(); appender.write_all(b"aa\n").unwrap(); fs::rename(&original, &moved).unwrap(); symlink(&redirect, &original).unwrap(); @@ -137,7 +246,11 @@ fn appender_rejects_group_writable_log_directory() { fs::set_permissions(dir.path(), fs::Permissions::from_mode(0o770)).unwrap(); assert!( - BoundedFileAppender::with_now(options(dir.path().join("telemt.log")), Box::new(fixed_now),) - .is_err() + BoundedFileAppender::with_now( + options(dir.path().join("telemt.log")), + Box::new(fixed_now), + true + ) + .is_err() ); } diff --git a/src/logging/tests.rs b/src/logging/tests.rs index ae57470..5c80eda 100644 --- a/src/logging/tests.rs +++ b/src/logging/tests.rs @@ -4,6 +4,7 @@ use super::*; fn test_parse_log_cli_options_default() { let args: Vec = vec![]; let options = parse_log_cli_options(&args).unwrap(); + assert!(!LoggingOptions::default().strict_runtime_paths); assert_eq!( resolve_log_destination(&LoggingConfig::default(), &options).unwrap(), LogDestination::Stderr diff --git a/src/maestro/bootstrap.rs b/src/maestro/bootstrap.rs index fd9abb0..f938cd4 100644 --- a/src/maestro/bootstrap.rs +++ b/src/maestro/bootstrap.rs @@ -15,20 +15,32 @@ use super::helpers::{ use super::runtime_tasks; use super::validate_synlimit_privilege_drop; +/// Process-level configuration and logging resources retained across runtime generations. pub(super) struct BootstrapState { + /// Monotonic process startup time. pub(super) process_started_at: Instant, + /// Process startup time as Unix epoch seconds. pub(super) process_started_at_epoch_secs: u64, + /// Startup component progress shared with the control plane. pub(super) startup_tracker: Arc, + /// Validated initial runtime configuration. pub(super) config: ProxyConfig, + /// Resolved source path used by reload operations. pub(super) config_path: PathBuf, + /// Whether the environment owns the log filter policy. pub(super) has_rust_log: bool, + /// Initial verbosity after CLI overrides. pub(super) effective_log_level: LogLevel, + /// Process-owned dynamic tracing filter. pub(super) runtime_log_filter: runtime_tasks::RuntimeLogFilter, + /// Keeps the file logging worker alive until process shutdown. pub(super) logging_guard: Option, } +/// Loads configuration and initializes process logging with the startup parent-path policy. pub(super) async fn bootstrap( privilege_drop_requested: bool, + strict_runtime_paths: bool, ) -> std::result::Result> { let process_started_at = Instant::now(); let process_started_at_epoch_secs = SystemTime::now() @@ -268,6 +280,7 @@ pub(super) async fn bootstrap( let logging_opts = crate::logging::LoggingOptions { destination: log_destination, disable_colors: true, + strict_runtime_paths, }; let (_, guard) = crate::logging::init_logging(&logging_opts, &initial_filter_spec); logging_guard = Some(guard); @@ -276,6 +289,7 @@ pub(super) async fn bootstrap( let logging_opts = crate::logging::LoggingOptions { destination: log_destination, disable_colors: true, + strict_runtime_paths, }; let (_, guard) = crate::logging::init_logging(&logging_opts, &initial_filter_spec); logging_guard = Some(guard); diff --git a/src/maestro/helpers.rs b/src/maestro/helpers.rs index 57a38e3..5ce7c88 100644 --- a/src/maestro/helpers.rs +++ b/src/maestro/helpers.rs @@ -30,6 +30,7 @@ pub(crate) fn print_maestro_line(message: impl AsRef) { ); } +/// Resolves the config source against startup cwd while retaining symlink components. pub(crate) fn resolve_runtime_config_path( config_path_cli: &str, startup_cwd: &Path, @@ -75,6 +76,7 @@ pub(crate) fn resolve_runtime_config_path( startup_cwd.join("config.toml") } +/// Selects the runtime directory from CLI, startup cwd, and explicit config location. pub(crate) fn resolve_runtime_base_dir( config_path: &Path, startup_cwd: &Path, @@ -120,14 +122,21 @@ fn normalize_runtime_dir(path: &Path, startup_cwd: &Path) -> PathBuf { /// Parsed CLI arguments. pub(crate) struct CliArgs { + /// Config source selected by positional arguments. pub config_path: String, + /// Whether the config source was explicitly provided. pub config_path_explicit: bool, + /// Runtime directory override from CLI. pub data_path: Option, + /// Whether CLI requests minimal logging output. pub silent: bool, + /// Verbosity override from CLI. pub log_level: Option, + /// Logging destination, rotation, and retention overrides. pub log_cli_options: LogCliOptions, } +/// Parses runtime arguments after early daemon and control-command handling. pub(crate) fn parse_cli() -> CliArgs { let mut config_path = "config.toml".to_string(); let mut config_path_explicit = false; @@ -222,6 +231,8 @@ pub(crate) fn parse_cli() -> CliArgs { } // Skip daemon-related flags (already parsed) "--daemon" | "-d" | "--foreground" | "-f" => {} + #[cfg(unix)] + "--strict-runtime-paths" => {} s if s.starts_with("--pid-file") => { if !s.contains('=') { // Skip the pid-file value consumed by daemon argument parsing. @@ -300,6 +311,15 @@ fn print_help() { eprintln!(" --daemon, -d Fork to background (daemonize)"); eprintln!(" --foreground, -f Explicit foreground mode (for systemd)"); eprintln!(" --pid-file PID file path (default: /var/run/telemt.pid)"); + eprintln!( + " --strict-runtime-paths Require trusted, symlink-free PID/log parents (default: off)" + ); + eprintln!( + " Applies to run/start/stop/reload/status; parent symlinks and" + ); + eprintln!( + " writable directories are allowed when this flag is absent" + ); eprintln!(" --run-as-user Drop privileges to this user after binding"); eprintln!(" --run-as-group Drop privileges to this group after binding"); eprintln!(" --working-dir Working directory for daemon mode"); diff --git a/src/maestro/mod.rs b/src/maestro/mod.rs index 07565ae..22c8d28 100644 --- a/src/maestro/mod.rs +++ b/src/maestro/mod.rs @@ -96,7 +96,10 @@ async fn run_inner( // Acquire PID file if daemonizing or if explicitly requested. // Keep it alive until shutdown for RAII cleanup. let _pid_file = if daemon_opts.daemonize || daemon_opts.pid_file.is_some() { - let mut pf = PidFile::new(daemon_opts.pid_file_path()); + let mut pf = PidFile::new( + daemon_opts.pid_file_path(), + daemon_opts.strict_runtime_paths, + ); if let Err(e) = pf.acquire() { eprintln!("[telemt] {}", e); std::process::exit(1); @@ -109,20 +112,25 @@ async fn run_inner( let user = daemon_opts.user.clone(); let group = daemon_opts.group.clone(); - orchestrator::run_telemt_core(user.is_some() || group.is_some(), || { - if (user.is_some() || group.is_some()) - && let Err(e) = drop_privileges(user.as_deref(), group.as_deref(), _pid_file.as_ref()) - { - error!(error = %e, "Failed to drop privileges"); - std::process::exit(1); - } - }) + orchestrator::run_telemt_core( + user.is_some() || group.is_some(), + daemon_opts.strict_runtime_paths, + || { + if (user.is_some() || group.is_some()) + && let Err(e) = + drop_privileges(user.as_deref(), group.as_deref(), _pid_file.as_ref()) + { + error!(error = %e, "Failed to drop privileges"); + std::process::exit(1); + } + }, + ) .await } #[cfg(not(unix))] async fn run_inner() -> std::result::Result<(), Box> { - orchestrator::run_telemt_core(false, || {}).await + orchestrator::run_telemt_core(false, false, || {}).await } #[cfg(test)] diff --git a/src/maestro/orchestrator.rs b/src/maestro/orchestrator.rs index 32c7869..c8d78fb 100644 --- a/src/maestro/orchestrator.rs +++ b/src/maestro/orchestrator.rs @@ -30,10 +30,11 @@ use super::{ runtime_tasks, shutdown, tls_bootstrap, }; -// Shared maestro startup and main loop. `drop_after_bind` runs on Unix after listeners are bound -// and privileged firewall setup completes; it is a no-op on other platforms. +/// Runs startup and the main loop with explicit runtime-path policy. +/// `drop_after_bind` runs after listeners and privileged firewall setup are ready. pub(super) async fn run_telemt_core( privilege_drop_requested: bool, + strict_runtime_paths: bool, drop_after_bind: impl FnOnce(), ) -> std::result::Result<(), Box> { let bootstrap::BootstrapState { @@ -46,7 +47,7 @@ pub(super) async fn run_telemt_core( effective_log_level, runtime_log_filter, logging_guard: _logging_guard, - } = bootstrap::bootstrap(privilege_drop_requested).await?; + } = bootstrap::bootstrap(privilege_drop_requested, strict_runtime_paths).await?; if privilege_drop_requested && config.server.conntrack_control.inline_conntrack_control { warn!("Inline conntrack control is disabled when process privileges are dropped"); diff --git a/src/util/secure_fs/mod.rs b/src/util/secure_fs/mod.rs index f124002..68e1bce 100644 --- a/src/util/secure_fs/mod.rs +++ b/src/util/secure_fs/mod.rs @@ -1,14 +1,15 @@ //! Descriptor-anchored filesystem operations for privileged runtime paths. //! //! Submodules: -//! - `path`: symlink-free directory traversal and anchored path ownership +//! - `path`: strict or compatible directory traversal and anchored path ownership //! - `write`: regular-file opening and durable atomic replacement mod path; mod write; pub(crate) use path::{ - AnchoredPath, chdir_nofollow_or_create, open_dir_nofollow, open_trusted_dir_nofollow_or_create, + AnchoredPath, chdir_nofollow_or_create, open_compatible_dir, open_dir_nofollow, + open_trusted_dir_nofollow_or_create, }; pub(crate) use write::{ atomic_replace, atomic_replace_async, open_append_regular, open_append_regular_at, diff --git a/src/util/secure_fs/path.rs b/src/util/secure_fs/path.rs index 124efc8..6f031aa 100644 --- a/src/util/secure_fs/path.rs +++ b/src/util/secure_fs/path.rs @@ -1,7 +1,7 @@ use std::ffi::{OsStr, OsString}; use std::io; use std::os::fd::OwnedFd; -use std::os::unix::fs::{MetadataExt, PermissionsExt}; +use std::os::unix::fs::{DirBuilderExt, MetadataExt, PermissionsExt}; use std::path::{Component, Path}; use nix::fcntl::{OFlag, open, openat}; @@ -53,6 +53,28 @@ impl AnchoredPath { Ok(Self { parent, name }) } + /// Anchors a runtime parent, applying trusted traversal only when explicitly requested. + pub(crate) fn open_runtime_parent( + path: &Path, + create_mode: Option, + strict_runtime_paths: bool, + ) -> io::Result { + if strict_runtime_paths { + return match create_mode { + Some(mode) => Self::open_trusted_parent_or_create(path, mode), + None => Self::open_trusted_parent(path), + }; + } + let name = path + .file_name() + .filter(|name| !name.is_empty()) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "path has no file name"))? + .to_os_string(); + let parent = + open_compatible_dir(path.parent().unwrap_or_else(|| Path::new(".")), create_mode)?; + Ok(Self { parent, name }) + } + fn open_with_parent_creation(path: &Path, create_mode: Option) -> io::Result { let name = path .file_name() @@ -97,6 +119,23 @@ pub(crate) fn open_trusted_dir_nofollow_or_create(path: &Path, mode: u32) -> io: open_dir_components(path, Some(mode), true) } +/// Follows parent-directory symlinks without imposing ownership or permission policy. +pub(crate) fn open_compatible_dir(path: &Path, create_mode: Option) -> io::Result { + let path = if path.as_os_str().is_empty() { + Path::new(".") + } else { + path + }; + if let Some(mode) = create_mode { + std::fs::DirBuilder::new() + .recursive(true) + .mode(mode) + .create(path)?; + } + // Retain the resolved directory inode so subsequent file operations do not rewalk the path. + open(path, DIRECTORY_FLAGS & !OFlag::O_NOFOLLOW, Mode::empty()).map_err(errno_to_io) +} + /// Opens a directory only when its entire path is owned by root or the effective user. fn open_trusted_dir_nofollow(path: &Path) -> io::Result { open_dir_components(path, None, true) diff --git a/src/util/secure_fs/tests.rs b/src/util/secure_fs/tests.rs index c0b80fc..9276344 100644 --- a/src/util/secure_fs/tests.rs +++ b/src/util/secure_fs/tests.rs @@ -1,4 +1,5 @@ use std::os::unix::fs::{PermissionsExt, symlink}; +use std::path::Path; use super::path::AnchoredPath; use super::write::atomic_replace_after_anchor; @@ -15,6 +16,50 @@ fn directory_walk_rejects_intermediate_symlink() { assert!(open_dir_nofollow(&link).is_err()); } +#[test] +fn runtime_parent_policy_creates_and_reads_relative_paths() { + use std::os::unix::fs::MetadataExt; + + let current = std::env::current_dir().unwrap(); + let directory = tempfile::tempdir_in(¤t).unwrap(); + let relative = directory.path().strip_prefix(¤t).unwrap(); + for strict_runtime_paths in [false, true] { + let mode = if strict_runtime_paths { + "strict" + } else { + "compatible" + }; + let path = relative.join(mode).join("nested/telemt.pid"); + let created = + AnchoredPath::open_runtime_parent(&path, Some(0o750), strict_runtime_paths).unwrap(); + let opened = AnchoredPath::open_runtime_parent(&path, None, strict_runtime_paths).unwrap(); + let created = std::fs::File::from(created.parent().try_clone().unwrap()) + .metadata() + .unwrap(); + let opened = std::fs::File::from(opened.parent().try_clone().unwrap()) + .metadata() + .unwrap(); + assert_eq!((created.dev(), created.ino()), (opened.dev(), opened.ino())); + assert_eq!(created.permissions().mode() & 0o777, 0o750); + } + assert!(AnchoredPath::open_runtime_parent(Path::new("telemt.pid"), None, false).is_ok()); +} + +#[test] +fn runtime_parent_policy_only_requires_trusted_ownership_when_strict() { + if !nix::unistd::Uid::effective().is_root() { + return; + } + let directory = tempfile::tempdir().unwrap(); + let parent = directory.path().join("foreign-owner"); + std::fs::create_dir(&parent).unwrap(); + nix::unistd::chown(&parent, Some(nix::unistd::Uid::from_raw(65534)), None).unwrap(); + let path = parent.join("telemt.pid"); + + assert!(AnchoredPath::open_runtime_parent(&path, None, false).is_ok()); + assert!(AnchoredPath::open_runtime_parent(&path, None, true).is_err()); +} + #[test] fn atomic_replace_does_not_follow_final_symlink() { let directory = tempfile::tempdir().unwrap();