From ae0360ee4d18e318a5253b65aad891c01b85d37e Mon Sep 17 00:00:00 2001 From: Liam Galvin Date: Tue, 27 Jul 2021 22:51:40 +0100 Subject: [PATCH] Add remaining google_project_iam_member rules (#958) Co-authored-by: Owen Rumney --- Makefile | 2 +- ...default_service_account_assignment_rule.go | 131 ++++++++++++++++++ ...lt_service_account_assignment_rule_test.go | 53 +++++++ ...evel_service_account_impersonation_rule.go | 74 ++++++++++ ...service_account_impersonation_rule_test.go | 53 +++++++ 5 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule.go create mode 100644 internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule_test.go create mode 100644 internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule.go create mode 100644 internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule_test.go diff --git a/Makefile b/Makefile index 54f38bc94f..f24e3785b6 100644 --- a/Makefile +++ b/Makefile @@ -72,7 +72,7 @@ clone-image: .PHONY: sanity sanity: test - go run ./cmd/tfsec -s -p --force-all-dirs ./example + go run ./cmd/tfsec -s -p --force-all-dirs ./example > /dev/null .PHONY: pr-lint pr-lint: diff --git a/internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule.go b/internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule.go new file mode 100644 index 0000000000..4fb11cf01c --- /dev/null +++ b/internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule.go @@ -0,0 +1,131 @@ +package iam + +import ( + "fmt" + "strings" + + "github.com/aquasecurity/tfsec/pkg/result" + "github.com/aquasecurity/tfsec/pkg/severity" + + "github.com/aquasecurity/tfsec/pkg/provider" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/hclcontext" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/block" + + "github.com/aquasecurity/tfsec/pkg/rule" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/scanner" +) + +func init() { + scanner.RegisterCheckRule(rule.Rule{ + Service: "iam", + ShortCode: "no-default-service-account-assignment", + Documentation: rule.RuleDocumentation{ + Summary: "Roles should not be assigned to default service accounts", + Explanation: `Default service accounts should not be used - consider creating specialised service accounts for individual purposes.`, + Impact: "Violation of principal of least privilege", + Resolution: "Use specialised service accounts for specific purposes.", + BadExample: []string{` +resource "google_project_iam_member" "project-123" { + project = "project-123" + role = "roles/whatever" + member = "123-compute@developer.gserviceaccount.com" +} +`, + ` +resource "google_project_iam_member" "project-123" { + project = "project-123" + role = "roles/whatever" + member = "123@appspot.gserviceaccount.com" +} +`, ` +data "google_compute_default_service_account" "default" { +} + +resource "google_project_iam_member" "project-123" { + project = "project-123" + role = "roles/whatever" + member = data.google_compute_default_service_account.default.id +} +`, + }, + GoodExample: []string{` +resource "google_service_account" "test" { + account_id = "account123" + display_name = "account123" +} + +resource "google_project_iam_member" "project-123" { + project = "project-123" + role = "roles/whatever" + member = "serviceAccount:${google_service_account.test.email}" +} +`, + }, + Links: []string{ + "https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam", + "", + }, + }, + Provider: provider.GoogleProvider, + RequiredTypes: []string{"resource"}, + RequiredLabels: []string{"google_project_iam_binding,", "google_project_iam_member"}, + DefaultSeverity: severity.Medium, + CheckFunc: func(set result.Set, resourceBlock block.Block, ctx *hclcontext.Context) { + + if memberAttr := resourceBlock.GetAttribute("member"); memberAttr != nil { + if memberAttr.IsString() && memberAttr.Value().IsKnown() { + if isMemberDefaultServiceAccount(memberAttr.Value().AsString()) { + set.Add( + result.New(resourceBlock). + WithAttribute(memberAttr). + WithDescription(fmt.Sprintf("Resource '%s' assigns a role to a default service account.", resourceBlock.FullName())), + ) + } + } else { + computeServiceAccounts := ctx.GetDatasByType("google_compute_default_service_account") + serviceAccounts := append(computeServiceAccounts, ctx.GetResourcesByType("google_app_engine_default_service_account")...) + for _, serviceAccount := range serviceAccounts { + if memberAttr.ReferencesBlock(serviceAccount) { + set.Add( + result.New(resourceBlock). + WithAttribute(memberAttr). + WithDescription(fmt.Sprintf("Resource '%s' assigns a role to a default service account.", resourceBlock.FullName())), + ) + } + } + } + } + + if membersAttr := resourceBlock.GetAttribute("members"); membersAttr != nil { + for _, member := range membersAttr.ValueAsStrings() { + if isMemberDefaultServiceAccount(member) { + set.Add( + result.New(resourceBlock). + WithAttribute(membersAttr). + WithDescription(fmt.Sprintf("Resource '%s' assigns a role to a default service account.", resourceBlock.FullName())), + ) + } + } + computeServiceAccounts := ctx.GetDatasByType("google_compute_default_service_account") + serviceAccounts := append(computeServiceAccounts, ctx.GetResourcesByType("google_app_engine_default_service_account")...) + for _, serviceAccount := range serviceAccounts { + if membersAttr.ReferencesBlock(serviceAccount) { + set.Add( + result.New(resourceBlock). + WithAttribute(membersAttr). + WithDescription(fmt.Sprintf("Resource '%s' assigns a role to a default service account.", resourceBlock.FullName())), + ) + } + } + } + + }, + }) +} + +func isMemberDefaultServiceAccount(member string) bool { + return strings.HasSuffix(member, "-compute@developer.gserviceaccount.com") || strings.HasSuffix(member, "@appspot.gserviceaccount.com") +} diff --git a/internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule_test.go b/internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule_test.go new file mode 100644 index 0000000000..ba13976b0a --- /dev/null +++ b/internal/app/tfsec/rules/google/iam/no_default_service_account_assignment_rule_test.go @@ -0,0 +1,53 @@ +package iam + +import ( + "strings" + "testing" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/scanner" + "github.com/aquasecurity/tfsec/internal/app/tfsec/testutil" +) + +func Test_GoogleNoDefaultServiceAccountAssignment_FailureExamples(t *testing.T) { + expectedCode := "google-iam-no-default-service-account-assignment" + + check, err := scanner.GetRuleById(expectedCode) + if err != nil { + t.FailNow() + } + for i, badExample := range check.Documentation.BadExample { + t.Logf("Running bad example for '%s' #%d", expectedCode, i+1) + if strings.TrimSpace(badExample) == "" { + t.Fatalf("bad example code not provided for %s", check.ID()) + } + defer func() { + if err := recover(); err != nil { + t.Fatalf("Scan (bad) failed: %s", err) + } + }() + results := testutil.ScanHCL(badExample, t) + testutil.AssertCheckCode(t, check.ID(), "", results) + } +} + +func Test_GoogleNoDefaultServiceAccountAssignment_SuccessExamples(t *testing.T) { + expectedCode := "google-iam-no-default-service-account-assignment" + + check, err := scanner.GetRuleById(expectedCode) + if err != nil { + t.FailNow() + } + for i, example := range check.Documentation.GoodExample { + t.Logf("Running good example for '%s' #%d", expectedCode, i+1) + if strings.TrimSpace(example) == "" { + t.Fatalf("good example code not provided for %s", check.ID()) + } + defer func() { + if err := recover(); err != nil { + t.Fatalf("Scan (good) failed: %s", err) + } + }() + results := testutil.ScanHCL(example, t) + testutil.AssertCheckCode(t, "", check.ID(), results) + } +} diff --git a/internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule.go b/internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule.go new file mode 100644 index 0000000000..234fd2e911 --- /dev/null +++ b/internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule.go @@ -0,0 +1,74 @@ +package iam + +import ( + "fmt" + + "github.com/aquasecurity/tfsec/pkg/result" + "github.com/aquasecurity/tfsec/pkg/severity" + + "github.com/aquasecurity/tfsec/pkg/provider" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/hclcontext" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/block" + + "github.com/aquasecurity/tfsec/pkg/rule" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/scanner" +) + +func init() { + scanner.RegisterCheckRule(rule.Rule{ + Service: "iam", + ShortCode: "no-project-level-service-account-impersonation", + Documentation: rule.RuleDocumentation{ + Summary: "Users should not be granted service account access at the project level", + Explanation: `Users with service account access at project level can impersonate any service account. Instead, they should be given access to particular service accounts as required.`, + Impact: "Privilege escalation, impersonation of any/all services", + Resolution: "Provide access at the service-level instead of project-level, if required", + BadExample: []string{ + ` +resource "google_project_iam_binding" "project-123" { + project = "project-123" + role = "roles/iam.serviceAccountUser" +} +`, + ` +resource "google_project_iam_binding" "project-123" { + project = "project-123" + role = "roles/iam.serviceAccountTokenCreator" +} +`, + }, + GoodExample: []string{` +resource "google_project_iam_binding" "project-123" { + project = "project-123" + role = "roles/nothingInParticular" +} + `}, + Links: []string{ + "https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam", + "https://cloud.google.com/iam/docs/impersonating-service-accounts", + }, + }, + Provider: provider.GoogleProvider, + RequiredTypes: []string{"resource"}, + RequiredLabels: []string{"google_project_iam_binding", "google_project_iam_member"}, + DefaultSeverity: severity.Medium, + CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) { + + roleAttr := resourceBlock.GetAttribute("role") + if roleAttr == nil || !roleAttr.IsString() || !roleAttr.Value().IsKnown() { + return + } + if roleAttr.Value().AsString() == "roles/iam.serviceAccountUser" || roleAttr.Value().AsString() == "roles/iam.serviceAccountTokenCreator" { + set.Add( + result.New(resourceBlock). + WithAttribute(roleAttr). + WithDescription(fmt.Sprintf("Resource '%s' grants service account access to a user at project level.", resourceBlock.FullName())), + ) + } + + }, + }) +} diff --git a/internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule_test.go b/internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule_test.go new file mode 100644 index 0000000000..6f177a7fcd --- /dev/null +++ b/internal/app/tfsec/rules/google/iam/no_project_level_service_account_impersonation_rule_test.go @@ -0,0 +1,53 @@ +package iam + +import ( + "strings" + "testing" + + "github.com/aquasecurity/tfsec/internal/app/tfsec/scanner" + "github.com/aquasecurity/tfsec/internal/app/tfsec/testutil" +) + +func Test_GoogleNoProjectLevelServiceAccountAccess_FailureExamples(t *testing.T) { + expectedCode := "google-iam-no-project-level-service-account-impersonation" + + check, err := scanner.GetRuleById(expectedCode) + if err != nil { + t.FailNow() + } + for i, badExample := range check.Documentation.BadExample { + t.Logf("Running bad example for '%s' #%d", expectedCode, i+1) + if strings.TrimSpace(badExample) == "" { + t.Fatalf("bad example code not provided for %s", check.ID()) + } + defer func() { + if err := recover(); err != nil { + t.Fatalf("Scan (bad) failed: %s", err) + } + }() + results := testutil.ScanHCL(badExample, t) + testutil.AssertCheckCode(t, check.ID(), "", results) + } +} + +func Test_GoogleNoProjectLevelServiceAccountAccess_SuccessExamples(t *testing.T) { + expectedCode := "google-iam-no-project-level-service-account-impersonation" + + check, err := scanner.GetRuleById(expectedCode) + if err != nil { + t.FailNow() + } + for i, example := range check.Documentation.GoodExample { + t.Logf("Running good example for '%s' #%d", expectedCode, i+1) + if strings.TrimSpace(example) == "" { + t.Fatalf("good example code not provided for %s", check.ID()) + } + defer func() { + if err := recover(); err != nil { + t.Fatalf("Scan (good) failed: %s", err) + } + }() + results := testutil.ScanHCL(example, t) + testutil.AssertCheckCode(t, "", check.ID(), results) + } +}