You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by zh...@apache.org on 2021/07/05 07:22:31 UTC
[bookkeeper] branch master updated: Fix Bouncy Castle fips
incompatible issue (#2740)
This is an automated email from the ASF dual-hosted git repository.
zhaijia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new d03b046 Fix Bouncy Castle fips incompatible issue (#2740)
d03b046 is described below
commit d03b046c2526a3b89305da6460b601656ecd0700
Author: Jia Zhai <zh...@apache.org>
AuthorDate: Mon Jul 5 15:22:23 2021 +0800
Fix Bouncy Castle fips incompatible issue (#2740)
### Motivation
More details are provided in [Pulsar # 10937](https://github.com/apache/pulsar/issues/10937).
In #2631, the default BouncyCastle was changed from non-fips into fips version. But the default version of BouncyCastle in Pulsar is the [non-fips](https://github.com/apache/pulsar/blob/v2.8.0/pulsar-client/pom.xml#L56) one(aimed to make it compatible with the old version of Pulsar).
Bouncy Castle provides both FIPS and non-FIPS versions, but in a JVM, it can not include both of the 2 versions(non-Fips and Fips), and we have to exclude the current version before including the other. This makes the backward compatible a little hard, and that's why Pulsar has to involve an individual module for [Bouncy Castle](https://pulsar.apache.org/docs/en/security-bouncy-castle).
And if we want to start BookKeeper with TLS enabled through Pulsar's binary, it will meet the following error:
```
Exception in thread "main" java.lang.NoClassDefFoundError: org/bouncycastle/jcajce/provider/BouncyCastleFipsProvider
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:315)
at org.apache.bookkeeper.common.util.ReflectionUtils.forName(ReflectionUtils.java:49)
at org.apache.bookkeeper.tls.SecurityProviderFactoryFactory.getSecurityProviderFactory(SecurityProviderFactoryFactory.java:39)
at org.apache.bookkeeper.proto.BookieServer.<init>(BookieServer.java:129)
at org.apache.bookkeeper.server.service.BookieService.<init>(BookieService.java:52)
at org.apache.bookkeeper.server.Main.buildBookieServer(Main.java:304)
at org.apache.bookkeeper.server.Main.doMain(Main.java:226)
at org.apache.bookkeeper.server.Main.main(Main.java:208)
Caused by: java.lang.ClassNotFoundException: org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
... 9 more
```
This fix is to use the reflection to get the loaded bc version to avoid the hard-coded bc version.
### Changes
Use the reflection to get the loaded bc version to avoid the hard-coded bc version
Add backward compatible test for bc-non-fips version
---
.../apache/bookkeeper/tls/TLSContextFactory.java | 94 +++++++++++++++++-----
.../java/org/apache/bookkeeper/tls/TestTLS.java | 9 +++
tests/backward-compat/bc-non-fips/pom.xml | 79 ++++++++++++++++++
.../org/apache/bookkeeper/tls/TestBCNonFips.java | 36 +++++++++
tests/backward-compat/pom.xml | 1 +
5 files changed, 201 insertions(+), 18 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/tls/TLSContextFactory.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/tls/TLSContextFactory.java
index 29dbd14..fdb2d01 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/tls/TLSContextFactory.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/tls/TLSContextFactory.java
@@ -37,6 +37,8 @@ import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
+import java.security.Provider;
+import java.security.Security;
import java.security.UnrecoverableKeyException;
import java.security.cert.CertificateException;
import java.security.spec.InvalidKeySpecException;
@@ -46,21 +48,78 @@ import java.util.concurrent.TimeUnit;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.TrustManagerFactory;
+import lombok.extern.slf4j.Slf4j;
import org.apache.bookkeeper.conf.AbstractConfiguration;
import org.apache.bookkeeper.conf.ClientConfiguration;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.commons.io.FileUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* A factory to manage TLS contexts.
*/
+@Slf4j
public class TLSContextFactory implements SecurityHandlerFactory {
- static {
- // Fixes loading PKCS8Key file: https://stackoverflow.com/a/18912362
- java.security.Security.addProvider(new org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider());
+ public static final Provider BC_PROVIDER = getProvider();
+ public static final String BC_FIPS_PROVIDER_CLASS = "org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider";
+ public static final String BC_NON_FIPS_PROVIDER_CLASS = "org.bouncycastle.jce.provider.BouncyCastleProvider";
+
+ // Security.getProvider("BC") / Security.getProvider("BCFIPS").
+ // also used to get Factories. e.g. CertificateFactory.getInstance("X.509", "BCFIPS")
+ public static final String BC_FIPS = "BCFIPS";
+ public static final String BC = "BC";
+
+ /**
+ * Get Bouncy Castle provider, and call Security.addProvider(provider) if success.
+ */
+ public static Provider getProvider() {
+ boolean isProviderInstalled =
+ Security.getProvider(BC) != null || Security.getProvider(BC_FIPS) != null;
+
+ if (isProviderInstalled) {
+ Provider provider = Security.getProvider(BC) != null
+ ? Security.getProvider(BC)
+ : Security.getProvider(BC_FIPS);
+ if (log.isDebugEnabled()) {
+ log.debug("Already instantiated Bouncy Castle provider {}", provider.getName());
+ }
+ return provider;
+ }
+
+ // Not installed, try load from class path
+ try {
+ return getBCProviderFromClassPath();
+ } catch (Exception e) {
+ log.warn("Not able to get Bouncy Castle provider for both FIPS and Non-FIPS from class path:", e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ /**
+ * Get Bouncy Castle provider from classpath, and call Security.addProvider.
+ * Throw Exception if failed.
+ */
+ public static Provider getBCProviderFromClassPath() throws Exception {
+ Class clazz;
+ try {
+ clazz = Class.forName(BC_FIPS_PROVIDER_CLASS);
+ } catch (ClassNotFoundException cnf) {
+ if (log.isDebugEnabled()) {
+ log.debug("Not able to get Bouncy Castle provider: {}, try to get FIPS provider {}",
+ BC_NON_FIPS_PROVIDER_CLASS, BC_FIPS_PROVIDER_CLASS);
+ }
+ // attempt to use the NON_FIPS provider.
+ clazz = Class.forName(BC_NON_FIPS_PROVIDER_CLASS);
+
+ }
+
+ @SuppressWarnings("unchecked")
+ Provider provider = (Provider) clazz.getDeclaredConstructor().newInstance();
+ Security.addProvider(provider);
+ if (log.isDebugEnabled()) {
+ log.debug("Found and Instantiated Bouncy Castle provider in classpath {}", provider.getName());
+ }
+ return provider;
}
/**
@@ -83,7 +142,6 @@ public class TLSContextFactory implements SecurityHandlerFactory {
}
}
- private static final Logger LOG = LoggerFactory.getLogger(TLSContextFactory.class);
private static final String TLSCONTEXT_HANDLER_NAME = "tls";
private String[] protocols;
private String[] ciphers;
@@ -130,7 +188,7 @@ public class TLSContextFactory implements SecurityHandlerFactory {
KeyManagerFactory kmf = null;
if (Strings.isNullOrEmpty(keyStoreLocation)) {
- LOG.error("Key store location cannot be empty when Mutual Authentication is enabled!");
+ log.error("Key store location cannot be empty when Mutual Authentication is enabled!");
throw new SecurityException("Key store location cannot be empty when Mutual Authentication is enabled!");
}
@@ -153,7 +211,7 @@ public class TLSContextFactory implements SecurityHandlerFactory {
TrustManagerFactory tmf;
if (Strings.isNullOrEmpty(trustStoreLocation)) {
- LOG.error("Trust Store location cannot be empty!");
+ log.error("Trust Store location cannot be empty!");
throw new SecurityException("Trust Store location cannot be empty!");
}
@@ -173,18 +231,18 @@ public class TLSContextFactory implements SecurityHandlerFactory {
private SslProvider getTLSProvider(String sslProvider) {
if (sslProvider.trim().equalsIgnoreCase("OpenSSL")) {
if (OpenSsl.isAvailable()) {
- LOG.info("Security provider - OpenSSL");
+ log.info("Security provider - OpenSSL");
return SslProvider.OPENSSL;
}
Throwable causeUnavailable = OpenSsl.unavailabilityCause();
- LOG.warn("OpenSSL Unavailable: ", causeUnavailable);
+ log.warn("OpenSSL Unavailable: ", causeUnavailable);
- LOG.info("Security provider - JDK");
+ log.info("Security provider - JDK");
return SslProvider.JDK;
}
- LOG.info("Security provider - JDK");
+ log.info("Security provider - JDK");
return SslProvider.JDK;
}
@@ -306,7 +364,7 @@ public class TLSContextFactory implements SecurityHandlerFactory {
|| tlsKeyStorePasswordFilePath.checkAndRefresh() || tlsTrustStoreFilePath.checkAndRefresh()
|| tlsTrustStorePasswordFilePath.checkAndRefresh()) {
try {
- LOG.info("Updating tls certs certFile={}, keyStoreFile={}, trustStoreFile={}",
+ log.info("Updating tls certs certFile={}, keyStoreFile={}, trustStoreFile={}",
tlsCertificateFilePath.getFileName(), tlsKeyStoreFilePath.getFileName(),
tlsTrustStoreFilePath.getFileName());
if (isServerCtx) {
@@ -315,7 +373,7 @@ public class TLSContextFactory implements SecurityHandlerFactory {
updateClientContext();
}
} catch (Exception e) {
- LOG.info("Failed to refresh tls certs", e);
+ log.info("Failed to refresh tls certs", e);
}
}
}
@@ -469,15 +527,15 @@ public class TLSContextFactory implements SecurityHandlerFactory {
if (protocols != null && protocols.length != 0) {
sslHandler.engine().setEnabledProtocols(protocols);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("Enabled cipher protocols: {} ", Arrays.toString(sslHandler.engine().getEnabledProtocols()));
+ if (log.isDebugEnabled()) {
+ log.debug("Enabled cipher protocols: {} ", Arrays.toString(sslHandler.engine().getEnabledProtocols()));
}
if (ciphers != null && ciphers.length != 0) {
sslHandler.engine().setEnabledCipherSuites(ciphers);
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("Enabled cipher suites: {} ", Arrays.toString(sslHandler.engine().getEnabledCipherSuites()));
+ if (log.isDebugEnabled()) {
+ log.debug("Enabled cipher suites: {} ", Arrays.toString(sslHandler.engine().getEnabledCipherSuites()));
}
return sslHandler;
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/tls/TestTLS.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/tls/TestTLS.java
index f4b59d8..d2c420c 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/tls/TestTLS.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/tls/TestTLS.java
@@ -234,6 +234,15 @@ public class TestTLS extends BookKeeperClusterTestCase {
}
/**
+ * Verify the BouncyCastleProvider Name is expected.
+ */
+ @Test
+ public void testGetBouncyCastleProviderName() throws Exception {
+ String bcName = TLSContextFactory.getProvider().getName();
+ Assert.assertEquals(bcName, TLSContextFactory.BC_FIPS);
+ }
+
+ /**
* Verify that a server will not start if tls is enabled but no cert is specified.
*/
@Test
diff --git a/tests/backward-compat/bc-non-fips/pom.xml b/tests/backward-compat/bc-non-fips/pom.xml
new file mode 100644
index 0000000..eba5b1d
--- /dev/null
+++ b/tests/backward-compat/bc-non-fips/pom.xml
@@ -0,0 +1,79 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation=" http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.bookkeeper.tests</groupId>
+ <artifactId>backward-compat</artifactId>
+ <version>4.15.0-SNAPSHOT</version>
+ <relativePath>..</relativePath>
+ </parent>
+
+ <groupId>org.apache.bookkeeper.tests.backward-compat</groupId>
+ <artifactId>bc-non-fips</artifactId>
+ <packaging>jar</packaging>
+ <name>Apache BookKeeper :: Tests :: Backward Compatibility :: Test Bouncy Castle Provider load non FIPS version</name>
+ <properties>
+ <bc-non-fips.version>1.68</bc-non-fips.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <version>${junit.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.bookkeeper</groupId>
+ <artifactId>bookkeeper-server</artifactId>
+ <version>${project.version}</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.bouncycastle</groupId>
+ <artifactId>*</artifactId>
+ </exclusion>
+ </exclusions>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.bouncycastle</groupId>
+ <artifactId>bcpkix-jdk15on</artifactId>
+ <version>${bc-non-fips.version}</version>
+ </dependency>
+
+ <dependency>
+ <groupId>org.bouncycastle</groupId>
+ <artifactId>bcprov-ext-jdk15on</artifactId>
+ <version>${bc-non-fips.version}</version>
+ </dependency>
+
+ </dependencies>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-deploy-plugin</artifactId>
+ <configuration>
+ <skip>true</skip>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+</project>
diff --git a/tests/backward-compat/bc-non-fips/src/test/java/org/apache/bookkeeper/tls/TestBCNonFips.java b/tests/backward-compat/bc-non-fips/src/test/java/org/apache/bookkeeper/tls/TestBCNonFips.java
new file mode 100644
index 0000000..eded70b
--- /dev/null
+++ b/tests/backward-compat/bc-non-fips/src/test/java/org/apache/bookkeeper/tls/TestBCNonFips.java
@@ -0,0 +1,36 @@
+/**
+ * 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.bookkeeper.tls;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Test Bouncy Castle Provider load non FIPS version.
+ */
+public class TestBCNonFips {
+
+ /**
+ * Verify the BouncyCastleProvider Name is expected.
+ */
+ @Test
+ public void testGetBouncyCastleProviderName() {
+ String bcName = TLSContextFactory.getProvider().getName();
+ Assert.assertEquals(bcName, TLSContextFactory.BC);
+ }
+}
diff --git a/tests/backward-compat/pom.xml b/tests/backward-compat/pom.xml
index af9dbfa..396840b 100644
--- a/tests/backward-compat/pom.xml
+++ b/tests/backward-compat/pom.xml
@@ -36,5 +36,6 @@
<module>old-cookie-new-cluster</module>
<module>current-server-old-clients</module>
<module>yahoo-custom-version</module>
+ <module>bc-non-fips</module>
</modules>
</project>