fix(tls): address review feedback on TLS handshake timeout connector

Reject unsupported URI schemes explicitly instead of silently falling
back to plaintext, strip IPv6 brackets before constructing the TLS
server name, and correct ALPN documentation/test for HTTP/1.1-only
mode.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: jszwedko

lib/agent-data-plane-config/src/shared.rs

@@ -122,6 +122,9 @@ pub struct Tls {
     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,
 }
 

lib/saluki-io/src/net/client/http/client.rs

@@ -316,8 +316,8 @@ impl HttpClientBuilder {
     ///
     /// Any ALPN protocols already present in `config` are discarded before the configuration is passed to
     /// `hyper-rustls`. [`Self::with_http_protocol`] remains the sole source of HTTP protocol selection:
-    /// [`HttpProtocol::Auto`] advertises HTTP/2 with HTTP/1.1 fallback, while [`HttpProtocol::Http1`] enables only
-    /// HTTP/1.1 and does not advertise ALPN.
+    /// [`HttpProtocol::Auto`] advertises HTTP/2 with HTTP/1.1 fallback, while [`HttpProtocol::Http1`] advertises
+    /// only HTTP/1.1 via ALPN.
     ///
     /// The supplied configuration is validated for FIPS compliance during [`Self::build`].
     pub fn with_client_tls_config(mut self, config: ClientConfig) -> Self {
@@ -469,13 +469,15 @@ mod tests {
         let (mut server_config, mut client_config) = mutual_tls_configs();
         server_config.alpn_protocols = vec![b"custom".to_vec(), b"http/1.1".to_vec()];
         client_config.alpn_protocols = vec![b"custom".to_vec()];
-        let builder = HttpClient::builder().with_client_tls_config(client_config);
+        let builder = HttpClient::builder()
+            .with_http_protocol(HttpProtocol::Http1)
+            .with_client_tls_config(client_config);
 
         let negotiated_alpn = send_request_to_tls_server(builder, server_config)
             .await
             .expect("client should normalize ALPN and complete an HTTP/1.1 request");
 
-        assert_eq!(negotiated_alpn, None);
+        assert_eq!(negotiated_alpn, Some(b"http/1.1".to_vec()));
     }
 
     fn mutual_tls_configs() -> (ServerConfig, ClientConfig) {

lib/saluki-io/src/net/client/http/conn.rs

@@ -385,7 +385,19 @@ impl Service<Uri> for HttpsCapableConnector {
     }
 
     fn call(&mut self, dst: Uri) -> Self::Future {
-        let is_https = dst.scheme_str() == Some("https");
+        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;
@@ -400,6 +412,7 @@ impl Service<Uri> for HttpsCapableConnector {
                 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();
@@ -429,6 +442,14 @@ impl Service<Uri> for HttpsCapableConnector {
     }
 }
 
+/// 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 (e.g. `[::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, matching the core Agent convention for this setting.
@@ -663,6 +684,62 @@ mod tests {
         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 alpn_protocols = configure_alpn_for_http_protocol(HttpProtocol::Auto);