Skip to content

GEP-91: Address connection coalescing security issue - API updates #3960

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
wants to merge 5 commits into
base: main
Choose a base branch
from
+1,291 −509
Open

GEP-91: Address connection coalescing security issue - API updates #3960

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c7e1ed
GEP-91: Address connection coalescing security issue - API updates
kl52752 Jul 22, 2025
d94294e
review
kl52752 Aug 6, 2025
5cf5c4e
rename GatewayTLSConfig to ListenerTLSConfig
kl52752 Aug 7, 2025
a1d2106
validation mode
kl52752 Aug 8, 2025
e003d67
gateway tls config
kl52752 Aug 12, 2025
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
104 changes: 86 additions & 18 deletions apis/v1/gateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,24 @@ type GatewaySpec struct {
//
// +optional
AllowedListeners *AllowedListeners `json:"allowedListeners,omitempty"`

// TLSConfigs stores TLS configurations for a Gateway.
//
// - If the `port` field in `TLSConfig` is not set, the TLS configuration applies
// to all listeners in the gateway. We call this `default` configuration.
// - If the `port` field in `TLSConfig` is set, the TLS configuration applies
// only to listeners with a matching port. Each port requires a unique TLS configuration.
// - Per-port configurations can override the `default` configuration.
// - The `default` configuration is optional. Clients can apply TLS configuration
// to a subset of listeners by creating only per-port configurations.
// Listeners with a port that does not match any TLS configuration will
// not have `frontendValidation` set.
//
// Support: Core
// +optional
//
// <gateway:experimental>
TLSConfigs []TLSConfig `json:"tlsConfigs,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

🤔 This field name feels a little funny to me. Something about configs as a plural word doesn't seem quite right. Some alternatives that may not be better:

  1. tlsByPort
  2. tlsPerPort
  3. tls
  4. frontendTLS

Since even from the outset port is optional, 1 or 2 probably aren't great. Since there's at least a theoretical world where we add something TLS related here that isn't tied to frontend TLS, we could regret 4 in the future. So maybe 3 is the least bad, although even that adds some confusion as that tls field would have no overlap with listener.tls. Open to other ideas here.

/cc @arkodg @youngnick

Copy link
Contributor

Choose a reason for hiding this comment

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

if there is no strict rule for pluralizing / s slice fields, then +1 for tls

Copy link
Contributor

Choose a reason for hiding this comment

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

I do like the idea of tlsByPort or tlsPerPort, but I think that might also be confusing with the defaulting behavior?

I think that tls is an acceptable alternative though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find the tls name for an array a bit confusing (specially the case when you iterate over tls). If we want "default" configuration to be explicit (see comment) we could consider creating a struct with default configuration (no port at all) and array with per ports override. This way it will be explicit, readable and not at all confusing :)

type GatewayTLSConfig struct {
defaultTLS *FrontendTLSValidation
tlsPerPort []TLSConfig
}

and in GatewaySpec we can name this field as tls

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a different proposal above, but this could also work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I personally prefer mine solution than Nicks. I like it that default configuration is extracted from per ports override and now when default configuration is required it makes much sense.
I need to add more validations for per port overrides like uniquest etc but port = 0 will be forbidden.

@shaneutt @robscott which version do you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

After some reflection, I think that @kl52752's option here is better, agreed. Separating the default config into its own section lets us say "the default config applies to all listeners with matching ports that terminate TLS, unless there is a matching config in the tlsPerPort section. In that case only the config in the tlsPerPort section applies."

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with that approach as well, thanks @kl52752!

Copy link
Member

Choose a reason for hiding this comment

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

👍

}

// AllowedListeners defines which ListenerSets can be attached to this Gateway.
Expand Down Expand Up @@ -414,7 +432,7 @@ type Listener struct {
// the Protocol field is "HTTPS" or "TLS". It is invalid to set this field
// if the Protocol field is "HTTP", "TCP", or "UDP".
//
// The association of SNIs to Certificate defined in GatewayTLSConfig is
// The association of SNIs to Certificate defined in ListenerTLSConfig is
// defined based on the Hostname field for this listener.
//
// The GatewayClass MUST use the longest matching SNI out of all
Expand All @@ -423,7 +441,7 @@ type Listener struct {
// Support: Core
//
// +optional
TLS *GatewayTLSConfig `json:"tls,omitempty"`
TLS *ListenerTLSConfig `json:"tls,omitempty"`

// AllowedRoutes defines the types of routes that MAY be attached to a
// Listener and the trusted namespaces where those Route resources MAY be
Expand Down Expand Up @@ -526,10 +544,10 @@ type GatewayBackendTLS struct {
ClientCertificateRef *SecretObjectReference `json:"clientCertificateRef,omitempty"`
}

// GatewayTLSConfig describes a TLS configuration.
// ListenerTLSConfig describes a TLS configuration for a listener.
//
// +kubebuilder:validation:XValidation:message="certificateRefs or options must be specified when mode is Terminate",rule="self.mode == 'Terminate' ? size(self.certificateRefs) > 0 || size(self.options) > 0 : true"
type GatewayTLSConfig struct {
type ListenerTLSConfig struct {
// Mode defines the TLS behavior for the TLS session initiated by the client.
// There are two possible modes:
//
Expand Down Expand Up @@ -578,18 +596,6 @@ type GatewayTLSConfig struct {
// +kubebuilder:validation:MaxItems=64
CertificateRefs []SecretObjectReference `json:"certificateRefs,omitempty"`

// FrontendValidation holds configuration information for validating the frontend (client).
// Setting this field will require clients to send a client certificate
// required for validation during the TLS handshake. In browsers this may result in a dialog appearing
// that requests a user to specify the client certificate.
// The maximum depth of a certificate chain accepted in verification is Implementation specific.
//
// Support: Extended
//
// +optional
// <gateway:experimental>
FrontendValidation *FrontendTLSValidation `json:"frontendValidation,omitempty"`

// Options are a list of key/value pairs to enable extended TLS
// configuration for each implementation. For example, configuring the
// minimum TLS version or supported cipher suites.
Expand Down Expand Up @@ -624,6 +630,32 @@ const (
TLSModePassthrough TLSModeType = "Passthrough"
)

// TLSConfig describes a TLS configuration that can be applied to all Gateway
// Listeners or to all Listeners matching the Port if set.
type TLSConfig struct {
// The Port indicates the Port Number to which the TLS configuration will be
// applied. If the field is not set the TLS Configuration will be applied to
// all Listeners.
//
// Support: Extended
//
// +optional
// <gateway:experimental>
Port *PortNumber `json:"port,omitempty"`
//
// FrontendValidation holds configuration information for validating the frontend (client).
// Setting this field will result in mutual authentication when connecting to the gateway.
// In browsers this may result in a dialog appearing
// that requests a user to specify the client certificate.
// The maximum depth of a certificate chain accepted in verification is Implementation specific.
//
// Support: Extended
//
// +optional
// <gateway:experimental>
FrontendValidation *FrontendTLSValidation `json:"frontendValidation,omitempty"`
}

// FrontendTLSValidation holds configuration information that can be used to validate
// the frontend initiating the TLS connection
type FrontendTLSValidation struct {
Expand All @@ -640,8 +672,8 @@ type FrontendTLSValidation struct {
// Support: Core - A single reference to a Kubernetes ConfigMap
// with the CA certificate in a key named `ca.crt`.
//
// Support: Implementation-specific (More than one reference, or other kinds
// of resources).
// Support: Implementation-specific (More than one certificate in a ConfigMap
// with different keys or more than one reference, or other kinds of resources).
//
// References to a resource in a different namespace are invalid UNLESS there
// is a ReferenceGrant in the target namespace that allows the certificate
Expand All @@ -654,8 +686,44 @@ type FrontendTLSValidation struct {
// +kubebuilder:validation:MaxItems=8
// +kubebuilder:validation:MinItems=1
CACertificateRefs []ObjectReference `json:"caCertificateRefs,omitempty"`

// FrontendValidationMode defines the mode for validating the client certificate.
// There are two possible modes:
//
// - AllowValidOnly: In this mode, the gateway will accept connections only if
// the client presents a valid certificate. This certificate must successfully
// pass validation against the CA certificates specified in `CACertificateRefs`.
// - AllowInsecureFallback: In this mode, the gateway will accept connections
// even if the client certificate is not presented or fails verification.
//
// This approach delegates client authorization to the backend and introduce
// a significant security risk. It should be used in testing environments or
// on a temporary basis in non-testing environments.
//
// Defaults to AllowValidOnly.
//
// Support: Core
//
// +optional
// +kubebuilder:default=AllowValidOnly
Mode FrontendValidationModeType `json:"mode,omitempty"`
}

// FrontendValidationModeType type defines how a Gateway validates client certificates.
//
// +kubebuilder:validation:Enum=AllowValidOnly;AllowInsecureFallback
type FrontendValidationModeType string

const (
// AllowValidOnly indicates that a client certificate is required
// during the TLS handshake and MUST pass validation.
AllowValidOnly FrontendValidationModeType = "AllowValidOnly"

// AllowInsecureFallback indicates that a client certificate may not be
// presented during the handshake or the validation against CA certificates may fail.
AllowInsecureFallback FrontendValidationModeType = "AllowInsecureFallback"
)

// AllowedRoutes defines which Routes may be attached to this Listener.
type AllowedRoutes struct {
// Namespaces indicates namespaces from which Routes may be attached to this
Expand Down
107 changes: 67 additions & 40 deletions apis/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.

4 changes: 2 additions & 2 deletions apis/v1beta1/gateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ type Listener = v1.Listener
// +k8s:deepcopy-gen=false
type ProtocolType = v1.ProtocolType

// GatewayTLSConfig describes a TLS configuration.
// ListenerTLSConfig describes a TLS configuration.
// +k8s:deepcopy-gen=false
type GatewayTLSConfig = v1.GatewayTLSConfig
type ListenerTLSConfig = v1.ListenerTLSConfig

// TLSModeType type defines how a Gateway handles TLS sessions.
//
Expand Down
2 changes: 1 addition & 1 deletion apisx/v1alpha1/shared_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type (
// +k8s:deepcopy-gen=false
AllowedRoutes = v1.AllowedRoutes
// +k8s:deepcopy-gen=false
GatewayTLSConfig = v1.GatewayTLSConfig
ListenerTLSConfig = v1.ListenerTLSConfig
// +k8s:deepcopy-gen=false
Group = v1.Group
// +k8s:deepcopy-gen=false
Expand Down
4 changes: 2 additions & 2 deletions apisx/v1alpha1/xlistenerset_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,14 @@ type ListenerEntry struct {
// the Protocol field is "HTTPS" or "TLS". It is invalid to set this field
// if the Protocol field is "HTTP", "TCP", or "UDP".
//
// The association of SNIs to Certificate defined in GatewayTLSConfig is
// The association of SNIs to Certificate defined in ListenerTLSConfig is
// defined based on the Hostname field for this listener.
//
// The GatewayClass MUST use the longest matching SNI out of all
// available certificates for any TLS handshake.
//
// +optional
TLS *GatewayTLSConfig `json:"tls,omitempty"`
TLS *ListenerTLSConfig `json:"tls,omitempty"`

// AllowedRoutes defines the types of routes that MAY be attached to a
// Listener and the trusted namespaces where those Route resources MAY be
Expand Down
2 changes: 1 addition & 1 deletion apisx/v1alpha1/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.

13 changes: 13 additions & 0 deletions applyconfiguration/apis/v1/frontendtlsvalidation.go
View file
Open in desktop

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

Loading