Merge pull request #53 from 34N0/fix-file-permissions

fix: tally file permissions

Cargo.toml

@@ -17,6 +17,7 @@ chrono = "0.4.31"
 clap = { version = "4.4.16", features = ["derive"] }
 cli-xtask = { version = "0.8.0", features = ["main", "lib-crate"] }
 colored = "2.1.0"
+libc = "0.2.152"
 log = "0.4"
 pam-bindings = "0.1.1"
 pam-client = "0.5.0"
@@ -63,4 +64,6 @@ assets = [
 ]
 
 [lints]
-workspace = true
+workspace = true
+
+[dependencies]

crates/lib/Cargo.toml

@@ -16,6 +16,7 @@ doc = false
 
 [dependencies]
 chrono.workspace = true
+libc.workspace = true
 log.workspace = true
 pam-bindings.workspace = true
 toml.workspace = true

crates/lib/src/lib.rs

@@ -104,7 +104,7 @@ impl PamHooks for Pamauthramp {
                 Actions::AUTHSUCC => Err(PamResultCode::PAM_AUTH_ERR),
             }
         })
-        .unwrap_or(PamResultCode::PAM_SUCCESS)
+        .unwrap_or_else(|e| e)
     }
 
     /// Handles the `acct_mgmt` PAM hook, which is invoked during the account management process.

crates/lib/src/tally.rs

@@ -38,6 +38,7 @@
 
 use std::{
     fs,
+    os::unix::fs::{chown, MetadataExt, PermissionsExt},
     path::{Path, PathBuf},
 };
 
@@ -111,7 +112,7 @@ impl Tally {
 
         if tally_file.exists() {
             Self::load_tally_from_file(&mut tally, user, &tally_file, settings)?;
-        } else {
+        } else if settings.action == Some(Actions::AUTHFAIL) {
             Self::create_tally_file(&mut tally, &tally_file, settings)?;
         };
 
@@ -207,8 +208,8 @@ impl Tally {
                 // Write the updated values back to the file
                 let toml_str = format!("[Fails]\ncount = {}", tally.failures_count);
                 std::fs::write(tally_file, toml_str).map_err(|e| {
-                    log_error!("PAM_SYSTEM_ERR: Error resetting tally: {}", e);
-                    PamResultCode::PAM_SYSTEM_ERR
+                    log_error!("PAM_PERM_DENIED: Error resetting tally: {}", e);
+                    PamResultCode::PAM_PERM_DENIED
                 })?;
 
                 // log account unlock
@@ -243,8 +244,8 @@ impl Tally {
                     tally.unlock_instant.unwrap()
                 );
                 std::fs::write(tally_file, toml_str).map_err(|e| {
-                    log_error!("PAM_SYSTEM_ERR: Error writing tally file: {}", e);
-                    PamResultCode::PAM_SYSTEM_ERR
+                    log_error!("PAM_PERM_DENIED: Error writing tally file: {}", e);
+                    PamResultCode::PAM_PERM_DENIED
                 })?;
 
                 if tally.failures_count > settings.config.free_tries {
@@ -276,21 +277,36 @@ impl Tally {
         _settings: &Settings,
     ) -> Result<(), PamResultCode> {
         fs::create_dir_all(tally_file.parent().unwrap()).map_err(|e| {
-            log_error!("PAM_SYSTEM_ERR: Error creating tally file: {}", e);
-            PamResultCode::PAM_SYSTEM_ERR
+            log_error!("PAM_PERM_DENIED: Error creating tally file: {}", e);
+            PamResultCode::PAM_PERM_DENIED
         })?;
 
+        // Write the TOML string to disk
         let toml_str = format!(
             "[Fails]\ncount = {}\ninstant = \"{}\"",
-            tally.failures_count, tally.failure_instant
+            tally.failures_count + 1,
+            tally.failure_instant
         );
 
-        // Write the TOML string to disk
         std::fs::write(tally_file, toml_str).map_err(|e| {
             log_error!("PAM_SYSTEM_ERR: Error writing tally file: {}", e);
             PamResultCode::PAM_SYSTEM_ERR
         })?;
 
+        //  set file permissions
+        fs::set_permissions(tally_file, fs::Permissions::from_mode(0o600))
+            .map_err(|_e| PamResultCode::PAM_SYSTEM_ERR)?;
+
+        // get created tally file meta
+        let tally_file_meta =
+            fs::metadata(tally_file).map_err(|_e| PamResultCode::PAM_SYSTEM_ERR)?;
+
+        // set tally file owner
+        let uid = unsafe { libc::getuid() };
+        if tally_file_meta.uid() != uid {
+            chown(tally_file, Some(uid), Some(uid)).map_err(|_e| PamResultCode::PAM_SYSTEM_ERR)?;
+        }
+
         Ok(())
     }
 }
@@ -363,14 +379,17 @@ mod tests {
 
         // Create settings and call open
         let settings = Settings {
-            user: Some(User::new(9999, "test_user_b", 9999)),
+            user: Some(User::new(1000, "test_user_b", 1000)),
+            action: Some(Actions::AUTHFAIL),
             config,
             ..Default::default()
         };
 
         // Test: Open nonexistent tally file
         let result = Tally::new_from_tally_file(&settings);
 
+        println!("{result:?}");
+
         // Check if the Tally struct is created with default values
         assert!(result.is_ok());
         let tally = result.unwrap();
@@ -381,7 +400,7 @@ mod tests {
         let toml_content = fs::read_to_string(tally_file_path).unwrap();
         // println!("{}", &toml_content);
         assert!(toml_content.contains("[Fails]"));
-        assert!(toml_content.contains("count = 0"));
+        assert!(toml_content.contains("count = 1"));
         assert!(!toml_content.contains("unlock_instant = "));
     }
 

crates/util/src/settings.rs

@@ -110,7 +110,7 @@ impl Settings<'_> {
         settings.action.get_or_insert(Actions::AUTHSUCC);
 
         // get user
-        settings.user = Some(user.ok_or(PamResultCode::PAM_SYSTEM_ERR)?);
+        settings.user = Some(user.ok_or(PamResultCode::PAM_USER_UNKNOWN)?);
 
         // pam hook
         settings.pam_hook = pam_hook;
@@ -183,6 +183,6 @@ mod tests {
         let flags: PamFlag = 0;
         let result = Settings::build(None, &args, flags, "test");
         assert!(result.is_err());
-        assert_eq!(result.unwrap_err(), PamResultCode::PAM_SYSTEM_ERR);
+        assert_eq!(result.unwrap_err(), PamResultCode::PAM_USER_UNKNOWN);
     }
 }

crates/xtask-test-integration/src/main.rs

@@ -1,4 +1,4 @@
-//! # XTask Module
+//! # `XTask` Module
 //!
 //! The `xtask` module provides a set of development tasks and commands for managing
 //! the `pam-authramp` project during development. It utilizes the `xshell` crate for

tests/test-pam-auth.rs

@@ -113,8 +113,9 @@ mod test_pam_auth {
             assert!(tally_file_path.exists(), "Tally file not created");
 
             // Expect tally count
-            let ini_content = fs::read_to_string(tally_file_path).unwrap();
-            assert!(ini_content.contains(&format!("count = {total_tries}")));
+            let toml_content = fs::read_to_string(tally_file_path).unwrap();
+            println!("{toml_content}");
+            assert!(toml_content.contains(&format!("count = {total_tries}")));
         });
     }
 
@@ -132,9 +133,9 @@ mod test_pam_auth {
             assert!(tally_file_path.exists(), "Tally file not created");
 
             // Expect tally count to increase
-            let ini_content = fs::read_to_string(&tally_file_path).unwrap();
+            let toml_content = fs::read_to_string(&tally_file_path).unwrap();
             assert!(
-                ini_content.contains("count = 1"),
+                toml_content.contains("count = 1"),
                 "Expected tally count to increase"
             );
 
@@ -148,9 +149,9 @@ mod test_pam_auth {
                 .expect("Account management failed");
 
             // Expect tally count to decrease
-            let ini_content = fs::read_to_string(&tally_file_path).unwrap();
+            let toml_content = fs::read_to_string(&tally_file_path).unwrap();
             assert!(
-                ini_content.contains("count = 0"),
+                toml_content.contains("count = 0"),
                 "Expected tally count = 0"
             );
         });