From 04d87b35c6a310886065f2a61a5a336df3bc4a79 Mon Sep 17 00:00:00 2001 From: Tin Lai Date: Thu, 4 Jun 2026 12:13:52 +1000 Subject: [PATCH 1/3] don't launch invisible pinentry on stale ttys ssh-agent requests carry no environment, so the agent reuses the last environment the main agent saw. by the time a background process triggers an ssh request, that tty is often stale; pinentry then attaches to a dead tty, can't be seen or answered, and busy-loops at 100% cpu. with the previous --timeout 0 these accumulated indefinitely. - only pass --ttyname when the recorded tty still opens as a terminal - fail fast on ssh-agent requests when locked with no way to prompt, instead of spawning a hidden pinentry - bound pinentry lifetime with --timeout 120 instead of 0 --- src/bin/rbw-agent/actions.rs | 29 +++++++++++++++++++++++---- src/pinentry.rs | 14 +++++++++++-- src/protocol.rs | 39 ++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/bin/rbw-agent/actions.rs b/src/bin/rbw-agent/actions.rs index 9ddd2ad9..52d8091a 100644 --- a/src/bin/rbw-agent/actions.rs +++ b/src/bin/rbw-agent/actions.rs @@ -850,14 +850,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>, ) -> anyhow::Result> { - 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 +909,12 @@ pub async fn find_ssh_private_key( state: std::sync::Arc>, request_public_key: ssh_agent_lib::ssh_key::PublicKey, ) -> anyhow::Result { - 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(); diff --git a/src/pinentry.rs b/src/pinentry.rs index ab316d72..39d0f300 100644 --- a/src/pinentry.rs +++ b/src/pinentry.rs @@ -15,8 +15,18 @@ 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. + let mut args = vec!["--timeout".into(), "120".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()]); } diff --git a/src/protocol.rs b/src/protocol.rs index ec0c06eb..1db340a8 100644 --- a/src/protocol.rs +++ b/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 @@ -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 { From dbb3c6594908af3db286c71c20e89ea1da1dd301 Mon Sep 17 00:00:00 2001 From: Tin Lai Date: Thu, 4 Jun 2026 20:33:37 +1000 Subject: [PATCH 2/3] make pinentry prompt timeout configurable add a `pinentry_timeout` config option controlling how long an unanswered pinentry prompt lingers before pinentry dismisses it. defaults to 120 seconds (previously hardcoded); set to 0 to disable the timeout and restore the old hang-forever behaviour. --- README.md | 5 +++++ src/bin/rbw-agent/actions.rs | 11 +++++++++++ src/bin/rbw/commands.rs | 8 ++++++++ src/config.rs | 7 +++++++ src/pinentry.rs | 6 ++++-- 5 files changed, 35 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index eb9074b8..8b0b8f64 100644 --- a/README.md +++ b/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 diff --git a/src/bin/rbw-agent/actions.rs b/src/bin/rbw-agent/actions.rs index 52d8091a..40f5493a 100644 --- a/src/bin/rbw-agent/actions.rs +++ b/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 { Ok(config.pinentry) } +async fn config_pinentry_timeout() -> anyhow::Result { + let config = rbw::config::Config::load_async().await?; + Ok(config.pinentry_timeout) +} + pub async fn subscribe_to_notifications( state: std::sync::Arc>, ) -> anyhow::Result<()> { diff --git a/src/bin/rbw/commands.rs b/src/bin/rbw/commands.rs index bddf0efe..fa97e196 100644 --- a/src/bin/rbw/commands.rs +++ b/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()?; diff --git a/src/config.rs b/src/config.rs index 248c603c..9b0c3a9a 100644 --- a/src/config.rs +++ b/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, // 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() diff --git a/src/pinentry.rs b/src/pinentry.rs index 39d0f300..8a000f39 100644 --- a/src/pinentry.rs +++ b/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>, @@ -17,8 +18,9 @@ pub async fn getpin( .stdout(std::process::Stdio::piped()); // 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. - let mut args = vec!["--timeout".into(), "120".into()]; + // 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 From 863f03de3fd0dd57c1d5460b632fad081106ef10 Mon Sep 17 00:00:00 2001 From: Tin Lai Date: Thu, 4 Jun 2026 21:00:42 +1000 Subject: [PATCH 3/3] clippy Signed-off-by: Tin Lai --- src/api.rs | 5 ++--- src/bin/rbw-agent/debugger.rs | 6 +++--- src/bin/rbw/commands.rs | 3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/api.rs b/src/api.rs index a817fb26..a4885708 100644 --- a/src/api.rs +++ b/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 { } } } - } _ => {} } diff --git a/src/bin/rbw-agent/debugger.rs b/src/bin/rbw-agent/debugger.rs index 3a104b5a..8a8a2f80 100644 --- a/src/bin/rbw-agent/debugger.rs +++ b/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}" )); } diff --git a/src/bin/rbw/commands.rs b/src/bin/rbw/commands.rs index fa97e196..fb64660f 100644 --- a/src/bin/rbw/commands.rs +++ b/src/bin/rbw/commands.rs @@ -1515,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::>()?;