Allow to configure TLS hostname verification in PulsarAdmin (apache#1702

)

* Allow to configure TLS hostname verification in PulsarAdmin

* Fixed test and typo

conf/client.conf

@@ -17,11 +17,12 @@
 # under the License.
 #
 
-# Pulsar Client configuration
+# Pulsar Client and pulsar-admin configuration
 webServiceUrl=http://localhost:8080/
 brokerServiceUrl=pulsar://localhost:6650/
 #authPlugin=
 #authParams=
 #useTls=
-#tlsAllowInsecureConnection
+tlsAllowInsecureConnection=false
+tlsEnableHostnameVerification=false
 #tlsTrustCertsFilePath

pulsar-broker/src/test/java/org/apache/pulsar/client/api/TlsHostVerification.java

@@ -0,0 +1,64 @@
+/**
+ * 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.pulsar.client.api;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.apache.pulsar.client.impl.auth.AuthenticationTls;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class TlsHostVerification extends TlsProducerConsumerBase {
+    @Test
+    public void testTlsHostVerificationAdminClient() throws Exception {
+        Map<String, String> authParams = new HashMap<>();
+        authParams.put("tlsCertFile", TLS_CLIENT_CERT_FILE_PATH);
+        authParams.put("tlsKeyFile", TLS_CLIENT_KEY_FILE_PATH);
+        PulsarAdmin adminClientTls = PulsarAdmin.builder()
+                .serviceHttpUrl("https://127.0.0.1:" + BROKER_WEBSERVICE_PORT_TLS)
+                .tlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH).allowTlsInsecureConnection(false)
+                .authentication(AuthenticationTls.class.getName(), authParams).enableTlsHostnameVerification(true)
+                .build();
+
+        try {
+            adminClientTls.tenants().getTenants();
+            Assert.fail("Admin call should be failed due to hostnameVerification enabled");
+        } catch (PulsarAdminException e) {
+            // Ok
+        }
+    }
+
+    @Test
+    public void testTlsHostVerificationDisabledAdminClient() throws Exception {
+        Map<String, String> authParams = new HashMap<>();
+        authParams.put("tlsCertFile", TLS_CLIENT_CERT_FILE_PATH);
+        authParams.put("tlsKeyFile", TLS_CLIENT_KEY_FILE_PATH);
+        PulsarAdmin adminClient = PulsarAdmin.builder()
+                .serviceHttpUrl("https://127.0.0.1:" + BROKER_WEBSERVICE_PORT_TLS)
+                .tlsTrustCertsFilePath(TLS_TRUST_CERT_FILE_PATH).allowTlsInsecureConnection(false)
+                .authentication(AuthenticationTls.class.getName(), authParams).enableTlsHostnameVerification(false)
+                .build();
+
+        // Should not fail, since verification is disabled
+        adminClient.tenants().getTenants();
+    }
+}

pulsar-broker/src/test/java/org/apache/pulsar/client/api/TlsProducerConsumerTest.java

@@ -80,10 +80,8 @@ public void testTlsClientAuthOverBinaryProtocol() throws Exception {
         // Test 1 - Using TLS on binary protocol without sending certs - expect failure
         internalSetUpForClient(false, "pulsar+ssl://localhost:" + BROKER_PORT_TLS);
         try {
-            ConsumerConfiguration conf = new ConsumerConfiguration();
-            conf.setSubscriptionType(SubscriptionType.Exclusive);
-            Consumer consumer = pulsarClient.subscribe("persistent://my-property/use/my-ns/my-topic1",
-                    "my-subscriber-name", conf);
+            pulsarClient.newConsumer().topic("persistent://my-property/use/my-ns/my-topic1")
+                    .subscriptionName("my-subscriber-name").subscriptionType(SubscriptionType.Exclusive).subscribe();
             Assert.fail("Server should have failed the TLS handshake since client didn't .");
         } catch (Exception ex) {
             // OK
@@ -92,10 +90,8 @@ public void testTlsClientAuthOverBinaryProtocol() throws Exception {
         // Test 2 - Using TLS on binary protocol - sending certs
         internalSetUpForClient(true, "pulsar+ssl://localhost:" + BROKER_PORT_TLS);
         try {
-            ConsumerConfiguration conf = new ConsumerConfiguration();
-            conf.setSubscriptionType(SubscriptionType.Exclusive);
-            Consumer consumer = pulsarClient.subscribe("persistent://my-property/use/my-ns/my-topic1",
-                    "my-subscriber-name", conf);
+            pulsarClient.newConsumer().topic("persistent://my-property/use/my-ns/my-topic1")
+                    .subscriptionName("my-subscriber-name").subscriptionType(SubscriptionType.Exclusive).subscribe();
         } catch (Exception ex) {
             Assert.fail("Should not fail since certs are sent.");
         }
@@ -112,10 +108,8 @@ public void testTlsClientAuthOverHTTPProtocol() throws Exception {
         // Test 1 - Using TLS on https without sending certs - expect failure
         internalSetUpForClient(false, "https://localhost:" + BROKER_WEBSERVICE_PORT_TLS);
         try {
-            ConsumerConfiguration conf = new ConsumerConfiguration();
-            conf.setSubscriptionType(SubscriptionType.Exclusive);
-            Consumer consumer = pulsarClient.subscribe("persistent://my-property/use/my-ns/my-topic1",
-                    "my-subscriber-name", conf);
+            pulsarClient.newConsumer().topic("persistent://my-property/use/my-ns/my-topic1")
+                    .subscriptionName("my-subscriber-name").subscriptionType(SubscriptionType.Exclusive).subscribe();
             Assert.fail("Server should have failed the TLS handshake since client didn't .");
         } catch (Exception ex) {
             // OK
@@ -124,10 +118,8 @@ public void testTlsClientAuthOverHTTPProtocol() throws Exception {
         // Test 2 - Using TLS on https - sending certs
         internalSetUpForClient(true, "https://localhost:" + BROKER_WEBSERVICE_PORT_TLS);
         try {
-            ConsumerConfiguration conf = new ConsumerConfiguration();
-            conf.setSubscriptionType(SubscriptionType.Exclusive);
-            Consumer consumer = pulsarClient.subscribe("persistent://my-property/use/my-ns/my-topic1",
-                    "my-subscriber-name", conf);
+            pulsarClient.newConsumer().topic("persistent://my-property/use/my-ns/my-topic1")
+                    .subscriptionName("my-subscriber-name").subscriptionType(SubscriptionType.Exclusive).subscribe();
         } catch (Exception ex) {
             Assert.fail("Should not fail since certs are sent.");
         }

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdmin.java

@@ -30,6 +30,8 @@
 import javax.ws.rs.client.WebTarget;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.http.conn.ssl.DefaultHostnameVerifier;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.pulsar.client.admin.internal.BrokerStatsImpl;
 import org.apache.pulsar.client.admin.internal.BrokersImpl;
 import org.apache.pulsar.client.admin.internal.ClustersImpl;
@@ -147,6 +149,12 @@ public PulsarAdmin(String serviceUrl, ClientConfigurationData clientConfigData)
                 }
 
                 clientBuilder.sslContext(sslCtx);
+                if (clientConfigData.isTlsHostnameVerificationEnable()) {
+                    clientBuilder.hostnameVerifier(new DefaultHostnameVerifier());
+                } else {
+                    // Disable hostname verification
+                    clientBuilder.hostnameVerifier(NoopHostnameVerifier.INSTANCE);
+                }
             } catch (Exception e) {
                 try {
                     if (auth != null) {

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/PulsarAdminBuilder.java

@@ -159,4 +159,14 @@ PulsarAdminBuilder authentication(String authPluginClassName, Map<String, String
      */
     PulsarAdminBuilder allowTlsInsecureConnection(boolean allowTlsInsecureConnection);
 
+    /**
+     * It allows to validate hostname verification when client connects to broker over TLS. It validates incoming x509
+     * certificate and matches provided hostname(CN/SAN) with expected broker's host name. It follows RFC 2818, 3.1.
+     * Server Identity hostname verification.
+     *
+     * @see <a href="https://tools.ietf.org/html/rfc2818">rfc2818</a>
+     *
+     * @param enableTlsHostnameVerification
+     */
+    PulsarAdminBuilder enableTlsHostnameVerification(boolean enableTlsHostnameVerification);
 }

pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/PulsarAdminBuilderImpl.java

@@ -87,4 +87,10 @@ public PulsarAdminBuilder allowTlsInsecureConnection(boolean allowTlsInsecureCon
         conf.setTlsAllowInsecureConnection(allowTlsInsecureConnection);
         return this;
     }
+
+    @Override
+    public PulsarAdminBuilder enableTlsHostnameVerification(boolean enableTlsHostnameVerification) {
+        conf.setTlsHostnameVerificationEnable(enableTlsHostnameVerification);
+        return this;
+    }
 }

pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/PulsarAdminTool.java

@@ -46,6 +46,13 @@ public class PulsarAdminTool {
     @Parameter(names = { "--auth-params" }, description = "Authentication parameters, e.g., \"key1:val1,key2:val2\".")
     String authParams = null;
 
+    @Parameter(names = { "--tls-allow-insecure" }, description = "Allow TLS insecure connection")
+    Boolean tlsAllowInsecureConnection;
+
+
+    @Parameter(names = { "--tls-enable-hostname-verification" }, description = "Enable TLS common name verification")
+    Boolean tlsEnableHostnameVerification;
+
     @Parameter(names = { "-h", "--help", }, help = true, description = "Show this help.")
     boolean help;
 
@@ -56,10 +63,16 @@ public class PulsarAdminTool {
                 : properties.getProperty("serviceUrl");
         authPluginClassName = properties.getProperty("authPlugin");
         authParams = properties.getProperty("authParams");
-        boolean tlsAllowInsecureConnection = Boolean.parseBoolean(properties.getProperty("tlsAllowInsecureConnection"));
+        boolean tlsAllowInsecureConnection = this.tlsAllowInsecureConnection != null ? this.tlsAllowInsecureConnection
+                : Boolean.parseBoolean(properties.getProperty("tlsAllowInsecureConnection", "false"));
+
+        boolean tlsEnableHostnameVerification = this.tlsEnableHostnameVerification != null
+                ? this.tlsEnableHostnameVerification
+                : Boolean.parseBoolean(properties.getProperty("tlsEnableHostnameVerification", "false"));
         String tlsTrustCertsFilePath = properties.getProperty("tlsTrustCertsFilePath");
 
         adminBuilder = PulsarAdmin.builder().allowTlsInsecureConnection(tlsAllowInsecureConnection)
+                .enableTlsHostnameVerification(tlsEnableHostnameVerification)
                 .tlsTrustCertsFilePath(tlsTrustCertsFilePath);
 
         jcommander = new JCommander();

pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/PulsarClientTool.java

@@ -53,6 +53,7 @@ public class PulsarClientTool {
 
     boolean useTls = false;
     boolean tlsAllowInsecureConnection = false;
+    boolean tlsEnableHostnameVerification = false;
     String tlsTrustCertsFilePath = null;
 
     JCommander commandParser;
@@ -69,7 +70,10 @@ public PulsarClientTool(Properties properties) throws MalformedURLException {
         this.authPluginClassName = properties.getProperty("authPlugin");
         this.authParams = properties.getProperty("authParams");
         this.useTls = Boolean.parseBoolean(properties.getProperty("useTls"));
-        this.tlsAllowInsecureConnection = Boolean.parseBoolean(properties.getProperty("tlsAllowInsecureConnection"));
+        this.tlsAllowInsecureConnection = Boolean
+                .parseBoolean(properties.getProperty("tlsAllowInsecureConnection", "false"));
+        this.tlsEnableHostnameVerification = Boolean
+                .parseBoolean(properties.getProperty("tlsEnableHostnameVerification", "false"));
         this.tlsTrustCertsFilePath = properties.getProperty("tlsTrustCertsFilePath");
 
         produceCommand = new CmdProduce();