From 2e19e397617b99c59a2b5ca1a63a377cc9c1704e Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:34:27 +0400 Subject: [PATCH 01/15] fix(ssh): bound agent requests and survive a refused signature --- crates/omnyssh-core/src/ssh/session.rs | 120 +++++++++++++++++++++---- 1 file changed, 103 insertions(+), 17 deletions(-) diff --git a/crates/omnyssh-core/src/ssh/session.rs b/crates/omnyssh-core/src/ssh/session.rs index 38d10c5..6befca3 100644 --- a/crates/omnyssh-core/src/ssh/session.rs +++ b/crates/omnyssh-core/src/ssh/session.rs @@ -398,8 +398,8 @@ pub(crate) async fn connect_and_auth(host: &Host) -> anyhow::Result Duration { // A chain that fails to resolve costs nothing to connect; the caller's own // attempt reports why. let hops = jump_chain(host).await.map_or(0, |chain| chain.len()); - CONNECT_TIMEOUT * (hops as u32 + 1) + // The agent's bound counts too: a caller that gives up mid-signature drops + // russh while it waits for us. + (CONNECT_TIMEOUT + AGENT_BUDGET) * (hops as u32 + 1) } /// The shared russh client configuration (timeouts + keepalives). @@ -668,6 +670,20 @@ async fn try_key_auth( Ok(ok) } +/// How long the SSH agent gets to answer the connect and the key listing. An +/// agent that accepts and never replies must not stall the whole login. +#[cfg(unix)] +const AGENT_TIMEOUT: Duration = Duration::from_secs(5); + +/// How long one agent signature may take. It can wait on the user (a confirm +/// dialog, Touch ID), so it gets longer than the listing. +#[cfg(unix)] +const SIGN_TIMEOUT: Duration = Duration::from_secs(15); + +/// Upper bound on the agent's share of one hop's login, for callers that time +/// the whole connect. +const AGENT_BUDGET: Duration = Duration::from_secs(20); + #[cfg(unix)] async fn try_agent_auth( handle: &mut Handle, @@ -675,27 +691,97 @@ async fn try_agent_auth( ) -> anyhow::Result { use russh::keys::agent::client::AgentClient; - let mut agent = AgentClient::connect_env() - .await - .context("connect to SSH agent")?; - - let identities = agent - .request_identities() - .await - .context("request agent identities")?; + let (agent, identities) = time::timeout(AGENT_TIMEOUT, async { + let mut agent = AgentClient::connect_env() + .await + .context("connect to SSH agent")?; + let identities = agent + .request_identities() + .await + .context("request agent identities")?; + Ok::<_, anyhow::Error>((agent, identities)) + }) + .await + .map_err(|_| anyhow!("SSH agent did not answer"))??; + let failed = Arc::new(tokio::sync::Notify::new()); + let mut signer = AgentSigner { + agent: Some(agent), + failed: Arc::clone(&failed), + }; for pubkey in identities { - let (agent_back, result) = handle.authenticate_future(user, pubkey, agent).await; - agent = agent_back; - match result { - Ok(true) => return Ok(true), - Ok(false) => continue, - Err(_) => continue, + let attempt = handle.authenticate_future(user, pubkey, signer); + tokio::pin!(attempt); + let (back, result) = tokio::select! { + biased; + done = &mut attempt => done, + () = failed.notified() => { + // russh got the buffer back unsigned, sent nothing and now waits + // for a reply that will not come. Let it finish handing the + // buffer over, then leave the agent out of this login. + let _ = time::timeout(Duration::from_millis(100), &mut attempt).await; + tracing::debug!("SSH agent did not sign; trying other methods"); + return Ok(false); + } + }; + signer = back; + if matches!(result, Ok(true)) { + return Ok(true); } } Ok(false) } +/// Signs through the SSH agent without ever leaving russh waiting. +/// +/// russh 0.46 treats a signer error as final for the connection: it keeps +/// waiting for the signature and swallows every later auth request, so one +/// refused or stalled signature hung the login. Handing the buffer back +/// unchanged makes russh send nothing and carry on, and [`try_agent_auth`] moves +/// on to the other methods. +#[cfg(unix)] +struct AgentSigner { + agent: Option>, + failed: Arc, +} + +#[cfg(unix)] +impl russh::Signer for AgentSigner { + type Error = russh::AgentAuthError; + type Future = std::pin::Pin< + Box)> + Send>, + >; + + fn auth_publickey_sign( + mut self, + key: &russh::keys::key::PublicKey, + to_sign: russh::CryptoVec, + ) -> Self::Future { + let key = key.clone(); + Box::pin(async move { + let mut signed = None; + if let Some(agent) = self.agent.take() { + // A timed-out request leaves the agent connection mid-reply, so + // it is dropped with the future. + if let Ok((agent, result)) = + time::timeout(SIGN_TIMEOUT, agent.sign_request(&key, to_sign.clone())).await + { + self.agent = Some(agent); + // An agent reply russh cannot read comes back unchanged. + signed = result.ok().filter(|data| data.len() != to_sign.len()); + } + } + match signed { + Some(data) => (self, Ok(data)), + None => { + self.failed.notify_one(); + (self, Ok(to_sign)) + } + } + }) + } +} + /// Try password-based authentication. /// /// # Errors From 1fdc50c5c234121a4d896a106065b3bad222707a Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:36:53 +0400 Subject: [PATCH 02/15] fix(ssh): offer the saved password by keyboard-interactive too --- crates/omnyssh-core/src/ssh/session.rs | 276 ++++++++++++++++++++----- 1 file changed, 219 insertions(+), 57 deletions(-) diff --git a/crates/omnyssh-core/src/ssh/session.rs b/crates/omnyssh-core/src/ssh/session.rs index 6befca3..7e38dcd 100644 --- a/crates/omnyssh-core/src/ssh/session.rs +++ b/crates/omnyssh-core/src/ssh/session.rs @@ -13,6 +13,7 @@ //! - Command timeout: 30 seconds use std::fmt; +use std::future::Future; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::time::Duration; @@ -461,6 +462,32 @@ async fn connect_direct( config: &Arc, host: &Host, ) -> anyhow::Result> { + let dial = || dial_direct(config, host); + finish_auth(dial().await?, host, dial).await +} + +/// Reaches `host` through the already-connected bastion `via`: a `direct-tcpip` +/// channel on the bastion carries a second SSH session to the target, which is +/// verified and authenticated in its own right. +async fn connect_tunnelled( + config: &Arc, + via: &Handle, + host: &Host, +) -> anyhow::Result> { + let dial = || dial_tunnelled(config, via, host); + finish_auth(dial().await?, host, dial).await +} + +/// A connection that has shaken hands and passed the host-key check, not yet +/// authenticated. +struct Dialed { + handle: Handle, + /// Set if the server ends the session with a DISCONNECT of its own. + hung_up: Arc, +} + +/// Opens a TCP connection to `host` and verifies its host key. +async fn dial_direct(config: &Arc, host: &Host) -> anyhow::Result { let addr = format!("{}:{}", host.hostname, host.port); let hung_up = Arc::new(AtomicBool::new(false)); let handle = time::timeout( @@ -474,18 +501,16 @@ async fn connect_direct( .await .map_err(|_| anyhow!("SSH connection timed out (10 s)"))? .context("SSH connection failed")?; - - finish_auth(handle, host, &hung_up).await + Ok(Dialed { handle, hung_up }) } -/// Reaches `host` through the already-connected bastion `via`: a `direct-tcpip` -/// channel on the bastion carries a second SSH session to the target, which is -/// verified and authenticated in its own right. -async fn connect_tunnelled( +/// Opens a `direct-tcpip` channel to `host` on the bastion `via` and runs the +/// SSH handshake over it. +async fn dial_tunnelled( config: &Arc, via: &Handle, host: &Host, -) -> anyhow::Result> { +) -> anyhow::Result { // The originator address is informational; ssh(1) reports the loopback it // forwards from, and servers only log it. // @@ -512,8 +537,7 @@ async fn connect_tunnelled( .await .map_err(|_| anyhow!("SSH connection timed out (10 s)"))? .context("SSH connection failed")?; - - finish_auth(handle, host, &hung_up).await + Ok(Dialed { handle, hung_up }) } /// The host-key verifier for `host`. The lookup uses the target's own @@ -527,21 +551,42 @@ fn known_hosts_handler(host: &Host, hung_up: &Arc) -> KnownHostsHand } } -/// Authenticates `handle` as `host`, converting a refusal into an error. -async fn finish_auth( - mut handle: Handle, +/// Authenticates the `first` connection as `host`, converting a refusal into +/// an error. `dial` opens another connection to the same hop, for the +/// keyboard-interactive fallback. +async fn finish_auth( + first: Dialed, host: &Host, - hung_up: &AtomicBool, -) -> anyhow::Result> { - match authenticate(&mut handle, host).await? { - AuthOutcome::Ok => return Ok(handle), - AuthOutcome::PassphraseRequired { path } => { - return Err(PassphraseRequired::new(path).into()) + dial: F, +) -> anyhow::Result> +where + F: Fn() -> Fut, + Fut: Future>, +{ + let Dialed { + mut handle, + hung_up, + } = first; + let encrypted_key = match authenticate(&mut handle, host).await? { + KeyAuth::Accepted => return Ok(handle), + KeyAuth::Rejected { encrypted_key } => encrypted_key, + }; + + // The server login password, never a key passphrase. Tried last: keys are + // what OmnySSH steers users towards. + if let Some(password) = &host.password { + match try_password(&mut handle, &dial, host, password).await { + Offer::Here => return Ok(password_login(host, handle)), + Offer::There(fresh) => return Ok(password_login(host, fresh)), + Offer::No => {} } - AuthOutcome::Failed => {} + } + + if let Some(path) = encrypted_key { + return Err(PassphraseRequired::new(path).into()); } let message = format!("SSH authentication failed for {}", host.name); - // `authenticate` folds a dropped link into "not accepted". A connection + // Every attempt folds a dropped link into "not accepted". A connection // that is gone refused us only if the server hung up itself, as OpenSSH // does after too many failed logins. russh records that as the session // winds down, so let it finish first. @@ -554,20 +599,75 @@ async fn finish_auth( Err(Refused(message).into()) } +fn password_login(host: &Host, handle: Handle) -> Handle { + tracing::info!( + host = %host.name, + "Connected via password authentication — consider setting up SSH key" + ); + handle +} + +/// Where an offered password got in, if anywhere. +enum Offer { + /// On the connection it was offered on. + Here, + /// On a fresh connection, by keyboard-interactive. + There(Handle), + No, +} + +/// Offers `password` the way ssh(1) does: by the password method, then by +/// keyboard-interactive, which is all some servers take (UniFi consoles turn the +/// password method off). russh 0.46 answers keyboard-interactive only as the +/// first method of a connection, so that part runs on a fresh one. The same +/// holds when the server already hung up on the key attempts. +async fn try_password( + handle: &mut Handle, + dial: &F, + host: &Host, + password: &str, +) -> Offer +where + F: Fn() -> Fut, + Fut: Future>, +{ + let offered = !handle.is_closed(); + if offered && try_password_auth(handle, &host.user, password).await { + return Offer::Here; + } + let mut fresh = match dial().await { + Ok(dialed) => dialed.handle, + Err(e) => { + tracing::debug!(host = %host.name, error = %e, "keyboard-interactive dial failed"); + return Offer::No; + } + }; + if keyboard_interactive(&mut fresh, &host.user, password).await + || (!offered && try_password_auth(&mut fresh, &host.user, password).await) + { + return Offer::There(fresh); + } + Offer::No +} + // --------------------------------------------------------------------------- // Authentication helpers // --------------------------------------------------------------------------- -enum AuthOutcome { - Ok, - Failed, - PassphraseRequired { path: String }, +enum KeyAuth { + Accepted, + /// No key got in; `encrypted_key` is one that was skipped for want of its + /// passphrase. + Rejected { + encrypted_key: Option, + }, } +/// Tries the agent, the identity file and the default keys, in that order. async fn authenticate( handle: &mut Handle, host: &Host, -) -> anyhow::Result { +) -> anyhow::Result { let user = host.user.clone(); let mut encrypted_key: Option = None; @@ -576,14 +676,14 @@ async fn authenticate( #[cfg(unix)] { if try_agent_auth(handle, &user).await.unwrap_or(false) { - return Ok(AuthOutcome::Ok); + return Ok(KeyAuth::Accepted); } } // 2. Try explicit identity_file from host config. if let Some(key_path) = &host.identity_file { match try_key_auth(handle, &user, key_path).await { - Ok(true) => return Ok(AuthOutcome::Ok), + Ok(true) => return Ok(KeyAuth::Accepted), Ok(false) => {} Err(e) => note_encrypted(&mut encrypted_key, e), } @@ -596,7 +696,7 @@ async fn authenticate( if key_path.exists() { let path_str = key_path.to_string_lossy().into_owned(); match try_key_auth(handle, &user, &path_str).await { - Ok(true) => return Ok(AuthOutcome::Ok), + Ok(true) => return Ok(KeyAuth::Accepted), Ok(false) => {} Err(e) if host.identity_file.is_none() => note_encrypted(&mut encrypted_key, e), Err(_) => {} @@ -604,27 +704,7 @@ async fn authenticate( } } - // 4. Try password authentication if provided. - // Password auth is NOT recommended for production use but is required for - // the initial connection before setting up key-based auth. This is the - // server login password, never the private-key passphrase. - if let Some(password) = &host.password { - if try_password_auth(handle, &user, password) - .await - .unwrap_or(false) - { - tracing::info!( - host = %host.name, - "Connected via password authentication — consider setting up SSH key" - ); - return Ok(AuthOutcome::Ok); - } - } - - if let Some(path) = encrypted_key { - return Ok(AuthOutcome::PassphraseRequired { path }); - } - Ok(AuthOutcome::Failed) + Ok(KeyAuth::Rejected { encrypted_key }) } fn note_encrypted(encrypted_key: &mut Option, err: anyhow::Error) { @@ -782,20 +862,69 @@ impl russh::Signer for AgentSigner { } } -/// Try password-based authentication. -/// -/// # Errors -/// Returns an error if the authentication attempt fails. +/// Password-method login; a failed request counts as refused. async fn try_password_auth( handle: &mut Handle, user: &str, password: &str, -) -> anyhow::Result { - let ok = handle +) -> bool { + handle .authenticate_password(user, password) .await - .context("authenticate with password")?; - Ok(ok) + .unwrap_or(false) +} + +/// Rounds of server prompts one keyboard-interactive login may take. +const KBD_ROUNDS: usize = 4; + +/// How long each keyboard-interactive reply may take; a wrong password makes +/// PAM stall for a few seconds. +const KBD_TIMEOUT: Duration = Duration::from_secs(10); + +/// Keyboard-interactive login that answers the server's password prompt with +/// `password`. Must be the first method on the connection (russh 0.46). +async fn keyboard_interactive( + handle: &mut Handle, + user: &str, + password: &str, +) -> bool { + use russh::client::KeyboardInteractiveAuthResponse as Reply; + + let mut password = Some(password); + let mut reply = time::timeout( + KBD_TIMEOUT, + handle.authenticate_keyboard_interactive_start(user, None), + ) + .await; + for _ in 0..KBD_ROUNDS { + let prompts = match reply { + Ok(Ok(Reply::Success)) => return true, + Ok(Ok(Reply::InfoRequest { prompts, .. })) => prompts, + _ => return false, + }; + // Always answered, even when we cannot: russh waits for the answer and + // swallows everything else until it has one. + let answers = kbd_answers(&prompts, &mut password); + reply = time::timeout( + KBD_TIMEOUT, + handle.authenticate_keyboard_interactive_respond(answers), + ) + .await; + } + false +} + +/// Answers to one round of keyboard-interactive prompts: the password goes, once, +/// to a lone hidden prompt; anything else (a code, a visible question) gets a +/// blank the server will refuse. +fn kbd_answers(prompts: &[russh::client::Prompt], password: &mut Option<&str>) -> Vec { + match prompts { + [only] if !only.echo => match password.take() { + Some(password) => vec![password.to_string()], + None => vec![String::new()], + }, + _ => vec![String::new(); prompts.len()], + } } // --------------------------------------------------------------------------- @@ -849,6 +978,39 @@ mod tests { ); } + fn prompt(text: &str, echo: bool) -> russh::client::Prompt { + russh::client::Prompt { + prompt: text.to_string(), + echo, + } + } + + #[test] + fn the_password_answers_one_hidden_prompt_only() { + let mut password = Some("secret"); + let hidden = [prompt("Password: ", false)]; + assert_eq!(kbd_answers(&hidden, &mut password), ["secret"]); + // A second ask (a code, a retry) must not get it again. + assert_eq!(kbd_answers(&hidden, &mut password), [""]); + } + + #[test] + fn other_prompts_are_answered_blank() { + let mut password = Some("secret"); + assert!(kbd_answers(&[], &mut password).is_empty()); + assert_eq!( + kbd_answers(&[prompt("Username: ", true)], &mut password), + [""] + ); + let two = [prompt("Password: ", false), prompt("Code: ", false)]; + assert_eq!(kbd_answers(&two, &mut password), ["", ""]); + assert_eq!( + password, + Some("secret"), + "never spent on a prompt it did not answer" + ); + } + #[test] fn a_locked_key_is_found_under_added_context() { let e = anyhow::Error::from(PassphraseRequired::new(String::from("/k/id"))) From 27e9a67253babc23d474724636e914a13e100d4d Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:37:14 +0400 Subject: [PATCH 03/15] fix(ssh): report why a host could not be reached --- crates/omnyssh-core/src/ssh/pool.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/omnyssh-core/src/ssh/pool.rs b/crates/omnyssh-core/src/ssh/pool.rs index e52160e..00f06f0 100644 --- a/crates/omnyssh-core/src/ssh/pool.rs +++ b/crates/omnyssh-core/src/ssh/pool.rs @@ -240,8 +240,12 @@ async fn run_ssh_poller( discovery_done = false; // Reset discovery flag on new connection } Err(e) => { - tracing::debug!(host = %host.name, error = %e, "connection failed"); - send_status(&tx, &host.name, ConnectionStatus::Failed(e.to_string())).await; + // The whole chain: "SSH connection failed" alone does not say + // whether the port was closed, the name did not resolve or the + // host key changed. + let reason = format!("{e:#}"); + tracing::debug!(host = %host.name, error = %reason, "connection failed"); + send_status(&tx, &host.name, ConnectionStatus::Failed(reason)).await; let delay = backoff.next_delay(); let Some(path) = passphrase_required(&e).map(str::to_owned) else { // Wait with backoff, allowing early refresh. From e7de07d19f100a7b651af3784aace8bc820a2613 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:37:32 +0400 Subject: [PATCH 04/15] fix(tui): keep login passwords out of the debug log --- crates/omnyssh/src/main.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/omnyssh/src/main.rs b/crates/omnyssh/src/main.rs index efdba01..aecdfb0 100644 --- a/crates/omnyssh/src/main.rs +++ b/crates/omnyssh/src/main.rs @@ -38,11 +38,14 @@ async fn main() -> anyhow::Result<()> { let file_appender = tracing_appender::rolling::daily(&log_dir, "omnyssh.log"); let (non_blocking, _guard) = tracing_appender::non_blocking(file_appender); + // russh dumps auth packets, login passwords included, at debug and trace. + // These caps come after RUST_LOG, so they hold whatever it asks for. + let filter = tracing_subscriber::EnvFilter::try_from_default_env() + .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new(log_level)) + .add_directive("russh::client::encrypted=info".parse()?) + .add_directive("russh::session=debug".parse()?); tracing_subscriber::fmt() - .with_env_filter( - tracing_subscriber::EnvFilter::try_from_default_env() - .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new(log_level)), - ) + .with_env_filter(filter) .with_writer(non_blocking) .init(); From c8b77f1c0f9ff947ed363171daae4b1386702ff7 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:38:35 +0400 Subject: [PATCH 05/15] fix(gui): show why a host is down on its card --- crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte | 7 +++++++ crates/omnyssh-gui/ui/src/lib/screens/serverCard.test.ts | 9 +++++++++ crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte b/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte index 1569c6c..9d3c3f7 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/Dashboard.svelte @@ -360,6 +360,13 @@ {/if} {/if} + + {#if card.failure} +

+ {$streamerMode ? 'Details hidden in streamer mode' : card.failure} +

+ {/if} + {#if card.detectedServices.length}
diff --git a/crates/omnyssh-gui/ui/src/lib/screens/serverCard.test.ts b/crates/omnyssh-gui/ui/src/lib/screens/serverCard.test.ts index 033b038..8b86f01 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/serverCard.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/screens/serverCard.test.ts @@ -66,6 +66,15 @@ describe('deriveCard — health state', () => { expect(card.offline).toBe(true); }); + it('says why a failed host is down, and nothing otherwise', () => { + const failed: ConnectionStatusDto = { kind: 'failed', message: 'SSH connection failed: Connection refused' }; + expect(deriveCard(host(), failed, undefined, undefined).failure).toBe(failed.message); + expect(deriveCard(host(), failed, metrics({ cpuPercent: 40 }), undefined).failure).toBe(failed.message); + expect(deriveCard(tcpHost(), failed, undefined, undefined).failure).toBe(failed.message); + expect(deriveCard(host(), CONNECTED, undefined, undefined).failure).toBeUndefined(); + expect(deriveCard(host(), { kind: 'connecting' }, undefined, undefined).failure).toBeUndefined(); + }); + it('a failed host keeps showing its last metrics rather than an offline state', () => { const card = deriveCard(host(), { kind: 'failed', message: 'refused' }, metrics({ cpuPercent: 40 }), undefined); expect(card.overall).toBe('off'); diff --git a/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts b/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts index ef79f73..efb1297 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts +++ b/crates/omnyssh-gui/ui/src/lib/screens/serverCard.ts @@ -46,6 +46,8 @@ export interface ServerCard { offline: boolean; /** Set only for a reachability host, which has no metrics to show. */ reachability?: Reachability; + /** Why the last connection attempt failed; unset unless it did. */ + failure?: string; metricRows: MetricRow[]; uptime?: string; osInfo?: string; @@ -162,6 +164,7 @@ export function deriveCard( overall: kind === 'connected' ? 'ok' : kind === 'failed' ? 'off' : 'unknown', offline: kind === 'failed', reachability: kind === 'connected' ? 'reachable' : kind === 'failed' ? 'unreachable' : 'checking', + failure: failureOf(status), metricRows: [], topProcesses: [], detectedServices: [], @@ -191,6 +194,7 @@ export function deriveCard( host, overall, offline, + failure: failureOf(status), metricRows, uptime: m?.uptime ?? undefined, osInfo: m?.osInfo ?? undefined, @@ -201,6 +205,10 @@ export function deriveCard( }; } +function failureOf(status: ConnectionStatusDto | undefined): string | undefined { + return status?.kind === 'failed' ? status.message : undefined; +} + /** Live dashboard cards, one per host, recomputed as any live store changes. */ export const serverCards = derived( [hosts, statuses, metrics, services, tunnels], From 7e5bcc5418f709d498dcb6630697b4f964b20d58 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:52:41 +0400 Subject: [PATCH 06/15] feat(ssh): ask for the login password when no key gets in --- crates/omnyssh-core/src/event.rs | 11 + crates/omnyssh-core/src/ssh/key_setup.rs | 13 +- crates/omnyssh-core/src/ssh/mod.rs | 1 + crates/omnyssh-core/src/ssh/password.rs | 260 +++++++++++++++++++++++ crates/omnyssh-core/src/ssh/pool.rs | 26 ++- crates/omnyssh-core/src/ssh/pty.rs | 207 +++++++++++++++++- crates/omnyssh-core/src/ssh/session.rs | 166 +++++++++++++-- crates/omnyssh-core/src/ssh/sftp.rs | 27 ++- crates/omnyssh-core/src/ssh/tunnel.rs | 54 ++++- crates/omnyssh/src/app/action.rs | 4 + crates/omnyssh/src/app/actions.rs | 17 ++ crates/omnyssh/src/app/file_manager.rs | 51 ++--- crates/omnyssh/src/app/input.rs | 23 ++ crates/omnyssh/src/app/mod.rs | 44 ++++ crates/omnyssh/src/ui/mod.rs | 2 + crates/omnyssh/src/ui/popup.rs | 109 ++++++++-- 16 files changed, 920 insertions(+), 95 deletions(-) create mode 100644 crates/omnyssh-core/src/ssh/password.rs diff --git a/crates/omnyssh-core/src/event.rs b/crates/omnyssh-core/src/event.rs index 4df00ef..9576928 100644 --- a/crates/omnyssh-core/src/event.rs +++ b/crates/omnyssh-core/src/event.rs @@ -153,6 +153,17 @@ pub enum CoreEvent { /// A private key is encrypted and no passphrase is cached for it yet. /// Frontends prompt once per key path and call [`crate::ssh::identity::unlock`]. KeyPassphraseRequired { host_name: HostId, key_path: String }, + /// A connection to `host_name` waits for the login password of `login` + /// (`user@host`). Frontends answer with [`crate::ssh::password::answer`]; + /// `retry` says the previous one was refused. + PasswordRequired { + request_id: u64, + host_name: HostId, + login: String, + retry: bool, + }, + /// The connection behind a [`CoreEvent::PasswordRequired`] stopped waiting. + PasswordPromptClosed(u64), // ----------------------------------------------------------------------- // Update checker events diff --git a/crates/omnyssh-core/src/ssh/key_setup.rs b/crates/omnyssh-core/src/ssh/key_setup.rs index 95dda30..16e4905 100644 --- a/crates/omnyssh-core/src/ssh/key_setup.rs +++ b/crates/omnyssh-core/src/ssh/key_setup.rs @@ -19,7 +19,7 @@ use tokio::time; use tracing::{error, info, warn}; use crate::ssh::client::Host; -use crate::ssh::session::{self, SshSession}; +use crate::ssh::session::{self, Passwords, SshSession}; // --------------------------------------------------------------------------- // Constants @@ -662,7 +662,9 @@ async fn setup_key_internal( test_host.identity_file = Some(private_key_path.to_string_lossy().to_string()); test_host.password = None; // Force key-only auth. - match time::timeout(verify_timeout, SshSession::connect(&test_host)).await { + // Keys only: a password typed earlier this session would let a broken key + // pass, and the next step turns password logins off. + match time::timeout(verify_timeout, connect_keys_only(&test_host)).await { Ok(Ok(test_session)) => { info!("Key authentication verified successfully!"); test_session.disconnect().await; @@ -763,7 +765,7 @@ async fn setup_key_internal( if let Some(ref tx) = progress_tx { let _ = tx.send(KeySetupStep::FinalCheck).await; } - match time::timeout(verify_timeout, SshSession::connect(&test_host)).await { + match time::timeout(verify_timeout, connect_keys_only(&test_host)).await { Ok(Ok(final_session)) => { info!("Final verification passed! Key setup complete."); final_session.disconnect().await; @@ -791,6 +793,11 @@ async fn setup_key_internal( } /// Attempts to rollback sshd_config to the most recent OmnySSH backup. +/// Connects with the host's keys and nothing else. +async fn connect_keys_only(host: &Host) -> Result { + SshSession::connect_with(host, Passwords::SavedOnly).await +} + async fn emergency_rollback(session: &SshSession) -> Result<()> { warn!("Attempting emergency rollback of sshd_config"); let rollback_cmd = build_rollback_command(); diff --git a/crates/omnyssh-core/src/ssh/mod.rs b/crates/omnyssh-core/src/ssh/mod.rs index f9a39bf..1a5974f 100644 --- a/crates/omnyssh-core/src/ssh/mod.rs +++ b/crates/omnyssh-core/src/ssh/mod.rs @@ -9,6 +9,7 @@ pub mod identity; pub mod jump; pub mod key_setup; pub mod metrics; +pub mod password; pub mod pool; pub mod probe; pub mod pty; diff --git a/crates/omnyssh-core/src/ssh/password.rs b/crates/omnyssh-core/src/ssh/password.rs new file mode 100644 index 0000000..18d7906 --- /dev/null +++ b/crates/omnyssh-core/src/ssh/password.rs @@ -0,0 +1,260 @@ +//! Login passwords typed at a prompt. +//! +//! A password is kept in process memory only — never written to disk — and +//! only once a server has accepted it. It is keyed by the login it was typed +//! for: user, host, port and the bastions on the way, so it is never offered to +//! another server that shares an address behind a different bastion. + +use std::collections::HashMap; +use std::sync::{Mutex, MutexGuard, OnceLock, PoisonError}; + +use async_trait::async_trait; +use thiserror::Error; +use tokio::sync::{mpsc, oneshot, watch}; + +use crate::event::CoreEvent; + +#[derive(Default)] +struct State { + /// Passwords a server accepted, by login key. + accepted: HashMap, + /// Prompts waiting for an answer, by request id. + pending: HashMap>>, + last_request: u64, +} + +fn state() -> MutexGuard<'static, State> { + static STATE: OnceLock> = OnceLock::new(); + STATE + .get_or_init(Mutex::default) + .lock() + .unwrap_or_else(PoisonError::into_inner) +} + +/// Bumped whenever a password is accepted, so a connection waiting for one can +/// go again. +fn accepted_signal() -> &'static watch::Sender<()> { + static ACCEPTED: OnceLock> = OnceLock::new(); + ACCEPTED.get_or_init(|| watch::channel(()).0) +} + +/// The password a server accepted for `key` this session. +pub(crate) fn accepted(key: &str) -> Option { + state().accepted.get(key).cloned() +} + +/// Remembers `password` for `key`; call only once the server took it. +pub(crate) fn remember(key: &str, password: &str) { + state() + .accepted + .insert(key.to_string(), password.to_string()); + accepted_signal().send_replace(()); +} + +/// Forgets `password` for `key` after the server turned it down. A newer one +/// remembered meanwhile stays. +pub(crate) fn forget(key: &str, password: &str) { + let mut state = state(); + if state.accepted.get(key).map(String::as_str) == Some(password) { + state.accepted.remove(key); + } +} + +/// Resolves once a password is remembered for `key`. +pub(crate) async fn remembered(key: &str) { + // Subscribe before checking, so one remembered in between is not missed. + let mut rx = accepted_signal().subscribe(); + while accepted(key).is_none() { + // The sender lives in a static and is never dropped. + let _ = rx.changed().await; + } +} + +/// An answer that has no prompt to go to. +#[derive(Debug, Error)] +pub enum PasswordError { + /// The request was answered already, or its connection stopped waiting. + #[error("no login is waiting for this password")] + NotRequested, +} + +/// Answers the prompt `request_id` of a [`CoreEvent::PasswordRequired`]: +/// `Some(password)` to try it, `None` to cancel the login. +/// +/// # Errors +/// [`PasswordError::NotRequested`] when no connection waits on that request. +pub fn answer(request_id: u64, password: Option) -> Result<(), PasswordError> { + let reply = state() + .pending + .remove(&request_id) + .ok_or(PasswordError::NotRequested)?; + reply + .send(password) + .map_err(|_| PasswordError::NotRequested) +} + +/// Asks the user for a login password while a connection authenticates. +#[async_trait] +pub(crate) trait AskPassword: Send { + /// The password for `login` (`user@host`), or `None` when the user + /// cancelled. `retry` says the previous one was refused. + async fn ask(&mut self, login: &str, retry: bool) -> Option; +} + +/// Asks through the frontends: sends [`CoreEvent::PasswordRequired`] and waits +/// for [`answer`]. +pub struct Prompter { + tx: mpsc::Sender, + host_name: String, +} + +impl Prompter { + /// A prompter for connections to `host_name`, reporting on `tx`. + pub fn new(tx: mpsc::Sender, host_name: impl Into) -> Self { + Self { + tx, + host_name: host_name.into(), + } + } +} + +#[async_trait] +impl AskPassword for Prompter { + async fn ask(&mut self, login: &str, retry: bool) -> Option { + let (reply, answer) = oneshot::channel(); + let request_id = { + let mut state = state(); + state.last_request += 1; + let id = state.last_request; + state.pending.insert(id, reply); + id + }; + let _open = OpenPrompt { + request_id, + tx: self.tx.clone(), + }; + let asked = self + .tx + .send(CoreEvent::PasswordRequired { + request_id, + host_name: self.host_name.clone(), + login: login.to_string(), + retry, + }) + .await; + if asked.is_err() { + return None; + } + answer.await.ok().flatten() + } +} + +/// A prompt on screen. Dropped unanswered — the connection gave up, say a +/// tunnel was stopped — it takes the prompt down again. +struct OpenPrompt { + request_id: u64, + tx: mpsc::Sender, +} + +impl Drop for OpenPrompt { + fn drop(&mut self) { + if state().pending.remove(&self.request_id).is_some() { + let _ = self + .tx + .try_send(CoreEvent::PasswordPromptClosed(self.request_id)); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::time::Duration; + + fn next(rx: &mut mpsc::Receiver) -> Option { + rx.try_recv().ok() + } + + async fn asked(rx: &mut mpsc::Receiver) -> u64 { + match tokio::time::timeout(Duration::from_secs(5), rx.recv()).await { + Ok(Some(CoreEvent::PasswordRequired { request_id, .. })) => request_id, + other => panic!("expected a prompt, got {other:?}"), + } + } + + #[tokio::test] + async fn an_answer_reaches_the_connection_that_asked() { + let (tx, mut rx) = mpsc::channel(8); + let mut prompter = Prompter::new(tx, "web-1"); + let asking = tokio::spawn(async move { prompter.ask("root@10.0.0.1", false).await }); + + let id = asked(&mut rx).await; + answer(id, Some(String::from("secret"))).expect("answer"); + assert_eq!(asking.await.expect("ran").as_deref(), Some("secret")); + assert!( + next(&mut rx).is_none(), + "an answered prompt is not closed again" + ); + assert!(matches!(answer(id, None), Err(PasswordError::NotRequested))); + } + + #[tokio::test] + async fn a_cancel_ends_the_login() { + let (tx, mut rx) = mpsc::channel(8); + let mut prompter = Prompter::new(tx, "web-1"); + let asking = tokio::spawn(async move { prompter.ask("root@10.0.0.1", true).await }); + + let id = asked(&mut rx).await; + answer(id, None).expect("cancel"); + assert_eq!(asking.await.expect("ran"), None); + } + + #[tokio::test] + async fn a_connection_that_stops_waiting_takes_its_prompt_down() { + let (tx, mut rx) = mpsc::channel(8); + let mut prompter = Prompter::new(tx, "web-1"); + let asking = tokio::spawn(async move { prompter.ask("root@10.0.0.1", false).await }); + + let id = asked(&mut rx).await; + asking.abort(); + let _ = asking.await; + match next(&mut rx) { + Some(CoreEvent::PasswordPromptClosed(closed)) => assert_eq!(closed, id), + other => panic!("expected the prompt to close, got {other:?}"), + } + assert!(matches!( + answer(id, Some(String::from("late"))), + Err(PasswordError::NotRequested) + )); + } + + #[tokio::test] + async fn a_waiter_wakes_when_its_login_is_remembered() { + let key = "waiter@10.9.9.9:22"; + let waiter = tokio::spawn(async move { remembered(key).await }); + tokio::time::sleep(Duration::from_millis(50)).await; + assert!(!waiter.is_finished()); + + remember("someone-else@10.9.9.9:22", "x"); + tokio::time::sleep(Duration::from_millis(50)).await; + assert!(!waiter.is_finished(), "another login does not wake it"); + + remember(key, "secret"); + tokio::time::timeout(Duration::from_secs(5), waiter) + .await + .expect("the waiter wakes") + .expect("the waiter ran"); + } + + #[test] + fn a_refused_password_is_forgotten_but_a_newer_one_stays() { + let key = "forget@10.8.8.8:22"; + remember(key, "old"); + forget(key, "old"); + assert_eq!(accepted(key), None); + + remember(key, "new"); + forget(key, "old"); + assert_eq!(accepted(key).as_deref(), Some("new")); + } +} diff --git a/crates/omnyssh-core/src/ssh/pool.rs b/crates/omnyssh-core/src/ssh/pool.rs index 00f06f0..018f45d 100644 --- a/crates/omnyssh-core/src/ssh/pool.rs +++ b/crates/omnyssh-core/src/ssh/pool.rs @@ -25,7 +25,8 @@ use crate::ssh::metrics::{ parse_cpu_proc_stat, parse_cpu_top, parse_cpu_top_macos, parse_disk_df, parse_loadavg, parse_ram_free, parse_ram_vmstat, parse_top_processes, parse_uptime, }; -use crate::ssh::session::{passphrase_required, SshSession}; +use crate::ssh::password; +use crate::ssh::session::{passphrase_required, password_required, SshSession}; // --------------------------------------------------------------------------- // Backoff schedule @@ -247,16 +248,23 @@ async fn run_ssh_poller( tracing::debug!(host = %host.name, error = %reason, "connection failed"); send_status(&tx, &host.name, ConnectionStatus::Failed(reason)).await; let delay = backoff.next_delay(); - let Some(path) = passphrase_required(&e).map(str::to_owned) else { + if let Some(path) = passphrase_required(&e).map(str::to_owned) { + identity::ask_passphrase_once(&tx, &host.name, &path).await; + // Only an unlock can change the outcome, so it ends the wait. + tokio::select! { + () = wait_backoff(delay, &mut refresh_rx) => {} + () = identity::unlocked(&path) => {} + } + } else if let Some(login) = password_required(&e).map(str::to_owned) { + // A poller never asks; a password typed for the login + // elsewhere (a terminal) ends the wait. + tokio::select! { + () = wait_backoff(delay, &mut refresh_rx) => {} + () = password::remembered(&login) => {} + } + } else { // Wait with backoff, allowing early refresh. wait_backoff(delay, &mut refresh_rx).await; - continue; - }; - identity::ask_passphrase_once(&tx, &host.name, &path).await; - // Only an unlock can change the outcome, so it ends the wait. - tokio::select! { - () = wait_backoff(delay, &mut refresh_rx) => {} - () = identity::unlocked(&path) => {} } continue; } diff --git a/crates/omnyssh-core/src/ssh/pty.rs b/crates/omnyssh-core/src/ssh/pty.rs index ad3964e..388a7c2 100644 --- a/crates/omnyssh-core/src/ssh/pty.rs +++ b/crates/omnyssh-core/src/ssh/pty.rs @@ -14,13 +14,15 @@ use std::sync::{Arc, Mutex}; use anyhow::{Context, Result}; +use async_trait::async_trait; use russh::ChannelMsg; use tokio::sync::mpsc; use crate::event::CoreEvent; use crate::ssh::client::Host; use crate::ssh::identity; -use crate::ssh::session::{connect_and_auth, passphrase_required, SshConnection}; +use crate::ssh::password::AskPassword; +use crate::ssh::session::{connect_and_auth, passphrase_required, Passwords, SshConnection}; /// Stable numeric identifier for a PTY session (mirrors [`crate::event::SessionId`]). pub type SessionId = u64; @@ -183,17 +185,31 @@ async fn session_task( tx: mpsc::Sender, raw_output: Option)>>, ) { - // Phase A/B: connect, authenticate, and open the remote shell. Failures are - // reported in the status bar and tear the tab down via PtyExited. - let result = async { - let handle = connect_and_auth(&host).await?; - open_shell(&handle, cols, rows).await.map(|ch| (handle, ch)) - } - .await; + // Phase A/B: connect, authenticate — asking for a password in the tab when + // the keys do not get in — and open the remote shell. Failures are reported + // in the status bar and tear the tab down via PtyExited. + let mut prompt = InlinePrompt { + id, + parser: &parser, + tx: &tx, + raw_output: raw_output.as_ref(), + ctrl_rx: &mut ctrl_rx, + size: (cols, rows), + closed: false, + }; + let connected = connect_and_auth(&host, Passwords::Ask(&mut prompt)).await; + let ((cols, rows), closed) = (prompt.size, prompt.closed); + let result = match connected { + Ok(handle) => open_shell(&handle, cols, rows).await.map(|ch| (handle, ch)), + Err(e) => Err(e), + }; let (_handle, mut channel) = match result { Ok(pair) => pair, Err(e) => { - let _ = tx.send(CoreEvent::Error(format!("Terminal: {e}"))).await; + // A tab closed at the password prompt needs no error. + if !closed { + let _ = tx.send(CoreEvent::Error(format!("Terminal: {e}"))).await; + } if let Some(path) = passphrase_required(&e) { identity::ask_passphrase(&tx, &host.name, path).await; } @@ -239,6 +255,123 @@ async fn session_task( // _handle drops here → russh closes the TCP connection. } +/// Asks for the login password inside the tab, the way ssh(1) does: the prompt +/// is drawn as terminal output, and keystrokes are read without echo until +/// Enter. Nothing typed here reaches the server as shell input. +struct InlinePrompt<'a> { + id: SessionId, + parser: &'a Arc>, + tx: &'a mpsc::Sender, + raw_output: Option<&'a mpsc::Sender<(SessionId, Vec)>>, + ctrl_rx: &'a mut mpsc::UnboundedReceiver, + /// The latest window size, for the shell opened once the login is done. + size: (u16, u16), + /// The tab was closed at the prompt. + closed: bool, +} + +impl InlinePrompt<'_> { + async fn print(&self, text: &str) { + feed_parser(self.parser, text.as_bytes()); + let _ = self.tx.send(CoreEvent::PtyOutput(self.id)).await; + if let Some(raw) = self.raw_output { + let _ = raw.send((self.id, text.as_bytes().to_vec())).await; + } + } +} + +#[async_trait] +impl<'a> AskPassword for InlinePrompt<'a> { + async fn ask(&mut self, login: &str, retry: bool) -> Option { + if retry { + self.print("Permission denied, please try again.\r\n").await; + } + self.print(&format!("{login}'s password: ")).await; + let mut line = PasswordLine::default(); + let typed = loop { + match self.ctrl_rx.recv().await { + Some(Ctrl::Input(bytes)) => { + if let Some(typed) = line.feed(&bytes) { + break typed; + } + } + Some(Ctrl::Resize { cols, rows }) => self.size = (cols, rows), + Some(Ctrl::Close) | None => { + self.closed = true; + return None; + } + } + }; + self.print("\r\n").await; + typed + } +} + +/// A password being typed at the inline prompt. +#[derive(Default)] +struct PasswordLine { + bytes: Vec, + escape: Escape, +} + +/// Where the prompt is inside a terminal escape sequence (arrow keys, function +/// keys, paste markers), which are dropped. +#[derive(Default, Clone, Copy)] +enum Escape { + #[default] + None, + Started, + Sequence, +} + +impl PasswordLine { + /// Takes keystrokes. `Some` once the line is done: the password on Enter, + /// `None` on Ctrl+C (or Ctrl+D on an empty line). Input after Enter is + /// dropped, never sent on. + fn feed(&mut self, input: &[u8]) -> Option> { + for &byte in input { + match self.escape { + Escape::Started => { + self.escape = if matches!(byte, b'[' | b'O') { + Escape::Sequence + } else { + Escape::None + }; + continue; + } + // Parameters and intermediates run 0x20..=0x3f; the final byte ends it. + Escape::Sequence => { + if !(0x20..=0x3f).contains(&byte) { + self.escape = Escape::None; + } + continue; + } + Escape::None => {} + } + match byte { + 0x1b => self.escape = Escape::Started, + b'\r' | b'\n' => { + return Some(Some(String::from_utf8_lossy(&self.bytes).into_owned())) + } + 0x03 => return Some(None), + 0x04 if self.bytes.is_empty() => return Some(None), + 0x15 => self.bytes.clear(), + // One character, not one byte. + 0x08 | 0x7f => { + while let Some(last) = self.bytes.pop() { + if last & 0xc0 != 0x80 { + break; + } + } + } + byte if byte < 0x20 => {} + byte => self.bytes.push(byte), + } + } + None + } +} + // --------------------------------------------------------------------------- // PtyManager // --------------------------------------------------------------------------- @@ -379,6 +512,62 @@ impl Default for PtyManager { mod tests { use super::*; + fn typed(chunks: &[&[u8]]) -> Option> { + let mut line = PasswordLine::default(); + chunks.iter().find_map(|chunk| line.feed(chunk)) + } + + #[test] + fn the_password_is_whatever_was_typed_before_enter() { + assert_eq!( + typed(&[b"se", b"cret\r"]), + Some(Some(String::from("secret"))) + ); + assert_eq!(typed(&[b"secret\n"]), Some(Some(String::from("secret")))); + assert_eq!(typed(&[b"\r"]), Some(Some(String::new()))); + assert_eq!(typed(&[b"secret"]), None, "not done before Enter"); + } + + #[test] + fn the_prompt_edits_like_a_line() { + assert_eq!( + typed(&["pässw\x7fword\r".as_bytes()]), + Some(Some(String::from("pässword"))) + ); + assert_eq!( + typed(&["pä\x7f\x7fx\r".as_bytes()]), + Some(Some(String::from("x"))), + "backspace removes a whole character" + ); + assert_eq!( + typed(&[b"wrong\x15right\r"]), + Some(Some(String::from("right"))) + ); + } + + #[test] + fn escape_sequences_are_not_part_of_the_password() { + // Arrow keys, an SS3 function key and bracketed-paste markers. + let keys: &[u8] = b"\x1b[Ase\x1bOPcr\x1b[1;5Det\x1b[200~!\x1b[201~\r"; + assert_eq!(typed(&[keys]), Some(Some(String::from("secret!")))); + } + + #[test] + fn ctrl_c_or_ctrl_d_on_an_empty_line_cancels() { + assert_eq!(typed(&[b"secr\x03"]), Some(None)); + assert_eq!(typed(&[b"\x04"]), Some(None)); + assert_eq!(typed(&[b"a\x04b\r"]), Some(Some(String::from("ab")))); + } + + #[test] + fn nothing_after_enter_is_kept() { + let mut line = PasswordLine::default(); + assert_eq!( + line.feed(b"secret\rls -la\r"), + Some(Some(String::from("secret"))) + ); + } + fn dummy_tx() -> mpsc::Sender { mpsc::channel(1).0 } diff --git a/crates/omnyssh-core/src/ssh/session.rs b/crates/omnyssh-core/src/ssh/session.rs index 7e38dcd..e954320 100644 --- a/crates/omnyssh-core/src/ssh/session.rs +++ b/crates/omnyssh-core/src/ssh/session.rs @@ -26,6 +26,7 @@ use tokio::time; use crate::ssh::client::Host; use crate::ssh::identity::{self, IdentityError}; +use crate::ssh::password::{self, AskPassword}; // --------------------------------------------------------------------------- // russh Handler implementation @@ -163,6 +164,12 @@ fn at_hop(e: anyhow::Error, context: String) -> anyhow::Error { message, } .into() + } else if let Some(login) = password_required(&e) { + PasswordRequired { + login: login.to_owned(), + message, + } + .into() } else if is_refused(&e) { Refused(message).into() } else { @@ -208,6 +215,65 @@ pub fn passphrase_required(e: &anyhow::Error) -> Option<&str> { .map(|locked| locked.path.as_str()) } +// --------------------------------------------------------------------------- +// PasswordRequired +// --------------------------------------------------------------------------- + +/// No key got in and there was no password to try. Not final either: once one +/// is typed for the login elsewhere ([`password::remembered`]) the same +/// connection can go again. +#[derive(Debug)] +pub(crate) struct PasswordRequired { + /// The login key the password is remembered under. + login: String, + message: String, +} + +impl fmt::Display for PasswordRequired { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for PasswordRequired {} + +/// The login a failed connection needs a password for, if that is why it failed. +pub(crate) fn password_required(e: &anyhow::Error) -> Option<&str> { + e.chain() + .find_map(|cause| cause.downcast_ref::()) + .map(|missing| missing.login.as_str()) +} + +// --------------------------------------------------------------------------- +// Passwords +// --------------------------------------------------------------------------- + +/// Which login passwords a connection may use. +pub(crate) enum Passwords<'a> { + /// The host's saved password and one typed for the login this session. + /// Background work: pollers, tunnels that retry, snippets. + Remembered, + /// The host's saved password only. Key setup checks a new key this way; a + /// remembered password would let a broken key pass. + SavedOnly, + /// As [`Passwords::Remembered`], then ask the user. A connection the user + /// started and is watching. + Ask(&'a mut dyn AskPassword), +} + +/// How many passwords the user may type for one login before it fails. +const PASSWORD_PROMPTS: usize = 3; + +/// The key a typed password is remembered under: the login plus every bastion +/// on the way, so a private address behind another bastion never gets it. +fn login_key(host: &Host, via: &[Host]) -> String { + let login = |h: &Host| format!("{}@{}:{}", h.user, h.hostname, h.port); + std::iter::once(login(host)) + .chain(via.iter().rev().map(login)) + .collect::>() + .join(" via ") +} + // --------------------------------------------------------------------------- // SshConnection // --------------------------------------------------------------------------- @@ -257,7 +323,8 @@ impl SshSession { /// 1. SSH agent (unix only, via `SSH_AUTH_SOCK`). /// 2. Identity file specified in the host config (`identity_file`). /// 3. Default key files (`~/.ssh/id_ed25519`, `id_rsa`, etc.). - /// 4. Password (if provided in host config). + /// 4. Password: one typed for this login earlier in the session, then the + /// one in the host config. /// /// A host with a `ProxyJump` is reached through its bastion chain; each hop /// authenticates the same way. @@ -270,8 +337,16 @@ impl SshSession { /// - Network error /// - An unresolvable `ProxyJump` chain (cycle or too many hops) pub async fn connect(host: &Host) -> anyhow::Result { + Self::connect_with(host, Passwords::Remembered).await + } + + /// [`SshSession::connect`] with a say over which passwords it may use. + pub(crate) async fn connect_with( + host: &Host, + passwords: Passwords<'_>, + ) -> anyhow::Result { Ok(Self { - handle: Arc::new(connect_and_auth(host).await?), + handle: Arc::new(connect_and_auth(host, passwords).await?), }) } @@ -369,27 +444,32 @@ const CONNECT_TIMEOUT: Duration = Duration::from_secs(10); /// # Errors /// Connection timeout (> 10 s per hop), host-key rejection, authentication /// failure, or an unresolvable `ProxyJump` chain. -pub(crate) async fn connect_and_auth(host: &Host) -> anyhow::Result { +pub(crate) async fn connect_and_auth( + host: &Host, + mut passwords: Passwords<'_>, +) -> anyhow::Result { let chain = jump_chain(host).await?; let config = client_config(); // Walk the bastions outward: the first is reached directly, every later one // through its predecessor. The target then rides the last hop. let mut jumps: Vec> = Vec::with_capacity(chain.len()); - for hop in &chain { + for (i, hop) in chain.iter().enumerate() { + let key = login_key(hop, &chain[..i]); let handle = match jumps.last() { - None => connect_direct(&config, hop).await, - Some(via) => connect_tunnelled(&config, via, hop).await, + None => connect_direct(&config, hop, &key, &mut passwords).await, + Some(via) => connect_tunnelled(&config, via, hop, &key, &mut passwords).await, } .map_err(|e| at_hop(e, format!("ProxyJump via '{}' failed", hop.name)))?; jumps.push(handle); } + let key = login_key(host, &chain); let handle = match (jumps.last(), chain.last()) { - (Some(via), Some(last)) => connect_tunnelled(&config, via, host) + (Some(via), Some(last)) => connect_tunnelled(&config, via, host, &key, &mut passwords) .await .map_err(|e| at_hop(e, format!("connecting via '{}' failed", last.name)))?, - _ => connect_direct(&config, host).await?, + _ => connect_direct(&config, host, &key, &mut passwords).await?, }; Ok(SshConnection { @@ -461,9 +541,11 @@ async fn jump_chain(host: &Host) -> anyhow::Result> { async fn connect_direct( config: &Arc, host: &Host, + key: &str, + passwords: &mut Passwords<'_>, ) -> anyhow::Result> { let dial = || dial_direct(config, host); - finish_auth(dial().await?, host, dial).await + finish_auth(dial().await?, host, key, dial, passwords).await } /// Reaches `host` through the already-connected bastion `via`: a `direct-tcpip` @@ -473,9 +555,11 @@ async fn connect_tunnelled( config: &Arc, via: &Handle, host: &Host, + key: &str, + passwords: &mut Passwords<'_>, ) -> anyhow::Result> { let dial = || dial_tunnelled(config, via, host); - finish_auth(dial().await?, host, dial).await + finish_auth(dial().await?, host, key, dial, passwords).await } /// A connection that has shaken hands and passed the host-key check, not yet @@ -551,13 +635,15 @@ fn known_hosts_handler(host: &Host, hung_up: &Arc) -> KnownHostsHand } } -/// Authenticates the `first` connection as `host`, converting a refusal into -/// an error. `dial` opens another connection to the same hop, for the -/// keyboard-interactive fallback. +/// Authenticates the `first` connection as `host` (remembered passwords under +/// `key`), converting a refusal into an error. `dial` opens another connection +/// to the same hop, for the keyboard-interactive fallback. async fn finish_auth( first: Dialed, host: &Host, + key: &str, dial: F, + passwords: &mut Passwords<'_>, ) -> anyhow::Result> where F: Fn() -> Fut, @@ -572,12 +658,24 @@ where KeyAuth::Rejected { encrypted_key } => encrypted_key, }; - // The server login password, never a key passphrase. Tried last: keys are - // what OmnySSH steers users towards. - if let Some(password) = &host.password { - match try_password(&mut handle, &dial, host, password).await { + // The server login password, never a key passphrase, and tried last: keys + // are what OmnySSH steers users towards. One typed this session goes first; + // it is newer than any saved one. + let typed = match passwords { + Passwords::SavedOnly => None, + _ => password::accepted(key), + }; + let saved = host.password.clone().filter(|p| typed.as_ref() != Some(p)); + let known = typed.is_some() || saved.is_some(); + for (password, was_typed) in typed + .map(|p| (p, true)) + .into_iter() + .chain(saved.map(|p| (p, false))) + { + match try_password(&mut handle, &dial, host, &password).await { Offer::Here => return Ok(password_login(host, handle)), Offer::There(fresh) => return Ok(password_login(host, fresh)), + Offer::No if was_typed => password::forget(key, &password), Offer::No => {} } } @@ -585,6 +683,40 @@ where if let Some(path) = encrypted_key { return Err(PassphraseRequired::new(path).into()); } + + if let Passwords::Ask(ask) = passwords { + let login = format!("{}@{}", host.user, host.hostname); + let mut retry = known; + for _ in 0..PASSWORD_PROMPTS { + let Some(password) = ask.ask(&login, retry).await else { + return Err(Refused(format!("SSH login cancelled for {}", host.name)).into()); + }; + match try_password(&mut handle, &dial, host, &password).await { + Offer::Here => { + password::remember(key, &password); + return Ok(password_login(host, handle)); + } + Offer::There(fresh) => { + password::remember(key, &password); + return Ok(password_login(host, fresh)); + } + Offer::No => retry = true, + } + } + return Err(Refused(format!("SSH authentication failed for {}", host.name)).into()); + } + + if !known && !matches!(passwords, Passwords::SavedOnly) { + return Err(PasswordRequired { + login: key.to_string(), + message: format!( + "SSH authentication failed for {}: no key was accepted and no password is saved", + host.name + ), + } + .into()); + } + let message = format!("SSH authentication failed for {}", host.name); // Every attempt folds a dropped link into "not accepted". A connection // that is gone refused us only if the server hung up itself, as OpenSSH diff --git a/crates/omnyssh-core/src/ssh/sftp.rs b/crates/omnyssh-core/src/ssh/sftp.rs index ac375e1..940ecaf 100644 --- a/crates/omnyssh-core/src/ssh/sftp.rs +++ b/crates/omnyssh-core/src/ssh/sftp.rs @@ -12,7 +12,8 @@ use tokio::sync::mpsc; use crate::event::{CoreEvent, TransferId}; use crate::ssh::client::Host; -use crate::ssh::session::SshSession; +use crate::ssh::password::Prompter; +use crate::ssh::session::{Passwords, SshSession}; // --------------------------------------------------------------------------- // FileEntry — represents one file or directory in a panel listing @@ -88,6 +89,30 @@ impl SftpManager { let session = SshSession::connect(host) .await .context("SFTP SSH connect")?; + Self::open(host, session, event_tx).await + } + + /// [`SftpManager::connect`] for a session the user opened: a login the keys + /// do not get into asks for the password through `prompter`. + /// + /// # Errors + /// As [`SftpManager::connect`], plus a cancelled password prompt. + pub async fn connect_asking( + host: &Host, + event_tx: mpsc::Sender, + mut prompter: Prompter, + ) -> anyhow::Result { + let session = SshSession::connect_with(host, Passwords::Ask(&mut prompter)) + .await + .context("SFTP SSH connect")?; + Self::open(host, session, event_tx).await + } + + async fn open( + host: &Host, + session: SshSession, + event_tx: mpsc::Sender, + ) -> anyhow::Result { let stream = session .open_sftp_channel() .await diff --git a/crates/omnyssh-core/src/ssh/tunnel.rs b/crates/omnyssh-core/src/ssh/tunnel.rs index 8aef399..e619fc7 100644 --- a/crates/omnyssh-core/src/ssh/tunnel.rs +++ b/crates/omnyssh-core/src/ssh/tunnel.rs @@ -31,8 +31,10 @@ use tokio::time; use crate::event::CoreEvent; use crate::ssh::client::Host; use crate::ssh::identity; +use crate::ssh::password::{self, Prompter}; use crate::ssh::session::{ - connect_and_auth, connect_budget, is_refused, passphrase_required, SshConnection, + connect_and_auth, connect_budget, is_refused, passphrase_required, password_required, + Passwords, SshConnection, }; // --------------------------------------------------------------------------- @@ -194,6 +196,9 @@ const STABLE_AFTER: Duration = Duration::from_secs(60); /// of its own. const AUTH_BUDGET: Duration = Duration::from_secs(20); +/// How long a tunnel the user started waits for its login password to be typed. +const PROMPT_BUDGET: Duration = Duration::from_secs(600); + /// How often a live tunnel checks that its connection still is. A dead peer /// is noticed by the keepalives first; this only picks that up. const LIVENESS_CHECK: Duration = Duration::from_secs(1); @@ -236,25 +241,37 @@ impl TunnelManager { /// Starts `host`'s tunnel, or restarts it when one is already running. The /// new run waits for the old one to release the ports before binding them. + /// The user started it, so a login that needs a password asks for one. /// /// Must be called within a tokio runtime. pub fn start(&mut self, host: Host) { + self.spawn(host, true); + } + + fn spawn(&mut self, host: Host, ask: bool) { let previous = self.runs.remove(&host.name).map(|mut run| { run.stop(); run.task }); let (stop, stop_rx) = oneshot::channel(); - let task = tokio::spawn(run_tunnel(host.clone(), self.tx.clone(), stop_rx, previous)); + let task = tokio::spawn(run_tunnel( + host.clone(), + self.tx.clone(), + stop_rx, + previous, + ask, + )); let stop = Some(stop); self.runs .insert(host.name.clone(), Run { host, stop, task }); } - /// Starts the tunnel of every host marked to start on launch. + /// Starts the tunnel of every host marked to start on launch. Nobody asked + /// for these, so none of them asks for a password. pub fn autostart(&mut self, hosts: &[Host]) { for host in hosts { if host.tunnel_autostart && !host.local_forwards.is_empty() { - self.start(host.clone()); + self.spawn(host.clone(), false); } } } @@ -286,7 +303,7 @@ impl TunnelManager { match hosts.iter().find(|h| h.name == name) { Some(host) if host.local_forwards.is_empty() => self.stop(&name), Some(host) if self.runs.get(&name).is_some_and(|r| changed(&r.host, host)) => { - self.start(host.clone()); + self.spawn(host.clone(), false); } Some(_) => {} None => self.stop(&name), @@ -322,6 +339,7 @@ async fn run_tunnel( tx: mpsc::Sender, mut stop: oneshot::Receiver<()>, previous: Option>, + ask: bool, ) { // A restart must not race its predecessor for the ports — even when it is // stopped first, or the next run would inherit that race. The predecessor has @@ -332,15 +350,16 @@ async fn run_tunnel( let status = tokio::select! { biased; _ = &mut stop => TunnelStatus::Stopped, - status = serve(&host, &tx) => status, + status = serve(&host, &tx, ask) => status, }; // `serve` is dropped by now, so the ports are free again. send_status(&tx, &host.name, status).await; } /// Holds the ports and keeps the connection up. Returns only once the tunnel -/// cannot go on. -async fn serve(host: &Host, tx: &mpsc::Sender) -> TunnelStatus { +/// cannot go on. With `ask`, the first login may ask for a password; redials +/// use the one it got. +async fn serve(host: &Host, tx: &mpsc::Sender, mut ask: bool) -> TunnelStatus { if host.local_forwards.is_empty() { return TunnelStatus::Failed(String::from("no port forwards are set up for this host")); } @@ -354,9 +373,17 @@ async fn serve(host: &Host, tx: &mpsc::Sender) -> TunnelStatus { send_status(tx, &host.name, TunnelStatus::Connecting).await; let mut retry = 0; loop { - let budget = connect_budget(host).await + AUTH_BUDGET; + let mut budget = connect_budget(host).await + AUTH_BUDGET; + let mut prompter = Prompter::new(tx.clone(), &host.name); + let passwords = if std::mem::take(&mut ask) { + budget += PROMPT_BUDGET; + Passwords::Ask(&mut prompter) + } else { + Passwords::Remembered + }; let mut locked = None; - let reason = match time::timeout(budget, connect_and_auth(host)).await { + let mut no_password = None; + let reason = match time::timeout(budget, connect_and_auth(host, passwords)).await { Ok(Ok(conn)) => { send_status(tx, &host.name, TunnelStatus::Up).await; let since = Instant::now(); @@ -369,6 +396,7 @@ async fn serve(host: &Host, tx: &mpsc::Sender) -> TunnelStatus { Ok(Err(e)) if is_refused(&e) => return TunnelStatus::Failed(format!("{e:#}")), Ok(Err(e)) => { locked = passphrase_required(&e).map(str::to_owned); + no_password = password_required(&e).map(str::to_owned); format!("{e:#}") } Err(_) => format!( @@ -386,6 +414,12 @@ async fn serve(host: &Host, tx: &mpsc::Sender) -> TunnelStatus { identity::unlocked(&path).await; continue; } + if let Some(login) = no_password { + // Likewise until a password for the login is typed elsewhere — in a + // terminal, say. + password::remembered(&login).await; + continue; + } time::sleep(RETRY_DELAYS[retry]).await; retry = (retry + 1).min(RETRY_DELAYS.len() - 1); } diff --git a/crates/omnyssh/src/app/action.rs b/crates/omnyssh/src/app/action.rs index ae8f606..fe13d70 100644 --- a/crates/omnyssh/src/app/action.rs +++ b/crates/omnyssh/src/app/action.rs @@ -47,6 +47,10 @@ pub enum AppAction { SubmitPassphrase, /// Dismiss the passphrase prompt without unlocking the key. DismissPassphrase, + /// Send the password typed for the login a connection waits on. + SubmitPassword, + /// Cancel that login. + DismissPassword, // ----------------------------------------------------------------------- // Detail View actions diff --git a/crates/omnyssh/src/app/actions.rs b/crates/omnyssh/src/app/actions.rs index 87c18fa..f6ddabd 100644 --- a/crates/omnyssh/src/app/actions.rs +++ b/crates/omnyssh/src/app/actions.rs @@ -310,6 +310,23 @@ impl App { } } + AppAction::SubmitPassword | AppAction::DismissPassword => { + if self.view.password_prompts.is_empty() { + return Ok(()); + } + let prompt = self.view.password_prompts.remove(0); + let password = + matches!(action, AppAction::SubmitPassword).then_some(prompt.field.value); + // The connection checks it with the server and asks again if + // it is refused. + if omnyssh_core::ssh::password::answer(prompt.request_id, password).is_err() { + self.view.status_message = Some(format!( + "{} is no longer waiting for a password", + prompt.login + )); + } + } + // --------------------------------------------------------------- // Detail View actions // --------------------------------------------------------------- diff --git a/crates/omnyssh/src/app/file_manager.rs b/crates/omnyssh/src/app/file_manager.rs index 6d617a6..a99f148 100644 --- a/crates/omnyssh/src/app/file_manager.rs +++ b/crates/omnyssh/src/app/file_manager.rs @@ -2,10 +2,10 @@ //! directory navigation and SFTP transfers. use std::collections::HashSet; -use std::time::Duration; use super::*; use omnyssh_core::ssh::identity; +use omnyssh_core::ssh::password::Prompter; use omnyssh_core::ssh::sftp::{self, FileEntry, SftpCommand, SftpManager}; // --------------------------------------------------------------------------- @@ -320,45 +320,32 @@ impl App { self.view.file_manager.connected_host = None; self.view.file_manager.remote = FilePanelView::default(); - self.view.status_message = Some(format!("Connecting to '{}'… (30s timeout)", host.name)); + self.view.status_message = Some(format!("Connecting to '{}'…", host.name)); self.view.file_manager.sftp_connecting = true; - // Spawn connection in background with 30s timeout to prevent UI freeze + // In the background: the login may wait on a password prompt. Every + // step of the connect has a bound of its own. let tx = self.core_tx.clone(); let host_clone = host.clone(); tokio::spawn(async move { - let connect_future = SftpManager::connect(&host_clone, tx.clone()); - let timeout_future = tokio::time::sleep(Duration::from_secs(30)); - - tokio::select! { - result = connect_future => { - match result { - Ok(mgr) => { - // Send the manager through a new event type - let _ = tx - .send(CoreEvent::SftpManagerReady { - host_name: host_clone.name.clone(), - manager: Box::new(mgr), - }) - .await; - } - Err(e) => { - if let Some(path) = omnyssh_core::ssh::session::passphrase_required(&e) - { - identity::ask_passphrase(&tx, &host_clone.name, path).await; - } - let _ = tx - .send(CoreEvent::SftpDisconnected { - reason: format!("{e:#}"), - }) - .await; - } - } + let prompter = Prompter::new(tx.clone(), &host_clone.name); + match SftpManager::connect_asking(&host_clone, tx.clone(), prompter).await { + Ok(mgr) => { + // Send the manager through a new event type + let _ = tx + .send(CoreEvent::SftpManagerReady { + host_name: host_clone.name.clone(), + manager: Box::new(mgr), + }) + .await; } - _ = timeout_future => { + Err(e) => { + if let Some(path) = omnyssh_core::ssh::session::passphrase_required(&e) { + identity::ask_passphrase(&tx, &host_clone.name, path).await; + } let _ = tx .send(CoreEvent::SftpDisconnected { - reason: "connection timed out (30s)".to_string(), + reason: format!("{e:#}"), }) .await; } diff --git a/crates/omnyssh/src/app/input.rs b/crates/omnyssh/src/app/input.rs index 48eb11c..1a10c49 100644 --- a/crates/omnyssh/src/app/input.rs +++ b/crates/omnyssh/src/app/input.rs @@ -18,6 +18,10 @@ impl App { if !self.view.passphrase_prompts.is_empty() && !matches!(screen, Screen::Terminal) { return Ok(self.handle_passphrase_key(key)); } + // Same rules for a login password. + if !self.view.password_prompts.is_empty() && !matches!(screen, Screen::Terminal) { + return Ok(self.handle_password_key(key)); + } // The update popup is modal — it captures all input until dismissed. // Ctrl+C still quits as an escape hatch. @@ -228,6 +232,25 @@ impl App { } } + fn handle_password_key(&mut self, key: KeyEvent) -> Option { + let prompt = self.view.password_prompts.first_mut()?; + let ctrl = key.modifiers.contains(KeyModifiers::CONTROL); + match key.code { + KeyCode::Esc => Some(AppAction::DismissPassword), + KeyCode::Char('c') if ctrl => Some(AppAction::DismissPassword), + KeyCode::Enter => Some(AppAction::SubmitPassword), + KeyCode::Backspace => { + prompt.field.backspace(); + None + } + KeyCode::Char(c) if !ctrl => { + prompt.field.insert_char(c); + None + } + _ => None, + } + } + /// Handles key events when the Terminal screen is active. /// /// Returns an [`AppAction`] to pass to `process_action`, or forwards the diff --git a/crates/omnyssh/src/app/mod.rs b/crates/omnyssh/src/app/mod.rs index f0a7202..595e441 100644 --- a/crates/omnyssh/src/app/mod.rs +++ b/crates/omnyssh/src/app/mod.rs @@ -157,6 +157,8 @@ pub struct ViewState { pub update_popup: Option, /// Encrypted keys waiting for a passphrase, one per key; the first is shown. pub passphrase_prompts: Vec, + /// Logins waiting for a password, oldest first; the first is shown. + pub password_prompts: Vec, } impl ViewState { @@ -176,6 +178,7 @@ impl ViewState { tick_count: 0, update_popup: None, passphrase_prompts: Vec::new(), + password_prompts: Vec::new(), } } } @@ -196,6 +199,17 @@ pub struct PassphrasePrompt { pub unlocking: bool, } +/// In-memory prompt for the login password a connection waits on. +pub struct PasswordPrompt { + pub request_id: u64, + pub host_name: String, + /// `user@host`, as the server is asked. + pub login: String, + /// The previous password for this login was refused. + pub retry: bool, + pub field: FormField, +} + // --------------------------------------------------------------------------- // App // --------------------------------------------------------------------------- @@ -467,6 +481,10 @@ impl App { .filter(|c| !matches!(c, '\r' | '\n')) .for_each(|c| prompt.field.insert_char(c)); } + } else if let Some(prompt) = self.view.password_prompts.first_mut() { + text.chars() + .filter(|c| !matches!(c, '\r' | '\n')) + .for_each(|c| prompt.field.insert_char(c)); } else { for key in crate::utils::paste::paste_to_keys(&text) { let action = self.handle_key(key).await?; @@ -841,6 +859,32 @@ impl App { } } + CoreEvent::PasswordRequired { + request_id, + host_name, + login, + retry, + } => { + // As with a passphrase, the terminal screen keeps its keys. + if self.state.read().await.screen == Screen::Terminal { + self.view.status_message = + Some(format!("{login} needs a password — Ctrl+Q to enter it")); + } + self.view.password_prompts.push(PasswordPrompt { + request_id, + host_name, + login, + retry, + field: FormField::default(), + }); + } + + CoreEvent::PasswordPromptClosed(request_id) => { + self.view + .password_prompts + .retain(|p| p.request_id != request_id); + } + // ---------------------------------------------------------------- // Update checker events // ---------------------------------------------------------------- diff --git a/crates/omnyssh/src/ui/mod.rs b/crates/omnyssh/src/ui/mod.rs index 47eb997..532d3ab 100644 --- a/crates/omnyssh/src/ui/mod.rs +++ b/crates/omnyssh/src/ui/mod.rs @@ -94,6 +94,8 @@ pub fn render(frame: &mut Frame, state: &AppState, view: &ViewState) { if !matches!(state.screen, Screen::Terminal) { if let Some(prompt) = view.passphrase_prompts.first() { popup::render_passphrase_prompt(frame, prompt, &view.theme); + } else if let Some(prompt) = view.password_prompts.first() { + popup::render_password_prompt(frame, prompt, &view.theme); } } } diff --git a/crates/omnyssh/src/ui/popup.rs b/crates/omnyssh/src/ui/popup.rs index c6f78be..f2d9ed9 100644 --- a/crates/omnyssh/src/ui/popup.rs +++ b/crates/omnyssh/src/ui/popup.rs @@ -9,8 +9,9 @@ use ratatui::{ }; use crate::app::{ - FormField, HostForm, PassphrasePrompt, SnippetForm, SnippetResultEntry, UpdateButton, - UpdatePopup, UpdatePopupPhase, FORM_FIELD_LABELS, SNIPPET_FORM_FIELD_LABELS, UPDATE_BUTTONS, + FormField, HostForm, PassphrasePrompt, PasswordPrompt, SnippetForm, SnippetResultEntry, + UpdateButton, UpdatePopup, UpdatePopupPhase, FORM_FIELD_LABELS, SNIPPET_FORM_FIELD_LABELS, + UPDATE_BUTTONS, }; use crate::ui::theme::Theme; use omnyssh_core::ssh::client::Host; @@ -1049,6 +1050,59 @@ pub fn render_broadcast_picker( /// Renders the passphrase prompt for an encrypted identity file. pub fn render_passphrase_prompt(frame: &mut Frame, prompt: &PassphrasePrompt, theme: &Theme) { + let input = if prompt.unlocking { + String::from(" Unlocking…") + } else { + masked(&prompt.field) + }; + render_secret_prompt( + frame, + theme, + SecretPrompt { + title: format!(" Unlock SSH key — {} ", prompt.host_name), + detail: &prompt.key_path, + label: " Passphrase (kept in memory until OmnySSH exits):", + input, + error: prompt.error.as_deref(), + action: ":unlock ", + }, + ); +} + +/// Renders the prompt for a login password a connection waits on. +pub fn render_password_prompt(frame: &mut Frame, prompt: &PasswordPrompt, theme: &Theme) { + render_secret_prompt( + frame, + theme, + SecretPrompt { + title: format!(" SSH login — {} ", prompt.host_name), + detail: &prompt.login, + label: " Password (kept in memory until OmnySSH exits):", + input: masked(&prompt.field), + error: prompt + .retry + .then_some("Permission denied, please try again."), + action: ":log in ", + }, + ); +} + +fn masked(field: &FormField) -> String { + format!(" {}| ", "*".repeat(field.value.chars().count())) +} + +/// What a prompt for a secret shows; the secret itself only as stars. +struct SecretPrompt<'a> { + title: String, + detail: &'a str, + label: &'a str, + input: String, + error: Option<&'a str>, + /// The Enter hint, e.g. `":unlock "`. + action: &'a str, +} + +fn render_secret_prompt(frame: &mut Frame, theme: &Theme, prompt: SecretPrompt<'_>) { // Six text rows plus the border, whatever the frame height. let screen = frame.area(); let width = (screen.width * 7 / 10).max(60).min(screen.width); @@ -1062,7 +1116,7 @@ pub fn render_passphrase_prompt(frame: &mut Frame, prompt: &PassphrasePrompt, th frame.render_widget(Clear, area); let block = Block::default() - .title(format!(" Unlock SSH key — {} ", prompt.host_name)) + .title(prompt.title) .title_alignment(Alignment::Center) .borders(Borders::ALL) .border_type(BorderType::Rounded) @@ -1078,27 +1132,21 @@ pub fn render_passphrase_prompt(frame: &mut Frame, prompt: &PassphrasePrompt, th frame.render_widget( Paragraph::new(Line::from(Span::styled( - format!(" {}", prompt.key_path), + format!(" {}", prompt.detail), Style::default().fg(theme.text_secondary), ))), rows[0], ); frame.render_widget( Paragraph::new(Line::from(Span::styled( - " Passphrase (kept in memory until OmnySSH exits):", + prompt.label, Style::default().fg(theme.text_primary), ))), rows[1], ); - - let input = if prompt.unlocking { - String::from(" Unlocking…") - } else { - format!(" {}| ", "*".repeat(prompt.field.value.chars().count())) - }; frame.render_widget( Paragraph::new(Line::from(Span::styled( - input, + prompt.input, Style::default() .fg(theme.form_focused_fg) .bg(theme.success_border) @@ -1107,7 +1155,7 @@ pub fn render_passphrase_prompt(frame: &mut Frame, prompt: &PassphrasePrompt, th rows[2], ); - if let Some(error) = &prompt.error { + if let Some(error) = prompt.error { frame.render_widget( Paragraph::new(Line::from(Span::styled( format!(" {error}"), @@ -1125,7 +1173,7 @@ pub fn render_passphrase_prompt(frame: &mut Frame, prompt: &PassphrasePrompt, th .fg(theme.text_success) .add_modifier(Modifier::BOLD), ), - Span::styled(":unlock ", Style::default().fg(theme.text_muted)), + Span::styled(prompt.action, Style::default().fg(theme.text_muted)), Span::styled( "Esc", Style::default() @@ -1879,4 +1927,37 @@ mod tests { } assert!(!screen.contains("secret"), "the passphrase is never drawn"); } + + #[test] + fn the_password_prompt_names_the_login_and_hides_the_password() { + let prompt = PasswordPrompt { + request_id: 1, + host_name: String::from("udm"), + login: String::from("root@192.168.1.1"), + retry: true, + field: FormField::with_value("hunter2"), + }; + let backend = ratatui::backend::TestBackend::new(80, 24); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|frame| render_password_prompt(frame, &prompt, &Theme::default())) + .expect("draw"); + let screen: String = terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect(); + for text in [ + "SSH login — udm", + "root@192.168.1.1", + "*******|", + "Permission denied, please try again.", + "Enter:log in", + ] { + assert!(screen.contains(text), "{text:?} is not on screen"); + } + assert!(!screen.contains("hunter2"), "the password is never drawn"); + } } From a8984700fd5c587a2f16488df0b9e9a2dd0e9891 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 12:52:41 +0400 Subject: [PATCH 07/15] feat(gui): ask for login passwords in a dialog --- crates/omnyssh-gui/src/bridge.rs | 17 ++ crates/omnyssh-gui/src/commands/auth.rs | 14 +- crates/omnyssh-gui/src/commands/sftp.rs | 6 +- crates/omnyssh-gui/src/events.rs | 20 +++ crates/omnyssh-gui/src/main.rs | 5 +- crates/omnyssh-gui/ui/e2e/password.spec.ts | 157 ++++++++++++++++++ crates/omnyssh-gui/ui/src/lib/bindings.ts | 28 ++++ .../ui/src/lib/components/AppShell.svelte | 2 + crates/omnyssh-gui/ui/src/lib/ipc/commands.ts | 6 + crates/omnyssh-gui/ui/src/lib/ipc/router.ts | 12 ++ .../ui/src/lib/ipc/subscribe.test.ts | 14 ++ .../omnyssh-gui/ui/src/lib/ipc/subscribe.ts | 8 +- .../ui/src/lib/screens/PasswordPrompt.svelte | 103 ++++++++++++ .../ui/src/lib/stores/password.test.ts | 36 ++++ .../omnyssh-gui/ui/src/lib/stores/password.ts | 20 +++ 15 files changed, 444 insertions(+), 4 deletions(-) create mode 100644 crates/omnyssh-gui/ui/e2e/password.spec.ts create mode 100644 crates/omnyssh-gui/ui/src/lib/screens/PasswordPrompt.svelte create mode 100644 crates/omnyssh-gui/ui/src/lib/stores/password.test.ts create mode 100644 crates/omnyssh-gui/ui/src/lib/stores/password.ts diff --git a/crates/omnyssh-gui/src/bridge.rs b/crates/omnyssh-gui/src/bridge.rs index 8aaef39..0de7892 100644 --- a/crates/omnyssh-gui/src/bridge.rs +++ b/crates/omnyssh-gui/src/bridge.rs @@ -64,6 +64,23 @@ pub async fn forward_core_events(app: AppHandle, mut rx: mpsc::Receiver { + let _ = events::PasswordRequired { + request_id, + host_name, + login, + retry, + } + .emit(&app); + } + CoreEvent::PasswordPromptClosed(request_id) => { + let _ = events::PasswordPromptClosed { request_id }.emit(&app); + } // Remote shell exit / dropped connection. Map the inner PTY id to its // public id (dropping routing state); `None` means the user already // closed the tab, so nothing is emitted (§3.4). diff --git a/crates/omnyssh-gui/src/commands/auth.rs b/crates/omnyssh-gui/src/commands/auth.rs index 64ffbdd..b61cc5d 100644 --- a/crates/omnyssh-gui/src/commands/auth.rs +++ b/crates/omnyssh-gui/src/commands/auth.rs @@ -1,4 +1,5 @@ -//! Unlock passphrase-protected identity files (in-memory cache only). +//! Unlock passphrase-protected identity files and answer login-password +//! prompts (in-memory only). use crate::error::CommandError; @@ -19,3 +20,14 @@ pub async fn unlock_identity(key_path: String, passphrase: String) -> Result<(), message: e.to_string(), }) } + +/// Answer the `password-required` prompt `request_id`: a password to try, or +/// `null` to cancel that login. The connection checks it with the server and +/// asks again if it is refused. +#[tauri::command] +#[specta::specta] +pub fn answer_password(request_id: u64, password: Option) -> Result<(), CommandError> { + omnyssh_core::ssh::password::answer(request_id, password).map_err(|e| CommandError { + message: e.to_string(), + }) +} diff --git a/crates/omnyssh-gui/src/commands/sftp.rs b/crates/omnyssh-gui/src/commands/sftp.rs index a1e856a..3668050 100644 --- a/crates/omnyssh-gui/src/commands/sftp.rs +++ b/crates/omnyssh-gui/src/commands/sftp.rs @@ -9,6 +9,7 @@ use tokio::sync::mpsc; use omnyssh_core::event::CoreEvent; use omnyssh_core::ssh::identity; +use omnyssh_core::ssh::password::Prompter; use omnyssh_core::ssh::sftp::{ list_local_dir as core_list_local_dir, preview_local_file as core_preview_local_file, SftpCommand, SftpManager, @@ -39,7 +40,10 @@ pub async fn sftp_open( // A dedicated channel per tab: its owner is the session id, so the forwarder can // attribute the core's session-less `sftp-*` events to this tab (§3.4). let (tx, rx) = mpsc::channel::(SFTP_EVENT_BUFFER); - let manager = match SftpManager::connect(&host, tx).await { + // The prompt goes out on the engine channel: this tab's own channel only + // carries `sftp-*` events. + let prompter = Prompter::new(state.engine_sender(), &host_name); + let manager = match SftpManager::connect_asking(&host, tx, prompter).await { Ok(manager) => manager, Err(e) => { if let Some(path) = omnyssh_core::ssh::session::passphrase_required(&e) { diff --git a/crates/omnyssh-gui/src/events.rs b/crates/omnyssh-gui/src/events.rs index 8388a36..1c88df7 100644 --- a/crates/omnyssh-gui/src/events.rs +++ b/crates/omnyssh-gui/src/events.rs @@ -186,6 +186,26 @@ pub struct KeyPassphraseRequired { pub key_path: String, } +/// A connection waits for the login password of `login` (`user@host`). Answered +/// with `answer_password`; the password only ever crosses inbound. `retry` says +/// the previous one was refused. +#[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] +#[serde(rename_all = "camelCase")] +pub struct PasswordRequired { + pub request_id: u64, + pub host_name: String, + pub login: String, + pub retry: bool, +} + +/// The connection behind a `password-required` stopped waiting (a tunnel was +/// stopped); its prompt goes away. +#[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] +#[serde(rename_all = "camelCase")] +pub struct PasswordPromptClosed { + pub request_id: u64, +} + /// A background error surfaced to the user. #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] pub struct Error { diff --git a/crates/omnyssh-gui/src/main.rs b/crates/omnyssh-gui/src/main.rs index eecea95..d264788 100644 --- a/crates/omnyssh-gui/src/main.rs +++ b/crates/omnyssh-gui/src/main.rs @@ -13,7 +13,7 @@ mod error; mod events; mod state; -use commands::auth::unlock_identity; +use commands::auth::{answer_password, unlock_identity}; use commands::hosts::{delete_host, list_hosts, refresh_metrics, reload_hosts, save_host}; use commands::keysetup::start_key_setup; use commands::sftp::{ @@ -115,6 +115,7 @@ fn specta_builder() -> Builder { tunnel_stop, refresh_metrics, unlock_identity, + answer_password, check_update, install_update, load_update_config, @@ -141,6 +142,8 @@ fn specta_builder() -> Builder { events::KeySetupRollback, events::UpdateAvailable, events::KeyPassphraseRequired, + events::PasswordRequired, + events::PasswordPromptClosed, events::Error ]) } diff --git a/crates/omnyssh-gui/ui/e2e/password.spec.ts b/crates/omnyssh-gui/ui/e2e/password.spec.ts new file mode 100644 index 0000000..c3ffc6f --- /dev/null +++ b/crates/omnyssh-gui/ui/e2e/password.spec.ts @@ -0,0 +1,157 @@ +import { expect, test, type Page } from '@playwright/test'; + +// Login passwords (tech-gui.md §4.2/§4.3). e2e runs against the static SPA with +// Tauri absent, so `__TAURI_INTERNALS__` is stubbed at the boundary (§6.4). The +// stub plays the core: opening files on a host without a usable key sends +// `password-required`, `answer_password` hands the answer back, a wrong password +// comes back as a new request marked `retry`, and every answer is recorded. +const HOSTS = [ + { name: 'nas', hostname: 'nas.example.com', user: 'admin', port: 22, tags: [], source: 'sshConfig', hasKey: false, localForwards: [], tunnelAutostart: false } +]; + +type Answer = { requestId: number; password: string | null }; + +async function boot(page: Page): Promise { + await page.addInitScript( + ({ hosts }) => { + let cbid = 0; + let request = 0; + const win = window as unknown as Record; + const listeners: Record = {}; + const answers: Array<{ requestId: number; password: string | null }> = []; + win.__answers = answers; + + function fire(event: string, payload: unknown): void { + for (const id of listeners[event] ?? []) { + const cb = win[`__cb${id}`] as ((e: unknown) => void) | undefined; + cb?.({ event, id, payload }); + } + } + win.__fire = fire; + + function ask(retry: boolean): number { + request += 1; + const requestId = request; + setTimeout( + () => fire('password-required', { requestId, hostName: 'nas', login: 'admin@nas.example.com', retry }), + 0 + ); + return requestId; + } + + let waiting: ((ok: boolean) => void) | null = null; + + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { + invoke: (cmd: string, args: Record) => { + switch (cmd) { + case 'list_hosts': + return Promise.resolve(hosts); + case 'sftp_open': + // Resolves only once the login is settled, as the core's connect does. + ask(false); + return new Promise((resolve, reject) => { + waiting = (ok) => (ok ? resolve(11) : reject({ message: 'SFTP SSH connect: SSH login cancelled for nas' })); + }); + case 'answer_password': { + const { requestId, password } = args as { requestId: number; password: string | null }; + answers.push({ requestId, password }); + if (password === null) waiting?.(false); + else if (password === 'sesame') waiting?.(true); + else ask(true); + return Promise.resolve(null); + } + case 'plugin:event|listen': { + const { event, handler } = args as { event: string; handler: number }; + (listeners[event] ||= []).push(handler); + return Promise.resolve(cbid); + } + default: + return Promise.resolve(null); + } + }, + transformCallback: (cb: unknown) => { + const id = ++cbid; + win[`__cb${id}`] = cb; + return id; + }, + unregisterCallback: (id: number) => { + delete win[`__cb${id}`]; + } + }; + }, + { hosts: HOSTS } + ); + await page.goto('/'); + await expect(page.getByText('1 host')).toBeVisible(); +} + +const answers = (page: Page): Promise => + page.evaluate(() => (window as unknown as { __answers: Answer[] }).__answers); + +test('a login without a usable key asks for the password until the server takes one', async ({ page }) => { + await boot(page); + await page.getByTitle('files on nas').click(); + + const dialog = page.getByRole('dialog', { name: 'SSH login' }); + await expect(dialog.getByText('SSH login — nas')).toBeVisible(); + await expect(dialog.getByText('admin@nas.example.com')).toBeVisible(); + const input = dialog.getByLabel('Password'); + await expect(input).toBeFocused(); + await expect(dialog.getByRole('button', { name: 'Log in' })).toBeDisabled(); + await expect(dialog.getByText('Permission denied')).toHaveCount(0); + + // A refused password comes back as a fresh prompt that says so. + await input.fill('wrong'); + await input.press('Enter'); + await expect(dialog.getByText('Permission denied, please try again.')).toBeVisible(); + await expect(input).toHaveValue(''); + await expect(input).toBeFocused(); + + await input.fill('sesame'); + await input.press('Enter'); + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await answers(page)).toEqual([ + { requestId: 1, password: 'wrong' }, + { requestId: 2, password: 'sesame' } + ]); +}); + +test('cancelling ends the login with the reason', async ({ page }) => { + await boot(page); + await page.getByTitle('files on nas').click(); + + const dialog = page.getByRole('dialog', { name: 'SSH login' }); + await expect(dialog.getByLabel('Password')).toBeFocused(); + await page.keyboard.press('Escape'); + + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await answers(page)).toEqual([{ requestId: 1, password: null }]); + await expect(page.getByRole('main').getByText('SFTP SSH connect: SSH login cancelled for nas')).toBeVisible(); +}); + +test('a prompt whose connection stopped waiting goes away by itself', async ({ page }) => { + await boot(page); + await page.evaluate(() => { + const fire = (window as unknown as { __fire: (e: string, p: unknown) => void }).__fire; + fire('password-required', { requestId: 40, hostName: 'nas', login: 'admin@nas.example.com', retry: false }); + }); + const dialog = page.getByRole('dialog', { name: 'SSH login' }); + await expect(dialog).toBeVisible(); + + await page.evaluate(() => { + const fire = (window as unknown as { __fire: (e: string, p: unknown) => void }).__fire; + fire('password-prompt-closed', { requestId: 40 }); + }); + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await answers(page)).toEqual([]); +}); + +test('streamer mode keeps the host out of the prompt', async ({ page }) => { + await page.addInitScript(() => localStorage.setItem('omnyssh-streamer-mode', 'true')); + await boot(page); + await page.getByTitle('files on nas').click(); + + const dialog = page.getByRole('dialog', { name: 'SSH login' }); + await expect(dialog.getByLabel('Password')).toBeFocused(); + await expect(dialog.getByText('nas.example.com')).toHaveCount(0); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/bindings.ts b/crates/omnyssh-gui/ui/src/lib/bindings.ts index d8397af..88a280c 100644 --- a/crates/omnyssh-gui/ui/src/lib/bindings.ts +++ b/crates/omnyssh-gui/ui/src/lib/bindings.ts @@ -347,6 +347,19 @@ async unlockIdentity(keyPath: string, passphrase: string) : Promise> { + try { + return { status: "ok", data: await TAURI_INVOKE("answer_password", { requestId, password }) }; +} catch (e) { + if(e instanceof Error) throw e; + else return { status: "error", error: e as any }; +} +}, /** * Query GitHub for a newer release (tech-gui.md §4.2). `None` means up to date — the * core swallows network/parse errors so a failed check never disrupts. @@ -412,6 +425,8 @@ keySetupFailed: KeySetupFailed, keySetupProgress: KeySetupProgress, keySetupRollback: KeySetupRollback, metricsUpdated: MetricsUpdated, +passwordPromptClosed: PasswordPromptClosed, +passwordRequired: PasswordRequired, servicesDetected: ServicesDetected, servicesFailed: ServicesFailed, sftpConnected: SftpConnected, @@ -434,6 +449,8 @@ keySetupFailed: "key-setup-failed", keySetupProgress: "key-setup-progress", keySetupRollback: "key-setup-rollback", metricsUpdated: "metrics-updated", +passwordPromptClosed: "password-prompt-closed", +passwordRequired: "password-required", servicesDetected: "services-detected", servicesFailed: "services-failed", sftpConnected: "sftp-connected", @@ -552,6 +569,17 @@ export type MetricsUpdated = { hostName: string; metrics: MetricsDto } * (tech-gui.md §4.1). `tcpPort` means reachability only — no login, no metrics. */ export type MonitorModeDto = "ssh" | "tcpPort" +/** + * The connection behind a `password-required` stopped waiting (a tunnel was + * stopped); its prompt goes away. + */ +export type PasswordPromptClosed = { requestId: number } +/** + * A connection waits for the login password of `login` (`user@host`). Answered + * with `answer_password`; the password only ever crosses inbound. `retry` says + * the previous one was refused. + */ +export type PasswordRequired = { requestId: number; hostName: string; login: string; retry: boolean } /** * A single process in the "top processes" panel (tech-gui.md §4.1). */ diff --git a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte index f89cbe7..486500e 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte @@ -10,6 +10,7 @@ import SupportModal from './SupportModal.svelte'; import KeySetupProgress from '$lib/screens/KeySetupProgress.svelte'; import PassphrasePrompt from '$lib/screens/PassphrasePrompt.svelte'; + import PasswordPrompt from '$lib/screens/PasswordPrompt.svelte'; import UpdateBanner from './UpdateBanner.svelte'; import { support } from '$lib/stores/support'; import { sidebarCollapsed, isCollapseChord } from '$lib/stores/ui'; @@ -45,5 +46,6 @@ {/if} +
diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts b/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts index 7daf597..b5e6eaf 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts @@ -229,3 +229,9 @@ export async function unlockIdentity(keyPath: string, passphrase: string): Promi const res = await commands.unlockIdentity(keyPath, passphrase); if (res.status === 'error') throw new Error(res.error.message); } + +/** Answer a login's password prompt; `null` cancels the login. */ +export async function answerPassword(requestId: number, password: string | null): Promise { + const res = await commands.answerPassword(requestId, password); + if (res.status === 'error') throw new Error(res.error.message); +} diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/router.ts b/crates/omnyssh-gui/ui/src/lib/ipc/router.ts index 4e9936b..7cc6f22 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/router.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/router.ts @@ -12,6 +12,7 @@ import type { KeySetupProgress, KeySetupRollback, MetricsDto, + PasswordRequired, ServiceDto, SftpConnected, SftpDirListed, @@ -39,6 +40,7 @@ import { reduceRollback } from '$lib/stores/keySetup'; import { enqueuePassphrase, passphraseQueue } from '$lib/stores/passphrase'; +import { passwordQueue, settlePassword } from '$lib/stores/password'; import { offerUpdate } from '$lib/stores/update'; import type { UpdateAvailable } from '$lib/bindings'; @@ -174,3 +176,13 @@ export function applyError(message: string): void { export function applyKeyPassphraseRequired(payload: KeyPassphraseRequired): void { passphraseQueue.update((queue) => enqueuePassphrase(queue, payload)); } + +// A login waiting for its password (tech-gui.md §4.3); answered by `answer_password`. +export function applyPasswordRequired(payload: PasswordRequired): void { + passwordQueue.update((queue) => [...queue, payload]); +} + +// The connection stopped waiting, so its dialog goes. +export function applyPasswordPromptClosed(requestId: number): void { + settlePassword(requestId); +} diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts b/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts index 6c43044..6359776 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts @@ -36,6 +36,8 @@ vi.mock('$lib/bindings', () => { keySetupRollback: channel('keySetupRollback'), updateAvailable: channel('updateAvailable'), keyPassphraseRequired: channel('keyPassphraseRequired'), + passwordRequired: channel('passwordRequired'), + passwordPromptClosed: channel('passwordPromptClosed'), error: channel('error') } }; @@ -50,6 +52,7 @@ import { sessions } from '$lib/stores/sessions'; import { sftp } from '$lib/stores/sftp'; import { lastError } from '$lib/stores/notifications'; import { passphrasePrompt, passphraseQueue } from '$lib/stores/passphrase'; +import { passwordPrompt, passwordQueue } from '$lib/stores/password'; import { startEventBridge } from './subscribe'; describe('startEventBridge', () => { @@ -60,6 +63,7 @@ describe('startEventBridge', () => { services.set(new Map()); lastError.set(null); passphraseQueue.set([]); + passwordQueue.set([]); clearRun(); }); @@ -94,6 +98,16 @@ describe('startEventBridge', () => { expect(get(passphrasePrompt)).toEqual({ hostName: 'web-1', keyPath: '/k/id_ed25519' }); }); + it('shows a password prompt and takes it down when its connection stops waiting', async () => { + await startEventBridge(); + const prompt = { requestId: 7, hostName: 'nas', login: 'admin@10.0.0.5', retry: false }; + listeners.passwordRequired({ payload: prompt }); + expect(get(passwordPrompt)).toEqual(prompt); + + listeners.passwordPromptClosed({ payload: { requestId: 7 } }); + expect(get(passwordPrompt)).toBeNull(); + }); + it('terminal-exited closes the tab whose backend id matches', async () => { await startEventBridge(); const tab = sessions.spawn('terminal', 'web-1'); diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.ts b/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.ts index a5d4b0c..864da42 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.ts @@ -25,7 +25,9 @@ import { applyTerminalExited, applyTransferProgress, applyTunnelStatusChanged, - applyKeyPassphraseRequired + applyKeyPassphraseRequired, + applyPasswordRequired, + applyPasswordPromptClosed } from './router'; export async function startEventBridge(): Promise<() => void> { @@ -51,6 +53,10 @@ export async function startEventBridge(): Promise<() => void> { offs.push(await events.keySetupRollback.listen((e) => applyKeySetupRollback(e.payload))); offs.push(await events.updateAvailable.listen((e) => applyUpdateAvailable(e.payload))); offs.push(await events.keyPassphraseRequired.listen((e) => applyKeyPassphraseRequired(e.payload))); + offs.push(await events.passwordRequired.listen((e) => applyPasswordRequired(e.payload))); + offs.push( + await events.passwordPromptClosed.listen((e) => applyPasswordPromptClosed(e.payload.requestId)) + ); offs.push(await events.error.listen((e) => applyError(e.payload.message))); } catch (err) { offs.forEach((off) => off()); diff --git a/crates/omnyssh-gui/ui/src/lib/screens/PasswordPrompt.svelte b/crates/omnyssh-gui/ui/src/lib/screens/PasswordPrompt.svelte new file mode 100644 index 0000000..cec8e51 --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/screens/PasswordPrompt.svelte @@ -0,0 +1,103 @@ + + +{#if $passwordPrompt} + {@const prompt = $passwordPrompt} + void answer(null)}> +
{ + e.preventDefault(); + submit(); + }} + > +
+
+ +

SSH login — {prompt.hostName}

+
+

+ {displayLogin(prompt.login, $streamerMode)} +

+
+ +

+ Kept in memory until you quit OmnySSH, never written to disk. +

+ {#if prompt.retry} +

Permission denied, please try again.

+ {/if} +
+ + +
+
+ +{/if} diff --git a/crates/omnyssh-gui/ui/src/lib/stores/password.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/password.test.ts new file mode 100644 index 0000000..b32fc6a --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/stores/password.test.ts @@ -0,0 +1,36 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { get } from 'svelte/store'; +import { displayLogin, passwordPrompt, passwordQueue, settlePassword } from './password'; + +const sftp = { requestId: 1, hostName: 'nas', login: 'admin@10.0.0.5', retry: false }; +const tunnel = { requestId: 2, hostName: 'db', login: 'root@10.0.0.6', retry: false }; + +describe('password prompt queue', () => { + beforeEach(() => passwordQueue.set([])); + + it('shows the oldest waiting login, then the next once it is settled', () => { + passwordQueue.set([sftp, tunnel]); + expect(get(passwordPrompt)).toEqual(sftp); + + settlePassword(sftp.requestId); + expect(get(passwordPrompt)).toEqual(tunnel); + + settlePassword(tunnel.requestId); + expect(get(passwordPrompt)).toBeNull(); + }); + + it('settling a request nobody shows changes nothing', () => { + passwordQueue.set([sftp]); + settlePassword(99); + expect(get(passwordQueue)).toEqual([sftp]); + }); +}); + +describe('displayLogin', () => { + it('keeps the user and masks only the host in streamer mode', () => { + expect(displayLogin('root@10.0.0.5', false)).toBe('root@10.0.0.5'); + const masked = displayLogin('root@10.0.0.5', true); + expect(masked.startsWith('root@')).toBe(true); + expect(masked).not.toContain('10.0.0.5'); + }); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/password.ts b/crates/omnyssh-gui/ui/src/lib/stores/password.ts new file mode 100644 index 0000000..d07bfb2 --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/stores/password.ts @@ -0,0 +1,20 @@ +import { derived, writable } from 'svelte/store'; +import type { PasswordRequired } from '$lib/bindings'; +import { displayHostname } from './streamer'; + +// Logins waiting for a password, oldest first. The dialog shows the first; each +// request is its own connection, so none is folded into another. +export const passwordQueue = writable([]); + +export const passwordPrompt = derived(passwordQueue, (queue) => queue[0] ?? null); + +/** Drop the prompt `requestId`: answered, cancelled, or no longer waited on. */ +export function settlePassword(requestId: number): void { + passwordQueue.update((queue) => queue.filter((p) => p.requestId !== requestId)); +} + +/** `user@host` as the dialog shows it: the host masked in streamer mode. */ +export function displayLogin(login: string, streamerOn: boolean): string { + const at = login.lastIndexOf('@'); + return `${login.slice(0, at + 1)}${displayHostname(login.slice(at + 1), streamerOn)}`; +} From 86efd8ee72c08552677db753fe8d9bbcf87a87a8 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 13:22:06 +0400 Subject: [PATCH 08/15] fix(ssh): keep saved passwords out of ProxyJump errors --- crates/omnyssh-core/src/ssh/session.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/omnyssh-core/src/ssh/session.rs b/crates/omnyssh-core/src/ssh/session.rs index e954320..21cb126 100644 --- a/crates/omnyssh-core/src/ssh/session.rs +++ b/crates/omnyssh-core/src/ssh/session.rs @@ -526,7 +526,9 @@ async fn jump_chain(host: &Host) -> anyhow::Result> { let known = tokio::task::spawn_blocking(crate::config::load_all_hosts) .await .context("host list load panicked")? - .map_err(|e| anyhow!("could not load hosts for ProxyJump resolution: {e:#}"))?; + // Only the outer error: a parse error quotes the offending line of + // hosts.toml, which may be a saved password. + .map_err(|e| anyhow!("could not load hosts for ProxyJump resolution: {e}"))?; let chain = crate::ssh::jump::resolve_chain(host, &known)?; tracing::debug!( From af533fef4d64a29803e2f9f113b891fd978d1e05 Mon Sep 17 00:00:00 2001 From: Tim Hartmann Date: Sat, 26 Sep 2026 13:22:06 +0400 Subject: [PATCH 09/15] fix(gui): stack dialogs in the order they open --- crates/omnyssh-gui/ui/src/lib/components/Modal.svelte | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/omnyssh-gui/ui/src/lib/components/Modal.svelte b/crates/omnyssh-gui/ui/src/lib/components/Modal.svelte index 71c8ee3..fbd63d8 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/Modal.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/Modal.svelte @@ -11,14 +11,16 @@ let { label, onClose, + backdropCloses = true, children - }: { label: string; onClose: () => void; children: Snippet } = $props(); + }: { label: string; onClose: () => void; backdropCloses?: boolean; children: Snippet } = $props(); // Dialogs can stack (the passphrase prompt opens on its own over any other): - // Escape closes only the top one. + // Escape closes only the top one, and the one opened last is drawn on top. const id = Symbol('dialog'); dialogs.update((open) => [...open, id]); onDestroy(() => dialogs.update((open) => open.filter((d) => d !== id))); + const layer = $derived(50 + $dialogs.indexOf(id)); function onKeydown(e: KeyboardEvent): void { if (e.key === 'Escape' && get(dialogs).at(-1) === id) { @@ -31,7 +33,8 @@