Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ec2): NAT provider's default outbound rules cannot be disabled #12674

Merged
merged 11 commits into from
Mar 4, 2021
57 changes: 55 additions & 2 deletions packages/@aws-cdk/aws-ec2/lib/nat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,26 @@ import { Port } from './port';
import { ISecurityGroup, SecurityGroup } from './security-group';
import { PrivateSubnet, PublicSubnet, RouterType, Vpc } from './vpc';

/**
* Direction of traffic to allow all by default.
*/
export enum NatTrafficDirection {
/**
* Allow all outbound traffic and disallow all inbound traffic.
*/
OUTBOUND_ONLY = 'OUTBOUND_ONLY',

/**
* Allow all outbound and inbound traffic.
*/
INBOUND_AND_OUTBOUND = 'INBOUND_AND_OUTBOUND',

/**
* Disallow all outbound and inbound traffic.
*/
NONE = 'NONE',
}

/**
* Pair represents a gateway created by NAT Provider
*/
Expand Down Expand Up @@ -148,7 +168,7 @@ export interface NatInstanceProps {
readonly securityGroup?: ISecurityGroup;

/**
* Allow all traffic through the NAT instance
* Allow all inbound 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
Expand All @@ -157,8 +177,24 @@ export interface NatInstanceProps {
* Provider to a Vpc.
*
* @default true
* @deprecated - Use `defaultAllowedTraffic`.
*/
readonly allowAllTraffic?: boolean;

/**
* Direction to allow all traffic through the NAT instance by default.
*
* By default, inbound and outbound traffic is allowed.
*
* If you set this to another value than INBOUND_AND_OUTBOUND, 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 NatTrafficDirection.INBOUND_AND_OUTBOUND
*/
readonly defaultAllowedTraffic?: NatTrafficDirection;
}

/**
Expand Down Expand Up @@ -205,18 +241,26 @@ export class NatInstanceProvider extends NatProvider implements IConnectable {

constructor(private readonly props: NatInstanceProps) {
super();

if (props.defaultAllowedTraffic !== undefined && props.allowAllTraffic !== undefined) {
throw new Error('Can not specify both of \'defaultAllowedTraffic\' and \'defaultAllowedTraffic\'; prefer \'defaultAllowedTraffic\'');
}
}

public configureNat(options: ConfigureNatOptions) {
const defaultDirection = this.props.defaultAllowedTraffic ??
(this.props.allowAllTraffic ?? true ? NatTrafficDirection.INBOUND_AND_OUTBOUND : NatTrafficDirection.OUTBOUND_ONLY);

// Create the NAT instances. They can share a security group and a Role.
const machineImage = this.props.machineImage || new NatInstanceImage();
this._securityGroup = this.props.securityGroup ?? new SecurityGroup(options.vpc, 'NatSecurityGroup', {
vpc: options.vpc,
description: 'Security Group for NAT instances',
allowAllOutbound: isOutboundAllowed(defaultDirection),
});
this._connections = new Connections({ securityGroups: [this._securityGroup] });

if (this.props.allowAllTraffic ?? true) {
if (isInboundAllowed(defaultDirection)) {
this.connections.allowFromAnyIpv4(Port.allTraffic());
}

Expand Down Expand Up @@ -325,3 +369,12 @@ export class NatInstanceImage extends LookupMachineImage {
});
}
}

function isOutboundAllowed(direction: NatTrafficDirection) {
return direction === NatTrafficDirection.INBOUND_AND_OUTBOUND ||
direction === NatTrafficDirection.OUTBOUND_ONLY;
}

function isInboundAllowed(direction: NatTrafficDirection) {
return direction === NatTrafficDirection.INBOUND_AND_OUTBOUND;
}
151 changes: 147 additions & 4 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,30 @@ import { countResources, expect as cdkExpect, haveResource, haveResourceLike, is
import { CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '@aws-cdk/core';
import { nodeunitShim, Test } from 'nodeunit-shim';
import {
AclCidr, AclTraffic, BastionHostLinux, CfnSubnet, CfnVPC, SubnetFilter, DefaultInstanceTenancy, GenericLinuxImage,
InstanceType, InterfaceVpcEndpoint, InterfaceVpcEndpointService, NatProvider, NetworkAcl, NetworkAclEntry, Peer, Port, PrivateSubnet,
PublicSubnet, RouterType, Subnet, SubnetType, TrafficDirection, Vpc,
AclCidr,
AclTraffic,
BastionHostLinux,
CfnSubnet,
CfnVPC,
SubnetFilter,
DefaultInstanceTenancy,
GenericLinuxImage,
InstanceType,
InterfaceVpcEndpoint,
InterfaceVpcEndpointService,
NatProvider,
NatTrafficDirection,
NetworkAcl,
NetworkAclEntry,
Peer,
Port,
PrivateSubnet,
PublicSubnet,
RouterType,
Subnet,
SubnetType,
TrafficDirection,
Vpc,
} from '../lib';

nodeunitShim({
Expand Down Expand Up @@ -904,6 +925,22 @@ nodeunitShim({
DestinationCidrBlock: '0.0.0.0/0',
InstanceId: { Ref: 'TheVPCPublicSubnet1NatInstanceCC514192' },
}));
cdkExpect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
SecurityGroupEgress: [
{
CidrIp: '0.0.0.0/0',
Description: 'Allow all outbound traffic by default',
IpProtocol: '-1',
},
],
SecurityGroupIngress: [
{
CidrIp: '0.0.0.0/0',
Description: 'from 0.0.0.0/0:ALL TRAFFIC',
IpProtocol: '-1',
},
],
}));

test.done();
},
Expand All @@ -929,7 +966,7 @@ nodeunitShim({
test.done();
},

'can configure Security Groups of NAT instances'(test: Test) {
'can configure Security Groups of NAT instances with allowAllTraffic false'(test: Test) {
// GIVEN
const stack = getTestStack();

Expand All @@ -948,6 +985,13 @@ nodeunitShim({

// THEN
cdkExpect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
SecurityGroupEgress: [
{
CidrIp: '0.0.0.0/0',
Description: 'Allow all outbound traffic by default',
IpProtocol: '-1',
},
],
SecurityGroupIngress: [
{
CidrIp: '1.2.3.4/32',
Expand All @@ -962,6 +1006,105 @@ nodeunitShim({
test.done();
},

'can configure Security Groups of NAT instances with defaultAllowAll INBOUND_AND_OUTBOUND'(test: Test) {
// GIVEN
const stack = getTestStack();

// WHEN
const provider = NatProvider.instance({
instanceType: new InstanceType('q86.mega'),
machineImage: new GenericLinuxImage({
'us-east-1': 'ami-1',
}),
defaultAllowedTraffic: NatTrafficDirection.INBOUND_AND_OUTBOUND,
});
new Vpc(stack, 'TheVPC', {
natGatewayProvider: provider,
});

// THEN
cdkExpect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
SecurityGroupEgress: [
{
CidrIp: '0.0.0.0/0',
Description: 'Allow all outbound traffic by default',
IpProtocol: '-1',
},
],
SecurityGroupIngress: [
{
CidrIp: '0.0.0.0/0',
Description: 'from 0.0.0.0/0:ALL TRAFFIC',
IpProtocol: '-1',
},
],
}));

test.done();
},

'can configure Security Groups of NAT instances with defaultAllowAll OUTBOUND_ONLY'(test: Test) {
// GIVEN
const stack = getTestStack();

// WHEN
const provider = NatProvider.instance({
instanceType: new InstanceType('q86.mega'),
machineImage: new GenericLinuxImage({
'us-east-1': 'ami-1',
}),
defaultAllowedTraffic: NatTrafficDirection.OUTBOUND_ONLY,
});
new Vpc(stack, 'TheVPC', {
natGatewayProvider: provider,
});

// THEN
cdkExpect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
SecurityGroupEgress: [
{
CidrIp: '0.0.0.0/0',
Description: 'Allow all outbound traffic by default',
IpProtocol: '-1',
},
],
}));

test.done();
},

'can configure Security Groups of NAT instances with defaultAllowAll NONE'(test: Test) {
// GIVEN
const stack = getTestStack();

// WHEN
const provider = NatProvider.instance({
instanceType: new InstanceType('q86.mega'),
machineImage: new GenericLinuxImage({
'us-east-1': 'ami-1',
}),
defaultAllowedTraffic: NatTrafficDirection.NONE,
});
new Vpc(stack, 'TheVPC', {
natGatewayProvider: provider,
});

// THEN
cdkExpect(stack).to(haveResource('AWS::EC2::SecurityGroup', {
SecurityGroupEgress: [
{
CidrIp: '255.255.255.255/32',
Description: 'Disallow all traffic',
FromPort: 252,
IpProtocol: 'icmp',
ToPort: 86,
},
],
}));

test.done();
},

},

'Network ACL association': {
Expand Down