From 102b6df7389f93229dfdb4463bb18986cb7ef432 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Tue, 12 Mar 2024 17:48:16 +0300 Subject: [PATCH] fix(terraform): fix policy document retrieval (#6276) --- pkg/iac/adapters/terraform/aws/iam/convert.go | 55 ++-- .../terraform/aws/iam/policies_test.go | 240 +++++++++++++++++- .../adapters/terraform/aws/iam/roles_test.go | 58 ++++- 3 files changed, 314 insertions(+), 39 deletions(-) diff --git a/pkg/iac/adapters/terraform/aws/iam/convert.go b/pkg/iac/adapters/terraform/aws/iam/convert.go index 56992b77bcb2..3a61791e5fb9 100644 --- a/pkg/iac/adapters/terraform/aws/iam/convert.go +++ b/pkg/iac/adapters/terraform/aws/iam/convert.go @@ -16,18 +16,7 @@ type wrappedDocument struct { } func ParsePolicyFromAttr(attr *terraform.Attribute, owner *terraform.Block, modules terraform.Modules) (*iam.Document, error) { - - documents := findAllPolicies(modules, owner, attr) - if len(documents) > 0 { - return &iam.Document{ - Parsed: documents[0].Document, - Metadata: documents[0].Source.GetMetadata(), - IsOffset: true, - }, nil - } - if attr.IsString() { - dataBlock, err := modules.GetBlockById(attr.Value().AsString()) if err != nil { parsed, err := iamgo.Parse([]byte(unescapeVars(attr.Value().AsString()))) @@ -40,7 +29,9 @@ func ParsePolicyFromAttr(attr *terraform.Attribute, owner *terraform.Block, modu IsOffset: false, HasRefs: len(attr.AllReferences()) > 0, }, nil - } else if dataBlock.Type() == "data" && dataBlock.TypeLabel() == "aws_iam_policy_document" { + } + + if dataBlock.Type() == "data" && dataBlock.TypeLabel() == "aws_iam_policy_document" { if doc, err := ConvertTerraformDocument(modules, dataBlock); err == nil { return &iam.Document{ Metadata: dataBlock.GetMetadata(), @@ -75,7 +66,7 @@ func ConvertTerraformDocument(modules terraform.Modules, block *terraform.Block) } if sourceDocumentsAttr := block.GetAttribute("source_policy_documents"); sourceDocumentsAttr.IsIterable() { - docs := findAllPolicies(modules, block, sourceDocumentsAttr) + docs := findAllPolicies(modules, sourceDocumentsAttr) for _, doc := range docs { statements, _ := doc.Document.Statements() for _, statement := range statements { @@ -100,7 +91,7 @@ func ConvertTerraformDocument(modules terraform.Modules, block *terraform.Block) } if overrideDocumentsAttr := block.GetAttribute("override_policy_documents"); overrideDocumentsAttr.IsIterable() { - docs := findAllPolicies(modules, block, overrideDocumentsAttr) + docs := findAllPolicies(modules, overrideDocumentsAttr) for _, doc := range docs { statements, _ := doc.Document.Statements() for _, statement := range statements { @@ -208,30 +199,24 @@ func parseStatement(statementBlock *terraform.Block) iamgo.Statement { return builder.Build() } -func findAllPolicies(modules terraform.Modules, parentBlock *terraform.Block, attr *terraform.Attribute) []wrappedDocument { +func findAllPolicies(modules terraform.Modules, attr *terraform.Attribute) []wrappedDocument { var documents []wrappedDocument - for _, ref := range attr.AllReferences() { - for _, b := range modules.GetBlocks() { - if b.Type() != "data" || b.TypeLabel() != "aws_iam_policy_document" { - continue - } - if ref.RefersTo(b.Reference()) { - document, err := ConvertTerraformDocument(modules, b) - if err != nil { - continue - } - documents = append(documents, *document) - continue - } - kref := *ref - kref.SetKey(parentBlock.Reference().RawKey()) - if kref.RefersTo(b.Reference()) { - document, err := ConvertTerraformDocument(modules, b) - if err != nil { - continue - } + + if !attr.IsIterable() { + return documents + } + + policyDocIDs := attr.AsStringValues().AsStrings() + for _, policyDocID := range policyDocIDs { + if policyDoc, err := modules.GetBlockById(policyDocID); err == nil { + if document, err := ConvertTerraformDocument(modules, policyDoc); err == nil { documents = append(documents, *document) } + } else if parsed, err := iamgo.Parse([]byte(unescapeVars(policyDocID))); err == nil { + documents = append(documents, wrappedDocument{ + Document: *parsed, + Source: attr, + }) } } return documents diff --git a/pkg/iac/adapters/terraform/aws/iam/policies_test.go b/pkg/iac/adapters/terraform/aws/iam/policies_test.go index 5e70f4da8bec..aa6b9f9b59f5 100644 --- a/pkg/iac/adapters/terraform/aws/iam/policies_test.go +++ b/pkg/iac/adapters/terraform/aws/iam/policies_test.go @@ -138,11 +138,10 @@ data "aws_iam_policy_document" "this" { } } - resource "aws_iam_policy" "this" { for_each = local.sqs - name = "test-${each.key}" - policy = data.aws_iam_policy_document.this[each.key].json + name = "test-${each.key}" + policy = data.aws_iam_policy_document.this[each.key].json }`, expected: []iam.Policy{ { @@ -169,6 +168,241 @@ resource "aws_iam_policy" "this" { }, }, }, + { + name: "policy_document with source_policy_documents", + terraform: ` +data "aws_iam_policy_document" "source" { + statement { + actions = ["ec2:*"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "source_document_example" { + source_policy_documents = [data.aws_iam_policy_document.source.json] + + statement { + actions = ["s3:*"] + + resources = [ + "arn:aws:s3:::somebucket", + "arn:aws:s3:::somebucket/*", + ] + } +} + +resource "aws_iam_policy" "this" { + name = "test-policy" + policy = data.aws_iam_policy_document.source_document_example.json +}`, + expected: []iam.Policy{ + { + Name: iacTypes.String("test-policy", iacTypes.NewTestMetadata()), + Builtin: iacTypes.Bool(false, iacTypes.NewTestMetadata()), + Document: func() iam.Document { + builder := iamgo.NewPolicyBuilder() + firstStatement := iamgo.NewStatementBuilder(). + WithActions([]string{"ec2:*"}). + WithResources([]string{"*"}). + WithEffect("Allow"). + Build() + + builder.WithStatement(firstStatement) + + secondStatement := iamgo.NewStatementBuilder(). + WithActions([]string{"s3:*"}). + WithResources([]string{"arn:aws:s3:::somebucket", "arn:aws:s3:::somebucket/*"}). + WithEffect("Allow"). + Build() + + builder.WithStatement(secondStatement) + + return iam.Document{ + Parsed: builder.Build(), + Metadata: iacTypes.NewTestMetadata(), + IsOffset: true, + HasRefs: false, + } + }(), + }, + }, + }, + { + name: "source_policy_documents with for-each", + terraform: ` +locals { + versions = ["2008-10-17", "2012-10-17"] +} + +resource "aws_iam_policy" "test_policy" { + name = "test-policy" + policy = data.aws_iam_policy_document.policy.json +} + +data "aws_iam_policy_document" "policy" { + source_policy_documents = [for s in data.aws_iam_policy_document.policy_source : s.json if s.version != "2008-10-17"] + statement { + actions = ["s3:*"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "policy_source" { + for_each = toset(local.versions) + version = each.value + statement { + actions = ["s3:PutObject"] + resources = ["*"] + } +}`, + expected: []iam.Policy{ + { + Name: iacTypes.String("test-policy", iacTypes.NewTestMetadata()), + Document: func() iam.Document { + builder := iamgo.NewPolicyBuilder(). + WithStatement( + iamgo.NewStatementBuilder(). + WithActions([]string{"s3:PutObject"}). + WithResources([]string{"*"}). + WithEffect("Allow"). + Build(), + ). + WithStatement( + iamgo.NewStatementBuilder(). + WithActions([]string{"s3:*"}). + WithResources([]string{"*"}). + WithEffect("Allow"). + Build(), + ) + + return iam.Document{ + Parsed: builder.Build(), + Metadata: iacTypes.NewTestMetadata(), + IsOffset: true, + HasRefs: false, + } + }(), + }, + }, + }, + { + name: "source_policy_documents with condition", + terraform: ` +locals { + versions = ["2008-10-17", "2012-10-17"] +} + +resource "aws_iam_policy" "test_policy" { + name = "test-policy" + policy = data.aws_iam_policy_document.policy.json +} + +data "aws_iam_policy_document" "policy" { + source_policy_documents = true ? [data.aws_iam_policy_document.policy_source.json] : [data.aws_iam_policy_document.policy_source2.json] +} + +data "aws_iam_policy_document" "policy_source" { + statement { + actions = ["s3:PutObject"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "policy_source2" { + statement { + actions = ["s3:PutObject2"] + resources = ["*"] + } +} +`, + expected: []iam.Policy{ + { + Name: iacTypes.String("test-policy", iacTypes.NewTestMetadata()), + Document: func() iam.Document { + builder := iamgo.NewPolicyBuilder(). + WithStatement( + iamgo.NewStatementBuilder(). + WithActions([]string{"s3:PutObject"}). + WithResources([]string{"*"}). + WithEffect("Allow"). + Build(), + ) + + return iam.Document{ + Parsed: builder.Build(), + Metadata: iacTypes.NewTestMetadata(), + IsOffset: true, + HasRefs: false, + } + }(), + }, + }, + }, + { + name: "raw source policy", + terraform: `resource "aws_iam_policy" "test_policy" { + name = "test-policy" + policy = data.aws_iam_policy_document.policy.json +} + +data "aws_iam_policy_document" "policy" { + source_policy_documents = [ + jsonencode({ + Statement = [ + { + Action = [ + "ec2:Describe*", + ] + Effect = "Allow" + Resource = "*" + }, + ] + }), + ] +} +`, + expected: []iam.Policy{ + { + Name: iacTypes.String("test-policy", iacTypes.NewTestMetadata()), + Document: func() iam.Document { + builder := iamgo.NewPolicyBuilder(). + WithStatement( + iamgo.NewStatementBuilder(). + WithActions([]string{"ec2:Describe*"}). + WithResources([]string{"*"}). + WithEffect("Allow"). + Build(), + ) + + return iam.Document{ + Parsed: builder.Build(), + Metadata: iacTypes.NewTestMetadata(), + IsOffset: true, + HasRefs: false, + } + }(), + }, + }, + }, + { + name: "invalid `override_policy_documents` attribute", + terraform: `resource "aws_iam_policy" "test_policy" { + name = "test-policy" + policy = data.aws_iam_policy_document.policy.json +} + +data "aws_iam_policy_document" "policy" { + source_policy_documents = data.aws_iam_policy_document.policy2.json +}`, + expected: []iam.Policy{ + { + Name: iacTypes.String("test-policy", iacTypes.NewTestMetadata()), + Document: iam.Document{ + IsOffset: true, + }, + }, + }, + }, } for _, test := range tests { diff --git a/pkg/iac/adapters/terraform/aws/iam/roles_test.go b/pkg/iac/adapters/terraform/aws/iam/roles_test.go index bd7058f74efa..61dfc1facb08 100644 --- a/pkg/iac/adapters/terraform/aws/iam/roles_test.go +++ b/pkg/iac/adapters/terraform/aws/iam/roles_test.go @@ -8,6 +8,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/adapters/terraform/tftestutil" "github.com/aquasecurity/trivy/pkg/iac/providers/aws/iam" iacTypes "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/liamg/iamgo" ) func Test_adaptRoles(t *testing.T) { @@ -170,7 +171,7 @@ resource "aws_iam_policy" "this" { for_each = local.roles name = format("%s-policy", each.key) description = "A test policy" - policy = data.aws_iam_policy_document.this.json + policy = data.aws_iam_policy_document.this[each.key].json } resource "aws_iam_role_policy_attachment" "this" { @@ -204,6 +205,61 @@ resource "aws_iam_role_policy_attachment" "this" { }, }, }, + { + name: "policy with condition", + terraform: ` +resource "aws_iam_role_policy" "test_policy" { + name = "test_policy" + role = aws_iam_role.test_role.id + policy = false ? data.aws_iam_policy_document.s3_policy.json : data.aws_iam_policy_document.s3_policy_one.json +} + +resource "aws_iam_role" "test_role" { + name = "test_role" + assume_role_policy = "" +} + +data "aws_iam_policy_document" "s3_policy_one" { + statement { + actions = ["s3:PutObject"] + resources = ["*"] + } +} + +data "aws_iam_policy_document" "s3_policy" { + statement { + actions = ["s3:CreateBucket"] + resources = ["*"] + } +}`, + expected: []iam.Role{ + { + Name: iacTypes.String("test_role", iacTypes.NewTestMetadata()), + Policies: []iam.Policy{ + { + Name: iacTypes.String("test_policy", iacTypes.NewTestMetadata()), + Builtin: iacTypes.Bool(false, iacTypes.NewTestMetadata()), + Document: func() iam.Document { + builder := iamgo.NewPolicyBuilder() + sb := iamgo.NewStatementBuilder() + sb.WithEffect(iamgo.EffectAllow) + sb.WithActions([]string{"s3:PutObject"}) + sb.WithResources([]string{"*"}) + + builder.WithStatement(sb.Build()) + + return iam.Document{ + Parsed: builder.Build(), + Metadata: iacTypes.NewTestMetadata(), + IsOffset: true, + HasRefs: false, + } + }(), + }, + }, + }, + }, + }, } for _, test := range tests {