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

Enhancement proposal for ARN linting #238

Closed
ashemedai opened this issue Jul 24, 2018 · 6 comments
Closed

Enhancement proposal for ARN linting #238

ashemedai opened this issue Jul 24, 2018 · 6 comments
Labels
enhancement New feature or request new rule New rule question Further information is requested

Comments

@ashemedai
Copy link

ashemedai commented Jul 24, 2018

Given that the ARN format always starts with arn:aws: and for each product has a clear defined syntax it seems like an ideal candidate to add to the linter (and the roadmap agrees).

My naive thinking currently says: check normal ARNs via something like regular expressions, keeping in mind that there's a fair amount that are essentially variable length. Could be a simple switch logic based on product and test for presence of fields/number of colons and test fields whether they comply to expected syntax.

Substitute syntax will be an interesting case to lint, like !Sub "arn:aws:iam::${SomeAccountId}:user.

What do you think, @cmmeyer and @kddejong? See any potential hurdles with the above?

There's time within our company to pick this up and work on it.

@kddejong kddejong added enhancement New feature or request question Further information is requested labels Jul 24, 2018
@kddejong
Copy link
Contributor

I have debated this a few times. There is also this article from AWS that has all the valid ARN syntaxes. I think there are some other tricky areas like IAM that doesn't have a region etc that would be helpful to help people to syntax.

Other areas of possibilities.

  • Cross region relationships that may not work. CWE can't use a Lambda in another region, etc.
  • Best practices could be done here... like not hard coding a region into a ARN.

I think the possibilities of getting this setup well could be awesome but we should probably start small.

Some part of me wants to relate this to #50 but instead of allowed values using a Regex.

@cmmeyer thoughts?

@SanderKnape
Copy link

@kddejong any new thoughts/ideas on this that we could perhaps contribute to?

@kddejong
Copy link
Contributor

@SanderKnape we are starting to cover this with Regex checking.

An Iam Role Arn has to match the following pattern.
"AllowedPatternRegex": "arn:(aws[a-zA-Z-]*)?:iam::(\\d{12}|aws):policy/[a-zA-Z_0-9+=,.@\\-_/]+"

We still have work to build out all those AllowedPatternRegex values but this capability now exists.

@PatMyron
Copy link
Contributor

PatMyron commented Feb 24, 2020

  • Best practices could be done here... like not hard coding a region into a ARN.

I like the idea of a non-Error level rule that could flag ARNs with hardcoded partitions, regions, accounts, or resources

@ashemedai
Copy link
Author

@kddejong Given all progress since this issue was created, does it make sense to keep it open still?

@kddejong
Copy link
Contributor

We have made a lot of progress in this space. We can add new issues as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new rule New rule question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants