ZOOKEEPER-4986: Disable reverse DNS lookup in TLS client and server (branch-3.9)

zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

@@ -1759,6 +1759,16 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
     This option requires the corresponding *hostnameVerification* option to be `true`, or it will be ignored.
     Default: true for quorum, false for clients
 
+* *ssl.allowReverseDnsLookup* and *ssl.quorum.allowReverseDnsLookup* :
+    (Java system properties: **zookeeper.ssl.allowReverseDnsLookup** and **zookeeper.ssl.quorum.allowReverseDnsLookup**)
+    **New in 3.9.5:**
+    Allow reverse DNS lookup in both server- and client hostname verifications if the hostname verification is enabled in
+    `ZKTrustManager`. Supported in both quorum and client TLS protocols. Not supported in FIPS mode. Reverse DNS lookups are
+    expensive and unnecessary in most cases. Make sure that certificates are created with all required Subject Alternative
+    Names (SAN) for successful identity verification. It's recommended to add SAN:IP entries for identity verification
+    of client certificates.
+    Default: false (for Client connections), true (for Quorum connections)
+
 * *ssl.crl* and *ssl.quorum.crl* :
     (Java system properties: **zookeeper.ssl.crl** and **zookeeper.ssl.quorum.crl**)
     **New in 3.5.5:**

zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java

@@ -52,6 +52,11 @@ protected boolean shouldVerifyClientHostname() {
         return false;
     }
 
+    @Override
+    protected boolean shouldAllowReverseDnsLookup() {
+        return false;
+    }
+
     public String getSslAuthProviderProperty() {
         return sslAuthProviderProperty;
     }
@@ -202,14 +207,16 @@ private TrustManager getTrustManager(ZKConfig config) throws X509Exception.Trust
         boolean sslOcspEnabled = config.getBoolean(getSslOcspEnabledProperty());
         boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
         boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
+        boolean allowReverseDnsLookup = allowReverseDnsLookup(config);
 
         if (trustStoreLocation.isEmpty()) {
             LOG.warn("{} not specified", getSslTruststoreLocationProperty());
             return null;
         } else {
             return createTrustManager(trustStoreLocation, trustStorePassword, trustStoreType,
                 sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled,
-                sslClientHostnameVerificationEnabled, getFipsMode(config));
+                sslClientHostnameVerificationEnabled, allowReverseDnsLookup,
+                getFipsMode(config));
         }
     }
 }

zookeeper-server/src/main/java/org/apache/zookeeper/common/QuorumX509Util.java

@@ -33,4 +33,8 @@ protected boolean shouldVerifyClientHostname() {
         return true;
     }
 
+    @Override
+    protected boolean shouldAllowReverseDnsLookup() {
+        return true;
+    }
 }

zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java

@@ -198,6 +198,7 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() {
     private final String sslContextSupplierClassProperty = getConfigPrefix() + "context.supplier.class";
     private final String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
     private final String sslClientHostnameVerificationEnabledProperty = getConfigPrefix() + "clientHostnameVerification";
+    private final String sslAllowReverseDnsLookupProperty = getConfigPrefix() + "allowReverseDnsLookup";
     private final String sslCrlEnabledProperty = getConfigPrefix() + "crl";
     private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
     private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
@@ -216,6 +217,8 @@ public X509Util() {
 
     protected abstract boolean shouldVerifyClientHostname();
 
+    protected abstract boolean shouldAllowReverseDnsLookup();
+
     public String getSslProtocolProperty() {
         return sslProtocolProperty;
     }
@@ -276,6 +279,10 @@ public String getSslClientHostnameVerificationEnabledProperty() {
         return sslClientHostnameVerificationEnabledProperty;
     }
 
+    public String getSslAllowReverseDnsLookupProperty() {
+        return sslAllowReverseDnsLookupProperty;
+    }
+
     public String getSslCrlEnabledProperty() {
         return sslCrlEnabledProperty;
     }
@@ -315,6 +322,10 @@ public boolean isClientHostnameVerificationEnabled(ZKConfig config) {
             && config.getBoolean(this.getSslClientHostnameVerificationEnabledProperty(), shouldVerifyClientHostname());
     }
 
+    public boolean allowReverseDnsLookup(ZKConfig config) {
+        return config.getBoolean(this.getSslAllowReverseDnsLookupProperty(), shouldAllowReverseDnsLookup());
+    }
+
     public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException {
         return getDefaultSSLContextAndOptions().getSSLContext();
     }
@@ -432,6 +443,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config
         boolean sslOcspEnabled = config.getBoolean(this.sslOcspEnabledProperty);
         boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config);
         boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config);
+        boolean allowReverseDnsLookup = allowReverseDnsLookup(config);
         boolean fipsMode = getFipsMode(config);
 
         if (trustStoreLocationProp.isEmpty()) {
@@ -441,7 +453,7 @@ public SSLContextAndOptions createSSLContextAndOptionsFromConfig(ZKConfig config
                 trustManagers = new TrustManager[]{
                     createTrustManager(trustStoreLocationProp, trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled,
                         sslOcspEnabled, sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled,
-                        fipsMode)};
+                        allowReverseDnsLookup, fipsMode)};
             } catch (TrustManagerException trustManagerException) {
                 throw new SSLContextException("Failed to create TrustManager", trustManagerException);
             } catch (IllegalArgumentException e) {
@@ -577,6 +589,7 @@ public static X509TrustManager createTrustManager(
         boolean ocspEnabled,
         final boolean serverHostnameVerificationEnabled,
         final boolean clientHostnameVerificationEnabled,
+        final boolean allowReverseDnsLookup,
         final boolean fipsMode) throws TrustManagerException {
         if (trustStorePassword == null) {
             trustStorePassword = "";
@@ -611,7 +624,7 @@ public static X509TrustManager createTrustManager(
                         LOG.debug("FIPS mode is OFF: creating ZKTrustManager");
                     }
                     return new ZKTrustManager((X509ExtendedTrustManager) tm, serverHostnameVerificationEnabled,
-                        clientHostnameVerificationEnabled);
+                        clientHostnameVerificationEnabled, allowReverseDnsLookup);
                 }
             }
             throw new TrustManagerException("Couldn't find X509TrustManager");

zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java

@@ -151,6 +151,7 @@ private void putSSLProperties(X509Util x509Util) {
         properties.put(x509Util.getSslContextSupplierClassProperty(), System.getProperty(x509Util.getSslContextSupplierClassProperty()));
         properties.put(x509Util.getSslClientHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslClientHostnameVerificationEnabledProperty()));
         properties.put(x509Util.getSslHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
+        properties.put(x509Util.getSslAllowReverseDnsLookupProperty(), System.getProperty(x509Util.getSslAllowReverseDnsLookupProperty()));
         properties.put(x509Util.getSslCrlEnabledProperty(), System.getProperty(x509Util.getSslCrlEnabledProperty()));
         properties.put(x509Util.getSslOcspEnabledProperty(), System.getProperty(x509Util.getSslOcspEnabledProperty()));
         properties.put(x509Util.getSslClientAuthProperty(), System.getProperty(x509Util.getSslClientAuthProperty()));

zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKTrustManager.java

@@ -42,6 +42,7 @@ public class ZKTrustManager extends X509ExtendedTrustManager {
     private final X509ExtendedTrustManager x509ExtendedTrustManager;
     private final boolean serverHostnameVerificationEnabled;
     private final boolean clientHostnameVerificationEnabled;
+    private final boolean allowReverseDnsLookup;
 
     private final ZKHostnameVerifier hostnameVerifier;
 
@@ -57,22 +58,26 @@ public class ZKTrustManager extends X509ExtendedTrustManager {
     ZKTrustManager(
         X509ExtendedTrustManager x509ExtendedTrustManager,
         boolean serverHostnameVerificationEnabled,
-        boolean clientHostnameVerificationEnabled) {
+        boolean clientHostnameVerificationEnabled,
+        boolean allowReverseDnsLookup) {
         this(x509ExtendedTrustManager,
                 serverHostnameVerificationEnabled,
                 clientHostnameVerificationEnabled,
-                new ZKHostnameVerifier());
+                new ZKHostnameVerifier(),
+                allowReverseDnsLookup);
     }
 
     ZKTrustManager(
             X509ExtendedTrustManager x509ExtendedTrustManager,
             boolean serverHostnameVerificationEnabled,
             boolean clientHostnameVerificationEnabled,
-            ZKHostnameVerifier hostnameVerifier) {
+            ZKHostnameVerifier hostnameVerifier,
+            boolean allowReverseDnsLookup) {
         this.x509ExtendedTrustManager = x509ExtendedTrustManager;
         this.serverHostnameVerificationEnabled = serverHostnameVerificationEnabled;
         this.clientHostnameVerificationEnabled = clientHostnameVerificationEnabled;
         this.hostnameVerifier = hostnameVerifier;
+        this.allowReverseDnsLookup = allowReverseDnsLookup;
     }
 
     @Override
@@ -166,31 +171,46 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) throws
      * @param certificate Peer's certificate
      * @throws CertificateException Thrown if the provided certificate doesn't match the peer hostname.
      */
-    private void performHostVerification(
-        InetAddress inetAddress,
-        X509Certificate certificate
-    ) throws CertificateException {
+    private void performHostVerification(InetAddress inetAddress, X509Certificate certificate)
+        throws CertificateException {
         String hostAddress = "";
         String hostName = "";
         try {
             hostAddress = inetAddress.getHostAddress();
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("Trying to verify host address first: {}", hostAddress);
-            }
             hostnameVerifier.verify(hostAddress, certificate);
         } catch (SSLException addressVerificationException) {
+            // If we fail with hostAddress, we should try the hostname.
+            // The inetAddress may have been created with a hostname, in which case getHostName() will
+            // return quickly below. If not, a reverse lookup will happen, which can be expensive.
+            // We provide the option to skip the reverse lookup if preferring to fail fast.
+
+            // Handle logging here to aid debugging. The easiest way to check for an existing
+            // hostname is through toString, see javadoc.
+            String inetAddressString = inetAddress.toString();
+            if (!inetAddressString.startsWith("/")) {
+                LOG.debug(
+                    "Failed to verify host address: {}, but inetAddress {} has a hostname, trying that",
+                    hostAddress, inetAddressString, addressVerificationException);
+            } else if (allowReverseDnsLookup) {
+                LOG.debug(
+                    "Failed to verify host address: {}, attempting to verify host name with reverse dns",
+                    hostAddress, addressVerificationException);
+            } else {
+                LOG.debug("Failed to verify host address: {}, but reverse dns lookup is disabled",
+                    hostAddress, addressVerificationException);
+                throw new CertificateException(
+                    "Failed to verify host address, and reverse lookup is disabled",
+                    addressVerificationException);
+            }
+
             try {
                 hostName = inetAddress.getHostName();
-                if (LOG.isDebugEnabled()) {
-                    LOG.debug(
-                        "Failed to verify host address: {}, trying to verify host name: {}",
-                        hostAddress, hostName);
-                }
                 hostnameVerifier.verify(hostName, certificate);
             } catch (SSLException hostnameVerificationException) {
                 LOG.error("Failed to verify host address: {}", hostAddress, addressVerificationException);
                 LOG.error("Failed to verify hostname: {}", hostName, hostnameVerificationException);
-                throw new CertificateException("Failed to verify both host address and host name", hostnameVerificationException);
+                throw new CertificateException("Failed to verify both host address and host name",
+                    hostnameVerificationException);
             }
         }
     }

zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java

@@ -88,6 +88,7 @@ public X509AuthenticationProvider() throws X509Exception {
             boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
             boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
             boolean hostnameVerificationEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
+            boolean allowReverseDnsLookup = Boolean.parseBoolean(config.getProperty(x509Util.getSslAllowReverseDnsLookupProperty()));
 
             X509KeyManager km = null;
             X509TrustManager tm = null;
@@ -120,6 +121,7 @@ public X509AuthenticationProvider() throws X509Exception {
                         ocspEnabled,
                         hostnameVerificationEnabled,
                         false,
+                        allowReverseDnsLookup,
                         fipsMode);
                 } catch (TrustManagerException e) {
                     LOG.error("Failed to create trust manager", e);

zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java

@@ -381,6 +381,7 @@ public void testLoadPEMTrustStore(
             false,
             true,
             true,
+            false,
             false);
     }
 
@@ -402,6 +403,7 @@ public void testLoadPEMTrustStoreNullPassword(
             false,
             true,
             true,
+            false,
             false);
 
     }
@@ -421,6 +423,7 @@ public void testLoadPEMTrustStoreAutodetectStoreFileType(
             false,
             true,
             true,
+            false,
             false);
     }
 
@@ -496,6 +499,7 @@ public void testLoadJKSTrustStore(
             true,
             true,
             true,
+            false,
             false);
     }
 
@@ -517,6 +521,7 @@ public void testLoadJKSTrustStoreNullPassword(
             false,
             true,
             true,
+            false,
             false);
     }
 
@@ -535,6 +540,7 @@ public void testLoadJKSTrustStoreAutodetectStoreFileType(
             true,
             true,
             true,
+            false,
             false);
     }
 
@@ -554,6 +560,7 @@ public void testLoadJKSTrustStoreWithWrongPassword(
                     true,
                     true,
                     true,
+                    false,
                     false);
         });
     }
@@ -629,6 +636,7 @@ public void testLoadPKCS12TrustStore(
             true,
             true,
             true,
+            false,
             false);
     }
 
@@ -650,6 +658,7 @@ public void testLoadPKCS12TrustStoreNullPassword(
             false,
             true,
             true,
+            false,
             false);
     }
 
@@ -668,6 +677,7 @@ public void testLoadPKCS12TrustStoreAutodetectStoreFileType(
             true,
             true,
             true,
+            false,
             false);
     }
 
@@ -687,6 +697,7 @@ public void testLoadPKCS12TrustStoreWithWrongPassword(
                     true,
                     true,
                     true,
+                    false,
                     false);
         });
     }