Add CONNECT support

[mattwoodyard]

A follow up to #1214. Add support for CONNECT to envoy. There are 2 use cases for CONNECT that I'd like to add. Envoy receiving a CONNECT request, connecting to an upstream, issuing a 200 and then downgrading to L4 (similar to websocket support). The second is envoy using CONNECT when initiating an upstream L4 connection (the moral equivalent of envoy using http_proxy for some https upstreams). I think the first one is pretty straightforward.

• Add a config option to enable CONNECT on the listener in http1_options
• Update codec to support the right authority in CONNECT
• Write a Http11ConnectHandlerImpl in the spirit of WsHandlerImpl

The second case is a little more unclear to me. Here's a straw man.

• Upstreams get a 'use connect' flag.
• When enabled any connections to that upstream are initiated with a CONNECT request exchange before the remain traffic is forwarded.

[mattklein123]

Both sound reasonable to me. For the upstream case, I assume we just need to support HTTP/1? I think this could be folded into the http1_codec_options which should be available to upstream clusters in v1 and v2 APIs.

[mattklein123]

Write a Http11ConnectHandlerImpl in the spirit of WsHandlerImpl

Please share as much of the code as possible, but yes, I think the flow will be very similar.

[jippi]

I would love to see support in evnoy for https://linkerd.io/features/http-proxy/

[mattklein123]

@jippi http_proxy is orthogonal to CONNECT. Envoy does support basic http_proxy support, but it's off by default. See the "allow_absolute_url" option here: https://www.envoyproxy.io/docs/envoy/latest/api-v1/network_filters/http_conn_man (and similar v2 option).

What is not supported is DNS lookup for previously unknown backends. This is tracked in #1606.

This could use better coverage in arch overview (https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/http_connection_management) if you feel like doing a doc PR.

[pschultz]

Please note that the default Go HTTP client looks at HTTP_PROXY for HTTPS requests. HTTPS_PROXY takes precedence, but if that's empty HTTP_PROXY is in effect. This means we cannot blindly set HTTP_PROXY to an Envoy listener for all clients right now.

We currently work around this by replacing the Proxy function in the http.DefaultTransport (so https requests don't pass through Envoy), which isn't great. CONNECT support (even without DNS lookups) would be much appreciated.

[salrashid123]

I'm interested in envoy support for CONNECT specifically to see how it can help istio's egress rules which at the moment requires rewriting at the client https:// --> http://...:443 (for example).

this FR should help with that in that if the running container in istio-k8s has an env setting, it'd 'speak' to envoy via CONNECT for https requests (or transparently)

edit:
here's a gist of a minimal squid proxy config which allows a specific domain through over http and https:

• squid_allow_deny_domain

I played around with a local config (without istio or envoy for that matter), the http_proxy= locally would be something like this:

I made up a sample app

    spec:
      containers:
      - name: myapp-container
        image: salrashid123/my_app
        ports:
        - containerPort: 8080
        env:
        - name: HTTP_PROXY
          value: "http://localhost:3128"
      - name: sslproxy
        image: salrashid123/sslproxy
        ports:
        - containerPort: 3128

(w/ istio in the picture the proxy is envoy itself)

I used squid which allows for really advanced content filtering (i.e if you happens to trust a CA squid provides, you can even inspect the transit traffic and make determinations based on some signal there; here's my example )....but i'd be happy w/ envoy and CONNECT if it allows istio to set simple host based rule (i.,e allow https traffic to foo.com but not.bar.com)

-- edit 1/20/18:
Just in context w/ this comment and istio design doc for its 0.3 egress rules talks about SNI for host-based ACLs. The corresponding issue for envoy is: #1843

[seanb4t]

This is also very important for a lot of corporate usecases where the destination may not be 'local' to where envoy is running. Having CONNECT support on a per destination basis is really key in those situations where corporate requirements have forward proxies ( non-transparent ) in place to transit security boundaries.

[jcampbell05]

We have written a very small test proxy to implement this situation (http://github.com/aiwip/super-proxy) however we think it would be much better to use a well tested and used L7 proxy.

What is the best way to implement this ?

[mattklein123]

The plan for implementing this is going to be:

1. Finish the ongoing dynamic forward proxy support
2. Implement CONNECT in the forward proxy filter using upgrade semantics, similar to websocket.

[jespersoderlund]

Will this also support responding to a CONNECT from an application?

[mattklein123]

Will this also support responding to a CONNECT from an application?

What do you mean exactly?

[mattklein123]

@alyssawilk I started to look briefly at implementing this. I think roughly it will involve:

1. Making sure we allow CONNECT to propagate into the filter chain (probably via an HCM config setting to allow CONNECT)
2. We likely need to modify the route matching config to handle no path and CONNECT
3. Then we can have the dynamic_proxy_filter handle CONNECT and use the tcp_proxy code to initiate outbound connections.

I'm no longer thinking that we need to involve any of the upgrade config in this. WDYT? Any other thoughts about the right way to approach this from a config perspective?

[chradcliffe]

I've been experimenting with the terminating CONNECT support on master and noticed some unexpected behaviour. The 200 response from Envoy does not get sent until actual TCP data is sent from the upstream. I thought it was an issue with my test setup, but then I decided to actually read the docs and found the following in the section on HTTP upgrades:

On receipt of initial TCP data from upstream, the router will synthesize 200 response headers, and then forward the TCP data as the HTTP response body.

So it appears that this is working as intended. My issue is that I can't get a flow like this to work with the current support:

Client -> Envoy : HTTP CONNECT
Envoy <-> Upstream : TCP handshake
Envoy -> Client : HTTP 200 OK // this is never sent in the current implementation
Client -- (via Envoy) --> Upstream : HTTP GET

The following, however, does work in the current implementation:

Client -> Envoy : HTTP CONNECT with tunnelled HTTP GET request
Envoy <-> Upstream : TCP handshake
Envoy -> Upstream : tunnelled HTTP GET request
Upstream - Envoy -> : tunnelled HTTP response
Envoy -> Client : HTTP 200 OK
Envoy -> Client : tunnelled HTTP response

But the HTTP RFC warns against sending payload with a HTTP CONNECT request:

A payload within a CONNECT request message has no defined semantics; sending a payload body on a CONNECT request might cause some existing implementations to reject the request.

Python's http.client.HTTPConnection CONNECT support, for example, sends headers only on the request by default, so afaik the current CONNECT support won't be compatible with that client.

Is there a plan to implement synthesis of the 200 response after the upstream handshake but prior to seeing upstream bytes? Is there an implementation detail that makes this more difficult than the current behaviour?

[alyssawilk]

Oh interesting, so in this case you need the payload data to be sent from the client, in order to trigger the server to send a response which causes the 200 headers?
Yeah, I can fix that. Woo alpha testers! :-)

[chradcliffe]

So I think ideally the 200 response should be triggered by the upstream TCP handshake completion. The clients will typically wait on the response before sending any tunnelled payload.

[mattklein123]

Going to call this done. Let's track bugs / features in new issues. Thank you @alyssawilk!!!

[davidkarlsen]

I'm very interested in the 2nd usecase with Istio. Is this possible - and how do I configure that?

[alyssawilk]

cc @lambdai