fix(ec2): NAT instances don't route ICMP or UDP

Route all traffic by default (not just TPC), and
give the user an opportunity to customize.

Fixes #7459.

packages/@aws-cdk/aws-ec2/README.md

@@ -159,6 +159,21 @@ If you prefer to use a custom AMI, use `machineImage:
 MachineImage.genericLinux({ ... })` and configure the right AMI ID for the
 regions you want to deploy to.
 
+By default, the NAT instances will route all traffic. To control what traffic
+gets routed, pass `allowAllTraffic: false` and access the
+`NatInstanceProvider.connections` member after having passed it to the VPC:
+
+```ts
+const provider = NatProvider.instance({
+  instanceType: /* ... */,
+  allowAllTraffic: false,
+});
+new Vpc(stack, 'TheVPC', {
+  natGatewayProvider: provider,
+});
+provider.connections.allowFrom(Peer.ipv4('1.2.3.4/8'), Port.tcp(80));
+```
+
 ### Advanced Subnet Configuration
 
 If the default VPC configuration (public and private subnets spanning the

packages/@aws-cdk/aws-ec2/lib/nat.ts

@@ -1,9 +1,10 @@
 import * as iam from '@aws-cdk/aws-iam';
+import { Connections, IConnectable } from './connections';
 import { Instance } from './instance';
 import { InstanceType } from './instance-types';
 import { IMachineImage, LookupMachineImage } from './machine-image';
 import { Port } from './port';
-import { SecurityGroup } from './security-group';
+import { ISecurityGroup, SecurityGroup } from './security-group';
 import { PrivateSubnet, PublicSubnet, RouterType, Vpc } from './vpc';
 
 /**
@@ -39,7 +40,7 @@ export abstract class NatProvider {
    * @see https://docs.aws.amazon.com/vpc/latest/userguide/vpc-nat-gateway.html
    */
   public static gateway(): NatProvider {
-    return new NatGateway();
+    return new NatGatewayProvider();
   }
 
   /**
@@ -53,8 +54,8 @@ export abstract class NatProvider {
    *
    * @see https://docs.aws.amazon.com/vpc/latest/userguide/VPC_NAT_Instance.html
    */
-  public static instance(props: NatInstanceProps): NatProvider {
-    return new NatInstance(props);
+  public static instance(props: NatInstanceProps): NatInstanceProvider {
+    return new NatInstanceProvider(props);
   }
 
   /**
@@ -64,11 +65,15 @@ export abstract class NatProvider {
 
   /**
    * Called by the VPC to configure NAT
+   *
+   * Don't call this directly, the VPC will call it automatically.
    */
   public abstract configureNat(options: ConfigureNatOptions): void;
 
   /**
    * Configures subnet with the gateway
+   *
+   * Don't call this directly, the VPC will call it automatically.
    */
   public abstract configureSubnet(subnet: PrivateSubnet): void;
 }
@@ -134,9 +139,32 @@ export interface NatInstanceProps {
    * @default - No SSH access will be possible.
    */
   readonly keyName?: string;
+
+  /**
+   * Security Group for NAT instances
+   *
+   * @default - A new security group will be created
+   */
+  readonly securityGroup?: ISecurityGroup;
+
+  /**
+   * Allow all traffic through the NAT instance
+   *
+   * If you set this to false, you must configure the NAT instance's security
+   * groups in another way, either by passing in a fully configured Security
+   * Group using the `securityGroup` property, or by configuring it using the
+   * `.securityGroup` or `.connections` members after passing the NAT Instance
+   * Provider to a Vpc.
+   *
+   * @default true
+   */
+  readonly allowAllTraffic?: boolean;
 }
 
-class NatGateway extends NatProvider {
+/**
+ * Provider for NAT Gateways
+ */
+class NatGatewayProvider extends NatProvider {
   private gateways: PrefSet<string> = new PrefSet<string>();
 
   public configureNat(options: ConfigureNatOptions) {
@@ -167,8 +195,13 @@ class NatGateway extends NatProvider {
   }
 }
 
-class NatInstance extends NatProvider {
+/**
+ * NAT provider which uses NAT Instances
+ */
+export class NatInstanceProvider extends NatProvider implements IConnectable {
   private gateways: PrefSet<Instance> = new PrefSet<Instance>();
+  private _securityGroup?: ISecurityGroup;
+  private _connections?: Connections;
 
   constructor(private readonly props: NatInstanceProps) {
     super();
@@ -177,11 +210,15 @@ class NatInstance extends NatProvider {
   public configureNat(options: ConfigureNatOptions) {
     // Create the NAT instances. They can share a security group and a Role.
     const machineImage = this.props.machineImage || new NatInstanceImage();
-    const sg = new SecurityGroup(options.vpc, 'NatSecurityGroup', {
+    this._securityGroup = this.props.securityGroup ?? new SecurityGroup(options.vpc, 'NatSecurityGroup', {
       vpc: options.vpc,
       description: 'Security Group for NAT instances',
     });
-    sg.connections.allowFromAnyIpv4(Port.allTcp());
+    this._connections = new Connections({ securityGroups: [this._securityGroup] });
+
+    if (this.props.allowAllTraffic ?? true) {
+      this.connections.allowFromAnyIpv4(Port.allTraffic());
+    }
 
     // FIXME: Ideally, NAT instances don't have a role at all, but
     // 'Instance' does not allow that right now.
@@ -196,7 +233,7 @@ class NatInstance extends NatProvider {
         sourceDestCheck: false,  // Required for NAT
         vpc: options.vpc,
         vpcSubnets: { subnets: [sub] },
-        securityGroup: sg,
+        securityGroup: this._securityGroup,
         role,
         keyName: this.props.keyName,
       });
@@ -210,6 +247,30 @@ class NatInstance extends NatProvider {
     }
   }
 
+  /**
+   * The Security Group associated with the NAT instances
+   */
+  public get securityGroup(): ISecurityGroup {
+    if (!this._securityGroup) {
+      throw new Error('Pass the NatInstanceProvider to a Vpc before accessing \'securityGroup\'');
+    }
+    return this._securityGroup;
+  }
+
+  /**
+   * Manage the Security Groups associated with the NAT instances
+   */
+  public get connections(): Connections {
+    if (!this._connections) {
+      throw new Error('Pass the NatInstanceProvider to a Vpc before accessing \'connections\'');
+    }
+    return this._connections;
+  }
+
+  public get configuredGateways(): GatewayConfig[] {
+    return this.gateways.values().map(x => ({az: x[0], gatewayId: x[1].instanceId}));
+  }
+
   public configureSubnet(subnet: PrivateSubnet) {
     const az = subnet.availabilityZone;
     const gatewayId = this.gateways.pick(az).instanceId;
@@ -219,10 +280,6 @@ class NatInstance extends NatProvider {
       enablesInternetConnectivity: true,
     });
   }
-
-  public get configuredGateways(): GatewayConfig[] {
-    return this.gateways.values().map(x => ({az: x[0], gatewayId: x[1].instanceId}));
-  }
 }
 
 /**

packages/@aws-cdk/aws-ec2/test/integ.nat-instances.lit.expected.json

@@ -522,10 +522,8 @@
         "SecurityGroupIngress": [
           {
             "CidrIp": "0.0.0.0/0",
-            "Description": "from 0.0.0.0/0:ALL PORTS",
-            "FromPort": 0,
-            "IpProtocol": "tcp",
-            "ToPort": 65535
+            "Description": "from 0.0.0.0/0:ALL TRAFFIC",
+            "IpProtocol": "-1"
           }
         ],
         "Tags": [

packages/@aws-cdk/aws-ec2/test/test.vpc.ts

@@ -2,8 +2,8 @@ import { countResources, expect, haveResource, haveResourceLike, isSuperObject,
 import { CfnOutput, Lazy, Stack, Tag } from '@aws-cdk/core';
 import { Test } from 'nodeunit';
 import { AclCidr, AclTraffic, CfnSubnet, CfnVPC, DefaultInstanceTenancy, GenericLinuxImage, InstanceType, InterfaceVpcEndpoint,
-  InterfaceVpcEndpointService, NatProvider, NetworkAcl, NetworkAclEntry, PrivateSubnet, PublicSubnet, RouterType, Subnet,
-  SubnetType, TrafficDirection, Vpc } from '../lib';
+  InterfaceVpcEndpointService, NatProvider, NetworkAcl, NetworkAclEntry, Peer, Port, PrivateSubnet, PublicSubnet,
+  RouterType, Subnet, SubnetType, TrafficDirection, Vpc } from '../lib';
 
 export = {
   'When creating a VPC': {
@@ -781,6 +781,39 @@ export = {
       test.done();
     },
 
+    'can configure Security Groups of NAT instances'(test: Test) {
+      // GIVEN
+      const stack = getTestStack();
+
+      // WHEN
+      const provider = NatProvider.instance({
+        instanceType: new InstanceType('q86.mega'),
+        machineImage: new GenericLinuxImage({
+          'us-east-1': 'ami-1',
+        }),
+        allowAllTraffic: false,
+      });
+      new Vpc(stack, 'TheVPC', {
+        natGatewayProvider: provider,
+      });
+      provider.connections.allowFrom(Peer.ipv4('1.2.3.4/32'), Port.tcp(86));
+
+      // THEN
+      expect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
+        SecurityGroupIngress: [
+          {
+            CidrIp: '1.2.3.4/32',
+            Description: 'from 1.2.3.4/32:86',
+            FromPort: 86,
+            IpProtocol: 'tcp',
+            ToPort: 86,
+          },
+        ],
+      }));
+
+      test.done();
+    },
+
   },
 
   'Network ACL association': {