Improve kms handling and rule granularity (#89)

CHANGELOG.md

@@ -1,6 +1,14 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [0.12.0] - 2019-12-17
+### Added
+- `KMSKeyCrossAccountTrustRule`
+### Changed
+- `GenericWildcardPrincipalRule`, `PartialWildcardPrincipalRule`, `FullWildcardPrincipalRule` no longer check for
+wildcards in KMSKey principals.
+- Improved granularity of most rules 
+
 ## [0.11.3] - 2019-12-17
 ### Improvements
 - `S3CrossAccountTrustRule` now accepts resource level exceptions

cfripper/rules/__init__.py

@@ -12,41 +12,80 @@
 CONDITIONS OF ANY KIND, either express or implied. See the License for the
 specific language governing permissions and limitations under the License.
 """
-from cfripper.rules.cloudformation_authentication import *  # noqa: F403
-from cfripper.rules.cross_account_trust import *  # noqa: F403
-from cfripper.rules.ebs_volume_has_sse import *  # noqa: F403
-from cfripper.rules.hardcoded_RDS_password import *  # noqa: F403
-from cfripper.rules.iam_managed_policy_wildcard_action import *  # noqa: F403
-from cfripper.rules.iam_roles import *  # noqa: F403
-from cfripper.rules.kms_key_wildcard_principal import *  # noqa: F403
-from cfripper.rules.managed_policy_on_user import *  # noqa: F403
-from cfripper.rules.policy_on_user import *  # noqa: F403
-from cfripper.rules.privilege_escalation import *  # noqa: F403
-from cfripper.rules.s3_bucket_policy import *  # noqa: F403
-from cfripper.rules.s3_public_access import *  # noqa: F403
-from cfripper.rules.security_group import *  # noqa: F403
-from cfripper.rules.sns_topic_policy_not_principal import *  # noqa: F403
-from cfripper.rules.sqs_queue_policy import *  # noqa: F403
-from cfripper.rules.wildcard_principals import *  # noqa: F403
+from cfripper.rules.base_rules import PrincipalCheckingRule
+from cfripper.rules.cloudformation_authentication import CloudFormationAuthenticationRule
+from cfripper.rules.cross_account_trust import (
+    CrossAccountCheckingRule,
+    CrossAccountTrustRule,
+    KMSKeyCrossAccountTrustRule,
+    S3CrossAccountTrustRule,
+)
+from cfripper.rules.ebs_volume_has_sse import EBSVolumeHasSSERule
+from cfripper.rules.hardcoded_RDS_password import HardcodedRDSPasswordRule
+from cfripper.rules.iam_managed_policy_wildcard_action import IAMManagedPolicyWildcardActionRule
+from cfripper.rules.iam_roles import (
+    IAMRolesOverprivilegedRule,
+    IAMRoleWildcardActionOnPermissionsPolicyRule,
+    IAMRoleWildcardActionOnTrustPolicyRule,
+)
+from cfripper.rules.kms_key_wildcard_principal import KMSKeyWildcardPrincipal
+from cfripper.rules.managed_policy_on_user import ManagedPolicyOnUserRule
+from cfripper.rules.policy_on_user import PolicyOnUserRule
+from cfripper.rules.privilege_escalation import PrivilegeEscalationRule
+from cfripper.rules.s3_bucket_policy import S3BucketPolicyPrincipalRule, S3BucketPolicyWildcardActionRule
+from cfripper.rules.s3_public_access import S3BucketPublicReadAclAndListStatementRule, S3BucketPublicReadWriteAclRule
+from cfripper.rules.security_group import (
+    SecurityGroupIngressOpenToWorld,
+    SecurityGroupMissingEgressRule,
+    SecurityGroupOpenToWorldRule,
+)
+from cfripper.rules.sns_topic_policy_not_principal import SNSTopicPolicyNotPrincipalRule
+from cfripper.rules.sqs_queue_policy import (
+    SQSQueuePolicyNotPrincipalRule,
+    SQSQueuePolicyPublicRule,
+    SQSQueuePolicyWildcardActionRule,
+)
+from cfripper.rules.wildcard_principals import (
+    FullWildcardPrincipalRule,
+    GenericWildcardPrincipalRule,
+    PartialWildcardPrincipalRule,
+)
 
 DEFAULT_RULES = {
-    "IAMRolesOverprivilegedRule": IAMRolesOverprivilegedRule,  # noqa: F405
-    "SecurityGroupOpenToWorldRule": SecurityGroupOpenToWorldRule,  # noqa: F405
-    "S3BucketPublicReadWriteAclRule": S3BucketPublicReadWriteAclRule,  # noqa: F405
-    "SecurityGroupIngressOpenToWorld": SecurityGroupIngressOpenToWorld,  # noqa: F405
-    "ManagedPolicyOnUserRule": ManagedPolicyOnUserRule,  # noqa: F405
-    "PolicyOnUserRule": PolicyOnUserRule,  # noqa: F405
-    "SNSTopicPolicyNotPrincipalRule": SNSTopicPolicyNotPrincipalRule,  # noqa: F405
-    "SQSQueuePolicyNotPrincipalRule": SQSQueuePolicyNotPrincipalRule,  # noqa: F405
-    "S3BucketPolicyPrincipalRule": S3BucketPolicyPrincipalRule,  # noqa: F405
-    "EBSVolumeHasSSERule": EBSVolumeHasSSERule,  # noqa: F405
-    "PrivilegeEscalationRule": PrivilegeEscalationRule,  # noqa: F405
-    "CrossAccountTrustRule": CrossAccountTrustRule,  # noqa: F405
-    "S3BucketPublicReadAclAndListStatementRule": S3BucketPublicReadAclAndListStatementRule,  # noqa: F405
-    "SQSQueuePolicyPublicRule": SQSQueuePolicyPublicRule,  # noqa: F405
-    "S3CrossAccountTrustRule": S3CrossAccountTrustRule,  # noqa: F405
-    "HardcodedRDSPasswordRule": HardcodedRDSPasswordRule,  # noqa: F405
-    "KMSKeyWildcardPrincipal": KMSKeyWildcardPrincipal,  # noqa: F405
-    "FullWildcardPrincipal": FullWildcardPrincipalRule,  # noqa: F405
-    "PartialWildcardPrincipal": PartialWildcardPrincipalRule,  # noqa: F405
+    "CrossAccountTrustRule": CrossAccountTrustRule,
+    "EBSVolumeHasSSERule": EBSVolumeHasSSERule,
+    "FullWildcardPrincipal": FullWildcardPrincipalRule,
+    "HardcodedRDSPasswordRule": HardcodedRDSPasswordRule,
+    "IAMRolesOverprivilegedRule": IAMRolesOverprivilegedRule,
+    "KMSKeyCrossAccountTrustRule": KMSKeyCrossAccountTrustRule,
+    "KMSKeyWildcardPrincipal": KMSKeyWildcardPrincipal,
+    "ManagedPolicyOnUserRule": ManagedPolicyOnUserRule,
+    "PartialWildcardPrincipal": PartialWildcardPrincipalRule,
+    "PolicyOnUserRule": PolicyOnUserRule,
+    "PrivilegeEscalationRule": PrivilegeEscalationRule,
+    "S3BucketPolicyPrincipalRule": S3BucketPolicyPrincipalRule,
+    "S3BucketPublicReadAclAndListStatementRule": S3BucketPublicReadAclAndListStatementRule,
+    "S3BucketPublicReadWriteAclRule": S3BucketPublicReadWriteAclRule,
+    "S3CrossAccountTrustRule": S3CrossAccountTrustRule,
+    "SecurityGroupIngressOpenToWorld": SecurityGroupIngressOpenToWorld,
+    "SecurityGroupOpenToWorldRule": SecurityGroupOpenToWorldRule,
+    "SNSTopicPolicyNotPrincipalRule": SNSTopicPolicyNotPrincipalRule,
+    "SQSQueuePolicyNotPrincipalRule": SQSQueuePolicyNotPrincipalRule,
+    "SQSQueuePolicyPublicRule": SQSQueuePolicyPublicRule,
+}
+
+NON_DEFAULT_RULES = {
+    "CloudFormationAuthenticationRule": CloudFormationAuthenticationRule,
+    "GenericWildcardPrincipalRule": GenericWildcardPrincipalRule,
+    "IAMManagedPolicyWildcardActionRule": IAMManagedPolicyWildcardActionRule,
+    "IAMRoleWildcardActionOnPermissionsPolicyRule": IAMRoleWildcardActionOnPermissionsPolicyRule,
+    "IAMRoleWildcardActionOnTrustPolicyRule": IAMRoleWildcardActionOnTrustPolicyRule,
+    "S3BucketPolicyWildcardActionRule": S3BucketPolicyWildcardActionRule,
+    "SecurityGroupMissingEgressRule": SecurityGroupMissingEgressRule,
+    "SQSQueuePolicyWildcardActionRule": SQSQueuePolicyWildcardActionRule,
+}
+
+BASE_CLASSES = {
+    "CrossAccountCheckingRule": CrossAccountCheckingRule,
+    "PrincipalCheckingRule": PrincipalCheckingRule,
 }

cfripper/rules/base_rules.py

@@ -21,6 +21,8 @@
 
 
 class PrincipalCheckingRule(Rule):
+    """Abstract class for rules that check principals"""
+
     _valid_principals = None
 
     def _get_whitelist_from_config(self, services: List[str] = None) -> Set[str]:

cfripper/rules/cloudformation_authentication.py

@@ -13,15 +13,18 @@
 specific language governing permissions and limitations under the License.
 """
 __all__ = ["CloudFormationAuthenticationRule"]
+
+from cfripper.model.enums import RuleGranularity
 from cfripper.model.rule import Rule
 
 
 class CloudFormationAuthenticationRule(Rule):
     """This rule checks for hardcoded credentials"""
 
     REASON = "Hardcoded credentials in {}"
+    GRANULARITY = RuleGranularity.RESOURCE
 
     def invoke(self, cfmodel):
         for logical_id, resource in cfmodel.Resources.items():
             if resource.has_hardcoded_credentials():
-                self.add_failure(type(self).__name__, self.REASON.format(logical_id))
+                self.add_failure(type(self).__name__, self.REASON.format(logical_id), resource_ids={logical_id})

cfripper/rules/cross_account_trust.py

@@ -12,13 +12,20 @@
 CONDITIONS OF ANY KIND, either express or implied. See the License for the
 specific language governing permissions and limitations under the License.
 """
-__all__ = ["CrossAccountCheckingRule", "CrossAccountTrustRule", "S3CrossAccountTrustRule"]
+__all__ = [
+    "CrossAccountCheckingRule",
+    "CrossAccountTrustRule",
+    "KMSKeyCrossAccountTrustRule",
+    "S3CrossAccountTrustRule",
+]
 
 import logging
 import re
 from typing import Set
 
+from pycfmodel.model.cf_model import CFModel
 from pycfmodel.model.resources.iam_role import IAMRole
+from pycfmodel.model.resources.kms_key import KMSKey
 from pycfmodel.model.resources.s3_bucket_policy import S3BucketPolicy
 
 from cfripper.config.regex import REGEX_CROSS_ACCOUNT_ROOT
@@ -108,3 +115,17 @@ def invoke(self, cfmodel):
             if isinstance(resource, S3BucketPolicy):
                 for statement in resource.Properties.PolicyDocument._statement_as_list():
                     self._do_statement_check(logical_id, statement)
+
+
+class KMSKeyCrossAccountTrustRule(CrossAccountCheckingRule):
+    """
+    This rule checks for KMS keys that allow cross-account principals to get access to the key
+    """
+
+    REASON = "{} has forbidden cross-account policy allow with {} for an KMS Key Policy"
+
+    def invoke(self, cfmodel: CFModel):
+        for logical_id, resource in cfmodel.Resources.items():
+            if isinstance(resource, KMSKey):
+                for statement in resource.Properties.KeyPolicy._statement_as_list():
+                    self._do_statement_check(logical_id, statement)

cfripper/rules/ebs_volume_has_sse.py

@@ -13,7 +13,7 @@
 specific language governing permissions and limitations under the License.
 """
 __all__ = ["EBSVolumeHasSSERule"]
-from cfripper.model.enums import RuleMode
+from cfripper.model.enums import RuleGranularity, RuleMode
 from cfripper.model.rule import Rule
 
 
@@ -24,9 +24,10 @@ class EBSVolumeHasSSERule(Rule):
 
     REASON = "EBS volume {} should have server-side encryption enabled"
     RULE_MODE = RuleMode.MONITOR
+    GRANULARITY = RuleGranularity.RESOURCE
 
     def invoke(self, cfmodel):
         for logical_id, resource in cfmodel.Resources.items():
             if resource.Type == "AWS::EC2::Volume":
                 if resource.Properties.get("Encrypted") != "true":
-                    self.add_failure(type(self).__name__, self.REASON.format(logical_id))
+                    self.add_failure(type(self).__name__, self.REASON.format(logical_id), resource_ids={logical_id})

cfripper/rules/hardcoded_RDS_password.py

@@ -15,6 +15,7 @@
 __all__ = ["HardcodedRDSPasswordRule"]
 from pycfmodel.model.parameter import Parameter
 
+from cfripper.model.enums import RuleGranularity
 from cfripper.model.rule import Rule
 
 
@@ -25,6 +26,7 @@ class HardcodedRDSPasswordRule(Rule):
 
     REASON_DEFAULT = "Default RDS {} password parameter (readable in plain-text) for {}."
     REASON_MISSING_NOECHO = "RDS {} password parameter missing NoEcho for {}."
+    GRANULARITY = RuleGranularity.RESOURCE
 
     def invoke(self, cfmodel):
         password_protected_cluster_ids = []
@@ -55,10 +57,16 @@ def _failure_added(self, logical_id, resource) -> bool:
         master_user_password = resource.Properties.get("MasterUserPassword", Parameter.NO_ECHO_NO_DEFAULT)
         resource_type = resource.Type.replace("AWS::RDS::DB", "")
         if master_user_password == Parameter.NO_ECHO_WITH_DEFAULT:
-            self.add_failure(type(self).__name__, self.REASON_DEFAULT.format(resource_type, logical_id))
+            self.add_failure(
+                type(self).__name__, self.REASON_DEFAULT.format(resource_type, logical_id), resource_ids={logical_id}
+            )
             return True
         elif master_user_password not in (Parameter.NO_ECHO_NO_DEFAULT, Parameter.NO_ECHO_WITH_VALUE):
-            self.add_failure(type(self).__name__, self.REASON_MISSING_NOECHO.format(resource_type, logical_id))
+            self.add_failure(
+                type(self).__name__,
+                self.REASON_MISSING_NOECHO.format(resource_type, logical_id),
+                resource_ids={logical_id},
+            )
             return True
 
         return False

cfripper/rules/iam_managed_policy_wildcard_action.py

@@ -16,6 +16,7 @@
 from pycfmodel.model.resources.iam_managed_policy import IAMManagedPolicy
 
 from cfripper.config.regex import REGEX_WILDCARD_POLICY_ACTION
+from cfripper.model.enums import RuleGranularity
 from cfripper.model.rule import Rule
 
 
@@ -24,11 +25,12 @@ class IAMManagedPolicyWildcardActionRule(Rule):
     This rule checks for wildcards in IAM Managed policies
     """
 
+    GRANULARITY = RuleGranularity.RESOURCE
     REASON = "IAM managed policy {} should not allow * action"
 
     def invoke(self, cfmodel):
         for logical_id, resource in cfmodel.Resources.items():
             if isinstance(resource, IAMManagedPolicy) and resource.Properties.PolicyDocument.allowed_actions_with(
                 REGEX_WILDCARD_POLICY_ACTION
             ):
-                self.add_failure(type(self).__name__, self.REASON.format(logical_id))
+                self.add_failure(type(self).__name__, self.REASON.format(logical_id), resource_ids={logical_id})

cfripper/rules/iam_roles.py

@@ -20,6 +20,7 @@
 from pycfmodel.model.resources.iam_role import IAMRole
 
 from cfripper.config.regex import REGEX_IS_STAR, REGEX_WILDCARD_POLICY_ACTION
+from cfripper.model.enums import RuleGranularity
 from cfripper.model.rule import Rule
 
 
@@ -28,6 +29,8 @@ class IAMRolesOverprivilegedRule(Rule):
     Rule that checks for wildcards in resources for a set of actions and restricts managed policies
     """
 
+    GRANULARITY = RuleGranularity.RESOURCE
+
     def invoke(self, cfmodel):
         for logical_id, resource in cfmodel.Resources.items():
             if isinstance(resource, IAMRole):
@@ -42,7 +45,9 @@ def check_managed_policies(self, logical_id, role):
         for managed_policy_arn in role.Properties.ManagedPolicyArns:
             if managed_policy_arn in self._config.forbidden_managed_policy_arns:
                 self.add_failure(
-                    type(self).__name__, f"Role {logical_id} has forbidden Managed Policy {managed_policy_arn}"
+                    type(self).__name__,
+                    f"Role {logical_id} has forbidden Managed Policy {managed_policy_arn}",
+                    resource_ids={logical_id},
                 )
 
     def check_inline_policies(self, logical_id, role):
@@ -60,6 +65,7 @@ def check_inline_policies(self, logical_id, role):
                                     type(self).__name__,
                                     f"Role '{logical_id}' contains an insecure permission '{action}' in policy "
                                     f"'{policy.PolicyName}'",
+                                    resource_ids={logical_id},
                                 )
 
 
@@ -68,26 +74,32 @@ class IAMRoleWildcardActionOnPermissionsPolicyRule(Rule):
     Rule that checks for wildcards in actions in IAM role policies
     """
 
+    GRANULARITY = RuleGranularity.RESOURCE
     REASON = "IAM role {} should not allow * action on its permissions policy {}"
 
     def invoke(self, cfmodel):
         for logical_id, resource in cfmodel.Resources.items():
             if isinstance(resource, IAMRole):
                 for policy in resource.Properties.Policies:
                     if policy.PolicyDocument.allowed_actions_with(REGEX_WILDCARD_POLICY_ACTION):
-                        self.add_failure(type(self).__name__, self.REASON.format(logical_id, policy.PolicyName))
+                        self.add_failure(
+                            type(self).__name__,
+                            self.REASON.format(logical_id, policy.PolicyName),
+                            resource_ids={logical_id},
+                        )
 
 
 class IAMRoleWildcardActionOnTrustPolicyRule(Rule):
     """
     Rule that checks for wildcards in actions in IAM role assume role policy documents
     """
 
+    GRANULARITY = RuleGranularity.RESOURCE
     REASON = "IAM role {} should not allow * action on its trust policy"
 
     def invoke(self, cfmodel):
         for logical_id, resource in cfmodel.Resources.items():
             if isinstance(resource, IAMRole) and resource.Properties.AssumeRolePolicyDocument.allowed_actions_with(
                 REGEX_WILDCARD_POLICY_ACTION
             ):
-                self.add_failure(type(self).__name__, self.REASON.format(logical_id))
+                self.add_failure(type(self).__name__, self.REASON.format(logical_id), resource_ids={logical_id})

cfripper/rules/kms_key_wildcard_principal.py

@@ -19,6 +19,7 @@
 from pycfmodel.model.cf_model import CFModel
 from pycfmodel.model.resources.kms_key import KMSKey
 
+from cfripper.model.enums import RuleGranularity
 from cfripper.model.rule import Rule
 
 logger = logging.getLogger(__file__)
@@ -29,6 +30,8 @@ class KMSKeyWildcardPrincipal(Rule):
     This rule checks for KMS keys that contain wildcards in the key policies
     """
 
+    GRANULARITY = RuleGranularity.RESOURCE
+
     REASON = "KMS Key policy {} should not allow wildcard principals"
     CONTAINS_WILDCARD_PATTERN = re.compile(r"^(\w*:)?\*$")
 
@@ -45,4 +48,6 @@ def invoke(self, cfmodel: CFModel):
                                         f"because there are conditions: {statement.Condition}"
                                     )
                                 else:
-                                    self.add_failure(type(self).__name__, self.REASON.format(logical_id))
+                                    self.add_failure(
+                                        type(self).__name__, self.REASON.format(logical_id), resource_ids={logical_id}
+                                    )

cfripper/rules/managed_policy_on_user.py

@@ -16,7 +16,7 @@
 from pycfmodel.model.cf_model import CFModel
 from pycfmodel.model.resources.iam_managed_policy import IAMManagedPolicy
 
-from cfripper.model.enums import RuleMode
+from cfripper.model.enums import RuleGranularity, RuleMode
 from cfripper.model.rule import Rule
 
 
@@ -27,8 +27,9 @@ class ManagedPolicyOnUserRule(Rule):
 
     REASON = "IAM managed policy {} should not apply directly to users. Should be on group"
     RULE_MODE = RuleMode.MONITOR
+    GRANULARITY = RuleGranularity.RESOURCE
 
     def invoke(self, cfmodel: CFModel):
         for logical_id, resource in cfmodel.Resources.items():
             if isinstance(resource, IAMManagedPolicy) and resource.Properties.Users:
-                self.add_failure(type(self).__name__, self.REASON.format(logical_id))
+                self.add_failure(type(self).__name__, self.REASON.format(logical_id), resource_ids={logical_id})