From 8d37d201a9c94b8df62be98506f4432ea0824d66 Mon Sep 17 00:00:00 2001 From: Martin Petkov Date: Fri, 9 Jun 2023 11:25:56 +0000 Subject: [PATCH 1/4] Add new rule: workload identity pool providers without conditions --- .../adapters/terraform/google/iam/adapt.go | 13 +-- .../terraform/google/iam/adapt_test.go | 55 +++++++++---- .../iam/workload_identity_pool_providers.go | 18 ++++ pkg/providers/google/iam/iam.go | 10 ++- ...ions_on_workload_identity_pool_provider.go | 45 ++++++++++ ...s_on_workload_identity_pool_provider.tf.go | 65 +++++++++++++++ ...on_workload_identity_pool_provider_test.go | 82 +++++++++++++++++++ 7 files changed, 267 insertions(+), 21 deletions(-) create mode 100644 internal/adapters/terraform/google/iam/workload_identity_pool_providers.go create mode 100644 rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go create mode 100644 rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.tf.go create mode 100644 rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider_test.go diff --git a/internal/adapters/terraform/google/iam/adapt.go b/internal/adapters/terraform/google/iam/adapt.go index 50f8b24e0..45d082af9 100644 --- a/internal/adapters/terraform/google/iam/adapt.go +++ b/internal/adapters/terraform/google/iam/adapt.go @@ -15,10 +15,11 @@ func Adapt(modules terraform.Modules) iam.IAM { } type adapter struct { - modules terraform.Modules - orgs map[string]iam.Organization - folders []parentedFolder - projects []parentedProject + modules terraform.Modules + orgs map[string]iam.Organization + folders []parentedFolder + projects []parentedProject + workloadIdentityPoolProviders []iam.WorkloadIdentityPoolProvider } func (a *adapter) Adapt() iam.IAM { @@ -27,6 +28,7 @@ func (a *adapter) Adapt() iam.IAM { a.adaptFolderIAM() a.adaptProjects() a.adaptProjectIAM() + a.adaptWorkloadIdentityPoolProviders() return a.merge() } @@ -96,7 +98,8 @@ FOLDER_ORG: } output := iam.IAM{ - Organizations: nil, + Organizations: nil, + WorkloadIdentityPoolProviders: a.workloadIdentityPoolProviders, } for _, org := range a.orgs { output.Organizations = append(output.Organizations, org) diff --git a/internal/adapters/terraform/google/iam/adapt_test.go b/internal/adapters/terraform/google/iam/adapt_test.go index 4b85d863c..00b75594c 100644 --- a/internal/adapters/terraform/google/iam/adapt_test.go +++ b/internal/adapters/terraform/google/iam/adapt_test.go @@ -23,38 +23,38 @@ func Test_Adapt(t *testing.T) { terraform: ` data "google_organization" "org" { domain = "example.com" - } + } - resource "google_project" "my_project" { + resource "google_project" "my_project" { name = "My Project" project_id = "your-project-id" org_id = data.google_organization.org.id auto_create_network = true - } + } - resource "google_folder" "department1" { + resource "google_folder" "department1" { display_name = "Department 1" parent = data.google_organization.org.id - } + } - resource "google_folder_iam_member" "admin" { + resource "google_folder_iam_member" "admin" { folder = google_folder.department1.name role = "roles/editor" member = "user:alice@gmail.com" - } + } resource "google_folder_iam_binding" "folder-123" { folder = google_folder.department1.name role = "roles/nothing" members = [ "user:not-alice@gmail.com", - ] + ] } resource "google_organization_iam_member" "org-123" { - org_id = data.google_organization.org.id - role = "roles/whatever" - member = "user:member@gmail.com" + org_id = data.google_organization.org.id + role = "roles/whatever" + member = "user:member@gmail.com" } resource "google_organization_iam_binding" "binding" { @@ -64,7 +64,13 @@ func Test_Adapt(t *testing.T) { members = [ "user:member_2@gmail.com", ] - } + } + + resource "google_iam_workload_identity_pool_provider" "example" { + workload_identity_pool_id = "example-pool" + workload_identity_pool_provider_id = "example-provider" + attribute_condition = "assertion.repository_owner=='your-github-organization'" + } `, expected: iam.IAM{ Organizations: []iam.Organization{ @@ -120,6 +126,15 @@ func Test_Adapt(t *testing.T) { }, }, }, + WorkloadIdentityPoolProviders: []iam.WorkloadIdentityPoolProvider{ + { + Metadata: defsecTypes.NewTestMetadata(), + + WorkloadIdentityPoolId: defsecTypes.String("example-pool", defsecTypes.NewTestMetadata()), + WorkloadIdentityPoolProviderId: defsecTypes.String("example-provider", defsecTypes.NewTestMetadata()), + AttributeCondition: defsecTypes.String("assertion.repository_owner=='your-github-organization'", defsecTypes.NewTestMetadata()), + }, + }, }, }, } @@ -166,9 +181,9 @@ func TestLines(t *testing.T) { } resource "google_organization_iam_member" "org-123" { - org_id = data.google_organization.org.id - role = "roles/whatever" - member = "user:member@gmail.com" + org_id = data.google_organization.org.id + role = "roles/whatever" + member = "user:member@gmail.com" } resource "google_organization_iam_binding" "binding" { @@ -178,6 +193,12 @@ func TestLines(t *testing.T) { members = [ "user:member_2@gmail.com", ] + } + + resource "google_iam_workload_identity_pool_provider" "example" { + workload_identity_pool_id = "example-pool" + workload_identity_pool_provider_id = "example-provider" + attribute_condition = "assertion.repository_owner=='your-github-organization'" }` modules := tftestutil.CreateModulesFromSource(t, src, ".tf") @@ -188,11 +209,13 @@ func TestLines(t *testing.T) { require.Len(t, adapted.Organizations[0].Folders, 1) require.Len(t, adapted.Organizations[0].Bindings, 1) require.Len(t, adapted.Organizations[0].Members, 1) + require.Len(t, adapted.WorkloadIdentityPoolProviders, 1) project := adapted.Organizations[0].Projects[0] folder := adapted.Organizations[0].Folders[0] binding := adapted.Organizations[0].Bindings[0] member := adapted.Organizations[0].Members[0] + pool := adapted.WorkloadIdentityPoolProviders[0] assert.Equal(t, 6, project.Metadata.Range().GetStartLine()) assert.Equal(t, 11, project.Metadata.Range().GetEndLine()) @@ -238,4 +261,6 @@ func TestLines(t *testing.T) { assert.Equal(t, 42, binding.Members[0].GetMetadata().Range().GetStartLine()) assert.Equal(t, 44, binding.Members[0].GetMetadata().Range().GetEndLine()) + + assert.Equal(t, 51, pool.Metadata.Range().GetEndLine()) } diff --git a/internal/adapters/terraform/google/iam/workload_identity_pool_providers.go b/internal/adapters/terraform/google/iam/workload_identity_pool_providers.go new file mode 100644 index 000000000..70d68511a --- /dev/null +++ b/internal/adapters/terraform/google/iam/workload_identity_pool_providers.go @@ -0,0 +1,18 @@ +package iam + +import ( + "github.com/aquasecurity/defsec/pkg/providers/google/iam" +) + +// See https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/iam_workload_identity_pool_provider + +func (a *adapter) adaptWorkloadIdentityPoolProviders() { + for _, resource := range a.modules.GetResourcesByType("google_iam_workload_identity_pool_provider") { + a.workloadIdentityPoolProviders = append(a.workloadIdentityPoolProviders, iam.WorkloadIdentityPoolProvider{ + Metadata: resource.GetMetadata(), + WorkloadIdentityPoolId: resource.GetAttribute("workload_identity_pool_id").AsStringValueOrDefault("", resource), + WorkloadIdentityPoolProviderId: resource.GetAttribute("workload_identity_pool_provider_id").AsStringValueOrDefault("", resource), + AttributeCondition: resource.GetAttribute("attribute_condition").AsStringValueOrDefault("", resource), + }) + } +} diff --git a/pkg/providers/google/iam/iam.go b/pkg/providers/google/iam/iam.go index fd3fca4ab..0e981335e 100755 --- a/pkg/providers/google/iam/iam.go +++ b/pkg/providers/google/iam/iam.go @@ -5,7 +5,8 @@ import ( ) type IAM struct { - Organizations []Organization + Organizations []Organization + WorkloadIdentityPoolProviders []WorkloadIdentityPoolProvider } type Organization struct { @@ -45,6 +46,13 @@ type Member struct { DefaultServiceAccount defsecTypes.BoolValue } +type WorkloadIdentityPoolProvider struct { + Metadata defsecTypes.Metadata + WorkloadIdentityPoolId defsecTypes.StringValue + WorkloadIdentityPoolProviderId defsecTypes.StringValue + AttributeCondition defsecTypes.StringValue +} + func (p *IAM) AllProjects() []Project { var projects []Project for _, org := range p.Organizations { diff --git a/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go new file mode 100644 index 000000000..e1fed31c2 --- /dev/null +++ b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go @@ -0,0 +1,45 @@ +package iam + +import ( + "github.com/aquasecurity/defsec/internal/rules" + "github.com/aquasecurity/defsec/pkg/providers" + "github.com/aquasecurity/defsec/pkg/scan" + "github.com/aquasecurity/defsec/pkg/severity" + "github.com/aquasecurity/defsec/pkg/state" +) + +var CheckNoConditionOnWorkloadIdentityPoolProvider = rules.Register( + scan.Rule{ + AVDID: "AVD-GCP-0068", + Provider: providers.GoogleProvider, + Service: "iam", + ShortCode: "no-conditions-workload-identity-pool-provider", + Summary: "A configuration for an external workload identity pool provider should have conditions set", + Impact: "Allows an external attacker to authenticate as the attached service account and act with its permissions", + Resolution: "Set conditions on this provider, for example by restricting it to only be allowed from repositories in your GitHub organization", + Explanation: `In GitHub Actions, one can authenticate to Google Cloud by setting values for workload_identity_provider and service_account and requesting a short-lived OIDC token which is then used to execute commands as that Service Account. If you don't specify a condition in the workload identity provider pool configuration, then any GitHub Action can assume this role and act as that Service Account.`, + Links: []string{ + "https://www.revblock.dev/exploiting-misconfigured-google-cloud-service-accounts-from-github-actions/", + }, + Terraform: &scan.EngineMetadata{ + GoodExamples: terraformNoConditionOnWorkloadIdentityPoolProviderGoodExamples, + BadExamples: terraformNoConditionOnWorkloadIdentityPoolProviderBadExamples, + Links: terraformNoConditionOnWorkloadIdentityPoolProviderLinks, + RemediationMarkdown: terraformNoConditionOnWorkloadIdentityPoolProviderMarkdown, + }, + Severity: severity.High, + }, + func(s *state.State) (results scan.Results) { + for _, provider := range s.Google.IAM.WorkloadIdentityPoolProviders { + if provider.AttributeCondition.IsEmpty() { + results.Add( + "Project has automatic network creation enabled.", + provider.AttributeCondition, + ) + } else { + results.AddPassed(provider) + } + } + return + }, +) diff --git a/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.tf.go b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.tf.go new file mode 100644 index 000000000..4eb8dbfdc --- /dev/null +++ b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.tf.go @@ -0,0 +1,65 @@ +package iam + +var terraformNoConditionOnWorkloadIdentityPoolProviderGoodExamples = []string{ + ` + resource "google_iam_workload_identity_pool" "github" { + provider = google + project = data.google_project.project.project_id + workload_identity_pool_id = "github" + } + + resource "google_iam_workload_identity_pool_provider" "github" { + provider = google + project = data.google_project.project.project_id + workload_identity_pool_id = google_iam_workload_identity_pool.github-actions[0].workload_identity_pool_id + workload_identity_pool_provider_id = "github" + + attribute_condition = "assertion.repository_owner=='your-github-organization'" + + attribute_mapping = { + "google.subject" = "assertion.sub" + "attribute.actor" = "assertion.actor" + "attribute.aud" = "assertion.aud" + "attribute.repository" = "assertion.repository" + } + + oidc { + issuer_uri = "https://token.actions.githubusercontent.com" + } + } + `, +} + +var terraformNoConditionOnWorkloadIdentityPoolProviderBadExamples = []string{ + ` + resource "google_iam_workload_identity_pool" "github" { + provider = google + project = data.google_project.project.project_id + workload_identity_pool_id = "github" + } + + resource "google_iam_workload_identity_pool_provider" "github" { + provider = google + project = data.google_project.project.project_id + workload_identity_pool_id = google_iam_workload_identity_pool.github-actions[0].workload_identity_pool_id + workload_identity_pool_provider_id = "github" + + attribute_mapping = { + "google.subject" = "assertion.sub" + "attribute.actor" = "assertion.actor" + "attribute.aud" = "assertion.aud" + "attribute.repository" = "assertion.repository" + } + + oidc { + issuer_uri = "https://token.actions.githubusercontent.com" + } + } + `, +} + +var terraformNoConditionOnWorkloadIdentityPoolProviderLinks = []string{ + `https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/iam_workload_identity_pool_provider#attribute_condition`, +} + +var terraformNoConditionOnWorkloadIdentityPoolProviderMarkdown = `` diff --git a/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider_test.go b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider_test.go new file mode 100644 index 000000000..9cc729018 --- /dev/null +++ b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider_test.go @@ -0,0 +1,82 @@ +package iam + +import ( + "testing" + + defsecTypes "github.com/aquasecurity/defsec/pkg/types" + + "github.com/aquasecurity/defsec/pkg/state" + + "github.com/aquasecurity/defsec/pkg/providers/google/iam" + "github.com/aquasecurity/defsec/pkg/scan" + + "github.com/stretchr/testify/assert" +) + +func TestCheckNoConditionOnWorkloadIdentityPoolProvider(t *testing.T) { + tests := []struct { + name string + input iam.IAM + expected bool + }{ + { + name: "Workload identity pool without condition", + input: iam.IAM{ + WorkloadIdentityPoolProviders: []iam.WorkloadIdentityPoolProvider{ + { + Metadata: defsecTypes.NewTestMetadata(), + WorkloadIdentityPoolId: defsecTypes.String("example-pool", defsecTypes.NewTestMetadata()), + WorkloadIdentityPoolProviderId: defsecTypes.String("example-provider", defsecTypes.NewTestMetadata()), + }, + }, + }, + expected: true, + }, + { + name: "Workload identity pool with empty condition", + input: iam.IAM{ + WorkloadIdentityPoolProviders: []iam.WorkloadIdentityPoolProvider{ + { + Metadata: defsecTypes.NewTestMetadata(), + WorkloadIdentityPoolId: defsecTypes.String("example-pool", defsecTypes.NewTestMetadata()), + WorkloadIdentityPoolProviderId: defsecTypes.String("example-provider", defsecTypes.NewTestMetadata()), + AttributeCondition: defsecTypes.String("", defsecTypes.NewTestMetadata()), + }, + }, + }, + expected: true, + }, + { + name: "Workload identity pool with non-empty condition", + input: iam.IAM{ + WorkloadIdentityPoolProviders: []iam.WorkloadIdentityPoolProvider{ + { + Metadata: defsecTypes.NewTestMetadata(), + WorkloadIdentityPoolId: defsecTypes.String("example-pool", defsecTypes.NewTestMetadata()), + WorkloadIdentityPoolProviderId: defsecTypes.String("example-provider", defsecTypes.NewTestMetadata()), + AttributeCondition: defsecTypes.String("assertion.repository_owner=='your-github-organization'", defsecTypes.NewTestMetadata()), + }, + }, + }, + expected: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var testState state.State + testState.Google.IAM = test.input + results := CheckNoConditionOnWorkloadIdentityPoolProvider.Evaluate(&testState) + var found bool + for _, result := range results { + if result.Status() == scan.StatusFailed && result.Rule().LongID() == CheckNoConditionOnWorkloadIdentityPoolProvider.Rule().LongID() { + found = true + } + } + if test.expected { + assert.True(t, found, "Rule should have been found") + } else { + assert.False(t, found, "Rule should not have been found") + } + }) + } +} From 9b6eb97039410961ca73b831137aa0b067403a55 Mon Sep 17 00:00:00 2001 From: Martin Petkov Date: Fri, 9 Jun 2023 11:56:36 +0000 Subject: [PATCH 2/4] Add docs --- avd_docs/google/iam/AVD-GCP-0068/Terraform.md | 24 +++++++++++++++++++ avd_docs/google/iam/AVD-GCP-0068/docs.md | 11 +++++++++ 2 files changed, 35 insertions(+) create mode 100644 avd_docs/google/iam/AVD-GCP-0068/Terraform.md create mode 100644 avd_docs/google/iam/AVD-GCP-0068/docs.md diff --git a/avd_docs/google/iam/AVD-GCP-0068/Terraform.md b/avd_docs/google/iam/AVD-GCP-0068/Terraform.md new file mode 100644 index 000000000..7f3559514 --- /dev/null +++ b/avd_docs/google/iam/AVD-GCP-0068/Terraform.md @@ -0,0 +1,24 @@ + +Set conditions on this provider, for example by restricting it to only be allowed from repositories in your GitHub organization. + +```hcl + resource "google_iam_workload_identity_pool_provider" "github" { + project = "example-project" + workload_identity_pool_id = "example-pool" + workload_identity_pool_provider_id = "example-provider" + + attribute_condition = "assertion.repository_owner=='your-github-organization'" + + attribute_mapping = { + "google.subject" = "assertion.sub" + "attribute.actor" = "assertion.actor" + "attribute.aud" = "assertion.aud" + "attribute.repository" = "assertion.repository" + } + } +``` + +#### Remediation Links + +- https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/iam_workload_identity_pool_provider#attribute_condition + diff --git a/avd_docs/google/iam/AVD-GCP-0068/docs.md b/avd_docs/google/iam/AVD-GCP-0068/docs.md new file mode 100644 index 000000000..993768913 --- /dev/null +++ b/avd_docs/google/iam/AVD-GCP-0068/docs.md @@ -0,0 +1,11 @@ + +In GitHub Actions, one can authenticate to Google Cloud by setting values for workload_identity_provider and service_account and requesting a short-lived OIDC token which is then used to execute commands as that Service Account. If you don't specify a condition in the workload identity provider pool configuration, then any GitHub Action can assume this role and act as that Service Account. + +### Impact +Privilege escalation, impersonation of service accounts + + +{{ remediationActions }} + +### Links +- https://www.revblock.dev/exploiting-misconfigured-google-cloud-service-accounts-from-github-actions/ From 654ef18e4841d7694f36d6d055ffe63c57c56466 Mon Sep 17 00:00:00 2001 From: Martin Petkov Date: Sat, 10 Jun 2023 04:34:49 +0000 Subject: [PATCH 3/4] Run make schema --- pkg/rego/schemas/cloud.json | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pkg/rego/schemas/cloud.json b/pkg/rego/schemas/cloud.json index db34a3a2a..8241cf676 100644 --- a/pkg/rego/schemas/cloud.json +++ b/pkg/rego/schemas/cloud.json @@ -5375,6 +5375,13 @@ "type": "object", "$ref": "#/definitions/jackfan.us.kg.aquasecurity.defsec.pkg.providers.google.iam.Organization" } + }, + "workloadidentitypoolproviders": { + "type": "array", + "items": { + "type": "object", + "$ref": "#/definitions/jackfan.us.kg.aquasecurity.defsec.pkg.providers.google.iam.WorkloadIdentityPoolProvider" + } } } }, @@ -5451,6 +5458,23 @@ } } }, + "jackfan.us.kg.aquasecurity.defsec.pkg.providers.google.iam.WorkloadIdentityPoolProvider": { + "type": "object", + "properties": { + "attributecondition": { + "type": "object", + "$ref": "#/definitions/jackfan.us.kg.aquasecurity.defsec.pkg.types.StringValue" + }, + "workloadidentitypoolid": { + "type": "object", + "$ref": "#/definitions/jackfan.us.kg.aquasecurity.defsec.pkg.types.StringValue" + }, + "workloadidentitypoolproviderid": { + "type": "object", + "$ref": "#/definitions/jackfan.us.kg.aquasecurity.defsec.pkg.types.StringValue" + } + } + }, "jackfan.us.kg.aquasecurity.defsec.pkg.providers.google.kms.KMS": { "type": "object", "properties": { From d3c31bff66b140bc9033a8052b17bcc95e8fac70 Mon Sep 17 00:00:00 2001 From: Martin Petkov Date: Wed, 14 Jun 2023 13:16:36 -0400 Subject: [PATCH 4/4] Update no_conditions_on_workload_identity_pool_provider.go Fix copy-paste error for the result message. --- .../iam/no_conditions_on_workload_identity_pool_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go index e1fed31c2..0640fbec3 100644 --- a/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go +++ b/rules/cloud/policies/google/iam/no_conditions_on_workload_identity_pool_provider.go @@ -33,7 +33,7 @@ var CheckNoConditionOnWorkloadIdentityPoolProvider = rules.Register( for _, provider := range s.Google.IAM.WorkloadIdentityPoolProviders { if provider.AttributeCondition.IsEmpty() { results.Add( - "Project has automatic network creation enabled.", + "This workload identity pool provider configuration has no conditions set.", provider.AttributeCondition, ) } else {