Add bcFips dependencies as server dependencies for single distribution that is FIPS capable #20400
Conversation
…n that is FIPS capable Signed-off-by: Craig Perkins <cwperx@amazon.com>
📝 WalkthroughWalkthroughReplaces runtime bc-fips JAR detection with environment-driven FIPS flags; moves BouncyCastle to Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI / Docker Compose
participant Image as Container Image
participant Entrypoint as docker-entrypoint.sh / opensearch-env
participant FIPS as FIPS installer/tasks
CI->>Image: start container with RUN_FIPS_INSTALLER=true
Image->>Entrypoint: container startup executes entrypoint scripts
Entrypoint->>Entrypoint: read RUN_FIPS_INSTALLER / OPENSEARCH_FIPS_MODE
alt FIPS enabled
Entrypoint->>FIPS: run FIPS setup (truststore/keystore, java.security.properties)
FIPS-->>Entrypoint: FIPS artifacts/configured
end
Entrypoint->>Image: continue normal startup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
🔇 Additional comments (1)
Comment |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/build.gradle (1)
134-136: Remove redundant testFipsRuntimeOnly bouncycastle dependencies.The bundle
libs.bundles.bouncycastledeclared asapiat line 81 already includesbc-fips,bctls-fips, andbcutil-fips. These are automatically available on the test runtime classpath, and the FIPS transitive dependency control is applied at the configuration level ingradle/fips.gradle. The explicittestFipsRuntimeOnlydeclarations at lines 134–136 are redundant and can be removed.
🤖 Fix all issues with AI agents
In @server/build.gradle:
- Line 81: The build file currently exposes BC FIPS via the public API by using
"api libs.bundles.bouncycastle" which makes BouncyCastle classes part of the
server module’s public contract; if that exposure is not intentional, change the
dependency to "implementation libs.bundles.bouncycastle" so BC FIPS is internal
to the module, and then review the testFipsRuntimeOnly entries (referenced as
testFipsRuntimeOnly at lines 134–136) to remove or consolidate them if they
become redundant due to the changed scope; if API exposure is intentional, add a
comment in the build.gradle explaining why BC FIPS must be public and keep the
testFipsRuntimeOnly entries only if they serve a separate runtime test classpath
purpose.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.mddistribution/build.gradleserver/build.gradle
💤 Files with no reviewable changes (1)
- distribution/build.gradle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: gradle-check
- GitHub Check: Analyze (java)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: detect-breaking-change
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
🔇 Additional comments (1)
CHANGELOG.md (1)
24-24: LGTM - Changelog entry is clear and appropriately placed.The changelog entry accurately describes the change. While the wording starts with "Add," placing it under the "Changed" section is appropriate since this modifies how dependencies are distributed rather than adding net-new functionality to OpenSearch.
|
❌ Gradle check result for d49db17: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
server/build.gradle
Outdated
| testRuntimeOnly project(':libs:opensearch-plugin-classloader') | ||
|
|
||
| api libs.bundles.lucene | ||
| api libs.bundles.bouncycastle |
There was a problem hiding this comment.
I believe we definitely should not include those: server is used by mostly (if not all) any single plugin out there, no matter if it has security related features or dependencies. Making it deployment only (as it is now) only complicates a few (like security, and probably that's about it). It introduces compile time dependency to BC where there is no such need at all (at least for now).
<dependency>
<groupId>org.bouncycastle</groupId>
<artifactId>bc-fips</artifactId>
<version>2.1.2</version>
<scope>compile</scope>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
There was a problem hiding this comment.
Won't that necessarily mean 2 separate distributions? One of the problems is that the security plugin cannot be installed in a distro built with -Pcrypto.standard=FIPS-140-3.
If we plan for a single distro, then what would the script from opensearch-project/technical-steering#77 (comment) do? Would that move jars around?
There was a problem hiding this comment.
I believe we could have one distribution (if that is the goal) but without introducing hard dependencies
There was a problem hiding this comment.
That is the goal. A single distribution that is FIPS capable. Simplest solution would be to always include the BCFIPS jars.
We do have a FIPS installer script to help prepare a cluster for running in FIPS approved mode.
There was a problem hiding this comment.
That is the goal. A single distribution that is FIPS capable. Simplest solution would be to always include the BCFIPS jars.
So let do that while building distribution, we don't need to introduce hard dependencies on component(s), those do have much larger impact on downstreams
There was a problem hiding this comment.
I don't see how we can avoid not having a separate distribution that we need to publish as a project then.
Currently we have 2 distros:
- min distro
- default distro (includes default package of plugins)
We should have FIPS variant of both:
- min distro FIPS (built with -Pcrypto.standard=FIPS-140-3)
- default distro FIPS (where all plugins are also built with -Pcrypto.standard=FIPS-140-3)
There was a problem hiding this comment.
I don't see how we can avoid not having a separate distribution that we need to publish as a project then.
Apologies if I am missing something, but I think we are mixing two separate issues here:
- Dependencies: to unify distribution(s), we could always include FIPS related jars when building min or default distro by just copying them during distribution builds (I suspect this is what this pull request tried to do by elevating FIPS to become mandatory dependency to one of the modules).
- Or, alternatively, if we target only FIPS-140-3 as the end goal, we could make -Pcrypto.standard=FIPS-140-3 a default - that would always produce a single flavor of the distribution as well, including all the JVM related flags
There was a problem hiding this comment.
for #2, how will dependency management work? Currently, it copies BCFips jars under lib/ when creating the distribution when running with -Pcrypto.standard=FIPS-140-3. How do downstream plugins know that those are available upstream? I suppose it is only the bcfips deps so plugins can update build logic accordingly, but that being said I don't see a big difference between 1 and 2, esp if the suggestion for 2) is to run with Pcrypto.standard=FIPS-140-3 by default.
There was a problem hiding this comment.
How do downstream plugins know that those are available upstream?
This will come with the distribution - and would require plugins to use the proper scope (runtime), which is fine. I suspect -Pcrypto.standard=FIPS-140-3 would also require JVM arguments to use approved only algorithms (if I am not mistaken).
There was a problem hiding this comment.
it just seems like inverse of the current situation. i.e. we switch to default to building with -Pcrypto.standard=FIPS-140-3 so if you don't want the BCFIPS deps then you can build from src/ w/o passing -Pcrypto.standard=FIPS-140-3 and we still maintain the 2 distros: 1) min and 2) default.
I think it makes sense to have these deps available by default, but also happy to instruct anyone making changes in plugins to take -Pcrypto.standard=FIPS-140-3 into account and ensure to treat bcfips deps as runtime deps when that flag is passed.
Signed-off-by: Craig Perkins <cwperx@amazon.com>
|
If BC libraries are always present on the classpath, this logic OpenSearch/distribution/docker/src/docker/bin/docker-entrypoint.sh Lines 76 to 78 in 375ad87 |
Is there an env var we can rely on there? |
No, there isn’t one currently. However, an environment variable could be introduced, similar to |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @libs/common/build.gradle:
- Line 29: The change adds a third-party BouncyCastle dependency with public
exposure via the api configuration (the line "api libs.bundles.bouncycastle")
which violates the project's "NO THIRD PARTY DEPENDENCIES" policy and the
existing usage patterns that restrict BC to FIPS/test scopes; either remove this
api dependency from libs/common or replace it with a more restrictive
configuration consistent with existing symbols (e.g., use fipsRuntimeOnly,
testFipsRuntimeOnly or bcFips instead of api), and update or remove the policy
comment block (the NO THIRD PARTY DEPENDENCIES note around lines 31-33) to
reflect the intentional policy change if you truly intend to allow it.
🧹 Nitpick comments (1)
libs/common/build.gradle (1)
56-58: License mapping configuration looks appropriate.The
dependencyLicensesconfiguration correctly maps BouncyCastle artifacts to the 'bouncycastle' license. The pattern/bc.*/appropriately captures all BC artifacts (bc-fips, bcpg-fips, bcpkix-fips, bctls-fips, bcutil-fips), though it is somewhat broad and could theoretically match other artifacts starting with "bc".If you want to be more specific, consider a more restrictive pattern:
Optional: More specific pattern
tasks.named("dependencyLicenses").configure { - mapping from: /bc.*/, to: 'bouncycastle' + mapping from: /bc(pg|pkix|tls|util)?-fips/, to: 'bouncycastle' }However, the current pattern is acceptable and more maintainable if BC adds new artifacts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
libs/common/build.gradlelibs/common/licenses/bc-fips-2.1.2.jar.sha1libs/common/licenses/bcpg-fips-2.1.11.jar.sha1libs/common/licenses/bcpkix-fips-2.1.9.jar.sha1libs/common/licenses/bctls-fips-2.1.20.jar.sha1libs/common/licenses/bcutil-fips-2.1.4.jar.sha1libs/common/licenses/bouncycastle-LICENSE.txtlibs/common/licenses/bouncycastle-NOTICE.txtlibs/ssl-config/build.gradleserver/build.gradle
💤 Files with no reviewable changes (1)
- libs/ssl-config/build.gradle
🚧 Files skipped from review as they are similar to previous changes (1)
- server/build.gradle
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: gradle-check
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: detect-breaking-change
- GitHub Check: Analyze (java)
- GitHub Check: Mend Security Check
| // This dependency is used only by :libs:core for null-checking interop with other tools | ||
| compileOnly "com.google.code.findbugs:jsr305:3.0.2" | ||
|
|
||
| api libs.bundles.bouncycastle |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the version catalog or gradle files that define the bouncycastle bundle
# 1. Search for gradle files in the root and common gradle configs
find . -name "*.gradle" -o -name "*.toml" | head -20
# 2. Look for bouncycastle bundle or version definitions
echo "=== Searching for bouncycastle definitions ==="
rg -i "bouncycastle" --type gradle --type toml -B2 -A2
# 3. Check server/build.gradle for BC dependencies
echo "=== Checking server module ==="
if [ -f "server/build.gradle" ]; then
head -100 server/build.gradle | rg -i "bouncycastle|libs.*common" -B2 -A2
fi
# 4. Check the actual libs/common/build.gradle around line 29-33
echo "=== Viewing libs/common/build.gradle context ==="
if [ -f "libs/common/build.gradle" ]; then
sed -n '20,60p' libs/common/build.gradle
fiRepository: opensearch-project/OpenSearch
Length of output: 21231
🏁 Script executed:
#!/bin/bash
# Search for version catalog definitions (usually in gradle/libs.versions.toml)
find . -name "libs.versions.toml" -o -name "versions.toml" | xargs cat 2>/dev/null || echo "No version catalog found"
# Also check for gradle.properties or other gradle config files
ls -la gradle/ 2>/dev/null || echo "No gradle directory"Repository: opensearch-project/OpenSearch
Length of output: 11597
🌐 Web query:
Are there any known security vulnerabilities in BouncyCastle FIPS versions including bc-fips 2.1.2 or related FIPS libraries?
💡 Result:
Short answer: Yes — several CVEs were disclosed affecting Bouncy Castle FIPS 2.1.0/2.1.x; most were fixed in the 2.1.1/2.1.2 patch releases. You should upgrade to the latest bc-fips release (≥2.1.2) or the current vendor-recommended patch.
Details (high-level):
- CVE-2025-9340 — out‑of‑bounds write in BaseCipher; fixed in 2.1.1. [1][2]
- CVE-2025-9341 — uncontrolled resource consumption (excessive allocation) in AESNativeCBC; fixed in 2.1.1 (and further addressed by 2.1.2 notes). [1][3][4]
- CVE-2025-12194 — uncontrolled resource consumption issues affecting 2.1.0→2.1.1; fixed in 2.1.2. [3][5]
- Other fixes referenced in the 2.1.2 release notes (including CVE-2025-9092 and various reliability/cleanup fixes). Upgrade notes are on the Bouncy Castle download/release pages. [3][5]
Sources:
- Bouncy Castle Java FIPS release/download notes (bc-fips-2.1.2 and 2.1.1). [3][5]
- GitLab Advisory Database entries for CVE-2025-9340 / CVE-2025-9341 / CVE-2025-12194. [1][2][4]
- NVD record for CVE-2025-9341. [6]
If you want, I can: (a) check the exact latest bc-fips version available today (with links and upgrade guidance), or (b) list which of your project versions are affected if you tell me your current bc-fips version.
Policy violation: adding third-party dependencies to libs/common contradicts the explicit architectural constraint.
This change directly violates the constraint stated on lines 31-33: "NO THIRD PARTY DEPENDENCIES". The comment on lines 31-33 should be updated or removed if this policy is being intentionally changed.
Additionally, using api scope exposes BouncyCastle classes in the public API surface of libs/common. While this propagates BC to consumers of libs/common (including server via transitive dependency), other modules in the codebase use more restrictive scopes (testFipsRuntimeOnly, fipsRuntimeOnly, bcFips) to limit BC exposure to test/FIPS-specific contexts. Consider whether api scope is appropriate here or if a more restrictive scope would better align with the existing pattern.
The BouncyCastle versions being used (bc-fips 2.1.2 and related FIPS libraries) are at current patched versions with known CVEs from earlier 2.1.x releases addressed.
🤖 Prompt for AI Agents
In @libs/common/build.gradle at line 29, The change adds a third-party
BouncyCastle dependency with public exposure via the api configuration (the line
"api libs.bundles.bouncycastle") which violates the project's "NO THIRD PARTY
DEPENDENCIES" policy and the existing usage patterns that restrict BC to
FIPS/test scopes; either remove this api dependency from libs/common or replace
it with a more restrictive configuration consistent with existing symbols (e.g.,
use fipsRuntimeOnly, testFipsRuntimeOnly or bcFips instead of api), and update
or remove the policy comment block (the NO THIRD PARTY DEPENDENCIES note around
lines 31-33) to reflect the intentional policy change if you truly intend to
allow it.
Signed-off-by: Craig Perkins <cwperx@amazon.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
distribution/docker/src/docker/bin/docker-entrypoint.sh (2)
76-82: Update stale comment to reflect environment variable-based control.The comment on line 78 states "If BouncyCastle FIPS is detected" but the logic now checks the
RUN_FIPS_INSTALLERenvironment variable rather than detecting BC FIPS JARs. This discrepancy could mislead future maintainers.📝 Suggested comment update
- # If BouncyCastle FIPS is detected - configure FIPS trust store in test mode + # When FIPS mode is enabled - configure FIPS trust store in test mode
83-87: Update stale comment to reflect environment variable-based control.Similar to the comment above, line 83 states "If BouncyCastle FIPS is detected" but the code now relies on the
RUN_FIPS_INSTALLERenvironment variable rather than detecting BC FIPS.📝 Suggested comment update
- # If BouncyCastle FIPS is detected - enforce keystore password policy. + # When FIPS mode is enabled - enforce keystore password policy.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
distribution/docker/build.gradledistribution/docker/docker-compose.ymldistribution/docker/src/docker/bin/docker-entrypoint.shqa/remote-clusters/docker-compose.fips.ymlqa/wildfly/docker-compose.fips.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: gradle-check
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: detect-breaking-change
- GitHub Check: Analyze (java)
- GitHub Check: Mend Security Check
🔇 Additional comments (5)
distribution/docker/build.gradle (1)
299-305: LGTM!The addition of
RUN_FIPS_INSTALLERenvironment variable is correctly placed within the FIPS JVM conditional block and follows the established pattern for other FIPS-related environment variables.qa/wildfly/docker-compose.fips.yml (1)
1-6: LGTM!The addition of
RUN_FIPS_INSTALLER: trueis appropriate for this FIPS-specific Docker Compose configuration and aligns with the PR's environment-driven FIPS enablement approach.qa/remote-clusters/docker-compose.fips.yml (1)
1-11: LGTM!Both cluster nodes correctly include
RUN_FIPS_INSTALLER: true, ensuring consistent FIPS setup across the multi-node test environment.distribution/docker/docker-compose.yml (2)
19-21: LGTM!The environment variable interpolation for
RUN_FIPS_INSTALLERfollows the established pattern used byKEYSTORE_PASSWORDandFIPS_GENERATE_TRUSTSTOREin the same configuration.
46-48: LGTM!Consistent with the opensearch-1 service configuration above.
|
❌ Gradle check result for ad529b6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @distribution/src/bin/opensearch-env:
- Around line 115-121: The bash FIPS activation has two bugs: the JVM flag is
missing the leading -D and the build token isn't being expanded. Update
opensearch-env to export OPENSEARCH_JAVA_OPTS using the correct JVM flag name
"-Djava.security.properties" (reference OPENSEARCH_JAVA_OPTS and the
FIPS_APPROVED_MODE branch) and add the token org.bouncycastle.fips.approved_only
to the expansionsForDistribution map in distribution/build.gradle so the
${org.bouncycastle.fips.approved_only} placeholder gets replaced at build time
and the FIPS_APPROVED_MODE check works as intended.
In @distribution/src/bin/opensearch-env.bat:
- Around line 33-37: Add the missing expansion for the token and set the
required JVM flag: add an entry for 'org.bouncycastle.fips.approved_only' to the
expansionsForDistribution map in distribution/build.gradle so the
${org.bouncycastle.fips.approved_only} token is substituted during packaging,
and update opensearch-env.bat to append both
-Djava.security.properties="%OPENSEARCH_PATH_CONF%\fips_java.security" and
-Dorg.bouncycastle.fips.approved_only=true to OPENSEARCH_JAVA_OPTS (ensure
OPENSEARCH_JAVA_OPTS is preserved when prepending/appending).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
distribution/src/bin/opensearch-envdistribution/src/bin/opensearch-env.bat
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: gradle-check
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: Analyze (java)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: detect-breaking-change
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, macos-15-intel)
🔇 Additional comments (2)
distribution/src/bin/opensearch-env (1)
143-144: No concerns.distribution/src/bin/opensearch-env.bat (1)
55-59: No concerns.
distribution/src/bin/opensearch-env
Outdated
| FIPS_APPROVED_MODE=${org.bouncycastle.fips.approved_only} | ||
|
|
||
| if [[ "$FIPS_APPROVED_MODE" == "true" ]]; then | ||
| echo "org.bouncycastle.fips.approved_only set to true, setting FIPS JVM options." | ||
| export OPENSEARCH_JAVA_OPTS="Djava.security.properties=${OPENSEARCH_PATH_CONF}/fips_java.security \ | ||
| ${OPENSEARCH_JAVA_OPTS}" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check 1: Search for org.bouncycastle.fips.approved_only token usage ==="
rg -n 'org\.bouncycastle\.fips\.approved_only' -S 2>/dev/null || echo "No matches found"
echo ""
echo "=== Check 2: Search for the token in shell environment files ==="
rg -n '\$\{org\.bouncycastle\.fips\.approved_only\}' distribution/src/bin/ 2>/dev/null || echo "No matches found"
echo ""
echo "=== Check 3: Search for java.security.properties usage pattern ==="
rg -n 'java\.security\.properties' -S 2>/dev/null || echo "No matches found"
echo ""
echo "=== Check 4: Search for fips_java.security file ==="
fd -a 'fips_java\.security' . 2>/dev/null || echo "File not found"
echo ""
echo "=== Check 5: Examine opensearch-env file around lines 115-125 ==="
sed -n '110,130p' distribution/src/bin/opensearch-env 2>/dev/null || echo "File not found"
echo ""
echo "=== Check 6: Look for build files with token replacement configuration ==="
fd -e 'pom.xml|build.gradle|gradle.properties|maven.properties' . | head -20 || trueRepository: opensearch-project/OpenSearch
Length of output: 3419
🏁 Script executed:
#!/bin/bash
# Check build process for token substitution mechanism
echo "=== Check 7: Look for build file configuration for property substitution ==="
find . -name 'build.gradle' -o -name 'pom.xml' | xargs grep -l 'copy\|filter\|expand' 2>/dev/null | head -5
echo ""
echo "=== Check 8: Examine build.gradle for distribution packaging and token filtering ==="
sed -n '430,450p' build.gradle 2>/dev/null || echo "Section not found"
echo ""
echo "=== Check 9: Check if tokens are filtered in distribution build ==="
grep -n 'copy\|expand\|filter' build.gradle 2>/dev/null | head -20 || true
echo ""
echo "=== Check 10: Compare bash vs batch script for FIPS block ==="
echo "--- Bash (opensearch-env) lines 115-121 ---"
sed -n '115,121p' distribution/src/bin/opensearch-env
echo ""
echo "--- Batch (opensearch-env.bat) lines 33-36 ---"
sed -n '33,36p' distribution/src/bin/opensearch-env.bat
echo ""
echo "=== Check 11: View fips_java.security file to understand append/override behavior ==="
head -5 distribution/src/config/fips_java.securityRepository: opensearch-project/OpenSearch
Length of output: 3236
🏁 Script executed:
#!/bin/bash
# Check how distribution scripts are packaged and if tokens are replaced
echo "=== Check 12: Search for copy tasks that handle distribution scripts ==="
rg -A 10 'opensearch-env' build.gradle | head -40
echo ""
echo "=== Check 13: Look for filter/expand tasks in distribution build ==="
rg -B 5 -A 10 'Copy\|copy.*opensearch' build.gradle | grep -A 10 'filter\|expand' | head -50
echo ""
echo "=== Check 14: Find where distribution/src/bin scripts are copied to dist ==="
rg 'src/bin' build.gradle | head -20
echo ""
echo "=== Check 15: Search for token substitution in distribution build ==="
rg 'expand\|filter' build.gradle | grep -i 'dist\|bin' || echo "No matching pattern"
echo ""
echo "=== Check 16: Check gradle distribution plugin configuration ==="
sed -n '1,100p' build.gradle | grep -A 30 'distributions\|distributions {' || trueRepository: opensearch-project/OpenSearch
Length of output: 450
🏁 Script executed:
#!/bin/bash
echo "=== Check 17: Look for distribution build configuration ==="
find distribution -name 'build.gradle' -o -name 'build.gradle.kts' | head -5
echo ""
echo "=== Check 18: Examine distribution/build.gradle if it exists ==="
if [ -f "distribution/build.gradle" ]; then
cat distribution/build.gradle | head -100
else
echo "File not found"
fi
echo ""
echo "=== Check 19: Search for how opensearch-env scripts are included in distribution ==="
rg -n 'opensearch-env' distribution/ -t gradle -t groovy 2>/dev/null | head -20 || true
echo ""
echo "=== Check 20: Check if there's a distributions block in root build.gradle ==="
grep -n 'distributions\|distZip\|distTar' build.gradle | head -30
echo ""
echo "=== Check 21: Look for how scripts are packaged in distributions ==="
grep -B 5 -A 10 'from.*bin\|scripts' build.gradle | head -80Repository: opensearch-project/OpenSearch
Length of output: 4830
🏁 Script executed:
#!/bin/bash
echo "=== Check 22: Examine distribution/archives/build.gradle ==="
head -150 distribution/archives/build.gradle
echo ""
echo "=== Check 23: Search for how opensearch-env is processed in archives build ==="
grep -n 'opensearch-env\|expand\|filter' distribution/archives/build.gradle | head -30
echo ""
echo "=== Check 24: Look for MavenFilteringHack or similar filtering mechanisms ==="
rg 'MavenFilteringHack\|expand\|filter' distribution/archives/build.gradle -B 3 -A 3 | head -60Repository: opensearch-project/OpenSearch
Length of output: 4335
🏁 Script executed:
#!/bin/bash
echo "=== Check 25: Search for binFiles function definition ==="
rg -n 'binFiles' distribution/archives/build.gradle -B 2 -A 20 | head -80
echo ""
echo "=== Check 26: Look for where binFiles is defined in the distribution plugin ==="
fd 'build.gradle*' distribution/ | xargs grep -l 'binFiles' 2>/dev/null
echo ""
echo "=== Check 27: Check buildSrc for distribution setup plugin ==="
find buildSrc -name '*distribution*' -o -name '*archive*' | head -10
echo ""
echo "=== Check 28: Examine the internal-distribution-archive-setup plugin ==="
find buildSrc -name '*.gradle' -o -name '*.java' | xargs grep -l 'internal-distribution-archive-setup' 2>/dev/null | head -3Repository: opensearch-project/OpenSearch
Length of output: 1846
🏁 Script executed:
#!/bin/bash
echo "=== Check 29: Find the class for internal-distribution-archive-setup plugin ==="
cat buildSrc/src/main/resources/META-INF/gradle-plugins/opensearch.internal-distribution-archive-setup.properties
echo ""
echo "=== Check 30: Search for binFiles definition in buildSrc ==="
rg -n 'binFiles' buildSrc/ -B 2 -A 15 | head -100
echo ""
echo "=== Check 31: Look for token/property expansion in distribution build ==="
rg -n 'expand\|filter\|MavenFilteringHack' distribution/ buildSrc/ | head -30
echo ""
echo "=== Check 32: Check distribution/build.gradle for token processing ==="
grep -n 'expand\|filter\|token\|property' distribution/build.gradle | head -20Repository: opensearch-project/OpenSearch
Length of output: 2274
🏁 Script executed:
#!/bin/bash
echo "=== Check 33: Find expansionsForDistribution function definition ==="
rg -n 'expansionsForDistribution' distribution/build.gradle -B 5 -A 30 | head -150
echo ""
echo "=== Check 34: Check if MavenFilteringHack handles the tokens ==="
find buildSrc -name 'MavenFilteringHack*' -o -name '*Filtering*' | head -10
echo ""
echo "=== Check 35: Examine MavenFilteringHack implementation ==="
find buildSrc -name '*.java' | xargs grep -l 'MavenFilteringHack' 2>/dev/null | head -3Repository: opensearch-project/OpenSearch
Length of output: 4004
🏁 Script executed:
#!/bin/bash
echo "=== Check 36: View the full expansionsForDistribution function ==="
sed -n '553,650p' distribution/build.gradle
echo ""
echo "=== Check 37: Search for org.bouncycastle.fips.approved_only in expansions ==="
grep -n 'bouncycastle\|fips' distribution/build.gradleRepository: opensearch-project/OpenSearch
Length of output: 3519
🏁 Script executed:
#!/bin/bash
echo "=== Check 38: Get the complete expansionsForDistribution to see if fips token is added ==="
sed -n '553,700p' distribution/build.gradle | tail -80
echo ""
echo "=== Check 39: Check if org.bouncycastle token is missing from expansions ==="
rg 'org\.bouncycastle' distribution/build.gradleRepository: opensearch-project/OpenSearch
Length of output: 2431
🏁 Script executed:
#!/bin/bash
echo "=== Check 40: Verify that the token ${org.bouncycastle.fips.approved_only} is NOT in expansions ==="
sed -n '553,750p' distribution/build.gradle | grep -i 'fips\|bouncycastle' || echo "Not found in expansions"
echo ""
echo "=== Check 41: Review both shell scripts side-by-side at the problematic lines ==="
echo "=== Bash version (opensearch-env, lines 115-121) ==="
sed -n '115,121p' distribution/src/bin/opensearch-env
echo ""
echo "=== Batch version (opensearch-env.bat, lines 33-36) ==="
sed -n '33,36p' distribution/src/bin/opensearch-env.bat
echo ""
echo "=== Check 42: Confirm fips_java.security file has the comment about single equals ==="
cat distribution/src/config/fips_java.security | head -3Repository: opensearch-project/OpenSearch
Length of output: 1404
Fix two critical bugs preventing FIPS mode from activating in bash distributions.
Line 119 is missing the -D prefix: Djava.security.properties should be -Djava.security.properties. The batch version (line 36 of opensearch-env.bat) correctly includes this prefix.
Additionally, the token ${org.bouncycastle.fips.approved_only} on line 115 is not defined in the expansionsForDistribution map in distribution/build.gradle. Unlike other similar tokens (e.g., ${opensearch.distribution.type}, ${opensearch.bundled_jdk}), this token will not be replaced during the build, causing the bash variable reference to evaluate incorrectly and the condition on line 117 to always fail.
Proposed fix
-FIPS_APPROVED_MODE=${org.bouncycastle.fips.approved_only}
+FIPS_APPROVED_MODE=@org.bouncycastle.fips.approved_only@
if [[ "$FIPS_APPROVED_MODE" == "true" ]]; then
echo "org.bouncycastle.fips.approved_only set to true, setting FIPS JVM options."
- export OPENSEARCH_JAVA_OPTS="Djava.security.properties=${OPENSEARCH_PATH_CONF}/fips_java.security \
+ export OPENSEARCH_JAVA_OPTS="-Djava.security.properties=${OPENSEARCH_PATH_CONF}/fips_java.security \
${OPENSEARCH_JAVA_OPTS}"
fiAlso add the token to the expansionsForDistribution map in distribution/build.gradle:
'opensearch.bundled_jdk': [
'def': jdk != JavaPackageType.NONE ? true : false
],
+
+'org.bouncycastle.fips.approved_only': [
+ 'def': 'false'
+],Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @distribution/src/bin/opensearch-env around lines 115 - 121, The bash FIPS
activation has two bugs: the JVM flag is missing the leading -D and the build
token isn't being expanded. Update opensearch-env to export OPENSEARCH_JAVA_OPTS
using the correct JVM flag name "-Djava.security.properties" (reference
OPENSEARCH_JAVA_OPTS and the FIPS_APPROVED_MODE branch) and add the token
org.bouncycastle.fips.approved_only to the expansionsForDistribution map in
distribution/build.gradle so the ${org.bouncycastle.fips.approved_only}
placeholder gets replaced at build time and the FIPS_APPROVED_MODE check works
as intended.
| set FIPS_APPROVED_MODE=${org.bouncycastle.fips.approved_only} | ||
| if "%FIPS_APPROVED_MODE%" == "true" ( | ||
| echo org.bouncycastle.fips.approved_only set to true, setting FIPS JVM options. | ||
| set OPENSEARCH_JAVA_OPTS=-Djava.security.properties="%OPENSEARCH_PATH_CONF%\fips_java.security" %OPENSEARCH_JAVA_OPTS% | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n distribution/src/bin/opensearch-env.bat | head -80Repository: opensearch-project/OpenSearch
Length of output: 3398
🏁 Script executed:
cat -n distribution/src/bin/opensearch-env.sh | grep -A 10 -B 5 "FIPS_APPROVED_MODE\|approved_only\|java.security.properties"Repository: opensearch-project/OpenSearch
Length of output: 143
🏁 Script executed:
find . -name "pom.xml" -o -name "build.gradle" -o -name "*.gradle" | head -5Repository: opensearch-project/OpenSearch
Length of output: 195
🏁 Script executed:
grep -r "org.bouncycastle.fips.approved_only" --include="*.xml" --include="*.gradle" --include="*.properties" | head -20Repository: opensearch-project/OpenSearch
Length of output: 145
🏁 Script executed:
grep -r "FIPS_APPROVED_MODE\|approved_only" --include="*.bat" --include="*.sh" | head -30Repository: opensearch-project/OpenSearch
Length of output: 376
🏁 Script executed:
find . -name "opensearch-env.sh" -type fRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
grep -B 10 -A 10 "org.bouncycastle.fips.approved_only" build.gradleRepository: opensearch-project/OpenSearch
Length of output: 1018
🏁 Script executed:
grep -B 5 -A 5 "FIPS\|approved_only" build.gradle | head -50Repository: opensearch-project/OpenSearch
Length of output: 839
🏁 Script executed:
grep -r "maven-resources-plugin\|resource\|filtering" --include="pom.xml" --include="build.gradle" | grep -i "fips\|resource" | head -20Repository: opensearch-project/OpenSearch
Length of output: 2253
🏁 Script executed:
cat build.gradle | grep -A 50 "fips\|FIPS" | head -100Repository: opensearch-project/OpenSearch
Length of output: 3457
🏁 Script executed:
find . -name "opensearch-env.sh" -type f 2>/dev/nullRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
cat gradle/fips.gradleRepository: opensearch-project/OpenSearch
Length of output: 6466
🏁 Script executed:
find distribution -name "build.gradle" -type f | xargs grep -l "fips\|resource\|filtering" 2>/dev/nullRepository: opensearch-project/OpenSearch
Length of output: 275
🏁 Script executed:
cat distribution/build.gradle | grep -A 30 -B 5 "fips\|resource\|copy" | head -80Repository: opensearch-project/OpenSearch
Length of output: 3196
🏁 Script executed:
cat distribution/build.gradle | grep -A 50 "processing\|filter\|token\|replace\|resources" | head -150Repository: opensearch-project/OpenSearch
Length of output: 5935
🏁 Script executed:
grep -r "MavenFilteringHack\|properties\|filtering" distribution/build.gradle | head -20Repository: opensearch-project/OpenSearch
Length of output: 869
🏁 Script executed:
grep -B 5 -A 15 "MavenFilteringHack" distribution/build.gradleRepository: opensearch-project/OpenSearch
Length of output: 2324
🏁 Script executed:
grep -r "org.bouncycastle.fips.approved_only" . --include="*.gradle" --include="*.properties" --include="*.xml"Repository: opensearch-project/OpenSearch
Length of output: 147
🏁 Script executed:
find . -path "*/bin/*" -name "opensearch*" -type f | grep -E "\.(sh|bat)$"Repository: opensearch-project/OpenSearch
Length of output: 477
🏁 Script executed:
grep -A 100 "expansionsForDistribution" distribution/build.gradle | head -150Repository: opensearch-project/OpenSearch
Length of output: 5300
🏁 Script executed:
grep -B 5 "org.bouncycastle.fips.approved_only" distribution/build.gradleRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
find . -name "*.properties" -type f | xargs grep -l "org.bouncycastle.fips.approved_only" 2>/dev/nullRepository: opensearch-project/OpenSearch
Length of output: 57
🏁 Script executed:
grep -r "properties\|expansion" distribution/src/config | head -20Repository: opensearch-project/OpenSearch
Length of output: 227
🏁 Script executed:
cat distribution/src/config/fips_java.securityRepository: opensearch-project/OpenSearch
Length of output: 630
🏁 Script executed:
grep -A 200 "ext.expansionsForDistribution" distribution/build.gradle | grep -E "org.bouncycastle|approved_only|fips" -iRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
grep "org.bouncycastle.fips.approved_only\|expansion.*fips" distribution/build.gradle -iRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
find . -name "*.gradle" -o -name "*.properties" | xargs grep -l "fips\|approved" 2>/dev/null | head -10Repository: opensearch-project/OpenSearch
Length of output: 408
🏁 Script executed:
grep -A 250 "ext.expansionsForDistribution" distribution/build.gradle | tail -150Repository: opensearch-project/OpenSearch
Length of output: 4437
🏁 Script executed:
grep -B 5 -A 50 "opensearch.bundled_jdk\|opensearch.distribution" distribution/build.gradle | head -100Repository: opensearch-project/OpenSearch
Length of output: 1774
🏁 Script executed:
find . -path "*gradle*" -name "*.java" | xargs grep -l "approved_only\|isInFipsJvm" 2>/dev/null | head -5Repository: opensearch-project/OpenSearch
Length of output: 293
🏁 Script executed:
cat buildSrc/src/main/java/org/opensearch/gradle/info/BuildParams.java | grep -A 20 "isInFipsJvm"Repository: opensearch-project/OpenSearch
Length of output: 543
🏁 Script executed:
grep -A 300 "ext.expansionsForDistribution" distribution/build.gradle | grep "org.bouncycastle\|approved_only" -iRepository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
grep -r "org.bouncycastle.fips.approved_only" . --include="*.gradle" --include="*.properties"Repository: opensearch-project/OpenSearch
Length of output: 147
🏁 Script executed:
grep -B 5 -A 50 "class MavenFilteringHack" buildSrc/src/main/java/org/opensearch/gradle/MavenFilteringHack.java 2>/dev/null || find buildSrc -name "MavenFilteringHack.java" -exec cat {} \;Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
find buildSrc -name "*.java" -type f | xargs grep -l "MavenFilteringHack" | head -1 | xargs cat | head -100Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
grep -A 20 "def filter" buildSrc/src/main/java/org/opensearch/gradle/MavenFilteringHack.javaRepository: opensearch-project/OpenSearch
Length of output: 174
🏁 Script executed:
grep -C 50 "opensearch.bundled_jdk" distribution/build.gradle | tail -80Repository: opensearch-project/OpenSearch
Length of output: 2321
🏁 Script executed:
find buildSrc -name "*.java" -type f -exec grep -l "MavenFilteringHack" {} \;Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
find buildSrc -name "*.java" -type f | xargs grep -l "class MavenFilteringHack"Repository: opensearch-project/OpenSearch
Length of output: 57
🏁 Script executed:
find . -name "*.java" -path "*/buildSrc/*" -exec grep -l "filter.*expansion" {} \; | head -3Repository: opensearch-project/OpenSearch
Length of output: 55
🏁 Script executed:
cat distribution/build.gradle | grep -B 20 "MavenFilteringHack.filter" | head -50Repository: opensearch-project/OpenSearch
Length of output: 1712
🏁 Script executed:
find . -type f -name "*.gradle" -path "*/buildSrc/*" 2>/dev/null | head -5Repository: opensearch-project/OpenSearch
Length of output: 366
🏁 Script executed:
find . -type f \( -name "*.java" -o -name "*.groovy" \) | xargs grep -l "class.*MavenFilteringHack\|def.*MavenFilteringHack" 2>/dev/null | head -5Repository: opensearch-project/OpenSearch
Length of output: 147
🏁 Script executed:
find . -type f \( -name "*.bat" -o -name "*.sh" \) -exec grep -l "org.bouncycastle.fips" {} \;Repository: opensearch-project/OpenSearch
Length of output: 114
🏁 Script executed:
find . -name "*fips*" -type f | grep -v ".git\|node_modules\|build" | head -20Repository: opensearch-project/OpenSearch
Length of output: 1052
🏁 Script executed:
grep -r "\${.*}" distribution/src/bin/ | head -10Repository: opensearch-project/OpenSearch
Length of output: 1041
🏁 Script executed:
cat ./buildSrc/src/main/groovy/org/opensearch/gradle/MavenFilteringHack.groovyRepository: opensearch-project/OpenSearch
Length of output: 2200
Fix the FIPS enablement in opensearch-env.bat to set both required JVM flags and add the missing token to the build expansions.
The token ${org.bouncycastle.fips.approved_only} won't be substituted during packaging because it's missing from the expansionsForDistribution map in distribution/build.gradle. Additionally, even if the token were replaced, the script only sets -Djava.security.properties but omits -Dorg.bouncycastle.fips.approved_only=true, which is required to actually enforce approved-only mode (as shown in build.gradle's FIPS test configuration). Without this flag, BouncyCastle operates in hybrid mode rather than approved-only mode, compromising FIPS compliance.
Both issues need fixing:
- Add
'org.bouncycastle.fips.approved_only': [...]to expansions indistribution/build.gradle - Add
-Dorg.bouncycastle.fips.approved_only=trueto the OPENSEARCH_JAVA_OPTS in the .bat script
🤖 Prompt for AI Agents
In @distribution/src/bin/opensearch-env.bat around lines 33 - 37, Add the
missing expansion for the token and set the required JVM flag: add an entry for
'org.bouncycastle.fips.approved_only' to the expansionsForDistribution map in
distribution/build.gradle so the ${org.bouncycastle.fips.approved_only} token is
substituted during packaging, and update opensearch-env.bat to append both
-Djava.security.properties="%OPENSEARCH_PATH_CONF%\fips_java.security" and
-Dorg.bouncycastle.fips.approved_only=true to OPENSEARCH_JAVA_OPTS (ensure
OPENSEARCH_JAVA_OPTS is preserved when prepending/appending).
|
❌ Gradle check result for 5379840: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
distribution/build.gradle (1)
312-312: Remove unusedbcFipsconfiguration from the configurations list.The
bcFipsconfiguration is declared at line 312 but is never assigned any dependencies and is not referenced anywhere in the distribution module's build logic. It has no dependencies and is not consumed by any tasks or copySpecs. This configuration declaration serves no purpose and should be removed from the list.Removal
['libs', 'libsPluginCli', 'libsKeystoreCli', 'libsFipsInstallerCli', 'bcFips'].each {Change to:
['libs', 'libsPluginCli', 'libsKeystoreCli', 'libsFipsInstallerCli'].each {
🤖 Fix all issues with AI agents
In @distribution/build.gradle:
- Around line 640-642: Update the user-facing documentation to explain FIPS
enablement: state that users must pass
-Dorg.bouncycastle.fips.approved_only=true to the JVM to enable FIPS mode,
mention that the Gradle/build flag represented by 'opensearch.fips.mode' reads
that system property, and note that when FIPS mode is enabled the build/startup
scripts will apply FIPS-specific JVM options automatically; add this guidance
near other build/startup configuration docs so users know how to enable and what
to expect.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
distribution/build.gradledistribution/src/bin/opensearch-envdistribution/src/bin/opensearch-env.bat
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: gradle-check
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: Analyze (java)
- GitHub Check: detect-breaking-change
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: Mend Security Check
🔇 Additional comments (6)
distribution/src/bin/opensearch-env.bat (2)
40-40: LGTM: Bundled JDK property rename.The rename from
opensearch.bundled_jdktoopensearch.bundled.jdkaligns with property naming consistency across the distribution configuration.
33-37: All FIPS mode configuration changes are correctly implemented.Verification confirms:
- The
opensearch.fips.modeproperty is properly defined indistribution/build.gradle(lines 640-642) and correctly defaults toSystem.getProperty('org.bouncycastle.fips.approved_only', 'false')- The
fips_java.securityconfiguration file exists atdistribution/src/config/fips_java.security- Both bash (
opensearch-env) and batch (opensearch-env.bat) scripts correctly reference the FIPS mode property and apply the Java security options when enabled- No runtime bc-fips JAR detection logic remains in the codebase (bc-fips references are limited to test resource documentation)
The shift from runtime JAR detection to build-time configuration is properly implemented. Ensure installation guides document that FIPS mode is controlled via the build-time
opensearch.fips.modeproperty (driven by theorg.bouncycastle.fips.approved_onlysystem property).distribution/src/bin/opensearch-env (3)
115-120: LGTM: Environment-driven FIPS mode activation.The FIPS mode check correctly uses the environment-driven configuration and properly appends the Java security properties to
OPENSEARCH_JAVA_OPTS. The implementation is consistent with the Windows batch script version.
123-123: LGTM: Bundled JDK property rename.Consistent with the Windows script, the property name has been updated to use dot notation.
142-142: LGTM: Documentation URL update.The URL has been updated to point to the correct OpenSearch documentation.
distribution/build.gradle (1)
636-638: LGTM: Bundled JDK property rename for consistency.The rename from
opensearch.bundled_jdktoopensearch.bundled.jdkmaintains consistency with the updated environment scripts.
| 'opensearch.fips.mode': [ | ||
| 'def': System.getProperty('org.bouncycastle.fips.approved_only', 'false') | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct BouncyCastle system property for FIPS approved mode
💡 Result:
Use the JVM system property:
-Dorg.bouncycastle.fips.approved_only=true. [1][2]
Sources:
[1] Bouncy-castle-fips-adapter docs (example showing org.bouncycastle.fips.approved_only).
[2] Bouncy Castle FIPS Java API / javadoc references.
🏁 Script executed:
#!/bin/bash
# Check for documentation and usage of FIPS mode property
echo "=== Searching for FIPS mode documentation ==="
fd -e md -e txt | xargs rg -l "fips.mode|FIPS.*mode" 2>/dev/null | head -20
echo -e "\n=== Checking gradle files for org.bouncycastle properties ==="
rg -n "org\.bouncycastle" -g "**/*.gradle" 2>/dev/null
echo -e "\n=== Checking for opensearch.fips.mode usage ==="
rg -n "opensearch\.fips\.mode" -C3 2>/dev/null
echo -e "\n=== Checking distribution/build.gradle around lines 640-642 ==="
sed -n '635,650p' distribution/build.gradleRepository: opensearch-project/OpenSearch
Length of output: 9647
Add user documentation for FIPS mode enablement.
The system property name org.bouncycastle.fips.approved_only is correct for BouncyCastle FIPS mode, and the default of 'false' appropriately disables FIPS by default. However, user-facing documentation is missing. Add guidance explaining:
- Users must pass
-Dorg.bouncycastle.fips.approved_only=trueto the JVM to enable FIPS mode - When FIPS mode is enabled, the build applies FIPS-specific JVM options via the startup scripts
🤖 Prompt for AI Agents
In @distribution/build.gradle around lines 640 - 642, Update the user-facing
documentation to explain FIPS enablement: state that users must pass
-Dorg.bouncycastle.fips.approved_only=true to the JVM to enable FIPS mode,
mention that the Gradle/build flag represented by 'opensearch.fips.mode' reads
that system property, and note that when FIPS mode is enabled the build/startup
scripts will apply FIPS-specific JVM options automatically; add this guidance
near other build/startup configuration docs so users know how to enable and what
to expect.
|
❌ Gradle check result for 0b2dfbe: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
|
❌ Gradle check result for b3faa21: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b3faa21: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b3faa21: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for b3faa21: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
3 participants
Description
This PR adds the following dependencies as api-level dependencies on
server/This is done so that these dependencies are always available on the runtime classpath under
lib/. They are currently only available under lib when OpenSearch is built with the-Pcrypto.standard=FIPS-140-3flag.The problem with conditionally including these
deps/under lib is that it complicates adapting the plugins for FIPS mode and would be simpler to always include these deps.Note: The security plugin will have adapt to this change and a couple other plugins in the default distribution may also need to adapt to the change.
Related Issues
See discussion on opensearch-project/technical-steering#77
Check List
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.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.