Filtering findings for Terraform modules based on attributes

[acdha]

Description

Like many people, I use the terraform-aws-modules/vpc/aws module and that module creates a variety of resources using large lists for things like ACLs:

module "vpc" {
  source                 = "terraform-aws-modules/vpc/aws"
  version                = "~> 5.8"
…
  public_inbound_acl_rules = concat(
    [
      for i in range(length(local.default_ingress_acls)) :
      merge(
        {
          rule_number = 1000 + i,
          rule_action = local.default_ingress_acls[i]["action"]
        },
        local.default_ingress_acls[i]
      )
    ],
…

This creates a problem on every project when using Trivy because you'll get findings for the resources created using count or for_each blocks inside the module based on those lists:

CRITICAL: Network ACL rule allows ingress from public internet.

Opening up ACLs to the public internet is potentially dangerous. You should restrict access to IP addresses or ranges that explicitly require it where possible.

See https://avd.aquasec.com/misconfig/avd-aws-0105

 terraform-aws-modules/vpc/aws/.terraform/modules/vpc/main.tf:204
   via terraform-aws-modules/vpc/aws/.terraform/modules/vpc/main.tf:191-206 (aws_network_acl_rule.public_inbound[13])
    via vpc.tf:55-250 (module.vpc)

 191   resource "aws_network_acl_rule" "public_inbound" {
 ...   
 204 [   cidr_block      = lookup(var.public_inbound_acl_rules[count.index], "cidr_block", null)
 ...   
 206   }

(this also repeats for AVD-AWS-102, etc.)

I think Trivy needs explicit support for filtering things like this safely. The current design for attribute filtering does not support indexed resources (as every input to that module uses) and since these rules are usually false-positives (most people use AWS to host public applications so they'll need to support ingress HTTP, HTTPS, ICMP for Path MTU discovery, etc.), it's really tempting to just toss #trivy:ignore:avd-aws-0102 trivy:ignore:avd-aws-0105 into that file — greatly increasing the odds of Trivy not catching an actual security problem when someone adds another rule – and even if indexed access were possible, it'd be risky because the indexes are not stable (e.g. in my example above, some of the rules come from a different module which has common firewall rules and an update could easily shift all of the index numbers).

Since Trivy has full knowledge of the resources created in a module, it seems like it should be possible to have some way to use that hierarchy to select a resource which would otherwise not be in scope at this point:

# #trivy:ignore:avd-aws-0102[module_resource_name=aws_network_acl_rule.public_inbound,type=ingress,protocol=1]
module "vpc" {
  source                 = "terraform-aws-modules/vpc/aws"
…

I don't love that syntax and it seems like it might be preferable to say that complex filtering has to be done in a .trivyignore.yaml file where it could look more like this:

- id: AVD-AWS-0102
  statement: ICMP Path MTU discovery
  paths:
    - vpc.tf
  resources:
    - path: module.vpc.aws_network_acl_rule.public_inbound
      attributes:
        type: ingress
        protocol: 1
- id: AVD-AWS-0105
  statement: Public web app ingress HTTPS
  paths:
    - vpc.tf
  resources:
    - path: module.vpc.aws_network_acl_rule.public_inbound
      attributes:
        type: ingress
        protocol: 6
        from_port: 443
        to_port: 443

One other thought which came out of this is that the error messages could be improved somewhat to list the attributes of the resource in question. When you have large lists being fed into a loop it'd be really nice to be able to see cidr_block=0.0.0.0/0 from_port=22 to_port=22 so you could easily know which source definitely it refers to rather than having to count them.

Target

None

Scanner

Misconfiguration

[simar7]

Thank you for the excellent feedback, @acdha! Those are certainly valid points. If I can summarize:

1. Trivy should support accessing individual blocks in the case of for_each or count variables. This would allow to target specific resources such as you described in the example.
2. In order to specify more granular ignores for the above attributes, Trivy should allow such a configuration to be passed in via the .trivyignore.yaml format for ease of use and readability.
3. While displaying the misconfigurations for such cases, we should expand the blocks to specifically mention which iterated item is being shown.

If I've got that right, I would say each of the above deserves a feature request of its own. Please let me know if I've missed something.

Thank you.

[acdha]

Re point one, I think the two main things I need are the ability to target resources indirectly and using attributes rather than indexes - it seems like people will miss things if maintenance shuffles integer indexes around.

[simar7]

Track #7180