Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
simar7 opened this issue Jul 5, 2024 Discussed in #7095 · 6 comments · Fixed by #7295
Closed
2 tasks done

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

simar7 opened this issue Jul 5, 2024 Discussed in #7095 · 6 comments · Fixed by #7295
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. scan/misconfiguration Issues relating to misconfiguration scanning

Comments

@simar7
Copy link
Member

simar7 commented Jul 5, 2024

Discussed in #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

Reproduction steps are in the Description field.

Target

Filesystem

Scanner

Misconfiguration

Output Format

None

Mode

Standalone

Debug Output

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} <nil>} {{{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.<root>          Setting project/module root to '.'
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.922519645 terraform.parser.<root>          Parsing FS from '.'
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.922541973 terraform.parser.<root>          Parsing 'vpc.tf'...
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.922752099 terraform.parser.<root>          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.<root>          Setting project/module root to '.'
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.922930861 terraform.parser.<root>          Parsing FS from '.'
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.922943454 terraform.parser.<root>          Parsing 'vpc.tf'...
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923088870 terraform.parser.<root>          Added file vpc.tf.
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923106798 terraform.parser.<root>          Evaluating module...
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923225096 terraform.parser.<root>          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.<root>          Added 0 variables from tfvars.
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923261689 terraform.parser.<root>          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.<root>.evaluator Filesystem key is 'b244507b0e4bf3b8e9c2c21a83d8ee7f6d1e6d810848458f51583decf4f5009d'
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923310735 terraform.parser.<root>.evaluator Starting module evaluation...
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923539318 terraform.parser.<root>.evaluator Starting submodule evaluation...
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923552198 terraform.parser.<root>.evaluator All submodules are evaluated at i=0
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923560535 terraform.parser.<root>.evaluator Starting post-submodule evaluation...
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923650707 terraform.parser.<root>.evaluator Finished processing 0 submodule(s).
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923659649 terraform.parser.<root>.evaluator Module evaluation complete.
2024-07-04T11:40:15+05:30	DEBUG	[misconf] 40:15.923669714 terraform.parser.<root>          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

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

@simar7 simar7 added kind/bug Categorizes issue or PR as related to a bug. scan/misconfiguration Issues relating to misconfiguration scanning labels Jul 5, 2024
@nikpivkin
Copy link
Contributor

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
Copy link
Member Author

simar7 commented Jul 8, 2024

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
Copy link
Contributor

@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
Copy link
Contributor

acdha commented Jul 11, 2024

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
Copy link
Contributor

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

@acdha
Copy link
Contributor

acdha commented Jul 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. scan/misconfiguration Issues relating to misconfiguration scanning
Projects
None yet
3 participants