stelligent / cfn_nag

Linting tool for CloudFormation templates
MIT License
1.26k stars 212 forks source link

W12 doesn't make sense #490

Open Slooz opened 4 years ago

Slooz commented 4 years ago

W12 is issued for the following template:

Resources:
  Policy:
    Type: AWS::IAM::Policy
    Properties:
      PolicyName: foo
      PolicyDocument:
        Statement:
          - Effect: Allow
            Action: cloudwatch:GetAuthorizationToken
            Resource: "*"
| WARN W12
|
| Resources: ["Policy"]
| Line Numbers: [3]
|
| IAM policy should not allow * resource

But the only valid value for the Resource property of the cloudwatch:GetAuthorizationToken action is *. See Actions, resources, and condition keys for Amazon Elastic Container Registry - AWS Identity and Access Management. This warning doesn't make sense.

arothian commented 4 years ago

Hi @Slooz , thanks for reporting. I think the correct action in this case would be ecr:GetAuthorizationToken

For W12, it current doesn't do any filtering based on the actions presented. Indeed, per the documentation this is probably fine for this use case. You can ignore this warning for this particular policy by adding metadata to the resource, take a look at the example here: https://github.com/stelligent/cfn_nag#per-resource-rule-suppression

Slooz commented 4 years ago

Whoops, sorry, you're right, it should be ecr.

You're right, but these warnings would be more valuable if the scanning was more intelligent. Having a blanket "*" == bad doesn't seem like the best logic.

Feel free to close if you think this would be out of scope, though.

guikcd commented 3 years ago

Consider the following policy document:

{
  "AWSTemplateFormatVersion" : "2010-09-09",
  "Description": "secretsmanager test",
  "Resources": {
    "policy": {
      "Type": "AWS::IAM::Policy",
      "Properties": {
          "PolicyName": "mypolicy",
          "PolicyDocument": {
            "Version": "2012-10-17",
            "Statement": [
              {
                "Sid": "VisualEditor0",
                "Effect": "Allow",
                "Action": [
                  "secretsmanager:GetResourcePolicy",
                  "secretsmanager:GetSecretValue",
                  "secretsmanager:DescribeSecret",
                  "secretsmanager:ListSecretVersionIds"
                ],
                "Resource": [
                  "arn:aws:secretsmanager:us-east-2:[your-account-number]:secret:mysecret-XXXX"
                ]
              },
              {
                "Sid": "VisualEditor1",
                "Effect": "Allow",
                "Action": [
                  "secretsmanager:GetRandomPassword",
                  "secretsmanager:ListSecrets"
                ],
                "Resource": "*"
              }
            ]
          }
        }
    }
  }
}

This generate a warning:

| WARN W12
|
| Resources: ["policy"]
| Line Numbers: [6]
|
| IAM policy should not allow * resource

You want to allow both listing secrets and getting random password for all secrets (to discover the secret id for example) BUT give only access to a specific secret only. You have to whitelist the entire policy document:

{
  "AWSTemplateFormatVersion" : "2010-09-09",
  "Description": "secretsmanager test",
  "Resources": {
    "policy": {
      "Type": "AWS::IAM::Policy",
      "Metadata": {
        "cfn_nag": {
          "rules_to_suppress": [{
            "id": "W12",
            "reason": "ListSecrets does not take a resource, but *"
          }]
        }
      },
      "Properties": {
          "PolicyName": "mypolicy",
          "PolicyDocument": {
            "Version": "2012-10-17",
            "Statement": [
              {
                "Sid": "VisualEditor0",
                "Effect": "Allow",
                "Action": [
                  "secretsmanager:GetResourcePolicy",
                  "secretsmanager:GetSecretValue",
                  "secretsmanager:DescribeSecret",
                  "secretsmanager:ListSecretVersionIds"
                ],
                "Resource": [
                  "arn:aws:secretsmanager:us-east-2:[your-account-number]:secret:mysecret-XXXX"
                ]
              },
              {
                "Sid": "VisualEditor1",
                "Effect": "Allow",
                "Action": [
                  "secretsmanager:GetRandomPassword",
                  "secretsmanager:ListSecrets"
                ],
                "Resource": "*"
              }
            ]
          }
        }
    }
  }
}

This can led to hide future mistakes in the first section of the policy or any other statement added later (I'm ok with the idea that it is recommended to add another policy for another need).

Building a list a Actions that need specific Resources would be great, but is maybe challenging (is there any list outside the documentation itself?).