From b003e49c9c47af36dbc4cba06c2507c8df2fa656 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Fri, 12 Jul 2024 07:54:43 -0700 Subject: [PATCH] rfc5321: split out banner_timeout from connect_timeout The motivation for this is: My test environment is not permitted to reach outbound port 25. If I run an ad-hoc test without setting up an explicit sink, I end up with messages that try to reach the public internet. Since they are blocked at a firewall, each of the MX hosts in the connection plan is subject to a 60s wait before trying the next thing. In addition, this can cause the shutdown to take longer while we wait for the in-flight delivery attempts to complete. Making a separate configuration option allows the local administrator to decide how to split the time waiting for a connection from the time waiting for the banner. refs: https://github.com/KumoCorp/kumomta/issues/196 --- crates/kumo-api-types/src/shaping.rs | 4 ++++ crates/kumod/src/smtp_dispatcher.rs | 14 ++++++-------- crates/rfc5321/src/client_types.rs | 12 ++++++++++++ crates/traffic-gen/src/main.rs | 17 ++++++++++------- docs/changelog/main.md | 7 +++++++ docs/reference/kumo/make_egress_path.md | 18 +++++++++++++++++- simple_policy.lua | 1 + 7 files changed, 57 insertions(+), 16 deletions(-) diff --git a/crates/kumo-api-types/src/shaping.rs b/crates/kumo-api-types/src/shaping.rs index 3743165fb..a25a30e68 100644 --- a/crates/kumo-api-types/src/shaping.rs +++ b/crates/kumo-api-types/src/shaping.rs @@ -730,6 +730,7 @@ MergedEntry { enable_dane: false, client_timeouts: SmtpClientTimeouts { connect_timeout: 60s, + banner_timeout: 60s, ehlo_timeout: 300s, mail_from_timeout: 300s, rcpt_to_timeout: 300s, @@ -865,6 +866,7 @@ MergedEntry { enable_dane: false, client_timeouts: SmtpClientTimeouts { connect_timeout: 60s, + banner_timeout: 60s, ehlo_timeout: 300s, mail_from_timeout: 300s, rcpt_to_timeout: 300s, @@ -942,6 +944,7 @@ MergedEntry { enable_dane: false, client_timeouts: SmtpClientTimeouts { connect_timeout: 60s, + banner_timeout: 60s, ehlo_timeout: 300s, mail_from_timeout: 300s, rcpt_to_timeout: 300s, @@ -1071,6 +1074,7 @@ MergedEntry { enable_dane: false, client_timeouts: SmtpClientTimeouts { connect_timeout: 60s, + banner_timeout: 60s, ehlo_timeout: 300s, mail_from_timeout: 300s, rcpt_to_timeout: 300s, diff --git a/crates/kumod/src/smtp_dispatcher.rs b/crates/kumod/src/smtp_dispatcher.rs index 0f81d3e5b..3ad3578d0 100644 --- a/crates/kumod/src/smtp_dispatcher.rs +++ b/crates/kumod/src/smtp_dispatcher.rs @@ -262,9 +262,11 @@ impl SmtpDispatcher { let tracer = self.tracer.clone(); async move { - let (stream, source_address) = egress_source - .connect_to(SocketAddr::new(address.addr, port)) - .await?; + let (stream, source_address) = tokio::time::timeout( + timeouts.connect_timeout, + egress_source.connect_to(SocketAddr::new(address.addr, port)), + ) + .await??; tracing::debug!( "connected to {address:?} port {port} via source address {source_address:?}" @@ -280,7 +282,7 @@ impl SmtpDispatcher { // Read banner let banner = client - .read_response(None, timeouts.connect_timeout) + .read_response(None, timeouts.banner_timeout) .await .context("reading banner")?; if banner.code != 220 { @@ -291,11 +293,7 @@ impl SmtpDispatcher { } }; - let connect_timeout = path_config.client_timeouts.connect_timeout; let mut client = tokio::select! { - _ = tokio::time::sleep(connect_timeout) => { - anyhow::bail!("exceeded timeout of {connect_timeout:?}"); - }, _ = shutdown.shutting_down() => anyhow::bail!("shutting down"), client = make_connection => { client }, } diff --git a/crates/rfc5321/src/client_types.rs b/crates/rfc5321/src/client_types.rs index 02bdc2d1a..6b98ae4af 100644 --- a/crates/rfc5321/src/client_types.rs +++ b/crates/rfc5321/src/client_types.rs @@ -9,6 +9,12 @@ pub struct SmtpClientTimeouts { )] pub connect_timeout: Duration, + #[serde( + default = "SmtpClientTimeouts::default_banner_timeout", + with = "duration_serde" + )] + pub banner_timeout: Duration, + #[serde( default = "SmtpClientTimeouts::default_ehlo_timeout", with = "duration_serde" @@ -66,6 +72,7 @@ impl Default for SmtpClientTimeouts { fn default() -> Self { Self { connect_timeout: Self::default_connect_timeout(), + banner_timeout: Self::default_banner_timeout(), ehlo_timeout: Self::default_ehlo_timeout(), mail_from_timeout: Self::default_mail_from_timeout(), rcpt_to_timeout: Self::default_rcpt_to_timeout(), @@ -83,6 +90,9 @@ impl SmtpClientTimeouts { fn default_connect_timeout() -> Duration { Duration::from_secs(60) } + fn default_banner_timeout() -> Duration { + Duration::from_secs(60) + } fn default_auth_timeout() -> Duration { Duration::from_secs(60) } @@ -115,6 +125,7 @@ impl SmtpClientTimeouts { let short = Duration::from_secs(20); Self { connect_timeout: short, + banner_timeout: short, ehlo_timeout: short, mail_from_timeout: short, rcpt_to_timeout: short, @@ -130,6 +141,7 @@ impl SmtpClientTimeouts { /// Compute theoretical maximum lifetime of a single message send pub fn total_message_send_duration(&self) -> Duration { self.connect_timeout + + self.banner_timeout + self.ehlo_timeout + self.auth_timeout + self.mail_from_timeout diff --git a/crates/traffic-gen/src/main.rs b/crates/traffic-gen/src/main.rs index 31bb3696c..e78bf61ff 100644 --- a/crates/traffic-gen/src/main.rs +++ b/crates/traffic-gen/src/main.rs @@ -169,15 +169,18 @@ impl Opt { } async fn make_client(&self) -> anyhow::Result { - let stream = TcpStream::connect(&self.target) - .await - .with_context(|| format!("connect to {}", self.target))?; - let mut client = - SmtpClient::with_stream(stream, &self.target, SmtpClientTimeouts::default()); + let timeouts = SmtpClientTimeouts::default(); + + let stream = + tokio::time::timeout(timeouts.connect_timeout, TcpStream::connect(&self.target)) + .await + .with_context(|| format!("timed out connecting to {}", self.target))? + .with_context(|| format!("failed to connect to {}", self.target))?; + let mut client = SmtpClient::with_stream(stream, &self.target, timeouts); // Read banner - let connect_timeout = client.timeouts().connect_timeout; - let banner = client.read_response(None, connect_timeout).await?; + let banner_timeout = client.timeouts().banner_timeout; + let banner = client.read_response(None, banner_timeout).await?; anyhow::ensure!(banner.code == 220, "unexpected banner: {banner:#?}"); // Say EHLO diff --git a/docs/changelog/main.md b/docs/changelog/main.md index e2d1ba47c..ab880d3d4 100644 --- a/docs/changelog/main.md +++ b/docs/changelog/main.md @@ -8,6 +8,13 @@ a multiarch image, supporting both `linux/amd64` and `linux/arm64` architectures. Simply use `docker pull ghcr.io/kumocorp/kumomta-dev:latest` to get the appropriate architecture. +* Split out the banner waiting portion of + [connect_timeout](../reference/kumo/make_egress_path.md#connect_timeout) into + a new [banner_timeout](../reference/kumo/make_egress_path.md#banner_timeout) + option to make it easier to manage the system behavior if, for example, the + connection is blocked by a firewall. You can now set the connection timeout + to a smaller value while keeping the banner timeout at a more RFC-compliant, + longer, value. #196 * New [kcli trace-smtp-client](../reference/kcli/trace-smtp-client.md) diagnostic command for observing outbound SMTP sessions. #87 * New [Extended configuration validation mode](../userguide/configuration/policy_helpers.md#validating-your-configuration). #211 diff --git a/docs/reference/kumo/make_egress_path.md b/docs/reference/kumo/make_egress_path.md index e4b13c1b0..69571662e 100644 --- a/docs/reference/kumo/make_egress_path.md +++ b/docs/reference/kumo/make_egress_path.md @@ -128,8 +128,24 @@ The following nine settings control the timeouts waiting for responses to variou The value is specified as a integer in seconds, or as a string using syntax like `"2min"` for a two minute duration. +## banner_timeout + +{{since('dev')}} + +How long to wait between a connection being established and receiving a 220 +from a receiving host. The default is `60s`. + +In earlier versions of KumoMTA, this was rolled together into the `connect_timeout`. + ## connect_timeout -How long to wait between starting an SMTP connection and receiving a 220 from a receiving host. The default is `60s`. + +How long to wait between starting an SMTP connection and receiving a 220 from a +receiving host. The default is `60s`. + +{{since('dev', inline=True)}} + The `connect_timeout` is now purely focused on the time it takes to + establish a working connection. The time allowed for receiving the + initial 220 banner has been separated out into `banner_timeout`. ## starttls_timeout How long to wait for a response after issuing a STARTTLS comand. The default is `5s`. diff --git a/simple_policy.lua b/simple_policy.lua index 3a3144e84..453e11fb8 100644 --- a/simple_policy.lua +++ b/simple_policy.lua @@ -262,6 +262,7 @@ kumo.on( idle_timeout = '25s', data_timeout = '20s', data_dot_timeout = '25s', + connect_timeout = '5s', connection_limit = 32, -- max_connection_rate = '1/s', -- max_ready = 16,