Merge commit from fork

squell · web-flow · commit 0926e85913f4 · 2025-11-10T13:38:06.000+01:00
Use auth user rather than current user in timestamp records
diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs
@@ -101,7 +101,7 @@ fn sudo_process() -> Result<(), Error> {
                     let user = CurrentUser::resolve()?;
                     let mut record_file =
                         SessionRecordFile::open_for_user(&user, Duration::seconds(0))?;
-                    record_file.disable(scope, None)?;
+                    record_file.disable(scope)?;
                 }
                 Ok(())
             }
diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs
@@ -158,15 +158,6 @@ fn auth_and_update_record_file(
         pwfeedback,
     }: Authentication,
 ) -> Result<PamContext, Error> {
-    let scope = RecordScope::for_process(&Process::new());
-    let mut auth_status = determine_auth_status(
-        must_authenticate,
-        context.use_session_records,
-        scope,
-        &context.current_user,
-        prior_validity,
-    );
-
     let auth_user = match credential {
         AuthenticatingUser::InvokingUser => {
             AuthUser::from_current_user(context.current_user.clone())
@@ -177,6 +168,16 @@ fn auth_and_update_record_file(
         }
     };
 
+    let scope = RecordScope::for_process(&Process::new());
+    let mut auth_status = determine_auth_status(
+        must_authenticate,
+        context.use_session_records,
+        scope,
+        &context.current_user,
+        &auth_user,
+        prior_validity,
+    );
+
     let mut pam_context = init_pam(InitPamArgs {
         launch: context.launch,
         use_stdin: context.stdin,
@@ -202,7 +203,7 @@ fn auth_and_update_record_file(
             allowed_attempts,
         )?;
         if let (Some(record_file), Some(scope)) = (&mut auth_status.record_file, scope) {
-            match record_file.create(scope, context.current_user.uid) {
+            match record_file.create(scope, &auth_user) {
                 Ok(_) => (),
                 Err(e) => {
                     auth_warn!("Could not update session record file with new record: {e}");
@@ -255,14 +256,15 @@ fn determine_auth_status(
     use_session_records: bool,
     record_for: Option<RecordScope>,
     current_user: &CurrentUser,
+    auth_user: &AuthUser,
     prior_validity: Duration,
 ) -> AuthStatus {
     if !must_policy_authenticate {
         AuthStatus::new(false, None)
     } else if let (true, Some(record_for)) = (use_session_records, record_for) {
         match SessionRecordFile::open_for_user(current_user, prior_validity) {
             Ok(mut sr) => {
-                match sr.touch(record_for, current_user.uid) {
+                match sr.touch(record_for, auth_user) {
                     // if a record was found and updated within the timeout, we do not need to authenticate
                     Ok(TouchResult::Updated { .. }) => AuthStatus::new(false, Some(sr)),
                     Ok(TouchResult::NotFound | TouchResult::Outdated { .. }) => {
diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs
@@ -4,6 +4,7 @@ use std::{
     path::PathBuf,
 };
 
+use crate::common::resolve::AuthUser;
 use crate::{
     common::resolve::CurrentUser,
     log::{auth_info, auth_warn},
@@ -179,7 +180,7 @@ impl SessionRecordFile {
     /// that record time to the current time. This will not create a new record
     /// when one is not found. A record will only be updated if it is still
     /// valid at this time.
-    pub fn touch(&mut self, scope: RecordScope, auth_user: UserId) -> io::Result<TouchResult> {
+    pub fn touch(&mut self, scope: RecordScope, auth_user: &AuthUser) -> io::Result<TouchResult> {
         // lock the file to indicate that we are currently in a writing operation
         let lock = FileLock::exclusive(&self.file, false)?;
         self.seek_to_first_record()?;
@@ -215,17 +216,12 @@ impl SessionRecordFile {
         Ok(TouchResult::NotFound)
     }
 
-    /// Disable all records that match the given scope. If an auth user id is
-    /// given then only records with the given scope that are targeting that
-    /// specific user will be disabled.
-    pub fn disable(&mut self, scope: RecordScope, auth_user: Option<UserId>) -> io::Result<()> {
+    /// Disable all records that match the given scope.
+    pub fn disable(&mut self, scope: RecordScope) -> io::Result<()> {
         let lock = FileLock::exclusive(&self.file, false)?;
         self.seek_to_first_record()?;
         while let Some(record) = self.next_record()? {
-            let must_disable = auth_user
-                .map(|tu| record.matches(&scope, tu))
-                .unwrap_or_else(|| record.scope == scope);
-            if must_disable {
+            if record.scope == scope {
                 self.file.seek(io::SeekFrom::Current(-SIZE_OF_BOOL))?;
                 write_bool(false, &mut self.file)?;
             }
@@ -237,7 +233,7 @@ impl SessionRecordFile {
     /// Create a new record for the given scope and auth user id.
     /// If there is an existing record that matches the scope and auth user,
     /// then that record will be updated.
-    pub fn create(&mut self, scope: RecordScope, auth_user: UserId) -> io::Result<CreateResult> {
+    pub fn create(&mut self, scope: RecordScope, auth_user: &AuthUser) -> io::Result<CreateResult> {
         // lock the file to indicate that we are currently writing to it
         let lock = FileLock::exclusive(&self.file, false)?;
         self.seek_to_first_record()?;
@@ -256,7 +252,7 @@ impl SessionRecordFile {
         }
 
         // record was not found in the list so far, create a new one
-        let record = SessionRecord::new(scope, auth_user)?;
+        let record = SessionRecord::new(scope, auth_user.uid)?;
 
         // make sure we really are at the end of the file
         self.file.seek(io::SeekFrom::End(0))?;
@@ -552,8 +548,8 @@ impl SessionRecord {
 
     /// Returns true if this record matches the specified scope and is for the
     /// specified target auth user.
-    pub fn matches(&self, scope: &RecordScope, auth_user: UserId) -> bool {
-        self.scope == *scope && self.auth_user == auth_user
+    pub fn matches(&self, scope: &RecordScope, auth_user: &AuthUser) -> bool {
+        self.scope == *scope && self.auth_user == auth_user.uid
     }
 
     /// Returns true if this record was written somewhere in the time range
@@ -566,11 +562,27 @@ impl SessionRecord {
 
 #[cfg(test)]
 mod tests {
+    use std::path::Path;
+
     use super::*;
+    use crate::common::{SudoPath, SudoString};
+    use crate::system::interface::GroupId;
     use crate::system::tests::tempfile;
+    use crate::system::User;
 
     static TEST_USER_ID: UserId = UserId::ROOT;
 
+    fn auth_user_from_uid(uid: libc::uid_t) -> AuthUser {
+        AuthUser::from_user_for_targetpw(User {
+            uid: UserId::new(uid),
+            gid: GroupId::new(0),
+            name: SudoString::new("dummy".to_owned()).unwrap(),
+            home: SudoPath::new(Path::new("/nonexistent").to_owned()).unwrap(),
+            shell: Path::new("/bin/sh").to_owned(),
+            groups: vec![],
+        })
+    }
+
     #[test]
     fn can_encode_and_decode() {
         let tty_sample = SessionRecord::new(
@@ -618,22 +630,22 @@ mod tests {
 
         let tty_sample = SessionRecord::new(scope, UserId::new(675)).unwrap();
 
-        assert!(tty_sample.matches(&scope, UserId::new(675)));
-        assert!(!tty_sample.matches(&scope, UserId::new(789)));
+        assert!(tty_sample.matches(&scope, &auth_user_from_uid(675)));
+        assert!(!tty_sample.matches(&scope, &auth_user_from_uid(789)));
         assert!(!tty_sample.matches(
             &RecordScope::Tty {
                 tty_device: DeviceId::new(20),
                 session_pid: ProcessId::new(1234),
                 init_time
             },
-            UserId::new(675),
+            &auth_user_from_uid(675),
         ));
         assert!(!tty_sample.matches(
             &RecordScope::Ppid {
                 group_pid: ProcessId::new(42),
                 init_time
             },
-            UserId::new(675),
+            &auth_user_from_uid(675),
         ));
 
         // make sure time is different
@@ -644,7 +656,7 @@ mod tests {
                 session_pid: ProcessId::new(1234),
                 init_time: ProcessCreateTime::new(1, 1)
             },
-            UserId::new(675),
+            &auth_user_from_uid(675),
         ));
     }
 
@@ -721,22 +733,22 @@ mod tests {
             session_pid: ProcessId::new(0),
             init_time: ProcessCreateTime::new(0, 0),
         };
-        let auth_user = UserId::new(2424);
-        let res = srf.create(tty_scope, auth_user).unwrap();
+        let auth_user = auth_user_from_uid(2424);
+        let res = srf.create(tty_scope, &auth_user).unwrap();
         let CreateResult::Created { time } = res else {
             panic!("Expected record to be created");
         };
 
         std::thread::sleep(std::time::Duration::from_millis(1));
-        let second = srf.touch(tty_scope, auth_user).unwrap();
+        let second = srf.touch(tty_scope, &auth_user).unwrap();
         let TouchResult::Updated { old_time, new_time } = second else {
             panic!("Expected record to be updated");
         };
         assert_eq!(time, old_time);
         assert_ne!(old_time, new_time);
 
         std::thread::sleep(std::time::Duration::from_millis(1));
-        let res = srf.create(tty_scope, auth_user).unwrap();
+        let res = srf.create(tty_scope, &auth_user).unwrap();
         let CreateResult::Updated { old_time, new_time } = res else {
             panic!("Expected record to be updated");
         };
diff --git a/test-framework/sudo-compliance-tests/src/sudo/timestamp.rs b/test-framework/sudo-compliance-tests/src/sudo/timestamp.rs
@@ -188,6 +188,43 @@ fn cached_credential_not_shared_with_self_across_ttys() {
     output.assert_exit_code(1);
 }
 
+#[test]
+fn cached_credential_not_shared_between_auth_users() {
+    const PASSWORD: &str = "passw0rd";
+    const PASSWORD2: &str = "notr00t";
+
+    let env = Env(format!(
+        "Defaults targetpw\nDefaults passwd_tries=1\n{USERNAME} ALL=(ALL:ALL) ALL"
+    ))
+    .user(User(USERNAME))
+    .user(User("user1").password(PASSWORD))
+    .user(User("user2").password(PASSWORD2))
+    .build();
+
+    // Enter password for first user to cache credential
+    let output = Command::new("sh")
+        .arg("-c")
+        .arg(format!("echo {PASSWORD} | sudo -S -u user1 true"))
+        .as_user(USERNAME)
+        .output(&env);
+    output.assert_success();
+
+    // Cached credential not used when password for different user necessary
+    let output = Command::new("sh")
+        .arg("-c")
+        .arg("sudo -u user2 true")
+        .as_user(USERNAME)
+        .output(&env);
+    output.assert_exit_code(1);
+
+    let diagnostic = if sudo_test::is_original_sudo() {
+        "a password is required"
+    } else {
+        "incorrect authentication attempt"
+    };
+    assert_contains!(output.stderr(), diagnostic);
+}
+
 #[test]
 fn double_negation_also_equals_never() {
     let env = Env([

Original file line number	Diff line number	Diff line change
`@@ -101,7 +101,7 @@ fn sudo_process() -> Result<(), Error> {`
`101`	`101`	`let user = CurrentUser::resolve()?;`
`102`	`102`	`let mut record_file =`
`103`	`103`	`SessionRecordFile::open_for_user(&user, Duration::seconds(0))?;`
`104`		`- record_file.disable(scope, None)?;`
	`104`	`+ record_file.disable(scope)?;`
`105`	`105`	`}`
`106`	`106`	`Ok(())`
`107`	`107`	`}`