Skip to content

feat: put object with instruction file configured #466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8e51328
step 1 - refactoring
kessplas Nov 25, 2024
898c55e
wip - instruction file puts
kessplas Dec 2, 2024
fd64f1c
cleanup
kessplas Dec 11, 2024
95b3b5a
Merge branch 'main' of github.com:aws/amazon-s3-encryption-client-jav…
kessplas Apr 14, 2025
3546538
Merge branch 'main' of github.com:aws/amazon-s3-encryption-client-jav…
kessplas Apr 14, 2025
ed761d9
more tests in new class
kessplas Apr 29, 2025
a34b347
Merge branch 'main' of github.com:aws/amazon-s3-encryption-client-jav…
kessplas May 9, 2025
d6da2cf
v3 to v2 tests for default client
kessplas May 9, 2025
60feafd
Merge branch 'main' of github.com:aws/amazon-s3-encryption-client-jav…
kessplas May 23, 2025
288ee65
Fixed nullptr exception in code
May 27, 2025
9c1c50d
testing whether v2 does or doesn't translate additional parameters fo…
May 28, 2025
ae6b6b4
Fixed nullptr errors for MultipartUploadObjectPipeline regarding _ins…
May 28, 2025
c9378cf
Added another convertRequest method to convert CreateMultipartUploadR…
May 28, 2025
ef353d7
Added test cases for converting MultipartUploadRequests
May 28, 2025
fbdbe92
Added tests for S3EncryptonClient with Instruction File + Multipart e…
May 29, 2025
9e25d8f
added .instructionFileConfig for PutEncryptedObjectPipeline builder
May 30, 2025
7bf259c
cleaned up ContentMetadataEncodingStrategy class
May 30, 2025
ff5e99c
Added test cases to S3AsyncEncryptionClientTest regarging MPU + instr…
May 30, 2025
a396a7c
fixed import issues
May 30, 2025
b1c0898
removed debugging lines
May 30, 2025
b4b2047
Replaced passing in ARN of my personal testing bucket with the BUCKET…
May 30, 2025
b9ae158
removed unused import
May 30, 2025
4f51c42
fixed all changes requested from PR
Jun 3, 2025
bf2be6e
Removed HelloWorldProgramExample.java
Jun 3, 2025
02f6f48
Fixed the second PR round of reviews
Jun 6, 2025
c319d44
testMultipartPutWithInstructionFile() should be working now
Jun 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public class S3AsyncEncryptionClient extends DelegatingS3AsyncClient {
private final boolean _enableDelayedAuthenticationMode;
private final boolean _enableMultipartPutObject;
private final long _bufferSize;
private InstructionFileConfig _instructionFileConfig;
private final InstructionFileConfig _instructionFileConfig;

private S3AsyncEncryptionClient(Builder builder) {
super(builder._wrappedClient);
Expand Down Expand Up @@ -151,6 +151,7 @@ public CompletableFuture<PutObjectResponse> putObject(PutObjectRequest putObject
.s3AsyncClient(_wrappedClient)
.cryptoMaterialsManager(_cryptoMaterialsManager)
.secureRandom(_secureRandom)
.instructionFileConfig(_instructionFileConfig)
.build();

return pipeline.putObject(putObjectRequest, requestBody);
Expand All @@ -169,6 +170,7 @@ private CompletableFuture<PutObjectResponse> multipartPutObject(PutObjectRequest
.s3AsyncClient(mpuClient)
.cryptoMaterialsManager(_cryptoMaterialsManager)
.secureRandom(_secureRandom)
.instructionFileConfig(_instructionFileConfig)
.build();
// Ensures parts are not retried to avoid corrupting ciphertext
AsyncRequestBody noRetryBody = new NoRetriesAsyncRequestBody(requestBody);
Expand Down Expand Up @@ -289,6 +291,7 @@ public void close() {
_instructionFileConfig.closeClient();
}


// This is very similar to the S3EncryptionClient builder
// Make sure to keep both clients in mind when adding new builder options
public static class Builder implements S3AsyncClientBuilder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ public PutObjectResponse putObject(PutObjectRequest putObjectRequest, RequestBod
.s3AsyncClient(_wrappedAsyncClient)
.cryptoMaterialsManager(_cryptoMaterialsManager)
.secureRandom(_secureRandom)
.instructionFileConfig(_instructionFileConfig)
.build();

ExecutorService singleThreadExecutor = Executors.newSingleThreadExecutor();
Expand Down Expand Up @@ -1164,6 +1165,7 @@ public S3EncryptionClient build() {
.s3AsyncClient(_wrappedAsyncClient)
.cryptoMaterialsManager(_cryptoMaterialsManager)
.secureRandom(_secureRandom)
.instructionFileConfig(_instructionFileConfig)
.build();

return new S3EncryptionClient(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ private ContentMetadata readFromMap(Map<String, String> metadata, GetObjectRespo

public ContentMetadata decode(GetObjectRequest request, GetObjectResponse response) {
Map<String, String> metadata = response.metadata();
ContentMetadataDecodingStrategy strategy;
if (metadata != null
&& metadata.containsKey(MetadataKeyConstants.CONTENT_IV)
&& (metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,94 @@
// SPDX-License-Identifier: Apache-2.0
package software.amazon.encryption.s3.internal;

import software.amazon.awssdk.protocols.jsoncore.JsonWriter;
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.encryption.s3.S3EncryptionClientException;
import software.amazon.encryption.s3.materials.EncryptedDataKey;
import software.amazon.encryption.s3.materials.EncryptionMaterials;

import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.HashMap;
import java.util.Map;

@FunctionalInterface
public interface ContentMetadataEncodingStrategy {
public class ContentMetadataEncodingStrategy {

Map<String, String> encodeMetadata(EncryptionMaterials materials, byte[] iv,
Map<String, String> metadata);
private static final Base64.Encoder ENCODER = Base64.getEncoder();
private final InstructionFileConfig _instructionFileConfig;

public ContentMetadataEncodingStrategy(InstructionFileConfig instructionFileConfig) {
_instructionFileConfig = instructionFileConfig;
}

public PutObjectRequest encodeMetadata(EncryptionMaterials materials, byte[] iv, PutObjectRequest putObjectRequest) {
if (_instructionFileConfig.isInstructionFilePutEnabled()) {
final String metadataString = metadataToString(materials, iv);
_instructionFileConfig.putInstructionFile(putObjectRequest, metadataString);
// the original request object is returned as-is
return putObjectRequest;
} else {
Map<String, String> newMetadata = addMetadataToMap(putObjectRequest.metadata(), materials, iv);
return putObjectRequest.toBuilder()
.metadata(newMetadata)
.build();
}
}

public CreateMultipartUploadRequest encodeMetadata(EncryptionMaterials materials, byte[] iv, CreateMultipartUploadRequest createMultipartUploadRequest) {
if(_instructionFileConfig.isInstructionFilePutEnabled()) {
final String metadataString = metadataToString(materials, iv);
PutObjectRequest putObjectRequest = ConvertSDKRequests.convertRequest(createMultipartUploadRequest);
_instructionFileConfig.putInstructionFile(putObjectRequest, metadataString);
// the original request object is returned as-is
return createMultipartUploadRequest;
} else {
Map<String, String> newMetadata = addMetadataToMap(createMultipartUploadRequest.metadata(), materials, iv);
return createMultipartUploadRequest.toBuilder()
.metadata(newMetadata)
.build();
}
}
private String metadataToString(EncryptionMaterials materials, byte[] iv) {
// this is just the metadata map serialized as JSON
// so first get the Map
final Map<String, String> metadataMap = addMetadataToMap(new HashMap<>(), materials, iv);
// then serialize it
try (JsonWriter jsonWriter = JsonWriter.create()) {
jsonWriter.writeStartObject();
for (Map.Entry<String, String> entry : metadataMap.entrySet()) {
jsonWriter.writeFieldName(entry.getKey()).writeValue(entry.getValue());
}
jsonWriter.writeEndObject();

return new String(jsonWriter.getBytes(), StandardCharsets.UTF_8);
} catch (JsonWriter.JsonGenerationException e) {
throw new S3EncryptionClientException("Cannot serialize materials to JSON.", e);
}
}

private Map<String, String> addMetadataToMap(Map<String, String> map, EncryptionMaterials materials, byte[] iv) {
Map<String, String> metadata = new HashMap<>(map);
EncryptedDataKey edk = materials.encryptedDataKeys().get(0);
metadata.put(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2, ENCODER.encodeToString(edk.encryptedDatakey()));
metadata.put(MetadataKeyConstants.CONTENT_IV, ENCODER.encodeToString(iv));
metadata.put(MetadataKeyConstants.CONTENT_CIPHER, materials.algorithmSuite().cipherName());
metadata.put(MetadataKeyConstants.CONTENT_CIPHER_TAG_LENGTH, Integer.toString(materials.algorithmSuite().cipherTagLengthBits()));
metadata.put(MetadataKeyConstants.ENCRYPTED_DATA_KEY_ALGORITHM, new String(edk.keyProviderInfo(), StandardCharsets.UTF_8));

try (JsonWriter jsonWriter = JsonWriter.create()) {
jsonWriter.writeStartObject();
for (Map.Entry<String, String> entry : materials.encryptionContext().entrySet()) {
jsonWriter.writeFieldName(entry.getKey()).writeValue(entry.getValue());
}
jsonWriter.writeEndObject();

String jsonEncryptionContext = new String(jsonWriter.getBytes(), StandardCharsets.UTF_8);
metadata.put(MetadataKeyConstants.ENCRYPTED_DATA_KEY_CONTEXT, jsonEncryptionContext);
} catch (JsonWriter.JsonGenerationException e) {
throw new S3EncryptionClientException("Cannot serialize encryption context to JSON.", e);
}
return metadata;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,146 @@

public class ConvertSDKRequests {

/**
* Converts a CreateMultipartUploadRequest to a PutObjectRequest. This conversion is necessary when
* Instruction File PutObject is enabled and a multipart upload is performed.The method copies all the
* relevant fields from the CreateMultipartUploadRequest to the PutObjectRequest.
* @param request The CreateMultipartUploadRequest to convert
* @return The converted PutObjectRequest
* @throws IllegalArgumentException if the request contains an invalid field
*/
public static PutObjectRequest convertRequest(CreateMultipartUploadRequest request) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We should add some comments here, i.e. that this is used to set the optional fields for Instruction File puts when the request is a multipart upload, because the instruction file is Put with an ordinary PutObject request.

And likewise for the other convertRequest method which is used to set the optional fields on high-level Multipart PutObject requests.

final PutObjectRequest.Builder output = PutObjectRequest.builder();
request
.toBuilder()
.sdkFields()
.forEach(f -> {
final Object value = f.getValueOrDefault(request);
if (value != null) {
switch (f.memberName()) {
case "ACL":
output.acl((String) value);
break;
case "Bucket":
output.bucket((String) value);
break;
case "BucketKeyEnabled":
output.bucketKeyEnabled((Boolean) value);
break;
case "CacheControl":
output.cacheControl((String) value);
break;
case "ChecksumAlgorithm":
output.checksumAlgorithm((String) value);
break;
Comment on lines +44 to +46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some field checksumType in the CreateMPURequest: https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/services/s3/model/CreateMultipartUploadRequest.html
There's no corresponding field in PutObjectRequest, but the various checksum algorithms are suspicious (checksumCRC32, checksumCRC32C,etc.).
I don't know how this works, but I'm wondering if we might need to set one of these fields depending on the value of checksumType. Do you know if that's the case?

case "ContentDisposition":
assert value instanceof String;
output.contentDisposition((String) value);
break;
case "ContentEncoding":
output.contentEncoding((String) value);
break;
case "ContentLanguage":
output.contentLanguage((String) value);
break;
case "ContentType":
output.contentType((String) value);
break;
case "ExpectedBucketOwner":
output.expectedBucketOwner((String) value);
break;
case "Expires":
output.expires((Instant) value);
break;
case "GrantFullControl":
output.grantFullControl((String) value);
break;
case "GrantRead":
output.grantRead((String) value);
break;
case "GrantReadACP":
output.grantReadACP((String) value);
break;
case "GrantWriteACP":
output.grantWriteACP((String) value);
break;
case "Key":
output.key((String) value);
break;
case "Metadata":
if (!isStringStringMap(value)) {
throw new IllegalArgumentException("Metadata must be a Map<String, String>");
}
@SuppressWarnings("unchecked")
Map<String, String> metadata = (Map<String, String>) value;
output.metadata(metadata);
break;
case "ObjectLockLegalHoldStatus":
output.objectLockLegalHoldStatus((String) value);
break;
case "ObjectLockMode":
output.objectLockMode((String) value);
break;
Comment on lines +92 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking: we don't need to copy the AsString methods for fields like these, right?

case "ObjectLockRetainUntilDate":
output.objectLockRetainUntilDate((Instant) value);
break;
case "RequestPayer":
output.requestPayer((String) value);
break;
case "ServerSideEncryption":
output.serverSideEncryption((String) value);
break;
case "SSECustomerAlgorithm":
output.sseCustomerAlgorithm((String) value);
break;
case "SSECustomerKey":
output.sseCustomerKey((String) value);
break;
Comment on lines +107 to +109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need sseCustomerKeyMD5?

case "SSEKMSEncryptionContext":
output.ssekmsEncryptionContext((String) value);
break;
case "SSEKMSKeyId":
output.ssekmsKeyId((String) value);
break;
case "StorageClass":
output.storageClass((String) value);
break;
case "Tagging":
output.tagging((String) value);
break;
case "WebsiteRedirectLocation":
output.websiteRedirectLocation((String) value);
break;
default:
// Rather than silently dropping the value,
// we loudly signal that we don't know how to handle this field.
throw new IllegalArgumentException(
f.memberName() + " is an unknown field. " +
"The S3 Encryption Client does not recognize this option and cannot set it on the PutObjectRequest." +
"This may be a new S3 feature." +
"Please report this to the Amazon S3 Encryption Client for Java: " +
"https://github.com/aws/amazon-s3-encryption-client-java/issues." +
"To work around this issue, you can disable Instruction File on PutObject or disable" +
"multi part upload, or use the Async client, or not set this value on PutObject." +
"You may be able to update this value after the PutObject request completes."
);
}
}
});
return output
// OverrideConfiguration is not as SDKField but still needs to be supported
.overrideConfiguration(request.overrideConfiguration().orElse(null))
.build();
}
/**
* Converts a PutObjectRequest to CreateMultipartUploadRequest.This conversion is necessary to convert an
* original PutObjectRequest into a CreateMultipartUploadRequest to initiate the
* multipart upload while maintaining the original request's configuration.
* @param request The PutObjectRequest to convert
* @return The converted CreateMultipartUploadRequest
* @throws IllegalArgumentException if the request contains an invalid field
*/
public static CreateMultipartUploadRequest convertRequest(PutObjectRequest request) {

final CreateMultipartUploadRequest.Builder output = CreateMultipartUploadRequest.builder();
request
.toBuilder()
Expand Down Expand Up @@ -126,7 +264,7 @@ public static CreateMultipartUploadRequest convertRequest(PutObjectRequest reque
// Rather than silently dropping the value,
// we loudly signal that we don't know how to handle this field.
throw new IllegalArgumentException(
f.locationName() + " is an unknown field. " +
f.memberName() + " is an unknown field. " +
"The S3 Encryption Client does not recognize this option and cannot set it on the CreateMultipartUploadRequest." +
"This may be a new S3 feature." +
"Please report this to the Amazon S3 Encryption Client for Java: " +
Expand Down
Loading