diff src/pam_ffi.rs @ 70:9f8381a1c09c

Implement low-level conversation primitives. This change does two primary things: 1. Introduces new Conversation traits, to be implemented both by the library and by PAM client applications. 2. Builds the memory-management infrastructure for passing messages through the conversation. ...and it adds tests for both of the above, including ASAN tests.
author Paul Fisher <paul@pfish.zone>
date Tue, 03 Jun 2025 01:21:59 -0400
parents 8f3ae0c7ab92
children
line wrap: on
line diff
--- a/src/pam_ffi.rs	Sun Jun 01 01:15:04 2025 -0400
+++ b/src/pam_ffi.rs	Tue Jun 03 01:21:59 2025 -0400
@@ -9,14 +9,12 @@
 // Temporarily allow dead code.
 #![allow(dead_code)]
 
-use crate::constants::InvalidEnum;
+use crate::constants::{InvalidEnum, NulError, TooBigError};
 use num_derive::FromPrimitive;
 use num_traits::FromPrimitive;
 use std::ffi::{c_char, c_int, c_void, CStr};
 use std::marker::{PhantomData, PhantomPinned};
-use std::num::TryFromIntError;
 use std::slice;
-use thiserror::Error;
 
 /// Makes whatever it's in not [`Send`], [`Sync`], or [`Unpin`].
 type Immovable = PhantomData<(*mut u8, PhantomPinned)>;
@@ -40,10 +38,12 @@
     ErrorMsg = 3,
     /// An informational message.
     TextInfo = 4,
-    /// Yes/No/Maybe conditionals. Linux-PAM specific.
+    /// Yes/No/Maybe conditionals. A Linux-PAM extension.
     RadioType = 5,
     /// For server–client non-human interaction.
+    ///
     /// NOT part of the X/Open PAM specification.
+    /// A Linux-PAM extension.
     BinaryPrompt = 7,
 }
 
@@ -68,20 +68,18 @@
     /// The style of message to request.
     style: c_int,
     /// A description of the data requested.
+    ///
     /// For most requests, this will be an owned [`CStr`], but for requests
-    /// with [`MessageStyle::BinaryPrompt`], this will be [`BinaryData`].
+    /// with [`MessageStyle::BinaryPrompt`], this will be [`BinaryData`]
+    /// (a Linux-PAM extension).
     data: *const c_void,
 }
 
-/// Returned when text that should not have any `\0` bytes in it does.
-/// Analogous to [`std::ffi::NulError`], but the data it was created from
-/// is borrowed.
-#[derive(Debug, Error)]
-#[error("null byte within input at byte {0}")]
-pub struct NulError(usize);
-
-#[repr(transparent)]
-pub struct TextResponseInner(ResponseInner);
+#[repr(C)]
+pub struct TextResponseInner {
+    data: *mut c_char,
+    _unused: c_int,
+}
 
 impl TextResponseInner {
     /// Allocates a new text response on the C heap.
@@ -91,7 +89,7 @@
     /// on the pointer you get back when you're done with it.
     pub fn alloc(text: impl AsRef<str>) -> Result<*mut Self, NulError> {
         let str_data = Self::malloc_str(text)?;
-        let inner = ResponseInner::alloc(str_data);
+        let inner = GenericResponse::alloc(str_data);
         Ok(inner as *mut Self)
     }
 
@@ -100,7 +98,7 @@
         // SAFETY: This data is either passed from PAM (so we are forced to
         // trust it) or was created by us in TextResponseInner::alloc.
         // In either case, it's going to be a valid null-terminated string.
-        unsafe { CStr::from_ptr(self.0.data as *const c_char) }
+        unsafe { CStr::from_ptr(self.data) }
     }
 
     /// Releases memory owned by this response.
@@ -109,7 +107,14 @@
     ///
     /// You are responsible for no longer using this after calling free.
     pub unsafe fn free(me: *mut Self) {
-        ResponseInner::free(me as *mut ResponseInner)
+        if !me.is_null() {
+            let data = (*me).data;
+            if !data.is_null() {
+                libc::memset(data as *mut c_void, 0, libc::strlen(data));
+            }
+            libc::free(data as *mut c_void);
+        }
+        libc::free(me as *mut c_void);
     }
 
     /// Allocates a string with the given contents on the C heap.
@@ -118,7 +123,7 @@
     ///
     /// - it allocates data on the C heap with [`libc::malloc`].
     /// - it doesn't take ownership of the data passed in.
-    fn malloc_str(text: impl AsRef<str>) -> Result<*const c_void, NulError> {
+    fn malloc_str(text: impl AsRef<str>) -> Result<*mut c_void, NulError> {
         let data = text.as_ref().as_bytes();
         if let Some(nul) = data.iter().position(|x| *x == 0) {
             return Err(NulError(nul));
@@ -126,14 +131,17 @@
         unsafe {
             let data_alloc = libc::calloc(data.len() + 1, 1);
             libc::memcpy(data_alloc, data.as_ptr() as *const c_void, data.len());
-            Ok(data_alloc as *const c_void)
+            Ok(data_alloc)
         }
     }
 }
 
-/// A [`ResponseInner`] with [`BinaryData`] in it.
-#[repr(transparent)]
-pub struct BinaryResponseInner(ResponseInner);
+/// A [`GenericResponse`] with [`BinaryData`] in it.
+#[repr(C)]
+pub struct BinaryResponseInner {
+    data: *mut BinaryData,
+    _unused: c_int,
+}
 
 impl BinaryResponseInner {
     /// Allocates a new binary response on the C heap.
@@ -144,17 +152,15 @@
     /// The referenced data is copied to the C heap. We do not take ownership.
     /// You are responsible for calling [`free`](Self::free)
     /// on the pointer you get back when you're done with it.
-    pub fn alloc(data: impl AsRef<[u8]>, data_type: u8) -> Result<*mut Self, TryFromIntError> {
+    pub fn alloc(data: &[u8], data_type: u8) -> Result<*mut Self, TooBigError> {
         let bin_data = BinaryData::alloc(data, data_type)?;
-        let inner = ResponseInner::alloc(bin_data as *const c_void);
+        let inner = GenericResponse::alloc(bin_data as *mut c_void);
         Ok(inner as *mut Self)
     }
 
     /// Gets the binary data in this response.
     pub fn contents(&self) -> &[u8] {
-        let data = self.data();
-        let length = (u32::from_be_bytes(data.total_length) - 5) as usize;
-        unsafe { slice::from_raw_parts(data.data.as_ptr(), length) }
+        self.data().contents()
     }
 
     /// Gets the `data_type` tag that was embedded with the message.
@@ -167,7 +173,7 @@
         // SAFETY: This was either something we got from PAM (in which case
         // we trust it), or something that was created with
         // BinaryResponseInner::alloc. In both cases, it points to valid data.
-        unsafe { &*(self.0.data as *const BinaryData) }
+        unsafe { &*(self.data) }
     }
 
     /// Releases memory owned by this response.
@@ -176,38 +182,9 @@
     ///
     /// You are responsible for not using this after calling free.
     pub unsafe fn free(me: *mut Self) {
-        ResponseInner::free(me as *mut ResponseInner)
-    }
-}
-
-#[repr(C)]
-pub struct ResponseInner {
-    /// Pointer to the data returned in a response.
-    /// For most responses, this will be a [`CStr`], but for responses to
-    /// [`MessageStyle::BinaryPrompt`]s, this will be [`BinaryData`]
-    data: *const c_void,
-    /// Unused.
-    return_code: c_int,
-}
-
-impl ResponseInner {
-    /// Allocates a response on the C heap pointing to the given data.
-    fn alloc(data: *const c_void) -> *mut Self {
-        unsafe {
-            let alloc = libc::calloc(1, size_of::<ResponseInner>()) as *mut ResponseInner;
-            (*alloc).data = data;
-            alloc
+        if !me.is_null() {
+            BinaryData::free((*me).data);
         }
-    }
-
-    /// Frees one of these that was created with [`Self::alloc`]
-    /// (or provided by PAM).
-    ///
-    /// # Safety
-    ///
-    /// It's up to you to stop using `me` after calling this.
-    unsafe fn free(me: *mut Self) {
-        libc::free((*me).data as *mut c_void);
         libc::free(me as *mut c_void)
     }
 }
@@ -216,6 +193,8 @@
 ///
 /// This is an unsized data type whose memory goes beyond its data.
 /// This must be allocated on the C heap.
+///
+/// A Linux-PAM extension.
 #[repr(C)]
 struct BinaryData {
     /// The total length of the structure; a u32 in network byte order (BE).
@@ -228,12 +207,12 @@
 }
 
 impl BinaryData {
-    fn alloc(
-        source: impl AsRef<[u8]>,
-        data_type: u8,
-    ) -> Result<*const BinaryData, TryFromIntError> {
-        let source = source.as_ref();
-        let buffer_size = u32::try_from(source.len() + 5)?;
+    /// Copies the given data to a new BinaryData on the heap.
+    fn alloc(source: &[u8], data_type: u8) -> Result<*mut BinaryData, TooBigError> {
+        let buffer_size = u32::try_from(source.len() + 5).map_err(|_| TooBigError {
+            max: (u32::MAX - 5) as usize,
+            actual: source.len(),
+        })?;
         let data = unsafe {
             let dest_buffer = libc::malloc(buffer_size as usize) as *mut BinaryData;
             let data = &mut *dest_buffer;
@@ -249,6 +228,69 @@
         };
         Ok(data)
     }
+
+    fn length(&self) -> usize {
+        u32::from_be_bytes(self.total_length).saturating_sub(5) as usize
+    }
+
+    fn contents(&self) -> &[u8] {
+        unsafe { slice::from_raw_parts(self.data.as_ptr(), self.length()) }
+    }
+
+    /// Clears this data and frees it.
+    fn free(me: *mut Self) {
+        if me.is_null() {
+            return;
+        }
+        unsafe {
+            let me_too = &mut *me;
+            let contents = slice::from_raw_parts_mut(me_too.data.as_mut_ptr(), me_too.length());
+            for v in contents {
+                *v = 0
+            }
+            me_too.data_type = 0;
+            me_too.total_length = [0; 4];
+            libc::free(me as *mut c_void);
+        }
+    }
+}
+
+/// Generic version of response data.
+///
+/// This has the same structure as [`BinaryResponseInner`]
+/// and [`TextResponseInner`].
+#[repr(C)]
+pub struct GenericResponse {
+    /// Pointer to the data returned in a response.
+    /// For most responses, this will be a [`CStr`], but for responses to
+    /// [`MessageStyle::BinaryPrompt`]s, this will be [`BinaryData`]
+    /// (a Linux-PAM extension).
+    data: *mut c_void,
+    /// Unused.
+    return_code: c_int,
+}
+
+impl GenericResponse {
+    /// Allocates a response on the C heap pointing to the given data.
+    fn alloc(data: *mut c_void) -> *mut Self {
+        unsafe {
+            let alloc = libc::calloc(1, size_of::<Self>()) as *mut Self;
+            (*alloc).data = data;
+            alloc
+        }
+    }
+
+    /// Frees a response on the C heap.
+    ///
+    /// # Safety
+    ///
+    /// It's on you to stop using this GenericResponse after freeing it.
+    pub unsafe fn free(me: *mut GenericResponse) {
+        if !me.is_null() {
+            libc::free((*me).data);
+        }
+        libc::free(me as *mut c_void);
+    }
 }
 
 /// An opaque pointer we provide to PAM for callbacks.
@@ -261,14 +303,47 @@
 /// The callback that PAM uses to get information in a conversation.
 ///
 /// - `num_msg` is the number of messages in the `pam_message` array.
-/// - `messages` is a pointer to an array of pointers to [`Message`]s.
-/// - `responses` is a pointer to an array of [`ResponseInner`]s,
+/// - `messages` is a pointer to some [`Message`]s (see note).
+/// - `responses` is a pointer to an array of [`GenericResponse`]s,
 ///   which PAM sets in response to a module's request.
+///   This is an array of structs, not an array of pointers to a struct.
+///   There should always be exactly as many `responses` as `num_msg`.
 /// - `appdata` is the `appdata` field of the [`Conversation`] we were passed.
+///
+/// NOTE: On Linux-PAM and other compatible implementations, `messages`
+/// is treated as a pointer-to-pointers, like `int argc, char **argv`.
+///
+/// ```text
+/// ┌──────────┐  points to  ┌─────────────┐       ╔═ Message ═╗
+/// │ messages │ ┄┄┄┄┄┄┄┄┄┄> │ messages[0] │ ┄┄┄┄> ║ style     ║
+/// └──────────┘             │ messages[1] │ ┄┄╮   ║ data      ║
+///                          │ ...         │   ┆   ╚═══════════╝
+///                                            ┆
+///                                            ┆    ╔═ Message ═╗
+///                                            ╰┄┄> ║ style     ║
+///                                                 ║ data      ║
+///                                                 ╚═══════════╝
+/// ```
+///
+/// On OpenPAM and other compatible implementations (like Solaris),
+/// `messages` is a pointer-to-pointer-to-array.
+///
+/// ```text
+/// ┌──────────┐  points to  ┌───────────┐       ╔═ Message[] ═╗
+/// │ messages │ ┄┄┄┄┄┄┄┄┄┄> │ *messages │ ┄┄┄┄> ║ style       ║
+/// └──────────┘             └───────────┘       ║ data        ║
+///                                              ╟─────────────╢
+///                                              ║ style       ║
+///                                              ║ data        ║
+///                                              ╟─────────────╢
+///                                              ║ ...         ║
+/// ```
+///
+/// ***THIS LIBRARY CURRENTLY SUPPORTS ONLY LINUX-PAM.***
 pub type ConversationCallback = extern "C" fn(
     num_msg: c_int,
     messages: *const *const Message,
-    responses: &mut *const ResponseInner,
+    responses: &mut *const GenericResponse,
     appdata: *const AppData,
 ) -> c_int;
 
@@ -316,7 +391,7 @@
 
 #[cfg(test)]
 mod test {
-    use super::{BinaryResponseInner, TextResponseInner};
+    use super::{BinaryResponseInner, GenericResponse, TextResponseInner};
 
     #[test]
     fn test_text_response() {
@@ -342,6 +417,14 @@
     }
 
     #[test]
+    fn test_free_safety() {
+        unsafe {
+            TextResponseInner::free(std::ptr::null_mut());
+            BinaryResponseInner::free(std::ptr::null_mut());
+        }
+    }
+
+    #[test]
     #[ignore]
     fn test_binary_response_too_big() {
         let big_data: Vec<u8> = vec![0xFFu8; 10_000_000_000];