fix(s3): Bucket.grantWrite() no longer adds s3:PutObject* permission

Right now, Bucket.grantWrite() adds an 's3:PutObject*' permission to the passed principal.
That means it grants the 's3:PubObjectAcl' permission,
which allows changing the ACL of an individual object written to the Bucket,
potentially giving it more visibility than other objects in the Bucket.

Change that behavior to grant 's3:PutObject' instead.
Since customers might be relying on the old overly-broad permissions,
gate that change behind a feature flag,
which means only newly created CDK projects will get this behavior.

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

@@ -5,8 +5,9 @@ import * as iam from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
 import {
   Fn, IResource, Lazy, RemovalPolicy, Resource, Stack, Token,
-  CustomResource, CustomResourceProvider, CustomResourceProviderRuntime,
+  CustomResource, CustomResourceProvider, CustomResourceProviderRuntime, FeatureFlags, ResourceProps,
 } from '@aws-cdk/core';
+import * as cxapi from '@aws-cdk/cx-api';
 import { Construct } from 'constructs';
 import { BucketPolicy } from './bucket-policy';
 import { IBucketNotificationDestination } from './destination';
@@ -383,6 +384,14 @@ abstract class BucketBase extends Resource implements IBucket {
    */
   protected abstract disallowPublicAccess?: boolean;
 
+  private readonly skipPutObjectAclInGrantWrite: boolean | undefined;
+
+  constructor(scope: Construct, id: string, props?: ResourceProps) {
+    super(scope, id, props);
+
+    this.skipPutObjectAclInGrantWrite = FeatureFlags.of(this).isEnabled(cxapi.S3_BUCKET_GRANT_WRITE_ACL);
+  }
+
   /**
    * Define a CloudWatch event that triggers when something happens to this repository
    *
@@ -584,7 +593,7 @@ abstract class BucketBase extends Resource implements IBucket {
    * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
    */
   public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*') {
-    return this.grant(identity, perms.BUCKET_WRITE_ACTIONS, perms.KEY_WRITE_ACTIONS,
+    return this.grant(identity, this.writeActions, perms.KEY_WRITE_ACTIONS,
       this.bucketArn,
       this.arnForObjects(objectsKeyPattern));
   }
@@ -598,7 +607,7 @@ abstract class BucketBase extends Resource implements IBucket {
    * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
    */
   public grantPut(identity: iam.IGrantable, objectsKeyPattern: any = '*') {
-    return this.grant(identity, perms.BUCKET_PUT_ACTIONS, perms.KEY_WRITE_ACTIONS,
+    return this.grant(identity, this.putActions, perms.KEY_WRITE_ACTIONS,
       this.arnForObjects(objectsKeyPattern));
   }
 
@@ -625,7 +634,7 @@ abstract class BucketBase extends Resource implements IBucket {
    * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
    */
   public grantReadWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*') {
-    const bucketActions = perms.BUCKET_READ_ACTIONS.concat(perms.BUCKET_WRITE_ACTIONS);
+    const bucketActions = perms.BUCKET_READ_ACTIONS.concat(this.writeActions);
     // we need unique permissions because some permissions are common between read and write key actions
     const keyActions = [...new Set([...perms.KEY_READ_ACTIONS, ...perms.KEY_WRITE_ACTIONS])];
 
@@ -673,6 +682,17 @@ abstract class BucketBase extends Resource implements IBucket {
     });
   }
 
+  private get writeActions(): string[] {
+    return [
+      ...perms.BUCKET_DELETE_ACTIONS,
+      ...this.putActions,
+    ];
+  }
+
+  private get putActions(): string[] {
+    return this.skipPutObjectAclInGrantWrite ? perms.BUCKET_PUT_ACTIONS : perms.LEGACY_BUCKET_PUT_ACTIONS;
+  }
+
   private urlJoin(...components: string[]): string {
     return components.reduce((result, component) => {
       if (result.endsWith('/')) {

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

@@ -4,18 +4,18 @@ export const BUCKET_READ_ACTIONS = [
   's3:List*',
 ];
 
-export const BUCKET_PUT_ACTIONS = [
+export const LEGACY_BUCKET_PUT_ACTIONS = [
   's3:PutObject*',
   's3:Abort*',
 ];
 
-export const BUCKET_DELETE_ACTIONS = [
-  's3:DeleteObject*',
+export const BUCKET_PUT_ACTIONS = [
+  's3:PutObject',
+  's3:Abort*',
 ];
 
-export const BUCKET_WRITE_ACTIONS = [
-  ...BUCKET_DELETE_ACTIONS,
-  ...BUCKET_PUT_ACTIONS,
+export const BUCKET_DELETE_ACTIONS = [
+  's3:DeleteObject*',
 ];
 
 export const KEY_READ_ACTIONS = [

packages/@aws-cdk/aws-s3/package.json

@@ -85,6 +85,7 @@
     "@aws-cdk/aws-iam": "0.0.0",
     "@aws-cdk/aws-kms": "0.0.0",
     "@aws-cdk/core": "0.0.0",
+    "@aws-cdk/cx-api": "0.0.0",
     "constructs": "^3.2.0"
   },
   "homepage": "https://github.com/aws/aws-cdk",
@@ -93,6 +94,7 @@
     "@aws-cdk/aws-iam": "0.0.0",
     "@aws-cdk/aws-kms": "0.0.0",
     "@aws-cdk/core": "0.0.0",
+    "@aws-cdk/cx-api": "0.0.0",
     "constructs": "^3.2.0"
   },
   "engines": {