Merge pull request #151 from Skyscanner/blocked-wildcard-resource-res…

…ource-exemptions

Update Wildcard Resource rule to allow for certain resources to be excluded

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [0.23.3] - 2021-02-11
+### Improvements
+- Update `WildcardResourceRule` to allow for certain resources to be excluded.
+
 ## [0.23.2] - 2021-02-04
 ### Bugfix
 - `GenericWildcardPrincipalRule` to ignore account IDs where full or partial wildcard is required in the Principal.

cfripper/__version__.py

@@ -1,3 +1,3 @@
-VERSION = (0, 23, 2)
+VERSION = (0, 23, 3)
 
 __version__ = ".".join(map(str, VERSION))

cfripper/rules/wildcard_resource_rule.py

@@ -4,23 +4,23 @@
 import logging
 from typing import Dict, Optional
 
-from pycfmodel.model.cf_model import CFModel
 from pycfmodel.model.resources.generic_resource import GenericResource
 from pycfmodel.model.resources.iam_role import IAMRole
 from pycfmodel.model.resources.kms_key import KMSKey
 from pycfmodel.model.resources.properties.policy_document import PolicyDocument
 from pycfmodel.model.resources.properties.statement import Statement
+from pycfmodel.model.resources.resource import Resource
 
 from cfripper.cloudformation_actions_only_accepts_wildcard import CLOUDFORMATION_ACTIONS_ONLY_ACCEPTS_WILDCARD
 from cfripper.config.regex import REGEX_IS_STAR
 from cfripper.model.enums import RuleGranularity
 from cfripper.model.result import Result
-from cfripper.rules.base_rules import Rule
+from cfripper.rules.base_rules import ResourceSpecificRule
 
 logger = logging.getLogger(__file__)
 
 
-class WildcardResourceRule(Rule):
+class WildcardResourceRule(ResourceSpecificRule):
     """
     Generic rule that detects actions that accept a resource and are using a wildcard.
 
@@ -41,27 +41,27 @@ class WildcardResourceRule(Rule):
         |`action`      | `Optional[str]`  | Action that has a wildcard resource. If None, means all actions |
     """
 
+    RESOURCE_TYPES = (Resource,)
     REASON_WITH_POLICY_NAME = '"{}" is using a wildcard resource in "{}" for "{}"'
     REASON_WITHOUT_POLICY_NAME = '"{}" is using a wildcard resource for "{}"'
     REASON_ALL_ACTIONS_WITH_POLICY_NAME = '"{}" is using a wildcard resource in "{}" allowing all actions'
     REASON_ALL_ACTIONS_WITHOUT_POLICY_NAME = '"{}" is using a wildcard resource allowing all actions'
 
-    def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
+    def resource_invoke(self, resource: Resource, logical_id: str, extras: Optional[Dict] = None) -> Result:
         result = Result()
-        for logical_id, resource in cfmodel.Resources.items():
-            for policy in resource.policy_documents:
-                self._check_policy_document(result, logical_id, policy.policy_document, policy.name, extras)
-            if isinstance(resource, IAMRole):
-                self._check_policy_document(
-                    result, logical_id, resource.Properties.AssumeRolePolicyDocument, None, extras
-                )
-            elif isinstance(resource, KMSKey):
-                self._check_policy_document(result, logical_id, resource.Properties.KeyPolicy, None, extras)
-            elif isinstance(resource, GenericResource):
-                if hasattr(resource, "Properties"):
-                    policy_document = resource.Properties.get("PolicyDocument")
-                    if policy_document:
-                        self._check_policy_document(result, logical_id, PolicyDocument(**policy_document), None, extras)
+
+        for policy in resource.policy_documents:
+            self._check_policy_document(result, logical_id, policy.policy_document, policy.name, extras)
+        if isinstance(resource, IAMRole):
+            self._check_policy_document(result, logical_id, resource.Properties.AssumeRolePolicyDocument, None, extras)
+        elif isinstance(resource, KMSKey):
+            self._check_policy_document(result, logical_id, resource.Properties.KeyPolicy, None, extras)
+        elif isinstance(resource, GenericResource):
+            if hasattr(resource, "Properties"):
+                policy_document = resource.Properties.get("PolicyDocument")
+                if policy_document:
+                    self._check_policy_document(result, logical_id, PolicyDocument(**policy_document), None, extras)
+
         return result
 
     def _check_policy_document(

tests/rules/test_WildcardResourceRule.py

@@ -1,4 +1,5 @@
 import pytest
+from pycfmodel.model.resources.iam_policy import IAMPolicy
 
 from cfripper.model.result import Failure
 from cfripper.rules.wildcard_resource_rule import WildcardResourceRule
@@ -94,6 +95,19 @@ def test_kms_key_with_wildcard_resource_not_whitelisted_is_not_flagged(kms_key_w
     assert result.failed_monitored_rules == []
 
 
+def test_exclude_certain_resources_on_rule(iam_policy_with_wildcard_resource_and_wildcard_action):
+    # Any subclass of this rule may want to exclude certain resource types. As a test, let's exclude IAM Policies.
+    rule = WildcardResourceRule(None)
+    rule._config.stack_name = "stack3"
+    rule.all_cf_actions = set()
+    rule.EXCLUDED_RESOURCE_TYPES = (IAMPolicy,)
+    result = rule.invoke(iam_policy_with_wildcard_resource_and_wildcard_action)
+
+    assert result.valid
+    assert result.failed_rules == []
+    assert result.failed_monitored_rules == []
+
+
 def test_policy_document_with_wildcard_resource_is_detected(iam_policy_with_wildcard_resource_and_wildcard_action):
     rule = WildcardResourceRule(None)
     rule._config.stack_name = "stack3"