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

Support for FIPS compliance mode #14912

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

beanuwave
Copy link

@beanuwave beanuwave commented Jul 23, 2024

Description

This PR makes FIPS mode available through the OPENSEARCH_CRYPTO_STANDARD=FIPS-140-3 environmental parameter instead of the tests.fips.enabled setting. It provides FIPS 140-3 support by replacing all BC dependencies with BCFIPS dependencies and making FIPS approved-only mode configurable at launch. Running this mode restricts the BCFIPS provider to rely solely on FIPS-certified ciphers.

  • The fips.gradle build script is removed in order to support a single-build solution.
  • All BC dependencies are replaced by BCFIPS.
  • Fixed creation of password protected internal keystore for valuable settings at node start. The add-string command was made with additional --stdin option, which interferes with password input.
  • The Password Matcher inside Identity-Shiro that relies on BC to check if hashed passwords match with OpenBSDBCrypt is replaced by the password4j implementation.
  • Adds full support for the BCFKS format (*.bcfks) for key and trust stores, also making it the default.
  • KeyStore instantiation was added to forbidden-apis in favour of KeyStoreFactory.
  • Google's truststore is converted to the BCFKS format.
  • Makes the best guess of which store type is provided based on the filename extension.
  • Store types are strictly limited to JKS, PKCS12, PKCS11, and BCFKS.
  • Refactors PemUtils to parse private keys in formats EC, PKCS8, PKCS1, and DSA, with or without encryption, and with or without parameters.
  • dependency ':libs:opensearch-common' was added to rest-client build, to support strict keystore types. It's also the reason for JVM bump JAVA8 to JAVA11.
  • allow ingest-attachment plugin to run in FIPS mode, since BC dependencies find no use.
  • The java.security file is added to the build to distinguish between FIPS and non-FIPS environments.
  • The fips_java.security file is altered due to evolving security standards.
  • The security.policy file is altered to grant necessary security permissions.
  • Increased security standards in KeyTabs and algorithms for Kerberos.
  • SecureRandom gets instantiated in to different way, depending on if it runs with FIPS or not.
  • Uninstalls SunJCE provider from security providers list at runtime when FIPS mode is enabled.

Runtime limitations (known so far) that come with enabling FIPS mode:

Admins can continue to manage their systems without being impacted by this change. However, for those keen on FIPS compliance, the most common obstacle will likely be the requirement to set a stronger password for the internal keystore and also convert key and truststores to *.bcfks format.

  • Does not allow empty passwords for keystores or private keys (they need to be at least 112 bits in strength).
  • The ssl.verification_mode=NONE setting is not permitted.
  • JKS and PKCS12 key and trust store types are not supported at all.
  • The internal keystore cannot be auto-migrated from versions 1 or 2.
  • Azure Classic Discovery plugin -> deprecated.
  • SQL-CLI client.
  • HDFS plugin won't connect since it's using RC4 cipher for token authentication.

Reasons for refactoring PemUtils, which is used by the Reindex API in cases of migrating data from a remote cluster that is TLS protected:

  • Lack of support for evolving standards like PKCS#8.
  • Password-Based Key Derivation Functions such as PBKDF-OPENSSL are not supported in FIPS mode in favor of the PBKDF2 standard.
  • Java type safety.
  • It is generally a good idea to let ASN1 annotation parsing be done by external security libraries.

Related Issues

opensearch-project/security#3420

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Contributor

❌ Gradle check result for 6016d5d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@beanuwave beanuwave changed the title Draft to allow run in FIPS compliace mode Draft to allow run in FIPS compliance mode Jul 24, 2024
Copy link
Contributor

❌ Gradle check result for 8e8ed47: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 6016d5d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@dblock
Copy link
Member

dblock commented Jul 24, 2024

Could use some help maybe from @cwperks or @peternied reviewing this, please.

Copy link
Contributor

❕ Gradle check result for 951e66d: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link
Contributor

❌ Gradle check result for af76585: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Iwan Igonin <[email protected]>

# Conflicts:
#	server/build.gradle
Signed-off-by: Iwan Igonin <[email protected]>

� Please enter the commit message for your changes. Lines starting
� with '�' will be ignored, and an empty message aborts the commit.
�
� interactive rebase in progress; onto 4b284c5
� Last commands done (2 commands done):
�    pick a47f4e6 Draft to allow run in FIPS compliace mode
�    pick 0bee0a8 make tests run without BC (not BCFIPS) libraries.
� Next commands to do (8 remaining commands):
�    pick 4fc6201 disable approved-only mode for launch configuration of testcluster
�    pick 321929f update all BC libraries to support JAVA 21
� You are currently rebasing branch 'fips_compliance2' on '4b284c54270'.
�
� Changes to be committed:
�	modified:   buildSrc/build.gradle
�	modified:   buildSrc/src/main/java/org/opensearch/gradle/OpenSearchTestBasePlugin.java
�	modified:   buildSrc/src/main/java/org/opensearch/gradle/info/BuildParams.java
�	modified:   client/rest/build.gradle
�	new file:   client/rest/licenses/bc-fips-1.0.2.4.jar.sha1
�	new file:   client/rest/licenses/bctls-fips-1.0.19.jar.sha1
�	new file:   client/rest/licenses/bouncycastle-LICENSE.txt
�	new file:   client/rest/licenses/bouncycastle-NOTICE.txt
�	modified:   client/rest/src/test/java/org/opensearch/client/RestClientBuilderIntegTests.java
�	modified:   distribution/src/config/fips_java.security
�	modified:   distribution/tools/keystore-cli/src/test/java/org/opensearch/common/settings/AddFileKeyStoreCommandTests.java
�	modified:   distribution/tools/keystore-cli/src/test/java/org/opensearch/common/settings/AddStringKeyStoreCommandTests.java
�	modified:   distribution/tools/keystore-cli/src/test/java/org/opensearch/common/settings/ChangeKeyStorePasswordCommandTests.java
�	modified:   distribution/tools/keystore-cli/src/test/java/org/opensearch/common/settings/KeyStoreWrapperTests.java
�	modified:   distribution/tools/keystore-cli/src/test/java/org/opensearch/common/settings/ListKeyStoreCommandTests.java
�	modified:   distribution/tools/keystore-cli/src/test/java/org/opensearch/common/settings/RemoveSettingKeyStoreCommandTests.java
�	modified:   distribution/tools/launchers/src/main/java/org/opensearch/tools/launchers/SystemJvmOptions.java
�	modified:   distribution/tools/plugin-cli/build.gradle
�	modified:   gradle/libs.versions.toml
�	modified:   libs/ssl-config/build.gradle
�	deleted:    libs/ssl-config/licenses/bc-fips-1.0.2.5.jar.sha1
�	new file:   libs/ssl-config/licenses/bouncycastle-LICENSE.txt
�	new file:   libs/ssl-config/licenses/bouncycastle-NOTICE.txt
�	modified:   libs/ssl-config/src/main/java/org/opensearch/common/ssl/DefaultJdkTrustConfig.java
�	modified:   libs/ssl-config/src/main/java/org/opensearch/common/ssl/PemUtils.java
�	modified:   libs/ssl-config/src/test/java/org/opensearch/common/ssl/PemKeyConfigTests.java
�	modified:   libs/ssl-config/src/test/java/org/opensearch/common/ssl/PemTrustConfigTests.java
�	modified:   libs/ssl-config/src/test/java/org/opensearch/common/ssl/PemUtilsTests.java
�	modified:   modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRestClientSslTests.java
�	modified:   modules/transport-netty4/build.gradle
�	modified:   modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportTests.java
�	modified:   modules/transport-netty4/src/test/java/org/opensearch/transport/netty4/ssl/SimpleSecureNetty4TransportTests.java
�	deleted:    modules/transport-netty4/src/test/resources/netty4-secure.jks
�	new file:   modules/transport-netty4/src/test/resources/netty4-secure.p12
�	modified:   plugins/discovery-azure-classic/src/internalClusterTest/java/org/opensearch/discovery/azure/classic/AzureDiscoveryClusterFormationTests.java
�	deleted:    plugins/identity-shiro/licenses/bcprov-jdk18on-1.78.jar.sha1
�	deleted:    plugins/identity-shiro/licenses/bcprov-jdk18on-LICENSE.txt
�	new file:   plugins/identity-shiro/licenses/password4j-1.8.2.jar.sha1
�	new file:   plugins/identity-shiro/licenses/password4j-LICENSE.txt
�	renamed:    plugins/identity-shiro/licenses/bcprov-jdk18on-NOTICE.txt -> plugins/identity-shiro/licenses/password4j-NOTICE.txt
�	modified:   plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/BCryptPasswordMatcher.java
�	modified:   plugins/repository-azure/build.gradle
�	modified:   plugins/telemetry-otel/build.gradle
�	modified:   server/build.gradle
�	new file:   server/licenses/bc-fips-1.0.2.4.jar.sha1
�	new file:   server/licenses/bctls-fips-1.0.19.jar.sha1
�	new file:   server/licenses/bouncycastle-LICENSE.txt
�	new file:   server/licenses/bouncycastle-NOTICE.txt
�	modified:   server/src/main/java/org/opensearch/bootstrap/Bootstrap.java
�	modified:   server/src/main/java/org/opensearch/common/settings/FipsSettings.java
�	modified:   server/src/main/java/org/opensearch/common/settings/KeyStoreWrapper.java
�	modified:   server/src/main/resources/org/opensearch/bootstrap/security.policy
�	modified:   server/src/main/resources/org/opensearch/bootstrap/test-framework.policy
�
Signed-off-by: Iwan Igonin <[email protected]>

# Conflicts:
#	buildSrc/version.properties
Signed-off-by: Iwan Igonin <[email protected]>
Summery:
- replace unsecure kerberos crypto algorithms
- add 'java.security.KeyStore' to forbidden-apis
- instantiate and use SecureRandom from BCFIPS library
- exclude SunJCE from security providers list at runtime, when running in FIPS JVM
- exclude Azure tests when running in FIPS JVM

Signed-off-by: Iwan Igonin <[email protected]>
Copy link
Contributor

✅ Gradle check result for 20a3362: SUCCESS

@terryquigleysas
Copy link

@reta the BCFIPS dependencies are going to leak into (mostly) every single project in the ecosystem (through clients, build tooling or core itself), no matter if BC/FIPS is on their agenda or not

I agree—the BC libraries are essentially everywhere at this point, since they're our primary security provider. Reducing their footprint hasn’t been on our agenda so far, but if you have any suggestions, I’m happy to consider them.

I am just catching up on the status of this again, so apologies if some of this has already been covered.

Do we think it would be worthwhile replacing all the instances of the Bouncy Castle libraries with the FIPS equivalents?
Any functionality no longer available can be replaced using other libs, as we have done with the security plugin and has been done in this PR. This would ensure more of the code paths are covered by existing tests.
(Assumes OpenSAML breaking acceptable for now and that the version 5.x upgrade may not resolve)

Any awkwardness may be around coordinating the separate components, as having the non-FIPS and FIPS jars on the same classpath breaks things. In the interim, again as we've done in the security plugin and referenced by @cwperks above, the code could be abstracted to support both sets of libraries, if required.

In a standard deployment (versions from 2.17), for example, we see the following non-FIPS libs for other components.
/plugins/opensearch-sql/bcprov-ext-jdk18on-1.75.jar
/plugins/opensearch-ml/bcprov-jdk18on-1.78.1.jar
/plugins/opensearch-flow-framework/bcprov-jdk18on-1.78.jar
/plugins/opensearch-performance-analyzer/bcpkix-jdk18on-1.78.1.jar
/plugins/opensearch-performance-analyzer/bcprov-jdk15to18-1.78.1.jar
/plugins/opensearch-security/bcprov-jdk18on-1.78.jar
/plugins/opensearch-security/bcpkix-jdk18on-1.78.jar
/performance-analyzer-rca/lib/bcutil-jdk15to18-1.78.1.jar
/performance-analyzer-rca/lib/bcprov-jdk15to18-1.78.1.jar
/performance-analyzer-rca/lib/bcpkix-jdk15to18-1.78.1.jar

The work we have completed from opensearch-project/security#3420 has allowed us to take a 2.18 deployment, swap in the FIPS equivalents, and run in approved mode with success. (One caveat being that we do not actively use the ML or SQL plugins but heavily use Core functionality and Security.)

@reta
Copy link
Collaborator

reta commented Jan 28, 2025

Any awkwardness may be around coordinating the separate components, as having the non-FIPS and FIPS jars on the same classpath breaks things. In the interim, again as we've done in the security plugin and referenced by @cwperks above, the code could be abstracted to support both sets of libraries, if required.

This great point @terryquigleysas , basically if we expand on that, how could we deal with the mix of FIPS/non-FIPS dependencies outside the plugins managed by OpenSearch Project? Fe, if user wants to run the distribution in FIPS mode with third party plugins installed, could we catch or swap these dependencies?

@terryquigleysas
Copy link

Any awkwardness may be around coordinating the separate components, as having the non-FIPS and FIPS jars on the same classpath breaks things. In the interim, again as we've done in the security plugin and referenced by @cwperks above, the code could be abstracted to support both sets of libraries, if required.

This great point @terryquigleysas , basically if we expand on that, how could we deal with the mix of FIPS/non-FIPS dependencies outside the plugins managed by OpenSearch Project? Fe, if user wants to run the distribution in FIPS mode with third party plugins installed, could we catch or swap these dependencies?

@reta In our container we remove all the non-FIPS libs and include the FIPS versions in the /lib dir only. This works for all the functionality we require.

I would lean towards only using the FIPS libraries throughout and standardizing the versions of those by either shipping them in the lib dir or mandating that plugins should use the FIPS versions declared in Core.

@cwperks
Copy link
Member

cwperks commented Jan 29, 2025

Fe, if user wants to run the distribution in FIPS mode with third party plugins installed, could we catch or swap these dependencies?

In this case should the cluster fail to bootup? If running in FIPS mode and non-FIPS dependencies are on the classpath then I would envision that the cluster would fail to bootup with an error message that lets the cluster administrator know that there are non-compliant dependencies on the classpath

@reta
Copy link
Collaborator

reta commented Jan 29, 2025

In this case should the cluster fail to bootup? If running in FIPS mode and non-FIPS dependencies are on the classpath then I would envision that the cluster would fail to bootup with an error message that lets the cluster administrator know that there are non-compliant dependencies on the classpath

I would expect something along these lines, but it could be too much?

@cwperks
Copy link
Member

cwperks commented Jan 29, 2025

I would lean towards only using the FIPS libraries throughout and standardizing the versions of those by either shipping them in the lib dir or mandating that plugins should use the FIPS versions declared in Core.

If it is completely backward compatible for the default distribution than I think this should be considered as well. There is a mechanism for forbidding dependencies in the core, but I'm not sure if the same check is extended to plugins outside of the core repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.