Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make plugin verification FIPS 140 compliant #44224

Merged
merged 6 commits into from
Jul 12, 2019

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Jul 11, 2019

This change makes the process of verifying the signature of
official plugins FIPS 140 compliant by defaulting to use the
BouncyCastle FIPS provider and adding a dependency to bcpg-fips
that implement parts of openPGP in a FIPS compliant manner.

In already FIPS 140 enabled environments that use the BouncyCastle
FIPS provider, the bcfips dependency is redundant but doesn't
cause an issue as it will be added only in the classpath for the
cli-tools

Resolves: #41263

This change makes the process of verifying the signature of
official plugins FIPS 140 compliant by defaulting to use the
BouncyCastle FIPS provider and adding a dependency to bcpg-fips
that implement parts of openPGP in a FIPS compliant manner.

In already FIPS 140 enabled environments that use the BouncyCastle
FIPS provider, the bcfips dependency is redundant but doesn't
cause an issue as it will be added only in the classpath of the
cli-tools
@jkakavas jkakavas added >enhancement :Core/Infra/Plugins Plugin API and infrastructure labels Jul 11, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@@ -24,8 +24,8 @@ archivesBaseName = 'elasticsearch-plugin-cli'
dependencies {
compileOnly project(":server")
compileOnly project(":libs:elasticsearch-cli")
compile "org.bouncycastle:bcpg-jdk15on:${versions.bouncycastle}"
compile "org.bouncycastle:bcprov-jdk15on:${versions.bouncycastle}"
compile "org.bouncycastle:bcpg-fips:1.0.3"
Copy link
Member Author

Choose a reason for hiding this comment

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

each X-fips library uses its own versioning, so we explicitly set it here

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

Will this work in java 9+? Everything I've read about bouncycastle fips looks like it only works in java 8. Also, how will this work when we are not running in a fips jvm, given we are using fips apis here?

}

// these two classes intentionally use JDK internal API
Copy link
Member

Choose a reason for hiding this comment

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

What apis are they using and why? I think that info is important to be in this comment as a future dev evaluating whether this can be removed would need to know it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added the list of the list of internal classes used. The "why" i think is self-explanatory, unless you mean the technical reasons behind using these internal APIs in the BC code ( assuming there were other alternatives that didn't involve doing so ) but I wouldn't be in the position to answer that.

@jkakavas
Copy link
Member Author

Will this work in java 9+? Everything I've read about bouncycastle fips looks like it only works in java 8.

Yes it already does. Problems with 9+ and FIPS are due to the fact that the option to pass parameters to the provider is removed when installing the FIPS Provider statically in java.security ( see my writeup here. Here we just use the FIPS Provider when applicable/necessary , see the calls to setProvider() i.e. here

Also, how will this work when we are not running in a fips jvm, given we are using fips apis here?

A fips jvm is nothing more than a normal JVM that uses a fips approved security provider implementation for everything. Here, we don't need such a setup, all the necessary APIs are provided by bcpg-fips and bcfips that are FIPS 140 approved and are brought in as dependencies.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@jkakavas jkakavas merged commit 5292e17 into elastic:master Jul 12, 2019
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Jul 12, 2019
This change makes the process of verifying the signature of
official plugins FIPS 140 compliant by defaulting to use the
BouncyCastle FIPS provider and adding a dependency to bcpg-fips
that implement parts of openPGP in a FIPS compliant manner.

In already FIPS 140 enabled environments that use the 
BouncyCastle FIPS provider, the bcfips dependency is redundant
but doesn't cause an issue as it will be added only in the classpath
 of the cli-tools
jkakavas added a commit that referenced this pull request Jul 12, 2019
This change makes the process of verifying the signature of
official plugins FIPS 140 compliant by defaulting to use the
BouncyCastle FIPS provider and adding a dependency to bcpg-fips
that implement parts of openPGP in a FIPS compliant manner.

In already FIPS 140 enabled environments that use the 
BouncyCastle FIPS provider, the bcfips dependency is redundant
but doesn't cause an issue as it will be added only in the classpath
 of the cli-tools

This is a backport of #44224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use RSA/DSA/EC instead of PGP signatures for official plugins
4 participants