Use BC libraries to parse PEM files, increase key length, allow gener…

[beanuwave]

NOTE: Basically a split up from the original PR

Description

This PR should provide a smoother transition to FIPS 140 standard by eliminating deprecated code, increase security standards and use more standardized approach to parse key files.

• Refactors PemUtils to parse private keys in formats EC, PKCS8, PKCS1, and DSA, with or without encryption, and with or without parameters.
• Elevate exclusion for cryptographic binaries from sub-modules to project level.
• Increase key size for google cloud storage.
• Increased security standards in KeyTabs and algorithms for Kerberos.
• Remove unused BC libraries from ingest-attachment plugin.
• Use KeyStoreUtils to create a keystore at runtime with default certificates to be used in tests.

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.

[github-actions]

❌ Gradle check result for 62d1786: 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?

[github-actions]

❌ Gradle check result for 99b1e83: 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?

[github-actions]

❌ Gradle check result for ac4fa36: 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?

[github-actions]

❌ Gradle check result for 7a837ea: 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?

[github-actions]

✅ Gradle check result for bd18542: SUCCESS

[codecov]

Codecov Report

Attention: Patch coverage is 84.48276% with 9 lines in your changes missing coverage. Please review.

Project coverage is 72.36%. Comparing base (e397903) to head (d4a2bba).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...opensearch/gradle/testclusters/OpenSearchNode.java	0.00%	5 Missing ⚠️
.../main/java/org/opensearch/common/ssl/PemUtils.java	93.02%	2 Missing and 1 partial ⚠️
.../opensearch/common/ssl/SslConfigurationLoader.java	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #17393      +/-   ##
============================================
- Coverage     72.47%   72.36%   -0.11%     
+ Complexity    65693    65636      -57     
============================================
  Files          5304     5304              
  Lines        304981   304744     -237     
  Branches      44238    44190      -48     
============================================
- Hits         221033   220531     -502     
- Misses        65808    66192     +384     
+ Partials      18140    18021     -119     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cwperks]

Thank you for splitting up the PR @beanuwave. I think this is much easier to review at half the number of files of the first PR. @reta when you have some time could you also take a look?

[reta]

Thank you for splitting up the PR @beanuwave. I think this is much easier to review at half the number of files for the first PR. @reta when you have some time could you also take a look?

Indeed, thank you @beanuwave , I is much easier to reason about the change and it looks good to me. Overall, I would like to highlight only once concern here, we are bringing Bouncycastle to core (but to generalize - the whole ecosystem including clients, plugins, extensions, ....) as hard, non optional dependency (as for this change, through libs/opensearch-ssl-config). Is it a good or bad thing? I don't know to be honest, but most (if not all) projects I am aware of do not enforce that (netty is one of those). @andrross @cwperks I think it is up to us to decide is it a way to go.

PS: We've already discussed the same concern on the original pull request #14912

[andrross]

we are bringing Bouncycastle to core (but to generalize - the whole ecosystem including clients, plugins, extensions, ....) as hard, non optional dependency (as for this change, through libs/opensearch-ssl-config). Is it a good or bad thing? I don't know to be honest, but most (if not all) projects I am aware of do not enforce that (netty is one of those).

@reta @cwperks I also don't know the right answer here, but my intuition is that it is probably okay to introduce this dependency. Going through your list of things in the ecosystem: Clients - I think this mainly means the Java high level REST client. I think we need to continue the work to move to opensearch-java where it has no dependency on the server. Plugins - they are already tightly coupled to the server (the run in the same JVM). Bringing this new transitive dependency doesn't seem too problematic to me. Extensions - the long-term goal of extensions should decouple it from the server implementation (including dependencies), but we're a long way from that being real so I don't see this as a show stopper either.

My overall thought is that OpenSearch is not a general purpose library like Netty or Lucene, so we don't need to be quite so conservative with dependencies. If there are concrete examples of how a bouncycastle dependency will cause problems I'd be happy to change my mind though.

[reta]

Thank you @beanuwave , @cwperks @andrross please take a look when you have time, thanks folks!

[github-actions]

❌ Gradle check result for 163ac40: 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?

[github-actions]

❕ Gradle check result for 9d181bd: 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.

[beanuwave]

@reta That's great news! I hope we can merge it once everyone has given their final review. I've just pushed the 1-liner for the policy file, as requested by @cwperks, plus the usual rebase against upstream.

[cwperks]

This change LGTM! Thank you for the persistence @beanuwave!

[github-actions]

❌ Gradle check result for eb8f19d: 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?

[github-actions]

✅ Gradle check result for d4a2bba: SUCCESS

[cwperks]

@beanuwave This is merged now. #14912 can be rebased.