Skip to content

Commit 5249bbf

Browse files
authored
fix: use the configured async client to get the instruction file (#366)
* add infrastructure and tests * update codebuild release
1 parent a21cc35 commit 5249bbf

15 files changed

+411
-242
lines changed

.github/workflows/build.yml

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ jobs:
4949
export AWS_S3EC_TEST_ALT_KMS_KEY_ARN=arn:aws:kms:${{ vars.CI_AWS_REGION }}:${{ secrets.CI_AWS_ACCOUNT_ID }}:key/${{ vars.CI_ALT_KMS_KEY_ID }}
5050
export AWS_S3EC_TEST_ALT_ROLE_ARN=arn:aws:iam::${{ secrets.CI_AWS_ACCOUNT_ID }}:role/service-role/${{ vars.CI_ALT_ROLE }}
5151
export AWS_S3EC_TEST_BUCKET=${{ vars.CI_S3_BUCKET }}
52+
export AWS_S3EC_TEST_ALT_BUCKET=${{ vars.CI_ALT_S3_BUCKET }}
5253
export AWS_S3EC_TEST_KMS_KEY_ID=arn:aws:kms:${{ vars.CI_AWS_REGION }}:${{ secrets.CI_AWS_ACCOUNT_ID }}:key/${{ vars.CI_KMS_KEY_ID }}
5354
export AWS_S3EC_TEST_KMS_KEY_ALIAS=arn:aws:kms:${{ vars.CI_AWS_REGION }}:${{ secrets.CI_AWS_ACCOUNT_ID }}:alias/${{ vars.CI_KMS_KEY_ALIAS }}
5455
export AWS_REGION=${{ vars.CI_AWS_REGION }}

cfn/S3EC-GitHub-CF-Template.yml

+31
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,36 @@ Resources:
6464
Resource:
6565
- !Join [ "", [ !GetAtt S3ECGitHubTestS3Bucket.Arn, '/*'] ]
6666

67+
S3ECGitHubTestS3BucketAlternate:
68+
Type: 'AWS::S3::Bucket'
69+
Properties:
70+
BucketName: s3ec-github-test-bucket-alternate
71+
LifecycleConfiguration:
72+
Rules:
73+
- Id: Expire in 14 days
74+
Status: Enabled
75+
ExpirationInDays: 14
76+
PublicAccessBlockConfiguration:
77+
BlockPublicAcls: false
78+
BlockPublicPolicy: false
79+
IgnorePublicAcls: false
80+
RestrictPublicBuckets: false
81+
82+
S3ECGitHubS3BucketPolicyAlternate:
83+
Type: 'AWS::IAM::ManagedPolicy'
84+
Properties:
85+
ManagedPolicyName: S3EC-GitHub-S3-Bucket-Policy-Alternate
86+
PolicyDocument:
87+
Version: 2012-10-17
88+
Statement:
89+
- Effect: Allow
90+
Action:
91+
- 's3:PutObject'
92+
- 's3:GetObject'
93+
- 's3:DeleteObject'
94+
Resource:
95+
- !Join [ "", [ !GetAtt S3ECGitHubTestS3BucketAlternate.Arn, '/*'] ]
96+
6797
S3ECGitHubKMSKeyPolicy:
6898
Type: 'AWS::IAM::ManagedPolicy'
6999
Properties:
@@ -149,6 +179,7 @@ Resources:
149179
ManagedPolicyArns:
150180
- !Ref S3ECGitHubKMSKeyPolicyAlternate
151181
- !Ref S3ECGitHubS3BucketPolicy
182+
- !Ref S3ECGitHubS3BucketPolicyAlternate
152183

153184
S3ECGitHubAssumeAlternatePolicy:
154185
Type: 'AWS::IAM::ManagedPolicy'

cfn/release.yml

+31
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,36 @@ Resources:
305305
Resource:
306306
- !Join [ "", [ !GetAtt S3ECReleaseTestS3Bucket.Arn, '/*' ] ]
307307

308+
S3ECReleaseTestS3BucketAlternate:
309+
Type: 'AWS::S3::Bucket'
310+
Properties:
311+
BucketName: !Sub "s3ec-release-test-bucket-alternate"
312+
LifecycleConfiguration:
313+
Rules:
314+
- Id: Expire in 14 days
315+
Status: Enabled
316+
ExpirationInDays: 14
317+
PublicAccessBlockConfiguration:
318+
BlockPublicAcls: false
319+
BlockPublicPolicy: false
320+
IgnorePublicAcls: false
321+
RestrictPublicBuckets: false
322+
323+
S3ECReleaseS3BucketPolicyAlternate:
324+
Type: 'AWS::IAM::ManagedPolicy'
325+
Properties:
326+
ManagedPolicyName: S3EC-Release-S3-Bucket-Policy-Alternate
327+
PolicyDocument:
328+
Version: 2012-10-17
329+
Statement:
330+
- Effect: Allow
331+
Action:
332+
- 's3:PutObject'
333+
- 's3:GetObject'
334+
- 's3:DeleteObject'
335+
Resource:
336+
- !Join [ "", [ !GetAtt S3ECReleaseTestS3BucketAlternate.Arn, '/*' ] ]
337+
308338
S3ECReleaseTestKMSKeyPolicy:
309339
Type: 'AWS::IAM::ManagedPolicy'
310340
Properties:
@@ -370,3 +400,4 @@ Resources:
370400
ManagedPolicyArns:
371401
- !Ref S3ECReleaseKMSKeyPolicyAlternate
372402
- !Ref S3ECReleaseS3BucketPolicy
403+
- !Ref S3ECReleaseS3BucketPolicyAlternate

codebuild/release/release-prod.yml

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ phases:
2323
- export AWS_S3EC_TEST_ALT_KMS_KEY_ARN=arn:aws:kms:us-west-2:${ACCOUNT}:key/94f7843c-ec71-4abd-957c-2fb67c991a37
2424
- export AWS_S3EC_TEST_ALT_ROLE_ARN=arn:aws:iam::${ACCOUNT}:role/service-role/S3EC-Release-test-role-alternate
2525
- export AWS_S3EC_TEST_BUCKET=s3ec-release-test-bucket
26+
- export AWS_S3EC_TEST_ALT_BUCKET=s3ec-release-test-bucket-alternate
2627
- export AWS_S3EC_TEST_KMS_KEY_ID=arn:aws:kms:us-west-2:${ACCOUNT}:key/af4ce40a-05ab-4f7c-b3fa-97bd0c9b7fb1
2728
- export AWS_S3EC_TEST_KMS_KEY_ALIAS=arn:aws:kms:us-west-2:${ACCOUNT}:alias/S3EC-Release-Testing-KMS-Key
2829
- export AWS_REGION=us-west-2

codebuild/release/release-staging.yml

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ phases:
2929
- export AWS_S3EC_TEST_ALT_KMS_KEY_ARN=arn:aws:kms:us-west-2:${ACCOUNT}:key/94f7843c-ec71-4abd-957c-2fb67c991a37
3030
- export AWS_S3EC_TEST_ALT_ROLE_ARN=arn:aws:iam::${ACCOUNT}:role/service-role/S3EC-Release-test-role-alternate
3131
- export AWS_S3EC_TEST_BUCKET=s3ec-release-test-bucket
32+
- export AWS_S3EC_TEST_ALT_BUCKET=s3ec-release-test-bucket-alternate
3233
- export AWS_S3EC_TEST_KMS_KEY_ID=arn:aws:kms:us-west-2:${ACCOUNT}:key/af4ce40a-05ab-4f7c-b3fa-97bd0c9b7fb1
3334
- export AWS_S3EC_TEST_KMS_KEY_ALIAS=arn:aws:kms:us-west-2:${ACCOUNT}:alias/S3EC-Release-Testing-KMS-Key
3435
- export AWS_REGION=us-west-2

codebuild/release/validate-staging.yml

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ phases:
2626
- export AWS_S3EC_TEST_ALT_KMS_KEY_ARN=arn:aws:kms:us-west-2:${ACCOUNT}:key/94f7843c-ec71-4abd-957c-2fb67c991a37
2727
- export AWS_S3EC_TEST_ALT_ROLE_ARN=arn:aws:iam::${ACCOUNT}:role/service-role/S3EC-Release-test-role-alternate
2828
- export AWS_S3EC_TEST_BUCKET=s3ec-release-test-bucket
29+
- export AWS_S3EC_TEST_ALT_BUCKET=s3ec-release-test-bucket-alternate
2930
- export AWS_S3EC_TEST_KMS_KEY_ID=arn:aws:kms:us-west-2:${ACCOUNT}:key/af4ce40a-05ab-4f7c-b3fa-97bd0c9b7fb1
3031
- export AWS_S3EC_TEST_KMS_KEY_ALIAS=arn:aws:kms:us-west-2:${ACCOUNT}:alias/S3EC-Release-Testing-KMS-Key
3132
- export AWS_REGION=us-west-2

src/main/java/software/amazon/encryption/s3/internal/ContentMetadataDecodingStrategy.java

+194-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,201 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package software.amazon.encryption.s3.internal;
44

5+
import software.amazon.awssdk.core.ResponseInputStream;
6+
import software.amazon.awssdk.core.async.AsyncResponseTransformer;
7+
import software.amazon.awssdk.protocols.jsoncore.JsonNode;
8+
import software.amazon.awssdk.protocols.jsoncore.JsonNodeParser;
9+
import software.amazon.awssdk.services.s3.S3AsyncClient;
510
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
611
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
12+
import software.amazon.awssdk.services.s3.model.NoSuchKeyException;
13+
import software.amazon.encryption.s3.S3EncryptionClientException;
14+
import software.amazon.encryption.s3.algorithms.AlgorithmSuite;
15+
import software.amazon.encryption.s3.materials.EncryptedDataKey;
16+
import software.amazon.encryption.s3.materials.S3Keyring;
717

8-
@FunctionalInterface
9-
public interface ContentMetadataDecodingStrategy {
10-
ContentMetadata decodeMetadata(GetObjectRequest request, GetObjectResponse response);
18+
import java.nio.charset.StandardCharsets;
19+
import java.util.Base64;
20+
import java.util.HashMap;
21+
import java.util.Map;
22+
23+
import static software.amazon.encryption.s3.S3EncryptionClientUtilities.INSTRUCTION_FILE_SUFFIX;
24+
25+
public class ContentMetadataDecodingStrategy {
26+
27+
private static final Base64.Decoder DECODER = Base64.getDecoder();
28+
29+
private final S3AsyncClient wrappedAsyncClient_;
30+
31+
public ContentMetadataDecodingStrategy(S3AsyncClient s3AsyncClient) {
32+
if (s3AsyncClient == null) {
33+
throw new S3EncryptionClientException("ContentMetadataDecodingStrategy requires a non-null async client.");
34+
}
35+
wrappedAsyncClient_ = s3AsyncClient;
36+
}
37+
38+
private ContentMetadata readFromMap(Map<String, String> metadata, GetObjectResponse response) {
39+
// Get algorithm suite
40+
final String contentEncryptionAlgorithm = metadata.get(MetadataKeyConstants.CONTENT_CIPHER);
41+
AlgorithmSuite algorithmSuite;
42+
String contentRange = response.contentRange();
43+
if (contentEncryptionAlgorithm == null
44+
|| contentEncryptionAlgorithm.equals(AlgorithmSuite.ALG_AES_256_CBC_IV16_NO_KDF.cipherName())) {
45+
algorithmSuite = AlgorithmSuite.ALG_AES_256_CBC_IV16_NO_KDF;
46+
} else if (contentEncryptionAlgorithm.equals(AlgorithmSuite.ALG_AES_256_GCM_IV12_TAG16_NO_KDF.cipherName())) {
47+
// If contentRange is provided, this is a ranged get.
48+
// ranged gets require legacy unauthenticated modes.
49+
// Change AES-GCM to AES-CTR to disable authentication when reading this message.
50+
algorithmSuite = (contentRange == null)
51+
? AlgorithmSuite.ALG_AES_256_GCM_IV12_TAG16_NO_KDF
52+
: AlgorithmSuite.ALG_AES_256_CTR_IV16_TAG16_NO_KDF;
53+
} else {
54+
throw new S3EncryptionClientException(
55+
"Unknown content encryption algorithm: " + contentEncryptionAlgorithm);
56+
}
57+
58+
// Do algorithm suite dependent decoding
59+
byte[] edkCiphertext;
60+
61+
// Currently, this is not stored within the metadata,
62+
// signal to keyring(s) intended for S3EC
63+
final String keyProviderId = S3Keyring.KEY_PROVIDER_ID;
64+
String keyProviderInfo;
65+
switch (algorithmSuite) {
66+
case ALG_AES_256_CBC_IV16_NO_KDF:
67+
// Extract encrypted data key ciphertext
68+
if (metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)) {
69+
edkCiphertext = DECODER.decode(metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1));
70+
} else if (metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2)) {
71+
// when using v1 to encrypt in its default mode, it may use the v2 EDK key
72+
// despite also using CBC as the content encryption algorithm, presumably due
73+
// to how the v2 changes were backported to v1
74+
edkCiphertext = DECODER.decode(metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2));
75+
} else {
76+
// this shouldn't happen under normal circumstances- only if out-of-band modification
77+
// to the metadata is performed. it is most likely that the data is unrecoverable in this case
78+
throw new S3EncryptionClientException("Malformed object metadata! Could not find the encrypted data key.");
79+
}
80+
81+
if (!metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_ALGORITHM)) {
82+
/*
83+
For legacy v1 EncryptionOnly objects,
84+
there is no EDK algorithm given, it is either plain AES or RSA
85+
In v3, we infer AES vs. RSA based on the length of the ciphertext.
86+
87+
In v1, whichever key material is provided in its EncryptionMaterials
88+
is used to decrypt the EDK.
89+
90+
In v3, this is not possible as the keyring code is factored such that
91+
the keyProviderInfo is known before the keyring is known.
92+
Ciphertext size is expected to be reliable as no AES data key should
93+
exceed 256 bits (32 bytes) + 16 padding bytes.
94+
95+
In the unlikely event that this assumption is false, the fix would be
96+
to refactor the keyring to always use the material given instead of
97+
inferring it this way.
98+
*/
99+
if (edkCiphertext.length > 48) {
100+
keyProviderInfo = "RSA";
101+
} else {
102+
keyProviderInfo = "AES";
103+
}
104+
} else {
105+
keyProviderInfo = metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_ALGORITHM);
106+
}
107+
break;
108+
case ALG_AES_256_GCM_IV12_TAG16_NO_KDF:
109+
case ALG_AES_256_CTR_IV16_TAG16_NO_KDF:
110+
// Check tag length
111+
final int tagLength = Integer.parseInt(metadata.get(MetadataKeyConstants.CONTENT_CIPHER_TAG_LENGTH));
112+
if (tagLength != algorithmSuite.cipherTagLengthBits()) {
113+
throw new S3EncryptionClientException("Expected tag length (bits) of: "
114+
+ algorithmSuite.cipherTagLengthBits()
115+
+ ", got: " + tagLength);
116+
}
117+
118+
// Extract encrypted data key ciphertext and provider id
119+
edkCiphertext = DECODER.decode(metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2));
120+
keyProviderInfo = metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_ALGORITHM);
121+
122+
break;
123+
default:
124+
throw new S3EncryptionClientException(
125+
"Unknown content encryption algorithm: " + algorithmSuite.id());
126+
}
127+
128+
// Build encrypted data key
129+
EncryptedDataKey edk = EncryptedDataKey.builder()
130+
.encryptedDataKey(edkCiphertext)
131+
.keyProviderId(keyProviderId)
132+
.keyProviderInfo(keyProviderInfo.getBytes(StandardCharsets.UTF_8))
133+
.build();
134+
135+
// Get encrypted data key encryption context
136+
final Map<String, String> encryptionContext = new HashMap<>();
137+
final String jsonEncryptionContext = metadata.get(MetadataKeyConstants.ENCRYPTED_DATA_KEY_CONTEXT);
138+
try {
139+
JsonNodeParser parser = JsonNodeParser.create();
140+
JsonNode objectNode = parser.parse(jsonEncryptionContext);
141+
142+
for (Map.Entry<String, JsonNode> entry : objectNode.asObject().entrySet()) {
143+
encryptionContext.put(entry.getKey(), entry.getValue().asString());
144+
}
145+
} catch (Exception e) {
146+
throw new RuntimeException(e);
147+
}
148+
149+
// Get content iv
150+
byte[] iv = DECODER.decode(metadata.get(MetadataKeyConstants.CONTENT_IV));
151+
152+
return ContentMetadata.builder()
153+
.algorithmSuite(algorithmSuite)
154+
.encryptedDataKey(edk)
155+
.encryptedDataKeyContext(encryptionContext)
156+
.contentIv(iv)
157+
.contentRange(contentRange)
158+
.build();
159+
}
160+
161+
public ContentMetadata decode(GetObjectRequest request, GetObjectResponse response) {
162+
Map<String, String> metadata = response.metadata();
163+
ContentMetadataDecodingStrategy strategy;
164+
if (metadata != null
165+
&& metadata.containsKey(MetadataKeyConstants.CONTENT_IV)
166+
&& (metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V1)
167+
|| metadata.containsKey(MetadataKeyConstants.ENCRYPTED_DATA_KEY_V2))) {
168+
return decodeFromObjectMetadata(request, response);
169+
} else {
170+
return decodeFromInstructionFile(request, response);
171+
}
172+
}
173+
174+
private ContentMetadata decodeFromObjectMetadata(GetObjectRequest request, GetObjectResponse response) {
175+
return readFromMap(response.metadata(), response);
176+
}
177+
178+
private ContentMetadata decodeFromInstructionFile(GetObjectRequest request, GetObjectResponse response) {
179+
GetObjectRequest instructionGetObjectRequest = GetObjectRequest.builder()
180+
.bucket(request.bucket())
181+
.key(request.key() + INSTRUCTION_FILE_SUFFIX)
182+
.build();
183+
184+
ResponseInputStream<GetObjectResponse> instruction;
185+
try {
186+
instruction = wrappedAsyncClient_.getObject(instructionGetObjectRequest, AsyncResponseTransformer.toBlockingInputStream()).join();
187+
} catch (NoSuchKeyException exception) {
188+
// Most likely, the customer is attempting to decrypt an object
189+
// which is not encrypted with the S3 EC.
190+
throw new S3EncryptionClientException("Instruction file not found! Please ensure the object you are" +
191+
" attempting to decrypt has been encrypted using the S3 Encryption Client.", exception);
192+
}
193+
194+
Map<String, String> metadata = new HashMap<>();
195+
JsonNodeParser parser = JsonNodeParser.create();
196+
JsonNode objectNode = parser.parse(instruction);
197+
for (Map.Entry<String, JsonNode> entry : objectNode.asObject().entrySet()) {
198+
metadata.put(entry.getKey(), entry.getValue().asString());
199+
}
200+
return readFromMap(metadata, response);
201+
}
11202
}

0 commit comments

Comments
 (0)