opensearch-project / security

🔐 Secure your cluster with TLS, numerous authentication backends, data masking, audit logging as well as role-based access control on indices, documents, and fields
https://opensearch.org/docs/latest/security-plugin/index/
Apache License 2.0
187 stars 270 forks source link

Make bouncycastle dependency abstracted #1500

Open cjcjameson opened 2 years ago

cjcjameson commented 2 years ago

Is your feature request related to a problem? Please describe.

The problem is that security scans complain about bouncycastle from time-to-time. Perhaps we can upgrade / fix the CVEs, but perhaps we can workaround the security issue by removing bouncycastle

Describe the solution you'd like

@wjcarpenter can give more details, but some abstraction?

Describe alternatives you've considered

We will also follow-up for the specific bouncycastle CVE reports ; cc @nicolasbockg

wjcarpenter commented 2 years ago

Also, if you want to use BouncyCastle's BCFIPS provider for other things in the same environment/classpath, the two providers don't get along well. Working around it is kind of inconvenient.

The Search Guard plugin directly uses org.bouncycastle.crypto.generators.OpenBSDBrcypt from the BC provider, which is not present in the BCFIPS provider.

setiah commented 2 years ago

What is a better alternative to bouncycastle we are considering here, which is less prone to CVE flagging?

Also could you please elaborate a bit more on the issue here

Also, if you want to use BouncyCastle's BCFIPS provider for other things in the same environment/classpath, the two providers don't get along well.

wjcarpenter commented 2 years ago

The CVE factor is just one of a few reasons for wanting to swap out BouncyCastle. For example, an organization might have a policy dictating which Java crypto providers were acceptable for applications they use. One very important reason is the desire to use only FIPS 140-2 certified providers, for which the normal BouncyCastle BC provider is not acceptable. BouncyCastle does have a FIPS 140-2 certified BCFIPS provider, however, it is not possible to use them both on the same classpath. https://github.com/bcgit/bc-java/issues/282 (Like many others, we have tried and seen the wreckage.)

Rather than hard-coding a specific provider in Search Guard plugin, it would be far better IMHO to state the algorithm/keysize requirements so that the consumer can use a Java crypto provide that meets those requirements. There would be no harm in having BC as a default in the Java code if there were some way to configure an override to that default.

wjcarpenter commented 2 years ago

It's probably the case that the majority of consumers are fine with using BC. It's just that when you need to replace it, you're really stuck. There is no workaround other than modifying the Search Guard source code.

setiah commented 2 years ago

Thanks for the details. It makes sense to make the crypto providers configurable (yet with strong/popular defaults). A FIPS 140-2 provider also seems to be a popular feature ask on security plugin side https://discuss.opendistrocommunity.dev/t/opendistro-fips-140-compliance/2103, https://github.com/opensearch-project/security/issues/1497

Since the looks to be on Security plugin side, i'll transfer this issue to the security repo for further follow up.

setiah commented 2 years ago

@CEHENKLE seems like I don't have permissions to transfer issue to security repo. Could you please help with this?

duhang commented 1 year ago

Has this issue been put on OpenSearch Roadmap right? We would really like to see this item being implemented. Many thanks!

peternied commented 1 year ago

@duhang Thanks for your interest, this is not being tracked in any of the security plugin teams backlogs. If you are interested in seeing this happen in a future OpenSearch release I would love review a draft pull request. I've also added the help wanted tag to signal this.