system/term: Resolve the current tty once and share it

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<Option<SudoPath>>,
 }
@@ -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,

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<PamCon
     pam.set_requesting_user(requesting_user)?;
 
     // attempt to set the TTY this session is communicating on
-    if let Ok(pam_tty) = current_tty_name() {
+    if let Some(pam_tty) = CurrentTty::resolve().name {
         pam.set_tty(&pam_tty)?;
     }
 

src/sudo/env/tests.rs

@@ -5,6 +5,7 @@ use crate::sudo::{
     env::environment::{Environment, get_target_environment},
 };
 use crate::system::interface::{GroupId, UserId};
+use crate::system::term::CurrentTty;
 use crate::system::{Group, Hostname, User};
 use std::collections::{HashMap, HashSet};
 
@@ -114,6 +115,11 @@ fn create_test_context(sudo_options: SudoRunOptions) -> 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 {

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(())
             }

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<PamContext> {
     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)

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("")
     };

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<Option<OsString>> {
-    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<OsString> {
+    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()
 }
 

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<F: AsFd> Terminal for F {
     }
 }
 
-/// Try to get the path of the current TTY
-pub(crate) fn current_tty_name() -> io::Result<OsString> {
-    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<OsString>,
+    /// Device of the controlling terminal
+    pub(crate) device: Option<DeviceId>,
+    /// Path of the controlling terminal device
+    pub(crate) path: Option<OsString>,
+}
+
+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<File> {
+        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<TtyGuard> {
-    // 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<TtyGuard> {
+        // 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,

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<RecordScope> {
-        let Ok(Some(tty_device)) = Process::tty_device_id(WithProcess::Current) else {
+    pub fn for_tty(process: &Process, tty_device: Option<DeviceId>) -> Option<RecordScope> {
+        let Some(tty_device) = tty_device else {
             return Self::for_ppid(process);
         };