fix(ecs_patterns): openListener should be false when custom sg is provided#35297
Merged
mergify[bot] merged 6 commits intoaws:mainfrom Aug 25, 2025
Merged
fix(ecs_patterns): openListener should be false when custom sg is provided#35297mergify[bot] merged 6 commits intoaws:mainfrom
mergify[bot] merged 6 commits intoaws:mainfrom
Conversation
aws-cdk-automation
previously requested changes
Aug 21, 2025
…m security groups Adds smart default logic that automatically sets openListener to false when custom load balancers are provided, preventing unintended creation of 0.0.0.0/0 ingress rules when users have configured custom security groups. - Add feature flag @aws-cdk/aws-ecs-patterns:smartDefaultOpenListener for backward compatibility - Detect custom load balancers and default openListener to false for improved security - Maintain explicit override capability with openListener: true - Add comprehensive test coverage including integration tests - Preserve backward compatibility when feature flag is disabled Closes aws#35292
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
Abogical
requested changes
Aug 22, 2025
Contributor
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Contributor
|
Comments on closed issues and PRs are hard for our team to see. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue # (if applicable)
Closes #35292.
Reason for this change
This change addresses a security vulnerability in the ECS patterns where providing a custom load balancer would still result in the creation of overly permissive internet access rules, potentially bypassing user-intended security controls.
The Security Issue:
The
openListenerproperty controls whether CDK automatically creates security group ingress rules allowing internet traffic to reach the load balancer:openListener: true(current default): CDK automatically creates anAWS::EC2::SecurityGroupIngressrule withCidrIp: '0.0.0.0/0', allowing anyone on the internet to access the load balancer on the listener portopenListener: false: CDK does not create automatic ingress rules, leaving security group management entirely to the userThe Problem:
Currently,
openListeneralways defaults totrue, regardless of whether users have provided custom load balancers with their own security groups. This creates a security gap where:0.0.0.0/0ingress rules, potentially bypassing the user's intended access controlsWhy This Matters:
When users provide custom load balancers, they typically want to manage security group rules themselves. The automatic creation of
0.0.0.0/0rules can unintentionally expose services to the internet, creating a security vulnerability that users may not immediately notice.Description of changes
This implementation adds a smart default mechanism for the
openListenerproperty inApplicationLoadBalancedServiceBasethat detects when users provide custom load balancers (which typically have custom security groups) and automatically defaultsopenListenertofalsefor improved security.Key changes made:
ApplicationLoadBalancedServiceBase.ts@aws-cdk/aws-ecs-patterns:smartDefaultOpenListenerfeature flag to ensure backward compatibilityopenListenerdefaults tofalseinstead oftrueopenListener: trueto override the smart default if neededTechnical implementation details:
openListenerdefaults tofalse, preventing automatic creation ofAWS::EC2::SecurityGroupIngressrules withCidrIp: '0.0.0.0/0'true)CloudFormation Impact:
openListener: true): CDK generatesAWS::EC2::SecurityGroupIngressresources allowing internet access (CidrIp: '0.0.0.0/0')openListener: false): No automatic ingress rules are created, users maintain full control over security group configurationDesign decisions made:
props.loadBalancer !== undefinedas the detection mechanism since custom load balancers typically indicate custom security group managementopenListener: trueto override smart defaults when neededAlternatives considered and rejected:
Problem Description:
flowchart TD A[User Creates ECS Service] --> B[User Provides Custom Load Balancer<br/>with Custom Security Groups] B --> C[User Configures Restrictive Rules<br/>e.g., only VPC access] C --> D[CDK Always Defaults openListener = true] D --> E[CDK Creates 0.0.0.0/0 Ingress Rule<br/>⚠️ BYPASSES User's Security Intent] E --> F[🚨 Unintended Internet Exposure<br/>Security Vulnerability] style E fill:#ffebee style F fill:#ffcdd2Smart Default Logic (Solution):
flowchart TD A[ECS Service Created] --> B{Feature Flag Enabled?} B -->|No| C[Legacy: openListener = true<br/>Creates 0.0.0.0/0 ingress rules] B -->|Yes| D{Custom Load Balancer Provided?} D -->|No| E[Default: openListener = true<br/>🌐 Creates 0.0.0.0/0 ingress rules] D -->|Yes| F[Smart Default: openListener = false<br/>🔒 No automatic ingress rules] style F fill:#e8f5e8 style E fill:#fff3e0 style C fill:#f5f5f5Describe any new or updated permissions being added
N/A - This change does not introduce new IAM permissions or modify existing permission requirements. The change only affects the default behavior of security group rule creation, using existing ECS and ELB permissions.
Description of how you validated changes
Unit tests: Added comprehensive test coverage with 5 new test cases:
openListenerdefaults tofalseopenListenerdefaults totrueopenListener: true(feature flag enabled) - verifies explicit values override smart defaultsIntegration tests: Created comprehensive integration test (
integ.alb-fargate-service-smart-defaults.ts) that:Manual validation:
openListener: trueoverride functionalityRegression testing:
Performance testing:
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license