Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix cloudfront_distribution_use_secure_cipher protocol filter #827

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix cloudfront_distribution_use_secure_cipher protocol filter #827

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc47788
fix protocol filter
sbldevnet Oct 1, 2024
6c835b4
fix protocol filter
sbldevnet Oct 1, 2024
643ce8d
Merge branch 'fix_cloudfront_distribution_use_secure_cipher' of githu…
sbldevnet Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions conformance_pack/cloudfront.sp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,16 +229,17 @@ query "cloudfront_distribution_use_secure_cipher" {
aws_cloudfront_distribution,
jsonb_array_elements(origins) as o
where
o -> 'CustomOriginConfig' -> 'OriginSslProtocols' -> 'Items' @> '["TLSv1.2%", "TLSv1.1%"]'
o -> 'CustomOriginConfig' -> 'OriginSslProtocols' -> 'Items' @> '["TLSv1"]'
or o -> 'CustomOriginConfig' -> 'OriginSslProtocols' -> 'Items' @> '["SSLv3"]'
)
select
b.arn as resource,
distinct b.arn as resource,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ sbldevnet Is this really required as we are using distinct in above CTE origin_protocols. ?

Copy link
Contributor Author

@sbldevnet sbldevnet Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khushboo9024 when there are two or more origins in the same distribution using the SSLv3 and TLSv1 protocols, the CTE origin_protocols returns the same distribution twice.

As the origin_protocols value is not used in the parent query I think it can be removed from the CTE select to avoid using the distinct in the parent query. I tested it locally and seems to work as expected.

Copy link
Contributor Author

@sbldevnet sbldevnet Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it OK the current fix or should I remove the origin_protocols from the parent query to avoid the distinct? @khushboo9024

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbldevnet I am OK with the current fix. 👍

case
when o.arn is not null then 'ok'
when o.arn is null then 'ok'
else 'alarm'
end as status,
case
when o.arn is not null then title || ' use secure cipher.'
when o.arn is null then title || ' uses secure cipher.'
else title || ' does not use secure cipher.'
end as reason
${local.tag_dimensions_sql}
Expand Down Expand Up @@ -592,4 +593,4 @@ query "cloudfront_distribution_latest_tls_version" {
from
aws_cloudfront_distribution;
EOQ
}
}