Rework GCP-001 (and add new raw_key check) (#846)

* Rework GCP-001 - resolves #137

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),
+				)
 			}
 		},
 	})

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),
+				)
+			}
+
+		},
+	})
+}

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,
 		},

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)
+		})
+	}
+
+}