diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index 2b2f930dd0f49..5bd9697cec170 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -2104,12 +2104,14 @@ export class Bucket extends BucketBase { if (!props.serverAccessLogsBucket && !props.serverAccessLogsPrefix) { return undefined; } + if ( - // The current bucket is being used and is configured for default SSE-KMS - !props.serverAccessLogsBucket && ( + // 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 && ( props.encryptionKey || - props.encryption === BucketEncryption.KMS || - props.encryption === BucketEncryption.KMS_MANAGED) || + props.encryption === BucketEncryption.KMS_MANAGED || + props.encryption === BucketEncryption.KMS )) || // Another bucket is being used that is configured for default SSE-KMS props.serverAccessLogsBucket?.encryptionKey ) { diff --git a/packages/@aws-cdk/aws-s3/test/bucket.test.ts b/packages/@aws-cdk/aws-s3/test/bucket.test.ts index e2a6d1238da97..cd0f962e8d703 100644 --- a/packages/@aws-cdk/aws-s3/test/bucket.test.ts +++ b/packages/@aws-cdk/aws-s3/test/bucket.test.ts @@ -338,26 +338,101 @@ describe('bucket', () => { }); - test('throws error if using KMS-Managed key and server access logging to self', () => { + test('logs to self, no encryption does not throw error', () => { + const stack = new cdk.Stack(); + expect(() => { + new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.UNENCRYPTED, serverAccessLogsPrefix: 'test' }); + }).not.toThrowError(); + }); + + test('logs to self, S3 encryption does not throw error', () => { + const stack = new cdk.Stack(); + expect(() => { + new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.S3_MANAGED, serverAccessLogsPrefix: 'test' }); + }).not.toThrowError(); + }); + + test('logs to self, KMS_MANAGED encryption throws error', () => { 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'); + }).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/); }); - test('throws error if using KMS CMK and server access logging to self', () => { + + test('logs to self, KMS key with no specific encryption specified throws error', () => { 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/); + }); + + test('logs to separate bucket, no encryption 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 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('throws error if enabling server access logging to bucket with SSE-KMS', () => { + + 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', () => { const stack = new cdk.Stack(); const key = new kms.Key(stack, 'TestKey'); - const targetBucket = new s3.Bucket(stack, 'TargetBucket', { encryptionKey: key } ); + const logBucket = new s3.Bucket(stack, 'testLogBucket', { encryptionKey: key }); expect(() => { - new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: targetBucket }); - }).toThrow('SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets'); + new s3.Bucket(stack, 'MyBucket', { serverAccessLogsBucket: logBucket }); + }).toThrow(/SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets/); }); test('bucket with versioning turned on', () => {