aquasecurity / trivy

Find vulnerabilities, misconfigurations, secrets, SBOM in containers, Kubernetes, code repositories, clouds and more
https://aquasecurity.github.io/trivy
Apache License 2.0
23.11k stars 2.28k forks source link

bug(misconf): Inconsistent in the issue count If terraform variables are not passed #7099

Closed simar7 closed 1 month ago

simar7 commented 3 months ago

Discussed in https://github.com/aquasecurity/trivy/discussions/7095

Originally posted by **cybersa** July 4, 2024 ### Description There is a inconsistent in the issue count if terraform variables are not passed. For Example: Consider this terraform script main.tf: ``` variable "public_subnets" { type = map( object({ cidr = string, az = string, }) ) description = "List of Public Subnets" } locals { env = "development" vpc_cidr_block = "10.241.4.0/22" } # VPC resource "aws_vpc" "vpc" { cidr_block = local.vpc_cidr_block enable_dns_support = true enable_dns_hostnames = true tags = { Name = "${local.env}-vpc" } } #Public Subnets resource "aws_subnet" "public_subnet" { for_each = var.public_subnets vpc_id = aws_vpc.vpc.id cidr_block = each.value["cidr"] availability_zone = each.value["az"] tags = { Name = "${local.env}-${each.key}-public-subnet" } map_public_ip_on_launch = true } ``` If I run trivy scan against above main.tf, there is no HIGH and CRITICAL issues. `trivy config --misconfig-scanners terraform -s "HIGH,CRITICAL" main.tf` If I pass below tfvars file into trivy config, there are 3 CRITICAL issues. vars.tfvars ``` public_subnets = { public-1 = { "cidr" = "10.241.7.0/26", "az" = "ca-central-1a" } public-2 = { "cidr" = "10.241.7.64/26", "az" = "ca-central-1b" } public-3 = { "cidr" = "10.241.7.128/26", "az" = "ca-central-1d" } } ``` command: `trivy config --misconfig-scanners terraform --tf-vars vars.tfvars -s "HIGH,CRITICAL" main.tf` Above command returns 3 CRITICAL issues. ### Desired Behavior Scan result should be consistent even if tfvars file is not passed. Because issue is on this line `map_public_ip_on_launch = true` and it is hardcoded in the terraform script. ### Actual Behavior Scan result is inconsistent. ### Reproduction Steps ```bash Reproduction steps are in the Description field. ``` ### Target Filesystem ### Scanner Misconfiguration ### Output Format None ### Mode Standalone ### Debug Output ```bash 2024-07-04T11:40:15+05:30 DEBUG Parsed severities severities=[HIGH CRITICAL] 2024-07-04T11:40:15+05:30 INFO Misconfiguration scanning is enabled 2024-07-04T11:40:15+05:30 DEBUG Policies successfully loaded from disk 2024-07-04T11:40:15+05:30 DEBUG Enabling misconfiguration scanners scanners=[terraform] 2024-07-04T11:40:15+05:30 DEBUG Initializing scan cache... type="memory" 2024-07-04T11:40:15+05:30 DEBUG [nuget] The nuget packages directory couldn't be found. License search disabled 2024-07-04T11:40:15+05:30 DEBUG Scanning files for misconfigurations... scanner="Terraform" 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.433552948 terraform.scanner Scanning [&{%!s(*mapfs.file=&{ [] {. 256 2147484096 {13950371612937561924 505949398 0x794e200} } {{{0 0} {[] {} 0xc0016760c0} map[vpc.tf:0xc0010d8110] 0}}}) }] at '.'... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.437246868 terraform.scanner.rego Overriding filesystem for checks! 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.439702865 terraform.scanner.rego Loaded 3 embedded libraries. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.497012263 terraform.scanner.rego Loaded 192 embedded policies. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.565207190 terraform.scanner.rego Loaded 195 checks from disk. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.565611084 terraform.scanner.rego Overriding filesystem for data! 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922492006 terraform.parser. Setting project/module root to '.' 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922519645 terraform.parser. Parsing FS from '.' 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922541973 terraform.parser. Parsing 'vpc.tf'... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922752099 terraform.parser. Added file vpc.tf. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922910728 terraform.scanner Scanning root module '.'... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922922567 terraform.parser. Setting project/module root to '.' 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922930861 terraform.parser. Parsing FS from '.' 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.922943454 terraform.parser. Parsing 'vpc.tf'... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923088870 terraform.parser. Added file vpc.tf. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923106798 terraform.parser. Evaluating module... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923225096 terraform.parser. Read 4 block(s) and 0 ignore(s) for module 'root' (1 file[s])... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923244432 terraform.parser. Added 0 variables from tfvars. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923261689 terraform.parser. Working directory for module evaluation is "/data/projects/miraterra/git/miraterrasoil-terraform/temp" 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923300053 terraform.parser..evaluator Filesystem key is 'b244507b0e4bf3b8e9c2c21a83d8ee7f6d1e6d810848458f51583decf4f5009d' 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923310735 terraform.parser..evaluator Starting module evaluation... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923539318 terraform.parser..evaluator Starting submodule evaluation... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923552198 terraform.parser..evaluator All submodules are evaluated at i=0 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923560535 terraform.parser..evaluator Starting post-submodule evaluation... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923650707 terraform.parser..evaluator Finished processing 0 submodule(s). 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923659649 terraform.parser..evaluator Module evaluation complete. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923669714 terraform.parser. Finished parsing module 'root'. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923678436 terraform.executor Adapting modules... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923754102 terraform.executor Adapted 1 module(s) into defsec state data. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923764380 terraform.executor Using max routines of 7 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923851292 terraform.executor Initialized 487 rule(s). 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.923859807 terraform.executor Created pool with 7 worker(s) to apply rules. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.924266210 terraform.scanner.rego Scanning 1 inputs... 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.927061100 terraform.executor Finished applying rules. 2024-07-04T11:40:15+05:30 DEBUG [misconf] 40:15.927084483 terraform.executor Applying ignores... 2024-07-04T11:40:15+05:30 DEBUG OS is not detected. 2024-07-04T11:40:15+05:30 INFO Detected config files num=2 2024-07-04T11:40:15+05:30 DEBUG Scanned config file path="." 2024-07-04T11:40:15+05:30 DEBUG Scanned config file path="vpc.tf" ``` ### Operating System Ubuntu 20.04 ### Version ```bash Version: 0.53.0 Vulnerability DB: Version: 2 UpdatedAt: 2023-09-11 06:16:57.742189926 +0000 UTC NextUpdate: 2023-09-11 12:16:57.742189326 +0000 UTC DownloadedAt: 2023-09-11 07:08:10.751619881 +0000 UTC Check Bundle: Digest: sha256:ef2d9ad4fce0f933b20a662004d7e55bf200987c180e7f2cd531af631f408bb3 DownloadedAt: 2024-07-03 11:55:33.672405891 +0000 UTC ``` ### Checklist - [X] Run `trivy clean --all` - [X] Read [the troubleshooting](https://aquasecurity.github.io/trivy/latest/docs/references/troubleshooting/)
nikpivkin commented 3 months ago

I don't agree that this is a problem. Since we can't evaluate the for-each expression, we can't be sure if instances of this block exist to make assumptions about its misconfiguration. Terraform considers such a configuration incomplete and requires all variables from the user.

simar7 commented 3 months ago

I don't agree that this is a problem. Since we can't evaluate the for-each expression, we can't be sure if instances of this block exist to make assumptions about its misconfiguration. Terraform considers such a configuration incomplete and requires all variables from the user.

Hmm you're right, I thought we could support the case where we evaluate what we can (excluding the missing tf-vars for-each). But it seems as you mentioned that terraform deems it to be an invalid configuration so it's probably best that we follow the same.

I do think however, we should show a log output to help the user understand that their input has an issue. Silently dropping looks misleading as it signals we were unable to find any misconfiguration. WDYT?

nikpivkin commented 3 months ago

@simar7 I agree about better logging. There are a couple of other places where the output could be improved.

For example here:

2024-07-04T11:40:15+05:30   DEBUG   [misconf] 40:15.433552948 terraform.scanner                Scanning [&{%!s(*mapfs.file=&{ [] {. 256 2147484096 {13950371612937561924 505949398 0x794e200} <nil>} {{{0 0} {[] {} 0xc0016760c0} map[vpc.tf:0xc0010d8110] 0}}}) }] at '.'...
acdha commented 2 months ago

I do think however, we should show a log output to help the user understand that their input has an issue. Silently dropping looks misleading as it signals we were unable to find any misconfiguration. WDYT?

I think a tool like Trivy has to prominently fail when there's any issue with the inputs. I just happened across this ticket while refactoring some code which used the remote backend to use the HTTP backend, and suddenly started getting a ton of “CRITICAL” warnings from things like network ACLs which had been in use for years, which was puzzling as builds on our main branches were not reporting any issues. After spending some time bisecting my changes, the key factor appears to be a few places where we were using terraform.workspace, which I had replaced since the HTTP backend does not support that concept. That apparently caused Trivy silently omit those checks, which I confirmed by modifying a copy of the passing branch to replace terraform.workspace references with a local.environment value. Terraform evaluates both of those identically, but with the hardcoded value Trivy evaluated those resources for the first time.

Fortunately they're false-positives but that could potentially have been serious. This was also complicated by the fact that these were managed by the terraform-aws-vpc module so the errors are indirect and difficult to suppress (I'm working on a separate thread about that).

nikpivkin commented 2 months ago

Hi @acdha ! Could you please provide an example using terraform.workspace for which no problems were found for further investigation?

acdha commented 2 months ago

Hi @acdha ! Could you please provide an example using terraform.workspace for which no problems were found for further investigation?

I'm working on that but it's a large project and since Trivy takes ~70 seconds to scan it the reduction process is taking longer than I'd hoped. I'll update next week.