EnableKeyRotation rule for KMS (#187)

* EnableKeyRotation rule for KMS

* Format

* PR Suggestions

Co-authored-by: Carles Lopez <carles.lopez@skyscanner.net>

CHANGELOG.md

@@ -1,6 +1,11 @@
 # Changelog
 All notable changes to this project will be documented in this file.
 
+## [1.0.7] - 2021-08-16
+### Improvements
+- Add `KMSKeyEnabledKeyRotation` rule
+- Bump `pycfmodel` to `0.10.4`
+
 ## [1.0.6] - 2021-07-28
 ### Improvements
 - Add `S3BucketPublicReadAclRule` rule

cfripper/__version__.py

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

cfripper/rules/__init__.py

@@ -15,6 +15,7 @@
 )
 from cfripper.rules.hardcoded_RDS_password import HardcodedRDSPasswordRule
 from cfripper.rules.iam_roles import IAMRolesOverprivilegedRule, IAMRoleWildcardActionOnPolicyRule
+from cfripper.rules.kms_key_rotation_enabled import KMSKeyEnabledKeyRotation
 from cfripper.rules.kms_key_wildcard_principal import KMSKeyWildcardPrincipalRule
 from cfripper.rules.managed_policy_on_user import ManagedPolicyOnUserRule
 from cfripper.rules.policy_on_user import PolicyOnUserRule
@@ -54,6 +55,7 @@
         IAMRoleWildcardActionOnPolicyRule,
         KMSKeyCrossAccountTrustRule,
         KMSKeyWildcardPrincipalRule,
+        KMSKeyEnabledKeyRotation,
         ManagedPolicyOnUserRule,
         PartialWildcardPrincipalRule,
         PolicyOnUserRule,

cfripper/rules/kms_key_rotation_enabled.py

@@ -0,0 +1,51 @@
+__all__ = ["KMSKeyEnabledKeyRotation"]
+
+import logging
+from typing import Dict, Optional
+
+from pycfmodel.model.cf_model import CFModel
+
+from cfripper.model.enums import RuleGranularity, RuleRisk
+from cfripper.model.result import Result
+from cfripper.rules.base_rules import Rule
+
+logger = logging.getLogger(__file__)
+
+
+class KMSKeyEnabledKeyRotation(Rule):
+    """
+    Check if EnableKeyRotation is true for symmetric KMS keys in principals in KMS Policies.
+
+    Fix:
+        Set EnableKeyRotation to true for any symmetric KMS key.
+
+    Filters context:
+        | Parameter           | Type                | Description                                                   |
+        |:-------------------:|:------------------:|:--------------------------------------------------------------:|
+        |`config`             | str                | `config` variable available inside the rule                    |
+        |`extras`             | str                | `extras` variable available inside the rule                    |
+        |`logical_id`         | str                | ID used in Cloudformation to refer the resource being analysed |
+        |`resource`           | `KMSKey`           | Resource that is being addressed                               |
+    """
+
+    GRANULARITY = RuleGranularity.RESOURCE
+    REASON = "KMS Key {} should have the key rotation enabled for symmetric keys"
+    RISK_VALUE = RuleRisk.HIGH
+
+    def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
+        result = Result()
+        for logical_id, resource in cfmodel.resources_filtered_by_type(("AWS::KMS::Key")).items():
+            if not hasattr(resource, "KeySpec") or resource.Properties.get("KeySpec") == "SYMMETRIC_DEFAULT":
+                if not hasattr(resource, "EnableKeyRotation") or resource.Properties.get("EnableKeyRotation") is False:
+                    self.add_failure_to_result(
+                        result,
+                        self.REASON.format(logical_id),
+                        resource_ids={logical_id},
+                        context={
+                            "config": self._config,
+                            "extras": extras,
+                            "logical_id": logical_id,
+                            "resource": resource,
+                        },
+                    )
+        return result

requirements.txt

@@ -11,7 +11,7 @@ click==7.1.2
 importlib-metadata==4.4.0
 jmespath==0.10.0
 pluggy==0.13.1
-pycfmodel==0.9.1
+pycfmodel==0.10.4
 pydantic==1.8.2
 pydash==4.7.6
 python-dateutil==2.8.1

setup.py

@@ -11,7 +11,7 @@
     "cfn_flip>=1.2.0",
     "click~=7.1.1",
     "pluggy~=0.13.1",
-    "pycfmodel>=0.8.2",
+    "pycfmodel>=0.10.4",
     "pydash~=4.7.6",
     "PyYAML>=4.2b1",
 ]

tests/rules/test_KMSEnabledKeyRotationRule.py

@@ -0,0 +1,50 @@
+import pytest
+from pytest import fixture
+
+from cfripper.config.config import Config
+from cfripper.model.enums import RuleGranularity, RuleMode, RuleRisk
+from cfripper.model.result import Failure
+from cfripper.rules import KMSKeyEnabledKeyRotation
+from tests.utils import compare_lists_of_failures, get_cfmodel_from
+
+
+@fixture()
+def bad_template():
+    return get_cfmodel_from("rules/KMSEnabledKeyRotation/bad_template_symmetric_no_property.yaml").resolve()
+
+
+@pytest.mark.parametrize(
+    "bad_template_path",
+    [
+        "rules/KMSEnabledKeyRotation/bad_template_symmetric_keyspec_property.yaml",
+        "rules/KMSEnabledKeyRotation/bad_template_symmetric_no_property.yaml",
+        "rules/KMSEnabledKeyRotation/bad_template_symmetric_property.yaml",
+    ],
+)
+def test_failures_are_raised(bad_template_path):
+    rule = KMSKeyEnabledKeyRotation(Config())
+    result = rule.invoke(get_cfmodel_from(bad_template_path).resolve())
+
+    assert not result.valid
+    assert compare_lists_of_failures(
+        result.failures,
+        [
+            Failure(
+                granularity=RuleGranularity.RESOURCE,
+                reason="KMS Key KMSKey should have the key rotation enabled for symmetric keys",
+                risk_value=RuleRisk.HIGH,
+                rule="KMSKeyEnabledKeyRotation",
+                rule_mode=RuleMode.BLOCKING,
+                actions=None,
+                resource_ids={"KMSKey"},
+            )
+        ],
+    )
+
+
+def test_rule_supports_filter_config(bad_template, default_allow_all_config):
+    rule = KMSKeyEnabledKeyRotation(default_allow_all_config)
+    result = rule.invoke(bad_template)
+
+    assert result.valid
+    assert compare_lists_of_failures(result.failures, [])

tests/test_templates/rules/KMSEnabledKeyRotation/bad_template_symmetric_keyspec_property.yaml

@@ -0,0 +1,16 @@
+Resources:
+  KMSKey:
+    Type: AWS::KMS::Key
+    Properties:
+      Description: An example multi-Region primary key
+      KeySpec: SYMMETRIC_DEFAULT
+      KeyPolicy:
+        Version: '2012-10-17'
+        Id: key-default-1
+        Statement:
+          - Sid: Enable IAM User Permissions
+            Effect: Allow
+            Principal:
+              AWS: arn:aws:iam::111122223333:root
+            Action: kms:*
+            Resource: '*'

tests/test_templates/rules/KMSEnabledKeyRotation/bad_template_symmetric_no_property.yaml

@@ -0,0 +1,15 @@
+Resources:
+  KMSKey:
+    Type: AWS::KMS::Key
+    Properties:
+      Description: An example multi-Region primary key
+      KeyPolicy:
+        Version: '2012-10-17'
+        Id: key-default-1
+        Statement:
+          - Sid: Enable IAM User Permissions
+            Effect: Allow
+            Principal:
+              AWS: arn:aws:iam::111122223333:root
+            Action: kms:*
+            Resource: '*'

tests/test_templates/rules/KMSEnabledKeyRotation/bad_template_symmetric_property.yaml

@@ -0,0 +1,16 @@
+Resources:
+  KMSKey:
+    Type: AWS::KMS::Key
+    Properties:
+      Description: An example multi-Region primary key
+      EnableKeyRotation: false
+      KeyPolicy:
+        Version: '2012-10-17'
+        Id: key-default-1
+        Statement:
+          - Sid: Enable IAM User Permissions
+            Effect: Allow
+            Principal:
+              AWS: arn:aws:iam::111122223333:root
+            Action: kms:*
+            Resource: '*'