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 <[email protected]>
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 <[email protected]>
Reviewers: [email protected]
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