diff --git a/src/pam/converse.rs b/src/pam/converse.rs index e2b199151..abad6facb 100644 --- a/src/pam/converse.rs +++ b/src/pam/converse.rs @@ -1,6 +1,7 @@ use std::io; use crate::cutils::string_from_ptr; +use crate::pam::rpassword::Hidden; use crate::system::{ signal::{self, SignalSet}, time::Duration, @@ -142,8 +143,11 @@ impl CLIConverser { impl Converser for CLIConverser { fn handle_normal_prompt(&self, msg: &str) -> PamResult { let (mut tty, _guard) = self.open()?; - tty.prompt(&format!("[{}: input needed] {msg} ", self.name))?; - Ok(tty.read_cleartext()?) + Ok(tty.read_input( + &format!("[{}: input needed] {msg} ", self.name), + None, + Hidden::No, + )?) } fn handle_hidden_prompt(&self, msg: &str) -> PamResult { @@ -151,12 +155,15 @@ impl Converser for CLIConverser { if self.bell && !self.use_stdin { tty.bell()?; } - tty.prompt(msg)?; - if self.password_feedback { - tty.read_password_with_feedback(self.password_timeout) - } else { - tty.read_password(self.password_timeout) - } + tty.read_input( + msg, + self.password_timeout, + if self.password_feedback { + Hidden::WithFeedback(()) + } else { + Hidden::Yes(()) + }, + ) .map_err(|err| { if let io::ErrorKind::TimedOut = err.kind() { PamError::TimedOut diff --git a/src/pam/rpassword.rs b/src/pam/rpassword.rs index 908fdb2fa..d5cf3de22 100644 --- a/src/pam/rpassword.rs +++ b/src/pam/rpassword.rs @@ -20,30 +20,23 @@ use std::{fs, mem}; use libc::{tcsetattr, termios, ECHO, ECHONL, ICANON, TCSANOW, VEOF, VERASE, VKILL}; -use crate::cutils::cerr; +use crate::cutils::{cerr, safe_isatty}; use crate::system::time::Duration; use super::securemem::PamBuffer; -struct HiddenInput { - tty: fs::File, +struct HiddenInput<'a> { + tty: BorrowedFd<'a>, term_orig: termios, } -impl HiddenInput { - fn new() -> io::Result> { - // control ourselves that we are really talking to a TTY - // mitigates: https://marc.info/?l=oss-security&m=168164424404224 - let Ok(tty) = fs::File::open("/dev/tty") else { - // if we have nothing to show, we have nothing to hide - return Ok(None); - }; - +impl HiddenInput<'_> { + fn new(tty: BorrowedFd) -> io::Result { // Make two copies of the terminal settings. The first one will be modified // and the second one will act as a backup for when we want to set the // terminal back to its original state. - let mut term = safe_tcgetattr(&tty)?; - let term_orig = safe_tcgetattr(&tty)?; + let mut term = safe_tcgetattr(tty)?; + let term_orig = safe_tcgetattr(tty)?; // Hide the password. This is what makes this function useful. term.c_lflag &= !ECHO; @@ -58,11 +51,11 @@ impl HiddenInput { // SAFETY: we are passing tcsetattr a valid file descriptor and pointer-to-struct cerr(unsafe { tcsetattr(tty.as_raw_fd(), TCSANOW, &term) })?; - Ok(Some(HiddenInput { tty, term_orig })) + Ok(HiddenInput { tty, term_orig }) } } -impl Drop for HiddenInput { +impl Drop for HiddenInput<'_> { fn drop(&mut self) { // Set the the mode back to normal // SAFETY: we are passing tcsetattr a valid file descriptor and pointer-to-struct @@ -89,17 +82,17 @@ fn erase_feedback(sink: &mut dyn io::Write, i: usize) { } } -enum Hidden<'a> { +pub(super) enum Hidden { No, - Yes(&'a HiddenInput), - WithFeedback(&'a HiddenInput), + Yes(T), + WithFeedback(T), } /// Reads a password from the given file descriptor while optionally showing feedback to the user. fn read_unbuffered( source: &mut dyn io::Read, sink: &mut dyn io::Write, - hide_input: Hidden<'_>, + hide_input: &Hidden, ) -> io::Result { struct ExitGuard<'a> { pw_len: usize, @@ -260,6 +253,8 @@ pub enum Terminal<'a> { impl Terminal<'_> { /// Open the current TTY for user communication pub fn open_tty() -> io::Result { + // control ourselves that we are really talking to a TTY + // mitigates: https://marc.info/?l=oss-security&m=168164424404224 Ok(Terminal::Tty( fs::OpenOptions::new() .read(true) @@ -273,48 +268,41 @@ impl Terminal<'_> { Ok(Terminal::StdIE(io::stdin().lock(), io::stderr().lock())) } - /// Reads input with TTY echo disabled - pub fn read_password(&mut self, timeout: Option) -> io::Result { - let hide_input = HiddenInput::new()?; - self.read_inner( - timeout, - hide_input.as_ref().map(Hidden::Yes).unwrap_or(Hidden::No), - ) - } - - /// Reads input with TTY echo disabled, but do provide visual feedback while typing. - pub fn read_password_with_feedback( + /// Reads input with TTY echo and visual feedback set according to the `hidden` parameter. + pub(super) fn read_input( &mut self, + prompt: &str, timeout: Option, + hidden: Hidden<()>, ) -> io::Result { - let hide_input = HiddenInput::new()?; - self.read_inner( - timeout, - hide_input - .as_ref() - .map(Hidden::WithFeedback) - .unwrap_or(Hidden::No), - ) - } - - /// Reads input with TTY echo enabled - pub fn read_cleartext(&mut self) -> io::Result { - self.read_inner(None, Hidden::No) - } + fn do_hide_input( + hidden: Hidden<()>, + input: BorrowedFd, + ) -> Result, io::Error> { + Ok(match hidden { + // If input is not a tty, we can't hide feedback. + _ if !safe_isatty(input) => Hidden::No, + + Hidden::No => Hidden::No, + Hidden::Yes(()) => Hidden::Yes(HiddenInput::new(input)?), + Hidden::WithFeedback(()) => Hidden::WithFeedback(HiddenInput::new(input)?), + }) + } - fn read_inner( - &mut self, - timeout: Option, - hide_input: Hidden<'_>, - ) -> io::Result { match self { Terminal::StdIE(stdin, stdout) => { + write_unbuffered(stdout, prompt.as_bytes())?; + + let hide_input = do_hide_input(hidden, stdin.as_fd())?; let mut reader = TimeoutRead::new(stdin.as_fd(), timeout); - read_unbuffered(&mut reader, stdout, hide_input) + read_unbuffered(&mut reader, stdout, &hide_input) } Terminal::Tty(file) => { + write_unbuffered(file, prompt.as_bytes())?; + + let hide_input = do_hide_input(hidden, file.as_fd())?; let mut reader = TimeoutRead::new(file.as_fd(), timeout); - read_unbuffered(&mut reader, &mut &*file, hide_input) + read_unbuffered(&mut reader, &mut &*file, &hide_input) } } } @@ -347,7 +335,7 @@ mod test { fn miri_test_read() { let mut data = "password123\nhello world".as_bytes(); let mut stdout = Vec::new(); - let buf = read_unbuffered(&mut data, &mut stdout, Hidden::No).unwrap(); + let buf = read_unbuffered(&mut data, &mut stdout, &Hidden::No).unwrap(); // check that the \n is not part of input assert_eq!( buf.iter() @@ -363,8 +351,10 @@ mod test { #[test] fn miri_test_longpwd() { let mut stdout = Vec::new(); - assert!(read_unbuffered(&mut "a".repeat(511).as_bytes(), &mut stdout, Hidden::No).is_ok()); - assert!(read_unbuffered(&mut "a".repeat(512).as_bytes(), &mut stdout, Hidden::No).is_err()); + assert!(read_unbuffered(&mut "a".repeat(511).as_bytes(), &mut stdout, &Hidden::No).is_ok()); + assert!( + read_unbuffered(&mut "a".repeat(512).as_bytes(), &mut stdout, &Hidden::No).is_err() + ); } #[test]