Repository: bridgecrewio/checkov
Pull request: fix(cloudformation): Updated AWS_CKV_7 to not require rotation on asymmetric keys
**By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.**

[//]: # "
    # PR Title
    Be aware that we use the title to create changelog automatically and therefore only allow specific prefixes
    - break:    to indicate a breaking change, this supersedes any of the types
    - feat:     to indicate new features or checks
    - fix:      to indicate a bugfix or handling of edge cases of existing checks
    - docs:     to indicate an update to our documentation
    - chore:    to indicate adjustments to workflow files or dependency updates
    - platform: to indicate a change needed for the platform
    Additionally a scope is needs to be added to the prefix, which indicates the targeted framework, in doubt choose 'general'.
    #    
    Allowed prefixs:
    ansible|argo|arm|azure|bicep|bitbucket|circleci|cloudformation|dockerfile|github|gha|gitlab|helm|kubernetes|kustomize|openapi|sca|secrets|serverless|terraform|general|graph|terraform_plan
    #
    ex.
    feat(terraform): add CKV_AWS_123 to ensure that VPC Endpoint Service is configured for Manual Acceptance
"

## Description

Updated cloudformation and terraform checks to not expect key rotation for asymmetric keys as listed here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-kms-key.html#cfn-kms-key-keyspec

Fixes #4475 

## New/Edited policies (Delete if not relevant)

### Description
*Include a description of what makes it a violation and any relevant external links.*

### Fix
*How does someone fix the issue in code and/or in runtime?*

## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have added tests that prove my feature, policy, or fix is effective and works
- [x] New and existing tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream modules


Problem statement:
AWS_CKV_7 False Positive on assymetric key check in Cloudformation
**Describe the issue**
In terraform, the check avoids false positives with an extra check against symmetric keys before checking whether rotation is enabled.  This same check hasn't been configured for cloudformation:

```
def scan_resource_conf(self, conf):
        # Only symmetric keys support auto rotation. The attribute is optional and defaults to symmetric.
        spec = conf.get('customer_master_key_spec')
        if not spec or 'SYMMETRIC_DEFAULT' in spec:
            return super().scan_resource_conf(conf)
        else:
            return CheckResult.PASSED
```

**Examples**

```
RSASigningKey:
  Type: 'AWS::KMS::Key'
  Properties:
    Description: RSA-3072 asymmetric KMS key for signing and verification
    KeySpec: RSA_3072
    KeyUsage: SIGN_VERIFY
    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: '*'
        - Sid: Allow administration of the key
          Effect: Allow
          Principal:
            AWS: 'arn:aws:iam::111122223333:role/Admin'
          Action:
            - 'kms:Create*'
            - 'kms:Describe*'
            - 'kms:Enable*'
            - 'kms:List*'
            - 'kms:Put*'
            - 'kms:Update*'
            - 'kms:Revoke*'
            - 'kms:Disable*'
            - 'kms:Get*'
            - 'kms:Delete*'
            - 'kms:ScheduleKeyDeletion'
            - 'kms:CancelKeyDeletion'
          Resource: '*'
        - Sid: Allow use of the key
          Effect: Allow
          Principal:
            AWS: 'arn:aws:iam::111122223333:role/Developer'
          Action:
            - 'kms:Sign'
            - 'kms:Verify'
            - 'kms:DescribeKey'
          Resource: '*'
```

**Version (please complete the following information):**
 - Checkov Version [e.g. 2.3.0]

**Additional context**

This blocks checkov working for assymetric keys in CFN.

Hints from issue discussion:

Commit message: Fixing some extraneous stuff spotted in PR review

Patch to review:
diff --git a/checkov/cloudformation/checks/resource/aws/KMSRotation.py b/checkov/cloudformation/checks/resource/aws/KMSRotation.py
index 2a76f73b5b..2843cff89d 100644
--- a/checkov/cloudformation/checks/resource/aws/KMSRotation.py
+++ b/checkov/cloudformation/checks/resource/aws/KMSRotation.py
@@ -1,4 +1,4 @@
-from checkov.common.models.enums import CheckCategories
+from checkov.common.models.enums import CheckCategories, CheckResult
 from checkov.cloudformation.checks.resource.base_resource_value_check import BaseResourceValueCheck
 
 
@@ -13,5 +13,16 @@ def __init__(self) -> None:
     def get_inspected_key(self) -> str:
         return "Properties/EnableKeyRotation"
 
+    def scan_resource_conf(self, conf):
+        # Only symmetric keys support auto rotation. The attribute is optional and defaults to symmetric.
+        if 'Properties' in conf.keys():
+            if 'KeySpec' in conf['Properties'].keys():
+                spec = conf['Properties']['KeySpec']
+                if not spec or 'SYMMETRIC_DEFAULT' in spec or 'HMAC' in spec:
+                    return super().scan_resource_conf(conf)
+                else:
+                    return CheckResult.PASSED
+        return super().scan_resource_conf(conf)
+
 
 check = KMSRotation()
diff --git a/checkov/terraform/checks/resource/aws/KMSRotation.py b/checkov/terraform/checks/resource/aws/KMSRotation.py
index d74bcb5206..20fc1d7c43 100644
--- a/checkov/terraform/checks/resource/aws/KMSRotation.py
+++ b/checkov/terraform/checks/resource/aws/KMSRotation.py
@@ -16,7 +16,7 @@ def get_inspected_key(self):
     def scan_resource_conf(self, conf):
         # Only symmetric keys support auto rotation. The attribute is optional and defaults to symmetric.
         spec = conf.get('customer_master_key_spec')
-        if not spec or 'SYMMETRIC_DEFAULT' in spec:
+        if not spec or 'SYMMETRIC_DEFAULT' in spec or 'HMAC' in spec:
             return super().scan_resource_conf(conf)
         else:
             return CheckResult.PASSED
diff --git a/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-FAILED.yml b/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-FAILED.yml
index b3426ffc51..b371e98fb1 100644
--- a/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-FAILED.yml
+++ b/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-FAILED.yml
@@ -1,7 +1,7 @@
 AWSTemplateFormatVersion: 2010-09-09
 Description: KMS key example template
 Resources:
-  myKey:
+  SymmetricKey:
     Type: AWS::KMS::Key
     Properties:
       KeyPolicy:
diff --git a/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED-Asymmetric.yml b/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED-Asymmetric.yml
new file mode 100644
index 0000000000..4cc0a39ec6
--- /dev/null
+++ b/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED-Asymmetric.yml
@@ -0,0 +1,24 @@
+AWSTemplateFormatVersion: 2010-09-09
+Description: KMS key example template
+Resources:
+  AsymmetricKey:
+    Type: AWS::KMS::Key
+    Properties:
+      KeySpec: RSA_4096
+      KeyPolicy:
+        Version: '2012-10-17'
+        Id: key-default-1
+        Statement:
+        - Sid: Enable IAM User Permissions
+          Effect: Allow
+          Principal:
+            AWS:
+              Fn::Join:
+              - ''
+              - - 'arn:aws:iam::'
+                - Ref: AWS::AccountId
+                - :root
+          Action: kms:*
+          Resource: '*'
+      EnableKeyRotation: true
+  
\ No newline at end of file
diff --git a/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED.yml b/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED-Symmetric.yml
similarity index 91%
rename from tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED.yml
rename to tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED-Symmetric.yml
index abdfc6c318..bbd51740ca 100644
--- a/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED.yml
+++ b/tests/cloudformation/checks/resource/aws/example_KMSRotation/KMSRotation-PASSED-Symmetric.yml
@@ -1,7 +1,7 @@
 AWSTemplateFormatVersion: 2010-09-09
 Description: KMS key example template
 Resources:
-  myKey:
+  SymmetricKey:
     Type: AWS::KMS::Key
     Properties:
       KeyPolicy:
@@ -19,4 +19,5 @@ Resources:
                 - :root
           Action: kms:*
           Resource: '*'
-      EnableKeyRotation: true
\ No newline at end of file
+      EnableKeyRotation: true
+  
\ No newline at end of file
diff --git a/tests/cloudformation/checks/resource/aws/test_KMSRotation.py b/tests/cloudformation/checks/resource/aws/test_KMSRotation.py
index 8deb05218e..b521dcf49f 100644
--- a/tests/cloudformation/checks/resource/aws/test_KMSRotation.py
+++ b/tests/cloudformation/checks/resource/aws/test_KMSRotation.py
@@ -13,14 +13,13 @@ def test_summary(self):
         current_dir = os.path.dirname(os.path.realpath(__file__))
 
         test_files_dir = current_dir + "/example_KMSRotation"
-        report = runner.run(root_folder=test_files_dir,runner_filter=RunnerFilter(checks=[check.id]))
+        report = runner.run(root_folder=test_files_dir, runner_filter=RunnerFilter(checks=[check.id]))
         summary = report.get_summary()
 
-        self.assertEqual(summary['passed'], 1)
-        self.assertEqual(summary['failed'], 1)
+        self.assertEqual(summary["passed"], 2)
+        self.assertEqual(summary["failed"], 1)
         self.assertEqual(summary['skipped'], 0)
         self.assertEqual(summary['parsing_errors'], 0)
 
-
 if __name__ == '__main__':
     unittest.main()


Code review comments:
diff_hunk: @@ -16,7 +16,7 @@ def get_inspected_key(self):
     def scan_resource_conf(self, conf):
         # Only symmetric keys support auto rotation. The attribute is optional and defaults to symmetric.
         spec = conf.get('customer_master_key_spec')
-        if not spec or 'SYMMETRIC_DEFAULT' in spec:
+        if not spec or 'SYMMETRIC_DEFAULT' in spec or 'HMAC' in spec:
             return super().scan_resource_conf(conf)
         else:
             return CheckResult.PASSED
path: checkov/terraform/checks/resource/aws/KMSRotation.py
text: @user1:
also would be great, if you can change the the result to `UNKNOWN`

@author:
Heya @user1,
Re-read the document and agree.  This check isn't relevant for assymetric keys, so once they're identified as not symmetric, the check should not apply.  Is `UNKNOWN` the right result in that instance?

@author:
Done the switch