From 0ec13b952428409ff284c9e1e7124f1d276b6e8b Mon Sep 17 00:00:00 2001 From: Colm O hEigeartaigh Date: Tue, 1 Sep 2026 11:44:49 +0100 Subject: [PATCH] Make Netty client TLS code use hostname verification --- .../NettyHttpClientPipelineFactory.java | 38 +++++++++++-- .../http/netty/client/NettyHttpConduit.java | 9 +-- .../NettyHttpClientPipelineFactoryTest.java | 57 +++++++++++++++++++ .../integration/SSLNettyClientTest.java | 21 +++++-- .../apache/cxf/transport/https/SSLUtils.java | 40 +++++++++---- .../cxf/transport/https/SSLUtilsTest.java | 25 ++++++++ 6 files changed, 166 insertions(+), 24 deletions(-) create mode 100644 rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactoryTest.java diff --git a/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactory.java b/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactory.java index a39ff9ad518..c3b53152b8b 100644 --- a/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactory.java +++ b/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactory.java @@ -82,26 +82,39 @@ public class NettyHttpClientPipelineFactory extends ChannelInitializer private final int readTimeout; private final int maxContentLength; private final boolean enableHttp2; + private final String peerHost; + private final int peerPort; + @Deprecated public NettyHttpClientPipelineFactory(TLSClientParameters clientParameters) { this(clientParameters, 0); } + @Deprecated public NettyHttpClientPipelineFactory(TLSClientParameters clientParameters, int readTimeout) { this(clientParameters, readTimeout, NettyHttpConduit.DEFAULT_MAX_RESPONSE_CONTENT_LENGTH); } + @Deprecated public NettyHttpClientPipelineFactory(TLSClientParameters clientParameters, int readTimeout, int maxResponseContentLength) { this(clientParameters, readTimeout, maxResponseContentLength, false); } + @Deprecated public NettyHttpClientPipelineFactory(TLSClientParameters clientParameters, int readTimeout, int maxResponseContentLength, boolean enableHttp2) { + this(clientParameters, readTimeout, maxResponseContentLength, enableHttp2, null, -1); + } + + public NettyHttpClientPipelineFactory(TLSClientParameters clientParameters, int readTimeout, + int maxResponseContentLength, boolean enableHttp2, String peerHost, int peerPort) { this.tlsClientParameters = clientParameters; this.readTimeout = readTimeout; this.maxContentLength = maxResponseContentLength; this.enableHttp2 = enableHttp2; + this.peerHost = peerHost; + this.peerPort = peerPort; } @Override @@ -200,9 +213,10 @@ private SslHandler configureClientSSLOnDemand(Channel channel) throws Exception final SSLEngine sslEngine; if (enableHttp2) { - final SSLContextInitParameters initParams = SSLUtils.getSSLContextInitParameters(tlsClientParameters); + final SSLContextInitParameters initParams = + SSLUtils.getSSLContextInitParameters(tlsClientParameters, peerHost != null); - sslEngine = SslContextBuilder + final SslContext sslContext = SslContextBuilder .forClient() .sslProvider(SslContext.defaultClientProvider()) .keyManager(new SimpleKeyManagerFactory() { @@ -255,10 +269,24 @@ protected TrustManager[] engineGetTrustManagers() { ApplicationProtocolNames.HTTP_1_1 ) ) - .build() - .newEngine(channel.alloc()); + .build(); + + sslEngine = peerHost != null + ? sslContext.newEngine(channel.alloc(), peerHost, peerPort) + : sslContext.newEngine(channel.alloc()); } else { - sslEngine = SSLUtils.createClientSSLEngine(tlsClientParameters); + if (peerHost != null) { + sslEngine = SSLUtils.createClientSSLEngine(tlsClientParameters, peerHost, peerPort); + } else { + sslEngine = SSLUtils.getSSLContext(tlsClientParameters).createSSLEngine(); + sslEngine.setUseClientMode(true); + } + } + + if (peerHost == null && !tlsClientParameters.isDisableCNCheck()) { + LOG.warning("No peer host information was supplied to the pipeline factory: TLS " + + "hostname verification cannot be performed. Use the constructor taking a " + + "peer host and port, or set disableCNCheck to accept the legacy behavior."); } return new SslHandler(sslEngine); diff --git a/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java b/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java index 9cbfc95d731..946324e8945 100644 --- a/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java +++ b/rt/transports/http-netty/netty-client/src/main/java/org/apache/cxf/transport/http/netty/client/NettyHttpConduit.java @@ -388,21 +388,22 @@ protected TLSClientParameters findTLSClientParameters() { protected void connect(boolean output) { final NettyHttpClientPipelineFactory handler; + final int port = url.getPort() != -1 ? url.getPort() + : "http".equals(url.getScheme()) ? 80 : 443; if ("https".equals(url.getScheme())) { TLSClientParameters clientParameters = findTLSClientParameters(); handler = new NettyHttpClientPipelineFactory(clientParameters, entity.getReceiveTimeout(), - entity.getMaxResponseContentLength(), enableHttp2); + entity.getMaxResponseContentLength(), enableHttp2, url.getHost(), port); } else { handler = new NettyHttpClientPipelineFactory(null, entity.getReceiveTimeout(), - entity.getMaxResponseContentLength(), enableHttp2); + entity.getMaxResponseContentLength(), enableHttp2, null, -1); } // Set handler bootstrap.handler(handler); ChannelFuture connFuture = - bootstrap.connect(new InetSocketAddress(url.getHost(), url.getPort() != -1 ? url.getPort() - : "http".equals(url.getScheme()) ? 80 : 443)); + bootstrap.connect(new InetSocketAddress(url.getHost(), port)); // Setup the call back on the NettyHttpClientRequest ChannelFutureListener listener = new ChannelFutureListener() { diff --git a/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactoryTest.java b/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactoryTest.java new file mode 100644 index 00000000000..2ef8e1257fe --- /dev/null +++ b/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/NettyHttpClientPipelineFactoryTest.java @@ -0,0 +1,57 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.transport.http.netty.client; + +import org.apache.cxf.configuration.jsse.TLSClientParameters; + +import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.ssl.SslHandler; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +public class NettyHttpClientPipelineFactoryTest { + + @Test + public void testHttp11SslEngineBoundToPeer() { + assertSslEngineBoundToPeer(false); + } + + @Test + public void testHttp2SslEngineBoundToPeer() { + assertSslEngineBoundToPeer(true); + } + + private void assertSslEngineBoundToPeer(boolean enableHttp2) { + NettyHttpClientPipelineFactory factory = new NettyHttpClientPipelineFactory( + new TLSClientParameters(), 0, NettyHttpConduit.DEFAULT_MAX_RESPONSE_CONTENT_LENGTH, + enableHttp2, "api.example.com", 8443); + EmbeddedChannel channel = new EmbeddedChannel(factory); + try { + SslHandler sslHandler = channel.pipeline().get(SslHandler.class); + assertNotNull(sslHandler); + assertEquals("api.example.com", sslHandler.engine().getPeerHost()); + assertEquals(8443, sslHandler.engine().getPeerPort()); + } finally { + channel.finishAndReleaseAll(); + } + } +} \ No newline at end of file diff --git a/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/integration/SSLNettyClientTest.java b/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/integration/SSLNettyClientTest.java index a914779659c..30277653022 100644 --- a/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/integration/SSLNettyClientTest.java +++ b/rt/transports/http-netty/netty-client/src/test/java/org/apache/cxf/transport/http/netty/client/integration/SSLNettyClientTest.java @@ -52,6 +52,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; public class SSLNettyClientTest extends AbstractBusClientServerTestBase { @@ -98,7 +99,7 @@ public static void stop() throws Exception { @Test public void testInvocation() throws Exception { - setupTLS(g); + setupTLS(g, true); setAddress(g, address); String response = g.greetMe("test"); assertEquals("Get a wrong response", "Hello test", response); @@ -121,7 +122,20 @@ public void handleResponse(Response res) { } - private static void setupTLS(Greeter port) + @Test + public void testRejectsCertificateForWrongHostByDefault() throws Exception { + setupTLS(g, false); + setAddress(g, address); + + try { + g.greetMe("test"); + fail("The certificate for www.service.com must not be accepted for localhost"); + } catch (Exception expected) { + // expected + } + } + + private static void setupTLS(Greeter port, boolean disableCNCheck) throws FileNotFoundException, IOException, GeneralSecurityException { String keyStoreLoc = "/keys/clientstore.jks"; @@ -140,8 +154,7 @@ private static void setupTLS(Greeter port) TrustManager[] myTrustStoreKeyManagers = getTrustManagers(trustStore); tlsCP.setTrustManagers(myTrustStoreKeyManagers); - - tlsCP.setDisableCNCheck(true); + tlsCP.setDisableCNCheck(disableCNCheck); httpConduit.setTlsClientParameters(tlsCP); } diff --git a/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java b/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java index cee024b09a6..ec82aad8dc0 100644 --- a/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java +++ b/rt/transports/http/src/main/java/org/apache/cxf/transport/https/SSLUtils.java @@ -118,6 +118,12 @@ public static HostnameVerifier getHostnameVerifier(TLSClientParameters tlsClient public static SSLContextInitParameters getSSLContextInitParameters(TLSParameterBase parameters) throws GeneralSecurityException { + return getSSLContextInitParameters(parameters, false); + } + + public static SSLContextInitParameters getSSLContextInitParameters(TLSParameterBase parameters, + boolean addHostnameVerifier) + throws GeneralSecurityException { final SSLContextInitParameters contextParameters = new SSLContextInitParameters(); @@ -131,6 +137,15 @@ public static SSLContextInitParameters getSSLContextInitParameters(TLSParameterB if (trustManagers == null && parameters instanceof TLSClientParameters) { trustManagers = org.apache.cxf.configuration.jsse.SSLUtils.getDefaultTrustStoreManagers(LOG); } + if (trustManagers != null && addHostnameVerifier && parameters instanceof TLSClientParameters) { + trustManagers = Arrays.copyOf(trustManagers, trustManagers.length); + HostnameVerifier verifier = getHostnameVerifier((TLSClientParameters)parameters); + for (int i = 0; i < trustManagers.length; i++) { + if (trustManagers[i] instanceof X509TrustManager) { + trustManagers[i] = new X509TrustManagerWrapper((X509TrustManager)trustManagers[i], verifier); + } + } + } contextParameters.setKeyManagers(configuredKeyManagers); contextParameters.setTrustManagers(trustManagers); @@ -153,17 +168,8 @@ public static SSLContext getSSLContext(TLSParameterBase parameters, boolean addH SSLContext ctx = provider == null ? SSLContext.getInstance(protocol) : SSLContext .getInstance(protocol, provider); - final SSLContextInitParameters initParams = getSSLContextInitParameters(parameters); - TrustManager[] tms = initParams.getTrustManagers(); - if (tms != null && addHNV && parameters instanceof TLSClientParameters) { - HostnameVerifier hnv = getHostnameVerifier((TLSClientParameters)parameters); - for (int i = 0; i < tms.length; i++) { - if (tms[i] instanceof X509TrustManager) { - tms[i] = new X509TrustManagerWrapper((X509TrustManager)tms[i], hnv); - } - } - } - ctx.init(initParams.getKeyManagers(), tms, parameters.getSecureRandom()); + final SSLContextInitParameters initParams = getSSLContextInitParameters(parameters, addHNV); + ctx.init(initParams.getKeyManagers(), initParams.getTrustManagers(), parameters.getSecureRandom()); if (parameters instanceof TLSClientParameters && ctx.getClientSessionContext() != null) { ctx.getClientSessionContext().setSessionTimeout(((TLSClientParameters)parameters).getSslCacheTimeout()); @@ -240,6 +246,10 @@ public static SSLEngine createServerSSLEngine(TLSServerParameters parameters) th return serverEngine; } + /** + * @deprecated use {@link #createClientSSLEngine(TLSClientParameters, String, int)} + */ + @Deprecated public static SSLEngine createClientSSLEngine(TLSClientParameters parameters) throws Exception { SSLContext sslContext = getSSLContext(parameters); SSLEngine clientEngine = sslContext.createSSLEngine(); @@ -247,6 +257,14 @@ public static SSLEngine createClientSSLEngine(TLSClientParameters parameters) th return clientEngine; } + public static SSLEngine createClientSSLEngine(TLSClientParameters parameters, + String peerHost, int peerPort) throws Exception { + SSLContext sslContext = getSSLContext(parameters, true); + SSLEngine clientEngine = sslContext.createSSLEngine(peerHost, peerPort); + clientEngine.setUseClientMode(true); + return clientEngine; + } + /* * The classes below are used by the HttpClient implementation to allow use of the * HostNameVerifier that is configured. HttpClient does not provide a hook or diff --git a/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java b/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java index b9bff35aec1..79a1f223aaa 100644 --- a/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java +++ b/rt/transports/http/src/test/java/org/apache/cxf/transport/https/SSLUtilsTest.java @@ -33,6 +33,7 @@ import javax.net.ssl.SSLEngine; import javax.net.ssl.X509TrustManager; +import org.apache.cxf.configuration.jsse.TLSClientParameters; import org.apache.cxf.configuration.jsse.TLSServerParameters; import org.apache.cxf.configuration.security.ClientAuthentication; import org.apache.cxf.transport.https.SSLUtils.SSLEngineWrapper; @@ -116,6 +117,30 @@ public void testCXF9065() throws NoSuchAlgorithmException, InterruptedException assertThat(wrapper.getSSLParameters(), is(not(nullValue()))); } + @Test + public void testCreateClientSSLEngineBoundToPeer() throws Exception { + TLSClientParameters parameters = new TLSClientParameters(); + SSLEngine clientEngine = SSLUtils.createClientSSLEngine(parameters, "api.example.com", 443); + + assertThat(clientEngine.getPeerHost(), is("api.example.com")); + assertThat(clientEngine.getPeerPort(), is(443)); + assertThat(clientEngine.getUseClientMode(), is(true)); + } + + @Test + public void testHostnameVerifierWrapsConfiguredTrustManager() throws Exception { + X509TrustManager trustManager = mock(X509TrustManager.class); + HostnameVerifier verifier = mock(HostnameVerifier.class); + TLSClientParameters parameters = new TLSClientParameters(); + parameters.setTrustManagers(new X509TrustManager[] {trustManager}); + parameters.setHostnameVerifier(verifier); + + var initParameters = SSLUtils.getSSLContextInitParameters(parameters, true); + + assertThat(initParameters.getTrustManagers()[0] instanceof X509TrustManagerWrapper, is(true)); + assertThat(parameters.getTrustManagers()[0], is(trustManager)); + } + /** * Regression test: a plain (non-extended) X509TrustManager delegate must still get * hostname verification. JSSE endpoint identification is suppressed by