Repository navigation
Do TLS with the target inside the proxy tunnel - #115
Conversation
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
singlerider
left a comment
singlerider
left a comment
There was a problem hiding this comment.
🔴 TunnelProxy::parse re-implements the userinfo split that proxy::parse_proxy_url already does for raw_connect. It also dials the proxy by handing the raw proxy URI to HttpConnector, which defaults a portless proxy to 80. So socks5://proxy reaches port 80 for request() but 1080 for raw_connect(), and http://proxy reaches 80 vs 8080. Build the tunnel from ProxyConfig (scheme, host, port, creds) so that one parser covers both paths.
🟢 Opening the tunnel inside OpenSslConnector and deciding TLS from the target URI is the correct fix. It also lets verify_certs, ciphers and ALPN apply to proxied HTTPS. Collapsing AnyClient into one PooledClient removes three-way dispatch.
🟢 tests/proxy_tls.rs checks the first tunneled byte is 0x16, which is a direct assertion that no plaintext leaked. 6/6 pass locally.
singlerider
left a comment
singlerider
left a comment
There was a problem hiding this comment.
Approving to avoid another review round. The 🔴 from my previous review still needs to be addressed before merge: build the tunnel from proxy::parse_proxy_url so that request() and raw_connect() share one parser and the same default proxy ports.
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.
2 participants
Fixes #89.
HTTPS through a CONNECT or SOCKS5 proxy never did TLS with the target. hyper-util's
TunnelandSocksV5call the inner connector with the proxy's URI, so our connector sawhttpand skipped TLS, and the request went into the tunnel as plaintext. In practice that means proxied HTTPS just failed (a real HTTPS server rejects plaintext, and the new tests gotclient error (Canceled)), unless the proxy passed plaintext along, in which case the request leaked.OpenSslConnectornow opens the tunnel itself, using the CONNECT and SOCKS5 handshakesraw_connectalready uses, and then does TLS with the target over it. Connections are still pooled, and the TCP connection to the proxy is made the same way as before. hyper-util's proxy connectors are gone, along with itsclient-proxyfeature.Behavior changes:
verify_certs, cipher and TLS-version settings, andalpn_protocolsnow apply to proxied HTTPS.socks5://user:pass@host) are sent. They were silently ignored before.https://proxy URL is refused for HTTPS targets with a clear error, since that would need TLS inside TLS. It never worked. Plain-HTTP targets through anhttps://proxy URL are unchanged.tests/proxy_tls.rsruns a small CONNECT proxy and a small SOCKS5 proxy in front of the TLS test server, and checks that the first byte through the tunnel is a TLS handshake, that certificate verification applies to the target, and that SOCKS5 credentials arrive.