bridgecrewio / checkov

Prevent cloud misconfigurations and find vulnerabilities during build-time in infrastructure as code, container images and open source packages with Checkov by Bridgecrew.
https://www.checkov.io/
Apache License 2.0
6.97k stars 1.1k forks source link

False positives for AWS checks when using count and v4.x.x provider syntax #3847

Open NoahCallaway opened 1 year ago

NoahCallaway commented 1 year ago

Describe the issue I'm seeing false positives for the following checks when called from a module with the terraform count attribute. If no count is specified these checks pass. Note: I'm also using AWS v4 provider syntax where separate blocks are used. e.g. https://github.com/bridgecrewio/checkov/issues/2399 I don't think this fix accounted for use inside a module and iterated using count

Affected Checks as follows:

Examples

Checks FAIL with

# main.tf
module "my_module" {
  count = 1
  source = "./my-module"
}

Checks PASS with

# main.tf
module "my_module" {
  source = "./my-module"
}
# my-module/main.tf
resource "aws_s3_bucket" "mybucket" {
  bucket = "mybucket"
}

# should prevent CKV_AWS_145: "Ensure that S3 buckets are encrypted with KMS by default"
# should prevent CKV_AWS_19: "Ensure all data stored in the S3 bucket is securely encrypted at rest"
resource "aws_s3_bucket_server_side_encryption_configuration" "example" {
  bucket = aws_s3_bucket.mybucket.bucket

  rule {
    apply_server_side_encryption_by_default {
      kms_master_key_id = aws_kms_key.mykey.arn
      sse_algorithm     = "aws:kms"
    }
  }
}

resource "aws_kms_key" "mykey" {
  description             = "This key is used to encrypt bucket objects"
  deletion_window_in_days = 10
}

# should prevent CKV_AWS_18: "Ensure the S3 bucket has access logging enabled"
resource "aws_s3_bucket_logging" "example" {
  bucket = aws_s3_bucket.mybucket.id

  target_bucket = aws_s3_bucket.mybucket.id
  target_prefix = "log/"
}

# should prevent CKV_AWS_21: "Ensure all data stored in the S3 bucket have versioning enabled"
resource "aws_s3_bucket_versioning" "bucket_versioning" {
  bucket = aws_s3_bucket.mybucket.id
  versioning_configuration {
    status = "Enabled"
  }
}
# provider.tf
terraform {
  required_version = ">= 0.13"
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "4.36.1"
    }
  }
}

provider "aws" {
  region  = "var.region"
}

Version (please complete the following information):

Additional context I'm running the checkov scans on .json plan files. checkov -f tfplan.json

gruebel commented 1 year ago

hey @NoahCallaway thanks fro reaching out. count and for_each are not fully supported yet, therefore it is no surprise it didn't work. We plan to start tackling it soon, but it will be first for normal resources.

ChanochShayner commented 1 year ago

hey @NoahCallaway :) We are just finished adding the support of for_each/count Meta-Arguments in Terraform modules, Currently, it's under two environment variables:

To recheck your setup please set those variables as True

Feel free to reach out in case something is not working.

NoahCallaway commented 1 year ago

Thanks both!

stale[bot] commented 11 months ago

Thanks for contributing to Checkov! We've automatically marked this issue as stale to keep our issues list tidy, because it has not had any activity for 6 months. It will be closed in 14 days if no further activity occurs. Commenting on this issue will remove the stale tag. If you want to talk through the issue or help us understand the priority and context, feel free to add a comment or join us in the Checkov slack channel at https://slack.bridgecrew.io Thanks!

martingaleano commented 11 months ago

Hi @ChanochShayner , thanks for your reply! Using this variables: CHECKOV_NEW_TF_PARSER CHECKOV_ENABLE_MODULES_FOREACH_HANDLING

only apply to Terraform Modules or it can be used with Terraform Resources? Because I'm still getting the same error with terraform resources.

Thanks @NoahCallaway

thomasbinny commented 10 months ago

Guys, I believe this issue is not fixed yet. please find the details below

xxxx binnythomas$ checkov --version
3.0.38

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = ">=4.0.0"
    }
  }

  required_version = ">= 1.0.11"
}

CHECKOV_ENABLE_MODULES_FOREACH_HANDLING=True CHECKOV_NEW_TF_PARSER=True checkov --directory ./infrastructure/terraform/build/

Still no luck it fails with false positives. Do let me know if i have missed something. @NoahCallaway Does it work for you?

I have just literally suppressed the lot of rules.

checkov -d . --skip-check "CKV_AWS_21,CKV_AWS_144,CKV_AWS_145,CKV2_AWS_6,CKV2_AWS_62,CKV2_AWS_61,CKV_AWS_18,CKV2_AWS_16,CKV_AWS_28,CKV_AWS_227,CKV_AWS_119"
thomasbinny commented 9 months ago

Hi @ChanochShayner , thanks for your reply! Using this variables: CHECKOV_NEW_TF_PARSER CHECKOV_ENABLE_MODULES_FOREACH_HANDLING

only apply to Terraform Modules or it can be used with Terraform Resources? Because I'm still getting the same error with terraform resources.

Thanks @NoahCallaway

@martingaleano Have you managed to fix this?

harishachappa commented 9 months ago

@ChanochShayner, @gruebel count.index on Terraform resource is still not working

oldskool commented 7 months ago

This also seems to impact CKV2_AWS_6. I've been able to narrow it down to checkov 2.3.215 as the release that started breaking on the use of a count variable on an s3 bucket like this:

resource "aws_s3_bucket" "bucket" {
  count  = length(var.buckets) > 0 ? 1 : 0
  bucket = "my-bucket"
}

resource "aws_s3_bucket_public_access_block" "bucket" {
  count  = length(var.buckets) > 0 ? 1 : 0
  bucket = aws_s3_bucket.bucket[0].id

  block_public_acls       = true
  block_public_policy     = true
  ignore_public_acls      = true
  restrict_public_buckets = true
}

The same code will pass on checkov 2.3.214. It appears that checkov 2.3.215 introduced the "new TF parser": https://github.com/bridgecrewio/checkov/compare/2.3.214...2.3.215

Not exactly sure why that was considered a patch update instead of a major or at least minor version bump.

stale[bot] commented 1 month ago

Thanks for contributing to Checkov! We've automatically marked this issue as stale to keep our issues list tidy, because it has not had any activity for 6 months. It will be closed in 14 days if no further activity occurs. Commenting on this issue will remove the stale tag. If you want to talk through the issue or help us understand the priority and context, feel free to add a comment or join us in the Checkov slack channel at codifiedsecurity.slack.com Thanks!