search

aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.71k stars 3.94k forks source link

IApplicationListener: Bad implicit modifications on security groups #25490

Open Shard opened 1 year ago

Shard commented 1 year ago

Describe the bug

I have a setup where there is an existing ALB, Target group and ASG of EC2 instances which is all manually managed. I have a new fargate cluster that is being managed by cdk that I want to attach to the existing ALB so i can share the domain name setup against that ALB but only for a path.

The code for adding the extra rule to the listener along with the fargate task looks like:

const vpc = EC2.Vpc.fromLookup(this, 'VPC', { vpcId: props.vpcId });
const cluster = new ECS.Cluster(this, 'Cluster', { vpc });
const appSecurityGroup = new EC2.SecurityGroup(this, this.getName('AppSG'), { vpc, allowAllOutbound: true });
appSecurityGroup.addIngressRule(EC2.Peer.anyIpv4(), EC2.Port.tcp(3000));

const lbListener = ApplicationListener.fromLookup(this, 'LBListener', {
  listenerPort: 443,
  loadBalancerTags: { 'foo': 'bar' },
});

const taskDefinition = new ECS.FargateTaskDefinition(this, 'TaskDefinition', {
  cpu: 256,
  memoryLimitMiB: 512,
});
taskDefinition.addContainer('Container', {
  logging: new ECS.AwsLogDriver({ streamPrefix: 'Container' }),
  image: ECS.ContainerImage.fromDockerImageAsset(imageAsset),
  portMappings: [{ containerPort: 3000 }],
  environment: ...,
})

const service = new ECS.FargateService(this, 'Service', {
  cluster,
  taskDefinition,
  securityGroups: [appSecurityGroup],
  assignPublicIp: true
});

const targetGroup = new ApplicationTargetGroup(this, 'TargetGroup', {
  vpc,
  port: 3000,
  protocol: ApplicationProtocol.HTTP,
  targetType: TargetType.IP,
  targets: [service],
  healthCheck: { path: '/health' }
});

lbListener.addAction('Action', {
  action: ListenerAction.forward([targetGroup]),
  priority: 1,
  conditions: [ListenerCondition.pathPatterns(['/rest/*'])],
})

The problem is cdk is deciding to change a security group not explicitly linked here, a security group that was applied to both the ec2 instance and ALB and it changes the outbound rules from allow all ipv4 to only allow port 3000 tcp, breaking the routing between the ALB and EC2 instances.

Expected Behavior

CDK shouldn't be changing security groups that haven't been linked to or provide a way to remove the connection so that it is not applied to security groups not managed by cdk.

Current Behavior

cdk deploy requests to change a security group it is not managing:

┌───┬────────────────────────┬─────┬────────────┬────────────────────────┐
│   │ Group                  │ Dir │ Protocol   │ Peer                   │
├───┼────────────────────────┼─────┼────────────┼────────────────────────┤
│ + │ ${AppSG.GroupId}       │ In  │ TCP 3000   │ Everyone (IPv4)        │
│ + │ ${AppSG.GroupId}       │ In  │ TCP 3000   │ sg-08xxxxxxxxxxxxx25   │
│ + │ ${AppSG.GroupId}       │ Out │ Everything │ Everyone (IPv4)        │
├───┼────────────────────────┼─────┼────────────┼────────────────────────┤
│ + │ sg-08xxxxxxxxxxxxx25   │ Out │ TCP 3000   │ ${AppSG.GroupId}       │
└───┴────────────────────────┴─────┴────────────┴────────────────────────┘

Reproduction Steps

  1. Create an ALB and attach to a target group
  2. Add the tag foo=bar to the ALB
  3. Run the cdk code above within a stack on the same account

Possible Solution

Allow an option in IApplicationListener to prevent implicitly creating connections to existing resources that must be modified by cdk.

Additional Information/Context

A workaround i have implemented but still don't understand how it works, is that i have created new security groups from the ones used by the ALB and EC2 instances, attached them and now cdk only modifies the original security group even though I can't find it being used anywhere under Network Interfaces.

CDK CLI Version

2.75.1

Framework Version

No response

Node.js Version

16.14.0

OS

Linux/Manjaro

Language

Typescript

Language Version

Typescript 5.0.4

Other information

The output relating to the erroneous SG outbound rule from cdk synth output:

AppSGfromLBListenerSecurityGroupsg08xxxxxxxxxxxxx251B698D9E300061C0107E:
    Type: AWS::EC2::SecurityGroupIngress
    Properties:
      IpProtocol: tcp
      Description: Load balancer to target
      FromPort: 3000
      GroupId:
        Fn::GetAtt:
          - ubapiAppSGB1423E6C
          - GroupId
      SourceSecurityGroupId: sg-08xxxxxxxxxxxxx25
      ToPort: 3000
    Metadata:
      aws:cdk:path: staging/ubapi-AppSG/from LBListenerSecurityGroupsg08xxxxxxxxxxxxx251B698D9E:3000
pahud commented 1 year ago

Hi

This is a known issue we are trying to improve. Please see this comment for more details behind this issue.

Can you add allowAllIpv6Outbound: true when you create the new SecurityGroup and see if it works for you?

github-actions[bot] commented 1 year ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

Shard commented 1 year ago

Hi @pahud,

Allowing ipv6 actually created other issues i believe are unrelated to this one. I came up with a hack that looks a bit like this:

EC2.SecurityGroup.fromSecurityGroupId(this, 'default-app', lbListener.connections.securityGroups[0].securityGroupId, {
      mutable: false,
      allowAllIpv6Outbound: false,
      allowAllOutbound: true,
    }).addEgressRule( EC2.Peer.anyIpv4(), EC2.Port.allTraffic());

It seems to have resolved the issue in the staging account, After a stack destroy and new deployment it doesn't end up messing with the egress rules. Will respond when I try it in production and see if i run into any different issues.

sungwoo-lifeoverflow commented 1 year ago

+1

Shard commented 1 year ago

If i destroy the CDK stack via the UI it seems to overwrite the SG again, im not sure if there is a difference between deleting in the UI and cdk destroy?

Otherwise it appears it won't work in production without creating downtime, so i will need to search for another solution to this.

fcoelho commented 1 year ago

We're running into a similar situation although our problem is with the security group attached to the load balancer itself.

When an ALB gets imported, it gets its security group loaded as mutable: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts#L997-L1001

We're currently looking at removing any permissions related to modifying security groups from the exec role, and the way the security group gets loaded currently prevents that.

My particular issues would be solved with a mutableSecurityGroups prop somewhere or being able to pass actual security group instances instead of just an ID to it

asifm-sportsbet commented 1 year ago

Any update on this? I ran into a similar issue where the SG that I imported to attach to a ALB caused CDK to overwrite the entire egress rule for the imported SG 🤯

JoshMcCullough commented 11 months ago

I ran into this today when deploying a stack and then tearing it down, it removed the egress rule (which was allow all egress) from an SG which caused one of our deployments to be unreachable until I fixed it.

Looking at deploying the stack again, I've created a new security group and attached it to each of my FargateService instances, and connection 8 ports to an existing ALB for funneling to the services. This adds the 8 ports to the newly created SG with peers set as two existing SGs related to the VPC that the stack / cluster / target groups "within" -- so 16 ports are added to the new SG. It also adds those 8 ports to each of the existing VPC-related SGs as well as 8 reciporical rules back to the new SG. So I end up with something I'm not comfortable deploying:

Security Group Changes
┌───┬──────────────────────────┬─────┬─────────────┬──────────────────────────┐
│   │ Group                    │ Dir │ Protocol    │ Peer                     │
├───┼──────────────────────────┼─────┼─────────────┼──────────────────────────┤
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 80      │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 80      │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 0-65535 │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 0-65535 │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8797    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8797    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 7773    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 7773    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8798    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8798    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 7772    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 7772    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8799    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8799    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8803    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8803    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8802    │ sg-0a944583aa51a5f9a     │
│ + │ ${SecurityGroup.GroupId} │ In  │ TCP 8802    │ sg-0f4724293d168cfaa     │
│ + │ ${SecurityGroup.GroupId} │ Out │ Everything  │ Everyone (IPv4)          │
├───┼──────────────────────────┼─────┼─────────────┼──────────────────────────┤
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 80      │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 8797    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 7773    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 8798    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 7772    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 8799    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 8803    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ In  │ TCP 8802    │ Everyone (IPv4)          │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 80      │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 0-65535 │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 8797    │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 7773    │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 8798    │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 7772    │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 8799    │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 8803    │ ${SecurityGroup.GroupId} │
│ + │ sg-0a944583aa51a5f9a     │ Out │ TCP 8802    │ ${SecurityGroup.GroupId} │
├───┼──────────────────────────┼─────┼─────────────┼──────────────────────────┤
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 80      │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 8797    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 7773    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 8798    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 7772    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 8799    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 8803    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ In  │ TCP 8802    │ Everyone (IPv4)          │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 80      │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 0-65535 │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 8797    │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 7773    │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 8798    │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 7772    │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 8799    │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 8803    │ ${SecurityGroup.GroupId} │
│ + │ sg-0f4724293d168cfaa     │ Out │ TCP 8802    │ ${SecurityGroup.GroupId} │
└───┴──────────────────────────┴─────┴─────────────┴──────────────────────────┘
b-tin commented 7 months ago 
// "aws-cdk-lib": "2.125.0",
import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2';

this.existingHttpsListener = elbv2.ApplicationListener.fromApplicationListenerAttributes(this, 'Us2ExistingHttpsListener', {
  listenerArn: this.props.albExistingHttpsListenerArn,
  securityGroup: this.existingAlbSecurityGroup
});

...
private createElbListener() {
  this..existingHttpsListener.addAction('Us2WebServerServiceActionDefault', {
    action: elbv2.ListenerAction.forward([this.elbTargetGroup]),
    conditions: [
      elbv2.ListenerCondition.hostHeaders([this.props.ecsServiceWebServerPublicDomainName]),
    ],
    priority: this.props.albHttpsListenerWebServerPriority
  });
}

I've used this to avoid updating existing ALB security group when adding new target group and new listener rule