diff --git a/CHANGELOG.md b/CHANGELOG.md index 47eb33c1..2546426e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,14 @@ Versions follow [Semantic Versioning](https://semver.org/). ## Unreleased +### Features +- **OmnySSH asks for the login password when no key gets in.** A host without a working key and without a saved password — typically one imported from `~/.ssh/config` — failed with "SSH authentication failed" and offered no way in short of saving its password to disk. A terminal now asks the way `ssh` does, right in the tab (`user@host's password:`), and a file session asks in a dialog. Three tries, then the login fails; Ctrl+C or Cancel ends it. The password is kept in memory until you quit, never written to disk, and only once the server has accepted it; the dashboard, and any tunnel on that host, pick it up and connect on their own — they never ask themselves. The first time OmnySSH meets a server, the prompt shows the host key it just recorded, so you can check it before typing. A saved password is still tried first, and a key whose passphrase is needed still comes before any password when the host names it as its identity file. + ### Bug Fixes +- **Devices that only take the password by keyboard-interactive log in.** UniFi consoles such as the Dream Machine Pro, and other servers with `PasswordAuthentication no`, accept a password only through keyboard-interactive — which is what `ssh` and PuTTY fall back to without telling you. OmnySSH sent the saved password by the password method alone, so these hosts showed as offline with "SSH authentication failed". It now offers the password the other way too, and remembers which one a server takes, so a wrong password costs one failed login, not two. A server that asks for a one-time code instead of a password is told so rather than sent the password. +- **A silent or refusing SSH agent no longer leaves every host stuck on "connecting".** Every login asks the agent first, and nothing bounded that: an agent that accepts connections but never answers — as the launchd agent does on some macOS Tahoe setups — or one that turns a signature down (a declined 1Password or Secretive approval, a key added with `ssh-add -c`) held the login forever, password hosts included. The agent now gets five seconds to list its keys and a minute to sign, a refused signature moves on to the next method, and a signature you turned down is not asked for again by background reconnects. +- **The dashboard card says why a host is down.** A failed host showed a grey "offline" with the reason nowhere on screen. The card now shows it — "SSH connection failed: Connection refused", an authentication failure, a timeout — hidden in streamer mode like the tunnel's reason. The terminal app keeps the whole cause in the host's detail view, and a terminal that fails to open no longer replaces its reason with "SSH session closed.". +- **`omny -v` no longer writes login passwords to its log.** At debug level the SSH library dumps the first login request it sends, and when a password was the first thing tried, that dump carried it. Those dumps are now kept out of the log whatever `RUST_LOG` asks for. - **The Linux AppImage opens on current graphics drivers.** On distributions with a recent Mesa — Arch and CachyOS, Bazzite, Nobara — it aborted at launch with `Could not create default EGL display: EGL_BAD_PARAMETER` and never showed a window, and the software-rendering restart added in 1.1.2 ran into the same abort: the failure happens while the graphics driver is being loaded, before WebKit picks a renderer, so no WebKit setting can reach it. The cause was the bundle itself. It carried the build machine's own Wayland and X client libraries and put them ahead of yours on the library path, so your Mesa was loaded against a libwayland older than the one it is built against and a symbol it needs was missing. Those ten libraries are no longer packed into the AppImage; your system's copies are used, as they already were for libX11. The `.deb`, `.rpm`, macOS and Windows builds were never affected. - **Keys with a passphrase work.** A host whose identity file is encrypted failed with a bare "authentication failed", because the key was always read without a passphrase. Both apps now ask for it, once per key, and keep it in memory until you quit — it is never written to disk. The dashboard and any tunnel waiting on that key reconnect as soon as it is unlocked; a terminal or file session that stopped on it has to be opened again. A wrong passphrase says so, a key changed on disk is asked for again, and hosts behind a `ProxyJump` bastion are asked the same way. Cancelling the prompt keeps it away until you open a terminal or file session that needs the key. The host's Password field stays the login password, not the key passphrase. diff --git a/crates/omnyssh-core/src/event.rs b/crates/omnyssh-core/src/event.rs index 4df00ef7..b193d472 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, and `new_host_key` is the + /// fingerprint of a host key first seen on this connection. + PasswordRequired { + request_id: u64, + host_name: HostId, + login: String, + retry: bool, + new_host_key: Option, + }, // ----------------------------------------------------------------------- // Update checker events diff --git a/crates/omnyssh-core/src/ssh/key_setup.rs b/crates/omnyssh-core/src/ssh/key_setup.rs index 95dda30f..9a24ed33 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; @@ -790,6 +792,11 @@ async fn setup_key_internal( } } +/// Connects with the host's keys and nothing else. +async fn connect_keys_only(host: &Host) -> Result { + SshSession::connect_with(host, Passwords::KeysOnly).await +} + /// Attempts to rollback sshd_config to the most recent OmnySSH backup. async fn emergency_rollback(session: &SshSession) -> Result<()> { warn!("Attempting emergency rollback of sshd_config"); diff --git a/crates/omnyssh-core/src/ssh/mod.rs b/crates/omnyssh-core/src/ssh/mod.rs index f9a39bfe..1a5974f5 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 00000000..070ba7ae --- /dev/null +++ b/crates/omnyssh-core/src/ssh/password.rs @@ -0,0 +1,342 @@ +//! 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 std::time::Duration; + +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, + /// How many times each login's password was remembered. + generations: HashMap, + /// How each login's password is sent, once a server has shown it. + methods: HashMap, + /// Prompts waiting for an answer, by request id. + pending: HashMap>>, + last_request: u64, +} + +/// How a server takes the login password. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Method { + /// The `password` method. + Password, + /// Keyboard-interactive, answering its password prompt. + KeyboardInteractive, +} + +/// How long a prompt waits for the user before the login gives up. Nothing +/// else would end it if the frontend lost the prompt (a reloaded page). +const PROMPT_TIMEOUT: Duration = Duration::from_secs(300); + +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) { + { + let mut state = state(); + state.accepted.insert(key.to_string(), password.to_string()); + *state.generations.entry(key.to_string()).or_default() += 1; + } + accepted_signal().send_replace(()); +} + +fn generation(key: &str) -> u64 { + state().generations.get(key).copied().unwrap_or_default() +} + +/// 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); + } +} + +/// How `key`'s server takes a password, if a login has shown it. +pub(crate) fn method(key: &str) -> Option { + state().methods.get(key).copied() +} + +/// Records how `key`'s server takes a password. +pub(crate) fn learn(key: &str, method: Method) { + state().methods.insert(key.to_string(), method); +} + +/// Resolves once a password is remembered for `key` from now on. One held +/// already did not get the caller in (it was held back from a new host key), +/// so waking on it would only redial straight into the same failure. +pub(crate) async fn remembered(key: &str) { + let mut rx = accepted_signal().subscribe(); + let seen = generation(key); + while generation(key) == seen || 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) +} + +/// What a password prompt shows. +pub(crate) struct Prompt<'a> { + /// `user@host` of the server asking. + pub login: &'a str, + /// The previous password for it was refused. + pub retry: bool, + /// The fingerprint of the server's host key when this connection is the + /// first to see it, so the user can check it before typing. + pub new_host_key: Option<&'a str>, +} + +/// Why a prompt produced no password. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum NoAnswer { + Cancelled, + /// Nobody answered within [`PROMPT_TIMEOUT`]. + TimedOut, +} + +/// Asks the user for a login password while a connection authenticates. +#[async_trait] +pub(crate) trait AskPassword: Send { + async fn ask(&mut self, prompt: Prompt<'_>) -> Result; +} + +/// 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, prompt: Prompt<'_>) -> Result { + 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 _pending = Pending(request_id); + let asked = self + .tx + .send(CoreEvent::PasswordRequired { + request_id, + host_name: self.host_name.clone(), + login: prompt.login.to_string(), + retry: prompt.retry, + new_host_key: prompt.new_host_key.map(str::to_string), + }) + .await; + if asked.is_err() { + return Err(NoAnswer::Cancelled); + } + // Answering an expired prompt later fails with NotRequested, which + // frontends report and close it on. + match tokio::time::timeout(PROMPT_TIMEOUT, answer).await { + Ok(Ok(Some(password))) => Ok(password), + Ok(_) => Err(NoAnswer::Cancelled), + Err(_) => Err(NoAnswer::TimedOut), + } + } +} + +/// A prompt waiting for its answer; unlisted once the login stops waiting, so +/// a late answer is refused. +struct Pending(u64); + +impl Drop for Pending { + fn drop(&mut self) { + state().pending.remove(&self.0); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn prompt(retry: bool) -> Prompt<'static> { + Prompt { + login: "root@10.0.0.1", + retry, + new_host_key: None, + } + } + + 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(prompt(false)).await }); + + let id = asked(&mut rx).await; + answer(id, Some(String::from("secret"))).expect("answer"); + assert_eq!(asking.await.expect("ran").as_deref(), Ok("secret")); + 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(prompt(true)).await }); + + let id = asked(&mut rx).await; + answer(id, None).expect("cancel"); + assert_eq!(asking.await.expect("ran"), Err(NoAnswer::Cancelled)); + } + + #[tokio::test] + async fn a_login_that_stops_waiting_refuses_a_late_answer() { + let (tx, mut rx) = mpsc::channel(8); + let mut prompter = Prompter::new(tx, "web-1"); + let asking = tokio::spawn(async move { prompter.ask(prompt(false)).await }); + + let id = asked(&mut rx).await; + asking.abort(); + let _ = asking.await; + assert!(matches!( + answer(id, Some(String::from("late"))), + Err(PasswordError::NotRequested) + )); + } + + #[tokio::test(start_paused = true)] + async fn an_unanswered_prompt_gives_up() { + let (tx, mut rx) = mpsc::channel(8); + let mut prompter = Prompter::new(tx, "web-1"); + let asking = tokio::spawn(async move { prompter.ask(prompt(false)).await }); + + let id = asked(&mut rx).await; + tokio::time::advance(PROMPT_TIMEOUT + Duration::from_secs(1)).await; + assert_eq!(asking.await.expect("ran"), Err(NoAnswer::TimedOut)); + assert!(matches!(answer(id, None), Err(PasswordError::NotRequested))); + } + + #[tokio::test] + async fn a_password_already_held_does_not_wake_a_waiter() { + let key = "held@10.6.6.6:22"; + remember(key, "secret"); + let waiter = tokio::spawn(async move { remembered(key).await }); + tokio::time::sleep(Duration::from_millis(50)).await; + assert!( + !waiter.is_finished(), + "only a newly typed password wakes it" + ); + remember(key, "newer"); + tokio::time::timeout(Duration::from_secs(5), waiter) + .await + .expect("the waiter wakes") + .expect("the waiter ran"); + } + + #[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 the_method_a_login_takes_is_remembered() { + let key = "method@10.7.7.7:22"; + assert_eq!(method(key), None); + learn(key, Method::KeyboardInteractive); + assert_eq!(method(key), Some(Method::KeyboardInteractive)); + } + + #[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 e52160e8..0a50b526 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, waiting_login, SshSession}; // --------------------------------------------------------------------------- // Backoff schedule @@ -240,19 +241,34 @@ 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 { + let Some(login) = waiting_login(&e).map(str::to_owned) 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. + // A poller never asks for a password, only for a passphrase. + // Unlocking the key, or typing the password elsewhere (a + // terminal), is what changes the outcome, so either ends the wait. + let locked = passphrase_required(&e).map(str::to_owned); + if let Some(path) = &locked { + identity::ask_passphrase_once(&tx, &host.name, path).await; + } tokio::select! { () = wait_backoff(delay, &mut refresh_rx) => {} - () = identity::unlocked(&path) => {} + () = password::remembered(&login) => {} + () = async { + match &locked { + Some(path) => identity::unlocked(path).await, + None => std::future::pending().await, + } + } => {} } continue; } diff --git a/crates/omnyssh-core/src/ssh/pty.rs b/crates/omnyssh-core/src/ssh/pty.rs index ad3964ea..2b0a6b85 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, NoAnswer, Prompt}; +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,21 +185,49 @@ 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)) + // 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), + asked: false, + closed: false, + }; + let connected = connect_and_auth(&host, Passwords::Ask(&mut prompt)).await; + // Keys typed past a password prompt must not reach the new shell (a + // password entered twice would be echoed there). Without a prompt they are + // the user's first command, and stay queued. + if prompt.asked { + prompt.flush(); + } + let ((cols, rows), closed) = (prompt.size, prompt.closed); + if closed && connected.is_ok() { + // Closed at the prompt: no shell for a tab that is gone. + let _ = tx.send(CoreEvent::PtyExited(id)).await; + return; } - .await; + 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; + // The tab goes first: the TUI reports a closed tab in the status bar, + // and the reason has to be the message that stays there. + let _ = tx.send(CoreEvent::PtyExited(id)).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; } - let _ = tx.send(CoreEvent::PtyExited(id)).await; return; } }; @@ -239,6 +269,145 @@ 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), + /// A password prompt was shown. + asked: bool, + /// The tab was closed at the prompt. + closed: bool, +} + +impl InlinePrompt<'_> { + /// Drops keystrokes typed ahead, as ssh(1) does before and after a password + /// prompt, keeping a resize and a close. + fn flush(&mut self) { + while let Ok(ctrl) = self.ctrl_rx.try_recv() { + match ctrl { + Ctrl::Input(_) => {} + Ctrl::Resize { cols, rows } => self.size = (cols, rows), + Ctrl::Close => self.closed = true, + } + } + } + + 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, prompt: Prompt<'_>) -> Result { + self.asked = true; + self.flush(); + if self.closed { + return Err(NoAnswer::Cancelled); + } + if prompt.retry { + self.print("Permission denied, please try again.\r\n").await; + } else if let Some(key) = prompt.new_host_key { + // A server first met on this connection: show its key before the + // password goes to it. + self.print(&format!("New host key recorded: {key}\r\n")) + .await; + } + self.print(&format!("{}'s password: ", prompt.login)).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 Err(NoAnswer::Cancelled); + } + } + }; + self.print("\r\n").await; + typed.ok_or(NoAnswer::Cancelled) + } +} + +/// 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 if matches!(byte, b'[' | b'O') => { + self.escape = Escape::Sequence; + continue; + } + // A lone Esc: drop it, keep what follows. + Escape::Started => self.escape = Escape::None, + // 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 +548,70 @@ 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 a_lone_escape_keeps_the_next_key() { + assert_eq!( + typed(&[b"\x1bsecret\r"]), + 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 38d10c5f..8e5e4eb8 100644 --- a/crates/omnyssh-core/src/ssh/session.rs +++ b/crates/omnyssh-core/src/ssh/session.rs @@ -2,7 +2,9 @@ //! //! Provides [`SshSession`] — a thin wrapper around a russh client handle that //! supports connecting, executing commands, and graceful disconnect. -//! Authentication order: SSH agent → identity file → default keys → password. +//! Authentication order: SSH agent → identity file → default keys → password +//! (by the password method or keyboard-interactive) → asking the user, when the +//! caller lets it. //! //! Hosts with a `ProxyJump` are reached through their bastions: each hop is //! connected and authenticated in turn, and the next hop rides a @@ -12,19 +14,26 @@ //! - Connect timeout: 10 seconds (per hop) //! - Command timeout: 30 seconds +#[cfg(unix)] +use std::collections::HashSet; use std::fmt; +use std::future::Future; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; +#[cfg(unix)] +use std::sync::{MutexGuard, OnceLock, PoisonError}; use std::time::Duration; use anyhow::{anyhow, Context}; use async_trait::async_trait; use russh::client::{self, Handle}; use russh::ChannelMsg; +use tokio::sync::watch; use tokio::time; use crate::ssh::client::Host; use crate::ssh::identity::{self, IdentityError}; +use crate::ssh::password::{self, AskPassword, Method, NoAnswer, Prompt}; // --------------------------------------------------------------------------- // russh Handler implementation @@ -45,6 +54,30 @@ pub(crate) struct KnownHostsHandler { /// OpenSSH does after too many failed logins. A link that just dies leaves /// it unset. hung_up: Arc, + /// Set when the session ends because the server offers no method russh knows. + no_method: Arc, + /// The fingerprint of a host key first seen, and recorded, on this connection. + new_key: Arc>>, + /// Dropped with the handler when the session ends, which wakes [`Link::ended`]. + _ended: watch::Sender<()>, +} + +/// What a connection's [`KnownHostsHandler`] reports while it runs. +struct Link { + hung_up: Arc, + no_method: Arc, + new_key: Arc>>, + /// Changes (to closed) once the session is over. + ended: watch::Receiver<()>, +} + +impl Link { + fn new_key(&self) -> Option { + self.new_key + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() + } } #[async_trait] @@ -84,6 +117,11 @@ impl client::Handler for KnownHostsHandler { "could not record host key in known_hosts" ), } + *self + .new_key + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = + Some(format!("SHA256:{}", server_public_key.fingerprint())); Ok(true) } // A previously recorded key changed — refuse; possible MITM. @@ -117,7 +155,12 @@ impl client::Handler for KnownHostsHandler { self.hung_up.store(true, Ordering::SeqCst); Ok(()) } - client::DisconnectReason::Error(e) => Err(e), + client::DisconnectReason::Error(e) => { + if matches!(e, russh::Error::NoAuthMethod) { + self.no_method.store(true, Ordering::SeqCst); + } + Err(e) + } } } } @@ -156,9 +199,19 @@ pub(crate) fn is_refused(e: &anyhow::Error) -> bool { /// chain. fn at_hop(e: anyhow::Error, context: String) -> anyhow::Error { let message = format!("{context}: {e:#}"); - if let Some(path) = passphrase_required(&e) { + if let Some(locked) = e + .chain() + .find_map(|c| c.downcast_ref::()) + { PassphraseRequired { - path: path.to_owned(), + path: locked.path.clone(), + login: locked.login.clone(), + message, + } + .into() + } else if let Some(login) = no_password(&e) { + NoPassword { + login: login.to_owned(), message, } .into() @@ -180,15 +233,21 @@ fn at_hop(e: anyhow::Error, context: String) -> anyhow::Error { pub(crate) struct PassphraseRequired { /// Canonical path of the encrypted key. path: String, + /// The login key; a password typed for it helps as much as the passphrase. + login: String, message: String, } impl PassphraseRequired { - fn new(path: String) -> Self { + fn new(path: String, login: &str) -> Self { // The path goes last: frontends cut messages at the first ':', and a // Windows path has one. let message = format!("SSH key requires a passphrase: {path}"); - Self { path, message } + Self { + path, + login: login.to_string(), + message, + } } } @@ -207,6 +266,89 @@ pub fn passphrase_required(e: &anyhow::Error) -> Option<&str> { .map(|locked| locked.path.as_str()) } +// --------------------------------------------------------------------------- +// NoPassword +// --------------------------------------------------------------------------- + +/// 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 NoPassword { + /// The login key the password is remembered under. + login: String, + message: String, +} + +impl NoPassword { + fn new(login: &str, host_name: &str) -> Self { + // No ':' — frontends cut messages there, and the hint must survive. + let message = format!( + "SSH authentication failed for {host_name} (no key was accepted and no password is saved; open a terminal to enter it)" + ); + Self { + login: login.to_string(), + message, + } + } +} + +impl fmt::Display for NoPassword { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for NoPassword {} + +/// The login a failed connection needs a password for, if that is why it failed. +pub(crate) fn no_password(e: &anyhow::Error) -> Option<&str> { + e.chain() + .find_map(|cause| cause.downcast_ref::()) + .map(|missing| missing.login.as_str()) +} + +/// The login key a connection that stopped for a missing credential waits on: +/// a password typed for it elsewhere lets it go again. +pub(crate) fn waiting_login(e: &anyhow::Error) -> Option<&str> { + e.chain().find_map(|cause| { + cause + .downcast_ref::() + .map(|locked| locked.login.as_str()) + .or_else(|| cause.downcast_ref::().map(|m| m.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, + /// Keys only. Key setup checks a new key this way; any password would let + /// a broken key pass. + KeysOnly, + /// 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 // --------------------------------------------------------------------------- @@ -256,7 +398,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. @@ -269,8 +412,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?), }) } @@ -368,27 +519,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 { @@ -398,8 +554,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, so a slow but working agent is not taken + // for a dead host. + (CONNECT_TIMEOUT + AGENT_BUDGET) * (hops as u32 + 1) } /// The shared russh client configuration (timeouts + keepalives). @@ -443,7 +601,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!( @@ -458,32 +618,84 @@ 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, key, dial, passwords).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, + key: &str, + passwords: &mut Passwords<'_>, +) -> anyhow::Result> { + let dial = || dial_tunnelled(config, via, host); + finish_auth(dial().await?, host, key, dial, passwords).await +} + +/// A connection that has shaken hands and passed the host-key check, not yet +/// authenticated. +struct Dialed { + handle: Handle, + link: Link, + /// The session has wound down far enough for `link` to say why it ended. + settled: bool, + /// A login request here got no answer in time; nothing more is sent on it. + broken: bool, +} + +impl Dialed { + /// Whether a login request can still go out on this connection. + async fn usable(&mut self) -> bool { + !self.broken && !self.closed().await + } + + /// Whether the connection is gone. The first time, it waits (briefly) for + /// the session to finish, so the handler has recorded how it ended. + async fn closed(&mut self) -> bool { + if !self.handle.is_closed() { + return false; + } + if !self.settled { + self.settled = true; + let _ = time::timeout(Duration::from_secs(1), &mut self.handle).await; + } + true + } +} + +/// 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 (handler, link) = known_hosts_handler(host); let handle = time::timeout( CONNECT_TIMEOUT, - client::connect( - Arc::clone(config), - addr, - known_hosts_handler(host, &hung_up), - ), + client::connect(Arc::clone(config), addr, handler), ) .await .map_err(|_| anyhow!("SSH connection timed out (10 s)"))? .context("SSH connection failed")?; - - finish_auth(handle, host, &hung_up).await + Ok(Dialed { + handle, + link, + settled: false, + broken: false, + }) } -/// 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. // @@ -498,92 +710,404 @@ async fn connect_tunnelled( .map_err(|_| anyhow!("SSH connection timed out (10 s)"))? .with_context(|| format!("open tunnel to {}:{}", host.hostname, host.port))?; - let hung_up = Arc::new(AtomicBool::new(false)); + let (handler, link) = known_hosts_handler(host); let handle = time::timeout( CONNECT_TIMEOUT, - client::connect_stream( - Arc::clone(config), - channel.into_stream(), - known_hosts_handler(host, &hung_up), - ), + client::connect_stream(Arc::clone(config), channel.into_stream(), handler), ) .await .map_err(|_| anyhow!("SSH connection timed out (10 s)"))? .context("SSH connection failed")?; - - finish_auth(handle, host, &hung_up).await + Ok(Dialed { + handle, + link, + settled: false, + broken: false, + }) } -/// The host-key verifier for `host`. The lookup uses the target's own -/// hostname/port even over a tunnel, so `known_hosts` entries match what an -/// `ssh -J` would record. -fn known_hosts_handler(host: &Host, hung_up: &Arc) -> KnownHostsHandler { - KnownHostsHandler { +/// The host-key verifier for `host`, and what it will report about the +/// connection. The lookup uses the target's own hostname/port even over a +/// tunnel, so `known_hosts` entries match what an `ssh -J` would record. +fn known_hosts_handler(host: &Host) -> (KnownHostsHandler, Link) { + let (ended_tx, ended) = watch::channel(()); + let link = Link { + hung_up: Arc::new(AtomicBool::new(false)), + no_method: Arc::new(AtomicBool::new(false)), + new_key: Arc::new(Mutex::new(None)), + ended, + }; + let handler = KnownHostsHandler { host: host.hostname.clone(), port: host.port, - hung_up: Arc::clone(hung_up), - } + hung_up: Arc::clone(&link.hung_up), + no_method: Arc::clone(&link.no_method), + new_key: Arc::clone(&link.new_key), + _ended: ended_tx, + }; + (handler, link) } -/// Authenticates `handle` as `host`, converting a refusal into an error. -async fn finish_auth( - mut handle: Handle, +/// 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 keyboard-interactive. +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()) + key: &str, + dial: F, + passwords: &mut Passwords<'_>, +) -> anyhow::Result> +where + F: Fn() -> Fut, + Fut: Future>, +{ + let Dialed { handle, link, .. } = first; + let asking = matches!(passwords, Passwords::Ask(_)); + let (handle, keys) = authenticate(handle, host, asking).await?; + let encrypted_key = match keys { + KeyAuth::Accepted => return Ok(handle), + KeyAuth::Rejected { encrypted_key } => encrypted_key, + }; + let mut first = Dialed { + handle, + link, + settled: false, + broken: false, + }; + if first.closed().await && first.link.no_method.load(Ordering::SeqCst) { + return Err(Refused(format!( + "SSH authentication failed for {}: the server offers no login method OmnySSH supports", + host.name + )) + .into()); + } + + // The server login password, never a key passphrase, and tried last: keys + // are what OmnySSH steers users towards. One typed this session goes first, + // as it is newer than a saved one — but never to a host key first seen on + // this connection: that is not the server it was typed for. + let new_key = first.link.new_key(); + let typed = match passwords { + Passwords::Remembered | Passwords::Ask(_) if new_key.is_none() => password::accepted(key), + _ => None, + }; + let saved = match passwords { + Passwords::KeysOnly => None, + _ => host.password.clone(), + } + .filter(|p| typed.as_ref() != Some(p)); + let known = typed.is_some() || saved.is_some(); + let mut refused = Vec::new(); + let mut spare = None; + for (password, was_typed) in typed + .map(|p| (p, true)) + .into_iter() + .chain(saved.map(|p| (p, false))) + { + match offer(&mut first, &mut spare, &dial, host, key, &password).await { + Offer::Here => return Ok(password_login(host, first.handle)), + Offer::There(handle) => return Ok(password_login(host, handle)), + Offer::Rejected => { + if was_typed { + password::forget(key, &password); + } + refused.push(password); + } + Offer::Unavailable(e) => return Err(e), } - AuthOutcome::Failed => {} } + + // A locked identity file is what the host is set up with: its passphrase + // comes before any password (#97). A locked default key does not hold up a + // password prompt — it may not be a key this server takes. + let locked = match encrypted_key { + Some((path, true)) => return Err(PassphraseRequired::new(path, key).into()), + Some((path, false)) => Some(path), + None => None, + }; + + if let Passwords::Ask(ask) = passwords { + let asked = ask_password( + &mut **ask, + &mut first, + &mut spare, + &dial, + host, + key, + &refused, + new_key.as_deref(), + ) + .await?; + let refusal = match asked { + Asked::In(Offer::There(handle)) => return Ok(password_login(host, handle)), + Asked::In(_) => return Ok(password_login(host, first.handle)), + Asked::Unanswered(NoAnswer::Cancelled) => { + format!("SSH login cancelled for {}", host.name) + } + Asked::Unanswered(NoAnswer::TimedOut) => format!( + "SSH login to {} gave up waiting for the password", + host.name + ), + Asked::Refused => format!("SSH authentication failed for {}", host.name), + }; + // No password got in: the key it skipped still may (#97). + return Err(match locked { + Some(path) => PassphraseRequired::new(path, key).into(), + None => Refused(refusal).into(), + }); + } + + if let Some(path) = locked { + return Err(PassphraseRequired::new(path, key).into()); + } + if !known && !matches!(passwords, Passwords::KeysOnly) { + return Err(NoPassword::new(key, &host.name).into()); + } + let message = format!("SSH authentication failed for {}", host.name); - // `authenticate` 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. - if handle.is_closed() { - let _ = time::timeout(Duration::from_secs(1), &mut handle).await; - if !hung_up.load(Ordering::SeqCst) { - return Err(anyhow!(message)); - } + // 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. + if first.closed().await && !first.link.hung_up.load(Ordering::SeqCst) { + return Err(anyhow!(message)); } Err(Refused(message).into()) } +/// How asking the user went. +enum Asked { + /// A password got in: [`Offer::Here`] or [`Offer::There`]. + In(Offer), + /// Every answer was refused. + Refused, + Unanswered(NoAnswer), +} + +/// Asks the user for the password, up to [`PASSWORD_PROMPTS`] times. +#[allow(clippy::too_many_arguments)] +async fn ask_password( + ask: &mut dyn AskPassword, + first: &mut Dialed, + spare: &mut Option, + dial: &F, + host: &Host, + key: &str, + refused: &[String], + new_key: Option<&str>, +) -> anyhow::Result +where + F: Fn() -> Fut, + Fut: Future>, +{ + let login = format!("{}@{}", host.user, host.hostname); + let mut refused = refused.to_vec(); + let mut retry = false; + for _ in 0..PASSWORD_PROMPTS { + let prompt = Prompt { + login: &login, + retry, + new_host_key: new_key, + }; + let password = match ask.ask(prompt).await { + Ok(password) => password, + Err(no_answer) => return Ok(Asked::Unanswered(no_answer)), + }; + retry = true; + // Sending it again would only cost another failed login. + if refused.contains(&password) { + continue; + } + // Only a hang-up this answer caused ends the asking; one from the key + // attempts before it just moves the answers to fresh connections. + let first_open = first.usable().await; + match offer(first, spare, dial, host, key, &password).await { + Offer::Rejected => refused.push(password), + Offer::Unavailable(e) => return Err(e), + accepted => { + password::remember(key, &password); + return Ok(Asked::In(accepted)); + } + } + // OpenSSH hangs up after too many failed logins; ssh(1) stops there too. + if first_open && first.closed().await && first.link.hung_up.load(Ordering::SeqCst) { + break; + } + } + Ok(Asked::Refused) +} + +fn password_login(host: &Host, handle: Handle) -> Handle { + tracing::info!( + host = %host.name, + "Connected via password authentication — consider setting up SSH key" + ); + handle +} + +/// How an offered password went. +enum Offer { + /// Got in on the first connection. + Here, + /// Got in on a fresh connection. + There(Handle), + /// The server said no. + Rejected, + /// No verdict: the connection could not be made or went away. + Unavailable(anyhow::Error), +} + +/// Offers `password` the way ssh(1) does: by the password method, and 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, kept in +/// `spare` for the next answer. Which of the two a login takes is remembered, +/// so a wrong password costs one failed login, not two. +async fn offer( + first: &mut Dialed, + spare: &mut Option, + dial: &F, + host: &Host, + key: &str, + password: &str, +) -> Offer +where + F: Fn() -> Fut, + Fut: Future>, +{ + let method = password::method(key); + let mut tried_password = false; + if method != Some(Method::KeyboardInteractive) && first.usable().await { + match password_auth(&mut first.handle, &host.user, password).await { + Some(true) => { + password::learn(key, Method::Password); + return Offer::Here; + } + Some(false) => tried_password = true, + // A server that refuses and hangs up (too many failures) gave its + // verdict; any other silence is none, and the connection is done. + None if first.closed().await && first.link.hung_up.load(Ordering::SeqCst) => { + tried_password = true + } + None => first.broken = true, + } + if tried_password && method == Some(Method::Password) { + return Offer::Rejected; + } + } + + let fresh = match spare.take() { + Some(conn) if !conn.handle.is_closed() => conn, + _ => match dial().await { + Ok(conn) => conn, + Err(e) => return Offer::Unavailable(e), + }, + }; + // With known_hosts unwritable every connection meets the key anew: this one + // must meet the same key as the first, whose fingerprint the user saw. + if let (Some(seen), Some(now)) = (first.link.new_key(), fresh.link.new_key()) { + if seen != now { + return Offer::Unavailable( + Refused(format!( + "SSH login to {} stopped: the host key changed between connections", + host.name + )) + .into(), + ); + } + } + let mut fresh = if method == Some(Method::Password) { + fresh + } else { + match kbd_login(fresh, &host.user, password).await { + (Some(conn), Kbd::Accepted) => { + password::learn(key, Method::KeyboardInteractive); + return Offer::There(conn.handle); + } + (conn, Kbd::Refused) => { + password::learn(key, Method::KeyboardInteractive); + *spare = conn; + return Offer::Rejected; + } + // The password method already turned this password down. + (_, Kbd::Broken) if tried_password => return Offer::Rejected, + // A second factor is a dead end whatever the password: say so. + (_, Kbd::WantsCode) => { + return Offer::Unavailable( + Refused(format!( + "SSH login to {} asks for a verification code, which OmnySSH cannot answer", + host.name + )) + .into(), + ) + } + (Some(conn), Kbd::NotOffered) => { + password::learn(key, Method::Password); + conn + } + _ => return Offer::Unavailable(anyhow!("SSH login to {} broke off", host.name)), + } + }; + if tried_password { + return Offer::Rejected; + } + match password_auth(&mut fresh.handle, &host.user, password).await { + Some(true) => { + password::learn(key, Method::Password); + Offer::There(fresh.handle) + } + Some(false) => Offer::Rejected, + None => Offer::Unavailable(anyhow!("SSH login to {} broke off", host.name)), + } +} + // --------------------------------------------------------------------------- // Authentication helpers // --------------------------------------------------------------------------- -enum AuthOutcome { - Ok, - Failed, - PassphraseRequired { path: String }, +/// How long one login request may wait for the server's verdict. +const AUTH_TIMEOUT: Duration = Duration::from_secs(30); + +enum KeyAuth { + Accepted, + /// No key got in. `encrypted_key` is one skipped for want of its + /// passphrase, and whether it is the host's own identity file. + Rejected { + encrypted_key: Option<(String, bool)>, + }, } +/// Tries the agent, the identity file and the default keys, in that order. +/// `user_started`: a login the user is watching also retries agent keys the +/// user turned down before. async fn authenticate( - handle: &mut Handle, + handle: Handle, host: &Host, -) -> anyhow::Result { + user_started: bool, +) -> anyhow::Result<(Handle, KeyAuth)> { let user = host.user.clone(); - let mut encrypted_key: Option = None; + let mut encrypted_key: Option<(String, bool)> = None; // 1. Try SSH agent first — it handles passphrase-protected keys and is the // most common auth method for non-interactive clients. #[cfg(unix)] - { - if try_agent_auth(handle, &user).await.unwrap_or(false) { - return Ok(AuthOutcome::Ok); + let handle = { + let (handle, accepted) = agent_login(handle, &user, user_started).await?; + if accepted { + return Ok((handle, KeyAuth::Accepted)); } - } + handle + }; + #[cfg(not(unix))] + let _ = user_started; + let mut handle = handle; // 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), + match try_key_auth(&mut handle, &user, key_path).await { + Ok(true) => return Ok((handle, KeyAuth::Accepted)), Ok(false) => {} - Err(e) => note_encrypted(&mut encrypted_key, e), + Err(e) => note_encrypted(&mut encrypted_key, e, true)?, } } @@ -593,46 +1117,47 @@ async fn authenticate( for key_path in default_key_paths() { 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), + match try_key_auth(&mut handle, &user, &path_str).await { + Ok(true) => return Ok((handle, KeyAuth::Accepted)), Ok(false) => {} - Err(e) if host.identity_file.is_none() => note_encrypted(&mut encrypted_key, e), - Err(_) => {} + Err(e) if host.identity_file.is_none() => { + note_encrypted(&mut encrypted_key, e, false)? + } + Err(e) => stalled(e)?, } } } - // 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); - } - } + Ok((handle, KeyAuth::Rejected { encrypted_key })) +} + +/// A key the server did not answer for in time: the connection is left in an +/// unknown state, so the login stops here. +#[derive(Debug, thiserror::Error)] +#[error("the SSH server did not answer the login in time")] +struct Stalled; - if let Some(path) = encrypted_key { - return Ok(AuthOutcome::PassphraseRequired { path }); +fn stalled(err: anyhow::Error) -> anyhow::Result<()> { + if err.is::() { + return Err(err); } - Ok(AuthOutcome::Failed) + tracing::debug!(error = %err, "public-key authentication attempt failed"); + Ok(()) } -fn note_encrypted(encrypted_key: &mut Option, err: anyhow::Error) { +fn note_encrypted( + encrypted_key: &mut Option<(String, bool)>, + err: anyhow::Error, + identity_file: bool, +) -> anyhow::Result<()> { match err.downcast_ref::() { - Some(IdentityError::Encrypted(path)) if encrypted_key.is_none() => { - *encrypted_key = Some(path.clone()); - } - _ => { - tracing::debug!(error = %err, "public-key authentication attempt failed"); + Some(IdentityError::Encrypted(path)) => { + if encrypted_key.is_none() { + *encrypted_key = Some((path.clone(), identity_file)); + } + Ok(()) } + _ => stalled(err), } } @@ -661,55 +1186,349 @@ async fn try_key_auth( .await .context("spawn_blocking panicked")??; - let ok = handle - .authenticate_publickey(user, Arc::new(key_pair)) - .await - .context("authenticate_publickey")?; + // russh signs file keys itself, so giving up on the wait leaves it free. + let ok = time::timeout( + AUTH_TIMEOUT, + handle.authenticate_publickey(user, Arc::new(key_pair)), + ) + .await + .map_err(|_| Stalled)? + .context("authenticate_publickey")?; 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, a PIN, Touch ID). +#[cfg(unix)] +const SIGN_TIMEOUT: Duration = Duration::from_secs(60); + +/// Upper bound on the agent's share of one hop's login, for callers that time +/// the whole connect (so a slow but working agent is not taken for a dead host). +const AGENT_BUDGET: Duration = Duration::from_secs(70); + +/// Agent keys whose signature was turned down or timed out, by fingerprint. +/// Background logins skip them, so a confirm dialog the user said no to does +/// not come back every time a poller reconnects. +#[cfg(unix)] +fn refused_agent_keys() -> MutexGuard<'static, HashSet> { + static REFUSED: OnceLock>> = OnceLock::new(); + REFUSED + .get_or_init(Mutex::default) + .lock() + .unwrap_or_else(PoisonError::into_inner) +} + +/// Runs the agent's keys in a task that owns the connection. A caller that gives +/// up mid-signature (a poller restarted, a tunnel stopped) then only detaches: +/// dropping the connection while russh waits for the signature would leave +/// russh spinning. The task itself ends within the signing bound. +#[cfg(unix)] +async fn agent_login( + handle: Handle, + user: &str, + user_started: bool, +) -> anyhow::Result<(Handle, bool)> { + let user = user.to_string(); + tokio::spawn(async move { + let mut handle = handle; + let accepted = try_agent_auth(&mut handle, &user, user_started) + .await + .unwrap_or(false); + (handle, accepted) + }) + .await + .context("SSH agent login failed") +} + #[cfg(unix)] async fn try_agent_auth( handle: &mut Handle, user: &str, + user_started: bool, ) -> 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 = connect_agent().await?; + 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 stalled = Arc::new(AtomicBool::new(false)); + let mut signer = AgentSigner { + agent: Some(agent), + failed: Arc::clone(&failed), + stalled: Arc::clone(&stalled), + }; 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, + if !user_started && refused_agent_keys().contains(&pubkey.fingerprint()) { + 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; the signer went with it. + let _ = time::timeout(Duration::from_millis(100), &mut attempt).await; + if stalled.load(Ordering::SeqCst) { + tracing::debug!("SSH agent stopped answering; trying other methods"); + return Ok(false); + } + // Turned down: the agent is fine, and a later key may sign. + let agent = time::timeout(AGENT_TIMEOUT, connect_agent()).await; + let Ok(Ok(agent)) = agent else { return Ok(false) }; + signer = AgentSigner { + agent: Some(agent), + failed: Arc::clone(&failed), + stalled: Arc::clone(&stalled), + }; + continue; + } + }; + signer = back; + if matches!(result, Ok(true)) { + return Ok(true); } } Ok(false) } -/// Try password-based authentication. +#[cfg(unix)] +async fn connect_agent( +) -> anyhow::Result> { + russh::keys::agent::client::AgentClient::connect_env() + .await + .context("connect to SSH agent") +} + +/// Signs through the SSH agent without ever leaving russh waiting. /// -/// # Errors -/// Returns an error if the authentication attempt fails. -async fn try_password_auth( +/// 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, + /// Set when a signature timed out: the agent is hung, not just unwilling. + stalled: 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. + match time::timeout(SIGN_TIMEOUT, agent.sign_request(&key, to_sign.clone())).await { + Ok((agent, result)) => { + self.agent = Some(agent); + // An agent reply russh cannot read comes back unchanged. + signed = result.ok().filter(|data| data.len() != to_sign.len()); + } + Err(_) => self.stalled.store(true, Ordering::SeqCst), + } + } + match signed { + Some(data) => { + refused_agent_keys().remove(&key.fingerprint()); + (self, Ok(data)) + } + None => { + refused_agent_keys().insert(key.fingerprint()); + self.failed.notify_one(); + (self, Ok(to_sign)) + } + } + }) + } +} + +/// Password-method login: `Some(verdict)`, or `None` when there is none — the +/// connection went away or the server took too long, and it is not used again. +async fn password_auth( handle: &mut Handle, user: &str, password: &str, -) -> anyhow::Result { - let ok = handle - .authenticate_password(user, password) - .await - .context("authenticate with password")?; - Ok(ok) +) -> Option { + // No reply for us to owe here, so giving up on the wait is safe. + match time::timeout(AUTH_TIMEOUT, handle.authenticate_password(user, password)).await { + Ok(Ok(true)) => Some(true), + Ok(Ok(false)) if !handle.is_closed() => Some(false), + _ => None, + } +} + +/// 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(20); + +/// How a keyboard-interactive login went. +enum Kbd { + Accepted, + /// The server was sent the password and did not take it. + Refused, + /// The server does not do keyboard-interactive, or not in a way the + /// password can answer. + NotOffered, + /// The server asked for a one-time code, not a password. + WantsCode, + /// No clean end (timeout, the session died): the connection is not reused. + Broken, +} + +/// Keyboard-interactive login answering the password prompt with `password`, +/// on a connection where it is the first method (russh 0.46). Runs in a task +/// that owns the connection, so a caller that gives up cannot leave russh +/// waiting for our answer. Hands the connection back unless it broke. +async fn kbd_login(conn: Dialed, user: &str, password: &str) -> (Option, Kbd) { + let (user, password) = (user.to_string(), password.to_string()); + tokio::spawn(async move { + let mut conn = conn; + match kbd_exchange(&mut conn, &user, &password).await { + Kbd::Broken => { + release(conn.handle).await; + (None, Kbd::Broken) + } + outcome => (Some(conn), outcome), + } + }) + .await + .unwrap_or((None, Kbd::Broken)) +} + +async fn kbd_exchange(conn: &mut Dialed, user: &str, password: &str) -> Kbd { + use russh::client::KeyboardInteractiveAuthResponse as Reply; + + let Dialed { handle, link, .. } = conn; + let mut password = Some(password); + let mut wants_code = false; + let mut reply = kbd_wait( + &mut link.ended, + handle.authenticate_keyboard_interactive_start(user, None), + ) + .await; + for round in 0.. { + let prompts = match reply { + Some(Ok(Reply::Success)) => return Kbd::Accepted, + Some(Ok(Reply::Failure)) if wants_code => return Kbd::WantsCode, + // Refused only if the password went out; prompts it could not + // answer (a user name, several fields) are as good as no offer. + Some(Ok(Reply::Failure)) if password.is_none() => return Kbd::Refused, + Some(Ok(Reply::Failure)) => return Kbd::NotOffered, + Some(Ok(Reply::InfoRequest { prompts, .. })) if round <= KBD_ROUNDS => prompts, + _ => return Kbd::Broken, + }; + // Always answered: russh waits for the answer and swallows everything + // else meanwhile. Past the round cap, no answers at all, which makes the + // server give up. + let answers = if round < KBD_ROUNDS { + kbd_answers(&prompts, &mut password, &mut wants_code) + } else { + Vec::new() + }; + reply = kbd_wait( + &mut link.ended, + handle.authenticate_keyboard_interactive_respond(answers), + ) + .await; + } + Kbd::Broken +} + +/// A keyboard-interactive step, given up after [`KBD_TIMEOUT`] or as soon as the +/// session ends (russh would otherwise spin on the closed channel until then). +async fn kbd_wait( + ended: &mut watch::Receiver<()>, + step: impl Future>, +) -> Option> { + tokio::select! { + reply = time::timeout(KBD_TIMEOUT, step) => reply.ok(), + _ = ended.changed() => None, + } +} + +/// Drops a connection that may have a server prompt waiting for us. russh waits +/// for that answer forever, spinning once the handle is gone, so a blank one is +/// sent first; a prompt arriving later then finds nobody to hand it to and ends +/// the session. +async fn release(mut handle: Handle) { + let _ = time::timeout( + Duration::ZERO, + handle.authenticate_keyboard_interactive_respond(Vec::new()), + ) + .await; +} + +/// Answers to one round of keyboard-interactive prompts: the password goes, once, +/// to a lone hidden prompt; anything else (a visible question, several fields) +/// gets blanks the server will refuse. A prompt for a one-time code gets a blank +/// too — the password must not end up at a 2FA service — and is noted in +/// `wants_code`. +fn kbd_answers( + prompts: &[russh::client::Prompt], + password: &mut Option<&str>, + wants_code: &mut bool, +) -> Vec { + match prompts { + [only] if !only.echo && asks_for_code(&only.prompt) => { + *wants_code = true; + vec![String::new()] + } + [only] if !only.echo => vec![password.take().unwrap_or_default().to_string()], + _ => vec![String::new(); prompts.len()], + } +} + +/// Whether a prompt asks for a one-time code rather than the password. A deny +/// list, so "Password:" in any language still gets the password. Whole words +/// only, and never from a `user@host` (OpenSSH puts one in front): a user or +/// host name is no hint. +fn asks_for_code(prompt: &str) -> bool { + let prompt = prompt.to_lowercase(); + let words: Vec<&str> = prompt + .split_whitespace() + .filter(|chunk| !chunk.contains('@')) + .flat_map(|chunk| chunk.split(|c: char| !c.is_alphanumeric())) + .filter(|w| !w.is_empty()) + .collect(); + words.windows(2).any(|pair| pair == ["one", "time"]) + || words.iter().any(|word| { + matches!( + *word, + "code" | "token" | "otp" | "passcode" | "2fa" | "yubikey" | "duo" | "pin" + ) || word.starts_with("verif") + }) } // --------------------------------------------------------------------------- @@ -753,9 +1572,13 @@ mod tests { #[test] fn a_locked_key_stays_recognisable_through_a_jump_host() { - let locked = anyhow::Error::from(PassphraseRequired::new(String::from("/k/id"))); + let locked = anyhow::Error::from(PassphraseRequired::new( + String::from("/k/id"), + "root@10.0.0.5:22", + )); let hop = at_hop(locked, String::from("ProxyJump via 'bastion' failed")); assert_eq!(passphrase_required(&hop), Some("/k/id")); + assert_eq!(waiting_login(&hop), Some("root@10.0.0.5:22")); assert!(!is_refused(&hop)); assert_eq!( hop.to_string(), @@ -763,9 +1586,108 @@ mod tests { ); } + #[test] + fn a_missing_password_stays_recognisable_through_a_jump_host() { + let missing = anyhow::Error::from(NoPassword::new("root@10.0.0.5:22", "db")); + let hop = at_hop(missing, String::from("connecting via 'bastion' failed")); + assert_eq!(no_password(&hop), Some("root@10.0.0.5:22")); + assert_eq!(waiting_login(&hop), Some("root@10.0.0.5:22")); + assert!(!is_refused(&hop)); + // Frontends cut at the first ':'; the reason must come before it. + assert!(!NoPassword::new("k", "db").to_string().contains(':')); + } + + #[test] + fn a_password_is_remembered_per_login_and_bastion() { + let host = |name: &str, user: &str, hostname: &str| Host { + name: name.to_string(), + user: user.to_string(), + hostname: hostname.to_string(), + ..Host::default() + }; + let db = host("db", "root", "10.0.0.5"); + let via_a = [host("a", "ops", "a.example.com")]; + let via_b = [host("b", "ops", "b.example.com")]; + assert_ne!(login_key(&db, &via_a), login_key(&db, &via_b)); + assert_ne!(login_key(&db, &[]), login_key(&db, &via_a)); + assert_ne!( + login_key(&db, &[]), + login_key(&host("db", "admin", "10.0.0.5"), &[]) + ); + } + + 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, mut code) = (Some("secret"), false); + let hidden = [prompt("Password: ", false)]; + assert_eq!(kbd_answers(&hidden, &mut password, &mut code), ["secret"]); + // A second ask (a retry) must not get it again. + assert_eq!(kbd_answers(&hidden, &mut password, &mut code), [""]); + assert!(!code); + } + + #[test] + fn other_prompts_are_answered_blank() { + let (mut password, mut code) = (Some("secret"), false); + assert!(kbd_answers(&[], &mut password, &mut code).is_empty()); + assert_eq!( + kbd_answers(&[prompt("Username: ", true)], &mut password, &mut code), + [""] + ); + let two = [prompt("Password: ", false), prompt("Code: ", false)]; + assert_eq!(kbd_answers(&two, &mut password, &mut code), ["", ""]); + assert_eq!( + password, + Some("secret"), + "never spent on a prompt it did not answer" + ); + } + + #[test] + fn a_code_prompt_never_gets_the_password() { + for text in [ + "Verification code: ", + "Enter PASSCODE:", + "One-time password (OATH) for `root':", + "Duo two-factor login", + "PIN: ", + ] { + let (mut password, mut code) = (Some("secret"), false); + assert_eq!( + kbd_answers(&[prompt(text, false)], &mut password, &mut code), + [""], + "{text}" + ); + assert!(code, "{text}"); + } + // Localised password prompts still get it. + for text in [ + "Password: ", + "Passwort: ", + "Contraseña: ", + "(root@udm) Password:", + "(vscode@gitcode) Password:", + "Password for duo-admin@host:", + ] { + let (mut password, mut code) = (Some("secret"), false); + assert_eq!( + kbd_answers(&[prompt(text, false)], &mut password, &mut code), + ["secret"], + "{text}" + ); + } + } + #[test] fn a_locked_key_is_found_under_added_context() { - let e = anyhow::Error::from(PassphraseRequired::new(String::from("/k/id"))) + let e = anyhow::Error::from(PassphraseRequired::new(String::from("/k/id"), "k")) .context("SFTP SSH connect"); assert_eq!(passphrase_required(&e), Some("/k/id")); } diff --git a/crates/omnyssh-core/src/ssh/sftp.rs b/crates/omnyssh-core/src/ssh/sftp.rs index ac375e1e..63e005a6 100644 --- a/crates/omnyssh-core/src/ssh/sftp.rs +++ b/crates/omnyssh-core/src/ssh/sftp.rs @@ -6,13 +6,20 @@ //! All operations are non-blocking from the UI perspective. //! Progress is reported via [`CoreEvent::FileTransferProgress`]. +use std::time::Duration; + use anyhow::Context; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::mpsc; +use tokio::time; 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}; + +/// How long the SFTP channel and subsystem may take once logged in. +const OPEN_TIMEOUT: Duration = Duration::from_secs(30); // --------------------------------------------------------------------------- // FileEntry — represents one file or directory in a panel listing @@ -78,23 +85,34 @@ pub struct SftpManager { impl SftpManager { /// Connects to `host` via SSH + SFTP subsystem and spawns the background task. + /// A login the keys do not get into asks for the password through `prompter`. /// /// On success sends [`CoreEvent::SftpConnected`] through `event_tx`. /// On failure the task sends [`CoreEvent::SftpDisconnected`]. /// /// # Errors - /// Returns an error if the SSH connection fails before the task is spawned. - pub async fn connect(host: &Host, event_tx: mpsc::Sender) -> anyhow::Result { - let session = SshSession::connect(host) + /// Returns an error if the SSH connection fails before the task is spawned, + /// including a cancelled password prompt. + pub async fn connect( + 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")?; - let stream = session - .open_sftp_channel() - .await - .context("open SFTP channel")?; - let sftp = russh_sftp::client::SftpSession::new(stream) - .await - .context("create SFTP session")?; + // The login is bounded step by step; the channel must not hang either. + let sftp = time::timeout(OPEN_TIMEOUT, async { + let stream = session + .open_sftp_channel() + .await + .context("open SFTP channel")?; + russh_sftp::client::SftpSession::new(stream) + .await + .context("create SFTP session") + }) + .await + .map_err(|_| anyhow::anyhow!("SFTP did not start within {}s", OPEN_TIMEOUT.as_secs()))??; let (cmd_tx, cmd_rx) = mpsc::channel::(64); let host_name = host.name.clone(); diff --git a/crates/omnyssh-core/src/ssh/tunnel.rs b/crates/omnyssh-core/src/ssh/tunnel.rs index 8aef399f..9bb999f7 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; use crate::ssh::session::{ - connect_and_auth, connect_budget, is_refused, passphrase_required, SshConnection, + connect_and_auth, connect_budget, is_refused, passphrase_required, waiting_login, Passwords, + SshConnection, }; // --------------------------------------------------------------------------- @@ -190,10 +192,12 @@ const RETRY_DELAYS: [Duration; 5] = [ /// alone would redial every second a server that accepts and then drops us. const STABLE_AFTER: Duration = Duration::from_secs(60); -/// Head room over [`connect_budget`] for authentication, which has no timeout -/// of its own. +/// Head room over [`connect_budget`] for the password steps of a login. const AUTH_BUDGET: Duration = Duration::from_secs(20); +/// How often a tunnel with no key that gets in and no password tries again. +const NO_PASSWORD_RETRY: Duration = Duration::from_secs(60); + /// 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); @@ -356,34 +360,50 @@ async fn serve(host: &Host, tx: &mpsc::Sender) -> TunnelStatus { loop { let budget = connect_budget(host).await + AUTH_BUDGET; let mut locked = None; - let reason = match time::timeout(budget, connect_and_auth(host)).await { - Ok(Ok(conn)) => { - send_status(tx, &host.name, TunnelStatus::Up).await; - let since = Instant::now(); - let reason = forward(Arc::new(conn), &listeners, tx, &host.name).await; - if since.elapsed() >= STABLE_AFTER { - retry = 0; + let mut waiting = None; + let reason = + match time::timeout(budget, connect_and_auth(host, Passwords::Remembered)).await { + Ok(Ok(conn)) => { + send_status(tx, &host.name, TunnelStatus::Up).await; + let since = Instant::now(); + let reason = forward(Arc::new(conn), &listeners, tx, &host.name).await; + if since.elapsed() >= STABLE_AFTER { + retry = 0; + } + reason } - reason - } - Ok(Err(e)) if is_refused(&e) => return TunnelStatus::Failed(format!("{e:#}")), - Ok(Err(e)) => { - locked = passphrase_required(&e).map(str::to_owned); - format!("{e:#}") - } - Err(_) => format!( - "no answer from {} within {}s", - host.hostname, - budget.as_secs() - ), - }; + Ok(Err(e)) if is_refused(&e) => return TunnelStatus::Failed(format!("{e:#}")), + Ok(Err(e)) => { + locked = passphrase_required(&e).map(str::to_owned); + waiting = waiting_login(&e).map(str::to_owned); + format!("{e:#}") + } + Err(_) => format!( + "no answer from {} within {}s", + host.hostname, + budget.as_secs() + ), + }; tracing::debug!(host = %host.name, %reason, "tunnel down"); send_status(tx, &host.name, TunnelStatus::Retrying(reason)).await; - if let Some(path) = locked { - // Redialling cannot help until the key is unlocked, and every try is - // a failed login on the server: hold the ports and wait for it. - identity::ask_passphrase_once(tx, &host.name, &path).await; - identity::unlocked(&path).await; + if let Some(login) = waiting { + // Redialling cannot help until the key is unlocked or a password is + // typed for the login (in a terminal, say), and every try is a failed + // login on the server: hold the ports and wait for either. + if let Some(path) = locked { + identity::ask_passphrase_once(tx, &host.name, &path).await; + tokio::select! { + () = identity::unlocked(&path) => {} + () = password::remembered(&login) => {} + } + } else { + // No key got in, which an agent unlocked meanwhile could change: + // look again now and then. + tokio::select! { + () = password::remembered(&login) => {} + () = time::sleep(NO_PASSWORD_RETRY) => {} + } + } continue; } time::sleep(RETRY_DELAYS[retry]).await; diff --git a/crates/omnyssh-core/tests/login.rs b/crates/omnyssh-core/tests/login.rs new file mode 100644 index 00000000..a647d910 --- /dev/null +++ b/crates/omnyssh-core/tests/login.rs @@ -0,0 +1,360 @@ +//! Logging in, end to end, against an in-process SSH server that can switch the +//! password method and keyboard-interactive on and off. +//! +//! On unix every test runs with an SSH agent whose one key the server accepts +//! but the agent refuses to sign with, so each login also goes through the path +//! where a refused signature must not leave the connection stuck. + +use std::net::SocketAddr; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex, Once}; +use std::time::Duration; + +use russh::keys::key::KeyPair; +use russh::server::{self, Auth, Msg, Response, Session}; +use russh::{Channel, ChannelId, CryptoVec}; +use tokio::net::TcpListener; +use tokio::sync::mpsc; + +use omnyssh_core::event::CoreEvent; +use omnyssh_core::ssh::client::Host; +use omnyssh_core::ssh::pty::PtyManager; +use omnyssh_core::ssh::session::SshSession; + +const PASSWORD: &str = "login-test"; + +/// Keeps trust-on-first-use off the real `~/.ssh`, and starts the agent. +fn isolate_home() { + static ONCE: Once = Once::new(); + ONCE.call_once(|| { + let home = tempfile::tempdir().expect("tempdir").keep(); + std::env::set_var("HOME", &home); + std::env::set_var("USERPROFILE", &home); + std::env::remove_var("SSH_AUTH_SOCK"); + #[cfg(unix)] + refusing_agent(&home); + }); +} + +/// An ssh-agent holding one key it asks confirmation for, with nothing to ask +/// with — so it turns every signature down. +#[cfg(unix)] +fn refusing_agent(home: &std::path::Path) { + use std::process::Command; + let socket = home.join("agent.sock"); + let key = home.join("agent_key"); + let started = Command::new("ssh-agent") + .arg("-a") + .arg(&socket) + .env_remove("SSH_ASKPASS") + .env_remove("DISPLAY") + .output() + .expect("these tests need ssh-agent on PATH"); + assert!(started.status.success(), "ssh-agent failed to start"); + let keygen = Command::new("ssh-keygen") + .args(["-q", "-t", "ed25519", "-N", ""]) + .arg("-f") + .arg(&key) + .status() + .expect("these tests need ssh-keygen on PATH"); + assert!(keygen.success()); + let added = Command::new("ssh-add") + .arg("-q") + .arg("-c") + .arg(&key) + .env("SSH_AUTH_SOCK", &socket) + .status() + .expect("these tests need ssh-add on PATH"); + assert!(added.success()); + std::env::set_var("SSH_AUTH_SOCK", &socket); +} + +// --------------------------------------------------------------------------- +// SSH server +// --------------------------------------------------------------------------- + +/// What the server takes, and what it saw. +#[derive(Clone, Default)] +struct Server { + password_method: bool, + /// Keyboard-interactive, with this prompt; `None` for not offered. + kbd_prompt: Option<&'static str>, + connections: Arc, + /// Password-method requests, whether the method is on or not. + password_tries: Arc, + /// Every keyboard-interactive answer it was sent. + kbd_answers: Arc>>, +} + +#[async_trait::async_trait] +impl server::Handler for Server { + type Error = russh::Error; + + async fn auth_password(&mut self, _user: &str, password: &str) -> Result { + self.password_tries.fetch_add(1, Ordering::SeqCst); + Ok(if self.password_method && password == PASSWORD { + Auth::Accept + } else { + Auth::Reject { + proceed_with_methods: None, + } + }) + } + + async fn auth_keyboard_interactive( + &mut self, + _user: &str, + _submethods: &str, + response: Option>, + ) -> Result { + let Some(prompt) = self.kbd_prompt else { + return Ok(Auth::Reject { + proceed_with_methods: None, + }); + }; + let Some(response) = response else { + return Ok(Auth::Partial { + name: "".into(), + instructions: "".into(), + prompts: vec![(prompt.into(), false)].into(), + }); + }; + let answers: Vec = response + .map(|a| String::from_utf8_lossy(a).into_owned()) + .collect(); + let accepted = answers == [PASSWORD]; + self.kbd_answers.lock().unwrap().extend(answers); + Ok(if accepted { + Auth::Accept + } else { + Auth::Reject { + proceed_with_methods: None, + } + }) + } + + // Every key is turned down (the agent's only after it was asked to sign). + async fn auth_publickey( + &mut self, + _user: &str, + _key: &russh::keys::key::PublicKey, + ) -> Result { + Ok(Auth::Reject { + proceed_with_methods: None, + }) + } + + async fn channel_open_session( + &mut self, + _channel: Channel, + _session: &mut Session, + ) -> Result { + Ok(true) + } + + async fn shell_request( + &mut self, + channel: ChannelId, + session: &mut Session, + ) -> Result<(), Self::Error> { + session.data(channel, CryptoVec::from_slice(b"logged-in\r\n")); + Ok(()) + } +} + +/// Serves `server` on a loopback port. +async fn serve(server: Server) -> SocketAddr { + let config = Arc::new(server::Config { + keys: vec![KeyPair::generate_ed25519()], + auth_rejection_time: Duration::ZERO, + auth_rejection_time_initial: Some(Duration::ZERO), + ..Default::default() + }); + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind"); + let addr = listener.local_addr().expect("addr"); + tokio::spawn(async move { + while let Ok((socket, _)) = listener.accept().await { + server.connections.fetch_add(1, Ordering::SeqCst); + let _ = server::run_stream(Arc::clone(&config), socket, server.clone()).await; + } + }); + addr +} + +/// Named for the test, which is also its user: what a login learns is kept per +/// user@host:port, and another test's server may get the same port later. +fn host(name: &str, addr: SocketAddr, password: Option<&str>) -> Host { + Host { + name: name.to_string(), + hostname: addr.ip().to_string(), + port: addr.port(), + user: name.to_string(), + password: password.map(str::to_string), + ..Host::default() + } +} + +// --------------------------------------------------------------------------- +// Saved passwords +// --------------------------------------------------------------------------- + +/// A server that turns the password method off (a UniFi console) takes the +/// saved password by keyboard-interactive, and the next login no longer tries +/// the password method first. +#[tokio::test] +async fn a_saved_password_gets_in_by_keyboard_interactive() { + isolate_home(); + let server = Server { + kbd_prompt: Some("Password: "), + ..Server::default() + }; + let addr = serve(server.clone()).await; + let udm = host("udm", addr, Some(PASSWORD)); + + SshSession::connect(&udm) + .await + .expect("first login") + .disconnect() + .await; + assert_eq!(server.password_tries.load(Ordering::SeqCst), 1); + + SshSession::connect(&udm).await.expect("second login"); + assert_eq!( + server.password_tries.load(Ordering::SeqCst), + 1, + "the method that works is remembered" + ); +} + +/// A refused saved password is sent once per login. Finding out that the +/// server has no keyboard-interactive costs one extra connection, once. +#[tokio::test] +async fn a_refused_password_is_sent_once_per_login() { + isolate_home(); + let server = Server { + password_method: true, + ..Server::default() + }; + let addr = serve(server.clone()).await; + let typo = host("typo", addr, Some("wrong")); + + let e = SshSession::connect(&typo).await.err().expect("refused"); + assert!(format!("{e:#}").contains("authentication failed"), "{e:#}"); + assert_eq!(server.password_tries.load(Ordering::SeqCst), 1); + assert_eq!(server.connections.load(Ordering::SeqCst), 2); + + SshSession::connect(&typo) + .await + .err() + .expect("refused again"); + assert_eq!(server.password_tries.load(Ordering::SeqCst), 2); + assert_eq!(server.connections.load(Ordering::SeqCst), 3); +} + +/// A prompt for a one-time code is not answered with the password. +#[tokio::test] +async fn a_code_prompt_never_gets_the_password() { + isolate_home(); + let server = Server { + kbd_prompt: Some("Verification code: "), + ..Server::default() + }; + let addr = serve(server.clone()).await; + + let e = SshSession::connect(&host("otp", addr, Some(PASSWORD))) + .await + .err() + .expect("no way in"); + assert!(format!("{e:#}").contains("verification code"), "{e:#}"); + assert!( + !server + .kbd_answers + .lock() + .unwrap() + .iter() + .any(|a| a == PASSWORD), + "the password went to a code prompt" + ); +} + +/// No key and no password: the error says so, with the hint before any ':' +/// (frontends cut there). +#[tokio::test] +async fn a_login_with_nothing_to_try_says_what_is_missing() { + isolate_home(); + let addr = serve(Server { + password_method: true, + ..Server::default() + }) + .await; + + let e = SshSession::connect(&host("bare", addr, None)) + .await + .err() + .expect("no way in"); + let message = e.to_string(); + let head = message.split(':').next().unwrap_or_default(); + assert!(head.contains("no password is saved"), "{message}"); +} + +// --------------------------------------------------------------------------- +// The terminal asks +// --------------------------------------------------------------------------- + +async fn screen_contains(pty: &PtyManager, id: u64, text: &str) -> bool { + for _ in 0..100 { + if let Some(parser) = pty.parser_for(id) { + if parser.lock().unwrap().screen().contents().contains(text) { + return true; + } + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + false +} + +/// A terminal asks for the password in the tab, says when one is refused, and +/// logs in with the next — on the same keyboard-interactive connection. +#[tokio::test] +async fn a_terminal_asks_for_the_password_in_the_tab() { + isolate_home(); + let server = Server { + kbd_prompt: Some("Password: "), + ..Server::default() + }; + let addr = serve(server.clone()).await; + let (tx, mut rx) = mpsc::channel::(256); + tokio::spawn(async move { while rx.recv().await.is_some() {} }); + + let mut pty = PtyManager::new(); + let id = pty + .open(&host("term", addr, None), 80, 24, tx) + .expect("open"); + let login = format!("term@{}'s password:", addr.ip()); + assert!(screen_contains(&pty, id, &login).await, "no prompt"); + + pty.write(id, b"wrong\r").expect("write"); + assert!( + screen_contains(&pty, id, "Permission denied, please try again.").await, + "no retry" + ); + pty.write(id, format!("{PASSWORD}\r").as_bytes()) + .expect("write"); + assert!(screen_contains(&pty, id, "logged-in").await, "no shell"); + + assert!( + !pty.parser_for(id) + .expect("session") + .lock() + .unwrap() + .screen() + .contents() + .contains(PASSWORD), + "the password is never echoed" + ); + assert_eq!( + server.connections.load(Ordering::SeqCst), + 2, + "the keys' connection, and one keyboard-interactive connection for both answers" + ); +} diff --git a/crates/omnyssh-core/tests/tunnel.rs b/crates/omnyssh-core/tests/tunnel.rs index 8089565b..265178c6 100644 --- a/crates/omnyssh-core/tests/tunnel.rs +++ b/crates/omnyssh-core/tests/tunnel.rs @@ -454,8 +454,7 @@ async fn a_rejected_password_fails_without_a_retry() { other => panic!("expected Failed, got {other:?}"), } - tokio::time::sleep(Duration::from_secs(3)).await; - assert_eq!(link.dials(), 1, "a refused login must not be retried"); + assert_no_redial(&link).await; assert!(!tunnels.is_running("typo")); assert!( !is_bound(local).await, @@ -511,8 +510,16 @@ async fn a_server_that_hangs_up_after_a_rejection_is_not_redialled() { assert_eq!(statuses.next("maxed").await, TunnelStatus::Connecting); let status = statuses.next("maxed").await; assert!(matches!(status, TunnelStatus::Failed(_)), "{status:?}"); + assert_no_redial(&link).await; +} + +/// One login is at most two connections — the refused one, and one that finds +/// out whether keyboard-interactive takes the password — and nothing follows. +async fn assert_no_redial(link: &Link) { + let dials = link.dials(); + assert!(dials <= 2, "{dials} connections for one login"); tokio::time::sleep(Duration::from_secs(3)).await; - assert_eq!(link.dials(), 1, "a refused login must not be retried"); + assert_eq!(link.dials(), dials, "a refused login must not be retried"); } /// A locked key is not a refusal: the tunnel asks for the passphrase once and @@ -544,8 +551,10 @@ async fn a_locked_key_waits_for_its_passphrase() { } other => panic!("expected Retrying, got {other:?}"), } + let dials = link.dials(); + assert!(dials <= 2, "{dials} connections for one login"); tokio::time::sleep(Duration::from_secs(3)).await; - assert_eq!(link.dials(), 1, "a locked key must not be redialled"); + assert_eq!(link.dials(), dials, "a locked key must not be redialled"); assert!( is_bound(local).await, "the tunnel keeps its ports while it waits" @@ -567,11 +576,16 @@ async fn a_locked_key_waits_for_its_passphrase() { assert!(statuses.rx.try_recv().is_err(), "the prompt is sent once"); omnyssh_core::ssh::identity::unlock(&key_path, "sesame").expect("unlock"); - // The server turns every key down and hangs up, so this dial is refused. + // The server turns every key down and hangs up, so this dial is refused — + // after one more connection for the saved password, which the hang-up cut off. statuses .until("locked", |s| matches!(s, TunnelStatus::Failed(_))) .await; - assert_eq!(link.dials(), 2, "the unlock redials at once"); + let redials = link.dials() - dials; + assert!( + (1..=2).contains(&redials), + "the unlock redials at once: {redials}" + ); } /// A host key that no longer matches `known_hosts` is refused for good. diff --git a/crates/omnyssh-gui/src/bridge.rs b/crates/omnyssh-gui/src/bridge.rs index 8aaef392..4e5172a6 100644 --- a/crates/omnyssh-gui/src/bridge.rs +++ b/crates/omnyssh-gui/src/bridge.rs @@ -64,6 +64,22 @@ pub async fn forward_core_events(app: AppHandle, mut rx: mpsc::Receiver { + let _ = events::PasswordRequired { + request_id, + host_name, + login, + retry, + new_host_key, + } + .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 64ffbddb..b61cc5db 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 a1e856a3..be3c6b8b 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(&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 8388a36a..b66ba5cd 100644 --- a/crates/omnyssh-gui/src/events.rs +++ b/crates/omnyssh-gui/src/events.rs @@ -186,6 +186,20 @@ 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; `newHostKey` is the fingerprint of a host key +/// first seen on this connection, to check before typing. +#[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, + pub new_host_key: Option, +} + /// 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 eecea956..2c9b617b 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,7 @@ fn specta_builder() -> Builder { events::KeySetupRollback, events::UpdateAvailable, events::KeyPassphraseRequired, + events::PasswordRequired, 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 00000000..f45d85ba --- /dev/null +++ b/crates/omnyssh-gui/ui/e2e/password.spec.ts @@ -0,0 +1,188 @@ +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, + newHostKey: null + }), + 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 }; + // Only the request the stub issued last is waited on, as in the core. + if (requestId !== request) return Promise.reject({ message: 'no login is waiting for this password' }); + 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 server met for the first time shows its host key before the password goes', 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, + newHostKey: 'SHA256:abcDEF123' + }); + }); + const dialog = page.getByRole('dialog', { name: 'SSH login' }); + await expect(dialog.getByText('SHA256:abcDEF123')).toBeVisible(); +}); + +test('a prompt its login gave up on closes and says so when answered', 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: 41, hostName: 'nas', login: 'admin@nas.example.com', retry: false, newHostKey: null }); + }); + const dialog = page.getByRole('dialog', { name: 'SSH login' }); + await dialog.getByLabel('Password').fill('late'); + await dialog.getByLabel('Password').press('Enter'); + await expect(page.getByRole('dialog')).toHaveCount(0); + await expect(page.getByText('That login stopped waiting for a password. Open it again.')).toBeVisible(); +}); + +test('a click beside the dialog does not cancel the login', 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.mouse.click(5, 5); + await expect(dialog).toBeVisible(); + 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 d8397afd..71643ac5 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,7 @@ keySetupFailed: KeySetupFailed, keySetupProgress: KeySetupProgress, keySetupRollback: KeySetupRollback, metricsUpdated: MetricsUpdated, +passwordRequired: PasswordRequired, servicesDetected: ServicesDetected, servicesFailed: ServicesFailed, sftpConnected: SftpConnected, @@ -434,6 +448,7 @@ keySetupFailed: "key-setup-failed", keySetupProgress: "key-setup-progress", keySetupRollback: "key-setup-rollback", metricsUpdated: "metrics-updated", +passwordRequired: "password-required", servicesDetected: "services-detected", servicesFailed: "services-failed", sftpConnected: "sftp-connected", @@ -552,6 +567,13 @@ 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" +/** + * 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; `newHostKey` is the fingerprint of a host key + * first seen on this connection, to check before typing. + */ +export type PasswordRequired = { requestId: number; hostName: string; login: string; retry: boolean; newHostKey: string | null } /** * 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 f89cbe7e..486500e9 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/components/Modal.svelte b/crates/omnyssh-gui/ui/src/lib/components/Modal.svelte index 71c8ee31..fbd63d8f 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 @@