Skip to content

Conversation

@jkakavas
Copy link
Contributor

This change enables us to run our test suites in JVMs configured in
FIPS 140 approved mode without a CI specific setup. It does so by:

  • Using BouncyCastle FIPS Cryptographic provider and BSJSSE in
    FIPS mode for JDK9 and later. 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

  • For JDK8 we still use SunJSSE in FIPS mode ( which also uses the
    BouncyCastle FIPS Cryptographic provider ) since this is what we
    have been testing with so far and this is what probably all existing
    users are currently using to run ES in FIPS 140 mode.

  • Configuring any given runtime Java in FIPS mode using 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

This will not be merged until #49096 is merged, because we will be removing all the FIPS 140 related logic from CI and we will need a way to define
FIPS 140 jobs in our repo.

This is a backport of #48378 and #49319

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
- Don't install ingest-attachment and don't run the related docs
tests, since ingest-attachment is not supported in FIPS 140 JVMs
- Move copying extra jars and extra config files earlier on in the
node configuration so that elasticsearch-keystore and
elasticsearch-plugin that run before the node starts have all files
(policy, properties, jars) available.
- BCJSSE needs a certificate to be explicitly added in a keystore
as a trustedcerty entry, it's not enough for it to be in privatekeyentry
for it to be trusted
- Set the value for BuildParams.inFipsJvm configuration time
…ing all custom FIPS related parts from our CI and will define a new job for fips
@jkakavas jkakavas added >enhancement :Delivery/Build Build or test infrastructure :Security/Security Security issues without another label v7.6.0 labels Nov 22, 2019
@jkakavas jkakavas requested a review from mark-vieira November 22, 2019 11:06
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Build)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Security)

@mark-vieira
Copy link
Contributor

This will not be merged until #49096 is merged, because we will be removing all the FIPS 140 related logic from CI and we will need a way to define FIPS 140 jobs in our repo.

FYI, it might be a bit until we can get that merged. I'll be out all next week and there's still testing that needs to be done here. If this is high priority perhaps we can make this compatible with the existing job definitions somehow?

@jkakavas
Copy link
Contributor Author

FYI, it might be a bit until we can get that merged. I'll be out all next week and there's still testing that needs to be done here. If this is high priority perhaps we can make this compatible with the existing job definitions somehow?

I don't think an extra couple of weeks would be detrimental, we still run the 7.x branches in FIPS 140 with the current CI setup, so I'd prefer to not introduce any temporary setup. No harm in getting this approved, and I'll have an extra one ready to raise with a job definition for FIPS tests to go along with it once we merge #49096

File securityPolicy = buildResources.copy("fips_java.policy")
File security8Policy = buildResources.copy("fips_java8.policy")
File bcfksKeystore = buildResources.copy("cacerts.bcfks")
GlobalInfoExtension globalInfo = project.rootProject.extensions.getByType(GlobalInfoExtension)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The major difference with #48378 and #49319 that are being backported is here, where we conditionally use a different policy/properties file for 8

@mark-vieira
Copy link
Contributor

@jkakavas Is #49096 strictly required for this? I don't want to hold this stuff up any longer for that work which requires quite a bit of testing still. Perhaps we can sync up on this.

@jkakavas
Copy link
Contributor Author

@jkakavas Is #49096 strictly required for this? I don't want to hold this stuff up any longer for that work which requires quite a bit of testing still. Perhaps we can sync up on this.

@mark-vieira I don't think it's strictly required. This seemed like the appropriate way to trigger fips builds now that we are stripping all relevant handling from the infra side. We could trigger CI builds in another way (haven't thought what that might be), maybe you have a better idea. Let's sync today

@polyfractal polyfractal added v7.7.0 and removed v7.6.0 labels Jan 15, 2020
@jkakavas
Copy link
Contributor Author

@mark-vieira merge conflicts are taken care of and CI is green, this is ready for a review

Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments but otherwise LGTM.

if (testClusters != null) {
testClusters.all { ElasticsearchCluster cluster ->
cluster.setTestDistribution(TestDistribution.DEFAULT)
globalInfo.ready {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only bit that needs to be inside this ready { } block is the bit that checks runtimeJavaVersion. Everything else can be done at configuration time.

}
project.tasks.withType(Test).configureEach { Test task ->
task.dependsOn(buildResources)
globalInfo.ready {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, we can limit the surface area of ready { } here to only that bit conditional on java runtime version.

@jkakavas jkakavas merged commit ee202a6 into elastic:7.x Jan 27, 2020
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Jan 30, 2020
This change changes the way to run our test suites in 
JVMs configured in FIPS 140 approved mode. It does so by:

- 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.

- When runtime java is 11 and higher, using BouncyCastle FIPS 
Cryptographic provider and BCJSSE 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

- When runtime java is 8, using BouncyCastle FIPS 
Cryptographic provider and SunJSSE in FIPS mode. 

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
jkakavas added a commit that referenced this pull request Jan 30, 2020
This change changes the way to run our test suites in 
JVMs configured in FIPS 140 approved mode. It does so by:

- 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.

- When runtime java is 11 and higher, using BouncyCastle FIPS 
Cryptographic provider and BCJSSE 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

- When runtime java is 8, using BouncyCastle FIPS 
Cryptographic provider and SunJSSE in FIPS mode. 

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
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Feb 5, 2020
The relevant handling part has been removed in infra, FIPS 140
testing needs to be enabled in the same manner as it happened for
master and 7.x in elastic#48378 and elastic#49485

resolves: elastic#51924
jkakavas added a commit that referenced this pull request Feb 5, 2020
The relevant handling part has been removed in infra, FIPS 140
testing needs to be enabled in the same manner as it happened for
master and 7.x in #48378 and #49485

resolves: #51924
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Feb 6, 2020
The relevant handling part has been removed in infra, FIPS 140
testing needs to be enabled in the same manner as it happened for
master and 7.x in elastic#48378 and elastic#49485

resolves: elastic#51980
jkakavas added a commit that referenced this pull request Feb 6, 2020
The relevant handling part has been removed in infra, FIPS 140
testing needs to be enabled in the same manner as it happened for
master and 7.x in #48378 and #49485

resolves: #51980
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure >enhancement :Security/Security Security issues without another label Team:Delivery Meta label for Delivery team v7.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants