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.

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

replace TLSv1.2_2019 with TLSv1.2_2021 as default policy #294

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

Conversation

jamerply
Copy link

@jamerply jamerply commented Jan 9, 2024

what

This PR updates the mimimum_protocol_version variable so that it defaults to TLSv1.2_2021 (the current recommended security policy recommended by AWS) instead of TLSv1.2_2019.

why

The most current security policy is no longer TLSv1.2_2019 but is TLSv1.2_2021.

references

See the "Security Policy" heading under the "Distribution Setting" section of the AWS CloudFront Documentation for further information.

@jamerply jamerply requested review from a team as code owners January 9, 2024 19:41
@hans-d hans-d added wip Work in Progress: Not ready for final review or merge and removed wip Work in Progress: Not ready for final review or merge labels Mar 2, 2024
@hans-d
Copy link

hans-d commented Mar 2, 2024

/terratest

Copy link

mergify bot commented Mar 9, 2024

Thanks @jamerply for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

@mergify mergify bot added the triage Needs triage label Mar 9, 2024
Copy link
Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

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

The problem is, this is a potentially breaking change that will manifest as some old clients failing to connect all of a sudden, a problem which will be difficult to trace back to updating this component.

I think instead we should just recommend that users explicitly set this to the newer version.

@jamerply
Copy link
Author

@Nuru

The problem is, this is a potentially breaking change that will manifest as some old clients failing to connect all of a sudden, a problem which will be difficult to trace back to updating this component.

If users have pinned their modules to a particular version per CloudPosse's own recommendation, wouldn't that prevent the breaking change unless they knowingly updated to the newer version?

@Gowiem
Copy link
Member

Gowiem commented May 10, 2024

@Nuru thoughts on just making this a major revision change? I agree that we should upgrade this for future consumers of this module, otherwise we're encouraging an old and outdated TLS. The other route is to remove the default altogether, but either way we'll want to do a major version rev. Let me know your thoughts and I can work with @jamerply to push this forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants