From a8fe41f154f343a982f9a8edfa765f6517137006 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sun, 1 Nov 2026 10:14:30 +0100 Subject: [PATCH] Verify the TLS endpoint of an HTTPS proxy against the proxy host The target name was passed as the endpoint name when connecting to the first hop, so the proxy certificate was verified against, and the SNI set to, the target virtual host instead of the proxy. The target name is now used only when upgrading to TLS for the target. --- .../io/BasicHttpClientConnectionManager.java | 3 +- .../PoolingHttpClientConnectionManager.java | 3 +- .../PoolingAsyncClientConnectionManager.java | 3 +- ...estPoolingHttpClientConnectionManager.java | 48 +++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java index 78e9476b1e..49cef36be2 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java @@ -491,7 +491,8 @@ public void connect(final ConnectionEndpoint endpoint, final TimeValue timeout, this.connectionOperator.connect( connection, firstHop, - route.getTargetName(), + // The target name applies to the target only, never to an intermediate proxy + route.getProxyHost() != null ? null : route.getTargetName(), route.getLocalSocketAddress(), connectTimeout, socketConfig, diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java index 2945e8ed7d..6d670bf3e6 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java @@ -565,7 +565,8 @@ public void connect(final ConnectionEndpoint endpoint, final TimeValue timeout, this.connectionOperator.connect( conn, firstHop, - route.getTargetName(), + // The target name applies to the target only, never to an intermediate proxy + route.getProxyHost() != null ? null : route.getTargetName(), unixDomainSocket, route.getLocalSocketAddress(), connectTimeout, diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java index c5215a4156..d030b9b3f3 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java @@ -523,7 +523,8 @@ public Future connect( connectionInitiator, firstHop, unixDomainSocket, - route.getTargetName(), + // The target name applies to the target only, never to an intermediate proxy + route.getProxyHost() != null ? null : route.getTargetName(), route.getLocalSocketAddress(), connectTimeout, connectAttachment, diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java index b045626750..e5f8416018 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java @@ -53,6 +53,7 @@ import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.io.SocketConfig; +import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.http2.HttpVersionPolicy; import org.apache.hc.core5.pool.PoolEntry; import org.apache.hc.core5.pool.StrictConnPool; @@ -407,5 +408,52 @@ void testConcurrentShutdown() throws InterruptedException { Assertions.assertTrue(mgr.isClosed(), "Connection manager should be shutdown after concurrent calls to shutdown."); } + @Test + void testHttpsProxyVerifiedAgainstProxyNotTargetName() throws Exception { + final HttpHost target = new HttpHost("https", "somehost", 443); + final URIAuthority targetName = new URIAuthority("virtualhost", 443); + final HttpHost proxy = new HttpHost("https", "someproxy", 8443); + final InetAddress remote = InetAddress.getByAddress(new byte[] {10, 0, 0, 1}); + final HttpRoute route = new HttpRoute(target, targetName, null, proxy, true); + Assertions.assertTrue(route.isTunnelled()); + + final PoolEntry entry = new PoolEntry<>(route, TimeValue.NEG_ONE_MILLISECOND); + entry.assignConnection(conn); + + Mockito.when(conn.isOpen()).thenReturn(false); + Mockito.when(future.get(1, TimeUnit.SECONDS)).thenReturn(entry); + Mockito.when(pool.lease( + Mockito.eq(route), + Mockito.eq(null), + Mockito.any(), + Mockito.eq(null))) + .thenReturn(future); + + final LeaseRequest connRequest = mgr.lease("some-id", route, null); + final ConnectionEndpoint endpoint = connRequest.get(Timeout.ofSeconds(1)); + Assertions.assertNotNull(endpoint); + + final HttpClientContext context = HttpClientContext.create(); + final TlsConfig tlsConfig = TlsConfig.custom().build(); + mgr.setDefaultTlsConfig(tlsConfig); + + Mockito.when(dnsResolver.resolve("someproxy", 8443)).thenReturn(Collections.singletonList(new InetSocketAddress(remote, 8443))); + Mockito.when(schemePortResolver.resolve(proxy.getSchemeName(), proxy)).thenReturn(8443); + Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); + Mockito.when(detachedSocketFactory.create(Mockito.any(), Mockito.any())).thenReturn(socket); + Mockito.when(tlsSocketStrategy.upgrade( + Mockito.any(), Mockito.any(), Mockito.anyInt(), Mockito.any(), Mockito.any())).thenReturn(upgradedSocket); + + mgr.connect(endpoint, null, context); + + Mockito.verify(tlsSocketStrategy).upgrade(socket, "someproxy", 8443, null, context); + + Mockito.when(conn.isOpen()).thenReturn(true); + Mockito.when(conn.getSocket()).thenReturn(socket); + + mgr.upgrade(endpoint, context); + + Mockito.verify(tlsSocketStrategy).upgrade(socket, "virtualhost", 443, tlsConfig, context); + } }