Skip to content

Commit 7720cfb

Browse files
authored
feat: Complete Phase 2 macOS Keychain integration for UseKeychain (#59) (#61)
* feat: Add UseKeychain SSH config option support (Phase 1 - Parsing) Implement parsing support for the UseKeychain SSH configuration option, an Apple-specific extension to OpenSSH for macOS Keychain integration. Phase 1 Implementation (Parsing Only): - Added use_keychain field to SshHostConfig (macOS only, conditional compilation) - Implemented parsing logic for "usekeychain" keyword in authentication options - Added configuration merge logic with proper precedence handling - Platform-specific warnings on non-macOS systems - Comprehensive test coverage (13 tests: 8 parser + 5 resolver) - Full documentation in README.md and man page Key Features: - Uses #[cfg(target_os = "macos")] for platform-specific code - Consistent with existing authentication options (parse_yes_no helper) - Cross-platform compatibility with IgnoreUnknown directive - Integration with AddKeysToAgent option Phase 2 (Future): - macOS Keychain API integration - Actual key storage/retrieval functionality Files Modified: - src/ssh/ssh_config/types.rs - Added use_keychain field - src/ssh/ssh_config/parser/options/authentication.rs - Parsing logic - src/ssh/ssh_config/parser/options/mod.rs - Dispatcher routing - src/ssh/ssh_config/resolver.rs - Merge logic - src/ssh/ssh_config/parser/tests.rs - Parser tests (8 tests) - src/ssh/ssh_config/resolver_tests.rs - Resolver tests (5 tests) - README.md - User documentation - docs/man/bssh.1 - Man page documentation Related: #59 * feat: Complete Phase 2 macOS Keychain integration for UseKeychain option Implement full macOS Keychain API integration for the UseKeychain SSH config option, enabling automatic passphrase storage and retrieval. Phase 2 Implementation (Keychain Integration): - Implemented macOS Security Framework integration in src/ssh/keychain_macos.rs - Added store_passphrase(), retrieve_passphrase(), and delete_passphrase() functions - Integrated keychain operations into SSH authentication flow (src/ssh/auth.rs) - Passphrases automatically stored after successful authentication - Passphrases automatically retrieved on subsequent connections - Secure memory handling using Zeroizing for all passphrase data - Helper function determine_use_keychain() to extract config value - Command executors updated to pass use_keychain from SSH config - Cross-platform compatibility maintained with conditional compilation Key Features: - Seamless macOS Keychain integration via Security Framework - Automatic passphrase storage after successful key-based authentication - Automatic retrieval from Keychain before prompting user - Secure zeroization of passphrases in memory - Integration with SSH config UseKeychain option - Works with both specified key files and default key locations - Full test coverage (7 tests in keychain_macos module) Security Considerations: - Passphrases stored using macOS Security Framework's GenericPassword API - Service name: "bssh-ssh-key-passphrase" - Account name: canonical path of SSH key file - Requires user authentication when Keychain is locked (managed by macOS) - All sensitive data uses Zeroizing for secure memory cleanup - Passphrase length validation (max 8KB) to prevent DoS - Path canonicalization to prevent path traversal attacks Files Modified: - Cargo.toml - Added security-framework dependency for macOS - src/ssh/keychain_macos.rs - New keychain module (498 lines, 7 tests) - src/ssh/auth.rs - Integrated keychain in authentication flow - src/app/initialization.rs - Added determine_use_keychain() helper - src/app/dispatcher.rs - Extract and pass use_keychain from SSH config - src/commands/exec.rs - Pass use_keychain to executor - src/executor/* - Threading use_keychain through execution pipeline - src/ssh/client/* - Updated connection methods with use_keychain parameter - README.md - Updated documentation to reflect Phase 2 completion - docs/man/bssh.1 - Updated man page documentation Testing: - All 357 tests pass including 7 new keychain tests - Tests cover: store/retrieve, deletion, nonexistent passphrases, updates, zeroization, length validation, and invalid paths - Integration with existing auth tests Related: #59 (Phase 2) Depends on: d58efcd (Phase 1 - Parsing) * fix: Add password authentication fallback to match OpenSSH behavior Add automatic password authentication fallback when all key-based authentication methods fail, matching standard OpenSSH behavior. Problem: - When connecting to new servers without authorized keys, connection would fail immediately without prompting for password - Users had to explicitly use --password flag even when no keys worked - This differs from OpenSSH which tries password as last resort Solution: Implemented 6-step authentication priority (matches OpenSSH): 1. Password (if --password flag set) 2. SSH agent (if explicitly requested) 3. Specified key file (if provided via -i) 4. SSH agent auto-detection 5. Default key locations (~/.ssh/id_*) 6. Password fallback (NEW) - Interactive terminals only Key Features: - Fallback only occurs in interactive terminals (stdin is TTY) - Non-interactive environments (CI/automation) get helpful error message - Maintains all existing security measures: - Passwords zeroized after use - Timing attack mitigation (50ms normalization) - No credential leakage in errors - Backward compatible with existing workflows Security: - Uses atty crate to detect interactive terminals - Non-interactive sessions cannot hang on password prompts - Error messages provide clear guidance on authentication options Testing: - Added test_password_fallback_in_non_interactive() - All 364 tests pass - Verified interactive and non-interactive behavior This fix significantly improves user experience by eliminating the need for --password flag in most common scenarios while maintaining security and automation compatibility. Fixes authentication regression reported by users where password prompts no longer appeared for new server connections. * fix: Add use_keychain support to interactive mode Interactive mode (direct hostname without -H flag) was missing use_keychain field integration from Phase 2, causing authentication to fail. Added conditional use_keychain field and updated all interactive command instantiation points. Fixes password prompt issue when using: bssh hostname * fix: Add password authentication fallback for interactive mode When publickey authentication fails in interactive mode, automatically retry with password authentication (matching OpenSSH behavior). Changes: - Add password fallback retry in connect_to_node_pty (2 paths) - Add password fallback retry in connect_to_node (2 paths) - Only retry when stdin is a TTY (interactive terminal) - Log retry attempt for debugging Behavior now matches OpenSSH: 1. Try publickey authentication first 2. On failure, prompt for password if in interactive terminal This fixes the issue where bssh would fail without password prompt when publickey authentication failed. * fix(security): validate SSH key file ownership before Keychain storage - Priority: CRITICAL - Added ownership validation to prevent storing passphrases for keys owned by other users - Check that current user owns the SSH key file before allowing Keychain storage - Warn if SSH key has overly permissive permissions (world-readable) - Also fixed unnecessary passphrase_bytes.clone() memory copy - Priority: LOW - Added libc dependency for macOS to access getuid() Security: Prevents unauthorized passphrase storage for other users' SSH keys * fix(security/perf): add user consent for password fallback and eliminate code duplication - Priority: HIGH Security improvements: - Added explicit user consent prompt before password fallback authentication - Password fallback now only occurs with user permission (yes/no prompt) - Prevents unexpected password prompts that could lead to credential exposure - Added 30-second timeout for consent prompt to prevent hanging Performance improvements: - Eliminated 249 lines of duplicated connection logic (4 identical retry blocks) - Created establish_connection() helper to centralize connection handling - Removed redundant password fallback attempts from connection.rs - Authentication fallback logic now centralized in auth module The password fallback with consent is now handled properly in the auth module's determine_method() flow, eliminating the need for duplicate retry logic in the connection layer. This reduces code complexity and improves maintainability. * fix(security): add rate limiting between authentication attempts - Priority: MEDIUM Security improvements: - Added 100ms delay before each connection attempt in establish_connection() - Added 1 second delay before password fallback consent prompt - Prevents rapid-fire authentication attempts that could trigger fail2ban - Helps mitigate brute-force attack attempts - Gives servers time to process authentication failures The rate limiting is applied at two levels: 1. Connection level: Small delay (100ms) before each connection attempt 2. Fallback level: Larger delay (1s) before password fallback prompt This prevents both automated attacks and accidental DoS from rapid retries. Performance: The double connection attempt issue is also resolved as a side effect of the earlier refactoring - we now have a single connection path with proper rate limiting instead of duplicate retry logic. * fix: Remove unnecessary mut from executor in exec.rs Variable shadowing is used for macOS-specific with_keychain call, eliminating the need for mutable binding on non-macOS platforms. This fixes the clippy unused_mut warning. * fix: Make determine_use_keychain import and function conditional - Add #[cfg(target_os = "macos")] to import in dispatcher.rs - Add #[allow(dead_code)] to non-macOS stub function in initialization.rs This fixes clippy unused_imports and dead_code warnings on non-macOS platforms while maintaining API consistency across platforms.
1 parent 1c4914f commit 7720cfb

28 files changed

+1398
-112
lines changed

Cargo.lock

Lines changed: 35 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ fastrand = "2.2"
4848
tokio-util = "0.7"
4949
shell-words = "1.1"
5050

51+
[target.'cfg(target_os = "macos")'.dependencies]
52+
security-framework = "2.9"
53+
libc = "0.2"
54+
5155
[dev-dependencies]
5256
tempfile = "3"
5357
mockito = "1"

README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,7 @@ These options provide essential authentication management, security enforcement,
479479
|--------|-------------|---------|
480480
| **IdentitiesOnly** | Only use identity files specified in config, ignore SSH agent (yes/no) | `IdentitiesOnly yes` |
481481
| **AddKeysToAgent** | Auto-add keys to SSH agent (yes/no/ask/confirm) | `AddKeysToAgent yes` |
482+
| **UseKeychain** | **[macOS only]** Use macOS Keychain for SSH key passphrases (yes/no) | `UseKeychain yes` |
482483
| **IdentityAgent** | Custom SSH agent socket path or "none" | `IdentityAgent ~/.1password/agent.sock` |
483484
| **PubkeyAcceptedAlgorithms** | Restrict allowed public key algorithms (max 50) | `PubkeyAcceptedAlgorithms ssh-ed25519,rsa-sha2-512` |
484485
| **RequiredRSASize** | Minimum RSA key size in bits (1024-16384, warns <2048) | `RequiredRSASize 2048` |
@@ -487,11 +488,18 @@ These options provide essential authentication management, security enforcement,
487488
**Key Benefits:**
488489
- **IdentitiesOnly**: Solves multi-account authentication conflicts
489490
- **AddKeysToAgent**: Eliminates manual ssh-add commands
491+
- **UseKeychain**: Seamlessly integrates with macOS Keychain for passphrase management (Apple-specific OpenSSH extension)
490492
- **IdentityAgent**: Enables modern agent tools (1Password, gpg-agent, etc.)
491493
- **PubkeyAcceptedAlgorithms**: Enforces security policies
492494
- **RequiredRSASize**: Prevents weak RSA keys
493495
- **FingerprintHash**: Flexibility for legacy systems
494496

497+
**Platform Notes:**
498+
- **UseKeychain** is an Apple-specific patch to OpenSSH and only available on macOS
499+
- Fully integrated with macOS Keychain via Security Framework for secure passphrase storage and retrieval
500+
- Passphrases are automatically stored after successful authentication and retrieved from Keychain on subsequent connections
501+
- For cross-platform configurations, use `IgnoreUnknown UseKeychain` to prevent errors on non-macOS systems
502+
495503
### SSH Config Examples
496504

497505
#### Certificate-based Authentication
@@ -622,6 +630,12 @@ Host personal
622630
Host *
623631
AddKeysToAgent yes
624632
633+
# macOS-specific: Use Keychain for SSH key passphrases
634+
# For cross-platform configs, add IgnoreUnknown to prevent errors on non-macOS systems
635+
Host *
636+
IgnoreUnknown UseKeychain
637+
UseKeychain yes
638+
625639
# Custom SSH agent integration (1Password, gpg-agent)
626640
Host secure-*
627641
IdentityAgent ~/.1password/agent.sock

docs/man/bssh.1

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,31 @@ Possible values:
676676
Example:
677677
.I AddKeysToAgent yes
678678

679+
.TP
680+
.B UseKeychain
681+
.B (macOS only)
682+
Specifies whether to use the macOS Keychain for storing SSH key passphrases.
683+
This is an Apple-specific patch to OpenSSH and is only available on macOS systems.
684+
When enabled, passphrases are automatically retrieved from and stored in the macOS Keychain.
685+
.RS
686+
.IP \[bu] 2
687+
.B Implementation:
688+
Fully integrated with macOS Keychain via Security Framework. Passphrases are securely stored after successful authentication and retrieved on subsequent connections.
689+
.IP \[bu] 2
690+
.B Cross-platform compatibility:
691+
Use
692+
.I IgnoreUnknown UseKeychain
693+
before
694+
.I UseKeychain
695+
in your SSH config to prevent errors on non-macOS systems.
696+
.RE
697+
.IP
698+
Example:
699+
.nf
700+
.I IgnoreUnknown UseKeychain
701+
.I UseKeychain yes
702+
.fi
703+
679704
.TP
680705
.B IdentityAgent
681706
Specifies the path to the SSH agent socket to use for authentication.

examples/interactive_demo.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ async fn main() -> anyhow::Result<()> {
5353
key_path: None,
5454
use_agent: false,
5555
use_password: false,
56+
#[cfg(target_os = "macos")]
57+
use_keychain: false,
5658
strict_mode: StrictHostKeyChecking::AcceptNew,
5759
jump_hosts: None,
5860
pty_config: PtyConfig::default(),

src/app/dispatcher.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ use bssh::{
3030
};
3131
use std::path::{Path, PathBuf};
3232

33+
#[cfg(target_os = "macos")]
34+
use super::initialization::determine_use_keychain;
3335
use super::initialization::{determine_ssh_key_path, AppContext};
3436
use super::utils::format_duration;
3537

@@ -232,6 +234,9 @@ async fn handle_interactive_command(
232234
None
233235
};
234236

237+
#[cfg(target_os = "macos")]
238+
let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname.as_deref());
239+
235240
let interactive_cmd = InteractiveCommand {
236241
single_node: merged_mode.0,
237242
multiplex: merged_mode.1,
@@ -245,6 +250,8 @@ async fn handle_interactive_command(
245250
key_path,
246251
use_agent: cli.use_agent,
247252
use_password: cli.password,
253+
#[cfg(target_os = "macos")]
254+
use_keychain,
248255
strict_mode: ctx.strict_mode,
249256
jump_hosts: cli.jump_hosts.clone(),
250257
pty_config,
@@ -289,6 +296,9 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu
289296
None
290297
};
291298

299+
#[cfg(target_os = "macos")]
300+
let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname.as_deref());
301+
292302
let interactive_cmd = InteractiveCommand {
293303
single_node: true,
294304
multiplex: false,
@@ -302,6 +312,8 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu
302312
key_path,
303313
use_agent: cli.use_agent,
304314
use_password: cli.password,
315+
#[cfg(target_os = "macos")]
316+
use_keychain,
305317
strict_mode: ctx.strict_mode,
306318
jump_hosts: cli.jump_hosts.clone(),
307319
pty_config,
@@ -345,6 +357,10 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu
345357
ctx.cluster_name.as_deref().or(cli.cluster.as_deref()),
346358
);
347359

360+
// Determine if we should use macOS Keychain for passphrases
361+
#[cfg(target_os = "macos")]
362+
let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname.as_deref());
363+
348364
let params = ExecuteCommandParams {
349365
nodes: ctx.nodes.clone(),
350366
command,
@@ -354,6 +370,8 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu
354370
strict_mode: ctx.strict_mode,
355371
use_agent: cli.use_agent,
356372
use_password: cli.password,
373+
#[cfg(target_os = "macos")]
374+
use_keychain,
357375
output_dir: cli.output_dir.as_deref(),
358376
timeout,
359377
jump_hosts: cli.jump_hosts.as_deref(),

src/app/initialization.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,31 @@ pub fn determine_ssh_key_path(
204204
.get_ssh_key(cluster_name)
205205
.map(|ssh_key| bssh::config::expand_tilde(std::path::Path::new(&ssh_key)))
206206
}
207+
208+
/// Determine whether to use macOS Keychain for SSH key passphrases
209+
///
210+
/// This checks the SSH config for the UseKeychain option for a specific hostname.
211+
/// The option is only available on macOS.
212+
///
213+
/// # Arguments
214+
/// * `ssh_config` - The loaded SSH configuration
215+
/// * `hostname` - The target hostname to check (optional)
216+
///
217+
/// # Returns
218+
/// `true` if UseKeychain is enabled in SSH config, `false` otherwise
219+
#[cfg(target_os = "macos")]
220+
pub fn determine_use_keychain(ssh_config: &SshConfig, hostname: Option<&str>) -> bool {
221+
if let Some(host) = hostname {
222+
let host_config = ssh_config.find_host_config(host);
223+
host_config.use_keychain.unwrap_or(false)
224+
} else {
225+
false
226+
}
227+
}
228+
229+
/// Non-macOS version of determine_use_keychain (always returns false)
230+
#[cfg(not(target_os = "macos"))]
231+
#[allow(dead_code)]
232+
pub fn determine_use_keychain(_ssh_config: &SshConfig, _hostname: Option<&str>) -> bool {
233+
false
234+
}

src/commands/exec.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ pub struct ExecuteCommandParams<'a> {
3131
pub strict_mode: StrictHostKeyChecking,
3232
pub use_agent: bool,
3333
pub use_password: bool,
34+
#[cfg(target_os = "macos")]
35+
pub use_keychain: bool,
3436
pub output_dir: Option<&'a Path>,
3537
pub timeout: Option<u64>,
3638
pub jump_hosts: Option<&'a str>,
@@ -196,6 +198,10 @@ async fn execute_command_without_forwarding(params: ExecuteCommandParams<'_>) ->
196198
.with_timeout(params.timeout)
197199
.with_jump_hosts(params.jump_hosts.map(|s| s.to_string()));
198200

201+
// Set keychain usage if on macOS
202+
#[cfg(target_os = "macos")]
203+
let executor = executor.with_keychain(params.use_keychain);
204+
199205
let results = executor.execute(params.command).await?;
200206

201207
// Save outputs to files if output_dir is specified

0 commit comments

Comments
 (0)