fix(s3): Reduce notifications handler permissions (#5925)

This restricts them to resources only the buckets that is required. To ensure permissions don't get too large minmize is also enabled on the policy. Closes #5925
aws · Sep 25, 2023 · 05663d2 · 05663d2
1 parent ab52bb5
commit 05663d2
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 13 deletions.
diff --git a/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource-handler.ts b/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource-handler.ts
@@ -52,24 +52,28 @@ export class NotificationsResourceHandler extends Construct {
   public readonly functionArn: string;
 
   /**
-   * The role of the handler's lambda function.
+   * The default policy of role of the handler's lambda function.
    */
-  public readonly role: iam.IRole;
+  private readonly policy: iam.Policy;
 
   constructor(scope: Construct, id: string, props: NotificationsResourceHandlerProps = {}) {
     super(scope, id);
 
-    this.role = props.role ?? new iam.Role(this, 'Role', {
+    const role = props.role ?? new iam.Role(this, 'Role', {
       assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
     });
 
-    this.role.addManagedPolicy(
+    role.addManagedPolicy(
       iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole'),
     );
-    this.role.addToPrincipalPolicy(new iam.PolicyStatement({
-      actions: ['s3:PutBucketNotification'],
-      resources: ['*'],
-    }));
+
+    // We create our own default policy so we can set minimize to true and statements size
+    // remain small
+    const policy = new iam.Policy(this, 'DefaultPolicy', {
+      document: new iam.PolicyDocument({ minimize: true }),
+    });
+    role.attachInlinePolicy(policy);
+    this.policy = policy;
 
     const resourceType = 'AWS::Lambda::Function';
     class InLineLambda extends cdk.CfnResource {
@@ -97,17 +101,18 @@ export class NotificationsResourceHandler extends Construct {
         Description: 'AWS CloudFormation handler for "Custom::S3BucketNotifications" resources (@aws-cdk/aws-s3)',
         Code: { ZipFile: handlerSourceWithoutComments },
         Handler: 'index.handler',
-        Role: this.role.roleArn,
+        Role: role.roleArn,
         Runtime: 'python3.9',
         Timeout: 300,
       },
     });
-    resource.node.addDependency(this.role);
+    resource.node.addDependency(role);
+    resource.node.addDependency(policy);
 
     this.functionArn = resource.getAtt('Arn').toString();
   }
 
   public addToRolePolicy(statement: iam.PolicyStatement) {
-    this.role.addToPrincipalPolicy(statement);
+    this.policy.addStatements(statement);
   }
 }
diff --git a/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts b/packages/aws-cdk-lib/aws-s3/lib/notifications-resource/notifications-resource.ts
@@ -116,13 +116,17 @@ export class BucketNotifications extends Construct {
       const handler = NotificationsResourceHandler.singleton(this, {
         role: this.handlerRole,
       });
+      handler.addToRolePolicy(new iam.PolicyStatement({
+        actions: ['s3:PutBucketNotification'],
+        resources: [this.bucket.bucketArn],
+      }));
 
       const managed = this.bucket instanceof Bucket;
 
       if (!managed) {
         handler.addToRolePolicy(new iam.PolicyStatement({
           actions: ['s3:GetBucketNotification'],
-          resources: ['*'],
+          resources: [this.bucket.bucketArn],
         }));
       }
 

diff --git a/packages/aws-cdk-lib/aws-s3/test/notification.test.ts b/packages/aws-cdk-lib/aws-s3/test/notification.test.ts
@@ -116,7 +116,7 @@ describe('notification', () => {
           ],
         },
       },
-      DependsOn: ['BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleDefaultPolicy2CF63D36',
+      DependsOn: ['BucketNotificationsHandler050a0587b7544547bf325f094a3db834DefaultPolicy8DF38B44',
         'BucketNotificationsHandler050a0587b7544547bf325f094a3db834RoleB6FB88EC'],
     });
   });
@@ -185,4 +185,122 @@ describe('notification', () => {
       Runtime: 'python3.9',
     });
   });
+  test('Check notifications handler permissions for single imported bucket', () => {
+    const stack = new cdk.Stack();
+
+    const bucket = s3.Bucket.fromBucketAttributes(stack, 'MyBucket', {
+      bucketName: 'foo-bar',
+    });
+
+    bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
+      bind: () => ({
+        arn: 'ARN',
+        type: s3.BucketNotificationDestinationType.TOPIC,
+      }),
+    });
+
+    Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Statement: [
+          {
+            Action: [
+              's3:GetBucketNotification',
+              's3:PutBucketNotification',
+            ],
+            Effect: 'Allow',
+            Resource: {
+              'Fn::Join':
+                    ['', [
+                      'arn:',
+                      { Ref: 'AWS::Partition' },
+                      ':s3:::foo-bar',
+                    ]],
+            },
+          },
+        ],
+      },
+    });
+  });
+  test('Check notifications handler permissions for two imported bucket', () => {
+    const stack = new cdk.Stack();
+
+    const bucket = s3.Bucket.fromBucketAttributes(stack, 'MyBucket', {
+      bucketName: 'foo-bar',
+    });
+
+    bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
+      bind: () => ({
+        arn: 'ARN',
+        type: s3.BucketNotificationDestinationType.TOPIC,
+      }),
+    });
+
+    const bucket2 = s3.Bucket.fromBucketAttributes(stack, 'MyBucket2', {
+      bucketName: 'foo-bar2',
+    });
+
+    bucket2.addEventNotification(s3.EventType.OBJECT_CREATED, {
+      bind: () => ({
+        arn: 'ARN',
+        type: s3.BucketNotificationDestinationType.TOPIC,
+      }),
+    });
+
+    Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Statement: [
+          {
+            Action: [
+              's3:GetBucketNotification',
+              's3:PutBucketNotification',
+            ],
+            Effect: 'Allow',
+            Resource: [{
+              'Fn::Join':
+                  ['', [
+                    'arn:',
+                    { Ref: 'AWS::Partition' },
+                    ':s3:::foo-bar',
+                  ]],
+            }, {
+              'Fn::Join':
+                  ['', [
+                    'arn:',
+                    { Ref: 'AWS::Partition' },
+                    ':s3:::foo-bar2',
+                  ]],
+            }],
+          },
+        ],
+      },
+    });
+  });
+  test('Check notifications handler permissions for single bucket', () => {
+    const stack = new cdk.Stack();
+
+    const bucket = new s3.Bucket(stack, 'MyBucket', {
+      bucketName: 'foo-bar',
+    });
+
+    bucket.addEventNotification(s3.EventType.OBJECT_CREATED, {
+      bind: () => ({
+        arn: 'ARN',
+        type: s3.BucketNotificationDestinationType.TOPIC,
+      }),
+    });
+
+    Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
+      PolicyDocument: {
+        Statement: [
+          {
+            Action: 's3:PutBucketNotification',
+            Effect: 'Allow',
+            Resource: {
+              'Fn::GetAtt': ['MyBucketF68F3FF0', 'Arn'],
+            },
+          },
+        ],
+      },
+    });
+  });
 });