checkov
checkov copied to clipboard
CKV2_AWS_32 Incomplete Documentation, Not As Described
Describe the issue
CKV2_AWS_32 says it checks to see if a strict response headers policy (e.g. aws_cloudfront_response_headers_policy
resource) is attached to the Cloudfront distribution. The documentation suggests this can be remediated by simply specifying the response_headers_policy_id
attribute, but this is not enough. To be able to get this check to pass, I had to actually read the source code for the check.
It's not just checking that a policy is attached, it's also reviewing the attached policy to confirm that each of the attributes in the policy are configured to very specific undocumented values with little flexibility. While I'm sure that the values are well researched and driven by best practices, I don't feel that this implementation is effective as it pretty much guarantees that in almost all cases people will disable it.
In addition to being very difficult to meet the requirements to get this check to pass, it's also very brittle - if I define the aws_cloudfront_response_headers_policy
in a different module or build the contents dynamically, even if they are correct, it's impossible to get the check to pass and I still end up adding an annotation anyway.
Lastly, the example given at https://docs.bridgecrew.io/docs/bc_aws_networking_65 does not even pass validation as response_headers_policy_id
does not belong under origin
, the correct place is inside a cache behaviour block.
Examples This code will fail the check:
resource "aws_cloudfront_distribution" "example" {
aliases = [local.cloudfront_hostname]
default_cache_behavior {
allowed_methods = ["GET", "HEAD", "OPTIONS", "PUT", "POST", "PATCH", "DELETE"]
cached_methods = ["GET", "HEAD"]
target_origin_id = local.origin_id
forwarded_values {
query_string = false
cookies {
forward = "none"
}
}
max_ttl = 60
viewer_protocol_policy = "redirect-to-https"
}
enabled = true
origin {
domain_name = local.alb_dns_name
origin_id = local.origin_id
custom_origin_config {
http_port = 80
https_port = 443
origin_protocol_policy = "http-only"
origin_ssl_protocols = ["TLSv1.2"]
}
}
restrictions {
geo_restriction {
restriction_type = "none"
}
}
viewer_certificate {
cloudfront_default_certificate = "true"
}
}
This code (which is written as per the documentation at https://docs.bridgecrew.io/docs/bc_aws_networking_65 suggests) will also fail the check:
resource "aws_cloudfront_distribution" "example" {
aliases = [local.cloudfront_hostname]
default_cache_behavior {
allowed_methods = ["GET", "HEAD", "OPTIONS", "PUT", "POST", "PATCH", "DELETE"]
cached_methods = ["GET", "HEAD"]
target_origin_id = local.origin_id
cache_policy_id = "ea271dfd-0d02-4c7c-a9b2-6b2bad9a679b"
origin_request_policy_id = "fd271dfc-0d02-4c7c-a9b2-6b2bad9a679b"
response_headers_policy_id = "ea271dfc-0d02-4c7c-a9b2-6b2bad9a679b"
viewer_protocol_policy = "redirect-to-https"
forwarded_values {
query_string = false
cookies {
forward = "none"
}
}
max_ttl = 60
viewer_protocol_policy = "redirect-to-https"
}
enabled = true
origin {
domain_name = local.alb_dns_name
origin_id = local.origin_id
custom_origin_config {
http_port = 80
https_port = 443
origin_protocol_policy = "http-only"
origin_ssl_protocols = ["TLSv1.2"]
}
}
restrictions {
geo_restriction {
restriction_type = "none"
}
}
viewer_certificate {
cloudfront_default_certificate = "true"
}
}
To get the above to pass the check, you need to also define (and reference) a aws_cloudfront_response_headers_policy
that matches the below policy exactly, with the exception of the aws_cloudfront_response_headers_policy.content_security_policy.content_security_policy
attribute, which is flexible but must at least contain the value default-src 'none';
resource "aws_cloudfront_response_headers_policy" "example" {
name = "mypolicy"
security_headers_config {
frame_options {
frame_option = "DENY"
override = true
}
content_type_options {
override = true
}
content_security_policy {
content_security_policy = "default-src 'none'; <more CSP directives here>"
override = true
}
referrer_policy {
override = true
referrer_policy = "same-origin"
}
strict_transport_security {
override = true
access_control_max_age_sec = 31536000
include_subdomains = true
preload = true
}
xss_protection {
override = true
mode_block = true
protection = true
}
}
}
Version (please complete the following information):
- Checkov Version 2.0.982
Additional context
Suggested resolution: Don't include the deep checks for aws_cloudfront_response_headers_policy
in this check, it might be a better idea to define those deep checks in a separate check that specifically examines aws_cloudfront_response_headers_policy
resources.
This would be more flexible and allow that resource to potentially be defined in a different place and still be subject to the same level of scrutiny, and encourage people to at least adopt the new aws_cloudfront_response_headers_policy
method, they can check to see that a policy is at least defined, even if they don't agree with the rigid requirements to meet the best practices.
@gruebel tagging you since you were the one who added this check (https://github.com/bridgecrewio/checkov/pull/1935), wondering if you can provide input. I find this check is useful, it just needs a bit more polish.
We are hitting this too. We use a var to pass in the policy and define it elsewhere. We thought we could just update the default to use one of AWS managed policies but go as described above, hopefully this can be addressed!
Sorry, I thought I actually answered here already 😄 @jamiepedwards yeah it makes sense to split the check, I also thought about splitting it in the mentioned connection check and then for each header one dedicated check, WDYT? @lorelei-rupp-imprivata also would like to hear your opinion 🙂
Sorry, I thought I actually answered here already 😄 @jamiepedwards yeah it makes sense to split the check, I also thought about splitting it in the mentioned connection check and then for each header one dedicated check, WDYT? @lorelei-rupp-imprivata also would like to hear your opinion 🙂
Yeah I like your idea of having it be a check on its own so we could have our other module scanned, but also like the idea to ensure cloudfronts are setting it too, but thats kinda hard if you define that in another module, it would have to rely on a default set, which I am ok with too
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!
I agree with the suggested solution. The check shouldn't be doing the extra deep check. Instead of a seperate check on the header policy, so it can reside in another module or somewhere else entirely
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!
Closing issue due to inactivity. If you feel this is in error, please re-open, or reach out to the community via slack: https://slack.bridgecrew.io Thanks!