zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From an...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
Date Sat, 15 Dec 2018 06:35:23 GMT
This is an automated email from the ASF dual-hosted git repository.

andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new e043c32  ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
e043c32 is described below

commit e043c322f12d56da0fc88131628edf0731c0f8e4
Author: Ilya Maykov <ilyam@fb.com>
AuthorDate: Sat Dec 15 07:34:54 2018 +0100

    ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store
    
    Allow reloading SSL trust stores and key stores from disk when the files on disk change.
    
    ## Added support for reloading key/trust stores when the file on disk changes
    - new property `sslQuorumReloadCertFiles` which controls the behavior for reloading the key and trust store files for `QuorumX509Util`. Reloading of key and trust store for `ClientX509Util` is not in this PR but could be added easily
    - this allows a ZK server to keep running on a machine that uses short-lived certs that refresh frequently without having to restart the ZK process.
    
    Author: Ilya Maykov <ilyam@fb.com>
    
    Reviewers: andor@apache.org
    
    Closes #680 from ivmaykov/ZOOKEEPER-3174
---
 .../apache/zookeeper/ClientCnxnSocketNetty.java    |   9 +-
 .../zookeeper/client/FourLetterWordMain.java       |  15 +-
 .../apache/zookeeper/common/FileChangeWatcher.java | 253 ++++++++++++++++++++
 .../java/org/apache/zookeeper/common/X509Util.java | 115 ++++++++-
 .../java/org/apache/zookeeper/common/ZKConfig.java |  13 +-
 .../zookeeper/server/NettyServerCnxnFactory.java   |   2 +
 .../server/auth/X509AuthenticationProvider.java    |  74 +++---
 .../org/apache/zookeeper/server/quorum/Leader.java |  18 +-
 .../zookeeper/server/quorum/QuorumCnxManager.java  |   4 +-
 .../apache/zookeeper/server/quorum/QuorumPeer.java |   1 +
 .../zookeeper/server/quorum/QuorumPeerConfig.java  |  22 +-
 .../zookeeper/server/quorum/QuorumPeerMain.java    |   3 +
 .../zookeeper/common/FileChangeWatcherTest.java    | 263 +++++++++++++++++++++
 .../org/apache/zookeeper/common/X509UtilTest.java  |  36 +--
 .../server/quorum/QuorumPeerConfigTest.java        |  23 +-
 .../zookeeper/server/quorum/QuorumSSLTest.java     |   4 +-
 .../UnifiedServerSocketModeDetectionTest.java      |   1 +
 .../server/quorum/UnifiedServerSocketTest.java     |   1 +
 .../org/apache/zookeeper/test/ClientSSLTest.java   |   4 +-
 .../org/apache/zookeeper/test/SSLAuthTest.java     |   5 +-
 20 files changed, 760 insertions(+), 106 deletions(-)

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
index 74d1283..c4a7301 100755
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
@@ -54,6 +54,7 @@ import org.apache.zookeeper.ClientCnxn.Packet;
 import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.common.ClientX509Util;
 import org.apache.zookeeper.common.NettyUtils;
+import org.apache.zookeeper.common.X509Util;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -436,9 +437,11 @@ public class ClientCnxnSocketNetty extends ClientCnxnSocket {
         // Basically we only need to create it once.
         private synchronized void initSSL(ChannelPipeline pipeline) throws SSLContextException {
             if (sslContext == null || sslEngine == null) {
-                sslContext = new ClientX509Util().createSSLContext(clientConfig);
-                sslEngine = sslContext.createSSLEngine(host,port);
-                sslEngine.setUseClientMode(true);
+                try (X509Util x509Util = new ClientX509Util()) {
+                    sslContext = x509Util.createSSLContext(clientConfig);
+                    sslEngine = sslContext.createSSLEngine(host, port);
+                    sslEngine.setUseClientMode(true);
+                }
             }
             pipeline.addLast("ssl", new SslHandler(sslEngine));
             LOG.info("SSL handler added for channel: {}", pipeline.channel());
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
index 41f5e9d..a98953f 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/client/FourLetterWordMain.java
@@ -34,6 +34,7 @@ import javax.net.ssl.SSLSocketFactory;
 import org.apache.zookeeper.common.ClientX509Util;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.common.X509Exception.SSLContextException;
+import org.apache.zookeeper.common.X509Util;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -90,12 +91,14 @@ public class FourLetterWordMain {
             new InetSocketAddress(InetAddress.getByName(null), port);
         if (secure) {
             LOG.info("using secure socket");
-            SSLContext sslContext = new ClientX509Util().getDefaultSSLContext();
-            SSLSocketFactory socketFactory = sslContext.getSocketFactory();
-            SSLSocket sslSock = (SSLSocket) socketFactory.createSocket();
-            sslSock.connect(hostaddress, timeout);
-            sslSock.startHandshake();
-            sock = sslSock;
+            try (X509Util x509Util = new ClientX509Util()) {
+                SSLContext sslContext = x509Util.getDefaultSSLContext();
+                SSLSocketFactory socketFactory = sslContext.getSocketFactory();
+                SSLSocket sslSock = (SSLSocket) socketFactory.createSocket();
+                sslSock.connect(hostaddress, timeout);
+                sslSock.startHandshake();
+                sock = sslSock;
+            }
         } else {
             sock = new Socket();
             sock.connect(hostaddress, timeout);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
new file mode 100644
index 0000000..8b49be9
--- /dev/null
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java
@@ -0,0 +1,253 @@
+/**
+ * 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 com.sun.nio.file.SensitivityWatchEventModifier;
+import org.apache.zookeeper.server.ZooKeeperThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.nio.file.ClosedWatchServiceException;
+import java.nio.file.FileSystem;
+import java.nio.file.Path;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.nio.file.WatchKey;
+import java.nio.file.WatchService;
+import java.util.function.Consumer;
+
+/**
+ * Instances of this class can be used to watch a directory for file changes. When a file is added to, deleted from,
+ * or is modified in the given directory, the callback provided by the user will be called from a background thread.
+ * Some things to keep in mind:
+ * <ul>
+ * <li>The callback should be thread-safe.</li>
+ * <li>Changes that happen around the time the thread is started may be missed.</li>
+ * <li>There is a delay between a file changing and the callback firing.</li>
+ * <li>The watch is not recursive - changes to subdirectories will not trigger a callback.</li>
+ * </ul>
+ */
+public final class FileChangeWatcher {
+    private static final Logger LOG = LoggerFactory.getLogger(FileChangeWatcher.class);
+
+    public enum State {
+        NEW,      // object created but start() not called yet
+        STARTING, // start() called but background thread has not entered main loop
+        RUNNING,  // background thread is running
+        STOPPING, // stop() called but background thread has not exited main loop
+        STOPPED   // stop() called and background thread has exited, or background thread crashed
+    }
+
+    private final WatcherThread watcherThread;
+    private State state; // protected by synchronized(this)
+
+    /**
+     * Creates a watcher that watches <code>dirPath</code> and invokes <code>callback</code> on changes.
+     *
+     * @param dirPath the directory to watch.
+     * @param callback the callback to invoke with events. <code>event.kind()</code> will return the type of event,
+     *                 and <code>event.context()</code> will return the filename relative to <code>dirPath</code>.
+     * @throws IOException if there is an error creating the WatchService.
+     */
+    public FileChangeWatcher(Path dirPath, Consumer<WatchEvent<?>> callback) throws IOException {
+        FileSystem fs = dirPath.getFileSystem();
+        WatchService watchService = fs.newWatchService();
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("Registering with watch service: " + dirPath);
+        }
+        dirPath.register(
+                watchService,
+                new WatchEvent.Kind<?>[]{
+                        StandardWatchEventKinds.ENTRY_CREATE,
+                        StandardWatchEventKinds.ENTRY_DELETE,
+                        StandardWatchEventKinds.ENTRY_MODIFY,
+                        StandardWatchEventKinds.OVERFLOW},
+                SensitivityWatchEventModifier.HIGH);
+        state = State.NEW;
+        this.watcherThread = new WatcherThread(watchService, callback);
+        this.watcherThread.setDaemon(true);
+    }
+
+    /**
+     * Returns the current {@link FileChangeWatcher.State}.
+     * @return the current state.
+     */
+    public synchronized State getState() {
+        return state;
+    }
+
+    /**
+     * Blocks until the current state becomes <code>desiredState</code>.
+     * Currently only used by tests, thus package-private.
+     * @param desiredState the desired state.
+     * @throws InterruptedException if the current thread gets interrupted.
+     */
+    synchronized void waitForState(State desiredState) throws InterruptedException {
+        while (this.state != desiredState) {
+            this.wait();
+        }
+    }
+
+    /**
+     * Sets the state to <code>newState</code>.
+     * @param newState the new state.
+     */
+    private synchronized void setState(State newState) {
+        state = newState;
+        this.notifyAll();
+    }
+
+    /**
+     * Atomically sets the state to <code>update</code> if and only if the
+     * state is currently <code>expected</code>.
+     * @param expected the expected state.
+     * @param update the new state.
+     * @return true if the update succeeds, or false if the current state
+     *         does not equal <code>expected</code>.
+     */
+    private synchronized boolean compareAndSetState(State expected, State update) {
+        if (state == expected) {
+            setState(update);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Atomically sets the state to <code>update</code> if and only if the
+     * state is currently one of <code>expectedStates</code>.
+     * @param expectedStates the expected states.
+     * @param update the new state.
+     * @return true if the update succeeds, or false if the current state
+     *         does not equal any of the <code>expectedStates</code>.
+     */
+    private synchronized boolean compareAndSetState(State[] expectedStates, State update) {
+        for (State expected : expectedStates) {
+            if (state == expected) {
+                setState(update);
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Tells the background thread to start. Does not wait for it to be running.
+     * Calling this method more than once has no effect.
+     */
+    public void start() {
+        if (!compareAndSetState(State.NEW, State.STARTING)) {
+            // If previous state was not NEW, start() has already been called.
+            return;
+        }
+        this.watcherThread.start();
+    }
+
+    /**
+     * Tells the background thread to stop. Does not wait for it to exit.
+     */
+    public void stop() {
+        if (compareAndSetState(
+                new State[]{State.RUNNING, State.STARTING},
+                State.STOPPING)) {
+            watcherThread.interrupt();
+        }
+    }
+
+    /**
+     * Inner class that implements the watcher thread logic.
+     */
+    private class WatcherThread extends ZooKeeperThread {
+        private static final String THREAD_NAME = "FileChangeWatcher";
+
+        final WatchService watchService;
+        final Consumer<WatchEvent<?>> callback;
+
+        WatcherThread(WatchService watchService, Consumer<WatchEvent<?>> callback) {
+            super(THREAD_NAME);
+            this.watchService = watchService;
+            this.callback = callback;
+        }
+
+        @Override
+        public void run() {
+            try {
+                LOG.info(getName() + " thread started");
+                if (!compareAndSetState(
+                        FileChangeWatcher.State.STARTING,
+                        FileChangeWatcher.State.RUNNING)) {
+                    // stop() called shortly after start(), before
+                    // this thread started running.
+                    FileChangeWatcher.State state = FileChangeWatcher.this.getState();
+                    if (state != FileChangeWatcher.State.STOPPING) {
+                        throw new IllegalStateException("Unexpected state: " + state);
+                    }
+                    return;
+                }
+                runLoop();
+            } catch (Exception e) {
+                LOG.warn("Error in runLoop()", e);
+                throw e;
+            } finally {
+                try {
+                    watchService.close();
+                } catch (IOException e) {
+                    LOG.warn("Error closing watch service", e);
+                }
+                LOG.info(getName() + " thread finished");
+                FileChangeWatcher.this.setState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+
+        private void runLoop() {
+            while (FileChangeWatcher.this.getState() == FileChangeWatcher.State.RUNNING) {
+                WatchKey key;
+                try {
+                    key = watchService.take();
+                } catch (InterruptedException|ClosedWatchServiceException e) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug(getName() + " was interrupted and is shutting down ...");
+                    }
+                    break;
+                }
+                for (WatchEvent<?> event : key.pollEvents()) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("Got file changed event: " + event.kind() + " with context: " + event.context());
+                    }
+                    try {
+                        callback.accept(event);
+                    } catch (Throwable e) {
+                        LOG.error("Error from callback", e);
+                    }
+                }
+                boolean isKeyValid = key.reset();
+                if (!isKeyValid) {
+                    // This is likely a problem, it means that file reloading is broken, probably because the
+                    // directory we are watching was deleted or otherwise became inaccessible (unmounted, permissions
+                    // changed, ???).
+                    // For now, we log an error and exit the watcher thread.
+                    LOG.error("Watch key no longer valid, maybe the directory is inaccessible?");
+                    break;
+                }
+            }
+        }
+    }
+}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
index e3625a5..4ea105b 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
@@ -19,8 +19,13 @@ package org.apache.zookeeper.common;
 
 
 import java.io.ByteArrayInputStream;
+import java.io.Closeable;
 import java.io.IOException;
 import java.net.Socket;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
 import java.security.GeneralSecurityException;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
@@ -58,7 +63,7 @@ import org.slf4j.LoggerFactory;
  *   Performance testing done by Facebook engineers shows that on Intel x86_64 machines, Java9 performs better with
  *   GCM and Java8 performs better with CBC, so these seem like reasonable defaults.
  */
-public abstract class X509Util {
+public abstract class X509Util implements Closeable, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
 
     static final String DEFAULT_PROTOCOL = "TLSv1.2";
@@ -93,6 +98,8 @@ public abstract class X509Util {
     private String[] cipherSuites;
 
     private AtomicReference<SSLContext> defaultSSLContext = new AtomicReference<>(null);
+    private FileChangeWatcher keyStoreFileWatcher;
+    private FileChangeWatcher trustStoreFileWatcher;
 
     public X509Util() {
         String cipherSuitesInput = System.getProperty(cipherSuitesProperty);
@@ -172,6 +179,11 @@ public abstract class X509Util {
         return result;
     }
 
+    private void resetDefaultSSLContext() throws X509Exception.SSLContextException {
+        SSLContext newContext = createSSLContext();
+        defaultSSLContext.set(newContext);
+    }
+
     private SSLContext createSSLContext() throws SSLContextException {
         /*
          * Since Configuration initializes the key store and trust store related
@@ -446,4 +458,105 @@ public abstract class X509Util {
         LOG.debug("Using Java8-optimized cipher suites for Java version {}", javaVersion);
         return DEFAULT_CIPHERS_JAVA8;
     }
+
+    private FileChangeWatcher newFileChangeWatcher(String fileLocation) throws IOException {
+        if (fileLocation == null || fileLocation.isEmpty()) {
+            return null;
+        }
+        final Path filePath = Paths.get(fileLocation).toAbsolutePath();
+        Path parentPath = filePath.getParent();
+        if (parentPath == null) {
+            throw new IOException(
+                    "Key/trust store path does not have a parent: " + filePath);
+        }
+        return new FileChangeWatcher(
+                parentPath,
+                watchEvent -> {
+                    handleWatchEvent(filePath, watchEvent);
+                });
+    }
+
+    /**
+     * Enables automatic reloading of the trust store and key store files when they change on disk.
+     *
+     * @throws IOException if creating the FileChangeWatcher objects fails.
+     */
+    public void enableCertFileReloading() throws IOException {
+        LOG.info("enabling cert file reloading");
+        ZKConfig config = new ZKConfig();
+        FileChangeWatcher newKeyStoreFileWatcher =
+                newFileChangeWatcher(config.getProperty(sslKeystoreLocationProperty));
+        if (newKeyStoreFileWatcher != null) {
+            // stop old watcher if there is one
+            if (keyStoreFileWatcher != null) {
+                keyStoreFileWatcher.stop();
+            }
+            keyStoreFileWatcher = newKeyStoreFileWatcher;
+            keyStoreFileWatcher.start();
+        }
+        FileChangeWatcher newTrustStoreFileWatcher =
+                newFileChangeWatcher(config.getProperty(sslTruststoreLocationProperty));
+        if (newTrustStoreFileWatcher != null) {
+            // stop old watcher if there is one
+            if (trustStoreFileWatcher != null) {
+                trustStoreFileWatcher.stop();
+            }
+            trustStoreFileWatcher = newTrustStoreFileWatcher;
+            trustStoreFileWatcher.start();
+        }
+    }
+
+    /**
+     * Disables automatic reloading of the trust store and key store files when they change on disk.
+     * Stops background threads and closes WatchService instances.
+     */
+    @Override
+    public void close() {
+        if (keyStoreFileWatcher != null) {
+            keyStoreFileWatcher.stop();
+            keyStoreFileWatcher = null;
+        }
+        if (trustStoreFileWatcher != null) {
+            trustStoreFileWatcher.stop();
+            trustStoreFileWatcher = null;
+        }
+    }
+
+    /**
+     * Handler for watch events that let us know a file we may care about has changed on disk.
+     *
+     * @param filePath the path to the file we are watching for changes.
+     * @param event    the WatchEvent.
+     */
+    private void handleWatchEvent(Path filePath, WatchEvent<?> event) {
+        boolean shouldResetContext = false;
+        Path dirPath = filePath.getParent();
+        if (event.kind().equals(StandardWatchEventKinds.OVERFLOW)) {
+            // If we get notified about possibly missed events, reload the key store / trust store just to be sure.
+            shouldResetContext = true;
+        } else if (event.kind().equals(StandardWatchEventKinds.ENTRY_MODIFY) ||
+                event.kind().equals(StandardWatchEventKinds.ENTRY_CREATE)) {
+            Path eventFilePath = dirPath.resolve((Path) event.context());
+            if (filePath.equals(eventFilePath)) {
+                shouldResetContext = true;
+            }
+        }
+        // Note: we don't care about delete events
+        if (shouldResetContext) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Attempting to reset default SSL context after receiving watch event: " +
+                        event.kind() + " with context: " + event.context());
+            }
+            try {
+                this.resetDefaultSSLContext();
+            } catch (SSLContextException e) {
+                throw new RuntimeException(e);
+            }
+        } else {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Ignoring watch event and keeping previous default SSL context. Event kind: " +
+                        event.kind() + " with context: " + event.context());
+            }
+        }
+    }
 }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
index effc0d5..086c07e 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java
@@ -103,12 +103,15 @@ public class ZKConfig {
         properties.put(KINIT_COMMAND, System.getProperty(KINIT_COMMAND));
         properties.put(JGSS_NATIVE, System.getProperty(JGSS_NATIVE));
 
-        ClientX509Util clientX509Util = new ClientX509Util();
-        putSSLProperties(clientX509Util);
-        properties.put(clientX509Util.getSslAuthProviderProperty(),
-                System.getProperty(clientX509Util.getSslAuthProviderProperty()));
+        try (ClientX509Util clientX509Util = new ClientX509Util()) {
+            putSSLProperties(clientX509Util);
+            properties.put(clientX509Util.getSslAuthProviderProperty(),
+                    System.getProperty(clientX509Util.getSslAuthProviderProperty()));
+        }
 
-        putSSLProperties(new QuorumX509Util());
+        try (X509Util x509Util = new QuorumX509Util()) {
+            putSSLProperties(x509Util);
+        }
     }
     
     private void putSSLProperties(X509Util x509Util) {
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
index 99de0e6..d96f56d 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
@@ -408,6 +408,8 @@ public class NettyServerCnxnFactory extends ServerCnxnFactory {
         }
         LOG.info("shutdown called {}", localAddress);
 
+        x509Util.close();
+
         if (login != null) {
             login.shutdown();
         }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
index d0ca079..22ad070 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
@@ -68,46 +68,46 @@ public class X509AuthenticationProvider implements AuthenticationProvider {
      */
     public X509AuthenticationProvider() throws X509Exception {
         ZKConfig config = new ZKConfig();
-        X509Util x509Util = new ClientX509Util();
-
-        String keyStoreLocation = config.getProperty(x509Util.getSslKeystoreLocationProperty(), "");
-        String keyStorePassword = config.getProperty(x509Util.getSslKeystorePasswdProperty(), "");
-        String keyStoreTypeProp = config.getProperty(x509Util.getSslKeystoreTypeProperty());
-
-        boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
-        boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
-        boolean hostnameVerificationEnabled = Boolean.parseBoolean(
-                config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
-
-        X509KeyManager km = null;
-        X509TrustManager tm = null;
-        if (keyStoreLocation.isEmpty()) {
-            LOG.warn("keystore not specified for client connection");
-        } else {
-            try {
-                km = X509Util.createKeyManager(keyStoreLocation, keyStorePassword, keyStoreTypeProp);
-            } catch (KeyManagerException e) {
-                LOG.error("Failed to create key manager", e);
+        try (X509Util x509Util = new ClientX509Util()) {
+            String keyStoreLocation = config.getProperty(x509Util.getSslKeystoreLocationProperty(), "");
+            String keyStorePassword = config.getProperty(x509Util.getSslKeystorePasswdProperty(), "");
+            String keyStoreTypeProp = config.getProperty(x509Util.getSslKeystoreTypeProperty());
+
+            boolean crlEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
+            boolean ocspEnabled = Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
+            boolean hostnameVerificationEnabled = Boolean.parseBoolean(
+                    config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
+
+            X509KeyManager km = null;
+            X509TrustManager tm = null;
+            if (keyStoreLocation.isEmpty()) {
+                LOG.warn("keystore not specified for client connection");
+            } else {
+                try {
+                    km = X509Util.createKeyManager(keyStoreLocation, keyStorePassword, keyStoreTypeProp);
+                } catch (KeyManagerException e) {
+                    LOG.error("Failed to create key manager", e);
+                }
             }
-        }
-        
-        String trustStoreLocation = config.getProperty(x509Util.getSslTruststoreLocationProperty(), "");
-        String trustStorePassword = config.getProperty(x509Util.getSslTruststorePasswdProperty(), "");
-        String trustStoreTypeProp = config.getProperty(x509Util.getSslTruststoreTypeProperty());
-
-        if (trustStoreLocation.isEmpty()) {
-            LOG.warn("Truststore not specified for client connection");
-        } else {
-            try {
-                tm = X509Util.createTrustManager(
-                        trustStoreLocation, trustStorePassword, trustStoreTypeProp, crlEnabled, ocspEnabled,
-                        hostnameVerificationEnabled, false);
-            } catch (TrustManagerException e) {
-                LOG.error("Failed to create trust manager", e);
+
+            String trustStoreLocation = config.getProperty(x509Util.getSslTruststoreLocationProperty(), "");
+            String trustStorePassword = config.getProperty(x509Util.getSslTruststorePasswdProperty(), "");
+            String trustStoreTypeProp = config.getProperty(x509Util.getSslTruststoreTypeProperty());
+
+            if (trustStoreLocation.isEmpty()) {
+                LOG.warn("Truststore not specified for client connection");
+            } else {
+                try {
+                    tm = X509Util.createTrustManager(
+                            trustStoreLocation, trustStorePassword, trustStoreTypeProp, crlEnabled, ocspEnabled,
+                            hostnameVerificationEnabled, false);
+                } catch (TrustManagerException e) {
+                    LOG.error("Failed to create trust manager", e);
+                }
             }
+            this.keyManager = km;
+            this.trustManager = tm;
         }
-        this.keyManager = km;
-        this.trustManager = tm;
     }
 
     /**
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
index 721a1b4..397ea6d 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
@@ -247,21 +247,16 @@ public class Leader implements LearnerMaster {
 
     private final ServerSocket ss;
 
-    Leader(QuorumPeer self,LeaderZooKeeperServer zk) throws IOException, X509Exception {
+    Leader(QuorumPeer self,LeaderZooKeeperServer zk) throws IOException {
         this.self = self;
         this.proposalStats = new BufferStats();
         try {
-            if (self.shouldUsePortUnification()) {
+            if (self.shouldUsePortUnification() || self.isSslQuorum()) {
+                boolean allowInsecureConnection = self.shouldUsePortUnification();
                 if (self.getQuorumListenOnAllIPs()) {
-                    ss = new UnifiedServerSocket(self.getX509Util(), true, self.getQuorumAddress().getPort());
+                    ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection, self.getQuorumAddress().getPort());
                 } else {
-                    ss = new UnifiedServerSocket(self.getX509Util(), true);
-                }
-            } else if (self.isSslQuorum()) {
-                if (self.getQuorumListenOnAllIPs()) {
-                    ss = self.getX509Util().createSSLServerSocket(self.getQuorumAddress().getPort());
-                } else {
-                    ss = self.getX509Util().createSSLServerSocket();
+                    ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection);
                 }
             } else {
                 if (self.getQuorumListenOnAllIPs()) {
@@ -274,9 +269,6 @@ public class Leader implements LearnerMaster {
             if (!self.getQuorumListenOnAllIPs()) {
                 ss.bind(self.getQuorumAddress());
             }
-        } catch (X509Exception e) {
-            LOG.error("Failed to setup ssl server socket", e);
-            throw e;
         } catch (BindException e) {
             if (self.getQuorumListenOnAllIPs()) {
                 LOG.error("Couldn't bind to port " + self.getQuorumAddress().getPort(), e);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
index 4175f3c..704580d 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
@@ -872,7 +872,7 @@ public class QuorumCnxManager {
                     if (self.shouldUsePortUnification()) {
                         ss = new UnifiedServerSocket(self.getX509Util(), true);
                     } else if (self.isSslQuorum()) {
-                        ss = self.getX509Util().createSSLServerSocket();
+                        ss = new UnifiedServerSocket(self.getX509Util(), false);
                     } else {
                         ss = new ServerSocket();
                     }
@@ -914,7 +914,7 @@ public class QuorumCnxManager {
                                      + "see ZOOKEEPER-2836");
                         }
                     }
-                } catch (IOException|X509Exception e) {
+                } catch (IOException e) {
                     if (shutdown) {
                         break;
                     }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
index 9d36fe2..a724903 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java
@@ -1324,6 +1324,7 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
     
     public void shutdown() {
         running = false;
+        x509Util.close();
         if (leader != null) {
             leader.shutdown("quorum Peer shutdown");
         }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
index 2c4dd0d..8bdeb33 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java
@@ -72,6 +72,7 @@ public class QuorumPeerConfig {
     protected boolean sslQuorum = false;
     protected boolean shouldUsePortUnification = false;
     protected int observerMasterPort;
+    protected boolean sslQuorumReloadCertFiles = false;
     protected File dataDir;
     protected File dataLogDir;
     protected String dynamicConfigFileStr = null;
@@ -321,6 +322,8 @@ public class QuorumPeerConfig {
                 sslQuorum = Boolean.parseBoolean(value);
             } else if (key.equals("portUnification")){
                 shouldUsePortUnification = Boolean.parseBoolean(value);
+            } else if (key.equals("sslQuorumReloadCertFiles")) {
+                sslQuorumReloadCertFiles = Boolean.parseBoolean(value);
             } else if ((key.startsWith("server.") || key.startsWith("group") || key.startsWith("weight")) && zkProp.containsKey("dynamicConfigFile")) {
                 throw new ConfigException("parameter: " + key + " must be in a separate dynamic config file");
             } else if (key.equals(QuorumAuth.QUORUM_SASL_AUTH_ENABLED)) {
@@ -462,15 +465,16 @@ public class QuorumPeerConfig {
      *             provider is not configured.
      */
     private void configureSSLAuth() throws ConfigException {
-        ClientX509Util clientX509Util = new ClientX509Util();
-        String sslAuthProp = "zookeeper.authProvider." + System.getProperty(clientX509Util.getSslAuthProviderProperty(), "x509");
-        if (System.getProperty(sslAuthProp) == null) {
-            if ("zookeeper.authProvider.x509".equals(sslAuthProp)) {
-                System.setProperty("zookeeper.authProvider.x509",
-                        "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
-            } else {
-                throw new ConfigException("No auth provider configured for the SSL authentication scheme '"
-                        + System.getProperty(clientX509Util.getSslAuthProviderProperty()) + "'.");
+        try (ClientX509Util clientX509Util = new ClientX509Util()) {
+            String sslAuthProp = "zookeeper.authProvider." + System.getProperty(clientX509Util.getSslAuthProviderProperty(), "x509");
+            if (System.getProperty(sslAuthProp) == null) {
+                if ("zookeeper.authProvider.x509".equals(sslAuthProp)) {
+                    System.setProperty("zookeeper.authProvider.x509",
+                            "org.apache.zookeeper.server.auth.X509AuthenticationProvider");
+                } else {
+                    throw new ConfigException("No auth provider configured for the SSL authentication scheme '"
+                            + System.getProperty(clientX509Util.getSslAuthProviderProperty()) + "'.");
+                }
             }
         }
     }
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
index a2e2bfc..24c8758 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java
@@ -203,6 +203,9 @@ public class QuorumPeerMain {
           quorumPeer.setLearnerType(config.getPeerType());
           quorumPeer.setSyncEnabled(config.getSyncEnabled());
           quorumPeer.setQuorumListenOnAllIPs(config.getQuorumListenOnAllIPs());
+          if (config.sslQuorumReloadCertFiles) {
+              quorumPeer.getX509Util().enableCertFileReloading();
+          }
 
           // sets quorum sasl authentication configurations
           quorumPeer.setQuorumSaslEnabled(config.quorumEnableSasl);
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
new file mode 100644
index 0000000..e4950f3
--- /dev/null
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
@@ -0,0 +1,263 @@
+/**
+ * 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.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.StandardWatchEventKinds;
+import java.nio.file.WatchEvent;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+public class FileChangeWatcherTest extends ZKTestCase {
+    private static File tempDir;
+    private static File tempFile;
+
+    private static final Logger LOG = LoggerFactory.getLogger(FileChangeWatcherTest.class);
+
+    @BeforeClass
+    public static void createTempFile() throws IOException {
+        tempDir = ClientBase.createEmptyTestDir();
+        tempFile = File.createTempFile("zk_test_", "", tempDir);
+        tempFile.deleteOnExit();
+    }
+
+    @AfterClass
+    public static void cleanupTempDir() {
+        try {
+            FileUtils.deleteDirectory(tempDir);
+        } catch (IOException e) {
+            // ignore
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileChanges() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            for (int i = 0; i < 3; i++) {
+                LOG.info("Modifying file, attempt " + (i + 1));
+                FileUtils.writeStringToFile(
+                        tempFile,
+                        "Hello world " + i + "\n",
+                        StandardCharsets.UTF_8,
+                        true);
+                synchronized (events) {
+                    if (events.size() < i + 1) {
+                        events.wait(3000L);
+                    }
+                    assertEquals("Wrong number of events", i + 1, events.size());
+                    WatchEvent<?> event = events.get(i);
+                    assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
+                    assertEquals(tempFile.getName(), event.context().toString());
+                }
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileTouched() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            LOG.info("Touching file");
+            FileUtils.touch(tempFile);
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
+                assertEquals(tempFile.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileAdded() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            File tempFile2 = File.createTempFile("zk_test_", "", tempDir);
+            tempFile2.deleteOnExit();
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_CREATE, event.kind());
+                assertEquals(tempFile2.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackWorksOnFileDeleted() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            tempFile.delete();
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_DELETE, event.kind());
+                assertEquals(tempFile.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+
+    @Test
+    public void testCallbackErrorDoesNotCrashWatcherThread() throws IOException, InterruptedException {
+        FileChangeWatcher watcher = null;
+        try {
+            final List<WatchEvent<?>> events = new ArrayList<>();
+            final AtomicInteger callCount = new AtomicInteger(0);
+            watcher = new FileChangeWatcher(
+                    tempDir.toPath(),
+                    event -> {
+                        LOG.info("Got an update: " + event.kind() + " " + event.context());
+                        synchronized (callCount) {
+                            if (callCount.getAndIncrement() == 0) {
+                                callCount.notifyAll();
+                                throw new RuntimeException("This error should not crash the watcher thread");
+                            }
+                        }
+                        synchronized (events) {
+                            events.add(event);
+                            events.notifyAll();
+                        }
+                    });
+            watcher.start();
+            watcher.waitForState(FileChangeWatcher.State.RUNNING);
+            Thread.sleep(1000L); // XXX hack
+            LOG.info("Modifying file");
+            FileUtils.writeStringToFile(tempFile, "Hello world\n", StandardCharsets.UTF_8, true);
+            synchronized (callCount) {
+                while (callCount.get() == 0) {
+                    callCount.wait(3000L);
+                }
+            }
+            LOG.info("Modifying file again");
+            FileUtils.writeStringToFile(tempFile, "Hello world again\n", StandardCharsets.UTF_8, true);
+            synchronized (events) {
+                if (events.isEmpty()) {
+                    events.wait(3000L);
+                }
+                assertEquals(2, callCount.get());
+                assertFalse(events.isEmpty());
+                WatchEvent<?> event = events.get(0);
+                assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
+                assertEquals(tempFile.getName(), event.context().toString());
+            }
+        } finally {
+            if (watcher != null) {
+                watcher.stop();
+                watcher.waitForState(FileChangeWatcher.State.STOPPED);
+            }
+        }
+    }
+}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
index 546cf55..1058010 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
@@ -70,7 +70,9 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
 
     @Before
     public void setUp() throws Exception {
-        x509TestContext.setSystemProperties(new ClientX509Util(), KeyStoreFileType.JKS, KeyStoreFileType.JKS);
+        try (X509Util x509util = new ClientX509Util()) {
+            x509TestContext.setSystemProperties(x509util, KeyStoreFileType.JKS, KeyStoreFileType.JKS);
+        }
         System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, "org.apache.zookeeper.server.NettyServerCnxnFactory");
         System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty");
         x509Util = new ClientX509Util();
@@ -83,12 +85,14 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
         System.clearProperty(x509Util.getSslCrlEnabledProperty());
         System.clearProperty(x509Util.getCipherSuitesProperty());
         System.clearProperty(x509Util.getSslProtocolProperty());
+        System.clearProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
         System.clearProperty("com.sun.net.ssl.checkRevocation");
         System.clearProperty("com.sun.security.enableCRLDP");
         Security.setProperty("ocsp.enable", Boolean.FALSE.toString());
         Security.setProperty("com.sun.security.enableCRLDP", Boolean.FALSE.toString());
         System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
         System.clearProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET);
+        x509Util.close();
     }
 
     @Test(timeout = 5000)
@@ -358,35 +362,37 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
 
     @Test
     public void testGetSslHandshakeDetectionTimeoutMillisProperty() {
-        X509Util x509Util = new ClientX509Util();
         Assert.assertEquals(
                 X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS,
                 x509Util.getSslHandshakeTimeoutMillis());
-        try {
-            String newPropertyString = Integer.toString(X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS + 1);
-            System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), newPropertyString);
-            // Note: need to create a new ClientX509Util to pick up modified property value
+        // Note: need to create a new ClientX509Util each time to pick up modified property value
+        String newPropertyString = Integer.toString(X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS + 1);
+        System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), newPropertyString);
+        try (X509Util tempX509Util = new ClientX509Util()) {
             Assert.assertEquals(
                     X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS + 1,
-                    new ClientX509Util().getSslHandshakeTimeoutMillis());
-            // 0 value not allowed, will return the default
-            System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "0");
+                    tempX509Util.getSslHandshakeTimeoutMillis());
+        }
+        // 0 value not allowed, will return the default
+        System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "0");
+        try (X509Util tempX509Util = new ClientX509Util()) {
             Assert.assertEquals(
                     X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS,
-                    new ClientX509Util().getSslHandshakeTimeoutMillis());
-            // Negative value not allowed, will return the default
-            System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "-1");
+                    tempX509Util.getSslHandshakeTimeoutMillis());
+        }
+        // Negative value not allowed, will return the default
+        System.setProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), "-1");
+        try (X509Util tempX509Util = new ClientX509Util()) {
             Assert.assertEquals(
                     X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS,
-                    new ClientX509Util().getSslHandshakeTimeoutMillis());
-        } finally {
-            System.clearProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
+                    tempX509Util.getSslHandshakeTimeoutMillis());
         }
     }
 
     // Warning: this will reset the x509Util
     private void setCustomCipherSuites() {
         System.setProperty(x509Util.getCipherSuitesProperty(), customCipherSuites[0] + "," + customCipherSuites[1]);
+        x509Util.close(); // remember to close old instance before replacing it
         x509Util = new ClientX509Util();
     }
 }
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
index f4a8b4b..f480f07 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java
@@ -91,17 +91,18 @@ public class QuorumPeerConfigTest {
      * https://issues.apache.org/jira/browse/ZOOKEEPER-2297
      */
     @Test
-    public void testCustomSSLAuth()
-            throws IOException{
-        System.setProperty(new ClientX509Util().getSslAuthProviderProperty(), "y509");
-        QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
-        try {
-            Properties zkProp = getDefaultZKProperties();
-            zkProp.setProperty("secureClientPort", "12345");
-            quorumPeerConfig.parseProperties(zkProp);
-            fail("ConfigException is expected");
-        } catch (ConfigException e) {
-            assertNotNull(e.getMessage());
+    public void testCustomSSLAuth() throws IOException {
+        try (ClientX509Util x509Util = new ClientX509Util()) {
+            System.setProperty(x509Util.getSslAuthProviderProperty(), "y509");
+            QuorumPeerConfig quorumPeerConfig = new QuorumPeerConfig();
+            try {
+                Properties zkProp = getDefaultZKProperties();
+                zkProp.setProperty("secureClientPort", "12345");
+                quorumPeerConfig.parseProperties(zkProp);
+                fail("ConfigException is expected");
+            } catch (ConfigException e) {
+                assertNotNull(e.getMessage());
+            }
         }
     }
 
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
index 67c15ad..e47b7ef 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
@@ -124,7 +124,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
     private static final char[] PASSWORD = "testpass".toCharArray();
     private static final String HOSTNAME = "localhost";
 
-    private QuorumX509Util quorumX509Util = new QuorumX509Util();
+    private QuorumX509Util quorumX509Util;
 
     private MainThread q1;
     private MainThread q2;
@@ -156,6 +156,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
 
     @Before
     public void setup() throws Exception {
+        quorumX509Util = new QuorumX509Util();
         ClientBase.setupTestEnv();
 
         tmpDir = createTmpDir().getAbsolutePath();
@@ -406,6 +407,7 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
         }
 
         Security.removeProvider("BC");
+        quorumX509Util.close();
     }
 
     private void clearSSLSystemProperties() {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java
index 61862a4..e9267b9 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketModeDetectionTest.java
@@ -171,6 +171,7 @@ public class UnifiedServerSocketModeDetectionTest extends ZKTestCase {
         forceClose(clientSocket);
         workerPool.shutdown();
         workerPool.awaitTermination(1000, TimeUnit.MILLISECONDS);
+        x509Util.close();
     }
 
     @Test
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
index 5e4e619..ddc05dc 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/UnifiedServerSocketTest.java
@@ -114,6 +114,7 @@ public class UnifiedServerSocketTest extends BaseX509ParameterizedTestCase {
     @After
     public void tearDown() throws Exception {
         x509TestContext.clearSystemProperties(x509Util);
+        x509Util.close();
     }
 
     private static void forceClose(Socket s) {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
index 08ffb4e..b486077 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientSSLTest.java
@@ -37,10 +37,11 @@ import org.junit.Test;
 
 public class ClientSSLTest extends QuorumPeerTestBase {
 
-    private ClientX509Util clientX509Util = new ClientX509Util();
+    private ClientX509Util clientX509Util;
 
     @Before
     public void setup() {
+        clientX509Util = new ClientX509Util();
         String testDataPath = System.getProperty("test.data.dir", "build/test/data");
         System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, "org.apache.zookeeper.server.NettyServerCnxnFactory");
         System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty");
@@ -60,6 +61,7 @@ public class ClientSSLTest extends QuorumPeerTestBase {
         System.clearProperty(clientX509Util.getSslKeystorePasswdProperty());
         System.clearProperty(clientX509Util.getSslTruststoreLocationProperty());
         System.clearProperty(clientX509Util.getSslTruststorePasswdProperty());
+        clientX509Util.close();
     }
 
     /**
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java
index 8fd35bc..6d30343 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SSLAuthTest.java
@@ -24,7 +24,6 @@ import org.apache.zookeeper.PortAssignment;
 import org.apache.zookeeper.TestableZooKeeper;
 import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.X509Util;
 import org.apache.zookeeper.server.ServerCnxnFactory;
 import org.junit.After;
 import org.junit.Assert;
@@ -33,10 +32,11 @@ import org.junit.Test;
 
 public class SSLAuthTest extends ClientBase {
     
-    private ClientX509Util clientX509Util = new ClientX509Util();
+    private ClientX509Util clientX509Util;
     
     @Before
     public void setUp() throws Exception {
+        clientX509Util = new ClientX509Util();
         String testDataPath = System.getProperty("test.data.dir", "build/test/data");
         System.setProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY, "org.apache.zookeeper.server.NettyServerCnxnFactory");
         System.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty");
@@ -71,6 +71,7 @@ public class SSLAuthTest extends ClientBase {
         System.clearProperty(clientX509Util.getSslTruststorePasswdProperty());
         System.clearProperty("javax.net.debug");
         System.clearProperty("zookeeper.authProvider.x509");
+        clientX509Util.close();
     }
 
     @Test


Mime
View raw message