Add resource type to failures (#213)

* add resource type to failure * updated new version and CHANGELOG.md * updated rules for resource type Co-authored-by: Ramon <ramon.guimera@skyscanner.net>
Skyscanner · Mar 23, 2022 · c7c7693 · c7c7693
1 parent 6c8de5a
commit c7c7693
Show file tree

Hide file tree

Showing 59 changed files with 305 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,10 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [1.7.0]
+### Updates
+- Added `resource_types` to failures.
+
 ## [1.6.0]
 ### Updates
 - Created `GenericResourceWildcardPrincipalRule` to be an abstract for wildcard principals for Generic resources.

diff --git a/cfripper/__version__.py b/cfripper/__version__.py
@@ -1,3 +1,3 @@
-VERSION = (1, 6, 0)
+VERSION = (1, 7, 0)
 
 __version__ = ".".join(map(str, VERSION))
diff --git a/cfripper/model/result.py b/cfripper/model/result.py
@@ -13,6 +13,7 @@ class Failure(BaseModel):
     rule_mode: RuleMode
     actions: Optional[set] = set()
     resource_ids: Optional[set] = set()
+    resource_types: Optional[set] = set()
 
     class Config(BaseModel.Config):
         extra = Extra.forbid
@@ -24,6 +25,7 @@ def serializable(self):
             "rule_mode": self.rule_mode,
             "risk_value": self.risk_value,
             "resource_ids": sorted(self.resource_ids or []),
+            "resource_types": sorted(self.resource_types or []),
             "actions": sorted(self.actions or []),
             "granularity": self.granularity,
         }
@@ -61,6 +63,7 @@ def add_failure(
         risk_value: RuleRisk,
         granularity: RuleGranularity,
         resource_ids=None,
+        resource_types=None,
         actions=None,
     ):
         self.failures.append(
@@ -70,6 +73,7 @@ def add_failure(
                 rule_mode=rule_mode,
                 risk_value=risk_value,
                 resource_ids=resource_ids,
+                resource_types=resource_types,
                 actions=actions,
                 granularity=granularity,
             )

diff --git a/cfripper/rules/base_rules.py b/cfripper/rules/base_rules.py
@@ -48,6 +48,7 @@ def add_failure_to_result(
         reason: str,
         granularity: Optional[RuleGranularity] = None,
         resource_ids: Optional[Set] = None,
+        resource_types: Optional[Set] = None,
         actions: Optional[Set] = None,
         risk_value: Optional[RuleRisk] = None,
         rule_mode: Optional[RuleMode] = None,
@@ -57,13 +58,13 @@ def add_failure_to_result(
         risk_value = risk_value or self.risk_value
         granularity = granularity or self.GRANULARITY
 
-        for fltr in self.rule_filters:
+        for rule_filter in self.rule_filters:
             try:
-                if fltr(**context):
-                    risk_value = fltr.risk_value or risk_value
-                    rule_mode = fltr.rule_mode or rule_mode
+                if rule_filter(**context):
+                    risk_value = rule_filter.risk_value or risk_value
+                    rule_mode = rule_filter.rule_mode or rule_mode
             except Exception:
-                logger.exception(f"Exception raised while evaluating filter for `{fltr.reason}`", extra=context)
+                logger.exception(f"Exception raised while evaluating filter for `{rule_filter.reason}`", extra=context)
 
         if rule_mode != RuleMode.ALLOWED:
             result.add_failure(
@@ -72,6 +73,7 @@ def add_failure_to_result(
                 rule_mode=rule_mode,
                 risk_value=risk_value,
                 resource_ids=resource_ids,
+                resource_types=resource_types,
                 actions=actions,
                 granularity=granularity,
             )
@@ -172,7 +174,9 @@ class BaseDangerousPolicyActions(ResourceSpecificRule, ABC):
     @classmethod
     @abstractmethod
     def DANGEROUS_ACTIONS(cls) -> List[str]:
-        # This is designed to be overwritten as a class variable
+        """
+        This is designed to be overwritten as a class variable
+        """
         raise NotImplementedError
 
     def resource_invoke(self, resource: Resource, logical_id: str, extras: Optional[Dict] = None) -> Result:
@@ -185,6 +189,7 @@ def resource_invoke(self, resource: Resource, logical_id: str, extras: Optional[
                     result,
                     self.REASON.format(logical_id, sorted(dangerous_actions)),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     actions=dangerous_actions,
                     context={
                         "config": self._config,

diff --git a/cfripper/rules/cloudformation_authentication.py b/cfripper/rules/cloudformation_authentication.py
@@ -59,5 +59,6 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                     self.REASON.format(logical_id),
                     resource_ids={logical_id},
                     context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
+                    resource_types={resource.Type},
                 )
         return result
diff --git a/cfripper/rules/cross_account_trust.py b/cfripper/rules/cross_account_trust.py
@@ -62,11 +62,11 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                             "resource": resource,
                             "statement": statement,
                         }
-                        self._do_statement_check(result, logical_id, statement, filters_available_context)
+                        self._do_statement_check(result, logical_id, statement, filters_available_context, resource)
         return result
 
     def _do_statement_check(
-        self, result: Result, logical_id: str, statement: Statement, filters_available_context: Dict
+        self, result: Result, logical_id: str, statement: Statement, filters_available_context: Dict, resource: Resource
     ):
         if statement.Effect == "Allow":
             for principal in statement.get_principal_list():
@@ -96,13 +96,15 @@ def _do_statement_check(
                             rule_mode=RuleMode.DEBUG,
                             resource_ids={logical_id},
                             context=filters_available_context,
+                            resource_types={resource.Type},
                         )
                     else:
                         self.add_failure_to_result(
                             result,
                             self.REASON.format(logical_id, principal),
                             resource_ids={logical_id},
                             context=filters_available_context,
+                            resource_types={resource.Type},
                         )
 
 
@@ -147,7 +149,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                             "resource": resource,
                             "statement": statement,
                         }
-                        self._do_statement_check(result, logical_id, statement, filters_available_context)
+                        self._do_statement_check(result, logical_id, statement, filters_available_context, resource)
 
         return result
 

diff --git a/cfripper/rules/ebs_volume_has_sse.py b/cfripper/rules/ebs_volume_has_sse.py
@@ -56,6 +56,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                         result,
                         self.REASON.format(logical_id),
                         resource_ids={logical_id},
+                        resource_types={resource.Type},
                         context={
                             "config": self._config,
                             "extras": extras,

diff --git a/cfripper/rules/ec2_security_group.py b/cfripper/rules/ec2_security_group.py
@@ -34,6 +34,7 @@ def analyse_ingress(
         logical_id: str,
         ingress: Union[SecurityGroupIngressProp, SecurityGroupIngressProperties],
         filters_available_context: Dict,
+        resource_type: str,
     ):
         if self.non_compliant_ip_range(ingress=ingress):
             open_ports = list(range(ingress.FromPort, ingress.ToPort + 1))
@@ -50,6 +51,7 @@ def analyse_ingress(
                     result,
                     self.REASON.format(self.get_open_ports_wording(non_allowed_open_ports), ip_range, logical_id,),
                     resource_ids={logical_id},
+                    resource_types={resource_type},
                     context=filters_available_context,
                 )
 
@@ -158,7 +160,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                     "logical_id": logical_id,
                     "resource": resource,
                 }
-                self.analyse_ingress(result, logical_id, ingress, filters_available_context)
+                self.analyse_ingress(result, logical_id, ingress, filters_available_context, resource.Type)
         return result
 
 
@@ -248,7 +250,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                 "logical_id": logical_id,
                 "resource": resource,
             }
-            self.analyse_ingress(result, logical_id, resource.Properties, filters_available_context)
+            self.analyse_ingress(result, logical_id, resource.Properties, filters_available_context, resource.Type)
         return result
 
 
@@ -324,6 +326,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                     result,
                     self.REASON.format(logical_id),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
                 )
         return result
diff --git a/cfripper/rules/hardcoded_RDS_password.py b/cfripper/rules/hardcoded_RDS_password.py
@@ -97,6 +97,7 @@ def _failure_added(
                 self.REASON_DEFAULT.format(resource_type, logical_id),
                 resource_ids={logical_id},
                 context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
+                resource_types={resource.Type},
             )
             return True
         elif master_user_password not in (Parameter.NO_ECHO_NO_DEFAULT, Parameter.NO_ECHO_WITH_VALUE):
@@ -105,6 +106,7 @@ def _failure_added(
                 self.REASON_MISSING_NOECHO.format(resource_type, logical_id),
                 resource_ids={logical_id},
                 context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
+                resource_types={resource.Type},
             )
             return True
 

diff --git a/cfripper/rules/iam_roles.py b/cfripper/rules/iam_roles.py
@@ -48,6 +48,7 @@ def check_managed_policies(self, result: Result, logical_id: str, role: IAMRole,
                     result,
                     f"Role {logical_id} has forbidden Managed Policy {managed_policy_arn}",
                     resource_ids={logical_id},
+                    resource_types={role.Type},
                     context={
                         "config": self._config,
                         "extras": extras,
@@ -74,6 +75,7 @@ def check_inline_policies(self, result: Result, logical_id: str, role: IAMRole,
                                     f"Role '{logical_id}' contains an insecure permission '{action}' in policy "
                                     f"'{policy.PolicyName}'",
                                     resource_ids={logical_id},
+                                    resource_types={role.Type},
                                     context={
                                         "config": self._config,
                                         "extras": extras,
@@ -112,6 +114,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                         result,
                         self.REASON.format(logical_id, "AssumeRolePolicy"),
                         resource_ids={logical_id},
+                        resource_types={resource.Type},
                         context={
                             "config": self._config,
                             "extras": extras,
@@ -128,6 +131,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                                 result,
                                 self.REASON.format(logical_id, f"{policy.PolicyName} policy"),
                                 resource_ids={logical_id},
+                                resource_types={resource.Type},
                                 context={
                                     "config": self._config,
                                     "extras": extras,
@@ -144,6 +148,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                     result,
                     self.REASON.format(logical_id, "AWS::IAM::ManagedPolicy"),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
                 )
         return result
diff --git a/cfripper/rules/kms_key_rotation_enabled.py b/cfripper/rules/kms_key_rotation_enabled.py
@@ -41,6 +41,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                         result,
                         self.REASON.format(logical_id),
                         resource_ids={logical_id},
+                        resource_types={resource.Type},
                         context={
                             "config": self._config,
                             "extras": extras,

diff --git a/cfripper/rules/managed_policy_on_user.py b/cfripper/rules/managed_policy_on_user.py
@@ -81,6 +81,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                     result,
                     self.REASON.format(logical_id),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
                 )
         return result
diff --git a/cfripper/rules/policy_on_user.py b/cfripper/rules/policy_on_user.py
@@ -81,6 +81,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                     result,
                     self.REASON.format(logical_id),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
                 )
         return result
diff --git a/cfripper/rules/s3_bucket_policy.py b/cfripper/rules/s3_bucket_policy.py
@@ -58,6 +58,7 @@ def resource_invoke(self, resource: S3BucketPolicy, logical_id: str, extras: Opt
                             result,
                             self.REASON.format(logical_id, account_id),
                             resource_ids={logical_id},
+                            resource_types={resource.Type},
                             context={
                                 "config": self._config,
                                 "extras": extras,

diff --git a/cfripper/rules/s3_lifecycle_configuration.py b/cfripper/rules/s3_lifecycle_configuration.py
@@ -57,6 +57,7 @@ def resource_invoke(self, resource: S3Bucket, logical_id: str, extras: Optional[
                 result,
                 self.REASON.format(logical_id),
                 resource_ids={logical_id},
+                resource_types={resource.Type},
                 context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
             )
         return result
diff --git a/cfripper/rules/s3_object_versioning.py b/cfripper/rules/s3_object_versioning.py
@@ -57,6 +57,7 @@ def resource_invoke(self, resource: S3Bucket, logical_id: str, extras: Optional[
                 result,
                 self.REASON.format(logical_id),
                 resource_ids={logical_id},
+                resource_types={resource.Type},
                 context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
             )
         return result
diff --git a/cfripper/rules/s3_public_access.py b/cfripper/rules/s3_public_access.py
@@ -56,6 +56,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                         result,
                         self.REASON.format(logical_id),
                         resource_ids={logical_id},
+                        resource_types={resource.Type},
                         context={
                             "config": self._config,
                             "extras": extras,
@@ -99,6 +100,7 @@ def resource_invoke(self, resource: S3Bucket, logical_id: str, extras: Optional[
                 result,
                 self.REASON.format(logical_id),
                 resource_ids={logical_id},
+                resource_types={resource.Type},
                 context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
             )
         return result
@@ -136,6 +138,7 @@ def resource_invoke(self, resource: S3Bucket, logical_id: str, extras: Optional[
                 result,
                 self.REASON.format(logical_id),
                 resource_ids={logical_id},
+                resource_types={resource.Type},
                 context={"config": self._config, "extras": extras, "logical_id": logical_id, "resource": resource},
             )
         return result
diff --git a/cfripper/rules/sns_topic_policy.py b/cfripper/rules/sns_topic_policy.py
@@ -40,6 +40,7 @@ def resource_invoke(self, resource: SNSTopicPolicy, logical_id: str, extras: Opt
                     result,
                     self.REASON.format(logical_id),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     context={
                         "config": self._config,
                         "extras": extras,

diff --git a/cfripper/rules/sqs_queue_policy.py b/cfripper/rules/sqs_queue_policy.py
@@ -44,6 +44,7 @@ def resource_invoke(self, resource: SQSQueuePolicy, logical_id: str, extras: Opt
                     result,
                     self.REASON.format(logical_id),
                     resource_ids={logical_id},
+                    resource_types={resource.Type},
                     context={
                         "config": self._config,
                         "extras": extras,
@@ -90,6 +91,7 @@ def resource_invoke(self, resource: SQSQueuePolicy, logical_id: str, extras: Opt
                             result,
                             self.REASON.format(logical_id),
                             resource_ids={logical_id},
+                            resource_types={resource.Type},
                             context={
                                 "config": self._config,
                                 "extras": extras,

diff --git a/cfripper/rules/wildcard_policies.py b/cfripper/rules/wildcard_policies.py
@@ -54,6 +54,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                             "logical_id": logical_id,
                             "resource": resource,
                         },
+                        resource_types={resource.Type},
                     )
         return result
 
@@ -83,6 +84,7 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
                                 "logical_id": logical_id,
                                 "resource": resource,
                             },
+                            resource_types={resource.Type},
                         )
         return result