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_iam_policy_length #153

Merged
merged 4 commits into from
Aug 11, 2021
Merged

Conversation

Rihoj
Copy link
Contributor

@Rihoj Rihoj commented Aug 8, 2021

Added a rule to check against IAM policy length. fixes #26

I am not sure how you would like to have an extraordinarily long policy shown in the documentation and alerts so I did my best. If you would like a change just let me know how and I will get it updated.

Copy link
Member

@wata727 wata727 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start. Thanks!

I think we often use the aws_iam_policy_document data source to declare IAM policies, and I couldn't think of a way to support the case, so I've put this issue on hold. However, it's a good idea to support just the string literal pattern.

if len(policy) > 2048 {
runner.EmitIssueOnExpr(
r,
fmt.Sprintf("The policy length is %d characters and is limited to 2048 characters.", len(policy)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2048 characters is a limit for inline policies. The managed policy limit is 6144 characters and should not count white spaces.
https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html#reference_iam-quotas-entity-length

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh. Yeah, that makes sense. Given it's entity based, and we would then also have to look at what it's being attached to we may not be able to apply a strict check to this except the highest option which is actually 10,240 (inline role policy).

However, we could look at doing this for specifically as
aws_iam_group_policy: 5120
aws_iam_role_policy: 10240
aws_iam_user_policy: 2048

Unless, you know a way we can find what entity the policy is going to be attached to I can't think of any other option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, since aws_iam_policy always creates a customer managed policy, isn't the upper limit 6,144?
I think the upper limit for inline_policy blocks such as aws_iam_role resources will be 10,240.
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role#example-of-exclusive-inline-policies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wata727 Hmm. yeah, That makes more sense... I think the wording in this document kept throwing me off: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_grammar.html#policies-grammar-notes

Reading the limits page again. I think you are right aws_iam_policy should be good to set to 6144. And I am not sure the inlines can be checked given the docs " But the total aggregate policy size (the sum size of all inline policies) per entity cannot exceed the following quotas"

My apologies on the confusion here.


// Name returns the rule name
func (r *AwsIAMPolicyInvalidLengthRule) Name() string {
return "aws_iam_policy_invalid_length"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I prefer names like aws_iam_policy_too_long_policy for such rules. Naming such as {resource}_invalid_{attribute} is for automatic generation, and it is better to have an obvious name for the violation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will keep that in mind, and depending on where we go with this I will update it accordingly.

@Rihoj
Copy link
Contributor Author

Rihoj commented Aug 10, 2021

As far as the data source goes, I wasn't even aware that one existed as I don't commonly write policies as code yet. (not enough buy in unfortunately.)

@Rihoj
Copy link
Contributor Author

Rihoj commented Aug 11, 2021

Depending on how this one goes I may pick up the inline ones next.

@wata727 wata727 merged commit 8047cf5 into terraform-linters:master Aug 11, 2021
@wata727
Copy link
Member

wata727 commented Aug 11, 2021

👍

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

Successfully merging this pull request may close these issues.

feat: detect iam policy documents greater or near 2048 maximum char limit
2 participants