feat(misconf): Ignoring specific values for terraform files when using for_each

[simar7]

Discussed in #6136

^{Originally posted by felipeng February 14, 2024}

Description

Hey,

can we have this feature , which is ignore specific values, but for terraform modules as well?

Is better to explain with an example of use case:

This terraform code creates a AWS Security Groups rules
locals.tf

locals {
  aws-sg = {
    service1 = {
      ingress_with_cidr_blocks = [
        {
          description = "HTTP"
          from_port   = 80
          to_port     = 80
          protocol    = "tcp"
          cidr_blocks = "0.0.0.0/0"
        }
      ]
    }
  }
}

with this terraform code that uses security-group module to create rules: sg.tf

module "aws-security-groups" {
  source   = "terraform-aws-modules/security-group/aws"
  version  = "~> 4.16.2"
  for_each = local.aws-sg

  name                     = each.key
  vpc_id                   = "vpc-123"
  ingress_with_cidr_blocks = try(each.value.ingress_with_cidr_blocks, null)
}

In this case, trivy will report: CRITICAL: Security group rule allows ingress from public internet.

trivy config .
2023-06-08T10:47:41.491-0400    INFO    Misconfiguration scanning is enabled
2023-06-08T10:47:41.896-0400    INFO    Detected config files: 1

terraform-aws-modules/security-group/aws/main.tf (terraform)

Tests: 3 (SUCCESSES: 2, FAILURES: 1, EXCEPTIONS: 0)
Failures: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 0, CRITICAL: 1)

CRITICAL: Security group rule allows ingress from public internet.
Opening up ports to the public internet is generally to be avoided. You should restrict access to IP addresses or ranges that explicitly require it where possible.

See https://avd.aquasec.com/misconfig/avd-aws-0107
 terraform-aws-modules/security-group/aws/main.tf:197-204
 191   resource "aws_security_group_rule" "ingress_with_cidr_blocks" {
 ...   
 197 ┌   cidr_blocks = split(
 198 │     ",",
 199 │     lookup(
 200 │       var.ingress_with_cidr_blocks[count.index],
 201 │       "cidr_blocks",
 202 │       join(",", var.ingress_cidr_blocks),
 203 └     ),
 ... 

So, since we really need to allow traffic from 0.0.0.0/0 to TCP/80 (HTTP), let's ignore it adding the #trivy:ignore:aws-ec2-no-public-ingress-sgr

#trivy:ignore:aws-ec2-no-public-ingress-sgr <- HERE
module "aws-security-groups" {
  source   = "terraform-aws-modules/security-group/aws"
  version  = "~> 4.16.2"
  for_each = local.aws-sg

  name                     = each.key
  vpc_id                   = "vpc-123"
  ingress_with_cidr_blocks = try(each.value.ingress_with_cidr_blocks, null)
}

And it works as expected, trivy ignores this check. However, let's say that I add another service and rule for SSH. BUT I forgot to restrict the FROM traffic and added 0.0.0.0/0 like so:

locals {
  aws-sg = {
    service1 = {
      ingress_with_cidr_blocks = [
        {
          description = "HTTP"
          from_port   = 80
          to_port     = 80
          protocol    = "tcp"
          cidr_blocks = "0.0.0.0/0"
        }
      ]
    }
    service2 = {
      ingress_with_cidr_blocks = [
        {
          description = "SSH"
          from_port   = 22
          to_port     = 22
          protocol    = "tcp"
          cidr_blocks = "0.0.0.0/0"
        }
      ]
    }
  }
}

In this case trivy completely ignores the aws-ec2-no-public-ingress-sgr checks.

trivy config .
2023-06-08T10:55:42.316-0400    INFO    Misconfiguration scanning is enabled
2023-06-08T10:55:42.748-0400    INFO    Detected config files: 1

It would be nice to be able to ignore based on the value, something like #trivy:ignore:aws-ec2-no-public-ingress-sgr[locals.aws-sg.service1], because there are situation that we don't want to fully ignore a check but only for some configuration.

Target

None

Scanner

Misconfiguration

[nikpivkin]

Trivy supports ignore by attributes for modules as well. In the example, I ignore a module by the name attribute:

locals {
  aws-sg = {
    service1 = {
      ingress_with_cidr_blocks = [
        {
          description = "HTTP"
          from_port   = 80
          to_port     = 80
          protocol    = "tcp"
          cidr_blocks = "0.0.0.0/0"
        }
      ]
    },
    service2 = {
      ingress_with_cidr_blocks = [
        {
          description = "SSH"
          from_port   = 22
          to_port     = 22
          protocol    = "tcp"
          cidr_blocks = "0.0.0.0/0"
        }
      ]
    }
  }
}

#tfsec:ignore:AVD-AWS-0107[name=service1]
module "aws-security-groups" {
  source   = "terraform-aws-modules/security-group/aws"
  version  = "~> 4.16.2"
  for_each = local.aws-sg

  name                     = each.key
  vpc_id                   = "vpc-123"
  ingress_with_cidr_blocks = try(each.value.ingress_with_cidr_blocks, null)
}

CRITICAL: Security group rule allows ingress from public internet.
═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Opening up ports to the public internet is generally to be avoided. You should restrict access to IP addresses or ranges that explicitly require it where possible.

See https://avd.aquasec.com/misconfig/avd-aws-0107
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 terraform-aws-modules/security-group/aws/main.tf:197-204
   via terraform-aws-modules/security-group/aws/main.tf:191-227 (aws_security_group_rule.ingress_with_cidr_blocks[0])
    via main.tf:29-37 (module.aws-security-groups["service2"])
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 191   resource "aws_security_group_rule" "ingress_with_cidr_blocks" {
 ...   
 197 ┌   cidr_blocks = split(
 198 │     ",",
 199 │     lookup(
 200 │       var.ingress_with_cidr_blocks[count.index],
 201 │       "cidr_blocks",
 202 │       join(",", var.ingress_cidr_blocks),
 203 └     ),
 ...   
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

But this way of ignoring does not support nested attributes and therefore does not allow referencing each.key, for example.

[felipeng]

Thank you! I didn't know that was possible, the current documentation doesn't mention this option

[nikpivkin]

@simar7 I didn't know about this feature either. Is it worth adding it to the documentation?

[simar7]

@simar7 I didn't know about this feature either. Is it worth adding it to the documentation?

Yes that'll be nice. 👍

[simar7]

I've updated the issue so we can use it to track the feature improvement for supporting for_each

[dr-yd]

This issue is a bit confusing - is specifying a human-read able name possible or not? And if it's possible, where are they listed? Only allowing a cryptic ID seems like a huge regression, especially for auditability. I just tried migrating from tfsec and Trivy just seems to ignore the names.

[simar7]

This issue is a bit confusing - is specifying a human-read able name possible or not? And if it's possible, where are they listed? Only allowing a cryptic ID seems like a huge regression, especially for auditability. I just tried migrating from tfsec and Trivy just seems to ignore the names.

Can you please share a reproducible example so we can take a look?

[dr-yd]

Turns out that I misinterpreted the situation since I ran it on a large codebase that was configured cleanly with tfsec but threw a lot of errors with Trivy. This should probably be its own issue (or probably multiple) but I'll respond here for now. Only some of the old IDs are ignored, others are simply false matches. For example, I have the following:

module/main.tf:

variable "kms_encryption" {
  type    = bool
  default = true
}

resource "aws_s3_bucket" "main" {
  bucket = "my-tf-test-bucket"
}

module "kms" {
  source = "./kms"
}

resource "aws_s3_bucket_server_side_encryption_configuration" "main" {
  bucket = aws_s3_bucket.main.id

  dynamic "rule" {
    for_each = var.kms_encryption ? [true] : []
    content {
      apply_server_side_encryption_by_default {
        kms_master_key_id = module.kms_key.arn
        sse_algorithm     = "aws:kms"
      }
      bucket_key_enabled = true
    }
  }

  dynamic "rule" {
    for_each = var.kms_encryption ? [] : [true]
    content {
      apply_server_side_encryption_by_default {
        sse_algorithm = "AES256"
      }
    }
  }
}

module/kms/main.tf:

resource "aws_kms_key" "main" {
  enable_key_rotation = true
}

output "arn" {
  value = aws_kms_key.main.arn
}

main.tf:

# tfsec:ignore:aws-s3-specify-public-access-block tfsec:ignore:aws-s3-block-public-acls tfsec:ignore:aws-s3-block-public-policy tfsec:ignore:aws-s3-enable-versioning tfsec:ignore:aws-s3-enable-bucket-logging tfsec:ignore:aws-s3-no-public-buckets tfsec:ignore:aws-s3-ignore-public-acls
module "aws-security-groups" {
  source = "./module"
}

With tfsec this passes, no problems detected. With Trivy, three rules match:

• Bucket has logging disabled is not ignored with tfsec:ignore:aws-s3-enable-bucket-logging which works for tfsec
• Bucket does not have encryption enabled is completely false because regardless of the variable, some encryption will be enabled. I can't find a human-readable ID for this.
• Bucket does not encrypt data with a customer managed key. Disabling that with the human-readable ID does work.

With knowing that the readable IDs should work, I should be able to get a better handle on this. In any case, I'd say that this should be covered in the migration guide - we have close to a hundred modules that are all built to conform to tfsec on low or have the proper ignores set where it's intentional. Trivy is probably not supposed to be a drop-in replacement but a large number of your users are going to be coming from tfsec.

[nikpivkin]

Hi @dr-yd !

You must use the IDs of the checks to ignore. For example tfsec:ignore:aws-s3-enable-bucket-logging -> tfsec:ignore:AVD-AWS-0089. You can find the actual IDs here.

As for other checks, the bug with dynamic blocks has been fixed, the fix will be included in the next release or you can use the canary release.

@simar7
Trivy allows to ignore checks by their alias, but they are not available on the misconfiguration site.
Example:

#tfsec:ignore:aws-iam-enforce-mfa
resource "aws_iam_group" "support" {
  name = "support"
}

#tfsec:ignore:*
resource "aws_iam_group_policy" "mfa" {

  group  = aws_iam_group.support.name
  policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "",
      "Effect": "Allow",
      "Action": "ec2:*",
      "Resource": "*"
    }
  ]
}
EOF
}

How about displaying the check aliases on the site and adding the aliases that were in tfsec for backwards compatibility?

[simar7]

@dr-yd for your issues here's an explanation:

Bucket has logging disabled is not ignored with tfsec:ignore:aws-s3-enable-bucket-logging which works for tfsec

The rule was updated in Trivy and can be referenced by trivy:ignore:AVD-AWS-0089 or it's short code trivy:ignore:enable-logging More info here

Bucket does not have encryption enabled is completely false because regardless of the variable, some encryption will be enabled. I can't find a human-readable ID for this.

trivy:ignore:AVD-AWS-0088 or trivy:ignore:enable-bucket-encryption. More info here

Bucket does not encrypt data with a customer managed key. Disabling that with the human-readable ID does work.

trivy:ignore:AVD-AWS-0132 or trivy:ignore:encryption-customer-key. More info here

[dr-yd]

@simar7, so the short code is reliably simply the slug of the page title? That's definitely something we can work with, thanks a lot for the input! (Maybe not 101% reliable like the AVD-AWS-xxx IDs, but that's a small price to pay for ease of use.)