From c1dd2da722ab2363745a0272d4d9c949222b36cb Mon Sep 17 00:00:00 2001 From: Clay Kauzlaric Date: Fri, 5 Jan 2024 16:47:18 -0500 Subject: [PATCH] allow multiple SANS in upstream validation (#5849) Signed-off-by: Clay Kauzlaric --- apis/projectcontour/v1/httpproxy.go | 13 +++ .../v1/zz_generated.deepcopy.go | 9 +- .../v1alpha1/zz_generated.deepcopy.go | 2 +- .../unreleased/5849-KauzClay-deprecation.md | 6 + changelogs/unreleased/5849-KauzClay-minor.md | 5 + examples/contour/01-crds.yaml | 104 ++++++++++++++++-- examples/render/contour-deployment.yaml | 104 ++++++++++++++++-- .../render/contour-gateway-provisioner.yaml | 104 ++++++++++++++++-- examples/render/contour-gateway.yaml | 104 ++++++++++++++++-- examples/render/contour.yaml | 104 ++++++++++++++++-- internal/dag/builder_test.go | 6 +- internal/dag/cache.go | 22 +++- internal/dag/cache_test.go | 93 ++++++++++++++++ internal/dag/dag.go | 15 +-- internal/dag/dag_test.go | 8 +- internal/dag/extension_processor.go | 2 +- internal/envoy/cluster.go | 2 +- internal/envoy/v3/auth.go | 19 ++-- internal/envoy/v3/auth_test.go | 44 +++++++- internal/envoy/v3/cluster_test.go | 14 +-- internal/envoy/v3/listener_test.go | 4 +- .../featuretests/v3/backendclientauth_test.go | 2 +- internal/featuretests/v3/envoy.go | 2 +- .../docs/main/config/api-reference.html | 20 +++- site/content/docs/main/config/upstream-tls.md | 14 ++- test/e2e/httpproxy/cel_validation_test.go | 69 ++++++++++++ test/e2e/httpproxy/httpproxy_test.go | 2 + 27 files changed, 783 insertions(+), 110 deletions(-) create mode 100644 changelogs/unreleased/5849-KauzClay-deprecation.md create mode 100644 changelogs/unreleased/5849-KauzClay-minor.md create mode 100644 test/e2e/httpproxy/cel_validation_test.go diff --git a/apis/projectcontour/v1/httpproxy.go b/apis/projectcontour/v1/httpproxy.go index 498be837a8b..107c44ca4e9 100644 --- a/apis/projectcontour/v1/httpproxy.go +++ b/apis/projectcontour/v1/httpproxy.go @@ -1306,14 +1306,27 @@ type HeaderValue struct { } // UpstreamValidation defines how to verify the backend service's certificate +// +kubebuilder:validation:XValidation:message="subjectNames[0] must equal subjectName if set",rule="has(self.subjectNames) ? self.subjectNames[0] == self.subjectName : true" type UpstreamValidation struct { // Name or namespaced name of the Kubernetes secret used to validate the certificate presented by the backend. // The secret must contain key named ca.crt. // The name can be optionally prefixed with namespace "namespace/name". // When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the secret. + // Max length should be the actual max possible length of a namespaced name (63 + 253 + 1 = 317) + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=317 CACertificate string `json:"caSecret"` // Key which is expected to be present in the 'subjectAltName' of the presented certificate. + // Deprecated: migrate to using the plural field subjectNames. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=250 SubjectName string `json:"subjectName"` + // List of keys, of which at least one is expected to be present in the 'subjectAltName of the + // presented certificate. + // +optional + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=8 + SubjectNames []string `json:"subjectNames"` } // DownstreamValidation defines how to verify the client certificate. diff --git a/apis/projectcontour/v1/zz_generated.deepcopy.go b/apis/projectcontour/v1/zz_generated.deepcopy.go index 3c3537ef24e..3207fef641a 100644 --- a/apis/projectcontour/v1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1/zz_generated.deepcopy.go @@ -919,7 +919,7 @@ func (in *RemoteJWKS) DeepCopyInto(out *RemoteJWKS) { if in.UpstreamValidation != nil { in, out := &in.UpstreamValidation, &out.UpstreamValidation *out = new(UpstreamValidation) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -1155,7 +1155,7 @@ func (in *Service) DeepCopyInto(out *Service) { if in.UpstreamValidation != nil { in, out := &in.UpstreamValidation, &out.UpstreamValidation *out = new(UpstreamValidation) - **out = **in + (*in).DeepCopyInto(*out) } if in.RequestHeadersPolicy != nil { in, out := &in.RequestHeadersPolicy, &out.RequestHeadersPolicy @@ -1434,6 +1434,11 @@ func (in *TimeoutPolicy) DeepCopy() *TimeoutPolicy { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UpstreamValidation) DeepCopyInto(out *UpstreamValidation) { *out = *in + if in.SubjectNames != nil { + in, out := &in.SubjectNames, &out.SubjectNames + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UpstreamValidation. diff --git a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go index 1727df16bd9..125d9949fe9 100644 --- a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go @@ -786,7 +786,7 @@ func (in *ExtensionServiceSpec) DeepCopyInto(out *ExtensionServiceSpec) { if in.UpstreamValidation != nil { in, out := &in.UpstreamValidation, &out.UpstreamValidation *out = new(v1.UpstreamValidation) - **out = **in + (*in).DeepCopyInto(*out) } if in.Protocol != nil { in, out := &in.Protocol, &out.Protocol diff --git a/changelogs/unreleased/5849-KauzClay-deprecation.md b/changelogs/unreleased/5849-KauzClay-deprecation.md new file mode 100644 index 00000000000..c4266be7d80 --- /dev/null +++ b/changelogs/unreleased/5849-KauzClay-deprecation.md @@ -0,0 +1,6 @@ +## Deprecate `subjectName` field on UpstreamValidation + +The `subjectName` field is being deprecated in favor of `subjectNames`, which is +an list of subjectNames. `subjectName` will continue to behave as it has. If +using `subjectNames`, the first entry in `subjectNames` must match the value of +`subjectName`. this will be enforced by CEL validation. \ No newline at end of file diff --git a/changelogs/unreleased/5849-KauzClay-minor.md b/changelogs/unreleased/5849-KauzClay-minor.md new file mode 100644 index 00000000000..afa791c072b --- /dev/null +++ b/changelogs/unreleased/5849-KauzClay-minor.md @@ -0,0 +1,5 @@ +## Allow Multiple SANs in Upstream Validation section of HTTPProxy + +This change introduces a max length of 250 characters to the field `subjectName` in the UpstreamValidation block. + +Allow multiple SANs in Upstream Validation by adding a new field `subjectNames` to the UpstreamValidtion block. This will exist side by side with the previous `subjectName` field. Using CEL validation, we can enforce that when both are present, the first entry in `subjectNames` must match the value of `subjectName`. \ No newline at end of file diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 6f3f1f4aa65..1e4531f61fb 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -5033,16 +5033,35 @@ spec: secret must contain key named ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max length should + be the actual max possible length of a namespaced name (63 + + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in the 'subjectAltName' - of the presented certificate. + description: 'Key which is expected to be present in the ''subjectAltName'' + of the presented certificate. Deprecated: migrate to using the + plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is expected to + be present in the 'subjectAltName of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == self.subjectName + : true' required: - services type: object @@ -6664,16 +6683,35 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the - secret. + secret. Max length should be the actual max possible + length of a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of + the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7059,16 +7097,36 @@ spec: ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max + length should be the actual max possible length of + a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present in + the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of the + presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == + self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7381,16 +7439,38 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to - the secret. + the secret. Max length should be the actual max + possible length of a namespaced name (63 + 253 + + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present - in the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field + subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one + is expected to be present in the 'subjectAltName + of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if + set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' required: - uri type: object diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index 5acc615ffaa..f605afb88f6 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -5252,16 +5252,35 @@ spec: secret must contain key named ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max length should + be the actual max possible length of a namespaced name (63 + + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in the 'subjectAltName' - of the presented certificate. + description: 'Key which is expected to be present in the ''subjectAltName'' + of the presented certificate. Deprecated: migrate to using the + plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is expected to + be present in the 'subjectAltName of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == self.subjectName + : true' required: - services type: object @@ -6883,16 +6902,35 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the - secret. + secret. Max length should be the actual max possible + length of a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of + the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7278,16 +7316,36 @@ spec: ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max + length should be the actual max possible length of + a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present in + the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of the + presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == + self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7600,16 +7658,38 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to - the secret. + the secret. Max length should be the actual max + possible length of a namespaced name (63 + 253 + + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present - in the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field + subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one + is expected to be present in the 'subjectAltName + of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if + set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' required: - uri type: object diff --git a/examples/render/contour-gateway-provisioner.yaml b/examples/render/contour-gateway-provisioner.yaml index 526c7b2ba5a..b812326a588 100644 --- a/examples/render/contour-gateway-provisioner.yaml +++ b/examples/render/contour-gateway-provisioner.yaml @@ -5044,16 +5044,35 @@ spec: secret must contain key named ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max length should + be the actual max possible length of a namespaced name (63 + + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in the 'subjectAltName' - of the presented certificate. + description: 'Key which is expected to be present in the ''subjectAltName'' + of the presented certificate. Deprecated: migrate to using the + plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is expected to + be present in the 'subjectAltName of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == self.subjectName + : true' required: - services type: object @@ -6675,16 +6694,35 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the - secret. + secret. Max length should be the actual max possible + length of a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of + the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7070,16 +7108,36 @@ spec: ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max + length should be the actual max possible length of + a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present in + the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of the + presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == + self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7392,16 +7450,38 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to - the secret. + the secret. Max length should be the actual max + possible length of a namespaced name (63 + 253 + + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present - in the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field + subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one + is expected to be present in the 'subjectAltName + of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if + set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' required: - uri type: object diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index 8b16a4a46a1..fde77fce0fc 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -5255,16 +5255,35 @@ spec: secret must contain key named ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max length should + be the actual max possible length of a namespaced name (63 + + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in the 'subjectAltName' - of the presented certificate. + description: 'Key which is expected to be present in the ''subjectAltName'' + of the presented certificate. Deprecated: migrate to using the + plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is expected to + be present in the 'subjectAltName of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == self.subjectName + : true' required: - services type: object @@ -6886,16 +6905,35 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the - secret. + secret. Max length should be the actual max possible + length of a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of + the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7281,16 +7319,36 @@ spec: ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max + length should be the actual max possible length of + a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present in + the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of the + presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == + self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7603,16 +7661,38 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to - the secret. + the secret. Max length should be the actual max + possible length of a namespaced name (63 + 253 + + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present - in the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field + subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one + is expected to be present in the 'subjectAltName + of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if + set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' required: - uri type: object diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index c7dc925b86d..37ce84b15e0 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -5252,16 +5252,35 @@ spec: secret must contain key named ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max length should + be the actual max possible length of a namespaced name (63 + + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in the 'subjectAltName' - of the presented certificate. + description: 'Key which is expected to be present in the ''subjectAltName'' + of the presented certificate. Deprecated: migrate to using the + plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is expected to + be present in the 'subjectAltName of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == self.subjectName + : true' required: - services type: object @@ -6883,16 +6902,35 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the - secret. + secret. Max length should be the actual max possible + length of a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of + the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7278,16 +7316,36 @@ spec: ca.crt. The name can be optionally prefixed with namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist - in the namespace to grant access to the secret. + in the namespace to grant access to the secret. Max + length should be the actual max possible length of + a namespaced name (63 + 253 + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present in - the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present in + the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one is + expected to be present in the 'subjectAltName of the + presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if set + rule: 'has(self.subjectNames) ? self.subjectNames[0] == + self.subjectName : true' weight: description: Weight defines percentage of traffic to balance traffic @@ -7600,16 +7658,38 @@ spec: namespace "namespace/name". When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to - the secret. + the secret. Max length should be the actual max + possible length of a namespaced name (63 + 253 + + 1 = 317) + maxLength: 317 + minLength: 1 type: string subjectName: - description: Key which is expected to be present - in the 'subjectAltName' of the presented certificate. + description: 'Key which is expected to be present + in the ''subjectAltName'' of the presented certificate. + Deprecated: migrate to using the plural field + subjectNames.' + maxLength: 250 + minLength: 1 type: string + subjectNames: + description: List of keys, of which at least one + is expected to be present in the 'subjectAltName + of the presented certificate. + items: + type: string + maxItems: 8 + minItems: 1 + type: array required: - caSecret - subjectName type: object + x-kubernetes-validations: + - message: subjectNames[0] must equal subjectName if + set + rule: 'has(self.subjectNames) ? self.subjectNames[0] + == self.subjectName : true' required: - uri type: object diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index 46a046b3030..a0a15db5e27 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -11349,7 +11349,7 @@ func TestDAGInsert(t *testing.T) { Protocol: "tls", UpstreamValidation: &PeerValidationContext{ CACertificate: caSecret(cert1), - SubjectName: "example.com", + SubjectNames: []string{"example.com"}, }, }, ), @@ -11382,7 +11382,7 @@ func TestDAGInsert(t *testing.T) { Protocol: "h2", UpstreamValidation: &PeerValidationContext{ CACertificate: caSecret(cert1), - SubjectName: "example.com", + SubjectNames: []string{"example.com"}, }, }, ), @@ -11457,7 +11457,7 @@ func TestDAGInsert(t *testing.T) { Protocol: "tls", UpstreamValidation: &PeerValidationContext{ CACertificate: caSecret(cert2), - SubjectName: "example.com", + SubjectNames: []string{"example.com"}, }, }, ), diff --git a/internal/dag/cache.go b/internal/dag/cache.go index 132df4cf9fd..21f2bdaa7a4 100644 --- a/internal/dag/cache.go +++ b/internal/dag/cache.go @@ -667,6 +667,8 @@ func (kc *KubernetesCache) LookupUpstreamValidation(uv *contour_api_v1.UpstreamV return nil, nil } + pvc := &PeerValidationContext{} + cacert, err := kc.LookupCASecret(caCertificate, targetNamespace) if err != nil { if _, ok := err.(DelegationNotPermittedError); ok { @@ -674,16 +676,26 @@ func (kc *KubernetesCache) LookupUpstreamValidation(uv *contour_api_v1.UpstreamV } return nil, fmt.Errorf("invalid CA Secret %q: %s", caCertificate, err) } + pvc.CACertificate = cacert + // CEL validation should enforce that SubjectName must be set if SubjectNames is used. So, SubjectName will always be present. if uv.SubjectName == "" { - // UpstreamValidation is requested, but SAN is not provided return nil, errors.New("missing subject alternative name") } - return &PeerValidationContext{ - CACertificate: cacert, - SubjectName: uv.SubjectName, - }, nil + switch l := len(uv.SubjectNames); { + case l == 0: + // UpstreamValidation was using old SubjectName field only, can internally move that into SubjectNames + pvc.SubjectNames = []string{uv.SubjectName} + case l > 0: + // UpstreamValidation is using new SubjectNames field, can use it directly. CEL validation should enforce that SubjectName is contained in SubjectNames + if uv.SubjectName != uv.SubjectNames[0] { + return nil, fmt.Errorf("first entry of SubjectNames (%s) does not match SubjectName (%s)", uv.SubjectNames[0], uv.SubjectName) + } + pvc.SubjectNames = uv.SubjectNames + } + + return pvc, nil } // LookupTLSSecretInsecure returns Secret with TLS certificate and private key from cache. diff --git a/internal/dag/cache_test.go b/internal/dag/cache_test.go index 540885c17ba..f01fa4c680b 100644 --- a/internal/dag/cache_test.go +++ b/internal/dag/cache_test.go @@ -2647,3 +2647,96 @@ func TestRouteTriggersRebuild(t *testing.T) { }) } } + +func TestLookupUpstreamValidation(t *testing.T) { + cache := func(objs ...any) *KubernetesCache { + cache := KubernetesCache{ + FieldLogger: fixture.NewTestLogger(t), + } + for _, o := range objs { + cache.Insert(o) + } + return &cache + } + + uv := func(subjectName string, subjectNames []string) *contour_api_v1.UpstreamValidation { + return &contour_api_v1.UpstreamValidation{ + CACertificate: "ca", + SubjectName: subjectName, + SubjectNames: subjectNames, + } + } + + secret := func() *v1.Secret { + return &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca", + Namespace: "default", + }, + Type: v1.SecretTypeOpaque, + Data: map[string][]byte{ + CACertificateKey: []byte(fixture.CERTIFICATE), + }, + } + } + + pvc := func(subjectNames []string) *PeerValidationContext { + return &PeerValidationContext{ + CACertificate: &Secret{ + Object: secret(), + ValidCASecret: &SecretValidationStatus{}, + }, + SubjectNames: subjectNames, + } + } + + tests := map[string]struct { + cache *KubernetesCache + meta types.NamespacedName + uv *contour_api_v1.UpstreamValidation + wantPvc *PeerValidationContext + wantErr error + }{ + "contains both SubjectName and SubjectNames correctly": { + cache: cache(secret()), + uv: uv("example.com", []string{"example.com", "extra.com"}), + meta: types.NamespacedName{Namespace: "default", Name: "ca"}, + wantPvc: pvc([]string{"example.com", "extra.com"}), + }, + "SubjectName does not match SubjectNames[0]": { + cache: cache(secret()), + uv: uv("example.com", []string{"wrong.com", "extra.com"}), + meta: types.NamespacedName{Namespace: "default", Name: "ca"}, + wantPvc: pvc([]string{"example.com", "extra.com"}), + wantErr: errors.New("first entry of SubjectNames (wrong.com) does not match SubjectName (example.com)"), + }, + "SubjectName missing": { + cache: cache(secret()), + uv: uv("", []string{"wrong.com", "extra.com"}), + meta: types.NamespacedName{Namespace: "default", Name: "ca"}, + wantPvc: pvc([]string{"example.com", "extra.com"}), + wantErr: errors.New("missing subject alternative name"), + }, + "SubjectNames missing": { + cache: cache(secret()), + uv: uv("example.com", []string{}), + meta: types.NamespacedName{Namespace: "default", Name: "ca"}, + wantPvc: pvc([]string{"example.com"}), + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + gotPvc, gotErr := tc.cache.LookupUpstreamValidation(tc.uv, tc.meta, "default") + + switch { + case tc.wantErr != nil: + require.Error(t, gotErr) + assert.EqualError(t, tc.wantErr, gotErr.Error()) + default: + assert.Nil(t, gotErr) + assert.Equal(t, tc.wantPvc, gotPvc) + } + }) + } +} diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 62c56c6a2d0..95aaf383e42 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -669,9 +669,9 @@ type PeerValidationContext struct { // CACertificate holds a reference to the Secret containing the CA to be used to // verify the upstream connection. CACertificate *Secret - // SubjectName holds an optional subject name which Envoy will check against the - // certificate presented by the upstream. - SubjectName string + // SubjectNames holds optional subject names which Envoy will check against the + // certificate presented by the upstream. The first entry must match the value of SubjectName + SubjectNames []string // SkipClientCertValidation when set to true will ensure Envoy requests but // does not verify peer certificates. SkipClientCertValidation bool @@ -698,13 +698,14 @@ func (pvc *PeerValidationContext) GetCACertificate() []byte { return pvc.CACertificate.Object.Data[CACertificateKey] } -// GetSubjectName returns the SubjectName from PeerValidationContext. -func (pvc *PeerValidationContext) GetSubjectName() string { +// GetSubjectName returns the SubjectNames from PeerValidationContext. +func (pvc *PeerValidationContext) GetSubjectNames() []string { if pvc == nil { // No validation required. - return "" + return nil } - return pvc.SubjectName + + return pvc.SubjectNames } // GetCRL returns the Certificate Revocation List. diff --git a/internal/dag/dag_test.go b/internal/dag/dag_test.go index a7ee4def4b9..bb7e1ed5015 100644 --- a/internal/dag/dag_test.go +++ b/internal/dag/dag_test.go @@ -84,16 +84,16 @@ func TestPeerValidationContext(t *testing.T) { }, }, }, - SubjectName: "subject", + SubjectNames: []string{"subject"}, } pvc2 := PeerValidationContext{} var pvc3 *PeerValidationContext - assert.Equal(t, pvc1.GetSubjectName(), "subject") + assert.Equal(t, pvc1.GetSubjectNames()[0], "subject") assert.Equal(t, pvc1.GetCACertificate(), []byte("cacert")) - assert.Equal(t, pvc2.GetSubjectName(), "") + assert.Equal(t, pvc2.GetSubjectNames(), []string(nil)) assert.Equal(t, pvc2.GetCACertificate(), []byte(nil)) - assert.Equal(t, pvc3.GetSubjectName(), "") + assert.Equal(t, pvc3.GetSubjectNames(), []string(nil)) assert.Equal(t, pvc3.GetCACertificate(), []byte(nil)) } diff --git a/internal/dag/extension_processor.go b/internal/dag/extension_processor.go index 0f97cb12713..1ccd293662e 100644 --- a/internal/dag/extension_processor.go +++ b/internal/dag/extension_processor.go @@ -174,7 +174,7 @@ func (p *ExtensionServiceProcessor) buildExtensionService( // future. // // TODO(jpeach): expose SNI in the API, https://github.com/projectcontour/contour/issues/2893. - extension.SNI = uv.SubjectName + extension.SNI = uv.SubjectNames[0] if extension.Protocol != "h2" { validCondition.AddErrorf(contour_api_v1.ConditionTypeSpecError, "InconsistentProtocol", diff --git a/internal/envoy/cluster.go b/internal/envoy/cluster.go index e6d4bf94fbe..7d5218df95a 100644 --- a/internal/envoy/cluster.go +++ b/internal/envoy/cluster.go @@ -48,7 +48,7 @@ func Clustername(cluster *dag.Cluster) string { } if uv := cluster.UpstreamValidation; uv != nil { buf += uv.CACertificate.Object.ObjectMeta.Name - buf += uv.SubjectName + buf += uv.SubjectNames[0] } buf += cluster.Protocol + cluster.SNI if !cluster.TimeoutPolicy.IdleConnectionTimeout.UseDefault() { diff --git a/internal/envoy/v3/auth.go b/internal/envoy/v3/auth.go index 411e9060462..de8d188c40c 100644 --- a/internal/envoy/v3/auth.go +++ b/internal/envoy/v3/auth.go @@ -50,14 +50,14 @@ func UpstreamTLSContext(peerValidationContext *dag.PeerValidationContext, sni st } } - if peerValidationContext.GetCACertificate() != nil && len(peerValidationContext.GetSubjectName()) > 0 { + if peerValidationContext.GetCACertificate() != nil && len(peerValidationContext.GetSubjectNames()) > 0 { // We have to explicitly assign the value from validationContext // to context.CommonTlsContext.ValidationContextType because the // latter is an interface. Returning nil from validationContext // directly into this field boxes the nil into the unexported // type of this grpc OneOf field which causes proto marshaling // to explode later on. - vc := validationContext(peerValidationContext.GetCACertificate(), peerValidationContext.GetSubjectName(), false, nil, false) + vc := validationContext(peerValidationContext.GetCACertificate(), peerValidationContext.GetSubjectNames(), false, nil, false) if vc != nil { // TODO: update this for SDS (CommonTlsContext_ValidationContextSdsSecretConfig) instead of inlining it. context.CommonTlsContext.ValidationContextType = vc @@ -68,7 +68,7 @@ func UpstreamTLSContext(peerValidationContext *dag.PeerValidationContext, sni st } // TODO: update this for SDS (CommonTlsContext_ValidationContextSdsSecretConfig) instead of inlining it. -func validationContext(ca []byte, subjectName string, skipVerifyPeerCert bool, crl []byte, onlyVerifyLeafCertCrl bool) *envoy_v3_tls.CommonTlsContext_ValidationContext { +func validationContext(ca []byte, subjectNames []string, skipVerifyPeerCert bool, crl []byte, onlyVerifyLeafCertCrl bool) *envoy_v3_tls.CommonTlsContext_ValidationContext { vc := &envoy_v3_tls.CommonTlsContext_ValidationContext{ ValidationContext: &envoy_v3_tls.CertificateValidationContext{ TrustChainVerification: envoy_v3_tls.CertificateValidationContext_VERIFY_TRUST_CHAIN, @@ -87,17 +87,18 @@ func validationContext(ca []byte, subjectName string, skipVerifyPeerCert bool, c } } - if len(subjectName) > 0 { - vc.ValidationContext.MatchTypedSubjectAltNames = []*envoy_v3_tls.SubjectAltNameMatcher{ - { + for _, san := range subjectNames { + vc.ValidationContext.MatchTypedSubjectAltNames = append( + vc.ValidationContext.MatchTypedSubjectAltNames, + &envoy_v3_tls.SubjectAltNameMatcher{ SanType: envoy_v3_tls.SubjectAltNameMatcher_DNS, Matcher: &matcher.StringMatcher{ MatchPattern: &matcher.StringMatcher_Exact{ - Exact: subjectName, + Exact: san, }, }, }, - } + ) } if len(crl) > 0 { @@ -129,7 +130,7 @@ func DownstreamTLSContext(serverSecret *dag.Secret, tlsMinProtoVersion, tlsMaxPr }, } if peerValidationContext != nil { - vc := validationContext(peerValidationContext.GetCACertificate(), "", peerValidationContext.SkipClientCertValidation, + vc := validationContext(peerValidationContext.GetCACertificate(), []string{}, peerValidationContext.SkipClientCertValidation, peerValidationContext.GetCRL(), peerValidationContext.OnlyVerifyLeafCertCrl) if vc != nil { context.CommonTlsContext.ValidationContextType = vc diff --git a/internal/envoy/v3/auth_test.go b/internal/envoy/v3/auth_test.go index 43987273b44..6ddd906a61e 100644 --- a/internal/envoy/v3/auth_test.go +++ b/internal/envoy/v3/auth_test.go @@ -67,7 +67,7 @@ func TestUpstreamTLSContext(t *testing.T) { }, "no alpn, missing ca": { validation: &dag.PeerValidationContext{ - SubjectName: "www.example.com", + SubjectNames: []string{"www.example.com"}, }, want: &envoy_v3_tls.UpstreamTlsContext{ CommonTlsContext: &envoy_v3_tls.CommonTlsContext{}, @@ -76,7 +76,7 @@ func TestUpstreamTLSContext(t *testing.T) { "no alpn, ca and altname": { validation: &dag.PeerValidationContext{ CACertificate: secret, - SubjectName: "www.example.com", + SubjectNames: []string{"www.example.com"}, }, want: &envoy_v3_tls.UpstreamTlsContext{ CommonTlsContext: &envoy_v3_tls.CommonTlsContext{ @@ -123,6 +123,46 @@ func TestUpstreamTLSContext(t *testing.T) { }, }, }, + "multiple subjectnames": { + validation: &dag.PeerValidationContext{ + CACertificate: secret, + SubjectNames: []string{ + "foo.com", + "bar.com", + }, + }, + want: &envoy_v3_tls.UpstreamTlsContext{ + CommonTlsContext: &envoy_v3_tls.CommonTlsContext{ + ValidationContextType: &envoy_v3_tls.CommonTlsContext_ValidationContext{ + ValidationContext: &envoy_v3_tls.CertificateValidationContext{ + TrustedCa: &envoy_api_v3_core.DataSource{ + Specifier: &envoy_api_v3_core.DataSource_InlineBytes{ + InlineBytes: []byte("ca"), + }, + }, + MatchTypedSubjectAltNames: []*envoy_v3_tls.SubjectAltNameMatcher{ + { + SanType: envoy_v3_tls.SubjectAltNameMatcher_DNS, + Matcher: &matcher.StringMatcher{ + MatchPattern: &matcher.StringMatcher_Exact{ + Exact: "foo.com", + }, + }, + }, + { + SanType: envoy_v3_tls.SubjectAltNameMatcher_DNS, + Matcher: &matcher.StringMatcher{ + MatchPattern: &matcher.StringMatcher_Exact{ + Exact: "bar.com", + }, + }, + }, + }, + }, + }, + }, + }, + }, } for name, tc := range tests { diff --git a/internal/envoy/v3/cluster_test.go b/internal/envoy/v3/cluster_test.go index f22273735fa..e8bb7eef649 100644 --- a/internal/envoy/v3/cluster_test.go +++ b/internal/envoy/v3/cluster_test.go @@ -300,7 +300,7 @@ func TestCluster(t *testing.T) { Protocol: "tls", UpstreamValidation: &dag.PeerValidationContext{ CACertificate: secret, - SubjectName: "foo.bar.io", + SubjectNames: []string{"foo.bar.io"}, }, }, want: &envoy_cluster_v3.Cluster{ @@ -315,7 +315,7 @@ func TestCluster(t *testing.T) { UpstreamTLSContext( &dag.PeerValidationContext{ CACertificate: secret, - SubjectName: "foo.bar.io", + SubjectNames: []string{"foo.bar.io"}, }, "", nil, @@ -329,7 +329,7 @@ func TestCluster(t *testing.T) { Protocol: "tls", UpstreamValidation: &dag.PeerValidationContext{ CACertificate: secret, - SubjectName: "foo.bar.io", + SubjectNames: []string{"foo.bar.io"}, }, UpstreamTLS: &dag.UpstreamTLS{ MinimumProtocolVersion: "1.3", @@ -348,7 +348,7 @@ func TestCluster(t *testing.T) { UpstreamTLSContext( &dag.PeerValidationContext{ CACertificate: secret, - SubjectName: "foo.bar.io", + SubjectNames: []string{"foo.bar.io"}, }, "", nil, @@ -942,7 +942,7 @@ func TestDNSNameCluster(t *testing.T) { }, }, }, - SubjectName: "foo.projectcontour.io", + SubjectNames: []string{"foo.projectcontour.io"}, }, }, want: &envoy_cluster_v3.Cluster{ @@ -973,7 +973,7 @@ func TestDNSNameCluster(t *testing.T) { }, }, }, - SubjectName: "foo.projectcontour.io", + SubjectNames: []string{"foo.projectcontour.io"}, }, "foo.projectcontour.io", nil, nil)), }, }, @@ -1104,7 +1104,7 @@ func TestClustername(t *testing.T) { }, }, }, - SubjectName: "foo.com", + SubjectNames: []string{"foo.com"}, }, } diff --git a/internal/envoy/v3/listener_test.go b/internal/envoy/v3/listener_test.go index 023ef43f149..c8bfa83e28f 100644 --- a/internal/envoy/v3/listener_test.go +++ b/internal/envoy/v3/listener_test.go @@ -241,7 +241,7 @@ func TestSocketAddress(t *testing.T) { } func TestDownstreamTLSContext(t *testing.T) { - const subjectName = "client-subject-name" + subjectNames := []string{"client-subject-name"} ca := []byte("client-ca-cert") crl := []byte("crl-data") @@ -328,7 +328,7 @@ func TestDownstreamTLSContext(t *testing.T) { }, }, }, - SubjectName: subjectName, + SubjectNames: subjectNames, } peerValidationContextSkipClientCertValidation := &dag.PeerValidationContext{ diff --git a/internal/featuretests/v3/backendclientauth_test.go b/internal/featuretests/v3/backendclientauth_test.go index b686c676de7..0cbe4d28e90 100644 --- a/internal/featuretests/v3/backendclientauth_test.go +++ b/internal/featuretests/v3/backendclientauth_test.go @@ -211,7 +211,7 @@ func TestBackendClientAuthenticationWithExtensionService(t *testing.T) { Type: "kubernetes.io/tls", Data: map[string][]byte{dag.CACertificateKey: []byte(featuretests.CERTIFICATE)}, }}, - SubjectName: "subjname"}, + SubjectNames: []string{"subjname"}}, "subjname", &dag.Secret{Object: sec1}, nil, diff --git a/internal/featuretests/v3/envoy.go b/internal/featuretests/v3/envoy.go index cf0a0814b71..139a8c18803 100644 --- a/internal/featuretests/v3/envoy.go +++ b/internal/featuretests/v3/envoy.go @@ -201,7 +201,7 @@ func tlsCluster(c *envoy_cluster_v3.Cluster, ca []byte, subjectName string, sni Type: "kubernetes.io/tls", Data: map[string][]byte{dag.CACertificateKey: ca}, }}, - SubjectName: subjectName}, + SubjectNames: []string{subjectName}}, sni, secret, upstreamTLS, diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index a78277849ba..76babb39d14 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -4694,7 +4694,8 @@

UpstreamValidation

Name or namespaced name of the Kubernetes secret used to validate the certificate presented by the backend. The secret must contain key named ca.crt. The name can be optionally prefixed with namespace “namespace/name”. -When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the secret.

+When cross-namespace reference is used, TLSCertificateDelegation resource must exist in the namespace to grant access to the secret. +Max length should be the actual max possible length of a namespaced name (63 + 253 + 1 = 317)

@@ -4706,7 +4707,22 @@

UpstreamValidation -

Key which is expected to be present in the ‘subjectAltName’ of the presented certificate.

+

Key which is expected to be present in the ‘subjectAltName’ of the presented certificate. +Deprecated: migrate to using the plural field subjectNames.

+ + + + +subjectNames +
+ +[]string + + + +(Optional) +

List of keys, of which at least one is expected to be present in the ‘subjectAltName of the +presented certificate.

diff --git a/site/content/docs/main/config/upstream-tls.md b/site/content/docs/main/config/upstream-tls.md index 72c83062391..aeade7fdd76 100644 --- a/site/content/docs/main/config/upstream-tls.md +++ b/site/content/docs/main/config/upstream-tls.md @@ -5,7 +5,11 @@ Applying the `projectcontour.io/upstream-protocol.tls` annotation to a Service o The same configuration can be specified by setting the protocol name in the `spec.routes.services[].protocol` field on the HTTPProxy object. If both the annotation and the protocol field are specified, the protocol field takes precedence. By default, the upstream TLS server certificate will not be validated, but validation can be requested by setting the `spec.routes.services[].validation` field. -This field has mandatory `caSecret` and `subjectName` fields, which specify the trusted root certificates with which to validate the server certificate and the expected server name. +This field has mandatory `caSecret`, `subjectName`, and `subjectNames` fields, which specify the trusted root certificates with which to validate the server certificate and the expected server name(s). + +_**Note:** +The `subjectName` field is deprecated in favor of `subjectNames`. When using `subjectNames`, the first entry must match the value for `subjectName`. The `subjectName` field also has a limit of 250 characters._ + The `caSecret` can be a namespaced name of the form `/`. If the CA secret's namespace is not the same namespace as the `HTTPProxy` resource, [TLS Certificate Delegation][4] must be used to allow the owner of the CA certificate secret to delegate, for the purposes of referencing the CA certificate in a different namespace, permission to Contour to read the Secret object from another namespace. _**Note:** @@ -60,7 +64,10 @@ Status: ## Upstream Validation When defining upstream services on a route, it's possible to configure the connection from Envoy to the backend endpoint to communicate over TLS. -Two configuration items are required, a CA certificate and a `SubjectName` which are both used to verify the backend endpoint's identity. + +A CA certificate and a Subject Name must be provided, which are both used to verify the backend endpoint's identity. + +If specifying multiple Subject Names, `SubjectNames` and `SubjectName` must be configured such that `SubjectNames[0] == SubjectName`. The CA certificate bundle for the backend service should be supplied in a Kubernetes Secret. The referenced Secret must be of type "Opaque" and have a data key named `ca.crt`. @@ -84,6 +91,9 @@ spec: validation: caSecret: foo-ca-cert subjectName: foo.marketing + subjectNames: + - foo.marketing + - bar.marketing ``` ## Envoy Client Certificate diff --git a/test/e2e/httpproxy/cel_validation_test.go b/test/e2e/httpproxy/cel_validation_test.go new file mode 100644 index 00000000000..584af6017bf --- /dev/null +++ b/test/e2e/httpproxy/cel_validation_test.go @@ -0,0 +1,69 @@ +// Copyright Project Contour Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build e2e + +package httpproxy + +import ( + "context" + "strings" + + . "github.com/onsi/ginkgo/v2" + contourv1 "github.com/projectcontour/contour/apis/projectcontour/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func testCELValidation(namespace string) { + Specify("UpstreamValidation is validated by CEL rule on creation", func() { + t := f.T() + + subjectNameNoMatch := &contourv1.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: "subjectname-no-match", + }, + Spec: contourv1.HTTPProxySpec{ + VirtualHost: &contourv1.VirtualHost{ + Fqdn: "example.com", + }, + Routes: []contourv1.Route{ + { + Services: []contourv1.Service{ + { + Name: "any-service-name", + Port: 80000, + UpstreamValidation: &contourv1.UpstreamValidation{ + CACertificate: "namespace/name", + SubjectNames: []string{"wrong.com", "example.com"}, + SubjectName: "example.com", + }, + }, + }, + }, + }, + }, + } + + err := f.Client.Create(context.TODO(), subjectNameNoMatch) + require.Error(t, err) + + isExpectedErr := func(err error) bool { + return strings.Contains(err.Error(), "subjectNames[0] must equal subjectName if set") + } + assert.True(t, isExpectedErr(err)) + + }) +} diff --git a/test/e2e/httpproxy/httpproxy_test.go b/test/e2e/httpproxy/httpproxy_test.go index c59dd17b18a..be030dfb3a0 100644 --- a/test/e2e/httpproxy/httpproxy_test.go +++ b/test/e2e/httpproxy/httpproxy_test.go @@ -60,6 +60,8 @@ var _ = AfterSuite(func() { var _ = Describe("HTTPProxy API validation", func() { f.NamespacedTest("httpproxy-required-field-validation", testRequiredFieldValidation) + f.NamespacedTest("httpproxy-cel-validation", testCELValidation) + f.NamespacedTest("httpproxy-invalid-wildcard-fqdn", testWildcardFQDN) f.NamespacedTest("invalid-cookie-rewrite-fields", testInvalidCookieRewriteFields)