🌱 Update TLS profiles to Mozilla guidelines v5.8#2631
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Updates the repo’s TLS profile definitions and generator to align with Mozilla’s TLS configuration guidelines v6.0, including new curve and cipher naming updates while keeping the legacy “old” profile available for backward compatibility.
Changes:
- Regenerated
mozilla_data.gofrom Mozilla guidelines v6 and added the X25519MLKEM768 hybrid curve to modern/intermediate profiles. - Updated cipher constants for CHACHA20_POLY1305 to the SHA256-suffixed names in the intermediate profile.
- Updated
update-tls-profiles.shto consume the new JSON structure (ciphers.iana) and to preserve the v5.7 “old” profile when it’s removed upstream.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/shared/util/tlsprofiles/tlsprofiles.go | Adds X25519MLKEM768 curve ID aliasing and allows it in custom curve parsing. |
| internal/shared/util/tlsprofiles/mozilla_data.go | Regenerates TLS profile data to Mozilla v6, adding the new curve and updated cipher constants. |
| hack/tools/update-tls-profiles.sh | Updates generator for Mozilla v6 JSON (ciphers.iana) and adds conditional handling for removed “old” profile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
89b7b9f to
89b7085
Compare
89b7085 to
c7b4c59
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
I think we should update the old profile to the last time it existed, which was 5.8:
https://ssl-config.mozilla.org/guidelines/5.8.json
So, perhaps run it against that version, then the latest.
|
I'm actually surprised it didn't break when 5.8 was introduced. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2631 +/- ##
=======================================
Coverage 68.95% 68.95%
=======================================
Files 139 139
Lines 9891 9891
=======================================
Hits 6820 6820
Misses 2562 2562
Partials 509 509
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks @camilamacedo86! |
The failure was on purpose, to detect a change in the policy, but because the change was incompatible (i.e. more than just a change in cipher suites), the script threw an error. |
c7b4c59 to
60f6e68
Compare
| # Check if old profile exists (used for both header note and profile generation) | ||
| has_old_profile=false | ||
| if ${JQ} -e '.configurations.old' ${TMPFILE} > /dev/null 2>&1; then | ||
| has_old_profile=true | ||
| old_profile_note="" | ||
| else | ||
| old_profile_note=" | ||
| // NOTE: The \"old\" profile is preserved from Mozilla v5.8 for backwards compatibility, | ||
| // as it was removed from Mozilla guidelines v${version}." | ||
| fi | ||
|
|
There was a problem hiding this comment.
At this point, the old configuration is gone in 6.0+, not sure we actually need this check anymore, but it can be handled later.
Minimal changes to make verify work: - Change INPUT URL from latest.json to 5.8.json - Update ciphers.go to ciphers.iana (Mozilla v5.8 format change) - Add X25519MLKEM768 post-quantum curve constant - Filter out unsupported ciphers not in Go's crypto/tls: - TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 - TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 - TLS_RSA_WITH_AES_256_CBC_SHA256 - Update CHACHA20_POLY1305 constants to include _SHA256 suffix Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
60f6e68 to
dbfcaeb
Compare
|
Did you remove the test changes? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| OUTPUT=internal/shared/util/tlsprofiles/mozilla_data.go | ||
| INPUT=https://ssl-config.mozilla.org/guidelines/latest.json | ||
| INPUT=https://ssl-config.mozilla.org/guidelines/5.8.json | ||
|
|
There was a problem hiding this comment.
PR metadata/title mentions updating to Mozilla TLS guidelines v6.0, but this script is pinned to guidelines/5.8.json (and the generated mozilla_data.go reports DATA VERSION 5.8). Please align the PR title/description with the actual pinned version, or update the script/data generation to the intended v6.0 source URL/version so make verify stays consistent with the stated change.
| // This is primarily so that we don't have to rewrite curve values in mozilla_data.go | ||
| const ( | ||
| X25519 tls.CurveID = tls.X25519 | ||
| prime256v1 tls.CurveID = tls.CurveP256 | ||
| secp384r1 tls.CurveID = tls.CurveP384 | ||
| secp521r1 tls.CurveID = tls.CurveP521 | ||
| X25519MLKEM768 tls.CurveID = tls.X25519MLKEM768 | ||
| X25519 tls.CurveID = tls.X25519 | ||
| prime256v1 tls.CurveID = tls.CurveP256 | ||
| secp384r1 tls.CurveID = tls.CurveP384 | ||
| secp521r1 tls.CurveID = tls.CurveP521 | ||
| ) | ||
|
|
||
| var curves = map[string]tls.CurveID{ | ||
| "X25519": tls.X25519, | ||
| "prime256v1": tls.CurveP256, | ||
| "secp384r1": tls.CurveP384, | ||
| "secp521r1": tls.CurveP521, | ||
| "X25519MLKEM768": tls.X25519MLKEM768, | ||
| "X25519": tls.X25519, | ||
| "prime256v1": tls.CurveP256, | ||
| "secp384r1": tls.CurveP384, | ||
| "secp521r1": tls.CurveP521, | ||
| } |
There was a problem hiding this comment.
New curve support (X25519MLKEM768) is added here, but there is no test that curveId/curveSlice.Set accepts this new value. Consider extending the existing TLS profile flag/unit tests to include X25519MLKEM768 so regressions in curve parsing are caught.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4510b1b
into
operator-framework:main
Problem
make verifyis broken. It starts to fail withSolution
Mozilla updated their TLS configuration guidelines to v6.0, which includes:
Updated update-tls-profiles.sh to handle the new JSON structure and added X25519MLKEM768 curve support to tlsprofiles package.
Why it happened?
The failure was caused by Mozilla updating their TLS guidelines from v5.7 to v6.0, which introduced breaking changes to the JSON structure.