diff src/libpam/handle.rs @ 163:a75a66cb4181

Add end-to-end tests; fix issues found by tests. - Create tests and installer/remover shell script - Fix Pointer/pointee problems - Add Debug formatting - Misc cleanup
author Paul Fisher <paul@pfish.zone>
date Mon, 14 Jul 2025 17:40:11 -0400
parents 634cd5f2ac8b
children 2f5913131295
line wrap: on
line diff
--- a/src/libpam/handle.rs	Mon Jul 14 15:07:16 2025 -0400
+++ b/src/libpam/handle.rs	Mon Jul 14 17:40:11 2025 -0400
@@ -18,12 +18,12 @@
 use std::mem::ManuallyDrop;
 use std::os::unix::ffi::OsStrExt;
 use std::ptr::NonNull;
-use std::{fmt, ptr};
+use std::{any, fmt, ptr};
 
 /// An owned PAM handle.
 pub struct LibPamTransaction<C: Conversation> {
-    /// The handle itself.
-    handle: ManuallyDrop<LibPamHandle>,
+    /// The handle itself.  We guarantee this will not be null.
+    handle: *mut LibPamHandle,
     /// The last return value from the handle.
     last_return: Cell<Result<()>>,
     /// If set, the Conversation that this PAM handle owns.
@@ -36,6 +36,16 @@
     conversation: Box<OwnedConversation<C>>,
 }
 
+impl<C: Conversation> fmt::Debug for LibPamTransaction<C> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct(any::type_name::<Self>())
+            .field("handle", &format!("{:p}", self.handle))
+            .field("last_return", &self.last_return.get())
+            .field("conversation", &format!("{:p}", self.conversation))
+            .finish()
+    }
+}
+
 #[derive(Debug, PartialEq)]
 pub struct TransactionBuilder {
     service_name: OsString,
@@ -76,35 +86,34 @@
     }
 
     /// Builds the PAM handle and starts the transaction.
-    pub fn build(self, conv: impl Conversation) -> Result<LibPamTransaction<impl Conversation>> {
+    pub fn build<C: Conversation>(self, conv: C) -> Result<LibPamTransaction<C>> {
         LibPamTransaction::start(self.service_name, self.username, conv)
     }
 }
 
 impl<C: Conversation> LibPamTransaction<C> {
     fn start(service_name: OsString, username: Option<OsString>, conversation: C) -> Result<Self> {
-        let conv = Box::new(OwnedConversation::new(conversation));
+        let mut conv = Box::new(OwnedConversation::new(conversation));
         let service_cstr = CString::new(service_name.as_bytes()).expect("null is forbidden");
         let username_cstr = memory::option_cstr_os(username.as_deref());
         let username_cstr = memory::prompt_ptr(username_cstr.as_deref());
 
         let mut handle: *mut libpam_sys::pam_handle = ptr::null_mut();
+        let conv_ptr: *mut OwnedConversation<_> = conv.as_mut() as _;
         // SAFETY: We've set everything up properly to call `pam_start`.
         // The returned value will be a valid pointer provided the result is OK.
         let result = unsafe {
             libpam_sys::pam_start(
                 service_cstr.as_ptr(),
                 username_cstr,
-                (conv.as_ref() as *const OwnedConversation<C>)
-                    .cast_mut()
-                    .cast(),
+                conv_ptr.cast(),
                 &mut handle,
             )
         };
         ErrorCode::result_from(result)?;
         let handle = NonNull::new(handle).ok_or(ErrorCode::BufferError)?;
         Ok(Self {
-            handle: ManuallyDrop::new(LibPamHandle(handle)),
+            handle: handle.as_ptr().cast(),
             last_return: Cell::new(Ok(())),
             conversation: conv,
         })
@@ -139,14 +148,16 @@
     /// Internal "end" function, which binary-ORs the status with `or_with`.
     fn end_internal(&mut self, or_with: i32) {
         let result = ErrorCode::result_to_c(self.last_return.get()) | or_with;
-        unsafe { libpam_sys::pam_end(self.handle.raw_mut(), result) };
+        unsafe { libpam_sys::pam_end(self.handle.cast(), result) };
     }
 }
 
 macro_rules! wrap {
     (fn $name:ident { $pam_func:ident }) => {
         fn $name(&mut self, flags: Flags) -> Result<()> {
-            ErrorCode::result_from(unsafe { libpam_sys::$pam_func(self.0.as_mut(), flags.bits()) })
+            ErrorCode::result_from(unsafe {
+                libpam_sys::$pam_func((self as *mut Self).cast(), flags.bits())
+            })
         }
     };
 }
@@ -160,7 +171,6 @@
 // TODO: pam_setcred - app
 //       pam_open_session - app
 //       pam_close_session - app
-//       pam_set/get_data - module
 
 impl<C: Conversation> Drop for LibPamTransaction<C> {
     /// Closes the PAM session on an owned PAM handle.
@@ -181,14 +191,14 @@
     // First have the kind that save the result after delegation.
     (fn $meth:ident(&self $(, $param:ident: $typ:ty)*) -> Result<$ret:ty>) => {
         fn $meth(&self $(, $param: $typ)*) -> Result<$ret> {
-            let result = self.handle.$meth($($param),*);
+            let result = unsafe { &*self.handle }.$meth($($param),*);
             self.last_return.set(split(&result));
             result
         }
     };
     (fn $meth:ident(&mut self $(, $param:ident: $typ:ty)*) -> Result<$ret:ty>) => {
         fn $meth(&mut self $(, $param: $typ)*) -> Result<$ret> {
-            let result = self.handle.$meth($($param),*);
+            let result = unsafe { &mut *self.handle }.$meth($($param),*);
             self.last_return.set(split(&result));
             result
         }
@@ -196,12 +206,12 @@
     // Then have the kind that are just raw delegates
     (fn $meth:ident(&self $(, $param:ident: $typ:ty)*) -> $ret:ty) => {
         fn $meth(&self $(, $param: $typ)*) -> $ret {
-            self.handle.$meth($($param),*)
+            unsafe { &*self.handle }.$meth($($param),*)
         }
     };
     (fn $meth:ident(&mut self $(, $param:ident: $typ:ty)*) -> $ret:ty) => {
         fn $meth(&mut self $(, $param: $typ)*) -> $ret {
-            self.handle.$meth($($param),*)
+            unsafe { &mut *self.handle }.$meth($($param),*)
         }
     };
     // Then have item getters / setters
@@ -245,22 +255,9 @@
 /// If [`Self::end`] is not called, this will always call `pam_end` reporting
 /// successful completion.
 #[repr(transparent)]
-pub struct LibPamHandle(NonNull<libpam_sys::pam_handle>);
+pub struct LibPamHandle(libpam_sys::pam_handle);
 
 impl LibPamHandle {
-    /// Takes ownership of the pointer to the given PAM handle.
-    ///
-    /// **Do not use this just to get a reference to a PAM handle.**
-    ///
-    /// # Safety
-    ///
-    /// - The pointer must point to a valid PAM handle.
-    /// - The conversation associated with the handle must remain valid
-    ///   for as long as the handle is open.
-    pub unsafe fn from_ptr(handle: NonNull<libpam_sys::pam_handle>) -> Self {
-        Self(handle)
-    }
-
     /// Ends the transaction, reporting `error_code` to cleanup callbacks.
     ///
     /// # References
@@ -268,9 +265,8 @@
     ///
     #[doc = guide!(adg: "adg-interface-by-app-expected.html#adg-pam_end")]
     #[doc = stdlinks!(3 pam_end)]
-    pub fn end(self, result: Result<()>) {
-        let mut me = ManuallyDrop::new(self);
-        unsafe { libpam_sys::pam_end(me.raw_mut(), ErrorCode::result_to_c(result)) };
+    pub fn end(&mut self, result: Result<()>) {
+        unsafe { libpam_sys::pam_end(self.inner_mut(), ErrorCode::result_to_c(result)) };
     }
 
     #[cfg_attr(
@@ -294,35 +290,22 @@
     ///
     #[doc = guide!(adg: "adg-interface-by-app-expected.html#adg-pam_end")]
     #[doc = stdlinks!(3 pam_end)]
-    pub fn end_silent(self, result: Result<()>) {
-        let mut me = ManuallyDrop::new(self);
+    pub fn end_silent(&mut self, result: Result<()>) {
         let result = ErrorCode::result_to_c(result);
         #[cfg(pam_impl = "LinuxPam")]
         let result = result | libpam_sys::PAM_DATA_SILENT;
         unsafe {
-            libpam_sys::pam_end(me.raw_mut(), result);
+            libpam_sys::pam_end(self.inner_mut(), result);
         }
     }
 
-    /// Consumes this and gives you back the raw PAM handle.
-    pub fn into_inner(self) -> NonNull<libpam_sys::pam_handle> {
-        let me = ManuallyDrop::new(self);
-        me.0
-    }
-
     /// Gets a reference to the inner PAM handle.
-    pub fn raw_ref(&self) -> &libpam_sys::pam_handle {
-        unsafe { self.0.as_ref() }
+    pub fn inner(&self) -> &libpam_sys::pam_handle {
+        &self.0
     }
     /// Gets a mutable reference to the inner PAM handle.
-    pub fn raw_mut(&mut self) -> &mut libpam_sys::pam_handle {
-        unsafe { self.0.as_mut() }
-    }
-}
-
-impl Drop for LibPamHandle {
-    fn drop(&mut self) {
-        unsafe { libpam_sys::pam_end(self.0.as_mut(), 0) };
+    pub fn inner_mut(&mut self) -> &mut libpam_sys::pam_handle {
+        &mut self.0
     }
 }
 
@@ -344,12 +327,7 @@
             // SAFETY: We're calling this function with a known value.
             #[cfg(pam_impl = "LinuxPam")]
             unsafe {
-                libpam_sys::pam_syslog(
-                    self.raw_ref(),
-                    level,
-                    b"%s\0".as_ptr().cast(),
-                    entry.as_ptr(),
-                )
+                libpam_sys::pam_syslog(self.inner(), level, b"%s\0".as_ptr().cast(), entry.as_ptr())
             }
             #[cfg(pam_impl = "Sun")]
             unsafe {
@@ -384,7 +362,7 @@
         let mut output: *const c_char = ptr::null();
         let ret = unsafe {
             libpam_sys::pam_get_user(
-                self.raw_mut(),
+                self.inner_mut(),
                 &mut output,
                 memory::prompt_ptr(prompt.as_deref()),
             )
@@ -440,7 +418,7 @@
         let mut ptr: *const c_void = ptr::null();
         unsafe {
             ErrorCode::result_from(libpam_sys::pam_get_data(
-                self.raw_ref(),
+                self.inner(),
                 full_key.as_ptr(),
                 &mut ptr,
             ))
@@ -455,7 +433,7 @@
         let data = Box::new(data);
         ErrorCode::result_from(unsafe {
             libpam_sys::pam_set_data(
-                self.raw_mut(),
+                self.inner_mut(),
                 full_key.as_ptr(),
                 Box::into_raw(data).cast(),
                 drop_module_data::<T>,
@@ -504,7 +482,7 @@
         // SAFETY: We're calling this with known-good values.
         let res = unsafe {
             libpam_sys::pam_get_authtok(
-                self.raw_mut(),
+                self.inner_mut(),
                 item_type.into(),
                 &mut output,
                 memory::prompt_ptr(prompt.as_deref()),
@@ -525,7 +503,7 @@
         let mut output: *mut c_char = ptr::null_mut();
         let result = unsafe {
             libpam_sys::__pam_get_authtok(
-                self.raw_mut(),
+                self.inner_mut(),
                 libpam_sys::PAM_HANDLE,
                 item_type.into(),
                 ptr::null(),
@@ -543,7 +521,7 @@
         let prompt = memory::option_cstr_os(prompt);
         let result = unsafe {
             libpam_sys::__pam_get_authtok(
-                self.raw_mut(),
+                self.inner_mut(),
                 libpam_sys::PAM_PROMPT,
                 item_type.into(),
                 memory::prompt_ptr(prompt.as_deref()),
@@ -559,15 +537,12 @@
 
     /// Gets the `PAM_CONV` item from the handle.
     fn conversation_item(&self) -> Result<&PamConv> {
-        let output: *const PamConv = ptr::null_mut();
+        let mut output: *const c_void = ptr::null();
         let result = unsafe {
-            libpam_sys::pam_get_item(
-                self.raw_ref(),
-                ItemType::Conversation.into(),
-                &mut output.cast(),
-            )
+            libpam_sys::pam_get_item(self.inner(), ItemType::Conversation.into(), &mut output)
         };
         ErrorCode::result_from(result)?;
+        let output: *const PamConv = output.cast();
         // SAFETY: We got this result from PAM, and we're checking if it's null.
         unsafe { output.as_ref() }.ok_or(ErrorCode::ConversationError)
     }