feat: -K for deleting timestamps

fix: finally, it was a real bug

Author: LeChatP

.github/workflows/tests.yml

@@ -25,6 +25,19 @@ jobs:
     - name: Install sudo
       run: apt install sudo -y
 
+    - name: Configure PAM
+      run: |
+        sudo bash -c 'echo "#%PAM-1.0
+        auth    [success=1 default=ignore]  pam_permit.so
+        auth    requisite                   pam_permit.so
+        auth    required                    pam_permit.so
+        account [success=1 default=ignore]  pam_permit.so
+        account requisite                   pam_permit.so
+        account required                    pam_permit.so
+        session [success=1 default=ignore]  pam_permit.so
+        session requisite                   pam_permit.so
+        session required                    pam_permit.so" | tee /etc/pam.d/dosr'
+
     - name: Install Dependencies
       run: cargo xtask dependencies -dip sudo
 

src/sr/finder/cmd.rs

@@ -27,17 +27,20 @@ fn match_path(
         return min;
     } else {
         debug!("match_path: user relative path");
+        let mut curmin = CmdMin::empty();
         all_paths_from_env(env_path, user_path)
             .iter()
             .find_map(|cmd_path| {
                 let min = match_single_path(cmd_path, role_path);
-                if min.better(&previous_min) {
+                if min.better(&previous_min) && min.better(&curmin) {
                     *final_path = Some(cmd_path.clone());
+                    curmin = min;
                     Some(min)
                 } else {
                     None
                 }
             })
+            .inspect(|m| debug!("match_path: found better match {:?} with {}", m, final_path.as_ref().unwrap().display()))
             .unwrap_or_default()
     }
 }

src/sr/main.rs

@@ -108,6 +108,10 @@ struct Cli {
     #[builder(default, with = || true)]
     /// Use stdin for password prompt
     stdin: bool,
+
+    #[builder(default, with = || false)]
+    /// Delete timestamp cookie after successful authentication
+    del_ts: bool,
 }
 
 impl Default for Cli {
@@ -169,6 +173,9 @@ where
             "-E" | "--preserve-env" => {
                 env.replace(EnvBehavior::Keep);
             }
+            "-K" | "--remove-timestamp" => {
+                args.del_ts = true;
+            }
             "-p" | "--prompt" => {
                 args.prompt = Some(
                     iter.next()
@@ -280,6 +287,12 @@ fn main_inner() -> SrResult<()> {
         return Ok(());
     }
     let user = make_cred();
+    if args.del_ts {
+        timeout::clear_cookies(&user).map_err(|e| {
+            error!("Failed to clear timestamp cookies: {}", e);
+            SrError::InsufficientPrivileges
+        })?;
+    }
     let execcfg = find_best_exec_settings(
         &args,
         &user,

src/sr/pam/mod.rs

@@ -25,11 +25,7 @@ mod rpassword;
 #[allow(dead_code, reason = "This file is part of sudo-rs.")]
 mod securemem;
 
-#[cfg(not(test))]
 const PAM_SERVICE: &str = "dosr";
-#[cfg(test)]
-const PAM_SERVICE: &str = "dosr_test";
-
 pub(crate) const PAM_PROMPT: &str = "Password: ";
 
 struct SrConversationHandler {

src/sr/timeout.rs

@@ -282,6 +282,14 @@ pub(crate) fn update_cookie(
     Ok(())
 }
 
+pub(crate) fn clear_cookies(user: &Cred) -> Result<(), Box<dyn Error>> {
+    let path = Path::new(TS_LOCATION).join(user.user.uid.as_raw().to_string());
+    if path.exists() {
+        remove_with_privileges(&path)?;
+    }
+    Ok(())
+}
+
 #[cfg(test)]
 mod test {
     use nix::unistd::{Pid, User};
@@ -307,6 +315,7 @@ mod test {
             tty: None,
             ppid: Pid::parent(),
         };
+        clear_cookies(&cred).unwrap();
         let constraint = STimeout {
             type_field: Some(TimestampType::TTY),
             duration: Some(chrono::Duration::seconds(10)),

tests/fixtures/perform_auth.json

@@ -0,0 +1,39 @@
+{
+    "version": "3.2.0",
+    "options": {
+        "authentication": "perform"
+    },
+    "roles": [
+        {
+            "name": "r_auth",
+            "actors": [
+                {
+                    "type": "user",
+                    "name": "root"
+                }
+            ],
+            "tasks": [
+                {
+                    "name": "t_auth",
+                    "commands": [
+                        "/usr/bin/true"
+                    ],
+                    "options": {
+                        "env": {
+                            "set": {
+                                "TASK": "t_auth"
+                            }
+                        }
+                    }
+                }
+            ],
+            "options": {
+                "env": {
+                    "set": {
+                        "ROLE": "r_auth"
+                    }
+                }
+            }
+        }
+    ]
+}

tests/helpers/test_runner.rs

@@ -3,6 +3,7 @@ use std::process::{Command, ExitStatus, Stdio};
 use std::io::Result as IoResult;
 
 use bon::bon;
+use log::warn;
 
 use crate::helpers::config_manager::ConfigManager;
 
@@ -58,25 +59,26 @@ impl TestRunner {
             for &user in user_list {
                 let user_check = Command::new("id")
                     .arg(user)
-                    .stdout(Stdio::null())
-                    .stderr(Stdio::null())
                     .status();
                 match user_check {
-                    Ok(_) => {}
-                    Err(e) => {
+                    Ok(e) => {
                         //check if error is due to user not existing
-                        if e.kind() != std::io::ErrorKind::NotFound {
-                            eprintln!("Warning: Failed to check user '{}': {}", user, e);
-                            continue;
-                        }
-                        // User does not exist, attempt to create
-                        let create_status = Command::new("sudo")
-                            .args(&["useradd", "-m", user])
-                            .status();
-                        if let Err(e) = create_status {
-                            eprintln!("Warning: Failed to create user '{}': {}", user, e);
+                        if !e.success() {
+                            // User does not exist, attempt to create
+                            let create_status = Command::new("useradd")
+                                .args(&["-m", user])
+                                .status();
+                            if let Err(e) = create_status {
+                                println!("Warning: Failed to create user '{}': {}", user, e);
+                            }
+                            println!("Created user '{}' for testing purposes", user);
+                            added_users.push(user.to_string());
                         }
-                        added_users.push(user.to_string());
+                        println!("User '{}' exists", user);
+                        
+                    }
+                    Err(e) => {
+                        println!("Warning: Failed to check user '{}': {}", user, e);
                     }
                 }
             }
@@ -86,25 +88,23 @@ impl TestRunner {
             for &group in group_list {
                 let group_check = Command::new("getent")
                     .args(&["group", group])
-                    .stdout(Stdio::null())
-                    .stderr(Stdio::null())
                     .status();
                 match group_check {
-                    Ok(_) => {}
-                    Err(e) => {
-                        //check if error is due to group not existing
-                        if e.kind() != std::io::ErrorKind::NotFound {
-                            eprintln!("Warning: Failed to check group '{}': {}", group, e);
-                            continue;
-                        }
-                        // Group does not exist, attempt to create
-                        let create_status = Command::new("sudo")
-                            .args(&["groupadd", group])
-                            .status();
-                        if let Err(e) = create_status {
-                            eprintln!("Warning: Failed to create group '{}': {}", group, e);
+                    Ok(e) => {
+                        if !e.success() {
+                            // Group does not exist, attempt to create
+                            let create_status = Command::new("groupadd")
+                                .args(&[group])
+                                .status();
+                            if let Err(e) = create_status {
+                                println!("Warning: Failed to create group '{}': {}", group, e);
+                            }
+                            added_groups.push(group.to_string());
+                            println!("Created group '{}' for testing purposes", group);
                         }
-                        added_groups.push(group.to_string());
+                    }
+                    Err(e) => {
+                        println!("Warning: Failed to check group '{}': {}", group, e);
                     }
                 }
             }
@@ -117,17 +117,19 @@ impl TestRunner {
             .stderr(Stdio::piped());
 
         let output = command.output()?;
+        println!("Output : {}", String::from_utf8(output.stdout.clone()).unwrap());
+        println!("Error  : {}", String::from_utf8(output.stderr.clone()).unwrap());
 
         // Clean up any users or groups we added
         for user in added_users {
-            let _ = Command::new("sudo")
-                .args(&["userdel", "-r", &user])
-                .status();
+            let _ = Command::new("userdel")
+                .args(&["-r", &user])
+                .status()?;
         }
         for group in added_groups {
-            let _ = Command::new("sudo")
-                .args(&["groupdel", &group])
-                .status();
+            let _ = Command::new("groupdel")
+                .args(&[&group])
+                .status()?;
         }
 
         Ok(CommandResult {

tests/integration_tests.rs

@@ -187,9 +187,13 @@ mod tests {
             .groups(&["nobody"])
             .call()
             .expect("Failed to run dosr -u nobody id");
-
+        if !result.success {
+            eprintln!("stderr: {}", result.stderr);
+            println!("stdout: {}", result.stdout);
+        }
         assert!(result.success, "Command failed: {}", result.stderr);
-        assert!(result.stdout.contains("gid=65534(nobody)"));
+        let re_gid = RegexBuilder::new().build(r"gid=\d+\(nobody\)").unwrap();
+        assert!(re_gid.is_match(&result.stdout.as_bytes()).is_ok_and(|b| b));
         assert_eq!(result.exit_code, 0);
     }
 
@@ -204,12 +208,47 @@ mod tests {
             .fixture_name("tests/fixtures/user_group.json")
             .env_vars(&[("LANG","en_US")])
             .call()
-            .expect("Failed to run dosr -u nobody -g daemon,nobody id");
+            .inspect_err(|e| eprintln!("Failed to run dosr -u nobody -g daemon,nobody id: {}",e)).unwrap();
+        if !result.success {
+            eprintln!("stderr: {}", result.stderr);
+            println!("stdout: {}", result.stdout);
+        }
         assert!(result.success, "Command failed: {}", result.stderr);
         let re_gid = RegexBuilder::new().build(r"gid=\d+\(daemon\)").unwrap();
-        let re_groups = RegexBuilder::new().build(r"groups=\d+\(daemon\),65534\(nobody\)").unwrap();
+        let re_groups = RegexBuilder::new().build(r"groups=\d+\(daemon\),\d+\(nobody\)").unwrap();
         assert!(re_gid.is_match(&result.stdout.as_bytes()).is_ok_and(|b| b), "stdout: {}", result.stdout);
         assert!(re_groups.is_match(&result.stdout.as_bytes()).is_ok_and(|b| b), "stdout: {}", result.stdout);
         assert_eq!(result.exit_code, 0);
     }
+
+    #[test]
+    #[serial]
+    fn test_dosr_auth() {
+        // check that the /etc/pam.d/dosr_test file exists
+        use std::fs;
+        if !fs::metadata("/etc/pam.d/dosr_test").is_ok() {
+            eprintln!("Skipping test_dosr_auth: /etc/pam.d/dosr_test not found");
+            return;
+        }
+        let runner = get_test_runner().expect("Failed to setup test environment");
+        let result = runner
+            .run_dosr(&["/usr/bin/true"])
+            .fixture_name("tests/fixtures/perform_auth.json")
+            .call()
+            .expect("Failed to run dosr with auth role");
+        assert!(result.success, "Command unexpectedly failed: {}", result.stderr);
+        assert_eq!(result.exit_code, 0);
+        // assert that a timestamp cookie was created
+        let path = std::path::Path::new("/var/run/sr/ts").join("0");
+        assert!(path.exists(), "Timestamp cookie was not created");
+        // run dosr -K to delete the timestamp cookie
+        let result = runner
+            .run_dosr(&["-K","/usr/bin/true"])
+            .fixture_name("tests/fixtures/perform_auth.json")
+            .call()
+            .expect("Failed to run dosr with auth role");
+        assert!(result.success, "Command unexpectedly failed: {}", result.stderr);
+        assert_eq!(result.exit_code, 0);
+        assert!(!path.exists(), "Timestamp cookie was not deleted");
+    }
 }