Fix: FFI return code handling

pam/Cargo.toml

@@ -36,3 +36,7 @@ crate-type = ["cdylib"]
 [[example]]
 name = "quiz"
 crate-type = ["cdylib"]
+
+[[example]]
+name = "data"
+crate-type = ["cdylib"]

pam/examples/data.rs

@@ -0,0 +1,33 @@
+use pam::constants::{PamFlag, PamResultCode};
+use pam::module::{PamHandle, PamHooks};
+use pam::pam_hooks;
+use std::ffi::CStr;
+
+struct Data;
+pam_hooks!(Data);
+
+impl PamHooks for Data {
+    fn sm_authenticate(pamh: &mut PamHandle, _args: Vec<&CStr>, _flags: PamFlag) -> PamResultCode {
+        // Store a value on the handle
+        if let Err(e) = pamh.set_data("greeting", Box::new(String::from("hello"))) {
+            eprintln!("set_data failed, error code: {e:?}");
+            return e;
+        }
+
+        // Read the data back
+        let greeting = match unsafe { pamh.get_data::<String>("greeting") } {
+            Ok(greeting) => greeting,
+            Err(e) => {
+                eprintln!("get_data failed, error code: {e:?}");
+                return e;
+            }
+        };
+
+        if greeting == "hello" {
+            eprintln!("data: {greeting}");
+            PamResultCode::PAM_SUCCESS
+        } else {
+            PamResultCode::PAM_AUTH_ERR
+        }
+    }
+}

pam/src/constants.rs

@@ -64,3 +64,55 @@ pub enum PamResultCode {
     PAM_CONV_AGAIN = 30,
     PAM_INCOMPLETE = 31,
 }
+
+impl TryFrom<c_int> for PamResultCode {
+    /// The original value is returned when it does not name a known result code.
+    type Error = c_int;
+
+    /// Map a raw [`c_int`] to a [`PamResultCode`].
+    fn try_from(value: c_int) -> Result<Self, Self::Error> {
+        Ok(match value {
+            0 => Self::PAM_SUCCESS,
+            1 => Self::PAM_OPEN_ERR,
+            2 => Self::PAM_SYMBOL_ERR,
+            3 => Self::PAM_SERVICE_ERR,
+            4 => Self::PAM_SYSTEM_ERR,
+            5 => Self::PAM_BUF_ERR,
+            6 => Self::PAM_PERM_DENIED,
+            7 => Self::PAM_AUTH_ERR,
+            8 => Self::PAM_CRED_INSUFFICIENT,
+            9 => Self::PAM_AUTHINFO_UNAVAIL,
+            10 => Self::PAM_USER_UNKNOWN,
+            11 => Self::PAM_MAXTRIES,
+            12 => Self::PAM_NEW_AUTHTOK_REQD,
+            13 => Self::PAM_ACCT_EXPIRED,
+            14 => Self::PAM_SESSION_ERR,
+            15 => Self::PAM_CRED_UNAVAIL,
+            16 => Self::PAM_CRED_EXPIRED,
+            17 => Self::PAM_CRED_ERR,
+            18 => Self::PAM_NO_MODULE_DATA,
+            19 => Self::PAM_CONV_ERR,
+            20 => Self::PAM_AUTHTOK_ERR,
+            21 => Self::PAM_AUTHTOK_RECOVERY_ERR,
+            22 => Self::PAM_AUTHTOK_LOCK_BUSY,
+            23 => Self::PAM_AUTHTOK_DISABLE_AGING,
+            24 => Self::PAM_TRY_AGAIN,
+            25 => Self::PAM_IGNORE,
+            26 => Self::PAM_ABORT,
+            27 => Self::PAM_AUTHTOK_EXPIRED,
+            28 => Self::PAM_MODULE_UNKNOWN,
+            29 => Self::PAM_BAD_ITEM,
+            30 => Self::PAM_CONV_AGAIN,
+            31 => Self::PAM_INCOMPLETE,
+            other => return Err(other),
+        })
+    }
+}
+
+impl PamResultCode {
+    /// Map a [`c_int`] to a [`PamResultCode`].
+    /// Unknown values are mapped to [`PamResultCode::PAM_SYSTEM_ERR`].
+    pub(crate) fn from_raw(value: c_int) -> Self {
+        Self::try_from(value).unwrap_or(Self::PAM_SYSTEM_ERR)
+    }
+}

pam/src/conv.rs

@@ -31,7 +31,7 @@ pub struct Inner {
         pam_message: &&PamMessage,
         pam_response: &mut *const PamResponse,
         appdata_ptr: *const libc::c_void,
-    ) -> PamResultCode,
+    ) -> c_int,
     appdata_ptr: *const libc::c_void,
 }
 
@@ -66,7 +66,8 @@ impl Conv<'_> {
             msg: msg_cstr.as_ptr(),
         };
 
-        let ret = (self.0.conv)(1, &&msg, &mut resp_ptr, self.0.appdata_ptr);
+        let ret =
+            PamResultCode::from_raw((self.0.conv)(1, &&msg, &mut resp_ptr, self.0.appdata_ptr));
         if PamResultCode::PAM_SUCCESS != ret {
             return Err(ret);
         }

pam/src/module.rs

@@ -1,6 +1,6 @@
 //! Functions for use in pam modules.
 
-use libc::c_char;
+use libc::{c_char, c_int};
 use std::ffi::{CStr, CString};
 use std::marker::PhantomData;
 
@@ -30,39 +30,39 @@ unsafe extern "C" {
         pamh: *const PamHandle,
         module_data_name: *const c_char,
         data: &mut *const libc::c_void,
-    ) -> PamResultCode;
+    ) -> c_int;
 
     fn pam_set_data(
-        pamh: *const PamHandle,
+        pamh: *mut PamHandle,
         module_data_name: *const c_char,
         data: *mut libc::c_void,
         cleanup: extern "C" fn(
             pamh: *const PamHandle,
             data: *mut libc::c_void,
-            error_status: PamResultCode,
+            error_status: c_int,
         ),
-    ) -> PamResultCode;
+    ) -> c_int;
 
     fn pam_get_item(
         pamh: *const PamHandle,
         item_type: crate::items::ItemType,
         item: &mut *const libc::c_void,
-    ) -> PamResultCode;
+    ) -> c_int;
 
     fn pam_set_item(
         pamh: *mut PamHandle,
         item_type: crate::items::ItemType,
         item: *const libc::c_void,
-    ) -> PamResultCode;
+    ) -> c_int;
 
     fn pam_get_user(
         pamh: *const PamHandle,
         user: &mut *const c_char,
         prompt: *const c_char,
-    ) -> PamResultCode;
+    ) -> c_int;
 }
 
-pub extern "C" fn cleanup<T>(_: *const PamHandle, c_data: *mut libc::c_void, _: PamResultCode) {
+extern "C" fn cleanup<T>(_: *const PamHandle, c_data: *mut libc::c_void, _: c_int) {
     unsafe {
         let _data: Box<T> = Box::from_raw(c_data.cast::<T>());
     }
@@ -89,7 +89,7 @@ impl PamHandle {
     pub unsafe fn get_data<'a, T>(&'a self, key: &str) -> PamResult<&'a T> {
         let c_key = CString::new(key).map_err(|_| PamResultCode::PAM_BUF_ERR)?;
         let mut ptr: *const libc::c_void = std::ptr::null();
-        let res = unsafe { pam_get_data(self, c_key.as_ptr(), &mut ptr) };
+        let res = PamResultCode::from_raw(unsafe { pam_get_data(self, c_key.as_ptr(), &mut ptr) });
         if PamResultCode::PAM_SUCCESS == res && !ptr.is_null() {
             let typed_ptr = ptr.cast::<T>();
             let data: &T = unsafe { &*typed_ptr };
@@ -109,19 +109,21 @@ impl PamHandle {
     ///
     /// - [`PamResultCode`] if the store itself fails.
     /// - [`PamResultCode::PAM_BUF_ERR`] if the key string contains a 0 byte.
-    pub fn set_data<T>(&self, key: &str, data: Box<T>) -> PamResult<()> {
+    pub fn set_data<T: 'static>(&mut self, key: &str, data: Box<T>) -> PamResult<()> {
         let c_key = CString::new(key).map_err(|_| PamResultCode::PAM_BUF_ERR)?;
-        let res = unsafe {
+        let ptr = Box::into_raw(data);
+        let res = PamResultCode::from_raw(unsafe {
             pam_set_data(
                 self,
                 c_key.as_ptr(),
-                Box::into_raw(data).cast::<libc::c_void>(),
+                ptr.cast::<libc::c_void>(),
                 cleanup::<T>,
             )
-        };
+        });
         if PamResultCode::PAM_SUCCESS == res {
             Ok(())
         } else {
+            drop(unsafe { Box::from_raw(ptr) });
             Err(res)
         }
     }
@@ -137,7 +139,7 @@ impl PamHandle {
     /// Returns an error if the underlying PAM function call fails.
     pub fn get_item<'a, T: crate::items::Item<'a>>(&'a self) -> PamResult<Option<T>> {
         let mut ptr: *const libc::c_void = std::ptr::null();
-        let res = unsafe { pam_get_item(self, T::type_id(), &mut ptr) };
+        let res = PamResultCode::from_raw(unsafe { pam_get_item(self, T::type_id(), &mut ptr) });
         if PamResultCode::PAM_SUCCESS != res {
             return Err(res);
         }
@@ -161,8 +163,9 @@ impl PamHandle {
     ///
     /// Returns an error if the underlying PAM function call fails.
     pub fn set_item_str<'a, T: crate::items::Item<'a>>(&mut self, item: T) -> PamResult<()> {
-        let res =
-            unsafe { pam_set_item(self, T::type_id(), item.into_raw().cast::<libc::c_void>()) };
+        let res = PamResultCode::from_raw(unsafe {
+            pam_set_item(self, T::type_id(), item.into_raw().cast::<libc::c_void>())
+        });
         if PamResultCode::PAM_SUCCESS == res {
             Ok(())
         } else {
@@ -190,7 +193,7 @@ impl PamHandle {
         let c_prompt = prompt_string
             .as_ref()
             .map_or(std::ptr::null(), |s| s.as_ptr());
-        let res = unsafe { pam_get_user(self, &mut ptr, c_prompt) };
+        let res = PamResultCode::from_raw(unsafe { pam_get_user(self, &mut ptr, c_prompt) });
         if PamResultCode::PAM_SUCCESS == res && !ptr.is_null() {
             let bytes = unsafe { CStr::from_ptr(ptr).to_bytes() };
             String::from_utf8(bytes.to_vec()).map_err(|_| PamResultCode::PAM_CONV_ERR)

pam/tests/integration/main.rs

@@ -1,5 +1,6 @@
 mod harness;
 
+mod test_data;
 mod test_quiz;
 mod test_trivial;
 mod test_username;

pam/tests/integration/test_data.rs

@@ -0,0 +1,18 @@
+use crate::harness::pamtester;
+
+#[test]
+fn test_data_example_module() {
+    let output = pamtester("data", &["auth required"], None, "authenticate", &[]);
+
+    let expected_stdout = "pamtester: successfully authenticated\n";
+    let expected_stderr = "data: hello\n";
+    let actual_stdout = String::from_utf8_lossy(&output.stdout);
+    let actual_stderr = String::from_utf8_lossy(&output.stderr);
+
+    assert!(
+        output.status.success(),
+        "stdout: {actual_stdout} stderr: {actual_stderr}"
+    );
+    assert_eq!(expected_stdout, actual_stdout);
+    assert_eq!(expected_stderr, actual_stderr);
+}