fix: Add password fallback and improve SSH debugging for compatibility

[inureyes]

Summary

This PR addresses SSH connection compatibility issues where bssh fails to connect to servers that work with OpenSSH (Issue #79).

Problem

• OpenSSH client successfully connects to certain servers using password authentication as fallback
• bssh fails with "SSH connection failed" without attempting password fallback
• Debug information was insufficient to diagnose the root cause

Root Cause Analysis

When the server's authorized_keys doesn't include the user's SSH public key, OpenSSH automatically falls back to password authentication. bssh was not implementing this fallback behavior.

Changes

1. Password Fallback (src/commands/interactive/connection.rs)

   • Added automatic password fallback when key authentication fails
   • Matches OpenSSH behavior for interactive sessions
   • Only prompts in interactive terminals (TTY check)

2. Improved Debug Logging (src/utils/logging.rs)

   • Support RUST_LOG environment variable for debugging
   • -vv flag now includes russh library debug logs
   • -vvv flag includes trace-level logs for all SSH components
   • Show module targets for better diagnostics

3. SSH Agent Auto-detection (src/ssh/auth.rs)

   • Auto-detect SSH agent when available (without requiring --use-agent flag)
   • Matches OpenSSH default behavior

Test Plan

• Test connection with password fallback (key rejected → password prompt)
• Test -vv flag shows russh debug logs
• Test RUST_LOG=debug works correctly
• Build passes with no warnings

Related Issues

Closes #79

[inureyes]

Security & Performance Review

Analysis Summary

• Scope: changed-files
• Languages: Rust
• Total issues: Analyzing...
• PR Changes: Password fallback, SSH agent auto-detection, RUST_LOG support

Prioritized Fix Roadmap

Status: Analysis in Progress

Currently analyzing:

• Password handling security
• Timing attack mitigations
• Credential exposure risks
• SSH agent auto-detection logic
• Logging security
• Code quality issues

This comment will be updated with findings.

[inureyes]

Security & Performance Review - Complete Analysis

Analysis Summary

• Scope: changed-files (3 files)
• Languages: Rust
• Files Analyzed:
  • src/commands/interactive/connection.rs (password fallback logic)
  • src/ssh/auth.rs (SSH agent auto-detection)
  • src/utils/logging.rs (RUST_LOG support)
• Risk Level: Medium

---

Security Review Results

Password Handling Security: PASS

• Password is wrapped in Zeroizing<String> for secure memory cleanup
• Password prompt uses rpassword crate (secure terminal input)
• Password fallback only triggers in interactive terminals (TTY check)
• Rate limiting delay (500ms) prevents rapid brute-force attempts
• Timing attack mitigation present (minimum 500ms authentication duration)

Credential Exposure Risk: PASS

• Passwords are not logged at any verbosity level
• Zeroizing wrapper ensures automatic memory zeroing on drop
• Password is passed by reference to AuthMethod::with_password()

SSH Agent Auto-detection Logic: PASS (with notes)

The change from if self.use_agent to if !self.use_agent is intentional and correct:

• Priority 2 (line 215): Handles explicit --use-agent flag
• Priority 4 (line 229): Auto-detects agent when flag NOT specified (new OpenSSH-like behavior)
• This matches OpenSSH default behavior where agent is tried automatically

Logging Security: PASS

• RUST_LOG support is safe - only enables more verbose logging
• Module targets shown with with_target(true) - useful for debugging, no security risk
• russh debug logs may expose protocol details but not credentials

---

Potential Issues Identified

LOW: Password Validation Enhancement

File: src/commands/interactive/connection.rs (line 120-133)
Issue: No validation on password input (empty password check, maximum length)
Risk: Low - empty passwords and very long inputs are handled by SSH server
Recommendation: Consider adding client-side validation for better UX

LOW: Inconsistent Error Context

File: src/commands/interactive/connection.rs (line 101 vs 103)
Issue: Error messages differ slightly between password fallback and direct failure:

• Line 101: "Password authentication failed for {host}:{port}"
• Line 103: "SSH connection failed to {host}:{port}"
  Risk: Minor - could confuse debugging
  Recommendation: Standardize error message format

INFO: Debug Log Verbosity

File: src/utils/logging.rs
Note: -vv and -vvv flags now enable russh library debug/trace logs
Security: These may expose SSH protocol details (algorithm negotiation, channel IDs)
Recommendation: Document that high verbosity should only be used for debugging

---

Code Quality Assessment

Positive Findings:

1. Proper async handling: Password prompt uses spawn_blocking to avoid blocking async runtime
2. Memory safety: Zeroizing wrapper used consistently for sensitive data
3. TTY detection: atty::is(atty::Stream::Stdin) prevents password prompts in non-interactive mode
4. Rate limiting: Built-in delays prevent brute-force attacks
5. Timing normalization: Authentication timing is normalized to prevent timing attacks
6. Clean error handling: Uses anyhow::Context for proper error chaining

Minor Suggestions:

1. The info log at line 232-234 could be debug level instead
2. Consider adding a maximum retry count for password fallback

---

Test Coverage

• All existing tests pass (16 unit tests, 5 streaming tests, 3 timeout tests, 3 upload tests, 13 doc tests)
• No new tests added for password fallback functionality

---

Verdict

APPROVED with minor suggestions

The PR implements password fallback and SSH agent auto-detection correctly:

• Security measures are properly implemented (Zeroizing, TTY check, rate limiting)
• The logic changes are intentional and match OpenSSH behavior
• No critical or high severity issues found
• Code quality is good with proper error handling

The changes improve compatibility with servers that require password authentication as fallback, matching OpenSSH's default behavior.

---

Review completed by AI Code Reviewer