diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-multiple-tls-configuration.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-multiple-tls-configuration.out.yaml index bbdb1f4df6..d1fde15677 100644 --- a/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-multiple-tls-configuration.out.yaml +++ b/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-multiple-tls-configuration.out.yaml @@ -32,8 +32,7 @@ gateways: conditions: - lastTransitionTime: null message: Secret envoy-gateway/tls-secret-ecdsa-2 public key algorithm must - be unique, matched certificate FQDN [foo.bar.com] has a conflicting algorithm - [ECDSA]. + be unique, certificate domain foo.bar.com has a conflicting algorithm [ECDSA]. reason: InvalidCertificateRef status: "False" type: ResolvedRefs diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.out.yaml deleted file mode 100644 index a28e3dc648..0000000000 --- a/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.out.yaml +++ /dev/null @@ -1,117 +0,0 @@ -gateways: -- apiVersion: gateway.networking.k8s.io/v1 - kind: Gateway - metadata: - creationTimestamp: null - name: gateway-1 - namespace: envoy-gateway - spec: - gatewayClassName: envoy-gateway-class - listeners: - - allowedRoutes: - namespaces: - from: All - hostname: example.com - name: tls - port: 443 - protocol: HTTPS - tls: - certificateRefs: - - group: null - kind: null - name: tls-secret-1 - mode: Terminate - status: - listeners: - - attachedRoutes: 1 - conditions: - - lastTransitionTime: null - message: Secret envoy-gateway/tls-secret-1 must contain valid tls.crt and - tls.key, hostname example.com does not match Common Name or DNS Names in - the certificate tls.crt. - reason: InvalidCertificateRef - status: "False" - type: ResolvedRefs - - lastTransitionTime: null - message: Listener is invalid, see other Conditions for details. - reason: Invalid - status: "False" - type: Programmed - name: tls - supportedKinds: - - group: gateway.networking.k8s.io - kind: HTTPRoute - - group: gateway.networking.k8s.io - kind: GRPCRoute -httpRoutes: -- apiVersion: gateway.networking.k8s.io/v1 - kind: HTTPRoute - metadata: - creationTimestamp: null - name: httproute-1 - namespace: default - spec: - parentRefs: - - name: gateway-1 - namespace: envoy-gateway - rules: - - backendRefs: - - name: service-1 - port: 8080 - matches: - - path: - value: / - status: - parents: - - conditions: - - lastTransitionTime: null - message: There are no ready listeners for this parent ref - reason: NoReadyListeners - status: "False" - type: Accepted - - lastTransitionTime: null - message: Resolved all the Object references for the Route - reason: ResolvedRefs - status: "True" - type: ResolvedRefs - controllerName: gateway.envoyproxy.io/gatewayclass-controller - parentRef: - name: gateway-1 - namespace: envoy-gateway -infraIR: - envoy-gateway/gateway-1: - proxy: - metadata: - labels: - gateway.envoyproxy.io/owning-gateway-name: gateway-1 - gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway - ownerReference: - kind: GatewayClass - name: envoy-gateway-class - name: envoy-gateway/gateway-1 - namespace: envoy-gateway-system -xdsIR: - envoy-gateway/gateway-1: - accessLog: - json: - - path: /dev/stdout - globalResources: - proxyServiceCluster: - name: envoy-gateway/gateway-1 - settings: - - addressType: IP - endpoints: - - host: 7.6.5.4 - port: 8080 - zone: zone1 - metadata: - name: envoy-envoy-gateway-gateway-1-196ae069 - namespace: envoy-gateway-system - sectionName: "8080" - name: envoy-gateway/gateway-1 - protocol: TCP - readyListener: - address: 0.0.0.0 - ipFamily: IPv4 - path: /ready - port: 19003 diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.in.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-tls-configuration-sni-san-mismatch-allowed.in.yaml similarity index 100% rename from internal/gatewayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.in.yaml rename to internal/gatewayapi/testdata/gateway-with-listener-with-tls-configuration-sni-san-mismatch-allowed.in.yaml diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-tls-configuration-sni-san-mismatch-allowed.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-tls-configuration-sni-san-mismatch-allowed.out.yaml new file mode 100644 index 0000000000..1ebb9fc1a5 --- /dev/null +++ b/internal/gatewayapi/testdata/gateway-with-listener-with-tls-configuration-sni-san-mismatch-allowed.out.yaml @@ -0,0 +1,180 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + hostname: example.com + name: tls + port: 443 + protocol: HTTPS + tls: + certificateRefs: + - group: null + kind: null + name: tls-secret-1 + mode: Terminate + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: tls + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/tls + ports: + - containerPort: 10443 + name: https-443 + protocol: HTTPS + servicePort: 443 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 443 + hostnames: + - example.com + isHTTP2: false + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: tls + name: envoy-gateway/gateway-1/tls + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10443 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 1 + hostname: example.com + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/example_com + pathMatch: + distinct: false + name: "" + prefix: / + tls: + alpnProtocols: null + certificates: + - certificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUREVENDQWZXZ0F3SUJBZ0lVRUZNaFA5ZUo5WEFCV3NRNVptNmJSazJjTE5Rd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0ZqRVVNQklHQTFVRUF3d0xabTl2TG1KaGNpNWpiMjB3SGhjTk1qUXdNakk1TURrek1ERXdXaGNOTXpRdwpNakkyTURrek1ERXdXakFXTVJRd0VnWURWUVFEREF0bWIyOHVZbUZ5TG1OdmJUQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFKbEk2WXhFOVprQ1BzNnBDUXhickNtZWl4OVA1RGZ4OVJ1NUxENFQKSm1kVzdJS2R0UVYvd2ZMbXRzdTc2QithVGRDaldlMEJUZmVPT1JCYlIzY1BBRzZFbFFMaWNsUVVydW4zcStncwpKcEsrSTdjSStqNXc4STY4WEg1V1E3clZVdGJ3SHBxYncrY1ZuQnFJVU9MaUlhdGpJZjdLWDUxTTF1RjljZkVICkU0RG5jSDZyYnI1OS9SRlpCc2toeHM1T3p3Sklmb2hreXZGd2V1VHd4Sy9WcGpJKzdPYzQ4QUJDWHBOTzlEL3EKRWgrck9hdWpBTWNYZ0hRSVRrQ2lpVVRjVW82TFNIOXZMWlB0YXFmem9acTZuaE1xcFc2NUUxcEF3RjNqeVRUeAphNUk4SmNmU0Zqa2llWjIwTFVRTW43TThVNHhIamFvL2d2SDBDQWZkQjdSTFUyc0NBd0VBQWFOVE1GRXdIUVlEClZSME9CQllFRk9SQ0U4dS8xRERXN2loWnA3Y3g5dFNtUG02T01COEdBMVVkSXdRWU1CYUFGT1JDRTh1LzFERFcKN2loWnA3Y3g5dFNtUG02T01BOEdBMVVkRXdFQi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBRnQ1M3pqc3FUYUg1YThFMmNodm1XQWdDcnhSSzhiVkxNeGl3TkdqYm1FUFJ6K3c2TngrazBBOEtFY0lEc0tjClNYY2k1OHU0b1didFZKQmx6YS9adWpIUjZQMUJuT3BsK2FveTc4NGJiZDRQMzl3VExvWGZNZmJCQ20xdmV2aDkKQUpLbncyWnRxcjRta2JMY3hFcWxxM3NCTEZBUzlzUUxuS05DZTJjR0xkVHAyYm9HK3FjZ3lRZ0NJTTZmOEVNdgpXUGlmQ01NR3V6Sy9HUkY0YlBPL1lGNDhld0R1M1VlaWgwWFhkVUFPRTlDdFVhOE5JaGMxVVBhT3pQcnRZVnFyClpPR2t2L0t1K0I3OGg4U0VzTzlYclFjdXdiT25KeDZLdFIrYWV5a3ZBcFhDUTNmWkMvYllLQUFSK1A4QUpvUVoKYndJVW1YaTRnajVtK2JLUGhlK2lyK0U9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0= + name: envoy-gateway/tls-secret-1 + privateKey: '[redacted]' + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/tls.go b/internal/gatewayapi/tls.go index 74857b15ae..9d7d864fce 100644 --- a/internal/gatewayapi/tls.go +++ b/internal/gatewayapi/tls.go @@ -12,17 +12,16 @@ import ( "time" corev1 "k8s.io/api/core/v1" - gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" ) // validateTLSSecretData ensures the cert and key provided in a secret // is not malformed and can be properly parsed -func validateTLSSecretsData(secrets []*corev1.Secret, host *gwapiv1.Hostname) ([]*x509.Certificate, error) { +func validateTLSSecretsData(secrets []*corev1.Secret) ([]*x509.Certificate, error) { var publicKeyAlgorithm string var certs []*x509.Certificate var parseErr error - pkaSecretSet := make(map[string][]string) + pkaSecretSet := make(map[string]string) for _, secret := range secrets { certData := secret.Data[corev1.TLSCertKey] @@ -48,18 +47,29 @@ func validateTLSSecretsData(secrets []*corev1.Secret, host *gwapiv1.Hostname) ([ return nil, fmt.Errorf("%s/%s must contain valid %s and %s, unable to decode pem data in %s", secret.Namespace, secret.Name, corev1.TLSCertKey, corev1.TLSPrivateKeyKey, corev1.TLSPrivateKeyKey) } - matchedFQDN, err := verifyHostname(cert, host) - if err != nil { - return nil, fmt.Errorf("%s/%s must contain valid %s and %s, hostname %s does not match Common Name or DNS Names in the certificate %s", secret.Namespace, secret.Name, corev1.TLSCertKey, corev1.TLSPrivateKeyKey, string(*host), corev1.TLSCertKey) + // SNI and SAN/Cert Domain mismatch is allowed + // Consider converting this into a warning once + // https://github.com/envoyproxy/gateway/issues/6717 is in + + // Extract certificate domains (SANs or CN) for uniqueness checking + var certDomains []string + if len(cert.DNSNames) > 0 { + certDomains = cert.DNSNames + } else if cert.Subject.CommonName != "" { + certDomains = []string{cert.Subject.CommonName} } - pkaSecretKey := fmt.Sprintf("%s/%s", publicKeyAlgorithm, matchedFQDN) - // Check whether the public key algorithm and matched certificate FQDN in the referenced secrets are unique. - if matchedFQDN, ok := pkaSecretSet[pkaSecretKey]; ok { - return nil, fmt.Errorf("%s/%s public key algorithm must be unique, matched certificate FQDN %s has a conflicting algorithm [%s]", - secret.Namespace, secret.Name, matchedFQDN, publicKeyAlgorithm) + // Check uniqueness for each domain in the certificate with this algorithm + for _, domain := range certDomains { + pkaSecretKey := fmt.Sprintf("%s/%s", publicKeyAlgorithm, domain) + + // Check whether the public key algorithm and certificate domain are unique + if _, ok := pkaSecretSet[pkaSecretKey]; ok { + return nil, fmt.Errorf("%s/%s public key algorithm must be unique, certificate domain %s has a conflicting algorithm [%s]", + secret.Namespace, secret.Name, domain, publicKeyAlgorithm) + } + pkaSecretSet[pkaSecretKey] = domain } - pkaSecretSet[pkaSecretKey] = matchedFQDN switch keyBlock.Type { case "PRIVATE KEY": @@ -86,26 +96,6 @@ func validateTLSSecretsData(secrets []*corev1.Secret, host *gwapiv1.Hostname) ([ return certs, parseErr } -// verifyHostname checks if the listener Hostname matches any domain in the certificate, returns a list of matched hosts. -func verifyHostname(cert *x509.Certificate, host *gwapiv1.Hostname) ([]string, error) { - var matchedHosts []string - - listenerContext := ListenerContext{ - Listener: &gwapiv1.Listener{Hostname: host}, - } - if len(cert.DNSNames) > 0 { - matchedHosts = computeHosts(cert.DNSNames, &listenerContext) - } else { - matchedHosts = computeHosts([]string{cert.Subject.CommonName}, &listenerContext) - } - - if len(matchedHosts) > 0 { - return matchedHosts, nil - } - - return nil, x509.HostnameError{Certificate: cert, Host: string(*host)} -} - func validateCertificate(data []byte) error { block, _ := pem.Decode(data) if block == nil { diff --git a/internal/gatewayapi/tls_test.go b/internal/gatewayapi/tls_test.go index 07e68e5f5a..901bbe2685 100644 --- a/internal/gatewayapi/tls_test.go +++ b/internal/gatewayapi/tls_test.go @@ -149,20 +149,13 @@ func TestValidateTLSSecretsData(t *testing.T) { Domain: "*", ExpectedErr: errors.New("test/secret must contain valid tls.crt and tls.key, FOO key format found in tls.key, supported formats are PKCS1, PKCS8 or EC"), }, - { - Name: "invalid-domain-cert", - CertFile: "rsa-cert-san.pem", - KeyFile: "rsa-pkcs8-san.key", - Domain: "*.example.com", - ExpectedErr: errors.New("test/secret must contain valid tls.crt and tls.key, hostname *.example.com does not match Common Name or DNS Names in the certificate tls.crt"), - }, } for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { secrets := createTestSecrets(t, tc.CertFile, tc.KeyFile) require.NotNil(t, secrets) - _, err := validateTLSSecretsData(secrets, &tc.Domain) + _, err := validateTLSSecretsData(secrets) if tc.ExpectedErr == nil { require.NoError(t, err) } else { diff --git a/internal/gatewayapi/validate.go b/internal/gatewayapi/validate.go index c0f324dc9f..76f30f76a0 100644 --- a/internal/gatewayapi/validate.go +++ b/internal/gatewayapi/validate.go @@ -460,7 +460,7 @@ func (t *Translator) validateTerminateModeAndGetTLSSecrets(listener *ListenerCon secrets = append(secrets, secret) } - certs, err := validateTLSSecretsData(secrets, listener.Hostname) + certs, err := validateTLSSecretsData(secrets) if err != nil { status.SetGatewayListenerStatusCondition(listener.gateway.Gateway, listener.listenerStatusIdx,