Skip to content

Commit

Permalink
Revert "fix(s3): logging bucket blocks KMS_MANAGED encryption (aws#23514
Browse files Browse the repository at this point in the history 
)"

This reverts commit 1e8926f.
  • Loading branch information
@Styerp
Styerp committed May 11, 2023
1 parent b76c182 commit 2e1172c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 83 deletions.
10 changes: 4 additions & 6 deletions packages/aws-cdk-lib/aws-s3/lib/bucket.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2125,14 +2125,12 @@ export class Bucket extends BucketBase {
if (!props.serverAccessLogsBucket && !props.serverAccessLogsPrefix) {
return undefined;
}

if (
// KMS can't be used for logging since the logging service can't use the key - logs don't write
// KMS_MANAGED can't be used for logging since the account can't access the logging service key - account can't read logs
(!props.serverAccessLogsBucket && (
// The current bucket is being used and is configured for default SSE-KMS
!props.serverAccessLogsBucket && (
props.encryptionKey ||
props.encryption === BucketEncryption.KMS_MANAGED ||
props.encryption === BucketEncryption.KMS )) ||
props.encryption === BucketEncryption.KMS ||
props.encryption === BucketEncryption.KMS_MANAGED) ||
// Another bucket is being used that is configured for default SSE-KMS
props.serverAccessLogsBucket?.encryptionKey
) {
Expand Down
78 changes: 9 additions & 69 deletions packages/aws-cdk-lib/aws-s3/test/bucket.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ describe('bucket', () => {

});


testDeprecated('logs to self, UNENCRYPTED does not throw error', () => {
const stack = new cdk.Stack();
expect(() => {
Expand All @@ -379,87 +380,26 @@ describe('bucket', () => {
}).not.toThrowError();
});

test('logs to self, KMS_MANAGED encryption throws error', () => {
test('throws error if using KMS-Managed key and server access logging to self', () => {
const stack = new cdk.Stack();
expect(() => {
new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.KMS_MANAGED, serverAccessLogsPrefix: 'test' });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
});

test('logs to self, KMS encryption without key throws error', () => {
const stack = new cdk.Stack();
expect(() => {
new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.KMS, serverAccessLogsPrefix: 'test' });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
});

test('logs to self, KMS encryption with key throws error', () => {
const stack = new cdk.Stack();
const key = new kms.Key(stack, 'TestKey');
expect(() => {
new s3.Bucket(stack, 'MyBucket', { encryptionKey: key, encryption: s3.BucketEncryption.KMS, serverAccessLogsPrefix: 'test' });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
}).toThrow('SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets');
});

test('logs to self, KMS key with no specific encryption specified throws error', () => {
test('throws error if using KMS CMK and server access logging to self', () => {
const stack = new cdk.Stack();
const key = new kms.Key(stack, 'TestKey');
expect(() => {
new s3.Bucket(stack, 'MyBucket', { encryptionKey: key, serverAccessLogsPrefix: 'test' });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
}).toThrow('SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets');
});

testDeprecated('logs to separate bucket, UNENCRYPTED does not throw error', () => {
const stack = new cdk.Stack();
const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryption: s3.BucketEncryption.UNENCRYPTED });
expect(() => {
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket });
}).not.toThrowError();
});

test('logs to separate bucket, S3_MANAGED encryption does not throw error', () => {
const stack = new cdk.Stack();
const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryption: s3.BucketEncryption.S3_MANAGED });
expect(() => {
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket });
}).not.toThrowError();
});

// When provided an external bucket (as an IBucket), we cannot detect KMS_MANAGED encryption. Since this
// check is impossible, we skip thist test.
// eslint-disable-next-line jest/no-disabled-tests
test.skip('logs to separate bucket, KMS_MANAGED encryption throws error', () => {
const stack = new cdk.Stack();
const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryption: s3.BucketEncryption.KMS_MANAGED });
expect(() => {
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
});

test('logs to separate bucket, KMS encryption without key throws error', () => {
const stack = new cdk.Stack();
const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryption: s3.BucketEncryption.KMS });
expect(() => {
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
});

test('logs to separate bucket, KMS encryption with key throws error', () => {
const stack = new cdk.Stack();
const key = new kms.Key(stack, 'TestKey');
const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryptionKey: key, encryption: s3.BucketEncryption.KMS });
expect(() => {
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
});

test('logs to separate bucket, KMS key with no specific encryption specified throws error', () => {
test('throws error if enabling server access logging to bucket with SSE-KMS', () => {
const stack = new cdk.Stack();
const key = new kms.Key(stack, 'TestKey');
const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryptionKey: key });
const targetBucket = new s3.Bucket(stack, 'TargetBucket', { encryptionKey: key } );
expect(() => {
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket });
}).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/);
new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: targetBucket });
}).toThrow('SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets');
});

test('bucket with versioning turned on', () => {
Expand Down
8 changes: 0 additions & 8 deletions packages/aws-cdk-lib/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@
"description": "Version 2 of the AWS Cloud Development Kit library",
"main": "index.js",
"types": "index.d.ts",
"typesVersions": {
"<=3.9": {
"*": [
".types-compat/ts3.9/*",
".types-compat/ts3.9/*/index.d.ts"
]
}
},
"repository": {
"type": "git",
"url": "https://github.com/aws/aws-cdk.git",
Expand Down

0 comments on commit 2e1172c

Please sign in to comment.