-
-
Notifications
You must be signed in to change notification settings - Fork 322
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
Default aws_iam_role_policy is too permissive (feedback is welcome) #181
Comments
Hi @florian0410. I can't agree with you more on this issue. Until this gets fixed, I've modified my local copy of the module to include an input variable to override the iam instance profile that get's created with this module. Outside the module, I've created an IAM instance profile associated to an IAM Role that has the AWS Elasticbeanstalk webtier managed policy attached to it. I could make a PR later this week if the maintainers think this is useful. |
Thank you for your answer @VictorCovalski, Since the maintainers seems to use this module less they don't do many PR but are still available for reviewing and validating these. So a PR seems to be the best way to propose a fix anyway. I was planning to create one someday but I'm still waiting for my previous one to be validated. Don't hesitate to ping me if you do a PR for some brainstorming. Since there are many ways to do, I'm not sure yet if I should go on a variable to specify an external iam_instance_profile as you do or if we should start slowly by having the |
+1 for this issue, following this for a resolution. I also am creating a local module on my end to fix. |
See #147 as related. |
Describe the Feature
The default EC2 instance profile give too many rights to deployed EC2 instance. It should not give any right at start.
Currently, a newly deployed Beanstalk environment attach a default instance profile
terraform-aws-elastic-beanstalk-environment/main.tf
Lines 82 to 86 in 003be6e
Which is a merge of
extended_ec2_policy_document
variable and the default policy document defined in the module.terraform-aws-elastic-beanstalk-environment/main.tf
Lines 290 to 294 in 003be6e
The
data.aws_iam_policy_document.default
is coresponding to the following definitionterraform-aws-elastic-beanstalk-environment/main.tf
Lines 130 to 288 in 003be6e
This policy document is way too permissive and is not required in a first time to ensure elastic beanstalk environment functionnalities.
Expected Behavior
These rights should not be set by default and user may define its own rights using
extended_ec2_policy_document
.Since the web_tier, elastic_beanstalk_multi_container_docker and worker_tier are set by default, any depoyed env have enough rights to ensure minimal requirements.
Use Case
We recently deployed multiple environments using this module but we realized recently that each environment that was deployed had access to all S3 bucket thanks to these default rights.
Alternatives Considered
I was thinking about a feature like those asked in Allow use of existing IAM role for EC2 instance profile #107 Allow use of existing IAM role for EC2 instance profile #113 but since default rights stay in place, it may be a requirement for these 2 requests too.
I tried to override the rights using
extended_ec2_policy_document
(using https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document#override_json) but since there is a block with an empty sid, I cannot fully override these. Plus I cannot override rights with an empty policy document.Additional Context
I didn't found any documentation about default required rights for beanstalk ec2 instance instead those about worker tier multicontainer tiers and webtier (https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/concepts-roles-instance.html).
I consider that the tiers rights are the only required to begin and so remove all unwanted default rights.
I'm still testing on my side some elements but it's hard to have a global testing coverage of all beanstalk features. Any feedback is welcome.
Since many users may have started use this module with these default rights, maybe that using a variable to keep these default rights could be a solution. Or add a note in the doc to explain how to keep older default rights using
extended_ec2_policy_document
.The text was updated successfully, but these errors were encountered: