Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import software.amazon.awssdk.services.s3.model.S3Request;
import software.amazon.encryption.s3.S3EncryptionClient;
import software.amazon.encryption.s3.S3EncryptionClientException;
import software.amazon.encryption.s3.algorithms.AlgorithmSuite;
import software.amazon.encryption.s3.internal.ApiNameVersion;

//= specification/s3-encryption/materials/s3-kms-keyring.md#interface
Expand Down Expand Up @@ -251,6 +252,17 @@ public byte[] decryptDataKey(DecryptionMaterials materials, byte[] encryptedData
throw new S3EncryptionClientException("Provided encryption context does not match information retrieved from S3");
}

// The content encryption algorithm is part of the KMS-authenticated encryption context, while the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like we can simplify this comment, possibly something along the lines of "The stored content-encryption algorithm (KMS-authenticated) MUST match the algorithm suite selected from content metadata (unauthenticated).

// algorithm suite is selected from the (unauthenticated) content metadata. The two must agree; if
// they do not, the content metadata does not correspond to how the object was sealed, so the data
// key is not released for a mismatched suite.
String storedContentAlgorithm = materials.encryptionContext().get(ENCRYPTION_CONTEXT_ALGORITHM_KEY);
if (storedContentAlgorithm != null
&& !storedContentAlgorithm.equals(expectedContentAlgorithm(materials.algorithmSuite()))) {
throw new S3EncryptionClientException("The content encryption algorithm in the KMS-authenticated "
+ "encryption context does not match the algorithm suite selected for decryption.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should include both values (content encryption algorithm and selected algorithm suite for decryption) in the error message.

}

//= specification/s3-encryption/materials/s3-kms-keyring.md#kms-context
//# To attempt to decrypt a particular [encrypted data key](../structures.md#encrypted-data-key), the KmsKeyring
//# MUST call [AWS KMS Decrypt](https://docs.aws.amazon.com/kms/latest/APIReference/API_Decrypt.html) with the configured AWS KMS client.
Expand Down Expand Up @@ -313,6 +325,23 @@ protected Map<String, DecryptDataKeyStrategy> decryptDataKeyStrategies() {
return decryptDataKeyStrategies;
}

// The canonical value the content-encryption algorithm (aws:x-amz-cek-alg) holds in the stored encryption

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we simplify this to "Return content-encryption algorithm for a given AlgorithmSuite"

// context for a given selected algorithm suite. The committing GCM/CTR pair share the committing suite ID,
// and the non-committing GCM/CTR pair share GCM's cipher name, because a ranged get selects the CTR variant
// of an object that was sealed under its GCM counterpart.
private static String expectedContentAlgorithm(AlgorithmSuite algorithmSuite) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we remove this private helper function and instead add contentAlgorithm (String) as a field in the AlgorithmSuite?

switch (algorithmSuite) {
case ALG_AES_256_GCM_HKDF_SHA512_COMMIT_KEY:
case ALG_AES_256_CTR_HKDF_SHA512_COMMIT_KEY:
return AlgorithmSuite.ALG_AES_256_GCM_HKDF_SHA512_COMMIT_KEY.idAsString();
case ALG_AES_256_GCM_IV12_TAG16_NO_KDF:
case ALG_AES_256_CTR_IV16_TAG16_NO_KDF:
return AlgorithmSuite.ALG_AES_256_GCM_IV12_TAG16_NO_KDF.cipherName();
default:
return algorithmSuite.cipherName();
}
}

public static class Builder extends S3Keyring.Builder<KmsKeyring, Builder> {
private KmsClient _kmsClient;
private String _wrappingKeyId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,29 @@
// SPDX-License-Identifier: Apache-2.0
package software.amazon.encryption.s3.materials;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.RetryingTest;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.services.kms.KmsClient;
import software.amazon.awssdk.services.kms.model.DecryptRequest;
import software.amazon.awssdk.services.kms.model.DecryptResponse;
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
import software.amazon.encryption.s3.S3EncryptionClientException;
import software.amazon.encryption.s3.algorithms.AlgorithmSuite;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class KmsKeyringTest {

private static final String KMS_KEY_ID = "arn:aws:kms:us-west-2:111122223333:key/example";

@RetryingTest(3)
public void buildKmsKeyringWithNullSecureRandomFails() {
assertThrows(S3EncryptionClientException.class, () -> KmsKeyring.builder().secureRandom(null));
Expand All @@ -19,4 +35,44 @@ public void buildKmsKeyringWithNullDataKeyGeneratorFails() {
assertThrows(S3EncryptionClientException.class, () -> KmsKeyring.builder().dataKeyGenerator(null));
}

// The data key was wrapped for a GCM object, so the KMS-authenticated encryption context carries
// aws:x-amz-cek-alg = AES/GCM/NoPadding. The decryption materials select the CBC suite (as they would
// if the content-algorithm metadata read CBC). The authenticated content algorithm and the selected
// suite disagree, so onDecrypt must fail rather than return the data key.
@Test
public void kmsContextDecryptFailsWhenAuthenticatedCekAlgDoesNotMatchSelectedSuite() {
Map<String, String> storedEncryptionContext = new HashMap<>();
storedEncryptionContext.put("kms_cmk_id", KMS_KEY_ID);
storedEncryptionContext.put("aws:x-amz-cek-alg", AlgorithmSuite.ALG_AES_256_GCM_IV12_TAG16_NO_KDF.cipherName());

// Releases the data key only for the encryption context it was wrapped under.
KmsClient kmsClient = mock(KmsClient.class);
when(kmsClient.decrypt(argThat((DecryptRequest request) ->
storedEncryptionContext.equals(request.encryptionContext()))))
.thenReturn(DecryptResponse.builder()
.keyId(KMS_KEY_ID)
.plaintext(SdkBytes.fromByteArray(new byte[32]))
.build());

KmsKeyring keyring = KmsKeyring.builder()
.kmsClient(kmsClient)
.wrappingKeyId(KMS_KEY_ID)
.build();

DecryptionMaterials materials = DecryptionMaterials.builder()
.s3Request(GetObjectRequest.builder().bucket("bucket").key("key").build())
.algorithmSuite(AlgorithmSuite.ALG_AES_256_CBC_IV16_NO_KDF)
.encryptionContext(storedEncryptionContext)
.build();

EncryptedDataKey edk = EncryptedDataKey.builder()
.keyProviderId(S3Keyring.KEY_PROVIDER_ID)
.keyProviderInfo("kms+context")
.encryptedDataKey(new byte[]{1, 2, 3})
.build();

assertThrows(S3EncryptionClientException.class,
() -> keyring.onDecrypt(materials, Collections.singletonList(edk)));
}

}
Loading