#335 Adding warning if a KMS key allows wildcarded principals in its …

…policy (#338) * #335 Adding warning if a KMS key allows wildcarded principals in its policy. * #335 Changing to failure, and adding logic to catch when AWS subkey is set to wildcard. * #335 Modifying KMS key wildcard principal rule to use new KMS key model from cfn-model and included tests for nested hash wildcard principal. * #335 Updating cfn-model version.
stelligent · Jan 14, 2020 · c1f1a78 · c1f1a78
1 parent 9e1a004
commit c1f1a78
Show file tree

Hide file tree

Showing 12 changed files with 381 additions and 3 deletions.
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -2,7 +2,7 @@ PATH
   remote: .
   specs:
     cfn-nag (0.0.0)
-      cfn-model (= 0.4.11)
+      cfn-model (= 0.4.12)
       jmespath (~> 1.3.1)
       logging (~> 2.2.2)
       netaddr (~> 2.0.4)
@@ -12,7 +12,7 @@ GEM
   remote: https://rubygems.org/
   specs:
     ast (2.4.0)
-    cfn-model (0.4.11)
+    cfn-model (0.4.12)
       kwalify (= 0.7.2)
       psych (~> 3)
     diff-lcs (1.3)

diff --git a/cfn-nag.gemspec b/cfn-nag.gemspec
@@ -25,7 +25,7 @@ Gem::Specification.new do |s|
   # versus what we used to run tests in cfn-nag before publishing cfn-nag
   # they are coupled and we are doing a good bit of experimenting in cfn-model
   # i might consider collapsing them again....
-  s.add_runtime_dependency('cfn-model', '0.4.11')
+  s.add_runtime_dependency('cfn-model', '0.4.12')
   s.add_runtime_dependency('jmespath', '~> 1.3.1')
   s.add_runtime_dependency('logging', '~> 2.2.2')
   s.add_runtime_dependency('netaddr', '~> 2.0.4')

diff --git a/lib/cfn-nag/custom_rules/KMSKeyWildcardPrincipalRule.rb b/lib/cfn-nag/custom_rules/KMSKeyWildcardPrincipalRule.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'cfn-nag/violation'
+require_relative 'base'
+
+class KMSKeyWildcardPrincipalRule < BaseRule
+  def rule_text
+    'KMS key should not allow * principal ' \
+    '(https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html)'
+  end
+
+  def rule_type
+    Violation::FAILING_VIOLATION
+  end
+
+  def rule_id
+    'F76'
+  end
+
+  def audit_impl(cfn_model)
+    # Select all AWS::KMS::Key resources to audit
+    violating_keys = cfn_model.resources_by_type('AWS::KMS::Key').select do |key|
+      # Return key if wildcard_allowed_principals boolean is not empty
+      !key.key_policy.policy_document.wildcard_allowed_principals.empty?
+    end
+
+    violating_keys.map(&:logical_resource_id)
+  end
+end
diff --git a/spec/custom_rules/KMSKeyWildcardPrincipalRule_spec.rb b/spec/custom_rules/KMSKeyWildcardPrincipalRule_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+require 'cfn-nag/custom_rules/KMSKeyWildcardPrincipalRule'
+require 'cfn-model'
+
+describe KMSKeyWildcardPrincipalRule do
+  describe 'AWS::KMS::Key' do
+    context 'when a principal is not set to a wildcard' do
+      it 'does not return an offending logical resource id' do
+        cfn_model = CfnParser.new.parse read_test_template('json/kms/kms_key_without_wildcard_principal.json')
+        actual_logical_resource_ids = KMSKeyWildcardPrincipalRule.new.audit_impl cfn_model
+
+        expect(actual_logical_resource_ids).to eq []
+      end
+    end
+    context 'when a principal is set to a wildcard' do
+      it 'returns an offending logical resource id' do
+        cfn_model = CfnParser.new.parse read_test_template('json/kms/kms_key_with_wildcard_principal.json')
+        actual_logical_resource_ids = KMSKeyWildcardPrincipalRule.new.audit_impl cfn_model
+
+        expect(actual_logical_resource_ids).to eq ['myKeyWildcardPrincipal']
+      end
+    end
+    context 'when a principal\'s AWS key is set to a wildcard' do
+      it 'returns an offending logical resource id' do
+        cfn_model = CfnParser.new.parse read_test_template('json/kms/kms_key_with_aws_wildcard_principal.json')
+        actual_logical_resource_ids = KMSKeyWildcardPrincipalRule.new.audit_impl cfn_model
+
+        expect(actual_logical_resource_ids).to eq ['myKeyAwsWildcardPrincipal']
+      end
+    end
+    context 'when a principal\'s AWS key is an array and contains a wildcard' do
+      it 'returns an offending logical resource id' do
+        cfn_model = CfnParser.new.parse read_test_template('json/kms/kms_key_with_aws_array_wildcard_principal.json')
+        actual_logical_resource_ids = KMSKeyWildcardPrincipalRule.new.audit_impl cfn_model
+
+        expect(actual_logical_resource_ids).to eq ['myKeyAwsArrayWildcardPrincipal']
+      end
+    end
+  end
+end
diff --git a/spec/test_templates/json/kms/kms_key_with_aws_array_wildcard_principal.json b/spec/test_templates/json/kms/kms_key_with_aws_array_wildcard_principal.json
@@ -0,0 +1,44 @@
+{
+  "Resources": {
+    "myKeyAwsArrayWildcardPrincipal" : {
+      "Type" : "AWS::KMS::Key",
+      "Properties" : {
+        "Description" : "An example CMK",
+				"EnableKeyRotation": "true",
+        "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:root", "*"]},
+              "Action": [
+                "kms:Create*",
+                "kms:CancelKeyDeletion"
+              ],
+              "Resource": "*"
+            },
+            {
+              "Sid": "Allow use of the key",
+              "Effect": "Allow",
+              "Principal": {"AWS": "arn:aws:iam::123456789012:user/Alice"},
+              "Action": [
+                "kms:GenerateDataKey",
+                "kms:GenerateDataKeyWithoutPlaintext"
+              ],
+              "Resource": "*"
+            }
+          ]
+        }
+      }
+    }
+  }
+}
diff --git a/spec/test_templates/json/kms/kms_key_with_aws_wildcard_principal.json b/spec/test_templates/json/kms/kms_key_with_aws_wildcard_principal.json
@@ -0,0 +1,44 @@
+{
+  "Resources": {
+    "myKeyAwsWildcardPrincipal" : {
+      "Type" : "AWS::KMS::Key",
+      "Properties" : {
+        "Description" : "An example CMK",
+				"EnableKeyRotation": "true",
+        "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": "*"},
+              "Action": [
+                "kms:Create*",
+                "kms:CancelKeyDeletion"
+              ],
+              "Resource": "*"
+            },
+            {
+              "Sid": "Allow use of the key",
+              "Effect": "Allow",
+              "Principal": {"AWS": "arn:aws:iam::123456789012:user/Alice"},
+              "Action": [
+                "kms:GenerateDataKey",
+                "kms:GenerateDataKeyWithoutPlaintext"
+              ],
+              "Resource": "*"
+            }
+          ]
+        }
+      }
+    }
+  }
+}
diff --git a/spec/test_templates/json/kms/kms_key_with_wildcard_principal.json b/spec/test_templates/json/kms/kms_key_with_wildcard_principal.json
@@ -0,0 +1,44 @@
+{
+  "Resources": {
+    "myKeyWildcardPrincipal" : {
+      "Type" : "AWS::KMS::Key",
+      "Properties" : {
+        "Description" : "An example CMK",
+				"EnableKeyRotation": "true",
+        "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": { "CanonicalUser": "79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be" },
+              "Action": [
+                "kms:Create*",
+                "kms:CancelKeyDeletion"
+              ],
+              "Resource": "*"
+            },
+            {
+              "Sid": "Allow use of the key",
+              "Effect": "Allow",
+              "Principal": "*",
+              "Action": [
+                "kms:GenerateDataKey",
+                "kms:GenerateDataKeyWithoutPlaintext"
+              ],
+              "Resource": "*"
+            }
+          ]
+        }
+      }
+    }
+  }
+}
diff --git a/spec/test_templates/json/kms/kms_key_without_wildcard_principal.json b/spec/test_templates/json/kms/kms_key_without_wildcard_principal.json
@@ -0,0 +1,44 @@
+{
+  "Resources": {
+    "myKeyNoWildcardPrincipal" : {
+      "Type" : "AWS::KMS::Key",
+      "Properties" : {
+        "Description" : "An example CMK",
+				"EnableKeyRotation": "true",
+        "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::123456789012:user/Alice" },
+              "Action": [
+                "kms:Create*",
+                "kms:CancelKeyDeletion"
+              ],
+              "Resource": "*"
+            },
+            {
+              "Sid": "Allow use of the key",
+              "Effect": "Allow",
+              "Principal": { "AWS": "arn:aws:iam::123456789012:user/Bob" },
+              "Action": [
+                "kms:GenerateDataKey",
+                "kms:GenerateDataKeyWithoutPlaintext"
+              ],
+              "Resource": "*"
+            }
+          ]
+        }
+      }
+    }
+  }
+}
diff --git a/spec/test_templates/yaml/kms/kms_key_with_aws_array_wildcard_principal.yml b/spec/test_templates/yaml/kms/kms_key_with_aws_array_wildcard_principal.yml
@@ -0,0 +1,35 @@
+---
+Resources:
+  myKeyAwsArrayWildcardPrincipal:
+    Type: AWS::KMS::Key
+    Properties:
+      Description: An example CMK
+      EnableKeyRotation: true
+      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:root
+                - '*'
+            Action:
+              - kms:Create*
+              - kms:CancelKeyDeletion
+            Resource: '*'
+          - Sid: Allow use of the key
+            Effect: Allow
+            Principal:
+              AWS: arn:aws:iam::123456789012:user/Alice
+            Action:
+              - kms:GenerateDataKey
+              - kms:GenerateDataKeyWithoutPlaintext
+            Resource: '*'
diff --git a/spec/test_templates/yaml/kms/kms_key_with_aws_wildcard_principal.yml b/spec/test_templates/yaml/kms/kms_key_with_aws_wildcard_principal.yml
@@ -0,0 +1,33 @@
+---
+Resources:
+  myKeyAwsWildcardPrincipal:
+    Type: AWS::KMS::Key
+    Properties:
+      Description: An example CMK
+      EnableKeyRotation: true
+      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: '*'
+            Action:
+              - kms:Create*
+              - kms:CancelKeyDeletion
+            Resource: '*'
+          - Sid: Allow use of the key
+            Effect: Allow
+            Principal:
+              AWS: arn:aws:iam::123456789012:user/Alice
+            Action:
+              - kms:GenerateDataKey
+              - kms:GenerateDataKeyWithoutPlaintext
+            Resource: '*'
diff --git a/spec/test_templates/yaml/kms/kms_key_with_wildcard_principal.yml b/spec/test_templates/yaml/kms/kms_key_with_wildcard_principal.yml
@@ -0,0 +1,32 @@
+---
+Resources:
+  myKeyWildcardPrincipal:
+    Type: AWS::KMS::Key
+    Properties:
+      Description: An example CMK
+      EnableKeyRotation: true
+      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:
+              CanonicalUser: 79a59df900b949e55d96a1e698fbacedfd6e09d98eacf8f8d5218e7cd47ef2be
+            Action:
+              - kms:Create*
+              - kms:CancelKeyDeletion
+            Resource: '*'
+          - Sid: Allow use of the key
+            Effect: Allow
+            Principal: '*'
+            Action:
+              - kms:GenerateDataKey
+              - kms:GenerateDataKeyWithoutPlaintext
+            Resource: '*'