ZOOKEEPER-3176: Quorum TLS - add SSL config options

Add  SSL config options for enabled protocols and client auth mode.
Improve handling of SSL config options for protocols and cipher suites - previously these came from system properties, now they can come from ZKConfig which means they are easier to isolate in tests, and now we don't need to parse system properties every time we create a secure socket.

## Added more options for ssl settings to X509Util and encapsulate them better
- previously, some SSL settings came from a `ZKConfig` and others came from global `System.getProperties()`. This made it hard to isolate certain settings in tests.
- now all SSL-related settings come from the `ZKConfig` object used to create the SSL context
- new settings added:
- `zookeeper.ssl(.quorum).enabledProtocols` - list of enabled protocols. If not set, defaults to a single-entry list with the value of `zookeeper.ssl(.quorum).protocol`.
- `zookeeper.ssl(.quorum).clientAuth` - can be "NONE", "WANT", or "NEED". This controls whether the server doesn't want / allows / requires the client to present an X509 certificate.
- `zookeeper.ssl(.quorum).handshakeDetectionTimeoutMillis` - timeout for the first read of 5 bytes to detect the transport mode (TLS or plaintext) of a client connection made to a `UnifiedServerSocket`

Author: Ilya Maykov <ilyam@fb.com>

Reviewers: andor@apache.org

Closes #681 from ivmaykov/ZOOKEEPER-3176

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

@@ -0,0 +1,192 @@
+/**
+ * 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.zookeeper.common;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.net.Socket;
+import java.util.Arrays;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLSocket;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Wrapper class for an SSLContext + some config options that can't be set on the context when it is created but
+ * must be set on a secure socket created by the context after the socket creation. By wrapping the options in this
+ * class we avoid reading from global system properties during socket configuration. This makes testing easier
+ * since we can create different X509Util instances with different configurations in a single test process, and
+ * unit test interactions between them.
+ */
+public class SSLContextAndOptions {
+    private static final Logger LOG = LoggerFactory.getLogger(SSLContextAndOptions.class);
+
+    private final X509Util x509Util;
+    private final String[] enabledProtocols;
+    private final String[] cipherSuites;
+    private final X509Util.ClientAuth clientAuth;
+    private final SSLContext sslContext;
+    private final int handshakeDetectionTimeoutMillis;
+
+    /**
+     * Note: constructor is intentionally package-private, only the X509Util class should be creating instances of this
+     * class.
+     * @param x509Util the X509Util that created this object.
+     * @param config a ZKConfig that holds config properties.
+     * @param sslContext the SSLContext.
+     */
+    SSLContextAndOptions(final X509Util x509Util, final ZKConfig config, final SSLContext sslContext) {
+        this.x509Util = requireNonNull(x509Util);
+        this.sslContext = requireNonNull(sslContext);
+        this.enabledProtocols = getEnabledProtocols(requireNonNull(config), sslContext);
+        this.cipherSuites = getCipherSuites(config);
+        this.clientAuth = getClientAuth(config);
+        this.handshakeDetectionTimeoutMillis = getHandshakeDetectionTimeoutMillis(config);
+    }
+
+    public SSLContext getSSLContext() {
+        return sslContext;
+    }
+
+    public SSLSocket createSSLSocket() throws IOException {
+        return configureSSLSocket((SSLSocket) sslContext.getSocketFactory().createSocket(), true);
+    }
+
+    public SSLSocket createSSLSocket(Socket socket, byte[] pushbackBytes) throws IOException {
+        SSLSocket sslSocket;
+        if (pushbackBytes != null && pushbackBytes.length > 0) {
+            sslSocket = (SSLSocket) sslContext.getSocketFactory().createSocket(
+                    socket, new ByteArrayInputStream(pushbackBytes), true);
+        } else {
+            sslSocket = (SSLSocket) sslContext.getSocketFactory().createSocket(
+                    socket, null, socket.getPort(), true);
+        }
+        return configureSSLSocket(sslSocket, false);
+    }
+
+    public SSLServerSocket createSSLServerSocket() throws IOException {
+        SSLServerSocket sslServerSocket =
+                (SSLServerSocket) sslContext.getServerSocketFactory().createServerSocket();
+        return configureSSLServerSocket(sslServerSocket);
+    }
+
+    public SSLServerSocket createSSLServerSocket(int port) throws IOException {
+        SSLServerSocket sslServerSocket =
+                (SSLServerSocket) sslContext.getServerSocketFactory().createServerSocket(port);
+        return configureSSLServerSocket(sslServerSocket);
+    }
+
+    public int getHandshakeDetectionTimeoutMillis() {
+        return handshakeDetectionTimeoutMillis;
+    }
+
+    private SSLSocket configureSSLSocket(SSLSocket socket, boolean isClientSocket) {
+        SSLParameters sslParameters = socket.getSSLParameters();
+        configureSslParameters(sslParameters, isClientSocket);
+        socket.setSSLParameters(sslParameters);
+        socket.setUseClientMode(isClientSocket);
+        return socket;
+    }
+
+    private SSLServerSocket configureSSLServerSocket(SSLServerSocket socket) {
+        SSLParameters sslParameters = socket.getSSLParameters();
+        configureSslParameters(sslParameters, false);
+        socket.setSSLParameters(sslParameters);
+        socket.setUseClientMode(false);
+        return socket;
+    }
+
+    private void configureSslParameters(SSLParameters sslParameters, boolean isClientSocket) {
+        if (cipherSuites != null) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Setup cipher suites for {} socket: {}",
+                        isClientSocket ? "client" : "server",
+                        Arrays.toString(cipherSuites));
+            }
+            sslParameters.setCipherSuites(cipherSuites);
+        }
+        if (enabledProtocols != null) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Setup enabled protocols for {} socket: {}",
+                        isClientSocket ? "client" : "server",
+                        Arrays.toString(enabledProtocols));
+            }
+            sslParameters.setProtocols(enabledProtocols);
+        }
+        if (!isClientSocket) {
+            switch (clientAuth) {
+                case NEED:
+                    sslParameters.setNeedClientAuth(true);
+                    break;
+                case WANT:
+                    sslParameters.setWantClientAuth(true);
+                    break;
+                default:
+                    sslParameters.setNeedClientAuth(false); // also clears the wantClientAuth flag according to docs
+                    break;
+            }
+        }
+    }
+
+    private String[] getEnabledProtocols(final ZKConfig config, final SSLContext sslContext) {
+        String enabledProtocolsInput = config.getProperty(x509Util.getSslEnabledProtocolsProperty());
+        if (enabledProtocolsInput == null) {
+            return new String[] { sslContext.getProtocol() };
+        }
+        return enabledProtocolsInput.split(",");
+    }
+
+    private String[] getCipherSuites(final ZKConfig config) {
+        String cipherSuitesInput = config.getProperty(x509Util.getSslCipherSuitesProperty());
+        if (cipherSuitesInput == null) {
+            return X509Util.getDefaultCipherSuites();
+        } else {
+            return cipherSuitesInput.split(",");
+        }
+    }
+
+    private X509Util.ClientAuth getClientAuth(final ZKConfig config) {
+        return X509Util.ClientAuth.fromPropertyValue(config.getProperty(x509Util.getSslClientAuthProperty()));
+    }
+
+    private int getHandshakeDetectionTimeoutMillis(final ZKConfig config) {
+        String propertyString = config.getProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
+        int result;
+        if (propertyString == null) {
+            result = X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
+        } else {
+            result = Integer.parseInt(propertyString);
+            if (result < 1) {
+                // Timeout of 0 is not allowed, since an infinite timeout can permanently lock up an
+                // accept() thread.
+                LOG.warn("Invalid value for {}: {}, using the default value of {}",
+                        x509Util.getSslHandshakeDetectionTimeoutMillisProperty(),
+                        result,
+                        X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS);
+                result = X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
+            }
+        }
+        return result;
+    }
+}

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

@@ -34,6 +34,7 @@ import java.security.Security;
 import java.security.cert.PKIXBuilderParameters;
 import java.security.cert.X509CertSelector;
 import java.util.Arrays;
+import java.util.Objects;
 import java.util.concurrent.atomic.AtomicReference;
 
 import javax.net.ssl.CertPathTrustManagerParameters;
@@ -97,7 +98,38 @@ public abstract class X509Util implements Closeable, AutoCloseable {
 
     public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
 
+    /**
+     * Enum specifying the client auth requirement of server-side TLS sockets created by this X509Util.
+     * <ul>
+     *     <li>NONE - do not request a client certificate.</li>
+     *     <li>WANT - request a client certificate, but allow anonymous clients to connect.</li>
+     *     <li>NEED - require a client certificate, disconnect anonymous clients.</li>
+     * </ul>
+     *
+     * If the config property is not set, the default value is NEED.
+     */
+    public enum ClientAuth {
+        NONE,
+        WANT,
+        NEED;
+
+        /**
+         * Converts a property value to a ClientAuth enum. If the input string is empty or null, returns
+         * <code>ClientAuth.NEED</code>.
+         * @param prop the property string.
+         * @return the ClientAuth.
+         * @throws IllegalArgumentException if the property value is not "NONE", "WANT", "NEED", or empty/null.
+         */
+        public static ClientAuth fromPropertyValue(String prop) {
+            if (prop == null || prop.length() == 0) {
+                return NEED;
+            }
+            return ClientAuth.valueOf(prop.toUpperCase());
+        }
+    }
+
     private String sslProtocolProperty = getConfigPrefix() + "protocol";
+    private String sslEnabledProtocolsProperty = getConfigPrefix() + "enabledProtocols";
     private String cipherSuitesProperty = getConfigPrefix() + "ciphersuites";
     private String sslKeystoreLocationProperty = getConfigPrefix() + "keyStore.location";
     private String sslKeystorePasswdProperty = getConfigPrefix() + "keyStore.password";
@@ -108,30 +140,36 @@ public abstract class X509Util implements Closeable, AutoCloseable {
     private String sslHostnameVerificationEnabledProperty = getConfigPrefix() + "hostnameVerification";
     private String sslCrlEnabledProperty = getConfigPrefix() + "crl";
     private String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
+    private String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
     private String sslHandshakeDetectionTimeoutMillisProperty = getConfigPrefix() + "handshakeDetectionTimeoutMillis";
 
-    private String[] cipherSuites;
+    private ZKConfig zkConfig;
+    private AtomicReference<SSLContextAndOptions> defaultSSLContextAndOptions = new AtomicReference<>(null);
 
-    private AtomicReference<SSLContext> defaultSSLContext = new AtomicReference<>(null);
     private FileChangeWatcher keyStoreFileWatcher;
     private FileChangeWatcher trustStoreFileWatcher;
 
     public X509Util() {
-        String cipherSuitesInput = System.getProperty(cipherSuitesProperty);
-        if (cipherSuitesInput == null) {
-            cipherSuites = getDefaultCipherSuites();
-        } else {
-            cipherSuites = cipherSuitesInput.split(",");
-        }
+        this(null);
+    }
+
+    public X509Util(ZKConfig zkConfig) {
+        this.zkConfig = zkConfig;
+        keyStoreFileWatcher = trustStoreFileWatcher = null;
     }
 
     protected abstract String getConfigPrefix();
+
     protected abstract boolean shouldVerifyClientHostname();
 
     public String getSslProtocolProperty() {
         return sslProtocolProperty;
     }
 
+    public String getSslEnabledProtocolsProperty() {
+        return sslEnabledProtocolsProperty;
+    }
+
     public String getCipherSuitesProperty() {
         return cipherSuitesProperty;
     }
@@ -140,6 +178,10 @@ public abstract class X509Util implements Closeable, AutoCloseable {
         return sslKeystoreLocationProperty;
     }
 
+    public String getSslCipherSuitesProperty() {
+        return cipherSuitesProperty;
+    }
+
     public String getSslKeystorePasswdProperty() {
         return sslKeystorePasswdProperty;
     }
@@ -172,6 +214,10 @@ public abstract class X509Util implements Closeable, AutoCloseable {
         return sslOcspEnabledProperty;
     }
 
+    public String getSslClientAuthProperty() {
+        return sslClientAuthProperty;
+    }
+
     /**
      * Returns the config property key that controls the amount of time, in milliseconds, that the first
      * UnifiedServerSocket read operation will block for when trying to detect the client mode (TLS or PLAINTEXT).
@@ -183,30 +229,37 @@ public abstract class X509Util implements Closeable, AutoCloseable {
     }
 
     public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextException {
-        SSLContext result = defaultSSLContext.get();
+        return getDefaultSSLContextAndOptions().getSSLContext();
+    }
+
+    public SSLContext createSSLContext(ZKConfig config) throws SSLContextException {
+        return createSSLContextAndOptions(config).getSSLContext();
+    }
+
+    public SSLContextAndOptions getDefaultSSLContextAndOptions() throws X509Exception.SSLContextException {
+        SSLContextAndOptions result = defaultSSLContextAndOptions.get();
         if (result == null) {
-            result = createSSLContext();
-            if (!defaultSSLContext.compareAndSet(null, result)) {
+            result = createSSLContextAndOptions();
+            if (!defaultSSLContextAndOptions.compareAndSet(null, result)) {
                 // lost the race, another thread already set the value
-                result = defaultSSLContext.get();
+                result = defaultSSLContextAndOptions.get();
             }
         }
         return result;
     }
 
-    private void resetDefaultSSLContext() throws X509Exception.SSLContextException {
-        SSLContext newContext = createSSLContext();
-        defaultSSLContext.set(newContext);
+    private void resetDefaultSSLContextAndOptions() throws X509Exception.SSLContextException {
+        SSLContextAndOptions newContext = createSSLContextAndOptions();
+        defaultSSLContextAndOptions.set(newContext);
     }
 
-    private SSLContext createSSLContext() throws SSLContextException {
+    private SSLContextAndOptions createSSLContextAndOptions() throws SSLContextException {
         /*
          * Since Configuration initializes the key store and trust store related
          * configuration from system property. Reading property from
          * configuration will be same reading from system property
          */
-        ZKConfig config=new ZKConfig();
-        return createSSLContext(config);
+        return createSSLContextAndOptions(zkConfig == null ? new ZKConfig() : zkConfig);
     }
 
     /**
@@ -217,24 +270,19 @@ public abstract class X509Util implements Closeable, AutoCloseable {
      * @return the handshake detection timeout, in milliseconds.
      */
     public int getSslHandshakeTimeoutMillis() {
-        String propertyString = System.getProperty(getSslHandshakeDetectionTimeoutMillisProperty());
-        int result;
-        if (propertyString == null) {
-            result = DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
-        } else {
-            result = Integer.parseInt(propertyString);
-            if (result < 1) {
-                // Timeout of 0 is not allowed, since an infinite timeout can permanently lock up an
-                // accept() thread.
-                LOG.warn("Invalid value for " + getSslHandshakeDetectionTimeoutMillisProperty() + ": " + result +
-                        ", using the default value of " + DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS);
-                result = DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
-            }
+        try {
+            SSLContextAndOptions ctx = getDefaultSSLContextAndOptions();
+            return ctx.getHandshakeDetectionTimeoutMillis();
+        } catch (SSLContextException e) {
+            LOG.error("Error creating SSL context and options", e);
+            return DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
+        } catch (Exception e) {
+            LOG.error("Error parsing config property " + getSslHandshakeDetectionTimeoutMillisProperty(), e);
+            return DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
         }
-        return result;
     }
 
-    public SSLContext createSSLContext(ZKConfig config) throws SSLContextException {
+    public SSLContextAndOptions createSSLContextAndOptions(ZKConfig config) throws SSLContextException {
         KeyManager[] keyManagers = null;
         TrustManager[] trustManagers = null;
 
@@ -284,12 +332,12 @@ public abstract class X509Util implements Closeable, AutoCloseable {
             }
         }
 
-        String protocol = System.getProperty(sslProtocolProperty, DEFAULT_PROTOCOL);
+        String protocol = config.getProperty(sslProtocolProperty, DEFAULT_PROTOCOL);
         try {
             SSLContext sslContext = SSLContext.getInstance(protocol);
             sslContext.init(keyManagers, trustManagers, null);
-            return sslContext;
-        } catch (NoSuchAlgorithmException|KeyManagementException sslContextInitException) {
+            return new SSLContextAndOptions(this, config, sslContext);
+        } catch (NoSuchAlgorithmException | KeyManagementException sslContextInitException) {
             throw new SSLContextException(sslContextInitException);
         }
     }
@@ -414,64 +462,40 @@ public abstract class X509Util implements Closeable, AutoCloseable {
     }
 
     public SSLSocket createSSLSocket() throws X509Exception, IOException {
-        SSLSocket sslSocket = (SSLSocket) getDefaultSSLContext().getSocketFactory().createSocket();
-        configureSSLSocket(sslSocket);
-        sslSocket.setUseClientMode(true);
-        return sslSocket;
+        return getDefaultSSLContextAndOptions().createSSLSocket();
     }
 
     public SSLSocket createSSLSocket(Socket socket, byte[] pushbackBytes) throws X509Exception, IOException {
-        SSLSocket sslSocket;
-        if (pushbackBytes != null && pushbackBytes.length > 0) {
-            sslSocket = (SSLSocket) getDefaultSSLContext().getSocketFactory().createSocket(
-                    socket, new ByteArrayInputStream(pushbackBytes), true);
-        } else {
-            sslSocket = (SSLSocket) getDefaultSSLContext().getSocketFactory().createSocket(
-                    socket, null, socket.getPort(), true);
-        }
-        configureSSLSocket(sslSocket);
-        sslSocket.setUseClientMode(false);
-        sslSocket.setNeedClientAuth(true);
-        return sslSocket;
-    }
-
-    private void configureSSLSocket(SSLSocket sslSocket) {
-        SSLParameters sslParameters = sslSocket.getSSLParameters();
-        LOG.debug("Setup cipher suites for client socket: {}", Arrays.toString(cipherSuites));
-        sslParameters.setCipherSuites(cipherSuites);
-        sslSocket.setSSLParameters(sslParameters);
+        return getDefaultSSLContextAndOptions().createSSLSocket(socket, pushbackBytes);
     }
 
     public SSLServerSocket createSSLServerSocket() throws X509Exception, IOException {
-        SSLServerSocket sslServerSocket = (SSLServerSocket) getDefaultSSLContext().getServerSocketFactory().createServerSocket();
-        configureSSLServerSocket(sslServerSocket);
-
-        return sslServerSocket;
+        return getDefaultSSLContextAndOptions().createSSLServerSocket();
     }
 
     public SSLServerSocket createSSLServerSocket(int port) throws X509Exception, IOException {
-        SSLServerSocket sslServerSocket = (SSLServerSocket) getDefaultSSLContext().getServerSocketFactory().createServerSocket(port);
-        configureSSLServerSocket(sslServerSocket);
-
-        return sslServerSocket;
+        return getDefaultSSLContextAndOptions().createSSLServerSocket(port);
     }
 
-    private void configureSSLServerSocket(SSLServerSocket sslServerSocket) {
-        SSLParameters sslParameters = sslServerSocket.getSSLParameters();
-        sslParameters.setNeedClientAuth(true);
-        LOG.debug("Setup cipher suites for server socket: {}", Arrays.toString(cipherSuites));
-        sslParameters.setCipherSuites(cipherSuites);
-        sslServerSocket.setSSLParameters(sslParameters);
+    static String[] getDefaultCipherSuites() {
+        return getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
     }
 
-    private String[] getDefaultCipherSuites() {
-        String javaVersion = System.getProperty("java.specification.version");
-        if ("9".equals(javaVersion)) {
-            LOG.debug("Using Java9-optimized cipher suites for Java version {}", javaVersion);
+    static String[] getDefaultCipherSuitesForJavaVersion(String javaVersion) {
+        Objects.requireNonNull(javaVersion);
+        if (javaVersion.matches("\\d+")) {
+            // Must be Java 9 or later
+            LOG.debug("Using Java9+ optimized cipher suites for Java version {}", javaVersion);
             return DEFAULT_CIPHERS_JAVA9;
+        } else if (javaVersion.startsWith("1.")) {
+            // Must be Java 1.8 or earlier
+            LOG.debug("Using Java8 optimized cipher suites for Java version {}", javaVersion);
+            return DEFAULT_CIPHERS_JAVA8;
+        } else {
+            LOG.debug("Could not parse java version {}, using Java8 optimized cipher suites",
+                    javaVersion);
+            return DEFAULT_CIPHERS_JAVA8;
         }
-        LOG.debug("Using Java8-optimized cipher suites for Java version {}", javaVersion);
-        return DEFAULT_CIPHERS_JAVA8;
     }
 
     private FileChangeWatcher newFileChangeWatcher(String fileLocation) throws IOException {
@@ -498,7 +522,7 @@ public abstract class X509Util implements Closeable, AutoCloseable {
      */
     public void enableCertFileReloading() throws IOException {
         LOG.info("enabling cert file reloading");
-        ZKConfig config = new ZKConfig();
+        ZKConfig config = zkConfig == null ? new ZKConfig() : zkConfig;
         FileChangeWatcher newKeyStoreFileWatcher =
                 newFileChangeWatcher(config.getProperty(sslKeystoreLocationProperty));
         if (newKeyStoreFileWatcher != null) {
@@ -563,7 +587,7 @@ public abstract class X509Util implements Closeable, AutoCloseable {
                         event.kind() + " with context: " + event.context());
             }
             try {
-                this.resetDefaultSSLContext();
+                this.resetDefaultSSLContextAndOptions();
             } catch (SSLContextException e) {
                 throw new RuntimeException(e);
             }

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

@@ -115,6 +115,12 @@ public class ZKConfig {
     }
     
     private void putSSLProperties(X509Util x509Util) {
+        properties.put(x509Util.getSslProtocolProperty(),
+                System.getProperty(x509Util.getSslProtocolProperty()));
+        properties.put(x509Util.getSslEnabledProtocolsProperty(),
+                System.getProperty(x509Util.getSslEnabledProtocolsProperty()));
+        properties.put(x509Util.getSslCipherSuitesProperty(),
+                System.getProperty(x509Util.getSslCipherSuitesProperty()));
         properties.put(x509Util.getSslKeystoreLocationProperty(),
                 System.getProperty(x509Util.getSslKeystoreLocationProperty()));
         properties.put(x509Util.getSslKeystorePasswdProperty(),
@@ -133,6 +139,8 @@ public class ZKConfig {
                 System.getProperty(x509Util.getSslCrlEnabledProperty()));
         properties.put(x509Util.getSslOcspEnabledProperty(),
                 System.getProperty(x509Util.getSslOcspEnabledProperty()));
+        properties.put(x509Util.getSslClientAuthProperty(),
+                System.getProperty(x509Util.getSslClientAuthProperty()));
         properties.put(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(),
                 System.getProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty()));
     }

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

@@ -482,6 +482,46 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
         }
     }
 
+    @Test
+    public void testGetDefaultCipherSuitesJava8() {
+        String[] cipherSuites = X509Util.getDefaultCipherSuitesForJavaVersion("1.8");
+        // Java 8 default should have the CBC suites first
+        Assert.assertTrue(cipherSuites[0].contains("CBC"));
+    }
+
+    @Test
+    public void testGetDefaultCipherSuitesJava9() {
+        String[] cipherSuites = X509Util.getDefaultCipherSuitesForJavaVersion("9");
+        // Java 9+ default should have the GCM suites first
+        Assert.assertTrue(cipherSuites[0].contains("GCM"));
+    }
+
+    @Test
+    public void testGetDefaultCipherSuitesJava10() {
+        String[] cipherSuites = X509Util.getDefaultCipherSuitesForJavaVersion("10");
+        // Java 9+ default should have the GCM suites first
+        Assert.assertTrue(cipherSuites[0].contains("GCM"));
+    }
+
+    @Test
+    public void testGetDefaultCipherSuitesJava11() {
+        String[] cipherSuites = X509Util.getDefaultCipherSuitesForJavaVersion("11");
+        // Java 9+ default should have the GCM suites first
+        Assert.assertTrue(cipherSuites[0].contains("GCM"));
+    }
+
+    @Test
+    public void testGetDefaultCipherSuitesUnknownVersion() {
+        String[] cipherSuites = X509Util.getDefaultCipherSuitesForJavaVersion("notaversion");
+        // If version can't be parsed, use the more conservative Java 8 default
+        Assert.assertTrue(cipherSuites[0].contains("CBC"));
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testGetDefaultCipherSuitesNullVersion() {
+        X509Util.getDefaultCipherSuitesForJavaVersion(null);
+    }
+
     // Warning: this will reset the x509Util
     private void setCustomCipherSuites() {
         System.setProperty(x509Util.getCipherSuitesProperty(), customCipherSuites[0] + "," + customCipherSuites[1]);