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
63 changes: 63 additions & 0 deletions config/v1/types_tlssecurityprofile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,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"
// 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 +193,26 @@ 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 +196 to +198
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.

//
// TLSProfiles Old, Intermediate, Modern are including by default the following
// curves: X25519, P-256, P-384, P-521, X25519MLKEM768
// TLSProfiles Custom do not include any curves by default.
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.

Same comment as above, this seems better suited to the higher-order field documentation.

// NOTE: since this field is optional, if no curves are specified, the default curves
// used by the underlying TLS library will be used.
Comment on lines +203 to +204
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.

What do you think of saying something like:

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.

It generally follows our more standard "omission means no opinion" for configuration APIs while clearing stating that the default behavior depends on the underlying implementation per-component.

//
// For example, to use X25519 and P-256 (yaml):
//
// curves:
// - X25519MLKEM768
//
// +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=5
// +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 @@ -241,6 +286,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"AES256-SHA",
"DES-CBC3-SHA",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS10,
},
TLSProfileIntermediateType: {
Expand All @@ -257,6 +308,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 +322,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"TLS_AES_256_GCM_SHA384",
"TLS_CHACHA20_POLY1305_SHA256",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS13,
},
}
34 changes: 34 additions & 0 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 @@ -325,6 +325,40 @@ 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.

TLSProfiles Old, Intermediate, Modern are including by default the following
curves: X25519, P-256, P-384, P-521, X25519MLKEM768
TLSProfiles Custom do not include any curves by default.
NOTE: since this field is optional, if no curves are specified, the default curves
used by the underlying TLS library will be used.

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

curves:
- X25519MLKEM768
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: 5
type: array
x-kubernetes-list-type: set
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
34 changes: 34 additions & 0 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 @@ -325,6 +325,40 @@ 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.

TLSProfiles Old, Intermediate, Modern are including by default the following
curves: X25519, P-256, P-384, P-521, X25519MLKEM768
TLSProfiles Custom do not include any curves by default.
NOTE: since this field is optional, if no curves are specified, the default curves
used by the underlying TLS library will be used.

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

curves:
- X25519MLKEM768
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: 5
type: array
x-kubernetes-list-type: set
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
5 changes: 5 additions & 0 deletions config/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config/v1/zz_generated.featuregated-crd-manifests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ apiservers.config.openshift.io:
Category: ""
FeatureGates:
- KMSEncryptionProvider
- TLSCurvesConfiguration
FilenameOperatorName: config-operator
FilenameOperatorOrdering: "01"
FilenameRunLevel: "0000_10"
Expand Down
Loading