diff --git a/Cargo.lock b/Cargo.lock index 14ea257eadc..0b33444f2b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2137,8 +2137,9 @@ dependencies = [ [[package]] name = "hyper-http-proxy" -version = "1.1.0" -source = "git+https://github.com/tobz/hyper-http-proxy.git?branch=main#c7c5aad8b0cba98dd3ca404b45031473ef2d03b6" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd1d471ea2f65ba45eddb1d6ab7d58ac2671d2d4ac14da5c6516ae1d97e1327a" dependencies = [ "bytes", "futures-util", diff --git a/Cargo.toml b/Cargo.toml index cec41161088..e02c8fd7ab3 100644 --- a/Cargo.toml +++ b/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" diff --git a/deny.toml b/deny.toml index a3c722b18b9..01908c0f3d1 100644 --- a/deny.toml +++ b/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", ] diff --git a/lib/saluki-io/src/net/client/http/client.rs b/lib/saluki-io/src/net/client/http/client.rs index 46d511a072e..b842c9fd4e9 100644 --- a/lib/saluki-io/src/net/client/http/client.rs +++ b/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::::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::::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"]); diff --git a/lib/saluki-io/src/net/client/http/conn.rs b/lib/saluki-io/src/net/client/http/conn.rs index 8db861cba4c..181ff668344 100644 --- a/lib/saluki-io/src/net/client/http/conn.rs +++ b/lib/saluki-io/src/net/client/http/conn.rs @@ -375,6 +375,13 @@ pub struct HttpsCapableConnector { conn_age_limit: Option, } +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 for HttpsCapableConnector { type Response = HttpsCapableConnection; type Error = BoxError;