Repository: bridgecrewio/checkov
Pull request: CKV2_GCP_8 - GCP public key ring check
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

This PR adds a graph .yaml check for public `GCP Cloud KMS Key Rings`. This code is modeled after the already existing check `CKV2_GCP_6` For Cloud KMS crypto keys.

Closes #2381 

Problem statement:
CKV2_GCP_8 - GCP public key ring check
**Describe the issue**
This issue is to track the creation of a new GCP terraform checkov check for public Cloud KMS key rings. Public KMS key rings will set ALL keys inside the key ring to public.

**Examples**
```
# This configuration is fine because it is not public (user:jane@example.com)
resource "google_kms_key_ring" "key_ring_good_1" {
  name = "key-ring-good1"
  location = "global"
}

resource "google_kms_key_ring_iam_member" "key_ring_iam_good1" {
  crypto_key_id = google_kms_key_ring.key_ring_good_1.id
  role = "roles/cloudkms.cryptoKeyEncrypter"
  member = "user:jane@example.com"
}

# This configuration is NOT GOOD because it will make the key ring public and ALL keys inside the key inherit this IAM policy
resource "google_kms_key_ring" "key_ring_bad_1" {
  name = "key-ring-bad1"
  location = "global"
}

resource "google_kms_key_ring_iam_member" "key_ring_iam_bad_1" {
  crypto_key_id = google_kms_key_ring.key_ring_bad_1.id
  role          = "roles/cloudkms.cryptoKeyEncrypter"
  member        = "allUsers"
}
```

Hints from issue discussion:

Commit message: copy/paste fix

Patch to review:
diff --git a/checkov/terraform/checks/graph_checks/gcp/GCPKMSKeyRingsAreNotPubliclyAccessible.yaml b/checkov/terraform/checks/graph_checks/gcp/GCPKMSKeyRingsAreNotPubliclyAccessible.yaml
new file mode 100644
index 0000000000..7109f747be
--- /dev/null
+++ b/checkov/terraform/checks/graph_checks/gcp/GCPKMSKeyRingsAreNotPubliclyAccessible.yaml
@@ -0,0 +1,64 @@
+metadata:
+  id: "CKV2_GCP_8"
+  name: "Ensure that Cloud KMS Key Rings are not anonymously or publicly accessible"
+  category: "ENCRYPTION"
+definition:
+  and:
+    - cond_type: filter
+      attribute: resource_type
+      value:
+        - google_kms_key_ring
+      operator: within
+    - and:
+        - or:
+            - cond_type: connection
+              operator: not_exists
+              resource_types:
+                - google_kms_key_ring
+              connected_resource_types:
+                - google_kms_key_ring_iam_member
+            - and:
+                - cond_type: connection
+                  operator: exists
+                  resource_types:
+                    - google_kms_key_ring
+                  connected_resource_types:
+                    - google_kms_key_ring_iam_member
+                - cond_type: attribute
+                  attribute: member
+                  operator: not_equals
+                  value: "allAuthenticatedUsers"
+                  resource_types:
+                    - google_kms_key_ring_iam_member
+                - cond_type: attribute
+                  attribute: member
+                  operator: not_equals
+                  value: "allUsers"
+                  resource_types:
+                    - google_kms_key_ring_iam_member
+        - or:
+              - cond_type: connection
+                operator: not_exists
+                resource_types:
+                  - google_kms_key_ring
+                connected_resource_types:
+                  - google_kms_key_ring_iam_binding
+              - and:
+                  - cond_type: connection
+                    operator: exists
+                    resource_types:
+                      - google_kms_key_ring
+                    connected_resource_types:
+                      - google_kms_key_ring_iam_binding
+                  - cond_type: attribute
+                    attribute: members
+                    operator: not_contains
+                    value: "allAuthenticatedUsers"
+                    resource_types:
+                      - google_kms_key_ring_iam_binding
+                  - cond_type: attribute
+                    attribute: members
+                    operator: not_contains
+                    value: "allUsers"
+                    resource_types:
+                      - google_kms_key_ring_iam_binding
diff --git a/tests/terraform/graph/checks/resources/GCPKMSKeyRingsAreNotPubliclyAccessible/expected.yaml b/tests/terraform/graph/checks/resources/GCPKMSKeyRingsAreNotPubliclyAccessible/expected.yaml
new file mode 100644
index 0000000000..8a45b2dabe
--- /dev/null
+++ b/tests/terraform/graph/checks/resources/GCPKMSKeyRingsAreNotPubliclyAccessible/expected.yaml
@@ -0,0 +1,8 @@
+pass:
+  - "google_kms_key_ring.key_ring_good_1"
+  - "google_kms_key_ring.key_ring_good_2"
+fail:
+  - "google_kms_key_ring.key_ring_bad_1"
+  - "google_kms_key_ring.key_ring_bad_2"
+  - "google_kms_key_ring.key_ring_bad_3"
+  - "google_kms_key_ring.key_ring_bad_4"
diff --git a/tests/terraform/graph/checks/resources/GCPKMSKeyRingsAreNotPubliclyAccessible/main.tf b/tests/terraform/graph/checks/resources/GCPKMSKeyRingsAreNotPubliclyAccessible/main.tf
new file mode 100644
index 0000000000..36234731e4
--- /dev/null
+++ b/tests/terraform/graph/checks/resources/GCPKMSKeyRingsAreNotPubliclyAccessible/main.tf
@@ -0,0 +1,79 @@
+resource "google_kms_key_ring" "key_ring_good_1" {
+  name = "key-ring-good1"
+  location = "global"
+}
+
+resource "google_kms_key_ring" "key_ring_good_2" {
+  name = "key-ring-good2"
+  location = "global"
+}
+
+resource "google_kms_key_ring" "key_ring_bad_1" {
+  name = "key-ring-bad1"
+  location = "global"
+}
+
+resource "google_kms_key_ring" "key_ring_bad_2" {
+  name = "key-ring-bad2"
+  location = "global"
+}
+
+resource "google_kms_key_ring" "key_ring_bad_3" {
+  name = "key-ring-bad3"
+  location = "global"
+}
+
+resource "google_kms_key_ring" "key_ring_bad_4" {
+  name = "key-ring-bad4"
+  location = "global"
+}
+
+# Non-public IAM policies
+
+resource "google_kms_key_ring_iam_member" "key_ring_iam_good1" {
+  key_ring_id = google_kms_key_ring.key_ring_good_1.id
+  role = "roles/cloudkms.cryptoKeyEncrypter"
+  member = "user:jane@example.com"
+}
+
+resource "google_kms_key_ring_iam_binding" "key_ring_iam_good2" {
+  key_ring_id = google_kms_key_ring.key_ring_good_2.id
+  role          = "roles/cloudkms.cryptoKeyEncrypter"
+
+  members = [
+    "user:jane@example.com",
+  ]
+}
+
+# Public IAM policies
+
+resource "google_kms_key_ring_iam_member" "key_ring_iam_bad_1" {
+  key_ring_id = google_kms_key_ring.key_ring_bad_1.id
+  role          = "roles/cloudkms.cryptoKeyEncrypter"
+  member        = "allUsers"
+}
+
+resource "google_kms_key_ring_iam_member" "key_ring_iam_bad_2" {
+  key_ring_id = google_kms_key_ring.key_ring_bad_2.id
+  role          = "roles/cloudkms.cryptoKeyEncrypter"
+  member        = "allAuthenticatedUsers"
+}
+
+
+resource "google_kms_key_ring_iam_binding" "key_ring_iam_bad_3" {
+  key_ring_id = google_kms_key_ring.key_ring_bad_3.id
+  role          = "roles/cloudkms.cryptoKeyEncrypter"
+
+  members = [
+    "allUsers",
+  ]
+}
+
+resource "google_kms_key_ring_iam_binding" "key_ring_iam_bad_4" {
+  key_ring_id = google_kms_key_ring.key_ring_bad_4.id
+  role          = "roles/cloudkms.cryptoKeyEncrypter"
+
+  members = [
+    "allAuthenticatedUsers",
+  ]
+}
diff --git a/tests/terraform/graph/checks/test_yaml_policies.py b/tests/terraform/graph/checks/test_yaml_policies.py
index 037e68dc98..c88d5ec58a 100644
--- a/tests/terraform/graph/checks/test_yaml_policies.py
+++ b/tests/terraform/graph/checks/test_yaml_policies.py
@@ -202,10 +202,13 @@ def test_SQLServerAuditingRetention90Days(self):
 
     def test_AWSSSMParameterShouldBeEncrypted(self):
         self.go("AWSSSMParametershouldbeEncrypted", "AWSSSMParameterShouldBeEncrypted")
-        
+
     def test_AWSNATGatewaysshouldbeutilized(self):
         self.go("AWSNATGatewaysshouldbeutilized")
 
+    def test_GCPKMSKeyRingsAreNotPubliclyAccessible(self):
+        self.go("GCPKMSKeyRingsAreNotPubliclyAccessible")
+
     def test_registry_load(self):
         registry = Registry(parser=NXGraphCheckParser(), checks_dir=str(
             Path(__file__).parent.parent.parent.parent.parent / "checkov" / "terraform" / "checks" / "graph_checks"))


Code review comments:
diff_hunk: @@ -0,0 +1,64 @@
+metadata:
+  id: "CKV2_GCP_8"
+  name: "Ensure that Cloud KMS Key Rings are not anonymously or publicly accessible"
+  category: "ENCRYPTION"
+definition:
+  and:
+    - cond_type: filter
+      attribute: resource_type
+      value:
+        - google_kms_key_ring
+      operator: within
+    - and:
+        - or:
+            - cond_type: connection
+              operator: not_exists
+              resource_types:
+                - google_kms_key_ring
+              connected_resource_types:
+                - google_kms_key_ring_iam_member
+            - and:
+                - cond_type: connection
+                  operator: exists
+                  resource_types:
+                    - google_kms_key_ring
+                  connected_resource_types:
+                    - google_kms_key_ring_iam_member
+                - cond_type: attribute
+                  attribute: member
+                  operator: not_equals
+                  value: "allAuthenticatedUsers"
+                  resource_types:
+                    - google_kms_key_ring_iam_member
+                - cond_type: attribute
+                  attribute: member
+                  operator: not_equals
+                  value: "allUsers"
+                  resource_types:
+                    - google_kms_key_ring_iam_member
+        - or:
+              - cond_type: connection
+                operator: not_exists
+                resource_types:
+                  - google_kms_key_ring
+                connected_resource_types:
+                  - google_kms_key_ring_iam_binding
+              - and:
+                  - cond_type: connection
+                    operator: exists
+                    resource_types:
+                      - google_kms_key_ring
+                    connected_resource_types:
+                      - google_kms_key_ring_iam_binding
+                  - cond_type: attribute
+                    attribute: members
+                    operator: not_contains
+                    value: "allAuthenticatedUsers"
+                    resource_types:
+                      - google_kms_key_ring_iam_binding
+                  - cond_type: attribute
+                    attribute: members
+                    operator: not_contains
+                    value: "allUsers"
+                    resource_types:
+                      - google_kms_key_ring_iam_binding
path: checkov/terraform/checks/graph_checks/gcp/GCPKMSKeyRingsAreNotPubliclyAccessible.yaml
text: @user1:
```suggestion
    - or:
        - cond_type: connection
          operator: not_exists
          resource_types:
            - google_kms_key_ring
          connected_resource_types:
            - google_kms_key_ring_iam_member
        - and:
            - cond_type: connection
              operator: exists
              resource_types:
                - google_kms_key_ring
              connected_resource_types:
                - google_kms_key_ring_iam_member
            - cond_type: attribute
              attribute: member
              operator: not_equals
              value: "allAuthenticatedUsers"
              resource_types:
                - google_kms_key_ring_iam_member
            - cond_type: attribute
              attribute: member
              operator: not_equals
              value: "allUsers"
              resource_types:
                - google_kms_key_ring_iam_member
    - or:
        - cond_type: connection
          operator: not_exists
          resource_types:
            - google_kms_key_ring
          connected_resource_types:
            - google_kms_key_ring_iam_binding
        - and:
            - cond_type: connection
              operator: exists
              resource_types:
                - google_kms_key_ring
              connected_resource_types:
                - google_kms_key_ring_iam_binding
            - cond_type: attribute
              attribute: members
              operator: not_contains
              value: "allAuthenticatedUsers"
              resource_types:
                - google_kms_key_ring_iam_binding
            - cond_type: attribute
              attribute: members
              operator: not_contains
              value: "allUsers"
              resource_types:
                - google_kms_key_ring_iam_binding
```
that `and` is not needed 🙂 

@author:
Oh sweet! No problem - I copied a previous example.