diff --git a/README.md b/README.md index 6a1a0e7f..5581fadb 100644 --- a/README.md +++ b/README.md @@ -252,7 +252,7 @@ bssh -C production --server-alive-interval 30 "long-running-job" bssh -C production --server-alive-interval 0 "quick-job" # Keepalive with custom max retries (default: 3) -bssh -C production --server-alive-interval 60 --server-alive-count-max 5 "long-running-job" +bssh -C production --server-alive-interval 30 --server-alive-count-max 5 "long-running-job" # Fail-fast mode: stop immediately on any failure (pdsh -k compatible) bssh -k -H "web1,web2,web3" "deploy.sh" @@ -733,7 +733,7 @@ defaults: parallel: 10 timeout: 300 # Command timeout in seconds (0 for unlimited) jump_host: bastion.example.com # Global default jump host (optional) - server_alive_interval: 60 # SSH keepalive interval in seconds (0 to disable) + server_alive_interval: 30 # SSH keepalive interval in seconds (0 to disable) server_alive_count_max: 3 # Max keepalive messages without response # Global interactive mode settings (optional) @@ -1196,7 +1196,7 @@ Options: -p, --parallel Maximum parallel connections [default: 10] --timeout Command timeout in seconds (0 for unlimited) [default: 300] --connect-timeout SSH connection timeout in seconds (minimum: 1) [default: 30] - --server-alive-interval SSH keepalive interval in seconds (0 to disable) [default: 60] + --server-alive-interval SSH keepalive interval in seconds (0 to disable) [default: 30] --server-alive-count-max Max keepalive messages without response [default: 3] --output-dir Output directory for command results -N, --no-prefix Disable hostname prefix in output (pdsh -N compatibility) @@ -1497,4 +1497,4 @@ See the [LICENSE](./LICENSE) file for details. - **v0.4.0 (2025/08/22):** Add password authentication, SSH key passphrase support, modern UI with colors, XDG config compliance, and Debian packaging - **v0.3.0 (2025/08/22):** Add native SFTP directory operations and recursive file transfer support - **v0.2.0 (2025/08/21):** Added Backend.AI multi-node session support with SSH authentication, host key verification, environment variable expansion, timeouts, and SCP file copy functionality. -- **v0.1.0 (2025/08/21):** Initial release with parallel SSH execution using async-ssh2-tokio \ No newline at end of file +- **v0.1.0 (2025/08/21):** Initial release with parallel SSH execution using async-ssh2-tokio diff --git a/src/cli/bssh.rs b/src/cli/bssh.rs index 008f1e61..cd050a35 100644 --- a/src/cli/bssh.rs +++ b/src/cli/bssh.rs @@ -192,7 +192,7 @@ pub struct Cli { #[arg( long = "server-alive-interval", value_name = "SECONDS", - help = "Keepalive interval in seconds (default: 60, 0 to disable)\nSends keepalive packets to prevent idle connection timeouts.\nMatches OpenSSH ServerAliveInterval option." + help = "Keepalive interval in seconds (default: 30, 0 to disable)\nSends keepalive packets to prevent idle connection timeouts.\nMatches OpenSSH ServerAliveInterval option." )] pub server_alive_interval: Option, diff --git a/src/config/types.rs b/src/config/types.rs index 7fd092fb..9a849f95 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -77,7 +77,7 @@ pub struct Defaults { pub jump_host: Option, /// SSH keepalive interval in seconds. /// Sends keepalive packets to prevent idle connection timeouts. - /// Default: 60 seconds. Set to 0 to disable. + /// Default: 30 seconds. Set to 0 to disable. pub server_alive_interval: Option, /// Maximum keepalive messages without response before disconnect. /// Default: 3 @@ -167,7 +167,7 @@ pub struct ClusterDefaults { pub jump_host: Option, /// SSH keepalive interval in seconds. /// Sends keepalive packets to prevent idle connection timeouts. - /// Default: 60 seconds. Set to 0 to disable. + /// Default: 30 seconds. Set to 0 to disable. pub server_alive_interval: Option, /// Maximum keepalive messages without response before disconnect. /// Default: 3 diff --git a/src/ssh/tokio_client/connection.rs b/src/ssh/tokio_client/connection.rs index 48121e46..e2283aa7 100644 --- a/src/ssh/tokio_client/connection.rs +++ b/src/ssh/tokio_client/connection.rs @@ -26,11 +26,16 @@ use std::{fmt::Debug, io}; use super::authentication::{AuthMethod, ServerCheckMethod}; /// Default keepalive interval in seconds. -/// Sends keepalive packets every 60 seconds to detect dead connections. -pub const DEFAULT_KEEPALIVE_INTERVAL: u64 = 60; +/// +/// This is intentionally below common 60-second idle reapers so the client +/// sends traffic before the remote side or an intermediate gateway decides the +/// session is idle. +pub const DEFAULT_KEEPALIVE_INTERVAL: u64 = 30; /// Default maximum keepalive attempts before considering connection dead. -/// With 60s interval and 3 max, connection failure is detected within 180s. +/// With the default interval and max, connection failure is detected within +/// about 120s: three unanswered probes plus the next timer tick that observes +/// they were missed. pub const DEFAULT_KEEPALIVE_MAX: usize = 3; /// SSH connection configuration for keepalive and timeout settings. @@ -44,12 +49,12 @@ pub const DEFAULT_KEEPALIVE_MAX: usize = 3; /// ```no_run /// use bssh::ssh::tokio_client::SshConnectionConfig; /// -/// // Use defaults (60s interval, 3 max attempts) +/// // Use defaults (30s interval, 3 max attempts) /// let config = SshConnectionConfig::default(); /// /// // Custom configuration /// let config = SshConnectionConfig::new() -/// .with_keepalive_interval(Some(30)) +/// .with_keepalive_interval(Some(15)) /// .with_keepalive_max(5); /// /// // Disable keepalive @@ -60,7 +65,7 @@ pub const DEFAULT_KEEPALIVE_MAX: usize = 3; pub struct SshConnectionConfig { /// Interval in seconds between keepalive packets. /// None disables keepalive. - /// Default: 60 seconds + /// Default: 30 seconds pub keepalive_interval: Option, /// Maximum number of keepalive packets to send without response @@ -85,10 +90,10 @@ impl SshConnectionConfig { } /// Set the keepalive interval in seconds. - /// Pass None to disable keepalive. + /// Pass None, or Some(0), to disable keepalive. #[must_use] pub fn with_keepalive_interval(mut self, interval: Option) -> Self { - self.keepalive_interval = interval; + self.keepalive_interval = interval.filter(|seconds| *seconds > 0); self } @@ -101,22 +106,16 @@ impl SshConnectionConfig { /// Convert this configuration to a russh client Config. /// - /// When keepalive is enabled, `inactivity_timeout` is set to `None` so the - /// keepalive mechanism is the sole dead-peer detector. russh's default - /// `inactivity_timeout` is 10 minutes and would otherwise tear down an - /// otherwise-healthy idle session at that mark regardless of keepalive - /// liveness. When keepalive is disabled, we preserve a generous - /// inactivity timeout so truly dead sockets are still reaped. + /// `inactivity_timeout` stays disabled for client sessions. A healthy + /// interactive SSH session can legitimately produce no inbound data for a + /// long time, so inactivity must not be treated as a local reason to close + /// it. When keepalive is enabled, russh's keepalive counter is the liveness + /// detector; when it is disabled, the client leaves idle sessions alone. pub fn to_russh_config(&self) -> Config { - let inactivity_timeout = if self.keepalive_interval.is_some() { - None - } else { - Some(Duration::from_secs(3600)) - }; Config { keepalive_interval: self.keepalive_interval.map(Duration::from_secs), keepalive_max: self.keepalive_max, - inactivity_timeout, + inactivity_timeout: None, ..Default::default() } } @@ -128,7 +127,7 @@ impl SshConnectionConfig { /// detect a broken TCP path even when no application data is flowing and /// even if SSH-level keepalive replies are dropped by a middlebox. pub fn to_tcp_keepalive(&self) -> Option { - let interval = self.keepalive_interval?; + let interval = self.keepalive_interval.filter(|seconds| *seconds > 0)?; // Start probing after `interval` seconds of idleness, probe every // half-interval, up to keepalive_max retries. let probe_interval = (interval / 2).max(1); @@ -200,7 +199,7 @@ impl Client { /// Authentification is tried on the first successful connection and the whole /// process aborted if this fails. /// - /// This method uses default keepalive settings (60s interval, 3 max attempts) + /// This method uses default keepalive settings (30s interval, 3 max attempts) /// to prevent idle connection timeouts. pub async fn connect( addr: impl ToSocketAddrsWithHostname, @@ -231,7 +230,7 @@ impl Client { /// #[tokio::main] /// async fn main() -> Result<(), bssh::ssh::tokio_client::Error> { /// let ssh_config = SshConnectionConfig::new() - /// .with_keepalive_interval(Some(30)) + /// .with_keepalive_interval(Some(15)) /// .with_keepalive_max(5); /// /// let client = Client::connect_with_ssh_config( diff --git a/tests/ssh_keepalive_test.rs b/tests/ssh_keepalive_test.rs index 12436048..a384f566 100644 --- a/tests/ssh_keepalive_test.rs +++ b/tests/ssh_keepalive_test.rs @@ -39,6 +39,11 @@ fn test_ssh_connection_config_default_values() { Some(DEFAULT_KEEPALIVE_INTERVAL), "Default keepalive interval should be {DEFAULT_KEEPALIVE_INTERVAL}" ); + let interval = config.keepalive_interval.unwrap_or(u64::MAX); + assert!( + interval < 60, + "Default keepalive interval should beat common 60-second idle reapers" + ); assert_eq!( config.keepalive_max, DEFAULT_KEEPALIVE_MAX, "Default keepalive max should be {DEFAULT_KEEPALIVE_MAX}" @@ -133,6 +138,18 @@ fn test_ssh_connection_config_to_russh_config() { ); } +#[test] +fn test_ssh_connection_config_to_russh_config_never_closes_healthy_idle_client() { + let config = SshConnectionConfig::default(); + + let russh_config = config.to_russh_config(); + + assert_eq!( + russh_config.inactivity_timeout, None, + "client-side inactivity timeout should not close healthy idle interactive sessions" + ); +} + #[test] fn test_ssh_connection_config_to_russh_config_disabled() { let config = SshConnectionConfig::new().with_keepalive_interval(None); @@ -143,6 +160,10 @@ fn test_ssh_connection_config_to_russh_config_disabled() { russh_config.keepalive_interval, None, "russh config should have disabled keepalive" ); + assert_eq!( + russh_config.inactivity_timeout, None, + "disabling keepalive should not install a hidden idle-session timeout" + ); } #[test] @@ -152,11 +173,13 @@ fn test_ssh_connection_config_zero_interval() { let russh_config = config.to_russh_config(); - // russh will interpret Duration::from_secs(0) as disabled assert_eq!( - russh_config.keepalive_interval, - Some(std::time::Duration::from_secs(0)), - "Zero interval should be passed through (russh interprets as disabled)" + config.keepalive_interval, None, + "Zero interval should be normalized to disabled" + ); + assert_eq!( + russh_config.keepalive_interval, None, + "Zero interval must not create an immediately-ready keepalive timer" ); }