(EC2): document how to workaround Network ACL rule 100

[MHacker9404]

The default NACL created with a custom VPC has a default "100 - Allow All" ingress rule. This should not be since we can't delete rules after the fact thru the CDK.

Reproduction Steps

What did you expect to happen?

What actually happened?

Environment

• CDK CLI Version :
• Framework Version:
• Node.js Version:
• OS :
• Language (Version):

Other

---

This is 🐛 Bug Report

[rix0rrr]

Some example code would have been helpful.

Nevertheless, I'm fairly sure that it's not the CDK adding that rule, but EC2's (and our CloudFormation's) underlying implementation.

Your solutions seem to be:

• Use a custom resource to remove rule 100
• Insert rules with a smaller number than 100 (potentially a deny-all rule at 99)
• File a bug report with CloudFormation asking them to make it possible to remove/overwrite rule 100.

I will keep this ticket open as a reminder that this is something that could and should probably be called out in documentation.

[MHacker9404]

That’s why I added the images – I didn’t have any code whatsoever to create the NACL. This was part of the default from the vpc itself – which your point #2 shows why this is a bad default. ` // build the VPC const subnetConfiguration: SubnetConfiguration[] = [ { name: 'public', cidrMask: 24, subnetType: SubnetType.PUBLIC, }, ]; const vpcProps: VpcProps = { cidr: '10.0.0.0/16', subnetConfiguration, maxAzs: 99, }; const vpc = new Vpc(this, 'VPC', vpcProps); ` From: Rico Huijbers <[email protected]> Sent: 24 February, 2021 08:19 To: aws/aws-cdk <[email protected]> Cc: Phil Boyd <[email protected]>; Author <[email protected]> Subject: Re: [aws/aws-cdk] (EC2): default NACL in custom VPC has invalid ingress rule.... (#13220) Some example code would have been helpful. Nevertheless, I'm fairly sure that it's not the CDK adding that rule, but EC2's (and our CloudFormation's) underlying implementation. Your solutions seem to be: * Use a custom resource to remove rule 100 * Insert rules with a smaller number than 100 (potentially a deny-all rule at 99) * File a bug report with CloudFormation asking them to make it possible to remove/overwrite rule 100. I will keep this ticket open as a reminder that this is something that could and should probably be called out in documentation. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#13220 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA75SHDDLK4I3OYBG5SRUE3TAT4DNANCNFSM4YCVW5RA>.

[rix0rrr]

Well the default ACL just gets created with the default settings. Not exactly sure what the process is, but if you care about NACLs then wouldn't creating a NetworkACL object allow you to modify the rules?

Does a fresh NetworkACL object also come with default rule 100?

[MHacker9404]

Then I have to figure out how to swap it out for the default NACL…

[rix0rrr]

That is true. It might just be that the CloudFormation story for Network ACLs has been underdeveloped.

But if that's true, then there's not a lot that the CDK can do to make it better, I'm afraid

[MHacker9404]

That’s why I’m in the process of trying to learn how do contribute – do a little digging and see how the NACL is created….

[NukaCody]

Quick dump of example code for anyone else that comes across this.

    //  Identify the group of subnets you want to wrap the nacl around (in this case it's my database subnets)
    const dbnacl = new NetworkAcl(this, 'DBNACL', {
      vpc,
      subnetSelection: {
        subnetGroupName: 'Database',
      },
    });
    dbnacl.applyRemovalPolicy(RemovalPolicy.DESTROY);
    // Grab the application subnets
    const appSubnets = vpc.selectSubnets({ subnetGroupName: 'Application' }).subnets;
    let ruleNumber = 10;

    // Loop through the application subnets to get their CIDR
    for (let subnet of appSubnets) {
      const s = subnet as Subnet;
      const suffix = s.subnetId.substring(s.subnetId.length - 3, s.subnetId.length);

      // Explicitly allow DB traffic to/from the app subnet CIDR range
      dbnacl.addEntry(`BlockAllButDBIngress${suffix}`, {
        cidr: AclCidr.ipv4(s.ipv4CidrBlock),
        ruleNumber: ruleNumber,
        traffic: AclTraffic.tcpPort(3306),
        direction: TrafficDirection.INGRESS,
        ruleAction: Action.ALLOW,
      });

      dbnacl.addEntry(`BlockAllButDBEgress${suffix}`, {
        cidr: AclCidr.ipv4(s.ipv4CidrBlock),
        ruleNumber: ruleNumber,
        traffic: AclTraffic.tcpPort(3306),
        direction: TrafficDirection.EGRESS,
        ruleAction: Action.ALLOW,
      });

      ruleNumber++;
    }

[zachgoll]

For anyone stumbling on this in the future, there's a much simpler solution than expressed above (I'm guessing due to CDK updates). Leaving an example below for a basic VPC with a public subnet and private subnet with a NAT Gateway (CDK default).

This solution of course mirrors the "allow everything" model, but the skeleton is there to customize it however you want. This solution creates a separate network ACL for the public and private subnets (regardless of how many subnets you have).

The not-so-obvious part of this solution is the fact that subnetSelection: { subnetType: ... } is the line that does all the network acl to subnet associations for you. It will dissociate the default network ACL (CDK default) and re-associate these newly created ACLs.

const vpc = new Vpc(this, 'VPC', {
    maxAzs: 2,
})

const publicNetworkACL = new NetworkAcl(this, 'PublicNetworkACL', {
    vpc,
    subnetSelection: { subnetType: SubnetType.PUBLIC },
})

publicNetworkACL.addEntry('AllowAllIngress', {
    cidr: AclCidr.anyIpv4(),
    ruleNumber: 100,
    traffic: AclTraffic.allTraffic(),
    direction: TrafficDirection.INGRESS,
    ruleAction: Action.ALLOW,
})

publicNetworkACL.addEntry('AllowAllEgress', {
    cidr: AclCidr.anyIpv4(),
    ruleNumber: 100,
    traffic: AclTraffic.allTraffic(),
    direction: TrafficDirection.EGRESS,
    ruleAction: Action.ALLOW,
})

const privateNetworkACL = new NetworkAcl(this, 'PrivateNetworkACL', {
    vpc: this.vpc,
    subnetSelection: { subnetType: SubnetType.PRIVATE_WITH_NAT },
})

privateNetworkACL.addEntry('AllowAllIngress', {
    cidr: AclCidr.anyIpv4(),
    ruleNumber: 100,
    traffic: AclTraffic.allTraffic(),
    direction: TrafficDirection.INGRESS,
    ruleAction: Action.ALLOW,
})

privateNetworkACL.addEntry('AllowAllEgress', {
    cidr: AclCidr.anyIpv4(),
    ruleNumber: 100,
    traffic: AclTraffic.allTraffic(),
    direction: TrafficDirection.EGRESS,
    ruleAction: Action.ALLOW,
})

Hope this helps!

[jpSimkins]

I already create my own NACLs for the VPC but the fact it exists has been a problem for compliance.

Having to go into every account to remove these rules is tedious and not really realistic when deploying a large system that spans multiple AWS accounts.

I have tried to do something like:

  ###########################################################################
  # Default Network ACLs (NACLs)
  # - AWS creates this default NACL and you cannot delete it
  # - We don't use it but need to have it not be open by default for compliance
  ###########################################################################

  # Deny all inbound traffic
  DefaultNaclInboundDenyAll:
    Type: AWS::EC2::NetworkAclEntry
    Properties:
      NetworkAclId: !GetAtt VPC.DefaultNetworkAcl
      RuleNumber: 100
      Protocol: -1 # -1 represents all protocols
      RuleAction: deny
      Egress: false
      CidrBlock: 0.0.0.0/0

  # Deny all outbound traffic
  DefaultNaclOutboundDenyAll:
    Type: AWS::EC2::NetworkAclEntry
    Properties:
      NetworkAclId: !GetAtt VPC.DefaultNetworkAcl
      RuleNumber: 100
      Protocol: -1 # -1 represents all protocols
      RuleAction: deny
      Egress: true
      CidrBlock: 0.0.0.0/0

But this ends up with an error when deploying:

The network acl entry identified by 100 already exists. (Service: AmazonEC2; Status Code: 400; Error Code: NetworkAclEntryAlreadyExists; Request ID: xxx...; Proxy: null)

To have it actually block, you will need to do this:

  ###########################################################################
  # Default Network ACLs (NACLs)
  # - AWS creates this default NACL and you cannot delete it
  # - We don't use it but need to have it not be open by default for compliance
  ###########################################################################

  # Deny all inbound traffic
  DefaultNaclInboundDenyAll:
    Type: AWS::EC2::NetworkAclEntry
    Properties:
      NetworkAclId: !GetAtt VPC.DefaultNetworkAcl
      RuleNumber: 99
      Protocol: -1 # -1 represents all protocols
      RuleAction: deny
      Egress: false
      CidrBlock: 0.0.0.0/0

  # Deny all outbound traffic
  DefaultNaclOutboundDenyAll:
    Type: AWS::EC2::NetworkAclEntry
    Properties:
      NetworkAclId: !GetAtt VPC.DefaultNetworkAcl
      RuleNumber: 99
      Protocol: -1 # -1 represents all protocols
      RuleAction: deny
      Egress: true
      CidrBlock: 0.0.0.0/0

But the allow rule still persists and will fail a compliance check.

Seems the only option is to create a custom resource/lambda to fix this. A hack at best for something that we should be able to control.

Would really like to see a fix for this.

[ALFmachine]

i am sure their might be a more elegant approach but this has been solving this issue along with a couple others for me for a while now. sharing in case its helpful to anyone

/**
 * use cases:
 * - configures default security group according to "CIS AWS Foundations Benchmark controls"
 *   section "4.3 – Ensure the default security group of every VPC restricts all traffic".
 *   See https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-cis-controls.html#securityhub-cis-controls-4.3
 *   See https://github.com/aws/aws-cdk/issues/13220#issuecomment-2274317510
 * - restricts all traffic to default nacl and configures per subnet group nacls
 * - tags subnets with proper naming tag and exposes custom subnet tagging
 */

import * as cdk from 'aws-cdk-lib'
import * as ec2 from 'aws-cdk-lib/aws-ec2'
import { AwsCustomResource, AwsCustomResourcePolicy, PhysicalResourceId } from 'aws-cdk-lib/custom-resources'
import { Construct } from 'constructs'

export class BaseVpc extends ec2.Vpc {
  constructor(scope: Construct, id: string, props: BaseVpcProps) {
    super(scope, id, props)

    this.tagSubnets({ name: props.vpcName, tags: props.tags })

    this.setSubnetNacls()

    this.restrictDefaultNacl()
  }

  private getStack() {
    return cdk.Stack.of(this)
  }

  private restrictDefaultNacl() {
    const defaultNacl = this.vpcDefaultNetworkAcl
    const options = [true, false]
    const stack = this.getStack()

    options.forEach((option) => {
      const type = option ? 'egress' : 'ingress'

      new AwsCustomResource(this, `restrict-default-nacl-${type}`, {
        onCreate: {
          action: 'deleteNetworkAclEntry',
          parameters: {
            Egress: option,
            NetworkAclId: defaultNacl,
            RuleNumber: 100,
          },
          physicalResourceId: PhysicalResourceId.of(`restrict-nacl-${this.vpcId}-${defaultNacl}-${type}`),
          service: 'EC2',
        },
        policy: AwsCustomResourcePolicy.fromSdkCalls({
          resources: [`arn:aws:ec2:${stack.region}:${stack.account}:network-acl/${defaultNacl}`],
        }),
      })
    })
  }

  private setSubnetNacls() {
    const stack = this.getStack()
    const subnetTypes: {
      rule?: Omit<ec2.NetworkAclEntryProps, 'networkAcl'> & {
        entryName: string
      }
      subnetType: ec2.SubnetType
    }[] = [
      {
        rule: {
          cidr: ec2.AclCidr.ipv4(this.vpcCidrBlock),
          entryName: 'allow all vpc',
          ruleNumber: 100,
          traffic: ec2.AclTraffic.allTraffic(),
        },
        subnetType: ec2.SubnetType.PRIVATE_ISOLATED,
      },
      { subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS },
      { subnetType: ec2.SubnetType.PUBLIC },
    ]

    subnetTypes.forEach(({ rule: ruling = {}, subnetType }) => {
      const {
        cidr = ec2.AclCidr.anyIpv4(),
        entryName,
        ruleAction = ec2.Action.ALLOW,
        ruleNumber = 100,
        traffic = ec2.AclTraffic.allTraffic(),
        ...rule
      } = ruling
      const directions = [ec2.TrafficDirection.INGRESS, ec2.TrafficDirection.EGRESS]
      const naming = `nacl-${subnetType.toLocaleLowerCase()}`

      const nacl = new ec2.NetworkAcl(this, naming, {
        networkAclName: `${stack.stackName}-${subnetType.toLocaleLowerCase()}`,
        subnetSelection: {
          subnetType,
        },
        vpc: this,
      })

      directions.forEach((direction) => {
        const dir = direction === 0 ? 'ingress' : 'egress'

        new ec2.NetworkAclEntry(this, `${naming}-${dir}-100`, {
          cidr,
          direction,
          networkAcl: nacl,
          networkAclEntryName: `${entryName} ${dir}`,
          ruleAction,
          ruleNumber,
          traffic,
          ...rule,
        })
      })
    })
  }

  private tagSubnets: (args: {
    name?: string
    tags?: {
      [name: string]: string
    }
  }) => void = ({ name, tags = {} }) => {
    const subnets = [...this.isolatedSubnets, ...this.privateSubnets, ...this.publicSubnets]

    for (const subnet of subnets) {
      const tagList = {
        ...tags,
        Name: [name || this.node.id, subnet.node.id.replace(/Subnet[0-9]$/, ''), subnet.availabilityZone].join('-'),
      }

      Object.entries(tagList).forEach(([name, value]) => {
        cdk.Aspects.of(subnet).add(new cdk.Tag(name, value))
      })
    }
  }
}

export interface BaseVpcProps extends ec2.VpcProps {
  tags?: {
    [key: string]: string
  }
}