From e0c424a8941b30a33480fce7ae1a63f39c3a9d39 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 5 Oct 2023 20:08:14 +0700 Subject: [PATCH 1/3] fix(terraform): do not scan local modules as root modules --- pkg/scanners/terraform/parser/evaluator.go | 9 +- pkg/scanners/terraform/scanner.go | 65 ++++++++++---- pkg/scanners/terraform/scanner_test.go | 98 ++++++++++++++++++++++ pkg/terraform/module.go | 5 ++ pkg/terraform/modules.go | 10 +++ 5 files changed, 169 insertions(+), 18 deletions(-) diff --git a/pkg/scanners/terraform/parser/evaluator.go b/pkg/scanners/terraform/parser/evaluator.go index a0bc6b872..f28066671 100644 --- a/pkg/scanners/terraform/parser/evaluator.go +++ b/pkg/scanners/terraform/parser/evaluator.go @@ -154,7 +154,7 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str parseDuration += time.Since(start) e.debug.Log("Starting submodule evaluation...") - var modules []*terraform.Module + var modules terraform.Modules for _, definition := range e.loadModules(ctx) { submodules, outputs, err := definition.Parser.EvaluateAll(ctx) if err != nil { @@ -190,7 +190,12 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str e.debug.Log("Module evaluation complete.") parseDuration += time.Since(start) - return append([]*terraform.Module{terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores)}, modules...), fsMap, parseDuration + + rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) + for _, m := range modules { + m.SetParent(rootModule) + } + return append(terraform.Modules{rootModule}, modules...), fsMap, parseDuration } func (e *evaluator) expandBlocks(blocks terraform.Blocks) terraform.Blocks { diff --git a/pkg/scanners/terraform/scanner.go b/pkg/scanners/terraform/scanner.go index ffc6b222e..c08ada2c4 100644 --- a/pkg/scanners/terraform/scanner.go +++ b/pkg/scanners/terraform/scanner.go @@ -10,23 +10,20 @@ import ( "sync" "time" - "github.com/aquasecurity/defsec/pkg/types" - - "github.com/aquasecurity/defsec/pkg/framework" + "golang.org/x/exp/slices" "github.com/aquasecurity/defsec/pkg/debug" - - "github.com/aquasecurity/defsec/pkg/scanners/options" - + "github.com/aquasecurity/defsec/pkg/extrafs" + "github.com/aquasecurity/defsec/pkg/framework" "github.com/aquasecurity/defsec/pkg/rego" + "github.com/aquasecurity/defsec/pkg/scan" "github.com/aquasecurity/defsec/pkg/scanners" + "github.com/aquasecurity/defsec/pkg/scanners/options" "github.com/aquasecurity/defsec/pkg/scanners/terraform/executor" "github.com/aquasecurity/defsec/pkg/scanners/terraform/parser" "github.com/aquasecurity/defsec/pkg/scanners/terraform/parser/resolvers" - - "github.com/aquasecurity/defsec/pkg/scan" - - "github.com/aquasecurity/defsec/pkg/extrafs" + "github.com/aquasecurity/defsec/pkg/terraform" + "github.com/aquasecurity/defsec/pkg/types" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -168,6 +165,31 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return regoScanner, nil } +// terraformRootModule represents the module to be used as the root module for Terraform deployment. +type terraformRootModule struct { + rootPath string + childs terraform.Modules + fsMap map[string]fs.FS +} + +func excludeNonRootModules(modules []terraformRootModule) []terraformRootModule { + var result []terraformRootModule + var childPaths []string + + for _, module := range modules { + childPaths = append(childPaths, module.childs.ChildModulesPaths()...) + } + + for _, module := range modules { + // if the path of the root module matches the path of the child module, + // then we should not scan it + if !slices.Contains(childPaths, module.rootPath) { + result = append(result, module) + } + } + return result +} + func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir string) (scan.Results, Metrics, error) { var metrics Metrics @@ -195,14 +217,12 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin var allResults scan.Results // parse all root module directories + var rootModules []terraformRootModule for _, dir := range rootDirs { s.debug.Log("Scanning root module '%s'...", dir) p := parser.New(target, "", s.parserOpt...) - s.execLock.RLock() - e := executor.New(s.executorOpt...) - s.execLock.RUnlock() if err := p.ParseFS(ctx, dir); err != nil { return nil, metrics, err @@ -220,12 +240,25 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin metrics.Parser.Timings.DiskIODuration += parserMetrics.Timings.DiskIODuration metrics.Parser.Timings.ParseDuration += parserMetrics.Timings.ParseDuration - results, execMetrics, err := e.Execute(modules) + p.GetFilesystemMap() + rootModules = append(rootModules, terraformRootModule{ + rootPath: dir, + childs: modules, + fsMap: p.GetFilesystemMap(), + }) + } + + rootModules = excludeNonRootModules(rootModules) + + for _, module := range rootModules { + s.execLock.RLock() + e := executor.New(s.executorOpt...) + s.execLock.RUnlock() + results, execMetrics, err := e.Execute(module.childs) if err != nil { return nil, metrics, err } - fsMap := p.GetFilesystemMap() for i, result := range results { if result.Metadata().Range().GetFS() != nil { continue @@ -234,7 +267,7 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin if key == "" { continue } - if filesystem, ok := fsMap[key]; ok { + if filesystem, ok := module.fsMap[key]; ok { override := scan.Results{ result, } diff --git a/pkg/scanners/terraform/scanner_test.go b/pkg/scanners/terraform/scanner_test.go index d21cc62e2..8f546bbda 100644 --- a/pkg/scanners/terraform/scanner_test.go +++ b/pkg/scanners/terraform/scanner_test.go @@ -1006,3 +1006,101 @@ deny[res] { } } + +func TestXxx(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "/code/app1/main.tf": ` +module "s3" { + source = "./modules/s3" + bucket_name = "test" +} +`, + "/code/app1/modules/s3/main.tf": ` +variable "bucket_name" { + type = string +} + +resource "aws_s3_bucket" "main" { + bucket = var.bucket_name +} +`, + "/code/app1/app2/main.tf": ` +module "s3" { + source = "../modules/s3" + bucket_name = "test" +} + +module "ec2" { + source = "./modules/ec2" +} +`, + "/code/app1/app2/modules/ec2/main.tf": ` +variable "security_group_description" { + type = string +} +resource "aws_security_group" "main" { + description = var.security_group_description +} +`, + "/rules/bucket_name.rego": ` +# METADATA +# schemas: +# - input: schema.input +# custom: +# avd_id: AVD-AWS-0001 +# input: +# selector: +# - type: cloud +# subtypes: +# - service: s3 +# provider: aws +package defsec.test.aws1 +deny[res] { + bucket := input.aws.s3.buckets[_] + bucket.name.value == "" + res := result.new("The name of the bucket must not be empty", bucket) +} +`, + "/rules/sec_group_description.rego": ` +# METADATA +# schemas: +# - input: schema.input +# custom: +# avd_id: AVD-AWS-0002 +# input: +# selector: +# - type: cloud +# subtypes: +# - service: ec2 +# provider: aws +package defsec.test.aws2 +deny[res] { + group := input.aws.ec2.securitygroups[_] + group.description.value == "" + res := result.new("The description of the security group must not be empty", group) +} +`, + }) + + debugLog := bytes.NewBuffer([]byte{}) + scanner := New( + options.ScannerWithDebug(debugLog), + options.ScannerWithPolicyFilesystem(fs), + options.ScannerWithPolicyDirs("rules"), + options.ScannerWithEmbeddedPolicies(false), + options.ScannerWithEmbeddedLibraries(false), + options.ScannerWithRegoOnly(true), + ScannerWithAllDirectories(true), + ) + + results, err := scanner.ScanFS(context.TODO(), fs, "code") + require.NoError(t, err) + + assert.Len(t, results.GetPassed(), 2) + require.Len(t, results.GetFailed(), 1) + assert.Equal(t, "AVD-AWS-0002", results.GetFailed()[0].Rule().AVDID) + + if t.Failed() { + fmt.Printf("Debug logs:\n%s\n", debugLog.String()) + } +} diff --git a/pkg/terraform/module.go b/pkg/terraform/module.go index 5041fce74..8c1cabf57 100644 --- a/pkg/terraform/module.go +++ b/pkg/terraform/module.go @@ -11,6 +11,7 @@ type Module struct { rootPath string modulePath string ignores Ignores + parent *Module } func NewModule(rootPath string, modulePath string, blocks Blocks, ignores Ignores) *Module { @@ -32,6 +33,10 @@ func NewModule(rootPath string, modulePath string, blocks Blocks, ignores Ignore } } +func (c *Module) SetParent(parent *Module) { + c.parent = parent +} + func (c *Module) RootPath() string { return c.rootPath } diff --git a/pkg/terraform/modules.go b/pkg/terraform/modules.go index 003a2a53b..cf78726a1 100644 --- a/pkg/terraform/modules.go +++ b/pkg/terraform/modules.go @@ -8,6 +8,16 @@ import ( type Modules []*Module +func (m Modules) ChildModulesPaths() []string { + var result []string + for _, module := range m { + if module.parent != nil { + result = append(result, module.modulePath) + } + } + return result +} + type ResourceIDResolutions map[string]bool func (r ResourceIDResolutions) Resolve(id string) { From 61a81e5484078e4236649bc691379f2f216d1b47 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 5 Oct 2023 20:20:03 +0700 Subject: [PATCH 2/3] test: rename test --- pkg/scanners/terraform/scanner_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scanners/terraform/scanner_test.go b/pkg/scanners/terraform/scanner_test.go index 8f546bbda..2e5bd80ec 100644 --- a/pkg/scanners/terraform/scanner_test.go +++ b/pkg/scanners/terraform/scanner_test.go @@ -1007,7 +1007,7 @@ deny[res] { } -func TestXxx(t *testing.T) { +func Test_DoNotScanNonRootModules(t *testing.T) { fs := testutil.CreateFS(t, map[string]string{ "/code/app1/main.tf": ` module "s3" { From d9f32200224722e5b4ef7ece4efd233d36666f74 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 5 Oct 2023 20:24:07 +0700 Subject: [PATCH 3/3] run go mod tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 074bd4d66..724229985 100644 --- a/go.mod +++ b/go.mod @@ -80,6 +80,7 @@ require ( github.com/docker/docker v24.0.5+incompatible github.com/mitchellh/mapstructure v1.5.0 github.com/testcontainers/testcontainers-go v0.22.0 + golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5 ) @@ -238,7 +239,6 @@ require ( go.opentelemetry.io/otel v1.14.0 // indirect go.opentelemetry.io/otel/trace v1.14.0 // indirect go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect - golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea // indirect golang.org/x/mod v0.10.0 // indirect golang.org/x/net v0.10.0 // indirect golang.org/x/oauth2 v0.7.0 // indirect