Make Bookkeeper FIPS compliant by default by Ghatage · Pull Request #2631 · apache/bookkeeper

Ghatage · 2021-02-27T21:29:52Z

Descriptions of the changes in this PR:
Mailing list thread: link

Motivation

FIPS is 'Federal Information Processing Standard'. Basically it's a set of guidelines for security functions such as encryption/decryption/RNG etc. Applications running in FIPS mode are said to be more secure as they adhere to more stringent standards.

Java's security framework is extensible via the JCE (Java crypto extension) allows us to use libraries which implement these functions. In general we use BouncyCastle as it has an active, supportive community and also maintains the fips versions of their libraries.

Pulsar currently has FIPS support but it's not on by default, I had a chat with @jiazhai about it and he mentioned that there is no specific reason as to why we run without it.

This is an attempt to get Apache BookKeeper FIPS compliant by default. If all tests pass and there are no performance regressions, we can merge this in.

Changes

Added dependency of FIPS approved BouncyCastle
Excluded the use of non FIPS BouncyCastle libraries.
Added flag to start up script which will make it fips compliant.

eolivelli

LGTM

eolivelli

looks good, but please fix this license errors


+ dev/check-binary-license dev/../bookkeeper-dist/server/target/bookkeeper-server-4.14.0-SNAPSHOT-bin.tar.gz
org.bouncycastle-bcpkix-jdk15on-1.61.jar unaccounted for in LICENSE
org.bouncycastle-bcprov-jdk15on-1.61.jar unaccounted for in LICENSE

probably you did not exclude the old BK dependencies from all of the places

Ghatage · 2021-03-02T20:11:57Z

@eolivelli PTAL. Resolved all LICENSE errors.

eolivelli

LGTM

@hsaputra do you want to take a look as well ? as you participated in the ML discussion

hsaputra

LGTM +1

hsaputra · 2021-03-03T23:15:55Z

Thanks @eolivelli - the changes LGTM

eolivelli · 2021-03-04T08:48:32Z

committed to master branch
thank you @Ghatage

### Motivation More details are provided in [Pulsar # 10937](apache/pulsar#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

### Motivation More details are provided in [Pulsar # 10937](apache/pulsar#10937). In apache#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 (cherry picked from commit d03b046)

More details are provided in [Pulsar # 10937](apache/pulsar#10937). In apache#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. 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 (cherry picked from commit d03b046) (cherry picked from commit e54be34)

### Motivation More details are provided in [Pulsar # 10937](apache/pulsar#10937). In apache#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

Ghatage added 2 commits February 27, 2021 13:26

Make Bookkeeper FIPS compliant by default

393a50a

Add LICENSE entry

96a630e

eolivelli approved these changes Mar 1, 2021

View reviewed changes

eolivelli requested changes Mar 1, 2021

View reviewed changes

Ghatage added 4 commits March 1, 2021 21:19

Exclude non-fips BwouncyCastle

5cd1021

Remove duplicate line

dfd79aa

Add details to LICENSE files

b74f0f6

Package LICENSE for bc fips

fe45f8f

eolivelli approved these changes Mar 3, 2021

View reviewed changes

eolivelli requested a review from hsaputra March 3, 2021 08:11

hsaputra approved these changes Mar 3, 2021

View reviewed changes

eolivelli added this to the 4.14.0 milestone Mar 4, 2021

eolivelli added area/bookie release/4.14.0 type/bug labels Mar 4, 2021

eolivelli assigned Ghatage Mar 4, 2021

eolivelli merged commit 09b3105 into apache:master Mar 4, 2021

eolivelli added type/improvement and removed type/bug labels Mar 4, 2021

Ghatage added the area/security label Mar 4, 2021

jiazhai mentioned this pull request Jun 16, 2021

Incompatible BouncyCastle FIPS changes in bookkeeper 4.14 apache/pulsar#10937

Closed

sijie mentioned this pull request Jun 16, 2021

ISSUE-10937: Incompatible BouncyCastle FIPS changes in bookkeeper 4.14 streamnative/pulsar-archived#2652

Open

jiazhai mentioned this pull request Jun 16, 2021

Fix Bouncy Castle fips incompatible issue #2740

Merged

lhotari mentioned this pull request Jun 13, 2024

Upgrade to Netty 4.1.111.Final and switch to use grpc-netty-shaded #4426

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Bookkeeper FIPS compliant by default#2631

Make Bookkeeper FIPS compliant by default#2631
eolivelli merged 6 commits intoapache:masterfrom
Ghatage:fipsCompliance

Ghatage commented Feb 27, 2021

Uh oh!

eolivelli left a comment

Uh oh!

eolivelli left a comment

Uh oh!

Ghatage commented Mar 2, 2021

Uh oh!

eolivelli left a comment

Uh oh!

hsaputra left a comment

Uh oh!

hsaputra commented Mar 3, 2021

Uh oh!

eolivelli commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Ghatage commented Feb 27, 2021

Motivation

Changes

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

Ghatage commented Mar 2, 2021

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

hsaputra left a comment

Choose a reason for hiding this comment

Uh oh!

hsaputra commented Mar 3, 2021

Uh oh!

eolivelli commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants