Repository: aquasecurity/tfsec (go)

Issue:
GKE Client Certificates
You've recently added a check for GKE client certificates.

```
  Result 6
  [google-gke-use-client-cert-auth][HIGH] Resource 'google_container_cluster.redacted' does not have master_auth.client_certificate_config.issue_client_certificate set to true
  /builds/QNB_T93t/0/uc/redacted/terraform/gcp.tf:306
     303 |     password = ""
     304 | 
     305 |     client_certificate_config {
     306 |       issue_client_certificate = false    bool: false
     307 |     }
     308 |   }
     309 | 
  Impact:     Less secure authentication method in use
  Resolution: Use client certificates for authentication
  More Info:
  - https://tfsec.dev/docs/google/gke/use-client-cert-auth#google/gke 
  - https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#issue_client_certificate 
```

According to the GKE hardening guide, client certificates are deprecated and OAuth tokens should be used.

https://cloud.google.com/kubernetes-engine/docs/how-to/hardening-your-cluster#restrict_authn_methods



Pull request:
Update GKE check to ensure legacy auth is not used
Resolves #1022 

Patch:
diff --git a/internal/app/tfsec/rules/google/gke/use_client_cert_auth_rule.go b/internal/app/tfsec/rules/google/gke/no_legacy_auth_rule.go
similarity index 79%
rename from internal/app/tfsec/rules/google/gke/use_client_cert_auth_rule.go
rename to internal/app/tfsec/rules/google/gke/no_legacy_auth_rule.go
index 9844c1e85..546e1d9f0 100644
--- a/internal/app/tfsec/rules/google/gke/use_client_cert_auth_rule.go
+++ b/internal/app/tfsec/rules/google/gke/no_legacy_auth_rule.go
@@ -17,7 +17,7 @@ func init() {
 	scanner.RegisterCheckRule(rule.Rule{
 		Provider:  provider.GoogleProvider,
 		Service:   "gke",
-		ShortCode: "use-client-cert-auth",
+		ShortCode: "no-legacy-auth",
 		Documentation: rule.RuleDocumentation{
 			Summary:     "Clusters should use client certificates for authentication",
 			Explanation: `Client certificates are the most secure and recommended method of authentication.`,
@@ -29,7 +29,7 @@ resource "google_service_account" "default" {
   display_name = "Service Account"
 }
 
-resource "google_container_cluster" "bad_example" {
+resource "google_container_cluster" "good_example" {
   name     = "my-gke-cluster"
   location = "us-central1"
 
@@ -40,7 +40,7 @@ resource "google_container_cluster" "bad_example" {
   initial_node_count       = 1
   master_auth {
     client_certificate_config {
-      issue_client_certificate = false
+      issue_client_certificate = true
     }
   }
 }
@@ -78,11 +78,6 @@ resource "google_container_cluster" "good_example" {
   # node pool and immediately delete it.
   remove_default_node_pool = true
   initial_node_count       = 1
-  master_auth {
-    client_certificate_config {
-      issue_client_certificate = true
-    }
-  }
 }
 
 resource "google_container_node_pool" "primary_preemptible_nodes" {
@@ -104,7 +99,8 @@ resource "google_container_node_pool" "primary_preemptible_nodes" {
 }
 `},
 			Links: []string{
-				"https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#issue_client_certificate",
+				"https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/container_cluster#master_auth",
+				"https://cloud.google.com/kubernetes-engine/docs/how-to/hardening-your-cluster#restrict_authn_methods",
 			},
 		},
 		RequiredTypes: []string{
@@ -115,14 +111,20 @@ resource "google_container_node_pool" "primary_preemptible_nodes" {
 		},
 		DefaultSeverity: severity.High,
 		CheckFunc: func(set result.Set, resourceBlock block.Block, _ block.Module) {
-			if issueClientCertificateAttr := resourceBlock.GetBlock("master_auth").GetBlock("client_certificate_config").GetAttribute("issue_client_certificate"); issueClientCertificateAttr.IsNil() { // alert on use of default value
-				set.AddResult().
-					WithDescription("Resource '%s' uses default value for master_auth.client_certificate_config.issue_client_certificate", resourceBlock.FullName())
-			} else if issueClientCertificateAttr.IsFalse() {
+			masterAuthBlock := resourceBlock.GetBlock("master_auth")
+			if masterAuthBlock.IsNil() {
+				return
+			}
+			if issueClientCertificateAttr := masterAuthBlock.GetBlock("client_certificate_config").GetAttribute("issue_client_certificate"); issueClientCertificateAttr.IsTrue() {
 				set.AddResult().
-					WithDescription("Resource '%s' does not have master_auth.client_certificate_config.issue_client_certificate set to true", resourceBlock.FullName()).
+					WithDescription("Resource '%s' uses client certificates which are no longer recommended", resourceBlock.FullName()).
 					WithAttribute(issueClientCertificateAttr)
 			}
+			if usernameAttr := masterAuthBlock.GetAttribute("username"); usernameAttr.IsString() {
+				set.AddResult().
+					WithDescription("Resource '%s' uses basic auth which is not recommended", resourceBlock.FullName()).
+					WithAttribute(usernameAttr)
+			}
 		},
 	})
 }


Test patch:
diff --git a/internal/app/tfsec/rules/google/gke/use_client_cert_auth_rule_test.go b/internal/app/tfsec/rules/google/gke/no_legacy_auth_rule_test.go
similarity index 93%
rename from internal/app/tfsec/rules/google/gke/use_client_cert_auth_rule_test.go
rename to internal/app/tfsec/rules/google/gke/no_legacy_auth_rule_test.go
index 326752658..c09f9ced8 100644
--- a/internal/app/tfsec/rules/google/gke/use_client_cert_auth_rule_test.go
+++ b/internal/app/tfsec/rules/google/gke/no_legacy_auth_rule_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func Test_GoogleUseClientCertAuth_FailureExamples(t *testing.T) {
-	expectedCode := "google-gke-use-client-cert-auth"
+	expectedCode := "google-gke-no-legacy-auth"
 
 	rule, err := scanner.GetRuleById(expectedCode)
 	if err != nil {
@@ -31,7 +31,7 @@ func Test_GoogleUseClientCertAuth_FailureExamples(t *testing.T) {
 }
 
 func Test_GoogleUseClientCertAuth_SuccessExamples(t *testing.T) {
-	expectedCode := "google-gke-use-client-cert-auth"
+	expectedCode := "google-gke-no-legacy-auth"
 
 	rule, err := scanner.GetRuleById(expectedCode)
 	if err != nil {