AndrewGuenther / cdk-fck-nat

CDK constructs for the fck-nat service
MIT License
63 stars 9 forks source link

Default security group not allowing ingress from NAT'd subnet(s) #118

Closed willfrew closed 1 year ago

willfrew commented 1 year ago

I was just implementing fck-nat in my stack and noticed that I needed to explicity add an ingress rule to the created security group in order for instances in a private subnet to have internet access.

I can definitely understand not adding open ingress rules to the security group by default but given that this wasn't in the docs, I thought I'd flag it in case it's a bug / regression.

Without the workaround below, instances in the 'private' subnet do not have internet connectivity. Said instances are in SGs of their own, but with all egress allowed.

I narrowed the issue down to the fck-net generated SG using AWS's Reachability Analyser, where the source was an instance in my private subnet and the destination was the ENI of the fck-nat gateway in the same AZ. After implementing the below workaround, it was now reachable.

    const fckNatInstanceProvider = new FckNatInstanceProvider({
      instanceType: ec2.InstanceType.of(ec2.InstanceClass.T4G, ec2.InstanceSize.MICRO),
    });

    const vpc = new ec2.Vpc(this, 'PrimaryVpc', {
      natGatewayProvider: fckNatInstanceProvider,
      subnetConfiguration: [{
        name: 'public',
        subnetType: ec2.SubnetType.PUBLIC,
      }, {
        name: 'private',
        subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS,
      },
      /* ... */
      }],
      // Including my other config in case it's relevant
      enableDnsSupport: true,
      enableDnsHostnames: true,
      availabilityZones: [ /* I'm using a subset of the available AZs */ ],
    });

    const natSubnets = vpc.selectSubnets({ subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }).subnets;
    natSubnets.forEach((subnet) => {
      fckNatInstanceProvider.securityGroup.addIngressRule(
        ec2.Peer.ipv4(subnet.ipv4CidrBlock),
        ec2.Port.allTraffic(),
        `Allow traffic into the NAT Gateway from ${subnet.subnetId}`,
      );
    });

I noticed that the cdk subnet type recently switched from PRIVATE_WITH_NAT to PRIVATE_WITH_EGRESS - may/may not be related(?)

AndrewGuenther commented 1 year ago

This was a conscious choice made to align with how the NatInstanceProvider base class we inherit from works. The problem with this interface is that the NatInstanceProvider must be created before your VPC is created, so much of the security group information/subnet information is not available at the time it is constructed. I'm not a huge fan of this interface, but it is the simplest way to provide a NAT construct in CDK.

I'm definitely open to suggestions on this, but it is important to me the fck-nat-cdk continues to use NatInstanceProvider as a base and that our default functionality doesn't significantly deviate from it.

AndrewGuenther commented 1 year ago

Resolving as stale.

neoakris commented 3 weeks ago

@AndrewGuenther I too just ran into this, where I thought fck-NAT was working because the cdk stack deployed. But when I went to test it I found things failed, because private IP instances didn't have internet connectivity.

Q1: Can this be reopened?

I discovered an easy way to manually test is to:

  1. add an ec2 instance connect endpoint to the vpc
  2. add an ec2 with a private ip to the vpc
  3. from the AWS Web GUI Console --> EC2 --> test vm with private ip --> there's a connect button --> then connect through vpc endpoint
  4. That allows a brower shell directly to a private ip ec2 instance.

ping 1.1.1.1 failed

until I added edited fck-NAT's SG, with the following inbound security group rules which I'd like to propose as reasonable default values. image

Q2: What do you think about a fix of this^ nature?

AndrewGuenther commented 3 weeks ago

I'm not inclined to re-open this. Making any assumptions about what security groups should be is not a safe default imo. Consumers have access to the security group to update it how they see fit and all instructions in https://fck-nat.dev/stable/deploying/ include a step for configuring the security group.