Add FIPS build mode and the *FipsTests/*FipsIT test-selection convention by beanuwave · Pull Request #6394 · opensearch-project/security

beanuwave · 2026-08-13T15:21:21Z

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/, so that bundle's classpath stays FIPS-capable for a manual java -cp 'deps/*' launch (its own launcher has not been self-contained since upstream Copy scripts from tools to bin/ directory in assembly and make scripts agnostic to directory #6023).
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.

github-actions · 2026-08-13T15:22:23Z

PR Code Analyzer ❗

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

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

Path	Line	Severity	Description
build.gradle	606	high	New dependency added: org.bouncycastle:bctls-fips:${versions.bouncycastle_tls} — added to integrationTestImplementation, testImplementation, compileOnly, and bundled into securityadmin standalone artifacts. Per mandatory rule, all dependency additions must be flagged regardless of apparent legitimacy. Maintainers should verify the artifact version and source.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 1 | Medium: 0 | 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.

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>

codecov · 2026-09-16T17:04:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.97%. Comparing base (9d74eb9) to head (ba9578b).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6394      +/-   ##
==========================================
+ Coverage   75.90%   75.97%   +0.07%     
==========================================
  Files         461      462       +1     
  Lines       30942    30970      +28     
  Branches     4668     4674       +6     
==========================================
+ Hits        23485    23530      +45     
+ Misses       5295     5270      -25     
- Partials     2162     2170       +8

Files with missing lines	Coverage Δ
...java/org/opensearch/security/support/FipsMode.java	`100.00% <100.00%> (ø)`

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2026-09-17T11:37:09Z

PR Reviewer Guide 🔍

(Review updated until commit `ba9578b`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

github-actions · 2026-09-17T11:37:30Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Prevent race on mutable static supplier The package-private static `envSupplier` is mutable and shared across all threads/tests, which creates a race hazard and lets any code on the classpath silently flip FIPS-mode detection. Make the production supplier resolution thread-safe (e.g., `volatile`) or, preferably, cache the resolved boolean once at class initialization so runtime queries cannot be perturbed by later env changes. src/main/java/org/opensearch/security/support/FipsMode.java [17-21] -static java.util.function.Supplier<String> envSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); +static volatile java.util.function.Supplier<String> envSupplier = () -> System.getenv("OPENSEARCH_FIPS_MODE"); public static boolean isEnabled() { return "true".equalsIgnoreCase(envSupplier.get()); } Suggestion importance[1-10]: 3 __ Why: The `envSupplier` is only mutated in tests via `@Before`/`@After`, and adding `volatile` provides marginal benefit since production code never modifies it. The suggestion is a minor defensive improvement with low impact.	Low

cwperks

ty for splitting these up @beanuwave ! This looks like a good place to begin with this series of PRs. Will any of these create any new CI checks to run existing tests in FIPS approved mode as a PR check?

reta · 2026-09-17T11:53:39Z

+    }
+
+    private FipsMode() {
+        throw new UnsupportedOperationException();


Wondering why do we need that? The private constructor prohibits class instantiation (https://docs.oracle.com/javase/specs/jls/se7/html/jls-6.html#d5e8146)

Suggested change

throw new UnsupportedOperationException();

You are right, it's unnesseccary since java provides Unsafe.allocateInstance and ReflectionFactory.newConstructorForSerialization and the supplier is the actual attack surface.

reta · 2026-09-17T11:58:09Z

+    @Test
+    public void constructor_isNotInstantiable() throws Exception {
+        Constructor<FipsMode> constructor = FipsMode.class.getDeclaredConstructor();
+        constructor.setAccessible(true);


The code outside the security module should not be able to widen accessibility rules (unless opened explicitly), I think we should be fine without reflection based tests ....

reta · 2026-09-17T12:13:24Z

+ * FIPS mode is driven by the OPENSEARCH_FIPS_MODE environment variable.
+ */
+
+ext.fipsTestsEnabled = 'true'.equalsIgnoreCase(System.getenv('OPENSEARCH_FIPS_MODE'))


Could we reuse FipsBuildParams from OpenSearch build please:

Suggested change

ext.fipsTestsEnabled = 'true'.equalsIgnoreCase(System.getenv('OPENSEARCH_FIPS_MODE'))

import org.opensearch.gradle.info.FipsBuildParams

ext.fipsTestsEnabled = FipsBuildParams.isInFipsMode()

reta · 2026-09-17T12:17:08Z

Thanks @beanuwave , a few minor comments, looks otherwise, same question as #6394 (review) : do we need update test matrix on CI to run fips / non-fips tests?

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 · 2026-09-17T15:33:46Z

Persistent review updated to latest commit ba9578b

beanuwave · 2026-09-17T16:15:31Z

@reta @cwperks Thanks for the review!

GitHub Actions are not part of this series. Looks like it will be a combined effort of extending .github/workflows/ci.yml and the opensearch-build action / integ-test config.

The opensearch-build change is off by default, so it can go in independently. The ci.yml change doesn't really make sense until the whole series is merged - a good reason for a follow-up PR.

reta · 2026-09-17T16:41:53Z

 */

-ext.fipsTestsEnabled = 'true'.equalsIgnoreCase(System.getenv('OPENSEARCH_FIPS_MODE'))
+def fipsBuildParams = rootProject.buildscript.classLoader.loadClass('org.opensearch.gradle.info.FipsBuildParams')


Wow, this is unexpected, the security plugin applies OpenSearch build tooling plugins, why we need that?

iigonin force-pushed the fips-split/1-build-tooling branch 4 times, most recently from 0251173 to a95d231 Compare August 28, 2026 10:43

iigonin force-pushed the fips-split/1-build-tooling branch from a95d231 to d5a4ab4 Compare September 16, 2026 16:28

beanuwave mentioned this pull request Sep 16, 2026

Run the Security plugin under FIPS approved-only mode with BC providers #5866

Open

5 tasks

beanuwave marked this pull request as ready for review September 17, 2026 07:18

beanuwave requested review from DarshitChanpura, Rishav9852Kumar, RyanL1997, cwperks, derek-ho, finnegancarroll, nibix, reta, shikharj05 and willyborankin as code owners September 17, 2026 07:18

cwperks added the skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. label Sep 17, 2026

cwperks reviewed Sep 17, 2026

View reviewed changes

reta reviewed Sep 17, 2026

View reviewed changes

Gate FIPS tests on FipsBuildParams.isInFipsApprovedOnlyMode()

ba9578b

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>

reta reviewed Sep 17, 2026

View reviewed changes

Navigation Menu

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

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

beanuwave commented Aug 13, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 13, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 16, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

cwperks left a comment

Uh oh!

reta Sep 17, 2026 •

edited

Loading

Uh oh!

beanuwave Sep 17, 2026

Uh oh!

reta Sep 17, 2026

Uh oh!

reta Sep 17, 2026

Uh oh!

reta commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

beanuwave commented Sep 17, 2026

Uh oh!

reta Sep 17, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-ext.fipsTestsEnabled = 'true'.equalsIgnoreCase(System.getenv('OPENSEARCH_FIPS_MODE'))
+import org.opensearch.gradle.info.FipsBuildParams
+ext.fipsTestsEnabled = FipsBuildParams.isInFipsMode()

Conversation

beanuwave commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Key changes

Reviewer notes

Testing

Check List

Uh oh!

github-actions Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Analyzer ❗

Uh oh!

codecov Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit ba9578b)

Uh oh!

github-actions Bot commented Sep 17, 2026

PR Code Suggestions ✨

Uh oh!

cwperks left a comment

Choose a reason for hiding this comment

Uh oh!

reta Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beanuwave Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

reta Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

reta Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

reta commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

beanuwave commented Sep 17, 2026

Uh oh!

reta Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

beanuwave commented Aug 13, 2026 •

edited

Loading

github-actions Bot commented Aug 13, 2026 •

edited

Loading

codecov Bot commented Sep 16, 2026 •

edited

Loading

github-actions Bot commented Sep 17, 2026 •

edited

Loading

(Review updated until commit `ba9578b`)

reta Sep 17, 2026 •

edited

Loading

reta Sep 17, 2026 •

edited

Loading