Skip to content

Commit

Permalink
Merge branch 'main' into merge-back/2.44.0
Browse files Browse the repository at this point in the history
  • Loading branch information
@mergify
mergify[bot] authored Sep 29, 2022
2 parents bf32cb1 + f7bbc94 commit c73494e
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 7 deletions.
81 changes: 77 additions & 4 deletions packages/@aws-cdk/aws-ec2/lib/security-group.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ abstract class SecurityGroupBase extends Resource implements ISecurityGroup {

public abstract readonly securityGroupId: string;
public abstract readonly allowAllOutbound: boolean;
public abstract readonly allowAllIpv6Outbound: boolean;

public readonly canInlineRule = false;
public readonly connections: Connections = new Connections({ securityGroups: [this] });
Expand Down Expand Up @@ -237,10 +238,25 @@ export interface SecurityGroupProps {
* outbound traffic. If this is set to false, no outbound traffic will be allowed by
* default and all egress traffic must be explicitly authorized.
*
* To allow all ipv6 traffic use allowAllIpv6Outbound
*
* @default true
*/
readonly allowAllOutbound?: boolean;

/**
* Whether to allow all outbound ipv6 traffic by default.
*
* If this is set to true, there will only be a single egress rule which allows all
* outbound ipv6 traffic. If this is set to false, no outbound traffic will be allowed by
* default and all egress ipv6 traffic must be explicitly authorized.
*
* To allow all ipv4 traffic use allowAllOutbound
*
* @default false
*/
readonly allowAllIpv6Outbound?: boolean;

/**
* Whether to disable inline ingress and egress rule optimization.
*
Expand Down Expand Up @@ -274,6 +290,17 @@ export interface SecurityGroupImportOptions {
*/
readonly allowAllOutbound?: boolean;

/**
* Mark the SecurityGroup as having been created allowing all outbound ipv6 traffic
*
* Only if this is set to false will egress rules for ipv6 be added to this security
* group. Be aware, this would undo any potential "all outbound traffic"
* default.
*
* @default false
*/
readonly allowAllIpv6Outbound?: boolean;

/**
* If a SecurityGroup is mutable CDK can add rules to existing groups
*
Expand Down Expand Up @@ -360,6 +387,7 @@ export class SecurityGroup extends SecurityGroupBase {
class MutableImport extends SecurityGroupBase {
public securityGroupId = securityGroupId;
public allowAllOutbound = options.allowAllOutbound ?? true;
public allowAllIpv6Outbound = options.allowAllIpv6Outbound ?? false;

public addEgressRule(peer: IPeer, connection: Port, description?: string, remoteRule?: boolean) {
// Only if allowAllOutbound has been disabled
Expand All @@ -372,6 +400,7 @@ export class SecurityGroup extends SecurityGroupBase {
class ImmutableImport extends SecurityGroupBase {
public securityGroupId = securityGroupId;
public allowAllOutbound = options.allowAllOutbound ?? true;
public allowAllIpv6Outbound = options.allowAllIpv6Outbound ?? false;

public addEgressRule(_peer: IPeer, _connection: Port, _description?: string, _remoteRule?: boolean) {
// do nothing
Expand Down Expand Up @@ -441,6 +470,11 @@ export class SecurityGroup extends SecurityGroupBase {
*/
public readonly allowAllOutbound: boolean;

/**
* Whether the SecurityGroup has been configured to allow all outbound ipv6 traffic
*/
public readonly allowAllIpv6Outbound: boolean;

private readonly securityGroup: CfnSecurityGroup;
private readonly directIngressRules: CfnSecurityGroup.IngressProperty[] = [];
private readonly directEgressRules: CfnSecurityGroup.EgressProperty[] = [];
Expand All @@ -458,6 +492,7 @@ export class SecurityGroup extends SecurityGroupBase {
const groupDescription = props.description || this.node.path;

this.allowAllOutbound = props.allowAllOutbound !== false;
this.allowAllIpv6Outbound = props.allowAllIpv6Outbound ?? false;

this.disableInlineRules = props.disableInlineRules !== undefined ?
!!props.disableInlineRules :
Expand All @@ -476,6 +511,7 @@ export class SecurityGroup extends SecurityGroupBase {
this.securityGroupName = this.securityGroup.ref;

this.addDefaultEgressRule();
this.addDefaultIpv6EgressRule();
}

public addIngressRule(peer: IPeer, connection: Port, description?: string, remoteRule?: boolean) {
Expand All @@ -496,21 +532,33 @@ export class SecurityGroup extends SecurityGroupBase {
}

public addEgressRule(peer: IPeer, connection: Port, description?: string, remoteRule?: boolean) {
if (this.allowAllOutbound) {
const isIpv6 = peer.toEgressRuleConfig().hasOwnProperty('cidrIpv6');

if (!isIpv6 && 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.
if (!remoteRule) { // Warn only if addEgressRule() was explicitely called
Annotations.of(this).addWarning('Ignoring Egress rule since \'allowAllOutbound\' is set to true; To add customized rules, set allowAllOutbound=false on the SecurityGroup');
}
return;
} else {
} else if (!isIpv6 && !this.allowAllOutbound) {
// 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 (isIpv6 && this.allowAllIpv6Outbound) {
// In the case of "allowAllIpv6Outbound", we don't add any more rules. There
// is only one rule which allows all traffic and that subsumes any other
// rule.
if (!remoteRule) { // Warn only if addEgressRule() was explicitely called
Annotations.of(this).addWarning('Ignoring Egress rule since \'allowAllIpv6Outbound\' is set to true; To add customized rules, set allowAllIpv6Outbound=false on the SecurityGroup');
}
return;
}

if (!peer.canInlineRule || !connection.canInlineRule || this.disableInlineRules) {
super.addEgressRule(peer, connection, description, remoteRule);
return;
Expand All @@ -532,7 +580,7 @@ export class SecurityGroup extends SecurityGroupBase {
// 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.');
throw new Error('Cannot add an "all traffic" egress rule in this way; set allowAllOutbound=true (for ipv6) or allowAllIpv6Outbound=true (for ipv6) on the SecurityGroup instead.');
}

this.addDirectEgressRule(rule);
Expand Down Expand Up @@ -596,6 +644,31 @@ export class SecurityGroup extends SecurityGroupBase {
}
}

/**
* Add a allow all ipv6 egress rule to the securityGroup
*
* This depends on allowAllIpv6Outbound:
*
* - If allowAllIpv6Outbound is true, we will add an allow all rule.
* - If allowAllOutbound is false, we don't do anything since EC2 does not add
* a default allow all ipv6 rule.
*/
private addDefaultIpv6EgressRule() {
const description = 'Allow all outbound ipv6 traffic by default';
const peer = Peer.anyIpv6();
if (this.allowAllIpv6Outbound) {
if (this.disableInlineRules) {
super.addEgressRule(peer, Port.allTraffic(), description, false);
} else {
this.directEgressRules.push({
ipProtocol: '-1',
cidrIp: peer.uniqueId,
description,
});
}
}
}

/**
* Remove the bogus rule if it exists
*/
Expand Down Expand Up @@ -721,7 +794,7 @@ function egressRulesEqual(a: CfnSecurityGroup.EgressProperty, b: CfnSecurityGrou
* Whether this rule refers to all traffic
*/
function isAllTrafficRule(rule: any) {
return rule.cidrIp === '0.0.0.0/0' && rule.ipProtocol === '-1';
return (rule.cidrIp === '0.0.0.0/0' || rule.cidrIpv6 === '::/0') && rule.ipProtocol === '-1';
}

/**
Expand Down
66 changes: 63 additions & 3 deletions packages/@aws-cdk/aws-ec2/test/security-group.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,59 @@ describe('security group', () => {
},
],
});
});

test('security group can allows all ipv6 outbound traffic by default', () => {
// GIVEN
const stack = new Stack();
const vpc = new Vpc(stack, 'VPC');

// WHEN
new SecurityGroup(stack, 'SG1', { vpc, allowAllIpv6Outbound: true });

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

test('can add ipv6 rules even if allowAllOutbound=true', () => {
// GIVEN
const stack = new Stack();
const vpc = new Vpc(stack, 'VPC');

// WHEN
const sg = new SecurityGroup(stack, 'SG1', { vpc });
sg.addEgressRule(Peer.ipv6('2001:db8::/128'), Port.tcp(80));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::EC2::SecurityGroup', {
SecurityGroupEgress: [
{
CidrIp: '0.0.0.0/0',
Description: 'Allow all outbound traffic by default',
IpProtocol: '-1',
},
{
CidrIpv6: '2001:db8::/128',
Description: 'from 2001:db8::/128:80',
FromPort: 80,
ToPort: 80,
IpProtocol: 'tcp',
},
],
});

});

Expand Down Expand Up @@ -96,8 +148,6 @@ describe('security group', () => {
},
],
});


});

test('all outbound rule cannot be added after creation', () => {
Expand All @@ -110,8 +160,18 @@ describe('security group', () => {
expect(() => {
sg.addEgressRule(Peer.anyIpv4(), Port.allTraffic(), 'All traffic');
}).toThrow(/Cannot add/);
});

test('all ipv6 outbound rule cannot be added after creation', () => {
// GIVEN
const stack = new Stack();
const vpc = new Vpc(stack, 'VPC');

// WHEN
const sg = new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: false });
expect(() => {
sg.addEgressRule(Peer.anyIpv6(), Port.allTraffic(), 'All traffic');
}).toThrow(/Cannot add/);
});

test('immutable imports do not add rules', () => {
Expand Down Expand Up @@ -171,7 +231,7 @@ describe('security group', () => {
// GIVEN
const stack = new Stack(undefined, 'TestStack', { env: { account: '12345678', region: 'dummy' } });
const vpc = new Vpc(stack, 'VPC');
const sg = new SecurityGroup(stack, 'SG', { vpc });
const sg = new SecurityGroup(stack, 'SG', { vpc, allowAllIpv6Outbound: true });

const peers = [
new SecurityGroup(stack, 'PeerGroup', { vpc }),
Expand Down

0 comments on commit c73494e

Please sign in to comment.