diff --git a/crates/agent-gui/src-tauri/src/runtime/terminal/ssh_auth.rs b/crates/agent-gui/src-tauri/src/runtime/terminal/ssh_auth.rs index 94ad47f2a..8d6df7b1c 100644 --- a/crates/agent-gui/src-tauri/src/runtime/terminal/ssh_auth.rs +++ b/crates/agent-gui/src-tauri/src/runtime/terminal/ssh_auth.rs @@ -34,6 +34,7 @@ pub(crate) fn resolve_ssh_auth_material( .trim() .to_string() }; + let key = normalize_ssh_private_key_material(&key); if key.is_empty() { return Err("SSH private key is empty".to_string()); } @@ -51,6 +52,90 @@ pub(crate) fn resolve_ssh_auth_material( } } +/// Normalize pasted or stored private key material so that common copy/paste +/// artifacts do not break key parsing. russh's PEM reader matches the +/// `-----BEGIN ...-----` marker by exact line equality and silently drops any +/// base64 line that carries extra characters (for example a trailing space), +/// so a key that "looks fine" in the settings UI can fail to decode with +/// `Could not read key`. This repairs: +/// - UTF-8 BOM, zero-width characters, and non-breaking spaces +/// - CRLF / lone CR line endings and literal `\n` / `\r\n` escape sequences +/// (keys copied out of JSON or shell one-liners) +/// - per-line leading/trailing whitespace (indented paste) +/// - PEM blocks collapsed onto a single line (newlines lost while pasting) +pub(crate) fn normalize_ssh_private_key_material(raw: &str) -> String { + let mut text = raw.to_string(); + for zero_width in ['\u{feff}', '\u{200b}', '\u{200c}', '\u{200d}'] { + text = text.replace(zero_width, ""); + } + text = text.replace('\u{a0}', " "); + text = text + .replace("\r\n", "\n") + .replace('\r', "\n") + .replace("\\r\\n", "\n") + .replace("\\n", "\n") + .replace("\\r", "\n"); + let joined = text + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .collect::>() + .join("\n"); + refold_pem_block(&joined).unwrap_or(joined) +} + +/// Rebuild the first PEM block found in `text` with canonical line folding: +/// the BEGIN/END markers on their own lines and the base64 body wrapped at +/// 64 columns. Returns `None` (leaving the input untouched) when there is no +/// PEM block or when the body carries PEM headers such as `Proc-Type:` / +/// `DEK-Info:` (legacy encrypted PEM), whose line structure must be kept. +pub(crate) fn refold_pem_block(text: &str) -> Option { + const BEGIN: &str = "-----BEGIN "; + const DASHES: &str = "-----"; + let begin_start = text.find(BEGIN)?; + let after_begin = &text[begin_start + BEGIN.len()..]; + let label_end = after_begin.find(DASHES)?; + let label = after_begin[..label_end].trim(); + if label.is_empty() { + return None; + } + let body_start = begin_start + BEGIN.len() + label_end + DASHES.len(); + let end_marker = format!("-----END {label}-----"); + let end_rel = text[body_start..].find(&end_marker)?; + let body_raw = &text[body_start..body_start + end_rel]; + if body_raw.contains(':') { + return None; + } + let body: String = body_raw.chars().filter(|c| !c.is_whitespace()).collect(); + if body.is_empty() { + return None; + } + let mut folded = format!("-----BEGIN {label}-----\n"); + for chunk in body.as_bytes().chunks(64) { + folded.push_str(std::str::from_utf8(chunk).ok()?); + folded.push('\n'); + } + folded.push_str(&end_marker); + Some(folded) +} + +/// Translate private key decode failures into actionable messages instead of +/// a generic "Invalid SSH private key". +pub(crate) fn describe_ssh_private_key_decode_error( + error: &russh::keys::Error, + has_passphrase: bool, +) -> String { + match error { + russh::keys::Error::KeyIsEncrypted if !has_passphrase => { + "SSH private key is encrypted; configure the key passphrase for this host".to_string() + } + _ if has_passphrase => { + format!("Invalid SSH private key or wrong passphrase: {error}") + } + _ => format!("Invalid SSH private key: {error}"), + } +} + pub(crate) fn expand_ssh_private_key_path(path: &str) -> PathBuf { let home = dirs::home_dir() .map(|path| path.to_string_lossy().into_owned()) @@ -248,8 +333,25 @@ pub(crate) async fn authenticate_ssh_handle( } ResolvedSshAuth::PrivateKey { key, passphrase } => { let key_pair = russh::keys::decode_secret_key(&key, passphrase.as_deref()) - .map_err(|error| format!("Invalid SSH private key: {error}"))?; - let key = PrivateKeyWithHashAlg::new(Arc::new(key_pair), Some(HashAlg::Sha256)); + .map_err(|error| { + describe_ssh_private_key_decode_error(&error, passphrase.is_some()) + })?; + // Negotiate the RSA signature hash from the server's + // `server-sig-algs` extension (RFC 8308). Hardcoding SHA-256 + // breaks against servers that only accept `ssh-rsa` (SHA-1); + // when the server does not advertise the extension, keep the + // previous SHA-256 behavior. Non-RSA keys skip the negotiation + // entirely because the hash algorithm is ignored for them. + let hash_alg = if key_pair.algorithm().is_rsa() { + handle + .best_supported_rsa_hash() + .await + .map_err(|error| format!("SSH private key authentication failed: {error}"))? + .unwrap_or(Some(HashAlg::Sha256)) + } else { + None + }; + let key = PrivateKeyWithHashAlg::new(Arc::new(key_pair), hash_alg); let result = handle .authenticate_publickey(host.username.as_str(), key) .await diff --git a/crates/agent-gui/src-tauri/src/runtime/terminal/tests.rs b/crates/agent-gui/src-tauri/src/runtime/terminal/tests.rs index 57889d411..d8c9ecc1c 100644 --- a/crates/agent-gui/src-tauri/src/runtime/terminal/tests.rs +++ b/crates/agent-gui/src-tauri/src/runtime/terminal/tests.rs @@ -770,3 +770,169 @@ fn read_tail_requires_terminal_id_when_project_has_multiple_sessions() { registry.close_all().expect("close terminal sessions"); } + +// Throwaway ed25519 keypair generated exclusively for these tests; it is not +// authorized on any host. +const TEST_ED25519_PRIVATE_KEY: &str = "-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAhgk+uA1+13AN5TzsuvFZ6XDF0GlH9Kalc5hiRwXZqwAAAAKDyBWEV8gVh +FQAAAAtzc2gtZWQyNTUxOQAAACAhgk+uA1+13AN5TzsuvFZ6XDF0GlH9Kalc5hiRwXZqwA +AAAECRQtp7Gi2+TPkNeccdy+icQHNF/IzJfSQKpKQV2gGOYCGCT64DX7XcA3lPOy68Vnpc +MXQaUf0pqVzmGJHBdmrAAAAAFmxpdmVhZ2VudC10ZXN0LWZpeHR1cmUBAgMEBQYH +-----END OPENSSH PRIVATE KEY-----"; + +// Same fixture key encrypted with the passphrase "test-passphrase". +const TEST_ED25519_ENCRYPTED_PRIVATE_KEY: &str = "-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAACmFlczI1Ni1jdHIAAAAGYmNyeXB0AAAAGAAAABCDMU8BW1 +9ccIJ7UHoiwkS7AAAAGAAAAAEAAAAzAAAAC3NzaC1lZDI1NTE5AAAAIGitZyZ1OZcCXZWJ +EE8s43cVaBefHroCHgCzq+B01aK3AAAAoD4IuQwaxZi4m/hCmL4GT9kUERgxkZmQVN8noi ++OqtXHQ2+W7ykmmIJ8iwHTpK3W5WWJMmW+6tKhubShGMti7DwxpwJzL4dIkmTqs+e0wMZP +MZP6dJOnynjSTFz0RzJPHmEEOoy5kMDEXZx7UtRGNH/PYzZ5OeG5k9MhwVSp42TYs18wMI +OC6JVzVSYPJ41KjMtWIJkGFfLqqIPlNM5J2WI= +-----END OPENSSH PRIVATE KEY-----"; + +fn ssh_private_key_host(private_key: &str, passphrase: &str) -> RuntimeSshHostConfig { + RuntimeSshHostConfig { + id: "keyed".to_string(), + name: "Keyed".to_string(), + host: "keyed.example.com".to_string(), + port: 22, + username: "deploy".to_string(), + auth_type: "privateKey".to_string(), + password: String::new(), + private_key: private_key.to_string(), + private_key_path: String::new(), + private_key_passphrase: passphrase.to_string(), + proxy: crate::commands::settings::RuntimeSshProxyConfig { + proxy_type: String::new(), + url: String::new(), + port: 0, + username: String::new(), + password: String::new(), + password_configured: false, + }, + } +} + +fn resolved_private_key(host: &RuntimeSshHostConfig) -> String { + match resolve_ssh_auth_material(host).expect("resolve private key auth") { + ResolvedSshAuth::PrivateKey { key, .. } => key, + _ => panic!("expected private key auth material"), + } +} + +#[test] +fn normalize_private_key_repairs_paste_artifacts() { + let canonical = normalize_ssh_private_key_material(TEST_ED25519_PRIVATE_KEY); + russh::keys::decode_secret_key(&canonical, None).expect("canonical key decodes"); + + let crlf = TEST_ED25519_PRIVATE_KEY.replace('\n', "\r\n"); + let indented: String = TEST_ED25519_PRIVATE_KEY + .lines() + .map(|line| format!(" {line}\n")) + .collect(); + let trailing_ws: String = TEST_ED25519_PRIVATE_KEY + .lines() + .map(|line| format!("{line} \n")) + .collect(); + let single_line = TEST_ED25519_PRIVATE_KEY.replace('\n', " "); + let escaped_newlines = TEST_ED25519_PRIVATE_KEY.replace('\n', "\\n"); + let with_bom = format!("\u{feff}{TEST_ED25519_PRIVATE_KEY}"); + let surrounded = format!("key material:\n{TEST_ED25519_PRIVATE_KEY}\n"); + + for (label, mangled) in [ + ("crlf", crlf), + ("indented", indented), + ("trailing-ws", trailing_ws), + ("single-line", single_line), + ("escaped-newlines", escaped_newlines), + ("bom", with_bom), + ("surrounded", surrounded), + ] { + let normalized = normalize_ssh_private_key_material(&mangled); + assert_eq!(normalized, canonical, "variant {label} normalizes"); + russh::keys::decode_secret_key(&normalized, None) + .unwrap_or_else(|error| panic!("variant {label} decodes: {error}")); + } +} + +#[test] +fn normalize_private_key_is_idempotent() { + let normalized = normalize_ssh_private_key_material(TEST_ED25519_PRIVATE_KEY); + assert_eq!(normalize_ssh_private_key_material(&normalized), normalized); +} + +#[test] +fn normalize_private_key_keeps_encrypted_pem_headers() { + let legacy_encrypted = "-----BEGIN RSA PRIVATE KEY-----\nProc-Type: 4,ENCRYPTED\nDEK-Info: AES-128-CBC,0123456789ABCDEF0123456789ABCDEF\n\nAAAABBBBCCCC\n-----END RSA PRIVATE KEY-----"; + let normalized = normalize_ssh_private_key_material(legacy_encrypted); + assert!(normalized.contains("Proc-Type: 4,ENCRYPTED\n")); + assert!(normalized.contains("DEK-Info: AES-128-CBC,")); + assert!(normalized.contains("\nAAAABBBBCCCC\n")); +} + +#[test] +fn normalize_private_key_passes_ppk_through() { + let ppk = "PuTTY-User-Key-File-3: ssh-ed25519\nEncryption: none\nComment: test\nPublic-Lines: 2\nAAAA\nBBBB"; + assert_eq!(normalize_ssh_private_key_material(ppk), ppk); +} + +#[test] +fn resolve_ssh_auth_material_normalizes_pasted_private_key() { + let mangled: String = TEST_ED25519_PRIVATE_KEY + .lines() + .map(|line| format!(" {line} \r\n")) + .collect(); + let host = ssh_private_key_host(&mangled, ""); + + let key = resolved_private_key(&host); + russh::keys::decode_secret_key(&key, None).expect("normalized pasted key decodes"); +} + +#[test] +fn resolve_ssh_auth_material_reads_key_from_file() { + let dir = tempfile::tempdir().expect("create temp dir"); + let key_path = dir.path().join("fixture_ed25519"); + std::fs::write(&key_path, format!("{TEST_ED25519_PRIVATE_KEY}\n")).expect("write fixture key"); + + let mut host = ssh_private_key_host("", ""); + host.private_key_path = key_path.to_string_lossy().into_owned(); + + let key = resolved_private_key(&host); + russh::keys::decode_secret_key(&key, None).expect("file-based key decodes"); +} + +#[test] +fn encrypted_private_key_decodes_with_passphrase() { + let host = ssh_private_key_host(TEST_ED25519_ENCRYPTED_PRIVATE_KEY, "test-passphrase"); + match resolve_ssh_auth_material(&host).expect("resolve encrypted key auth") { + ResolvedSshAuth::PrivateKey { key, passphrase } => { + russh::keys::decode_secret_key(&key, passphrase.as_deref()) + .expect("encrypted key decodes with passphrase"); + } + _ => panic!("expected private key auth material"), + } +} + +#[test] +fn private_key_decode_error_explains_missing_passphrase() { + let key = normalize_ssh_private_key_material(TEST_ED25519_ENCRYPTED_PRIVATE_KEY); + + let missing = russh::keys::decode_secret_key(&key, None) + .map(|_| ()) + .expect_err("encrypted key without passphrase fails"); + let message = describe_ssh_private_key_decode_error(&missing, false); + assert!( + message.contains("passphrase"), + "missing-passphrase message should mention the passphrase: {message}" + ); + + let wrong = russh::keys::decode_secret_key(&key, Some("wrong")) + .map(|_| ()) + .expect_err("encrypted key with wrong passphrase fails"); + let message = describe_ssh_private_key_decode_error(&wrong, true); + assert!( + message.contains("wrong passphrase"), + "wrong-passphrase message should hint at the passphrase: {message}" + ); +}