fix(io): bound the TLS handshake performed when tunneling HTTPS through a proxy

Cargo.toml

@@ -119,7 +119,7 @@ http = { version = "1", default-features = false }
 http-body = { version = "1", default-features = false }
 http-body-util = { version = "0.1", default-features = false }
 hyper = { version = "1", default-features = false }
-hyper-http-proxy = { version = "1.1", default-features = false, features = [
+hyper-http-proxy = { version = "1.2", default-features = false, features = [
   "rustls-tls-native-roots",
 ] }
 hyper-rustls = { version = "0.27", default-features = false, features = [
@@ -258,11 +258,6 @@ antithesis-instrumentation = { version = "0.1" }
 antithesis_sdk = { git = "https://github.com/antithesishq/antithesis-sdk-rust", rev = "78c9db56771f0f6b01cb5404765c5a96852c159c", default-features = false } # 0.2.8 is pinned to rand 0.8, rev version allows us to select workspace rand
 mime = "0.3"
 
-[patch.crates-io]
-# Forked version of `hyper-http-proxy` that removes an unused dependency on `rustls-native-certs`, which transitively depends
-# on a version of `rustls-pemfile` that is no longer maintained and triggers a hit when running `cargo deny`.
-hyper-http-proxy = { git = "https://github.com/tobz/hyper-http-proxy.git", branch = "main" }
-
 [profile.devel]
 inherits = "dev"
 

deny.toml

@@ -36,10 +36,6 @@ allow-git = [
   # while the crate is consumed from GitHub directly.
   "https://github.com/DataDog/rustls-cng-crypto",
 
-  # Forked version of `hyper-http-proxy` that removes an unused dependency on `rustls-native-certs`, which transitively depends
-  # on a version of `rustls-pemfile` that is no longer maintained and triggers a hit when running `cargo deny`.
-  "https://github.com/tobz/hyper-http-proxy.git",
-
   # Yet-to-be-published version of incremental Protocol Buffers encoding library.
   "https://github.com/tobz/piecemeal.git",
 ]

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

@@ -376,10 +376,15 @@ impl HttpClientBuilder {
             None => self.tls_builder.build()?,
         };
         let connector = self.connector_builder.build(tls_config)?;
+        let tls_handshake_timeout = connector.tls_handshake_timeout();
         // TODO(fips): Look into updating `hyper-http-proxy` to use the provided connector for establishing the
         // connection to the proxy itself, even when the proxy is at an HTTPS URL, to ensure our desired TLS stack is
         // being used.
         let mut proxy_connector = hyper_http_proxy::ProxyConnector::new(connector)?;
+        // A zero timeout means the handshake deadline is disabled, matching `HttpsCapableConnector`'s own
+        // handling of `Duration::ZERO` for direct connections.
+        let proxy_tls_handshake_timeout = (!tls_handshake_timeout.is_zero()).then_some(tls_handshake_timeout);
+        proxy_connector.set_tls_handshake_timeout(proxy_tls_handshake_timeout);
         if let Some(proxies) = &self.proxies {
             for proxy in proxies {
                 proxy_connector.add_proxy(proxy.to_owned());
@@ -515,6 +520,101 @@ mod tests {
         server_task.abort();
     }
 
+    #[tokio::test]
+    async fn tls_handshake_timeout_fires_against_a_stalled_proxy_tunnel() {
+        initialize_crypto_provider();
+
+        let listener = TcpListener::bind("127.0.0.1:0").await.expect("should bind listener");
+        let proxy_addr = listener.local_addr().expect("should have local address");
+        let proxy_task = tokio::spawn(async move {
+            let (mut stream, _) = listener.accept().await.expect("proxy should accept a connection");
+            let mut request = [0; 4096];
+            let bytes_read = stream
+                .read(&mut request)
+                .await
+                .expect("proxy should read the CONNECT request");
+            assert!(bytes_read > 0, "proxy should receive a CONNECT request");
+            stream
+                .write_all(b"HTTP/1.1 200 OK\r\n\r\n")
+                .await
+                .expect("proxy should write the CONNECT response");
+            // Complete the CONNECT tunnel but never speak TLS, 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 proxy_uri: Uri = format!("http://{proxy_addr}").parse().expect("proxy URI should parse");
+        let mut client = HttpClient::builder()
+            .with_proxies(vec![Proxy::new(hyper_http_proxy::Intercept::All, proxy_uri)])
+            .with_tls_config(|builder| builder.with_root_cert_store(RootCertStore::empty()))
+            .with_tls_handshake_timeout(Duration::from_millis(200))
+            .with_http_protocol(HttpProtocol::Http1)
+            .build()
+            .expect("client should build");
+        let request = Request::get("https://example.invalid/")
+            .body(Empty::<Bytes>::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:#}"
+        );
+
+        proxy_task.abort();
+    }
+
+    #[tokio::test]
+    async fn zero_tls_handshake_timeout_disables_the_proxy_tunnel_deadline() {
+        initialize_crypto_provider();
+
+        let listener = TcpListener::bind("127.0.0.1:0").await.expect("should bind listener");
+        let proxy_addr = listener.local_addr().expect("should have local address");
+        let proxy_task = tokio::spawn(async move {
+            let (mut stream, _) = listener.accept().await.expect("proxy should accept a connection");
+            let mut request = [0; 4096];
+            let bytes_read = stream
+                .read(&mut request)
+                .await
+                .expect("proxy should read the CONNECT request");
+            assert!(bytes_read > 0, "proxy should receive a CONNECT request");
+            stream
+                .write_all(b"HTTP/1.1 200 OK\r\n\r\n")
+                .await
+                .expect("proxy should write the CONNECT response");
+            // Complete the CONNECT tunnel but never speak TLS, so the request only fails if some
+            // deadline (mistakenly) bounds the handshake.
+            tokio::time::sleep(Duration::from_secs(30)).await;
+            drop(stream);
+        });
+
+        let proxy_uri: Uri = format!("http://{proxy_addr}").parse().expect("proxy URI should parse");
+        let mut client = HttpClient::builder()
+            .with_proxies(vec![Proxy::new(hyper_http_proxy::Intercept::All, proxy_uri)])
+            .with_tls_config(|builder| builder.with_root_cert_store(RootCertStore::empty()))
+            .with_tls_handshake_timeout(Duration::ZERO)
+            .with_http_protocol(HttpProtocol::Http1)
+            .build()
+            .expect("client should build");
+        let request = Request::get("https://example.invalid/")
+            .body(Empty::<Bytes>::new())
+            .expect("request should build");
+
+        // A zero handshake timeout means "disabled", so the request should still be pending well past
+        // the point where a mistakenly active zero-length deadline would have already failed it.
+        let result = timeout(Duration::from_millis(300), client.send(request)).await;
+        assert!(
+            result.is_err(),
+            "a disabled timeout should not fail the proxy tunnel's TLS handshake, got: {result:?}"
+        );
+
+        proxy_task.abort();
+    }
+
     fn mutual_tls_configs() -> (ServerConfig, ClientConfig) {
         let server_cert = SelfSignedCert::localhost();
         let client_cert = SelfSignedCert::new(["saluki-client"]);

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

@@ -375,6 +375,13 @@ pub struct HttpsCapableConnector {
     conn_age_limit: Option<Duration>,
 }
 
+impl HttpsCapableConnector {
+    /// Returns the timeout applied to the TLS handshake for HTTPS connections.
+    pub(crate) fn tls_handshake_timeout(&self) -> Duration {
+        self.tls_handshake_timeout
+    }
+}
+
 impl Service<Uri> for HttpsCapableConnector {
     type Response = HttpsCapableConnection;
     type Error = BoxError;