From df10b9a5a24687e598aee69ac584392c50479c42 Mon Sep 17 00:00:00 2001 From: liquidsec Date: Thu, 24 Sep 2026 15:12:04 -0400 Subject: [PATCH 1/2] do TLS with the target inside the proxy tunnel, not skip it hyper-util's Tunnel and SocksV5 call the inner connector with the proxy's URI, so our connector saw an http scheme and never did TLS. An https request through a CONNECT or SOCKS5 proxy went into the tunnel as plaintext, which a real HTTPS server rejects, and verify_certs and the other TLS settings did nothing. The connector now opens the tunnel itself with the CONNECT and SOCKS5 handshakes raw_connect already uses, then does TLS with the target over it. Connections are still pooled. SOCKS5 credentials in the proxy URL are now sent, which they weren't before. An https:// proxy URL is refused for an https target, since that needs TLS inside TLS. Fixes #89 --- Cargo.lock | 21 ---- Cargo.toml | 2 +- README.md | 2 + src/client/hyper.rs | 162 ++++++++++++++++++------- tests/proxy_tls.rs | 286 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 410 insertions(+), 63 deletions(-) create mode 100644 tests/proxy_tls.rs diff --git a/Cargo.lock b/Cargo.lock index fc635ff..73f1dd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,12 +85,6 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" -[[package]] -name = "base64" -version = "0.22.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" - [[package]] name = "bitflags" version = "2.11.1" @@ -450,16 +444,13 @@ version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ - "base64", "bytes", "futures-channel", "futures-util", "http", "http-body", "hyper", - "ipnet", "libc", - "percent-encoding", "pin-project-lite", "socket2", "tokio", @@ -486,12 +477,6 @@ dependencies = [ "rustversion", ] -[[package]] -name = "ipnet" -version = "2.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" - [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -601,12 +586,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "percent-encoding" -version = "2.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" - [[package]] name = "pin-project-lite" version = "0.2.17" diff --git a/Cargo.toml b/Cargo.toml index 072a839..9b37c37 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,7 @@ thiserror = "2" # HTTP - our single-stack foundation hyper = { version = "1", features = ["client", "http1", "http2"] } -hyper-util = { version = "0.1", features = ["client-legacy", "client-proxy", "http1", "http2", "tokio"] } +hyper-util = { version = "0.1", features = ["client-legacy", "http1", "http2", "tokio"] } http-body-util = "0.1" bytes = "1" diff --git a/README.md b/README.md index d6e47ca..0cab0a8 100644 --- a/README.md +++ b/README.md @@ -361,6 +361,8 @@ Run with `-v` to see it happen: the debug log records both the cookies each hop `proxy` routes a request through an HTTP or SOCKS5 proxy; `no_proxy` is a per-request list of hosts that bypass it and connect directly — the `NO_PROXY` equivalent. It's accepted by `request()`, `download()`, `raw_connect()`, and `BatchConfig`, and as the repeatable `--no-proxy` CLI flag. +An HTTPS request through a proxy is TLS with the target, carried inside the tunnel the proxy opens, so `verify_certs` and the other TLS settings apply to the target as usual. SOCKS5 credentials go in the URL (`socks5://user:pass@host:1080`). An `https://` proxy URL is refused for HTTPS targets, since that would need TLS inside TLS. + ```python # Proxy everything except internal hosts and the loopback range. r = await client.request( diff --git a/src/client/hyper.rs b/src/client/hyper.rs index a8a31c9..96237b6 100644 --- a/src/client/hyper.rs +++ b/src/client/hyper.rs @@ -206,6 +206,43 @@ struct OpenSslConnector { // on the `RedirectHop` (or final `Response`). peer_slot: PeerSlot, connect_timeout: Duration, + // Set when requests go through a CONNECT or SOCKS5 proxy. The connector + // opens the tunnel itself, so that TLS is with the target, inside it. + proxy: Option, +} + +/// A proxy the connector tunnels through before talking to the target. +#[derive(Clone)] +struct TunnelProxy { + /// Dialed the same way a direct connection's target is. + uri: http::Uri, + kind: super::proxy::ProxyScheme, + username: Option, + password: Option, +} + +impl TunnelProxy { + fn parse(proxy_url: &str, kind: super::proxy::ProxyScheme) -> Result { + let proxy_uri: http::Uri = proxy_url.parse().map_err(|e: http::uri::InvalidUri| { + ClientError::invalid_url(format!("invalid proxy URL: {}", e)) + })?; + // Credentials come from the URL's userinfo, `socks5://user:pass@host`. + let userinfo = proxy_uri + .authority() + .and_then(|a| a.as_str().rsplit_once('@')) + .map(|(u, _)| u.to_string()); + let (username, password) = match userinfo.as_deref().map(|u| u.split_once(':')) { + Some(Some((user, pass))) => (Some(user.to_string()), Some(pass.to_string())), + Some(None) => (userinfo.clone(), None), + None => (None, None), + }; + Ok(TunnelProxy { + uri: proxy_uri, + kind, + username, + password, + }) + } } /// Encode a list of ALPN protocol names into the wire format OpenSSL @@ -396,8 +433,14 @@ impl OpenSslConnector { cert_slot, peer_slot, connect_timeout, + proxy: None, }) } + + fn with_proxy(mut self, proxy: TunnelProxy) -> Self { + self.proxy = Some(proxy); + self + } } // hyper-util's Client needs a Service that returns an async connection. @@ -499,11 +542,21 @@ impl tower_service::Service for OpenSslConnector { fn call(&mut self, uri: http::Uri) -> Self::Future { let host = uri.host().unwrap_or("").to_string(); let is_https = uri.scheme_str() == Some("https"); + let port = uri.port_u16().unwrap_or(if is_https { 443 } else { 80 }); + let proxy = self.proxy.clone(); // Compute the slot key from the original URI before we hand it // off to hyper's HttpConnector — that's the authority callers - // will look up by. - let slot_key = peer_slot_key(&uri); - let http_fut = self.http.call(uri); + // will look up by. Through a proxy the socket's peer is the proxy, + // not the target, so there is no peer IP to record. + let slot_key = if proxy.is_some() { + None + } else { + peer_slot_key(&uri) + }; + let http_fut = match &proxy { + Some(p) => self.http.call(p.uri.clone()), + None => self.http.call(uri), + }; let ssl_connector = self.ssl.clone(); let cert_slot = self.cert_slot.clone(); let peer_slot = self.peer_slot.clone(); @@ -511,7 +564,7 @@ impl tower_service::Service for OpenSslConnector { Box::pin(async move { let tcp = http_fut.await?; - let tcp_stream = tcp.into_inner(); + let mut tcp_stream = tcp.into_inner(); // Record peer IP for this fresh connection. Best-effort — // failure to read peer_addr (vanishingly rare) is not fatal. @@ -521,6 +574,45 @@ impl tower_service::Service for OpenSslConnector { map.insert(key, peer.ip()); } + // Open the tunnel to the target before anything else, so that + // the TLS below is with the target and runs inside it. + if let Some(p) = proxy { + use super::proxy::{self as tunnel, ProxyScheme}; + let handshake = async { + match p.kind { + ProxyScheme::Http => { + tunnel::perform_http_connect(&mut tcp_stream, &host, port).await + } + ProxyScheme::Socks5 => { + tunnel::perform_socks5( + &mut tcp_stream, + &host, + port, + p.username.as_deref(), + p.password.as_deref(), + ) + .await + } + } + }; + tokio::time::timeout(connect_timeout, handshake) + .await + .map_err(|_| -> Box { + Box::new(std::io::Error::new( + std::io::ErrorKind::TimedOut, + format!( + "proxy tunnel to {}:{} timed out after {}s", + host, + port, + connect_timeout.as_secs() + ), + )) + })? + .map_err(|e| -> Box { + Box::new(std::io::Error::other(e.message)) + })?; + } + // Plain HTTP — return raw TCP stream, no TLS handshake if !is_https { return Ok(ConnectionStream::Plain(tcp_stream)); @@ -586,31 +678,25 @@ impl tower_service::Service for OpenSslConnector { // `GET http://target/path HTTP/1.1`. Uses raw http1::SendRequest to bypass // hyper Client's URI normalization (which strips scheme+authority). // - CONNECT tunnel (HTTPS targets): proxy opens a raw TCP tunnel via CONNECT. -// Uses hyper_util's Tunnel connector. // - SOCKS5: works for both HTTP and HTTPS targets. +// +// For both tunnel kinds `OpenSslConnector` opens the tunnel itself and then +// does TLS with the target over it. hyper_util's `Tunnel` and `SocksV5` can't +// be used for this: they hand the inner connector the proxy's URI, so TLS +// would be decided by the proxy's scheme and HTTPS would go into the tunnel +// as plaintext. -type DirectClient = Client; -type TunnelProxyClient = - Client, FullBody>; -type Socks5ProxyClient = - Client, FullBody>; +type PooledClient = Client; /// The cached hyper client + its cert info slot. /// hyper's Client uses Arc internally, so Clone shares the connection pool. #[derive(Clone)] struct CachedClient { - inner: AnyClient, + inner: PooledClient, cert_slot: CertSlot, peer_slot: PeerSlot, } -#[derive(Clone)] -enum AnyClient { - Direct(DirectClient), - Tunnel(TunnelProxyClient), - Socks5(Socks5ProxyClient), -} - /// TLS config fields that must be part of the client cache key. /// Requests with different TLS settings must not share a cached client. #[derive(Clone, Hash, Eq, PartialEq)] @@ -682,6 +768,15 @@ impl HyperClient { "http" | "https" => { // HTTP proxy: use forward proxy for HTTP targets, tunnel for HTTPS let target_is_https = target_uri.scheme_str() == Some("https"); + if target_is_https && proxy_scheme == "https" { + // That would be TLS to the target inside TLS to + // the proxy, which isn't supported. + return Err(ClientError::other(format!( + "https:// proxies are not supported for HTTPS targets; \ + use http:// or socks5:// ({})", + proxy_url + ))); + } if target_is_https { Ok(ConnMode::Tunnel { proxy_url: proxy_url.to_string(), @@ -724,8 +819,9 @@ impl HyperClient { let connector = OpenSslConnector::new(config, cert_slot.clone(), peer_slot.clone())?; let builder = Client::builder(TokioExecutor::new()); - let inner = match mode { - ConnMode::Direct(_) => AnyClient::Direct(builder.build(connector)), + use super::proxy::ProxyScheme; + let connector = match mode { + ConnMode::Direct(_) => connector, ConnMode::ForwardProxy(_) => { // Forward proxy doesn't use a cached hyper Client — it dispatches // directly via http1::SendRequest in send_inner. This branch should @@ -733,24 +829,13 @@ impl HyperClient { unreachable!("ForwardProxy uses dispatch_forward_proxy, not get_or_build") } ConnMode::Tunnel { proxy_url, .. } => { - let proxy_uri: http::Uri = - proxy_url.parse().map_err(|e: http::uri::InvalidUri| { - ClientError::invalid_url(format!("invalid proxy URL: {}", e)) - })?; - use hyper_util::client::legacy::connect::proxy::Tunnel; - let tunnel = Tunnel::new(proxy_uri, connector); - AnyClient::Tunnel(builder.build(tunnel)) + connector.with_proxy(TunnelProxy::parse(proxy_url, ProxyScheme::Http)?) } ConnMode::Socks5 { proxy_url, .. } => { - let proxy_uri: http::Uri = - proxy_url.parse().map_err(|e: http::uri::InvalidUri| { - ClientError::invalid_url(format!("invalid proxy URL: {}", e)) - })?; - use hyper_util::client::legacy::connect::proxy::SocksV5; - let socks = SocksV5::new(proxy_uri, connector); - AnyClient::Socks5(builder.build(socks)) + connector.with_proxy(TunnelProxy::parse(proxy_url, ProxyScheme::Socks5)?) } }; + let inner = builder.build(connector); let cached = CachedClient { inner, @@ -765,7 +850,7 @@ impl HyperClient { // ── Request dispatch ────────────────────────────────────────────── async fn dispatch_request( - client: &AnyClient, + client: &PooledClient, uri: &http::Uri, config: &RequestConfig, log: &DebugLog, @@ -788,12 +873,7 @@ async fn dispatch_request( } debug_record(log, v, 1, " Sending request..."); - let hyper_response = match client { - AnyClient::Direct(c) => c.request(request).await, - AnyClient::Tunnel(c) => c.request(request).await, - AnyClient::Socks5(c) => c.request(request).await, - } - .map_err(|e| { + let hyper_response = client.request(request).await.map_err(|e| { let msg = format!("request failed: {}", e); let err_str = e.to_string().to_lowercase(); if err_str.contains("ssl") || err_str.contains("tls") || err_str.contains("certificate") { diff --git a/tests/proxy_tls.rs b/tests/proxy_tls.rs new file mode 100644 index 0000000..db37397 --- /dev/null +++ b/tests/proxy_tls.rs @@ -0,0 +1,286 @@ +//! HTTPS through a proxy has to be TLS to the target, carried inside the +//! tunnel the proxy opens. These tests run a small CONNECT proxy and a small +//! SOCKS5 proxy in front of the TLS test server, and look at the first byte +//! that goes through the tunnel: a TLS handshake starts with 0x16, and a +//! plaintext request starts with the method. + +#[allow(dead_code)] +mod tls_server; + +use blasthttp::client::HttpClient; +use blasthttp::client::hyper::HyperClient; +use blasthttp::config::RequestConfig; +use std::sync::{Arc, Mutex}; +use tls_server::{TlsServerConfig, TlsTestServer}; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::{TcpListener, TcpStream}; + +/// The first TLS record type: a handshake, which is what a ClientHello is. +const TLS_HANDSHAKE: u8 = 0x16; + +/// What a test proxy saw: the first byte the client sent through the tunnel, +/// and for SOCKS5 the credentials it offered. +#[derive(Default)] +struct Seen { + first_byte: Option, + socks_auth: Option<(String, String)>, +} + +/// Record the first byte from the client, then pipe both ways until either +/// side closes. +async fn tunnel(mut client: TcpStream, target: &str, seen: &Mutex) { + let Ok(mut upstream) = TcpStream::connect(target).await else { + return; + }; + let mut first = [0u8; 1]; + if client.read_exact(&mut first).await.is_err() { + return; + } + seen.lock().unwrap().first_byte = Some(first[0]); + if upstream.write_all(&first).await.is_err() { + return; + } + let _ = tokio::io::copy_bidirectional(&mut client, &mut upstream).await; +} + +/// A CONNECT proxy that accepts any target. +async fn spawn_connect_proxy() -> (String, Arc>) { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + let seen = Arc::new(Mutex::new(Seen::default())); + let state = seen.clone(); + + tokio::spawn(async move { + while let Ok((mut client, _)) = listener.accept().await { + let state = state.clone(); + tokio::spawn(async move { + // Read the request head a byte at a time, so nothing past it + // (the start of the tunnel) is consumed here. + let mut head = Vec::new(); + while !head.ends_with(b"\r\n\r\n") { + let mut b = [0u8; 1]; + if client.read_exact(&mut b).await.is_err() { + return; + } + head.push(b[0]); + } + let head = String::from_utf8_lossy(&head); + let Some(target) = head + .strip_prefix("CONNECT ") + .and_then(|rest| rest.split_whitespace().next()) + else { + return; + }; + let target = target.to_string(); + if client + .write_all(b"HTTP/1.1 200 Connection established\r\n\r\n") + .await + .is_err() + { + return; + } + tunnel(client, &target, &state).await; + }); + } + }); + + (url, seen) +} + +/// A SOCKS5 proxy (RFC 1928). Takes no-auth, or username/password +/// (RFC 1929) when the client offers it, and records the credentials. +async fn spawn_socks5_proxy() -> (u16, Arc>) { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let seen = Arc::new(Mutex::new(Seen::default())); + let state = seen.clone(); + + tokio::spawn(async move { + while let Ok((mut c, _)) = listener.accept().await { + let state = state.clone(); + tokio::spawn(async move { + let result: std::io::Result<()> = async { + // Greeting: VER NMETHODS METHODS... + let mut hdr = [0u8; 2]; + c.read_exact(&mut hdr).await?; + let mut methods = vec![0u8; hdr[1] as usize]; + c.read_exact(&mut methods).await?; + if methods.contains(&0x02) { + c.write_all(&[0x05, 0x02]).await?; + // VER ULEN UNAME PLEN PASSWD + let mut ver_ulen = [0u8; 2]; + c.read_exact(&mut ver_ulen).await?; + let mut user = vec![0u8; ver_ulen[1] as usize]; + c.read_exact(&mut user).await?; + let mut plen = [0u8; 1]; + c.read_exact(&mut plen).await?; + let mut pass = vec![0u8; plen[0] as usize]; + c.read_exact(&mut pass).await?; + state.lock().unwrap().socks_auth = Some(( + String::from_utf8_lossy(&user).into_owned(), + String::from_utf8_lossy(&pass).into_owned(), + )); + c.write_all(&[0x01, 0x00]).await?; + } else { + c.write_all(&[0x05, 0x00]).await?; + } + // Request: VER CMD RSV ATYP DST.ADDR DST.PORT + let mut req = [0u8; 4]; + c.read_exact(&mut req).await?; + let host = match req[3] { + 0x01 => { + let mut ip = [0u8; 4]; + c.read_exact(&mut ip).await?; + std::net::Ipv4Addr::from(ip).to_string() + } + 0x03 => { + let mut len = [0u8; 1]; + c.read_exact(&mut len).await?; + let mut name = vec![0u8; len[0] as usize]; + c.read_exact(&mut name).await?; + String::from_utf8_lossy(&name).into_owned() + } + _ => return Ok(()), + }; + let mut port = [0u8; 2]; + c.read_exact(&mut port).await?; + let port = u16::from_be_bytes(port); + c.write_all(&[0x05, 0x00, 0x00, 0x01, 0, 0, 0, 0, 0, 0]) + .await?; + tunnel(c, &format!("{}:{}", host, port), &state).await; + Ok(()) + } + .await; + let _ = result; + }); + } + }); + + (port, seen) +} + +fn https_config(server: &TlsTestServer, proxy: String) -> RequestConfig { + let mut config = RequestConfig::new(format!("{}/secret?token=hunter2", server.url())); + config.proxy = Some(proxy); + config.verify_certs = Some(false); // self-signed cert + config.timeout_seconds = Some(5); + config +} + +#[tokio::test] +async fn https_through_a_connect_proxy_is_tls_inside_the_tunnel() { + let server = TlsTestServer::start(TlsServerConfig::default()).await; + let (proxy, seen) = spawn_connect_proxy().await; + + let resp = HyperClient::new() + .send(&https_config(&server, proxy)) + .await + .expect("request failed"); + + assert_eq!(seen.lock().unwrap().first_byte, Some(TLS_HANDSHAKE)); + assert_eq!(resp.status, 200); + assert!(resp.cert_info.is_some(), "should carry the target's cert"); + server.shutdown().await; +} + +#[tokio::test] +async fn https_through_a_socks5_proxy_is_tls_inside_the_tunnel() { + let server = TlsTestServer::start(TlsServerConfig::default()).await; + let (port, seen) = spawn_socks5_proxy().await; + + let resp = HyperClient::new() + .send(&https_config( + &server, + format!("socks5://127.0.0.1:{}", port), + )) + .await + .expect("request failed"); + + assert_eq!(seen.lock().unwrap().first_byte, Some(TLS_HANDSHAKE)); + assert_eq!(resp.status, 200); + server.shutdown().await; +} + +#[tokio::test] +async fn verify_certs_applies_to_the_target_through_a_connect_proxy() { + // The test server's cert is self-signed, so with verification on the + // request has to fail. Before the fix there was no TLS with the target, + // so there was nothing to verify and the request failed for another + // reason. + let server = TlsTestServer::start(TlsServerConfig::default()).await; + let (proxy, seen) = spawn_connect_proxy().await; + let mut config = https_config(&server, proxy); + config.verify_certs = Some(true); + + let result = HyperClient::new().send(&config).await; + + // A handshake with the target did start, so the failure is the cert + // check and not a broken tunnel. + assert_eq!(seen.lock().unwrap().first_byte, Some(TLS_HANDSHAKE)); + assert!( + result.is_err(), + "self-signed target should fail verification" + ); + server.shutdown().await; +} + +#[tokio::test] +async fn verify_certs_applies_to_the_target_through_a_socks5_proxy() { + let server = TlsTestServer::start(TlsServerConfig::default()).await; + let (port, seen) = spawn_socks5_proxy().await; + let mut config = https_config(&server, format!("socks5://127.0.0.1:{}", port)); + config.verify_certs = Some(true); + + let result = HyperClient::new().send(&config).await; + + // A handshake with the target did start, so the failure is the cert + // check and not a broken tunnel. + assert_eq!(seen.lock().unwrap().first_byte, Some(TLS_HANDSHAKE)); + assert!( + result.is_err(), + "self-signed target should fail verification" + ); + server.shutdown().await; +} + +#[tokio::test] +async fn socks5_credentials_in_the_proxy_url_are_sent() { + let server = TlsTestServer::start(TlsServerConfig::default()).await; + let (port, seen) = spawn_socks5_proxy().await; + + let resp = HyperClient::new() + .send(&https_config( + &server, + format!("socks5://alice:s3cret@127.0.0.1:{}", port), + )) + .await + .expect("request failed"); + + assert_eq!(resp.status, 200); + assert_eq!( + seen.lock().unwrap().socks_auth, + Some(("alice".to_string(), "s3cret".to_string())) + ); + server.shutdown().await; +} + +#[tokio::test] +async fn https_proxy_url_is_refused_for_an_https_target() { + // TLS to the target inside TLS to the proxy isn't supported, and this + // never worked: the target used to get plaintext. Say so up front. + let server = TlsTestServer::start(TlsServerConfig::default()).await; + let (proxy, _) = spawn_connect_proxy().await; + let proxy = proxy.replacen("http://", "https://", 1); + + let err = HyperClient::new() + .send(&https_config(&server, proxy)) + .await + .expect_err("https:// proxy should be refused"); + + assert!( + err.to_string().contains("https:// proxies"), + "error should say why: {}", + err + ); + server.shutdown().await; +} From e29bdfba7bcbf358b52d00c3d26338bd97acf804 Mon Sep 17 00:00:00 2001 From: liquidsec Date: Thu, 1 Oct 2026 15:51:42 -0400 Subject: [PATCH 2/2] Dial the tunnel's proxy through the same parser raw_connect uses TunnelProxy::parse split the userinfo itself and then handed the raw proxy URL to HttpConnector, which fills in a missing port from the URL scheme: 443 for https, 80 for anything else. So a portless proxy landed somewhere different depending on which path you took. socks5://proxy reached port 80 for request() but 1080 for raw_connect(), and http://proxy reached 80 against 8080. Build it from parse_proxy_url instead. The ProxyConfig that comes back carries the scheme, host, port and credentials, so the connector dials an explicit http://host:port and both paths agree on where the proxy is. The scheme is in there too, which means Tunnel and Socks5 no longer have to tell the constructor which one they are. --- src/client/hyper.rs | 90 ++++++++++++++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 25 deletions(-) diff --git a/src/client/hyper.rs b/src/client/hyper.rs index 96237b6..07f8dfe 100644 --- a/src/client/hyper.rs +++ b/src/client/hyper.rs @@ -214,7 +214,10 @@ struct OpenSslConnector { /// A proxy the connector tunnels through before talking to the target. #[derive(Clone)] struct TunnelProxy { - /// Dialed the same way a direct connection's target is. + /// The proxy's own endpoint as `http://host:port`, dialed the same way a + /// direct connection's target is. Always carries an explicit port, because + /// `HttpConnector` would otherwise fill in 80 and disagree with the + /// scheme's default. uri: http::Uri, kind: super::proxy::ProxyScheme, username: Option, @@ -222,25 +225,23 @@ struct TunnelProxy { } impl TunnelProxy { - fn parse(proxy_url: &str, kind: super::proxy::ProxyScheme) -> Result { - let proxy_uri: http::Uri = proxy_url.parse().map_err(|e: http::uri::InvalidUri| { - ClientError::invalid_url(format!("invalid proxy URL: {}", e)) - })?; - // Credentials come from the URL's userinfo, `socks5://user:pass@host`. - let userinfo = proxy_uri - .authority() - .and_then(|a| a.as_str().rsplit_once('@')) - .map(|(u, _)| u.to_string()); - let (username, password) = match userinfo.as_deref().map(|u| u.split_once(':')) { - Some(Some((user, pass))) => (Some(user.to_string()), Some(pass.to_string())), - Some(None) => (userinfo.clone(), None), - None => (None, None), - }; + /// Built from the same parse `raw_connect` uses, so both paths agree on + /// the port, the host and the credentials for any given proxy URL. + fn from_url(proxy_url: &str) -> Result { + let cfg = super::proxy::parse_proxy_url(proxy_url)?; + let uri = http::Uri::builder() + .scheme("http") + .authority(format!("{}:{}", cfg.host, cfg.port)) + .path_and_query("/") + .build() + .map_err(|e| { + ClientError::invalid_url(format!("invalid proxy URL '{}': {}", proxy_url, e)) + })?; Ok(TunnelProxy { - uri: proxy_uri, - kind, - username, - password, + uri, + kind: cfg.scheme, + username: cfg.username, + password: cfg.password, }) } } @@ -819,7 +820,6 @@ impl HyperClient { let connector = OpenSslConnector::new(config, cert_slot.clone(), peer_slot.clone())?; let builder = Client::builder(TokioExecutor::new()); - use super::proxy::ProxyScheme; let connector = match mode { ConnMode::Direct(_) => connector, ConnMode::ForwardProxy(_) => { @@ -828,11 +828,10 @@ impl HyperClient { // never be reached. unreachable!("ForwardProxy uses dispatch_forward_proxy, not get_or_build") } - ConnMode::Tunnel { proxy_url, .. } => { - connector.with_proxy(TunnelProxy::parse(proxy_url, ProxyScheme::Http)?) - } - ConnMode::Socks5 { proxy_url, .. } => { - connector.with_proxy(TunnelProxy::parse(proxy_url, ProxyScheme::Socks5)?) + // The scheme comes out of the proxy URL itself, so these two modes + // build the tunnel the same way. + ConnMode::Tunnel { proxy_url, .. } | ConnMode::Socks5 { proxy_url, .. } => { + connector.with_proxy(TunnelProxy::from_url(proxy_url)?) } }; let inner = builder.build(connector); @@ -3025,4 +3024,45 @@ mod tests { assert_eq!(out, SAMPLE); assert!(!cut); } + + /// The pooled path and `raw_connect` have to land on the same proxy, so + /// the endpoint the connector dials must match what `parse_proxy_url` + /// resolved. Portless URLs are where these used to disagree: handing the + /// raw URL to `HttpConnector` got port 80 for both schemes. + #[test] + fn test_tunnel_proxy_endpoint_matches_the_shared_parser() { + for url in [ + "http://proxy", + "http://proxy:3128", + "socks5://proxy", + "socks5://proxy:9050", + "socks5h://alice:s3cret@proxy", + ] { + let cfg = super::super::proxy::parse_proxy_url(url).unwrap(); + let tunnel = TunnelProxy::from_url(url).unwrap(); + assert_eq!( + tunnel.uri.authority().map(|a| a.as_str()), + Some(format!("{}:{}", cfg.host, cfg.port).as_str()), + "dialed endpoint disagrees with the parser for {}", + url + ); + } + } + + #[test] + fn test_tunnel_proxy_carries_socks5_credentials() { + let tunnel = TunnelProxy::from_url("socks5://alice:s3cret@proxy:1080").unwrap(); + assert_eq!(tunnel.username.as_deref(), Some("alice")); + assert_eq!(tunnel.password.as_deref(), Some("s3cret")); + assert!(matches!( + tunnel.kind, + super::super::proxy::ProxyScheme::Socks5 + )); + } + + #[test] + fn test_tunnel_proxy_rejects_what_the_parser_rejects() { + assert!(TunnelProxy::from_url("ftp://proxy:21").is_err()); + assert!(TunnelProxy::from_url("http://:8080").is_err()); + } }