diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts index 1509b353c7cec..9f82bae8f8098 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -179,16 +179,15 @@ export class AutoScalingGroup extends cdk.Construct implements cdk.ITaggable, el constructor(parent: cdk.Construct, name: string, props: AutoScalingGroupProps) { super(parent, name); - this.securityGroup = new ec2.SecurityGroup(this, 'InstanceSecurityGroup', { vpc: props.vpc }); + this.securityGroup = new ec2.SecurityGroup(this, 'InstanceSecurityGroup', { + vpc: props.vpc, + allowAllOutbound: props.allowAllOutbound !== false + }); this.connections = new ec2.Connections({ securityGroup: this.securityGroup }); this.securityGroups.push(this.securityGroup); this.tags = new TagManager(this, {initialTags: props.tags}); this.tags.setTag(NAME_TAG, this.path, { overwrite: false }); - if (props.allowAllOutbound !== false) { - this.connections.allowTo(new ec2.AnyIPv4(), new ec2.AllConnections(), 'Outbound traffic allowed by default'); - } - this.role = new iam.Role(this, 'InstanceRole', { assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com') }); diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json index 12f63b4c44098..0ca90cc9a329f 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-classic-loadbalancer.expected.json @@ -446,10 +446,8 @@ "SecurityGroupEgress": [ { "CidrIp": "0.0.0.0/0", - "Description": "Outbound traffic allowed by default", - "FromPort": -1, - "IpProtocol": "-1", - "ToPort": -1 + "Description": "Allow all outbound traffic by default", + "IpProtocol": "-1" } ], "SecurityGroupIngress": [], @@ -656,4 +654,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json index 38158f31daeb9..167f41eed8cac 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json +++ b/packages/@aws-cdk/aws-autoscaling/test/integ.asg-w-elbv2.expected.json @@ -312,10 +312,8 @@ "SecurityGroupEgress": [ { "CidrIp": "0.0.0.0/0", - "Description": "Outbound traffic allowed by default", - "FromPort": -1, - "IpProtocol": "-1", - "ToPort": -1 + "Description": "Allow all outbound traffic by default", + "IpProtocol": "-1" } ], "SecurityGroupIngress": [], diff --git a/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts index 453ded434a48e..a2a63050b9658 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts @@ -27,10 +27,8 @@ export = { "SecurityGroupEgress": [ { "CidrIp": "0.0.0.0/0", - "Description": "Outbound traffic allowed by default", - "FromPort": -1, + "Description": "Allow all outbound traffic by default", "IpProtocol": "-1", - "ToPort": -1 } ], "SecurityGroupIngress": [], diff --git a/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json b/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json index a64931c5829e0..3cb6c923102c3 100644 --- a/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json +++ b/packages/@aws-cdk/aws-codedeploy/test/integ.deployment-group.expected.json @@ -446,10 +446,8 @@ "SecurityGroupEgress": [ { "CidrIp": "0.0.0.0/0", - "Description": "Outbound traffic allowed by default", - "FromPort": -1, - "IpProtocol": "-1", - "ToPort": -1 + "Description": "Allow all outbound traffic by default", + "IpProtocol": "-1" } ], "SecurityGroupIngress": [], @@ -626,7 +624,15 @@ "Type": "AWS::EC2::SecurityGroup", "Properties": { "GroupDescription": "aws-cdk-codedeploy-server-dg/ELB/SecurityGroup", - "SecurityGroupEgress": [], + "SecurityGroupEgress": [ + { + "CidrIp":"255.255.255.255/32", + "Description":"Disallow all traffic", + "FromPort":252, + "IpProtocol":"icmp", + "ToPort":86 + } + ], "SecurityGroupIngress": [ { "CidrIp": "0.0.0.0/0", diff --git a/packages/@aws-cdk/aws-ec2/README.md b/packages/@aws-cdk/aws-ec2/README.md index 7940ddb4513bb..9249905c54bad 100644 --- a/packages/@aws-cdk/aws-ec2/README.md +++ b/packages/@aws-cdk/aws-ec2/README.md @@ -174,29 +174,43 @@ Application subnets will route to the NAT Gateway. ### Allowing Connections -In AWS, all connections to and from EC2 instances are governed by *Security -Groups*. You can think of these as a firewall with rules. All Constructs that -create instances on your behalf implicitly have such a security group. -Unless otherwise indicated using properites, the security groups start out -empty; that is, no connections are allowed by default. - -In general, whenever you link two Constructs together (such as the load balancer and the -fleet in the previous example), the security groups will be automatically updated to allow -network connections between the indicated instances. In other cases, you will need to -configure these allows connections yourself, for example if the connections you want to -allow do not originate from instances in a CDK construct, or if you want to allow -connections among instances inside a single security group. - -All Constructs with security groups have a member called `connections`, which -can be used to configure permissible connections. In the most general case, a -call to allow connections needs both a connection peer and the type of -connection to allow: +In AWS, all network traffic in and out of **Elastic Network Interfaces** (ENIs) +is controlled by **Security Groups**. You can think of Security Groups as a +firewall with a set of rules. By default, Security Groups allow no incoming +(ingress) traffic and all outgoing (egress) traffic. You can add ingress rules +to them to allow incoming traffic streams. To exert fine-grained control over +egress traffic, set `allowAllOutbound: false` on the `SecurityGroup`, after +which you can add egress traffic rules. + +You can manipulate Security Groups directly: ```ts -lb.connections.allowFrom(new ec2.AnyIPv4(), new ec2.TcpPort(443), 'Allow inbound'); +const mySecurityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', { + vpc, + description: 'Allow ssh access to ec2 instances', + allowAllOutbound: true // Can be set to false +}); +mySecurityGroup.addIngressRule(new ec2.AnyIPv4(), new ec2.TcpPort(22), 'allow ssh access from the world'); +``` + +All constructs that create ENIs on your behalf (typically constructs that create +EC2 instances or other VPC-connected resources) will all have security groups +automatically assigned. Those constructs have an attribute called +**connections**, which is an object that makes it convenient to update the +security groups. If you want to allow connections between two constructs that +have security groups, you have to add an **Egress* rule to one Security Group, +and an **Ingress** rule to the other. The connections object will automatically +take care of this for you: + +```ts +// Allow connections from anywhere +loadBalancer.connections.allowFromAnyIpv4(new ec2.TcpPort(443), 'Allow inbound HTTPS'); + +// The same, but an explicit IP address +loadBalancer.connections.allowFrom(new ec2.CidrIpv4('1.2.3.4/32'), new ec2.TcpPort(443), 'Allow inbound HTTPS'); -// Or using a convenience function -lb.connections.allowFromAnyIpv4(new ec2.TcpPort(443), 'Allow inbound'); +// Allow connection between AutoScalingGroups +appFleet.connections.allowTo(dbFleet, new ec2.TcpPort(443), 'App can call database'); ``` ### Connection Peers @@ -228,10 +242,10 @@ The connections that are allowed are specified by port ranges. A number of class the connection specifier: ```ts -new ec2.TcpPort(80); -new ec2.TcpPortRange(60000, 65535); -new ec2.TcpAllPorts(); -new ec2.AllConnections(); +new ec2.TcpPort(80) +new ec2.TcpPortRange(60000, 65535) +new ec2.TcpAllPorts() +new ec2.AllConnections() ``` > NOTE: This set is not complete yet; for example, there is no library support for ICMP at the moment. diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts b/packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts index b7ab7925d8dd5..1aae45389a5ee 100644 --- a/packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts @@ -340,6 +340,24 @@ export class IcmpTypeAndCode implements IPortRange { } } +/** + * ICMP Ping traffic + */ +export class IcmpPing implements IPortRange { + public readonly canInlineRule = true; + + public toRuleJSON(): any { + return { + ipProtocol: Protocol.Icmp, + fromPort: 8, + }; + } + + public toString() { + return `ICMP PING`; + } +} + /** * All ICMP Codes for a given ICMP Type */ @@ -384,18 +402,16 @@ export class IcmpAllTypesAndCodes implements IPortRange { /** * All Traffic */ -export class AllConnections implements IPortRange { +export class AllTraffic implements IPortRange { public readonly canInlineRule = true; public toRuleJSON(): any { return { ipProtocol: '-1', - fromPort: -1, - toPort: -1, }; } public toString() { return 'ALL TRAFFIC'; } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group.ts b/packages/@aws-cdk/aws-ec2/lib/security-group.ts index e124433616da1..49b187bc107bd 100644 --- a/packages/@aws-cdk/aws-ec2/lib/security-group.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group.ts @@ -115,6 +115,17 @@ export interface SecurityGroupProps { * The VPC in which to create the security group. */ vpc: VpcNetworkRef; + + /** + * Whether to allow all outbound traffic by default. + * + * If this is set to true, there will only be a single egress rule which allows all + * outbound traffic. If this is set to false, no outbound traffic will be allowed by + * default and all egress traffic must be explicitly authorized. + * + * @default true + */ + allowAllOutbound?: boolean; } /** @@ -149,11 +160,16 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable { private readonly directIngressRules: cloudformation.SecurityGroupResource.IngressProperty[] = []; private readonly directEgressRules: cloudformation.SecurityGroupResource.EgressProperty[] = []; + private readonly allowAllOutbound: boolean; + constructor(parent: Construct, name: string, props: SecurityGroupProps) { super(parent, name); this.tags = new TagManager(this, { initialTags: props.tags}); const groupDescription = props.description || this.path; + + this.allowAllOutbound = props.allowAllOutbound !== false; + this.securityGroup = new cloudformation.SecurityGroupResource(this, 'Resource', { groupName: props.groupName, groupDescription, @@ -166,6 +182,8 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable { this.securityGroupId = this.securityGroup.securityGroupId; this.groupName = this.securityGroup.securityGroupName; this.vpcId = this.securityGroup.securityGroupVpcId; + + this.addDefaultEgressRule(); } public addIngressRule(peer: ISecurityGroupRule, connection: IPortRange, description?: string) { @@ -186,6 +204,18 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable { } public addEgressRule(peer: ISecurityGroupRule, connection: IPortRange, description?: string) { + if (this.allowAllOutbound) { + // In the case of "allowAllOutbound", we don't add any more rules. There + // is only one rule which allows all traffic and that subsumes any other + // rule. + return; + } else { + // Otherwise, if the bogus rule exists we can now remove it because the + // presence of any other rule will get rid of EC2's implicit "all + // outbound" rule anyway. + this.removeNoTrafficRule(); + } + if (!peer.canInlineRule || !connection.canInlineRule) { super.addEgressRule(peer, connection, description); return; @@ -195,11 +225,22 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable { description = `from ${peer.uniqueId}:${connection}`; } - this.addDirectEgressRule({ + const rule = { ...peer.toEgressRuleJSON(), ...connection.toRuleJSON(), description - }); + }; + + if (isAllTrafficRule(rule)) { + // We cannot allow this; if someone adds the rule in this way, it will be + // removed again if they add other rules. We also can't automatically switch + // to "allOutbound=true" mode, because we might have already emitted + // EgressRule objects (which count as rules added later) and there's no way + // to recall those. Better to prevent this for now. + throw new Error('Cannot add an "all traffic" egress rule in this way; set allowAllOutbound=true on the SecurityGroup instead.'); + } + + this.addDirectEgressRule(rule); } /** @@ -233,8 +274,66 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable { private hasEgressRule(rule: cloudformation.SecurityGroupResource.EgressProperty): boolean { return this.directEgressRules.findIndex(r => egressRulesEqual(r, rule)) > -1; } + + /** + * Add the default egress rule to the securityGroup + * + * This depends on allowAllOutbound: + * + * - If allowAllOutbound is true, we *TECHNICALLY* don't need to do anything, because + * EC2 is going to create this default rule anyway. But, for maximum readability + * of the template, we will add one anyway. + * - If allowAllOutbound is false, we add a bogus rule that matches no traffic in + * order to get rid of the default "all outbound" rule that EC2 creates by default. + * If other rules happen to get added later, we remove the bogus rule again so + * that it doesn't clutter up the template too much (even though that's not + * strictly necessary). + */ + private addDefaultEgressRule() { + if (this.allowAllOutbound) { + this.directEgressRules.push(ALLOW_ALL_RULE); + } else { + this.directEgressRules.push(MATCH_NO_TRAFFIC); + } + } + + /** + * Remove the bogus rule if it exists + */ + private removeNoTrafficRule() { + const i = this.directEgressRules.findIndex(r => egressRulesEqual(r, MATCH_NO_TRAFFIC)); + if (i > -1) { + this.directEgressRules.splice(i, 1); + } + } } +/** + * Egress rule that purposely matches no traffic + * + * This is used in order to disable the "all traffic" default of Security Groups. + * + * No machine can ever actually have the 255.255.255.255 IP address, but + * in order to lock it down even more we'll restrict to a nonexistent + * ICMP traffic type. + */ +const MATCH_NO_TRAFFIC = { + cidrIp: '255.255.255.255/32', + description: 'Disallow all traffic', + ipProtocol: 'icmp', + fromPort: 252, + toPort: 86 +}; + +/** + * Egress rule that matches all traffic + */ +const ALLOW_ALL_RULE = { + cidrIp: '0.0.0.0/0', + description: 'Allow all outbound traffic by default', + ipProtocol: '-1', +}; + export interface ConnectionRule { /** * The IP protocol name (tcp, udp, icmp) or number (see Protocol Numbers). @@ -315,3 +414,10 @@ function egressRulesEqual(a: cloudformation.SecurityGroupResource.EgressProperty && a.destinationPrefixListId === b.destinationPrefixListId && a.destinationSecurityGroupId === b.destinationSecurityGroupId; } + +/** + * Whether this rule refers to all traffic + */ +function isAllTrafficRule(rule: any) { + return rule.cidrIp === '0.0.0.0/0' && rule.ipProtocol === '-1'; +} diff --git a/packages/@aws-cdk/aws-ec2/test/test.connections.ts b/packages/@aws-cdk/aws-ec2/test/test.connections.ts index 909328f644f19..6a56884152f91 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.connections.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.connections.ts @@ -1,13 +1,15 @@ import { expect, haveResource } from '@aws-cdk/assert'; import { Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; + import { - AllConnections, + AllTraffic, AnyIPv4, AnyIPv6, Connections, IcmpAllTypeCodes, IcmpAllTypesAndCodes, + IcmpPing, IcmpTypeAndCode, IConnectable, PrefixList, @@ -25,6 +27,114 @@ import { } from "../lib"; export = { + 'security group can allows all outbound traffic by default'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: true }); + + // THEN + expect(stack).to(haveResource('AWS::EC2::SecurityGroup', { + SecurityGroupEgress: [ + { + CidrIp: "0.0.0.0/0", + Description: "Allow all outbound traffic by default", + IpProtocol: "-1" + } + ], + })); + + test.done(); + }, + + 'no new outbound rule is added if we are allowing all traffic anyway'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + const sg = new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: true }); + sg.addEgressRule(new AnyIPv4(), new TcpPort(86), 'This does not show up'); + + // THEN + expect(stack).to(haveResource('AWS::EC2::SecurityGroup', { + SecurityGroupEgress: [ + { + CidrIp: "0.0.0.0/0", + Description: "Allow all outbound traffic by default", + IpProtocol: "-1" + }, + ], + })); + + test.done(); + }, + + 'security group disallow outbound traffic by default'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: false }); + + // THEN + expect(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(); + }, + + 'bogus outbound rule disappears if another rule is added'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + const sg = new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: false }); + sg.addEgressRule(new AnyIPv4(), new TcpPort(86), 'This replaces the other one'); + + // THEN + expect(stack).to(haveResource('AWS::EC2::SecurityGroup', { + SecurityGroupEgress: [ + { + CidrIp: "0.0.0.0/0", + Description: "This replaces the other one", + FromPort: 86, + IpProtocol: "tcp", + ToPort: 86 + } + ], + })); + + test.done(); + }, + + 'all outbound rule cannot be added after creation'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + const sg = new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: false }); + test.throws(() => { + sg.addEgressRule(new AnyIPv4(), new AllTraffic(), 'All traffic'); + }, /Cannot add/); + + test.done(); + }, + 'peering between two security groups does not recursive infinitely'(test: Test) { // GIVEN const stack = new Stack(undefined, 'TestStack', { env: { account: '12345678', region: 'dummy' }}); @@ -47,7 +157,7 @@ export = { // GIVEN const stack = new Stack(); const vpc = new VpcNetwork(stack, 'VPC'); - const sg1 = new SecurityGroup(stack, 'SomeSecurityGroup', { vpc }); + const sg1 = new SecurityGroup(stack, 'SomeSecurityGroup', { vpc, allowAllOutbound: false }); const somethingConnectable = new SomethingConnectable(new Connections({ securityGroup: sg1 })); const securityGroup = SecurityGroupRef.import(stack, 'ImportedSG', { securityGroupId: 'sg-12345' }); @@ -103,7 +213,8 @@ export = { new IcmpTypeAndCode(5, 1), new IcmpAllTypeCodes(8), new IcmpAllTypesAndCodes(), - new AllConnections() + new IcmpPing(), + new AllTraffic() ]; // WHEN diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts index 953a0cacaad61..7df533982bab5 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancing/lib/load-balancer.ts @@ -207,7 +207,7 @@ export class LoadBalancer extends cdk.Construct implements IConnectable, codedep constructor(parent: cdk.Construct, name: string, props: LoadBalancerProps) { super(parent, name); - this.securityGroup = new SecurityGroup(this, 'SecurityGroup', { vpc: props.vpc }); + this.securityGroup = new SecurityGroup(this, 'SecurityGroup', { vpc: props.vpc, allowAllOutbound: false }); this.connections = new Connections({ securityGroup: this.securityGroup }); // Depending on whether the ELB has public or internal IPs, pick the right backend subnets diff --git a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json index 25da8feca4a7f..50513f3f5ba0f 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancing/test/integ.elb.expected.json @@ -175,7 +175,15 @@ "Type": "AWS::EC2::SecurityGroup", "Properties": { "GroupDescription": "aws-cdk-elb-integ/LB/SecurityGroup", - "SecurityGroupEgress": [], + "SecurityGroupEgress": [ + { + "CidrIp":"255.255.255.255/32", + "Description":"Disallow all traffic", + "FromPort":252, + "IpProtocol":"icmp", + "ToPort":86 + } + ], "SecurityGroupIngress": [ { "CidrIp": "0.0.0.0/0", @@ -225,4 +233,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts index c2a0213cf0003..bb4e15ef8ce22 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts @@ -64,7 +64,8 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic this.securityGroup = props.securityGroup || new ec2.SecurityGroup(this, 'SecurityGroup', { vpc: props.vpc, - description: `Automatically created Security Group for ELB ${this.uniqueId}` + description: `Automatically created Security Group for ELB ${this.uniqueId}`, + allowAllOutbound: false }); this.connections = new ec2.Connections({ securityGroup: this.securityGroup }); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json index 47090c4d5e411..6e2cd595b15b8 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/integ.alb.expected.json @@ -333,7 +333,15 @@ "Type": "AWS::EC2::SecurityGroup", "Properties": { "GroupDescription": "Automatically created Security Group for ELB awscdkelbv2integLB9950B1E4", - "SecurityGroupEgress": [], + "SecurityGroupEgress": [ + { + "CidrIp":"255.255.255.255/32", + "Description":"Disallow all traffic", + "FromPort":252, + "IpProtocol":"icmp", + "ToPort":86 + } + ], "SecurityGroupIngress": [ { "CidrIp": "0.0.0.0/0", diff --git a/packages/@aws-cdk/aws-lambda/lib/lambda.ts b/packages/@aws-cdk/aws-lambda/lib/lambda.ts index ba409aafd394e..ada7082d963bd 100644 --- a/packages/@aws-cdk/aws-lambda/lib/lambda.ts +++ b/packages/@aws-cdk/aws-lambda/lib/lambda.ts @@ -140,6 +140,16 @@ export interface FunctionProps { */ securityGroup?: ec2.SecurityGroupRef; + /** + * Whether to allow the Lambda to send all network traffic + * + * If set to false, you must individually add traffic rules to allow the + * Lambda to connect to network targets. + * + * @default true + */ + allowAllOutbound?: boolean; + /** * Enabled DLQ. If `deadLetterQueue` is undefined, * an SQS queue with default options will be defined for your Function. @@ -312,16 +322,22 @@ export class Function extends FunctionRef { * Lambda creation properties. */ private configureVpc(props: FunctionProps): cloudformation.FunctionResource.VpcConfigProperty | undefined { + if ((props.securityGroup || props.allowAllOutbound !== undefined) && !props.vpc) { + throw new Error(`Cannot configure 'securityGroup' or 'allowAllOutbound' without configuring a VPC`); + } + if (!props.vpc) { return undefined; } - let securityGroup = props.securityGroup; - if (!securityGroup) { - securityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', { - vpc: props.vpc, - description: 'Automatic security group for Lambda Function ' + this.uniqueId, - }); + if (props.securityGroup && props.allowAllOutbound !== undefined) { + throw new Error(`Configure 'allowAllOutbound' directly on the supplied SecurityGroup.`); } + const securityGroup = props.securityGroup || new ec2.SecurityGroup(this, 'SecurityGroup', { + vpc: props.vpc, + description: 'Automatic security group for Lambda Function ' + this.uniqueId, + allowAllOutbound: props.allowAllOutbound + }); + this._connections = new ec2.Connections({ securityGroup }); // Pick subnets, make sure they're not Public. Routing through an IGW diff --git a/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json b/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json index 3862d4993d222..386f53a95073c 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json +++ b/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.expected.json @@ -355,10 +355,8 @@ "SecurityGroupEgress": [ { "CidrIp": "0.0.0.0/0", - "Description": "Talk to everyone", - "FromPort": 0, - "IpProtocol": "tcp", - "ToPort": 65535 + "Description": "Allow all outbound traffic by default", + "IpProtocol": "-1" } ], "SecurityGroupIngress": [], @@ -405,4 +403,4 @@ ] } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.ts b/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.ts index e2f622f8bdafb..8f59243ce5db5 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.ts +++ b/packages/@aws-cdk/aws-lambda/test/integ.vpc-lambda.ts @@ -7,13 +7,11 @@ const app = new cdk.App(); const stack = new cdk.Stack(app, 'aws-cdk-vpc-lambda'); const vpc = new ec2.VpcNetwork(stack, 'VPC', { maxAZs: 2 }); -const fn = new lambda.Function(stack, 'MyLambda', { +new lambda.Function(stack, 'MyLambda', { code: new lambda.InlineCode('def main(event, context): pass'), handler: 'index.main', runtime: lambda.Runtime.Python36, vpc }); -fn.connections.allowToAnyIPv4(new ec2.TcpAllPorts(), 'Talk to everyone'); - app.run(); diff --git a/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts b/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts index 903fa1c738a57..2af214ae84f0d 100644 --- a/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts +++ b/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts @@ -20,7 +20,8 @@ export = { code: new lambda.InlineCode('foo'), handler: 'index.handler', runtime: lambda.Runtime.NodeJS610, - vpc: this.vpc + vpc: this.vpc, + allowAllOutbound: false }); } @@ -50,7 +51,7 @@ export = { // WHEN this.lambda.connections.allowTo(somethingConnectable, new ec2.TcpAllPorts(), 'Lambda can call connectable'); - // THEN: SomeSecurityGroup accepts connections from Lambda + // THEN: Lambda can connect to SomeSecurityGroup expect(this.stack).to(haveResource("AWS::EC2::SecurityGroupEgress", { GroupId: {"Fn::GetAtt": ["LambdaSecurityGroupE74659A1", "GroupId"]}, IpProtocol: "tcp", @@ -60,7 +61,7 @@ export = { ToPort: 65535 })); - // THEN: Lambda can connect to SomeSecurityGroup + // THEN: SomeSecurityGroup accepts connections from Lambda expect(this.stack).to(haveResource("AWS::EC2::SecurityGroupIngress", { IpProtocol: "tcp", Description: "Lambda can call connectable", diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json b/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json index c95a923aec045..35e7814dcf7eb 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json +++ b/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json @@ -380,7 +380,13 @@ "Type": "AWS::EC2::SecurityGroup", "Properties": { "GroupDescription": "RDS security group", - "SecurityGroupEgress": [], + "SecurityGroupEgress": [ + { + "CidrIp":"0.0.0.0/0", + "Description":"Allow all outbound traffic by default", + "IpProtocol":"-1" + } + ], "SecurityGroupIngress": [], "VpcId": { "Ref": "VPCB9E5F0B4" @@ -481,4 +487,4 @@ ] } } -} \ No newline at end of file +} diff --git a/tools/cdk-build-tools/bin/cdk-test.ts b/tools/cdk-build-tools/bin/cdk-test.ts index f8aa6b9fea07d..2a803249109cc 100644 --- a/tools/cdk-build-tools/bin/cdk-test.ts +++ b/tools/cdk-build-tools/bin/cdk-test.ts @@ -39,6 +39,11 @@ async function main() { default: require.resolve('nodeunit/bin/nodeunit'), defaultDescription: 'nodeunit provided by node dependencies' }) + .option('build', { + type: 'boolean', + desc: 'Rebuild the packages before running the tests', + default: true, + }) .argv as any; const options = cdkBuildOptions(); @@ -50,7 +55,9 @@ async function main() { // Always recompile before running tests, so it's impossible to forget. // During a normal build, this means we'll compile twice, but the // hash calculation makes that cheaper on CPU (if not on disk). - await compileCurrentPackage(timers, { jsii: args.jsii, tsc: args.tsc }); + if (args.build) { + await compileCurrentPackage(timers, { jsii: args.jsii, tsc: args.tsc }); + } const testFiles = await unitTestFiles(); if (testFiles.length > 0) {