Mercurial > crates > nonstick
changeset 143:ebb71a412b58
Turn everything into OsString and Just Walk Out! for strings with nul.
To reduce the hazard surface of the API, this replaces most uses of &str
with &OsStr (and likewise with String/OsString).
Also, I've decided that instead of dealing with callers putting `\0`
in their parameters, I'm going to follow the example of std::env and
Just Walk Out! (i.e., panic!()).
This makes things a lot less annoying for both me and (hopefully) users.
author | Paul Fisher <paul@pfish.zone> |
---|---|
date | Sat, 05 Jul 2025 22:12:46 -0400 |
parents | 5c1e315c18ff |
children | 56b559b7ecea |
files | libpam-sys/libpam-sys-helpers/src/memory.rs src/conv.rs src/handle.rs src/libpam/answer.rs src/libpam/environ.rs src/libpam/handle.rs src/libpam/memory.rs src/libpam/module.rs src/libpam/question.rs |
diffstat | 9 files changed, 213 insertions(+), 191 deletions(-) [+] |
line wrap: on
line diff
--- a/libpam-sys/libpam-sys-helpers/src/memory.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/libpam-sys/libpam-sys-helpers/src/memory.rs Sat Jul 05 22:12:46 2025 -0400 @@ -371,11 +371,11 @@ let header: &Self = ptr.as_ref().unwrap_unchecked(); (&Self::buffer_of(ptr)[5..], header.data_type) } - + /// Zeroes out the data of this payload. - /// + /// /// # Safety - /// + /// /// - The pointer must point to a valid `BinaryPayload`. /// - The binary payload must not be used in the future, /// since its length metadata is gone and so its buffer is unknown. @@ -455,7 +455,10 @@ /// allocated by) [`Self::new`]. For instance, passing a pointer allocated /// by `malloc` to `OwnedBinaryPayload::<Vec<u8>>::from_ptr` is not allowed. pub unsafe fn from_ptr(ptr: NonNull<BinaryPayload>) -> Self { - Self(O::from_ptr(ptr.cast(), BinaryPayload::total_bytes(ptr.as_ptr()))) + Self(O::from_ptr( + ptr.cast(), + BinaryPayload::total_bytes(ptr.as_ptr()), + )) } }
--- a/src/conv.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/conv.rs Sat Jul 05 22:12:46 2025 -0400 @@ -5,6 +5,7 @@ use crate::constants::{ErrorCode, Result}; use std::cell::Cell; +use std::ffi::{OsStr, OsString}; use std::fmt; use std::fmt::Debug; use std::result::Result as StdResult; @@ -31,13 +32,15 @@ /// use nonstick::ErrorCode; /// /// fn cant_respond(message: Exchange) { + /// // "question" is kind of a bad name in the context of + /// // a one-way message, but it's for consistency. /// match message { /// Exchange::Info(i) => { - /// eprintln!("fyi, {}", i.question()); + /// eprintln!("fyi, {:?}", i.question()); /// i.set_answer(Ok(())) /// } /// Exchange::Error(e) => { - /// eprintln!("ERROR: {}", e.question()); + /// eprintln!("ERROR: {:?}", e.question()); /// e.set_answer(Ok(())) /// } /// // We can't answer any questions. @@ -118,7 +121,7 @@ /// A Q&A that asks the user for text and does not show it while typing. /// /// In other words, a password entry prompt. - MaskedQAndA<'a, Q=&'a str, A=String>, + MaskedQAndA<'a, Q=&'a OsStr, A=OsString>, Exchange::MaskedPrompt ); @@ -128,7 +131,7 @@ /// This is the normal "ask a person a question" prompt. /// When the user types, their input will be shown to them. /// It can be used for things like usernames. - QAndA<'a, Q=&'a str, A=String>, + QAndA<'a, Q=&'a OsStr, A=OsString>, Exchange::Prompt ); @@ -138,7 +141,7 @@ /// This message type is theoretically useful for "yes/no/maybe" /// questions, but nowhere in the documentation is it specified /// what the format of the answer will be, or how this should be shown. - RadioQAndA<'a, Q=&'a str, A=String>, + RadioQAndA<'a, Q=&'a OsStr, A=OsString>, Exchange::RadioPrompt ); @@ -203,7 +206,7 @@ /// While this does not have an answer, [`Conversation`] implementations /// should still call [`set_answer`][`QAndA::set_answer`] to verify that /// the message has been displayed (or actively discarded). - InfoMsg<'a, Q = &'a str, A = ()>, + InfoMsg<'a, Q = &'a OsStr, A = ()>, Exchange::Info ); @@ -213,7 +216,7 @@ /// While this does not have an answer, [`Conversation`] implementations /// should still call [`set_answer`][`QAndA::set_answer`] to verify that /// the message has been displayed (or actively discarded). - ErrorMsg<'a, Q = &'a str, A = ()>, + ErrorMsg<'a, Q = &'a OsStr, A = ()>, Exchange::Error ); @@ -286,10 +289,11 @@ /// /// ``` /// # use nonstick::{Conversation, Result}; +/// # use std::ffi::OsString; /// // Bring this trait into scope to get `masked_prompt`, among others. /// use nonstick::ConversationAdapter; /// -/// fn ask_for_token(convo: &impl Conversation) -> Result<String> { +/// fn ask_for_token(convo: &impl Conversation) -> Result<OsString> { /// convo.masked_prompt("enter your one-time token") /// } /// ``` @@ -299,6 +303,7 @@ /// ``` /// use nonstick::{Conversation, ConversationAdapter}; /// # use nonstick::{BinaryData, Result}; +/// # use std::ffi::{OsStr, OsString}; /// mod some_library { /// # use nonstick::Conversation; /// pub fn get_auth_data(conv: &impl Conversation) { /* ... */ @@ -310,23 +315,23 @@ /// /// impl ConversationAdapter for MySimpleConvo { /// // ... -/// # fn prompt(&self, request: &str) -> Result<String> { +/// # fn prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { /// # unimplemented!() /// # } /// # -/// # fn masked_prompt(&self, request: &str) -> Result<String> { +/// # fn masked_prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { /// # unimplemented!() /// # } /// # -/// # fn error_msg(&self, message: &str) { +/// # fn error_msg(&self, message: impl AsRef<OsStr>) { /// # unimplemented!() /// # } /// # -/// # fn info_msg(&self, message: &str) { +/// # fn info_msg(&self, message: impl AsRef<OsStr>) { /// # unimplemented!() /// # } /// # -/// # fn radio_prompt(&self, request: &str) -> Result<String> { +/// # fn radio_prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { /// # unimplemented!() /// # } /// # @@ -353,13 +358,13 @@ Demux(self) } /// Prompts the user for something. - fn prompt(&self, request: &str) -> Result<String>; + fn prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString>; /// Prompts the user for something, but hides what the user types. - fn masked_prompt(&self, request: &str) -> Result<String>; + fn masked_prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString>; /// Alerts the user to an error. - fn error_msg(&self, message: &str); + fn error_msg(&self, message: impl AsRef<OsStr>); /// Sends an informational message to the user. - fn info_msg(&self, message: &str); + fn info_msg(&self, message: impl AsRef<OsStr>); /// \[Linux extension] Prompts the user for a yes/no/maybe conditional. /// /// PAM documentation doesn't define the format of the response. @@ -368,7 +373,7 @@ /// this will return [`ErrorCode::ConversationError`]. /// If implemented on an implementation that doesn't support radio prompts, /// this will never be called. - fn radio_prompt(&self, request: &str) -> Result<String> { + fn radio_prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { let _ = request; Err(ErrorCode::ConversationError) } @@ -391,29 +396,33 @@ } macro_rules! conv_fn { - ($(#[$m:meta])* $fn_name:ident($($param:tt: $pt:ty),+) -> $resp_type:ty { $msg:ty }) => { + ($(#[$m:meta])* $fn_name:ident($param:tt: $pt:ty) -> $resp_type:ty { $msg:ty }) => { $(#[$m])* - fn $fn_name(&self, $($param: $pt),*) -> Result<$resp_type> { - let prompt = <$msg>::new($($param),*); + fn $fn_name(&self, $param: impl AsRef<$pt>) -> Result<$resp_type> { + let prompt = <$msg>::new($param.as_ref()); self.communicate(&[prompt.exchange()]); prompt.answer() } }; - ($(#[$m:meta])*$fn_name:ident($($param:tt: $pt:ty),+) { $msg:ty }) => { + ($(#[$m:meta])*$fn_name:ident($param:tt: $pt:ty) { $msg:ty }) => { $(#[$m])* - fn $fn_name(&self, $($param: $pt),*) { - self.communicate(&[<$msg>::new($($param),*).exchange()]); + fn $fn_name(&self, $param: impl AsRef<$pt>) { + self.communicate(&[<$msg>::new($param.as_ref()).exchange()]); } }; } impl<C: Conversation> ConversationAdapter for C { - conv_fn!(prompt(message: &str) -> String { QAndA }); - conv_fn!(masked_prompt(message: &str) -> String { MaskedQAndA } ); - conv_fn!(error_msg(message: &str) { ErrorMsg }); - conv_fn!(info_msg(message: &str) { InfoMsg }); - conv_fn!(radio_prompt(message: &str) -> String { RadioQAndA }); - conv_fn!(binary_prompt((data, data_type): (&[u8], u8)) -> BinaryData { BinaryQAndA }); + conv_fn!(prompt(message: OsStr) -> OsString { QAndA }); + conv_fn!(masked_prompt(message: OsStr) -> OsString { MaskedQAndA } ); + conv_fn!(error_msg(message: OsStr) { ErrorMsg }); + conv_fn!(info_msg(message: OsStr) { InfoMsg }); + conv_fn!(radio_prompt(message: OsStr) -> OsString { RadioQAndA }); + fn binary_prompt(&self, (data, typ): (&[u8], u8)) -> Result<BinaryData> { + let prompt = BinaryQAndA::new((data, typ)); + self.communicate(&[prompt.exchange()]); + prompt.answer() + } } /// A [`Conversation`] which asks the questions one at a time. @@ -459,7 +468,6 @@ #[cfg(test)] mod tests { use super::*; - use crate::constants::ErrorCode; #[test] fn test_demux() { @@ -470,28 +478,28 @@ } impl ConversationAdapter for DemuxTester { - fn prompt(&self, request: &str) -> Result<String> { - match request { - "what" => Ok("whatwhat".to_owned()), + fn prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { + match request.as_ref().to_str().unwrap() { + "what" => Ok("whatwhat".into()), "give_err" => Err(ErrorCode::PermissionDenied), _ => panic!("unexpected prompt!"), } } - fn masked_prompt(&self, request: &str) -> Result<String> { - assert_eq!("reveal", request); - Ok("my secrets".to_owned()) + fn masked_prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { + assert_eq!("reveal", request.as_ref()); + Ok("my secrets".into()) } - fn error_msg(&self, message: &str) { + fn error_msg(&self, message: impl AsRef<OsStr>) { self.error_ran.set(true); - assert_eq!("whoopsie", message); + assert_eq!("whoopsie", message.as_ref()); } - fn info_msg(&self, message: &str) { + fn info_msg(&self, message: impl AsRef<OsStr>) { self.info_ran.set(true); - assert_eq!("did you know", message); + assert_eq!("did you know", message.as_ref()); } - fn radio_prompt(&self, request: &str) -> Result<String> { - assert_eq!("channel?", request); - Ok("zero".to_owned()) + fn radio_prompt(&self, request: impl AsRef<OsStr>) -> Result<OsString> { + assert_eq!("channel?", request.as_ref()); + Ok("zero".into()) } fn binary_prompt(&self, data_and_type: (&[u8], u8)) -> Result<BinaryData> { assert_eq!((&[10, 9, 8][..], 66), data_and_type); @@ -501,11 +509,11 @@ let tester = DemuxTester::default(); - let what = QAndA::new("what"); - let pass = MaskedQAndA::new("reveal"); - let err = ErrorMsg::new("whoopsie"); - let info = InfoMsg::new("did you know"); - let has_err = QAndA::new("give_err"); + let what = QAndA::new("what".as_ref()); + let pass = MaskedQAndA::new("reveal".as_ref()); + let err = ErrorMsg::new("whoopsie".as_ref()); + let info = InfoMsg::new("did you know".as_ref()); + let has_err = QAndA::new("give_err".as_ref()); let conv = tester.into_conversation(); @@ -532,7 +540,7 @@ { let conv = tester.into_conversation(); - let radio = RadioQAndA::new("channel?"); + let radio = RadioQAndA::new("channel?".as_ref()); let bin = BinaryQAndA::new((&[10, 9, 8], 66)); conv.communicate(&[radio.exchange(), bin.exchange()]); @@ -556,11 +564,13 @@ assert_eq!("oh no", error.question()); error.set_answer(Ok(())) } - Exchange::Prompt(prompt) => prompt.set_answer(match prompt.question() { - "should_err" => Err(ErrorCode::PermissionDenied), - "question" => Ok("answer".to_owned()), - other => panic!("unexpected question {other:?}"), - }), + Exchange::Prompt(prompt) => { + prompt.set_answer(match prompt.question().to_str().unwrap() { + "should_err" => Err(ErrorCode::PermissionDenied), + "question" => Ok("answer".into()), + other => panic!("unexpected question {other:?}"), + }) + } Exchange::MaskedPrompt(ask) => { assert_eq!("password!", ask.question()); ask.set_answer(Ok("open sesame".into())) @@ -571,7 +581,7 @@ } Exchange::RadioPrompt(ask) => { assert_eq!("radio?", ask.question()); - ask.set_answer(Ok("yes".to_owned())) + ask.set_answer(Ok("yes".into())) } } } else {
--- a/src/handle.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/handle.rs Sat Jul 05 22:12:46 2025 -0400 @@ -5,6 +5,7 @@ use crate::environ::{EnvironMap, EnvironMapMut}; use crate::logging::{Level, Location}; use crate::{guide, linklist, man7, manbsd, stdlinks}; +use std::ffi::{OsStr, OsString}; macro_rules! trait_item { ($(#[$md:meta])* get = $getter:ident, item = $item:literal $(, see = $see:path)?) => { @@ -26,7 +27,7 @@ #[doc = guide!(adg: "adg-interface-by-app-expected.html#adg-pam_get_item")] #[doc = guide!(mwg: "mwg-expected-by-module-item.html#mwg-pam_get_item")] #[doc = stdlinks!(3 pam_get_item)] - fn $getter(&self) -> Result<Option<String>>; + fn $getter(&self) -> Result<Option<OsString>>; }; ($(#[$md:meta])* set = $setter:ident, item = $item:literal $(, see = $see:path)?) => { $(#[$md])* @@ -37,8 +38,10 @@ )? /// /// Sets the item's value. PAM copies the string's contents. - /// If the string contains a null byte, this will return - /// a `ConversationError`. + /// + /// # Panics + /// + /// If the string contains a nul byte, this will panic. /// /// # References /// @@ -47,7 +50,7 @@ #[doc = guide!(adg: "adg-interface-by-app-expected.html#adg-pam_set_item")] #[doc = guide!(mwg: "mwg-expected-by-module-item.html#mwg-pam_set_item")] #[doc = stdlinks!(3 pam_set_item)] - fn $setter(&mut self, value: Option<&str>) -> Result<()>; + fn $setter(&mut self, value: Option<&OsStr>) -> Result<()>; }; } @@ -116,13 +119,13 @@ /// // Get the username using a custom prompt. /// // If this were actually called right after the above, /// // both user and user_2 would have the same value. - /// let user_2 = handle.username(Some("who ARE you even???"))?; + /// let user_2 = handle.username(Some("who ARE you even???".as_ref()))?; /// # Ok(()) /// # } /// ``` #[doc = stdlinks!(3 pam_get_user)] #[doc = guide!(mwg: "mwg-expected-by-module-item.html#mwg-pam_get_user")] - fn username(&mut self, prompt: Option<&str>) -> Result<String>; + fn username(&mut self, prompt: Option<&OsStr>) -> Result<OsString>; /// The contents of the environment to set, read-only. fn environ(&self) -> impl EnvironMap; @@ -331,18 +334,18 @@ /// // Get the user's password using the default prompt. /// let pass = handle.authtok(None)?; /// // Get the user's password using a custom prompt. - /// let pass = handle.authtok(Some("Reveal your secrets!"))?; + /// let pass = handle.authtok(Some("Reveal your secrets!".as_ref()))?; /// Ok(()) /// # } /// ``` #[doc = man7!(3 pam_get_authtok)] #[doc = manbsd!(3 pam_get_authtok)] - fn authtok(&mut self, prompt: Option<&str>) -> Result<String>; + fn authtok(&mut self, prompt: Option<&OsStr>) -> Result<OsString>; /// Retrieves the user's old authentication token when changing passwords. /// /// - fn old_authtok(&mut self, prompt: Option<&str>) -> Result<String>; + fn old_authtok(&mut self, prompt: Option<&OsStr>) -> Result<OsString>; trait_item!( /// Gets the user's authentication token (e.g., password).
--- a/src/libpam/answer.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/libpam/answer.rs Sat Jul 05 22:12:46 2025 -0400 @@ -5,9 +5,10 @@ use crate::libpam::memory::{CHeapBox, CHeapPayload, CHeapString, Immovable}; use crate::{ErrorCode, Result}; use libpam_sys_helpers::memory::BinaryPayload; -use std::ffi::{c_int, c_void, CStr}; +use std::ffi::{c_int, c_void, CStr, OsStr}; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; +use std::os::unix::ffi::OsStrExt; use std::ptr::NonNull; use std::{iter, ptr, slice}; @@ -31,10 +32,14 @@ // all allocated answer memory. for (input, output) in iter::zip(value, outputs.iter_mut()) { match input { - OwnedExchange::MaskedPrompt(p) => TextAnswer::fill(output, p.answer()?.as_ref())?, - OwnedExchange::Prompt(p) => TextAnswer::fill(output, &(p.answer()?))?, - OwnedExchange::Error(p) => TextAnswer::fill(output, p.answer().map(|_| "")?)?, - OwnedExchange::Info(p) => TextAnswer::fill(output, p.answer().map(|_| "")?)?, + OwnedExchange::MaskedPrompt(p) => TextAnswer::fill(output, &p.answer()?)?, + OwnedExchange::Prompt(p) => TextAnswer::fill(output, &p.answer()?)?, + OwnedExchange::Error(p) => { + TextAnswer::fill(output, p.answer().map(|_| "".as_ref())?)? + } + OwnedExchange::Info(p) => { + TextAnswer::fill(output, p.answer().map(|_| "".as_ref())?)? + } // If we're here, that means that we *got* a Linux-PAM // question from PAM, so we're OK to answer it. OwnedExchange::RadioPrompt(p) => TextAnswer::fill(output, &(p.answer()?))?, @@ -98,16 +103,15 @@ /// Generic version of answer data. /// -/// This has the same structure as [`BinaryAnswer`](crate::libpam::answer::BinaryAnswer) -/// and [`TextAnswer`](crate::libpam::answer::TextAnswer). +/// This has the same structure as [`BinaryAnswer`] and [`TextAnswer`]. #[repr(C)] #[derive(Debug, Default)] pub struct Answer { /// Owned pointer to the data returned in an answer. /// For most answers, this will be a - /// [`CHeapString`](crate::libpam::memory::CHeapString), - /// but for [`BinaryQAndA`](crate::conv::BinaryQAndA)s - /// (a Linux-PAM extension), this will be a [`CHeapBox`] of + /// [`CHeapString`](CHeapString), but for + /// [`BinaryQAndA`](crate::conv::BinaryQAndA)s (a Linux-PAM extension), + /// this will be a [`CHeapBox`] of /// [`CBinaryData`](crate::libpam::memory::CBinaryData). pub data: Option<CHeapBox<c_void>>, /// Unused. Just here for the padding. @@ -131,8 +135,8 @@ } /// Converts the `Answer` to a `TextAnswer` with the given text. - fn fill(dest: &mut Answer, text: &str) -> Result<()> { - let allocated = CHeapString::new(text)?; + fn fill(dest: &mut Answer, text: &OsStr) -> Result<()> { + let allocated = CHeapString::new(text.as_bytes()); let _ = dest .data .replace(unsafe { CHeapBox::cast(allocated.into_box()) }); @@ -237,7 +241,7 @@ macro_rules! answered { ($typ:ty, $msg:path, $data:expr) => {{ - let qa = <$typ>::new(""); + let qa = <$typ>::new("".as_ref()); qa.set_answer(Ok($data)); $msg(qa) }}; @@ -250,16 +254,16 @@ assert_eq!("", up.contents().unwrap()); } - fn round_trip(msgs: Vec<OwnedExchange>) -> Answers { - let n = msgs.len(); - let sent = Answers::build(msgs).unwrap(); + fn round_trip(exchanges: Vec<OwnedExchange>) -> Answers { + let n = exchanges.len(); + let sent = Answers::build(exchanges).unwrap(); unsafe { Answers::from_c_heap(NonNull::new_unchecked(sent.into_ptr()), n) } } #[test] fn test_round_trip() { let mut answers = round_trip(vec![ - answered!(QAndA, OwnedExchange::Prompt, "whats going on".to_owned()), + answered!(QAndA, OwnedExchange::Prompt, "whats going on".into()), answered!(MaskedQAndA, OwnedExchange::MaskedPrompt, "well then".into()), answered!(ErrorMsg, OwnedExchange::Error, ()), answered!(InfoMsg, OwnedExchange::Info, ()), @@ -285,11 +289,7 @@ }; let mut answers = round_trip(vec![ binary_msg, - answered!( - RadioQAndA, - OwnedExchange::RadioPrompt, - "beep boop".to_owned() - ), + answered!(RadioQAndA, OwnedExchange::RadioPrompt, "beep boop".into()), ]); if let [bin, radio] = &mut answers[..] { @@ -307,13 +307,19 @@ #[test] fn test_text_answer() { let mut answer: CHeapBox<Answer> = CHeapBox::default(); - TextAnswer::fill(&mut answer, "hello").unwrap(); + TextAnswer::fill(&mut answer, "hello".as_ref()).unwrap(); let zeroth_text = unsafe { TextAnswer::upcast(&mut answer) }; let data = zeroth_text.contents().expect("valid"); assert_eq!("hello", data); zeroth_text.zero_contents(); zeroth_text.zero_contents(); - TextAnswer::fill(&mut answer, "hell\0").expect_err("should error; contains nul"); + } + + #[test] + #[should_panic] + fn test_text_answer_nul() { + TextAnswer::fill(&mut CHeapBox::default(), "hell\0".as_ref()) + .expect_err("should error; contains nul"); } #[test]
--- a/src/libpam/environ.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/libpam/environ.rs Sat Jul 05 22:12:46 2025 -0400 @@ -199,7 +199,7 @@ #[test] fn test_split_kv() { fn test(input: &str, key: &str, value: &str) { - let data = CHeapString::new(input).unwrap(); + let data = CHeapString::new(input); let key = os(key); let value = os(value); @@ -216,10 +216,7 @@ let ptrs: NonNull<Option<CHeapString>> = memory::calloc(strings.len() + 1); unsafe { for (idx, &text) in strings.iter().enumerate() { - ptr::write( - ptrs.as_ptr().add(idx), - Some(CHeapString::new(text).unwrap()), - ) + ptr::write(ptrs.as_ptr().add(idx), Some(CHeapString::new(text))) } ptr::write(ptrs.as_ptr().add(strings.len()), None); EnvList::from_ptr(ptrs.cast())
--- a/src/libpam/handle.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/libpam/handle.rs Sat Jul 05 22:12:46 2025 -0400 @@ -13,8 +13,9 @@ use libpam_sys_helpers::constants; use num_enum::{IntoPrimitive, TryFromPrimitive}; use std::cell::Cell; -use std::ffi::{c_char, c_int, CString}; +use std::ffi::{c_char, c_int, CString, OsStr, OsString}; use std::mem::ManuallyDrop; +use std::os::unix::ffi::OsStrExt; use std::ptr; use std::ptr::NonNull; @@ -36,20 +37,20 @@ #[derive(Debug, PartialEq)] pub struct HandleBuilder { - service_name: String, - username: Option<String>, + service_name: OsString, + username: Option<OsString>, } impl HandleBuilder { /// Updates the service name. - pub fn service_name(mut self, service_name: String) -> Self { + pub fn service_name(mut self, service_name: OsString) -> Self { self.service_name = service_name; self } /// Sets the username. Setting this will avoid the need for an extra /// round trip through the conversation and may otherwise improve /// the login experience. - pub fn username(mut self, username: String) -> Self { + pub fn username(mut self, username: OsString) -> Self { self.username = Some(username); self } @@ -71,17 +72,18 @@ /// #[doc = stdlinks!(3 pam_start)] #[doc = guide!(adg: "adg-interface-by-app-expected.html#adg-pam_start")] - pub fn build_with_service(service_name: String) -> HandleBuilder { + pub fn build_with_service(service_name: OsString) -> HandleBuilder { HandleBuilder { service_name, username: None, } } - fn start(service_name: String, username: Option<String>, conversation: C) -> Result<Self> { + fn start(service_name: OsString, username: Option<OsString>, conversation: C) -> Result<Self> { let conv = Box::new(OwnedConversation::new(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()); + 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(); // SAFETY: We've set everything up properly to call `pam_start`. @@ -191,11 +193,11 @@ }; // Then have item getters / setters (get = $get:ident$(, set = $set:ident)?) => { - delegate!(fn $get(&self) -> Result<Option<String>>); + delegate!(fn $get(&self) -> Result<Option<OsString>>); $(delegate!(set = $set);)? }; (set = $set:ident) => { - delegate!(fn $set(&mut self, value: Option<&str>) -> Result<()>); + delegate!(fn $set(&mut self, value: Option<&OsStr>) -> Result<()>); }; } @@ -207,7 +209,7 @@ delegate!(fn log(&self, level: Level, location: Location<'_>, 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<String>); + delegate!(fn username(&mut self, prompt: Option<&OsStr>) -> Result<OsString>); delegate!(get = user_item, set = set_user_item); delegate!(get = service, set = set_service); delegate!(get = user_prompt, set = set_user_prompt); @@ -221,12 +223,12 @@ /// Macro to implement getting/setting a CStr-based item. macro_rules! cstr_item { (get = $getter:ident, item = $item_type:path) => { - fn $getter(&self) -> Result<Option<String>> { + fn $getter(&self) -> Result<Option<OsString>> { unsafe { self.get_cstr_item($item_type) } } }; (set = $setter:ident, item = $item_type:path) => { - fn $setter(&mut self, value: Option<&str>) -> Result<()> { + fn $setter(&mut self, value: Option<&OsStr>) -> Result<()> { unsafe { self.set_cstr_item($item_type, value) } } }; @@ -328,7 +330,7 @@ Ok(cstr) => cstr, _ => return, }; - #[cfg(pam_impl = "linux-pam")] + #[cfg(pam_impl = "LinuxPam")] { _ = loc; // SAFETY: We're calling this function with a known value. @@ -336,7 +338,7 @@ libpam_sys::pam_syslog(self, level as c_int, "%s\0".as_ptr().cast(), entry.as_ptr()) } } - #[cfg(pam_impl = "openpam")] + #[cfg(pam_impl = "OpenPam")] { let func = CString::new(loc.function).unwrap_or(CString::default()); // SAFETY: We're calling this function with a known value. @@ -353,20 +355,18 @@ fn log(&self, _level: Level, _loc: Location<'_>, _entry: &str) {} - fn username(&mut self, prompt: Option<&str>) -> Result<String> { - let prompt = memory::option_cstr(prompt)?; + fn username(&mut self, prompt: Option<&OsStr>) -> Result<OsString> { + let prompt = memory::option_cstr_os(prompt); let mut output: *const c_char = ptr::null(); let ret = unsafe { libpam_sys::pam_get_user( self.raw_mut(), &mut output, - memory::prompt_ptr(prompt.as_ref()), + memory::prompt_ptr(prompt.as_deref()), ) }; ErrorCode::result_from(ret)?; - unsafe { memory::copy_pam_string(output) } - .transpose() - .unwrap_or(Err(ErrorCode::ConversationError)) + Ok(unsafe { memory::copy_pam_string(output).ok_or(ErrorCode::ConversationError)? }) } fn environ(&self) -> impl EnvironMap { @@ -407,11 +407,11 @@ } impl PamHandleModule for RawPamHandle { - fn authtok(&mut self, prompt: Option<&str>) -> Result<String> { + fn authtok(&mut self, prompt: Option<&OsStr>) -> Result<OsString> { self.get_authtok(prompt, ItemType::AuthTok) } - fn old_authtok(&mut self, prompt: Option<&str>) -> Result<String> { + fn old_authtok(&mut self, prompt: Option<&OsStr>) -> Result<OsString> { self.get_authtok(prompt, ItemType::OldAuthTok) } @@ -432,8 +432,8 @@ // Implementations of internal functions. impl RawPamHandle { #[cfg(any(pam_impl = "LinuxPam", pam_impl = "OpenPam"))] - fn get_authtok(&mut self, prompt: Option<&str>, item_type: ItemType) -> Result<String> { - let prompt = memory::option_cstr(prompt)?; + fn get_authtok(&mut self, prompt: Option<&OsStr>, item_type: ItemType) -> Result<OsString> { + let prompt = memory::option_cstr_os(prompt); let mut output: *const c_char = ptr::null_mut(); // SAFETY: We're calling this with known-good values. let res = unsafe { @@ -441,14 +441,12 @@ self.raw_mut(), item_type.into(), &mut output, - memory::prompt_ptr(prompt.as_ref()), + memory::prompt_ptr(prompt.as_deref()), ) }; ErrorCode::result_from(res)?; // SAFETY: We got this string from PAM. - unsafe { memory::copy_pam_string(output) } - .transpose() - .unwrap_or(Err(ErrorCode::ConversationError)) + unsafe { memory::copy_pam_string(output) }.ok_or(ErrorCode::ConversationError) } #[cfg(not(any(pam_impl = "LinuxPam", pam_impl = "OpenPam")))] @@ -461,12 +459,12 @@ /// # Safety /// /// You better be requesting an item which is a C string. - unsafe fn get_cstr_item(&self, item_type: ItemType) -> Result<Option<String>> { + unsafe fn get_cstr_item(&self, item_type: ItemType) -> Result<Option<OsString>> { let mut output = ptr::null(); let ret = unsafe { libpam_sys::pam_get_item(self.raw_ref(), item_type as c_int, &mut output) }; ErrorCode::result_from(ret)?; - memory::copy_pam_string(output.cast()) + Ok(memory::copy_pam_string(output.cast())) } /// Sets a C string item. @@ -474,13 +472,13 @@ /// # Safety /// /// You better be setting an item which is a C string. - unsafe fn set_cstr_item(&mut self, item_type: ItemType, data: Option<&str>) -> Result<()> { - let data_str = memory::option_cstr(data)?; + unsafe fn set_cstr_item(&mut self, item_type: ItemType, data: Option<&OsStr>) -> Result<()> { + let data_str = memory::option_cstr_os(data); let ret = unsafe { libpam_sys::pam_set_item( self.raw_mut(), item_type as c_int, - memory::prompt_ptr(data_str.as_ref()).cast(), + memory::prompt_ptr(data_str.as_deref()).cast(), ) }; ErrorCode::result_from(ret)
--- a/src/libpam/memory.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/libpam/memory.rs Sat Jul 05 22:12:46 2025 -0400 @@ -1,12 +1,11 @@ //! Things for dealing with memory. -use crate::ErrorCode; -use crate::Result; use libpam_sys_helpers::memory::{Buffer, OwnedBinaryPayload}; -use std::ffi::{c_char, CStr, CString}; +use std::ffi::{c_char, CStr, CString, OsStr, OsString}; use std::marker::{PhantomData, PhantomPinned}; use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut}; +use std::os::unix::ffi::{OsStrExt, OsStringExt}; use std::ptr::NonNull; use std::{mem, ptr, slice}; @@ -33,15 +32,16 @@ pub struct Immovable(pub PhantomData<(*mut u8, PhantomPinned)>); /// Safely converts a `&str` option to a `CString` option. -pub fn option_cstr(prompt: Option<&str>) -> Result<Option<CString>> { - prompt - .map(CString::new) - .transpose() - .map_err(|_| ErrorCode::ConversationError) +pub fn option_cstr(prompt: Option<&[u8]>) -> Option<CString> { + prompt.map(|p| CString::new(p).expect("nul is not allowed")) +} + +pub fn option_cstr_os(prompt: Option<&OsStr>) -> Option<CString> { + option_cstr(prompt.map(OsStr::as_bytes)) } /// Gets the pointer to the given CString, or a null pointer if absent. -pub fn prompt_ptr(prompt: Option<&CString>) -> *const c_char { +pub fn prompt_ptr(prompt: Option<&CStr>) -> *const c_char { match prompt { Some(c_str) => c_str.as_ptr(), None => ptr::null(), @@ -168,10 +168,10 @@ impl CHeapString { /// Creates a new C heap string with the given contents. - pub fn new(text: &str) -> Result<Self> { - let data = text.as_bytes(); + pub fn new(text: impl AsRef<[u8]>) -> Self { + let data = text.as_ref(); if data.contains(&0) { - return Err(ErrorCode::ConversationError); + panic!("you're not allowed to create a cstring with a nul inside!"); } // +1 for the null terminator let data_alloc: NonNull<c_char> = calloc(data.len() + 1); @@ -179,7 +179,7 @@ unsafe { let dest = slice::from_raw_parts_mut(data_alloc.as_ptr().cast(), data.len()); dest.copy_from_slice(data); - Ok(Self(CHeapBox::from_ptr(data_alloc))) + Self(CHeapBox::from_ptr(data_alloc)) } } @@ -247,16 +247,13 @@ /// # Safety /// /// It's on you to provide a valid string. -pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Result<Option<String>> { - let borrowed = match NonNull::new(result_ptr.cast_mut()) { - Some(data) => Some( - CStr::from_ptr(data.as_ptr()) - .to_str() - .map_err(|_| ErrorCode::ConversationError)?, - ), - None => return Ok(None), - }; - Ok(borrowed.map(String::from)) +pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Option<OsString> { + NonNull::new(result_ptr.cast_mut()) + .map(NonNull::as_ptr) + .map(|p| CStr::from_ptr(p)) + .map(CStr::to_bytes) + .map(Vec::from) + .map(OsString::from_vec) } #[cfg(test)] @@ -285,37 +282,46 @@ drop(dropbox); assert_eq!(3, drop_count.get()); } + #[test] fn test_strings() { - let str = CHeapString::new("hello there").unwrap(); + let str = CHeapString::new("hello there"); let str_ptr = str.into_ptr().as_ptr(); - CHeapString::new("hell\0 there").unwrap_err(); unsafe { let copied = copy_pam_string(str_ptr).unwrap(); - assert_eq!("hello there", copied.unwrap()); + assert_eq!("hello there", copied); 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_ptr).unwrap().unwrap(); + let zeroed = copy_pam_string(str_ptr).unwrap(); assert!(zeroed.is_empty()); let _ = CHeapString::from_ptr(str_ptr); } } #[test] + #[should_panic] + fn test_nul_string() { + CHeapString::new("hell\0 there"); + } + + #[test] fn test_option_str() { - let good = option_cstr(Some("whatever")).unwrap(); + let good = option_cstr(Some("whatever".as_ref())); assert_eq!("whatever", good.unwrap().to_str().unwrap()); - let no_str = option_cstr(None).unwrap(); + let no_str = option_cstr(None); assert!(no_str.is_none()); - let bad_str = option_cstr(Some("what\0ever")).unwrap_err(); - assert_eq!(ErrorCode::ConversationError, bad_str); + } + #[test] + #[should_panic] + fn test_nul_cstr() { + option_cstr(Some("what\0ever".as_ref())); } #[test] fn test_prompt() { let prompt_cstr = CString::new("good").ok(); - let prompt = prompt_ptr(prompt_cstr.as_ref()); + let prompt = prompt_ptr(prompt_cstr.as_deref()); assert!(!prompt.is_null()); let no_prompt = prompt_ptr(None); assert!(no_prompt.is_null());
--- a/src/libpam/module.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/libpam/module.rs Sat Jul 05 22:12:46 2025 -0400 @@ -22,7 +22,7 @@ /// /// impl<T: PamHandleModule> PamModule<T> for MyPamModule { /// fn authenticate(handle: &mut T, args: Vec<&CStr>, flags: Flags) -> PamResult<()> { -/// let password = handle.authtok(Some("what's your password?"))?; +/// let password = handle.authtok(Some("what's your password?".as_ref()))?; /// let response = /// format!("If you say your password is {password:?}, who am I to disagree?"); /// handle.info_msg(&response); @@ -31,7 +31,7 @@ /// /// fn account_management(handle: &mut T, args: Vec<&CStr>, flags: Flags) -> PamResult<()> { /// let username = handle.username(None)?; -/// let response = format!("Hello {username}! I trust you unconditionally."); +/// let response = format!("Hello {username:?}! I trust you unconditionally."); /// handle.info_msg(&response); /// Ok(()) /// }
--- a/src/libpam/question.rs Sat Jul 05 21:49:27 2025 -0400 +++ b/src/libpam/question.rs Sat Jul 05 22:12:46 2025 -0400 @@ -5,9 +5,10 @@ use crate::libpam::memory; use crate::ErrorCode; use crate::Result; -use libpam_sys_helpers::memory as pammem; +use libpam_sys_helpers::memory as pam_mem; use num_enum::{IntoPrimitive, TryFromPrimitive}; -use std::ffi::{c_int, c_void, CStr}; +use std::ffi::{c_int, c_void, CStr, OsStr}; +use std::os::unix::ffi::OsStrExt; use std::ptr::NonNull; mod style_const { @@ -69,12 +70,10 @@ /// # Safety /// /// It's up to you to pass this only on types with a string value. - unsafe fn string_data(&self) -> Result<&str> { + unsafe fn string_data(&self) -> &OsStr { match self.data.as_ref() { - None => Ok(""), - Some(data) => CStr::from_ptr(data.as_ptr().cast()) - .to_str() - .map_err(|_| ErrorCode::ConversationError), + None => "".as_ref(), + Some(data) => OsStr::from_bytes(CStr::from_ptr(data.as_ptr().cast()).to_bytes()), } } @@ -82,7 +81,7 @@ unsafe fn binary_data(&self) -> (&[u8], u8) { self.data .as_ref() - .map(|data| pammem::BinaryPayload::contents(data.as_ptr().cast())) + .map(|data| pam_mem::BinaryPayload::contents(data.as_ptr().cast())) .unwrap_or_default() } } @@ -90,9 +89,9 @@ impl TryFrom<&Exchange<'_>> for Question { type Error = ErrorCode; fn try_from(msg: &Exchange) -> Result<Self> { - let alloc = |style, text| -> Result<_> { + let alloc = |style, text: &OsStr| -> Result<_> { Ok((style, unsafe { - memory::CHeapBox::cast(memory::CHeapString::new(text)?.into_box()) + memory::CHeapBox::cast(memory::CHeapString::new(text.as_bytes()).into_box()) })) }; // We will only allocate heap data if we have a valid input. @@ -136,7 +135,7 @@ Style::BinaryPrompt => self .data .as_mut() - .map(|p| pammem::BinaryPayload::zero(p.as_ptr().cast())), + .map(|p| pam_mem::BinaryPayload::zero(p.as_ptr().cast())), #[cfg(feature = "linux-pam-ext")] Style::RadioType => self .data @@ -168,14 +167,14 @@ let prompt = unsafe { match style { Style::PromptEchoOff => { - Self::MaskedPrompt(MaskedQAndA::new(question.string_data()?)) + Self::MaskedPrompt(MaskedQAndA::new(question.string_data())) } - Style::PromptEchoOn => Self::Prompt(QAndA::new(question.string_data()?)), - Style::ErrorMsg => Self::Error(ErrorMsg::new(question.string_data()?)), - Style::TextInfo => Self::Info(InfoMsg::new(question.string_data()?)), + Style::PromptEchoOn => Self::Prompt(QAndA::new(question.string_data())), + Style::ErrorMsg => Self::Error(ErrorMsg::new(question.string_data())), + Style::TextInfo => Self::Info(InfoMsg::new(question.string_data())), #[cfg(feature = "linux-pam-ext")] Style::RadioType => { - Self::RadioPrompt(crate::conv::RadioQAndA::new(question.string_data()?)) + Self::RadioPrompt(crate::conv::RadioQAndA::new(question.string_data())) } #[cfg(feature = "linux-pam-ext")] Style::BinaryPrompt => { @@ -188,8 +187,8 @@ } #[cfg(feature = "linux-pam-ext")] -impl From<pammem::TooBigError> for ErrorCode { - fn from(_: pammem::TooBigError) -> Self { +impl From<pam_mem::TooBigError> for ErrorCode { + fn from(_: pam_mem::TooBigError) -> Self { ErrorCode::BufferError } } @@ -219,12 +218,12 @@ fn standard() { assert_matches!( (Exchange::MaskedPrompt, "hocus pocus"), - MaskedQAndA::new("hocus pocus") + MaskedQAndA::new("hocus pocus".as_ref()) ); - assert_matches!((Exchange::Prompt, "what"), QAndA::new("what")); - assert_matches!((Exchange::Prompt, "who"), QAndA::new("who")); - assert_matches!((Exchange::Info, "hey"), InfoMsg::new("hey")); - assert_matches!((Exchange::Error, "gasp"), ErrorMsg::new("gasp")); + assert_matches!((Exchange::Prompt, "what"), QAndA::new("what".as_ref())); + assert_matches!((Exchange::Prompt, "who"), QAndA::new("who".as_ref())); + assert_matches!((Exchange::Info, "hey"), InfoMsg::new("hey".as_ref())); + assert_matches!((Exchange::Error, "gasp"), ErrorMsg::new("gasp".as_ref())); } #[test] @@ -237,7 +236,7 @@ ); assert_matches!( (Exchange::RadioPrompt, "you must choose"), - RadioQAndA::new("you must choose") + RadioQAndA::new("you must choose".as_ref()) ); } }