-
Notifications
You must be signed in to change notification settings - Fork 618
gep: refine CACertificateRefs description for frontend TLS #4183
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: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,21 +35,28 @@ This proposal adds the ability to validate the TLS certificate presented by the | |
| These two validation mechanisms operate independently and can be used simultaneously. | ||
| * Introduce a `caCertificateRefs` field within `FrontendTLSValidation` that can be used to specify a list of CA Certificates that can be used as a trust anchor to validate the certificates presented by the client. | ||
| * Add a new `FrontendValidationModeType` enum within `FrontendTLSValidation` indicating how gateway should validate client certificates. As for now we support following values but it might change in the future: | ||
| 1) `AllowValidOnly` (Core Support) | ||
| 2) `AllowInsecureFallback` (Extended Support) | ||
| 1. `AllowValidOnly` (Core Support) | ||
| 2. `AllowInsecureFallback` (Extended Support) | ||
|
|
||
| `AllowInsecureFallback` mode indicates the gateway will accept connections even if the client certificate is not presented or fails verification. | ||
| `AllowInsecureFallback` mode indicates 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. | ||
| When `FrontendValidationModeType` is changed from `AllowValidOnly` to `AllowInsecureFallback` the `InsecureFrontendValidationMode` condition MUST be set to `True` with Reason `ConfigurationChanged` on gateway. | ||
| This condition remains set to `True` even if `FrontendValidationModeType` is later changed back to `AllowValidOnly`. | ||
|
|
||
| * Introduce a `ObjectReference` structure that can be used to specify `caCertificateRefs` references. | ||
| * Invalid `caCertificateRefs` directly affect the `ResolvedRefs` and `Accepted` conditions of the targeted listeners. | ||
snorwin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| A listener is considered targeted if and only if it is serving HTTPS and either its port matches the port of the | ||
snorwin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| per-port configuration, or it is using the default configuration. This behavior is important to ensure that when all | ||
| CA certificates are invalid, listeners do not implicitly fall back to skipping client | ||
| certificate verification. | ||
|
|
||
| ### Impact on listeners | ||
|
|
||
| This proposal removes frontendTLSValidation from Listener's TLS configuration and introduces gateways level per port configuration. This is a breaking change for exisitng implementation which uses this feature from Experimental API. | ||
| Once gateway level TLS is configured (either by default or for a specific port), the TLS settings will apply to all existing and newly created Listeners serving HTTPS that match the configuration. | ||
|
|
||
| #### GO | ||
| #### Go | ||
|
|
||
| ```go | ||
| // ObjectReference identifies an API object including its namespace. | ||
|
|
@@ -142,27 +149,47 @@ type TLSPortConfig struct { | |
| // FrontendTLSValidation holds configuration information that can be used to validate | ||
| // the frontend initiating the TLS connection | ||
| type FrontendTLSValidation struct { | ||
| // CACertificateRefs contains one or more references to | ||
| // Kubernetes objects that contain TLS certificates of | ||
| // the Certificate Authorities that can be used | ||
| // as a trust anchor to validate the certificates presented by the client. | ||
| // CACertificateRefs contains one or more references to Kubernetes | ||
| // objects that contain a PEM-encoded TLS CA certificate bundle, which | ||
| // is used as a trust anchor to validate the certificates presented by | ||
| // the client. | ||
| // | ||
| // A CACertificateRef is invalid if: | ||
| // | ||
| // * It refers to a resource that cannot be resolved (e.g., the | ||
| // referenced resource does not exist) or is misconfigured (e.g., a | ||
| // ConfigMap does not contain a key named `ca.crt`). In this case, the | ||
snorwin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Reason must be set to `InvalidCACertificateRef` and the Message of | ||
| // the Condition must indicate which reference is invalid and why. | ||
|
Comment on lines
+164
to
+165
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| // | ||
| // A single CA certificate reference to a Kubernetes ConfigMap | ||
| // has "Core" support. | ||
| // Implementations MAY choose to support attaching multiple CA certificates to | ||
| // a Listener, but this behavior is implementation-specific. | ||
| // * It refers to an unknown or unsupported kind of resource. In this | ||
| // case, the Reason must be set to `InvalidKind` and the Message of | ||
| // the Condition must explain which kind of resource is unknown or | ||
| // unsupported. | ||
| // | ||
| // Support: Core - A single reference to a Kubernetes ConfigMap | ||
| // with the CA certificate in a key named `ca.crt`. | ||
| // * It refers to a resource in another namespace UNLESS there is a | ||
| // ReferenceGrant in the target namespace that allows the CA | ||
| // certificate to be attached. If a ReferenceGrant does not allow this | ||
| // reference, the `ResolvedRefs` condition MUST be set with | ||
| // the Reason `RefNotPermitted`. | ||
|
Comment on lines
+172
to
+176
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| // | ||
| // Support: Implementation-specific (More than one certificate in a ConfigMap with different keys or more than one reference, or other kinds of resources). | ||
| // In all cases, the implementation MUST ensure that the `ResolvedRefs` | ||
| // condition is set to `status: False` on all targeted listeners (i.e., | ||
| // listeners serving HTTPS on a matching port). The condition MUST | ||
| // include a Reason and Message that indicate the cause of the error. If | ||
| // ALL CACertificateRefs are invalid, the implementation MUST also ensure | ||
| // the `Accepted` condition on the listener is set to `status: False`, with | ||
| // the Reason `NoValidCACertificate`. | ||
| // Implementations MAY choose to support attaching multiple CA certificates | ||
| // to a listener, but this behavior is implementation-specific. | ||
|
Comment on lines
+189
to
+190
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 can we talk through this one a bit more?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this duplicates the intent of the following: It was inspired by the description of the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you walk through some scenarios where this is going to be needed, and share your experience and thoughts on why the behavior is "implementation-specific" and is not standardizable?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that multiple references is a common case for rotating the certificates. User might want to attach a new CA cert to gateway and be in the state where gateway will check both CA certificates to ensure smooth transition to a new certificate for all clients. Then old CA certificate will be deleted. We can achieve that with a new ConfigMap but having multiple certificates in one map is convenient
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there are use cases where clients connect from different environments and therefore rely on different identity providers (CAs) to sign their client certificates. To support this, we need the ability to configure multiple CACertificateRefs. The second question is whether this should be implementation-specific. During the refinement of the BackendTLSPolicy, we weren’t able to find a common approach that worked for all. To keep things flexible, we defined only a single CA certificate bundle (specified as ConfigMap) as "Core" and left anything beyond that to implementations. For consistency, I followed the same approach here. |
||
| // | ||
| // References to a resource in a different namespace are invalid UNLESS there | ||
| // is a ReferenceGrant in the target namespace that allows the certificate | ||
| // to be attached. If a ReferenceGrant does not allow this reference, the | ||
| // "ResolvedRefs" condition MUST be set to False for this listener with the | ||
| // "RefNotPermitted" reason. | ||
| // 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, other kinds | ||
| // of resources, or a single reference that includes multiple certificates. | ||
| // | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MaxItems=8 | ||
| // +kubebuilder:validation:MinItems=1 | ||
snorwin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| CACertificateRefs []ObjectReference `json:"caCertificateRefs,omitempty"` | ||
|
|
@@ -182,7 +209,9 @@ type FrontendTLSValidation struct { | |
| // | ||
| // Defaults to AllowValidOnly. | ||
| // | ||
| // Support: Core | ||
| // Support: Core - AllowValidOnly | ||
| // | ||
| // Support: Extended - AllowInsecureFallback | ||
| // | ||
| // +optional | ||
| // +kubebuilder:default=AllowValidOnly | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.