False positves CKV2_AWS_67

[aaleksandrov]

Describe the issue CKV2_AWS_67 generates false positives when using default AWS managed encryption key (AES256)

Examples

Check: CKV2_AWS_67: "Ensure AWS S3 bucket encrypted with Customer Managed Key (CMK) has regular rotation"

	FAILED for resource: module.batch-stream-ingestion.aws_s3_bucket_server_side_encryption_configuration.sse_config

	File: /src/modules/batch-stream-ingestion/main.tf:80-88


resource "aws_s3_bucket_server_side_encryption_configuration" "sse_config" {
   bucket = aws_s3_bucket.firehose_s3_ingestion_result_bucket.bucket
     rule {
       apply_server_side_encryption_by_default {
         sse_algorithm = "AES256"
        }
     }
}

Version (please complete the following information):

• Checkov Version: latest from master

[stepintooracledba]

This also fails on a S3 bucket where Customer Managed Key is used. CMK already has rotation enabled. False Alerts on Default and CMK Keys..

[rafaljanicki]

And it also fails on keys that are stored in a different account

[ubbeK]

Looks like this PR is causing this: https://github.com/bridgecrewio/checkov/pull/6239

[tsmithv11]

@aaleksandrov this check explicitly looks for CMKs not AWS managed keys which are considered less secure, so that is not a false positive. We have others like this like CKV_AWS_181

@stepintooracledba according to the docs, rotation is off by default: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_key#enable_key_rotation. Can you provide documentation or a counter example? If so, we'll get it fixed.

@rafaljanicki sounds like you may be right. Do you have a counter example in TF you can share? We'll get this policy updated based on that.

[aaleksandrov]

this check explicitly looks for CMKs not AWS managed keys which are considered less secure, so that is not a false positive. We have others like this like CKV_AWS_181

@tsmithv11 It's quite confusing that this check basically includes 2 checks. If it's desired behavior can you at least change the message? To "Ensure AWS S3 bucket encrypted with Customer Managed Key (CMK) and it has regular rotation"

[tsmithv11]

@aaleksandrov no problem. I opened #6434 with this adjustment.

[rafaljanicki]

@rafaljanicki sounds like you may be right. Do you have a counter example in TF you can share? We'll get this policy updated based on that.

Sorry for late reply, I was off on vacations past two weeks. I don't have a counter example as mine is quite complex, but it's as simple as:

1. Have a key in account A. The key has a policy that allows account B to use it
2. In account B have a bucket that uses that key

That's it, the check fails on such scenario as it cannot determine if that key has a regular rotation or not if the Checkov doesn't have a cross account access (in our case it doesn't as we scope it to a single account and run against all of them)

---

On a separate note, I don't understand that check as it sounds like a duplicate. There are checks for KMS keys being rotated as well as there are checks for S3 buckets having encryption enabled

[stefanrusu-loctax]

this check explicitly looks for CMKs not AWS managed keys

Hi @tsmithv11. This check also catches AWS managed keys in come circumstances:

Check: CKV2_AWS_67: "Ensure AWS S3 bucket is encrypted with a Customer Managed Key (CMK) and the key has regular rotation"
	FAILED for resource: aws_s3_bucket_server_side_encryption_configuration.bucket
	File: /plan.json:98-106

		99  |           "values": {
		100 |             "expected_bucket_owner": null,
		101 |             "rule": [
		102 |               {
		103 |                 "apply_server_side_encryption_by_default": [{ "kms_master_key_id": "", "sse_algorithm": "aws:kms" }],
		104 |                 "bucket_key_enabled": true
		105 |               }
		106 |             ]

This is the corresponding resource:

resource "aws_s3_bucket_server_side_encryption_configuration" "bucket" {
  bucket = "${aws_s3_bucket.bucket.id}"
  rule {
    bucket_key_enabled = true
    apply_server_side_encryption_by_default = {
      sse_algorithm = "aws:kms"
    }
  }
  depends_on = [
    "aws_s3_bucket.bucket",
  ]
}

This is a valid TF state for indicating SSE-KMS with aws/s3 KMS key. I believe the empty kms_master_key_id is what trips this check. While their documentation states that kms_master_key_id is an optional, in practice having an empty string value behaves the same as planning this returns No changes. Your infrastructure matches the configuration.

[tsmithv11]

Alright folks, it seems this check is not salvageable. I'll go ahead and deprecate it.