(aws-s3): Logging Bucket blocks KMS_MANAGED encryption

[biffgaut]

Describe the bug

PR 23385 introduced a check to block launching S3 access log buckets with KMS encryption. S3 does not support access log buckets with KMS encryption, so previously these stacks would launch but fail to generate any logs. This PR would throw an Error when this occurred.

The PR included KMS_MANAGED in the list of blocked encryptions, but tests show that S3 buckets with KMS_MANAGED encryption can accepts logs. So the application threw an error for configurations that were valid.

KMS_MANAGED needs to be removed from the list of encryptions flagged by the code.

Expected Behavior

This code should not throw an error:

    const stack = new cdk.Stack();
    new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.KMS_MANAGED, serverAccessLogsPrefix: 'test' });

Current Behavior

That code throws 'SSE-S3 is the only supported default bucket encryption for Server Access Logging target buckets'

Reproduction Steps

    // This code should not throw an error upon deployment
    new s3.Bucket(this, 'selfKmsManagedEnc', {
      bucketName: 'bgc-source-selflogging-kmsmanaged-encryption',
      removalPolicy: cdk.RemovalPolicy.DESTROY,
      autoDeleteObjects: true,
      serverAccessLogsPrefix: 'logs',
      encryption: BucketEncryption.KMS_MANAGED
    });

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.57.0

Framework Version

No response

Node.js Version

v14.21.1

OS

macOS Monterey 12.7.1

Language

Typescript

Language Version

No response

Other information

No response

[ibliskavka]

@biffgaut, this came as a surprise to me too. We have been using access logging buckets with KMS encryption, but online docs state that it is not.

Source:

• If default encryption is turned on for the target bucket, AES256 (SSE-S3) must be selected

• If you use default encryption on the target bucket, then confirm that AES-256 (SSE-S3) is selected as the encryption key. Encryption using AWS-KMS (SSE-KMS) is not supported. For instructions on how to configure default encryption using the Amazon S3 console, see Turning on Amazon S3 default bucket encryption.

[biffgaut]

Pretty sure that if you look at your log buckets you'll find that the logs are not actually getting created (if your log bucket is indeed KMS encrypted). This was addressed in a recent release - the new error message didn't break any working code, it just alerted people that their code had not actually been working.

KMS_MANAGED encryption is a different beast - if you log to a bucket with KMS_MANAGED encryption the infrastructure created by the CDK Bucket construct will create the log files. But the encryption key is not available to your account and you will be unable to read those log files. So flagging an error for KMS_MANAGED log buckets is also appropriate, just for a different reason.

So the code snippet in Expected Behavior above should throw an error, so I'm going to close this issue.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[jeffgardnerdev]

@biffgaut I am getting this error after upgrading an existing app but can confirm that the configuration is working properly--access logs are being sent to the KMS encrypted log target bucket and I am able to download and view them.

My log target bucket has the encryptionKey and bucketKeyEnabled: true props passed in.

I'll note that logs were not getting delivered initially but it started working when I gave logging.s3.amazonaws.com the proper permissions in the KMS key's key policy.

I know that the documentation indicates that only SSE-S3 is supported, but it doesn't seem to be the case.

[zdf1230]

I agreed with @jeffgardnerdev. I am using KMS encryption with logging.s3.amazonaws.com permission attached to the key of course. It works fine on my side - log is generating. So, I don't think we should have these checks which are breaking changes.

[zdf1230]

I put a fix for this. #25350. It is available after v2.81.0.