Fix repository-gcs KeyStore.getInstance type/provider order in createFipsCompliantGoogleTrustStore

[gszkopinski]

Description

GoogleCloudStorageService.createFipsCompliantGoogleTrustStore calls:

KeyStore trustStore = KeyStore.getInstance("BCFIPS", "BCFIPS");

"BCFIPS" is the provider name registered by BouncyCastleFipsProvider; the keystore type it provides is "BCFKS" — which is exactly what the in-method comment one line above already says (// Use BCFKS KeyStore type which is required for FIPS mode). The call as written therefore throws on every invocation:

KeyStoreException: BCFIPS not found
  caused by: NoSuchAlgorithmException: no such algorithm: BCFIPS for provider BCFIPS

This makes any GCS snapshot operation fail on a FIPS-enabled OpenSearch 3.7.0 distribution: the buggy method is unconditionally entered whenever Security.getProvider("BCFIPS") != null (the default for 3.x official images) and no custom gcs.client.*.truststore.* is configured.

The fix is to pass the correct keystore type ("BCFKS") while keeping the same provider ("BCFIPS"):

-    KeyStore trustStore = KeyStore.getInstance("BCFIPS", "BCFIPS");
+    KeyStore trustStore = KeyStore.getInstance("BCFKS", "BCFIPS");

After this change the method behaves as the comment and the PR that introduced it (#21122) intended: a BCFKS keystore created via the BCFIPS provider, populated with Google's default trust store entries.

Related Issues

Resolves #22287

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

No new tests are added: the buggy line is on a code path that is not currently exercised by any test (otherwise it would have failed in CI). Happy to add a test that asserts loadTrustStore() returns a usable BCFKS keystore under FIPS if reviewers prefer.

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]

PR Reviewer Guide 🔍

(Review updated until commit f75385d)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 9dc7f65

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate provider availability before use Verify that the BCFIPS provider is registered and available before attempting to create the KeyStore instance. If the provider is not available, this will throw a NoSuchProviderException at runtime, which could cause service initialization failures. plugins/repository-gcs/src/main/java/org/opensearch/repositories/gcs/GoogleCloudStorageService.java [307] +if (Security.getProvider("BCFIPS") == null) { + throw new IllegalStateException("BCFIPS provider is not registered"); +} KeyStore trustStore = KeyStore.getInstance("BCFKS", "BCFIPS"); Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a valid runtime concern by recommending validation of the BCFIPS provider before use. However, this is a defensive programming practice rather than a critical bug fix, as the method already declares GeneralSecurityException which would handle NoSuchProviderException. The score reflects its value as a proactive error handling improvement.	Medium

---

Previous suggestions

Suggestions up to commit 2b85907

Category	Suggestion	Impact
Possible issue	Validate BCFIPS provider availability Verify that the BCFIPS provider is registered and available before attempting to create the KeyStore instance. If the provider is not available, this will throw a NoSuchProviderException at runtime, which could cause service initialization failures in FIPS environments. plugins/repository-gcs/src/main/java/org/opensearch/repositories/gcs/GoogleCloudStorageService.java [307] +if (Security.getProvider("BCFIPS") == null) { + throw new IllegalStateException("BCFIPS provider is not registered"); +} KeyStore trustStore = KeyStore.getInstance("BCFKS", "BCFIPS"); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the BCFIPS provider should be validated before use. However, since the method already throws GeneralSecurityException which includes NoSuchProviderException, and this is primarily asking to verify/ensure a condition rather than fixing a critical bug, the score is moderate. The suggestion would improve error handling and provide clearer failure messages.	Medium

Suggestions up to commit 26559fb

Category	Suggestion	Impact
Possible issue	Validate BCFIPS provider availability Verify that the BCFIPS provider is registered and available before attempting to create the KeyStore instance. If the provider is not available, this will throw a NoSuchProviderException at runtime, which could cause service initialization failures in FIPS environments. plugins/repository-gcs/src/main/java/org/opensearch/repositories/gcs/GoogleCloudStorageService.java [307] +if (Security.getProvider("BCFIPS") == null) { + throw new IllegalStateException("BCFIPS provider is not registered"); +} KeyStore trustStore = KeyStore.getInstance("BCFKS", "BCFIPS"); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the BCFIPS provider should be validated before use to prevent runtime NoSuchProviderException. However, since the method already declares GeneralSecurityException in its signature which covers this exception, and this is primarily a defensive check rather than fixing a critical bug, the score is moderate.	Medium

[reta]

Thanks @gszkopinski

[reta]

@gszkopinski could you please fix DCO check? (git commit --amend --sign-off)

[github-actions]

❌ Gradle check result for 26559fb: 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?

[github-actions]

Persistent review updated to latest commit 2b85907

[github-actions]

Persistent review updated to latest commit 9dc7f65

[github-actions]

❌ Gradle check result for 9dc7f65: 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?

[github-actions]

Persistent review updated to latest commit f75385d

[github-actions]

❌ Gradle check result for f75385d: 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?

[gszkopinski]

Hello @reta ! I can't see the error on the Jenkins/Gradle side. It requires permissions regarding GitHub organizations and teams that I don't want to share.

Is it possible to display the error and indicate whether or not it stems from my pull request?