[improve][client] Seed MessageCryptoBc from the BC-FIPS DRBG when that provider is registered - #26154
Conversation
…ted JVMs Two small fixes for JVMs running a restricted security configuration (e.g. BC-FIPS in approved-only mode): - MessageCryptoBc: when the BCFIPS provider is registered, source randomness for data-key and IV generation from its SP 800-90A DRBG instead of the SUN NativePRNGNonBlocking algorithm, which does not exist in FIPS providers. Only registered providers are consulted so class loading still avoids BouncyCastle classpath resolution. - KeyManagerProxy: fall back to the platform default keystore type when the hardcoded JKS type is unavailable. JKS remains preferred because it accepts certificate lists that do not form a linked chain, which PKCS12 rejects (verified by KeyManagerProxyTest's multiple-CA case).
53ddc42 to
bb1ca41
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves Pulsar client-side TLS/crypto helper compatibility with FIPS-restricted JVM security configurations by avoiding hardcoded SUN-provider assumptions and selecting FIPS-appropriate primitives when the BC-FIPS provider is registered.
Changes:
MessageCryptoBc: PreferSecureRandomfrom the registeredBCFIPSprovider (its DRBG) when available; otherwise keep existingNativePRNGNonBlockingbehavior.KeyManagerProxy: Prefer JKS for the in-memory keystore but fall back toKeyStore.getDefaultType()when JKS is unavailable (e.g., under FIPS restrictions).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pulsar-common/src/main/java/org/apache/pulsar/common/util/KeyManagerProxy.java | Adds JKS → default keystore-type fallback for the in-memory keystore used by the auto-refreshing PEM key manager. |
| pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java | Switches SecureRandom selection to use BC-FIPS DRBG when BCFIPS is registered, preserving non-FIPS behavior otherwise. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ypto Resolves the KeyManagerProxy conflict by taking master's deletion. PIP-478 removed KeyManagerProxy along with the rest of the PIP-337 SSL machinery, and its replacement already covers what this branch changed there: the in-memory carrier keystore is now built through JcaKeyStores, which selects BCFKS then PKCS12 under a pinned JCA provider and never asks for JKS. The JKS-to-default- type fallback therefore has no site left, and nothing outside the deleted file referenced the class. The MessageCryptoBc change is unaffected and still applies: master's static initializer still seeds from NativePRNGNonBlocking unconditionally.
david-streamlio
changed the title
PIP-478 has landed since this proposal was written, and it moved or closed several of the things the Motivation section cites as current state. Reviewers who check the first claim and find it already fixed will discount the rest, so the audit is done against master rather than left to them. SecurityUtility is gone (apache#26322 deleted it with the rest of the PIP-337 TLS stack), and the PIP leaned on it in six places -- the Background paragraph establishing that Pulsar is already FIPS-capable at the JCA layer, the packaging jar-swap rationale, the EC curve allowlist, the fipsMode validator's first assertion, the High Level Design, and the integration test. Each now names JcaProviders, whose ResolvedBouncyCastleProvider(Provider, boolean fips) reports the flavour as a value rather than a static predicate. Its registration-first resolution order is recorded too: a provider an operator registered in java.security is preserved rather than shadowed by a classpath-constructed one, which is the property a FIPS deployment depends on. Motivation item 2 is closed. PulsarSslConfiguration, PulsarSslFactory and DefaultPulsarSslFactory no longer exist; every listener builds from a PulsarTlsFactory, and the broker binary listener carries engineProvider(TlsFactorySupport.engineProvider(conf.getTlsProvider())). Rewritten to what survives -- an unset tlsProvider still resolves to the native engine, because tcnative remains an unconditional pulsar-common dependency, so FIPS mode must pin the engine and drop the jar. Motivation item 3 is stale in form: the declared defaults are now empty rather than the literal Conscrypt, and unset *means* Conscrypt only through resolveWebJsseProvider, on the web listeners. The concern survives; the citations did not. Items 1, 4, 5 and 6 were re-verified and hold, with line numbers corrected. Item 1 gains the version catalog's own statement of the gap (libs.versions.toml:62-63). Adds the gap the PIP did not list: DefaultBrokerTlsFactory resolves its JSSE provider as resolveJsseProvider(null, ...), so the broker's jsseProvider and brokerClientJsseProvider keys are declared but ignored -- item 2's shape in a newer key. apache#26326 fixes it and adds the jcaProvider configuration surface that design (a) assumes, so the dependency is now stated rather than implied. Housekeeping: apache#26152 and apache#26153 are merged, not open; apache#26154 no longer claims the KeyManagerProxy half, which PIP-478 obsoleted; and the Configuration table's four pre-PIP-478 provider rows become two rows naming the keys fipsMode actually evaluates.
|
@lhotari tagging you here because PIP-478 changed the shape of this PR, and the surviving half sits next to your work. It was two changes. The second made What's left is one file, +10/-1: It reads the provider with CI is 44/44 on |
… unavailable The static initializer wrapped both the BCFIPS and the non-FIPS branch in one try, so a NoSuchAlgorithmException from either fell back to new SecureRandom(). On a JVM where BCFIPS is registered but its DEFAULT SP 800-90A DRBG cannot be obtained, that resolves by provider search order and can land outside the validated module, with nothing at run time to indicate it. NIST SP 800-38D only permits a random 96-bit GCM IV from an approved DRBG, so a silent fallback there defeats the reason the branch exists. Split the two paths: - BCFIPS registered and its DRBG unavailable is now fatal. An IllegalStateException naming the provider and the reason, with the original exception as cause, surfaces as ExceptionInInitializerError. Registering BCFIPS is an operator asking for FIPS-approved randomness; refusing to start is safer than starting outside the module. - Without BCFIPS the behaviour is unchanged: NativePRNGNonBlocking, falling back to the platform default where it is not available. No FIPS guarantee was being claimed on that path. Addresses the review comment on the original change.
e9ab7dc to
44d5479
Compare
|
@merlimat @lhotari gentle ping on this one — it has been open a while and I think it is ready for a look. It is small and self-contained now: one file, What it does. When the BC-FIPS provider is registered, seed Copilot raised that the original version still fell back silently when BCFIPS was registered but its DRBG could not be obtained; that is fixed in State: CI green on Happy to adjust the approach if you would rather this were handled differently — and no rush, just did not want it to go stale. |
Motivation
MessageCryptoBcpinsSecureRandom.getInstance("NativePRNGNonBlocking")for data-key and GCM-IV generation. That algorithm does not exist in FIPS providers, and even when the fallback engages, randomness is sourced outside the FIPS-validated module. NIST SP 800-38D permits random 96-bit GCM IVs only when they come from an approved DRBG, so on a JVM running a restricted security configuration (e.g. BC-FIPS in approved-only mode) client-side encryption silently generates keys and IVs with non-approved randomness.This is one of a set of small preparatory cleanups for FIPS-restricted deployments; the broader proposal is #26155.
Modifications
MessageCryptoBc: when aBCFIPSprovider is registered, obtainSecureRandom.getInstance("DEFAULT", bcfips)— the BC-FIPS SP 800-90A DRBG — so key and IV generation stays within the FIPS-validated module. Only registered providers are consulted (Security.getProvider), deliberately avoiding BouncyCastle classpath resolution during class loading, consistent with the existing lazyBcProviderHolderdesign. Behaviour on standard JVMs is unchanged:NativePRNGNonBlocking, falling back tonew SecureRandom().This PR was originally two changes; the second is now obsolete. It also made
KeyManagerProxyfall back fromKeyStore.getInstance("JKS")toKeyStore.getDefaultType()on JVMs where the JKS type is unavailable. PIP-478 has since removedKeyManagerProxyalong with the rest of the PIP-337 SSL machinery, and its replacement already covers the concern: the in-memory carrier keystore is built throughJcaKeyStores, which selectsBCFKSthenPKCS12under a pinned JCA provider and never asks for JKS. That half was dropped when master was merged in, so what remains is the one file above.Verifying this change
Covered by existing tests:
RawBatchMessageContainerImplTestexercisesMessageCryptoBcencryption end to end. TheBCFIPSbranch cannot be exercised without a FIPS-registered JVM; it is compile-verified, and theDEFAULTSecureRandom service is confirmed present inbc-fips2.0.1.Does this pull request potentially affect one of the following parts: