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

[iam] iam.Group should check if inline policy exceeds maximum size #11562

Closed
2 tasks
AlJohri opened this issue Nov 18, 2020 · 5 comments
Closed
2 tasks

[iam] iam.Group should check if inline policy exceeds maximum size #11562

AlJohri opened this issue Nov 18, 2020 · 5 comments
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@AlJohri
Copy link

AlJohri commented Nov 18, 2020

I received the following error after deploying my CDK stack. For some reason, it took a very long time for this error to appear.

iam Maximum policy size of 5120 bytes exceeded for group XYZ

Use Case

This error seems simple to test at synthesis time in CDK and would save a lot of debugging time.

Proposed Solution

Check the total size of the combined inline policies for an iam.Group and see if it is less than 5120 bytes.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@AlJohri AlJohri added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 18, 2020
@AlJohri AlJohri changed the title [iam] iam should check if inline policy exceeds maximum size [iam] iam.Group should check if inline policy exceeds maximum size Nov 18, 2020
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Nov 18, 2020
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2 labels Nov 23, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Nov 23, 2020
@swar8080
Copy link
Contributor

swar8080 commented Jan 17, 2021

Hi @rix0rrr, I looked into how to implement this. Not sure if this is a bigger change then expected, so I figured i'd summarize an approach for feedback before writing code.

For reference here's the documentation on policy character limits: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html#reference_iam-quotas-entity-length

Quick summary of existing classes involved

  • PolicyDocument: Responsible for constructing the policy document JSON. Has no knowledge of what type of policy the document is for.
  • Policy: Encapsulates the PolicyDocument and keeps track of all the users, groups, and roles associated to that document.
  • Identities (User, Group, Role): Knows which policies it's associated with but has no access to the underlying PolicyDocument.

High level approach

  • A new PolicyDocumentValidator interface is added with method validatePolicyLength(length: number, policyDocument: any): string | null. It would optionally return a message describing any length validation violations.
  • A PolicyDocumentValidator would be an optional instance variable on PolicyDocument.
  • During post-processing of PolicyDocument (using cdk.IPostProcessor), after all tokens in the document have been resolved, the PolicyDocument generates its JSON string representation and passes the size to its PolicyDocumentValidator.

PolicyDocumentValidator concrete implementations

  • User, Group, Role: Would keep a running sum of the lengths of inline policies passed to them and compare that value to the maximum allowed. These implementations would need to keep track of which policyDocument they have already seen to avoid double counting, since policy documents are resolved multiple times (during the prepare and synth phases).
  • Policy - The Policy's implementation of PolicyDocumentValidator would be passed to its PolicyDocument. When called, the Policy would iterate through the referenced users, groups, and roles attached to the policy and pass them the length of the policy document for validation. The Policy would either warn or throw any validation error messages returned by those calls.

Other Decisions

  • Warning or error when size is exceeded? AWS doesn't count whitespace towards the limit, which JSON.stringify removes, but there could be minor differences in how characters are counted that could cause false positives.
  • Should managed and trust policies also be validated as part of this ticket?

@andreialecu
Copy link
Contributor

andreialecu commented Feb 27, 2021

Got recently hit by this while doing unrelated changes to a stack.

It resulted in CloudFormation getting stuck for 1 hour, then failing with this error, which is not a very good experience.

The problem seems to be that cdk makes it easy to create huge IAM policies. I was able to refactor the stack to work around this.

To exemplify, there were a lot of policies being added similar to this below, from various stacks:

    policy.addToPolicy(
      new iam.PolicyStatement({
        resources: [repository.repositoryArn],
        actions: [
          "ecr:InitiateLayerUpload",
          "ecr:UploadLayerPart",
          "ecr:CompleteLayerUpload",
          "ecr:BatchCheckLayerAvailability",
          "ecr:PutImage",
        ],
      })
    );

Instead, a single policy with multiple arns listed in resources should've been added.

@andreialecu
Copy link
Contributor

Got bit by this again via this innocent construct which was executed on multiple task definitions:

taskDefinition.taskRole.grantPassRole(this.someGroup);
taskDefinition.executionRole?.grantPassRole(this.someGroup);

It appears that only about 7 task definitions can be supported by the above code, after which it runs into the maximum size exeeded error.

I was able to deploy a workaround similar to my previous comment above, but this seems like it should be handled better.

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 3, 2022
@github-actions github-actions bot closed this as completed Jun 8, 2022
@jedrus2000
Copy link

Issue should be reopened, its really hurts deployment experience of CDK stacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

No branches or pull requests

6 participants