x/net/http2/h2c: H2C handler uses incorrect headers to detect upgrade #45785

Gerg · 2021-04-26T19:27:54Z

What version of Go are you using (`go version`)?

$ go version
go version go1.16.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/pivotal/.cache/go-build"
GOENV="/home/pivotal/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/pivotal/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/pivotal/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.2"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build592242489=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Running a simple h2c server:

package main

import (
        "fmt"
        "net/http"

        "golang.org/x/net/http2"
        "golang.org/x/net/http2/h2c"
)

func main() {
        h2s := &http2.Server{}

        handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                fmt.Fprintf(w, "Hello, %v; HTTP Version: %v", r.URL.Path, r.Proto)
        })

        server := &http.Server{
                Addr:    "0.0.0.0:8080",
                Handler: h2c.NewHandler(handler, h2s),
        }

        err := server.ListenAndServe()
        if err != nil {
                panic(err)
        }
}

What did you expect to see?

Based on my reading of RFC 7540, Section 3.2, the required headers for an h2c upgrade are Upgrade: h2c and HTTP2-Settings:

The client does so by making an HTTP/1.1 request that includes an Upgrade header field with the "h2c" token. Such an HTTP/1.1 request MUST include exactly one HTTP2-Settings (Section 3.2.1) header field.

Curling with those headers, I would expect to receive a HTTP/1.1 101 Switching Protocols response.

What did you see instead?

The h2c handler does not accept the upgrade:

$ curl localhost:8080 -H "Upgrade: h2c" -H "HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA" -vvv
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: */*
> Upgrade: h2c
> HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Mon, 26 Apr 2021 19:09:10 GMT
< Content-Length: 32
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact
Hello, /; HTTP Version: HTTP/1.1

This is because the h2c package is not checking the appropriate headers:

// isH2CUpgrade returns true if the header properly request an upgrade to h2c
// as specified by Section 3.2.
func isH2CUpgrade(h http.Header) bool {
	return httpguts.HeaderValuesContainsToken(h[textproto.CanonicalMIMEHeaderKey("Upgrade")], "h2c") &&
		httpguts.HeaderValuesContainsToken(h[textproto.CanonicalMIMEHeaderKey("Connection")], "HTTP2-Settings")
}

Source: https://github.com/golang/net/blob/5f58ad60dda6b6eba34c424201d17c9fdc37953d/http2/h2c/h2c.go#L373-L378

While the Connection: HTTP2-Settings header does appear in the example in Section 3.2, I can't find anything in the spec that says it is required or that it should be used to detect h2c upgrades.

After adding the Connection: HTTP2-Settings header, the h2c upgrade succeeds:

$ curl localhost:8080 -H "Upgrade: h2c" -H "HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA" -H "Connection: HTTP2-Settings" -vvv
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: */*
> Upgrade: h2c
> HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA
> Connection: HTTP2-Settings
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 101 Switching Protocols
< Connection: Upgrade
< Upgrade: h2c

This is causing problems because we are running our H2C apps behind a httputil.ReverseProxy, which removes Connection headers, thereby blocking the h2c upgrade.

The text was updated successfully, but these errors were encountered:

cherrymui · 2021-04-26T20:43:33Z

cc @bradfitz @tombergan

Gerg · 2021-04-26T20:57:37Z

My colleague, @ameowlia, pointed out that Section 3.2.1 of the spec does say that Connection: HTTP2-Settings should be sent by the client:

Since the upgrade is only intended to apply to the immediate connection, a client sending the HTTP2-Settings header field MUST also send "HTTP2-Settings" as a connection option in the Connection header field to prevent it from being forwarded (see Section 6.1 of [RFC7230]).

Still, this feels like an odd choice for detecting h2c upgrades, especially in lieu of checking for the HTTP2-Settings header itself. Furthermore, this check isn't compatible with Golang's ReverseProxy implementation, as I described above.

For comparison, I testing the same request against a simple Python server that supports h2c:

#!/usr/bin/env python3

from quart import Quart, request

app = Quart(__name__)


@app.route("/")
async def hello():
    return f"Hello, {request.full_path}; HTTP Version: {request.http_version}."


if __name__ == "__main__":
    app.run(port=8081)

It successfully upgraded without the Connection: HTTP2-Settings header:

$ curl localhost:8081 -H "Upgrade: h2c" -H "HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA" -vvv
*   Trying 127.0.0.1:8081...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8081 (#0)
> GET / HTTP/1.1
> Host: localhost:8081
> User-Agent: curl/7.68.0
> Accept: */*
> Upgrade: h2c
> HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 101
< date: Mon, 26 Apr 2021 20:54:51 GMT
< server: hypercorn-h11
< connection: upgrade
< upgrade: h2c

- Golang h2c package doesn't play well with reverse proxy due to golang/go#45785

networkimprov · 2021-05-16T06:39:28Z

cc @fraenkel

fraenkel · 2021-05-16T14:18:01Z

I don't believe this is an issue given that this is following the spec as identified above.
From https://datatracker.ietf.org/doc/html/rfc7540#section-3.2.1

A request that upgrades from HTTP/1.1 to HTTP/2 MUST include exactly
one "HTTP2-Settings" header field.

 a client sending the HTTP2-Settings header field MUST
   also send "HTTP2-Settings" as a connection option in the Connection
   header field

Gerg · 2021-05-16T16:47:33Z

Based on that, should httputil.ReverseProxy preserve the Connection: HTTP2-Settings header? There is already precedent for preserving headers needed for upgrades:

go/src/net/http/httputil/reverseproxy.go

Lines 272 to 277 in ce92a20

    
           // After stripping all the hop-by-hop connection headers above, add back any 
        
           // necessary for protocol upgrades, such as for websockets. 
        
           if reqUpType != "" { 
        
           	outreq.Header.Set("Connection", "Upgrade") 
        
           	outreq.Header.Set("Upgrade", reqUpType) 
        
           }

Otherwise, it seems impossible to implement the H2C upgrade flow spanning a ReverseProxy.

fraenkel · 2021-05-16T17:13:16Z

See https://blog.assetnote.io/2021/03/18/h2c-smuggling/

Gerg · 2021-05-17T17:18:46Z

I see why it makes sense to strip HTTP2-Settings from the Connection header as a default.

Would you be open to making the preserved headers configurable on ReverseProxy? In our case, we have a trusted client and also don't enforce any access controls via our reverse proxy, so I don't think h2c smuggling is a concern. If not, I suppose we can always shim the ReverseProxy's Transport to add the headers back.

seankhliao · 2022-08-01T21:08:37Z

Closing as this appears to be working as intended.
See #53002 for the new design for ReverseProxy that can handle hop-by-hop headers

gopherbot added this to the Unreleased milestone Apr 26, 2021

cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 26, 2021

Gerg mentioned this issue Apr 26, 2021

Intent to support HTTP/2 cloudfoundry/routing-release#200

Closed

Gerg added a commit to Gerg/envoy_http2_tls_no_alpn that referenced this issue Apr 27, 2021

Add python h2c app

e6fca26

- Golang h2c package doesn't play well with reverse proxy due to golang/go#45785

Gerg mentioned this issue May 18, 2021

proposal: x/net/http2: Add support for client-side H2C upgrade flow #46249

Open

seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Aug 1, 2022

golang locked and limited conversation to collaborators Aug 1, 2023

gopherbot added the FrozenDueToAge label Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/net/http2/h2c: H2C handler uses incorrect headers to detect upgrade #45785

x/net/http2/h2c: H2C handler uses incorrect headers to detect upgrade #45785

Gerg commented Apr 26, 2021

cherrymui commented Apr 26, 2021

Gerg commented Apr 26, 2021

networkimprov commented May 16, 2021

fraenkel commented May 16, 2021

Gerg commented May 16, 2021

fraenkel commented May 16, 2021

Gerg commented May 17, 2021

seankhliao commented Aug 1, 2022

x/net/http2/h2c: H2C handler uses incorrect headers to detect upgrade #45785

x/net/http2/h2c: H2C handler uses incorrect headers to detect upgrade #45785

Comments

Gerg commented Apr 26, 2021

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

cherrymui commented Apr 26, 2021

Gerg commented Apr 26, 2021

networkimprov commented May 16, 2021

fraenkel commented May 16, 2021

Gerg commented May 16, 2021

fraenkel commented May 16, 2021

Gerg commented May 17, 2021

seankhliao commented Aug 1, 2022

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?