Make Netty client TLS code use hostname verification

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<Channel>
     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);

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() {

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();
+        }
+    }
+}

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<GreetMeResponse> 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);
     }
 

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,13 +246,25 @@ 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();
         clientEngine.setUseClientMode(true);
         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

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