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

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 — see Core / distribution follow-ups at the end of this
    document.

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.

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>
@github-actions

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

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

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

PathLineSeverityDescription
build.gradle600highNew dependency 'org.bouncycastle:bctls-fips' added across multiple configurations (integrationTestImplementation, compileOnly, testImplementation, and bundled into securityadmin archives). Per mandatory flagging rule, all new dependency additions must be flagged for maintainer verification regardless of apparent legitimacy.
src/test/resources/fips-jvm-truststore.bcfks1mediumBinary BCFKS truststore file added. Its contents cannot be inspected in the diff — a rogue or attacker-controlled CA certificate embedded here would silently allow MITM attacks against all TLS connections made by test workers running in FIPS mode. Maintainers should verify the truststore's certificate chain out-of-band.
src/main/java/org/opensearch/security/support/FipsMode.java17mediumThe FIPS mode supplier is a mutable public static field (`public static java.util.function.Supplier envSupplier`). Any code in the JVM can replace it at runtime (e.g., `FipsMode.envSupplier = () -> "false"`) to silently disable FIPS mode detection without touching environment variables or restart. The test-testability intent is clear from FipsModeTest, but the field should be package-private or otherwise access-controlled in production code.

The table above displays the top 10 most important findings.

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


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.

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