From f74b68663363249a76fbd9707dbd1cb1f6c45c5f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 21 Oct 2026 03:54:37 +0700 Subject: [PATCH 1/3] test(linux-ssh): run the tunnel against OpenSSH 9.9 and 10.3 across keys, agents and key exchange --- .github/workflows/build-linux.yml | 5 + linux/Cargo.lock | 1 + linux/crates/app/src/ui/error_text/ssh.rs | 37 +- linux/crates/app/src/ui/error_text/tests.rs | 51 +- linux/crates/core/src/lib.rs | 2 + linux/crates/core/src/ssh_algorithm_kind.rs | 23 + linux/crates/core/src/ssh_failure.rs | 49 +- linux/crates/ssh/Cargo.toml | 1 + linux/crates/ssh/src/argv.rs | 64 ++- linux/crates/ssh/src/identity_pin.rs | 168 +++++++ linux/crates/ssh/src/lib.rs | 1 + linux/crates/ssh/src/session.rs | 14 + linux/crates/ssh/src/stderr_classify.rs | 172 ++++++- .../fixtures/stderr/no_matching_cipher.txt | 1 + .../stderr/no_matching_key_exchange.txt | 1 + .../too_many_authentication_failures.txt | 3 + linux/crates/ssh/tests/openssh_matrix.rs | 444 ++++++++++++++++++ .../crates/test-fixtures/src/fixture_image.rs | 4 + linux/crates/test-fixtures/src/lib.rs | 4 + .../test-fixtures/src/openssh_fixture.rs | 66 ++- .../test-fixtures/src/openssh_version.rs | 56 +++ linux/crates/test-fixtures/src/ssh_agent.rs | 43 +- .../test-fixtures/src/ssh_auth_variant.rs | 22 +- .../crates/test-fixtures/src/ssh_key_pair.rs | 98 +++- .../crates/test-fixtures/src/ssh_key_type.rs | 37 ++ linux/crates/test-fixtures/tests/fixtures.rs | 44 +- linux/docs/testing.md | 7 +- 27 files changed, 1348 insertions(+), 70 deletions(-) create mode 100644 linux/crates/core/src/ssh_algorithm_kind.rs create mode 100644 linux/crates/ssh/src/identity_pin.rs create mode 100644 linux/crates/ssh/tests/fixtures/stderr/no_matching_cipher.txt create mode 100644 linux/crates/ssh/tests/fixtures/stderr/no_matching_key_exchange.txt create mode 100644 linux/crates/ssh/tests/fixtures/stderr/too_many_authentication_failures.txt create mode 100644 linux/crates/ssh/tests/openssh_matrix.rs create mode 100644 linux/crates/test-fixtures/src/openssh_version.rs create mode 100644 linux/crates/test-fixtures/src/ssh_key_type.rs diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index b480d45cec..5e9eff4798 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -409,6 +409,10 @@ jobs: include: - package: tablepro-ssh apt: openssh-client + - package: tablepro-ssh + release: openssh-9.9 + apt: openssh-client + openssh: "9.9" - package: tablepro-test-fixtures apt: openssh-client - package: tablepro-driver-postgres @@ -471,6 +475,7 @@ jobs: TABLEPRO_TEST_MARIADB: ${{ matrix.mariadb }} TABLEPRO_TEST_MSSQL: ${{ matrix.mssql }} TABLEPRO_TEST_CLICKHOUSE: ${{ matrix.clickhouse }} + TABLEPRO_TEST_OPENSSH: ${{ matrix.openssh }} run: cargo nextest run --locked --profile ci -p "$PACKAGE" --run-ignored only - name: Upload JUnit report if: always() diff --git a/linux/Cargo.lock b/linux/Cargo.lock index 46d73fb052..24d807ad7e 100644 --- a/linux/Cargo.lock +++ b/linux/Cargo.lock @@ -4613,6 +4613,7 @@ dependencies = [ "rustix", "secrecy", "tablepro-core", + "tablepro-test-fixtures", "tablepro-test-support", "tempfile", "testcontainers", diff --git a/linux/crates/app/src/ui/error_text/ssh.rs b/linux/crates/app/src/ui/error_text/ssh.rs index db150bd5d0..8dbdcd065b 100644 --- a/linux/crates/app/src/ui/error_text/ssh.rs +++ b/linux/crates/app/src/ui/error_text/ssh.rs @@ -1,6 +1,6 @@ use std::io::ErrorKind; -use tablepro_core::{SshDestinationPart, SshDestinationProblem, SshFailure, SshRuntimeDirProblem}; +use tablepro_core::{SshAlgorithmKind, SshDestinationPart, SshDestinationProblem, SshFailure, SshRuntimeDirProblem}; /// One sentence per way OpenSSH can fail, each naming what to change. pub fn ssh_failure_message(failure: &SshFailure) -> String { @@ -64,6 +64,15 @@ pub fn ssh_failure_message(failure: &SshFailure) -> String { "The SSH server {host} refused the login for {user}. Check the key, the agent or the password.", &[("host", host), ("user", user)], ), + SshFailure::AuthenticationAttemptsExceeded { host } => crate::i18n::gettext_f( + "The SSH server {host} closed the connection after too many failed sign-in attempts. An SSH agent with many keys can use them all up: use Private key authentication with the right key file, or set IdentityFile and IdentitiesOnly yes for the host in ~/.ssh/config.", + &[("host", host)], + ), + SshFailure::NoCommonAlgorithm { + host, + kind, + server_offer, + } => no_common_algorithm_message(host, *kind, &server_offer.join(", ")), SshFailure::InteractionCancelled => crate::i18n::gettext("The SSH login was cancelled."), SshFailure::ForwardRejected { target, detail } => crate::i18n::gettext_f( "The SSH server would not forward to {target}: {detail}", @@ -129,6 +138,32 @@ fn unknown_host_key_message(host: &str, fingerprint: &str, trust_allowed: bool, ) } +fn no_common_algorithm_message(host: &str, kind: SshAlgorithmKind, offer: &str) -> String { + let arguments = [("host", host), ("offer", offer)]; + match kind { + SshAlgorithmKind::KeyExchange => crate::i18n::gettext_f( + "The SSH server {host} and this computer's OpenSSH have no key exchange method in common. The server allows {offer}.", + &arguments, + ), + SshAlgorithmKind::HostKey => crate::i18n::gettext_f( + "The SSH server {host} and this computer's OpenSSH have no host key type in common. The server offers {offer}.", + &arguments, + ), + SshAlgorithmKind::Cipher => crate::i18n::gettext_f( + "The SSH server {host} and this computer's OpenSSH have no cipher in common. The server allows {offer}.", + &arguments, + ), + SshAlgorithmKind::Mac => crate::i18n::gettext_f( + "The SSH server {host} and this computer's OpenSSH have no MAC in common. The server allows {offer}.", + &arguments, + ), + SshAlgorithmKind::Compression => crate::i18n::gettext_f( + "The SSH server {host} and this computer's OpenSSH have no compression method in common. The server allows {offer}.", + &arguments, + ), + } +} + fn protocol_message(status: Option, detail: &str) -> String { match (status, detail.is_empty()) { (Some(status), true) => crate::i18n::gettext_f( diff --git a/linux/crates/app/src/ui/error_text/tests.rs b/linux/crates/app/src/ui/error_text/tests.rs index e27a8fcd01..e7856ebbbe 100644 --- a/linux/crates/app/src/ui/error_text/tests.rs +++ b/linux/crates/app/src/ui/error_text/tests.rs @@ -4,8 +4,8 @@ use tablepro_core::credentials::{SecretError, SecretKind}; use tablepro_core::sql_syntax::script::{OpenConstruct, ScriptDiagnostic}; use tablepro_core::{ BusyReason, ConfigError, DriverError, ErrorCategory, LossPhase, NetworkEndpoint, ReadOnlyRefusal, ServerCode, - ServerDiagnostics, SshDestinationPart, SshDestinationProblem, SshFailure, SshRuntimeDirProblem, StatementOutcome, - TimeoutPhase, TlsFailure, TransportError, + ServerDiagnostics, SshAlgorithmKind, SshDestinationPart, SshDestinationProblem, SshFailure, SshRuntimeDirProblem, + StatementOutcome, TimeoutPhase, TlsFailure, TransportError, }; use tablepro_session::runtime::{TaskFailure, TaskPanic}; use tablepro_session::{ConnectError, OperationError}; @@ -338,6 +338,53 @@ fn ssh_failures_say_what_to_change() { assert!(declined.contains("not trusted"), "{declined}"); } +#[test] +fn too_many_attempts_says_how_to_name_the_key() { + let message = ssh_failure_message(&SshFailure::AuthenticationAttemptsExceeded { + host: "bastion".to_owned(), + }); + + assert!(message.contains("bastion"), "{message}"); + assert!(message.contains("too many failed sign-in attempts"), "{message}"); + assert!(message.contains("Private key"), "{message}"); + assert!(message.contains("IdentitiesOnly yes"), "{message}"); +} + +#[test] +fn each_list_with_no_common_algorithm_has_its_own_words() { + let kinds = [ + SshAlgorithmKind::KeyExchange, + SshAlgorithmKind::HostKey, + SshAlgorithmKind::Cipher, + SshAlgorithmKind::Mac, + SshAlgorithmKind::Compression, + ]; + let texts: Vec = kinds + .iter() + .map(|&kind| { + ssh_failure_message(&SshFailure::NoCommonAlgorithm { + host: "bastion".to_owned(), + kind, + server_offer: vec!["first".to_owned(), "second".to_owned()], + }) + }) + .collect(); + + for text in &texts { + assert!(text.contains("bastion") && text.contains("first, second"), "{text}"); + } + assert_eq!( + texts.iter().collect::>().len(), + kinds.len(), + "{texts:?}" + ); + assert_eq!( + texts[0], + "The SSH server bastion and this computer's OpenSSH have no key exchange method in common. \ + The server allows first, second." + ); +} + /// Under StrictHostKeyChecking yes ssh refuses before it shows the key, /// so there is no fingerprint to print and no review the user can do. #[test] diff --git a/linux/crates/core/src/lib.rs b/linux/crates/core/src/lib.rs index fb8756efd2..573a9baa69 100644 --- a/linux/crates/core/src/lib.rs +++ b/linux/crates/core/src/lib.rs @@ -53,6 +53,7 @@ mod server_info; mod server_name_override; mod session_fate; pub mod sql_syntax; +mod ssh_algorithm_kind; mod ssh_destination_part; mod ssh_destination_problem; mod ssh_failure; @@ -129,6 +130,7 @@ pub use server_diagnostics::{ServerCode, ServerDiagnostics}; pub use server_info::ServerInfo; pub use server_name_override::ServerNameOverride; pub use session_fate::SessionFate; +pub use ssh_algorithm_kind::SshAlgorithmKind; pub use ssh_destination_part::SshDestinationPart; pub use ssh_destination_problem::SshDestinationProblem; pub use ssh_failure::SshFailure; diff --git a/linux/crates/core/src/ssh_algorithm_kind.rs b/linux/crates/core/src/ssh_algorithm_kind.rs new file mode 100644 index 0000000000..1cced617c0 --- /dev/null +++ b/linux/crates/core/src/ssh_algorithm_kind.rs @@ -0,0 +1,23 @@ +use std::fmt; + +/// Which list of an SSH handshake had no algorithm both sides allow. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SshAlgorithmKind { + KeyExchange, + HostKey, + Cipher, + Mac, + Compression, +} + +impl fmt::Display for SshAlgorithmKind { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::KeyExchange => "key exchange method", + Self::HostKey => "host key type", + Self::Cipher => "cipher", + Self::Mac => "MAC", + Self::Compression => "compression method", + }) + } +} diff --git a/linux/crates/core/src/ssh_failure.rs b/linux/crates/core/src/ssh_failure.rs index cfc9002101..c6e495e327 100644 --- a/linux/crates/core/src/ssh_failure.rs +++ b/linux/crates/core/src/ssh_failure.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use thiserror::Error; -use crate::{ErrorCategory, SshDestinationProblem, SshRuntimeDirProblem}; +use crate::{ErrorCategory, SshAlgorithmKind, SshDestinationProblem, SshRuntimeDirProblem}; #[derive(Debug, Clone, PartialEq, Eq, Error)] #[non_exhaustive] @@ -70,6 +70,17 @@ pub enum SshFailure { methods: Vec, }, + /// The server's MaxAuthTries ran out, as with an agent holding more keys than it allows. + #[error("{host} closed the connection after too many authentication attempts")] + AuthenticationAttemptsExceeded { host: String }, + + #[error("{host} and the OpenSSH client share no {kind} (the server offers {})", .server_offer.join(", "))] + NoCommonAlgorithm { + host: String, + kind: SshAlgorithmKind, + server_offer: Vec, + }, + #[error("the SSH prompt was cancelled")] InteractionCancelled, @@ -98,6 +109,7 @@ impl SshFailure { pub fn category(&self) -> ErrorCategory { match self { Self::AuthenticationRejected { .. } + | Self::AuthenticationAttemptsExceeded { .. } | Self::InteractionCancelled | Self::HostKeyUnknown { .. } | Self::HostKeyChanged { .. } @@ -109,7 +121,8 @@ impl SshFailure { | Self::ConfigRejected { .. } | Self::ConfigUnreadable { .. } | Self::ControlPathTooLong { .. } - | Self::RuntimeDirUnsafe { .. } => ErrorCategory::Configuration, + | Self::RuntimeDirUnsafe { .. } + | Self::NoCommonAlgorithm { .. } => ErrorCategory::Configuration, Self::ForwardRejected { .. } | Self::ChannelOpenFailed { .. } | Self::MasterExited { .. } => { ErrorCategory::Network } @@ -255,6 +268,38 @@ mod tests { assert_eq!(SshFailure::Internal.category(), ErrorCategory::Internal); } + /// Neither clears on a retry, so both park the reconnect monitor + /// rather than counting as internal. + #[test] + fn exhausted_attempts_and_no_common_algorithm_need_the_user() { + let exhausted = SshFailure::AuthenticationAttemptsExceeded { + host: "bastion".to_owned(), + }; + let no_common = SshFailure::NoCommonAlgorithm { + host: "bastion".to_owned(), + kind: SshAlgorithmKind::KeyExchange, + server_offer: vec!["mlkem768x25519-sha256".to_owned()], + }; + + assert_eq!(exhausted.category(), ErrorCategory::Authentication); + assert_eq!(no_common.category(), ErrorCategory::Configuration); + } + + #[test] + fn no_common_algorithm_names_the_list_and_the_server_offer() { + let failure = SshFailure::NoCommonAlgorithm { + host: "bastion".to_owned(), + kind: SshAlgorithmKind::KeyExchange, + server_offer: vec!["mlkem768x25519-sha256".to_owned(), "sntrup761x25519-sha512".to_owned()], + }; + + assert_eq!( + failure.to_string(), + "bastion and the OpenSSH client share no key exchange method \ + (the server offers mlkem768x25519-sha256, sntrup761x25519-sha512)" + ); + } + #[test] fn authentication_rejected_lists_the_methods_tried() { let failure = SshFailure::AuthenticationRejected { diff --git a/linux/crates/ssh/Cargo.toml b/linux/crates/ssh/Cargo.toml index 4e3825f4c5..f6d8069ff2 100644 --- a/linux/crates/ssh/Cargo.toml +++ b/linux/crates/ssh/Cargo.toml @@ -20,6 +20,7 @@ uuid.workspace = true zeroize.workspace = true [dev-dependencies] +tablepro-test-fixtures.workspace = true tablepro-test-support.workspace = true tempfile.workspace = true testcontainers.workspace = true diff --git a/linux/crates/ssh/src/argv.rs b/linux/crates/ssh/src/argv.rs index 9467778565..c73dbec022 100644 --- a/linux/crates/ssh/src/argv.rs +++ b/linux/crates/ssh/src/argv.rs @@ -46,6 +46,7 @@ pub(crate) fn master_args( liveness: &LivenessPolicy, host_keys: Option, config_file: Option<&Path>, + identities_only: bool, ) -> Vec { let mut args: Vec = Vec::new(); if let Some(config_file) = config_file { @@ -84,6 +85,10 @@ pub(crate) fn master_args( SshAuth::Agent => {} SshAuth::PrivateKey { path, .. } => { if let Some(path) = path { + // Otherwise ssh offers the agent's other keys first and can run out the server's MaxAuthTries. + if identities_only { + push_option(&mut args, "IdentitiesOnly=yes".into()); + } args.push("-i".into()); args.push(path.as_os_str().to_owned()); } @@ -183,6 +188,7 @@ mod tests { &LivenessPolicy::DESKTOP, None, None, + false, ); assert_eq!( strings(&args), @@ -260,6 +266,7 @@ mod tests { &LivenessPolicy::DESKTOP, host_keys, None, + false, ); let strict: Vec<&str> = strings(&args) .into_iter() @@ -279,6 +286,7 @@ mod tests { &LivenessPolicy::DESKTOP, None, None, + false, )) .join(" "); assert!(!without.contains(" -p ") && !without.contains(" -l ")); @@ -288,6 +296,7 @@ mod tests { &LivenessPolicy::DESKTOP, None, None, + false, ); assert_eq!( strings(&with[with.len() - 6..]), @@ -305,6 +314,7 @@ mod tests { &LivenessPolicy::DESKTOP, None, None, + false, ); let args = strings(&args); assert_eq!(args.iter().filter(|arg| **arg == "-J").count(), 1); @@ -320,6 +330,7 @@ mod tests { &LivenessPolicy::DESKTOP, None, Some(Path::new("/run/user/1000/m/ssh_config")), + false, ); assert_eq!(strings(&args)[..3], ["-F", "/run/user/1000/m/ssh_config", "-M"]); @@ -328,35 +339,39 @@ mod tests { #[test] fn private_key_path_adds_minus_i() { - let auth = SshAuth::PrivateKey { - path: Some(PathBuf::from("/home/deploy/.ssh/id_ed25519")), - passphrase: None, + let args = |path: Option<&str>, identities_only: bool| { + let auth = SshAuth::PrivateKey { + path: path.map(PathBuf::from), + passphrase: None, + }; + let args = master_args( + &config(None, None, auth), + Path::new("/tmp/control"), + &LivenessPolicy::DESKTOP, + None, + None, + identities_only, + ); + strings(&args).join(" ") }; - let args = master_args( - &config(None, None, auth), - Path::new("/tmp/control"), - &LivenessPolicy::DESKTOP, - None, - None, + + let pinned = args(Some("/home/deploy/.ssh/id_ed25519"), true); + let unpinned = args(Some("/home/deploy/.ssh/id_ed25519"), false); + let without_path = args(None, true); + + assert!( + pinned.contains(" -o IdentitiesOnly=yes -i /home/deploy/.ssh/id_ed25519 -- db-bastion"), + "{pinned}" ); - let joined = strings(&args).join(" "); assert!( - joined.contains(" -i /home/deploy/.ssh/id_ed25519 -- db-bastion"), - "{joined}" + unpinned.contains(" -i /home/deploy/.ssh/id_ed25519 -- db-bastion"), + "{unpinned}" ); - - let without_path = SshAuth::PrivateKey { - path: None, - passphrase: None, - }; - let args = master_args( - &config(None, None, without_path), - Path::new("/tmp/control"), - &LivenessPolicy::DESKTOP, - None, - None, + assert!(!unpinned.contains("IdentitiesOnly"), "{unpinned}"); + assert!( + !without_path.contains(" -i ") && !without_path.contains("IdentitiesOnly"), + "{without_path}" ); - assert!(!strings(&args).contains(&"-i")); } #[test] @@ -381,6 +396,7 @@ mod tests { &LivenessPolicy::DESKTOP, None, None, + false, ); let preferred = strings(&args) .into_iter() diff --git a/linux/crates/ssh/src/identity_pin.rs b/linux/crates/ssh/src/identity_pin.rs new file mode 100644 index 0000000000..60b2a314c4 --- /dev/null +++ b/linux/crates/ssh/src/identity_pin.rs @@ -0,0 +1,168 @@ +use std::ffi::OsString; +use std::path::{Path, PathBuf}; +use std::process::Stdio; +use std::time::Duration; + +use tokio::process::Command; + +const PROBE_LIMIT: Duration = Duration::from_secs(5); + +/// Whether the master gets `IdentitiesOnly=yes` for the key file `key`. +/// ssh puts a key file the agent holds first, with the agent's +/// certificates for it after, so only a key the agent lacks needs the +/// agent's other keys held back. ssh matches the file to the agent by +/// its public half, which an encrypted PEM file without `.pub` hides. +pub(crate) async fn pin_key_file(ssh_keygen: &Path, key: &Path, ssh_g_output: Option<&str>) -> bool { + let Some(fingerprint) = fingerprint(ssh_keygen, key).await else { + return false; + }; + let Some(socket) = agent_socket(ssh_g_output, std::env::var_os("SSH_AUTH_SOCK")) else { + return true; + }; + // ssh-add ships beside ssh-keygen in every OpenSSH client package. + match agent_fingerprints(&ssh_keygen.with_file_name("ssh-add"), &socket).await { + Some(held) => !held.contains(&fingerprint), + None => true, + } +} + +/// The SHA256 fingerprint `ssh-keygen -l` prints when it can read the +/// public half without the passphrase, as ssh must. +async fn fingerprint(ssh_keygen: &Path, key: &Path) -> Option { + let output = Command::new(ssh_keygen) + .arg("-l") + .arg("-f") + .arg(key) + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .kill_on_drop(true) + .output(); + // A FIFO or a stalled mount never answers; the key then goes to ssh as before. + let output = tokio::time::timeout(PROBE_LIMIT, output).await.ok()?.ok()?; + if !output.status.success() { + return None; + } + second_field(&String::from_utf8_lossy(&output.stdout)) +} + +/// The agent ssh would ask, from the `identityagent` line `ssh -G` +/// prints with its tokens expanded, or `SSH_AUTH_SOCK` when it prints +/// none. +fn agent_socket(ssh_g_output: Option<&str>, environment: Option) -> Option { + let configured = ssh_g_output + .and_then(|output| output.lines().find_map(|line| line.strip_prefix("identityagent "))) + .map(str::trim); + let socket = match configured { + Some("none") => return None, + Some("SSH_AUTH_SOCK") | None => environment, + // ssh_config(5): a value starting with `$` names the variable that holds the socket. + Some(value) => match value.strip_prefix('$') { + Some(variable) if !variable.starts_with('{') => std::env::var_os(variable), + _ => Some(OsString::from(value)), + }, + }; + socket.filter(|socket| !socket.is_empty()).map(PathBuf::from) +} + +/// What `ssh-add -l` lists, or `None` when the agent could not be asked. +async fn agent_fingerprints(ssh_add: &Path, socket: &Path) -> Option> { + let output = Command::new(ssh_add) + .arg("-l") + .env("SSH_AUTH_SOCK", socket) + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .kill_on_drop(true) + .output(); + let output = tokio::time::timeout(PROBE_LIMIT, output).await.ok()?.ok()?; + match output.status.code() { + Some(0) => Some( + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(second_field) + .collect(), + ), + // An agent with no keys exits 1. + Some(1) => Some(Vec::new()), + _ => None, + } +} + +fn second_field(line: &str) -> Option { + line.split_whitespace().nth(1).map(str::to_owned) +} + +#[cfg(test)] +mod tests { + use tablepro_test_fixtures::{SshAgent, SshKeyPair, SshKeyType}; + + use super::*; + + #[test] + fn the_agent_is_the_one_ssh_g_names_or_ssh_auth_sock() { + let environment = || Some(OsString::from("/run/user/1000/agent.sock")); + let from_env = Some(PathBuf::from("/run/user/1000/agent.sock")); + + assert_eq!(agent_socket(None, environment()), from_env); + assert_eq!(agent_socket(Some("user deploy\nport 22\n"), environment()), from_env); + assert_eq!( + agent_socket(Some("identityagent SSH_AUTH_SOCK\n"), environment()), + from_env + ); + assert_eq!( + agent_socket(Some("identityagent /home/u/.1password/agent.sock\n"), environment()), + Some(PathBuf::from("/home/u/.1password/agent.sock")) + ); + assert_eq!(agent_socket(Some("identityagent none\n"), environment()), None); + assert_eq!(agent_socket(None, Some(OsString::new())), None); + assert_eq!(agent_socket(None, None), None); + } + + /// ssh reads an OpenSSH key's public half from the file itself and a + /// PEM key's from its `.pub`; an encrypted PEM key alone hides it. + #[tokio::test] + async fn a_fingerprint_needs_the_public_half_in_the_clear() { + let dir = tempfile::tempdir().unwrap(); + let generate = |name: &str, format: &[&str]| { + let path = dir.path().join(name); + let status = std::process::Command::new("ssh-keygen") + .args(["-q", "-t", "rsa", "-b", "2048", "-N", "secret phrase", "-f"]) + .arg(&path) + .args(format) + .status() + .unwrap(); + assert!(status.success()); + path + }; + let openssh = generate("openssh", &[]); + let pem = generate("pem", &["-m", "PEM"]); + std::fs::remove_file(openssh.with_extension("pub")).unwrap(); + let keygen = Path::new("ssh-keygen"); + + assert!( + fingerprint(keygen, &openssh) + .await + .is_some_and(|print| print.starts_with("SHA256:")) + ); + assert!(fingerprint(keygen, &pem).await.is_some()); + std::fs::remove_file(pem.with_extension("pub")).unwrap(); + assert_eq!(fingerprint(keygen, &pem).await, None); + assert_eq!(fingerprint(keygen, &dir.path().join("missing")).await, None); + } + + #[tokio::test] + async fn a_key_file_is_pinned_unless_the_agent_already_holds_it() { + let agent = SshAgent::start().await.unwrap(); + let held = SshKeyPair::generate("tablepro-held").await.unwrap(); + let other = SshKeyPair::generate_with(SshKeyType::EcdsaP256, None, "tablepro-other") + .await + .unwrap(); + agent.add(&held).await.unwrap(); + let ssh_g = format!("identityagent {}\n", agent.socket().display()); + let keygen = Path::new("ssh-keygen"); + + assert!(!pin_key_file(keygen, held.private_key(), Some(&ssh_g)).await); + assert!(pin_key_file(keygen, other.private_key(), Some(&ssh_g)).await); + assert!(pin_key_file(keygen, held.private_key(), Some("identityagent none\n")).await); + assert!(!pin_key_file(keygen, &held.private_key().with_extension("missing"), Some(&ssh_g)).await); + } +} diff --git a/linux/crates/ssh/src/lib.rs b/linux/crates/ssh/src/lib.rs index 1659c3e376..62493bad5e 100644 --- a/linux/crates/ssh/src/lib.rs +++ b/linux/crates/ssh/src/lib.rs @@ -7,6 +7,7 @@ mod destination; mod forward; mod hop_config; mod host_key_checking; +mod identity_pin; mod known_hosts; mod password_owner; mod prompt; diff --git a/linux/crates/ssh/src/session.rs b/linux/crates/ssh/src/session.rs index f45b410499..2074e33db7 100644 --- a/linux/crates/ssh/src/session.rs +++ b/linux/crates/ssh/src/session.rs @@ -20,6 +20,7 @@ use crate::argv::{ControlOp, master_args}; use crate::askpass_bridge::AskpassBridge; use crate::hop_config; use crate::host_key_checking::HostKeyChecking; +use crate::identity_pin::pin_key_file; use crate::known_hosts::{known_hosts_lookup, spawn_failure}; use crate::password_owner::password_owner; use crate::route::{JumpHop, Route, lookup, trace_route}; @@ -71,6 +72,7 @@ struct MasterPlan { strict_hosts: Vec, route: Route, master_config: Option, + identities_only: bool, } impl MasterPlan { @@ -93,6 +95,16 @@ impl MasterPlan { SshAuth::Agent | SshAuth::PrivateKey { .. } | SshAuth::KeyboardInteractive => None, }; let host_keys = resolved.as_deref().and_then(HostKeyChecking::from_ssh_g); + let identities_only = match &config.auth { + SshAuth::PrivateKey { path: Some(key), .. } => tokio::select! { + pin = pin_key_file(&services.ssh_keygen_program, key, resolved.as_deref()) => pin, + () = cancel.cancelled() => return Err(SshFailure::InteractionCancelled.into()), + }, + SshAuth::PrivateKey { path: None, .. } + | SshAuth::Agent + | SshAuth::Password { .. } + | SshAuth::KeyboardInteractive => false, + }; let route = tokio::select! { route = trace_route(&services.ssh_program, config_file, resolved.as_deref()) => route, () = cancel.cancelled() => return Err(SshFailure::InteractionCancelled.into()), @@ -106,6 +118,7 @@ impl MasterPlan { strict_hosts: strict_hosts(resolved.as_deref(), &route.hops), route, master_config, + identities_only, }) } @@ -124,6 +137,7 @@ impl MasterPlan { liveness, self.host_keys, self.master_config.as_deref(), + self.identities_only, )) .env("SSH_ASKPASS", &services.askpass_program) .env("SSH_ASKPASS_REQUIRE", "force") diff --git a/linux/crates/ssh/src/stderr_classify.rs b/linux/crates/ssh/src/stderr_classify.rs index fd996e7ca1..5f673a5b50 100644 --- a/linux/crates/ssh/src/stderr_classify.rs +++ b/linux/crates/ssh/src/stderr_classify.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use tablepro_core::{NetworkEndpoint, SshFailure, TimeoutPhase, TransportError}; +use tablepro_core::{NetworkEndpoint, SshAlgorithmKind, SshFailure, TimeoutPhase, TransportError}; const PROTOCOL_DETAIL_LIMIT: usize = 4096; @@ -26,6 +26,8 @@ pub(crate) struct ClassifyContext<'a> { pub(crate) fn classify(status: Option, stderr: &str, context: &ClassifyContext<'_>) -> TransportError { let failure = host_key_failure(stderr, context) + .or_else(|| negotiation_failure(stderr, context)) + .or_else(|| attempts_exceeded(stderr, context)) .or_else(|| authentication_failure(stderr)) .or_else(|| forwarding_failure(stderr, context)) .or_else(|| configuration_failure(stderr)); @@ -118,6 +120,62 @@ fn unknown_host_key(host: &str, algorithm: &str, fingerprint: &str, trust_allowe } } +// A server's login banner reaches stderr as the server wrote it, so only a +// whole line in ssh's own form counts. +fn negotiation_failure(stderr: &str, context: &ClassifyContext<'_>) -> Option { + stderr.lines().find_map(|line| { + let (remote, rest) = line + .trim() + .strip_prefix("Unable to negotiate with ")? + .split_once(": ")?; + let (reason, offer) = rest.split_once(". Their offer: ")?; + // ssh_err()'s words for the five SSH_ERR_NO_*_ALG_MATCH codes. + let kind = match reason { + "no matching key exchange method found" => SshAlgorithmKind::KeyExchange, + "no matching host key type found" => SshAlgorithmKind::HostKey, + "no matching cipher found" => SshAlgorithmKind::Cipher, + "no matching MAC found" => SshAlgorithmKind::Mac, + "no matching compression method found" => SshAlgorithmKind::Compression, + _ => return None, + }; + // ext-info-* (RFC 8308) and kex-strict-* ride in the key exchange list + // as signals, not methods a client could pick. + let server_offer = offer + .split(',') + .map(str::trim) + .filter(|name| !name.is_empty() && !name.starts_with("ext-info-") && !name.starts_with("kex-strict-")) + .map(str::to_owned) + .collect(); + Some(SshFailure::NoCommonAlgorithm { + host: remote_host(remote, context)?, + kind, + server_offer, + }) + }) +} + +fn attempts_exceeded(stderr: &str, context: &ClassifyContext<'_>) -> Option { + stderr.lines().find_map(|line| { + let record = line + .trim() + .strip_prefix("Received disconnect from ")? + .strip_suffix(": Too many authentication failures")?; + let (remote, _code) = record.rsplit_once(':')?; + Some(SshFailure::AuthenticationAttemptsExceeded { + host: remote_host(remote, context)?, + }) + }) +} + +fn remote_host(remote: &str, context: &ClassifyContext<'_>) -> Option { + let (host, _port) = remote.split_once(" port ")?; + // ssh names a hop behind a jump host UNKNOWN, since it sees only the proxy's pipe. + Some(match host.trim() { + "" | "UNKNOWN" => context.host.to_owned(), + host => host.to_owned(), + }) +} + fn authentication_failure(stderr: &str) -> Option { let line = line_containing(stderr, ": Permission denied (")?; let (user_host, methods) = line.split_once(": Permission denied (")?; @@ -346,11 +404,123 @@ mod tests { )); } + #[test] + fn stderr_classification_fixture_too_many_authentication_failures() { + assert_eq!( + fixture(include_str!( + "../tests/fixtures/stderr/too_many_authentication_failures.txt" + )), + TransportError::Ssh(SshFailure::AuthenticationAttemptsExceeded { + host: "127.0.0.1".to_owned(), + }) + ); + } + + /// The server's offer keeps its methods and drops the extension + /// signals that ride in the same list. + #[test] + fn stderr_classification_fixture_no_matching_key_exchange() { + assert_eq!( + fixture(include_str!("../tests/fixtures/stderr/no_matching_key_exchange.txt")), + TransportError::Ssh(SshFailure::NoCommonAlgorithm { + host: "127.0.0.1".to_owned(), + kind: SshAlgorithmKind::KeyExchange, + server_offer: vec!["mlkem768x25519-sha256".to_owned(), "sntrup761x25519-sha512".to_owned()], + }) + ); + } + + #[test] + fn every_list_ssh_can_fail_to_negotiate_is_named() { + let cases = [ + ("key exchange method", SshAlgorithmKind::KeyExchange), + ("host key type", SshAlgorithmKind::HostKey), + ("cipher", SshAlgorithmKind::Cipher), + ("MAC", SshAlgorithmKind::Mac), + ("compression method", SshAlgorithmKind::Compression), + ]; + for (words, kind) in cases { + let stderr = + format!("Unable to negotiate with 192.0.2.7 port 22: no matching {words} found. Their offer: a,b\n"); + assert_eq!( + fixture(&stderr), + TransportError::Ssh(SshFailure::NoCommonAlgorithm { + host: "192.0.2.7".to_owned(), + kind, + server_offer: vec!["a".to_owned(), "b".to_owned()], + }), + "{words}" + ); + } + assert!(matches!( + fixture(include_str!("../tests/fixtures/stderr/no_matching_cipher.txt")), + TransportError::Ssh(SshFailure::NoCommonAlgorithm { kind: SshAlgorithmKind::Cipher, server_offer, .. }) + if server_offer.len() == 6 + )); + } + + /// Behind a jump host ssh knows the destination only as the proxy's + /// pipe, so the configured host names it. + #[test] + fn a_hop_behind_a_jump_host_is_named_by_its_configured_host() { + assert_eq!( + fixture( + "Unable to negotiate with UNKNOWN port 65535: no matching host key type found. Their offer: ssh-rsa\n" + ), + TransportError::Ssh(SshFailure::NoCommonAlgorithm { + host: "bastion".to_owned(), + kind: SshAlgorithmKind::HostKey, + server_offer: vec!["ssh-rsa".to_owned()], + }) + ); + assert_eq!( + fixture("Received disconnect from UNKNOWN port 65535:2: Too many authentication failures\n"), + TransportError::Ssh(SshFailure::AuthenticationAttemptsExceeded { + host: "bastion".to_owned(), + }) + ); + assert_eq!( + fixture("Received disconnect from fd00::5 port 22:2: Too many authentication failures\n"), + TransportError::Ssh(SshFailure::AuthenticationAttemptsExceeded { + host: "fd00::5".to_owned(), + }) + ); + } + + /// A login banner is printed to stderr as the server wrote it, so + /// its words alone classify nothing. + #[test] + fn a_banner_with_ssh_s_words_is_not_ssh_s_error() { + let banner = "Too many authentication failures will lock your account.\n\ + Unable to negotiate with support: no matching cipher found. Their offer: call us\n"; + + assert_eq!( + fixture(&format!("{banner}Connection reset by 192.0.2.7 port 22\n")), + TransportError::Ssh(SshFailure::Protocol { + status: Some(255), + detail: format!("{banner}Connection reset by 192.0.2.7 port 22") + .trim() + .to_owned(), + }) + ); + assert_eq!( + fixture(&format!( + "{banner}{}", + include_str!("../tests/fixtures/stderr/too_many_authentication_failures.txt") + )), + TransportError::Ssh(SshFailure::AuthenticationAttemptsExceeded { + host: "127.0.0.1".to_owned(), + }) + ); + } + #[test] fn crlf_line_endings_classify_the_same() { for stderr in [ include_str!("../tests/fixtures/stderr/auth_rejected.txt"), include_str!("../tests/fixtures/stderr/host_key_changed.txt"), + include_str!("../tests/fixtures/stderr/too_many_authentication_failures.txt"), + include_str!("../tests/fixtures/stderr/no_matching_key_exchange.txt"), ] { assert_eq!(fixture(&stderr.replace('\n', "\r\n")), fixture(stderr)); } diff --git a/linux/crates/ssh/tests/fixtures/stderr/no_matching_cipher.txt b/linux/crates/ssh/tests/fixtures/stderr/no_matching_cipher.txt new file mode 100644 index 0000000000..aead7dfd5c --- /dev/null +++ b/linux/crates/ssh/tests/fixtures/stderr/no_matching_cipher.txt @@ -0,0 +1 @@ +Unable to negotiate with 127.0.0.1 port 2222: no matching cipher found. Their offer: chacha20-poly1305@openssh.com,aes128-gcm@openssh.com,aes256-gcm@openssh.com,aes128-ctr,aes192-ctr,aes256-ctr diff --git a/linux/crates/ssh/tests/fixtures/stderr/no_matching_key_exchange.txt b/linux/crates/ssh/tests/fixtures/stderr/no_matching_key_exchange.txt new file mode 100644 index 0000000000..d4af23e231 --- /dev/null +++ b/linux/crates/ssh/tests/fixtures/stderr/no_matching_key_exchange.txt @@ -0,0 +1 @@ +Unable to negotiate with 127.0.0.1 port 2223: no matching key exchange method found. Their offer: mlkem768x25519-sha256,sntrup761x25519-sha512,ext-info-s,kex-strict-s-v00@openssh.com diff --git a/linux/crates/ssh/tests/fixtures/stderr/too_many_authentication_failures.txt b/linux/crates/ssh/tests/fixtures/stderr/too_many_authentication_failures.txt new file mode 100644 index 0000000000..3a4a1d3c85 --- /dev/null +++ b/linux/crates/ssh/tests/fixtures/stderr/too_many_authentication_failures.txt @@ -0,0 +1,3 @@ +Warning: Permanently added '[127.0.0.1]:2222' (ED25519) to the list of known hosts. +Received disconnect from 127.0.0.1 port 2222:2: Too many authentication failures +Disconnected from 127.0.0.1 port 2222 diff --git a/linux/crates/ssh/tests/openssh_matrix.rs b/linux/crates/ssh/tests/openssh_matrix.rs new file mode 100644 index 0000000000..96360c9cd2 --- /dev/null +++ b/linux/crates/ssh/tests/openssh_matrix.rs @@ -0,0 +1,444 @@ +//! The tunnel against the OpenSSH release `TABLEPRO_TEST_OPENSSH` picks, across client keys, agents, key exchange +//! and a server that goes away (#3662). + +use std::collections::HashSet; +use std::error::Error; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::Duration; + +use secrecy::SecretString; +use tablepro_core::credentials::CredentialInteraction; +use tablepro_core::{LivenessPolicy, NetworkEndpoint, SshAlgorithmKind, SshFailure, TransportError}; +use tablepro_ssh::{SshAuth, SshConfig, SshDestination, SshRuntimeCell, SshServices, SshSession}; +use tablepro_test_fixtures::{OpenSshFixture, SshAgent, SshAuthVariant, SshKeyPair, SshKeyType}; +use tablepro_test_support::FakePrompter; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, BufReader}; +use tokio_util::sync::CancellationToken; + +type TestResult = Result>; + +const PASSPHRASE: &str = "correct horse battery"; +const WAIT: Duration = Duration::from_secs(15); +const POST_QUANTUM_KEX: [&str; 2] = ["mlkem768x25519-sha256", "sntrup761x25519-sha512"]; +/// sshd's default, set on the server so the decoy count follows from it. +const MAX_AUTH_TRIES: usize = 6; + +/// The system ssh under a private ssh_config whose known_hosts already +/// trusts the server, so a test that sees a prompt has found a bug. +struct Client { + _dir: tempfile::TempDir, + services: SshServices, +} + +impl Client { + async fn new(server: &OpenSshFixture) -> TestResult { + Self::with(server, &["IdentityAgent none".to_owned()]).await + } + + async fn with_agent(server: &OpenSshFixture, agent: &SshAgent) -> TestResult { + Self::with(server, &[format!("IdentityAgent {}", agent.socket().display())]).await + } + + /// `options` go under `Host *` after the known_hosts lines. + async fn with(server: &OpenSshFixture, options: &[String]) -> TestResult { + let dir = tempfile::tempdir()?; + let known_hosts = dir.path().join("known_hosts"); + let host_key = server.host_public_key().await?; + std::fs::write( + &known_hosts, + format!("[{}]:{} {host_key}\n", server.host(), server.port()), + )?; + let mut config = format!( + "Host *\n UserKnownHostsFile {}\n GlobalKnownHostsFile /dev/null\n", + known_hosts.display() + ); + for option in options { + config.push_str(&format!(" {option}\n")); + } + let config_file = dir.path().join("ssh_config"); + std::fs::write(&config_file, config)?; + let wrapper = dir.path().join("ssh"); + tablepro_test_support::write_executable( + &wrapper, + format!("#!/bin/sh\nexec ssh -F '{}' \"$@\"\n", config_file.display()).as_bytes(), + 0o755, + )?; + let runtime = dir.path().join("runtime").join("app").join("app.tablepro.TablePro"); + let services = SshServices { + runtime: Arc::new(SshRuntimeCell::new(runtime)), + ssh_program: wrapper, + ssh_keygen_program: PathBuf::from("ssh-keygen"), + askpass_program: PathBuf::from(env!("CARGO_BIN_EXE_tablepro-askpass")), + config_file: None, + }; + Ok(Self { _dir: dir, services }) + } + + /// A session to `server` that may not ask the user anything. + async fn connect(&self, server: &OpenSshFixture, auth: SshAuth) -> Result, TransportError> { + let destination = SshDestination::new( + server.host(), + Some(server.port()), + Some(server.password_credentials().username), + ) + .map_err(|problem| TransportError::Ssh(problem.into()))?; + let config = SshConfig { + destination, + jump_hosts: Vec::new(), + auth, + }; + let prompter = Arc::new(FakePrompter::new(Vec::new())); + let session = SshSession::connect( + &config, + &self.services, + &LivenessPolicy::DESKTOP, + CredentialInteraction::Attended(prompter.clone()), + CancellationToken::new(), + ) + .await; + let prompts = prompter.requests(); + assert!(prompts.is_empty(), "ssh asked the user: {prompts:?}"); + session + } +} + +fn key_file(key: &SshKeyPair, passphrase: Option<&str>) -> SshAuth { + SshAuth::PrivateKey { + path: Some(key.private_key().to_owned()), + passphrase: passphrase.map(SecretString::from), + } +} + +/// The server's identification line, read through a forward to sshd's +/// own port, so it proves the tunnel carries data to the release under +/// test. +async fn banner_through(session: &Arc) -> TestResult { + let target = NetworkEndpoint::new("127.0.0.1", OpenSshFixture::CONTAINER_PORT)?; + let transport = session.forward(target, &LivenessPolicy::DESKTOP).await?; + let stream = transport.open().await?; + let mut banner = String::new(); + tokio::time::timeout(WAIT, BufReader::new(stream).read_line(&mut banner)).await??; + Ok(banner.trim_end().to_owned()) +} + +/// Connect, read the banner through the tunnel and close, as one cell of +/// the matrix. +async fn tunnel(client: &Client, server: &OpenSshFixture, auth: SshAuth) -> TestResult { + let session = client.connect(server, auth).await?; + let banner = banner_through(&session).await?; + let expected = server.version().banner_prefix(); + if !banner.starts_with(&expected) { + return Err(format!("the tunnel reached {banner:?}, not {expected}").into()); + } + session.shutdown(Duration::from_secs(5)).await; + Ok(()) +} + +async fn decoys() -> TestResult> { + let mut keys = Vec::new(); + for index in 0..MAX_AUTH_TRIES { + keys.push(SshKeyPair::generate(&format!("tablepro-decoy-{index}")).await?); + } + Ok(keys) +} + +fn process_alive(pid: u32) -> bool { + Path::new(&format!("/proc/{pid}")).exists() +} + +#[tokio::test] +#[ignore = "requires docker"] +async fn every_client_key_type_opens_the_tunnel() -> TestResult { + let mut keys = Vec::new(); + for key_type in SshKeyType::ALL { + for passphrase in [None, Some(PASSPHRASE)] { + keys.push(SshKeyPair::generate_with(key_type, passphrase, "tablepro-matrix").await?); + } + } + let server = OpenSshFixture::start(SshAuthVariant::PublicKeys(keys)).await?; + let SshAuthVariant::PublicKeys(keys) = server.variant() else { + return Err("the fixture lost its keys".into()); + }; + let client = Client::new(&server).await?; + + let mut failures = Vec::new(); + for key in keys { + if let Err(error) = tunnel(&client, &server, key_file(key, key.passphrase())).await { + failures.push(format!( + "{:?}, passphrase {}: {error}", + key.key_type(), + key.passphrase().is_some() + )); + } + } + + assert!(failures.is_empty(), "{failures:#?}"); + Ok(()) +} + +/// sshd sends only the algorithms it accepts as server-sig-algs, and the +/// client signs with the one it names. +#[tokio::test] +#[ignore = "requires docker"] +async fn an_rsa_key_signs_with_the_sha2_algorithm_the_server_accepts() -> TestResult { + for algorithm in ["rsa-sha2-256", "rsa-sha2-512"] { + let key = SshKeyPair::generate_with(SshKeyType::Rsa, None, "tablepro-rsa").await?; + let private_key = key.private_key().to_owned(); + let accepted = format!("PubkeyAcceptedAlgorithms {algorithm}"); + let server = OpenSshFixture::start_configured(SshAuthVariant::PublicKey(key), &[&accepted]).await?; + let client = Client::new(&server).await?; + let auth = SshAuth::PrivateKey { + path: Some(private_key), + passphrase: None, + }; + + tunnel(&client, &server, auth) + .await + .map_err(|error| format!("{algorithm}: {error}"))?; + } + Ok(()) +} + +#[tokio::test] +#[ignore = "requires docker"] +async fn an_agent_key_opens_the_tunnel() -> TestResult { + let agent = SshAgent::start().await?; + let key = SshKeyPair::generate_with(SshKeyType::EcdsaP256, None, "tablepro-agent").await?; + agent.add(&key).await?; + let server = OpenSshFixture::start(SshAuthVariant::PublicKey(key)).await?; + let client = Client::with_agent(&server, &agent).await?; + + tunnel(&client, &server, SshAuth::Agent).await +} + +/// A key file whose key the agent holds signs through the agent, so a +/// passphrase-protected file never asks for its passphrase (#2583). +#[tokio::test] +#[ignore = "requires docker"] +async fn a_key_file_the_agent_holds_asks_for_no_passphrase() -> TestResult { + let agent = SshAgent::start().await?; + let key = SshKeyPair::generate_with(SshKeyType::Ed25519, Some(PASSPHRASE), "tablepro-agent").await?; + agent.add(&key).await?; + let private_key = key.private_key().to_owned(); + let server = OpenSshFixture::start(SshAuthVariant::PublicKey(key)).await?; + let client = Client::with_agent(&server, &agent).await?; + let auth = SshAuth::PrivateKey { + path: Some(private_key), + passphrase: None, + }; + + tunnel(&client, &server, auth).await +} + +/// An encrypted PEM file with no `.pub` hides its public half, so ssh +/// cannot match it to the agent's copy. The agent still signs, rather +/// than the file asking for its passphrase. +#[tokio::test] +#[ignore = "requires docker"] +async fn an_encrypted_pem_key_the_agent_holds_asks_for_no_passphrase() -> TestResult { + let agent = SshAgent::start().await?; + let key = SshKeyPair::generate_with(SshKeyType::Rsa, Some(PASSPHRASE), "tablepro-pem").await?; + agent.add(&key).await?; + let private_key = key.private_key().to_owned(); + let rewritten = std::process::Command::new("ssh-keygen") + .args(["-q", "-p", "-m", "PEM", "-P", PASSPHRASE, "-N", PASSPHRASE, "-f"]) + .arg(&private_key) + .status()?; + assert!(rewritten.success()); + std::fs::remove_file(private_key.with_extension("pub"))?; + let server = OpenSshFixture::start(SshAuthVariant::PublicKey(key)).await?; + let client = Client::with_agent(&server, &agent).await?; + let auth = SshAuth::PrivateKey { + path: Some(private_key), + passphrase: None, + }; + + tunnel(&client, &server, auth).await +} + +/// The agent holds the chosen key and a certificate for it that is on +/// no disk, and the server takes only the certificate. ssh offers a key +/// file the agent holds first anyway, so the certificate stays in play. +#[tokio::test] +#[ignore = "requires docker"] +async fn an_agent_certificate_for_the_chosen_key_file_is_still_offered() -> TestResult { + let authority = SshKeyPair::generate("tablepro-user-ca").await?; + let server = OpenSshFixture::start(SshAuthVariant::UserCertificateAuthority(authority)).await?; + let SshAuthVariant::UserCertificateAuthority(authority) = server.variant() else { + return Err("the fixture lost its authority".into()); + }; + let key = SshKeyPair::generate("tablepro-certified").await?; + let signed = std::process::Command::new("ssh-keygen") + .args(["-q", "-s"]) + .arg(authority.private_key()) + .args(["-I", "tablepro-certified", "-n"]) + .arg(server.password_credentials().username) + .arg(key.private_key().with_extension("pub")) + .status()?; + assert!(signed.success()); + let agent = SshAgent::start().await?; + agent.add(&key).await?; + std::fs::remove_file(format!("{}-cert.pub", key.private_key().display()))?; + assert_eq!( + agent.identities().await?.len(), + 2, + "the agent holds the key and its certificate" + ); + let client = Client::with_agent(&server, &agent).await?; + + tunnel(&client, &server, key_file(&key, None)).await +} + +/// The #2601 case: an agent holding more keys than the server's +/// MaxAuthTries, none of them the one the connection names. +#[tokio::test] +#[ignore = "requires docker"] +async fn a_chosen_key_file_is_offered_before_an_agent_full_of_other_keys() -> TestResult { + let agent = SshAgent::start().await?; + for decoy in decoys().await? { + agent.add(&decoy).await?; + } + let key = SshKeyPair::generate("tablepro-chosen").await?; + let private_key = key.private_key().to_owned(); + let max_auth_tries = format!("MaxAuthTries {MAX_AUTH_TRIES}"); + let server = OpenSshFixture::start_configured(SshAuthVariant::PublicKey(key), &[&max_auth_tries]).await?; + let client = Client::with_agent(&server, &agent).await?; + let auth = SshAuth::PrivateKey { + path: Some(private_key), + passphrase: None, + }; + + tunnel(&client, &server, auth).await +} + +/// In agent mode the agent's order is ssh's, so a key behind +/// MaxAuthTries others is never reached; the error says why. +#[tokio::test] +#[ignore = "requires docker"] +async fn an_agent_with_more_keys_than_max_auth_tries_names_the_cause() -> TestResult { + let agent = SshAgent::start().await?; + for decoy in decoys().await? { + agent.add(&decoy).await?; + } + let key = SshKeyPair::generate("tablepro-last").await?; + agent.add(&key).await?; + let max_auth_tries = format!("MaxAuthTries {MAX_AUTH_TRIES}"); + let server = OpenSshFixture::start_configured(SshAuthVariant::PublicKey(key), &[&max_auth_tries]).await?; + let client = Client::with_agent(&server, &agent).await?; + + let session = client.connect(&server, SshAuth::Agent).await; + + assert!( + matches!( + session, + Err(TransportError::Ssh(SshFailure::AuthenticationAttemptsExceeded { .. })) + ), + "{session:?}" + ); + Ok(()) +} + +/// A server limited to the post-quantum key exchanges. A client that has +/// one connects; one that has neither, as OpenSSH before 9.9 and a client +/// configured without them, gets an error that names the key exchange. +#[tokio::test] +#[ignore = "requires docker"] +async fn a_post_quantum_only_server_connects_or_names_the_key_exchange() -> TestResult { + let key = SshKeyPair::generate("tablepro-kex").await?; + let private_key = key.private_key().to_owned(); + let kex = format!("KexAlgorithms {}", POST_QUANTUM_KEX.join(",")); + let server = OpenSshFixture::start_configured(SshAuthVariant::PublicKey(key), &[&kex]).await?; + let auth = || SshAuth::PrivateKey { + path: Some(private_key.clone()), + passphrase: None, + }; + let named = |session: &Result, TransportError>| { + matches!( + session, + Err(TransportError::Ssh(SshFailure::NoCommonAlgorithm { + kind: SshAlgorithmKind::KeyExchange, + server_offer, + .. + })) if server_offer == &POST_QUANTUM_KEX + ) + }; + let listed = tokio::process::Command::new("ssh").args(["-Q", "kex"]).output().await?; + let client_kex: HashSet = String::from_utf8_lossy(&listed.stdout) + .lines() + .map(str::to_owned) + .collect(); + + let client = Client::new(&server).await?; + if POST_QUANTUM_KEX.iter().any(|name| client_kex.contains(*name)) { + tunnel(&client, &server, auth()).await?; + } else { + let session = client.connect(&server, auth()).await; + assert!(named(&session), "{session:?}"); + } + let classical = Client::with( + &server, + &[ + "IdentityAgent none".to_owned(), + "KexAlgorithms curve25519-sha256".to_owned(), + ], + ) + .await?; + let session = classical.connect(&server, auth()).await; + assert!(named(&session), "{session:?}"); + Ok(()) +} + +/// sshd stops under an open forward (#1769): the session reports closed, +/// the master exits rather than spinning, and the stream ends. +#[tokio::test] +#[ignore = "requires docker"] +async fn stopping_sshd_under_an_open_forward_closes_the_tunnel() -> TestResult { + let key = SshKeyPair::generate("tablepro-loss").await?; + let private_key = key.private_key().to_owned(); + let server = OpenSshFixture::start(SshAuthVariant::PublicKey(key)).await?; + let client = Client::new(&server).await?; + let session = client + .connect( + &server, + SshAuth::PrivateKey { + path: Some(private_key), + passphrase: None, + }, + ) + .await?; + let master = session.master_pid().ok_or("no ssh master")?; + let target = NetworkEndpoint::new("127.0.0.1", OpenSshFixture::CONTAINER_PORT)?; + let transport = session.forward(target, &LivenessPolicy::DESKTOP).await?; + let mut stream = BufReader::new(transport.open().await?); + let mut banner = String::new(); + tokio::time::timeout(WAIT, stream.read_line(&mut banner)).await??; + assert!(banner.starts_with(&server.version().banner_prefix()), "{banner:?}"); + + // Every sshd process, the listener and this session's, as when the + // service stops. + let stopped = server.exec(&["pkill", "sshd"]).await?; + assert!(stopped.succeeded(), "{}", stopped.stderr_text()); + + tokio::time::timeout(WAIT, session.closed()).await?; + assert!(session.is_closed()); + let deadline = tokio::time::Instant::now() + WAIT; + while process_alive(master) && tokio::time::Instant::now() < deadline { + tokio::time::sleep(Duration::from_millis(100)).await; + } + assert!(!process_alive(master), "the ssh master outlived its connection"); + // An end of stream and a reset both end it; a read that never returns would be the bug. + let mut rest = Vec::new(); + let _ended = tokio::time::timeout(WAIT, stream.read_to_end(&mut rest)).await?; + let again = session + .forward( + NetworkEndpoint::new("127.0.0.1", OpenSshFixture::CONTAINER_PORT)?, + &LivenessPolicy::DESKTOP, + ) + .await; + assert!( + matches!(again, Err(TransportError::Ssh(SshFailure::MasterExited { .. }))), + "{again:?}" + ); + Ok(()) +} diff --git a/linux/crates/test-fixtures/src/fixture_image.rs b/linux/crates/test-fixtures/src/fixture_image.rs index 0e6b0cdc0d..1a60dd566b 100644 --- a/linux/crates/test-fixtures/src/fixture_image.rs +++ b/linux/crates/test-fixtures/src/fixture_image.rs @@ -79,6 +79,10 @@ impl FixtureImage { name: "linuxserver/openssh-server", tag: "version-10.3_p1-r1", }; + pub const OPENSSH_9_9: Self = Self { + name: "linuxserver/openssh-server", + tag: "version-9.9_p2-r0", + }; pub fn generic(self) -> GenericImage { GenericImage::new(self.name, self.tag) diff --git a/linux/crates/test-fixtures/src/lib.rs b/linux/crates/test-fixtures/src/lib.rs index f151519581..f5e7ed6fc6 100644 --- a/linux/crates/test-fixtures/src/lib.rs +++ b/linux/crates/test-fixtures/src/lib.rs @@ -17,6 +17,7 @@ mod mysql_flavour; mod mysql_options; mod mysql_version; mod openssh_fixture; +mod openssh_version; mod pgbouncer_fixture; mod pki_algorithm; mod pki_file; @@ -28,6 +29,7 @@ mod server_version; mod ssh_agent; mod ssh_auth_variant; mod ssh_key_pair; +mod ssh_key_type; mod test_network; mod test_pki; @@ -49,6 +51,7 @@ pub use mysql_flavour::MysqlFlavour; pub use mysql_options::MysqlOptions; pub use mysql_version::MysqlVersion; pub use openssh_fixture::OpenSshFixture; +pub use openssh_version::OpenSshVersion; pub use pgbouncer_fixture::PgBouncerFixture; pub use pki_algorithm::PkiAlgorithm; pub use pki_file::PkiFile; @@ -60,5 +63,6 @@ pub use server_version::ServerVersion; pub use ssh_agent::SshAgent; pub use ssh_auth_variant::SshAuthVariant; pub use ssh_key_pair::SshKeyPair; +pub use ssh_key_type::SshKeyType; pub use test_network::TestNetwork; pub use test_pki::TestPki; diff --git a/linux/crates/test-fixtures/src/openssh_fixture.rs b/linux/crates/test-fixtures/src/openssh_fixture.rs index 4355496a16..b0f5b49c30 100644 --- a/linux/crates/test-fixtures/src/openssh_fixture.rs +++ b/linux/crates/test-fixtures/src/openssh_fixture.rs @@ -10,8 +10,8 @@ use uuid::Uuid; use crate::container_file::with_files; use crate::{ - ContainerFile, ExecOutput, FixtureCredentials, FixtureError, FixtureImage, HostCertificate, SshAuthVariant, - TestNetwork, + ContainerFile, ExecOutput, FixtureCredentials, FixtureError, HostCertificate, OpenSshVersion, ServerVersion, + SshAuthVariant, TestNetwork, }; const USERNAME: &str = "tablepro"; @@ -21,6 +21,7 @@ const BANNER_RETRY: Duration = Duration::from_millis(250); pub struct OpenSshFixture { container: ContainerAsync, + version: OpenSshVersion, variant: SshAuthVariant, host_certificate: Option, password: String, @@ -33,25 +34,43 @@ impl OpenSshFixture { pub const DROP_IN: &'static str = "/config/sshd/sshd_config.d/tablepro.conf"; pub const SSHD_CONFIG: &'static str = "/config/sshd/sshd_config"; pub const CONTAINER_PORT: u16 = 2222; + const AUTHORIZED_KEYS_DIR: &'static str = "/tablepro/authorized_keys"; + const USER_CA: &'static str = "/tablepro/user_ca.pub"; pub async fn start(variant: SshAuthVariant) -> Result { - Self::launch(variant, None).await + Self::launch(variant, &[], None).await } /// Start a server that reaches the network's other containers by /// alias. Each server gets its own alias, so a two-hop chain can put /// two of them on one network. pub async fn start_in(variant: SshAuthVariant, network: &TestNetwork) -> Result { - Self::launch(variant, Some(network)).await + Self::launch(variant, &[], Some(network)).await } - async fn launch(variant: SshAuthVariant, network: Option<&TestNetwork>) -> Result { + /// Start a server with `sshd_config` lines such as `KexAlgorithms ...` + /// added after the variant's own. + pub async fn start_configured(variant: SshAuthVariant, sshd_config: &[&str]) -> Result { + Self::launch(variant, sshd_config, None).await + } + + async fn launch( + variant: SshAuthVariant, + sshd_config: &[&str], + network: Option<&TestNetwork>, + ) -> Result { + let version = OpenSshVersion::selected()?; let password = FixtureCredentials::generate_password(); let host_certificate = match variant { SshAuthVariant::HostCertificate => Some(HostCertificate::generate().await?), - SshAuthVariant::Password | SshAuthVariant::PublicKey(_) | SshAuthVariant::KeyboardInteractive => None, + SshAuthVariant::Password + | SshAuthVariant::PublicKey(_) + | SshAuthVariant::PublicKeys(_) + | SshAuthVariant::UserCertificateAuthority(_) + | SshAuthVariant::KeyboardInteractive => None, }; - let mut request = FixtureImage::OPENSSH + let mut request = version + .image() .generic() .with_exposed_port(Self::CONTAINER_PORT.tcp()) .with_wait_for(WaitFor::message_on_stdout("[ls.io-init] done.")) @@ -61,10 +80,27 @@ impl OpenSshFixture { "PASSWORD_ACCESS", if variant.password_access() { "true" } else { "false" }, ); - if let SshAuthVariant::PublicKey(key) = &variant { - request = request.with_env_var("PUBLIC_KEY", key.public_key()); + let mut drop_in = Self::drop_in(&variant); + for line in sshd_config { + drop_in.push_str(line); + drop_in.push('\n'); + } + let mut files = vec![ContainerFile::readable(Self::DROP_IN, drop_in)]; + // The image's init appends each file in the directory to authorized_keys. + let authorized_keys = variant.authorized_keys(); + if !authorized_keys.is_empty() { + request = request.with_env_var("PUBLIC_KEY_DIR", Self::AUTHORIZED_KEYS_DIR); + } + for (index, key) in authorized_keys.into_iter().enumerate() { + let target = format!("{}/key-{index}.pub", Self::AUTHORIZED_KEYS_DIR); + files.push(ContainerFile::readable(target, format!("{key}\n"))); + } + if let SshAuthVariant::UserCertificateAuthority(authority) = &variant { + files.push(ContainerFile::readable( + Self::USER_CA, + format!("{}\n", authority.public_key()), + )); } - let mut files = vec![ContainerFile::readable(Self::DROP_IN, Self::drop_in(&variant))]; if let Some(certificate) = &host_certificate { files.extend(certificate.files()?); } @@ -81,6 +117,7 @@ impl OpenSshFixture { wait_for_banner(&host, port).await?; Ok(Self { container, + version, variant, host_certificate, password, @@ -102,11 +139,18 @@ impl OpenSshFixture { SshAuthVariant::HostCertificate => { configuration.push_str(&format!("HostCertificate {}\n", HostCertificate::CERTIFICATE)); } - SshAuthVariant::Password | SshAuthVariant::PublicKey(_) => {} + SshAuthVariant::UserCertificateAuthority(_) => { + configuration.push_str(&format!("TrustedUserCAKeys {}\n", Self::USER_CA)); + } + SshAuthVariant::Password | SshAuthVariant::PublicKey(_) | SshAuthVariant::PublicKeys(_) => {} } configuration } + pub fn version(&self) -> OpenSshVersion { + self.version + } + pub fn host(&self) -> &str { &self.host } diff --git a/linux/crates/test-fixtures/src/openssh_version.rs b/linux/crates/test-fixtures/src/openssh_version.rs new file mode 100644 index 0000000000..c049f90777 --- /dev/null +++ b/linux/crates/test-fixtures/src/openssh_version.rs @@ -0,0 +1,56 @@ +use crate::{FixtureImage, ServerVersion}; + +/// The OpenSSH release an `OpenSshFixture` runs: the current one and the +/// last 9.x, the first with the ML-KEM key exchange. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum OpenSshVersion { + V9_9, + #[default] + V10_3, +} + +impl OpenSshVersion { + /// What the server's identification string names after `SSH-2.0-`. + pub fn banner_prefix(self) -> String { + format!("SSH-2.0-OpenSSH_{}", self.name()) + } +} + +impl ServerVersion for OpenSshVersion { + const VARIABLE: &'static str = "TABLEPRO_TEST_OPENSSH"; + const ALL: &'static [Self] = &[Self::V9_9, Self::V10_3]; + + fn name(self) -> &'static str { + match self { + Self::V9_9 => "9.9", + Self::V10_3 => "10.3", + } + } + + fn image(self) -> FixtureImage { + match self { + Self::V9_9 => FixtureImage::OPENSSH_9_9, + Self::V10_3 => FixtureImage::OPENSSH, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn each_version_pins_the_image_of_its_release() { + crate::server_version::tests::check_releases::("linuxserver/openssh-server"); + for &version in OpenSshVersion::ALL { + assert!( + version + .image() + .tag + .starts_with(&format!("version-{}_p", version.name())), + "{version:?}" + ); + } + assert_eq!(OpenSshVersion::default().image(), FixtureImage::OPENSSH); + } +} diff --git a/linux/crates/test-fixtures/src/ssh_agent.rs b/linux/crates/test-fixtures/src/ssh_agent.rs index b856b307c0..3d3a5ada0d 100644 --- a/linux/crates/test-fixtures/src/ssh_agent.rs +++ b/linux/crates/test-fixtures/src/ssh_agent.rs @@ -1,13 +1,12 @@ -use std::ffi::OsStr; use std::path::{Path, PathBuf}; -use std::process::{Child, Command, Output, Stdio}; +use std::process::{Child, Command, Stdio}; use std::time::Duration; use tempfile::TempDir; use tokio::time::Instant; use crate::host_command::run_host_command_with_env; -use crate::{FixtureError, SshKeyPair}; +use crate::{FixtureError, ScriptedAskpass, SshKeyPair}; const SOCKET_TIMEOUT: Duration = Duration::from_secs(10); const SOCKET_RETRY: Duration = Duration::from_millis(20); @@ -50,9 +49,26 @@ impl SshAgent { } /// Load a key, and its `-cert.pub` certificate when one sits beside - /// it, the way `ssh-add` does. + /// it, the way `ssh-add` does. A key with a passphrase is unlocked + /// with it. pub async fn add(&self, key: &SshKeyPair) -> Result<(), FixtureError> { - self.ssh_add([key.private_key().as_os_str()]).await.map(drop) + let mut environment = vec![("SSH_AUTH_SOCK", self.socket.as_os_str().to_owned())]; + // Kept until ssh-add has run, since dropping it deletes the script. + let _askpass = match key.passphrase() { + Some(passphrase) => { + let askpass = ScriptedAskpass::new()?; + environment.extend([ + ("SSH_ASKPASS", askpass.path().as_os_str().to_owned()), + ("SSH_ASKPASS_REQUIRE", "force".into()), + (ScriptedAskpass::ANSWER_VARIABLE, passphrase.into()), + ]); + Some(askpass) + } + None => None, + }; + run_host_command_with_env("ssh-add", [key.private_key().as_os_str()], environment) + .await + .map(drop) } /// The public keys the agent holds, one OpenSSH line each. @@ -79,11 +95,6 @@ impl SshAgent { } } - async fn ssh_add<'a>(&self, arguments: impl IntoIterator) -> Result { - let environment = [("SSH_AUTH_SOCK", self.socket.as_os_str())]; - run_host_command_with_env("ssh-add", arguments, environment).await - } - async fn wait_for_socket(&self) -> Result<(), FixtureError> { let deadline = Instant::now() + SOCKET_TIMEOUT; while Instant::now() < deadline { @@ -122,6 +133,18 @@ mod tests { assert_eq!(agent.identities().await.unwrap(), vec![key.public_key().to_owned()]); } + #[tokio::test] + async fn a_key_with_a_passphrase_is_unlocked_into_the_agent() { + let agent = SshAgent::start().await.unwrap(); + let key = SshKeyPair::generate_with(crate::SshKeyType::Ed25519, Some("open sesame"), "tablepro-agent-test") + .await + .unwrap(); + + agent.add(&key).await.unwrap(); + + assert_eq!(agent.identities().await.unwrap(), vec![key.public_key().to_owned()]); + } + #[tokio::test] async fn dropping_the_agent_stops_it() { let agent = SshAgent::start().await.unwrap(); diff --git a/linux/crates/test-fixtures/src/ssh_auth_variant.rs b/linux/crates/test-fixtures/src/ssh_auth_variant.rs index 229e706477..84a85f0c99 100644 --- a/linux/crates/test-fixtures/src/ssh_auth_variant.rs +++ b/linux/crates/test-fixtures/src/ssh_auth_variant.rs @@ -4,12 +4,32 @@ use crate::SshKeyPair; pub enum SshAuthVariant { Password, PublicKey(SshKeyPair), + /// Several keys in authorized_keys, so one server takes a matrix of + /// client keys. + PublicKeys(Vec), + /// TrustedUserCAKeys names this key, so only a certificate it signed + /// logs in. + UserCertificateAuthority(SshKeyPair), KeyboardInteractive, HostCertificate, } impl SshAuthVariant { pub fn password_access(&self) -> bool { - !matches!(self, Self::PublicKey(_)) + !matches!( + self, + Self::PublicKey(_) | Self::PublicKeys(_) | Self::UserCertificateAuthority(_) + ) + } + + /// The authorized_keys lines the server starts with. + pub fn authorized_keys(&self) -> Vec<&str> { + match self { + Self::PublicKey(key) => vec![key.public_key()], + Self::PublicKeys(keys) => keys.iter().map(SshKeyPair::public_key).collect(), + Self::UserCertificateAuthority(_) | Self::Password | Self::KeyboardInteractive | Self::HostCertificate => { + Vec::new() + } + } } } diff --git a/linux/crates/test-fixtures/src/ssh_key_pair.rs b/linux/crates/test-fixtures/src/ssh_key_pair.rs index 99af356686..597752d036 100644 --- a/linux/crates/test-fixtures/src/ssh_key_pair.rs +++ b/linux/crates/test-fixtures/src/ssh_key_pair.rs @@ -3,31 +3,43 @@ use std::path::{Path, PathBuf}; use tempfile::TempDir; -use crate::FixtureError; use crate::host_command::run_host_command; +use crate::{FixtureError, SshKeyType}; #[derive(Debug)] pub struct SshKeyPair { _dir: TempDir, + key_type: SshKeyType, private_key: PathBuf, public_key: String, + passphrase: Option, } impl SshKeyPair { + /// An ed25519 key with no passphrase. pub async fn generate(comment: &str) -> Result { + Self::generate_with(SshKeyType::Ed25519, None, comment).await + } + + /// A key of `key_type`, its private half encrypted with `passphrase` + /// when there is one. + pub async fn generate_with( + key_type: SshKeyType, + passphrase: Option<&str>, + comment: &str, + ) -> Result { let dir = tempfile::tempdir()?; - let private_key = dir.path().join("id_ed25519"); - let arguments: [&OsStr; 9] = [ - "-q".as_ref(), - "-t".as_ref(), - "ed25519".as_ref(), - "-N".as_ref(), - "".as_ref(), - "-C".as_ref(), - comment.as_ref(), - "-f".as_ref(), + let private_key = dir.path().join(key_type.file_name()); + let mut arguments: Vec<&OsStr> = vec![OsStr::new("-q")]; + arguments.extend(key_type.keygen_arguments().iter().map(OsStr::new)); + arguments.extend([ + OsStr::new("-N"), + OsStr::new(passphrase.unwrap_or_default()), + OsStr::new("-C"), + OsStr::new(comment), + OsStr::new("-f"), private_key.as_os_str(), - ]; + ]); run_host_command("ssh-keygen", arguments).await?; let public_key = tokio::fs::read_to_string(private_key.with_extension("pub")) .await? @@ -35,11 +47,17 @@ impl SshKeyPair { .to_owned(); Ok(Self { _dir: dir, + key_type, private_key, public_key, + passphrase: passphrase.map(str::to_owned), }) } + pub fn key_type(&self) -> SshKeyType { + self.key_type + } + pub fn private_key(&self) -> &Path { &self.private_key } @@ -47,4 +65,60 @@ impl SshKeyPair { pub fn public_key(&self) -> &str { &self.public_key } + + pub fn passphrase(&self) -> Option<&str> { + self.passphrase.as_deref() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn each_key_type_writes_its_own_public_key_line() { + for key_type in SshKeyType::ALL { + let key = SshKeyPair::generate_with(key_type, None, "tablepro-key-test") + .await + .unwrap(); + + assert!( + key.public_key() + .starts_with(&format!("{} ", key_type.public_key_type())), + "{key_type:?}: {}", + key.public_key() + ); + assert!(key.public_key().ends_with(" tablepro-key-test")); + assert_eq!(key.key_type(), key_type); + } + } + + /// `ssh-keygen -y` derives the public key from the private one, and + /// with `-P ''` it fails on a key that needs a passphrase. + #[tokio::test] + async fn a_passphrase_encrypts_the_private_key() { + let derive = |key: &SshKeyPair, passphrase: &str| { + std::process::Command::new("ssh-keygen") + .args(["-y", "-P", passphrase, "-f"]) + .arg(key.private_key()) + .output() + .unwrap() + }; + let plain = SshKeyPair::generate("tablepro-key-test").await.unwrap(); + let encrypted = SshKeyPair::generate_with(SshKeyType::EcdsaP256, Some("open sesame"), "tablepro-key-test") + .await + .unwrap(); + + assert!(derive(&plain, "").status.success()); + assert!(!derive(&encrypted, "").status.success()); + let unlocked = derive(&encrypted, "open sesame"); + assert!(unlocked.status.success()); + assert!( + encrypted + .public_key() + .starts_with(String::from_utf8_lossy(&unlocked.stdout).trim()) + ); + assert_eq!(encrypted.passphrase(), Some("open sesame")); + assert_eq!(plain.passphrase(), None); + } } diff --git a/linux/crates/test-fixtures/src/ssh_key_type.rs b/linux/crates/test-fixtures/src/ssh_key_type.rs new file mode 100644 index 0000000000..81a4fbbef2 --- /dev/null +++ b/linux/crates/test-fixtures/src/ssh_key_type.rs @@ -0,0 +1,37 @@ +/// A client key algorithm `ssh-keygen` generates for a test. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SshKeyType { + Ed25519, + /// 3072 bits, `ssh-keygen`'s default size. + Rsa, + EcdsaP256, +} + +impl SshKeyType { + pub const ALL: [Self; 3] = [Self::Ed25519, Self::Rsa, Self::EcdsaP256]; + + pub(crate) fn keygen_arguments(self) -> &'static [&'static str] { + match self { + Self::Ed25519 => &["-t", "ed25519"], + Self::Rsa => &["-t", "rsa", "-b", "3072"], + Self::EcdsaP256 => &["-t", "ecdsa", "-b", "256"], + } + } + + pub(crate) fn file_name(self) -> &'static str { + match self { + Self::Ed25519 => "id_ed25519", + Self::Rsa => "id_rsa", + Self::EcdsaP256 => "id_ecdsa", + } + } + + /// The key type an OpenSSH public key line starts with. + pub fn public_key_type(self) -> &'static str { + match self { + Self::Ed25519 => "ssh-ed25519", + Self::Rsa => "ssh-rsa", + Self::EcdsaP256 => "ecdsa-sha2-nistp256", + } + } +} diff --git a/linux/crates/test-fixtures/tests/fixtures.rs b/linux/crates/test-fixtures/tests/fixtures.rs index 72b3bd2db9..91c3600938 100644 --- a/linux/crates/test-fixtures/tests/fixtures.rs +++ b/linux/crates/test-fixtures/tests/fixtures.rs @@ -9,10 +9,10 @@ use std::process::Stdio; use tablepro_test_fixtures::{ ClickHouseFixture, ExecOutput, FixtureCredentials, HbaMode, HostKeyRevocation, MssqlFixture, MysqlFixture, - MysqlFlavour, MysqlOptions, OpenSshFixture, PgBouncerFixture, PkiFile, PoolMode, PostgresFixture, PostgresVersion, - ScriptedAskpass, ServerVersion, SshAuthVariant, SshKeyPair, TestNetwork, TestPki, + MysqlFlavour, MysqlOptions, OpenSshFixture, OpenSshVersion, PgBouncerFixture, PkiFile, PoolMode, PostgresFixture, + PostgresVersion, ScriptedAskpass, ServerVersion, SshAuthVariant, SshKeyPair, SshKeyType, TestNetwork, TestPki, }; -use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; use tokio::net::TcpStream; use tokio_rustls::TlsConnector; @@ -345,6 +345,44 @@ async fn openssh_fixture_enables_tcp_forwarding() -> TestResult { Ok(()) } +#[tokio::test] +#[ignore = "requires docker"] +async fn openssh_fixture_runs_the_selected_release_with_its_keys_and_lines() -> TestResult { + let keys = vec![ + SshKeyPair::generate("tablepro-first").await?, + SshKeyPair::generate_with(SshKeyType::Rsa, None, "tablepro-second").await?, + ]; + let fixture = OpenSshFixture::start_configured(SshAuthVariant::PublicKeys(keys), &["MaxAuthTries 3"]).await?; + let SshAuthVariant::PublicKeys(keys) = fixture.variant() else { + return Err("the fixture lost its keys".into()); + }; + + let config = fixture + .exec(&["sshd.pam", "-T", "-f", OpenSshFixture::SSHD_CONFIG]) + .await?; + let authorized = fixture.exec(&["cat", "/config/.ssh/authorized_keys"]).await?; + let mut banner = String::new(); + BufReader::new(TcpStream::connect((fixture.host(), fixture.port())).await?) + .read_line(&mut banner) + .await?; + + assert!( + config.stdout_text().contains("maxauthtries 3"), + "{}", + config.stderr_text() + ); + for key in keys { + assert!( + authorized.stdout_text().lines().any(|line| line == key.public_key()), + "{}", + authorized.stdout_text() + ); + } + assert_eq!(fixture.version(), OpenSshVersion::selected()?); + assert!(banner.starts_with(&fixture.version().banner_prefix()), "{banner:?}"); + Ok(()) +} + #[tokio::test] #[ignore = "requires docker"] async fn keyboard_interactive_fixture_accepts_only_keyboard_interactive() -> TestResult { diff --git a/linux/docs/testing.md b/linux/docs/testing.md index 2855c9be25..f7d8b48733 100644 --- a/linux/docs/testing.md +++ b/linux/docs/testing.md @@ -112,7 +112,7 @@ Every TLS fixture has two principals: SQL Server has only the `sa` principal, because its TLS contract carries no client identity. -`HbaMode` picks what the PostgreSQL fixture accepts: `Password` (the image defaults, no TLS), `HostSslOnly`, or `ClientCertificate`. Docker maps a published port anew each time a container starts, so a test that restarts the server starts it with `PostgresFixture::start_restartable`, which picks the host port itself; `stop` and `start_again` then bring it back where the session's connection points, with its data. `OpenSshFixture` takes an `SshAuthVariant`: `Password`, `PublicKey`, `KeyboardInteractive` or `HostCertificate`. `HostKeyRevocation` builds a KRL from the running server's host key, and `ScriptedAskpass` writes a 0700 script for `SSH_ASKPASS`. +`HbaMode` picks what the PostgreSQL fixture accepts: `Password` (the image defaults, no TLS), `HostSslOnly`, or `ClientCertificate`. Docker maps a published port anew each time a container starts, so a test that restarts the server starts it with `PostgresFixture::start_restartable`, which picks the host port itself; `stop` and `start_again` then bring it back where the session's connection points, with its data. `OpenSshFixture` takes an `SshAuthVariant`: `Password`, `PublicKey`, `PublicKeys` (several keys in one authorized_keys), `UserCertificateAuthority` (logins by certificate only), `KeyboardInteractive` or `HostCertificate`, and `start_configured` appends `sshd_config` lines such as `KexAlgorithms`. `SshKeyPair::generate_with` makes an ed25519, RSA or ECDSA P-256 key, encrypted when given a passphrase, and `SshAgent` is a private `ssh-agent` that loads one. `HostKeyRevocation` builds a KRL from the running server's host key, and `ScriptedAskpass` writes a 0700 script for `SSH_ASKPASS`. ### Conformance @@ -252,10 +252,11 @@ A fixture starts its engine's default release unless the engine's variable names | `TABLEPRO_TEST_MARIADB` | 10.11, 11.4, 12.3 | 11.4 | | `TABLEPRO_TEST_MSSQL` | 2017, 2019, 2022, 2025 | 2022 | | `TABLEPRO_TEST_CLICKHOUSE` | 26.3, 26.8 | 26.8 | +| `TABLEPRO_TEST_OPENSSH` | 9.9, 10.3 | 10.3 | -`crates/test-fixtures/src/fixture_image.rs` pins each release's image, and each engine's version type (`PostgresVersion`, `MysqlVersion`, `MariadbVersion`, `MssqlVersion`, `ClickHouseVersion`) reads its variable through `ServerVersion::selected`. An empty value picks the default; a release the fixtures do not pin fails the fixture with the ones they do. PostgreSQL 13 is past end of life and stays only for the catalogue test of the driver's query for servers before 14. +`crates/test-fixtures/src/fixture_image.rs` pins each release's image, and each engine's version type (`PostgresVersion`, `MysqlVersion`, `MariadbVersion`, `MssqlVersion`, `ClickHouseVersion`, `OpenSshVersion`) reads its variable through `ServerVersion::selected`. An empty value picks the default; a release the fixtures do not pin fails the fixture with the ones they do. PostgreSQL 13 is past end of life and stays only for the catalogue test of the driver's query for servers before 14. -The `integration` job runs each driver's suite on the defaults, then once more for each entry with a `release`: PostgreSQL 14 and 18, MySQL 8.0 with MariaDB 10.11, MySQL 9.7 with MariaDB 12.3, SQL Server 2017, 2019 and 2025, and ClickHouse 26.3. The README's [supported servers](../README.md#supported-servers) table says why those releases and not others. A test that checks the server's version reads it from the fixture (`PostgresFixture::version`, `MysqlFixture::server_version`, `MssqlFixture::version`, `ClickHouseFixture::version`), never from a literal, so it holds on every release. +The `integration` job runs each driver's suite on the defaults, then once more for each entry with a `release`: PostgreSQL 14 and 18, MySQL 8.0 with MariaDB 10.11, MySQL 9.7 with MariaDB 12.3, SQL Server 2017, 2019 and 2025, and ClickHouse 26.3, and the SSH suite on OpenSSH 9.9. The README's [supported servers](../README.md#supported-servers) table says why those database releases and not others; OpenSSH 9.9 is the last 9.x. A test that checks the server's version reads it from the fixture (`PostgresFixture::version`, `MysqlFixture::server_version`, `MssqlFixture::version`, `ClickHouseFixture::version`, `OpenSshFixture::version`), never from a literal, so it holds on every release. Locally, set the variables for the run: From 90879f922c74aaffe6e76c4df17037671d287b29 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 21 Oct 2026 03:55:13 +0700 Subject: [PATCH 2/3] docs(linux-ssh): add the changelog fragments for #3720 --- linux/changelog.d/3720.changed.md | 1 + linux/changelog.d/3720.fixed.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 linux/changelog.d/3720.changed.md create mode 100644 linux/changelog.d/3720.fixed.md diff --git a/linux/changelog.d/3720.changed.md b/linux/changelog.d/3720.changed.md new file mode 100644 index 0000000000..249907ef66 --- /dev/null +++ b/linux/changelog.d/3720.changed.md @@ -0,0 +1 @@ +SSH errors name a key exchange or cipher the server and OpenSSH do not share, and a login cut off after too many tries. diff --git a/linux/changelog.d/3720.fixed.md b/linux/changelog.d/3720.fixed.md new file mode 100644 index 0000000000..22a5ac7666 --- /dev/null +++ b/linux/changelog.d/3720.fixed.md @@ -0,0 +1 @@ +An SSH tunnel with a private key failed with “Too many authentication failures” when the agent held six or more keys. From 0b2f99fe24f85ed90c997c08c6c443ade5ebfb08 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 21 Oct 2026 04:03:49 +0700 Subject: [PATCH 3/3] build(linux): let tablepro-ssh depend on the test fixtures in cargo deny --- linux/deny.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/deny.toml b/linux/deny.toml index 8eee771e4f..61f5763fe6 100644 --- a/linux/deny.toml +++ b/linux/deny.toml @@ -53,7 +53,7 @@ deny = [ { crate = "backoff", reason = "unmaintained" }, { crate = "instant", reason = "unmaintained" }, { crate = "rsa", wrappers = ["sqlx-mysql"], reason = "RUSTSEC-2023-0071 is accepted only for sqlx-mysql's public-key encryption; any other user of rsa needs a new decision in ADR 0009" }, - { crate = "tablepro-test-fixtures", wrappers = ["tablepro", "tablepro-driver-clickhouse", "tablepro-driver-mssql", "tablepro-driver-mysql", "tablepro-driver-postgres", "tablepro-session"], reason = "container fixtures are a dev-only leaf; extend the wrappers with each new dependent" }, + { crate = "tablepro-test-fixtures", wrappers = ["tablepro", "tablepro-driver-clickhouse", "tablepro-driver-mssql", "tablepro-driver-mysql", "tablepro-driver-postgres", "tablepro-session", "tablepro-ssh"], reason = "container fixtures are a dev-only leaf; extend the wrappers with each new dependent" }, { crate = "libsqlite3-sys", deny-multiple-versions = true, reason = "two SQLite builds in one binary conflict" }, ]