-
Notifications
You must be signed in to change notification settings - Fork 591
update TLS security profile to align with Mozilla v5.7 guidelines for Go #2704
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
base: master
Are you sure you want to change the base?
Conversation
sanchezl
commented
Feb 10, 2026
- update TLS security profile to align with Mozilla v5.7 guidelines for Go
- make update
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Hello @sanchezl! Some important instructions when contributing to openshift/api: |
📝 WalkthroughWalkthroughThis pull request updates TLS security profile configurations and documentation across Go type definitions, generated swagger documentation, and multiple Kubernetes CRD YAML files. The changes reflect an upgrade from Mozilla TLS configuration guidelines version 5.0 to 5.7, with corresponding revisions to cipher suite examples and descriptions. Several DHE and legacy cipher entries are removed from old and intermediate profiles. Cipher examples are updated from DES-CBC3-SHA to ECDHE-RSA-AES128-GCM-SHA256, and documentation is clarified to specify that TLS 1.3 cipher suites are not configurable and cipher lists consist of ciphersuites followed by Go-specific ciphers. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Review Summary by QodoUpdate TLS security profiles to Mozilla v5.7 guidelines
WalkthroughsDescription• Update TLS security profile to Mozilla v5.7 guidelines • Remove deprecated DHE cipher suites from old/intermediate profiles • Update cipher example from DES-CBC3-SHA to ECDHE-RSA-AES128-GCM-SHA256 • Add clarification that TLS 1.3 cipher suites are not configurable Diagramflowchart LR
A["Mozilla v5.0 TLS Guidelines"] -->|Update to v5.7| B["Updated TLS Profiles"]
B --> C["Remove DHE Ciphers"]
B --> D["Update Documentation"]
B --> E["Modern Cipher Examples"]
C --> F["Old Profile"]
C --> G["Intermediate Profile"]
D --> H["Add TLS 1.3 Clarification"]
E --> I["ECDHE-RSA-AES128-GCM-SHA256"]
File Changes1. config/v1/types_tlssecurityprofile.go
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@config/v1/types_tlssecurityprofile.go`:
- Around line 10-13: Update the grammar in the top-of-file comment describing
TLS profiles in config/v1/types_tlssecurityprofile.go: change the phrase "The
cipher lists consist the configuration's" to "The cipher lists consist of the
configuration's" (or equivalent correct wording) in the comment associated with
the TLSSecurityProfile/type description to ensure user-facing docs read
correctly.
- Around line 147-155: Update the comment above the ciphers field in
types_tlssecurityprofile.go to tighten the wording: change "Operators may remove
entries their operands do not support." to "Operators may remove entries that
their operands do not support.", rephrase the example line to "For example, to
allow only ECDHE-RSA-AES128-GCM-SHA256 (yaml):" and simplify the TLS 1.3 note to
"TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are fixed and enabled
whenever TLS 1.3 is negotiated." Ensure these edits are applied to the ciphers
comment block so the meaning remains identical but grammar and clarity are
improved.
In
`@payload-manifests/crds/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yaml`:
- Around line 318-327: Update the ciphers description to improve grammar and
clarity: reword the paragraph that currently begins "ciphers is used to specify
the cipher algorithms that are negotiated during the TLS handshake..." to a
clearer sentence such as "The ciphers field specifies the cipher algorithms
negotiated during the TLS handshake." Also clarify the next sentence to
"Operators may remove entries that their operands do not support." and keep the
example and TLS 1.3 note intact; edit the description associated with the
ciphers field in the CustomNoUpgrade CRD so it reads smoothly and consistently.
- Around line 414-417: Update the profile description string that currently
reads "The profiles are based on version 5.7 of the Mozilla Server Side TLS
configuration guidelines. The cipher lists consist the configuration's
"ciphersuites" followed by the Go-specific "ciphers" from the guidelines." to
use correct grammar: change "consist the" to "consist of" so the sentence reads
"...The cipher lists consist of the configuration's "ciphersuites" followed by
the Go-specific "ciphers"...". Locate and edit the exact description text in the
CRD resource block containing the "The profiles are based on version 5.7 of the
Mozilla Server Side TLS configuration guidelines" string.
In
`@payload-manifests/crds/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yaml`:
- Around line 414-417: Fix the grammatical typo in the CRD comment by inserting
the missing word "of" into the sentence fragment "The cipher lists consist the
configuration's" so it reads "The cipher lists consist of the configuration's";
update the string containing that sentence in the manifest comment block (the
multi-line comment that includes "The profiles are based on version 5.7 of the
Mozilla Server Side TLS... See:
https://ssl-config.mozilla.org/guidelines/5.7.json") to reflect the corrected
wording.
| // The profiles are based on version 5.7 of the Mozilla Server Side TLS | ||
| // configuration guidelines. The cipher lists consist the configuration's | ||
| // "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | ||
| // See: https://ssl-config.mozilla.org/guidelines/5.7.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammar in TLS profile description
Use “consist of” for correct phrasing in the user-facing doc.
Proposed fix
- // configuration guidelines. The cipher lists consist the configuration's
+ // configuration guidelines. The cipher lists consist of the configuration's📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // The profiles are based on version 5.7 of the Mozilla Server Side TLS | |
| // configuration guidelines. The cipher lists consist the configuration's | |
| // "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | |
| // See: https://ssl-config.mozilla.org/guidelines/5.7.json | |
| // The profiles are based on version 5.7 of the Mozilla Server Side TLS | |
| // configuration guidelines. The cipher lists consist of the configuration's | |
| // "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | |
| // See: https://ssl-config.mozilla.org/guidelines/5.7.json |
🤖 Prompt for AI Agents
In `@config/v1/types_tlssecurityprofile.go` around lines 10 - 13, Update the
grammar in the top-of-file comment describing TLS profiles in
config/v1/types_tlssecurityprofile.go: change the phrase "The cipher lists
consist the configuration's" to "The cipher lists consist of the
configuration's" (or equivalent correct wording) in the comment associated with
the TLSSecurityProfile/type description to ensure user-facing docs read
correctly.
| // ciphers is used to specify the cipher algorithms that are negotiated | ||
| // during the TLS handshake. Operators may remove entries their operands | ||
| // do not support. For example, to use DES-CBC3-SHA (yaml): | ||
| // during the TLS handshake. Operators may remove entries their operands | ||
| // do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml): | ||
| // | ||
| // ciphers: | ||
| // - DES-CBC3-SHA | ||
| // - ECDHE-RSA-AES128-GCM-SHA256 | ||
| // | ||
| // TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are not configurable | ||
| // and are always enabled when TLS 1.3 is negotiated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tighten wording in cipher description
Minor grammar fix to avoid ambiguity.
Proposed fix
- // during the TLS handshake. Operators may remove entries their operands
- // do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml):
+ // during the TLS handshake. Operators may remove entries that their operands
+ // do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ciphers is used to specify the cipher algorithms that are negotiated | |
| // during the TLS handshake. Operators may remove entries their operands | |
| // do not support. For example, to use DES-CBC3-SHA (yaml): | |
| // during the TLS handshake. Operators may remove entries their operands | |
| // do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml): | |
| // | |
| // ciphers: | |
| // - DES-CBC3-SHA | |
| // - ECDHE-RSA-AES128-GCM-SHA256 | |
| // | |
| // TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are not configurable | |
| // and are always enabled when TLS 1.3 is negotiated. | |
| // ciphers is used to specify the cipher algorithms that are negotiated | |
| // during the TLS handshake. Operators may remove entries that their operands | |
| // do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml): | |
| // | |
| // ciphers: | |
| // - ECDHE-RSA-AES128-GCM-SHA256 | |
| // | |
| // TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are not configurable | |
| // and are always enabled when TLS 1.3 is negotiated. |
🤖 Prompt for AI Agents
In `@config/v1/types_tlssecurityprofile.go` around lines 147 - 155, Update the
comment above the ciphers field in types_tlssecurityprofile.go to tighten the
wording: change "Operators may remove entries their operands do not support." to
"Operators may remove entries that their operands do not support.", rephrase the
example line to "For example, to allow only ECDHE-RSA-AES128-GCM-SHA256 (yaml):"
and simplify the TLS 1.3 note to "TLS 1.3 cipher suites (e.g.
TLS_AES_128_GCM_SHA256) are fixed and enabled whenever TLS 1.3 is negotiated."
Ensure these edits are applied to the ciphers comment block so the meaning
remains identical but grammar and clarity are improved.
| ciphers is used to specify the cipher algorithms that are negotiated | ||
| during the TLS handshake. Operators may remove entries their operands | ||
| do not support. For example, to use DES-CBC3-SHA (yaml): | ||
| during the TLS handshake. Operators may remove entries their operands | ||
| do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml): | ||
|
|
||
| ciphers: | ||
| - DES-CBC3-SHA | ||
| - ECDHE-RSA-AES128-GCM-SHA256 | ||
|
|
||
| TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are not configurable | ||
| and are always enabled when TLS 1.3 is negotiated. | ||
| items: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammar in ciphers description
Minor wording tweak for readability.
Proposed fix
- during the TLS handshake. Operators may remove entries their operands
- do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml):
+ during the TLS handshake. Operators may remove entries that their operands
+ do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ciphers is used to specify the cipher algorithms that are negotiated | |
| during the TLS handshake. Operators may remove entries their operands | |
| do not support. For example, to use DES-CBC3-SHA (yaml): | |
| during the TLS handshake. Operators may remove entries their operands | |
| do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml): | |
| ciphers: | |
| - DES-CBC3-SHA | |
| - ECDHE-RSA-AES128-GCM-SHA256 | |
| TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are not configurable | |
| and are always enabled when TLS 1.3 is negotiated. | |
| items: | |
| ciphers is used to specify the cipher algorithms that are negotiated | |
| during the TLS handshake. Operators may remove entries that their operands | |
| do not support. For example, to use only ECDHE-RSA-AES128-GCM-SHA256 (yaml): | |
| ciphers: | |
| - ECDHE-RSA-AES128-GCM-SHA256 | |
| TLS 1.3 cipher suites (e.g. TLS_AES_128_GCM_SHA256) are not configurable | |
| and are always enabled when TLS 1.3 is negotiated. | |
| items: |
🤖 Prompt for AI Agents
In
`@payload-manifests/crds/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yaml`
around lines 318 - 327, Update the ciphers description to improve grammar and
clarity: reword the paragraph that currently begins "ciphers is used to specify
the cipher algorithms that are negotiated during the TLS handshake..." to a
clearer sentence such as "The ciphers field specifies the cipher algorithms
negotiated during the TLS handshake." Also clarify the next sentence to
"Operators may remove entries that their operands do not support." and keep the
example and TLS 1.3 note intact; edit the description associated with the
ciphers field in the CustomNoUpgrade CRD so it reads smoothly and consistently.
| The profiles are based on version 5.7 of the Mozilla Server Side TLS | ||
| configuration guidelines. The cipher lists consist the configuration's | ||
| "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | ||
| See: https://ssl-config.mozilla.org/guidelines/5.7.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammar in profile description
“Consist of” is the correct phrasing.
Proposed fix
- configuration guidelines. The cipher lists consist the configuration's
+ configuration guidelines. The cipher lists consist of the configuration's📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The profiles are based on version 5.7 of the Mozilla Server Side TLS | |
| configuration guidelines. The cipher lists consist the configuration's | |
| "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | |
| See: https://ssl-config.mozilla.org/guidelines/5.7.json | |
| The profiles are based on version 5.7 of the Mozilla Server Side TLS | |
| configuration guidelines. The cipher lists consist of the configuration's | |
| "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | |
| See: https://ssl-config.mozilla.org/guidelines/5.7.json |
🤖 Prompt for AI Agents
In
`@payload-manifests/crds/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yaml`
around lines 414 - 417, Update the profile description string that currently
reads "The profiles are based on version 5.7 of the Mozilla Server Side TLS
configuration guidelines. The cipher lists consist the configuration's
"ciphersuites" followed by the Go-specific "ciphers" from the guidelines." to
use correct grammar: change "consist the" to "consist of" so the sentence reads
"...The cipher lists consist of the configuration's "ciphersuites" followed by
the Go-specific "ciphers"...". Locate and edit the exact description text in the
CRD resource block containing the "The profiles are based on version 5.7 of the
Mozilla Server Side TLS configuration guidelines" string.
| The profiles are based on version 5.7 of the Mozilla Server Side TLS | ||
| configuration guidelines. The cipher lists consist the configuration's | ||
| "ciphersuites" followed by the Go-specific "ciphers" from the guidelines. | ||
| See: https://ssl-config.mozilla.org/guidelines/5.7.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor grammatical issue: missing "of".
Line 415-416 reads "The cipher lists consist the configuration's" but should be "The cipher lists consist of the configuration's".
📝 Proposed fix
The profiles are based on version 5.7 of the Mozilla Server Side TLS
- configuration guidelines. The cipher lists consist the configuration's
+ configuration guidelines. The cipher lists consist of the configuration's
"ciphersuites" followed by the Go-specific "ciphers" from the guidelines.🤖 Prompt for AI Agents
In
`@payload-manifests/crds/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yaml`
around lines 414 - 417, Fix the grammatical typo in the CRD comment by inserting
the missing word "of" into the sentence fragment "The cipher lists consist the
configuration's" so it reads "The cipher lists consist of the configuration's";
update the string containing that sentence in the manifest comment block (the
multi-line comment that includes "The profiles are based on version 5.7 of the
Mozilla Server Side TLS... See:
https://ssl-config.mozilla.org/guidelines/5.7.json") to reflect the corrected
wording.
Code Review by Qodo
1. Old profile lost DHE
|
|
@sanchezl: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |