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

[RFC] Proposal for supporting FIPS 140-2 enforced mode #3420

Open
terryquigleysas opened this issue Sep 28, 2023 · 24 comments
Open

[RFC] Proposal for supporting FIPS 140-2 enforced mode #3420

terryquigleysas opened this issue Sep 28, 2023 · 24 comments
Labels
enhancement New feature or request triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@terryquigleysas
Copy link
Contributor

terryquigleysas commented Sep 28, 2023

Is your feature request related to a problem? Please describe.
Feature Request #1497

Describe the solution you'd like

Problem Statement

We would like to contribute to OpenSearch to support running in FIPS-140-2 compliant mode. We propose delivering this in several phases, as discussed in the feature request above, starting with core changes and aiming towards a desired state of providing configurable options.

This RFC is to ensure our approach would be seen as a feasible and acceptable contribution.

Phases

Phase 1: Remove hardcoded Bouncy Castle references

Security plugin

Update code, retaining current functionality

  • Proposed libraries: Bouncy Castle FIPS, Password4j, rfksystems Blake2b
    • Alternatively contribute to Password4j to expose Blake2b functionality and reduce the number of libraries brought in
  • Security policy changes
    • Complicated by the plugin structure
  • NB OpenSearch must still work for rolling upgrades

Performance Analyzer (potentially)

  • This codebase is separate from OpenSearch Security and may also lead us to have to make changes to OpenSearch core
  • Who do we liaise with?

Unknown unknowns (e.g. behavior of other plugins, scripts etc.)

  • Emphasis on not inadvertently breaking anything

Phase 2: Introduce FIPS-compliant alternatives as default for:

Bcrypt password hashing

  • PBKDF2

Blake2b for masking

  • e.g. SHA3

Certificate handling (potentially)

Cipher lists (potentially)

Any additional security policy changes

Add FIPS mode configuration flag

  • This may lead us to have to make changes to OpenSearch core

Phase 3: Testing and rework

By now we will be carrying out extensive testing and verification and expect that additional requirements may arise.

Additional work for any issues found in our testing

Extend unit tests

Extend integration tests

Phase 4: Configurability

Additional configuration options

  • Configure additional security providers
  • Configure hashing algorithms
  • Validation

Contingency for unknown unknowns

Phase 5: Documentation

All required configuration options and settings

JDK 11 requirement

Limitations

Not in scope

Changing an existing cluster from non-FIPS to FIPS compliant

Dashboards, Data Prepper etc. - our focus is on server only

Any, as yet unknown, OpenSearch plugins that require extensive work for FIPS-compliance

These could be actioned by the wider community

Help Required

We have accessed and used:

  • YouTube videos on developing and contributing
  • Documentation and GitHub pages
  • Blog items
  • Slack

We expect we will need some additional help with:

  • Processes
    • Proposal example
    • Creation of project stories / issues / epics / labels?
    • Backporting and releasing
    • Documentation
    • Tests
      • How do we ensure we don't break something unexpectedly?
      • Benchmarking
  • Wider impact analysis
  • Anything else we haven't thought of
@terryquigleysas terryquigleysas added enhancement New feature or request untriaged Require the attention of the repository maintainers and may need to be prioritized labels Sep 28, 2023
@davidlago davidlago removed the untriaged Require the attention of the repository maintainers and may need to be prioritized label Oct 2, 2023
@stephen-crawford
Copy link
Contributor

stephen-crawford commented Oct 9, 2023

[Triage] Hey @terryquigleysas, just checking in. I know you were hoping to sync up in a triaging meeting. Could you suggest a date you may be able to attend and we can make sure to prepare a slot to discuss this?

@terryquigleysas
Copy link
Contributor Author

@scrawfor99 Sounds good. I can catch up with you on Slack to confirm.

@peternied
Copy link
Member

Thanks for bringing up this proposal @terryquigleysas - overall the plan seems solid, but I think the details are where this project will hinge have to do with 1) making iterative improvements for updates, 2) the security approval process and 3) the project management of this release.

  1. Above you've called out that there is Proposed libraries: Bouncy Castle FIPS, Password4j, rfksystems Blake2b, this seems like it might be breakable into 3 different parts - or 3 separate pull requests. I think starting with figuring out how to sub in a different Bouncy Castle implementation would be a very manageable change that builds towards the ultimate goal. I think there might be many small parts that are composed up into the ultimate objective to have a fips switch that you can turn on,

  2. The maintainers of security can help with this - before we get to the end state including this feature with the generally available releases there will need to be a more in-depth review. After an minimal viable feature is available we can see about more detailed analysis.

  3. Lastly, I've got notions about project planning and I might suggestion looking at the structure of one of the other [Meta] issues in the project, for a way to pivot the project that would help determine viability and then iterate through maturity. Starting with design / proof of concepts, then implementation, then integration, release criteria, and follow up improvements.

Let us know how I can help, and I can't wait to see some pull requests. Feel free to @mention me throughout the project.

@stephen-crawford
Copy link
Contributor

@terryquigleysas are you still interested in pursuing this change? Thank you

@stephen-crawford
Copy link
Contributor

[Triage] Going to close this. Please reopen if further work continues on this effort.

@ihendry2
Copy link

Hi we (@terryquigleysas and our team) are still interested in this change and are actively pursuing it.
At present locally we are at phase 2 of the above plan and are having issues at the integration stage.
We are mindful of your suggestions and are currently trying to work within those parameters as well.

We have also been held up with other various issues.

As such can you re-open this RFC and we will update it when we have more detail.

@davidlago
Copy link

Thanks @ihendry2. I did not see any action on this issue... where is the work on that phase 1/2 happening? Could you please link the artifacts (issues/PRs) here so that we can get the right level of engagement from the maintainer team? Thanks!

@ihendry2
Copy link

ihendry2 commented Dec 6, 2023

At present the changes are being worked on locally. When we have something viable we will update asap.

@terryquigleysas
Copy link
Contributor Author

@scrawfor99 @davidlago A couple of major, unexpected events have got in the way of my looking at this among other things. I expect to pick it up in the new year and hopefully others can be brought on board.
One question in the meantime - are there still issues where settings added to the policy file can be ignored? We are seeing this as we bring in new libraries against the 2.11 code? (For example #3213)

@stephen-crawford
Copy link
Contributor

Hi @terryquigleysas, the issue you linked was specifically a problem with the way that Bouncy Castle was being used. Since it was applied twice one setting would take priority over the other.

@terryquigleysas
Copy link
Contributor Author

terryquigleysas commented Dec 22, 2023

@scrawfor99 Thanks for the reply. We are also seeing issues on adding password4j. For example it has a static initializer to check a property - this requires additional permissions. When added to the security plugin policy file they have no effect. The same permissions added to the JDK policy file are honoured. Is that the expected behaviour?

@terryquigleysas
Copy link
Contributor Author

@scrawfor99 @davidlago I hope you all had a great break.
Just to follow up on my question from before:
"We are also seeing issues on adding password4j. For example it has a static initializer to check a property - this requires additional permissions. When added to the security plugin policy file they have no effect. The same permissions added to the JDK policy file are honoured. Is that the expected behaviour?"

@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Jan 29, 2024
@stephen-crawford stephen-crawford added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Jan 29, 2024
@kaimst
Copy link

kaimst commented Feb 5, 2024

Hello @terryquigleysas @peternied @scrawfor99,
We are really interested in seeing support for FIPS 140-2 in OpenSearch and we're curious if there's a timeline for the roadmap outlined above.
Also, we'd be happy to contribute. Any updates you could provide would be greatly appreciated.

@stephen-crawford
Copy link
Contributor

HI @kaimst, thanks for reaching out. As of now, the security maintainers have not invested any time into implementing FIPS enforced mode. However, @terryquigleysas and some of the engineers at his company have put time into setting up the first couple steps of their plan. Perhaps Terry can provide additional info on their efforts and sync on any contributions towards the effort you may be able to provide. Thanks all!

@terryquigleysas
Copy link
Contributor Author

@kaimst The stage we are at right now is putting together a proof of concept for our internal use. We haven't been able to devote the time we'd like recently but hopefully that is changing and we will have something ready soon. When we do we will report back with any additional findings.

The pain points we have seen so far are mostly around a) how the FIPS/non-FIPS Bouncy Castle libraries behave across core and some plugins b) Java security policies, again across core and plugins.

@kaimst
Copy link

kaimst commented Feb 7, 2024

@scrawfor99 @terryquigleysas
Thanks for the info !
If there is any way we can support or contribute to your work on the FIPS implementation please let me know.

@terryquigleysas
Copy link
Contributor Author

I have posted some proof of concept code to favour FIPS-compliant options 2.11...terryquigleysas:security:2.11

To support this we have also altered our test deployments to:

  • Exclude current Bouncy Castle jars from core and plugins (of patterns "bcpkix-jdk15*","bcprov-jdk15to18*","bcprov-ext-jdk18on*")
  • Include Bouncy Castle FIPS jars in $installation_dir/lib
  • Force application of -Djava.security.policy=$plugins_dir/opensearch-security/plugin-security.policy - we are seeing unexpected, likely buggy, behaviour in this area
  • Ensure the Java environment is configured for FIPS. Sample security file:
fips.provider.1=org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider
fips.provider.2=org.bouncycastle.jsse.provider.BouncyCastleJsseProvider fips:org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider
fips.provider.3=SUN
fips.provider.4=SunEC
fips.provider.5=SunJSSE
fips.provider.6=SunJCE
fips.provider.7=SunRsaSign
fips.provider.8=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg
fips.keystore.type=jks
ssl.KeyManagerFactory.algorithm=PKIX
ssl.TrustManagerFactory.algorithm=PKIX
securerandom.strongAlgorithms=NativePRNGBlocking:SUN,DRBG:SUN,PKCS11:SunPKCS11-NSS-FIPS

@willyborankin
Copy link
Collaborator

I would call settings not fips.provider but rather security.provider wdyt?

@terryquigleysas
Copy link
Contributor Author

@willyborankin We use the fips namespace on our deployments. It's an example not something that would be included in any of our commits.
Elasticsearch 7.10 also uses security.provider as you suggest, it would be up to the owner of the environment https://github.com/elastic/elasticsearch/blob/7.10/buildSrc/src/main/resources/fips_java_bcjsse_11.security

@KarstenSchnitter
Copy link

SAP would contribute to this effort over @kaimst and the SAP Cloud Logging team. Is there a way to create and implement a roadmap?

@peternied
Copy link
Member

@KarstenSchnitter We would welcome that effort. I'm not sure what you mean about a roadmap, do you mean on this project board [1] or as part of the community projects [2]?

If you were looking to have a feature slated on the roadmap, I'd recommend creating a meta issue that encapsulates what you are looking for, such as the following:

I've got notions about project planning and I might suggestion looking at the structure of one of the other [https://github.com//issues/2573] issues in the project, for a way to pivot the project that would help determine viability and then iterate through maturity. Starting with design / proof of concepts, then implementation, then integration, release criteria, and follow up improvements.

From @peternied in #3420 (comment)

I'd recommend working iteratively to get faster turn around on feedback, please ping me (Peter Nied) on our slack instance if there are more specific questions that I can get you answers to.

@terryquigleysas
Copy link
Contributor Author

A quick update on this.

Subsequent discussions, based on findings as we were implementing our internal proof of concept (sample code above), and preferred approaches suggested by OpenSearch mean we are proceeding as follows.

One of the key changes was the request to move away from a single binary FIPS mode flag as requested in #1497. The favoured approach is now to add more configurability to control the relevant areas.

This has the benefit of us being able to gradually introduce changes, for example, to remove problematic Bouncy Castle references or add options to configure FIPS-compliant algorithms. Please refer to current and future issues that we are linking to this RFC.

There are still challenges, not least around the handling of the Bouncy Castle libraries. More thoughts on an approach for #1500 would be most welcome.

@mmomjya
Copy link

mmomjya commented Aug 20, 2024

@terryquigleysas In the proposal Phase 1 I see Password4j library as a replacement for BC BCrypt algorithm. However I didn't find any document that states that Password4j is a FIPS certified. Are you sure on that. Is it FIPS certified or not?

@dancristiancecoi
Copy link
Contributor

dancristiancecoi commented Aug 20, 2024

@terryquigleysas In the proposal Phase 1 I see Password4j library as a replacement for BC BCrypt algorithm. However I didn't find any document that states that Password4j is a FIPS certified. Are you sure on that. Is it FIPS certified or not?

@mmomjya The main consideration for moving to Password4j was the ability to use PBKDF2, which is considered FIPS compliant as long as the underlying hashing algorithm it uses is approved for FIPS (e.g. HMAC-SHA-256). Blowfish (which is used by BCrypt) was never approved. Based on the documentation, Password4j does not have its own implementations of HMAC-SHA-* algorithms but relies on what is provided by the JCA.

As for FIPS certification, I suspect that Password4J is not certified because that is an extremely lengthy and costly process. However as long as we use the right underlying algorithm we should be compliant.

Disclaimer: we are not security experts and this is simply our understanding as developers. We plan on doing further validation and testing ( e.g running OpenSearch on a RHEL 8 FIPS system that will block non-FIPS approved crypto ) before we finish with this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

No branches or pull requests

10 participants