[WIP] Add BouncyCastleFipsProvider as test dependency #41024

jkakavas · 2019-04-09T17:37:08Z

This commit sets up a fake ivy repository to use for adding the
JAR that provides the BCFIPS Security Provider as a
testRuntimeOnly dependency in all test tasks.

It also disables jarHell task for many projects that either have the
plain BouncyCastle provider jar as a dependency already ( plugin-cli,
security-cli, etc. ) or depend on the security plugin, that in
turn has a compileOnly dependency on the plain BouncyCastle provider
(because of opensaml)

This change was driven by the fact that the extensions mechanism
(/jre/lib/ext) was removed in JDK9 and BouncyCastle does not yet
offer a module for the BCFIPS provider that we could use to build
an image to run our tests in a FIPS approved mode. This
change enables us to run any JDK version JVM in FIPS approved
mode.

This commit sets up a fake ivy repository to use for adding the JAR that provides the BCFIPS Security Provider as a dependency and adds it as a testRuntimeOnly dependency in all test tasks. It also disables jarHell task for many projects that either have the plain BouncyCastle provider jar as a dependency already ( plugin-cli, security-cli, etc. ) or depend on the security plugin, that in turn has a compileOnly dependency on the plain BouncyCastle provider (because of opensaml) This change was driven by the fact that the extensions mechanism (/jre/lib/ext) was removed in JDK9 and BouncyCastle does not yet offer a module for the BCFIPS provider that we could use to build an image to run our tests in a FIPS approved mode. This change enables us to run any JDK version JVM in fips approved mode.

elasticmachine · 2019-04-09T17:37:10Z

Pinging @elastic/es-core-infra

elasticmachine · 2019-04-09T17:37:11Z

Pinging @elastic/es-security

jkakavas · 2019-04-09T17:39:50Z

plugins/discovery-azure-classic/build.gradle

@@ -63,6 +63,7 @@ File keystore = new File(project.buildDir, 'keystore/test-node.jks')

 // generate the keystore
 task createKey(type: LoggedExec) {
+  onlyIf { inFipsJvm == false }


this task would fail as the JVM is setup for FIPS (BCFKS keystores, etc) but the dependency is not added when this is evaluated so the necessary classes are not available. We don't use the produced keystore eitherway as this can only work with a JKS keystore which is not available in FIPS - and this is why the integTest task is also disabled

I'm not sure I follow. If integTest is disabled, what would try to run this task in FIPS mode?

I guess it's because of processTestResources.dependsOn(createKey) below. I'd be happy to explore alternatives if you have suggestions!

jkakavas · 2019-04-09T17:42:26Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

-            String inFipsJvmScript = 'print(java.security.Security.getProviders()[0].name.toLowerCase().contains("fips"));'
-            boolean inFipsJvm = Boolean.parseBoolean(runJavaAsScript(project, runtimeJavaHome, inFipsJvmScript))
+            // Java home name checking is fragile, but we control the environment
+            boolean inFipsJvm = runtimeJavaHome.contains("fips")


Previously, the Provider was statically added as a system wide dependency ( in lib/ext of a jdk8 JVM ) so we could evaluate the getProviders() result here. Now the preference order is still set in java.security but the dependency is not yet available so the provider won't be loaded

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

mark-vieira · 2019-04-09T20:59:05Z

distribution/tools/plugin-cli/build.gradle

-}
+if (inFipsJvm){
+  // Disable jarHell in FIPS as we add the BC FIPS Provider as a dependency for the tests
+  jarHell.enabled = false


I'd move this PrecommitTasks.configureJarHell() where the JarHell task is actually created rather than have to explicitly disable this all over the place.

Are we fine with disabling jarHell for all projects while running tests in a FIPS JVM ? If so, I'd be more than happy to

Yes. We have numerous other builds that will catch any issues here.

mark-vieira · 2019-04-09T21:00:58Z

plugins/discovery-azure-classic/build.gradle

@@ -63,6 +63,7 @@ File keystore = new File(project.buildDir, 'keystore/test-node.jks')

 // generate the keystore
 task createKey(type: LoggedExec) {
+  onlyIf { inFipsJvm == false }


I'm not sure I follow. If integTest is disabled, what would try to run this task in FIPS mode?

alpar-t

Added some notes.
Maybe I missed this but it seems this won't for rest tests as the tests themselves will run on fips jvm but the cluster will not.

alpar-t · 2019-04-11T13:28:20Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -590,6 +589,16 @@ class BuildPlugin implements Plugin<Project> {
                url "https://s3.amazonaws.com/download.elasticsearch.org/lucenesnapshots/${revision}"
            }
        }
+        String compilerJavaHome = findCompilerJavaHome()
+        String runtimeJavaHome = findRuntimeJavaHome(compilerJavaHome)
+        if (runtimeJavaHome.contains("fips")) {


Could we take this opportunity to clean things up and have something like -Dfips.enabled=true or -Dtests/fips=true rather than rely on the runtime java home name ?
My understanding is that we no longer need to make modifications to that location, so it will only really act as a flag.

Yes, good idea ! What we would need to do is to bundle the java.policy and java.security (one for each jdk version) as test resources and adjust the system properties ( -Djava.security.policy and -Djava.security.properties IIRC) the same way we do for the keystore password. Would that be ok ?

I think that's better as it will have all the fips config contained in our build.
You'll probably want to look at the ExportElasticsearchBuildResourcesTask tasks , there's already one created for each project.

alpar-t · 2019-04-11T13:29:36Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy

+
+        // Java home name checking is fragile, but we control the environment
+        if (project.runtimeJavaHome.contains("fips")){
+            task.enabled = false


I think it's worth to have a comment here explaining why we can't have jar hell in fips.

alpar-t · 2019-04-11T13:30:54Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -906,6 +915,10 @@ class BuildPlugin implements Plugin<Project> {
            File heapdumpDir = new File(project.buildDir, 'heapdump')

            project.tasks.withType(Test) { Test test ->
+                RepositoryHandler repos = project.repositories
+                if (project.ext.inFipsJvm) {
+                    project.dependencies.add('testRuntimeOnly', "org.bouncycastle:bc-fips:1.0.1:jar")


I don't think we carry this classpath tot he clusters running in the rest tests, so these will probably not be running with fips enabled. We should have a smoke test to verify that fips is really enabled everywhere we think it is when we build.

Maybe I missed this but it seems this won't for rest tests as the tests themselves will run on fips jvm but the cluster will not.

Good catch, thanks

How about the integTest clusters? Those are covered by the above, correct ?

yes internal clusters run in the same jvm as tests so would are covered.

jkakavas · 2019-04-16T09:43:59Z

Marking this as a [WIP], until we have some feedback for JDK11, see #37250 (comment)

This change enables us to run our test suites in JVMs configured in FIPS 140 approved mode. It does so by: - Using BouncyCastle FIPS Cryptographic provider and BSJSSE in FIPS mode. These are used as testRuntime dependencies for unit tests and internal clusters, and copied (relevant jars) explicitly to the lib directory for testclusters used in REST tests - Configuring any given runtime Java in FIPS mode with the bundled policy and security properties files, setting the system properties java.security.properties and java.security.policy with the == operator that overrides the default JVM properties and policy. Running the tests in FIPS 140 approved mode doesn't require an additional configuration either in CI workers or locally and is controlled by specifying -Dtests.fips.enabled=true Closes: #37250 Supersedes: #41024

This change enables us to run our test suites in JVMs configured in FIPS 140 approved mode. It does so by: - Using BouncyCastle FIPS Cryptographic provider and BSJSSE in FIPS mode. These are used as testRuntime dependencies for unit tests and internal clusters, and copied (relevant jars) explicitly to the lib directory for testclusters used in REST tests - Configuring any given runtime Java in FIPS mode with the bundled policy and security properties files, setting the system properties java.security.properties and java.security.policy with the == operator that overrides the default JVM properties and policy. Running the tests in FIPS 140 approved mode doesn't require an additional configuration either in CI workers or locally and is controlled by specifying -Dtests.fips.enabled=true Closes: elastic#37250 Supersedes: elastic#41024

jkakavas added 2 commits April 9, 2019 17:38

Merge remote-tracking branch 'origin/master' into bcfips-as-dependency

9cd6107

jkakavas added >non-issue :Delivery/Build Build or test infrastructure :Security/Security Security issues without another label v8.0.0 v7.2.0 v6.7.2 v7.0.1 labels Apr 9, 2019

jkakavas requested review from alpar-t and mark-vieira April 9, 2019 17:37

jkakavas commented Apr 9, 2019

View reviewed changes

mark-vieira requested changes Apr 9, 2019

View reviewed changes

address feedback

0a216c6

jkakavas requested a review from mark-vieira April 10, 2019 07:49

alpar-t reviewed Apr 11, 2019

View reviewed changes

This was referenced Apr 15, 2019

Add FIPS specific testclusters configuration #41199

Merged

Add a FIPS 140-2 enabled JDK 11 JVM in CI #37250

Closed

jkakavas added the WIP label Apr 16, 2019

jkakavas changed the title ~~Add BouncyCastleFipsProvider as test dependency~~ [WIP] Add BouncyCastleFipsProvider as test dependency Apr 16, 2019

colings86 added v6.7.3 and removed v6.7.2 labels Apr 17, 2019

jaymode added v7.0.2 and removed v7.0.1 labels Apr 24, 2019

jakelandis added the v6.8.0 label May 3, 2019

jakelandis removed the v6.7.3 label May 3, 2019

jakelandis added v6.8.1 and removed v6.8.0 labels May 19, 2019

jakelandis added v7.3.0 v6.8.2 and removed v7.2.0 v6.8.1 labels Jun 17, 2019

jpountz added v7.4.0 and removed v7.3.0 labels Jul 3, 2019

jakelandis added v6.8.3 and removed v6.8.2 labels Jul 24, 2019

pcsanwald added v6.8.4 and removed v6.8.3 labels Aug 29, 2019

colings86 added v7.5.0 and removed v7.4.0 labels Aug 30, 2019

tomcallahan added v6.8.5 and removed v6.8.4 labels Oct 22, 2019

jkakavas closed this Oct 23, 2019

jkakavas mentioned this pull request Oct 23, 2019

Enable tests in FIPS 140 in JDK 11 #48378

Merged

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add BouncyCastleFipsProvider as test dependency #41024

[WIP] Add BouncyCastleFipsProvider as test dependency #41024

jkakavas commented Apr 9, 2019

elasticmachine commented Apr 9, 2019

elasticmachine commented Apr 9, 2019

jkakavas Apr 9, 2019

mark-vieira Apr 9, 2019

jkakavas Apr 10, 2019

jkakavas Apr 9, 2019 •

edited

Loading

mark-vieira Apr 9, 2019

jkakavas Apr 10, 2019

mark-vieira Apr 17, 2019

mark-vieira Apr 9, 2019

alpar-t left a comment

alpar-t Apr 11, 2019

jkakavas Apr 11, 2019

alpar-t Apr 12, 2019

alpar-t Apr 11, 2019

alpar-t Apr 11, 2019

jkakavas Apr 11, 2019

alpar-t Apr 12, 2019

jkakavas commented Apr 16, 2019

[WIP] Add BouncyCastleFipsProvider as test dependency #41024

[WIP] Add BouncyCastleFipsProvider as test dependency #41024

Conversation

jkakavas commented Apr 9, 2019

elasticmachine commented Apr 9, 2019

elasticmachine commented Apr 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpar-t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Apr 16, 2019

jkakavas Apr 9, 2019 •

edited

Loading