Repository: tfsec/tfsec (go)

Issue:
GCP001 & GCP002 Checks are not applicable
## GCP001 & GCP002 Checks are not applicable
 I've looked at these two rules and believe they may not be relevant checks in their current form. Based on the documentation and some testing, google automatically encrypts disks and buckets if no client-managed encryption is specified.

In some general documentation it is stated [buckets](https://cloud.google.com/storage/docs/encryption) and [disks](https://cloud.google.com/compute/docs/disks/customer-supplied-encryption) are encrypted by default. The options both checks are looking for are used to define customer managed encryption keys.

For disks if you do create the `disk_encryption_key` block and leave it blank, it will [automatically generate keys](https://github.com/terraform-providers/terraform-provider-google/blob/5dfc9b8b72090b9a7c36cb80ad0394b2db43e108/google/resource_compute_disk.go#L274). Buckets are similar in that if you do not specify any encryption it defaults to google managed keys.

### Possible actions
Not sure if deleting a rule is the best course of action. One possibility could be modifying these to warn in the event someone is specifying customer managed keys. ie: remind them to ensure they are securely managing their keys, or to check that the specified `default_kms_key_name` is accessible where the object is being created.


Pull request:
Rework GCP-001 (and add new raw_key check)
Resolves #137

Patch:
diff --git a/internal/app/tfsec/rules/gcp001.go b/internal/app/tfsec/rules/gcp001.go
index c5c4b3678..2cf226e5f 100644
--- a/internal/app/tfsec/rules/gcp001.go
+++ b/internal/app/tfsec/rules/gcp001.go
@@ -19,20 +19,16 @@ import (
 
 // GoogleUnencryptedDisk See https://github.com/tfsec/tfsec#included-checks for check info
 const GoogleUnencryptedDisk = "GCP001"
-const GoogleUnencryptedDiskDescription = "Unencrypted compute disk."
-const GoogleUnencryptedDiskImpact = "Data could be readable if compromised"
-const GoogleUnencryptedDiskResolution = "Enable encrytion for compute disks"
+const GoogleUnencryptedDiskDescription = "Encrypted compute disk with unmanaged keys."
+const GoogleUnencryptedDiskImpact = "Encryption of disk using unmanaged keys."
+const GoogleUnencryptedDiskResolution = "Enable encrytion using a customer-managed key."
 const GoogleUnencryptedDiskExplanation = `
 By default, Compute Engine encrypts all data at rest. Compute Engine handles and manages this encryption for you without any additional actions on your part.
 
 If the <code>disk_encryption_key</code> block is included in the resource declaration then it *must* include a <code>raw_key</code> or <code>kms_key_self_link</code>.
-
-To use the default offering of Google managed keys, do not include a <code>disk_encryption_key</code> block at all.
 `
 const GoogleUnencryptedDiskBadExample = `
 resource "google_compute_disk" "bad_example" {
-	# ... 
-	disk_encryption_key {}
 	# ...
 }`
 const GoogleUnencryptedDiskGoodExample = `
@@ -42,11 +38,7 @@ resource "google_compute_disk" "good_example" {
 	}
 }
 
-resource "google_compute_disk" "good_example" {
-	disk_encryption_key {
-		raw_key = "something"
-	}
-}`
+`
 
 func init() {
 	scanner.RegisterCheckRule(rule.Rule{
@@ -70,15 +62,13 @@ func init() {
 		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
 
 			keyBlock := resourceBlock.GetBlock("disk_encryption_key")
-			if keyBlock != nil {
-				if keyBlock.GetAttribute("raw_key") == nil && keyBlock.GetAttribute("kms_key_self_link") == nil {
-					set.Add(
-						result.New(resourceBlock).
-							WithDescription(fmt.Sprintf("Resource '%s' defines an unencrypted disk. You should specify raw_key or kms_key_self_link.", resourceBlock.FullName())).
-							WithRange(keyBlock.Range()).
-							WithSeverity(severity.Error),
-					)
-				}
+			if keyBlock == nil {
+				set.Add(
+					result.New(resourceBlock).
+						WithDescription(fmt.Sprintf("Resource '%s' defines a disk encrypted with an auto-generated key.", resourceBlock.FullName())).
+						WithRange(resourceBlock.Range()).
+						WithSeverity(severity.Error),
+				)
 			}
 		},
 	})
diff --git a/internal/app/tfsec/rules/gcp013.go b/internal/app/tfsec/rules/gcp013.go
new file mode 100644
index 000000000..63a1c4136
--- /dev/null
+++ b/internal/app/tfsec/rules/gcp013.go
@@ -0,0 +1,80 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/block"
+	"github.com/tfsec/tfsec/internal/app/tfsec/hclcontext"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/tfsec/tfsec/pkg/provider"
+	"github.com/tfsec/tfsec/pkg/result"
+	"github.com/tfsec/tfsec/pkg/rule"
+	"github.com/tfsec/tfsec/pkg/severity"
+)
+
+const GCPRawEncryptionKeySpecifiedForComputeDisk = "GCP013"
+const GCPRawEncryptionKeySpecifiedForComputeDiskDescription = "The encryption key used to encrypt a compute disk has been specified in plaintext."
+const GCPRawEncryptionKeySpecifiedForComputeDiskImpact = "The encryption key should be considered compromised as it is not stored securely."
+const GCPRawEncryptionKeySpecifiedForComputeDiskResolution = "Reference a managed key rather than include the key in raw format."
+const GCPRawEncryptionKeySpecifiedForComputeDiskExplanation = `
+Sensitve values such as raw encryption keys should not be included in your Terraform code, and should be stored securely by a secrets manager.
+`
+const GCPRawEncryptionKeySpecifiedForComputeDiskBadExample = `
+resource "google_compute_disk" "good_example" {
+	disk_encryption_key {
+		raw_key="b2ggbm8gdGhpcyBpcyBiYWQ="
+	}
+}
+`
+const GCPRawEncryptionKeySpecifiedForComputeDiskGoodExample = `
+resource "google_compute_disk" "good_example" {
+	disk_encryption_key {
+		kms_key_self_link = google_kms_crypto_key.my_crypto_key.id
+	}
+}
+`
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		ID: GCPRawEncryptionKeySpecifiedForComputeDisk,
+		Documentation: rule.RuleDocumentation{
+			Summary:     GCPRawEncryptionKeySpecifiedForComputeDiskDescription,
+			Explanation: GCPRawEncryptionKeySpecifiedForComputeDiskExplanation,
+			Impact:      GCPRawEncryptionKeySpecifiedForComputeDiskImpact,
+			Resolution:  GCPRawEncryptionKeySpecifiedForComputeDiskResolution,
+			BadExample:  GCPRawEncryptionKeySpecifiedForComputeDiskBadExample,
+			GoodExample: GCPRawEncryptionKeySpecifiedForComputeDiskGoodExample,
+			Links: []string{
+                              "https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_disk#kms_key_self_link",
+				"https://cloud.google.com/compute/docs/disks/customer-supplied-encryption",
+			},
+		},
+		Provider:        provider.GCPProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"google_compute_disk"},
+		DefaultSeverity: severity.Error,
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+
+			keyBlock := resourceBlock.GetBlock("disk_encryption_key")
+			if keyBlock == nil {
+				return
+			}
+
+			rawKeyAttr := keyBlock.GetAttribute("raw_key")
+			if rawKeyAttr == nil {
+				return
+			}
+
+			if rawKeyAttr.IsString() {
+				set.Add(
+					result.New(resourceBlock).
+						WithDescription(fmt.Sprintf("Resource '%s' specifies an encryption key in raw format.", resourceBlock.FullName())).
+						WithRange(rawKeyAttr.Range()).
+						WithAttributeAnnotation(rawKeyAttr).
+						WithSeverity(severity.Error),
+				)
+			}
+
+		},
+	})
+}


Test patch:
diff --git a/internal/app/tfsec/test/gcp001_test.go b/internal/app/tfsec/test/gcp001_test.go
index 50a00b36a..d8706ca39 100644
--- a/internal/app/tfsec/test/gcp001_test.go
+++ b/internal/app/tfsec/test/gcp001_test.go
@@ -18,7 +18,6 @@ func Test_GoogleUnencryptedDisk(t *testing.T) {
 			name: "check google_compute_disk with empty disk_encryption_key block",
 			source: `
 resource "google_compute_disk" "my-disk" {
-	disk_encryption_key {}
 }`,
 			mustIncludeResultCode: rules.GoogleUnencryptedDisk,
 		},
diff --git a/internal/app/tfsec/test/gcp013_test.go b/internal/app/tfsec/test/gcp013_test.go
new file mode 100644
index 000000000..9e15f2197
--- /dev/null
+++ b/internal/app/tfsec/test/gcp013_test.go
@@ -0,0 +1,48 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/rules"
+)
+
+func Test_GCPRawEncryptionKeySpecifiedForComputeDisk(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "Fails with raw key supplied",
+			source: `
+resource "google_compute_disk" "good_example" {
+	disk_encryption_key {
+		raw_key="b2ggbm8gdGhpcyBpcyBiYWQ="
+	}
+}
+`,
+			mustIncludeResultCode: rules.GCPRawEncryptionKeySpecifiedForComputeDisk,
+		},
+		{
+			name: "Passes without raw key",
+			source: `
+resource "google_compute_disk" "good_example" {
+	disk_encryption_key {
+		kms_key_self_link = google_kms_crypto_key.my_crypto_key.id
+	}
+}
+`,
+			mustExcludeResultCode: rules.GCPRawEncryptionKeySpecifiedForComputeDisk,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanHCL(test.source, t)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}