aws-s3: Unable to Disable ACLs for Access Log Bucket

[kfdarlington]

Describe the bug

General S3 best practices now recommend that bucket ACLs be disabled, leading bucket ownership controls to be configured as BUCKET_OWNER_ENFORCED.

While I can successfully attach a bucket policy to an access log bucket that grants logging.s3.amazonaws.com with s3:PutObject permissions per this documentation, I am unable to set that access log bucket's ownership to BUCKET_OWNER_ENFORCED. S3 throws an exception during bucket creation and modification because CDK automatically sets the bucket's AccessControl to LogDeliveryWrite when it is provided to another bucket's serverAccessLogsBucket property.

Expected Behavior

Bucket creation and modification succeeds for an access log bucket with ownership controls set to BUCKET_OWNER_ENFORCED.

Current Behavior

Bucket creation and modification fails with

Bucket cannot have ACLs set with ObjectOwnership's BucketOwnerEnforced setting (Service: Amazon S3; Status Code: 400; Error Code: InvalidBucketAclWithObjectOwnership; Request ID: <redacted>; S3 Extended Request ID: <redacted>; Proxy: null)

for an access log bucket with ownership controls set to BUCKET_OWNER_ENFORCED.

Reproduction Steps

const accessLogBucket = new Bucket(scope, "TestAccessLogBucket", {
      bucketName: "TestAccessLogBucket",
      encryption: BucketEncryption.S3_MANAGED,
      enforceSSL: true,
      objectOwnership: ObjectOwnership.BUCKET_OWNER_ENFORCED,
      versioned: true
});

const bucket = new Bucket(scope, "TestBucket", {
      bucketName: "TestBucket",
      encryption: BucketEncryption.S3_MANAGED,
      enforceSSL: true,
      objectOwnership: ObjectOwnership.BUCKET_OWNER_ENFORCED,
      serverAccessLogsBucket: accessLogBucket,
      versioned: true
});

Possible Solution

A potential backward compatible change would check whether a bucket's BUCKET_OWNER_ENFORCED property disables ACLs for a bucket prior to automatically setting AccessControl to LogDeliveryWrite when that bucket is provided to serverAccessLogsBucket. If ACLs are disabled, AccessControl would be ignored and the customer would need to add a bucket policy providing logging.s3.amazonaws.com with s3:PutObject permissions.

Alternatively, we may be able to expose a bucket's logging configuration to allow for manually setting the access log bucket.

Additional Information/Context

No response

CDK CLI Version

2.91.0

Framework Version

No response

Node.js Version

v14.21.3

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response

[khushail]

Thanks for reaching out @kristian-d. Using bucket policy is the recommended approach in our docs to enable server access logging -

You can use either a bucket policy or bucket access control lists (ACLs) to grant log delivery permissions. However, we recommend that you use a bucket policy. If the target bucket uses the bucket owner enforced setting for Object Ownership, ACLs are disabled and no longer affect permissions. In that case, you must use a bucket policy to grant access permissions to the logging service principal. For more information, see Permissions for log delivery. When you create new buckets, ACLs are disabled by default.

Also here is another reference of the suggestion in our docs for allowing access log delivery using a Bucket Policy.

With that being said, it would be helpful to understand your usecase. Could you please elaborate your usecase for better understanding. Thanks

[peterwoodworth]

@kristian-d this was fixed a while back in this PR; it requires a feature flag to be set in your cdk.json:

    "@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy": true,

[kfdarlington]

@peterwoodworth Thanks! I've confirmed that the flag worked as intended for my use case. I missed this option in the documentation. For others finding this thread in the future, the context's feature flag is described here.

I do think the suggestion from @kylelaker in the linked PR

Instead of being behind a feature flag, this could just be done automatically if BUCKET_OWNER_ENFORCED is set because that would break any services that require ACLs anyway

(and from myself above) would be worth reconsideration as it would avoid an explicitly defined feature flag and better facilitate best practices.

[peterwoodworth]

Yes this makes sense @kristian-d, thanks for the feedback 🙂

I think this didn't happen due to what this comment refers to about this PR. If this isn't true then we could reconsider

[kfdarlington]

From my understanding of that thread and its linked PR, the problem was caused by updating S3 access log bucket default configurations to exclude AccessControl: LogDeliveryWrite and disable ACLs. Doing so could cause problems for any dual-purpose access log bucket that does require ACLs (though the CloudFront example in particular was disproved by @kylelaker here in that there is already an ACL conflict preventing the use case).

A solution that conditionally avoids setting AccessControl: LogDeliveryWrite when a customer explicitly disables ACLs on their access log bucket with ObjectOwnership: BucketOwnerEnforced shouldn't impact existing or new access log buckets that do require ACLs.

[peterwoodworth]

This makes sense to me - we'd be willing to accept a contribution for this @kristian-d, assuming I'm not missing anything else