diff --git a/src/main/java/software/amazon/encryption/s3/S3AsyncEncryptionClient.java b/src/main/java/software/amazon/encryption/s3/S3AsyncEncryptionClient.java index 8426db12a..0524e6e41 100644 --- a/src/main/java/software/amazon/encryption/s3/S3AsyncEncryptionClient.java +++ b/src/main/java/software/amazon/encryption/s3/S3AsyncEncryptionClient.java @@ -5,7 +5,6 @@ import static software.amazon.encryption.s3.S3EncryptionClientUtilities.DEFAULT_BUFFER_SIZE_BYTES; import static software.amazon.encryption.s3.S3EncryptionClientUtilities.MAX_ALLOWED_BUFFER_SIZE_BYTES; import static software.amazon.encryption.s3.S3EncryptionClientUtilities.MIN_ALLOWED_BUFFER_SIZE_BYTES; -import static software.amazon.encryption.s3.internal.ApiNameVersion.API_NAME_INTERCEPTOR; import java.net.URI; import java.security.KeyPair; @@ -56,6 +55,7 @@ import software.amazon.awssdk.services.s3.model.UploadPartResponse; import software.amazon.awssdk.services.s3.multipart.MultipartConfiguration; import software.amazon.encryption.s3.algorithms.AlgorithmSuite; +import software.amazon.encryption.s3.internal.ApiNameVersion; import software.amazon.encryption.s3.internal.GetEncryptedObjectPipeline; import software.amazon.encryption.s3.internal.InstructionFileConfig; import software.amazon.encryption.s3.internal.NoRetriesAsyncRequestBody; @@ -240,12 +240,12 @@ public CompletableFuture getObject(GetObjectRequest getObjectRequest, @Override public CompletableFuture deleteObject(DeleteObjectRequest deleteObjectRequest) { final DeleteObjectRequest actualRequest = deleteObjectRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectRequest.overrideConfiguration())) .build(); final CompletableFuture response = _wrappedClient.deleteObject(actualRequest); final String instructionObjectKey = deleteObjectRequest.key() + ".instruction"; final CompletableFuture instructionResponse = _wrappedClient.deleteObject(builder -> builder - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectRequest.overrideConfiguration())) .bucket(deleteObjectRequest.bucket()) .key(instructionObjectKey)); // Delete the instruction file, then delete the object @@ -271,7 +271,7 @@ public CompletableFuture deleteObjects(DeleteObjectsReque // Add the original objects objectsToDelete.addAll(deleteObjectsRequest.delete().objects()); return _wrappedClient.deleteObjects(deleteObjectsRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectsRequest.overrideConfiguration())) .delete(builder -> builder.objects(objectsToDelete)) .build()); } diff --git a/src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java b/src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java index b530c99c7..3138b2b60 100644 --- a/src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java +++ b/src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java @@ -7,7 +7,6 @@ import static software.amazon.encryption.s3.S3EncryptionClientUtilities.MAX_ALLOWED_BUFFER_SIZE_BYTES; import static software.amazon.encryption.s3.S3EncryptionClientUtilities.MIN_ALLOWED_BUFFER_SIZE_BYTES; import static software.amazon.encryption.s3.S3EncryptionClientUtilities.instructionFileKeysToDelete; -import static software.amazon.encryption.s3.internal.ApiNameVersion.API_NAME_INTERCEPTOR; import java.io.IOException; import java.net.URI; @@ -75,6 +74,7 @@ import software.amazon.awssdk.services.s3.model.UploadPartRequest; import software.amazon.awssdk.services.s3.model.UploadPartResponse; import software.amazon.encryption.s3.algorithms.AlgorithmSuite; +import software.amazon.encryption.s3.internal.ApiNameVersion; import software.amazon.encryption.s3.internal.ContentMetadata; import software.amazon.encryption.s3.internal.ContentMetadataDecodingStrategy; import software.amazon.encryption.s3.internal.ContentMetadataEncodingStrategy; @@ -570,7 +570,7 @@ private T onAbort(UploadObjectObserver observer, T t) { public DeleteObjectResponse deleteObject(DeleteObjectRequest deleteObjectRequest) throws AwsServiceException, SdkClientException { DeleteObjectRequest actualRequest = deleteObjectRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectRequest.overrideConfiguration())) .build(); try { @@ -583,7 +583,7 @@ public DeleteObjectResponse deleteObject(DeleteObjectRequest deleteObjectRequest //# - DeleteObject MUST delete the associated instruction file using the default instruction file suffix. String instructionObjectKey = deleteObjectRequest.key() + DEFAULT_INSTRUCTION_FILE_SUFFIX; _wrappedAsyncClient.deleteObject(builder -> builder - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectRequest.overrideConfiguration())) .bucket(deleteObjectRequest.bucket()) .key(instructionObjectKey)).join(); // Return original deletion @@ -610,7 +610,7 @@ public DeleteObjectResponse deleteObject(DeleteObjectRequest deleteObjectRequest public DeleteObjectsResponse deleteObjects(DeleteObjectsRequest deleteObjectsRequest) throws AwsServiceException, SdkClientException { DeleteObjectsRequest actualRequest = deleteObjectsRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectsRequest.overrideConfiguration())) .build(); try { //= specification/s3-encryption/client.md#required-api-operations @@ -622,7 +622,7 @@ public DeleteObjectsResponse deleteObjects(DeleteObjectsRequest deleteObjectsReq //# - DeleteObjects MUST delete each of the corresponding instruction files using the default instruction file suffix. List deleteObjects = instructionFileKeysToDelete(deleteObjectsRequest); _wrappedAsyncClient.deleteObjects(DeleteObjectsRequest.builder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(deleteObjectsRequest.overrideConfiguration())) .bucket(deleteObjectsRequest.bucket()) .delete(builder -> builder.objects(deleteObjects)) .build()).join(); diff --git a/src/main/java/software/amazon/encryption/s3/internal/ApiNameVersion.java b/src/main/java/software/amazon/encryption/s3/internal/ApiNameVersion.java index 610f1e3ff..2bbeab65b 100644 --- a/src/main/java/software/amazon/encryption/s3/internal/ApiNameVersion.java +++ b/src/main/java/software/amazon/encryption/s3/internal/ApiNameVersion.java @@ -9,21 +9,36 @@ import java.io.IOException; import java.net.URL; import java.util.Enumeration; +import java.util.Optional; import java.util.Properties; -import java.util.function.Consumer; /** * Provides the information for the ApiName APIs for the AWS SDK */ public class ApiNameVersion { private static final ApiName API_NAME = ApiNameVersion.apiNameWithVersion(); - // This is used in overrideConfiguration - public static final Consumer API_NAME_INTERCEPTOR = - builder -> builder.addApiName(API_NAME); public static final String NAME = "AmazonS3Encrypt"; public static final String API_VERSION_UNKNOWN = "4-unknown"; + /** + * Returns an {@link AwsRequestOverrideConfiguration} which includes the S3EC API name while + * preserving any override configuration the caller already set on their request. This ensures + * caller-supplied configuration (e.g. custom headers, credentials providers, or signer + * overrides) is not dropped when the S3EC adds its API name. + * + * @param existingOverrideConfiguration the override configuration from the original request, if any + * @return an override configuration containing the S3EC API name merged with the existing configuration + */ + public static AwsRequestOverrideConfiguration addApiNameToOverrideConfiguration( + Optional existingOverrideConfiguration) { + return existingOverrideConfiguration + .map(AwsRequestOverrideConfiguration::toBuilder) + .orElseGet(AwsRequestOverrideConfiguration::builder) + .addApiName(API_NAME) + .build(); + } + public static ApiName apiNameWithVersion() { return ApiName.builder() .name(NAME) diff --git a/src/main/java/software/amazon/encryption/s3/internal/GetEncryptedObjectPipeline.java b/src/main/java/software/amazon/encryption/s3/internal/GetEncryptedObjectPipeline.java index 92fd59019..665aab536 100644 --- a/src/main/java/software/amazon/encryption/s3/internal/GetEncryptedObjectPipeline.java +++ b/src/main/java/software/amazon/encryption/s3/internal/GetEncryptedObjectPipeline.java @@ -2,8 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 package software.amazon.encryption.s3.internal; -import static software.amazon.encryption.s3.internal.ApiNameVersion.API_NAME_INTERCEPTOR; - import java.nio.ByteBuffer; import java.util.Arrays; import java.util.Collections; @@ -64,7 +62,7 @@ public CompletableFuture getObject(GetObjectRequest getObjectRequest, Asy //# and end of the cipher blocks for the given range. String cryptoRange = RangedGetUtils.getCryptoRangeAsString(getObjectRequest.range()); GetObjectRequest adjustedRangeRequest = getObjectRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(getObjectRequest.overrideConfiguration())) .range(cryptoRange) .build(); if (!_enableLegacyUnauthenticatedModes && getObjectRequest.range() != null) { diff --git a/src/main/java/software/amazon/encryption/s3/internal/MultipartUploadObjectPipeline.java b/src/main/java/software/amazon/encryption/s3/internal/MultipartUploadObjectPipeline.java index 01a5e49b0..f1d685192 100644 --- a/src/main/java/software/amazon/encryption/s3/internal/MultipartUploadObjectPipeline.java +++ b/src/main/java/software/amazon/encryption/s3/internal/MultipartUploadObjectPipeline.java @@ -2,8 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 package software.amazon.encryption.s3.internal; -import static software.amazon.encryption.s3.internal.ApiNameVersion.API_NAME_INTERCEPTOR; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -80,7 +78,7 @@ public CreateMultipartUploadResponse createMultipartUpload(CreateMultipartUpload final byte[] contentIV = materials.algorithmSuite().isCommitting() ? materials.messageId() : materials.iv(); CreateMultipartUploadRequest createMpuRequest = _contentMetadataEncodingStrategy.encodeMetadata(materials, contentIV, request); request = createMpuRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(createMpuRequest.overrideConfiguration())) .build(); //= specification/s3-encryption/client.md#optional-api-operations @@ -135,7 +133,7 @@ public UploadPartResponse uploadPart(UploadPartRequest request, RequestBody requ // Once we have (a valid) ciphertext length, set the request contentLength UploadPartRequest actualRequest = request.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(request.overrideConfiguration())) .contentLength(ciphertextLength) .build(); @@ -201,7 +199,7 @@ public CompleteMultipartUploadResponse completeMultipartUpload(CompleteMultipart } CompleteMultipartUploadRequest actualRequest = request.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(request.overrideConfiguration())) .build(); //= specification/s3-encryption/client.md#optional-api-operations @@ -215,7 +213,7 @@ public CompleteMultipartUploadResponse completeMultipartUpload(CompleteMultipart public AbortMultipartUploadResponse abortMultipartUpload(AbortMultipartUploadRequest request) { _multipartUploadMaterials.remove(request.uploadId()); AbortMultipartUploadRequest actualRequest = request.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(request.overrideConfiguration())) .build(); //= specification/s3-encryption/client.md#optional-api-operations //# - AbortMultipartUpload MUST abort the multipart upload. diff --git a/src/main/java/software/amazon/encryption/s3/internal/PutEncryptedObjectPipeline.java b/src/main/java/software/amazon/encryption/s3/internal/PutEncryptedObjectPipeline.java index 96be243d3..0dbd35998 100644 --- a/src/main/java/software/amazon/encryption/s3/internal/PutEncryptedObjectPipeline.java +++ b/src/main/java/software/amazon/encryption/s3/internal/PutEncryptedObjectPipeline.java @@ -2,8 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 package software.amazon.encryption.s3.internal; -import static software.amazon.encryption.s3.internal.ApiNameVersion.API_NAME_INTERCEPTOR; - import java.security.SecureRandom; import java.util.concurrent.CompletableFuture; @@ -81,7 +79,7 @@ public CompletableFuture putObject(PutObjectRequest request, final byte[] contentIV = materials.algorithmSuite().isCommitting() ? materials.messageId() : materials.iv(); PutObjectRequest modifiedRequest = _contentMetadataEncodingStrategy.encodeMetadata(materials, contentIV, request); PutObjectRequest encryptedPutRequest = modifiedRequest.toBuilder() - .overrideConfiguration(API_NAME_INTERCEPTOR) + .overrideConfiguration(ApiNameVersion.addApiNameToOverrideConfiguration(request.overrideConfiguration())) .contentLength(encryptedContent.getCiphertextLength()) .build(); return _s3AsyncClient.putObject(encryptedPutRequest, new NoRetriesAsyncRequestBody(encryptedContent.getAsyncCiphertext())); diff --git a/src/test/java/software/amazon/encryption/s3/S3EncryptionClientRequestOverrideConfigurationTest.java b/src/test/java/software/amazon/encryption/s3/S3EncryptionClientRequestOverrideConfigurationTest.java new file mode 100644 index 000000000..99c000e4e --- /dev/null +++ b/src/test/java/software/amazon/encryption/s3/S3EncryptionClientRequestOverrideConfigurationTest.java @@ -0,0 +1,185 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package software.amazon.encryption.s3; + +import org.junitpioneer.jupiter.RetryingTest; +import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration; +import software.amazon.awssdk.core.async.AsyncRequestBody; +import software.amazon.awssdk.core.async.AsyncResponseTransformer; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.interceptor.Context; +import software.amazon.awssdk.core.interceptor.ExecutionAttributes; +import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; +import software.amazon.awssdk.core.ResponseBytes; +import software.amazon.awssdk.http.SdkHttpMethod; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3AsyncClient; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; + +import javax.crypto.KeyGenerator; +import javax.crypto.SecretKey; +import java.security.NoSuchAlgorithmException; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static software.amazon.encryption.s3.utils.S3EncryptionClientTestResources.BUCKET; +import static software.amazon.encryption.s3.utils.S3EncryptionClientTestResources.S3_REGION; +import static software.amazon.encryption.s3.utils.S3EncryptionClientTestResources.appendTestSuffix; +import static software.amazon.encryption.s3.utils.S3EncryptionClientTestResources.deleteObject; + +/** + * Verifies that the S3EC preserves the per-request {@link AwsRequestOverrideConfiguration} set by + * the caller. The client adds its own API name to every request's override configuration; these + * tests ensure that doing so does not discard a caller-supplied override configuration (such as a + * custom header, credentials provider, or signer override). + *

+ * Each test attaches a custom header via the request-level override configuration and inspects the + * request that actually reaches the wrapped client using an {@link ExecutionInterceptor}. + */ +public class S3EncryptionClientRequestOverrideConfigurationTest { + + private static final String CUSTOM_HEADER_NAME = "x-amz-meta-s3ec-override-repro"; + private static final String CUSTOM_HEADER_VALUE = "custom-value"; + + private static SecretKey AES_KEY; + + private static SecretKey aesKey() throws NoSuchAlgorithmException { + if (AES_KEY == null) { + KeyGenerator keyGen = KeyGenerator.getInstance("AES"); + keyGen.init(256); + AES_KEY = keyGen.generateKey(); + } + return AES_KEY; + } + + /** + * Captures the outgoing HTTP request for a given method so the test can assert on the + * headers that actually reach the wire. + */ + private static class CapturingInterceptor implements ExecutionInterceptor { + private final SdkHttpMethod methodToCapture; + private final AtomicReference capturedRequest = new AtomicReference<>(); + + CapturingInterceptor(SdkHttpMethod methodToCapture) { + this.methodToCapture = methodToCapture; + } + + @Override + public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) { + SdkHttpRequest request = context.httpRequest(); + if (request.method() == methodToCapture) { + capturedRequest.set(request); + } + } + + SdkHttpRequest captured() { + return capturedRequest.get(); + } + } + + @RetryingTest(3) + public void putObjectPreservesRequestOverrideConfiguration() throws NoSuchAlgorithmException { + final String objectKey = appendTestSuffix("override-config-repro-put"); + + CapturingInterceptor interceptor = new CapturingInterceptor(SdkHttpMethod.PUT); + S3AsyncClient wrappedAsyncClient = S3AsyncClient.builder() + .region(Region.of(S3_REGION.toString())) + .overrideConfiguration(ClientOverrideConfiguration.builder() + .addExecutionInterceptor(interceptor) + .build()) + .build(); + + S3AsyncClient s3Client = S3AsyncEncryptionClient.builderV4() + .wrappedClient(wrappedAsyncClient) + .aesKey(aesKey()) + .build(); + + final String input = "PutObjectOverrideConfig"; + final AwsRequestOverrideConfiguration overrideConfig = AwsRequestOverrideConfiguration.builder() + .putHeader(CUSTOM_HEADER_NAME, CUSTOM_HEADER_VALUE) + .build(); + + try { + s3Client.putObject(builder -> builder + .bucket(BUCKET) + .key(objectKey) + .overrideConfiguration(overrideConfig) + .build(), + AsyncRequestBody.fromString(input)).join(); + + SdkHttpRequest sentRequest = interceptor.captured(); + assertNotNull(sentRequest, "No PutObject request was captured by the interceptor"); + + // The S3EC API name must still be present (existing behavior must be preserved). + Optional userAgent = sentRequest.firstMatchingHeader("User-Agent"); + assertTrue(userAgent.isPresent() && userAgent.get().contains("AmazonS3Encrypt"), + "Expected the S3EC API name to be present in the User-Agent header"); + + // The caller-provided override configuration (custom header) must NOT be dropped. + Optional customHeader = sentRequest.firstMatchingHeader(CUSTOM_HEADER_NAME); + assertTrue(customHeader.isPresent(), + "Caller-provided request override configuration (custom header) was dropped on PutObject"); + assertEquals(CUSTOM_HEADER_VALUE, customHeader.get()); + } finally { + deleteObject(BUCKET, objectKey, s3Client); + s3Client.close(); + wrappedAsyncClient.close(); + } + } + + @RetryingTest(3) + public void getObjectPreservesRequestOverrideConfiguration() throws NoSuchAlgorithmException { + final String objectKey = appendTestSuffix("override-config-repro-get"); + + CapturingInterceptor interceptor = new CapturingInterceptor(SdkHttpMethod.GET); + S3AsyncClient wrappedAsyncClient = S3AsyncClient.builder() + .region(Region.of(S3_REGION.toString())) + .overrideConfiguration(ClientOverrideConfiguration.builder() + .addExecutionInterceptor(interceptor) + .build()) + .build(); + + S3AsyncClient s3Client = S3AsyncEncryptionClient.builderV4() + .wrappedClient(wrappedAsyncClient) + .aesKey(aesKey()) + .build(); + + final String input = "GetObjectOverrideConfig"; + final AwsRequestOverrideConfiguration overrideConfig = AwsRequestOverrideConfiguration.builder() + .putHeader(CUSTOM_HEADER_NAME, CUSTOM_HEADER_VALUE) + .build(); + + try { + // Put without an override so the capturing interceptor only sees the GET below. + s3Client.putObject(builder -> builder + .bucket(BUCKET) + .key(objectKey) + .build(), + AsyncRequestBody.fromString(input)).join(); + + ResponseBytes objectResponse = s3Client.getObject(builder -> builder + .bucket(BUCKET) + .key(objectKey) + .overrideConfiguration(overrideConfig) + .build(), + AsyncResponseTransformer.toBytes()).join(); + assertEquals(input, objectResponse.asUtf8String()); + + SdkHttpRequest sentRequest = interceptor.captured(); + assertNotNull(sentRequest, "No GetObject request was captured by the interceptor"); + + Optional customHeader = sentRequest.firstMatchingHeader(CUSTOM_HEADER_NAME); + assertTrue(customHeader.isPresent(), + "Caller-provided request override configuration (custom header) was dropped on GetObject"); + assertEquals(CUSTOM_HEADER_VALUE, customHeader.get()); + } finally { + deleteObject(BUCKET, objectKey, s3Client); + s3Client.close(); + wrappedAsyncClient.close(); + } + } +}