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

Remove "allow-all-egress" security-group rule #127

Open
dlacosteGFM opened this issue Jun 6, 2023 · 3 comments
Open

Remove "allow-all-egress" security-group rule #127

dlacosteGFM opened this issue Jun 6, 2023 · 3 comments

Comments

@dlacosteGFM
Copy link
Contributor

Describe the Feature

TL;DR

Add a new flag to make allow-all-egress optional

Detail

This is really simple: EFS doesn't do egress (logically) so the "allow egress to 0.0.0.0/0" rule is redundant.

On top of that, we got a question during a compliance audit about the rule: we aren't supposed to be doing "open to 0.0.0.0/0" rules if we can avoid it (see references below)

Therefore I made a PR to just "make that an additional parameter that defaults to true so nobody has to change anything, but you can change it to false if your PCI auditor is upset.

References

  • PR with proposed change: Make allow_all_egress a variable #126
  • PCI DSS 3.2.1 rule 1.1.7 - Requirement to review firewall and router rule sets every 6 months
  • PCI DSS 3.2.1 rule 1.2.1 - Restrict inbound and outbound traffic to that which is necessary for the environment

Expected Behavior

Expected behavior for existing users who make no change: no impact, same result.

Expected behavior for those who set the allow_all_egress parameter to false when they use this module: that one security group rule is not added.

Use Case

The use case is for anyone trying to keep PCI auditors happy (open to 0.0.0.0/0 rules are frowned upon).

Describe Ideal Solution

PR: #126

Alternatives Considered

We tried maintaining our own security-group instead but this is MUCH cleaner and easier (and more shareable!)

Additional Context

No response

@dlacosteGFM
Copy link
Contributor Author

We applied the PR in both test and production with no impact: everything continued working with no issue (which makes sense, as EFS does not do egress :) )

@ezeroti
Copy link

ezeroti commented Jun 9, 2023

i need the same! any alternative workaround?

@dlacosteGFM
Copy link
Contributor Author

Hello @ezeroti (sorry for the delay but I went on vacation the day after you posted your comment)

I have made a public repo available with the change made: you could use that if you want (diff below) or you could make the same on your own (just fork this repo, apply the diff, and then refer to that from your terraform call)

  module "efs_api_cps" {
-   source = "git::https://github.com/cloudposse/terraform-aws-efs.git?ref=0.34.0"
+   source = "git::https://github.com/dlacosteGFM/terraform-aws-efs.git?ref=0.34.0%2Begress_flag"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants