# HG changeset patch # User Paul Fisher # Date 1750701824 14400 # Node ID 51c9d7e8261a6bc0a1b12cfdbf78938c505c46d5 # Parent db167f96ba4621263a36f30f4777f4fa1e9214ad Return owned strings rather than borrowed strings. It's going to be irritating to have to work with strings borrowed from the PAM handle rather than just using your own. They're cheap enough to copy. diff -r db167f96ba46 -r 51c9d7e8261a src/handle.rs --- a/src/handle.rs Mon Jun 23 13:04:27 2025 -0400 +++ b/src/handle.rs Mon Jun 23 14:03:44 2025 -0400 @@ -24,7 +24,7 @@ /// [man]: https://www.man7.org/linux/man-pages/man3/pam_get_item.3.html /// [adg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/adg-interface-by-app-expected.html#adg-pam_get_item /// [mwg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/mwg-expected-by-module-item.html#mwg-pam_get_item - fn $getter(&self) -> Result>; + fn $getter(&self) -> Result>; }; ($(#[$md:meta])* set = $setter:ident, item = $item:literal $(, see = $see:path)?) => { $(#[$md])* @@ -111,7 +111,7 @@ /// /// [man]: https://www.man7.org/linux/man-pages/man3/pam_get_user.3.html /// [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<&str>; + fn username(&mut self, prompt: Option<&str>) -> Result; trait_item!( /// The identity of the user for whom service is being requested. @@ -282,7 +282,7 @@ /// /// [man]: https://www.man7.org/linux/man-pages/man3/pam_get_authtok.3.html /// [mwg]: https://www.chiark.greenend.org.uk/doc/libpam-doc/html/mwg-expected-by-module-item.html#mwg-pam_get_item - fn authtok(&mut self, prompt: Option<&str>) -> Result<&str>; + fn authtok(&mut self, prompt: Option<&str>) -> Result; trait_item!( /// Gets the user's authentication token (e.g., password). diff -r db167f96ba46 -r 51c9d7e8261a src/libpam/handle.rs --- a/src/libpam/handle.rs Mon Jun 23 13:04:27 2025 -0400 +++ b/src/libpam/handle.rs Mon Jun 23 14:03:44 2025 -0400 @@ -65,7 +65,7 @@ /// Macro to implement getting/setting a CStr-based item. macro_rules! cstr_item { (get = $getter:ident, item = $item_type:path) => { - fn $getter(&self) -> Result> { + fn $getter(&self) -> Result> { unsafe { self.get_cstr_item($item_type) } } }; @@ -98,14 +98,14 @@ } } - fn username(&mut self, prompt: Option<&str>) -> Result<&str> { + fn username(&mut self, prompt: Option<&str>) -> Result { let prompt = memory::option_cstr(prompt)?; let mut output: *const c_char = ptr::null(); let ret = unsafe { pam_ffi::pam_get_user(self, &mut output, memory::prompt_ptr(prompt.as_ref())) }; ErrorCode::result_from(ret)?; - unsafe { memory::wrap_string(output) } + unsafe { memory::copy_pam_string(output) } .transpose() .unwrap_or(Err(ErrorCode::ConversationError)) } @@ -140,7 +140,7 @@ } impl PamHandleModule for LibPamHandle { - fn authtok(&mut self, prompt: Option<&str>) -> Result<&str> { + fn authtok(&mut self, prompt: Option<&str>) -> Result { let prompt = memory::option_cstr(prompt)?; let mut output: *const c_char = ptr::null_mut(); // SAFETY: We're calling this with known-good values. @@ -154,7 +154,7 @@ }; ErrorCode::result_from(res)?; // SAFETY: We got this string from PAM. - unsafe { memory::wrap_string(output) } + unsafe { memory::copy_pam_string(output) } .transpose() .unwrap_or(Err(ErrorCode::ConversationError)) } @@ -179,11 +179,11 @@ /// # Safety /// /// You better be requesting an item which is a C string. - unsafe fn get_cstr_item(&self, item_type: ItemType) -> Result> { + unsafe fn get_cstr_item(&self, item_type: ItemType) -> Result> { let mut output = ptr::null(); let ret = unsafe { pam_ffi::pam_get_item(self, item_type as c_int, &mut output) }; ErrorCode::result_from(ret)?; - memory::wrap_string(output.cast()) + memory::copy_pam_string(output.cast()) } /// Sets a C string item. @@ -235,7 +235,7 @@ } }; (get = $get:ident$(, set = $set:ident)?) => { - delegate!(fn $get(&self) -> Result>); + delegate!(fn $get(&self) -> Result>); $(delegate!(set = $set);)? }; (set = $set:ident) => { @@ -251,7 +251,7 @@ fn log(&self, level: Level, entry: &str) { self.handle.log(level, entry) } - delegate!(fn username(&mut self, prompt: Option<&str>) -> Result<&str>); + delegate!(fn username(&mut self, prompt: Option<&str>) -> Result); delegate!(get = user_item, set = set_user_item); delegate!(get = service, set = set_service); delegate!(get = user_prompt, set = set_user_prompt); diff -r db167f96ba46 -r 51c9d7e8261a src/libpam/memory.rs --- a/src/libpam/memory.rs Mon Jun 23 13:04:27 2025 -0400 +++ b/src/libpam/memory.rs Mon Jun 23 14:03:44 2025 -0400 @@ -52,22 +52,16 @@ /// # Safety /// /// It's on you to provide a valid string. -pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Result { - Ok(wrap_string(result_ptr)? - .map(String::from) - .unwrap_or_default()) -} - -/// Wraps a string returned from PAM as an `Option<&str>`. -pub unsafe fn wrap_string<'a>(data: *const c_char) -> Result> { - match NonNull::new(data.cast_mut()) { - Some(data) => Ok(Some( +pub unsafe fn copy_pam_string(result_ptr: *const c_char) -> Result> { + 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 => Ok(None), - } + ), + None => return Ok(None), + }; + Ok(borrowed.map(String::from)) } /// Allocates a string with the given contents on the C heap. @@ -192,11 +186,11 @@ malloc_str("hell\0 there").unwrap_err(); unsafe { let copied = copy_pam_string(str).unwrap(); - assert_eq!("hello there", copied); + assert_eq!("hello there", copied.unwrap()); zero_c_string(str); let idx_three = str.add(3).as_mut().unwrap(); *idx_three = 0x80u8 as i8; - let zeroed = copy_pam_string(str).unwrap(); + let zeroed = copy_pam_string(str).unwrap().unwrap(); assert!(zeroed.is_empty()); free(str); }