From commits-return-8540-archive-asf-public=cust-asf.ponee.io@zookeeper.apache.org Tue Oct 6 09:38:10 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mxout1-he-de.apache.org (mxout1-he-de.apache.org [95.216.194.37]) by mx-eu-01.ponee.io (Postfix) with ESMTPS id 7F742180636 for ; Tue, 6 Oct 2020 11:38:10 +0200 (CEST) Received: from mail.apache.org (mailroute1-lw-us.apache.org [207.244.88.153]) by mxout1-he-de.apache.org (ASF Mail Server at mxout1-he-de.apache.org) with SMTP id 786D2649D8 for ; Tue, 6 Oct 2020 09:38:09 +0000 (UTC) Received: (qmail 34369 invoked by uid 500); 6 Oct 2020 09:38:07 -0000 Mailing-List: contact commits-help@zookeeper.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zookeeper.apache.org Delivered-To: mailing list commits@zookeeper.apache.org Received: (qmail 34144 invoked by uid 99); 6 Oct 2020 09:38:06 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 06 Oct 2020 09:38:06 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id CAA408290B; Tue, 6 Oct 2020 09:30:27 +0000 (UTC) Date: Tue, 06 Oct 2020 09:30:27 +0000 To: "commits@zookeeper.apache.org" Subject: [zookeeper] branch master updated: ZOOKEEPER-3950: Add support for BCFKS key/trust store format MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <160197662764.4582.15285508285545283810@gitbox.apache.org> From: symat@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: zookeeper X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 663081d249626d20d6a2cfa75396b2a3d4454085 X-Git-Newrev: ec1503bb00945471baa248f392eed51064bb48ab X-Git-Rev: ec1503bb00945471baa248f392eed51064bb48ab X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. symat pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/zookeeper.git The following commit(s) were added to refs/heads/master by this push: new ec1503b ZOOKEEPER-3950: Add support for BCFKS key/trust store format ec1503b is described below commit ec1503bb00945471baa248f392eed51064bb48ab Author: Mate Szalay-Beko AuthorDate: Tue Oct 6 09:29:46 2020 +0000 ZOOKEEPER-3950: Add support for BCFKS key/trust store format 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 Reviewers: Enrico Olivelli , Norbert Kalmar , Andor Molnar Closes #1480 from symat/zookeeper-3950 --- .../src/main/resources/markdown/zookeeperAdmin.md | 8 +- .../{JKSFileLoader.java => BCFKSFileLoader.java} | 34 ++-- .../common/FileKeyStoreLoaderBuilderProvider.java | 2 + .../org/apache/zookeeper/common/JKSFileLoader.java | 9 +- .../apache/zookeeper/common/KeyStoreFileType.java | 12 +- .../apache/zookeeper/common/PKCS12FileLoader.java | 9 +- .../common/StandardTypeFileKeyStoreLoader.java | 19 ++- .../java/org/apache/zookeeper/common/X509Util.java | 12 +- .../zookeeper/common/BCFKSFileLoaderTest.java | 188 +++++++++++++++++++++ .../zookeeper/common/KeyStoreFileTypeTest.java | 6 + .../apache/zookeeper/common/X509TestContext.java | 51 +++++- .../apache/zookeeper/common/X509TestHelpers.java | 45 ++++- 12 files changed, 328 insertions(+), 67 deletions(-) diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md index 76305a5..afb6bc4 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md @@ -1532,7 +1532,9 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp (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 + Default: null. + **New in 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**) @@ -1545,7 +1547,9 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp (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 + Default: null. + **New in 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**) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/BCFKSFileLoader.java similarity index 60% copy from zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java copy to zookeeper-server/src/main/java/org/apache/zookeeper/common/BCFKSFileLoader.java index 8be7c37..d59f40a 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/BCFKSFileLoader.java @@ -1,4 +1,4 @@ -/* +/** * 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 @@ -18,34 +18,22 @@ package org.apache.zookeeper.common; -import java.security.KeyStore; -import java.security.KeyStoreException; /** - * Implementation of {@link FileKeyStoreLoader} that loads from JKS files. + * Implementation of {@link FileKeyStoreLoader} that loads from BCKFS files. */ -class JKSFileLoader extends StandardTypeFileKeyStoreLoader { - - 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"); +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 { - + static class Builder extends FileKeyStoreLoader.Builder { @Override - JKSFileLoader build() { - return new JKSFileLoader(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword); + BCFKSFileLoader build() { + return new BCFKSFileLoader(keyStorePath, trustStorePath, keyStorePassword, trustStorePassword); } - } - } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoaderBuilderProvider.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoaderBuilderProvider.java index 169fb9e..f4798af 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/FileKeyStoreLoaderBuilderProvider.java +++ b/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()); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java index 8be7c37..d8709d7 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/JKSFileLoader.java +++ b/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. @@ -31,12 +29,7 @@ class JKSFileLoader extends StandardTypeFileKeyStoreLoader { 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 { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/KeyStoreFileType.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/KeyStoreFileType.java index ecc1e5f..69283f3 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/KeyStoreFileType.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/KeyStoreFileType.java @@ -20,12 +20,13 @@ package org.apache.zookeeper.common; /** * 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"); + PKCS12(".p12"), + BCFKS(".bcfks"); private final String defaultFileExtension; @@ -55,7 +56,7 @@ public enum KeyStoreFileType { * @return the KeyStoreFileType, or null if * propertyValue is null or empty. * @throws IllegalArgumentException if propertyValue 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) { @@ -69,11 +70,12 @@ public enum KeyStoreFileType { * If the file name ends with ".jks", returns StoreFileType.JKS. * If the file name ends with ".pem", returns StoreFileType.PEM. * If the file name ends with ".p12", returns StoreFileType.PKCS12. + * If the file name ends with ".bckfs", returns StoreFileType.BCKFS. * 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('.'); @@ -100,7 +102,7 @@ public enum KeyStoreFileType { * propertyValue is null or empty. * @return a KeyStoreFileType. * @throws IllegalArgumentException if propertyValue is not - * one of "JKS", "PEM", "PKCS12", or empty/null. + * one of "JKS", "PEM", "PKCS12", "BCFKS", or empty/null. * @throws IllegalArgumentException if propertyValueis empty * or null and the type could not be determined from the file name. */ diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/PKCS12FileLoader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/PKCS12FileLoader.java index 35e89a1..8903f28 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/PKCS12FileLoader.java +++ b/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. @@ -31,12 +29,7 @@ class PKCS12FileLoader extends StandardTypeFileKeyStoreLoader { 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 { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/StandardTypeFileKeyStoreLoader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/StandardTypeFileKeyStoreLoader.java index b32ee07..8f068f7 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/StandardTypeFileKeyStoreLoader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/StandardTypeFileKeyStoreLoader.java @@ -35,12 +35,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 @@ -61,7 +66,9 @@ abstract class StandardTypeFileKeyStoreLoader extends FileKeyStoreLoader { } } - 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(); 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 24c7f35..52cb5fe 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 @@ -419,9 +419,9 @@ public abstract class X509Util implements Closeable, AutoCloseable { * @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. */ @@ -455,9 +455,9 @@ public abstract class X509Util implements Closeable, AutoCloseable { * @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. diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/BCFKSFileLoaderTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/BCFKSFileLoaderTest.java new file mode 100644 index 0000000..1850b65 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/BCFKSFileLoaderTest.java @@ -0,0 +1,188 @@ +/** + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.IOException; +import java.security.KeyStore; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + + +public class BCFKSFileLoaderTest extends BaseX509ParameterizedTestCase { + + + @ParameterizedTest + @MethodSource("data") + public void testLoadKeyStore( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath(); + KeyStore ks = new BCFKSFileLoader.Builder() + .setKeyStorePath(path) + .setKeyStorePassword(x509TestContext.getKeyStorePassword()) + .build() + .loadKeyStore(); + assertEquals(1, ks.size()); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadKeyStoreWithWrongPassword( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(Exception.class, () -> { + String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath(); + new BCFKSFileLoader.Builder() + .setKeyStorePath(path) + .setKeyStorePassword("wrong password") + .build() + .loadKeyStore(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadKeyStoreWithWrongFilePath( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(IOException.class, () -> { + String path = x509TestContext.getKeyStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath(); + new BCFKSFileLoader.Builder() + .setKeyStorePath(path + ".does_not_exist") + .setKeyStorePassword(x509TestContext.getKeyStorePassword()) + .build() + .loadKeyStore(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadKeyStoreWithNullFilePath( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(NullPointerException.class, () -> { + new BCFKSFileLoader.Builder() + .setKeyStorePassword(x509TestContext.getKeyStorePassword()) + .build() + .loadKeyStore(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadKeyStoreWithWrongFileType( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(IOException.class, () -> { + // 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(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadTrustStore( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath(); + KeyStore ts = new BCFKSFileLoader.Builder() + .setTrustStorePath(path) + .setTrustStorePassword(x509TestContext.getTrustStorePassword()) + .build() + .loadTrustStore(); + assertEquals(1, ts.size()); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadTrustStoreWithWrongPassword( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(Exception.class, () -> { + String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath(); + new BCFKSFileLoader.Builder() + .setTrustStorePath(path) + .setTrustStorePassword("wrong password") + .build() + .loadTrustStore(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadTrustStoreWithWrongFilePath( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(IOException.class, () -> { + String path = x509TestContext.getTrustStoreFile(KeyStoreFileType.BCFKS).getAbsolutePath(); + new BCFKSFileLoader.Builder() + .setTrustStorePath(path + ".does_not_exist") + .setTrustStorePassword(x509TestContext.getTrustStorePassword()) + .build() + .loadTrustStore(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadTrustStoreWithNullFilePath( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(NullPointerException.class, () -> { + new BCFKSFileLoader.Builder() + .setTrustStorePassword(x509TestContext.getTrustStorePassword()) + .build() + .loadTrustStore(); + }); + } + + @ParameterizedTest + @MethodSource("data") + public void testLoadTrustStoreWithWrongFileType( + X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex) + throws Exception { + init(caKeyType, certKeyType, keyPassword, paramIndex); + assertThrows(IOException.class, () -> { + // 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(); + }); + } + +} diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/KeyStoreFileTypeTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/KeyStoreFileTypeTest.java index 9f5dd7a..3f7faaa 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/KeyStoreFileTypeTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/KeyStoreFileTypeTest.java @@ -31,6 +31,7 @@ public class KeyStoreFileTypeTest extends ZKTestCase { assertEquals("PEM", KeyStoreFileType.PEM.getPropertyValue()); assertEquals("JKS", KeyStoreFileType.JKS.getPropertyValue()); assertEquals("PKCS12", KeyStoreFileType.PKCS12.getPropertyValue()); + assertEquals("BCFKS", KeyStoreFileType.BCFKS.getPropertyValue()); } @Test @@ -38,6 +39,7 @@ public class KeyStoreFileTypeTest extends ZKTestCase { assertEquals(KeyStoreFileType.PEM, KeyStoreFileType.fromPropertyValue("PEM")); assertEquals(KeyStoreFileType.JKS, KeyStoreFileType.fromPropertyValue("JKS")); assertEquals(KeyStoreFileType.PKCS12, KeyStoreFileType.fromPropertyValue("PKCS12")); + assertEquals(KeyStoreFileType.BCFKS, KeyStoreFileType.fromPropertyValue("BCFKS")); assertNull(KeyStoreFileType.fromPropertyValue("")); assertNull(KeyStoreFileType.fromPropertyValue(null)); } @@ -47,6 +49,7 @@ public class KeyStoreFileTypeTest extends ZKTestCase { assertEquals(KeyStoreFileType.PEM, KeyStoreFileType.fromPropertyValue("pem")); assertEquals(KeyStoreFileType.JKS, KeyStoreFileType.fromPropertyValue("jks")); assertEquals(KeyStoreFileType.PKCS12, KeyStoreFileType.fromPropertyValue("pkcs12")); + assertEquals(KeyStoreFileType.BCFKS, KeyStoreFileType.fromPropertyValue("bcfks")); assertNull(KeyStoreFileType.fromPropertyValue("")); assertNull(KeyStoreFileType.fromPropertyValue(null)); } @@ -66,6 +69,8 @@ public class KeyStoreFileTypeTest extends ZKTestCase { assertEquals(KeyStoreFileType.PEM, KeyStoreFileType.fromFilename("/path/to/key/dir/mykey.pem")); assertEquals(KeyStoreFileType.PKCS12, KeyStoreFileType.fromFilename("mykey.p12")); assertEquals(KeyStoreFileType.PKCS12, KeyStoreFileType.fromFilename("/path/to/key/dir/mykey.p12")); + assertEquals(KeyStoreFileType.BCFKS, KeyStoreFileType.fromFilename("mykey.bcfks")); + assertEquals(KeyStoreFileType.BCFKS, KeyStoreFileType.fromFilename("/path/to/key/dir/mykey.bcfks")); } @Test @@ -81,6 +86,7 @@ public class KeyStoreFileTypeTest extends ZKTestCase { assertEquals(KeyStoreFileType.JKS, KeyStoreFileType.fromPropertyValueOrFileName("JKS", "prod.key")); assertEquals(KeyStoreFileType.PEM, KeyStoreFileType.fromPropertyValueOrFileName("PEM", "prod.key")); assertEquals(KeyStoreFileType.PKCS12, KeyStoreFileType.fromPropertyValueOrFileName("PKCS12", "prod.key")); + assertEquals(KeyStoreFileType.BCFKS, KeyStoreFileType.fromPropertyValueOrFileName("BCFKS", "prod.key")); // Falls back to filename detection if no property value assertEquals(KeyStoreFileType.JKS, KeyStoreFileType.fromPropertyValueOrFileName("", "prod.jks")); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java index 71fde1e..3238026 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java @@ -53,6 +53,7 @@ public class X509TestContext { private File trustStoreJksFile; private File trustStorePemFile; private File trustStorePkcs12File; + private File trustStoreBcfksFile; private final X509KeyType keyStoreKeyType; private final KeyPair keyStoreKeyPair; @@ -62,6 +63,7 @@ public class X509TestContext { private File keyStoreJksFile; private File keyStorePemFile; private File keyStorePkcs12File; + private File keyStoreBcfksFile; private final Boolean hostnameVerification; @@ -160,7 +162,9 @@ public class X509TestContext { return getTrustStorePemFile(); case PKCS12: return getTrustStorePkcs12File(); - default: + case BCFKS: + return getTrustStoreBcfksFile(); + default: throw new IllegalArgumentException("Invalid trust store type: " + storeFileType + ", must be one of: " @@ -210,6 +214,23 @@ public class X509TestContext { return trustStorePkcs12File; } + private File getTrustStoreBcfksFile() throws IOException { + if (trustStoreBcfksFile == null) { + File trustStoreBcfksFile = File.createTempFile( + TRUST_STORE_PREFIX, KeyStoreFileType.BCFKS.getDefaultFileExtension(), tempDir); + trustStoreBcfksFile.deleteOnExit(); + try (final FileOutputStream trustStoreOutputStream = new FileOutputStream(trustStoreBcfksFile)) { + byte[] bytes = X509TestHelpers.certToBCFKSTrustStoreBytes(trustStoreCertificate, trustStorePassword); + trustStoreOutputStream.write(bytes); + trustStoreOutputStream.flush(); + } catch (GeneralSecurityException e) { + throw new IOException(e); + } + this.trustStoreBcfksFile = trustStoreBcfksFile; + } + return trustStoreBcfksFile; + } + public X509KeyType getKeyStoreKeyType() { return keyStoreKeyType; } @@ -235,9 +256,9 @@ public class X509TestContext { } /** - * Returns the path to the key store file in the given format (JKS or PEM). Note that the file is created lazily, + * Returns the path to the key store file in the given format (JKS, PEM, ...). Note that the file is created lazily, * the first time this method is called. The key store file is temporary and will be deleted on exit. - * @param storeFileType the store file type (JKS or PEM). + * @param storeFileType the store file type (JKS, PEM, ...). * @return the path to the key store file. * @throws IOException if there is an error creating the key store file. */ @@ -249,6 +270,8 @@ public class X509TestContext { return getKeyStorePemFile(); case PKCS12: return getKeyStorePkcs12File(); + case BCFKS: + return getKeyStoreBcfksFile(); default: throw new IllegalArgumentException("Invalid key store type: " + storeFileType @@ -303,6 +326,24 @@ public class X509TestContext { return keyStorePkcs12File; } + private File getKeyStoreBcfksFile() throws IOException { + if (keyStoreBcfksFile == null) { + File keyStoreBcfksFile = File.createTempFile( + KEY_STORE_PREFIX, KeyStoreFileType.BCFKS.getDefaultFileExtension(), tempDir); + keyStoreBcfksFile.deleteOnExit(); + try (final FileOutputStream keyStoreOutputStream = new FileOutputStream(keyStoreBcfksFile)) { + byte[] bytes = X509TestHelpers.certAndPrivateKeyToBCFKSBytes( + keyStoreCertificate, keyStoreKeyPair.getPrivate(), keyStorePassword); + keyStoreOutputStream.write(bytes); + keyStoreOutputStream.flush(); + } catch (GeneralSecurityException e) { + throw new IOException(e); + } + this.keyStoreBcfksFile = keyStoreBcfksFile; + } + return keyStoreBcfksFile; + } + /** * Sets the SSL system properties such that the given X509Util object can be used to create SSL Contexts that * will use the trust store and key store files created by this test context. Example usage: @@ -314,8 +355,8 @@ public class X509TestContext { * SSLContext ctx = x509Util.getDefaultSSLContext(); * * @param x509Util the X509Util. - * @param keyStoreFileType the store file type to use for the key store (JKS or PEM). - * @param trustStoreFileType the store file type to use for the trust store (JKS or PEM). + * @param keyStoreFileType the store file type to use for the key store (JKS, PEM, ...). + * @param trustStoreFileType the store file type to use for the trust store (JKS, PEM, ...). * @throws IOException if there is an error creating the key store file or trust store file. */ public void setSystemProperties(X509Util x509Util, KeyStoreFileType keyStoreFileType, KeyStoreFileType trustStoreFileType) throws IOException { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java index 4f16968..fb1371a 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java @@ -72,7 +72,7 @@ import org.slf4j.LoggerFactory; /** * This class contains helper methods for creating X509 certificates and key pairs, and for serializing them - * to JKS or PEM files. + * to JKS, PEM or other keystore type files. */ public class X509TestHelpers { @@ -325,6 +325,24 @@ public class X509TestHelpers { return certToTrustStoreBytes(cert, keyPassword, trustStore); } + /** + * Encodes the given X509Certificate as a BCFKS TrustStore, optionally protecting the cert with a password (though + * it's unclear why one would do this since certificates only contain public information and do not need to be + * kept secret). Returns the byte array encoding of the trust store, which may be written to a file and loaded to + * instantiate the trust store at a later point or in another process. + * @param cert the certificate to serialize. + * @param keyPassword an optional password to encrypt the trust store. If empty or null, the cert will not be encrypted. + * @return the serialized bytes of the BCFKS trust store. + * @throws IOException + * @throws GeneralSecurityException + */ + public static byte[] certToBCFKSTrustStoreBytes( + X509Certificate cert, + String keyPassword) throws IOException, GeneralSecurityException { + KeyStore trustStore = KeyStore.getInstance("BCFKS"); + return certToTrustStoreBytes(cert, keyPassword, trustStore); + } + private static byte[] certToTrustStoreBytes(X509Certificate cert, String keyPassword, KeyStore trustStore) throws IOException, GeneralSecurityException { char[] keyPasswordChars = keyPassword == null ? new char[0] : keyPassword.toCharArray(); trustStore.load(null, keyPasswordChars); @@ -351,7 +369,7 @@ public class X509TestHelpers { public static byte[] certAndPrivateKeyToJavaKeyStoreBytes( X509Certificate cert, PrivateKey privateKey, String keyPassword) throws IOException, GeneralSecurityException { KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - return certAndPrivateKeyToPKCS12Bytes(cert, privateKey, keyPassword, keyStore); + return certAndPrivateKeyToBytes(cert, privateKey, keyPassword, keyStore); } /** @@ -368,10 +386,29 @@ public class X509TestHelpers { public static byte[] certAndPrivateKeyToPKCS12Bytes( X509Certificate cert, PrivateKey privateKey, String keyPassword) throws IOException, GeneralSecurityException { KeyStore keyStore = KeyStore.getInstance("PKCS12"); - return certAndPrivateKeyToPKCS12Bytes(cert, privateKey, keyPassword, keyStore); + return certAndPrivateKeyToBytes(cert, privateKey, keyPassword, keyStore); + } + + /** + * Encodes the given X509Certificate and private key as a BCFKS KeyStore, optionally protecting the private key + * (and possibly the cert?) with a password. Returns the byte array encoding of the key store, which may be written + * to a file and loaded to instantiate the key store at a later point or in another process. + * @param cert the X509 certificate to serialize. + * @param privateKey the private key to serialize. + * @param keyPassword an optional key password. If empty or null, the private key will not be encrypted. + * @return the serialized bytes of the BCFKS key store. + * @throws IOException + * @throws GeneralSecurityException + */ + public static byte[] certAndPrivateKeyToBCFKSBytes( + X509Certificate cert, + PrivateKey privateKey, + String keyPassword) throws IOException, GeneralSecurityException { + KeyStore keyStore = KeyStore.getInstance("BCFKS"); + return certAndPrivateKeyToBytes(cert, privateKey, keyPassword, keyStore); } - private static byte[] certAndPrivateKeyToPKCS12Bytes( + private static byte[] certAndPrivateKeyToBytes( X509Certificate cert, PrivateKey privateKey, String keyPassword, KeyStore keyStore) throws IOException, GeneralSecurityException { char[] keyPasswordChars = keyPassword == null ? new char[0] : keyPassword.toCharArray(); keyStore.load(null, keyPasswordChars);