PAM: Improve error messages and align them with sudo.ws when no tty is available

src/pam/converse.rs

@@ -129,27 +129,109 @@ impl Drop for SignalGuard {
 }
 
 impl CLIConverser {
-    fn open(&self) -> PamResult<(Terminal<'_>, SignalGuard)> {
-        let term = if self.use_askpass {
-            Terminal::open_askpass()?
-        } else if self.use_stdin {
-            Terminal::open_stdie()?
-        } else {
-            let mut tty = Terminal::open_tty()?;
-            if self.bell.replace(false) {
-                tty.bell()?;
-            }
-
-            tty
+    fn open(&self, style: PamMessageStyle) -> PamResult<(Terminal<'_>, SignalGuard)> {
+        let term = match style {
+            PamMessageStyle::PromptEchoOff if self.use_askpass => Terminal::open_askpass()?,
+            PamMessageStyle::PromptEchoOff if self.use_stdin => Terminal::open_stdie()?,
+            PamMessageStyle::PromptEchoOff => match Terminal::open_tty() {
+                Ok(mut tty) => {
+                    if self.bell.replace(false) {
+                        tty.bell()?;
+                    }
+
+                    tty
+                }
+                Err(PamError::TtyRequired) => {
+                    match Self::tty_unavailable_fallback(style, AskpassEnv::from_env()) {
+                        TtyUnavailableFallback::Askpass => Terminal::open_askpass()?,
+                        TtyUnavailableFallback::Stdie => Terminal::open_stdie()?,
+                        TtyUnavailableFallback::Error => {
+                            return Err(Self::tty_required_error());
+                        }
+                    }
+                }
+                Err(err) => return Err(err),
+            },
+            _ if self.use_stdin => Terminal::open_stdie()?,
+            _ => match Terminal::open_tty() {
+                Ok(mut tty) => {
+                    if self.bell.replace(false) {
+                        tty.bell()?;
+                    }
+
+                    tty
+                }
+                Err(PamError::TtyRequired) => Terminal::open_stdie()?,
+                Err(err) => return Err(err),
+            },
         };
 
         Ok((term, SignalGuard::unblock_interrupts()))
     }
+
+    fn tty_unavailable_fallback(
+        style: PamMessageStyle,
+        askpass_env: AskpassEnv,
+    ) -> TtyUnavailableFallback {
+        match style {
+            PamMessageStyle::PromptEchoOff if askpass_env.can_use_askpass() => {
+                TtyUnavailableFallback::Askpass
+            }
+            PamMessageStyle::PromptEchoOff => TtyUnavailableFallback::Error,
+            PamMessageStyle::PromptEchoOn
+            | PamMessageStyle::ErrorMessage
+            | PamMessageStyle::TextInfo => TtyUnavailableFallback::Stdie,
+        }
+    }
+
+    fn tty_required_error() -> PamError {
+        if std::env::var_os("SSH_CONNECTION").is_some() && std::env::var_os("SSH_TTY").is_none() {
+            Self::tty_required_error_ssh()
+        } else {
+            Self::tty_required_error_no_tty_prompt()
+        }
+    }
+
+    fn tty_required_error_no_tty_prompt() -> PamError {
+        PamError::TtyRequiredNoTtyPrompt
+    }
+
+    fn tty_required_error_ssh() -> PamError {
+        PamError::TtyRequiredSsh
+    }
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum TtyUnavailableFallback {
+    Askpass,
+    Stdie,
+    Error,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+struct AskpassEnv {
+    has_display: bool,
+    has_askpass: bool,
+}
+
+impl AskpassEnv {
+    fn from_env() -> Self {
+        Self {
+            has_display: std::env::var_os("DISPLAY").is_some()
+                || std::env::var_os("WAYLAND_DISPLAY").is_some()
+                || std::env::var_os("WAYLAND_SOCKET").is_some(),
+            has_askpass: std::env::var_os("SUDO_ASKPASS").is_some(),
+        }
+    }
+
+    fn can_use_askpass(self) -> bool {
+        self.has_display && self.has_askpass
+    }
 }
 
 impl Converser for CLIConverser {
     fn handle_normal_prompt(&self, msg: &str) -> PamResult<PamBuffer> {
-        let (mut tty, _guard) = self.open()?;
+        let (mut tty, _guard) = self.open(PamMessageStyle::PromptEchoOn)?;
         let input_needed = xlat!("input needed");
         tty.read_input(
             &format!("[{}: {input_needed} {msg} ", self.name),
@@ -159,7 +241,7 @@ impl Converser for CLIConverser {
     }
 
     fn handle_hidden_prompt(&self, msg: &str) -> PamResult<PamBuffer> {
-        let (mut tty, _guard) = self.open()?;
+        let (mut tty, _guard) = self.open(PamMessageStyle::PromptEchoOff)?;
         tty.read_input(
             msg,
             self.password_timeout,
@@ -172,12 +254,12 @@ impl Converser for CLIConverser {
     }
 
     fn handle_error(&self, msg: &str) -> PamResult<()> {
-        let (mut tty, _) = self.open()?;
+        let (mut tty, _) = self.open(PamMessageStyle::ErrorMessage)?;
         Ok(tty.prompt(&format!("[{} error] {msg}\n", self.name))?)
     }
 
     fn handle_info(&self, msg: &str) -> PamResult<()> {
-        let (mut tty, _) = self.open()?;
+        let (mut tty, _) = self.open(PamMessageStyle::TextInfo)?;
         Ok(tty.prompt(&format!("[{}] {msg}\n", self.name))?)
     }
 }
@@ -473,4 +555,108 @@ mod test {
 
         assert!(hello.panicked); // allowed now
     }
+
+    #[test]
+    fn tty_unavailable_fallback_for_hidden_prompt_uses_askpass_only_with_display_and_askpass() {
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOff,
+                AskpassEnv {
+                    has_display: true,
+                    has_askpass: true,
+                },
+            ),
+            TtyUnavailableFallback::Askpass
+        );
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOff,
+                AskpassEnv {
+                    has_display: true,
+                    has_askpass: false,
+                },
+            ),
+            TtyUnavailableFallback::Error
+        );
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOff,
+                AskpassEnv {
+                    has_display: false,
+                    has_askpass: true,
+                },
+            ),
+            TtyUnavailableFallback::Error
+        );
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOff,
+                AskpassEnv {
+                    has_display: false,
+                    has_askpass: false,
+                },
+            ),
+            TtyUnavailableFallback::Error
+        );
+    }
+
+    #[test]
+    fn tty_unavailable_fallback_for_non_hidden_prompt_uses_stdio() {
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOn,
+                AskpassEnv {
+                    has_display: true,
+                    has_askpass: true,
+                },
+            ),
+            TtyUnavailableFallback::Stdie
+        );
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOn,
+                AskpassEnv {
+                    has_display: true,
+                    has_askpass: false,
+                },
+            ),
+            TtyUnavailableFallback::Stdie
+        );
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOn,
+                AskpassEnv {
+                    has_display: false,
+                    has_askpass: true,
+                },
+            ),
+            TtyUnavailableFallback::Stdie
+        );
+        assert_eq!(
+            CLIConverser::tty_unavailable_fallback(
+                PamMessageStyle::PromptEchoOn,
+                AskpassEnv {
+                    has_display: false,
+                    has_askpass: false,
+                },
+            ),
+            TtyUnavailableFallback::Stdie
+        );
+    }
+
+    #[test]
+    fn tty_required_error_no_tty_prompt_uses_s_hint() {
+        assert!(matches!(
+            CLIConverser::tty_required_error_no_tty_prompt(),
+            PamError::TtyRequiredNoTtyPrompt
+        ));
+    }
+
+    #[test]
+    fn tty_required_error_ssh_uses_ssh_t_hint() {
+        assert!(matches!(
+            CLIConverser::tty_required_error_ssh(),
+            PamError::TtyRequiredSsh
+        ));
+    }
 }

src/pam/error.rs

@@ -177,6 +177,8 @@ pub enum PamError {
     Pam(PamErrorType),
     IoError(std::io::Error),
     TtyRequired,
+    TtyRequiredNoTtyPrompt,
+    TtyRequiredSsh,
     EnvListFailure,
     InteractionRequired,
     NoPasswordProvided,
@@ -225,6 +227,14 @@ impl fmt::Display for PamError {
             PamError::Pam(tp) => xlat_write!(f, "PAM error: {error}", error = tp.get_err_msg()),
             PamError::IoError(e) => xlat_write!(f, "IO error: {error}", error = e),
             PamError::TtyRequired => xlat_write!(f, "A terminal is required to authenticate"),
+            PamError::TtyRequiredNoTtyPrompt => xlat_write!(
+                f,
+                "A terminal is required to authenticate; either use the -S option to read the password from standard input or configure an askpass helper"
+            ),
+            PamError::TtyRequiredSsh => xlat_write!(
+                f,
+                "A terminal is required to authenticate; either use ssh's -t option or configure an askpass helper"
+            ),
             PamError::EnvListFailure => {
                 xlat_write!(
                     f,

test-framework/sudo-compliance-tests/src/sudo/pam.rs

@@ -472,6 +472,50 @@ auth requisite pam_deny.so
         .assert_success();
 }
 
+#[test]
+fn no_tty_pam_text_info_falls_back_to_stdio() {
+    let env = Env("ALL ALL=(ALL:ALL) ALL")
+        .user(USERNAME)
+        .file(
+            "/etc/pam.d/sudo",
+            [
+                "auth optional pam_echo.so Hello sudo-rs, I am PAM",
+                "auth sufficient pam_permit.so",
+            ]
+            .join("\n"),
+        )
+        .build();
+
+    Command::new("sh")
+        .args(["-c", "sudo true </dev/null >/tmp/repro.log 2>&1"])
+        .as_user(USERNAME)
+        .output(&env)
+        .assert_success();
+}
+
+#[test]
+fn no_tty_pam_text_info_uses_stdio_fallback() {
+    let env = Env("ALL ALL=(ALL:ALL) NOPASSWD: ALL")
+        .file(
+            "/etc/pam.d/sudo",
+            [
+                "auth sufficient pam_permit.so",
+                "account sufficient pam_permit.so",
+                "session optional pam_echo.so Hello sudo-rs, I am PAM",
+                "session sufficient pam_permit.so",
+            ]
+            .join("\n"),
+        )
+        .user(USERNAME)
+        .build();
+
+    Command::new("sh")
+        .args(["-c", "sudo true </dev/null"])
+        .as_user(USERNAME)
+        .output(&env)
+        .assert_success();
+}
+
 #[test]
 #[cfg_attr(
     target_os = "freebsd",