pam: Some cleanups in Converse system

- Take an initialized Converse as argument to PamSession::start.
- Save a few copies of username/password strings in PasswordConv.
- Free strdup'd strings on error in the pam_conversation callback.

greetd/src/pam/converse.rs

@@ -15,21 +15,16 @@ pub trait Converse {
     ///
     /// This would typically be the username. The exact question is provided as the
     /// `msg` argument if you wish to display it to your user.
-    fn prompt_echo(&mut self, msg: &CStr) -> ::std::result::Result<CString, ()>;
+    fn prompt_echo<'a>(&'a mut self, msg: &CStr) -> ::std::result::Result<&'a CStr, ()>;
     /// PAM requests a value that should be typed blindly by the user
     ///
     /// This would typically be the password. The exact question is provided as the
     /// `msg` argument if you wish to display it to your user.
-    fn prompt_blind(&mut self, msg: &CStr) -> ::std::result::Result<CString, ()>;
+    fn prompt_blind<'a>(&'a mut self, msg: &CStr) -> ::std::result::Result<&'a CStr, ()>;
     /// This is an informational message from PAM
     fn info(&mut self, msg: &CStr);
     /// This is an error message from PAM
     fn error(&mut self, msg: &CStr);
-    /// Get the username that is being authenticated
-    ///
-    /// This method is not a PAM callback, but is rather used by the `Authenticator` to
-    /// setup the environment when opening a session.
-    fn username(&self) -> &str;
 }
 
 /// A minimalistic conversation handler, that uses given login and password
@@ -37,38 +32,29 @@ pub trait Converse {
 /// This conversation handler is not really interactive, but simply returns to
 /// PAM the value that have been set using the `set_credentials` method.
 pub struct PasswordConv {
-    login: String,
-    passwd: String,
+    login: CString,
+    passwd: CString,
 }
 
 impl PasswordConv {
     /// Create a new `PasswordConv` handler
-    pub fn new() -> PasswordConv {
+    pub fn new(login: &str, password: &str) -> PasswordConv {
         PasswordConv {
-            login: String::new(),
-            passwd: String::new(),
+            login: CString::new(login).unwrap(),
+            passwd: CString::new(password).unwrap(),
         }
     }
-
-    /// Set the credentials that this handler will provide to PAM
-    pub fn set_credentials<U: Into<String>, V: Into<String>>(&mut self, login: U, password: V) {
-        self.login = login.into();
-        self.passwd = password.into();
-    }
 }
 
 impl Converse for PasswordConv {
-    fn prompt_echo(&mut self, _msg: &CStr) -> Result<CString, ()> {
-        CString::new(self.login.clone()).map_err(|_| ())
+    fn prompt_echo<'a>(&'a mut self, _msg: &CStr) -> Result<&'a CStr, ()> {
+        Ok(&self.login)
     }
-    fn prompt_blind(&mut self, _msg: &CStr) -> Result<CString, ()> {
-        CString::new(self.passwd.clone()).map_err(|_| ())
+    fn prompt_blind<'a>(&'a mut self, _msg: &CStr) -> Result<&'a CStr, ()> {
+        Ok(&self.passwd)
     }
     fn info(&mut self, _msg: &CStr) {}
     fn error(&mut self, msg: &CStr) {
         eprintln!("[PAM ERROR] {}", msg.to_string_lossy());
     }
-    fn username(&self) -> &str {
-        &self.login
-    }
 }

greetd/src/pam/ffi.rs

@@ -31,35 +31,28 @@ pub extern "C" fn converse<C: Converse>(
 
     let mut result: PamReturnCode = PamReturnCode::SUCCESS;
     for i in 0..num_msg as isize {
-        unsafe {
-            // get indexed values
-            let m: &mut PamMessage = &mut **(msg.offset(i));
-            let r: &mut PamResponse = &mut *(resp.offset(i));
-            let msg = CStr::from_ptr(m.msg);
-            // match on msg_style
-            match PamMessageStyle::from(m.msg_style) {
-                PamMessageStyle::PROMPT_ECHO_ON => {
-                    if let Ok(handler_response) = handler.prompt_echo(msg) {
-                        r.resp = strdup(handler_response.as_ptr());
-                    } else {
-                        result = PamReturnCode::CONV_ERR;
-                    }
-                }
-                PamMessageStyle::PROMPT_ECHO_OFF => {
-                    if let Ok(handler_response) = handler.prompt_blind(msg) {
-                        r.resp = strdup(handler_response.as_ptr());
-                    } else {
-                        result = PamReturnCode::CONV_ERR;
-                    }
-                }
-                PamMessageStyle::ERROR_MSG => {
-                    handler.error(msg);
+        // get indexed values
+        let m: &mut PamMessage = unsafe { &mut **(msg.offset(i)) };
+        let r: &mut PamResponse = unsafe { &mut *(resp.offset(i)) };
+        let msg = unsafe { CStr::from_ptr(m.msg) };
+        // match on msg_style
+        match PamMessageStyle::from(m.msg_style) {
+            PamMessageStyle::PROMPT_ECHO_ON => {
+                if let Ok(handler_response) = handler.prompt_echo(msg) {
+                    r.resp = unsafe { strdup(handler_response.as_ptr()) };
+                } else {
                     result = PamReturnCode::CONV_ERR;
                 }
-                PamMessageStyle::TEXT_INFO => {
-                    handler.info(msg);
+            }
+            PamMessageStyle::PROMPT_ECHO_OFF => {
+                if let Ok(handler_response) = handler.prompt_blind(msg) {
+                    r.resp = unsafe { strdup(handler_response.as_ptr()) };
+                } else {
+                    result = PamReturnCode::CONV_ERR;
                 }
             }
+            PamMessageStyle::ERROR_MSG => handler.error(msg),
+            PamMessageStyle::TEXT_INFO => handler.info(msg),
         }
         if result != PamReturnCode::SUCCESS {
             break;
@@ -68,6 +61,16 @@ pub extern "C" fn converse<C: Converse>(
 
     // free allocated memory if an error occured
     if result != PamReturnCode::SUCCESS {
+
+        // Free any strdup'd response strings
+        for i in 0..num_msg as isize {
+            let r: &mut PamResponse = unsafe { &mut *(resp.offset(i)) };
+            if !r.resp.is_null() {
+                unsafe { free(r.resp as *mut c_void) };
+            }
+        }
+
+        // Free the response array
         unsafe { free(resp as *mut c_void) };
     } else {
         unsafe { *out_resp = resp };

greetd/src/pam/mod.rs

@@ -1,4 +1,4 @@
-mod converse;
+pub mod converse;
 mod env;
 mod ffi;
 pub mod session;

greetd/src/pam/session.rs

@@ -17,8 +17,7 @@ pub struct PamSession<'a> {
 }
 
 impl<'a> PamSession<'a> {
-    pub fn start(service: &str) -> Result<PamSession, Box<dyn Error>> {
-        let mut pam_conv = Box::new(PasswordConv::new());
+    pub fn start(service: &str, mut pam_conv: Box<PasswordConv>) -> Result<PamSession, Box<dyn Error>> {
         let conv = make_conversation(&mut *pam_conv);
         let mut pam_handle: *mut PamHandle = ptr::null_mut();
 

greetd/src/session.rs

@@ -21,6 +21,7 @@ use users::os::unix::UserExt;
 use users::User;
 
 use crate::pam::session::PamSession;
+use crate::pam::converse::PasswordConv;
 use crate::pollable::signals::blocked_sigset;
 use crate::terminal;
 
@@ -135,8 +136,8 @@ impl<'a> Session<'a> {
         provided_env: Vec<String>,
         vt: usize,
     ) -> Result<Session<'a>, Box<dyn Error>> {
-        let mut pam_session = PamSession::start(service)?;
-        pam_session.converse.set_credentials(username, password);
+        let pass_conv = Box::new(PasswordConv::new(username, password));
+        let mut pam_session = PamSession::start(service, pass_conv)?;
         pam_session.authenticate(PamFlag::NONE)?;
         pam_session.acct_mgmt(PamFlag::NONE)?;