From b9d5b43d5ec86993702971f09ec72739b9ee1ee0 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 25 Oct 2018 15:40:54 +0200 Subject: [PATCH] fix(aws-ec2): fix retention of all egress traffic rule (#998) Fix the issue where "all outbound traffic allowed" rules would be overwritten if any other egress rules are added to the Security Group. To solve this, we make `allowAllOutbound` a property of a SecurityGroup (defaults to `true`). By making the SecurityGroup aware of this configuration property, we can make sure that future egress rules don't get added to the SecurityGroup. There's no need to, and adding them would only make CloudFormation delete the "all traffic allowed" rule. Also update documentation on Security Groups in the `README`. Fixes #987. --- .../aws-autoscaling/lib/auto-scaling-group.ts | 9 +- ...g.asg-w-classic-loadbalancer.expected.json | 8 +- .../test/integ.asg-w-elbv2.expected.json | 6 +- .../test/test.auto-scaling-group.ts | 4 +- .../test/integ.deployment-group.expected.json | 16 ++- packages/@aws-cdk/aws-ec2/README.md | 62 ++++++---- .../aws-ec2/lib/security-group-rule.ts | 24 +++- .../@aws-cdk/aws-ec2/lib/security-group.ts | 110 +++++++++++++++- .../@aws-cdk/aws-ec2/test/test.connections.ts | 117 +++++++++++++++++- .../lib/load-balancer.ts | 2 +- .../test/integ.elb.expected.json | 12 +- .../lib/alb/application-load-balancer.ts | 3 +- .../test/integ.alb.expected.json | 10 +- packages/@aws-cdk/aws-lambda/lib/lambda.ts | 28 ++++- .../test/integ.vpc-lambda.expected.json | 8 +- .../aws-lambda/test/integ.vpc-lambda.ts | 4 +- .../aws-lambda/test/test.vpc-lambda.ts | 7 +- .../aws-rds/test/integ.cluster.expected.json | 10 +- tools/cdk-build-tools/bin/cdk-test.ts | 9 +- 19 files changed, 369 insertions(+), 80 deletions(-) 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) {