changeset 100:3f11b8d30f63

Implement environment variable management. This actually wires up the environment variable handling to libpam, so that applications and modules can manage the environment through the authentication process.
author Paul Fisher <paul@pfish.zone>
date Tue, 24 Jun 2025 17:08:01 -0400
parents 8840fa6534f6
children 94b51fa4f797
files build.rs src/environ.rs src/handle.rs src/lib.rs src/libpam/answer.rs src/libpam/conversation.rs src/libpam/environ.rs src/libpam/memory.rs
diffstat 8 files changed, 213 insertions(+), 125 deletions(-) [+]
line wrap: on
line diff
--- a/build.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/build.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -14,13 +14,13 @@
             .allowlist_var(".*")
             .allowlist_function("pam_start")
             .allowlist_function("pam_[gs]et_item")
-            .allowlist_function("pam_get_user")
-            .allowlist_function("pam_get_authtok")
+            .allowlist_function("pam_get_(user|authtok)")
             .allowlist_function("pam_end")
             .allowlist_function("pam_strerror")
             .allowlist_function("pam_authenticate")
             .allowlist_function("pam_chauthtok")
             .allowlist_function("pam_acct_mgmt")
+            .allowlist_function("pam_(ge|pu)tenv(list)?")
             .default_macro_constant_type(MacroTypeVariation::Unsigned);
 
         let linux_builder = common_builder
--- a/src/environ.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/environ.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -3,26 +3,31 @@
 //! PAM modules can set environment variables to apply to a user session.
 //! This module manages the interface for doing all of that.
 
+use crate::constants::Result;
 use std::ffi::{OsStr, OsString};
 
 /// A key/value map for environment variables, as [`OsString`]s.
 ///
 /// This is a limited subset of what [`HashMap`](std::collections::HashMap)
 /// can do. Notably, we do *not* support mutable iteration.
-pub trait EnvironMap {
+pub trait EnvironMap<'a> {
     /// Gets the environment variable of the given key.
-    fn get(&self, val: &OsStr) -> Option<&OsStr>;
+    fn get(&self, key: impl AsRef<OsStr>) -> Option<OsString>;
 
     /// Iterates over the key/value pairs of the environment.
-    fn iter(&self) -> impl Iterator<Item = (&OsStr, &OsStr)>;
+    fn iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>>;
 }
 
-pub trait EnvironMapMut: EnvironMap {
+pub trait EnvironMapMut<'a>: EnvironMap<'a> {
     /// Sets the environment variable with the given key,
     /// returning the old one if present.
-    fn insert(&mut self, key: &OsStr, val: &OsStr) -> Option<OsString>;
+    fn insert(
+        &mut self,
+        key: impl AsRef<OsStr>,
+        val: impl AsRef<OsStr>,
+    ) -> Result<Option<OsString>>;
 
     /// Removes the environment variable from the map, returning its old value
     /// if present.
-    fn remove(&mut self, key: &OsStr) -> Option<OsString>;
+    fn remove(&mut self, key: impl AsRef<OsStr>) -> Result<Option<OsString>>;
 }
--- a/src/handle.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/handle.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -198,8 +198,8 @@
     trait_item!(
         /// Sets the identity of the remote user logging in.
         ///
-        /// This is usually set by the application before making calls
-        /// into a PAM session. (TODO: check this!)
+        /// This may be set by the application before making calls
+        /// into a PAM transaction.
         set = set_remote_user,
         item = "PAM_RUSER"
     );
@@ -220,8 +220,8 @@
     trait_item!(
         /// Sets the location where the user is coming from.
         ///
-        /// This is usually set by the application before making calls
-        /// into a PAM session. (TODO: check this!)
+        /// This may be set by the application before making calls
+        /// into a PAM transaction.
         set = set_remote_host,
         item = "PAM_RHOST",
         see = Self::remote_host
--- a/src/lib.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/lib.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -43,7 +43,7 @@
 pub use crate::{
     constants::{ErrorCode, Flags, Result},
     conv::{BinaryData, Conversation, ConversationAdapter},
-    environ::EnvironMap,
+    environ::{EnvironMap, EnvironMapMut},
     handle::{PamHandleApplication, PamHandleModule, PamShared},
     module::PamModule,
 };
--- a/src/libpam/answer.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/libpam/answer.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -9,6 +9,7 @@
 use std::ops::{Deref, DerefMut};
 use std::ptr::NonNull;
 use std::{iter, mem, ptr, slice};
+use std::mem::ManuallyDrop;
 
 /// The corridor via which the answer to Messages navigate through PAM.
 #[derive(Debug)]
@@ -48,9 +49,7 @@
     /// This object is consumed and the `Answer` pointer now owns its data.
     /// It can be recreated with [`Self::from_c_heap`].
     pub fn into_ptr(self) -> NonNull<Answer> {
-        let ret = self.base;
-        mem::forget(self);
-        ret
+        ManuallyDrop::new(self).base
     }
 
     /// Takes ownership of a list of answers allocated on the C heap.
--- a/src/libpam/conversation.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/libpam/conversation.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -55,7 +55,7 @@
             conv.communicate(&borrowed?);
 
             // Send our answers back.
-            let owned = Answers::build(messages).map_err(|_| ErrorCode::ConversationError)?;
+            let owned = Answers::build(messages)?;
             *answers_ptr = owned.into_ptr().as_ptr();
             Ok(())
         };
--- a/src/libpam/environ.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/libpam/environ.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -1,12 +1,13 @@
 #![allow(unused_variables)] // for now
-use crate::environ::EnvironMapMut;
+use crate::constants::{ErrorCode, Result};
+use crate::environ::{EnvironMap, EnvironMapMut};
 use crate::libpam::memory::CHeapString;
-use crate::{EnvironMap, LibPamHandle};
-use std::ffi::{c_char, OsStr, OsString};
-use std::os::unix::ffi::OsStrExt;
+use crate::libpam::{memory, pam_ffi, LibPamHandle};
+use std::ffi::{c_char, CStr, CString, OsStr, OsString};
+use std::marker::PhantomData;
+use std::os::unix::ffi::{OsStrExt, OsStringExt};
+use std::ptr;
 use std::ptr::NonNull;
-use std::{iter, ptr};
-use crate::libpam::memory;
 
 pub struct LibPamEnviron<'a> {
     source: &'a LibPamHandle,
@@ -16,6 +17,58 @@
     source: &'a mut LibPamHandle,
 }
 
+impl LibPamHandle {
+    fn environ_get(&self, key: &OsStr) -> Option<OsString> {
+        let key = CString::new(key.as_bytes()).ok()?;
+        // SAFETY: We are a valid handle and are calling with a good key.
+        unsafe {
+            copy_env(pam_ffi::pam_getenv(
+                (self as *const LibPamHandle).cast_mut(),
+                key.as_ptr(),
+            ))
+        }
+    }
+
+    fn environ_set(&mut self, key: &OsStr, value: Option<&OsStr>) -> Result<Option<OsString>> {
+        let old = self.environ_get(key);
+        let total_len = key.len() + value.map(OsStr::len).unwrap_or_default() + 2;
+        let mut result = Vec::with_capacity(total_len);
+        result.extend(key.as_bytes());
+        if let Some(value) = value {
+            result.push(b'=');
+            result.extend(value.as_bytes());
+        }
+        let put = CString::new(result).map_err(|_| ErrorCode::ConversationError)?;
+        // SAFETY: This is a valid handle and a valid environment string.
+        ErrorCode::result_from(unsafe { pam_ffi::pam_putenv(self, put.as_ptr()) })?;
+        Ok(old)
+    }
+
+    fn environ_iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>> {
+        // SAFETY: This is a valid PAM handle.
+        unsafe {
+            NonNull::new(pam_ffi::pam_getenvlist(
+                (self as *const LibPamHandle).cast_mut(),
+            ))
+            .map(|ptr| EnvList::from_ptr(ptr.cast()))
+            .ok_or(ErrorCode::BufferError)
+        }
+    }
+}
+
+/// Copies the data of the given C string pointer to an OsString,
+/// or None if src is null.
+unsafe fn copy_env(src: *const c_char) -> Option<OsString> {
+    let val = match NonNull::new(src.cast_mut()) {
+        None => return None,
+        Some(ptr) => ptr.as_ptr(),
+    };
+    // SAFETY: We were just returned this string from PAM.
+    // We have to trust it.
+    let c_str = unsafe { CStr::from_ptr(val) };
+    Some(OsString::from_vec(c_str.to_bytes().to_vec()))
+}
+
 impl<'a> LibPamEnviron<'a> {
     pub fn new(source: &'a LibPamHandle) -> Self {
         Self { source }
@@ -26,83 +79,94 @@
     pub fn new(source: &'a mut LibPamHandle) -> Self {
         Self { source }
     }
+}
 
-    fn immut(&self) -> LibPamEnviron {
-        LibPamEnviron {
-            source: self.source,
+impl EnvironMap<'_> for LibPamEnviron<'_> {
+    fn get(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
+        self.source.environ_get(key.as_ref())
+    }
+
+    fn iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>> {
+        self.source.environ_iter()
+    }
+}
+
+impl EnvironMap<'_> for LibPamEnvironMut<'_> {
+    fn get(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
+        self.source.environ_get(key.as_ref())
+    }
+
+    fn iter(&self) -> Result<impl Iterator<Item = (OsString, OsString)>> {
+        self.source.environ_iter()
+    }
+}
+
+impl EnvironMapMut<'_> for LibPamEnvironMut<'_> {
+    fn insert(
+        &mut self,
+        key: impl AsRef<OsStr>,
+        val: impl AsRef<OsStr>,
+    ) -> Result<Option<OsString>> {
+        self.source.environ_set(key.as_ref(), Some(val.as_ref()))
+    }
+
+    fn remove(&mut self, key: impl AsRef<OsStr>) -> Result<Option<OsString>> {
+        self.source.environ_set(key.as_ref(), None)
+    }
+}
+
+struct EnvList<'a> {
+    /// Pointer to the start of the environment variable list.
+    ///
+    /// This can't be a `CHeapBox` because it's not just a single
+    /// `Option<EnvVar>`.
+    start: NonNull<Option<EnvVar>>,
+    /// The environment variable we're about to iterate into.
+    current: NonNull<Option<EnvVar>>,
+    _owner: PhantomData<&'a LibPamHandle>,
+}
+
+impl EnvList<'_> {
+    unsafe fn from_ptr(ptr: NonNull<*mut c_char>) -> Self {
+        Self {
+            start: ptr.cast(),
+            current: ptr.cast(),
+            _owner: Default::default(),
+        }
+    }
+}
+impl Iterator for EnvList<'_> {
+    type Item = (OsString, OsString);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        // SAFETY: We were given a pointer to a valid environment list,
+        // and we only ever advance it to the exact end of the list.
+        match unsafe { self.current.as_mut() } {
+            None => None,
+            Some(item) => {
+                let ret = item.as_kv();
+                // SAFETY: We know we're still pointing to a valid pointer,
+                // and advancing it one more is allowed.
+                unsafe {
+                    self.current = self.current.add(1);
+                    ptr::drop_in_place(item as *mut EnvVar);
+                }
+                Some(ret)
+            }
         }
     }
 }
 
-impl EnvironMap for LibPamEnviron<'_> {
-    fn get(&self, val: &OsStr) -> Option<&OsStr> {
-        todo!()
-    }
-
-    fn iter(&self) -> impl Iterator<Item = (&OsStr, &OsStr)> {
-        iter::from_fn(|| todo!())
-    }
-}
-
-impl EnvironMap for LibPamEnvironMut<'_> {
-    fn get(&self, val: &OsStr) -> Option<&OsStr> {
-        todo!()
-    }
-
-    fn iter(&self) -> impl Iterator<Item = (&OsStr, &OsStr)> {
-        iter::from_fn(|| todo!())
-    }
-}
-
-impl EnvironMapMut for LibPamEnvironMut<'_> {
-    fn insert(&mut self, key: &OsStr, val: &OsStr) -> Option<OsString> {
-        todo!()
-    }
-
-    fn remove(&mut self, key: &OsStr) -> Option<OsString> {
-        todo!()
-    }
-}
-
-struct EnvList {
-    /// Pointer to the start of the environment variable list.
-    ///
-    /// This can't be a `CHeapBox` because it's not just a single
-    /// `Option<EnvVar>`.
-    vars: NonNull<Option<EnvVar>>,
-}
-
-impl EnvList {
-    unsafe fn from_ptr(ptr: NonNull<*mut c_char>) -> Self {
-        Self{vars: ptr.cast()}
-    }
-
-    fn iter(&self) -> impl Iterator<Item = (&OsStr, &OsStr)> {
-        let mut current = self.vars;
-        iter::from_fn(move || {
-            match unsafe {current.as_ref()} {
-                None => None,
-                Some(item) => {
-                    let ret = item.as_kv();
-                    current = unsafe {current.add(1) };
-                    Some(ret)
-                }
+impl Drop for EnvList<'_> {
+    fn drop(&mut self) {
+        // SAFETY: We own self.start, and we know that self.current points to
+        // either an item we haven't used, or to the None end.
+        unsafe {
+            while let Some(var_ref) = self.current.as_mut() {
+                ptr::drop_in_place(var_ref as *mut EnvVar);
+                self.current = self.current.add(1);
             }
-        })
-    }
-}
-
-impl Drop for EnvList {
-    fn drop(&mut self) {
-        // SAFETY: We own this pointer, and we know it's valid environment data
-        // from PAM.
-        unsafe {
-            let mut var = self.vars;
-            while let Some(var_ref) = var.as_mut() {
-                ptr::drop_in_place(var_ref as *mut EnvVar);
-                var = var.add(1);
-            }
-            memory::free(self.vars.as_ptr())
+            memory::free(self.start.as_ptr())
         }
     }
 }
@@ -110,27 +174,30 @@
 struct EnvVar(CHeapString);
 
 impl EnvVar {
-    fn as_kv(&self) -> (&OsStr, &OsStr) {
+    fn as_kv(&self) -> (OsString, OsString) {
         let bytes = self.0.to_bytes();
         let mut split = bytes.splitn(2, |&b| b == b'=');
         (
-            OsStr::from_bytes(split.next().unwrap_or_default()),
-            OsStr::from_bytes(split.next().unwrap_or_default()),
+            OsString::from_vec(split.next().unwrap_or_default().into()),
+            OsString::from_vec(split.next().unwrap_or_default().into()),
         )
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::libpam::memory::CHeapBox;
     use super::*;
 
+    fn os(text: &str) -> OsString {
+        OsString::from_vec(text.into())
+    }
+
     #[test]
     fn test_split_kv() {
         fn test(input: &str, key: &str, value: &str) {
             let data = CHeapString::new(input).unwrap();
-            let key = OsStr::from_bytes(key.as_bytes());
-            let value = OsStr::from_bytes(value.as_bytes());
+            let key = os(key);
+            let value = os(value);
 
             assert_eq!(EnvVar(data).as_kv(), (key, value));
         }
@@ -141,26 +208,46 @@
         test("", "", "");
     }
 
+    fn env_list(strings: &[&'static str]) -> EnvList<'static> {
+        let ptrs: NonNull<Option<CHeapString>> = memory::calloc(strings.len() + 1).unwrap();
+        unsafe {
+            for (idx, &text) in strings.iter().enumerate() {
+                ptr::write(
+                    ptrs.add(idx).as_ptr(),
+                    Some(CHeapString::new(text).unwrap()),
+                )
+            }
+            ptr::write(ptrs.add(strings.len()).as_ptr(), None);
+            EnvList::from_ptr(ptrs.cast())
+        }
+    }
+
     #[test]
     fn test_iter() {
-        let bx = CHeapBox::new([
-            Some("ONE=two"),
-            Some("BIRDS=birds=birds"),
-            Some("me"),
-            Some("you="),
-            None,
-        ].map(|txt| txt.map(|txt| CHeapString::new(txt).unwrap()))).unwrap();
-        let env_ptr = CHeapBox::into_ptr(bx);
+        let envs = env_list(&["ONE=two", "BIRDS=birds=birds", "me", "you="]);
+        let result: Vec<_> = envs.collect();
+        assert_eq!(
+            vec![
+                (os("ONE"), os("two")),
+                (os("BIRDS"), os("birds=birds")),
+                (os("me"), os("")),
+                (os("you"), os("")),
+            ],
+            result
+        );
+    }
 
-        let envs = unsafe {EnvList::from_ptr(env_ptr.cast())};
-        let bytes = |data: &'static str| OsStr::from_bytes(data.as_ref());
-        let result: Vec<_> = envs.iter().collect();
-        assert_eq!(vec![
-            (bytes("ONE"), bytes("two")),
-            (bytes("BIRDS"), bytes("birds=birds")),
-            (bytes("me"), bytes("")),
-            (bytes("you"), bytes("")),
-        ],
-        result);
+    #[test]
+    fn test_iter_partial() {
+        let mut envs = env_list(&[
+            "iterating=this",
+            "also=here",
+            "but not=this one",
+            "or even=the last",
+        ]);
+
+        assert_eq!(Some((os("iterating"), os("this"))), envs.next());
+        assert_eq!(Some((os("also"), os("here"))), envs.next());
+        // let envs drop
     }
 }
--- a/src/libpam/memory.rs	Tue Jun 24 14:54:47 2025 -0400
+++ b/src/libpam/memory.rs	Tue Jun 24 17:08:01 2025 -0400
@@ -6,7 +6,7 @@
 use std::ffi::{c_char, CStr, CString};
 use std::fmt::{Display, Formatter, Result as FmtResult};
 use std::marker::{PhantomData, PhantomPinned};
-use std::mem::offset_of;
+use std::mem::{offset_of, ManuallyDrop};
 use std::ops::{Deref, DerefMut};
 use std::ptr::NonNull;
 use std::result::Result as StdResult;
@@ -96,9 +96,7 @@
 
     /// Converts this CBox into a raw pointer.
     pub fn into_ptr(this: Self) -> NonNull<T> {
-        let ret = this.0;
-        mem::forget(this);
-        ret
+        ManuallyDrop::new(this).0
     }
 
     /// Gets a pointer from this but doesn't convert this into a raw pointer.
@@ -177,9 +175,8 @@
     ///
     /// You are responsible for freeing it later.
     pub fn into_ptr(self) -> NonNull<c_char> {
-        let ptr = CHeapBox::as_ptr(&self.0);
-        mem::forget(self);
-        ptr
+        let this = ManuallyDrop::new(self);
+        CHeapBox::as_ptr(&this.0)
     }
 
     /// Converts this into a dumb box. It will no longer be zeroed upon drop.