Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(aws-ec2): configuration so the default security group will not allow inbound and outbound traffic #19394

Closed
2 tasks
SamStephens opened this issue Mar 14, 2022 · 8 comments · Fixed by #25297
Closed
2 tasks
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@SamStephens
Copy link
Contributor

Description

AWS Foundational Security Best Practices state The VPC default security group should not allow inbound and outbound traffic.

However the CDK does not provide any hooks to do this, or even any access to the default security group at all, other than its ID via Vpc#vpcDefaultSecurityGroup.

Lack of proper support for this in the CDK makes it difficult to meet the standards AWS is setting with the Foundational Security Best Practices. Looking at discussions in other issues, people are creating custom CDK resources to perform this lockdown.

Use Case

To comply with security best practices as provided by AWS Foundational Security Best Practices.

Proposed Solution

A boolean property on the VPC object, that defaults to false which keeps the existing behavior, and if set to true, will lock down the security group.

Other information

Related issues:

Acknowledge

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@SamStephens SamStephens added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 14, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Mar 14, 2022
@corymhall
Copy link
Contributor

It looks like we would have to do this with a custom resource, but I think it is something that we should do.

Thanks so much for submitting this pull request. I am marking this pull request as p2, which means that we are unable to work on it immediately, but it's definitely still on our radar.

We use +1s to help prioritize our work, and are happy to revaluate this pull request based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

@corymhall corymhall added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 15, 2022
@corymhall corymhall removed their assignment Mar 15, 2022
@albertocorrales
Copy link

I was looking for something like this too, so I'm looking forward to having this implemented.

In the meantime, is there any way to workaround this? Like a command to get the default SG for a VPC and update the rules.

Thanks,

Alberto.

@SamStephens
Copy link
Contributor Author

I was looking for something like this too, so I'm looking forward to having this implemented.

In the meantime, is there any way to workaround this? Like a command to get the default SG for a VPC and update the rules.

Thanks,

Alberto.

The only workaround within the CDK I've seen is using custom resources as I linked to in the issue description.

Outside of CDK, one interesting possibility is that AWS provides an example of how to use AWS Config to detect and remediate this issue.

@github-actions github-actions bot added p1 and removed p2 labels Sep 11, 2022
@github-actions
Copy link

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

@corymhall corymhall self-assigned this Apr 20, 2023
@mergify mergify bot closed this as completed in #25297 May 2, 2023
mergify bot pushed a commit that referenced this issue May 2, 2023
…lag) (#25297)

This PR implements functionality which will remove the default ingress/egress rules from the VPC default security group. When a VPC is created, the default security group is created as well with default ingress/egress rules which allow _all_ traffic. It is not possible to delete the default security group, but you should never use it. As a result there are a log of security standards that recommend removing the default rules so that the security group denies all traffic by default. See [this rule](https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-2).

Since the default security group cannot be managed through a CloudFormation resource, this PR introduces a new Custom Resource which will remove the ingress/egress rules.

I also think that this should be the default behavior so I have introduced a new feature flag to make this the default for new apps. As a result I had to update _a lot_ of integration tests. Since This feature flag would only be introduced on new VPCs it didn't make sense to run the update workflow on all these integration tests so I updated them to disable this new feature.

I added one new integration test to test this functionality.

fixes #19394

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented May 2, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@SamStephens
Copy link
Contributor Author

Thanks @corymhall

@mwebber
Copy link

mwebber commented May 16, 2023

@corymhall This is great, but it looks like the fix for this might be incomplete.

I upgraded to CDK 2.79.1 (which includes the fix), set the feature flag "@aws-cdk/aws-ec2:restrictDefaultSecurityGroup": true, and redeployed a Stack which defines a VPC.

I noticed that in Outbound Rules for the default security group, the rule allowing outbound IPv4 traffic had been removed, but the rule allowing outbound IPv6 traffic was still there (all IPv6 traffic allowed to ::/0).
All Inbound Rules were removed as expected.

I observed this same result in all the VPCs that I re-deployed with the new flag enabled.
I did not test the initial deploy of a new VPC.

@mwebber
Copy link

mwebber commented Jun 11, 2024

The bug I mentioned in the comment above (IPv6 traffic still allowed) is #29709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants