(aws-ecs): FargateService adds In- and EgressRules for all SecurityGroups

[bedaka]

What is the problem?

When configuring a FargateService with multiple SecurityGroups additional Egress Rule changes (e.g. for Load Balancer to Target) are created for all of them. This applies even for external SGs that are imported by fromSecurityGroupId() and have set {mutable: false}.

Reproduction Steps

I created a minimal example in this github repository

      const externalDbSg = SecurityGroup.fromSecurityGroupId(
      this,
      "ExternalDbSg",
      Fn.importValue("external-database-sg"),
      { mutable: false, allowAllOutbound: true }
    );

    const fargateSG = new SecurityGroup(this, "FargateSg", {
      vpc,
    });

    const targetGroup = new ApplicationTargetGroup(this, "TargetGroup", {
      vpc,
      port: 8080,
    });

    new ApplicationLoadBalancer(this, "Alb", {
      vpc,
      internetFacing: true,
    }).addListener("Listener", {
      port: 443,
      certificates: [ListenerCertificate.fromArn("arn")],
      defaultAction: ListenerAction.forward([targetGroup]),
    });

    const task = new TaskDefinition(this, "Task", {
      compatibility: Compatibility.FARGATE,
      cpu: "512",
      memoryMiB: "1024",
    });

    task.addContainer("Image", {
      image: ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
      portMappings: [{ containerPort: 8080 }],
    });

    const service = new FargateService(this, "FargateService", {
      cluster: cluster,
      taskDefinition: task,
      securityGroups: [fargateSG, externalDbSg],
    });

    service.attachToApplicationTargetGroup(targetGroup);

What did you expect to happen?

The Rules are only created for the SG I created for the FargateService (and is mutable).

I'm migrating the service from Cloudformation to CDK and before there was no issue attaching multiple SGs a FargateService. I understand that now where the L2 Construct is creating the Rule for Traffic from the LoadBalancer itself and has no real way of "knowing which SG belongs to the service". But there should be a way (that I might be missing) of implementing this so that not every Egress rule is created multiple times.

What actually happened?

For every SG assigned to the FargateService Egress rules are created for the Resources that have a connection to the service. In my provided example this is only the Load Balancer but this applies for all connections added to the service. While a single additional Egress rule is not that much of an issue, this can become quite irritating when working with multiple services that have access to multiple Resources.

-- EDIT --

I made some pictures to make the situation clear

• ExternalSg imported with mutable: true
• ExternalSg imported with mutable:false
• Desired Outcome

CDK CLI Version

2.1.0

Framework Version

2.1.0

Node.js Version

14.17.3

OS

Ubuntu 20.04

Language

Typescript

Language Version

3.9.7

Other information

No response

[stoicad]

We are also trying to attach an existing security group to an ELB without modifying the egress/ingress rules.

I am doing it like this: const securityGroup = SecurityGroup.fromSecurityGroupId(this, "ELB Security Group", securityGroupId, { mutable: false, allowAllOutbound: true });

however, when looking at the cdk.context.json the generate JSON looks like this: "security-group:account=XXXXXXXXX:region=us-east-1:securityGroupId=sg-id": { "securityGroupId": "sg-id", "allowAllOutbound": false }

which is totally wrong and it's causing a wrong generation of Cloud Formation (we are not allowed to touch security groups rules).

2 possible workarounds:

1. go to cdk.context.json and set allowAllOutbound to true
2. go to cdk.json and add under context node the following JSON section: "security-group:account=XXXXXXXXX:region=us-east-1:securityGroupId=sg-id": { "securityGroupId": "sg-id", "allowAllOutbound": true }

This will fix the outbound rules.

For inbound rules, at the listener level, you need to set the open property to false e.g.

const httpsListener = this.elb.addListener("HTTPS", { protocol: ApplicationProtocol.HTTPS, port: 443, sslPolicy: SslPolicy.FORWARD_SECRECY_TLS12_RES_GCM, certificates: [{ certificateArn: this.getCertificateArn() }], open: false, });

Hope it helps!

[bedaka]

While the issue pointed out by @stoicad might have the same / similar route cause it's not quite the same and I don't think this workaround would work for my scenario.

In case my description was not clear enough, this is the SecurityErgressRule that should NOT be created.

"AlbSecurityGrouptoEcsFargateStackExternalDbSgXXXXXXXXXXXX": {
      "Type": "AWS::EC2::SecurityGroupEgress",
      "Properties": {
        "GroupId": {
          "Fn::GetAtt": [
            "AlbSecurityGroupXXXXXX",
            "GroupId"
          ]
        },
        "IpProtocol": "tcp",
        "Description": "Load balancer to target",
        "DestinationSecurityGroupId": {
          "Fn::ImportValue": "external-database-sg"
        },
        "FromPort": 8080,
        "ToPort": 8080
      },
      "Metadata": {
        "aws:cdk:path": "EcsFargateStack/Alb/SecurityGroup/to EcsFargateStackExternalDbSgXXXXXXX:8080"
      }
    },

as it makes no sense to allow the LB to talk to the SecurityGroup that manages access to a database.

[stoicad]

@bedaka, could you check the cdk.context.json file, in particular the value of the allowAllOutbound under the security group attached to ALB? Is it true or false? or it's not there at all?

[bedaka]

@stoicad We do not use a cdk.context.json file in our project and there is also no such setting in the cdk.json context node. However I tested setting the context in my example like this: this.node.setContext("allowAllOutbound", true); and run it both with false and true but it did not change the resulting Cloudformation.

[stoicad]

@bedaka, yes I think the issue is basically related to the allowAllOutbound property which is ignored when it's set programmatically. However, when this property it's set in the configuration, on my side it does work. Have you tried to set the allowAllOutbound in the cdk.json? (this should be a new entry inside the context node, it doesn't have to be already there, but you should use your account id and the SG id)

"security-group:account=account-id:region=us-east-1:securityGroupId=sg-id": { "securityGroupId": "sg-id", "allowAllOutbound": true }

let me know how it goes.

[bedaka]

@stoicad thank you for your help so far. I was able to follow your suggestion and in fact creating a loadBalancer SG explicitly and adding allowAllOutbound: true solved the creation of the Egress Groups in question to the LB.

@madeline-k (sorry for pinging directly) HOWEVER this workaround does not solve the underlying issue, that I tried to point at. I updated my minimal example to show the issue with a little bit more complex setup. Please imagine the following:

• There are two SGs attached to the Service (FargateSg, ExternalSg (for example to grant access to a DB))
• The ExternalSg is imported with { mutable: false, allowAllOutbound: true }
• You grant the fargate service access to a third SG via service.connection.allowTo(testSG, Port.tcp(1234), "Fargate to Test)

This will results in the following rule being created which is not expected/wanted:

    "TestSgfromEcsFargateStackExternalSgXXXX": {
      "Type": "AWS::EC2::SecurityGroupIngress",
      "Properties": {
        "IpProtocol": "tcp",
        "Description": "Fargate to Test",
        "FromPort": 1234,
        "GroupId": {
          "Fn::GetAtt": [ "TestSgXXXX", "GroupId"]
        },
        "SourceSecurityGroupId": {
          "Fn::ImportValue": "external-sg"    // THIS IS WRONG
        },
        "ToPort": 1234
      },
      "Metadata": {
        "aws:cdk:path": "EcsFargateStack/TestSg/from EcsFargateStackExternalSgXXXX:1234"
      }
    },

It seems like this is possible because the IngressRule is not attached to the externalSg and it is merely used as the SourceSecurityGroup. It seems like there is an attribute next to mutable missing (for example external: false) which allows to disable the usage of an imported group as a SourceSecurityGroup.

I updated my example accordingly.

[madeline-k]

Thanks for opening this issue and your detailed descriptions, @bedaka. It looks like there is a bug here, and also an opportunity to make security group management better for ecs services. I am not sure right away what is the right direction here. I will need to dive deeper and come back to it.

[PauSabatesC]

Any updates on this?

[samuelthan]

Any updates on this pls ?
manually manipulating the cdk.json as a workaround was a pain to manage especially if we have multiple LBs and security groups and multiple region to support ...

[ykachube]

Same issue, any update will be appreciated

[n-miles]

This is a really wild bug and I'm pretty surprised that this hasn't been fixed. This is very disruptive, and seems like a huge security issue. Is there any ETA on this getting addressed?

[djj106]

Same issue here. Any ETA on a fix?

[evgenyka]

Raising the priority because of the customer impact.

[pahud]

I think you should not assign the externalDbSg to the fargate service. If your intention is allow the ingress from fargate to your DB, you should add an ingress of your DB Sg that allows fargateSG.

const service = new FargateService(this, "FargateService", {
      cluster: cluster,
      taskDefinition: task,
      securityGroups: [fargateSG, externalDbSg],
    });

When

service.attachToApplicationTargetGroup(targetGroup);

or

targetGroup.addTarget(service)

It essentially adds an ingress that allow the security group(s) on ALB to the service, which makes sense when you only have one SG on the fargate service but would be a problem when you have multiple SGs on the service.

I would suggest this way:

    const service = new ecs.FargateService(this, "FargateService", {
      cluster,
      taskDefinition: task,
      securityGroups: [fargateSG],
    });

    service.attachToApplicationTargetGroup(targetGroup);

Can you explain why you have to attach externalDbSg to the fargate service?

[github-actions[bot]]

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.

[SC-CTS]

Please don't close this, still relevant.