kubernetes-sigs / gateway-api

Repository for the next iteration of composite service (e.g. Ingress) and load balancing APIs.
https://gateway-api.sigs.k8s.io
Apache License 2.0
1.78k stars 461 forks source link

Conflicting SNI's between HTTPRoute & TLSRoute #623

Open stevesloka opened 3 years ago

stevesloka commented 3 years ago

What happened: If I have the same fqdn defined in both an HTTPRoute as well as a TLSRoute, what could the conflict resolution be in that case?

The current docs on a TLSRouteMatch sort of lead me to think that TLSRoutes should match first before HTTPRoutes, but might be nice to raise a Condition for this in the event that one is ignored over another.

Just curious if others have thought about this yet or if I'm maybe misunderstanding the spec. =)

hbagdi commented 3 years ago

At any time, a listener on the Gateway can only select routes of one kind, so conflicts shouldn't happen between route of different types. I'm curious how you are running into this.

stevesloka commented 3 years ago

I thought you could duplicate listener ports in a Gateway and then the controller should collapse them:

If the implementation does collapse compatible Listeners, the hostname provided in the incoming client request MUST be matched to a Listener to find the correct set of Routes. The incoming hostname MUST be matched using the Hostname field for each Listener in order of most to least specific. That is, exact matches must be processed before wildcard matches.

I guess we can apply the same conflict logic within the same type (e.g. HTTPRoutes), but do across types which would make the behavior consistent in a way, but would need to raise a condition on the object to alert the user.

// ref: https://gateway-api.sigs.k8s.io/references/spec/#networking.x-k8s.io/v1alpha1.Gateway

hbagdi commented 3 years ago

I see the problem.

 87     // 1. Either each Listener within the group specifies the "HTTP"            
 88     //    Protocol or each Listener within the group specifies either                                                                                                                                          
 89     //    the "HTTPS" or "TLS" Protocol.

I think the or in "HTTPS" or "TLS" is the reason for the confusion. I could be wrong here but I think multiplexing HTTPS and TLS routing configuration on the same port isn't something we have discussed before. Collapsing should really happen only for listeners which have the same protocol type. I don't think many implementations would be able to support such a configuration. Is that something you already support (or even plan to support)?

/cc @robscott @jpeach @Miciah

stevesloka commented 3 years ago

Is that something you already support (or even plan to support)?

Today Contour supports a single insecure/secure listener, so all traffic routes through those two listeners essentially. Someone could potentially create a TLS route with the same fqdn as an HTTPRoute and you'd have a conflict. We handle this today by only supporting one or the other if users create them on the same fqdn.

jpeach commented 3 years ago

I see the problem.

 87     // 1. Either each Listener within the group specifies the "HTTP"            
 88     //    Protocol or each Listener within the group specifies either                                                                                                                                          
 89     //    the "HTTPS" or "TLS" Protocol.

I think the or in "HTTPS" or "TLS" is the reason for the confusion. I could be wrong here but I think multiplexing HTTPS and TLS routing configuration on the same port isn't something we have discussed before. Collapsing should really happen only for listeners which have the same protocol type. I don't think many implementations would be able to support such a configuration. Is that something you already support (or even plan to support)?

/cc @robscott @jpeach @Miciah

IIRC an implementation can either conflict the route on this, or could choose HTTPS (on the principle that most specific wins).

hbagdi commented 3 years ago

Today Contour supports a single insecure/secure listener, so all traffic routes through those two listeners essentially. Someone could potentially create a TLS route with the same fqdn as an HTTPRoute and you'd have a conflict. We handle this today by only supporting one or the other if users create them on the same fqdn.

TLSRoute implies that the traffic flowing through the byte stream could be non-HTTP. Can you multiple HTTP/non-HTTP traffic on the same TCP port via Contour? Only then can this conflict occur.

IIRC an implementation can either conflict the route on this, or could choose HTTPS (on the principle that most specific wins).

@jpeach, is such a conflict even possible though?

stevesloka commented 3 years ago

Can you multiple HTTP/non-HTTP traffic on the same TCP port via Contour?

No you can't, but if someone tries to configure this, there should be some set of conditions to warn them that one was rejected and the other is used. I was just trying to think through what should happen in that case, which resource "wins" and what conditions to set so the user is aware of what's happening.

hbagdi commented 3 years ago

No you can't, but if someone tries to configure this, there should be some set of conditions to warn them that one was rejected and the other is used.

In that case, shouldn't there be a conflict of listeners error on the Gateway status?

Let's say we have the following:

  listeners:  # Use GatewayClass defaults for listener definition.
  - protocol: HTTP
    port: 80
    routes:
      kind: HTTPRoute
  - protocol: TLS
    port: 80
    routes:
      kind: TLSRoute

In this case, shouldn't the controller signal an error on the Gateway resource stating that these two listeners are incompatible?

stevesloka commented 3 years ago

In this case, shouldn't the controller signal an error on the Gateway resource stating that these two listeners are incompatible?

Sorry @hbagdi I feel like I haven't been super clear. I agree with you on that this should be an error but trying to understand who would win in this case? Do all the corresponding routes that are selected by these two listeners get rejected?

If the HTTP listener was valid and the TLS was added, what should we do? I'm not sure if there are any good rules to apply short of rejecting both, but worry that you'd drop all traffic in this case.

k8s-triage-robot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

jpeach commented 3 years ago

To summarize my interpretation:

You end up with both an HTTPRoute and a TLSRoute binding to the same hostname (i.e. SNI name).

Either each Listener within the group specifies the “HTTP” Protocol or each Listener within the group specifies either the “HTTPS” or “TLS” Protocol.

So the spec claims that this ought to be OK (the rationale is that you can still route properly based on the TLS SNI). However, there's no language that prevents an implementation rejecting this configuration though (it's stated as "an implementation might", not MUST).

So I can think of 3 options:

  1. Reject the TLS listener as Conflicted.

  2. Accept the HTTPRoute and refuse the TLSRoute. This follows the "most specific wins" rule. In this case, HTTP is more specific than TLS, so the HTTPRoute binds to the name and the TLSRoute is refused. The listener that matches TLSRoutes would get the false condition ResolvedRefs with the reason DegradedRoutes and the TLSRoute would get the false condition Admitted.

  3. Accept the configuration. Another way to resolve the conflict is to configure the listener using ALPN protocol names. You could send HTTP protocols to the HTTPRoute and everything else to the TLSRoute. That probably has limited value in practice, but it would be a direct mapping of the requested configuration. If you do this, there won't be any errors in this config.

youngnick commented 3 years ago

Catching up on this one, I think @jpeach has nailed this, and I think we should do his option 2 above.

robscott commented 3 years ago

I think option 1 may be preferable/simpler to handle. I can't think of a use case where you'd want an HTTPS and TLS listener for the same hostname on a Gateway. If users attached different certs to each listener it could get especially difficult to implement. It seems significantly simpler to me to consider that invalid.

howardjohn commented 3 years ago

I may be missing something but seems like 1 and 2 are the same? Or is the difference just rejecting the listener vs rejecting all routes on the listener? If the latter, I slightly prefer 1, seems a bit simpler.

I agree (3) could be done but seems a bit complex and not likely portable, so I would prefer to defer it (likely indefinitely)

robscott commented 3 years ago

Yeah, my interpretation was that 1 would reject the entire listener instead of just specific routes. That would be my preferred option.

youngnick commented 3 years ago

I think 1 is fine too, and agree that it is simpler.

k8s-triage-robot commented 3 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

hbagdi commented 3 years ago

/lifecycle frozen

shaneutt commented 1 year ago

Since the problem here comes up in conflicts between HTTPRoutes and TLSRoutes we don't expect this to be needed for v1.0.0 as we expect only GatewayClass, Gateway and HTTPRoute to be going GA for that release. We should however choose one of the options for resolving this problem and get this solved prior to beta for TLSRoute.

mlavacca commented 3 months ago

Since We want to promote TLSRoute for 1.2, I'm adding this to the TLSRoute epic, as we need to figure out how to deal with such a situation.

Said that, I lean toward solution 1, for its simplicity.

shaneutt commented 2 weeks ago

Does anyone have capacity for this one? Should we sync about the future plans for TLSRoute soon to make sure we're all on the same page?

youngnick commented 2 weeks ago

I think we should talk more about TLSRoute as part of scoping for 1.3, once 1.2 is released, yes.