This is an automated email from the ASF dual-hosted git repository.
eolivelli pushed a commit to branch branch-3.8
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.8 by this push:
new aa3bc78a8 ZOOKEEPER-4393 Problem to connect to zookeeper in FIPS mode
(#2008)
aa3bc78a8 is described below
commit aa3bc78a88045df643e61390b7325fb1e6113df1
Author: Andor Molnár <[email protected]>
AuthorDate: Thu Jun 15 13:58:01 2023 +0200
ZOOKEEPER-4393 Problem to connect to zookeeper in FIPS mode (#2008)
(cherry picked from commit b3487c525542c44de1c65f2207cb5563afbf644d)
---
.../apache/zookeeper/ClientCnxnSocketNetty.java | 9 ++
.../zookeeper/client/FourLetterWordMain.java | 43 ++++----
.../zookeeper/common/SSLContextAndOptions.java | 31 ++++--
.../java/org/apache/zookeeper/common/X509Util.java | 112 +++++++++++++--------
.../java/org/apache/zookeeper/common/ZKConfig.java | 2 +
.../zookeeper/server/NettyServerCnxnFactory.java | 4 +-
.../server/auth/X509AuthenticationProvider.java | 4 +-
.../org/apache/zookeeper/common/X509UtilTest.java | 33 ++++--
.../zookeeper/server/quorum/QuorumSSLTest.java | 98 ++++++++++++------
.../org/apache/zookeeper/test/ClientSSLTest.java | 53 ++++++++--
10 files changed, 269 insertions(+), 120 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 df5397855..059627ea7 100755
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
@@ -50,6 +50,7 @@ import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLParameters;
import org.apache.zookeeper.ClientCnxn.EndOfStreamException;
import org.apache.zookeeper.ClientCnxn.Packet;
import org.apache.zookeeper.client.ZKClientConfig;
@@ -448,6 +449,14 @@ public class ClientCnxnSocketNetty extends
ClientCnxnSocket {
sslContext = x509Util.createSSLContext(clientConfig);
sslEngine = sslContext.createSSLEngine(host, port);
sslEngine.setUseClientMode(true);
+ if (x509Util.getFipsMode(clientConfig) &&
x509Util.isServerHostnameVerificationEnabled(clientConfig)) {
+ SSLParameters sslParameters =
sslEngine.getSSLParameters();
+
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
+ sslEngine.setSSLParameters(sslParameters);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Server hostname verification: enabled
HTTPS style endpoint identification algorithm");
+ }
+ }
}
}
pipeline.addLast("ssl", new SslHandler(sslEngine));
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 42a4e284e..de2f79dcf 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
@@ -92,27 +92,28 @@ public class FourLetterWordMain {
boolean secure,
int timeout) throws IOException, SSLContextException {
LOG.info("connecting to {} {}", host, port);
- Socket sock;
- InetSocketAddress hostaddress = host != null
- ? new InetSocketAddress(host, port)
- : new InetSocketAddress(InetAddress.getByName(null), port);
- if (secure) {
- LOG.info("using secure socket");
- 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);
- }
- sock.setSoTimeout(timeout);
+
+ Socket sock = null;
BufferedReader reader = null;
try {
+ InetSocketAddress hostaddress = host != null
+ ? new InetSocketAddress(host, port)
+ : new InetSocketAddress(InetAddress.getByName(null), port);
+ if (secure) {
+ LOG.info("using secure socket");
+ 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);
+ }
+ sock.setSoTimeout(timeout);
OutputStream outstream = sock.getOutputStream();
outstream.write(cmd.getBytes(UTF_8));
outstream.flush();
@@ -133,7 +134,9 @@ public class FourLetterWordMain {
} catch (SocketTimeoutException e) {
throw new IOException("Exception while executing four letter word:
" + cmd, e);
} finally {
- sock.close();
+ if (sock != null) {
+ sock.close();
+ }
if (reader != null) {
reader.close();
}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
index 01e97c3f5..4e61d7b9b 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
@@ -19,6 +19,7 @@
package org.apache.zookeeper.common;
import static java.util.Objects.requireNonNull;
+import io.netty.handler.ssl.DelegatingSslContext;
import io.netty.handler.ssl.IdentityCipherSuiteFilter;
import io.netty.handler.ssl.JdkSslContext;
import io.netty.handler.ssl.SslContext;
@@ -29,6 +30,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLSocket;
@@ -53,6 +55,8 @@ public class SSLContextAndOptions {
private final X509Util.ClientAuth clientAuth;
private final SSLContext sslContext;
private final int handshakeDetectionTimeoutMillis;
+ private final ZKConfig config;
+
/**
* Note: constructor is intentionally package-private, only the X509Util
class should be creating instances of this
@@ -70,6 +74,7 @@ public class SSLContextAndOptions {
this.cipherSuitesAsList =
Collections.unmodifiableList(Arrays.asList(ciphers));
this.clientAuth = getClientAuth(config);
this.handshakeDetectionTimeoutMillis =
getHandshakeDetectionTimeoutMillis(config);
+ this.config = config;
}
public SSLContext getSSLContext() {
@@ -101,18 +106,32 @@ public class SSLContextAndOptions {
return configureSSLServerSocket(sslServerSocket);
}
- public SslContext createNettyJdkSslContext(SSLContext sslContext, boolean
isClientSocket) {
- return new JdkSslContext(
+ public SslContext createNettyJdkSslContext(SSLContext sslContext) {
+ SslContext sslContext1 = new JdkSslContext(
sslContext,
- isClientSocket,
+ false,
cipherSuitesAsList,
IdentityCipherSuiteFilter.INSTANCE,
null,
- isClientSocket
- ? X509Util.ClientAuth.NONE.toNettyClientAuth()
- : clientAuth.toNettyClientAuth(),
+ clientAuth.toNettyClientAuth(),
enabledProtocols,
false);
+
+ if (x509Util.getFipsMode(config) &&
x509Util.isClientHostnameVerificationEnabled(config)) {
+ return new DelegatingSslContext(sslContext1) {
+ @Override
+ protected void initEngine(SSLEngine sslEngine) {
+ SSLParameters sslParameters = sslEngine.getSSLParameters();
+ sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
+ sslEngine.setSSLParameters(sslParameters);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Client hostname verification: enabled HTTPS
style endpoint identification algorithm");
+ }
+ }
+ };
+ } else {
+ return sslContext1;
+ }
}
public int getHandshakeDetectionTimeoutMillis() {
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 0cc3eb218..1b5cf6943 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
@@ -66,6 +66,7 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
private static final String REJECT_CLIENT_RENEGOTIATION_PROPERTY =
"jdk.tls.rejectClientInitiatedRenegotiation";
+ private static final String FIPS_MODE_PROPERTY = "zookeeper.fips-mode";
static {
// Client-initiated renegotiation in TLS is unsafe and
@@ -143,36 +144,30 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
}
}
- private String sslProtocolProperty = getConfigPrefix() + "protocol";
- private String sslEnabledProtocolsProperty = getConfigPrefix() +
"enabledProtocols";
- private String cipherSuitesProperty = getConfigPrefix() + "ciphersuites";
- private String sslKeystoreLocationProperty = getConfigPrefix() +
"keyStore.location";
- private String sslKeystorePasswdProperty = getConfigPrefix() +
"keyStore.password";
- private String sslKeystorePasswdPathProperty = getConfigPrefix() +
"keyStore.passwordPath";
- private String sslKeystoreTypeProperty = getConfigPrefix() +
"keyStore.type";
- private String sslTruststoreLocationProperty = getConfigPrefix() +
"trustStore.location";
- private String sslTruststorePasswdProperty = getConfigPrefix() +
"trustStore.password";
- private String sslTruststorePasswdPathProperty = getConfigPrefix() +
"trustStore.passwordPath";
- private String sslTruststoreTypeProperty = getConfigPrefix() +
"trustStore.type";
- private String sslContextSupplierClassProperty = getConfigPrefix() +
"context.supplier.class";
- private String sslHostnameVerificationEnabledProperty = getConfigPrefix()
+ "hostnameVerification";
- private String sslCrlEnabledProperty = getConfigPrefix() + "crl";
- private String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
- private String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
- private String sslHandshakeDetectionTimeoutMillisProperty =
getConfigPrefix() + "handshakeDetectionTimeoutMillis";
-
- private ZKConfig zkConfig;
- private AtomicReference<SSLContextAndOptions> defaultSSLContextAndOptions
= new AtomicReference<>(null);
+ private final String sslProtocolProperty = getConfigPrefix() + "protocol";
+ private final String sslEnabledProtocolsProperty = getConfigPrefix() +
"enabledProtocols";
+ private final String cipherSuitesProperty = getConfigPrefix() +
"ciphersuites";
+ private final String sslKeystoreLocationProperty = getConfigPrefix() +
"keyStore.location";
+ private final String sslKeystorePasswdProperty = getConfigPrefix() +
"keyStore.password";
+ private final String sslKeystorePasswdPathProperty = getConfigPrefix() +
"keyStore.passwordPath";
+ private final String sslKeystoreTypeProperty = getConfigPrefix() +
"keyStore.type";
+ private final String sslTruststoreLocationProperty = getConfigPrefix() +
"trustStore.location";
+ private final String sslTruststorePasswdProperty = getConfigPrefix() +
"trustStore.password";
+ private final String sslTruststorePasswdPathProperty = getConfigPrefix() +
"trustStore.passwordPath";
+ private final String sslTruststoreTypeProperty = getConfigPrefix() +
"trustStore.type";
+ private final String sslContextSupplierClassProperty = getConfigPrefix() +
"context.supplier.class";
+ private final String sslHostnameVerificationEnabledProperty =
getConfigPrefix() + "hostnameVerification";
+ private final String sslCrlEnabledProperty = getConfigPrefix() + "crl";
+ private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
+ private final String sslClientAuthProperty = getConfigPrefix() +
"clientAuth";
+ private final String sslHandshakeDetectionTimeoutMillisProperty =
getConfigPrefix() + "handshakeDetectionTimeoutMillis";
+
+ private final AtomicReference<SSLContextAndOptions>
defaultSSLContextAndOptions = new AtomicReference<>(null);
private FileChangeWatcher keyStoreFileWatcher;
private FileChangeWatcher trustStoreFileWatcher;
public X509Util() {
- this(null);
- }
-
- public X509Util(ZKConfig zkConfig) {
- this.zkConfig = zkConfig;
keyStoreFileWatcher = trustStoreFileWatcher = null;
}
@@ -258,6 +253,22 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
return sslHandshakeDetectionTimeoutMillisProperty;
}
+ public String getFipsModeProperty() {
+ return FIPS_MODE_PROPERTY;
+ }
+
+ public boolean getFipsMode(ZKConfig config) {
+ return config.getBoolean(FIPS_MODE_PROPERTY, true);
+ }
+
+ public boolean isServerHostnameVerificationEnabled(ZKConfig config) {
+ return
config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
+ }
+
+ public boolean isClientHostnameVerificationEnabled(ZKConfig config) {
+ return isServerHostnameVerificationEnabled(config) &&
shouldVerifyClientHostname();
+ }
+
public SSLContext getDefaultSSLContext() throws
X509Exception.SSLContextException {
return getDefaultSSLContextAndOptions().getSSLContext();
}
@@ -289,7 +300,7 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
* configuration from system property. Reading property from
* configuration will be same reading from system property
*/
- return createSSLContextAndOptions(zkConfig == null ? new ZKConfig() :
zkConfig);
+ return createSSLContextAndOptions(new ZKConfig());
}
/**
@@ -369,14 +380,18 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
boolean sslCrlEnabled = config.getBoolean(this.sslCrlEnabledProperty);
boolean sslOcspEnabled =
config.getBoolean(this.sslOcspEnabledProperty);
- boolean sslServerHostnameVerificationEnabled =
config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
- boolean sslClientHostnameVerificationEnabled =
sslServerHostnameVerificationEnabled && shouldVerifyClientHostname();
+ boolean sslServerHostnameVerificationEnabled =
isServerHostnameVerificationEnabled(config);
+ boolean sslClientHostnameVerificationEnabled =
isClientHostnameVerificationEnabled(config);
+ boolean fipsMode = getFipsMode(config);
if (trustStoreLocationProp.isEmpty()) {
LOG.warn("{} not specified", getSslTruststoreLocationProperty());
} else {
try {
- trustManagers = new
TrustManager[]{createTrustManager(trustStoreLocationProp,
trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled, sslOcspEnabled,
sslServerHostnameVerificationEnabled, sslClientHostnameVerificationEnabled)};
+ trustManagers = new TrustManager[]{
+ createTrustManager(trustStoreLocationProp,
trustStorePasswordProp, trustStoreTypeProp, sslCrlEnabled,
+ sslOcspEnabled, sslServerHostnameVerificationEnabled,
sslClientHostnameVerificationEnabled,
+ fipsMode)};
} catch (TrustManagerException trustManagerException) {
throw new SSLContextException("Failed to create TrustManager",
trustManagerException);
} catch (IllegalArgumentException e) {
@@ -481,16 +496,17 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
/**
* Creates a trust manager by loading the trust store from the given file
* of the given type, optionally decrypting it using the given password.
- * @param trustStoreLocation the location of the trust store file.
- * @param trustStorePassword optional password to decrypt the trust store
- * (only applies to JKS trust stores). If empty,
- * assumes the trust store is not encrypted.
- * @param trustStoreTypeProp must be JKS, PEM, PKCS12, BCFKS or null. If
- * null, attempts to autodetect the trust store
- * type from the file extension (e.g. .jks /
.pem).
- * @param crlEnabled enable CRL (certificate revocation list) checks.
- * @param ocspEnabled enable OCSP (online certificate status protocol)
- * checks.
+ *
+ * @param trustStoreLocation the location of the trust
store file.
+ * @param trustStorePassword optional password to decrypt
the trust store
+ * (only applies to JKS trust
stores). If empty,
+ * assumes the trust store is not
encrypted.
+ * @param trustStoreTypeProp must be JKS, PEM, PKCS12,
BCFKS or null. If
+ * null, attempts to autodetect
the trust store
+ * type from the file extension
(e.g. .jks / .pem).
+ * @param crlEnabled enable CRL (certificate
revocation list) checks.
+ * @param ocspEnabled enable OCSP (online
certificate status protocol)
+ * checks.
* @param serverHostnameVerificationEnabled if true, verify hostnames of
* remote servers that client
* sockets created by this
@@ -510,7 +526,8 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
boolean crlEnabled,
boolean ocspEnabled,
final boolean serverHostnameVerificationEnabled,
- final boolean clientHostnameVerificationEnabled) throws
TrustManagerException {
+ final boolean clientHostnameVerificationEnabled,
+ final boolean fipsMode) throws TrustManagerException {
if (trustStorePassword == null) {
trustStorePassword = "";
}
@@ -534,7 +551,17 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
for (final TrustManager tm : tmf.getTrustManagers()) {
if (tm instanceof X509ExtendedTrustManager) {
- return new ZKTrustManager((X509ExtendedTrustManager) tm,
serverHostnameVerificationEnabled, clientHostnameVerificationEnabled);
+ if (fipsMode) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("FIPS mode is ON: selecting standard
x509 trust manager {}", tm);
+ }
+ return (X509TrustManager) tm;
+ }
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("FIPS mode is OFF: creating ZKTrustManager");
+ }
+ return new ZKTrustManager((X509ExtendedTrustManager) tm,
serverHostnameVerificationEnabled,
+ clientHostnameVerificationEnabled);
}
}
throw new TrustManagerException("Couldn't find X509TrustManager");
@@ -600,7 +627,7 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
*/
public void enableCertFileReloading() throws IOException {
LOG.info("enabling cert file reloading");
- ZKConfig config = zkConfig == null ? new ZKConfig() : zkConfig;
+ ZKConfig config = new ZKConfig();
FileChangeWatcher newKeyStoreFileWatcher =
newFileChangeWatcher(config.getProperty(sslKeystoreLocationProperty));
if (newKeyStoreFileWatcher != null) {
// stop old watcher if there is one
@@ -627,6 +654,7 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
*/
@Override
public void close() {
+ defaultSSLContextAndOptions.set(null);
if (keyStoreFileWatcher != null) {
keyStoreFileWatcher.stop();
keyStoreFileWatcher = null;
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 1d1a6c9ea..0d98d35ba 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
@@ -82,6 +82,7 @@ public class ZKConfig {
public ZKConfig(File configFile) throws ConfigException {
this();
addConfiguration(configFile);
+ LOG.info("ZK Config {}", this.properties);
}
private void init() {
@@ -130,6 +131,7 @@ public class ZKConfig {
properties.put(x509Util.getSslOcspEnabledProperty(),
System.getProperty(x509Util.getSslOcspEnabledProperty()));
properties.put(x509Util.getSslClientAuthProperty(),
System.getProperty(x509Util.getSslClientAuthProperty()));
properties.put(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(),
System.getProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty()));
+ properties.put(x509Util.getFipsModeProperty(),
System.getProperty(x509Util.getFipsModeProperty()));
}
/**
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 040650e4a..de580d7ac 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
@@ -565,7 +565,7 @@ public class NettyServerCnxnFactory extends
ServerCnxnFactory {
SslContext nettySslContext;
if (authProviderProp == null) {
SSLContextAndOptions sslContextAndOptions =
x509Util.getDefaultSSLContextAndOptions();
- nettySslContext =
sslContextAndOptions.createNettyJdkSslContext(sslContextAndOptions.getSSLContext(),
false);
+ nettySslContext =
sslContextAndOptions.createNettyJdkSslContext(sslContextAndOptions.getSSLContext());
} else {
SSLContext sslContext =
SSLContext.getInstance(ClientX509Util.DEFAULT_PROTOCOL);
X509AuthenticationProvider authProvider =
(X509AuthenticationProvider) ProviderRegistry.getProvider(
@@ -577,7 +577,7 @@ public class NettyServerCnxnFactory extends
ServerCnxnFactory {
}
sslContext.init(new
X509KeyManager[]{authProvider.getKeyManager()}, new
X509TrustManager[]{authProvider.getTrustManager()}, null);
- nettySslContext =
x509Util.getDefaultSSLContextAndOptions().createNettyJdkSslContext(sslContext,
false);
+ nettySslContext =
x509Util.getDefaultSSLContextAndOptions().createNettyJdkSslContext(sslContext);
}
if (supportPlaintext) {
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 255e5cf8a..52eb7a7a9 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
@@ -98,6 +98,7 @@ public class X509AuthenticationProvider implements
AuthenticationProvider {
x509Util.getSslTruststorePasswdProperty(),
x509Util.getSslTruststorePasswdPathProperty());
String trustStoreTypeProp =
config.getProperty(x509Util.getSslTruststoreTypeProperty());
+ boolean fipsMode = x509Util.getFipsMode(config);
if (trustStoreLocation.isEmpty()) {
LOG.warn("Truststore not specified for client connection");
@@ -110,7 +111,8 @@ public class X509AuthenticationProvider implements
AuthenticationProvider {
crlEnabled,
ocspEnabled,
hostnameVerificationEnabled,
- false);
+ false,
+ fipsMode);
} catch (TrustManagerException e) {
LOG.error("Failed to create trust manager", e);
}
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 c6812ae04..ddffd426d 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
@@ -360,7 +360,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
false,
false,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -380,7 +381,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
false,
false,
true,
- true);
+ true,
+ false);
}
@@ -398,7 +400,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
false,
false,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -472,7 +475,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
true,
true,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -492,7 +496,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
false,
false,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -509,7 +514,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
true,
true,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -527,7 +533,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
true,
true,
true,
- true);
+ true,
+ false);
});
}
@@ -601,7 +608,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
true,
true,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -621,7 +629,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
false,
false,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -638,7 +647,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
true,
true,
true,
- true);
+ true,
+ false);
}
@ParameterizedTest
@@ -656,7 +666,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
true,
true,
true,
- true);
+ true,
+ false);
});
}
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 b5af84f1e..44d4a3af7 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
@@ -32,6 +32,8 @@ import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
import java.math.BigInteger;
import java.net.InetSocketAddress;
import java.net.URLDecoder;
@@ -116,11 +118,24 @@ import
org.bouncycastle.operator.jcajce.JcaDigestCalculatorProviderBuilder;
import org.bouncycastle.util.io.pem.PemWriter;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
public class QuorumSSLTest extends QuorumPeerTestBase {
+ @Retention(RetentionPolicy.RUNTIME)
+ @ParameterizedTest(name = "fipsEnabled = {0}")
+ @ValueSource(booleans = { false, true})
+ private @interface TestBothFipsModes {
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @ParameterizedTest(name = "fipsEnabled = {0}")
+ @ValueSource(booleans = { false })
+ private @interface TestNoFipsOnly {
+ }
+
private static final String SSL_QUORUM_ENABLED = "sslQuorum=true\n";
private static final String PORT_UNIFICATION_ENABLED =
"portUnification=true\n";
private static final String PORT_UNIFICATION_DISABLED =
"portUnification=false\n";
@@ -478,9 +493,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
System.clearProperty(quorumX509Util.getSslProtocolProperty());
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testQuorumSSL() throws Exception {
+ public void testQuorumSSL(boolean fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
q1 = new MainThread(1, clientPortQp1, quorumConfiguration,
SSL_QUORUM_ENABLED);
q2 = new MainThread(2, clientPortQp2, quorumConfiguration,
SSL_QUORUM_ENABLED);
@@ -499,9 +516,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
assertFalse(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3,
CONNECTION_TIMEOUT));
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testQuorumSSL_withPasswordFromFile() throws Exception {
+ public void testQuorumSSL_withPasswordFromFile(boolean fipsEnabled) throws
Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
final Path secretFile =
SecretUtilsTest.createSecretFile(String.valueOf(PASSWORD));
System.clearProperty(quorumX509Util.getSslKeystorePasswdProperty());
@@ -523,9 +542,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3,
CONNECTION_TIMEOUT));
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testQuorumSSLWithMultipleAddresses() throws Exception {
+ public void testQuorumSSLWithMultipleAddresses(boolean fipsEnabled) throws
Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
System.setProperty(QuorumPeer.CONFIG_KEY_MULTI_ADDRESS_ENABLED,
"true");
quorumConfiguration = generateMultiAddressQuorumConfiguration();
@@ -548,9 +569,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testRollingUpgrade() throws Exception {
+ public void testRollingUpgrade(boolean fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
// Form a quorum without ssl
q1 = new MainThread(1, clientPortQp1, quorumConfiguration);
q2 = new MainThread(2, clientPortQp2, quorumConfiguration);
@@ -596,9 +619,10 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
}
}
- @Test
+ @TestNoFipsOnly
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testHostnameVerificationWithInvalidHostname() throws Exception
{
+ public void testHostnameVerificationWithInvalidHostname(boolean
fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
String badhostnameKeystorePath = tmpDir + "/badhost.jks";
X509Certificate badHostCert = buildEndEntityCert(
defaultKeyPair,
@@ -613,9 +637,10 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
testHostnameVerification(badhostnameKeystorePath, false);
}
- @Test
+ @TestNoFipsOnly
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testHostnameVerificationWithInvalidIPAddress() throws
Exception {
+ public void testHostnameVerificationWithInvalidIPAddress(boolean
fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
String badhostnameKeystorePath = tmpDir + "/badhost.jks";
X509Certificate badHostCert = buildEndEntityCert(
defaultKeyPair,
@@ -630,9 +655,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
testHostnameVerification(badhostnameKeystorePath, false);
}
- @Test
+ @TestNoFipsOnly
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void
testHostnameVerificationWithInvalidIpAddressAndInvalidHostname() throws
Exception {
+ public void
testHostnameVerificationWithInvalidIpAddressAndInvalidHostname(boolean
fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
String badhostnameKeystorePath = tmpDir + "/badhost.jks";
X509Certificate badHostCert = buildEndEntityCert(
defaultKeyPair,
@@ -647,9 +674,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
testHostnameVerification(badhostnameKeystorePath, false);
}
- @Test
+ @TestNoFipsOnly
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testHostnameVerificationForInvalidMultiAddressServerConfig()
throws Exception {
+ public void
testHostnameVerificationForInvalidMultiAddressServerConfig(boolean fipsEnabled)
throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
System.setProperty(QuorumPeer.CONFIG_KEY_MULTI_ADDRESS_ENABLED,
"true");
quorumConfiguration = generateMultiAddressQuorumConfiguration();
@@ -667,9 +696,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
testHostnameVerification(badhostnameKeystorePath, false);
}
- @Test
+ @TestNoFipsOnly
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testHostnameVerificationWithInvalidIpAddressAndValidHostname()
throws Exception {
+ public void
testHostnameVerificationWithInvalidIpAddressAndValidHostname(boolean
fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
String badhostnameKeystorePath = tmpDir + "/badhost.jks";
X509Certificate badHostCert = buildEndEntityCert(
defaultKeyPair,
@@ -684,9 +715,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
testHostnameVerification(badhostnameKeystorePath, true);
}
- @Test
+ @TestNoFipsOnly
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testHostnameVerificationWithValidIpAddressAndInvalidHostname()
throws Exception {
+ public void
testHostnameVerificationWithValidIpAddressAndInvalidHostname(boolean
fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
String badhostnameKeystorePath = tmpDir + "/badhost.jks";
X509Certificate badHostCert = buildEndEntityCert(
defaultKeyPair,
@@ -751,9 +784,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3,
CONNECTION_TIMEOUT));
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testCertificateRevocationList() throws Exception {
+ public void testCertificateRevocationList(boolean fipsEnabled) throws
Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
q1 = new MainThread(1, clientPortQp1, quorumConfiguration,
SSL_QUORUM_ENABLED);
q2 = new MainThread(2, clientPortQp2, quorumConfiguration,
SSL_QUORUM_ENABLED);
@@ -817,9 +852,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
assertFalse(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3,
CONNECTION_TIMEOUT));
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testOCSP() throws Exception {
+ public void testOCSP(boolean fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
Integer ocspPort = PortAssignment.unique();
q1 = new MainThread(1, clientPortQp1, quorumConfiguration,
SSL_QUORUM_ENABLED);
@@ -891,9 +928,11 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
}
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testCipherSuites() throws Exception {
+ public void testCipherSuites(boolean fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
// Get default cipher suites from JDK
SSLServerSocketFactory ssf = (SSLServerSocketFactory)
SSLServerSocketFactory.getDefault();
List<String> defaultCiphers = new ArrayList<String>();
@@ -932,9 +971,10 @@ public class QuorumSSLTest extends QuorumPeerTestBase {
assertFalse(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3,
CONNECTION_TIMEOUT));
}
- @Test
+ @TestBothFipsModes
@Timeout(value = 5, unit = TimeUnit.MINUTES)
- public void testProtocolVersion() throws Exception {
+ public void testProtocolVersion(boolean fipsEnabled) throws Exception {
+ System.setProperty(quorumX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
System.setProperty(quorumX509Util.getSslProtocolProperty(), "TLSv1.2");
q1 = new MainThread(1, clientPortQp1, quorumConfiguration,
SSL_QUORUM_ENABLED);
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 43878e0b0..392c636de 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
@@ -22,11 +22,13 @@
package org.apache.zookeeper.test;
+import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
+import java.net.InetAddress;
import java.nio.file.Path;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.PortAssignment;
@@ -42,6 +44,9 @@ import org.apache.zookeeper.server.quorum.QuorumPeerTestBase;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
public class ClientSSLTest extends QuorumPeerTestBase {
@@ -73,6 +78,8 @@ public class ClientSSLTest extends QuorumPeerTestBase {
System.clearProperty(clientX509Util.getSslTruststoreLocationProperty());
System.clearProperty(clientX509Util.getSslTruststorePasswdProperty());
System.clearProperty(clientX509Util.getSslTruststorePasswdPathProperty());
+ System.clearProperty(clientX509Util.getFipsModeProperty());
+
System.clearProperty(clientX509Util.getSslHostnameVerificationEnabledProperty());
clientX509Util.close();
}
@@ -106,12 +113,36 @@ public class ClientSSLTest extends QuorumPeerTestBase {
* 2. setting jvm flags for serverCnxn, keystore, truststore.
* Finally, a zookeeper client should be able to connect to the secure
port and
* communicate with server via secure connection.
- * <p>
+ * <p/>
* Note that in this test a ZK server has two ports -- clientPort and
secureClientPort.
+ * <p/>
+ * This test covers the positive scenarios for hostname verification.
*/
- @Test
- public void testClientServerSSL() throws Exception {
- testClientServerSSL(true);
+ @ParameterizedTest(name = "fipsEnabled={0}, hostnameVerification={1}")
+ @CsvSource({"true,true", "true,false", "false,true", "false,false"})
+ public void testClientServerSSL_positive(String fipsEnabled, String
hostnameVerification) throws Exception {
+ // Arrange
+ System.setProperty(clientX509Util.getFipsModeProperty(), fipsEnabled);
+
System.setProperty(clientX509Util.getSslHostnameVerificationEnabledProperty(),
hostnameVerification);
+
+ // Act & Assert
+ testClientServerSSL(hostnameVerification.equals("true") ? "localhost"
: InetAddress.getLocalHost().getHostName(),
+ true, CONNECTION_TIMEOUT);
+ }
+
+ /**
+ * This test covers the negative scenarios for hostname verification.
+ */
+ @ParameterizedTest(name = "fipsEnabled={0}")
+ @ValueSource(booleans = { true, false })
+ public void testClientServerSSL_negative(boolean fipsEnabled) {
+ // Arrange
+ System.setProperty(clientX509Util.getFipsModeProperty(),
Boolean.toString(fipsEnabled));
+
System.setProperty(clientX509Util.getSslHostnameVerificationEnabledProperty(),
"true");
+
+ // Act & Assert
+ assertThrows(AssertionError.class, () ->
+ testClientServerSSL(InetAddress.getLocalHost().getHostName(),
true, 5000));
}
@Test
@@ -128,6 +159,10 @@ public class ClientSSLTest extends QuorumPeerTestBase {
}
public void testClientServerSSL(boolean useSecurePort) throws Exception {
+ testClientServerSSL("localhost", useSecurePort, CONNECTION_TIMEOUT);
+ }
+
+ public void testClientServerSSL(String hostname, boolean useSecurePort,
long connectTimeout) throws Exception {
final int SERVER_COUNT = 3;
final int[] clientPorts = new int[SERVER_COUNT];
final Integer[] secureClientPorts = new Integer[SERVER_COUNT];
@@ -159,11 +194,11 @@ public class ClientSSLTest extends QuorumPeerTestBase {
assertTrue(ClientBase.waitForServerUp("127.0.0.1:" +
clientPorts[i], TIMEOUT),
"waiting for server " + i + " being up");
final int port = useSecurePort ? secureClientPorts[i] :
clientPorts[i];
- ZooKeeper zk = ClientBase.createZKClient("127.0.0.1:" + port,
TIMEOUT);
- // Do a simple operation to make sure the connection is fine.
- zk.create("/test", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
CreateMode.PERSISTENT);
- zk.delete("/test", -1);
- zk.close();
+ try (ZooKeeper zk = ClientBase.createZKClient(hostname + ":" +
port, TIMEOUT, connectTimeout)) {
+ // Do a simple operation to make sure the connection is fine.
+ zk.create("/test", "".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
CreateMode.PERSISTENT);
+ zk.delete("/test", -1);
+ }
}
for (int i = 0; i < mt.length; i++) {
