Skip to content

Add FIPS build mode and the *FipsTests/*FipsIT test-selection convention - #6394

Draft
beanuwave wants to merge 1 commit into
opensearch-project:mainfrom
sternadsoftware:fips-split/1-build-tooling
Draft

Add FIPS build mode and the *FipsTests/*FipsIT test-selection convention#6394
beanuwave wants to merge 1 commit into
opensearch-project:mainfrom
sternadsoftware:fips-split/1-build-tooling

Conversation

@beanuwave

@beanuwave beanuwave commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Description

Category: Enhancement

Establishes how FIPS mode is decided and applied, with no behaviour change to the
default build. Nothing in this PR turns FIPS on for users; it makes FIPS a build
and test mode the rest of the stack can rely on.

Key changes

  • Single source of truth. FipsMode.isEnabled() (env OPENSEARCH_FIPS_MODE=true)
    replaces CryptoServicesRegistrar.isInApprovedOnlyMode() as the way the plugin
    asks "are we in FIPS mode". Intent is decoupled from provider state; later PRs
    cross-check the two at startup.
  • BCFIPS is always built. compileOnly in both modes (the jars are shipped by
    core); the compile-time FipsBuildParams fork is gone. bctls-fips is added,
    and the securityadmin standalone bundles ship the BC FIPS jars under deps/.
  • FIPS engages through java.security, not code. No provider is registered
    programmatically. The runtime launcher merges in fips_java.security when
    OPENSEARCH_FIPS_MODE=true; tests swap the file wholesale via
    -Djava.security.properties==<file>.
  • gradle/fips.gradle is the single place where the mode is decided and
    applied to the test surface: which classes exist in each mode, and the JVM args
    the workers need. Keeping both together matters — selecting the FIPS classes
    without the matching JVM args produces a suite that looks FIPS-shaped but runs
    on ordinary providers.
  • Test convention. A FIPS variant is <Base>FipsTests (unit) or <Base>FipsIT
    (integration): it extends its baseline, overrides what the FIPS providers
    change, and adds FIPS-only assertions. The two modes select disjoint sets —
    outside FIPS the variants are excluded from compilation and from every Test
    task; inside FIPS each baseline that has a variant is excluded so the variant
    replaces it. Baselines therefore stay free of FipsMode.isEnabled() branches.
    This follows core's convention (gradle/fips.gradle there).
  • BCFipsEntropyDaemonFilter. Because java_test.security registers
    BouncyCastleFipsProvider in every test JVM including non-FIPS runs, any suite
    touching JCA leaves a "BC FIPS Entropy Daemon" thread behind, which
    RandomizedRunner reports as a leak. The framework's BouncyCastleThreadFilter
    does not cover it. Applied to the suites that trip over it here and reused by
    later PRs.

Reviewer notes

  • This PR lands inert: no variant classes exist yet, so fips.gradle selects
    nothing and the default build is unchanged. The machinery is exercised from
    PR 5 onward.
  • BCFipsEntropyDaemonFilter arguably belongs in core's test framework rather
    than this plugin.

Testing

./gradlew test integrationTest                              # unchanged
OPENSEARCH_FIPS_MODE=true ./gradlew test integrationTest    # runs, no variants yet

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • Commits are signed per the DCO using --signoff

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

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit a95d231.

Hard block: Issues at High severity or above will block this PR from merging.

PathLineSeverityDescription
build.gradle601highNew dependency 'org.bouncycastle:bctls-fips:${versions.bouncycastle_tls}' added in multiple scopes (integrationTestImplementation, compileOnly, testImplementation, and detachedConfiguration). Per mandatory flagging rule, all dependency additions must be flagged regardless of apparent legitimacy — maintainers must verify the artifact matches the expected BouncyCastle FIPS TLS library.
src/test/resources/fips-jvm-truststore.bcfks1mediumA new binary BCFKS truststore file is added to source. Binary content is opaque in the diff and cannot be inspected for embedded payloads or unexpected certificates beyond what is described. Maintainers should verify the file's contents match the expected CA certificate set for FIPS test JVM trust.
gradle/fips.gradle86lowHardcoded truststore password 'changeit' is passed as a JVM argument to test workers via -Djavax.net.ssl.trustStorePassword=changeit. Acceptable for a test-only truststore, but if this truststore were ever used in non-test contexts the credential would be exposed in process listings.
src/main/java/org/opensearch/security/support/FipsMode.java17lowThe static field 'envSupplier' has package-private (default) access rather than private, allowing any class in the same package to replace the FIPS mode detection logic at runtime. While intentional for test injection, it means production code's FIPS detection can be silently overridden without reflection.

The table above displays the top 10 most important findings.

Total: 4 | Critical: 0 | High: 1 | Medium: 1 | Low: 2


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@iigonin
iigonin force-pushed the fips-split/1-build-tooling branch 3 times, most recently from 06bb65f to 0251173 Compare August 27, 2026 16:14
Introduces gradle/fips.gradle as the single place where FIPS mode is decided
and applied to the build's test surface: which test classes exist in each mode
and the JVM arguments test workers need to actually be in FIPS mode. Mode is
driven by the OPENSEARCH_FIPS_MODE environment variable, surfaced to production
code through the new FipsMode enum.

BC FIPS artifacts move to compileOnly in both modes (they are provided by
OpenSearch core), bctls-fips is added, and the securityadmin standalone bundles
now ship the BC FIPS jars in deps/.

Because java_test.security registers BouncyCastleFipsProvider in every test JVM
-- including non-FIPS runs -- any suite that touches JCA now leaves a
"BC FIPS Entropy Daemon" thread behind, which RandomizedRunner reports as a
leak. BCFipsEntropyDaemonFilter covers it; the framework's BouncyCastleThreadFilter
does not. It is applied to the suites that trip over it here, and reused by
later FIPS work.

No FIPS variant test classes exist yet, so this otherwise lands inert: the
default build is unchanged and fips.gradle currently selects nothing.

Signed-off-by: Iwan Igonin <iigonin@sternad.de>
Co-authored-by: Benny Goerzig <benny.goerzig@sap.com>
Co-authored-by: Karsten Schnitter <k.schnitter@sap.com>
Co-authored-by: Kai Sternad <k.sternad@sternad.de>
@iigonin
iigonin force-pushed the fips-split/1-build-tooling branch from 0251173 to a95d231 Compare August 28, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants