Skip to content
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

Support Termination mode for TLS listener protocol (TLSRoutes, TCPRoutes) #5461

Closed
Rycieos opened this issue Jun 9, 2023 · 8 comments · Fixed by #5481
Closed

Support Termination mode for TLS listener protocol (TLSRoutes, TCPRoutes) #5461

Rycieos opened this issue Jun 9, 2023 · 8 comments · Fixed by #5481
Assignees
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@Rycieos
Copy link

Rycieos commented Jun 9, 2023

This is an enhancement request, but it could also be considered a bug, as the Kubernetes Gateway API might require this support (see @skriss's comment).

Description

A Gateway object with a Listener that accepts TLSRoutes should support mode: Terminate as detailed in the GatewayTLSConfig spec.

User story

I have an application that speaks a nonstandard application protocol over TCP. I want the traffic wrapped in TLS. And since I want my application to be as simple as possible, I want the Gateway to terminate the TLS tunnel.

This can be specified with this example Gateway Spec:

apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
spec:
  listeners:
  - allowedRoutes:
      kinds:
      - group: gateway.networking.k8s.io
        kind: TLSRoute
    name: tls
    port: 5000
    protocol: TLS
    tls:
      mode: Terminate
      certificateRefs:
      - name: my-secret

And a TLSRoute like:

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: TLSRoute
spec:
  hostnames:
    - example.com
  rules:
    - backendRefs:
      - kind: Service
        name: example
        port: 3102

Issue

Currently, if this Gateway is created, Contour returns an error on the Gateway object with the message:

Listener.TLS.Mode must be "Passthrough" when protocol is "TLS".

@Rycieos Rycieos added kind/feature Categorizes issue or PR as related to a new feature. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Jun 9, 2023
@skriss skriss added the area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. label Jun 9, 2023
@skriss
Copy link
Member

skriss commented Jun 9, 2023

@Rycieos I couldn't help taking a quick look at this before I head out for the weekend. I think it might be as simple as skriss@83ea358?diff=split; I did some quick testing locally and it seems to produce the desired outcome (envoy listener filter chain match using SNI; terminate TLS at Envoy; TCP proxying to backend service). We already support this use case for HTTPProxy's TCPProxy which helps a lot.

I pushed an image for the above: docker.io/steveheptio/contour:many-listeners-tlsroute-terminate in case you want to try it out. But, I'll plan to spend some more time on this next week, so if you want to wait until I've had more than 30 minutes to look at it before spending time testing, that's totally understandable too 😀

@Rycieos
Copy link
Author

Rycieos commented Jun 9, 2023

@skriss thanks for looking into this so quickly! I'll give that a test first thing Monday.

@skriss
Copy link
Member

skriss commented Jun 12, 2023

@Rycieos just doing some research upstream on the state of this feature, and I believe it's actually not intended to be supported (thanks @sunjayBhatia for pointing me to some of these references). See:

However, you should still be able to solve your use case by:

  • creating multiple Listeners, each with a different hostname, with protocol=TLS and tls.mode=Terminate
  • attach a TCPRoute to each Listener, each routing to the appropriate backend for the Listener's hostname

(This approach is what John H. is referring to in https://kubernetes.slack.com/archives/CR0H13KGA/p1670888417032899?thread_ts=1670887225.139909&cid=CR0H13KGA and https://kubernetes.slack.com/archives/CR0H13KGA/p1670888614417199?thread_ts=1670887225.139909&cid=CR0H13KGA.

Note that Contour still needs to add support for TCPRoute, including in combo with the TLS protocol, not just the TCP protocol, in order for you to be able to do this.

@Rycieos
Copy link
Author

Rycieos commented Jun 12, 2023

@skriss I tested your quick patch; turns out the testing wasn't fast.

You fixed the surface issue: the Listener was accepted, and a TLSRoute was accepted by it as well. Unfortunately, it doesn't work, as the the Service did not expose the port requested by the Listener. Probably an oversight? (Extra strangely, the Service exposes port 80, but doesn't accept connections.)

As to your research, that surprises me. Not supporting TLS termination on TLSRoutes does not make sense to me; 90% of the work is already done with HTTPS termination. And the suggested workaround is basically worthless; the main benefit of the Gateway API to me is I can prescribe a wildcard DNS record and certificate, and let my developers pick subdomains as they want with TLSRoute objects. Needing to make a new Listener on the Gateway for each domain completely defeats the point.

Obviously this isn't the place to debate API design. If you think that the community would be receptive to this feedback, I'd be happy to explain my use case if you can point me towards where to send it.

@skriss
Copy link
Member

skriss commented Jun 12, 2023

You fixed the surface issue: the Listener was accepted, and a TLSRoute was accepted by it as well. Unfortunately, it doesn't work, as the the Service did not expose the port requested by the Listener. Probably an oversight? (Extra strangely, the Service exposes port 80, but doesn't accept connections.)

Hmm, the Gateway provisioner should be adding the port to the Envoy service. However, I'm going to hold off on investing more time here until we get an upstream resolution to the below discussion.

As to your research, that surprises me. Not supporting TLS termination on TLSRoutes does not make sense to me; 90% of the work is already done with HTTPS termination. And the suggested workaround is basically worthless; the main benefit of the Gateway API to me is I can prescribe a wildcard DNS record and certificate, and let my developers pick subdomains as they want with TLSRoute objects. Needing to make a new Listener on the Gateway for each domain completely defeats the point.

Obviously this isn't the place to debate API design. If you think that the community would be receptive to this feedback, I'd be happy to explain my use case if you can point me towards where to send it.

I do think it's worth bringing up the details of your use case as an actual user. I'd suggest opening a new issue or a discussion, as I don't think there's anything existing that specifically covers this topic.

@skriss skriss added the blocked/needs-gateway-api Categorizes the issue or PR as blocked because it needs changes in Gateway API. label Jun 12, 2023
@Rycieos
Copy link
Author

Rycieos commented Jun 12, 2023

Hmm, the Gateway provisioner should be adding the port to the Envoy service.

Thinking on this again, that might be my fault. I was testing with the Helm chart instead of the beta provisioner, which probably creates the Service at install time. I'll test with the provisioner tomorrow.

I do think it's worth bringing up the details of your use case as an actual user. I'd suggest opening a new issue or a discussion, as I don't think there's anything existing that specifically covers this topic.

I opened the above mentioned issue.

@Rycieos
Copy link
Author

Rycieos commented Jun 13, 2023

Hmm, the Gateway provisioner should be adding the port to the Envoy service.

Thinking on this again, that might be my fault.

Yup, it was my fault. Using the provisioner with your custom build, I can get the TLSRoutes to work as I want. I tested with a web server as the backend, and a browser as a client. My custom TCP traffic isn't working currently, but I bet that's an unrelated issue.

@skriss skriss changed the title Support Termination mode for TLSRoutes Support Termination mode for TLS listener protocol (TLSRoutes, TCPRoutes) Jun 15, 2023
@skriss
Copy link
Member

skriss commented Jun 15, 2023

I've updated this to generically cover supporting TLS termination mode for the TLS protocol, which will cover both TLSRoutes and TCPRoutes (pending finalization of upstream dicsussions)

@skriss skriss self-assigned this Jun 15, 2023
@skriss skriss removed lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. blocked/needs-gateway-api Categorizes the issue or PR as blocked because it needs changes in Gateway API. labels Jul 18, 2023
skriss added a commit that referenced this issue Jul 21, 2023
Adds support for TLS termination with the TLS
listener protocol. Envoy is configured to terminate
TLS and then to proxy TCP traffic to the backend.

Closes #5461.

Signed-off-by: Steve Kriss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. kind/feature Categorizes issue or PR as related to a new feature.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants