# HG changeset patch # User Paul Fisher # Date 1750753525 14400 # Node ID b87100c5eed49a91ab2d886c3ab0129e09f0971c # Parent efe2f5f8b5b25c4d4d6059dd8a8b41a9d3d47e17 Start on environment variables, and make pointers nicer. This starts work on the PAM environment handling, and in so doing, introduces the CHeapBox and CHeapString structs. These are analogous to Box and CString, but they're located on the C heap rather than being Rust-managed memory. This is because environment variables deal with even more pointers and it turns out we can lose a lot of manual freeing using homemade smart pointers. diff -r efe2f5f8b5b2 -r b87100c5eed4 src/environ.rs --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/environ.rs Tue Jun 24 04:25:25 2025 -0400 @@ -0,0 +1,28 @@ +//! Traits and stuff for managing the environment of a PAM handle. +//! +//! PAM modules can set environment variables to apply to a user session. +//! This module manages the interface for doing all of that. + +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 { + /// Gets the environment variable of the given key. + fn get(&self, val: &OsStr) -> Option<&OsStr>; + + /// Iterates over the key/value pairs of the environment. + fn iter(&self) -> impl Iterator; +} + +pub trait EnvironMapMut: EnvironMap { + /// Sets the environment variable with the given key, + /// returning the old one if present. + fn insert(&mut self, key: &OsStr, val: &OsStr) -> Option; + + /// Removes the environment variable from the map, returning its old value + /// if present. + fn remove(&mut self, key: &OsStr) -> Option; +} diff -r efe2f5f8b5b2 -r b87100c5eed4 src/handle.rs --- a/src/handle.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/handle.rs Tue Jun 24 04:25:25 2025 -0400 @@ -2,6 +2,7 @@ use crate::constants::{Flags, Result}; use crate::conv::Conversation; +use crate::environ::{EnvironMap, EnvironMapMut}; use crate::logging::Level; macro_rules! trait_item { @@ -69,7 +70,7 @@ /// ```no_run /// # use nonstick::{PamShared}; /// # use nonstick::logging::Level; - /// # let pam_hdl: Box = unimplemented!(); + /// # fn _test(pam_hdl: impl PamShared) { /// # let delay_ms = 100; /// # let url = "https://zombo.com"; /// // Usually, instead of calling this manually, just use the macros. @@ -79,6 +80,7 @@ /// nonstick::debug!(pam_hdl, "sending GET request to {url}"); /// // But if you really want to, you can call this yourself: /// pam_hdl.log(Level::Warning, "this is unnecessarily verbose"); + /// # } /// ``` fn log(&self, level: Level, entry: &str); @@ -113,6 +115,12 @@ /// [mwg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/mwg-expected-by-module-item.html#mwg-pam_get_user fn username(&mut self, prompt: Option<&str>) -> Result; + /// The contents of the environment to set, read-only. + fn environ(&self) -> impl EnvironMap; + + /// A writable version of the environment. + fn environ_mut(&mut self) -> impl EnvironMapMut; + trait_item!( /// The identity of the user for whom service is being requested. /// @@ -250,10 +258,10 @@ pub trait PamHandleApplication: PamShared { /// Starts the authentication process for the user. fn authenticate(&mut self, flags: Flags) -> Result<()>; - + /// Does "account management". fn account_management(&mut self, flags: Flags) -> Result<()>; - + /// Changes the authentication token. fn change_authtok(&mut self, flags: Flags) -> Result<()>; } diff -r efe2f5f8b5b2 -r b87100c5eed4 src/lib.rs --- a/src/lib.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/lib.rs Tue Jun 24 04:25:25 2025 -0400 @@ -31,16 +31,19 @@ pub mod handle; +mod environ; #[cfg(feature = "link")] mod libpam; pub mod logging; #[cfg(feature = "link")] +#[doc(inline)] pub use crate::libpam::{LibPamHandle, OwnedLibPamHandle}; #[doc(inline)] pub use crate::{ constants::{ErrorCode, Flags, Result}, conv::{BinaryData, Conversation, ConversationAdapter}, + environ::EnvironMap, handle::{PamHandleApplication, PamHandleModule, PamShared}, module::PamModule, }; diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/answer.rs --- a/src/libpam/answer.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/answer.rs Tue Jun 24 04:25:25 2025 -0400 @@ -2,7 +2,7 @@ use crate::libpam::conversation::OwnedMessage; use crate::libpam::memory; -use crate::libpam::memory::CBinaryData; +use crate::libpam::memory::{CBinaryData, CHeapBox, CHeapString}; pub use crate::libpam::pam_ffi::Answer; use crate::{ErrorCode, Result}; use std::ffi::CStr; @@ -13,7 +13,9 @@ /// The corridor via which the answer to Messages navigate through PAM. #[derive(Debug)] pub struct Answers { - base: *mut Answer, + /// The actual list of answers. This can't be a [`CHeapBox`] because + /// this is the pointer to the start of an array, not a single Answer. + base: NonNull, count: usize, } @@ -21,7 +23,7 @@ /// Builds an Answers out of the given answered Message Q&As. pub fn build(value: Vec) -> Result { let mut outputs = Self { - base: memory::calloc(value.len()), + base: memory::calloc(value.len())?, count: value.len(), }; // Even if we fail during this process, we still end up freeing @@ -45,7 +47,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) -> *mut Answer { + pub fn into_ptr(self) -> NonNull { let ret = self.base; mem::forget(self); ret @@ -57,7 +59,7 @@ /// /// It's up to you to make sure you pass a valid pointer, /// like one that you got from PAM, or maybe [`Self::into_ptr`]. - pub unsafe fn from_c_heap(base: *mut Answer, count: usize) -> Self { + pub unsafe fn from_c_heap(base: NonNull, count: usize) -> Self { Answers { base, count } } } @@ -66,25 +68,26 @@ type Target = [Answer]; fn deref(&self) -> &Self::Target { // SAFETY: This is the memory we manage ourselves. - unsafe { slice::from_raw_parts(self.base, self.count) } + unsafe { slice::from_raw_parts(self.base.as_ptr(), self.count) } } } impl DerefMut for Answers { fn deref_mut(&mut self) -> &mut Self::Target { // SAFETY: This is the memory we manage ourselves. - unsafe { slice::from_raw_parts_mut(self.base, self.count) } + unsafe { slice::from_raw_parts_mut(self.base.as_ptr(), self.count) } } } impl Drop for Answers { fn drop(&mut self) { // SAFETY: We allocated this ourselves, or it was provided to us by PAM. + // We own these pointers, and they will never be used after this. unsafe { for answer in self.iter_mut() { - answer.free_contents() + ptr::drop_in_place(answer) } - memory::free(self.base) + memory::free(self.base.as_ptr()) } } } @@ -106,23 +109,25 @@ /// Converts the `Answer` to a `TextAnswer` with the given text. fn fill(dest: &mut Answer, text: &str) -> Result<()> { - let allocated = memory::malloc_str(text)?; - dest.free_contents(); - dest.data = allocated.as_ptr().cast(); + let allocated = CHeapString::new(text)?; + let _ = dest + .data + .replace(unsafe { CHeapBox::cast(allocated.into_box()) }); Ok(()) } /// Gets the string stored in this answer. pub fn contents(&self) -> Result<&str> { - if self.0.data.is_null() { - Ok("") - } else { - // SAFETY: This data is either passed from PAM (so we are forced - // to trust it) or was created by us in TextAnswerInner::alloc. - // In either case, it's going to be a valid null-terminated string. - unsafe { CStr::from_ptr(self.0.data.cast()) } - .to_str() - .map_err(|_| ErrorCode::ConversationError) + match self.0.data.as_ref() { + None => Ok(""), + Some(data) => { + // SAFETY: This data is either passed from PAM (so we are forced + // to trust it) or was created by us in TextAnswerInner::alloc. + // In either case, it's going to be a valid null-terminated string. + unsafe { CStr::from_ptr(CHeapBox::as_ptr(data).as_ptr().cast()) } + .to_str() + .map_err(|_| ErrorCode::ConversationError) + } } } @@ -131,14 +136,14 @@ /// When this `TextAnswer` is part of an [`Answers`], /// this is optional (since that will perform the `free`), /// but it will clear potentially sensitive data. - pub fn free_contents(&mut self) { + pub fn zero_contents(&mut self) { // SAFETY: We own this data and know it's valid. // If it's null, this is a no-op. // After we're done, it will be null. unsafe { - memory::zero_c_string(self.0.data.cast()); - memory::free(self.0.data); - self.0.data = ptr::null_mut() + if let Some(ptr) = self.0.data.as_ref() { + CHeapString::zero(CHeapBox::as_ptr(ptr).cast()); + } } } } @@ -168,8 +173,7 @@ /// We do not take ownership of the original data. pub fn fill(dest: &mut Answer, data_and_type: (&[u8], u8)) -> Result<()> { let allocated = CBinaryData::alloc(data_and_type)?; - dest.free_contents(); - dest.data = allocated.as_ptr().cast(); + let _ = dest.data.replace(unsafe { CHeapBox::cast(allocated) }); Ok(()) } @@ -177,7 +181,11 @@ pub fn data(&self) -> Option> { // SAFETY: We either got this data from PAM or allocated it ourselves. // Either way, we trust that it is either valid data or null. - NonNull::new(self.0.data.cast::()) + self.0 + .data + .as_ref() + .map(CHeapBox::as_ptr) + .map(NonNull::cast) } /// Zeroes out the answer data, frees it, and points our data to `null`. @@ -189,26 +197,9 @@ // SAFETY: We know that our data pointer is either valid or null. // Once we're done, it's null and the answer is safe. unsafe { - if let Some(ptr) = NonNull::new(self.0.data) { - CBinaryData::zero_contents(ptr.cast()) + if let Some(ptr) = self.0.data.as_ref() { + CBinaryData::zero_contents(CHeapBox::as_ptr(ptr).cast()) } - memory::free(self.0.data); - self.0.data = ptr::null_mut() - } - } -} - -impl Answer { - /// Frees the contents of this answer. - /// - /// After this is done, this answer's `data` will be `null`, - /// which is a valid (empty) state. - fn free_contents(&mut self) { - // SAFETY: We have either an owned valid pointer, or null. - // We can free our owned pointer, and `free(null)` is a no-op. - unsafe { - memory::free(self.data); - self.data = ptr::null_mut(); } } } @@ -229,7 +220,7 @@ fn assert_text_answer(want: &str, answer: &mut Answer) { let up = unsafe { TextAnswer::upcast(answer) }; assert_eq!(want, up.contents().unwrap()); - up.free_contents(); + up.zero_contents(); assert_eq!("", up.contents().unwrap()); } @@ -293,42 +284,33 @@ #[test] fn test_text_answer() { - let answer_ptr: *mut Answer = memory::calloc(1); - let answer = unsafe { &mut *answer_ptr }; - TextAnswer::fill(answer, "hello").unwrap(); - let zeroth_text = unsafe { TextAnswer::upcast(answer) }; + let mut answer: CHeapBox = CHeapBox::default(); + TextAnswer::fill(&mut answer, "hello").unwrap(); + let zeroth_text = unsafe { TextAnswer::upcast(&mut answer) }; let data = zeroth_text.contents().expect("valid"); assert_eq!("hello", data); - zeroth_text.free_contents(); - zeroth_text.free_contents(); - TextAnswer::fill(answer, "hell\0").expect_err("should error; contains nul"); - unsafe { memory::free(answer_ptr) } + zeroth_text.zero_contents(); + zeroth_text.zero_contents(); + TextAnswer::fill(&mut answer, "hell\0").expect_err("should error; contains nul"); } #[test] fn test_binary_answer() { use crate::conv::BinaryData; - let answer_ptr: *mut Answer = memory::calloc(1); - let answer = unsafe { &mut *answer_ptr }; + let mut answer: CHeapBox = CHeapBox::default(); let real_data = BinaryData::new([1, 2, 3, 4, 5, 6, 7, 8], 9); - BinaryAnswer::fill(answer, (&real_data).into()).expect("alloc should succeed"); - let bin_answer = unsafe { BinaryAnswer::upcast(answer) }; + BinaryAnswer::fill(&mut answer, (&real_data).into()).expect("alloc should succeed"); + let bin_answer = unsafe { BinaryAnswer::upcast(&mut answer) }; assert_eq!(real_data, unsafe { CBinaryData::as_binary_data(bin_answer.data().unwrap()) }); - answer.free_contents(); - answer.free_contents(); - unsafe { memory::free(answer_ptr) } } #[test] #[ignore] fn test_binary_answer_too_big() { let big_data: Vec = vec![0xFFu8; 0x1_0000_0001]; - let answer_ptr: *mut Answer = memory::calloc(1); - let answer = unsafe { &mut *answer_ptr }; - BinaryAnswer::fill(answer, (&big_data, 100)).expect_err("this is too big!"); - answer.free_contents(); - unsafe { memory::free(answer) } + let mut answer: CHeapBox = CHeapBox::default(); + BinaryAnswer::fill(&mut answer, (&big_data, 100)).expect_err("this is too big!"); } } diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/conversation.rs --- a/src/libpam/conversation.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/conversation.rs Tue Jun 24 04:25:25 2025 -0400 @@ -12,6 +12,7 @@ use std::ffi::c_int; use std::iter; use std::marker::PhantomData; +use std::ptr::NonNull; use std::result::Result as StdResult; impl LibPamConversation<'_> { @@ -55,7 +56,7 @@ // Send our answers back. let owned = Answers::build(messages).map_err(|_| ErrorCode::ConversationError)?; - *answers_ptr = owned.into_ptr(); + *answers_ptr = owned.into_ptr().as_ptr(); Ok(()) }; ErrorCode::result_to_c(internal()) @@ -81,6 +82,8 @@ // We have to trust that the responses from PAM match up // with the questions we sent. unsafe { + let response_pointer = + NonNull::new(response_pointer).ok_or(ErrorCode::ConversationError)?; let mut owned_responses = Answers::from_c_heap(response_pointer, messages.len()); for (msg, response) in iter::zip(messages, owned_responses.iter_mut()) { convert(msg, response); diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/environ.rs --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/libpam/environ.rs Tue Jun 24 04:25:25 2025 -0400 @@ -0,0 +1,166 @@ +#![allow(unused_variables)] // for now +use crate::environ::EnvironMapMut; +use crate::libpam::memory::CHeapString; +use crate::{EnvironMap, LibPamHandle}; +use std::ffi::{c_char, OsStr, OsString}; +use std::os::unix::ffi::OsStrExt; +use std::ptr::NonNull; +use std::{iter, ptr}; +use crate::libpam::memory; + +pub struct LibPamEnviron<'a> { + source: &'a LibPamHandle, +} + +pub struct LibPamEnvironMut<'a> { + source: &'a mut LibPamHandle, +} + +impl<'a> LibPamEnviron<'a> { + pub fn new(source: &'a LibPamHandle) -> Self { + Self { source } + } +} + +impl<'a> LibPamEnvironMut<'a> { + 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, val: &OsStr) -> Option<&OsStr> { + todo!() + } + + fn iter(&self) -> impl Iterator { + iter::from_fn(|| todo!()) + } +} + +impl EnvironMap for LibPamEnvironMut<'_> { + fn get(&self, val: &OsStr) -> Option<&OsStr> { + todo!() + } + + fn iter(&self) -> impl Iterator { + iter::from_fn(|| todo!()) + } +} + +impl EnvironMapMut for LibPamEnvironMut<'_> { + fn insert(&mut self, key: &OsStr, val: &OsStr) -> Option { + todo!() + } + + fn remove(&mut self, key: &OsStr) -> Option { + 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`. + vars: NonNull>, +} + +impl EnvList { + unsafe fn from_ptr(ptr: NonNull<*mut c_char>) -> Self { + Self{vars: ptr.cast()} + } + + fn iter(&self) -> impl Iterator { + 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 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()) + } + } +} + +struct EnvVar(CHeapString); + +impl EnvVar { + fn as_kv(&self) -> (&OsStr, &OsStr) { + 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()), + ) + } +} + +#[cfg(test)] +mod tests { + use crate::libpam::memory::CHeapBox; + use super::*; + + #[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()); + + assert_eq!(EnvVar(data).as_kv(), (key, value)); + } + test("THIS=that", "THIS", "that"); + test("THESE=those, no one=knows", "THESE", "those, no one=knows"); + test("HERE=", "HERE", ""); + test("SOME", "SOME", ""); + test("", "", ""); + } + + #[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 = 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); + } +} diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/handle.rs --- a/src/libpam/handle.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/handle.rs Tue Jun 24 04:25:25 2025 -0400 @@ -1,11 +1,13 @@ use super::conversation::LibPamConversation; use crate::constants::{ErrorCode, Result}; use crate::conv::Message; +use crate::environ::EnvironMapMut; use crate::handle::PamShared; +use crate::libpam::environ::{LibPamEnviron, LibPamEnvironMut}; pub use crate::libpam::pam_ffi::LibPamHandle; use crate::libpam::{memory, pam_ffi}; use crate::logging::Level; -use crate::{Conversation, Flags, PamHandleApplication, PamHandleModule}; +use crate::{Conversation, EnvironMap, Flags, PamHandleApplication, PamHandleModule}; use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::cell::Cell; use std::ffi::{c_char, c_int, CString}; @@ -45,15 +47,20 @@ impl HandleBuilder { /// Creates a new HandleBuilder for the given service. fn new(service_name: String) -> Self { - Self{service_name, username: Default::default()} + Self { + service_name, + username: Default::default(), + } } /// Updates the service name. pub fn service_name(mut self, service_name: String) -> Self { - self.service_name = service_name; self + self.service_name = service_name; + self } /// Updates the username. pub fn username(mut self, username: String) -> Self { - self.username = Some(username); self + self.username = Some(username); + self } pub fn build(self, conv: &impl Conversation) -> Result { @@ -65,7 +72,11 @@ pub fn build_with_service(service_name: String) -> HandleBuilder { HandleBuilder::new(service_name) } - fn start(service_name: String, username: Option, conversation: &impl Conversation) -> Result { + fn start( + service_name: String, + username: Option, + conversation: &impl Conversation, + ) -> Result { let conv = LibPamConversation::wrap(conversation); let service_cstr = CString::new(service_name).map_err(|_| ErrorCode::ConversationError)?; let username_cstr = memory::prompt_ptr(memory::option_cstr(username.as_deref())?.as_ref()); @@ -73,9 +84,10 @@ let mut handle: *mut LibPamHandle = ptr::null_mut(); // 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 { pam_ffi::pam_start(service_cstr.as_ptr(), username_cstr, &conv, &mut handle) }; + let result = + unsafe { pam_ffi::pam_start(service_cstr.as_ptr(), username_cstr, &conv, &mut handle) }; ErrorCode::result_from(result)?; - Ok(Self{ + Ok(Self { handle: HandleWrap(handle), last_return: Cell::new(Ok(())), _conversation_lifetime: Default::default(), @@ -85,21 +97,21 @@ impl PamHandleApplication for OwnedLibPamHandle<'_> { fn authenticate(&mut self, flags: Flags) -> Result<()> { - let ret = unsafe { pam_ffi::pam_authenticate(self.handle.0, flags.bits() as c_int)}; + let ret = unsafe { pam_ffi::pam_authenticate(self.handle.0, flags.bits() as c_int) }; let result = ErrorCode::result_from(ret); self.last_return.set(result); result } fn account_management(&mut self, flags: Flags) -> Result<()> { - let ret = unsafe { pam_ffi::pam_acct_mgmt(self.handle.0, flags.bits() as c_int)}; + let ret = unsafe { pam_ffi::pam_acct_mgmt(self.handle.0, flags.bits() as c_int) }; let result = ErrorCode::result_from(ret); self.last_return.set(result); result } fn change_authtok(&mut self, flags: Flags) -> Result<()> { - let ret = unsafe { pam_ffi::pam_chauthtok(self.handle.0, flags.bits() as c_int)}; + let ret = unsafe { pam_ffi::pam_chauthtok(self.handle.0, flags.bits() as c_int) }; let result = ErrorCode::result_from(ret); self.last_return.set(result); result @@ -180,6 +192,14 @@ .unwrap_or(Err(ErrorCode::ConversationError)) } + fn environ(&self) -> impl EnvironMap { + LibPamEnviron::new(self) + } + + fn environ_mut(&mut self) -> impl EnvironMapMut { + LibPamEnvironMut::new(self) + } + cstr_item!(get = user_item, item = ItemType::User); cstr_item!(set = set_user_item, item = ItemType::User); cstr_item!(get = service, item = ItemType::Service); @@ -290,6 +310,7 @@ } macro_rules! delegate { + // 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),*); @@ -304,6 +325,18 @@ result } }; + // 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),*) + } + }; + (fn $meth:ident(&mut self $(, $param:ident: $typ:ty)*) -> $ret:ty) => { + fn $meth(&mut self $(, $param: $typ)*) -> $ret { + self.handle.$meth($($param),*) + } + }; + // Then have item getters / setters (get = $get:ident$(, set = $set:ident)?) => { delegate!(fn $get(&self) -> Result>); $(delegate!(set = $set);)? @@ -318,9 +351,9 @@ } impl PamShared for OwnedLibPamHandle<'_> { - fn log(&self, level: Level, entry: &str) { - self.handle.log(level, entry) - } + delegate!(fn log(&self, level: Level, entry: &str) -> ()); + delegate!(fn environ(&self) -> impl EnvironMap); + delegate!(fn environ_mut(&mut self) -> impl EnvironMapMut); delegate!(fn username(&mut self, prompt: Option<&str>) -> Result); delegate!(get = user_item, set = set_user_item); delegate!(get = service, set = set_service); diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/memory.rs --- a/src/libpam/memory.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/memory.rs Tue Jun 24 04:25:25 2025 -0400 @@ -2,17 +2,39 @@ use crate::Result; use crate::{BinaryData, ErrorCode}; +use std::error::Error; 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::ops::{Deref, DerefMut}; use std::ptr::NonNull; +use std::result::Result as StdResult; use std::{mem, ptr, slice}; +/// Raised from `calloc` when you have no memory! +#[derive(Debug)] +pub struct NoMem; + +impl Display for NoMem { + fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { + write!(f, "out of memory!") + } +} + +impl Error for NoMem {} + +impl From for ErrorCode { + fn from(_: NoMem) -> Self { + ErrorCode::BufferError + } +} + /// Allocates `count` elements to hold `T`. #[inline] -pub fn calloc(count: usize) -> *mut T { +pub fn calloc(count: usize) -> StdResult, NoMem> { // SAFETY: it's always safe to allocate! Leaking memory is fun! - unsafe { libc::calloc(count, size_of::()) }.cast() + NonNull::new(unsafe { libc::calloc(count, size_of::()) }.cast()).ok_or(NoMem) } /// Wrapper for [`libc::free`] to make debugging calls/frees easier. @@ -46,6 +68,170 @@ } } +/// It's like a [`Box`], but C heap managed. +#[derive(Debug)] +#[repr(transparent)] +pub struct CHeapBox(NonNull); + +// Lots of "as" and "into" associated functions. +#[allow(clippy::wrong_self_convention)] +impl CHeapBox { + /// Creates a new CHeapBox holding the given data. + pub fn new(value: T) -> Result { + let memory = calloc(1)?; + unsafe { ptr::write(memory.as_ptr(), value) } + // SAFETY: We literally just allocated this. + Ok(Self(memory)) + } + + /// Takes ownership of the given pointer. + /// + /// # Safety + /// + /// You have to provide a valid pointer to the start of an allocation + /// that was made with `malloc`. + pub unsafe fn from_ptr(ptr: NonNull) -> Self { + Self(ptr) + } + + /// Converts this CBox into a raw pointer. + pub fn into_ptr(this: Self) -> NonNull { + let ret = this.0; + mem::forget(this); + ret + } + + /// Gets a pointer from this but doesn't convert this into a raw pointer. + /// + /// You are responsible for ensuring the CHeapBox lives long enough. + pub fn as_ptr(this: &Self) -> NonNull { + this.0 + } + + /// Converts this into a Box of a different type. + /// + /// # Safety + /// + /// The different type has to be compatible in size/alignment and drop behavior. + pub unsafe fn cast(this: Self) -> CHeapBox { + mem::transmute(this) + } +} + +impl Default for CHeapBox { + fn default() -> Self { + Self::new(Default::default()).expect("allocation should not fail") + } +} + +impl Deref for CHeapBox { + type Target = T; + fn deref(&self) -> &Self::Target { + // SAFETY: We own this pointer and it is guaranteed valid. + unsafe { Self::as_ptr(self).as_ref() } + } +} + +impl DerefMut for CHeapBox { + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: We own this pointer and it is guaranteed valid. + unsafe { Self::as_ptr(self).as_mut() } + } +} + +impl Drop for CHeapBox { + fn drop(&mut self) { + // SAFETY: We own a valid pointer, and will never use it after this. + unsafe { + let ptr = self.0.as_ptr(); + ptr::drop_in_place(ptr); + free(ptr) + } + } +} + +/// A null-terminated string allocated on the C heap. +/// +/// Basically [`CString`], but managed by malloc. +#[derive(Debug)] +#[repr(transparent)] +pub struct CHeapString(CHeapBox); + +impl CHeapString { + /// Creates a new C heap string with the given contents. + pub fn new(text: &str) -> Result { + let data = text.as_bytes(); + if data.contains(&0) { + return Err(ErrorCode::ConversationError); + } + // +1 for the null terminator + let data_alloc: NonNull = calloc(data.len() + 1)?; + // SAFETY: we just allocated this and we have enough room. + unsafe { + libc::memcpy(data_alloc.as_ptr().cast(), data.as_ptr().cast(), data.len()); + Ok(Self(CHeapBox::from_ptr(data_alloc))) + } + } + + /// Converts this C heap string into a raw pointer. + /// + /// You are responsible for freeing it later. + pub fn into_ptr(self) -> NonNull { + let ptr = CHeapBox::as_ptr(&self.0); + mem::forget(self); + ptr + } + + /// Converts this into a dumb box. It will no longer be zeroed upon drop. + pub fn into_box(self) -> CHeapBox { + unsafe { mem::transmute(self) } + } + + /// Takes ownership of a C heap string. + /// + /// # Safety + /// + /// You have to provide a pointer to the start of an allocation that is + /// a valid 0-terminated C string. + unsafe fn from_ptr(ptr: *mut c_char) -> Option { + NonNull::new(ptr).map(|p| unsafe { Self(CHeapBox::from_ptr(p)) }) + } + + unsafe fn from_box(bx: CHeapBox) -> Self { + Self(CHeapBox::cast(bx)) + } + + /// Zeroes the contents of a C string. + /// + /// # Safety + /// + /// You have to provide a valid pointer to a null-terminated C string. + pub unsafe fn zero(ptr: NonNull) { + let cstr = ptr.as_ptr(); + let len = libc::strlen(cstr.cast()); + for x in 0..len { + ptr::write_volatile(cstr.byte_offset(x as isize), mem::zeroed()) + } + } +} + +impl Drop for CHeapString { + fn drop(&mut self) { + // SAFETY: We own a valid C String + unsafe { Self::zero(CHeapBox::as_ptr(&self.0)) } + } +} + +impl Deref for CHeapString { + type Target = CStr; + + fn deref(&self) -> &Self::Target { + // SAFETY: We know we own a valid C string pointer. + let ptr = CHeapBox::as_ptr(&self.0).as_ptr(); + unsafe { CStr::from_ptr(ptr) } + } +} + /// Creates an owned copy of a string that is returned from a /// pam_get_whatever function. /// @@ -64,42 +250,6 @@ Ok(borrowed.map(String::from)) } -/// Allocates a string with the given contents on the C heap. -/// -/// This is like [`CString::new`], but: -/// -/// - it allocates data on the C heap with [`libc::malloc`]. -/// - it doesn't take ownership of the data passed in. -pub fn malloc_str(text: &str) -> Result> { - let data = text.as_bytes(); - if data.contains(&0) { - return Err(ErrorCode::ConversationError); - } - // +1 for the null terminator - let data_alloc: *mut c_char = calloc(data.len() + 1); - // SAFETY: we just allocated this and we have enough room. - unsafe { - libc::memcpy(data_alloc.cast(), data.as_ptr().cast(), data.len()); - Ok(NonNull::new_unchecked(data_alloc)) - } -} - -/// Writes zeroes over the contents of a C string. -/// -/// This won't overwrite a null pointer. -/// -/// # Safety -/// -/// It's up to you to provide a valid C string. -pub unsafe fn zero_c_string(cstr: *mut c_char) { - if !cstr.is_null() { - let len = libc::strlen(cstr.cast()); - for x in 0..len { - ptr::write_volatile(cstr.byte_offset(x as isize), mem::zeroed()) - } - } -} - /// Binary data used in requests and responses. /// /// This is an unsized data type whose memory goes beyond its data. @@ -119,13 +269,12 @@ impl CBinaryData { /// Copies the given data to a new BinaryData on the heap. - pub fn alloc((data, data_type): (&[u8], u8)) -> Result> { + pub fn alloc((data, data_type): (&[u8], u8)) -> Result> { let buffer_size = u32::try_from(data.len() + 5).map_err(|_| ErrorCode::ConversationError)?; // SAFETY: We're only allocating here. - let dest = unsafe { - let mut dest_buffer: NonNull = - NonNull::new_unchecked(calloc::(buffer_size as usize).cast()); + unsafe { + let mut dest_buffer: NonNull = calloc::(buffer_size as usize)?.cast(); let dest = dest_buffer.as_mut(); dest.total_length = buffer_size.to_be_bytes(); dest.data_type = data_type; @@ -134,9 +283,8 @@ data.as_ptr().cast(), data.len(), ); - dest_buffer - }; - Ok(dest) + Ok(CHeapBox::from_ptr(dest_buffer)) + } } fn length(&self) -> usize { @@ -175,24 +323,43 @@ #[cfg(test)] mod tests { - use super::{ - copy_pam_string, free, malloc_str, option_cstr, prompt_ptr, zero_c_string, CString, - ErrorCode, - }; + use super::*; + use std::hint; + #[test] + fn test_box() { + #[allow(non_upper_case_globals)] + static mut drop_count: u32 = 0; + + struct Dropper(i32); + + impl Drop for Dropper { + fn drop(&mut self) { + unsafe { drop_count += 1 } + } + } + + let mut dropbox = CHeapBox::new(Dropper(9)).unwrap(); + hint::black_box(dropbox.0); + dropbox = CHeapBox::new(Dropper(10)).unwrap(); + assert_eq!(1, unsafe { drop_count }); + hint::black_box(dropbox.0); + drop(dropbox); + assert_eq!(2, unsafe { drop_count }); + } #[test] fn test_strings() { - let str = malloc_str("hello there").unwrap(); - let str = str.as_ptr(); - malloc_str("hell\0 there").unwrap_err(); + let str = CHeapString::new("hello there").unwrap(); + let str_ptr = str.into_ptr().as_ptr(); + CHeapString::new("hell\0 there").unwrap_err(); unsafe { - let copied = copy_pam_string(str).unwrap(); + let copied = copy_pam_string(str_ptr).unwrap(); assert_eq!("hello there", copied.unwrap()); - zero_c_string(str); - let idx_three = str.add(3).as_mut().unwrap(); + CHeapString::zero(NonNull::new(str_ptr).unwrap()); + let idx_three = str_ptr.add(3).as_mut().unwrap(); *idx_three = 0x80u8 as i8; - let zeroed = copy_pam_string(str).unwrap().unwrap(); + let zeroed = copy_pam_string(str_ptr).unwrap().unwrap(); assert!(zeroed.is_empty()); - free(str); + let _ = CHeapString::from_ptr(str_ptr); } } diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/mod.rs --- a/src/libpam/mod.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/mod.rs Tue Jun 24 04:25:25 2025 -0400 @@ -8,6 +8,7 @@ mod answer; mod conversation; +mod environ; mod handle; mod memory; mod module; diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/pam_ffi.rs --- a/src/libpam/pam_ffi.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/pam_ffi.rs Tue Jun 24 04:25:25 2025 -0400 @@ -2,7 +2,7 @@ #![allow(non_camel_case_types, non_upper_case_globals)] -use crate::libpam::memory::Immovable; +use crate::libpam::memory::{CHeapBox, Immovable}; use std::ffi::{c_int, c_uint, c_void, CStr}; use std::marker::PhantomData; use std::ptr; @@ -26,15 +26,14 @@ /// This has the same structure as [`BinaryAnswer`](crate::libpam::answer::BinaryAnswer) /// and [`TextAnswer`](crate::libpam::answer::TextAnswer). #[repr(C)] -#[derive(Debug)] +#[derive(Debug, Default)] pub struct Answer { - /// Pointer to the data returned in an answer. - /// For most answers, this will be a [`CStr`], + /// Owned pointer to the data returned in an answer. + /// For most answers, this will be a [`CHeapString`], /// but for [`BinaryQAndA`](crate::conv::BinaryQAndA)s (a Linux-PAM extension), - /// this will be [`CBinaryData`](crate::libpam::memory::CBinaryData). - /// - /// No matter what, this can be freed with a simple [`libc::free`]. - pub data: *mut c_void, + /// this will be a [`CHeapBox`] of + /// [`CBinaryData`](crate::libpam::memory::CBinaryData). + pub data: Option>, /// Unused. Just here for the padding. return_code: c_int, _marker: Immovable, @@ -57,7 +56,7 @@ /// For most requests, this will be an owned [`CStr`], /// but for requests with style `PAM_BINARY_PROMPT`, /// this will be `CBinaryData` (a Linux-PAM extension). - pub data: *mut c_void, + pub data: Option>, pub _marker: Immovable, } diff -r efe2f5f8b5b2 -r b87100c5eed4 src/libpam/question.rs --- a/src/libpam/question.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/libpam/question.rs Tue Jun 24 04:25:25 2025 -0400 @@ -4,15 +4,14 @@ use crate::conv::{BinaryQAndA, RadioQAndA}; use crate::conv::{ErrorMsg, InfoMsg, MaskedQAndA, Message, QAndA}; use crate::libpam::conversation::OwnedMessage; -use crate::libpam::memory::{CBinaryData, Immovable}; +use crate::libpam::memory::{CBinaryData, CHeapBox, CHeapString, Immovable}; +use crate::libpam::pam_ffi; pub use crate::libpam::pam_ffi::Question; -use crate::libpam::{memory, pam_ffi}; use crate::ErrorCode; use crate::Result; use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::ffi::{c_void, CStr}; -use std::ptr::NonNull; -use std::{ptr, slice}; +use std::slice; /// Abstraction of a collection of questions to be sent in a PAM conversation. /// @@ -204,20 +203,19 @@ /// /// It's up to you to pass this only on types with a string value. unsafe fn string_data(&self) -> Result<&str> { - if self.data.is_null() { - Ok("") - } else { - CStr::from_ptr(self.data.cast()) + match self.data.as_ref() { + None => Ok(""), + Some(data) => CStr::from_ptr(CHeapBox::as_ptr(data).cast().as_ptr()) .to_str() - .map_err(|_| ErrorCode::ConversationError) + .map_err(|_| ErrorCode::ConversationError), } } /// Gets this message's data pointer as borrowed binary data. unsafe fn binary_data(&self) -> (&[u8], u8) { - NonNull::new(self.data) - .map(|nn| nn.cast()) - .map(|ptr| CBinaryData::data(ptr)) + self.data + .as_ref() + .map(|data| CBinaryData::data(CHeapBox::as_ptr(data).cast())) .unwrap_or_default() } } @@ -225,9 +223,13 @@ impl TryFrom<&Message<'_>> for Question { type Error = ErrorCode; fn try_from(msg: &Message) -> Result { - let alloc = |style, text| Ok((style, memory::malloc_str(text)?.cast())); + let alloc = |style, text| -> Result<_> { + Ok((style, unsafe { + CHeapBox::cast(CHeapString::new(text)?.into_box()) + })) + }; // We will only allocate heap data if we have a valid input. - let (style, data): (_, NonNull) = match *msg { + let (style, data): (_, CHeapBox) = match *msg { Message::MaskedPrompt(p) => alloc(Style::PromptEchoOff, p.question()), Message::Prompt(p) => alloc(Style::PromptEchoOn, p.question()), Message::Error(p) => alloc(Style::ErrorMsg, p.question()), @@ -235,16 +237,15 @@ #[cfg(feature = "linux-pam-extensions")] Message::RadioPrompt(p) => alloc(Style::RadioType, p.question()), #[cfg(feature = "linux-pam-extensions")] - Message::BinaryPrompt(p) => Ok(( - Style::BinaryPrompt, - CBinaryData::alloc(p.question())?.cast(), - )), + Message::BinaryPrompt(p) => Ok((Style::BinaryPrompt, unsafe { + CHeapBox::cast(CBinaryData::alloc(p.question())?) + })), #[cfg(not(feature = "linux-pam-extensions"))] Message::RadioPrompt(_) | Message::BinaryPrompt(_) => Err(ErrorCode::ConversationError), }?; Ok(Self { style: style.into(), - data: data.as_ptr(), + data: Some(data), _marker: Default::default(), }) } @@ -258,23 +259,26 @@ // This is nice-to-have. We'll try to zero out the data // in the Question. If it's not a supported format, we skip it. if let Ok(style) = Style::try_from(self.style) { - match style { + let _ = match style { #[cfg(feature = "linux-pam-extensions")] - Style::BinaryPrompt => { - if let Some(d) = NonNull::new(self.data) { - CBinaryData::zero_contents(d.cast()) - } - } + Style::BinaryPrompt => self + .data + .as_ref() + .map(|p| CBinaryData::zero_contents(CHeapBox::as_ptr(p).cast())), #[cfg(feature = "linux-pam-extensions")] - Style::RadioType => memory::zero_c_string(self.data.cast()), + Style::RadioType => self + .data + .as_ref() + .map(|p| CHeapString::zero(CHeapBox::as_ptr(p).cast())), Style::TextInfo | Style::ErrorMsg | Style::PromptEchoOff - | Style::PromptEchoOn => memory::zero_c_string(self.data.cast()), - } + | Style::PromptEchoOn => self + .data + .as_ref() + .map(|p| CHeapString::zero(CHeapBox::as_ptr(p).cast())), + }; }; - memory::free(self.data); - self.data = ptr::null_mut(); } } } diff -r efe2f5f8b5b2 -r b87100c5eed4 src/logging.rs --- a/src/logging.rs Mon Jun 23 19:10:34 2025 -0400 +++ b/src/logging.rs Tue Jun 24 04:25:25 2025 -0400 @@ -76,11 +76,12 @@ /// # Example /// /// ```no_run -/// # let pam_handle: Box = unimplemented!(); +/// # fn _test(pam_handle: impl nonstick::PamShared) { /// # let load_error = "xxx"; /// nonstick::error!(pam_handle, "error loading data from data source: {load_error}"); /// // Will log a message like "error loading data from data source: timed out" /// // at ERROR level on syslog. +/// # } /// ``` #[macro_export] macro_rules! error { ($handle:expr, $($arg:tt)+) => { $crate::__log_internal!($handle, Error, $($arg)+);}} @@ -92,11 +93,12 @@ /// # Example /// /// ```no_run -/// # let pam_handle: Box = unimplemented!(); +/// # fn _test(pam_handle: impl nonstick::PamShared) { /// # let latency_ms = "xxx"; /// nonstick::warn!(pam_handle, "loading took too long: {latency_ms} ms"); /// // Will log a message like "loading took too long: 495 ms" /// // at WARN level on syslog. +/// # } /// ``` #[macro_export] macro_rules! warn { ($handle:expr, $($arg:tt)+) => { $crate::__log_internal!($handle, Warning, $($arg)+);}} @@ -108,10 +110,11 @@ /// # Example /// /// ```no_run -/// # let pam_handle: Box = unimplemented!(); +/// # fn _test(pam_handle: impl nonstick::PamShared) { /// nonstick::info!(pam_handle, "using remote backend to load user data"); /// // Will log a message like "using remote backend to load user data" /// // at INFO level on syslog. +/// # } /// ``` #[macro_export] macro_rules! info { ($handle:expr, $($arg:tt)+) => { $crate::__log_internal!($handle, Info, $($arg)+);}} @@ -124,12 +127,13 @@ /// # Example /// /// ```no_run -/// # let pam_handle: Box = unimplemented!(); +/// # fn _test(pam_handle: impl nonstick::PamShared) { /// # let userinfo_url = "https://zombo.com/"; /// nonstick::debug!(pam_handle, "making HTTP GET request to {userinfo_url}"); /// // Will log a message like /// // "pam_http/lib.rs:39:14: making HTTP GET request to https://zombo.com/" /// // at DEBUG level on syslog. +/// # } /// ``` #[macro_export] macro_rules! debug {($handle:expr, $($arg:tt)+) => {