feat(s3): use Bucket Policy for Server Access Logging grant (under feature flag)

packages/@aws-cdk/aws-s3/README.md

@@ -349,7 +349,25 @@ const bucket = new s3.Bucket(this, 'MyBucket', {
 });
 ```
 
-[S3 Server access logging]: https://docs.aws.amazon.com/AmazonS3/latest/dev/ServerLogs.html
+### Allowing access log delivery using a Bucket Policy (recommended)
+
+When possible, it is recommended to use a bucket policy to grant access instead of
+using ACLs. When the `@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy` feature flag
+is enabled, this is done by default for server access logs. If S3 Server Access Logs
+are the only logs delivered to your bucket (or if all other services logging to the
+bucket support using bucket policy instead of ACLs), you can set object ownership
+to [bucket owner enforced](#bucket-owner-enforced-recommended), as is recommended.
+
+```ts
+const accessLogsBucket = new s3.Bucket(this, 'AccessLogsBucket', {
+  objectOwnership: s3.ObjectOwnership.BUCKET_OWNER_ENFORCED,
+});
+
+const bucket = new s3.Bucket(this, 'MyBucket', {
+  serverAccessLogsBucket: accessLogsBucket,
+  serverAccessLogsPrefix: 'logs',
+});
+```
 
 ## S3 Inventory
 
@@ -485,14 +503,23 @@ new s3.Bucket(this, 'MyBucket', {
 
 ### Bucket owner enforced (recommended)
 
-ACLs are disabled, and the bucket owner automatically owns and has full control over every object in the bucket. ACLs no longer affect permissions to data in the S3 bucket. The bucket uses policies to define access control.
+ACLs are disabled, and the bucket owner automatically owns and has full control
+over every object in the bucket. ACLs no longer affect permissions to data in the
+S3 bucket. The bucket uses policies to define access control.
 
 ```ts
 new s3.Bucket(this, 'MyBucket', {
   objectOwnership: s3.ObjectOwnership.BUCKET_OWNER_ENFORCED,
 });
 ```
 
+Some services may not not support log delivery to buckets that have object ownership
+set to bucket owner enforced, such as
+[S3 buckets using ACLs](#allowing-access-log-delivery-using-a-bucket-policy-recommended)
+or [CloudFront Distributions][CloudFront S3 Bucket].
+
+[CloudFront S3 Bucket]: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/AccessLogs.html#AccessLogsBucketAndFileOwnership
+
 ## Bucket deletion
 
 When a bucket is removed from a stack (or the stack is deleted), the S3

packages/@aws-cdk/aws-s3/lib/bucket.ts

@@ -1832,7 +1832,9 @@ export class Bucket extends BucketBase {
     }
 
     if (props.serverAccessLogsBucket instanceof Bucket) {
-      props.serverAccessLogsBucket.allowLogDelivery();
+      props.serverAccessLogsBucket.allowLogDelivery(this, props.serverAccessLogsPrefix);
+    } else if (props.serverAccessLogsPrefix) {
+      this.allowLogDelivery(this, props.serverAccessLogsPrefix);
     }
 
     for (const inventory of props.inventories ?? []) {
@@ -2189,17 +2191,35 @@ export class Bucket extends BucketBase {
   }
 
   /**
-   * Allows the LogDelivery group to write, fails if ACL was set differently.
+   * Allows Log Delivery to the S3 bucket, using a Bucket Policy if the relevant feature
+   * flag is enabled, otherwise the canned ACL is used.
+   *
+   * If log delivery is to be allowed using the ACL and an ACL has already been set, this fails.
    *
    * @see
-   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl
-   */
-  private allowLogDelivery() {
-    if (this.accessControl && this.accessControl !== BucketAccessControl.LOG_DELIVERY_WRITE) {
+   * https://docs.aws.amazon.com/AmazonS3/latest/userguide/enable-server-access-logging.html
+   */
+  private allowLogDelivery(from: IBucket, prefix?: string) {
+    if (FeatureFlags.of(this).isEnabled(cxapi.S3_SERVER_ACCESS_LOGS_USE_BUCKET_POLICY)) {
+      this.addToResourcePolicy(new iam.PolicyStatement({
+        effect: iam.Effect.ALLOW,
+        principals: [new iam.ServicePrincipal('logging.s3.amazonaws.com')],
+        actions: ['s3:PutObject'],
+        resources: [this.arnForObjects(prefix ? `${prefix}*`: '*')],
+        conditions: {
+          ArnLike: {
+            'aws:SourceArn': from.bucketArn,
+          },
+          StringEquals: {
+            'aws:SourceAccount': from.env.account,
+          },
+        },
+      }));
+    } else if (this.accessControl && this.accessControl !== BucketAccessControl.LOG_DELIVERY_WRITE) {
       throw new Error("Cannot enable log delivery to this bucket because the bucket's ACL has been set and can't be changed");
+    } else {
+      this.accessControl = BucketAccessControl.LOG_DELIVERY_WRITE;
     }
-
-    this.accessControl = BucketAccessControl.LOG_DELIVERY_WRITE;
   }
 
   private parseInventoryConfiguration(): CfnBucket.InventoryConfigurationProperty[] | undefined {

packages/@aws-cdk/aws-s3/test/bucket.test.ts

@@ -2205,6 +2205,71 @@ describe('bucket', () => {
     ).toThrow(/Cannot enable log delivery to this bucket because the bucket's ACL has been set and can't be changed/);
   });
 
+  test('Bucket Allow Log delivery should use the recommended policy when flag enabled', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    stack.node.setContext('@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy', true);
+
+    // WHEN
+    const bucket = new s3.Bucket(stack, 'TestBucket', { serverAccessLogsPrefix: 'test' });
+
+    // THEN
+    const template = Template.fromStack(stack);
+    template.hasResourceProperties('AWS::S3::Bucket', {
+      AccessControl: Match.absent(),
+    });
+    template.hasResourceProperties('AWS::S3::BucketPolicy', {
+      Bucket: stack.resolve(bucket.bucketName),
+      PolicyDocument: Match.objectLike({
+        Statement: Match.arrayWith([Match.objectLike({
+          Effect: 'Allow',
+          Principal: { Service: 'logging.s3.amazonaws.com' },
+          Action: 's3:PutObject',
+          Resource: stack.resolve(`${bucket.bucketArn}/test*`),
+          Condition: {
+            ArnLike: {
+              'aws:SourceArn': stack.resolve(bucket.bucketArn),
+            },
+            StringEquals: {
+              'aws:SourceAccount': { 'Ref': 'AWS::AccountId' },
+            },
+          },
+        })]),
+      }),
+    });
+  });
+
+  test('Log Delivery bucket policy should properly set source bucket ARN/Account', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    stack.node.setContext('@aws-cdk/aws-s3:serverAccessLogsUseBucketPolicy', true);
+
+    // WHEN
+    const targetBucket = new s3.Bucket(stack, 'TargetBucket');
+    const sourceBucket = new s3.Bucket(stack, 'SourceBucket', { serverAccessLogsBucket: targetBucket });
+
+    // THEN
+    Template.fromStack(stack).hasResourceProperties('AWS::S3::BucketPolicy', {
+      Bucket: stack.resolve(targetBucket.bucketName),
+      PolicyDocument: Match.objectLike({
+        Statement: Match.arrayWith([Match.objectLike({
+          Effect: 'Allow',
+          Principal: { Service: 'logging.s3.amazonaws.com' },
+          Action: 's3:PutObject',
+          Resource: stack.resolve(`${targetBucket.bucketArn}/*`),
+          Condition: {
+            ArnLike: {
+              'aws:SourceArn': stack.resolve(sourceBucket.bucketArn),
+            },
+            StringEquals: {
+              'aws:SourceAccount': stack.resolve(sourceBucket.env.account),
+            },
+          },
+        })]),
+      }),
+    });
+  });
+
   test('Defaults for an inventory bucket', () => {
     // Given
     const stack = new cdk.Stack();

packages/@aws-cdk/aws-s3/test/integ.bucket.server-access-logs.js.snapshot/aws-cdk-s3-access-logs.assets.json

@@ -1,15 +1,15 @@
 {
-  "version": "20.0.0",
+  "version": "22.0.0",
   "files": {
-    "8091e7796cbf83b5d8109a10ea23dfbe9ac04ad84434da5dd183b89eb89e351b": {
+    "f58a2b25314952a1a5a6b42c6b9092caf2710430af09ba4f5d807e60f2fd3542": {
       "source": {
         "path": "aws-cdk-s3-access-logs.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "8091e7796cbf83b5d8109a10ea23dfbe9ac04ad84434da5dd183b89eb89e351b.json",
+          "objectKey": "f58a2b25314952a1a5a6b42c6b9092caf2710430af09ba4f5d807e60f2fd3542.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }

packages/@aws-cdk/aws-s3/test/integ.bucket.server-access-logs.js.snapshot/aws-cdk-s3-access-logs.template.json

@@ -2,12 +2,58 @@
  "Resources": {
   "MyAccessLogsBucketF7FE6635": {
    "Type": "AWS::S3::Bucket",
-   "Properties": {
-    "AccessControl": "LogDeliveryWrite"
-   },
    "UpdateReplacePolicy": "Delete",
    "DeletionPolicy": "Delete"
   },
+  "MyAccessLogsBucketPolicyEA9AB063": {
+   "Type": "AWS::S3::BucketPolicy",
+   "Properties": {
+    "Bucket": {
+     "Ref": "MyAccessLogsBucketF7FE6635"
+    },
+    "PolicyDocument": {
+     "Statement": [
+      {
+       "Action": "s3:PutObject",
+       "Condition": {
+        "ArnLike": {
+         "aws:SourceArn": {
+          "Fn::GetAtt": [
+           "MyBucketF68F3FF0",
+           "Arn"
+          ]
+         }
+        },
+        "StringEquals": {
+         "aws:SourceAccount": {
+          "Ref": "AWS::AccountId"
+         }
+        }
+       },
+       "Effect": "Allow",
+       "Principal": {
+        "Service": "logging.s3.amazonaws.com"
+       },
+       "Resource": {
+        "Fn::Join": [
+         "",
+         [
+          {
+           "Fn::GetAtt": [
+            "MyAccessLogsBucketF7FE6635",
+            "Arn"
+           ]
+          },
+          "/example*"
+         ]
+        ]
+       }
+      }
+     ],
+     "Version": "2012-10-17"
+    }
+   }
+  },
   "MyBucketF68F3FF0": {
    "Type": "AWS::S3::Bucket",
    "Properties": {

packages/@aws-cdk/aws-s3/test/integ.bucket.server-access-logs.js.snapshot/cdk.out

@@ -1 +1 @@
-{"version":"20.0.0"}
+{"version":"22.0.0"}

packages/@aws-cdk/aws-s3/test/integ.bucket.server-access-logs.js.snapshot/integ.json

@@ -1,5 +1,5 @@
 {
-  "version": "20.0.0",
+  "version": "22.0.0",
   "testCases": {
     "integ.bucket.server-access-logs": {
       "stacks": [

packages/@aws-cdk/aws-s3/test/integ.bucket.server-access-logs.js.snapshot/manifest.json

@@ -1,12 +1,6 @@
 {
-  "version": "20.0.0",
+  "version": "22.0.0",
   "artifacts": {
-    "Tree": {
-      "type": "cdk:tree",
-      "properties": {
-        "file": "tree.json"
-      }
-    },
     "aws-cdk-s3-access-logs.assets": {
       "type": "cdk:asset-manifest",
       "properties": {
@@ -23,7 +17,7 @@
         "validateOnSynth": false,
         "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
         "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
-        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/8091e7796cbf83b5d8109a10ea23dfbe9ac04ad84434da5dd183b89eb89e351b.json",
+        "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/f58a2b25314952a1a5a6b42c6b9092caf2710430af09ba4f5d807e60f2fd3542.json",
         "requiresBootstrapStackVersion": 6,
         "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
         "additionalDependencies": [
@@ -45,6 +39,12 @@
             "data": "MyAccessLogsBucketF7FE6635"
           }
         ],
+        "/aws-cdk-s3-access-logs/MyAccessLogsBucket/Policy/Resource": [
+          {
+            "type": "aws:cdk:logicalId",
+            "data": "MyAccessLogsBucketPolicyEA9AB063"
+          }
+        ],
         "/aws-cdk-s3-access-logs/MyBucket/Resource": [
           {
             "type": "aws:cdk:logicalId",
@@ -65,6 +65,12 @@
         ]
       },
       "displayName": "aws-cdk-s3-access-logs"
+    },
+    "Tree": {
+      "type": "cdk:tree",
+      "properties": {
+        "file": "tree.json"
+      }
     }
   }
 }