diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 79a222de..99c98886 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -855,6 +855,14 @@ but not with themselves; omit the key (plain `#[serial]`) when in doubt. - Password authentication (discouraged in production) - Public key authentication preferred +#### macOS `UseKeychain` + +On macOS, the resolved `UseKeychain yes` SSH configuration option enables passphrase lookup and storage for encrypted private-key authentication. Keychain access is deferred until an encrypted key actually needs a passphrase, is skipped entirely by `BatchMode yes`, and is serialized with terminal passphrase prompts so parallel connections cannot display competing authentication UI. + +bssh owns generic-password records under the `bssh-ssh-key-passphrase` service and uses the canonical private-key path as the account. This namespace is intentionally separate from the Data Protection Keychain access group used by Apple's `/usr/bin/ssh`; that access group requires an Apple-only code-signing entitlement, so third-party binaries cannot reuse entries created by `ssh-add --apple-use-keychain`. A retrieved passphrase is accepted only if it decrypts the key. Missing or stale records fall back to the terminal prompt, and a newly entered passphrase is stored only after successful decryption. + +Other platforms accept the Apple-specific keyword for portable configuration parsing but ignore its runtime behavior and recommend `IgnoreUnknown UseKeychain` when the same file must also work with upstream OpenSSH clients. + ### Host Verification - known_hosts file verification diff --git a/README.md b/README.md index 30a58fae..a88122ed 100644 --- a/README.md +++ b/README.md @@ -975,7 +975,8 @@ These options provide essential authentication management, security enforcement, **Platform Notes:** - **UseKeychain** is an Apple-specific patch to OpenSSH and only available on macOS - Fully integrated with macOS Keychain via Security Framework for secure passphrase storage and retrieval -- Passphrases are automatically stored after successful authentication and retrieved from Keychain on subsequent connections +- Passphrases are stored only after they successfully decrypt the private key and are retrieved from Keychain on subsequent connections +- bssh stores passphrases in its own `bssh-ssh-key-passphrase` Keychain service. Apple's `/usr/bin/ssh` uses an entitlement-protected access group, so entries created by `ssh-add --apple-use-keychain` cannot be read directly by third-party binaries and the first bssh connection may still prompt once. - For cross-platform configurations, use `IgnoreUnknown UseKeychain` to prevent errors on non-macOS systems ### SSH Config Examples diff --git a/docs/man/bssh.1 b/docs/man/bssh.1 index 8c823f90..0a5b9f7f 100644 --- a/docs/man/bssh.1 +++ b/docs/man/bssh.1 @@ -1013,7 +1013,14 @@ When enabled, passphrases are automatically retrieved from and stored in the mac .RS .IP \[bu] 2 .B Implementation: -Fully integrated with macOS Keychain via Security Framework. Passphrases are securely stored after successful authentication and retrieved on subsequent connections. +Fully integrated with macOS Keychain via Security Framework. Passphrases are securely stored after successful private-key decryption and retrieved on subsequent connections. +.IP \[bu] 2 +.B Interoperability: +bssh uses its own +.I bssh-ssh-key-passphrase +Keychain service. Entries created by +.I ssh-add --apple-use-keychain +belong to an Apple entitlement-protected access group and cannot be read directly by third-party binaries, so bssh may prompt once before storing its own entry. .IP \[bu] 2 .B Cross-platform compatibility: Use diff --git a/src/ssh/ssh_config/dump.rs b/src/ssh/ssh_config/dump.rs index df20ea68..21113d67 100644 --- a/src/ssh/ssh_config/dump.rs +++ b/src/ssh/ssh_config/dump.rs @@ -78,6 +78,8 @@ pub fn render_resolved_config(original_host: &str, config: &SshHostConfig) -> Re config.hostbased_authentication.unwrap_or(false), )?; output.bool("identitiesonly", config.identities_only.unwrap_or(false))?; + #[cfg(target_os = "macos")] + output.bool("usekeychain", config.use_keychain.unwrap_or(false))?; output.bool( "kbdinteractiveauthentication", config.keyboard_interactive_authentication.unwrap_or(true), diff --git a/src/ssh/ssh_config/parser/options/authentication.rs b/src/ssh/ssh_config/parser/options/authentication.rs index 54692e7c..956be6ef 100644 --- a/src/ssh/ssh_config/parser/options/authentication.rs +++ b/src/ssh/ssh_config/parser/options/authentication.rs @@ -406,11 +406,7 @@ pub(super) fn parse_authentication_option( #[cfg(target_os = "macos")] { if value { - tracing::debug!( - "UseKeychain enabled at line {} (Note: Currently supports parsing only. \ - Keychain integration will be implemented in a future release)", - line_number - ); + tracing::debug!("UseKeychain enabled at line {line_number}"); } host.use_keychain = Some(value); } diff --git a/src/ssh/ssh_config/parser/options/support.rs b/src/ssh/ssh_config/parser/options/support.rs index 0a7f7153..d494b2f8 100644 --- a/src/ssh/ssh_config/parser/options/support.rs +++ b/src/ssh/ssh_config/parser/options/support.rs @@ -109,7 +109,7 @@ pub(super) const ACCEPTED_KEYWORDS: &[(&str, &str, KeywordSupport)] = &[ Runtime(Authentication), ), ("enablesshkeysign", "enablesshkeysign", Unimplemented), - ("usekeychain", "usekeychain", Unimplemented), + ("usekeychain", "usekeychain", Runtime(Authentication)), ( "stricthostkeychecking", "stricthostkeychecking", @@ -306,8 +306,8 @@ mod tests { use std::collections::HashSet; const ACCEPTED_SPELLING_COUNT: usize = 108; - const RUNTIME_SPELLING_COUNT: usize = 58; - const UNIMPLEMENTED_SPELLING_COUNT: usize = 50; + const RUNTIME_SPELLING_COUNT: usize = 59; + const UNIMPLEMENTED_SPELLING_COUNT: usize = 49; #[test] fn accepted_keywords_and_aliases_have_one_consistent_classification() { @@ -360,6 +360,7 @@ mod tests { ("passwordauthentication", Authentication), ("preferredauthentications", Authentication), ("numberofpasswordprompts", Authentication), + ("usekeychain", Authentication), ("stricthostkeychecking", HostVerification), ("userknownhostsfile", HostVerification), ("globalknownhostsfile", HostVerification), @@ -434,7 +435,6 @@ mod tests { "hostbasedauthentication", "hostbasedacceptedalgorithms", "enablesshkeysign", - "usekeychain", "casignaturealgorithms", "nohostauthenticationforlocalhost", "visualhostkey", diff --git a/src/ssh/ssh_config/parser/tests.rs b/src/ssh/ssh_config/parser/tests.rs index c018542c..381cd5b6 100644 --- a/src/ssh/ssh_config/parser/tests.rs +++ b/src/ssh/ssh_config/parser/tests.rs @@ -1383,6 +1383,7 @@ Host example.com let hosts = parse(content).unwrap(); assert_eq!(hosts.len(), 1); assert_eq!(hosts[0].use_keychain, Some(true)); + assert!(!hosts[0].unimplemented_options.contains_key("usekeychain")); } #[test] diff --git a/src/ssh/ssh_config/resolver_tests.rs b/src/ssh/ssh_config/resolver_tests.rs index 72a06a76..267638c3 100644 --- a/src/ssh/ssh_config/resolver_tests.rs +++ b/src/ssh/ssh_config/resolver_tests.rs @@ -407,18 +407,18 @@ Host example.com #[test] #[cfg(target_os = "macos")] - fn test_use_keychain_override() { + fn test_use_keychain_host_specific_value_precedes_global_default() { let content = r#" -Host * - UseKeychain no - Host example.com UseKeychain yes + +Host * + UseKeychain no "#; let hosts = parse(content).unwrap(); let config = find_host_config(&hosts, "example.com"); - // The first matching block wins (yes) + // OpenSSH uses the first value obtained, so specific blocks precede defaults. assert_eq!(config.use_keychain, Some(true)); } @@ -443,8 +443,8 @@ Host example.com #[test] #[cfg(target_os = "macos")] - fn test_use_keychain_last_match_wins() { - // SSH config merges all matching blocks, with later values overriding earlier ones + fn test_use_keychain_first_match_wins() { + // SSH config merges all matching blocks while preserving the first value obtained. let content = r#" Host example.com UseKeychain yes @@ -455,8 +455,8 @@ Host example.com let hosts = parse(content).unwrap(); let config = find_host_config(&hosts, "example.com"); - // Should use the last matching value (no) due to merge logic - assert_eq!(config.use_keychain, Some(false)); + // OpenSSH keeps the first value obtained across matching blocks. + assert_eq!(config.use_keychain, Some(true)); } #[test] diff --git a/src/ssh/ssh_config/types.rs b/src/ssh/ssh_config/types.rs index 50f3c741..1db46073 100644 --- a/src/ssh/ssh_config/types.rs +++ b/src/ssh/ssh_config/types.rs @@ -179,9 +179,8 @@ pub struct SshHostConfig { pub identities_only: Option, pub add_keys_to_agent: Option, // yes/no/ask/confirm pub identity_agent: Option, // socket path or "none" - /// UseKeychain option (macOS only) - specifies whether to integrate with macOS Keychain - /// Note: This is an Apple-specific patch to OpenSSH. Currently supports parsing only. - /// Keychain integration will be implemented in a future release. + /// UseKeychain option (macOS only) - integrates private-key passphrases with macOS Keychain. + /// This is an Apple-specific OpenSSH extension. #[cfg(target_os = "macos")] pub use_keychain: Option, // Security & algorithm management diff --git a/src/ssh/tokio_client/authentication.rs b/src/ssh/tokio_client/authentication.rs index 4a3e04da..eb7e17e4 100644 --- a/src/ssh/tokio_client/authentication.rs +++ b/src/ssh/tokio_client/authentication.rs @@ -667,7 +667,6 @@ async fn load_policy_private_key( // authentication, and use of pre-collected credentials remain concurrent. let _prompt_guard = AUTH_PROMPT_MUTEX.lock().await; - #[cfg(target_os = "macos")] #[cfg(target_os = "macos")] if use_keychain { #[cfg(test)] @@ -680,8 +679,13 @@ async fn load_policy_private_key( .await; match retrieved { Ok(Some(passphrase)) => { - return russh::keys::load_secret_key(key_file_path, Some(&passphrase)) - .map_err(super::Error::KeyInvalid); + match russh::keys::load_secret_key(key_file_path, Some(&passphrase)) { + Ok(key) => return Ok(key), + Err(error) => tracing::warn!( + "Stored Keychain passphrase could not decrypt '{}': {error}; prompting for a replacement", + key_file_path.display() + ), + } } Ok(None) => {} Err(error @ super::Error::AuthenticationPromptTimeout { .. }) => return Err(error), @@ -709,6 +713,9 @@ async fn load_policy_private_key( .await?; let passphrase = Zeroizing::new(passphrase); + let key = russh::keys::load_secret_key(key_file_path, Some(&passphrase)) + .map_err(super::Error::KeyInvalid)?; + #[cfg(target_os = "macos")] if use_keychain { let stored = bounded_auth_prompt("macOS Keychain storage", async { @@ -725,7 +732,7 @@ async fn load_policy_private_key( } } - russh::keys::load_secret_key(key_file_path, Some(&passphrase)).map_err(super::Error::KeyInvalid) + Ok(key) } async fn default_hashes( handle: &mut Handle, @@ -1806,6 +1813,47 @@ mod policy_execution_tests { ); } + #[cfg(target_os = "macos")] + #[tokio::test] + #[serial_test::serial] + async fn use_keychain_decrypts_an_encrypted_policy_key() { + let directory = tempfile::TempDir::new().unwrap(); + let key_path = directory.path().join("encrypted-key"); + let expected = + russh::keys::PrivateKey::random(&mut rand::rng(), Algorithm::Ed25519).unwrap(); + let encrypted = expected + .clone() + .encrypt(&mut rand::rng(), "keychain-test-passphrase") + .unwrap(); + std::fs::write( + &key_path, + encrypted.to_openssh(LineEnding::LF).unwrap().as_bytes(), + ) + .unwrap(); + + if let Err(error) = + crate::ssh::keychain_macos::store_passphrase(&key_path, "keychain-test-passphrase") + .await + { + let message = format!("{error:#}"); + if message.contains("authorization was canceled") + || message.contains("Keychain access is denied") + || message.contains("Keychain is locked") + { + eprintln!("skipping Keychain-backed test: {message}"); + return; + } + panic!("failed to prepare Keychain-backed test: {message}"); + } + + let loaded = load_policy_private_key(&key_path, None, true, true).await; + let cleanup = crate::ssh::keychain_macos::delete_passphrase(&key_path).await; + let loaded = loaded.expect("UseKeychain should decrypt the configured key"); + cleanup.expect("test Keychain entry should be deleted"); + + assert_eq!(loaded.public_key(), expected.public_key()); + } + #[tokio::test(start_paused = true)] async fn password_prompts_have_a_typed_timeout_bound() { let error = serialized_bounded_auth_prompt("password", || async { diff --git a/tests/ssh_config_dump_test.rs b/tests/ssh_config_dump_test.rs index 3eaa144e..30057acb 100644 --- a/tests/ssh_config_dump_test.rs +++ b/tests/ssh_config_dump_test.rs @@ -64,6 +64,27 @@ fn dump_exits_without_proxy_agent_prompt_or_connection_side_effects() { assert!(stdout.contains("identityagent /missing/agent.sock\n")); } +#[cfg(target_os = "macos")] +#[test] +fn use_keychain_is_reported_as_runtime_supported() { + let directory = tempdir().expect("temporary directory should be created"); + let config = directory.path().join("config"); + fs::write(&config, "Host target\n UseKeychain yes\n").expect("config should be written"); + + let output = run(&["-G", "-F", path(&config), "target"]); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + output.stderr.is_empty(), + "runtime-supported UseKeychain emitted a diagnostic: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!(String::from_utf8_lossy(&output.stdout).contains("usekeychain yes\n")); +} + #[test] fn match_and_include_restore_parent_scope_for_destination() { let directory = tempdir().expect("temporary directory should be created");