camel-quarkus icon indicating copy to clipboard operation
camel-quarkus copied to clipboard

Extracion of crypto-pgp and making crypto work on FIPS

Open JiriOndrusek opened this issue 1 year ago • 3 comments

fixes https://github.com/apache/camel-quarkus/issues/6088

Crypto component was split into crypto and crypto-pgp to support FIPS. See the commit. (this is the reason of why this PR is opened against camel-main)

  • crypto-pgp is not registering BC as a provider; component contains only PGPDataFormat
  • crypto contains all other (CryptoDataFormat, components)

How to run crypto on FIPS the BC dependency has to be excluded and BCFIPS added (or another BC implementation for FIPS) .

Limitation: Because of the BCFIPS, it is not possible to use crypto and crypto-pgp together on FIPS system (if BCFIPS is utilized)

This PR contains:

  • new extension based on crypto-pgp component (uses BC). The component and all tests are extracted from the crypto component. All tests work on FIPS system.
  • fixes in bouncycastle-support to work correctly on FIPS system
  • added fips profile in crypto integration test module, to allow execution on FIPS systems
  • some *.adoc addition for FIPS in crypto and crypto-pgp etensions
  • it is not possible to utilize certificate-generator-support because the tests require DES and the certificate-generator-support uses RSA.

JiriOndrusek avatar Jun 28 '24 14:06 JiriOndrusek

@ppalaga FYI

JiriOndrusek avatar Jun 28 '24 14:06 JiriOndrusek

The failure seems merely linked to uncommitted changes.

aldettinger avatar Jul 01 '24 07:07 aldettinger

That's quite a pr. Many thanks for transparently explaining the intent in the first note and putting so much useful comment in the code. It really helps one without much fips knowledge to review :)

aldettinger avatar Jul 01 '24 07:07 aldettinger

@ppalaga

It is still not clear to me what happens when bcprov is excluded from crypto-pgp and replaced with BCFIPS. Have you tried that by any chance?

There might be more problems related to this question. The major one is that bcpg depends on bcprov. Class BcKeyFingerprintCalculator references org.bouncycastle.crypto.Digest; The same class is not part of the bcfips. (I checked the jar downloaded by maven, and you can see it e.g in this fork)

Therefore it is not possible to replace bcprov with bcfips.

  • The only workaround I'm aware of is to have a different bouncycastle fips implementation, which uses different packages. In this case the bcprov and custom_bcfips can coexist in the project and should work together)
  • For illustration, the bcprov package org.bouncycastle contains 11 sub-packages; bcfip's org.bouncycastle contains 2 sub-packages. (So I expect more possible issues.)

JiriOndrusek avatar Jul 08 '24 13:07 JiriOndrusek

@JiriOndrusek The ci failure seems linked to needed regeneration, beyond looks good

Yes, I forgot to regenerate POMS when I was maintaining camel-main, hopefully now it is ok

JiriOndrusek avatar Jul 08 '24 14:07 JiriOndrusek

@ppalaga

It is still not clear to me what happens when bcprov is excluded from crypto-pgp and replaced with BCFIPS. Have you tried that by any chance?

There might be more problems related to this question. The major one is that bcpg depends on bcprov. Class BcKeyFingerprintCalculator references org.bouncycastle.crypto.Digest; The same class is not part of the bcfips. (I checked the jar downloaded by maven, and you can see it e.g in this fork)

Therefore it is not possible to replace bcprov with bcfips.

Good to know, thanks for the information!

ppalaga avatar Jul 09 '24 11:07 ppalaga

I switched this PR to draft and I'll rebase to main, as soon as Upgrade to Camel 4.7.0 is merged (https://github.com/apache/camel-quarkus/pull/6264)

JiriOndrusek avatar Jul 10 '24 08:07 JiriOndrusek

@ppalaga I applied the idea of crypto not needing BC. • I excluded BC from camel-crypto (until getting change from Camel) via BOM • User has to add dependency to BCFIPS and quarkus-security and register BCFIPS provider to make crypto work on FIPS -> I described that in README.adoc • the tests for crypto extension got simplified

JiriOndrusek avatar Jul 11 '24 09:07 JiriOndrusek

The crypto/crypto-pgp changes are present in Camel 3.7.0. I rebased this PR against main and I think that it could be approved/merged. All the questions and suggestions are answered or fixed.

JiriOndrusek avatar Aug 08 '24 14:08 JiriOndrusek

@ppalaga WDYT? Can we merge this PR?

JiriOndrusek avatar Aug 12 '24 07:08 JiriOndrusek