Support downstream h2c upgrade in the H2 codec implementation

[gnz00]

Description There is currently a gap in support for H2 connections without ALPN. Envoy currently supports prior knowledge H2 connections, however, does not support the h2c upgrade header which is the default implementation of some clients when TLS is not provided and prior knowledge is not explicitly specified.

Relevant Links RFC-7540: h2c upgrade

Changes To support the default codec_type value of auto the protocol detection will need to detect if the request is an H1 upgrade and send the request to the H2 path here: https://github.com/envoyproxy/envoy/blob/master/source/common/http/conn_manager_utility.cc#L44

I believe nghttp2 should handle the h2c upgrade, however, Envoy currently throws this exception: https://github.com/envoyproxy/envoy/blob/master/source/common/http/http2/codec_impl.cc#L665

[debug][http2] [source/common/http/http2/codec_impl.cc:735] [C65066] setting stream-level initial window size to 268435456
[debug][http2] [source/common/http/http2/codec_impl.cc:757] [C65066] updating connection-level initial window size to 268435456
[trace][http2] [source/common/http/http2/codec_impl.cc:353] [C65066] dispatching 182 bytes
[debug][http] [source/common/http/conn_manager_impl.cc:278] [C65066] dispatch error: Received bad client magic byte string

Prior to invoking dispatch the codec implementation should detect an H1 upgrade request and send the response of 101 Switching Protocols before buffering the frames to nghttp2. I assume there may be some low level API for nghttp2 to handle this scenario, or there may be some munging of the request to get nghttp2 to properly handle it, e.g. presenting the "magic byte".

@mattklein123 This is the implementation discussion that originated in #6972

[gnz00]

Looks like it might be as simple as invoking nghttp2_session_upgrade2 at the connection creation here: https://github.com/envoyproxy/envoy/blob/master/source/common/http/http2/codec_impl.cc#L895

[mattklein123]

Implementing this on the downstream side is definitely possible but not trivial. Roughly I think the flow would look like: 1) Use auto codec, which would detect HTTP/1.1 for an h2c upgrade 2) Look for h2c upgrade in HTTP/1.1 headers. 3) Recreate codec as an h2 codec, probably passing in some type of upgrade factory which includes teh upgrade settings. 4) Go from there.

This will definitely require working out some details WRT to codec recreation. cc @alyssawilk

[gnz00]

@mattklein123 I think enhancing the H2 codec to handle the upgrade should be preferred, as even if someone opts to use codec_type: HTTP2 this currently throws an exception. It would also avoid having to recreate the codec. However, I believe you chose this approach because it is not possible to detect the H1 headers during the codec creation?

[mattklein123]

Correct. The h2 codec does not understand how to parse h1.

[gnz00]

Ideally, we would want to solve this for both auto and http2 codec_type configurations. It isn’t clear to me if the solution you outlined solves this for both. As it is part of the H2 spec, should the H2 codec have limited support for the H1 headers?

[mattklein123]

As it is part of the H2 spec, should the H2 codec have limited support for the H1 headers?

It could, but I think it would complicate things quite a bit. Both approaches are valid, but given that AUTO would still have to support upgrade via the h1 codec, I think it's better to support at the HCM layer, and I would just say that AUTO is required for h2c. Or we could have a different type AUTO_H2_H2C_ONLY or something like that.

[gnz00]

In the approach I was referring to, I imagined that AUTO would also detect the headers and send to the H2 path. As far as implementation details in the H2 codec, I am wondering if we might just be able to use nghttp2_session_upgrade2 as a replacement for nghttp2_submit_settings and then swallow any errors. This could be behind some kind of configuration if it has any added overhead (although looking at the nghttp2 code it looks relatively minimal).

I'll defer to y'all as I am not fully aware of the complexities of the protocol detection and codec implementations.

In pursuit of the approach you outlined, does the upgrade_config provide a mechanism for implementing the codec recreation? I haven't fully walked that code but end users could opt-in to upgrades via a route configuration:

{
  "upgrade_type": "h2c",
  "enabled": "true"
}

Although, now I'm wondering, if the upgrade is only implemented in the H1 codec, what is the expected behavior when a user specifies codec_type=HTTP1?

[mattklein123]

I think "upgrade" (web socket, CONNECT, etc.) is orthogonal to downstream h2c. I think the approach that I laid out is the way to go, otherwise you are going to have to bake an HTTP/1.1 codec into the HTTP/2 codec, and I have no idea how that would work for AUTO anyway.

[alyssawilk]

Yeah, if you're looking for a sanity check, Matt's spot on with the capabilities of the h2 codec, and how we'd want to handle upgrades (parsing as H1, detaching from the H1 codec and switching to the H2). It's a bunch of non-trivial work. I don't think getting nghttp2 to handle http/1 is plausible, and I think any sort of peek-and-decide-on-codec logic is going to to be too hacky.

[pitiwari]

@mattklein123 @alyssawilk i am also hitting similar issue so wanted to see if issue which i am hitting is same. When we send curl with --http2 and https in a request then it works but if we send curl with --http2 with http in a request we get 403

/usr/local/opt/curl-openssl/bin/curl --http2 -s -i -k -m 4 -X GET http://dummyip/piyush.txt -I HTTP/1.1 403 Forbidden date: Tue, 13 Aug 2019 22:08:28 GMT server: envoy connection: close content-length: 0

/usr/local/opt/curl-openssl/bin/curl --http2 -s -i -k -m 4 -X GET https://dummyip/piyush2.txt -I HTTP/1.1 302 Found

[rdsubhas]

This issue is a huge deal breaker for anyone adopting envoy or istio in a Java based organization. It's not only a curl issue.

Java 9 and above, and Spring 1.5 and above – supports HTTP/2. So with every outgoing request, they send h2c upgrade headers.

Both sender (Java URLConnection) and receiver (spring, tomcat) properly conform to https://httpwg.org/specs/rfc7540.html#discover-http. There is nothing special here. The server simply talks HTTP/1.1 if HTTP/2 is not supported.

When we add envoy/istio in our infrasructure, entire Java ecosystem is broken. All Java 9+ and Spring 1.5+ (which is basically 99% of Java, minus 1 or 2 Java 8 services) are broken. And there is no way to disable those Upgrade headers globally on JDK level. They are sent as indicative headers.

We tried to add an envoy filter to remove these two Upgrade and Connection headers, but envoy sends back 403 even before the request is sent to upstream, hence request filters also don't work.

Given the abrupt total breakage of an entire ecosystem when two headers (Upgrade and Connection) which are optional in the http2 spec are present – we hope this is classified as a major issue. Envoy should really ignore these headers if it doesn't support h2c to honor the spec.

[alyssawilk]

Is your desire here to have the upgrade ignored, and handled as HTTP/1? If so you can configure Envoy to accept this type of upgrade: https://www.envoyproxy.io/docs/envoy/v1.8.0/intro/arch_overview/websocket (set the upgrade_tye to h2c), Envoy will pass the requests to the filter chain, and you can strip the actual upgrade headers before they are sent upstream as you're currently trying to do.

[petedmarsh]

Hi! I may have some time to implement this, but I would need some help wrt this part:

Recreate codec as an h2 codec, probably passing in some type of upgrade factory which includes teh upgrade settings.

If you could point me to some docs or similar parts of the code I can take a look, thanks!