Complete polkit authentication dialog and harden session handling

shell/ferese-polkit-agent/src/session.rs

@@ -8,9 +8,9 @@ use std::time::Duration;
 
 use glib::gobject_ffi;
 use libloading::Library;
-use zeroize::Zeroize;
+use zeroize::{Zeroize, Zeroizing};
 
-use crate::PromptEvent;
+use crate::{AuthenticationRequest, Generation, PromptEvent};
 
 enum SessionEvent {
     Request(String, bool),
@@ -63,7 +63,7 @@ pub fn check() -> Result<(), String> {
     Api::load().map(|_| ())
 }
 
-fn username(uid: u32) -> String {
+pub(crate) fn username(uid: u32) -> String {
     let output = Command::new("getent").args(["passwd", &uid.to_string()]).output();
     output
         .ok()
@@ -80,10 +80,20 @@ fn send(writer: &mut BufWriter<ChildStdin>, event: &PromptEvent) -> Result<(), S
     writer.flush().map_err(|e| e.to_string())
 }
 
-fn prompt_process(
-    message: String,
-    user: String,
-) -> Result<(Child, BufWriter<ChildStdin>, mpsc::Receiver<PromptEvent>), String> {
+struct PromptProcess {
+    child: Child,
+    writer: BufWriter<ChildStdin>,
+    input: mpsc::Receiver<PromptEvent>,
+}
+
+impl Drop for PromptProcess {
+    fn drop(&mut self) {
+        let _ = self.child.kill();
+        let _ = self.child.wait();
+    }
+}
+
+fn prompt_process(request: AuthenticationRequest) -> Result<PromptProcess, String> {
     let mut child = Command::new(std::env::current_exe().map_err(|e| e.to_string())?)
         .arg("--prompt")
         .stdin(Stdio::piped())
@@ -93,8 +103,7 @@ fn prompt_process(
         .map_err(|e| e.to_string())?;
     let stdin = child.stdin.take().ok_or("Prompt has no input")?;
     let stdout = child.stdout.take().ok_or("Prompt has no output")?;
-    let mut writer = BufWriter::new(stdin);
-    send(&mut writer, &PromptEvent::Start { message, user })?;
+    let writer = BufWriter::new(stdin);
     let (tx, rx) = mpsc::channel();
     std::thread::spawn(move || {
         for line in BufReader::new(stdout).lines() {
@@ -109,10 +118,16 @@ fn prompt_process(
             }
         }
     });
-    Ok((child, writer, rx))
+    let mut process = PromptProcess {
+        child,
+        writer,
+        input: rx,
+    };
+    send(&mut process.writer, &PromptEvent::Start(request))?;
+    Ok(process)
 }
 
-unsafe extern "C" fn request(_session: Object, prompt: *const c_char, echo: c_int, data: *mut c_void) {
+unsafe extern "C" fn request_signal(_session: Object, prompt: *const c_char, echo: c_int, data: *mut c_void) {
     let tx = unsafe { &*(data as *const Sender<SessionEvent>) };
     let prompt = unsafe { CStr::from_ptr(prompt) }.to_string_lossy().into_owned();
     let _ = tx.send(SessionEvent::Request(prompt, echo != 0));
@@ -148,17 +163,74 @@ unsafe fn connect(session: Object, name: &'static CStr, callback: gobject_ffi::G
     }
 }
 
+#[derive(Debug, PartialEq, Eq)]
+enum SessionOutcome {
+    Completed(bool),
+    Cancelled,
+    IdentityChanged { uid: u32, generation: Generation },
+}
+
+struct SessionGuard {
+    object: Object,
+    signal_data: *mut Sender<SessionEvent>,
+    cancel: unsafe extern "C" fn(Object),
+    completed: bool,
+}
+
+impl Drop for SessionGuard {
+    fn drop(&mut self) {
+        unsafe {
+            if !self.completed {
+                (self.cancel)(self.object);
+            }
+            gobject_ffi::g_object_unref(self.object.cast());
+            drop(Box::from_raw(self.signal_data));
+        }
+    }
+}
+
+fn permitted_selection(request: &AuthenticationRequest, uid: u32) -> bool {
+    request.identities.iter().any(|identity| identity.uid == uid)
+}
+
+fn response_buffer(value: &str) -> Option<Zeroizing<Vec<u8>>> {
+    if value.as_bytes().contains(&0) {
+        return None;
+    }
+    // Allocate room for the terminator up front so appending it cannot leave
+    // a password copy in an old allocation. Drop wipes the entire FFI buffer.
+    let mut response = Zeroizing::new(Vec::with_capacity(value.len() + 1));
+    response.extend_from_slice(value.as_bytes());
+    response.push(0);
+    Some(response)
+}
+
 fn run_session(
     api: &Api,
-    uid: u32,
+    request: &AuthenticationRequest,
     cookie: &CString,
     context: &glib::MainContext,
-    writer: &mut BufWriter<ChildStdin>,
-    input: &mpsc::Receiver<PromptEvent>,
+    process: &mut PromptProcess,
     cancelled: &AtomicBool,
-    child: &mut Child,
-) -> Result<Option<bool>, String> {
-    let identity = unsafe { (api.user_new)(uid as c_int) };
+) -> Result<SessionOutcome, String> {
+    let generation = request.generation;
+    send(
+        &mut process.writer,
+        &PromptEvent::IdentityChanged {
+            generation,
+            uid: request.selected_uid,
+        },
+    )?;
+    if generation.attempt > 0 {
+        send(
+            &mut process.writer,
+            &PromptEvent::Error {
+                generation,
+                text: "Password not accepted. Try again.".into(),
+            },
+        )?;
+    }
+    let identity = unsafe { (api.user_new)(request.selected_uid as c_int) };
     if identity.is_null() {
         return Err("Could not create polkit identity".into());
     }
@@ -169,7 +241,14 @@ fn run_session(
     }
 
     let (tx, rx) = mpsc::channel();
-    let signal_data = Box::into_raw(Box::new(tx)).cast();
+    let signal_data = Box::into_raw(Box::new(tx));
+    let mut session_guard = SessionGuard {
+        object: session,
+        signal_data,
+        cancel: api.cancel,
+        completed: false,
+    };
+    let signal_data = signal_data.cast();
     unsafe {
         connect(
             session,
@@ -178,7 +257,7 @@ fn run_session(
                 unsafe extern "C" fn(Object, *const c_char, c_int, *mut c_void),
                 unsafe extern "C" fn(),
             >(
-                request as unsafe extern "C" fn(Object, *const c_char, c_int, *mut c_void),
+                request_signal as unsafe extern "C" fn(Object, *const c_char, c_int, *mut c_void),
             )),
             signal_data,
         );
@@ -221,6 +300,7 @@ fn run_session(
 
     let mut outcome = None;
     let mut stopped = false;
+    let mut selected_uid = None;
     while outcome.is_none() {
         while context.pending() {
             context.iteration(false);
@@ -228,78 +308,381 @@ fn run_session(
         while let Ok(event) = rx.try_recv() {
             match event {
                 SessionEvent::Request(prompt, echo) => {
-                    send(writer, &PromptEvent::Request { prompt, echo })?;
+                    send(
+                        &mut process.writer,
+                        &PromptEvent::Request {
+                            generation,
+                            prompt,
+                            echo,
+                        },
+                    )?;
                 }
-                SessionEvent::Info(text) => send(writer, &PromptEvent::Info { text })?,
-                SessionEvent::Error(text) => send(writer, &PromptEvent::Error { text })?,
+                SessionEvent::Info(text) => send(&mut process.writer, &PromptEvent::Info { generation, text })?,
+                SessionEvent::Error(text) => send(&mut process.writer, &PromptEvent::Error { generation, text })?,
                 SessionEvent::Completed(success) => outcome = Some(success),
             }
         }
         if outcome.is_some() {
             break;
         }
-        while let Ok(event) = input.try_recv() {
+        while let Ok(event) = process.input.try_recv() {
             match event {
-                PromptEvent::Response { mut value } => {
-                    if let Ok(response) = CString::new(value.as_str()) {
-                        unsafe { (api.response)(session, response.as_ptr()) };
+                PromptEvent::Response {
+                    generation: response_generation,
+                    uid,
+                    mut value,
+                } => {
+                    if uid == request.selected_uid
+                        && response_generation == generation
+                        && !stopped
+                        && selected_uid.is_none()
+                        && let Some(response) = response_buffer(&value)
+                    {
+                        value.zeroize();
+                        unsafe { (api.response)(session, response.as_ptr().cast()) };
                     }
                     value.zeroize();
                 }
+                PromptEvent::SelectIdentity {
+                    generation: selection_generation,
+                    uid,
+                } if permitted_selection(request, uid)
+                    && selection_generation.attempt == 0
+                    && selection_generation.selection > generation.selection
+                    && selected_uid.is_none_or(|(_, pending): (u32, Generation)| {
+                        selection_generation.selection > pending.selection
+                    })
+                    && (!stopped || selected_uid.is_some()) =>
+                {
+                    selected_uid = Some((uid, selection_generation));
+                }
                 PromptEvent::Cancel => cancelled.store(true, Ordering::Release),
                 _ => {}
             }
         }
-        if !stopped && (cancelled.load(Ordering::Acquire) || child.try_wait().map_err(|e| e.to_string())?.is_some()) {
+        if !stopped
+            && (selected_uid.is_some()
+                || cancelled.load(Ordering::Acquire)
+                || process.child.try_wait().map_err(|e| e.to_string())?.is_some())
+        {
             unsafe { (api.cancel)(session) };
             stopped = true;
         }
         std::thread::sleep(Duration::from_millis(10));
     }
 
-    unsafe {
-        gobject_ffi::g_object_unref(session.cast());
-        drop(Box::from_raw(signal_data as *mut Sender<SessionEvent>));
+    session_guard.completed = true;
+    if cancelled.load(Ordering::Acquire) {
+        Ok(SessionOutcome::Cancelled)
+    } else if let Some((uid, generation)) = selected_uid {
+        Ok(SessionOutcome::IdentityChanged { uid, generation })
+    } else if stopped {
+        Ok(SessionOutcome::Cancelled)
+    } else {
+        Ok(SessionOutcome::Completed(outcome.unwrap()))
     }
-    Ok(outcome.filter(|_| !stopped))
 }
 
-pub fn authenticate(uid: u32, message: String, cookie: String, cancelled: Arc<AtomicBool>) -> Result<bool, String> {
+pub fn authenticate(
+    mut request: AuthenticationRequest,
+    cookie: String,
+    cancelled: Arc<AtomicBool>,
+) -> Result<bool, String> {
+    if cancelled.load(Ordering::Acquire) {
+        return Ok(false);
+    }
     let api = Api::load()?;
     let cookie = CString::new(cookie).map_err(|_| "Invalid authentication cookie")?;
     let context = glib::MainContext::new();
     context
         .with_thread_default(|| {
-            let (mut child, mut writer, input) = prompt_process(message, username(uid))?;
-            let result = loop {
-                match run_session(
-                    &api,
-                    uid,
-                    &cookie,
-                    &context,
-                    &mut writer,
-                    &input,
-                    &cancelled,
-                    &mut child,
-                ) {
-                    Ok(Some(true)) => break Ok(true),
-                    Ok(Some(false)) if !cancelled.load(Ordering::Acquire) => {
-                        if let Err(error) = send(
-                            &mut writer,
-                            &PromptEvent::Error {
-                                text: "Password not accepted. Try again.".into(),
-                            },
-                        ) {
-                            break Err(error);
-                        }
-                    }
-                    Ok(_) => break Ok(false),
-                    Err(error) => break Err(error),
-                }
-            };
-            let _ = child.kill();
-            let _ = child.wait();
-            result
+            let mut process = prompt_process(request.clone())?;
+            authenticate_sessions(&api, &mut request, &cookie, &context, &mut process, &cancelled)
         })
         .map_err(|error| error.to_string())?
 }
+
+const MAX_AUTH_ATTEMPTS: usize = 3;
+
+fn authenticate_sessions(
+    api: &Api,
+    request: &mut AuthenticationRequest,
+    cookie: &CString,
+    context: &glib::MainContext,
+    process: &mut PromptProcess,
+    cancelled: &AtomicBool,
+) -> Result<bool, String> {
+    let mut failures = 0;
+    loop {
+        match run_session(api, request, cookie, context, process, cancelled) {
+            Ok(SessionOutcome::Completed(true)) => break Ok(true),
+            Ok(SessionOutcome::IdentityChanged { uid, generation }) if !cancelled.load(Ordering::Acquire) => {
+                request.selected_uid = uid;
+                request.generation = generation;
+            }
+            Ok(SessionOutcome::Completed(false)) if !cancelled.load(Ordering::Acquire) => {
+                failures += 1;
+                // Switching accounts never resets this request's attempt budget.
+                if failures >= MAX_AUTH_ATTEMPTS {
+                    break Err(format!("Authentication failed after {MAX_AUTH_ATTEMPTS} attempts"));
+                }
+                request.generation.attempt += 1;
+            }
+            Ok(_) => break Ok(false),
+            Err(error) => break Err(error),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::tests::request_fixture;
+
+    static RESPONSES: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0);
+
+    unsafe extern "C" fn fake_initiate(session: Object) {
+        unsafe {
+            gobject_ffi::g_signal_emit_by_name(session.cast(), c"request".as_ptr(), c"Password:".as_ptr(), 0 as c_int);
+        }
+    }
+
+    unsafe extern "C" fn fake_cancel(session: Object) {
+        unsafe {
+            gobject_ffi::g_signal_emit_by_name(session.cast(), c"completed".as_ptr(), 0 as c_int);
+        }
+    }
+
+    unsafe extern "C" fn fake_response(session: Object, _value: *const c_char) {
+        RESPONSES.fetch_add(1, Ordering::SeqCst);
+        unsafe {
+            gobject_ffi::g_signal_emit_by_name(session.cast(), c"completed".as_ptr(), 1 as c_int);
+        }
+    }
+
+    #[test]
+    fn retries_stop_after_three_failures_even_when_the_account_changes() {
+        let mut api = Api::load().unwrap();
+        api.initiate = fake_initiate;
+        api.response = fake_failed_response;
+        api.cancel = fake_cancel;
+        let context = glib::MainContext::new();
+        context
+            .with_thread_default(|| {
+                let mut child = Command::new("cat")
+                    .stdin(Stdio::piped())
+                    .stdout(Stdio::piped())
+                    .spawn()
+                    .unwrap();
+                let writer = BufWriter::new(child.stdin.take().unwrap());
+                let output = child.stdout.take().unwrap();
+                let (tx, input) = mpsc::channel();
+                let starts = Arc::new(std::sync::atomic::AtomicUsize::new(0));
+                let counter = starts.clone();
+                let prompt = std::thread::spawn(move || {
+                    let mut uid = 1000;
+                    for line in BufReader::new(output).lines() {
+                        let Ok(line) = line else { break };
+                        if let Ok(PromptEvent::Request { generation, .. }) = serde_json::from_str::<PromptEvent>(&line)
+                        {
+                            let count = counter.fetch_add(1, Ordering::SeqCst) + 1;
+                            let event = if count == 3 {
+                                uid = 1001;
+                                PromptEvent::SelectIdentity {
+                                    uid,
+                                    generation: Generation {
+                                        selection: generation.selection + 1,
+                                        attempt: 0,
+                                    },
+                                }
+                            } else {
+                                PromptEvent::Response {
+                                    generation,
+                                    uid,
+                                    value: "incorrect".into(),
+                                }
+                            };
+                            if tx.send(event).is_err() {
+                                break;
+                            }
+                        }
+                    }
+                });
+                let mut process = PromptProcess { child, writer, input };
+                let mut request = request_fixture();
+                let result = authenticate_sessions(
+                    &api,
+                    &mut request,
+                    &CString::new("test-cookie").unwrap(),
+                    &context,
+                    &mut process,
+                    &AtomicBool::new(false),
+                );
+                drop(process);
+                prompt.join().unwrap();
+                assert_eq!(result, Err("Authentication failed after 3 attempts".into()));
+                assert_eq!(starts.load(Ordering::SeqCst), 4);
+                assert_eq!(request.selected_uid, 1001);
+            })
+            .unwrap();
+    }
+
+    unsafe extern "C" fn fake_failed_response(session: Object, _value: *const c_char) {
+        unsafe {
+            gobject_ffi::g_signal_emit_by_name(session.cast(), c"completed".as_ptr(), 0 as c_int);
+        }
+    }
+
+    #[test]
+    fn account_changes_cancel_old_sessions_and_reject_stale_passwords() {
+        // Use actual GLib session objects and callbacks, with no PAM helper or
+        // authority contacted. Only the session's initiate/response/cancel calls are fake.
+        let mut api = Api::load().unwrap();
+        api.initiate = fake_initiate;
+        api.response = fake_response;
+        api.cancel = fake_cancel;
+        let context = glib::MainContext::new();
+        context
+            .with_thread_default(|| {
+                let mut child = Command::new("sleep").arg("5").stdin(Stdio::piped()).spawn().unwrap();
+                let writer = BufWriter::new(child.stdin.take().unwrap());
+                let cookie = CString::new("test-cookie").unwrap();
+                let cancelled = AtomicBool::new(false);
+                let mut request = request_fixture();
+                let (tx, input) = mpsc::channel();
+                let mut process = PromptProcess { child, writer, input };
+                tx.send(PromptEvent::SelectIdentity {
+                    uid: 1001,
+                    generation: Generation {
+                        selection: 1,
+                        attempt: 0,
+                    },
+                })
+                .unwrap();
+                tx.send(PromptEvent::Response {
+                    generation: Generation::default(),
+                    uid: 1000,
+                    value: "stale-response".into(),
+                })
+                .unwrap();
+                assert_eq!(
+                    run_session(&api, &request, &cookie, &context, &mut process, &cancelled).unwrap(),
+                    SessionOutcome::IdentityChanged {
+                        uid: 1001,
+                        generation: Generation {
+                            selection: 1,
+                            attempt: 0
+                        }
+                    }
+                );
+                assert_eq!(RESPONSES.load(Ordering::SeqCst), 0);
+                request.selected_uid = 1001;
+                request.generation = Generation {
+                    selection: 1,
+                    attempt: 0,
+                };
+                tx.send(PromptEvent::Response {
+                    generation: Generation::default(),
+                    uid: 1000,
+                    value: "stale-response".into(),
+                })
+                .unwrap();
+                // Matching UID alone is insufficient: reject responses from a
+                // previous account selection and from an earlier retry.
+                tx.send(PromptEvent::Response {
+                    generation: Generation::default(),
+                    uid: 1001,
+                    value: "same-user-old-generation".into(),
+                })
+                .unwrap();
+                request.generation.attempt = 1;
+                tx.send(PromptEvent::Response {
+                    generation: Generation {
+                        attempt: 0,
+                        ..request.generation
+                    },
+                    uid: 1001,
+                    value: "same-user-old-attempt".into(),
+                })
+                .unwrap();
+                tx.send(PromptEvent::Response {
+                    generation: request.generation,
+                    uid: 1001,
+                    value: "new-response".into(),
+                })
+                .unwrap();
+                assert_eq!(
+                    run_session(&api, &request, &cookie, &context, &mut process, &cancelled).unwrap(),
+                    SessionOutcome::Completed(true)
+                );
+                assert_eq!(RESPONSES.load(Ordering::SeqCst), 1);
+                tx.send(PromptEvent::Cancel).unwrap();
+                assert_eq!(
+                    run_session(&api, &request, &cookie, &context, &mut process, &cancelled).unwrap(),
+                    SessionOutcome::Cancelled
+                );
+            })
+            .unwrap();
+    }
+
+    #[test]
+    fn account_switches_keep_the_latest_generation() {
+        let mut api = Api::load().unwrap();
+        api.initiate = fake_initiate;
+        api.cancel = fake_cancel;
+        let context = glib::MainContext::new();
+        context
+            .with_thread_default(|| {
+                let mut child = Command::new("sleep").arg("5").stdin(Stdio::piped()).spawn().unwrap();
+                let writer = BufWriter::new(child.stdin.take().unwrap());
+                let (tx, input) = mpsc::channel();
+                let mut process = PromptProcess { child, writer, input };
+                for (selection, uid) in [(2, 1001), (3, 1000), (1, 1001), (2, 1001)] {
+                    tx.send(PromptEvent::SelectIdentity {
+                        generation: Generation { selection, attempt: 0 },
+                        uid,
+                    })
+                    .unwrap();
+                }
+                assert_eq!(
+                    run_session(
+                        &api,
+                        &request_fixture(),
+                        &CString::new("test-cookie").unwrap(),
+                        &context,
+                        &mut process,
+                        &AtomicBool::new(false),
+                    )
+                    .unwrap(),
+                    SessionOutcome::IdentityChanged {
+                        uid: 1000,
+                        generation: Generation {
+                            selection: 3,
+                            attempt: 0
+                        }
+                    }
+                );
+            })
+            .unwrap();
+    }
+
+    #[test]
+    fn ffi_response_is_nul_terminated_and_owned_by_a_zeroizing_buffer() {
+        let response = response_buffer("test-password").unwrap();
+        assert_eq!(
+            CStr::from_bytes_with_nul(&response).unwrap().to_bytes(),
+            b"test-password"
+        );
+        assert!(response_buffer("invalid\0password").is_none());
+        assert_eq!(response_buffer("").unwrap().as_slice(), &[0]);
+    }
+
+    #[test]
+    fn account_switches_accept_only_offered_identities() {
+        let request = request_fixture();
+        assert!(permitted_selection(&request, 1001));
+        assert!(permitted_selection(&request, 1000));
+        assert!(!permitted_selection(&request, 0));
+        assert!(!permitted_selection(&request, u32::MAX));
+    }
+}