Skip to content

Test cloudfront distributions using deprecated ssl protocols#290

Open
prajwal-choudhari-comprinno wants to merge 2 commits intocomprinnotech:testcases_devfrom
prajwal-choudhari-comprinno:test_cloudfront_distributions_using_deprecated_ssl_protocols
Open

Test cloudfront distributions using deprecated ssl protocols#290
prajwal-choudhari-comprinno wants to merge 2 commits intocomprinnotech:testcases_devfrom
prajwal-choudhari-comprinno:test_cloudfront_distributions_using_deprecated_ssl_protocols

Conversation

@prajwal-choudhari-comprinno

Description
This PR adds the cloudfront_distributions_using_deprecated_ssl_protocols check, which verifies that CloudFront distributions are configured to use only modern and secure SSL/TLS protocols, such as TLSv1.2 or higher.

It includes:

A new test file (tests/test_cloudfront_distributions_using_deprecated_ssl_protocols.py) with comprehensive unit tests for the check.
Test cases cover:

  1. The check returns NOT_APPLICABLE when no CloudFront distributions are present in the account.
  2. Distributions configured to use secure protocols like TLSv1.2_2021 correctly result in a PASSED status.
  3. Distributions using deprecated protocols (e.g., SSLv3) are correctly identified, leading to a FAILED status.
  4. The check gracefully handles ClientError exceptions during AWS API calls, returning an UNKNOWN status.

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

)
report = self.check.execute(self.mock_session)
assert report.status == CheckStatus.UNKNOWN
assert "Error retrieving CloudFront distributions." in report.resource_ids_status[0].summary No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Summary of Improvements

  1. Fix Logical Bug: ViewerCertificate is in the DistributionConfig, not the Distribution

    • Your current check uses:

      security_policy = distribution.get('ViewerCertificate', {}).get('MinimumProtocolVersion', 'TLSv1.2')

      Fix: Fetch ViewerCertificate from get_distribution_config(distribution_id) instead, which is the correct source.

  2. Correct and Align Test Cases

    • Your test mocks call get_distribution_config, but the check never calls it. That’s why even deprecated protocols get marked as secure in the test.
    • ✅ Update the check to call client.get_distribution_config(Id=distribution_id) to fetch ViewerCertificate accurately.

Enhancements

  1. Improve Test Coverage

    • Add test for mixed distributions (some secure, some deprecated).
    • Validate ARN consistency in the test using .endswith("distribution/dist-id").
  2. Add Logging/Debug Info (Optional)

    • Useful for troubleshooting real-world deployments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants