Conformance test for multiple TLS certificates

[bowei]

What would you like to be added:

We should have conformance tests where the Gateway HTTPS listener references multiple TLS certificates.

Why this is needed:

This covers the Core listener surface area.

[robscott]

This is one of the bullet points of #1104 (although that doesn't explicitly require multiple certs), adding a link from there as well.

[ksankeerth]

I’m interested in working on this conformance test. I went through the documentation. From my understanding, the conformance test for Gateway HTTPS listener references multiple TLS certificates should include the below aspects

1. The GatewayClass MUST use the longest matching SNI out of all available certificates for any TLS handshake.
2. The SNIs of all the certificates MUST match with the listener's hostname

Did I miss anything here? @robscott @bowei

[youngnick]

Sorry for the delay, @ksankeerth, that looks right to me.

[ksankeerth]

Thanks @youngnick. I'll send a PR to cover these tests. Let's get others' opinions after the PR

[ksankeerth]

Hi @youngnick, I sent a PR with a conformance test. The test cases were written to check the longest SNI when multiple TLS certs are available. If possible, please review it.

[skriss]

My understanding was that multiple certificate refs per Listener was "custom" conformance, per: https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1beta1/gateway_types.go#L312-L314 https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1beta1/gateway_types.go#L328-L330

Am I missing something here?

(Per @robscott's point in https://github.com/kubernetes-sigs/gateway-api/issues/1330#issuecomment-1215990890, we do still need a test for a single TLS cert, though).

[rainest]

No, I think you're correct @skriss. It looks like the language is a bit confusing, where https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.Listener has

The GatewayClass MUST use the longest matching SNI out of all available certificates for any TLS handshake. Support: Core

under the tls field, which I think meant to indicate that the tls field itself is core, and then also included some additional tips related to if. After, https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.GatewayTLSConfig, the type that goes in the tls field, indicates that multiple certificate support is expanded. The longest SNI bit should maybe reside in the GatewayTLSConfig docs instead of Listener, or the Listener doc should be amended to note that this only applies if you do implement the optional expanded feature (and presumably that should be "Gateway" instead of "GatewayClass").

The proposed change tests the longest SNI bit only (though I'm not sure there's anything more to test for multiple refs, and comments above seem to agree).

[ksankeerth]

@rainest Sorry for the confusion related to the proposed changes. I also looked at the same doc. The Longest Matching SNI was mentioned as support: core. The second proposed test seems unnecessary. As “Host Header” and “Hostname” should match, I thought the “SAN” or “CommonName” of certificates also match with “Hostname”. I think it’s not necessary for Support: core. Even in the PR, I didn’t include any tests for that. #1375

The proposed change tests the longest SNI bit only (though I'm not sure there's anything more to test for multiple refs, and comments above seem to agree).

[youngnick]

We discussed this one in a triage meeting, and although we have #1375 open, before we merge that one, we need another PR to move the multiple certs field to Extended support, and we can discuss there if we all agree (seems likely we do though).

@ksankeerth, any chance you are game to give that one a go?

[ksankeerth]

We discussed this one in a triage meeting, and although we have #1375 open, before we merge that one, we need another PR to move the multiple certs field to Extended support, and we can discuss there if we all agree (seems likely we do though).

@ksankeerth, any chance you are game to give that one a go?

@youngnick I submitted a PR for this. https://github.com/kubernetes-sigs/gateway-api/pull/1526

[Xunzhuo]

Hey @ksankeerth, since you are working on it now. May I remove help and assign this to you?

[shaneutt]

It's marked to fix this, so I think we're good @Xunzhuo we just need to catch up on our PR review backlog :sweat_smile:

/assign @ksankeerth

[shaneutt]

@robscott and I talked about this one in grooming, we've kinda gotten stuck and need to rethink how we're going to move forward

/assign @robscott

[robscott]

I think this issue has gotten stuck, and to unblock it we're going to need a clear understanding of how underlying dataplanes handle multiple certificates. Before we can call this extended, we need to:

1. List out how underlying dataplanes handle multiple TLS certs, particularly when they're conflicting or overlapping. At a minimum this should cover NGINX, HAProxy, and Envoy.
2. Determine if there is sufficiently overlapping behavior between implementations, if so, define what that is and build that into Gateway API spec.

Until we have this figured out, we can't move forward with graduating multiple TLS certs to "Extended" support.

[rainest]

Kong's implementation:

1. Allows provisioning an a single alternate certificate, in order to offer different key types (e.g. RSA and ECDSA) depending on what a client supports.
2. For overlapping certificates, will serve whichever certificate has the more specific hostname. Given a request for foo.example.com, Kong will serve a foo.example.com over a *.example.com certificate.
3. Does not allow exact overlaps. Attempting to configure a second foo.example.com certificate outside the alternate system in (1) will fail.

We intended to add support for (1) to meet the spec as it is. The behavior around overlapping/duplicate certificates doesn't fall under the existing spec, as the additional certificates are configured as part of a Listener, which already specifies a hostname independent of the certificates attached to the Listener.

Our implementation does not automatically serve certificates based on their CN or SANs. Our SNI->selected certificate configuration is independent of the certificate contents, so we create a mapping from the Listener hostname to the certificates the Listener references.

I think the spec could reasonably specify rules for overlapping hostnames across different Listeners, as I wouldn't expect the "choose the most specific hostname you can" rule would be controversial.

I don't think we could easily specify conformance rules for handling of additional certificates configured within a Listener. I would not be surprised if additional certificates vary in purpose across vendors.