(aws-ec2): restrictDefaultSecurityGroup does not remove IPv6 egress rule

[jpickwell]

Describe the bug

Setting restrictDefaultSecurityGroup to true for a dual-stack VPC will not remove the IPv6 egress rule.

Expected Behavior

For a dual-stack VPC with restrictDefaultSecurityGroup set to true, all (IPv4 and IPv6) ingress and egress rules should be removed.

Current Behavior

For a dual-stack VPC with restrictDefaultSecurityGroup set to true, only IPv4 ingress and egress rules are removed.

Reproduction Steps

import * as cdk from 'aws-cdk-lib';
import * as ec2 from 'aws-cdk-lib/aws-ec2';

const app = new cdk.App();
const stack = new cdk.Stack(app, 'stack');

const vpc = new ec2.Vpc(stack, 'vpc', {
  ipAddresses: ec2.IpAddresses.cidr('10.0.0.0/24'),
  ipProtocol: ec2.IpProtocol.DUAL_STACK,
  restrictDefaultSecurityGroup: true,
});

app.synth();

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.135.0 (build d46c474)

Framework Version

No response

Node.js Version

v20.12.0

OS

macOS Sonoma 14.4.1 (23E224)

Language

TypeScript

Language Version

TypeScript (5.4.3)

Other information

No response

[nmussy]

Regardless of the DUAL_STACK prop, we could always include both 0.0.0.0/0 and ::/0 here, correct?

aws-cdk/packages/@aws-cdk/custom-resource-handlers/lib/aws-ec2/restrict-default-security-group-handler/index.ts

Lines 27 to 37 in fff9cf6

	function egressRuleParams(groupId: string): sdk.RevokeSecurityGroupEgressCommandInput | sdk.AuthorizeSecurityGroupEgressCommandInput {
	return {
	GroupId: groupId,
	IpPermissions: [{
	IpRanges: [{
	CidrIp: '0.0.0.0/0',
	}],
	IpProtocol: '-1',
	}],
	};
	}

[mwebber]

I have also observed this.
The feature was originally introduced in #25297, with a subsequent fix applied in #27039.

@nmussy

Regardless of the DUAL_STACK prop, we could always include both 0.0.0.0/0 and ::/0 here, correct?

I think that routine needs to do 2 things:

• remove the rules from the default when @aws-cdk/aws-ec2:restrictDefaultSecurityGroup feature flag changed to true
• restore the rules from the default when @aws-cdk/aws-ec2:restrictDefaultSecurityGroup feature flag changed to false

In the second case there, if it's not dual stack, then it should not add ::/0 back in, I guess.

[khushail]

Apologies for the delay in getting back.

Thanks @mwebber for sharing your insights and linking the PRs- #25297 ,#27039

I would reach out to team to share their insights on the changes implemented through PRs and current issue.

[godwingrs22]

Thanks @jpickwell for bringing up this issue and thanks @mwebber and @nmussy for the suggestions.

For VPCs that are not with DUAL_STACK, the ::/0 rule is not present by default in the security group. The ::/0 rule is only added when there is an associated IPv6 CIDR block in VPC - which means only when DUAL_STACK is used.

I think we should handle this with another new feature flag to remove the IPv6 egress rule in DUAL_STACK VPC, since making this change directly in the existing flag would be a breaking change - some customers may be relying on the default security group's IPv6 egress rule in their applications.

Please feel free to raise a PR.

[mwebber]

The ::/0 rule is only added when there is an associated IPv6 CIDR block in VPC - which means only when DUAL_STACK is used.

I don't think that's strictly true. Some of our VPCs were created before DUAL_STACK was available, but they do attach a IPv6 CIDR block to the VPC. For some example code, see #894 (comment).

The "correct" solution to this is for CloudFormation to add support for creating the default security group as empty.

[godwingrs22]

Hi @mwebber,

I don't think that's strictly true. Some of our VPCs were created before DUAL_STACK was available, but they do attach a IPv6 CIDR block to the VPC. For some example code, see #894 (comment).

Yes you are right. The presence of IPv6 ::/0 rules in the default security group is tied to whether the VPC has an IPv6 CIDR block associated with it, regardless of how that association was made (whether through DUAL_STACK or other methods).

The "correct" solution to this is for CloudFormation to add support for creating the default security group as empty.

I agree that having native CloudFormation support would be the ideal solution. However, since we don't have visibility into when/if this might be supported by the service team, and we already have a custom resource handling the behavior, extending with a feature flag seems like a practical easier solution.