-
Notifications
You must be signed in to change notification settings - Fork 674
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
SSL Passthrough #15
Comments
related to #14 |
@rusenask Thanks for raising this issue. I need to ask a few clarifying questions to check I understand what a successful outcome of this issue would look like. q. In this scenario who is doing TLS processing? Is it the load balancer in front of Contour? Is it Contour? Is it the service the ingress points to? Is it two or maybe all three? q. If TLS processing is being done by the final k8s service, it feels like you'd want to configure Envoy as a TCP proxy, not a HTTPS proxy. Did I get that right? |
@rothgar that issue should probably be configured through another annotation:
Service that ingress points to. LB and Contour would just pass through traffic to the destination, no termination should happen.
Not sure about Envoy TCP proxy but what I disliked in nginx-ingress was that if you set it to TCP proxy then it can't match hostnames anymore, it's more like 8080:namespace/serviceName:port:
Desired behaviour in this case would be that you could still route based on hostnames to different services and ports. I hope this makes sense. |
Ingress objects are strongly coupled to HTTP. This sounds like a good use case for using a service load balancer directly. Have you tried that? What are the reasons why you decided not to use this approach?
If you're in TCP mode, then i'm guessing there is no HTTP request to inspect to route the traffic. It sounds like you have to dedicate the public IP to forwarding the traffic to only one service, which is very similar to the service load balancer described above. While I think Envoy can do this, I have some doubts about how to express this in the limitations of the Ingress object. |
@rusenask ping, i'd like to keep this conversation going if you have the time. |
@davecheney hey, apologies. TLS functionality would be nice so you could mix HTTP and HTTPS (passthrough) services in one ingress. I know that you could achieve the same thing with multiple service load balancers but having it in on ingress saves you one IP (atleast in GKE) and having external load balancers in GKE costs a bit. At least in nginx-ingress it have this and I thought it's quite nice as you can have HTTP+HTTPS+GRPC services defined in one ingress. The only downside is that nginx keeps killing long connections. I don't need contour to provision TLS certs as I request them from my backend. |
Thanks for your reply. I'll be honest, I'm not sure I understand what you're asking for, but I do understand supporting the
annotation. So I'll work on that and we can see if there is anything left to do afterwards. |
The way I think this can be accomplished is to use a tcp proxy filter on the tls listener. There are a number of caveats with this approach:
If someone wants to attempt this, I'll mark it for 0.5, but I think the limitations are severe enough that you'd be better off using a service loadbalancer in TCP mode, you're not getting much value by adding Envoy into the mix. |
I'm going to remove the milestone from this as i'm not sure how to implement it within the framework that the k8s Ingress document provides. |
this is only supported by Haproxy with L4 TCP SNI inspection.
currently Envoy doesn't support this feature but there is an old issue still active and @PiotrSikora working on a PR.. |
Blocked on envoyproxy/envoy#1843 |
Now that envoyproxy/envoy#1843 was already addressed and released on Envoy 1.7.0, it can be done once #443 gets fixed. Before doing anything and knowing that Contour is taking the use
I don't know if it's even possible but appears to be easily understandable. |
@davecheney, @rosskukulinski can we push this forward knowing that |
Thanks for the ping. I recently finished the upgrade of Envoy to 1.7.0 for the 0.7 milestone so I think we have all the raw materials needed to do this. However,
This isn't the only config that would change. K8s services, what envoy calls clusters, are tired implicitly to routes in the ingressroute spec. Some thought about how to change the routes... section of the spect will probably be needed. |
As it seems to be declared as out-of-scope in #787, when do you think would be the best release/time window to implement this? I'm open to collaborate on this. |
I know you've been waiting a long time for this so I want to try to give you the most complete explanation of the current status. #787 will add TCP port forwarding for TLS connections. They will be decrypted at the Envoy edge and the SNI handshake used to direct the unencrypted traffic to the target pods. This ticket is a request to add nginx's However, envoyproxy/envoy#1843, suggests that it is possible to do the SNI handshake, but somehow forward the encrypted traffic to the backend service. From my understanding of the nginx documentation this is not now Given the ambiguity, I forwarding encrypted traffic through Envoy, what I believe this ticket is asking for, is not in scope for Contour 0.8, but obviously doing most of this plumbing will make it possible to add it in the future. |
@davecheney The only difference between this and #787 (assuming you're using filter chain matching in Envoy) is whether or not |
@PiotrSikora thanks for confirming. |
Cannot explain better than how @PiotrSikora did, thanks! And also for the tip that the absence of |
@davecheney just to let you know that we're going to start testing this is the next days. I came across with a patch that should work. Obviously this wouldn't work without the invaluable hint from @PiotrSikora and with your fantastic work on making TCP proxying / forwarding a reality. Here it is: From 865201dacdea553f30d1d8bc0f114275c3392bfa Mon Sep 17 00:00:00 2001
From: Gorka Lerchundi Osa <g.lerchundi@saltosystems.com>
Date: Wed, 19 Dec 2018 22:02:27 +0100
Subject: [PATCH] tcpproxy: add tls passthrough
---
internal/contour/listener.go | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/internal/contour/listener.go b/internal/contour/listener.go
index a13caca..1ae9b85 100644
--- a/internal/contour/listener.go
+++ b/internal/contour/listener.go
@@ -16,12 +16,13 @@ package contour
import (
"sync"
- "github.com/envoyproxy/go-control-plane/envoy/api/v2"
+ v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
+ "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth"
"github.com/envoyproxy/go-control-plane/envoy/api/v2/listener"
"github.com/gogo/protobuf/proto"
"github.com/heptio/contour/internal/dag"
"github.com/heptio/contour/internal/envoy"
- "k8s.io/api/core/v1"
+ v1 "k8s.io/api/core/v1"
)
const (
@@ -230,11 +231,6 @@ func (v *listenerVisitor) visit(vertex dag.Vertex) {
// the listener properly.
v.http = true
case *dag.SecureVirtualHost:
- data := vh.Data()
- if data == nil {
- // no secret for this vhost, skip it
- return
- }
filters := []listener.Filter{
envoy.HTTPConnectionManager(ENVOY_HTTPS_LISTENER, v.httpsAccessLog()),
}
@@ -246,11 +242,15 @@ func (v *listenerVisitor) visit(vertex dag.Vertex) {
alpnProtos = nil // do not offer ALPN
}
+ var tlsContext *auth.DownstreamTlsContext
+ if data := vh.Data(); data != nil {
+ tlsContext = envoy.DownstreamTLSContext(data[v1.TLSCertKey], data[v1.TLSPrivateKeyKey], vh.MinProtoVersion, alpnProtos...)
+ }
fc := listener.FilterChain{
FilterChainMatch: &listener.FilterChainMatch{
ServerNames: []string{vh.Host},
},
- TlsContext: envoy.DownstreamTLSContext(data[v1.TLSCertKey], data[v1.TLSPrivateKeyKey], vh.MinProtoVersion, alpnProtos...),
+ TlsContext: tlsContext,
Filters: filters,
UseProxyProto: bv(v.UseProxyProto),
}
--
2.20.1 Will tell you something with our conclusions or problems we encounter on the road. In case someone is interested this is already implemented & published in: |
@glerchundi thank you for working on this. I need you to not set patches but signed PRs. Sorry this is a lawyer thing. |
Of course Dave, will do once I empirically verify it’s something that
works. I included the patch only to show you how it’s being done.
Good night,
…On Wed, 19 Dec 2018 at 22:26, Dave Cheney ***@***.***> wrote:
@glerchundi <https://github.com/glerchundi> thank you for working on
this. I need you to not set patches but signed PRs. Sorry this is a lawyer
thing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACIPln-oCqKGjZOd2iKsAtq_3SWXux3xks5u6q8HgaJpZM4QMmnF>
.
|
Allow an IngressRoute for TCP forwarding but without a `tls` section which implicitly configures it as a TLS Passthrough. TLS Passthrough only reads the very first TLS packets and routes the traffic without doing any kind of TLS termination. The chosen `virtualhost` is discovered by reading the TLS SNI (Server Name Indication) extension. Close projectcontour#15
Allow an IngressRoute for TCP forwarding but without a `tls` section which implicitly configures it as a TLS Passthrough. TLS Passthrough only reads the very first TLS packets and routes the traffic without doing any kind of TLS termination. The chosen `virtualhost` is discovered by reading the TLS SNI (Server Name Indication) extension. Closes projectcontour#15
Allow an IngressRoute for TCP forwarding but without a `tls` section which implicitly configures it as a TLS Passthrough. TLS Passthrough only reads the very first TLS packets and routes the traffic without doing any kind of TLS termination. The chosen `virtualhost` is discovered by reading the TLS SNI (Server Name Indication) extension. Closes projectcontour#15 Signed-off-by: Gorka Lerchundi Osa <glertxundi@gmail.com>
…nit-tests-adobe unit tests for adobe customization
Using `(*testing.T)Run()` to run named subtest removed a bit of test boilerpplate, and makes it possible to run individual test cases. Signed-off-by: James Peach <jpeach@vmware.com>
Hello, it would be good to know whether Contour supports ssl passthrough and if it doesn't - whether it would be possible/reasonable to add it.
NGINX ingress controller has this
ingress.kubernetes.io/ssl-passthrough: "true"
annotation, it might make sense to keep the format the same to make it easier to switch between ingresses.The text was updated successfully, but these errors were encountered: