Skip to content

Commit 6af9b02

Browse files
WesleyRosenblumWesleyRosenblum
committed
Limit ability to create discovery AWS KMS Keyrings to explicit creation
1 parent 15c1def commit 6af9b02

File tree

4 files changed

+70
-21
lines changed

4 files changed

+70
-21
lines changed

‎src/main/java/com/amazonaws/encryptionsdk/keyrings/AwsKmsKeyring.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.amazonaws.encryptionsdk.model.DecryptionMaterials;
2424
import com.amazonaws.encryptionsdk.model.EncryptionMaterials;
2525
import com.amazonaws.encryptionsdk.model.KeyBlob;
26+
import org.apache.commons.lang3.Validate;
2627

2728
import java.util.ArrayList;
2829
import java.util.HashSet;
@@ -47,12 +48,20 @@ class AwsKmsKeyring implements Keyring {
4748
private final AwsKmsCmkId generatorKeyId;
4849
private final boolean isDiscovery;
4950

50-
AwsKmsKeyring(DataKeyEncryptionDao dataKeyEncryptionDao, List<AwsKmsCmkId> keyIds, AwsKmsCmkId generatorKeyId) {
51+
AwsKmsKeyring(DataKeyEncryptionDao dataKeyEncryptionDao, List<AwsKmsCmkId> keyIds, AwsKmsCmkId generatorKeyId, boolean isDiscovery) {
5152
requireNonNull(dataKeyEncryptionDao, "dataKeyEncryptionDao is required");
5253
this.dataKeyEncryptionDao = dataKeyEncryptionDao;
5354
this.keyIds = keyIds == null ? emptyList() : unmodifiableList(new ArrayList<>(keyIds));
5455
this.generatorKeyId = generatorKeyId;
55-
this.isDiscovery = this.generatorKeyId == null && this.keyIds.isEmpty();
56+
this.isDiscovery = isDiscovery;
57+
58+
if(isDiscovery) {
59+
Validate.isTrue(generatorKeyId == null && this.keyIds.isEmpty(),
60+
"AWS KMS Discovery keyrings cannot specify any key IDs");
61+
} else {
62+
Validate.isTrue(generatorKeyId != null || !this.keyIds.isEmpty(),
63+
"GeneratorKeyId or KeyIds are required for non-discovery AWS KMS Keyrings.");
64+
}
5665

5766
if (this.keyIds.contains(generatorKeyId)) {
5867
throw new IllegalArgumentException("KeyIds should not contain the generatorKeyId");

‎src/main/java/com/amazonaws/encryptionsdk/keyrings/AwsKmsKeyringBuilder.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,13 @@ public class AwsKmsKeyringBuilder {
2424
private List<String> grantTokens;
2525
private List<AwsKmsCmkId> keyIds;
2626
private AwsKmsCmkId generatorKeyId;
27+
private final boolean isDiscovery;
2728

28-
private AwsKmsKeyringBuilder() {
29+
private AwsKmsKeyringBuilder(boolean isDiscovery) {
2930
// Use AwsKmsKeyringBuilder.standard() or StandardKeyrings.awsKms() to instantiate
31+
// or AwsKmsKeyringBuilder.discovery() or StandardKeyrings.awsKmsDiscovery() to instantiate
32+
// a discovery keyring builder.
33+
this.isDiscovery = isDiscovery;
3034
}
3135

3236
/**
@@ -35,7 +39,18 @@ private AwsKmsKeyringBuilder() {
3539
* @return The {@code AwsKmsKeyringBuilder}
3640
*/
3741
public static AwsKmsKeyringBuilder standard() {
38-
return new AwsKmsKeyringBuilder();
42+
return new AwsKmsKeyringBuilder(false);
43+
}
44+
45+
/**
46+
* Constructs a new instance of {@code AwsKmsKeyringBuilder} that produces an AWS KMS Discovery keyring.
47+
* AWS KMS Discovery keyrings do not specify any CMKs to decrypt with, and thus will attempt to decrypt
48+
* using any encrypted data key in an encrypted message. AWS KMS Discovery keyrings do not perform encryption.
49+
*
50+
* @return The {@code AwsKmsKeyringBuilder}
51+
*/
52+
public static AwsKmsKeyringBuilder discovery() {
53+
return new AwsKmsKeyringBuilder(true);
3954
}
4055

4156
/**
@@ -98,6 +113,9 @@ public Keyring build() {
98113
if(awsKmsClientSupplier == null) {
99114
awsKmsClientSupplier = AwsKmsClientSupplier.builder().build();
100115
}
101-
return new AwsKmsKeyring(DataKeyEncryptionDao.awsKms(awsKmsClientSupplier, grantTokens), keyIds, generatorKeyId);
116+
117+
return new AwsKmsKeyring(DataKeyEncryptionDao.awsKms(awsKmsClientSupplier, grantTokens),
118+
keyIds, generatorKeyId, isDiscovery);
102119
}
120+
103121
}

‎src/main/java/com/amazonaws/encryptionsdk/keyrings/StandardKeyrings.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,25 +76,26 @@ public static AwsKmsKeyringBuilder awsKms() {
7676
}
7777

7878
/**
79-
* Constructs a {@code Keyring} which interacts with AWS Key Management Service (KMS) to attempt to
80-
* decrypt all data keys provided to it. AWS KMS Discovery keyrings do not perform encryption.
79+
* Returns an {@link AwsKmsKeyringBuilder} for use in constructing an AWS KMS Discovery keyring.
80+
* AWS KMS Discovery keyrings do not specify any CMKs to decrypt with, and thus will attempt to decrypt
81+
* using any encrypted data key in an encrypted message. AWS KMS Discovery keyrings do not perform encryption.
8182
* <p></p>
82-
* To create an AWS KMS Regional Discovery Keyring, use {@link #awsKms()} and the
83+
* To create an AWS KMS Regional Discovery Keyring, construct an {@link AwsKmsClientSupplier} using
8384
* {@link AwsKmsClientSupplier#builder()} to specify which regions to include/exclude.
8485
* <p></p>
8586
* For example, to include only CMKs in the us-east-1 region:
8687
* <pre>
87-
* StandardKeyrings.awsKms()
88+
* StandardKeyrings.awsKmsDiscovery()
8889
* .awsKmsClientSupplier(
8990
* AwsKmsClientSupplier.builder()
9091
* .allowedRegions(Collections.singleton("us-east-1")).build())
9192
* .build();
9293
* </pre>
9394
*
94-
* @return The {@code Keyring}
95+
* @return The {@code AwsKmsKeyringBuilder}
9596
*/
96-
public static Keyring awsKmsDiscovery() {
97-
return AwsKmsKeyringBuilder.standard().build();
97+
public static AwsKmsKeyringBuilder awsKmsDiscovery() {
98+
return AwsKmsKeyringBuilder.discovery();
9899
}
99100

100101
/**

‎src/test/java/com/amazonaws/encryptionsdk/keyrings/AwsKmsKeyringTest.java

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ void setup() {
9595
List<AwsKmsCmkId> keyIds = new ArrayList<>();
9696
keyIds.add(AwsKmsCmkId.fromString(KEY_ID_1));
9797
keyIds.add(AwsKmsCmkId.fromString(KEY_ID_2));
98-
keyring = new AwsKmsKeyring(dataKeyEncryptionDao, keyIds, AwsKmsCmkId.fromString(GENERATOR_KEY_ID));
98+
keyring = new AwsKmsKeyring(dataKeyEncryptionDao, keyIds, AwsKmsCmkId.fromString(GENERATOR_KEY_ID), false);
9999
}
100100

101101
@Test
@@ -106,23 +106,44 @@ void testMalformedArns() {
106106
.build();
107107

108108
List<EncryptedDataKey> encryptedDataKeys = new ArrayList<>();
109-
encryptedDataKeys.add(new KeyBlob(AWS_KMS_PROVIDER_ID, "badArn".getBytes(PROVIDER_ENCODING), new byte[]{}));
109+
encryptedDataKeys.add(new KeyBlob(AWS_KMS_PROVIDER_ID, "arn:badArn".getBytes(PROVIDER_ENCODING), new byte[]{}));
110110
encryptedDataKeys.add(ENCRYPTED_KEY_1);
111111

112112
decryptionMaterials = keyring.onDecrypt(decryptionMaterials, encryptedDataKeys);
113113
assertEquals(PLAINTEXT_DATA_KEY, decryptionMaterials.getCleartextDataKey());
114114

115+
decryptionMaterials = DecryptionMaterials.newBuilder()
116+
.setAlgorithm(ALGORITHM_SUITE)
117+
.setEncryptionContext(ENCRYPTION_CONTEXT)
118+
.build();
119+
115120
// Malformed Arn for a non KMS provider shouldn't fail
116121
encryptedDataKeys.clear();
117-
encryptedDataKeys.add(new KeyBlob("OtherProviderId", "badArn".getBytes(PROVIDER_ENCODING), new byte[]{}));
118-
keyring.onDecrypt(decryptionMaterials, encryptedDataKeys);
122+
encryptedDataKeys.add(new KeyBlob("OtherProviderId", "arn:badArn".getBytes(PROVIDER_ENCODING), new byte[]{}));
123+
assertFalse(keyring.onDecrypt(decryptionMaterials, encryptedDataKeys).hasCleartextDataKey());
119124
}
120125

121126
@Test
122127
void testGeneratorKeyInKeyIds() {
123128
assertThrows(IllegalArgumentException.class, () -> new AwsKmsKeyring(dataKeyEncryptionDao,
124129
Collections.singletonList(AwsKmsCmkId.fromString(GENERATOR_KEY_ID)),
125-
AwsKmsCmkId.fromString(GENERATOR_KEY_ID)));
130+
AwsKmsCmkId.fromString(GENERATOR_KEY_ID), false));
131+
}
132+
133+
@Test
134+
void testNotDiscoveryNoKeysIds() {
135+
assertThrows(IllegalArgumentException.class, () -> new AwsKmsKeyring(dataKeyEncryptionDao,
136+
null,null, false));
137+
}
138+
139+
@Test
140+
void testDiscoveryWithKeyId() {
141+
assertThrows(IllegalArgumentException.class, () -> new AwsKmsKeyring(dataKeyEncryptionDao,
142+
null,
143+
AwsKmsCmkId.fromString(GENERATOR_KEY_ID), true));
144+
assertThrows(IllegalArgumentException.class, () -> new AwsKmsKeyring(dataKeyEncryptionDao,
145+
Collections.singletonList(AwsKmsCmkId.fromString(GENERATOR_KEY_ID)),
146+
null, true));
126147
}
127148

128149
@Test
@@ -207,7 +228,7 @@ void testEncryptNullGenerator() {
207228
.build();
208229

209230
Keyring keyring = new AwsKmsKeyring(dataKeyEncryptionDao,
210-
Collections.singletonList(AwsKmsCmkId.fromString(KEY_ID_1)), null);
231+
Collections.singletonList(AwsKmsCmkId.fromString(KEY_ID_1)), null, false);
211232

212233
encryptionMaterials = keyring.onEncrypt(encryptionMaterials);
213234

@@ -222,7 +243,7 @@ void testEncryptNullGenerator() {
222243

223244
@Test
224245
void testDiscoveryEncrypt() {
225-
keyring = new AwsKmsKeyring(dataKeyEncryptionDao, null, null);
246+
keyring = new AwsKmsKeyring(dataKeyEncryptionDao, null, null, true);
226247

227248
EncryptionMaterials encryptionMaterials = EncryptionMaterials.newBuilder()
228249
.setAlgorithm(ALGORITHM_SUITE)
@@ -237,7 +258,7 @@ void testDiscoveryEncrypt() {
237258
@Test
238259
void testEncryptNoGeneratorOrCleartextDataKey() {
239260
keyring = new AwsKmsKeyring(dataKeyEncryptionDao,
240-
Collections.singletonList(AwsKmsCmkId.fromString(KEY_ID_1)), null);
261+
Collections.singletonList(AwsKmsCmkId.fromString(KEY_ID_1)), null, false);
241262

242263
EncryptionMaterials encryptionMaterials = EncryptionMaterials.newBuilder().setAlgorithm(ALGORITHM_SUITE).build();
243264
assertThrows(AwsCryptoException.class, () -> keyring.onEncrypt(encryptionMaterials));
@@ -297,7 +318,7 @@ void testDecryptFirstKeyWrongProvider() {
297318

298319
@Test
299320
void testDiscoveryDecrypt() {
300-
keyring = new AwsKmsKeyring(dataKeyEncryptionDao, null, null);
321+
keyring = new AwsKmsKeyring(dataKeyEncryptionDao, null, null, true);
301322

302323
DecryptionMaterials decryptionMaterials = DecryptionMaterials.newBuilder()
303324
.setAlgorithm(ALGORITHM_SUITE)

0 commit comments

Comments
 (0)