aquasecurity / trivy

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

feat(misconf): Filtering findings for Terraform modules based on attributes #7180

Open simar7 opened 1 month ago

simar7 commented 1 month ago

TODO:

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

Originally posted by **acdha** July 12, 2024 ### 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: ```hcl 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](https://aquasecurity.github.io/trivy/dev/docs/scanner/misconfiguration/#ignoring-by-attributes) 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: ```hcl # #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: ```yaml - 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
nikpivkin commented 1 month ago

@simar7 I'll divide this into 3 tasks.

  1. I implemented filtering by count and each objects, but we decided not to add that. It would be easy to add it. https://github.com/aquasecurity/trivy/pull/6302/commits/835351defb6280f93cae61386d616525123f328e
  2. I think we need to have a better discussion about the file scheme. We need to somehow distinguish resources from different projects. For example, there could be a case where the scan target is a directory containing unrelated root modules foo and bar, which have resources with the same logical path. Maybe we should add some kind of entrypoint field? The path field for a resource makes no sense if it is in a remote module. Only the logical path is enough.
  3. Right now we don't display the actual value of attributes in the report. What would it look like? Now for displaying misconfig we just read the file and display a piece of code according to the location of the finding. In order to display the expanded block in the report, we need to modify the HCL file, overwrite the file in the file system, and update the finding metadata to refer to the new location.