ZOOKEEPER-4468: Backport BCFKS key/trust store format support to branch 3.5

Backporting ZOOKEEPER-3950 to branch-3.5.

This is a cherry-pick from #1482, also included checkstyle fix from #1516. This PR is basically the same as #1480 on the master branch, only the unit tests needed to be changed back from junit5 to junit4.

The BCFKS key store format is widely used in the industry, as it provides an open source alternative if someone has to use FIPS compliant key stores due to some regulatory constraints.

Currently in the ZooKeeper java client, only PEM, JKS and PEM12 is supported. I extend the list of supported key store formats with BCFKS.

I also tested this patch on a real FIPS compliant cluster, having the appropriate java security configs, security providers and also running a RedHat-based Linux distro (Centos 7.8) with FIPS mode enabled.

I tested both the client and the quorum SSL too. If someone wants to test this patch, and the keystore/truststore file names are not ending with ".bckfs", then (beside the usual SSL configs) make sure to also set the following parameters in the zoo.cfg:
```
ssl.keyStore.type=bcfks
ssl.trustStore.type=bcfks
ssl.quorum.keyStore.type=bcfks
ssl.quorum.trustStore.type=bcfks
```

and also provide the following parameters for the command line java client:
```
 -Dzookeeper.ssl.keyStore.type=bcfks -Dzookeeper.ssl.trustStore.type=bcfks
```

This patch doesn't contain any modification for the c-client (that can be handled with a separate Jira, but I don't plan to work on that part right now).

Author: Mate Szalay-Beko <[email protected]>

Reviewers: Norbert Kalmar <[email protected]>, Andor Molnar <[email protected]>

Closes #1815 from symat/ZOOKEEPER-4468

Author: symat

zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

@@ -985,9 +985,11 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
 * *ssl.keyStore.type* and *ssl.quorum.keyStore.type* :
     (Java system properties: **zookeeper.ssl.keyStore.type** and **zookeeper.ssl.quorum.keyStore.type**)
     **New in 3.5.5:**
-    Specifies the file format of client and quorum keystores. Values: JKS, PEM, PKCS12 or null (detect by filename).    
-    Default: null     
-    
+    Specifies the file format of client and quorum keystores. Values: JKS, PEM, PKCS12 or null (detect by filename).
+    Default: null.
+    **New in 3.5.10, 3.6.3, 3.7.0:**
+    The format BCFKS was added.
+
 * *ssl.trustStore.location* and *ssl.trustStore.password* and *ssl.quorum.trustStore.location* and *ssl.quorum.trustStore.password* :
     (Java system properties: **zookeeper.ssl.trustStore.location** and **zookeeper.ssl.trustStore.password** and **zookeeper.ssl.quorum.trustStore.location** and **zookeeper.ssl.quorum.trustStore.password**)
     **New in 3.5.5:**
@@ -998,8 +1000,10 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
 * *ssl.trustStore.type* and *ssl.quorum.trustStore.type* :
     (Java system properties: **zookeeper.ssl.trustStore.type** and **zookeeper.ssl.quorum.trustStore.type**)
     **New in 3.5.5:**
-    Specifies the file format of client and quorum trustStores. Values: JKS, PEM, PKCS12 or null (detect by filename).    
-    Default: null     
+    Specifies the file format of client and quorum trustStores. Values: JKS, PEM, PKCS12 or null (detect by filename).
+    Default: null.
+    **New in 3.5.10, 3.6.3, 3.7.0:**
+    The format BCFKS was added.
 
 * *ssl.protocol* and *ssl.quorum.protocol* :
     (Java system properties: **zookeeper.ssl.protocol** and **zookeeper.ssl.quorum.protocol**)

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

@@ -0,0 +1,39 @@
+/**
+ * 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;
+
+
+/**
+ * Implementation of {@link FileKeyStoreLoader} that loads from BCKFS files.
+ */
+class BCFKSFileLoader extends StandardTypeFileKeyStoreLoader {
+    private BCFKSFileLoader(String keyStorePath,
+                            String trustStorePath,
+                            String keyStorePassword,
+                            String trustStorePassword) {
+        super(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword, SupportedStandardKeyFormat.BCFKS);
+    }
+
+    static class Builder extends FileKeyStoreLoader.Builder<BCFKSFileLoader> {
+        @Override
+        BCFKSFileLoader build() {
+            return new BCFKSFileLoader(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword);
+        }
+    }
+}

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

@@ -38,6 +38,8 @@ public class FileKeyStoreLoaderBuilderProvider {
                 return new PEMFileLoader.Builder();
             case PKCS12:
                 return new PKCS12FileLoader.Builder();
+            case BCFKS:
+              return new BCFKSFileLoader.Builder();
             default:
                 throw new AssertionError(
                         "Unexpected StoreFileType: " + type.name());

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

@@ -18,8 +18,6 @@
 
 package org.apache.zookeeper.common;
 
-import java.security.KeyStore;
-import java.security.KeyStoreException;
 
 /**
  * Implementation of {@link FileKeyStoreLoader} that loads from JKS files.
@@ -29,12 +27,7 @@ private JKSFileLoader(String keyStorePath,
                           String trustStorePath,
                           String keyStorePassword,
                           String trustStorePassword) {
-        super(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword);
-    }
-
-    @Override
-    protected KeyStore keyStoreInstance() throws KeyStoreException {
-        return KeyStore.getInstance("JKS");
+        super(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword, SupportedStandardKeyFormat.JKS);
     }
 
     static class Builder extends FileKeyStoreLoader.Builder<JKSFileLoader> {

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

@@ -20,10 +20,13 @@
 
 /**
  * This enum represents the file type of a KeyStore or TrustStore.
- * Currently, JKS (Java keystore), PEM, and PKCS12 types are supported.
+ * Currently, JKS (Java keystore), PEM, PKCS12, and BCFKS types are supported.
  */
 public enum KeyStoreFileType {
-    JKS(".jks"), PEM(".pem"), PKCS12(".p12");
+    JKS(".jks"),
+    PEM(".pem"),
+    PKCS12(".p12"),
+    BCFKS(".bcfks");
 
     private final String defaultFileExtension;
 
@@ -53,7 +56,7 @@ public String getDefaultFileExtension() {
      * @return the KeyStoreFileType, or <code>null</code> if
      *         <code>propertyValue</code> is <code>null</code> or empty.
      * @throws IllegalArgumentException if <code>propertyValue</code> is not
-     *         one of "JKS", "PEM", "PKCS12", or empty/null.
+     *         one of "JKS", "PEM", "BCFKS", "PKCS12", or empty/null.
      */
     public static KeyStoreFileType fromPropertyValue(String propertyValue) {
         if (propertyValue == null || propertyValue.length() == 0) {
@@ -67,11 +70,12 @@ public static KeyStoreFileType fromPropertyValue(String propertyValue) {
      * If the file name ends with ".jks", returns <code>StoreFileType.JKS</code>.
      * If the file name ends with ".pem", returns <code>StoreFileType.PEM</code>.
      * If the file name ends with ".p12", returns <code>StoreFileType.PKCS12</code>.
+     * If the file name ends with ".bckfs", returns <code>StoreFileType.BCKFS</code>.
      * Otherwise, throws an IllegalArgumentException.
      * @param filename the filename of the key store or trust store file.
      * @return a KeyStoreFileType.
      * @throws IllegalArgumentException if the filename does not end with
-     *         ".jks", ".pem", or "p12".
+     *         ".jks", ".pem", "p12" or "bcfks".
      */
     public static KeyStoreFileType fromFilename(String filename) {
         int i = filename.lastIndexOf('.');
@@ -99,7 +103,7 @@ public static KeyStoreFileType fromFilename(String filename) {
      *                 <code>propertyValue</code> is null or empty.
      * @return a KeyStoreFileType.
      * @throws IllegalArgumentException if <code>propertyValue</code> is not
-     *         one of "JKS", "PEM", "PKCS12", or empty/null.
+     *         one of "JKS", "PEM", "PKCS12", "BCFKS", or empty/null.
      * @throws IllegalArgumentException if <code>propertyValue</code>is empty
      *         or null and the type could not be determined from the file name.
      */

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

@@ -18,8 +18,6 @@
 
 package org.apache.zookeeper.common;
 
-import java.security.KeyStore;
-import java.security.KeyStoreException;
 
 /**
  * Implementation of {@link FileKeyStoreLoader} that loads from PKCS12 files.
@@ -29,12 +27,7 @@ private PKCS12FileLoader(String keyStorePath,
                              String trustStorePath,
                              String keyStorePassword,
                              String trustStorePassword) {
-        super(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword);
-    }
-
-    @Override
-    protected KeyStore keyStoreInstance() throws KeyStoreException {
-        return KeyStore.getInstance("PKCS12");
+        super(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword, SupportedStandardKeyFormat.PKCS12);
     }
 
     static class Builder extends FileKeyStoreLoader.Builder<PKCS12FileLoader> {

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

@@ -34,11 +34,17 @@
 abstract class StandardTypeFileKeyStoreLoader extends FileKeyStoreLoader {
     private static final char[] EMPTY_CHAR_ARRAY = new char[0];
 
-    StandardTypeFileKeyStoreLoader(String keyStorePath,
-                                   String trustStorePath,
-                                   String keyStorePassword,
-                                   String trustStorePassword) {
+    protected final SupportedStandardKeyFormat format;
+
+    protected enum SupportedStandardKeyFormat {
+        JKS, PKCS12, BCFKS
+    }
+
+
+    StandardTypeFileKeyStoreLoader(String keyStorePath, String trustStorePath, String keyStorePassword,
+        String trustStorePassword, SupportedStandardKeyFormat format) {
         super(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword);
+        this.format = format;
     }
 
     @Override
@@ -59,7 +65,9 @@ public KeyStore loadTrustStore() throws IOException, GeneralSecurityException {
         }
     }
 
-    protected abstract KeyStore keyStoreInstance() throws KeyStoreException;
+    private KeyStore keyStoreInstance() throws KeyStoreException {
+        return KeyStore.getInstance(format.name());
+    }
 
     private static char[] passwordStringToCharArray(String password) {
         return password == null ? EMPTY_CHAR_ARRAY : password.toCharArray();

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

@@ -380,9 +380,9 @@ public SSLContextAndOptions createSSLContextAndOptions(ZKConfig config) throws S
      * @param keyStoreLocation the location of the key store file.
      * @param keyStorePassword optional password to decrypt the key store. If
      *                         empty, assumes the key store is not encrypted.
-     * @param keyStoreTypeProp must be JKS, PEM, or null. If null, attempts to
-     *                         autodetect the key store type from the file
-     *                         extension (.jks / .pem).
+     * @param keyStoreTypeProp must be JKS, PEM, PKCS12, BCFKS or null. If null,
+     *                         attempts to autodetect the key store type from
+     *                         the file extension (e.g. .jks / .pem).
      * @return the key manager.
      * @throws KeyManagerException if something goes wrong.
      */
@@ -425,9 +425,9 @@ public static X509KeyManager createKeyManager(
      * @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, or null. If null, attempts
-     *                           to autodetect the trust store type from the
-     *                           file extension (.jks / .pem).
+     * @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.

zookeeper-server/src/test/java/org/apache/zookeeper/common/BCFKSFileLoaderTest.java

@@ -0,0 +1,160 @@
+/**
+ * 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.IOException;
+import java.security.KeyStore;
+import java.util.Collection;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+
+@RunWith(Parameterized.class)
+public class BCFKSFileLoaderTest extends BaseX509ParameterizedTestCase {
+
+
+    @Parameterized.Parameters
+    public static Collection<Object[]> params() {
+        return BaseX509ParameterizedTestCase.defaultParams();
+    }
+
+    public BCFKSFileLoaderTest(
+      final X509KeyType caKeyType,
+      final X509KeyType certKeyType,
+      final String keyPassword,
+      final Integer paramIndex) {
+        super(paramIndex, () -> {
+            try {
+                return X509TestContext.newBuilder()
+                  .setTempDir(tempDir)
+                  .setKeyStorePassword(keyPassword)
+                  .setKeyStoreKeyType(certKeyType)
+                  .setTrustStorePassword(keyPassword)
+                  .setTrustStoreKeyType(caKeyType)
+                  .build();
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        });
+    }
+
+    @Test
+    public void testLoadKeyStore() throws Exception {
+        String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath();
+        KeyStore ks = new BCFKSFileLoader.Builder()
+          .setKeyStorePath(path)
+          .setKeyStorePassword(x509TestContext.getKeyStorePassword())
+          .build()
+          .loadKeyStore();
+        Assert.assertEquals(1, ks.size());
+    }
+
+    @Test(expected = Exception.class)
+    public void testLoadKeyStoreWithWrongPassword() throws Exception {
+        String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath();
+        new BCFKSFileLoader.Builder()
+          .setKeyStorePath(path)
+          .setKeyStorePassword("wrong password")
+          .build()
+          .loadKeyStore();
+    }
+
+    @Test(expected = IOException.class)
+    public void testLoadKeyStoreWithWrongFilePath() throws Exception {
+        String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath();
+        new BCFKSFileLoader.Builder()
+          .setKeyStorePath(path + ".does_not_exist")
+          .setKeyStorePassword(x509TestContext.getKeyStorePassword())
+          .build()
+          .loadKeyStore();
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testLoadKeyStoreWithNullFilePath() throws Exception {
+        new BCFKSFileLoader.Builder()
+          .setKeyStorePassword(x509TestContext.getKeyStorePassword())
+          .build()
+          .loadKeyStore();
+    }
+
+    @Test(expected = IOException.class)
+    public void testLoadKeyStoreWithWrongFileType() throws Exception {
+        // Trying to load a PEM file with BCFKS loader should fail
+        String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.PEM).getAbsolutePath();
+        new BCFKSFileLoader.Builder()
+          .setKeyStorePath(path)
+          .setKeyStorePassword(x509TestContext.getKeyStorePassword())
+          .build()
+          .loadKeyStore();
+    }
+
+    @Test
+    public void testLoadTrustStore() throws Exception {
+        String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath();
+        KeyStore ts = new BCFKSFileLoader.Builder()
+          .setTrustStorePath(path)
+          .setTrustStorePassword(x509TestContext.getTrustStorePassword())
+          .build()
+          .loadTrustStore();
+        Assert.assertEquals(1, ts.size());
+    }
+
+    @Test(expected = Exception.class)
+    public void testLoadTrustStoreWithWrongPassword() throws Exception {
+        String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath();
+        new BCFKSFileLoader.Builder()
+          .setTrustStorePath(path)
+          .setTrustStorePassword("wrong password")
+          .build()
+          .loadTrustStore();
+    }
+
+    @Test(expected = IOException.class)
+    public void testLoadTrustStoreWithWrongFilePath() throws Exception {
+        String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath();
+        new BCFKSFileLoader.Builder()
+          .setTrustStorePath(path + ".does_not_exist")
+          .setTrustStorePassword(x509TestContext.getTrustStorePassword())
+          .build()
+          .loadTrustStore();
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testLoadTrustStoreWithNullFilePath() throws Exception {
+        new BCFKSFileLoader.Builder()
+          .setTrustStorePassword(x509TestContext.getTrustStorePassword())
+          .build()
+          .loadTrustStore();
+    }
+
+    @Test(expected = IOException.class)
+    public void testLoadTrustStoreWithWrongFileType() throws Exception {
+        // Trying to load a PEM file with BCFKS loader should fail
+        String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.PEM).getAbsolutePath();
+        new BCFKSFileLoader.Builder()
+          .setTrustStorePath(path)
+          .setTrustStorePassword(x509TestContext.getTrustStorePassword())
+          .build()
+          .loadTrustStore();
+    }
+
+
+}