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

Validate IAM Actions #1117

Closed
trav-c opened this issue Sep 2, 2019 · 10 comments · Fixed by #3560
Closed

Validate IAM Actions #1117

trav-c opened this issue Sep 2, 2019 · 10 comments · Fixed by #3560
Labels
enhancement New feature or request

Comments

@trav-c
Copy link
Contributor

trav-c commented Sep 2, 2019

cfn-lint version:
0.21.4

Description of issue.
At present cfn-lint does not appear to validate the contents of the Action list in IAM policies, while it would probably not be possible to validate them in all cases (eg when Fn::Sub etc is used), it seems like it would be possible to verify that the entry is in fact valid for static entries.

Validating this could help to catch errors where the IAM policy will not actually perform as intended by the the author, for example I just encountered a scenario where an IAM policy Action listed 'dyanmodb:PutItem' rather than 'dynamodb:PutItem'

This sort of error does not prevent the stack from being created/updated, but can prevent the resources it deploys from functioning as intended.

(Simplified) Incorrect fragment example from an AWS::IAM::Role/Policies/PolicyDocument

- Sid: SamlSpTableDataAccess
  Effect: Allow
  Action:
    - dyanmodb:PutItem
        ^^
  Resource: '*'

Which should instead be

- Sid: SamlSpTableDataAccess
  Effect: Allow
  Action:
    - dynamodb:PutItem
        ^^
  Resource: '*'
@kddejong
Copy link
Contributor

kddejong commented Sep 2, 2019

@trav-c Can you try this on your template I want to see if it gets caught.
cfn-lint -e path/to/template.yaml This rule has been marked as experimental for a little bit. I would like to get it out of this state so any testing would be greatly appreciated.

Should result in something like this

➜  cfn-lint -e test/fixtures/templates/bad/resources/iam/managed_policy_permissions.yaml
W3037 Invalid service "kafk" found in permissions
test/fixtures/templates/bad/resources/iam/managed_policy_permissions.yaml:10:13

W3037 Invalid permission "SendMessage" for "sns" found in permissions
test/fixtures/templates/bad/resources/iam/managed_policy_permissions.yaml:14:13

W3037 Invalid service "cldoufront" found in permissions
test/fixtures/templates/bad/resources/iam/managed_policy_permissions.yaml:38:9

@trav-c
Copy link
Contributor Author

trav-c commented Sep 3, 2019

@kddejong I tested this (cfn-lint -e) against the template with the original problem and can confirm that it did indeed correctly detect the service misspelling (alerted once per affected SID, It also flagged dynamodb:ConditionCheck as an invalid permission, which appears to be in conflict with the documentation, however does appear to align with the actual implementation.

ConditionCheck is listed on the DynamoDB IAM permissions reference documentation:

However the actual IAM management console shows it as an 'Unrecognized Action', so I'm not sure which one is right.

The actual check results for 'cfn-lint -e' on the affected template where:

[trav@WS/sleet elastic-noggin-infra]$ cfn-lint -e /tmp/test.yml 
W3037 Invalid permission "ConditionCheck" for "dynamodb" found in permissions
/tmp/test.yml:194:25

W3037 Invalid service "dyanmodb" found in permissions
/tmp/test.yml:194:25

W3037 Invalid permission "ConditionCheck" for "dynamodb" found in permissions
/tmp/test.yml:252:25

W3037 Invalid service "dyanmodb" found in permissions
/tmp/test.yml:252:25

Running it across my full template set I did find one false positive from it though, it flagged 'kms:GenerateDataKey*' as an invalid permission, but according to

Wildcards are permitted in the action (with partial matches).

EDIT: It appears that I was incorrect about this one (partial wildcards), it was actually correctly flagging kms:GenerateKeyData* rather than kms:GenerateDataKey*

It also flagged dynamodb:EnclosingOperation, which appears as though it might be in the same category as ConditionCheck above.

@trav-c
Copy link
Contributor Author

trav-c commented Sep 3, 2019

Found what appears to be another false positive.

The experimental check flags:

  • waf:DeleteLoggingConfiguration
  • waf:PutLoggingConfiguration

As invalid.

These are not listed on the IAM permissions reference page for the WAF service

But do appear to be valid and recognised by IAM in the web console.

@benbridts
Copy link
Contributor

I once did a full review of all permissions found in the policy generator and once that are used in managed policies. Even those two aren't in sync (there are permissions in the managed policies that are not in the generator). Having it as a warning seems the right error level for that

@0xdabbad00
Copy link

0xdabbad00 commented Sep 3, 2019

Just want to give a head's up that I've been working on my own IAM linter that will be open-sourced (BSD-3) in the coming weeks that you'll be able to use for this. It is being released as a library for use with other tools (specifically for my other tool CloudMapper). I think mine is a little more thorough. For example, I scrape the web docs in order to identify the relevant resources at a per-action level, whereas it looks like your dataset will only get fine-grained to the per-service level.

I've reported in a lot of issues found by mine to AWS security who is fixing the docs and some other things. Some of these are issues similar to what you're experiencing with the false positives noted above about valid privileges being reported as invalid. My understanding is AWS is working to improve the docs to account for these. I'll comment here once the project is released.

@kddejong
Copy link
Contributor

kddejong commented Sep 4, 2019

@trav-c I did update the policy document and its released in v0.24.0.

cfn-lint --update-iam to manually do that updating.

@kddejong kddejong added the enhancement New feature or request label Oct 7, 2019
@0xdabbad00
Copy link

The IAM linter I work on is finally out https://duo.com/blog/an-aws-iam-policy-linter-parliament

@kddejong
Copy link
Contributor

Hoping I can find some time in the next day or two to check this out and see how we can integrate it. Great work @0xdabbad00 and team.

@johnk-novu
Copy link

Hoping I can find some time in the next day or two to check this out and see how we can integrate it. Great work @0xdabbad00 and team.

have you had a chance to look at this? we would love to have cfn-lint catch IAM policy problems!

@PatMyron PatMyron changed the title [RFE] Validate IAM PolicyDocument Actions Validate IAM PolicyDocument Actions Jun 6, 2020
@PatMyron PatMyron changed the title Validate IAM PolicyDocument Actions Validate IAM Actions Sep 16, 2020
@PatMyron
Copy link
Contributor

Another Python library validating IAM policies in CloudFormation templates:
https://github.com/awslabs/aws-cloudformation-iam-policy-validator

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

Successfully merging a pull request may close this issue.

6 participants