projectcontour / contour

Contour is a Kubernetes ingress controller using Envoy proxy.
https://projectcontour.io
Apache License 2.0
3.71k stars 673 forks source link

SNI lost when using httpproxy as TLS, TCP reverse Proxy #4373

Open Jochen-dba opened 2 years ago

Jochen-dba commented 2 years ago

Hello,

I'm trying to use HTTPPROY as TLS, TCP reverse Proxy for MongoDB. This is my YAML:

apiVersion: projectcontour.io/v1
kind: HTTPProxy
metadata:
  name: my-replica-set-0.externaldomain.com-httpproxy
spec:
  ingressClassName: contour
  virtualhost:
    fqdn: my-replica-set-0.externaldomain.com
    tls:
      secretName: my-replica-set-0.externaldomain.com-cert
  tcpproxy:
    services:
      - name: my-replica-set-0-svc
        port: 27017
        protocol: tls

Why?:

I successfully terminate TLS (certificates valid for external FQDN) using spec.virtualhost.tls The TCP traffic which is encapsulated in TLS is forwarded: spec.tcpproxy I use upstream TLS: spec.tcpproxy.services.protocol

All of this works - the only issue is: it seems like due to TLS termination SNI from the Client is not passed into the upstream TLS connection. Unfortunately the MongoDB Server requires the SNI for Split-Horizon (which is in turn required for clients outside of k8s): https://docs.mongodb.com/kubernetes-operator/master/reference/k8s-operator-specification/#spec.connectivity.replicaSetHorizons

Using spec.virtualhost.tls.passthrough is unfortunately not an option, as this would present the certificates which are only valid for *.cluster-local to the client.

What could I do to preserve the external clients SNI while terminating TLS and using upstream TLS via HTTPPROXY with TLS encapsulated TCP traffic? Is there a feature to achieve this I'm not aware of?

Many, many thanks & best Regards, Jochen

tsaarni commented 2 years ago

Hi @Jochen-dba,

I believe this feature is currently not implemented but it sounds like a good feature.

I think "passing" the SNI from downstream to upstream is not supported by Envoy, but it would be possible to set the SNI to a specific value. We have that for HTTP connections (via header policy) but it seems to be missing for TCPProxy.

youngnick commented 2 years ago

Agreed, it seems reasonable to set the SNI for a forwarded connection to whatever it was on reception.

Jochen-dba commented 2 years ago

Hello and good morning from europe @tsaarni, @youngnick,

hope you've had a good weekend! I'm very happy to hear that you like the idea of having this feature. I would also like to point out, why this would be worth investing (at least a little) effort: From my experience the adoption of stateful deployments like database systems on kubernetes is rising fast, as kubernetes is a trending technology, and helps many companies which were struggling to provide IaaS until now. As these companies deploy more non-HTTP workloads TCP is getting more important on kubernetes.

I had a closer look at header policy and as expected it cannot be used for TCP as you already correctly pointed out.

I also had a closer look at Upstream Validation: https://projectcontour.io/docs/v1.10.0/config/upstream-tls/#upstream-validation

According to https://github.com/projectcontour/contour/issues/2893 the ExtensionService processing configures Envoy to send a SNI server name when upstream TLS validation is enabled. This sounds like this is what we need. And to me it also seems plausible, that you need to send SNI in order to give the upstream service a chance to show the correct certificate.

Surprisingly to me the API already allows us to set spec.tcpproxy.services.validation.caSecret and spec.tcpproxy.services.validation.subjectName. I would expect three things to happen after validation is enabled for a tcpproxy.service:

  1. The existence of spec.tcpproxy.services.validation.caSecret is verified before HTTPProxy Object goes into STATUS: Valid
  2. The upstream TLS certificate is validated against spec.tcpproxy.services.validation.subjectName before HTTPProxy Object goes into STATUS: Valid
  3. TLS SNI is sent to upstream service

Unfortunately my tests showed, that none of those three things happen. My expectations 1) and 2) come from the documentation: https://projectcontour.io/docs/v1.10.0/config/upstream-tls/#upstream-validation

Further I was able to verify that at least my expectations 1) and 2) indeed are satisfied for spec.routes.services.validation.subjectName and spec.routes.services.validation.caSecret I could not verify expectation no. 3) as the non-TCPPROXY traffic cannot be consumed by my upstream service.

So to summarise my findings:

  1. I assume, that spec.routes.services.validation already does exactly what we need for http (non-tcp) traffic.
  2. spec.tcpproxy.services.validation is already present in the API, but it doesn't matter what values you set - it seems like it's just not used in the code (e.g. caSecret is not even checked for existence)
  3. Most likely (only based on my assumption) the code which is used to read and accodringly use spec.routes.services.validation just needs to be used in exactly the same way for spec.tcpproxy.services.validation to implement this feature

In case my findings are correct this could maybe easily be implemented and we might be able to achieve a "quick win" here. What do you think? Would you be interested to have a closer look at my findings?

Many thanks again & best Regards, Jochen

youngnick commented 2 years ago

Thanks for the great investigation and detailed report @Jochen-dba.

I agree that the spec.tcpproxy.services.validation should work the same as spec.routes.services.validation, the fact that it doesn't is a bug.

I also agree that replicating that behavior seems like it should be straightforward. Writing end-to-end tests to validate it may be a bit tricky, but worthwhile anyway.

I've labelled this and added it to the milestone, we'll see if we can get it done.

I've also marked it as help-wanted, as it seems eminently doable even for someone not super familiar with the code.

Jochen-dba commented 2 years ago

Hey @youngnick,

thanks a lot - sounds great! As I never used Go before I was not confident enough to try this on my own. But I will try to have a look at least and maybe get support in our site.

Best Regards, Jochen

sunjayBhatia commented 2 years ago

@Jochen-dba did you get a change to work on this? Just checking in since contour v1.21 is coming soon in case you wanted to get your work into that release

Jochen-dba commented 2 years ago

Hello @sunjayBhatia,

unfortunately I didn't manage to work on this until now. But it is still on my high priority tasks list.

Best regards, Jochen