[Backport 2.15] Add SSE-KMS encryption context and bucket owner verification to S3 repository plugin

[yzhang5-2023]

Summary

Backport of #18312 to branch-2.15.

Adds support for:

• SSE-KMS encryption context in the S3 repository plugin
• Typed encryption settings (AES256, aws:kms, bucket_default) replacing the previous boolean server_side_encryption flag
• KMS key ID and encryption context configuration
• S3 Bucket Key support for performance optimization
• Expected bucket owner verification for all S3 operations

Changes

• New SseKmsUtil.java utility class centralizing encryption configuration for S3 request builders
• Updated S3Repository with new typed settings definitions
• Updated S3BlobStore, S3BlobContainer, UploadRequest
• Updated async operations: AsyncTransferManager, AsyncPartsHandler
• Updated test files to cover new SSE-KMS configuration paths

Conflict Resolutions (2.15-specific)

The following adaptations were made when cherry-picking to branch-2.15:

• Preserved metadata field in UploadRequest.java (added in 2.15.0, not present in main)
• Kept S3Error import in S3BlobContainer.java (used in 2.15.0)
• Skipped S3AsyncDeleteHelper.java change (file does not exist in 2.15.0)

Testing

• All existing S3 repository tests pass
• New tests added for SSE-KMS configuration paths

Related: #18312

Signed-off-by: yzhang5 yzhang5@atlassian.com

[github-actions]

PR Code Analyzer ❗

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

Path	Line	Severity	Description
plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java	643	low	Debug log statement includes server_side_encryption_kms_key_id and server_side_encryption_encryption_context fields. While KMS key IDs are not secret credentials, encryption context values may contain sensitive metadata depending on user configuration. This is at DEBUG level only and follows the existing logging pattern, but it is worth reviewing whether these values should be redacted or omitted from logs.

The table above displays the top 10 most important findings.

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

---

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.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 Security concerns Sensitive information exposure: The serverSideEncryptionKmsKey and serverSideEncryptionEncryptionContext are logged at DEBUG level in readRepositoryMetadata (lines 654-657 of S3Repository.java). KMS key IDs and encryption contexts are sensitive configuration values that could expose information about the encryption setup if debug logging is enabled in production. Consider masking or omitting these values from log output.

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: SSE-KMS encryption context and typed encryption settings support Relevant files: plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/utils/SseKmsUtil.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobStore.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobContainer.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/UploadRequest.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/AsyncTransferManager.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/AsyncPartsHandler.java Sub-PR theme: Expected bucket owner verification for all S3 operations Relevant files: plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobContainer.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobStore.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3RetryingInputStream.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/AsyncTransferManager.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/AsyncPartsHandler.java plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/UploadRequest.java

⚡ Recommended focus areas for review Code Duplication The four overloaded configureEncryptionSettings methods contain nearly identical logic, differing only in whether they accept S3BlobStore or UploadRequest. The two builder types (CreateMultipartUploadRequest.Builder and PutObjectRequest.Builder) also share the same encryption fields. Consider extracting a common interface or using a generic approach to reduce duplication and maintenance burden. public static void configureEncryptionSettings(CreateMultipartUploadRequest.Builder builder, S3BlobStore blobStore) { if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AES256.toString())) { builder.serverSideEncryption(ServerSideEncryption.AES256); } else if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AWS_KMS.toString())) { builder.serverSideEncryption(ServerSideEncryption.AWS_KMS); builder.ssekmsKeyId(blobStore.serverSideEncryptionKmsKey()); builder.bucketKeyEnabled(blobStore.serverSideEncryptionBucketKey()); builder.ssekmsEncryptionContext(blobStore.serverSideEncryptionEncryptionContext()); } } public static void configureEncryptionSettings(PutObjectRequest.Builder builder, S3BlobStore blobStore) { if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AES256.toString())) { builder.serverSideEncryption(ServerSideEncryption.AES256); } else if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AWS_KMS.toString())) { builder.serverSideEncryption(ServerSideEncryption.AWS_KMS); builder.ssekmsKeyId(blobStore.serverSideEncryptionKmsKey()); builder.bucketKeyEnabled(blobStore.serverSideEncryptionBucketKey()); builder.ssekmsEncryptionContext(blobStore.serverSideEncryptionEncryptionContext()); } } public static void configureEncryptionSettings(CreateMultipartUploadRequest.Builder builder, UploadRequest uploadRequest) { if (uploadRequest.getServerSideEncryptionType().equals(ServerSideEncryption.AES256.toString())) { builder.serverSideEncryption(ServerSideEncryption.AES256); } else if (uploadRequest.getServerSideEncryptionType().equals(ServerSideEncryption.AWS_KMS.toString())) { builder.serverSideEncryption(ServerSideEncryption.AWS_KMS); builder.ssekmsKeyId(uploadRequest.getServerSideEncryptionKmsKey()); builder.bucketKeyEnabled(uploadRequest.getServerSideEncryptionBucketKey()); builder.ssekmsEncryptionContext(uploadRequest.getServerSideEncryptionEncryptionContext()); } } public static void configureEncryptionSettings(PutObjectRequest.Builder builder, UploadRequest uploadRequest) { if (uploadRequest.getServerSideEncryptionType().equals(ServerSideEncryption.AES256.toString())) { builder.serverSideEncryption(ServerSideEncryption.AES256); } else if (uploadRequest.getServerSideEncryptionType().equals(ServerSideEncryption.AWS_KMS.toString())) { builder.serverSideEncryption(ServerSideEncryption.AWS_KMS); builder.ssekmsKeyId(uploadRequest.getServerSideEncryptionKmsKey()); builder.bucketKeyEnabled(uploadRequest.getServerSideEncryptionBucketKey()); builder.ssekmsEncryptionContext(uploadRequest.getServerSideEncryptionEncryptionContext()); } } Null Pointer Risk configureEncryptionSettings calls blobStore.serverSideEncryptionType().equals(...) without a null check. If serverSideEncryptionType is null (e.g., not set), this will throw a NullPointerException. The same applies to the UploadRequest overloads. if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AES256.toString())) { builder.serverSideEncryption(ServerSideEncryption.AES256); } else if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AWS_KMS.toString())) { builder.serverSideEncryption(ServerSideEncryption.AWS_KMS); builder.ssekmsKeyId(blobStore.serverSideEncryptionKmsKey()); builder.bucketKeyEnabled(blobStore.serverSideEncryptionBucketKey()); builder.ssekmsEncryptionContext(blobStore.serverSideEncryptionEncryptionContext()); } Encoding Side Effect serverSideEncryptionEncryptionContext() applies Base64 encoding every time it is called. If this method is called multiple times (e.g., once per request), the value will be re-encoded each time from the raw stored string, which is correct but may be surprising. However, if the stored value is already Base64-encoded (e.g., after a reload), double-encoding could occur. Verify that the raw string is always stored and encoding is only applied at call time. public String serverSideEncryptionEncryptionContext() { return serverSideEncryptionEncryptionContext.isEmpty() ? null : Base64.getEncoder().encodeToString(serverSideEncryptionEncryptionContext.getBytes(StandardCharsets.UTF_8)); } Volatile Misuse Fields serverSideEncryptionType, serverSideEncryptionKmsKey, serverSideEncryptionBucketKey, and serverSideEncryptionEncryptionContext are declared volatile in UploadRequest, but UploadRequest is a value object constructed once and never mutated after construction. These fields should be final instead of volatile. private volatile String serverSideEncryptionType; private volatile String serverSideEncryptionKmsKey; private volatile boolean serverSideEncryptionBucketKey; private volatile String serverSideEncryptionEncryptionContext; Missing Setting Registration The new settings (SERVER_SIDE_ENCRYPTION_TYPE_SETTING, SERVER_SIDE_ENCRYPTION_KMS_KEY_SETTING, SERVER_SIDE_ENCRYPTION_BUCKET_KEY_SETTING, SERVER_SIDE_ENCRYPTION_ENCRYPTION_CONTEXT_SETTING, EXPECTED_BUCKET_OWNER_SETTING) must be registered in the plugin's getSettings() method. If they are not registered, OpenSearch will reject repository configurations using these settings. Verify that all new settings are included in the settings registration. static final Setting<String> SERVER_SIDE_ENCRYPTION_TYPE_SETTING = Setting.simpleString( "server_side_encryption_type", BUCKET_DEFAULT_ENCRYPTION_TYPE, value -> { if (!(value.equals(ServerSideEncryption.AES256.toString()) || value.equals(ServerSideEncryption.AWS_KMS.toString()) || value.equals(BUCKET_DEFAULT_ENCRYPTION_TYPE))) { throw new IllegalArgumentException("server_side_encryption_type must be one of [AES256, aws:kms, bucket_default]"); } } ); /** * The KMS key id to be used for SSE-KMS. Must be used when server_side_encryption_type setting is set to aws:kms. */ static final Setting<String> SERVER_SIDE_ENCRYPTION_KMS_KEY_SETTING = Setting.simpleString("server_side_encryption_kms_key_id"); /** * Whether to use S3 Bucket Keys along with SSE-KMS. * Defaults to true. */ static final Setting<Boolean> SERVER_SIDE_ENCRYPTION_BUCKET_KEY_SETTING = Setting.boolSetting( "server_side_encryption_bucket_key_enabled", true ); /** * Optional additional encryption context passed to S3 for use in KMS crypto operations. The setting value must be formatted as a key-value pair JSON object. */ static final Setting<String> SERVER_SIDE_ENCRYPTION_ENCRYPTION_CONTEXT_SETTING = Setting.simpleString( "server_side_encryption_encryption_context" ); /** * Optional setting to specify the expected S3 bucket owner. This is used to verify S3 bucket ownership before reading/writing data from/to a bucket. */ static final Setting<String> EXPECTED_BUCKET_OWNER_SETTING = Setting.simpleString("expected_bucket_owner", value -> { if (!(value.matches("\\d{12}") || value.isEmpty())) { throw new IllegalArgumentException("expected_bucket_owner must be a 12 digit AWS account id"); } });

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard against NullPointerException in encryption type comparison The serverSideEncryptionType() method may return null if the setting is empty or unset, which would cause a NullPointerException when calling .equals() on it. Add a null check before comparing, or use a null-safe comparison like ServerSideEncryption.AES256.toString().equals(blobStore.serverSideEncryptionType()). plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/utils/SseKmsUtil.java [20-29] public static void configureEncryptionSettings(CreateMultipartUploadRequest.Builder builder, S3BlobStore blobStore) { - if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AES256.toString())) { + String sseType = blobStore.serverSideEncryptionType(); + if (ServerSideEncryption.AES256.toString().equals(sseType)) { builder.serverSideEncryption(ServerSideEncryption.AES256); - } else if (blobStore.serverSideEncryptionType().equals(ServerSideEncryption.AWS_KMS.toString())) { - ... + } else if (ServerSideEncryption.AWS_KMS.toString().equals(sseType)) { + builder.serverSideEncryption(ServerSideEncryption.AWS_KMS); + builder.ssekmsKeyId(blobStore.serverSideEncryptionKmsKey()); + builder.bucketKeyEnabled(blobStore.serverSideEncryptionBucketKey()); + builder.ssekmsEncryptionContext(blobStore.serverSideEncryptionEncryptionContext()); } } Suggestion importance[1-10]: 6 __ Why: The serverSideEncryptionType() could potentially return null if the setting is unset, causing a NPE when calling .equals(). Using ServerSideEncryption.AES256.toString().equals(sseType) is a safer null-safe comparison pattern. However, looking at the code, SERVER_SIDE_ENCRYPTION_TYPE_SETTING has a default value of BUCKET_DEFAULT_ENCRYPTION_TYPE, so null is unlikely in practice.	Low
	Guard against NullPointerException on null encryption context The serverSideEncryptionEncryptionContext field could be null (e.g., when SERVER_SIDE_ENCRYPTION_ENCRYPTION_CONTEXT_SETTING returns null for an unset simpleString setting), which would cause a NullPointerException when calling .isEmpty(). Add a null check before calling .isEmpty(). plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3BlobStore.java [232-236] public String serverSideEncryptionEncryptionContext() { - return serverSideEncryptionEncryptionContext.isEmpty() - ? null - : Base64.getEncoder().encodeToString(serverSideEncryptionEncryptionContext.getBytes(StandardCharsets.UTF_8)); + if (serverSideEncryptionEncryptionContext == null || serverSideEncryptionEncryptionContext.isEmpty()) { + return null; + } + return Base64.getEncoder().encodeToString(serverSideEncryptionEncryptionContext.getBytes(StandardCharsets.UTF_8)); } Suggestion importance[1-10]: 5 __ Why: The serverSideEncryptionEncryptionContext field could be null if Setting.simpleString returns null for an unset setting, causing a NPE on .isEmpty(). Adding a null check before .isEmpty() prevents this potential runtime error.	Low
General	Remove unnecessary volatile modifier from immutable fields The SSE-related fields in UploadRequest are declared as volatile, but UploadRequest is a value object that is constructed once and never mutated after construction. Using volatile on these fields is unnecessary and misleading; they should be declared final like the other fields in the class. plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/async/UploadRequest.java [31-35] -private volatile String serverSideEncryptionType; -private volatile String serverSideEncryptionKmsKey; -private volatile boolean serverSideEncryptionBucketKey; -private volatile String serverSideEncryptionEncryptionContext; +private final String serverSideEncryptionType; +private final String serverSideEncryptionKmsKey; +private final boolean serverSideEncryptionBucketKey; +private final String serverSideEncryptionEncryptionContext; private final String expectedBucketOwner; Suggestion importance[1-10]: 5 __ Why: The volatile modifier on serverSideEncryptionType, serverSideEncryptionKmsKey, serverSideEncryptionBucketKey, and serverSideEncryptionEncryptionContext fields is inconsistent with the immutable nature of UploadRequest — all other fields are final. Changing them to final improves correctness and clarity.	Low
Security	Avoid logging sensitive encryption key material The serverSideEncryptionEncryptionContext and serverSideEncryptionKmsKey values are logged in plaintext, which could expose sensitive KMS key IDs and encryption context to logs. Consider masking or omitting these sensitive values in the debug log output. plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java [645-658] logger.debug( "using bucket [{}], chunk_size [{}], buffer_size [{}], cannedACL [{}], storageClass [{}], " - + "server_side_encryption_type [{}], server_side_encryption_kms_key_id [{}], server_side_encryption_bucket_key_enabled [{}], server_side_encryption_encryption_context [{}], expected_bucket_owner [{}], ", + + "server_side_encryption_type [{}], server_side_encryption_kms_key_id [{}], server_side_encryption_bucket_key_enabled [{}], expected_bucket_owner [{}]", bucket, chunkSize, bufferSize, cannedACL, storageClass, serverSideEncryptionType, - serverSideEncryptionKmsKey, + serverSideEncryptionKmsKey != null && !serverSideEncryptionKmsKey.isEmpty() ? "****" : "", serverSideEncryptionBucketKey, - serverSideEncryptionEncryptionContext, expectedBucketOwner ); Suggestion importance[1-10]: 4 __ Why: Logging KMS key IDs and encryption context in plaintext debug logs could expose sensitive configuration. However, this is a debug-level log and the improved_code changes the format string and argument count inconsistently (removes serverSideEncryptionEncryptionContext from args but keeps the format string mismatched), reducing the suggestion's reliability.	Low

[github-actions]

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

[yzhang5-2023]

Hi @jed326 and @gbbafna — pinging you as the original merger and approver of #18312 since this is a backport of that PR.

CI Status Note

The gradle-check failure on this PR appears to be the known CI infrastructure issue with backport-branch PRs, not a problem with the code:

• The Jenkins job checks out origin/main of OpenSearch.git (not the PR's target branch) and completes in ~1m 51s without actually building the PR's code
• I verified this by re-submitting the same commit from a personal fork ([Backport 2.17] Add SSE-KMS encryption context and bucket owner verification to S3 repository plugin (re-submission) #21282, now closed) — same failure pattern
• Looking at recent precedent: other backport PRs to release/2.x branches show the same gradle-check=FAILURE pattern, e.g. [Backport 2.x] [Backport 2.19] Attempt to fix the jdt formatter error #20999 was merged with this exact failure when @cwperks noted "Merging so that 2.19.6 manifest generation is unblocked"

What's in this PR

This is a clean cherry-pick of #18312 to the target branch with version-specific conflict resolutions:

• Preserved the metadata field in UploadRequest.java (added in this version, not present in main)
• Kept the S3Error import in S3BlobContainer.java
• Skipped S3AsyncDeleteHelper.java change (file does not exist in this version)

All S3 plugin unit tests pass locally, and this backport has been validated in production at Atlassian.

Question

Could you please advise on the right path forward? Either:

1. Proceed with review and merge despite the gradle-check failure (as is common practice for backport PRs), or
2. Recommend the trigger-bot/label workflow if that would be preferred

Happy to make any adjustments. Thanks!