stelligent / cfn_nag

Linting tool for CloudFormation templates
MIT License
1.25k stars 209 forks source link

Denying wildcard reports error: "should not allow * resource with PassRole action" #584

Closed denenr closed 2 years ago

denenr commented 2 years ago

Since #243, any AWS::IAM::Role, AWS::IAM::Policy, or AWS::IAM::ManagedPolicy is flagged as an error if it has statements affecting Action: iam:PassRole on Resource: *, no matter the effect of the statements. Denying can only harden security, and the error descriptions indicate that the rules were meant to report when the action is allowed ("should not allow"), so I propose to amend these rules to only consider statements with Effect: Allow.

Given the following template:

AWSTemplateFormatVersion: '2010-09-09'
Description: |
  Demonstration of false positive error: "should not allow * resource with PassRole action".
  Note that the effect of every statement is `Deny`.

Resources:
  RoleFalsePositive:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Version: '2012-10-17'
        Statement:
          - Effect: Allow
            Principal: '*'
            Action: sts:AssumeRole
      Policies:
        - PolicyName: does-not-matter
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Effect: Deny
                Action: '*'
                Resource: '*'

  PolicyFalsePositive:
    Type: AWS::IAM::Policy
    Properties:
      Roles: []
      PolicyName: does-not-matter
      PolicyDocument:
        Version: '2012-10-17'
        Statement:
          - Effect: Deny
            Action: '*'
            Resource: '*'

  ManagedPolicyFalsePositive:
    Type: AWS::IAM::ManagedPolicy
    Properties:
      PolicyDocument:
        Version: '2012-10-17'
        Statement:
          - Effect: Deny
            Action: '*'
            Resource: '*'

cfn_nag reports the following:

------------------------------------------------------------
.\template.false-positive-allow-passrole-wildcard.yaml
------------------------------------------------------------------------------------------------------------------------
| FAIL F40
|
| Resource: ["ManagedPolicyFalsePositive"]
| Line Numbers: [38]
|
| IAM managed policy should not allow a * resource with PassRole action
------------------------------------------------------------
| FAIL F39
|
| Resource: ["PolicyFalsePositive"]
| Line Numbers: [26]
|
| IAM policy should not allow * resource with PassRole action
------------------------------------------------------------
| FAIL F38
|
| Resource: ["RoleFalsePositive"]
| Line Numbers: [8]
|
| IAM role should not allow * resource with PassRole action on its permissions policy

Failures count: 3
Warnings count: 0

Affected lines of code:

Those lines are identical (not considering indentation). Prefixing them with statement.effect == 'Allow' && would be enough:

# original
passrole_action?(statement) && wildcard_resource?(statement)

# amended
statement.effect == 'Allow' && passrole_action?(statement) && wildcard_resource?(statement)
arothian commented 2 years ago

@denenr , thanks for reporting and including these details. I agree with the evaluation that the PassRole related rules should not trigger for Deny statements and we'll get this updated!