diff --git a/src/common/context.rs b/src/common/context.rs index 41412ad29..bed66c522 100644 --- a/src/common/context.rs +++ b/src/common/context.rs @@ -6,7 +6,7 @@ use crate::exec::RunOptions; use crate::sudo::{SudoEditOptions, SudoListOptions, SudoRunOptions, SudoValidateOptions}; use crate::sudoers::Sudoers; use crate::sudoers::{DirChange, Restrictions}; -use crate::system::{Group, Hostname, User, audit::sudo_call}; +use crate::system::{Group, Hostname, User, audit::sudo_call, term::CurrentTty}; use super::{ SudoPath, @@ -32,6 +32,7 @@ pub struct Context { // system pub hostname: Hostname, pub current_user: CurrentUser, + pub tty: CurrentTty, // sudoedit pub files_to_edit: Vec>, } @@ -89,6 +90,7 @@ impl Context { hostname, command, current_user, + tty: CurrentTty::resolve(), target_user, target_group, use_session_records: !sudo_options.reset_timestamp, @@ -162,6 +164,7 @@ impl Context { hostname, command, current_user, + tty: CurrentTty::resolve(), target_user, target_group, use_session_records: !sudo_options.reset_timestamp, @@ -186,6 +189,7 @@ impl Context { hostname, command: Default::default(), current_user, + tty: CurrentTty::resolve(), target_user, target_group, use_session_records: !sudo_options.reset_timestamp, @@ -233,6 +237,7 @@ impl Context { hostname, command, current_user, + tty: CurrentTty::resolve(), target_user, target_group, use_session_records: !sudo_options.reset_timestamp, diff --git a/src/su/mod.rs b/src/su/mod.rs index 664ee105b..ae35f060e 100644 --- a/src/su/mod.rs +++ b/src/su/mod.rs @@ -3,7 +3,7 @@ use crate::common::error::Error; use crate::log::user_warn; use crate::pam::{PamContext, PamError, PamErrorType}; -use crate::system::term::current_tty_name; +use crate::system::term::CurrentTty; use std::env; @@ -42,7 +42,7 @@ fn authenticate(requesting_user: &str, user: &str, login: bool) -> Result Context { hostname: Hostname::fake("test-ubuntu"), command, current_user: current_user.clone(), + tty: CurrentTty { + name: None, + device: None, + path: None, + }, target_user: if sudo_options.user.as_deref() == Some("test") { current_user.into() } else { diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs index ff6adcf7c..f83d9f0d7 100644 --- a/src/sudo/mod.rs +++ b/src/sudo/mod.rs @@ -6,7 +6,7 @@ use crate::log::dev_info; use crate::system::User; use crate::system::interface::UserId; use crate::system::timestamp::RecordScope; -use crate::system::{Process, timestamp::SessionRecordFile}; +use crate::system::{Process, WithProcess, timestamp::SessionRecordFile}; #[cfg(test)] pub(crate) use cli::SudoAction; #[cfg(not(test))] @@ -101,12 +101,14 @@ fn sudo_process() -> Result<(), Error> { SudoAction::ResetTimestamp(_) => { let user = CurrentUser::resolve()?; let process = Process::new(); - for record_scope in [RecordScope::for_tty, RecordScope::for_ppid] { - if let Some(scope) = record_scope(&process) { - let mut record_file = - SessionRecordFile::open_for_user(&user, Duration::default())?; - record_file.disable(scope)?; - } + let tty_device = Process::tty_device_id(WithProcess::Current).ok().flatten(); + for scope in RecordScope::for_tty(&process, tty_device) + .into_iter() + .chain(RecordScope::for_ppid(&process)) + { + let mut record_file = + SessionRecordFile::open_for_user(&user, Duration::default())?; + record_file.disable(scope)?; } Ok(()) } diff --git a/src/sudo/pam.rs b/src/sudo/pam.rs index 22270132e..266664138 100644 --- a/src/sudo/pam.rs +++ b/src/sudo/pam.rs @@ -1,11 +1,10 @@ -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::time::Duration; use crate::common::context::LaunchType; use crate::common::error::Error; use crate::log::{dev_info, user_warn}; use crate::pam::{PamContext, PamError, PamErrorType, PamResult}; -use crate::system::term::current_tty_name; pub(super) struct InitPamArgs<'a> { pub(super) launch: LaunchType, @@ -20,6 +19,7 @@ pub(super) struct InitPamArgs<'a> { pub(super) requesting_user: &'a str, pub(super) target_user: &'a str, pub(super) hostname: &'a str, + pub(super) tty_name: Option<&'a OsStr>, } pub(super) fn init_pam( @@ -36,6 +36,7 @@ pub(super) fn init_pam( requesting_user, target_user, hostname, + tty_name, }: InitPamArgs, ) -> PamResult { let service_name = match launch { @@ -87,8 +88,8 @@ pub(super) fn init_pam( } // attempt to set the TTY this session is communicating on - if let Ok(pam_tty) = current_tty_name() { - pam.set_tty(&pam_tty)?; + if let Some(pam_tty) = tty_name { + pam.set_tty(pam_tty)?; } Ok(pam) diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs index 2382337de..6e9fe6739 100644 --- a/src/sudo/pipeline.rs +++ b/src/sudo/pipeline.rs @@ -13,7 +13,6 @@ use crate::sudoers::{ AuthenticatingUser, Authentication, AuthenticationScope, Authorization, Judgement, Logging, Sudoers, }; -use crate::system::term::{current_tty_name, lock_tty}; use crate::system::timestamp::{RecordScope, SessionRecordFile, TouchResult}; use crate::system::{Process, escape_os_str_lossy}; @@ -172,11 +171,11 @@ fn auth_and_update_record_file( let _guard = if context.non_interactive || context.stdin || context.askpass { None } else { - lock_tty() + context.tty.lock() }; let scope = match scope { - AuthenticationScope::Tty => RecordScope::for_tty(&Process::new()), + AuthenticationScope::Tty => RecordScope::for_tty(&Process::new(), context.tty.device), AuthenticationScope::PPid => RecordScope::for_ppid(&Process::new()), }; let mut auth_status = determine_auth_status( @@ -201,6 +200,7 @@ fn auth_and_update_record_file( requesting_user: &context.current_user.name, target_user: &context.target_user.name, hostname: &context.hostname, + tty_name: context.tty.name.as_deref(), })?; if auth_status.must_authenticate { if context.non_interactive && !noninteractive_auth { @@ -284,8 +284,8 @@ fn log_command_execution(log: Logging, context: &Context) { if matches!(log, Logging::Disabled) { return; } - let tty_info = if let Ok(tty_name) = current_tty_name() { - format!("TTY={} ;", escape_os_str_lossy(&tty_name)) + let tty_info = if let Some(tty_name) = &context.tty.name { + format!("TTY={} ;", escape_os_str_lossy(tty_name)) } else { String::from("") }; diff --git a/src/system/term/find_tty.rs b/src/system/term/find_tty.rs index b6071857a..ca14311d5 100644 --- a/src/system/term/find_tty.rs +++ b/src/system/term/find_tty.rs @@ -5,26 +5,16 @@ use std::{ path::Path, }; -use crate::system::{DeviceId, Process, WithProcess, term::Terminal}; +use crate::system::{DeviceId, term::Terminal}; -pub(super) fn ttyname_from_dev() -> io::Result> { - let Ok(Some(tty_dev)) = Process::tty_device_id(WithProcess::Current) else { - return Ok(None); - }; - - let tty_name = dev_check(Path::new("/dev/console"), tty_dev) +pub(super) fn ttyname_from_dev(tty_dev: DeviceId) -> Option { + dev_check(Path::new("/dev/console"), tty_dev) .or_else(|| ttyname_from_stdioe(tty_dev)) .or_else(|| find_tty_in_dir(Path::new("/dev/pts"), tty_dev)) - .or_else(|| find_tty_in_dir(Path::new("/dev"), tty_dev)); - - if tty_name.is_some() { - Ok(tty_name) - } else { - Err(io::ErrorKind::NotFound.into()) - } + .or_else(|| find_tty_in_dir(Path::new("/dev"), tty_dev)) } -fn is_our_tty(metadata: fs::Metadata, tty_dev: DeviceId) -> bool { +pub(super) fn is_our_tty(metadata: fs::Metadata, tty_dev: DeviceId) -> bool { metadata.file_type().is_char_device() && metadata.rdev() == tty_dev.inner() } diff --git a/src/system/term/mod.rs b/src/system/term/mod.rs index d56c4cb9e..fde11b068 100644 --- a/src/system/term/mod.rs +++ b/src/system/term/mod.rs @@ -3,9 +3,12 @@ mod user_term; use std::{ ffi::{CString, OsString, c_char, c_uchar}, fmt, - fs::File, + fs::{File, OpenOptions}, io, - os::fd::{AsFd, AsRawFd, FromRawFd, OwnedFd}, + os::{ + fd::{AsFd, AsRawFd, FromRawFd, OwnedFd}, + unix::fs::OpenOptionsExt, + }, ptr::null_mut, }; @@ -14,7 +17,8 @@ use libc::{TIOCSWINSZ, ioctl, winsize}; use crate::cutils::{cerr, is_fifo_or_sock, os_string_from_ptr, safe_isatty}; use super::file::FileLock; -use super::interface::ProcessId; +use super::interface::{DeviceId, ProcessId}; +use super::{Process, WithProcess}; mod find_tty; @@ -255,39 +259,65 @@ impl Terminal for F { } } -/// Try to get the path of the current TTY -pub(crate) fn current_tty_name() -> io::Result { - if let Some(tty) = find_tty::ttyname_from_dev()? { - return Ok(tty); +/// The TTY of the current process, resolved once and shared by all its users +#[derive(Debug)] +pub(crate) struct CurrentTty { + /// Name to report (e.g. for PAM_TTY) + pub(crate) name: Option, + /// Device of the controlling terminal + pub(crate) device: Option, + /// Path of the controlling terminal device + pub(crate) path: Option, +} + +impl CurrentTty { + pub(crate) fn resolve() -> Self { + let device = Process::tty_device_id(WithProcess::Current).ok().flatten(); + let path = device.and_then(find_tty::ttyname_from_dev); + let name = match device { + Some(_) => path.clone(), + None => io::stdin() + .ttyname() + .or_else(|_| io::stdout().ttyname()) + .or_else(|_| io::stderr().ttyname()) + .ok(), + }; + + Self { name, device, path } } - io::stdin() - .ttyname() - .or_else(|_| io::stdout().ttyname()) - .or_else(|_| io::stderr().ttyname()) -} + /// Open the controlling terminal device, if its path still refers to it + fn open_verified(&self) -> Option { + let (path, device) = (self.path.as_ref()?, self.device?); + let file = OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOCTTY | libc::O_NONBLOCK) + .open(path) + .ok()?; -#[expect(unused)] -pub(crate) struct TtyGuard(File, FileLock); + find_tty::is_our_tty(file.metadata().ok()?, device).then_some(file) + } -/// Lock the tty to prevent contention over who owns the password prompt -pub(crate) fn lock_tty() -> Option { - // Prefer the real device: flock() on /dev/tty's single shared inode serializes all terminals. - let tty = match find_tty::ttyname_from_dev() { - Ok(Some(path)) => File::open(path), - _ => File::open("/dev/tty"), + /// Lock the tty to prevent contention over who owns the password prompt + pub(crate) fn lock(&self) -> Option { + // Prefer the real device: flock() on /dev/tty's single shared inode serializes all terminals. + let tty = self + .open_verified() + .or_else(|| File::open("/dev/tty").ok())?; + // og-sudo uses fcntl(F_SETLKW) instead of the flock() that FileLock::exclusive does. + // This does mean in the unlikely case that sudo-rs and og-sudo are used inside the + // same pipeline, they will still fight for access to the tty. Adding a separate lock + // implementation for the tty is probably not worth it and changing the timestamp code + // to use fcntl(F_SETLKW) is iffy as flock() is much better behaved. + let lock = FileLock::exclusive(&tty, false).ok()?; + + Some(TtyGuard(tty, lock)) } - .ok()?; - // og-sudo uses fcntl(F_SETLKW) instead of the flock() that FileLock::exclusive does. - // This does mean in the unlikely case that sudo-rs and og-sudo are used inside the - // same pipeline, they will still fight for access to the tty. Adding a separate lock - // implementation for the tty is probably not worth it and changing the timestamp code - // to use fcntl(F_SETLKW) is iffy as flock() is much better behaved. - let lock = FileLock::exclusive(&tty, false).ok()?; - - Some(TtyGuard(tty, lock)) } +#[expect(unused)] +pub(crate) struct TtyGuard(File, FileLock); + #[repr(transparent)] pub(crate) struct TermSize { raw: winsize, diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs index b3be8e466..a484b0978 100644 --- a/src/system/timestamp.rs +++ b/src/system/timestamp.rs @@ -421,8 +421,8 @@ impl RecordScope { /// This tries tying credentials to the TTY first before falling back to parent PID mode. /// This should never produce an error since any actual error should just be /// ignored and no session record file should be used in that case. - pub fn for_tty(process: &Process) -> Option { - let Ok(Some(tty_device)) = Process::tty_device_id(WithProcess::Current) else { + pub fn for_tty(process: &Process, tty_device: Option) -> Option { + let Some(tty_device) = tty_device else { return Self::for_ppid(process); };