fix(misconf): improve CIDR related checks

[nikpivkin]

Discussed in #8181

^{Originally posted by tamirkiviti13 December 26, 2024}

IDs

AVD-AWS-0104

Description

Trivy wrongly detects an issue of allowing access to /o CIDR range, even though a restrictive CIDR range is set by default in the variable.

Reproduction Steps

1. Create a main.tf file with egress in it:

resource "aws_security_group" "teja-test-sg" {
  name        = "teja-test-sg"
  description = "Security group with ingress and egress rules"

  # Ingress rules (inbound traffic)
  ingress {
    from_port   = 22
    to_port     = 22
    protocol    = "tcp"
    cidr_blocks = var.bank_cidrs # Allows SSH from anywhere; restrict as needed
  }

  ingress {
    from_port   = 80
    to_port     = 80
    protocol    = "tcp"
    cidr_blocks = var.bank_cidrs # Allows HTTP traffic
  }

  ingress {
    from_port   = 443
    to_port     = 443
    protocol    = "tcp"
    cidr_blocks = var.bank_cidrs # Allows HTTPS traffic
  }

  # Egress rules (outbound traffic)
  egress {
    from_port   = 0
    to_port     = 0
    protocol    = "-1"           # -1 allows all protocols
    cidr_blocks = var.bank_cidrs # Allows all outbound traffic
  }

  tags = {
    Name = "test-sg"
  }
}

2. Create a variable.tf file:

variable "aws_region" {
  type    = string
  default = "us-west-1"
}

variable "bank_cidrs" {
  type    = list(any)
  default = ["10.0.0.0/8", "192.168.164.0/23", "22.0.0.0/8", "24.0.0.0/8", "20.0.0.0/8"]
}

3. Execute trivy. Even though the variable defines a default list of CIDR ranges, trivy reports that the TF allows access to /0.
   ...

### Target

Filesystem

### Scanner

Misconfiguration

### Target OS

_No response_

### Debug Output

```bash
2024-12-26T14:10:48+02:00	DEBUG	Default config file "file_path=trivy.yaml" not found, using built in values
2024-12-26T14:10:48+02:00	DEBUG	Cache dir	dir="/Users/tamirkiviti/Library/Caches/trivy"
2024-12-26T14:10:48+02:00	DEBUG	Cache dir	dir="/Users/tamirkiviti/Library/Caches/trivy"
2024-12-26T14:10:48+02:00	DEBUG	Parsed severities	severities=[UNKNOWN LOW MEDIUM HIGH CRITICAL]
2024-12-26T14:10:48+02:00	DEBUG	Ignore statuses	statuses=[]
2024-12-26T14:10:48+02:00	INFO	[misconfig] Misconfiguration scanning is enabled
2024-12-26T14:10:48+02:00	DEBUG	[misconfig] Checks successfully loaded from disk
2024-12-26T14:10:48+02:00	DEBUG	Enabling misconfiguration scanners	scanners=[azure-arm cloudformation dockerfile helm kubernetes terraform terraformplan-json terraformplan-snapshot]
2024-12-26T14:10:48+02:00	DEBUG	Initializing scan cache...	type="memory"
2024-12-26T14:10:48+02:00	DEBUG	[misconfig] Scanning files for misconfigurations...	scanner="Terraform"
2024-12-26T14:10:48+02:00	DEBUG	[terraform scanner] Scanning directory	file_path="."
2024-12-26T14:10:48+02:00	DEBUG	[rego] Overriding filesystem for checks
2024-12-26T14:10:48+02:00	DEBUG	[rego] Embedded libraries are loaded	count=15
2024-12-26T14:10:48+02:00	DEBUG	[rego] Embedded checks are loaded	count=511
2024-12-26T14:10:49+02:00	DEBUG	[rego] Checks from disk are loaded	count=526
2024-12-26T14:10:49+02:00	DEBUG	[rego] Overriding filesystem for data
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Setting project/module root	module="root" file_path="."
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing FS	module="root" file_path="."
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing	module="root" file_path="main.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added file	module="root" file_path="main.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing	module="root" file_path="provider.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added file	module="root" file_path="provider.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing	module="root" file_path="variable.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added file	module="root" file_path="variable.tf"
2024-12-26T14:10:49+02:00	INFO	[terraform scanner] Scanning root module	file_path="."
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Setting project/module root	module="root" file_path="."
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing FS	module="root" file_path="."
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing	module="root" file_path="main.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added file	module="root" file_path="main.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing	module="root" file_path="provider.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added file	module="root" file_path="provider.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Parsing	module="root" file_path="variable.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added file	module="root" file_path="variable.tf"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Loading module	module="root" module="root"
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Read block(s) and ignore(s)	module="root" blocks=5 ignores=0
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Added input variables from tfvars	module="root" count=0
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Working directory for module evaluation	module="root" file_path="/tmp/trivy"
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Starting module evaluation...	path="."
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Starting iteration	iteration=0
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Starting iteration	iteration=1
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Context unchanged	iteration=1
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Starting post-submodules evaluation...
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Starting iteration	iteration=0
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Starting iteration	iteration=1
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Context unchanged	iteration=1
2024-12-26T14:10:49+02:00	DEBUG	[terraform evaluator] Module evaluation complete.
2024-12-26T14:10:49+02:00	DEBUG	[terraform parser] Finished parsing module	module="root"
2024-12-26T14:10:49+02:00	DEBUG	[terraform executor] Adapting modules...
2024-12-26T14:10:49+02:00	DEBUG	[terraform executor] Adapted module(s) into state data.	count=1
2024-12-26T14:10:49+02:00	DEBUG	[rego] Scanning inputs	count=1
2024-12-26T14:10:49+02:00	DEBUG	[terraform executor] Finished applying rules.
2024-12-26T14:10:49+02:00	DEBUG	[terraform executor] Applying ignores...
2024-12-26T14:10:49+02:00	DEBUG	OS is not detected.
2024-12-26T14:10:49+02:00	INFO	Detected config files	num=2
2024-12-26T14:10:49+02:00	DEBUG	Scanned config file	file_path="."
2024-12-26T14:10:49+02:00	DEBUG	Scanned config file	file_path="main.tf"
2024-12-26T14:10:49+02:00	DEBUG	Specified ignore file does not exist	file=".trivyignore"
2024-12-26T14:10:49+02:00	DEBUG	[vex] VEX filtering is disabled

main.tf (terraform)

Tests: 5 (SUCCESSES: 0, FAILURES: 5)
Failures: 5 (UNKNOWN: 0, LOW: 4, MEDIUM: 0, HIGH: 0, CRITICAL: 1)

AVD-AWS-0104 (CRITICAL): Security group rule allows egress to multiple public internet addresses.
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Opening up ports to connect out to the public internet is generally to be avoided. You should restrict access to IP addresses or ranges that are explicitly required where possible.


See https://avd.aquasec.com/misconfig/aws-vpc-no-public-egress-sgr
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main.tf:32
   via main.tf:28-33 (egress)
    via main.tf:1-38 (aws_security_group.teja-test-sg)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   resource "aws_security_group" "teja-test-sg" {
   .
  32 [     cidr_blocks = var.bank_cidrs # Allows all outbound traffic
  ..
  38   }
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────


AVD-AWS-0124 (LOW): Security group rule does not have a description.
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Security group rules should include a description for auditing purposes.

Simplifies auditing, debugging, and managing security groups.


See https://avd.aquasec.com/misconfig/aws-vpc-add-description-to-security-group-rule
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main.tf:6-11
   via main.tf:1-38 (aws_security_group.teja-test-sg)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   resource "aws_security_group" "teja-test-sg" {
   .
   6 ┌   ingress {
   7 │     from_port   = 22
   8 │     to_port     = 22
   9 │     protocol    = "tcp"
  10 │     cidr_blocks = var.bank_cidrs # Allows SSH from anywhere; restrict as needed
  11 └   }
  ..
  38   }
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────


AVD-AWS-0124 (LOW): Security group rule does not have a description.
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Security group rules should include a description for auditing purposes.

Simplifies auditing, debugging, and managing security groups.


See https://avd.aquasec.com/misconfig/aws-vpc-add-description-to-security-group-rule
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main.tf:28-33
   via main.tf:1-38 (aws_security_group.teja-test-sg)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   resource "aws_security_group" "teja-test-sg" {
   .
  28 ┌   egress {
  29 │     from_port   = 0
  30 │     to_port     = 0
  31 │     protocol    = "-1"           # -1 allows all protocols
  32 │     cidr_blocks = var.bank_cidrs # Allows all outbound traffic
  33 └   }
  ..
  38   }
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────


AVD-AWS-0124 (LOW): Security group rule does not have a description.
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Security group rules should include a description for auditing purposes.

Simplifies auditing, debugging, and managing security groups.


See https://avd.aquasec.com/misconfig/aws-vpc-add-description-to-security-group-rule
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main.tf:20-25
   via main.tf:1-38 (aws_security_group.teja-test-sg)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   resource "aws_security_group" "teja-test-sg" {
   .
  20 ┌   ingress {
  21 │     from_port   = 443
  22 │     to_port     = 443
  23 │     protocol    = "tcp"
  24 │     cidr_blocks = var.bank_cidrs # Allows HTTPS traffic
  25 └   }
  ..
  38   }
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────


AVD-AWS-0124 (LOW): Security group rule does not have a description.
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Security group rules should include a description for auditing purposes.

Simplifies auditing, debugging, and managing security groups.


See https://avd.aquasec.com/misconfig/aws-vpc-add-description-to-security-group-rule
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 main.tf:13-18
   via main.tf:1-38 (aws_security_group.teja-test-sg)
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   resource "aws_security_group" "teja-test-sg" {
   .
  13 ┌   ingress {
  14 │     from_port   = 80
  15 │     to_port     = 80
  16 │     protocol    = "tcp"
  17 │     cidr_blocks = var.bank_cidrs # Allows HTTP traffic
  18 └   }
  ..
  38   }
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Version

Version: 0.58.0
Vulnerability DB:
  Version: 2
  UpdatedAt: 2024-09-24 12:14:59.750688839 +0000 UTC
  NextUpdate: 2024-09-24 18:14:59.750688689 +0000 UTC
  DownloadedAt: 2024-09-24 13:40:15.391303 +0000 UTC
Java DB:
  Version: 1
  UpdatedAt: 2024-07-30 01:03:52.606721445 +0000 UTC
  NextUpdate: 2024-08-02 01:03:52.606721274 +0000 UTC
  DownloadedAt: 2024-07-30 06:28:56.172075 +0000 UTC
Check Bundle:
  Digest: sha256:f6901e03f486a48f47aa17a78d89d18e6c31ded82aff83ed19d0d73935a1a059
  DownloadedAt: 2024-12-26 10:47:24.191759 +0000 UTC

Checklist

• Read the documentation regarding wrong detection
• Ran Trivy with -f json that shows data sources and confirmed that the security advisory in data sources was correct

[nikpivkin]

@itaysk @tamirkiviti13 @simar7 The description of some checks (e.g. https://github.com/aquasecurity/trivy-checks/blob/main/checks/cloud/google/compute/no_public_egress.rego) includes the wording Network security rules should not use very broad subnets. However, the term very broad subnets may not be accurate and may be confusing. When is a subnet considered too broad? This raises questions, as no clear criteria are given in the description.

In addition, there is an inconsistency between the description and the logic of the checks. The description talks about subnets that allow all traffic, i.e. cover the entire range of IP addresses, but in practice the check is triggered if the subnet:

1. Is public.
2. Contains more than one IP address.

Was there a reason why the checks did not originally check subnets containing only the entire IP range? Perhaps the check was intended to be broader to identify other potentially risky configurations, but this should be reflected in the description.

If the purpose of the inspection is to identify subnets that allow all incoming traffic, the logic should be refined to match the description. Otherwise, the description should be adapted to clearly explain the current inspection logic.

[simar7]

Using /0 is often a common security misconfiguration that is widely seen in practice, usually done to debug network flows but often left unsecured and shipped in production. This check simply aims to find /0 or ::/0 (incase of ipv6). I think we need to improve the logic to check that. That's my understanding of this check.

[itaysk]

@nikpivkin very good catch and discussion.

I wanted to track the origin of this, the commit history is hard to follow with many renames and merges but looks like it is inspired by these original checks:

https://github.com/aquasecurity/tfsec/blob/84e24dad91645b2d6250fecea501c60be6e3962a/internal/app/tfsec/checks/aws_open_security_group_rules.go
It checks only for broad networks (/0) and the description is "Resource '%s' defines a fully open %s security group rule"

https://github.com/aquasecurity/tfsec/blob/84e24dad91645b2d6250fecea501c60be6e3962a/internal/app/tfsec/checks/aws_public_ip.go
It checks only for public IP and the description is "Resource '%s' has a public IP address associated."
(there are additional similar checks)

Along the way maybe they were conflated somehow.

Anyway, I think there's no question about checking for unrestrictive CIDR (/0) like your PR does.
To confirm, do we not want to check for publicity anymore?

[nikpivkin]

@itaysk Okay, got it.
Now, all CIDR-related checks can be divided into two categories:

1. Checking rules that allow access from or to any IP address configured through security groups or firewalls.
2. Checking the publicity of resource instances.

I think we should focus on the first category.