fix: avoid tty stuck forever

README.md

@@ -96,6 +96,11 @@ configuration options:
 * `pinentry`: The
   [pinentry](https://www.gnupg.org/related_software/pinentry/index.html)
   executable to use. Defaults to `pinentry`.
+* `pinentry_timeout`: The number of seconds before an unanswered pinentry
+  prompt times out and is dismissed. This prevents prompts that can't be seen
+  (for example one launched against a stale terminal by an ssh-agent request)
+  from lingering forever. Setting this to `0` disables the timeout. Defaults to
+  `120`.
 
 ### Profiles
 

src/api.rs

@@ -1738,10 +1738,10 @@ fn classify_login_error(error_res: &ConnectErrorRes, code: u16) -> Error {
         "invalid_client" => {
             return Error::IncorrectApiKey;
         }
-        "" => {
+        ""
             // bitwarden_rs returns an empty error and error_description for
             // this case, for some reason
-            if error_desc.is_none() || error_desc == Some("") {
+            if (error_desc.is_none() || error_desc == Some("")) => {
                 if let Some(error_model) = error_res.error_model.as_ref() {
                     let message = error_model.message.as_str().to_string();
                     match message.as_str() {
@@ -1759,7 +1759,6 @@ fn classify_login_error(error_res: &ConnectErrorRes, code: u16) -> Error {
                     }
                 }
             }
-        }
         _ => {}
     }
 

src/bin/rbw-agent/actions.rs

@@ -30,6 +30,7 @@ pub async fn register(
             };
             let client_id = rbw::pinentry::getpin(
                 &config_pinentry().await?,
+                config_pinentry_timeout().await?,
                 "API key client__id",
                 &format!("Log in to {host}"),
                 err.as_deref(),
@@ -40,6 +41,7 @@ pub async fn register(
             .context("failed to read client_id from pinentry")?;
             let client_secret = rbw::pinentry::getpin(
                 &config_pinentry().await?,
+                config_pinentry_timeout().await?,
                 "API key client__secret",
                 &format!("Log in to {host}"),
                 err.as_deref(),
@@ -105,6 +107,7 @@ pub async fn login(
             };
             let password = rbw::pinentry::getpin(
                 &config_pinentry().await?,
+                config_pinentry_timeout().await?,
                 "Master Password",
                 &format!("Log in to {host}"),
                 err.as_deref(),
@@ -249,6 +252,7 @@ async fn two_factor(
         };
         let code = rbw::pinentry::getpin(
             &config_pinentry().await?,
+            config_pinentry_timeout().await?,
             provider.header(),
             provider.message(),
             err.as_deref(),
@@ -415,6 +419,7 @@ async fn unlock_state(
             };
             let password = rbw::pinentry::getpin(
                 &config_pinentry().await?,
+                config_pinentry_timeout().await?,
                 "Master Password",
                 &format!(
                     "Unlock the local database for '{}'",
@@ -624,6 +629,7 @@ async fn decrypt_cipher(
             };
             let password = rbw::pinentry::getpin(
                 &config_pinentry().await?,
+                config_pinentry_timeout().await?,
                 "Master Password",
                 "Accessing this entry requires the master password",
                 err.as_deref(),
@@ -818,6 +824,11 @@ async fn config_pinentry() -> anyhow::Result<String> {
     Ok(config.pinentry)
 }
 
+async fn config_pinentry_timeout() -> anyhow::Result<u64> {
+    let config = rbw::config::Config::load_async().await?;
+    Ok(config.pinentry_timeout)
+}
+
 pub async fn subscribe_to_notifications(
     state: std::sync::Arc<tokio::sync::Mutex<crate::state::State>>,
 ) -> anyhow::Result<()> {
@@ -850,14 +861,34 @@ pub async fn subscribe_to_notifications(
         .map_or_else(|| Ok(()), |err| Err(anyhow::anyhow!(err.to_string())))
 }
 
+// ssh-agent requests don't carry an environment of their own, so we fall back
+// to the last environment the main agent saw (see State::last_environment).
+// that environment is frequently stale by the time a background process makes
+// an ssh request, so if the vault is locked and there's no longer any way to
+// show a prompt, bail out instead of launching a pinentry that nobody can see
+// (which otherwise ends up invisible and spinning at 100% cpu forever).
+fn ensure_ssh_prompt_possible(
+    needs_unlock: bool,
+    environment: &rbw::protocol::Environment,
+) -> anyhow::Result<()> {
+    if needs_unlock && !environment.can_prompt() {
+        return Err(anyhow::anyhow!(
+            "rbw is locked and no usable terminal or display is available to \
+             prompt for the master password; run `rbw unlock` first"
+        ));
+    }
+    Ok(())
+}
+
 pub async fn get_ssh_public_keys(
     state: std::sync::Arc<tokio::sync::Mutex<crate::state::State>>,
 ) -> anyhow::Result<Vec<String>> {
-    let environment = {
+    let (environment, needs_unlock) = {
         let state = state.lock().await;
         state.set_timeout();
-        state.last_environment().clone()
+        (state.last_environment().clone(), state.needs_unlock())
     };
+    ensure_ssh_prompt_possible(needs_unlock, &environment)?;
     unlock_state(state.clone(), &environment).await?;
 
     let db = load_db().await?;
@@ -889,11 +920,12 @@ pub async fn find_ssh_private_key(
     state: std::sync::Arc<tokio::sync::Mutex<crate::state::State>>,
     request_public_key: ssh_agent_lib::ssh_key::PublicKey,
 ) -> anyhow::Result<ssh_agent_lib::ssh_key::PrivateKey> {
-    let environment = {
+    let (environment, needs_unlock) = {
         let state = state.lock().await;
         state.set_timeout();
-        state.last_environment().clone()
+        (state.last_environment().clone(), state.needs_unlock())
     };
+    ensure_ssh_prompt_possible(needs_unlock, &environment)?;
     unlock_state(state.clone(), &environment).await?;
 
     let request_bytes = request_public_key.to_bytes();

src/bin/rbw-agent/debugger.rs

@@ -27,7 +27,7 @@ pub fn disable_tracing() -> anyhow::Result<()> {
     if ret != 0 {
         let e = std::io::Error::last_os_error();
         return Err(anyhow::anyhow!(
-            "failed to deny debugger attach, agent memory may be readable by other processes: {}", e
+            "failed to deny debugger attach, agent memory may be readable by other processes: {e}"
         ));
     }
 
@@ -38,11 +38,11 @@ pub fn disable_tracing() -> anyhow::Result<()> {
     };
     // safety: correct argument
     // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/setrlimit.2.html
-    let ret = unsafe { libc::setrlimit(libc::RLIMIT_CORE, &rlim) };
+    let ret = unsafe { libc::setrlimit(libc::RLIMIT_CORE, &raw const rlim) };
     if ret != 0 {
         let e = std::io::Error::last_os_error();
         return Err(anyhow::anyhow!(
-            "failed to disable core dumps, agent memory may be dumped to disk: {}", e
+            "failed to disable core dumps, agent memory may be dumped to disk: {e}"
         ));
     }
 

src/bin/rbw/commands.rs

@@ -1269,6 +1269,11 @@ pub fn config_set(key: &str, value: &str) -> anyhow::Result<()> {
             config.sync_interval = interval;
         }
         "pinentry" => config.pinentry = value.to_string(),
+        "pinentry_timeout" => {
+            config.pinentry_timeout = value
+                .parse()
+                .context("failed to parse value for pinentry_timeout")?;
+        }
         _ => return Err(anyhow::anyhow!("invalid config key: {key}")),
     }
     config.save()?;
@@ -1298,6 +1303,9 @@ pub fn config_unset(key: &str) -> anyhow::Result<()> {
             config.lock_timeout = rbw::config::default_lock_timeout();
         }
         "pinentry" => config.pinentry = rbw::config::default_pinentry(),
+        "pinentry_timeout" => {
+            config.pinentry_timeout = rbw::config::default_pinentry_timeout();
+        }
         _ => return Err(anyhow::anyhow!("invalid config key: {key}")),
     }
     config.save()?;
@@ -1507,8 +1515,7 @@ pub fn search(
         .filter(|entry| {
             entry
                 .as_ref()
-                .map(|entry| entry.search_match(term, folder))
-                .unwrap_or(true)
+                .map_or(true, |entry| entry.search_match(term, folder))
         })
         .map(|entry| entry.map(std::convert::Into::into))
         .collect::<Result<_, anyhow::Error>>()?;

src/config.rs

@@ -18,6 +18,8 @@ pub struct Config {
     pub sync_interval: u64,
     #[serde(default = "default_pinentry")]
     pub pinentry: String,
+    #[serde(default = "default_pinentry_timeout")]
+    pub pinentry_timeout: u64,
     pub client_cert_path: Option<std::path::PathBuf>,
     // backcompat, no longer generated in new configs
     #[serde(skip_serializing)]
@@ -36,6 +38,7 @@ impl Default for Config {
             lock_timeout: default_lock_timeout(),
             sync_interval: default_sync_interval(),
             pinentry: default_pinentry(),
+            pinentry_timeout: default_pinentry_timeout(),
             client_cert_path: None,
             device_id: None,
         }
@@ -54,6 +57,10 @@ pub fn default_pinentry() -> String {
     "pinentry".to_string()
 }
 
+pub fn default_pinentry_timeout() -> u64 {
+    120
+}
+
 impl Config {
     pub fn new() -> Self {
         Self::default()

src/pinentry.rs

@@ -6,6 +6,7 @@ use tokio::io::AsyncWriteExt as _;
 
 pub async fn getpin(
     pinentry: &str,
+    timeout: u64,
     prompt: &str,
     desc: &str,
     err: Option<&str>,
@@ -15,8 +16,19 @@ pub async fn getpin(
     let mut opts = tokio::process::Command::new(pinentry);
     opts.stdin(std::process::Stdio::piped())
         .stdout(std::process::Stdio::piped());
-    let mut args = vec!["--timeout".into(), "0".into()];
-    if let Some(tty) = environment.tty() {
+    // a non-zero timeout bounds how long a prompt can sit around; with
+    // --timeout 0 a prompt that nobody can see (e.g. one launched against a
+    // stale tty for an ssh-agent request) would hang forever. configurable
+    // via the `pinentry_timeout` config option.
+    let mut args = vec!["--timeout".into(), timeout.to_string().into()];
+    // only hand pinentry a --ttyname if that terminal still exists - otherwise
+    // a terminal pinentry attaches to a dead tty, can't be seen or answered,
+    // and busy-loops at 100% cpu. dropping the flag lets pinentry fall back to
+    // its own tty detection (or a graphical prompt).
+    if environment.has_usable_tty() {
+        // unwrap is safe because has_usable_tty() only returns true when a tty
+        // is present
+        let tty = environment.tty().unwrap();
         args.extend(["--ttyname".into(), tty.into()]);
     }
 

src/protocol.rs

@@ -153,6 +153,29 @@ impl Environment {
         self.tty.as_ref().map(|tty| tty.0.as_os_str())
     }
 
+    /// Returns true if the recorded tty still refers to a terminal that we can
+    /// actually open. The tty stored in an `Environment` can go stale (most
+    /// notably for ssh-agent requests, which reuse the last environment the
+    /// main agent saw), and handing a dead tty to pinentry produces an
+    /// invisible prompt that spins at 100% cpu.
+    pub fn has_usable_tty(&self) -> bool {
+        self.tty().is_some_and(tty_is_usable)
+    }
+
+    /// Returns true if there is some way to actually show a pinentry prompt to
+    /// the user - either a live controlling terminal or a graphical display.
+    pub fn can_prompt(&self) -> bool {
+        if self.has_usable_tty() {
+            return true;
+        }
+        // a graphical pinentry can display a prompt without any controlling
+        // terminal
+        self.env_vars().iter().any(|(k, v)| {
+            matches!(k.to_str(), Some("DISPLAY" | "WAYLAND_DISPLAY"))
+                && !v.is_empty()
+        })
+    }
+
     pub fn env_vars(
         &self,
     ) -> std::collections::HashMap<std::ffi::OsString, std::ffi::OsString>
@@ -165,6 +188,22 @@ impl Environment {
     }
 }
 
+// checks whether the given path still refers to a terminal we can open. opened
+// non-blocking and with O_NOCTTY so we neither block on a dead tty nor
+// accidentally acquire it as our controlling terminal. if the tty has gone
+// away (or now belongs to a different user after the device number was reused)
+// the open fails and we report it as unusable.
+fn tty_is_usable(tty: &std::ffi::OsStr) -> bool {
+    use std::os::unix::fs::OpenOptionsExt as _;
+
+    std::fs::OpenOptions::new()
+        .read(true)
+        .write(true)
+        .custom_flags(libc::O_NONBLOCK | libc::O_NOCTTY)
+        .open(tty)
+        .is_ok_and(|f| rustix::termios::isatty(&f))
+}
+
 #[derive(serde::Serialize, serde::Deserialize, Debug)]
 #[serde(tag = "type")]
 pub enum Action {