Skip to content

Commit c0300d2

Browse files
authored
fix(s3): scope BucketNotificationsHandler IAM permissions to specific bucket ARNs (#35334)
### Issue # (if applicable) Closes #35331 #5925 ### Reason for this change The S3 BucketNotificationsHandler Lambda function created by CDK for S3 bucket notifications receives overly broad IAM permissions with `"Resource": "*"` instead of being scoped to specific bucket ARNs. This violates the principle of least privilege and creates a security risk where the handler could potentially modify notifications for any S3 bucket in the account, preventing security teams from approving deployments. ### Description of changes Implemented scoped IAM permissions for the S3 `BucketNotificationsHandler` by leveraging the existing `grantBucketNotifications()` method and the `@aws-cdk/aws-iam:minimizePolicies` feature flag: - **Simplified approach**: Removed complex Aspect-based implementation in favor of using existing CDK mechanisms - **Feature flag utilization**: The `@aws-cdk/aws-iam:minimizePolicies` feature flag automatically consolidates IAM policies with specific bucket ARNs - **Grant-based permissions**: Uses the existing `grantBucketNotifications()` method which already provides scoped permissions to specific bucket ARNs - **Automatic consolidation**: When multiple buckets use the same handler, policies are automatically consolidated into a single statement with all bucket ARNs - **Backward compatibility**: Maintains singleton pattern and all existing functionality - no breaking changes **Before**: IAM policy contained `"Resource": "*"` (wildcard permissions) **After**: IAM policy contains `"Resource": [{"Fn::GetAtt": ["BucketName", "Arn"]}, ...]` (scoped permissions) ### Describe any new or updated permissions being added **Security improvement**: This change reduces permissions rather than adding new ones. The handler now receives more restrictive IAM permissions scoped only to the specific S3 buckets it needs to manage, implementing the principle of least privilege. No new permissions are required. ### Description of how you validated changes - **Unit tests**: All existing 22 notification tests pass successfully (100% success rate). Tests already expect scoped permissions due to the existing grant method implementation - **Integration tests**: Existing integration test snapshots show the correct IAM policy structure with specific bucket ARNs instead of wildcards. All integration tests pass - **CloudFormation validation**: Verified that generated templates contain specific bucket ARNs instead of wildcards and deploy successfully - **Regression testing**: Confirmed all existing S3 bucket notification functionality works unchanged, including EventBridge notifications, filter rules, and both managed and unmanaged bucket scenarios ### Potential Concerns 1. **Simplified Implementation**: The current approach leverages existing CDK mechanisms (`grantBucketNotifications()` method and `@aws-cdk/aws-iam:minimizePolicies` feature flag) rather than implementing custom Aspect logic. This is actually a strength as it reduces complexity and maintenance burden while achieving the same security outcome. 2. **Feature Flag Dependency**: The solution relies on the `@aws-cdk/aws-iam:minimizePolicies` feature flag for policy consolidation. This flag is already widely adopted and is part of CDK's recommended feature flags, making this a low-risk dependency. 3. **Potential for Regressions**: The PR fixes a security vulnerability that has been present for a while. The existing comprehensive test suite provides good coverage, and the integration tests serve as the best defense against future regressions. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 04eae8c commit c0300d2

File tree

89 files changed

+24078
-22427
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+24078
-22427
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-event-sources/test/integ.s3.imported-bucket.js.snapshot/TestStack1.assets.json

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-event-sources/test/integ.s3.imported-bucket.js.snapshot/TestStack2.assets.json

Lines changed: 5 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-event-sources/test/integ.s3.imported-bucket.js.snapshot/TestStack2.template.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,21 @@
157157
"s3:PutBucketNotification"
158158
],
159159
"Effect": "Allow",
160-
"Resource": "*"
160+
"Resource": {
161+
"Fn::Join": [
162+
"",
163+
[
164+
"arn:",
165+
{
166+
"Ref": "AWS::Partition"
167+
},
168+
":s3:::",
169+
{
170+
"Fn::ImportValue": "TestStack1:ExportsOutputRefbucket43879C716CF1CFA3"
171+
}
172+
]
173+
]
174+
}
161175
}
162176
],
163177
"Version": "2012-10-17"

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-event-sources/test/integ.s3.imported-bucket.js.snapshot/cdk.out

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-event-sources/test/integ.s3.imported-bucket.js.snapshot/integ.json

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-lambda-event-sources/test/integ.s3.imported-bucket.js.snapshot/issue4324DefaultTestDeployAssert63CED672.assets.json

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)