Update PassRole rules to not trigger on deny statements. Fixes #584 (#…

…586)
stelligent · Jan 3, 2022 · 0f8532f · 0f8532f
1 parent 0de7906
commit 0f8532f
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 12 deletions.
diff --git a/lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb b/lib/cfn-nag/custom_rules/IamRolePassRoleWildcardResourceRule.rb
@@ -23,7 +23,7 @@ def audit_impl(cfn_model)
     violating_roles = cfn_model.resources_by_type('AWS::IAM::Role').select do |role|
       violating_policies = role.policy_objects.select do |policy|
         violating_statements = policy.policy_document.statements.select do |statement|
-          passrole_action?(statement) && wildcard_resource?(statement)
+          statement.effect == 'Allow' && passrole_action?(statement) && wildcard_resource?(statement)
         end
         !violating_statements.empty?
       end

diff --git a/lib/cfn-nag/custom_rules/passrole_base_rule.rb b/lib/cfn-nag/custom_rules/passrole_base_rule.rb
@@ -16,7 +16,7 @@ def audit_impl(cfn_model)
 
     violating_policies = policies.select do |policy|
       violating_statements = policy.policy_document.statements.select do |statement|
-        passrole_action?(statement) && wildcard_resource?(statement)
+        statement.effect == 'Allow' && passrole_action?(statement) && wildcard_resource?(statement)
       end
       !violating_statements.empty?
     end

diff --git a/spec/custom_rules/SPCMRule_spec.rb b/spec/custom_rules/SPCMRule_spec.rb
@@ -38,7 +38,7 @@
       rule = SPCMRule.new
       rule.spcm_threshold = 1
       actual_logical_resource_ids = rule.audit_impl cfn_model
-      expected_logical_resource_ids = %w[InlinePolicyPass]
+      expected_logical_resource_ids = %w[InlinePolicyPass InlinePolicyDenyPass]
 
       expect(actual_logical_resource_ids).to eq expected_logical_resource_ids
     end

diff --git a/...yaml/iam_passrole_resource_wildcard/iam_inline_policy_passrole_resource_wildcard_pass.yml b/...yaml/iam_passrole_resource_wildcard/iam_inline_policy_passrole_resource_wildcard_pass.yml
@@ -3,7 +3,7 @@ Resources:
 
   GenericGroup:
     Type: AWS::IAM::Group
-    Properties: 
+    Properties:
       GroupName: GenericGroup
 
   InlinePolicyPass:
@@ -15,7 +15,7 @@ Resources:
         Statement:
           -
             Effect: "Allow"
-            Action: 
+            Action:
               - "s3:ListBucket"
               - "s3:GetBucketLocation"
             Resource: "arn:aws:s3:::*"
@@ -29,5 +29,31 @@ Resources:
             Effect: Allow
             Action: "iam:PassRole"
             Resource: "arn:aws:s3:::*"
+      Groups:
+        - !Ref GenericGroup
+
+  InlinePolicyDenyPass:
+    Type: "AWS::IAM::Policy"
+    Properties:
+      PolicyName: WildcardDenyResourcePolicy
+      PolicyDocument:
+        Version: "2012-10-17"
+        Statement:
+          -
+            Effect: "Allow"
+            Action:
+              - "s3:ListBucket"
+              - "s3:GetBucketLocation"
+            Resource: "arn:aws:s3:::*"
+          -
+            Effect: Allow
+            Action:
+              - "s3:ListBucket"
+              - "s3:GetBucketLocation"
+            Resource: "*"
+          -
+            Effect: Deny
+            Action: "iam:PassRole"
+            Resource: "*"
       Groups:
         - !Ref GenericGroup
diff --git a/...aml/iam_passrole_resource_wildcard/iam_managed_policy_passrole_resource_wildcard_pass.yml b/...aml/iam_passrole_resource_wildcard/iam_managed_policy_passrole_resource_wildcard_pass.yml
@@ -3,7 +3,7 @@ Resources:
 
   GenericGroup:
     Type: AWS::IAM::Group
-    Properties: 
+    Properties:
       GroupName: GenericGroup
 
   ManagedPolicyPass1:
@@ -14,7 +14,7 @@ Resources:
         Statement:
           -
             Effect: "Allow"
-            Action: 
+            Action:
               - "s3:ListBucket"
               - "s3:GetBucketLocation"
             Resource: "arn:aws:s3:::*"
@@ -31,7 +31,7 @@ Resources:
               - "s3:ListBucket"
               - "s3:GetBucketLocation"
             Resource: "*"
-            
+
   ManagedPolicyPass3:
     Type: "AWS::IAM::ManagedPolicy"
     Properties:
@@ -43,4 +43,15 @@ Resources:
             Action: "iam:PassRole"
             Resource: "arn:aws:s3:::*"
       Groups:
-        - !Ref GenericGroup
+        - !Ref GenericGroup
+
+  ManagedPolicyPass4:
+    Type: "AWS::IAM::ManagedPolicy"
+    Properties:
+      PolicyDocument:
+        Version: "2012-10-17"
+        Statement:
+          -
+            Effect: Deny
+            Action: "iam:PassRole"
+            Resource: "*"
diff --git a/...emplates/yaml/iam_passrole_resource_wildcard/iam_role_passrole_resource_wildcard_pass.yml b/...emplates/yaml/iam_passrole_resource_wildcard/iam_role_passrole_resource_wildcard_pass.yml
@@ -1,8 +1,30 @@
 ---
 Resources:
-
+  RoleDeny:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument:
+        Version: "2012-10-17"
+        Statement:
+          -
+            Effect: Allow
+            Principal:
+              Service:
+                - cloudformation.amazonaws.com
+            Action:
+              - sts:AssumeRole
+      Policies:
+        -
+          PolicyName: PolicyDeny
+          PolicyDocument:
+            Version: "2012-10-17"
+            Statement:
+              -
+                Effect: Deny
+                Action: "iam:PassRole"
+                Resource: "*"
   RoleFail:
-    Type: AWS::IAM::Role 
+    Type: AWS::IAM::Role
     Properties:
       AssumeRolePolicyDocument:
         Version: "2012-10-17"
@@ -22,7 +44,7 @@ Resources:
             Statement:
               -
                 Effect: "Allow"
-                Action: 
+                Action:
                   - "s3:ListBucket"
                   - "s3:GetBucketLocation"
                 Resource: "arn:aws:s3:::*"