diff --git a/docs/agent-data-plane/configuration/configuration.md b/docs/agent-data-plane/configuration/configuration.md index d52e537fc41..70e62d8b22f 100644 --- a/docs/agent-data-plane/configuration/configuration.md +++ b/docs/agent-data-plane/configuration/configuration.md @@ -29,7 +29,6 @@ tracking. | -------------------------------------------- | ----------------------------------------------- | ------- | | `dogstatsd_experimental_http.enabled` | Enable experimental HTTP/H2C DSD listener | [#1682] | | `dogstatsd_experimental_http.listen_address` | Bind address for experimental HTTP DSD listener | [#1682] | -| `tls_handshake_timeout` | HTTP TLS handshake timeout | [#178] | ### Not Planned @@ -801,6 +800,7 @@ Both commands scrub recognized secret values before writing JSON to standard out | `syslog_rfc` | Use RFC-style syslog header | | `syslog_uri` | Syslog destination URI | | `telemetry.dogstatsd_origin` | Per-origin processed-metrics telemetry | +| `tls_handshake_timeout` | HTTP TLS handshake timeout | | `use_proxy_for_cloud_metadata` | Proxy cloud metadata endpoints | | `use_v2_api.series` | Send series via V2 protobuf endpoint | | `use_v3_api.series.enabled` | Global V3 series mode | diff --git a/lib/agent-data-plane-config-system/src/translators/datadog_translator.rs b/lib/agent-data-plane-config-system/src/translators/datadog_translator.rs index 638874e15f7..97cf361e939 100644 --- a/lib/agent-data-plane-config-system/src/translators/datadog_translator.rs +++ b/lib/agent-data-plane-config-system/src/translators/datadog_translator.rs @@ -841,6 +841,10 @@ impl DatadogConfigWitness for DatadogTranslator<'_> { self.config.shared.endpoints.tls.min_tls_version = value; } + fn consume_tls_handshake_timeout(&mut self, value: Duration) { + self.config.shared.endpoints.tls.handshake_timeout = value; + } + fn consume_multi_region_failover_api_key(&mut self, value: String) { self.config.domains.multi_region_failover.api_key = non_empty(value); } diff --git a/lib/agent-data-plane-config/src/shared.rs b/lib/agent-data-plane-config/src/shared.rs index 9f127b69b97..de58519214d 100644 --- a/lib/agent-data-plane-config/src/shared.rs +++ b/lib/agent-data-plane-config/src/shared.rs @@ -120,6 +120,12 @@ pub struct Tls { /// Path to which TLS session keys are logged, for debugging. pub sslkeylogfile: String, + + /// Timeout for completing the TLS handshake after a connection is established. + /// + /// Defaults to 10 seconds. Bounds only the handshake step, distinct from the overall request timeout. A value + /// of zero disables the handshake-specific deadline, leaving the overall request timeout as the only bound. + pub handshake_timeout: Duration, } /// Payload compression settings applied before transmission. diff --git a/lib/datadog-agent/config-testing/build/registry_gen.rs b/lib/datadog-agent/config-testing/build/registry_gen.rs index 606126a826b..9a65a213d7d 100644 --- a/lib/datadog-agent/config-testing/build/registry_gen.rs +++ b/lib/datadog-agent/config-testing/build/registry_gen.rs @@ -172,6 +172,7 @@ static GOLDEN_ORDER: &[(&str, &[&str])] = &[ "forwarder_storage_max_size_in_bytes", "forwarder_storage_path", "forwarder_outdated_file_in_days", + "tls_handshake_timeout", ], ), ( diff --git a/lib/datadog-agent/config-testing/src/config_registry/forwarder.rs b/lib/datadog-agent/config-testing/src/config_registry/forwarder.rs index e48fad7abbe..1b7cef62cb2 100644 --- a/lib/datadog-agent/config-testing/src/config_registry/forwarder.rs +++ b/lib/datadog-agent/config-testing/src/config_registry/forwarder.rs @@ -313,6 +313,17 @@ crate::declare_annotations! { test_json: None, pipeline_affinity: PipelineAffinity::CrossCutting, }; + /// `tls_handshake_timeout`-HTTP TLS handshake timeout + TLS_HANDSHAKE_TIMEOUT = SalukiAnnotation { + schema: &schema::TLS_HANDSHAKE_TIMEOUT, + support_level: SupportLevel::Full, + additional_yaml_paths: &[], + env_var_override: None, + used_by: &[structs::FORWARDER_CONFIGURATION], + value_type_override: None, + test_json: None, + pipeline_affinity: PipelineAffinity::CrossCutting, + }; /// `forwarder_apikey_validation_interval`-API key check interval (minutes) FORWARDER_APIKEY_VALIDATION_INTERVAL = SalukiAnnotation { schema: &schema::FORWARDER_APIKEY_VALIDATION_INTERVAL, diff --git a/lib/datadog-agent/config-testing/src/config_registry/unsupported.rs b/lib/datadog-agent/config-testing/src/config_registry/unsupported.rs index 3807475e59d..43ed24bd7e6 100644 --- a/lib/datadog-agent/config-testing/src/config_registry/unsupported.rs +++ b/lib/datadog-agent/config-testing/src/config_registry/unsupported.rs @@ -60,17 +60,6 @@ crate::declare_annotations! { test_json: None, pipeline_affinity: PipelineAffinity::CrossCutting, }; - /// `tls_handshake_timeout`-HTTP TLS handshake timeout - TLS_HANDSHAKE_TIMEOUT = SalukiAnnotation { - schema: &schema::TLS_HANDSHAKE_TIMEOUT, - support_level: SupportLevel::Incompatible(Severity::Medium), - additional_yaml_paths: &[], - env_var_override: None, - used_by: &[], - value_type_override: None, - test_json: None, - pipeline_affinity: PipelineAffinity::CrossCutting, - }; /// `aggregator_buffer_size`-Channel buffer depth for aggregator queues AGGREGATOR_BUFFER_SIZE = SalukiAnnotation { schema: &schema::AGGREGATOR_BUFFER_SIZE, diff --git a/lib/datadog-agent/config/schema/schema_overlay.yaml b/lib/datadog-agent/config/schema/schema_overlay.yaml index 2f5242b4771..207a70c6cac 100644 --- a/lib/datadog-agent/config/schema/schema_overlay.yaml +++ b/lib/datadog-agent/config/schema/schema_overlay.yaml @@ -2593,12 +2593,18 @@ inventory: config_registry_filename: dogstatsd.rs tls_handshake_timeout: - support: none - severity: medium - planned: true + support: full pipelines: [cross_cutting] description: "HTTP TLS handshake timeout" - documentation: "Existing request timeout covers the gap." + documentation: | + ADP applies `tls_handshake_timeout` to the TLS handshake step of outbound Datadog intake connections, + independent of `forwarder_timeout`, which bounds the full request. ADP owns the TLS layer directly rather than + delegating the handshake to an opaque connect-and-handshake future, so it can time out the handshake without + also aborting an in-progress TCP connect. + test_support: + used_by: [ForwarderConfiguration] + additional_attributes: + config_registry_filename: forwarder.rs issue: "#178" use_dogstatsd: diff --git a/lib/datadog-agent/config/src/classifier/classifier.rs b/lib/datadog-agent/config/src/classifier/classifier.rs index 9c390ee3b85..ff7aaf538ce 100644 --- a/lib/datadog-agent/config/src/classifier/classifier.rs +++ b/lib/datadog-agent/config/src/classifier/classifier.rs @@ -115,7 +115,9 @@ mod tests { #[test] fn incompatible_non_default() { let c = classifier(); - let result = c.classify("tls_handshake_timeout", &Value::Number(999.into())).unwrap(); + let result = c + .classify("dogstatsd_stats_buffer", &Value::Number(999.into())) + .unwrap(); assert!(matches!(result.support_level, SupportLevel::Incompatible(_))); assert!(!result.is_default); } @@ -139,8 +141,10 @@ mod tests { #[test] fn duration_default_null_is_not_default() { let c = classifier(); - // tls_handshake_timeout has a duration default (10s); a null value can't be normalized. - let result = c.classify("tls_handshake_timeout", &Value::Null).unwrap(); + // dogstatsd_packet_buffer_flush_timeout has a duration default (100ms); a null value can't be normalized. + let result = c + .classify("dogstatsd_packet_buffer_flush_timeout", &Value::Null) + .unwrap(); assert!(!result.is_default); } @@ -149,14 +153,17 @@ mod tests { let c = classifier(); // Neither an empty string nor arbitrary text parses as a duration, so neither matches. assert!( - !c.classify("tls_handshake_timeout", &Value::String("".into())) + !c.classify("dogstatsd_packet_buffer_flush_timeout", &Value::String("".into())) .unwrap() .is_default ); assert!( - !c.classify("tls_handshake_timeout", &Value::String("something".into())) - .unwrap() - .is_default + !c.classify( + "dogstatsd_packet_buffer_flush_timeout", + &Value::String("something".into()) + ) + .unwrap() + .is_default ); } @@ -165,7 +172,7 @@ mod tests { let c = classifier(); // The default is also matched when supplied as a Go duration string rather than nanoseconds. let result = c - .classify("tls_handshake_timeout", &Value::String("10s".into())) + .classify("dogstatsd_packet_buffer_flush_timeout", &Value::String("100ms".into())) .unwrap(); assert!(result.is_default); } @@ -173,7 +180,7 @@ mod tests { #[test] fn incompatible_severity_levels() { let c = classifier(); - let result = c.classify("tls_handshake_timeout", &Value::Number(30.into())).unwrap(); + let result = c.classify("dogstatsd_stats_buffer", &Value::Number(30.into())).unwrap(); assert!(matches!( result.support_level, SupportLevel::Incompatible(Severity::Medium) @@ -183,10 +190,13 @@ mod tests { #[test] fn duration_default_as_nanoseconds_is_default() { let c = classifier(); - // The Agent transmits tls_handshake_timeout (schema default "10s") as integer nanoseconds. - // The classifier must recognize this as the default and not flag it as an override. + // The Agent transmits dogstatsd_packet_buffer_flush_timeout (schema default "100ms") as integer + // nanoseconds. The classifier must recognize this as the default and not flag it as an override. let result = c - .classify("tls_handshake_timeout", &Value::Number(10_000_000_000i64.into())) + .classify( + "dogstatsd_packet_buffer_flush_timeout", + &Value::Number(100_000_000i64.into()), + ) .unwrap(); assert!(result.is_default); } @@ -194,9 +204,12 @@ mod tests { #[test] fn duration_non_default_nanoseconds_is_not_default() { let c = classifier(); - // 5s in nanoseconds is not the 10s default. + // 5ms in nanoseconds is not the 100ms default. let result = c - .classify("tls_handshake_timeout", &Value::Number(5_000_000_000i64.into())) + .classify( + "dogstatsd_packet_buffer_flush_timeout", + &Value::Number(5_000_000i64.into()), + ) .unwrap(); assert!(!result.is_default); } diff --git a/lib/datadog-agent/config/src/generated/classifier_data.rs b/lib/datadog-agent/config/src/generated/classifier_data.rs index 7a52e50683a..0cac8b298ec 100644 --- a/lib/datadog-agent/config/src/generated/classifier_data.rs +++ b/lib/datadog-agent/config/src/generated/classifier_data.rs @@ -319,13 +319,6 @@ pub(crate) static CLASSIFIER_ENTRIES: &[ClassifierEntry] = &[ pipeline_affinity: PipelineAffinity::Pipelines(&[Pipeline::DogStatsD]), default: DefaultValue::Json("[]"), }, - ClassifierEntry { - yaml_path: "tls_handshake_timeout", - aliases: &[], - support_level: SupportLevel::Incompatible(Severity::Medium), - pipeline_affinity: PipelineAffinity::CrossCutting, - default: DefaultValue::DurationNanos(10000000000), - }, ClassifierEntry { yaml_path: "use_dogstatsd", aliases: &[], diff --git a/lib/datadog-agent/config/src/generated/datadog_configuration.rs b/lib/datadog-agent/config/src/generated/datadog_configuration.rs index 5eaef3ea71b..a11ee8bb42b 100644 --- a/lib/datadog-agent/config/src/generated/datadog_configuration.rs +++ b/lib/datadog-agent/config/src/generated/datadog_configuration.rs @@ -412,6 +412,13 @@ pub struct DatadogConfiguration { #[serde(default)] pub telemetry: Telemetry, + #[serde( + default = "duration_defaults::tls_handshake_timeout", + + deserialize_with = "crate::duration_de::deserialize_go_duration" + )] + pub tls_handshake_timeout: std::time::Duration, + #[serde(default)] pub use_proxy_for_cloud_metadata: bool, @@ -561,6 +568,7 @@ impl Default for DatadogConfiguration { syslog_rfc: Default::default(), syslog_uri: Default::default(), telemetry: Default::default(), + tls_handshake_timeout: duration_defaults::tls_handshake_timeout(), use_proxy_for_cloud_metadata: Default::default(), use_v2_api: Default::default(), use_v3_api: Default::default(), @@ -1869,4 +1877,7 @@ mod duration_defaults { pub(super) fn expected_tags_duration() -> std::time::Duration { std::time::Duration::from_nanos(0) } + pub(super) fn tls_handshake_timeout() -> std::time::Duration { + std::time::Duration::from_nanos(10000000000) + } } diff --git a/lib/datadog-agent/config/src/generated/env_keys.rs b/lib/datadog-agent/config/src/generated/env_keys.rs index d7c04c0dbac..3220c3976ba 100644 --- a/lib/datadog-agent/config/src/generated/env_keys.rs +++ b/lib/datadog-agent/config/src/generated/env_keys.rs @@ -995,6 +995,11 @@ pub static DATADOG_ENV_KEYS: &[EnvKey] = &[ path: &["telemetry", "dogstatsd_origin"], decode: EnvDecode::Bool, }, + EnvKey { + env_vars: &["DD_TLS_HANDSHAKE_TIMEOUT"], + path: &["tls_handshake_timeout"], + decode: EnvDecode::DurationString, + }, EnvKey { env_vars: &["DD_USE_PROXY_FOR_CLOUD_METADATA"], path: &["use_proxy_for_cloud_metadata"], diff --git a/lib/datadog-agent/config/src/generated/witness.rs b/lib/datadog-agent/config/src/generated/witness.rs index cf85f5e9966..6d9b0ded899 100644 --- a/lib/datadog-agent/config/src/generated/witness.rs +++ b/lib/datadog-agent/config/src/generated/witness.rs @@ -209,6 +209,7 @@ pub trait DatadogConfigWitness { fn consume_syslog_rfc(&mut self, value: bool); fn consume_syslog_uri(&mut self, value: String); fn consume_telemetry_dogstatsd_origin(&mut self, value: bool); + fn consume_tls_handshake_timeout(&mut self, value: std::time::Duration); fn consume_use_proxy_for_cloud_metadata(&mut self, value: bool); fn consume_use_v2_api_series(&mut self, value: bool); fn consume_use_v3_api_series_enabled(&mut self, value: String); @@ -524,6 +525,7 @@ pub fn drive(config: &DatadogConfiguration, consumer: &mut impl DatadogConfigWit consumer.consume_syslog_rfc(config.syslog_rfc.clone()); consumer.consume_syslog_uri(config.syslog_uri.clone()); consumer.consume_telemetry_dogstatsd_origin(config.telemetry.dogstatsd_origin.clone()); + consumer.consume_tls_handshake_timeout(config.tls_handshake_timeout.clone()); consumer.consume_use_proxy_for_cloud_metadata(config.use_proxy_for_cloud_metadata.clone()); consumer.consume_use_v2_api_series(config.use_v2_api.series.clone()); consumer.consume_use_v3_api_series_enabled(config.use_v3_api.series.enabled.clone()); diff --git a/lib/saluki-components/src/common/datadog/config.rs b/lib/saluki-components/src/common/datadog/config.rs index 1764143a068..882a090ed30 100644 --- a/lib/saluki-components/src/common/datadog/config.rs +++ b/lib/saluki-components/src/common/datadog/config.rs @@ -2,7 +2,7 @@ use std::time::Duration; use agent_data_plane_config::shared::{Endpoints, MetricsEncoding}; use facet::Facet; -use saluki_config::GenericConfiguration; +use saluki_config::{DurationString, GenericConfiguration}; use saluki_error::GenericError; use saluki_io::net::client::http::{HttpProtocol, TlsMinimumVersion}; use serde::Deserialize; @@ -53,6 +53,10 @@ fn default_min_tls_version() -> String { MIN_TLS_VERSION_TLS12.to_string() } +fn default_tls_handshake_timeout() -> DurationString { + DurationString::new(Duration::from_secs(10)) +} + fn min_tls_version_from_config_value(value: &str) -> TlsMinimumVersion { let trimmed = value.trim(); match trimmed.to_lowercase().as_str() { @@ -350,6 +354,15 @@ pub struct ForwarderConfiguration { #[facet(opaque)] parsed_min_tls_version: TlsMinimumVersion, + /// Timeout for completing the TLS handshake after a connection is established, for Datadog intake forwarding. + /// + /// Defaults to 10 seconds. This bounds only the TLS handshake step, distinct from `forwarder_timeout`, which + /// bounds the entire request. A value of `0` disables the handshake deadline entirely, matching the core Agent + /// convention for this setting. + #[serde(default = "default_tls_handshake_timeout")] + #[facet(opaque)] + tls_handshake_timeout: DurationString, + /// Whether to signal that the backend should allow arbitrary tag values. /// /// Defaults to `false`. If set to `true`, the Datadog forwarder adds `Allow-Arbitrary-Tag-Value: true` to every @@ -436,6 +449,11 @@ impl ForwarderConfiguration { Duration::from_secs(self.request_timeout_secs) } + /// Returns the TLS handshake timeout. + pub const fn tls_handshake_timeout(&self) -> Duration { + self.tls_handshake_timeout.as_duration() + } + /// Returns the maximum number of pending requests for an individual endpoint. pub const fn endpoint_buffer_size(&self) -> usize { self.endpoint_buffer_size diff --git a/lib/saluki-components/src/common/datadog/io.rs b/lib/saluki-components/src/common/datadog/io.rs index b9c8833d9fc..29b502800db 100644 --- a/lib/saluki-components/src/common/datadog/io.rs +++ b/lib/saluki-components/src/common/datadog/io.rs @@ -331,6 +331,7 @@ where .with_request_timeout(config.request_timeout()) .with_max_idle_conns_per_host(config.max_idle_connections_per_host()) .with_min_tls_version(config.min_tls_version()) + .with_tls_handshake_timeout(config.tls_handshake_timeout()) .with_http_protocol(config.http_protocol()) .with_bytes_sent_counter(telemetry.bytes_sent().clone()) .with_endpoint_telemetry( diff --git a/lib/saluki-io/src/net/client/http/client.rs b/lib/saluki-io/src/net/client/http/client.rs index a14846a1f46..46d511a072e 100644 --- a/lib/saluki-io/src/net/client/http/client.rs +++ b/lib/saluki-io/src/net/client/http/client.rs @@ -181,6 +181,17 @@ impl HttpClientBuilder { self } + /// Sets the timeout for completing the TLS handshake after a connection is established. + /// + /// This bounds only the TLS handshake step, distinct from the connect timeout, which bounds the underlying TCP + /// (or other transport) connection setup that precedes it. + /// + /// Defaults to 10 seconds. + pub fn with_tls_handshake_timeout(mut self, timeout: Duration) -> Self { + self.connector_builder = self.connector_builder.with_tls_handshake_timeout(timeout); + self + } + /// Sets the per-request timeout. /// /// The request timeout applies to each individual request made to the remote host, including each request made when @@ -467,6 +478,43 @@ mod tests { assert_eq!(negotiated_alpn, None); } + #[tokio::test] + async fn tls_handshake_timeout_fires_against_a_stalled_server() { + initialize_crypto_provider(); + let server_cert = SelfSignedCert::localhost(); + + let listener = TcpListener::bind("127.0.0.1:0").await.expect("should bind listener"); + let port = listener.local_addr().expect("should have local address").port(); + let server_task = tokio::spawn(async move { + let (stream, _) = listener.accept().await.expect("server should accept a connection"); + // Accept the TCP connection but never complete the TLS handshake, so the client's handshake + // deadline is the only thing that can end the connection attempt. + tokio::time::sleep(Duration::from_secs(30)).await; + drop(stream); + }); + + let mut client = HttpClient::builder() + .with_tls_config(|builder| builder.with_root_cert_store(root_store(&server_cert))) + .with_tls_handshake_timeout(Duration::from_millis(200)) + .with_http_protocol(HttpProtocol::Http1) + .build() + .expect("client should build"); + let request = Request::get(format!("https://localhost:{port}/")) + .body(Empty::::new()) + .expect("request should build"); + + let error = timeout(Duration::from_secs(5), client.send(request)) + .await + .expect("request should not hit the outer test timeout") + .expect_err("handshake should time out before completing"); + assert!( + format!("{error:#}").contains("TLS handshake timed out"), + "expected a TLS handshake timeout, got: {error:#}" + ); + + server_task.abort(); + } + fn mutual_tls_configs() -> (ServerConfig, ClientConfig) { let server_cert = SelfSignedCert::localhost(); let client_cert = SelfSignedCert::new(["saluki-client"]); diff --git a/lib/saluki-io/src/net/client/http/conn.rs b/lib/saluki-io/src/net/client/http/conn.rs index bfcc427abbd..8db861cba4c 100644 --- a/lib/saluki-io/src/net/client/http/conn.rs +++ b/lib/saluki-io/src/net/client/http/conn.rs @@ -1,24 +1,26 @@ +#[cfg(unix)] +use std::path::PathBuf; use std::{ future::Future, io, pin::Pin, + sync::Arc, task::{Context, Poll}, time::{Duration, Instant}, }; -#[cfg(unix)] -use std::{path::PathBuf, sync::Arc}; use http::{Extensions, Uri}; -use hyper_rustls::{HttpsConnector, HttpsConnectorBuilder, MaybeHttpsStream}; +use hyper_rustls::MaybeHttpsStream; use hyper_util::{ client::legacy::connect::{CaptureConnection, Connected, Connection, HttpConnector}, rt::TokioIo, }; use metrics::Counter; use pin_project_lite::pin_project; -use rustls::ClientConfig; +use rustls::{pki_types::ServerName, ClientConfig}; use saluki_error::GenericError; use tokio::net::TcpStream; +use tokio_rustls::TlsConnector; #[cfg(target_os = "linux")] use tokio_vsock::{VsockAddr, VsockStream}; use tower::{BoxError, Service}; @@ -359,9 +361,15 @@ pub enum HttpProtocol { } /// A connector that supports HTTP or HTTPS. +/// +/// Unlike [`hyper_rustls::HttpsConnector`], which fuses the transport connect and TLS handshake into a single +/// opaque future, this connector performs them as two distinct steps. That split allows a timeout to be scoped to +/// just the handshake, rather than the combined connect-and-handshake duration. #[derive(Clone)] pub struct HttpsCapableConnector { - inner: HttpsConnector, + inner: InnerConnector, + tls_config: Arc, + tls_handshake_timeout: Duration, bytes_sent: Option, error_telemetry: Option, conn_age_limit: Option, @@ -377,31 +385,91 @@ impl Service for HttpsCapableConnector { } fn call(&mut self, dst: Uri) -> Self::Future { - let inner = self.inner.call(dst); + let is_https = match dst.scheme_str() { + Some("https") => true, + Some("http") => false, + scheme => { + let scheme = scheme.map(str::to_owned); + return Box::pin(async move { + Err(Box::new(io::Error::new( + io::ErrorKind::InvalidInput, + format!("unsupported URI scheme: {scheme:?}"), + )) as BoxError) + }); + } + }; + let transport_fut = self.inner.call(dst.clone()); + let tls_config = Arc::clone(&self.tls_config); + let tls_handshake_timeout = self.tls_handshake_timeout; let bytes_sent = self.bytes_sent.clone(); let error_telemetry = self.error_telemetry.clone(); let conn_age_limit = self.conn_age_limit; + Box::pin(async move { - match inner.await { - Ok(inner) => Ok(HttpsCapableConnection { - inner, - bytes_sent, - error_telemetry, - conn_age_limit, - }), - Err(error) => { - if is_tls_error(error.as_ref()) { + let transport = transport_fut.await?; + + let inner = if is_https { + let host = dst.host().ok_or_else(|| -> BoxError { + Box::new(io::Error::new(io::ErrorKind::InvalidInput, "URI has no host")) + })?; + let host = strip_ipv6_brackets(host); + let server_name = ServerName::try_from(host) + .map_err(|error| -> BoxError { Box::new(error) })? + .to_owned(); + + let handshake = TlsConnector::from(tls_config).connect(server_name, TokioIo::new(transport)); + + match await_handshake_with_deadline(tls_handshake_timeout, handshake).await { + Ok(stream) => MaybeHttpsStream::from(stream), + Err(error) => { if let Some(error_telemetry) = &error_telemetry { error_telemetry.increment_tls_error(); } + return Err(error); } - Err(error) } - } + } else { + MaybeHttpsStream::from(transport) + }; + + Ok(HttpsCapableConnection { + inner, + bytes_sent, + error_telemetry, + conn_age_limit, + }) }) } } +/// Strips the surrounding brackets from a bracketed IPv6 host, as found in a URI authority. +/// +/// [`rustls::pki_types::ServerName`] accepts unbracketed IPv6 addresses but rejects the bracketed form that +/// [`http::Uri::host`] returns (for example, `[::1]`), so this normalizes the host before constructing the server name. +fn strip_ipv6_brackets(host: &str) -> &str { + host.strip_prefix('[').and_then(|h| h.strip_suffix(']')).unwrap_or(host) +} + +/// Awaits a TLS handshake future, bounding it by `timeout` unless `timeout` is zero. +/// +/// A zero duration means the handshake deadline is disabled. +/// `tokio::time::timeout` with a zero duration fires immediately rather than never, so that case is handled by +/// awaiting the handshake directly instead of wrapping it in a timeout. +async fn await_handshake_with_deadline(timeout: Duration, handshake: F) -> Result +where + F: Future>, + E: std::error::Error + Send + Sync + 'static, +{ + if timeout.is_zero() { + return handshake.await.map_err(|error| Box::new(error) as BoxError); + } + + match tokio::time::timeout(timeout, handshake).await { + Ok(result) => result.map_err(|error| Box::new(error) as BoxError), + Err(_) => Err(Box::new(io::Error::new(io::ErrorKind::TimedOut, "TLS handshake timed out")) as BoxError), + } +} + fn build_dns_resolver(error_telemetry: &Option) -> SystemResolver { let mut r = SystemResolver::new(); if let Some(et) = error_telemetry { @@ -414,6 +482,7 @@ fn build_dns_resolver(error_telemetry: &Option) - #[derive(Default)] pub struct HttpsCapableConnectorBuilder { connect_timeout: Option, + tls_handshake_timeout: Option, bytes_sent: Option, error_telemetry: Option, conn_age_limit: Option, @@ -433,6 +502,14 @@ impl HttpsCapableConnectorBuilder { self } + /// Sets the timeout for completing the TLS handshake after a connection is established. + /// + /// Defaults to 10 seconds. + pub fn with_tls_handshake_timeout(mut self, timeout: Duration) -> Self { + self.tls_handshake_timeout = Some(timeout); + self + } + /// Sets the HTTP protocol selection for client connections. /// /// Defaults to [`HttpProtocol::Auto`]. @@ -499,8 +576,9 @@ impl HttpsCapableConnectorBuilder { } /// Builds the `HttpsCapableConnector` from the given TLS configuration. - pub fn build(self, tls_config: ClientConfig) -> Result { + pub fn build(self, mut tls_config: ClientConfig) -> Result { let connect_timeout = self.connect_timeout.unwrap_or(Duration::from_secs(30)); + let tls_handshake_timeout = self.tls_handshake_timeout.unwrap_or(Duration::from_secs(10)); // Create the HTTP connector, and ensure that we don't enforce _only_ HTTP, since that will break being able to // wrap this in an HTTPS connector. @@ -519,17 +597,12 @@ impl HttpsCapableConnectorBuilder { vsock_addr: self.vsock_addr, }; - // Create the HTTPS connector. - let https_connector_builder = HttpsConnectorBuilder::new().with_tls_config(tls_config).https_or_http(); - let https_connector = match self.http_protocol { - HttpProtocol::Auto => https_connector_builder - .enable_all_versions() - .wrap_connector(inner_connector), - HttpProtocol::Http1 => https_connector_builder.enable_http1().wrap_connector(inner_connector), - }; + tls_config.alpn_protocols = http_protocol_alpns(self.http_protocol); Ok(HttpsCapableConnector { - inner: https_connector, + inner: inner_connector, + tls_config: Arc::new(tls_config), + tls_handshake_timeout, bytes_sent: self.bytes_sent, error_telemetry: self.error_telemetry, conn_age_limit: self.conn_age_limit, @@ -537,29 +610,12 @@ impl HttpsCapableConnectorBuilder { } } -#[cfg(test)] -fn configure_tls_alpn_for_http_protocol(mut tls_config: ClientConfig, protocol: HttpProtocol) -> ClientConfig { +/// Selects the ALPN protocols to advertise for the given HTTP protocol. +fn http_protocol_alpns(protocol: HttpProtocol) -> Vec> { match protocol { - HttpProtocol::Auto => { - tls_config.alpn_protocols = vec![b"h2".to_vec(), b"http/1.1".to_vec()]; - } - HttpProtocol::Http1 => { - tls_config.alpn_protocols.clear(); - } + HttpProtocol::Auto => vec![b"h2".to_vec(), b"http/1.1".to_vec()], + HttpProtocol::Http1 => Vec::new(), } - - tls_config -} - -fn is_tls_error(error: &(dyn std::error::Error + 'static)) -> bool { - let mut current = Some(error); - while let Some(error) = current { - if error.downcast_ref::().is_some() { - return true; - } - current = error.source(); - } - false } fn is_dns_error(error: &(dyn std::error::Error + 'static)) -> bool { @@ -593,38 +649,109 @@ pub(super) fn check_connection_state(captured_conn: CaptureConnection) { #[cfg(test)] mod tests { - use super::{configure_tls_alpn_for_http_protocol, HttpProtocol}; + use std::{io, time::Duration}; - fn empty_tls_config() -> rustls::ClientConfig { - rustls::ClientConfig::builder_with_provider(default_crypto_provider().into()) - .with_safe_default_protocol_versions() - .expect("default protocol versions should be valid") - .with_root_certificates(rustls::RootCertStore::empty()) - .with_no_client_auth() + use super::{await_handshake_with_deadline, http_protocol_alpns, HttpProtocol}; + + #[tokio::test(start_paused = true)] + async fn handshake_deadline_of_zero_disables_the_timeout() { + let handshake = async { + tokio::time::sleep(Duration::from_secs(3600)).await; + Ok::<_, io::Error>(()) + }; + + let result = await_handshake_with_deadline(Duration::ZERO, handshake).await; + assert!(result.is_ok()); } - #[cfg(not(windows))] - fn default_crypto_provider() -> rustls::crypto::CryptoProvider { - rustls::crypto::aws_lc_rs::default_provider() + #[tokio::test(start_paused = true)] + async fn handshake_deadline_times_out_when_exceeded() { + let handshake = async { + tokio::time::sleep(Duration::from_secs(3600)).await; + Ok::<_, io::Error>(()) + }; + + let result = await_handshake_with_deadline(Duration::from_secs(10), handshake).await; + let error = result.expect_err("expected handshake to time out"); + assert!(error.to_string().contains("TLS handshake timed out")); } - #[cfg(windows)] - fn default_crypto_provider() -> rustls::crypto::CryptoProvider { - rustls_cng_crypto::default_provider() + #[tokio::test] + async fn handshake_deadline_propagates_success() { + let handshake = async { Ok::<_, io::Error>(42) }; + + let result = await_handshake_with_deadline(Duration::from_secs(10), handshake).await; + assert_eq!(result.unwrap(), 42); + } + + #[test] + fn strip_ipv6_brackets_unwraps_bracketed_addresses() { + use super::strip_ipv6_brackets; + + assert_eq!(strip_ipv6_brackets("[::1]"), "::1"); + assert_eq!(strip_ipv6_brackets("[2001:db8::1]"), "2001:db8::1"); + } + + #[test] + fn strip_ipv6_brackets_leaves_unbracketed_hosts_alone() { + use super::strip_ipv6_brackets; + + assert_eq!(strip_ipv6_brackets("example.com"), "example.com"); + assert_eq!(strip_ipv6_brackets("::1"), "::1"); + } + + #[cfg(unix)] + #[tokio::test] + async fn call_rejects_unsupported_uri_scheme() { + use std::sync::Arc; + + use rustls::{ClientConfig, RootCertStore}; + use tower::Service as _; + + use super::{HttpsCapableConnector, InnerConnector}; + use crate::net::dns::SystemResolver; + + let inner = InnerConnector { + http: SystemResolver::new().into_http_connector(), + connect_timeout: Duration::from_secs(1), + error_telemetry: None, + unix_socket_path: None, + #[cfg(target_os = "linux")] + vsock_addr: None, + }; + + let tls_config = Arc::new( + ClientConfig::builder() + .with_root_certificates(RootCertStore::empty()) + .with_no_client_auth(), + ); + + let mut connector = HttpsCapableConnector { + inner, + tls_config, + tls_handshake_timeout: Duration::from_secs(1), + bytes_sent: None, + error_telemetry: None, + conn_age_limit: None, + }; + + let uri: http::Uri = "ftp://example.com/".parse().unwrap(); + let error = connector.call(uri).await.err().expect("expected scheme to be rejected"); + assert!(error.to_string().contains("unsupported URI scheme")); } #[test] fn auto_protocol_advertises_h2_and_http1_alpn() { - let tls_config = configure_tls_alpn_for_http_protocol(empty_tls_config(), HttpProtocol::Auto); + let alpn_protocols = http_protocol_alpns(HttpProtocol::Auto); - assert_eq!(tls_config.alpn_protocols, vec![b"h2".to_vec(), b"http/1.1".to_vec()]); + assert_eq!(alpn_protocols, vec![b"h2".to_vec(), b"http/1.1".to_vec()]); } #[test] fn http1_protocol_leaves_alpn_empty() { - let tls_config = configure_tls_alpn_for_http_protocol(empty_tls_config(), HttpProtocol::Http1); + let alpn_protocols = http_protocol_alpns(HttpProtocol::Http1); - assert!(tls_config.alpn_protocols.is_empty()); + assert!(alpn_protocols.is_empty()); } // vsock takes priority over unix when both are configured, matching Agent behavior.