Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 70 additions & 0 deletions config/v1/types_tlssecurityprofile.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,27 @@ 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

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?

Choose a reason for hiding this comment

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

//
// +kubebuilder:validation:Enum=X25519;P-256;P-384;P-521;X25519MLKEM768
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"
)

// TLSProfileSpec is the desired behavior of a TLSSecurityProfile.
type TLSProfileSpec struct {
// ciphers is used to specify the cipher algorithms that are negotiated
Expand All @@ -213,6 +234,37 @@ 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.
//
// TLSProfiles Old, Intermediate, Modern are including by default the following
// curves: X25519, P-256, P-384, 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.
Comment on lines +241 to +245
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TLSProfiles Old, Intermediate, Modern are including by default the following
// curves: X25519, P-256, P-384, 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.
// The Old, Intermediate, Modern profiles include the following
// curves: X25519, P-256, P-384, X25519MLKEM768.
// The Custom profile does 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.

Is that a "SHOULD" be used, or a "MUST" be used? That is, does a component violate the API contract if it simply does not allow ECDHE when curves is empty?

Copy link
Author

Choose a reason for hiding this comment

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

I think that this depends on the component desiderata: if it must always be PQC compliant so if curves is empty the component have to use a default curve in order to allow ECDHE and explicity declare which one into its documentation, otherwise if it is allowed for the component to work using a standard DHE , if curves is empty there is no violation if it does not allow ECDHE

//
// For example, to use X25519 and P-256 (yaml):
//
// # Example: Force PQC-only encryption
// apiVersion: config.openshift.io/v1
// kind: APIServer
// spec:
// tlsSecurityProfile:
// type: Custom
// custom:
// ciphers:
// - TLS_AES_128_GCM_SHA256
// - TLS_AES_256_GCM_SHA384
// - TLS_CHACHA20_POLY1305_SHA256
// curves:
// - X25519MLKEM768 # PQC-only: only hybrid quantum-resistant curve
// minTLSVersion: VersionTLS13
Comment on lines +249 to +262
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this render all right in the oc explain output?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question, we generally don't have cross-field examples, but I guess in this case, you cannot specify arbitrary curves that the ciphers you have don't support?

Would anything validate that the curves list is correct for the ciphers you have? Or are all curves supported by all ciphers? Would there ever be a need to say, specify a subset of curves per cipher?

(This might be completely invalid, but as an example, you would want X25519MLKEM768 for TLS_CHACHA20_POLY1305_SHA256, and P-384 for TLS_AES_256_GCM_SHA384, and only those combinations)

Choose a reason for hiding this comment

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

IMO the user facing docs call out that the custom TLS profile "can cause some problems" if misconfigured which is a good hint to the administrator that they should:

  1. Know what they are doing
  2. Validate their config before using the profile in a production environment

I think the best approach here is to offer reasonable valid defaults while keeping custom profiles as a "power user" option.

//
// +optional
Copy link
Contributor

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

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

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=atomic
// +kubebuilder:validation:MaxItems=5
Curves []TLSCurve `json:"curves,omitempty"`
Comment on lines +265 to +267
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to allow duplicates, or should we have some validation to prevent that (something like // +kubebuilder:validation:XValidation:rule=`self.all(x, self.exists_one(y, x == y))`,message="curves cannot contain duplicates")?

Copy link
Author

@davidesalerno davidesalerno Dec 2, 2025

Choose a reason for hiding this comment

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

I think it is a really good idea!

What do you think if we switch to listType=set

Copy link
Contributor

Choose a reason for hiding this comment

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

I think sets should work to ensure that only one entry of each TLSCurve exists

// 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 @@ -283,6 +335,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"AES256-SHA",
"DES-CBC3-SHA",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS10,
},
TLSProfileIntermediateType: {
Expand All @@ -299,6 +357,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 @@ -307,6 +371,12 @@ var TLSProfiles = map[TLSProfileType]*TLSProfileSpec{
"TLS_AES_256_GCM_SHA384",
"TLS_CHACHA20_POLY1305_SHA256",
},
Curves: []TLSCurve{
TLSCurveX25519,
TLSCurveP256,
TLSCurveP384,
TLSCurveX25519MLKEM768,
},
MinTLSVersion: VersionTLS13,
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,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\nthe TLS handshake. Operators may
remove entries their operands do\nnot support.\n\nTLSProfiles
Old, Intermediate, Modern are including by default the following\ncurves:
X25519, P-256, P-384, X25519MLKEM768\nTLSProfiles Custom
do not include any curves by default.\nNOTE: since this
field is optional, if no curves are specified, the default
curves\nused by the underlying TLS library will be used.\n\nFor
example, to use X25519 and P-256 (yaml):\n\n# Example: Force
PQC-only encryption\napiVersion: config.openshift.io/v1\nkind:
APIServer\nspec:\n tlsSecurityProfile:\n type: Custom\n
\ custom:\n ciphers:\n\t - TLS_AES_128_GCM_SHA256\n
\ - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n
\ curves:\n - X25519MLKEM768 # PQC-only: only
hybrid quantum-resistant curve\n minTLSVersion: VersionTLS13"
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
type: string
maxItems: 5
type: array
x-kubernetes-list-type: atomic
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,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\nthe TLS handshake. Operators may
remove entries their operands do\nnot support.\n\nTLSProfiles
Old, Intermediate, Modern are including by default the following\ncurves:
X25519, P-256, P-384, X25519MLKEM768\nTLSProfiles Custom
do not include any curves by default.\nNOTE: since this
field is optional, if no curves are specified, the default
curves\nused by the underlying TLS library will be used.\n\nFor
example, to use X25519 and P-256 (yaml):\n\n# Example: Force
PQC-only encryption\napiVersion: config.openshift.io/v1\nkind:
APIServer\nspec:\n tlsSecurityProfile:\n type: Custom\n
\ custom:\n ciphers:\n\t - TLS_AES_128_GCM_SHA256\n
\ - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n
\ curves:\n - X25519MLKEM768 # PQC-only: only
hybrid quantum-resistant curve\n minTLSVersion: VersionTLS13"
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
type: string
maxItems: 5
type: array
x-kubernetes-list-type: atomic
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,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\nthe TLS handshake. Operators may
remove entries their operands do\nnot support.\n\nTLSProfiles
Old, Intermediate, Modern are including by default the following\ncurves:
X25519, P-256, P-384, X25519MLKEM768\nTLSProfiles Custom
do not include any curves by default.\nNOTE: since this
field is optional, if no curves are specified, the default
curves\nused by the underlying TLS library will be used.\n\nFor
example, to use X25519 and P-256 (yaml):\n\n# Example: Force
PQC-only encryption\napiVersion: config.openshift.io/v1\nkind:
APIServer\nspec:\n tlsSecurityProfile:\n type: Custom\n
\ custom:\n ciphers:\n\t - TLS_AES_128_GCM_SHA256\n
\ - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n
\ curves:\n - X25519MLKEM768 # PQC-only: only
hybrid quantum-resistant curve\n minTLSVersion: VersionTLS13"
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
type: string
maxItems: 5
type: array
x-kubernetes-list-type: atomic
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,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\nthe TLS handshake. Operators may
remove entries their operands do\nnot support.\n\nTLSProfiles
Old, Intermediate, Modern are including by default the following\ncurves:
X25519, P-256, P-384, X25519MLKEM768\nTLSProfiles Custom
do not include any curves by default.\nNOTE: since this
field is optional, if no curves are specified, the default
curves\nused by the underlying TLS library will be used.\n\nFor
example, to use X25519 and P-256 (yaml):\n\n# Example: Force
PQC-only encryption\napiVersion: config.openshift.io/v1\nkind:
APIServer\nspec:\n tlsSecurityProfile:\n type: Custom\n
\ custom:\n ciphers:\n\t - TLS_AES_128_GCM_SHA256\n
\ - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n
\ curves:\n - X25519MLKEM768 # PQC-only: only
hybrid quantum-resistant curve\n minTLSVersion: VersionTLS13"
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
type: string
maxItems: 5
type: array
x-kubernetes-list-type: atomic
Comment on lines +333 to +364
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Schema for tlsSecurityProfile.custom.curves looks correct; fix minor doc/example mismatch.

The field type/enum/maxItems/list-type are consistent with the new TLSCurve API and look fine. The description, however, says “For example, to use X25519 and P-256 (yaml):” but the embedded YAML shows a PQC‑only example using only X25519MLKEM768. Consider aligning the prose and example to avoid confusion.

🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-TechPreviewNoUpgrade.crd.yaml
around lines 333 to 364, the prose says "For example, to use X25519 and P-256
(yaml):" but the embedded YAML shows a PQC-only example using only
X25519MLKEM768; update the doc so the prose and example match by either (A)
changing the prose to indicate this is a PQC-only example using X25519MLKEM768,
or (B) replacing the embedded YAML with a short example that actually lists
X25519 and P-256 under curves (and keeps the surrounding example text
consistent); ensure comments and inline notes reflect whichever example you
choose.

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

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

Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,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\nthe TLS handshake. Operators may
remove entries their operands do\nnot support.\n\nTLSProfiles
Old, Intermediate, Modern are including by default the following\ncurves:
X25519, P-256, P-384, X25519MLKEM768\nTLSProfiles Custom
do not include any curves by default.\nNOTE: since this
field is optional, if no curves are specified, the default
curves\nused by the underlying TLS library will be used.\n\nFor
example, to use X25519 and P-256 (yaml):\n\n# Example: Force
PQC-only encryption\napiVersion: config.openshift.io/v1\nkind:
APIServer\nspec:\n tlsSecurityProfile:\n type: Custom\n
\ custom:\n ciphers:\n\t - TLS_AES_128_GCM_SHA256\n
\ - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n
\ curves:\n - X25519MLKEM768 # PQC-only: only
hybrid quantum-resistant curve\n minTLSVersion: VersionTLS13"
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
type: string
maxItems: 5
type: array
x-kubernetes-list-type: atomic
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,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\nthe TLS handshake. Operators may
remove entries their operands do\nnot support.\n\nTLSProfiles
Old, Intermediate, Modern are including by default the following\ncurves:
X25519, P-256, P-384, X25519MLKEM768\nTLSProfiles Custom
do not include any curves by default.\nNOTE: since this
field is optional, if no curves are specified, the default
curves\nused by the underlying TLS library will be used.\n\nFor
example, to use X25519 and P-256 (yaml):\n\n# Example: Force
PQC-only encryption\napiVersion: config.openshift.io/v1\nkind:
APIServer\nspec:\n tlsSecurityProfile:\n type: Custom\n
\ custom:\n ciphers:\n\t - TLS_AES_128_GCM_SHA256\n
\ - TLS_AES_256_GCM_SHA384\n - TLS_CHACHA20_POLY1305_SHA256\n
\ curves:\n - X25519MLKEM768 # PQC-only: only
hybrid quantum-resistant curve\n minTLSVersion: VersionTLS13"
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
type: string
maxItems: 5
type: array
x-kubernetes-list-type: atomic
minTLSVersion:
description: |-
minTLSVersion is used to specify the minimal version of the TLS protocol
Expand Down
Loading