ZOOKEEPER-4393 Problem to connect to zookeeper in FIPS mode (#2008)

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;
@@ -452,6 +453,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));

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();
             }

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() {

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

@@ -68,6 +68,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
@@ -145,36 +146,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;
     }
 
@@ -260,6 +255,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();
     }
@@ -295,7 +306,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());
     }
 
     /**
@@ -375,14 +386,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) {
@@ -487,16 +502,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
@@ -516,7 +532,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 = "";
         }
@@ -540,7 +557,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");
@@ -606,7 +633,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
@@ -633,6 +660,7 @@ public abstract class X509Util implements Closeable, AutoCloseable {
      */
     @Override
     public void close() {
+        defaultSSLContextAndOptions.set(null);
         if (keyStoreFileWatcher != null) {
             keyStoreFileWatcher.stop();
             keyStoreFileWatcher = null;

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()));
     }
 
     /**

zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java

@@ -578,7 +578,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(
@@ -590,7 +590,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) {

zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java

@@ -104,6 +104,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");
@@ -116,7 +117,8 @@ public class X509AuthenticationProvider implements AuthenticationProvider {
                         crlEnabled,
                         ocspEnabled,
                         hostnameVerificationEnabled,
-                        false);
+                        false,
+                        fipsMode);
                 } catch (TrustManagerException e) {
                     LOG.error("Failed to create trust manager", e);
                 }

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);
         });
     }
 

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<>();
@@ -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);

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++) {