From cc72c083c0b70409d78da11507ca5e80e726bb69 Mon Sep 17 00:00:00 2001 From: Ilya Maykov Date: Wed, 24 Oct 2018 18:54:06 -0700 Subject: [PATCH] ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store --- .../zookeeper/ClientCnxnSocketNetty.java | 9 +- .../zookeeper/client/FourLetterWordMain.java | 15 +- .../zookeeper/common/FileChangeWatcher.java | 253 +++++++++++++++++ .../org/apache/zookeeper/common/X509Util.java | 115 +++++++- .../org/apache/zookeeper/common/ZKConfig.java | 13 +- .../server/NettyServerCnxnFactory.java | 2 + .../auth/X509AuthenticationProvider.java | 74 ++--- .../zookeeper/server/quorum/Leader.java | 18 +- .../server/quorum/QuorumCnxManager.java | 4 +- .../zookeeper/server/quorum/QuorumPeer.java | 1 + .../server/quorum/QuorumPeerConfig.java | 22 +- .../server/quorum/QuorumPeerMain.java | 3 + .../common/FileChangeWatcherTest.java | 263 ++++++++++++++++++ .../apache/zookeeper/common/X509UtilTest.java | 36 ++- .../server/quorum/QuorumPeerConfigTest.java | 23 +- .../server/quorum/QuorumSSLTest.java | 4 +- .../UnifiedServerSocketModeDetectionTest.java | 1 + .../quorum/UnifiedServerSocketTest.java | 1 + .../apache/zookeeper/test/ClientSSLTest.java | 4 +- .../apache/zookeeper/test/SSLAuthTest.java | 5 +- 20 files changed, 760 insertions(+), 106 deletions(-) create mode 100644 zookeeper-server/src/main/java/org/apache/zookeeper/common/FileChangeWatcher.java create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java 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 74d1283ec20..c4a73016b59 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.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 @@ protected void initChannel(SocketChannel ch) throws Exception { // 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 41f5e9dfc11..a98953ffea6 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 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 static String send4LetterWord(String host, int port, String cmd, boolean 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 00000000000..8b49be9774e --- /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: + * + */ +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 dirPath and invokes callback on changes. + * + * @param dirPath the directory to watch. + * @param callback the callback to invoke with events. event.kind() will return the type of event, + * and event.context() will return the filename relative to dirPath. + * @throws IOException if there is an error creating the WatchService. + */ + public FileChangeWatcher(Path dirPath, Consumer> 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 desiredState. + * 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 newState. + * @param newState the new state. + */ + private synchronized void setState(State newState) { + state = newState; + this.notifyAll(); + } + + /** + * Atomically sets the state to update if and only if the + * state is currently expected. + * @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 expected. + */ + private synchronized boolean compareAndSetState(State expected, State update) { + if (state == expected) { + setState(update); + return true; + } else { + return false; + } + } + + /** + * Atomically sets the state to update if and only if the + * state is currently one of expectedStates. + * @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 expectedStates. + */ + 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> callback; + + WatcherThread(WatchService watchService, Consumer> 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 e3625a51c60..4ea105b3e13 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 @@ 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 @@ * 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 defaultSSLContext = new AtomicReference<>(null); + private FileChangeWatcher keyStoreFileWatcher; + private FileChangeWatcher trustStoreFileWatcher; public X509Util() { String cipherSuitesInput = System.getProperty(cipherSuitesProperty); @@ -172,6 +179,11 @@ public SSLContext getDefaultSSLContext() throws X509Exception.SSLContextExceptio 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 @@ private String[] getDefaultCipherSuites() { 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 effc0d52e52..086c07ee812 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 @@ protected void handleBackwardCompatibility() { 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 99de0e6eda8..d96f56de9f0 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 void shutdown() { } 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 d0ca079638b..22ad0709713 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 721a1b4f7d9..397ea6d4e86 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 boolean isQuorumSynced(QuorumVerifier qv) { 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 boolean isQuorumSynced(QuorumVerifier qv) { 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 4175f3cbed1..704580d9386 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 void run() { 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 void run() { + "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 9d36fe26f74..a7249032fcd 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 @@ private synchronized void updateServerState(){ 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 2c4dd0de7a1..8bdeb33208e 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 void parseProperties(Properties zkProp) 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 void parseProperties(Properties zkProp) * 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 a2e2bfc8576..24c87588baf 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 void runFromConfig(QuorumPeerConfig config) 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 00000000000..e4950f38c9e --- /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> 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> 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> 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> 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> 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 546cf553460..1058010febb 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 X509UtilTest( @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 void cleanUp() { 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 void testLoadJKSTrustStoreWithWrongPassword() throws Exception { @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 f4a8b4b5b84..f480f070da4 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 void testConfigureSSLAuthGetsConfiguredIfSecurePortConfigured() * 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 67c15ade2c3..e47b7ef5898 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 void cleanUp() throws Exception { } 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 61862a47a5b..e9267b9ef8a 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 void tearDown() throws Exception { 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 5e4e619da94..ddc05dc9c42 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 void setUp() throws Exception { @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 08ffb4eef58..b4860772d41 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 @@ 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 void teardown() throws Exception { 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 8fd35bc6185..6d303435439 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.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 @@ 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 void teardown() throws Exception { System.clearProperty(clientX509Util.getSslTruststorePasswdProperty()); System.clearProperty("javax.net.debug"); System.clearProperty("zookeeper.authProvider.x509"); + clientX509Util.close(); } @Test