aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.57k stars 3.88k forks source link

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

Open jpickwell opened 6 months ago

jpickwell commented 6 months ago

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 commented 5 months ago

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

https://github.com/aws/aws-cdk/blob/fff9cf694b14811682c8671a1e55afa53151df8b/packages/%40aws-cdk/custom-resource-handlers/lib/aws-ec2/restrict-default-security-group-handler/index.ts#L27-L37

mwebber commented 3 months ago

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:

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