Skip to content

NE-2334: Implement enhancement in OpenShift API to support for TLS curves in TLSProfile #2583

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

Open
davidesalerno wants to merge 1 commit into
base: master
Choose a base branch
from
Open

NE-2334: Implement enhancement in OpenShift API to support for TLS curves in TLSProfile #2583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 34 additions & 0 deletions config/v1/types_ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,37 @@ const (
// "ControlPlane" is for having router pods placed on control-plane nodes by default.
DefaultPlacementControlPlane DefaultPlacement = "ControlPlane"
)

// IngressCustomTLSProfile defines the parameters for a custom TLS profile on Ingress.
type IngressCustomTLSProfile struct {
// ciphers is the list of allowed cipher suites.
// +optional
Ciphers []string `json:"ciphers,omitempty"`

// minTLSVersion is the minimum TLS version allowed.
// +optional
MinTLSVersion TLSProtocolVersion `json:"minTLSVersion,omitempty"`

// curves is the list of allowed elliptic curves.
// This field is specific to Ingress to support legacy clients or PQC
// configurations (e.g., X25519MLKEM768) that core components may not yet support.
//
// +optional
// +listType=set
// +openshift:enable:FeatureGate=TLSCurvesConfiguration
Curves []TLSCurve `json:"curves,omitempty"`
}


// IngressTLSProfileSpec defines the TLS settings for the Ingress Controller.
// This struct diverges from the standard configv1.TLSSecurityProfile to allow
// specific customizations (like Curves) that may not be supported by core components.
type IngressTLSProfileSpec struct {
// type is the TLS security profile type.
// +optional
Type TLSProfileType `json:"type,omitempty"`

// custom is a user-defined TLS security profile.
// +optional
Custom *IngressCustomTLSProfile `json:"custom,omitempty"`
}
80 changes: 77 additions & 3 deletions config/v1/types_tlssecurityprofile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type TLSSecurityProfile struct {
// The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
// by the "old" profile ciphers.
//
// The curve list is including by default the following curves:
// X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.
//
Comment on lines +28 to +30
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Documentation lists incorrect curve names and count.

The documentation states the default curves include P256r1MLKEM1024, P384r1MLKEM1024, but:

  1. The actual constant is P256r1MLKEM768 (not MLKEM1024) - see line 190
  2. The actual defaults (lines 300-305) only include 4 curves: X25519, P-256, P-384, X25519MLKEM768

The same inconsistency appears at lines 75-77 and 99-100. Update the documentation to match the actual implementation.

🤖 Prompt for AI Agents 
In `@config/v1/types_tlssecurityprofile.go` around lines 28 - 30, Update the doc
comments in types_tlssecurityprofile.go so they match the actual constants and
defaults: replace incorrect mentions of P256r1MLKEM1024 and P384r1MLKEM1024 with
the actual constant names (e.g., P256r1MLKEM768 and X25519MLKEM768 as defined)
and state the correct default curve list (X25519, P-256, P-384, X25519MLKEM768)
to match the implementation (see constants like P256r1MLKEM768 and the default
curve preferences definitions).

// This profile is equivalent to a Custom profile specified as:
// minTLSVersion: VersionTLS10
// ciphers:
Expand Down Expand Up @@ -69,6 +72,9 @@ type TLSSecurityProfile struct {
// The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
// by the "intermediate" profile ciphers.
//
// The curve list is including by default the following curves:
// X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.
//
// This profile is equivalent to a Custom profile specified as:
// minTLSVersion: VersionTLS12
// ciphers:
Expand All @@ -90,7 +96,8 @@ type TLSSecurityProfile struct {

// modern is a TLS security profile for use with clients that support TLS 1.3 and
// do not need backward compatibility for older clients.
//
// the curve list is including by default the following curves:
// X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.
// This profile is equivalent to a Custom profile specified as:
// minTLSVersion: VersionTLS13
// ciphers:
Expand All @@ -103,8 +110,11 @@ type TLSSecurityProfile struct {
Modern *ModernTLSProfile `json:"modern,omitempty"`

// custom is a user-defined TLS security profile. Be extremely careful using a custom
// profile as invalid configurations can be catastrophic. An example custom profile
// looks like this:
// profile as invalid configurations can be catastrophic.
//
// The curve list for this profile is empty by default.
//
// An example custom profile looks like this:
//
// minTLSVersion: VersionTLS11
// ciphers:
Expand Down Expand Up @@ -157,6 +167,31 @@ const (
TLSProfileCustomType TLSProfileType = "Custom"
)

// TLSCurve is a named curve identifier that can be used in TLSProfile.Curves.
// There is a one-to-one mapping between these names and the curve IDs defined
// in crypto/tls package based on IANA's "TLS Supported Groups" registry:
// https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8
Copy link

@kannon92 kannon92 Dec 16, 2025

Choose a reason for hiding this comment

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

Looking at cryto/tls, I see the following values.

type CurveID uint16

const (
	CurveP256          CurveID = 23
	CurveP384          CurveID = 24
	CurveP521          CurveID = 25
	X25519             CurveID = 29
	X25519MLKEM768     CurveID = 4588
	SecP256r1MLKEM768  CurveID = 4587
	SecP384r1MLKEM1024 CurveID = 4589
)

Why are we not including all the values?

Copy link

@kannon92 kannon92 Dec 16, 2025

Choose a reason for hiding this comment

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

ref: https://github.com/golang/go/blob/master/src/crypto/tls/common.go#L145

Copy link
Author

@davidesalerno davidesalerno Dec 22, 2025

Choose a reason for hiding this comment

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

Ok so I think we can align to those values the available curve supported

//
// +kubebuilder:validation:Enum=X25519;P-256;P-384;P-521;X25519MLKEM768;P256r1MLKEM768;P384r1MLKEM1024
type TLSCurve string

const (
// TLSCurveX25519 represents X25519.
TLSCurveX25519 TLSCurve = "X25519"
// TLSCurveP256 represents P-256 (secp256r1).
TLSCurveP256 TLSCurve = "P-256"
Comment on lines +181 to +182
Copy link
Contributor

@candita candita Jan 29, 2026

Choose a reason for hiding this comment

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

Naming nit - please include the whole (well-known) curve name here and elsewhere, e.g.:

Suggested change
// TLSCurveP256 represents P-256 (secp256r1).
TLSCurveP256 TLSCurve = "P-256"
// TLSCurveSecp256r1 represents P-256 (secp256r1).
TLSCurveSecp256r1 TLSCurve = "secp256r1"

// TLSCurveP384 represents P-384 (secp384r1).
TLSCurveP384 TLSCurve = "P-384"
// TLSCurveP521 represents P-521 (secp521r1).
TLSCurveP521 TLSCurve = "P-521"
// TLSCurveX25519MLKEM768 represents X25519MLKEM768.
TLSCurveX25519MLKEM768 TLSCurve = "X25519MLKEM768"
// TLSCurveP256r1MLKEM768 represents P256r1MLKEM768 (secp256r1MLKEM768).
TLSCurveP256r1MLKEM768 TLSCurve = "P256r1MLKEM768"
// TLSCurveP384r1MLKEM1024 represents P384r1MLKEM1024 (secp384r1MLKEM1024).
TLSCurveP384r1MLKEM1024 TLSCurve = "P384r1MLKEM1024"
)

// TLSProfileSpec is the desired behavior of a TLSSecurityProfile.
type TLSProfileSpec struct {
// ciphers is used to specify the cipher algorithms that are negotiated
Expand All @@ -168,6 +203,24 @@ type TLSProfileSpec struct {
//
// +listType=atomic
Ciphers []string `json:"ciphers"`
// curves is used to specify the elliptic curves that are used during
// the TLS handshake. Operators may remove entries their operands do
// not support.
Comment on lines +206 to +208
Copy link
Contributor

@everettraven everettraven Jan 12, 2026

Choose a reason for hiding this comment

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

Suggested change
// curves is used to specify the elliptic curves that are used during
// the TLS handshake. Operators may remove entries their operands do
// not support.
// curves is an optional field used to specify the elliptic curves that are used during
// the TLS handshake. Operators may remove entries their operands do
// not support.

//
// When omitted, this means no opinion and the platform is left to choose reasonable defaults which are subject to
// change over time and may be different per platform component depending on the underlying TLS libraries they use.
//
// For example, to use X25519 and P-256 (yaml):
//
// curves:
// - X25519
// - P-256
//
// +optional
Copy link
Contributor

@yuqi-zhang yuqi-zhang Nov 20, 2025

Choose a reason for hiding this comment

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

optional fields should have godoc around what happens if the field is not set (i.e. what is the default behaviour)

Copy link
Author

@davidesalerno davidesalerno Nov 27, 2025

Choose a reason for hiding this comment

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

I added a note to address this case. What do you think?

Copy link
Contributor

@yuqi-zhang yuqi-zhang Dec 4, 2025

Choose a reason for hiding this comment

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

Technically you don't need the NOTE: since this field is optional, in the godoc, since it will be explained as an optional field, but up to you if you want to highlight that. Otherwise looks fine to me

// +listType=set
// +kubebuilder:validation:MaxItems=7
// +openshift:enable:FeatureGate=TLSCurvesConfiguration
Curves []TLSCurve `json:"curves,omitempty"`
Copy link
Contributor

@everettraven everettraven Jan 12, 2026

Choose a reason for hiding this comment

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

Would there be any difference in setting this to curves: [] and omitting the field altogether?

// minTLSVersion is used to specify the minimal version of the TLS protocol
// that is negotiated during the TLS handshake. For example, to use TLS
// versions 1.1, 1.2 and 1.3 (yaml):
Expand Down Expand Up @@ -204,6 +257,9 @@ const (
// configuration guidelines (2019-06-28) with TLS 1.3 cipher suites prepended for
// forward compatibility. See: https://ssl-config.mozilla.org/guidelines/5.0.json
//
// TLSProfiles Old, Intermediate, Modern are including by default the following
// curves: X25519, P-256, P-384, P-521, X25519MLKEM768
//
Comment on lines +260 to +262
Copy link

@coderabbitai coderabbitai bot Jan 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Documentation accurately reflects actual defaults - LGTM.

The comment correctly states that defaults include X25519, P-256, P-384, P-521, X25519MLKEM768. However, note that P-521 is listed here but is not actually in the defaults at lines 300-305, 322-327, 336-341. If P-521 should be in the defaults, add it; otherwise, remove it from this documentation.

🤖 Prompt for AI Agents 
In `@config/v1/types_tlssecurityprofile.go` around lines 260 - 262, The comment
above the TLSProfiles defaults incorrectly lists P-521 as included; update the
documentation to match the actual defaults or add P-521 to the profiles so they
match: either remove P-521 from the top comment "// TLSProfiles Old,
Intermediate, Modern are including by default the following curves: X25519,
P-256, P-384, P-521, X25519MLKEM768" or add P-521 to the default curve lists
used by the TLSProfiles (the Old, Intermediate, Modern definitions referenced
around lines where defaults are defined) so the comment and the TLSProfiles
defaults are consistent.

// NOTE: The caller needs to make sure to check that these constants are valid
// for their binary. Not all entries map to values for all binaries. In the case
// of ties, the kube-apiserver wins. Do not fail, just be sure to include only
Expand Down Expand Up @@ -241,6 +297,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"AES256-SHA",
"DES-CBC3-SHA",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS10,
},
TLSProfileIntermediateType: {
Expand All @@ -257,6 +319,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"DHE-RSA-AES128-GCM-SHA256",
"DHE-RSA-AES256-GCM-SHA384",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS12,
},
TLSProfileModernType: {
Expand All @@ -265,6 +333,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"TLS_AES_256_GCM_SHA384",
"TLS_CHACHA20_POLY1305_SHA256",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS13,
},
}
48 changes: 45 additions & 3 deletions ...zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,11 @@ spec:
custom:
description: |-
custom is a user-defined TLS security profile. Be extremely careful using a custom
profile as invalid configurations can be catastrophic. An example custom profile
looks like this:
profile as invalid configurations can be catastrophic.

The curve list for this profile is empty by default.

An example custom profile looks like this:

minTLSVersion: VersionTLS11
ciphers:
Expand All @@ -325,6 +328,38 @@ spec:
type: string
type: array
x-kubernetes-list-type: atomic
curves:
description: |-
curves is used to specify the elliptic curves that are used during
the TLS handshake. Operators may remove entries their operands do
not support.

When omitted, this means no opinion and the platform is left to choose reasonable defaults which are subject to
change over time and may be different per platform component depending on the underlying TLS libraries they use.

For example, to use X25519 and P-256 (yaml):

curves:
- X25519
- P-256
items:
description: |-
TLSCurve is a named curve identifier that can be used in TLSProfile.Curves.
There is a one-to-one mapping between these names and the curve IDs defined
in crypto/tls package based on IANA's "TLS Supported Groups" registry:
https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8
enum:
- X25519
- P-256
- P-384
- P-521
- X25519MLKEM768
- P256r1MLKEM768
- P384r1MLKEM1024
type: string
maxItems: 7
type: array
x-kubernetes-list-type: set
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand All @@ -348,6 +383,9 @@ spec:
The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
by the "intermediate" profile ciphers.

The curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.

This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS12
ciphers:
Expand All @@ -368,7 +406,8 @@ spec:
description: |-
modern is a TLS security profile for use with clients that support TLS 1.3 and
do not need backward compatibility for older clients.

the curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.
This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS13
ciphers:
Expand All @@ -385,6 +424,9 @@ spec:
The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
by the "old" profile ciphers.

The curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.

This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS10
ciphers:
Expand Down
16 changes: 13 additions & 3 deletions config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-Default.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,11 @@ spec:
custom:
description: |-
custom is a user-defined TLS security profile. Be extremely careful using a custom
profile as invalid configurations can be catastrophic. An example custom profile
looks like this:
profile as invalid configurations can be catastrophic.

The curve list for this profile is empty by default.

An example custom profile looks like this:

minTLSVersion: VersionTLS11
ciphers:
Expand Down Expand Up @@ -279,6 +282,9 @@ spec:
The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
by the "intermediate" profile ciphers.

The curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.

This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS12
ciphers:
Expand All @@ -299,7 +305,8 @@ spec:
description: |-
modern is a TLS security profile for use with clients that support TLS 1.3 and
do not need backward compatibility for older clients.

the curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.
This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS13
ciphers:
Expand All @@ -316,6 +323,9 @@ spec:
The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
by the "old" profile ciphers.

The curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.

This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS10
ciphers:
Expand Down
16 changes: 13 additions & 3 deletions ...enerated.crd-manifests/0000_10_config-operator_01_apiservers-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,11 @@ spec:
custom:
description: |-
custom is a user-defined TLS security profile. Be extremely careful using a custom
profile as invalid configurations can be catastrophic. An example custom profile
looks like this:
profile as invalid configurations can be catastrophic.

The curve list for this profile is empty by default.

An example custom profile looks like this:

minTLSVersion: VersionTLS11
ciphers:
Expand Down Expand Up @@ -348,6 +351,9 @@ spec:
The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
by the "intermediate" profile ciphers.

The curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.

This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS12
ciphers:
Expand All @@ -368,7 +374,8 @@ spec:
description: |-
modern is a TLS security profile for use with clients that support TLS 1.3 and
do not need backward compatibility for older clients.

the curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.
This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS13
ciphers:
Expand All @@ -385,6 +392,9 @@ spec:
The cipher list includes TLS 1.3 ciphers for forward compatibility, followed
by the "old" profile ciphers.

The curve list is including by default the following curves:
X25519, P-256, P-384, P-521, X25519MLKEM768, P256r1MLKEM1024, P384r1MLKEM1024.

This profile is equivalent to a Custom profile specified as:
minTLSVersion: VersionTLS10
ciphers:
Expand Down
Loading