From 98f96d81625e5c7ac6d7b6ed56415b9383871596 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 16 Dec 2025 15:16:05 +0100 Subject: [PATCH 1/3] Remove process field from Context This simplifies some things and is required for correctness once we implement --background as we need to kill the forked process rather than the original process that no longer exists. --- src/common/context.rs | 7 +------ src/su/context.rs | 6 +----- src/su/mod.rs | 5 ++--- src/sudo/env/tests.rs | 3 +-- src/sudo/pipeline.rs | 4 +--- src/sudo/pipeline/edit.rs | 6 ++---- src/system/mod.rs | 8 -------- 7 files changed, 8 insertions(+), 31 deletions(-) diff --git a/src/common/context.rs b/src/common/context.rs index 2c6992e7f..249e8d63e 100644 --- a/src/common/context.rs +++ b/src/common/context.rs @@ -5,7 +5,7 @@ use crate::exec::RunOptions; use crate::sudo::{SudoEditOptions, SudoListOptions, SudoRunOptions, SudoValidateOptions}; use crate::sudoers::Sudoers; use crate::sudoers::{DirChange, Restrictions}; -use crate::system::{audit::sudo_call, Group, Hostname, Process, User}; +use crate::system::{audit::sudo_call, Group, Hostname, User}; use super::{ command::CommandAndArguments, @@ -30,7 +30,6 @@ pub struct Context { // system pub hostname: Hostname, pub current_user: CurrentUser, - pub process: Process, // sudoedit pub files_to_edit: Vec>, } @@ -98,7 +97,6 @@ impl Context { bell: sudo_options.bell, prompt, non_interactive: sudo_options.non_interactive, - process: Process::new(), files_to_edit: vec![], }) } @@ -166,7 +164,6 @@ impl Context { bell: sudo_options.bell, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, - process: Process::new(), files_to_edit, }) } @@ -190,7 +187,6 @@ impl Context { bell: sudo_options.bell, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, - process: Process::new(), files_to_edit: vec![], }) } @@ -237,7 +233,6 @@ impl Context { bell: sudo_options.bell, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, - process: Process::new(), files_to_edit: vec![], }) } diff --git a/src/su/context.rs b/src/su/context.rs index 239910581..c751e79fd 100644 --- a/src/su/context.rs +++ b/src/su/context.rs @@ -9,7 +9,7 @@ use std::{ use crate::common::{error::Error, resolve::CurrentUser}; use crate::exec::{RunOptions, Umask}; use crate::log::user_warn; -use crate::system::{Group, Process, User}; +use crate::system::{Group, User}; use crate::{common::resolve::is_valid_executable, system::interface::UserId}; type Environment = HashMap; @@ -32,7 +32,6 @@ pub(crate) struct SuContext { pub(crate) user: User, pub(crate) requesting_user: CurrentUser, group: Group, - pub(crate) process: Process, } /// check that a shell is not restricted / exists in /etc/shells @@ -50,8 +49,6 @@ fn is_restricted(shell: &Path) -> bool { impl SuContext { pub(crate) fn from_env(options: SuRunOptions) -> Result { - let process = crate::system::Process::new(); - // resolve environment, reset if this is a login let mut environment = if options.login { Environment::default() @@ -197,7 +194,6 @@ impl SuContext { user, requesting_user, group, - process, }) } } diff --git a/src/su/mod.rs b/src/su/mod.rs index 85e7acd6f..4279af086 100644 --- a/src/su/mod.rs +++ b/src/su/mod.rs @@ -5,6 +5,7 @@ use crate::exec::ExitReason; use crate::log::user_warn; use crate::pam::{PamContext, PamError, PamErrorType}; use crate::system::term::current_tty_name; +use crate::system::Process; use std::{env, process}; @@ -105,8 +106,6 @@ fn run(options: SuRunOptions) -> Result<(), Error> { let mut environment = context.environment.clone(); environment.extend(pam.env()?); - let pid = context.process.pid; - // run command and return corresponding exit code let command_exit_reason = crate::exec::run_command(context.as_run_options(), environment); @@ -115,7 +114,7 @@ fn run(options: SuRunOptions) -> Result<(), Error> { match command_exit_reason? { ExitReason::Code(code) => process::exit(code), ExitReason::Signal(signal) => { - crate::system::kill(pid, signal)?; + crate::system::kill(Process::process_id(), signal)?; } } diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs index 7bfb3efed..aac931689 100644 --- a/src/sudo/env/tests.rs +++ b/src/sudo/env/tests.rs @@ -5,7 +5,7 @@ use crate::sudo::{ env::environment::{get_target_environment, Environment}, }; use crate::system::interface::{GroupId, UserId}; -use crate::system::{Group, Hostname, Process, User}; +use crate::system::{Group, Hostname, User}; use std::collections::{HashMap, HashSet}; const TESTS: &str = " @@ -130,7 +130,6 @@ fn create_test_context(sudo_options: SudoRunOptions) -> Context { stdin: sudo_options.stdin, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, - process: Process::new(), use_session_records: false, bell: false, files_to_edit: vec![], diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs index 8eb30a737..f673397f9 100644 --- a/src/sudo/pipeline.rs +++ b/src/sudo/pipeline.rs @@ -105,8 +105,6 @@ pub fn run(mut cmd_opts: SudoRunOptions) -> Result<(), Error> { environment::dangerous_extend(&mut target_env, trusted_vars); - let pid = context.process.pid; - // prepare switch of apparmor profile #[cfg(feature = "apparmor")] if let Some(profile) = &controls.apparmor_profile { @@ -128,7 +126,7 @@ pub fn run(mut cmd_opts: SudoRunOptions) -> Result<(), Error> { match command_exit_reason? { ExitReason::Code(code) => exit(code), ExitReason::Signal(signal) => { - crate::system::kill(pid, signal)?; + crate::system::kill(Process::process_id(), signal)?; } } diff --git a/src/sudo/pipeline/edit.rs b/src/sudo/pipeline/edit.rs index 5ac597b2f..2706fdcca 100644 --- a/src/sudo/pipeline/edit.rs +++ b/src/sudo/pipeline/edit.rs @@ -5,7 +5,7 @@ use crate::common::{Context, Error}; use crate::exec::ExitReason; use crate::log::{user_error, user_info}; use crate::sudoers::Authorization; -use crate::system::audit; +use crate::system::{audit, Process}; pub fn run_edit(edit_opts: SudoEditOptions) -> Result<(), Error> { let policy = super::read_sudoers()?; @@ -20,8 +20,6 @@ pub fn run_edit(edit_opts: SudoEditOptions) -> Result<(), Error> { let mut pam_context = super::auth_and_update_record_file(&context, auth)?; - let pid = context.process.pid; - let mut opened_files = Vec::with_capacity(context.files_to_edit.len()); for (path, arg) in context.files_to_edit.iter().zip(&context.command.arguments) { if let Some(path) = path { @@ -67,7 +65,7 @@ pub fn run_edit(edit_opts: SudoEditOptions) -> Result<(), Error> { match command_exit_reason? { ExitReason::Code(code) => exit(code), ExitReason::Signal(signal) => { - crate::system::kill(pid, signal)?; + crate::system::kill(Process::process_id(), signal)?; } } diff --git a/src/system/mod.rs b/src/system/mod.rs index 774ed291a..f23ec2e99 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -610,21 +610,13 @@ impl WithProcess { #[derive(Debug, Clone)] pub struct Process { - pub pid: ProcessId, pub parent_pid: Option, pub session_id: ProcessId, } -impl Default for Process { - fn default() -> Self { - Self::new() - } -} - impl Process { pub fn new() -> Process { Process { - pid: Self::process_id(), parent_pid: Self::parent_id(), session_id: Self::session_id(), } From 9f0b6acde5f70ae7daea29e15c45a661ca2fdc58 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 9 Dec 2025 16:00:32 +0100 Subject: [PATCH 2/3] Implement --background flag --- src/common/context.rs | 6 ++++++ src/exec/mod.rs | 22 ++++++++++++++++++---- src/exec/use_pty/parent.rs | 27 +++++++++++++++++++++++++++ src/su/context.rs | 1 + src/sudo/cli/mod.rs | 10 ++++++++++ src/sudo/cli/tests.rs | 2 +- src/sudo/env/tests.rs | 1 + 7 files changed, 64 insertions(+), 5 deletions(-) diff --git a/src/common/context.rs b/src/common/context.rs index 249e8d63e..2d1a2208d 100644 --- a/src/common/context.rs +++ b/src/common/context.rs @@ -24,6 +24,7 @@ pub struct Context { pub askpass: bool, pub stdin: bool, pub bell: bool, + pub background: bool, pub prompt: Option, pub non_interactive: bool, pub use_session_records: bool, @@ -95,6 +96,7 @@ impl Context { askpass: sudo_options.askpass, stdin: sudo_options.stdin, bell: sudo_options.bell, + background: sudo_options.background, prompt, non_interactive: sudo_options.non_interactive, files_to_edit: vec![], @@ -162,6 +164,7 @@ impl Context { askpass: sudo_options.askpass, stdin: sudo_options.stdin, bell: sudo_options.bell, + background: false, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, files_to_edit, @@ -185,6 +188,7 @@ impl Context { askpass: sudo_options.askpass, stdin: sudo_options.stdin, bell: sudo_options.bell, + background: false, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, files_to_edit: vec![], @@ -231,6 +235,7 @@ impl Context { askpass: sudo_options.askpass, stdin: sudo_options.stdin, bell: sudo_options.bell, + background: false, prompt: sudo_options.prompt, non_interactive: sudo_options.non_interactive, files_to_edit: vec![], @@ -275,6 +280,7 @@ impl Context { group: &self.target_group, umask: controls.umask, + background: self.background, use_pty: controls.use_pty, noexec: controls.noexec, }) diff --git a/src/exec/mod.rs b/src/exec/mod.rs index 39f4fb9ef..7531be11d 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -13,7 +13,7 @@ use std::{ os::unix::ffi::OsStrExt, os::unix::process::CommandExt, path::{Path, PathBuf}, - process::Command, + process::{self, Command}, time::Duration, }; @@ -24,13 +24,12 @@ use crate::{ exec::no_pty::exec_no_pty, log::{dev_info, dev_warn, user_error}, system::{ - _exit, + ForkResult, Group, User, _exit, fork, interface::ProcessId, - kill, killpg, mark_fds_as_cloexec, set_target_user, + kill, killpg, mark_fds_as_cloexec, set_target_user, setpgid, signal::{consts::*, signal_name, SignalNumber, SignalSet}, term::UserTerm, wait::{Wait, WaitError, WaitOptions}, - Group, User, }, }; @@ -71,6 +70,7 @@ pub struct RunOptions<'a> { pub group: &'a Group, pub umask: Umask, + pub background: bool, pub use_pty: bool, pub noexec: bool, } @@ -83,6 +83,19 @@ pub fn run_command( options: RunOptions<'_>, env: impl IntoIterator, impl AsRef)>, ) -> io::Result { + if options.background { + // SAFETY: There should be no other threads at this point. + match unsafe { fork() }? { + ForkResult::Parent(_) => process::exit(0), + ForkResult::Child => { + // Child continues in an orphaned process group. + // Reads from the terminal fail with EIO. + // Writes succeed unless tostop is set on the terminal. + setpgid(ProcessId::new(0), ProcessId::new(0))?; + } + } + } + // FIXME: should we pipe the stdio streams? let qualified_path = options.command; let mut command = Command::new(qualified_path); @@ -185,6 +198,7 @@ pub fn run_command( command, user_tty, options.user, + options.background, ), Err(err) => { dev_info!("Could not open user's terminal, not allocating a pty: {err}"); diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index 09d7d3f6a..ba5f9124f 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -1,8 +1,12 @@ use std::collections::VecDeque; use std::ffi::c_int; use std::io; +use std::os::fd::{FromRawFd, OwnedFd}; use std::process::{Command, Stdio}; +use libc::{close, O_CLOEXEC}; + +use crate::cutils::cerr; use crate::exec::event::{EventHandle, EventRegistry, PollEvent, Process, StopReason}; use crate::exec::use_pty::monitor::exec_monitor; use crate::exec::use_pty::SIGCONT_FG; @@ -31,6 +35,7 @@ pub(in crate::exec) fn exec_pty( mut command: Command, user_tty: UserTerm, pty_owner: &User, + background: bool, ) -> io::Result { // Allocate a pseudoterminal. let pty = get_pty(pty_owner)?; @@ -81,6 +86,10 @@ pub(in crate::exec) fn exec_pty( ParentEvent::Pty, ); + if background { + tty_pipe.disable_input(&mut registry); + } + let user_tty = tty_pipe.left_mut(); // Check if we are the foreground process @@ -118,6 +127,24 @@ pub(in crate::exec) fn exec_pty( // change the terminal mode. exec_bg = true; } + } else if background { + // Running in background (sudo -b), no access to terminal input. + // In non-pty mode, the command runs in an orphaned process + // group and reads from the controlling terminal fail with EIO. + // We cannot do the same while running in a pty but if we set + // stdin to a half-closed pipe, reads from it will get EOF. + exec_bg = true; + + let mut pipes = [-1, -1]; + // SAFETY: A valid pointer to a mutable array of 2 fds is passed in. + unsafe { + cerr(libc::pipe2(pipes.as_mut_ptr(), O_CLOEXEC))?; + } + // SAFETY: pipe2 created two owned pipe fds. + unsafe { + command.stdin(OwnedFd::from_raw_fd(pipes[0])); + close(pipes[1]); + } } if !io::stdout().is_terminal_for_pgrp(parent_pgrp) { diff --git a/src/su/context.rs b/src/su/context.rs index c751e79fd..6b47ecd27 100644 --- a/src/su/context.rs +++ b/src/su/context.rs @@ -210,6 +210,7 @@ impl SuContext { group: &self.group, umask: Umask::Preserve, + background: false, use_pty: true, noexec: false, } diff --git a/src/sudo/cli/mod.rs b/src/sudo/cli/mod.rs index f36e765ed..81728d887 100644 --- a/src/sudo/cli/mod.rs +++ b/src/sudo/cli/mod.rs @@ -385,6 +385,8 @@ pub struct SudoRunOptions { pub askpass: bool, // -B pub bell: bool, + // -b + pub background: bool, // -E /* ignored, part of env_var_list */ // -k @@ -416,6 +418,7 @@ impl TryFrom for SudoRunOptions { fn try_from(mut opts: SudoOptions) -> Result { let askpass = mem::take(&mut opts.askpass); let bell = mem::take(&mut opts.bell); + let background = mem::take(&mut opts.background); let reset_timestamp = mem::take(&mut opts.reset_timestamp); let non_interactive = mem::take(&mut opts.non_interactive); let stdin = mem::take(&mut opts.stdin); @@ -466,6 +469,7 @@ impl TryFrom for SudoRunOptions { Ok(Self { askpass, bell, + background, reset_timestamp, non_interactive, stdin, @@ -487,6 +491,8 @@ struct SudoOptions { askpass: bool, // -B bell: bool, + // -b + background: bool, // -D chdir: Option, // -g @@ -698,6 +704,9 @@ impl SudoOptions { "-B" | "--bell" => { options.bell = true; } + "-b" | "--background" => { + options.background = true; + } "-E" | "--preserve-env" => { user_warn!( "preserving the entire environment is not supported, '{flag}' is ignored", @@ -878,6 +887,7 @@ fn reject_all(context: &str, opts: SudoOptions) -> Result<(), String> { check_options!( askpass, bell, + background, chdir, edit, group, diff --git a/src/sudo/cli/tests.rs b/src/sudo/cli/tests.rs index d22541f13..0228fe8be 100644 --- a/src/sudo/cli/tests.rs +++ b/src/sudo/cli/tests.rs @@ -330,7 +330,7 @@ fn help() { let cmd = SudoAction::try_parse_from(["sudo", "-h"]).unwrap(); assert!(cmd.is_help()); - let cmd = SudoAction::try_parse_from(["sudo", "-bh"]); + let cmd = SudoAction::try_parse_from(["sudo", "-zh"]); assert!(cmd.is_err()); let cmd = SudoAction::try_parse_from(["sudo", "--help"]).unwrap(); diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs index aac931689..baf368885 100644 --- a/src/sudo/env/tests.rs +++ b/src/sudo/env/tests.rs @@ -132,6 +132,7 @@ fn create_test_context(sudo_options: SudoRunOptions) -> Context { non_interactive: sudo_options.non_interactive, use_session_records: false, bell: false, + background: false, files_to_edit: vec![], } } From c9692eb1c76b7d41227d058d73a6ae9bb99fe1f4 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 16 Dec 2025 16:26:35 +0100 Subject: [PATCH 3/3] Add a basic test for --background --- src/exec/use_pty/parent.rs | 2 +- .../sudo-compliance-tests/src/sudo.rs | 1 + .../src/sudo/flag_background.rs | 28 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 test-framework/sudo-compliance-tests/src/sudo/flag_background.rs diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index ba5f9124f..85c3662ec 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -60,7 +60,7 @@ pub(in crate::exec) fn exec_pty( // `policy_init_session`. // FIXME (ogsudo): initializes ttyblock sigset here by calling `init_ttyblock` - // Fetch the parent process group so we can signals to it. + // Fetch the parent process group so we can send signals to it. let parent_pgrp = getpgrp(); // Set all the IO streams for the command to the follower side of the pty. diff --git a/test-framework/sudo-compliance-tests/src/sudo.rs b/test-framework/sudo-compliance-tests/src/sudo.rs index 8d23a259a..02a994e66 100644 --- a/test-framework/sudo-compliance-tests/src/sudo.rs +++ b/test-framework/sudo-compliance-tests/src/sudo.rs @@ -3,6 +3,7 @@ mod apparmor; mod child_process; mod cli; mod env_reset; +mod flag_background; mod flag_chdir; mod flag_group; mod flag_help; diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_background.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_background.rs new file mode 100644 index 000000000..739553eac --- /dev/null +++ b/test-framework/sudo-compliance-tests/src/sudo/flag_background.rs @@ -0,0 +1,28 @@ +use sudo_test::{Command, Env}; + +use crate::SUDOERS_ALL_ALL_NOPASSWD; + +#[test] +fn runs_in_background() { + let env = Env(SUDOERS_ALL_ALL_NOPASSWD).build(); + + Command::new("sudo") + .args([ + "-b", + "sh", + "-c", + "touch /tmp/barrier1; until [ -f /tmp/barrier2 ]; do sleep 0.1; done; touch /tmp/barrier3", + ]) + .output(&env) + .assert_success(); + + Command::new("sh") + .args([ + "-c", + "until [ -f /tmp/barrier1 ]; do sleep 0.1; done + touch /tmp/barrier2 + until [ -f /tmp/barrier3 ]; do sleep 0.1; done", + ]) + .output(&env) + .assert_success(); +}