Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Ignore artifacts:
build
coverage

Large diffs are not rendered by default.

128 changes: 125 additions & 3 deletions src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,30 @@
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.ContentMetadata;
import software.amazon.encryption.s3.internal.ContentMetadataDecodingStrategy;
import software.amazon.encryption.s3.internal.ContentMetadataEncodingStrategy;
import software.amazon.encryption.s3.internal.ConvertSDKRequests;
import software.amazon.encryption.s3.internal.GetEncryptedObjectPipeline;
import software.amazon.encryption.s3.internal.InstructionFileConfig;
import software.amazon.encryption.s3.internal.MultiFileOutputStream;
import software.amazon.encryption.s3.internal.MultipartUploadObjectPipeline;
import software.amazon.encryption.s3.internal.PutEncryptedObjectPipeline;
import software.amazon.encryption.s3.internal.ReEncryptInstructionFileRequest;
import software.amazon.encryption.s3.internal.ReEncryptInstructionFileResponse;
import software.amazon.encryption.s3.internal.UploadObjectObserver;
import software.amazon.encryption.s3.materials.AesKeyring;
import software.amazon.encryption.s3.materials.CryptographicMaterialsManager;
import software.amazon.encryption.s3.materials.DecryptMaterialsRequest;
import software.amazon.encryption.s3.materials.DecryptionMaterials;
import software.amazon.encryption.s3.materials.DefaultCryptoMaterialsManager;
import software.amazon.encryption.s3.materials.EncryptedDataKey;
import software.amazon.encryption.s3.materials.EncryptionMaterials;
import software.amazon.encryption.s3.materials.Keyring;
import software.amazon.encryption.s3.materials.KmsKeyring;
import software.amazon.encryption.s3.materials.MultipartConfiguration;
import software.amazon.encryption.s3.materials.PartialRsaKeyPair;
import software.amazon.encryption.s3.materials.RawKeyring;
import software.amazon.encryption.s3.materials.RsaKeyring;
import software.amazon.encryption.s3.materials.S3Keyring;

Expand All @@ -71,6 +81,7 @@
import java.security.Provider;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -83,7 +94,8 @@
import java.util.function.Consumer;

import static software.amazon.encryption.s3.S3EncryptionClientUtilities.DEFAULT_BUFFER_SIZE_BYTES;
import static software.amazon.encryption.s3.S3EncryptionClientUtilities.INSTRUCTION_FILE_SUFFIX;

import static software.amazon.encryption.s3.S3EncryptionClientUtilities.DEFAULT_INSTRUCTION_FILE_SUFFIX;
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;
Expand All @@ -99,6 +111,9 @@ public class S3EncryptionClient extends DelegatingS3Client {
public static final ExecutionAttribute<Map<String, String>> ENCRYPTION_CONTEXT = new ExecutionAttribute<>("EncryptionContext");
public static final ExecutionAttribute<MultipartConfiguration> CONFIGURATION = new ExecutionAttribute<>("MultipartConfiguration");

//Used for specifying custom instruction file suffix on a per-request basis
public static final ExecutionAttribute<String> CUSTOM_INSTRUCTION_FILE_SUFFIX = new ExecutionAttribute<>("CustomInstructionFileSuffix");

private final S3Client _wrappedClient;
private final S3AsyncClient _wrappedAsyncClient;
private final CryptographicMaterialsManager _cryptoMaterialsManager;
Expand Down Expand Up @@ -145,6 +160,18 @@ public static Consumer<AwsRequestOverrideConfiguration.Builder> withAdditionalCo
builder.putExecutionAttribute(S3EncryptionClient.ENCRYPTION_CONTEXT, encryptionContext);
}

/**
* Attaches a custom instruction file suffix to a request. Must be used as a parameter to
* {@link S3Request#overrideConfiguration()} in the request.
* This allows specifying a custom suffix for the instruction file on a per-request basis.
* @param customInstructionFileSuffix the custom suffix to use for the instruction file.
* @return Consumer for use in overrideConfiguration()
*/
public static Consumer<AwsRequestOverrideConfiguration.Builder> withCustomInstructionFileSuffix(String customInstructionFileSuffix) {
return builder ->
builder.putExecutionAttribute(S3EncryptionClient.CUSTOM_INSTRUCTION_FILE_SUFFIX, customInstructionFileSuffix);
}

/**
* Attaches multipart configuration to a request. Must be used as a parameter to
* {@link S3Request#overrideConfiguration()} in the request.
Expand All @@ -156,7 +183,6 @@ public static Consumer<AwsRequestOverrideConfiguration.Builder> withAdditionalCo
builder.putExecutionAttribute(S3EncryptionClient.CONFIGURATION, multipartConfiguration);
}


/**
* Attaches encryption context and multipart configuration to a request.
* * Must be used as a parameter to
Expand All @@ -174,6 +200,102 @@ public static Consumer<AwsRequestOverrideConfiguration.Builder> withAdditionalCo
.putExecutionAttribute(S3EncryptionClient.CONFIGURATION, multipartConfiguration);
}

/**
* Re-encrypts an instruction file with a new keyring while preserving the original encrypted object in S3.
* This enables:
* 1. Key rotation by updating instruction file metadata without re-encrypting object content
* 2. Sharing encrypted objects with partners by creating new instruction files with a custom suffix using their public keys
* <p>
* Key rotation scenarios:
* - Legacy to V3: Can rotate same wrapping key from legacy wrapping algorithms to fully supported wrapping algorithms
* - Within V3: When rotating the wrapping key, the new keyring must be different from the current keyring
* - Enforce Rotation: When enabled, ensures old keyring cannot decrypt data encrypted by new keyring
*
* @param reEncryptInstructionFileRequest the request containing bucket, object key, new keyring, and optional instruction file suffix
* @return ReEncryptInstructionFileResponse containing the bucket, object key, and instruction file suffix used
* @throws S3EncryptionClientException if the new keyring has the same materials description as the current one
*/
public ReEncryptInstructionFileResponse reEncryptInstructionFile(ReEncryptInstructionFileRequest reEncryptInstructionFileRequest) {
if (!_instructionFileConfig.isInstructionFilePutEnabled()) {
throw new S3EncryptionClientException("Instruction file put operations must be enabled to re-encrypt instruction files");
}

//Build request to retrieve the encrypted object and its associated instruction file
final GetObjectRequest request = GetObjectRequest.builder()
.bucket(reEncryptInstructionFileRequest.bucket())
.key(reEncryptInstructionFileRequest.key())
.build();

ResponseInputStream<GetObjectResponse> response = this.getObject(request);
ContentMetadataDecodingStrategy decodingStrategy = new ContentMetadataDecodingStrategy(_instructionFileConfig);
ContentMetadata contentMetadata = decodingStrategy.decode(request, response.response());

//Extract cryptographic parameters from the current instruction file that MUST be preserved during re-encryption
final AlgorithmSuite algorithmSuite = contentMetadata.algorithmSuite();
final EncryptedDataKey originalEncryptedDataKey = contentMetadata.encryptedDataKey();
final Map<String, String> currentKeyringMaterialsDescription = contentMetadata.encryptedDataKeyMatDescOrContext();
final byte[] iv = contentMetadata.contentIv();

//Decrypt the data key using the current keyring
DecryptionMaterials decryptedMaterials = this._cryptoMaterialsManager.decryptMaterials(
DecryptMaterialsRequest.builder()
.algorithmSuite(algorithmSuite)
.encryptedDataKeys(Collections.singletonList(originalEncryptedDataKey))
.s3Request(request)
.build()
);

final byte[] plaintextDataKey = decryptedMaterials.plaintextDataKey();

//Prepare encryption materials with the decrypted data key
EncryptionMaterials encryptionMaterials = EncryptionMaterials.builder()
.algorithmSuite(algorithmSuite)
.plaintextDataKey(plaintextDataKey)
.s3Request(request)
.build();

//Re-encrypt the data key with the new keyring while preserving other cryptographic parameters
RawKeyring newKeyring = reEncryptInstructionFileRequest.newKeyring();
EncryptionMaterials encryptedMaterials = newKeyring.onEncrypt(encryptionMaterials);

final Map<String, String> newMaterialsDescription = encryptedMaterials.materialsDescription().getMaterialsDescription();
//Validate that the new keyring has different materials description than the old keyring
if (newMaterialsDescription.equals(currentKeyringMaterialsDescription)) {
throw new S3EncryptionClientException("New keyring must have new materials description!");
}

// If enforceRotation is set to true, ensure that the old keyring cannot decrypt the newly encrypted data key
if (reEncryptInstructionFileRequest.enforceRotation()) {
enforceRotation(encryptedMaterials, 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: IMO, enforce doesn't seem right -- to me, it implies this method is taking some action to perform the rotation. But it's really just validating that the rotation was done correctly.
I think verify or validate would have been better, but it seems like a fair amount of work to change. I'll leave it up to you/Kess to decide if this rename makes sense and/or is worth the effort to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kess and I discussed this and we decided to keep enforceRotation since it is an optional attribute in the reEncryptInstructionFile request and customers are only interacting with it by setting the boolean value. The internal method "enforceRotation" is not being called by the client and so the API isn't being affected.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant that we should change enforce anywhere it exists, including in the request object, since it isn't a great word to describe the effect of setting that flag. But this is fine as-is.

}

//Create or update instruction file with the re-encrypted metadata while preserving IV
ContentMetadataEncodingStrategy encodeStrategy = new ContentMetadataEncodingStrategy(_instructionFileConfig);
encodeStrategy.encodeMetadata(encryptedMaterials, iv, PutObjectRequest.builder()
.bucket(reEncryptInstructionFileRequest.bucket())
.key(reEncryptInstructionFileRequest.key())
.build(), reEncryptInstructionFileRequest.instructionFileSuffix());

return new ReEncryptInstructionFileResponse(reEncryptInstructionFileRequest.bucket(),
reEncryptInstructionFileRequest.key(), reEncryptInstructionFileRequest.instructionFileSuffix(), reEncryptInstructionFileRequest.enforceRotation());

}

private void enforceRotation(EncryptionMaterials newEncryptionMaterials, GetObjectRequest request) {
try {
DecryptionMaterials decryptedMaterials = this._cryptoMaterialsManager.decryptMaterials(
DecryptMaterialsRequest.builder()
.algorithmSuite(newEncryptionMaterials.algorithmSuite())
.encryptedDataKeys(newEncryptionMaterials.encryptedDataKeys())
.s3Request(request)
.build()
);
} catch (S3EncryptionClientException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this catch too general? Could there be other S3Exceptions that are surfaced for other reasons that would get swallowed here?
My (very mild) concern would be that some unrelated S3Exception gets raised, then this method swallows it and raises no exception, indicating "rotation was done correctly".
Let me know if this isn't a valid concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The catch won't be too general and it would fail closed. This is because we already validated the DecryptionMaterials after we decrypted the original keyring in the CMM and so in "onDecrypt" all that will be checked is whether the old keyring can decrypt the data key and if it is able to, then "key rotation" did not happen.

return;
}
throw new S3EncryptionClientException("Re-encryption failed due to enforced rotation! Old keyring is still able to decrypt the newly encrypted data key");
}

/**
* See {@link S3EncryptionClient#putObject(PutObjectRequest, RequestBody)}.
* <p>
Expand Down Expand Up @@ -382,7 +504,7 @@ public DeleteObjectResponse deleteObject(DeleteObjectRequest deleteObjectRequest
// Delete the object
DeleteObjectResponse deleteObjectResponse = _wrappedAsyncClient.deleteObject(actualRequest).join();
// If Instruction file exists, delete the instruction file as well.
String instructionObjectKey = deleteObjectRequest.key() + INSTRUCTION_FILE_SUFFIX;
String instructionObjectKey = deleteObjectRequest.key() + DEFAULT_INSTRUCTION_FILE_SUFFIX;
_wrappedAsyncClient.deleteObject(builder -> builder
.overrideConfiguration(API_NAME_INTERCEPTOR)
.bucket(deleteObjectRequest.bucket())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
public class S3EncryptionClientUtilities {

public static final String INSTRUCTION_FILE_SUFFIX = ".instruction";
public static final String DEFAULT_INSTRUCTION_FILE_SUFFIX = ".instruction";
public static final long MIN_ALLOWED_BUFFER_SIZE_BYTES = AlgorithmSuite.ALG_AES_256_GCM_IV12_TAG16_NO_KDF.cipherBlockSizeBytes();
public static final long MAX_ALLOWED_BUFFER_SIZE_BYTES = AlgorithmSuite.ALG_AES_256_GCM_IV12_TAG16_NO_KDF.cipherMaxContentLengthBytes();

Expand All @@ -32,7 +32,7 @@ public class S3EncryptionClientUtilities {
*/
static List<ObjectIdentifier> instructionFileKeysToDelete(final DeleteObjectsRequest request) {
return request.delete().objects().stream()
.map(o -> o.toBuilder().key(o.key() + INSTRUCTION_FILE_SUFFIX).build())
.map(o -> o.toBuilder().key(o.key() + DEFAULT_INSTRUCTION_FILE_SUFFIX).build())
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ public class ContentMetadata {

private final EncryptedDataKey _encryptedDataKey;
private final String _encryptedDataKeyAlgorithm;
private final Map<String, String> _encryptedDataKeyContext;

/**
* This field stores either encryption context or material description.
* We use a single field to store both in order to maintain backwards
* compatibility with V2, which treated both as the same.
*/
private final Map<String, String> _encryptionContextOrMatDesc;

private final byte[] _contentIv;
private final String _contentCipher;
Expand All @@ -27,7 +33,7 @@ private ContentMetadata(Builder builder) {

_encryptedDataKey = builder._encryptedDataKey;
_encryptedDataKeyAlgorithm = builder._encryptedDataKeyAlgorithm;
_encryptedDataKeyContext = builder._encryptedDataKeyContext;
_encryptionContextOrMatDesc = builder._encryptionContextOrMatDesc;

_contentIv = builder._contentIv;
_contentCipher = builder._contentCipher;
Expand All @@ -51,14 +57,15 @@ public String encryptedDataKeyAlgorithm() {
return _encryptedDataKeyAlgorithm;
}


/**
* Note that the underlying implementation uses a Collections.unmodifiableMap which is
* immutable.
*/
@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "False positive; underlying"
+ " implementation is immutable")
public Map<String, String> encryptedDataKeyContext() {
return _encryptedDataKeyContext;
public Map<String, String> encryptedDataKeyMatDescOrContext() {
return _encryptionContextOrMatDesc;
}

public byte[] contentIv() {
Expand All @@ -85,7 +92,7 @@ public static class Builder {

private EncryptedDataKey _encryptedDataKey;
private String _encryptedDataKeyAlgorithm;
private Map<String, String> _encryptedDataKeyContext;
private Map<String, String> _encryptionContextOrMatDesc;

private byte[] _contentIv;
private String _contentCipher;
Expand All @@ -111,8 +118,8 @@ public Builder encryptedDataKeyAlgorithm(String encryptedDataKeyAlgorithm) {
return this;
}

public Builder encryptedDataKeyContext(Map<String, String> encryptedDataKeyContext) {
_encryptedDataKeyContext = Collections.unmodifiableMap(encryptedDataKeyContext);
public Builder encryptionContextOrMatDesc(Map<String, String> encryptionContextOrMatDesc) {
_encryptionContextOrMatDesc = Collections.unmodifiableMap(encryptionContextOrMatDesc);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
import software.amazon.encryption.s3.S3EncryptionClient;
import software.amazon.encryption.s3.S3EncryptionClientException;
import software.amazon.encryption.s3.algorithms.AlgorithmSuite;
import software.amazon.encryption.s3.materials.EncryptedDataKey;
Expand All @@ -24,7 +25,7 @@
import java.util.Map;
import java.util.concurrent.CompletionException;

import static software.amazon.encryption.s3.S3EncryptionClientUtilities.INSTRUCTION_FILE_SUFFIX;
import static software.amazon.encryption.s3.S3EncryptionClientUtilities.DEFAULT_INSTRUCTION_FILE_SUFFIX;

public class ContentMetadataDecodingStrategy {

Expand Down Expand Up @@ -136,8 +137,8 @@ private ContentMetadata readFromMap(Map<String, String> metadata, GetObjectRespo
.keyProviderInfo(keyProviderInfo.getBytes(StandardCharsets.UTF_8))
.build();

// Get encrypted data key encryption context
final Map<String, String> encryptionContext = new HashMap<>();
// Get encrypted data key encryption context or materials description (depending on the keyring)
final Map<String, String> encryptionContextOrMatDesc = new HashMap<>();
// The V2 client treats null value here as empty, do the same to avoid incompatibility
String jsonEncryptionContext = metadata.getOrDefault(MetadataKeyConstants.ENCRYPTED_DATA_KEY_CONTEXT, "{}");
// When the encryption context contains non-US-ASCII characters,
Expand All @@ -149,7 +150,7 @@ private ContentMetadata readFromMap(Map<String, String> metadata, GetObjectRespo
JsonNode objectNode = parser.parse(decodedJsonEncryptionContext);

for (Map.Entry<String, JsonNode> entry : objectNode.asObject().entrySet()) {
encryptionContext.put(entry.getKey(), entry.getValue().asString());
encryptionContextOrMatDesc.put(entry.getKey(), entry.getValue().asString());
}
} catch (Exception e) {
throw new RuntimeException(e);
Expand All @@ -161,7 +162,7 @@ private ContentMetadata readFromMap(Map<String, String> metadata, GetObjectRespo
return ContentMetadata.builder()
.algorithmSuite(algorithmSuite)
.encryptedDataKey(edk)
.encryptedDataKeyContext(encryptionContext)
.encryptionContextOrMatDesc(encryptionContextOrMatDesc)
.contentIv(iv)
.contentRange(contentRange)
.build();
Expand Down Expand Up @@ -224,9 +225,13 @@ private ContentMetadata decodeFromObjectMetadata(GetObjectRequest request, GetOb
}

private ContentMetadata decodeFromInstructionFile(GetObjectRequest request, GetObjectResponse response) {
String instructionFileSuffix = request.overrideConfiguration()
.flatMap(config -> config.executionAttributes().getOptionalAttribute(S3EncryptionClient.CUSTOM_INSTRUCTION_FILE_SUFFIX))
.orElse(DEFAULT_INSTRUCTION_FILE_SUFFIX);

GetObjectRequest instructionGetObjectRequest = GetObjectRequest.builder()
.bucket(request.bucket())
.key(request.key() + INSTRUCTION_FILE_SUFFIX)
.key(request.key() + instructionFileSuffix)
.build();

ResponseInputStream<GetObjectResponse> instruction;
Expand Down
Loading