fix: Fix detection of default service account usage on GCP compute in…

…stance (#853) * fix: Fix detection of default service account usage on GCP compute instance Resolves #821 Signed-off-by: Liam Galvin <[email protected]>
aquasecurity · Aug 16, 2022 · db4834d · db4834d
1 parent 4ec703d
commit db4834d
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 30 deletions.
diff --git a/internal/adapters/terraform/google/compute/adapt_test.go b/internal/adapters/terraform/google/compute/adapt_test.go
@@ -141,8 +141,8 @@ func TestLines(t *testing.T) {
 	assert.Equal(t, 22, instance.ShieldedVM.SecureBootEnabled.GetMetadata().Range().GetStartLine())
 	assert.Equal(t, 22, instance.ShieldedVM.SecureBootEnabled.GetMetadata().Range().GetEndLine())
 
-	assert.Equal(t, 32, instance.ServiceAccount.GetMetadata().Range().GetStartLine())
-	assert.Equal(t, 35, instance.ServiceAccount.GetMetadata().Range().GetEndLine())
+	assert.Equal(t, 11, instance.ServiceAccount.GetMetadata().Range().GetStartLine())
+	assert.Equal(t, 43, instance.ServiceAccount.GetMetadata().Range().GetEndLine())
 
 	assert.Equal(t, 33, instance.ServiceAccount.Email.GetMetadata().Range().GetStartLine())
 	assert.Equal(t, 33, instance.ServiceAccount.Email.GetMetadata().Range().GetEndLine())

diff --git a/internal/adapters/terraform/google/compute/instances.go b/internal/adapters/terraform/google/compute/instances.go
@@ -21,9 +21,10 @@ func adaptInstances(modules terraform.Modules) (instances []compute.Instance) {
 				VTPMEnabled:                defsecTypes.BoolDefault(false, instanceBlock.GetMetadata()),
 			},
 			ServiceAccount: compute.ServiceAccount{
-				Metadata: instanceBlock.GetMetadata(),
-				Email:    defsecTypes.StringDefault("", instanceBlock.GetMetadata()),
-				Scopes:   nil,
+				Metadata:  instanceBlock.GetMetadata(),
+				Email:     defsecTypes.StringDefault("", instanceBlock.GetMetadata()),
+				IsDefault: defsecTypes.BoolDefault(false, instanceBlock.GetMetadata()),
+				Scopes:    nil,
 			},
 			CanIPForward:                instanceBlock.GetAttribute("can_ip_forward").AsBoolValueOrDefault(false, instanceBlock),
 			OSLoginEnabled:              defsecTypes.BoolDefault(true, instanceBlock.GetMetadata()),
@@ -57,11 +58,6 @@ func adaptInstances(modules terraform.Modules) (instances []compute.Instance) {
 			instance.ShieldedVM.SecureBootEnabled = shieldedBlock.GetAttribute("enable_secure_boot").AsBoolValueOrDefault(false, shieldedBlock)
 		}
 
-		if serviceAccountBlock := instanceBlock.GetBlock("service_account"); serviceAccountBlock.IsNotNil() {
-			instance.ServiceAccount.Metadata = serviceAccountBlock.GetMetadata()
-			instance.ServiceAccount.Email = serviceAccountBlock.GetAttribute("email").AsStringValueOrDefault("", serviceAccountBlock)
-		}
-
 		// metadata
 		if metadataAttr := instanceBlock.GetAttribute("metadata"); metadataAttr.IsNotNil() {
 			if val := metadataAttr.MapValue("enable-oslogin"); val.Type() == cty.Bool {
@@ -101,17 +97,22 @@ func adaptInstances(modules terraform.Modules) (instances []compute.Instance) {
 			instance.AttachedDisks = append(instance.AttachedDisks, disk)
 		}
 
-		if instanceBlock.GetBlock("service_account").IsNotNil() {
-			emailAttr := instanceBlock.GetBlock("service_account").GetAttribute("email")
-			instance.ServiceAccount.Email = emailAttr.AsStringValueOrDefault("", instanceBlock)
+		if serviceAccountBlock := instanceBlock.GetBlock("service_account"); serviceAccountBlock.IsNotNil() {
+			emailAttr := serviceAccountBlock.GetAttribute("email")
+			instance.ServiceAccount.Email = emailAttr.AsStringValueOrDefault("", serviceAccountBlock)
+
+			if instance.ServiceAccount.Email.IsEmpty() || instance.ServiceAccount.Email.EndsWith("[email protected]") {
+				instance.ServiceAccount.IsDefault = defsecTypes.Bool(true, serviceAccountBlock.GetMetadata())
+			}
 
 			if emailAttr.IsResourceBlockReference("google_service_account") {
 				if accBlock, err := modules.GetReferencedBlock(emailAttr, instanceBlock); err == nil {
-					instance.ServiceAccount.Email = defsecTypes.String(accBlock.FullName(), emailAttr.GetMetadata())
+					instance.ServiceAccount.IsDefault = defsecTypes.Bool(false, serviceAccountBlock.GetMetadata())
+					instance.ServiceAccount.Email = accBlock.GetAttribute("email").AsStringValueOrDefault("", accBlock)
 				}
 			}
 
-			if scopesAttr := instanceBlock.GetBlock("service_account").GetAttribute("scopes"); scopesAttr.IsNotNil() {
+			if scopesAttr := serviceAccountBlock.GetAttribute("scopes"); scopesAttr.IsNotNil() {
 				instance.ServiceAccount.Scopes = append(instance.ServiceAccount.Scopes, scopesAttr.AsStringValues()...)
 			}
 		}

diff --git a/internal/adapters/terraform/google/compute/instances_test.go b/internal/adapters/terraform/google/compute/instances_test.go
@@ -20,7 +20,7 @@ func Test_adaptInstances(t *testing.T) {
 		{
 			name: "defined",
 			terraform: `
-			resource "google_service_account" "default" {
+			resource "google_service_account" "myaccount" {
 			  }
 		  
 			resource "google_compute_instance" "example" {
@@ -45,7 +45,7 @@ func Test_adaptInstances(t *testing.T) {
 				  }
 
 				  service_account {
-					email  = google_service_account.default.email
+					email  = google_service_account.myaccount.email
 					scopes = ["cloud-platform"]
 				  }
 				  can_ip_forward = true
@@ -76,10 +76,11 @@ func Test_adaptInstances(t *testing.T) {
 					},
 					ServiceAccount: compute.ServiceAccount{
 						Metadata: defsecTypes.NewTestMetadata(),
-						Email:    defsecTypes.String("google_service_account.default", defsecTypes.NewTestMetadata()),
+						Email:    defsecTypes.String("", defsecTypes.NewTestMetadata()),
 						Scopes: []defsecTypes.StringValue{
 							defsecTypes.String("cloud-platform", defsecTypes.NewTestMetadata()),
 						},
+						IsDefault: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
 					},
 					CanIPForward:                defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
 					OSLoginEnabled:              defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
@@ -117,8 +118,38 @@ func Test_adaptInstances(t *testing.T) {
 						VTPMEnabled:                defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
 					},
 					ServiceAccount: compute.ServiceAccount{
-						Metadata: defsecTypes.NewTestMetadata(),
-						Email:    defsecTypes.String("", defsecTypes.NewTestMetadata()),
+						Metadata:  defsecTypes.NewTestMetadata(),
+						Email:     defsecTypes.String("", defsecTypes.NewTestMetadata()),
+						IsDefault: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+					},
+					CanIPForward:                defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+					OSLoginEnabled:              defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
+					EnableProjectSSHKeyBlocking: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+					EnableSerialPort:            defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+				},
+			},
+		},
+		{
+			name: "default service account",
+			terraform: `
+			resource "google_compute_instance" "example" {
+				service_account {}
+			}
+`,
+			expected: []compute.Instance{
+				{
+					Metadata: defsecTypes.NewTestMetadata(),
+					Name:     defsecTypes.String("", defsecTypes.NewTestMetadata()),
+					ShieldedVM: compute.ShieldedVMConfig{
+						Metadata:                   defsecTypes.NewTestMetadata(),
+						SecureBootEnabled:          defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+						IntegrityMonitoringEnabled: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+						VTPMEnabled:                defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+					},
+					ServiceAccount: compute.ServiceAccount{
+						Metadata:  defsecTypes.NewTestMetadata(),
+						Email:     defsecTypes.String("", defsecTypes.NewTestMetadata()),
+						IsDefault: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
 					},
 					CanIPForward:                defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
 					OSLoginEnabled:              defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),

diff --git a/internal/rules/google/compute/no_default_service_account.go b/internal/rules/google/compute/no_default_service_account.go
@@ -32,7 +32,7 @@ var CheckNoDefaultServiceAccount = rules.Register(
 			if instance.IsUnmanaged() {
 				continue
 			}
-			if instance.ServiceAccount.Email.IsEmpty() || instance.ServiceAccount.Email.EndsWith("[email protected]") {
+			if instance.ServiceAccount.IsDefault.IsTrue() {
 				results.Add(
 					"Instance uses the default service account.",
 					instance.ServiceAccount.Email,

diff --git a/internal/rules/google/compute/no_default_service_account_test.go b/internal/rules/google/compute/no_default_service_account_test.go
@@ -20,14 +20,15 @@ func TestCheckNoDefaultServiceAccount(t *testing.T) {
 		expected bool
 	}{
 		{
-			name: "Instance service account missing email",
+			name: "Instance service account not specified",
 			input: compute.Compute{
 				Instances: []compute.Instance{
 					{
 						Metadata: defsecTypes.NewTestMetadata(),
 						ServiceAccount: compute.ServiceAccount{
-							Metadata: defsecTypes.NewTestMetadata(),
-							Email:    defsecTypes.String("", defsecTypes.NewTestMetadata()),
+							Metadata:  defsecTypes.NewTestMetadata(),
+							Email:     defsecTypes.String("", defsecTypes.NewTestMetadata()),
+							IsDefault: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
 						},
 					},
 				},
@@ -41,8 +42,9 @@ func TestCheckNoDefaultServiceAccount(t *testing.T) {
 					{
 						Metadata: defsecTypes.NewTestMetadata(),
 						ServiceAccount: compute.ServiceAccount{
-							Metadata: defsecTypes.NewTestMetadata(),
-							Email:    defsecTypes.String("[email protected]", defsecTypes.NewTestMetadata()),
+							Metadata:  defsecTypes.NewTestMetadata(),
+							Email:     defsecTypes.String("[email protected]", defsecTypes.NewTestMetadata()),
+							IsDefault: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
 						},
 					},
 				},
@@ -56,8 +58,9 @@ func TestCheckNoDefaultServiceAccount(t *testing.T) {
 					{
 						Metadata: defsecTypes.NewTestMetadata(),
 						ServiceAccount: compute.ServiceAccount{
-							Metadata: defsecTypes.NewTestMetadata(),
-							Email:    defsecTypes.String("[email protected]", defsecTypes.NewTestMetadata()),
+							Metadata:  defsecTypes.NewTestMetadata(),
+							Email:     defsecTypes.String("[email protected]", defsecTypes.NewTestMetadata()),
+							IsDefault: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
 						},
 					},
 				},

diff --git a/pkg/providers/google/compute/instance.go b/pkg/providers/google/compute/instance.go
@@ -20,8 +20,9 @@ type Instance struct {
 
 type ServiceAccount struct {
 	defsecTypes.Metadata
-	Email  defsecTypes.StringValue
-	Scopes []defsecTypes.StringValue
+	Email     defsecTypes.StringValue
+	IsDefault defsecTypes.BoolValue
+	Scopes    []defsecTypes.StringValue
 }
 
 type NetworkInterface struct {