Repository navigation
crypto/tls: enforcing ALPN breaks HTTP/1.1 fallbacks on misconfigured servers #46310
Description
In 90d6bbb we fixed crypto/tls to require a successful ALPN negotiation if both sides support it. That is, if a client sends an ALPN extension, and the server has configured NextProtos, we reject the connection if there is no overlap between client and server protocols.
This is what the spec requires, and has a security benefit, because it protects against cross-protocol attacks.
However, it's also kind of surprising, because if a client doesn't support ALPN it will never have its connection rejected.
Moreover, we used not to enforce this, so there is a risk that servers configured a partial NextProtos list, expecting other protocols to just fallback to not negotiating ALPN.
In particular, we have anecdotal evidence of servers with NextProtos: []string{"h2"} that expect a client that tries to negotiate http/1.1 to fallback successfully. These servers will break in Go 1.17.
I searched all code on the Modules Mirror for NextProtos uses, and it looks like this is the only case in which a fallback seems to be expected.
Details
1 "bep/1.0"
1 "dilithium"
1 "gemini"
1 "go-multicast-quic"
1 "h2", "h2c", "h2i"
1 "h2", "hq"
1 "h2", "http/1.1", "http/1.0"
1 "h2", "http/1.1", acme.ALPNProto
1 "h3-29", "h3-32"
1 "h3r", "h3-29"
1 "http"
1 "http", "something-else"
1 "http/1.0"
1 "http/1.1", "http/1.0", acme.ALPNProto
1 "http/1.1", ACMETLS1Protocol
1 "http/1.1", http2.NextProtoTLS, "h2-14"
1 "http/2", "http/1.1"
1 "http3-reflector-server"
1 "https"
1 "hybs-rtv1"
1 "istio", "h2"
1 "ldap"
1 "nextProtos"
1 "ntske/1"
1 "pop3"
1 "quic-matrix-ygg"
1 "quic-vpc-sbs"
1 "qush"
1 "rtmp-over-quic", "wq-vvv-01"
1 "securelink"
1 "smtp"
1 "spdy/3"
1 "stream"
1 "stream;level=2;pp=1"
1 "x-amzn-mqtt-ca"
1 NextProtoTLS, "h2"
1 ProtoSSH
1 ProtocolName
1 SocketProxyNextProto
1 connector.ProtocolName, authproxy.SocketProxyNextProto, http2.NextProtoTLS
1 firstMatchProto
1 getALPN(ietfQUICDraft24VersionNumber)
1 getALPN(versionNumber)
1 http2WithTLSVersionID
1 proto
1 protoName
1 quicconn.ProtoSSH
1 t.protocolName
2 "adc", "nmdc"
2 "bw.mux"
2 "emqx-wormhole"
2 "foo/bar"
2 "go-p2p"
2 "h2", "acme-tls/1"
2 "h2;rate=30", "spdy/3.1;rate=50", "http/1.1"
2 "http", "ftp"
2 "http/1.1", "http/1.0"
2 "http2"
2 "imap"
2 "irc"
2 "mqtt"
2 "netceptor"
2 "orbit-simple-example"
2 "qperf"
2 "quic-git"
2 "quic-hello-example"
2 "quic-stunnel"
2 "spdy/3.1", "http/1.1"
2 "spdy/3.1;rate=50", "http/1.1"
2 ACMETLS1Protocol
2 common.KQuicProxy
2 http2.NextProtoTLS, "h2-14"
2 nProto
2 param.Args.Password,"quic-echo-example"
2 protocol
2 protocol.ProtocolName
2 tlsProtocolName
3 "h1"
3 "quick"
3 "quicssh"
3 acme.ACMETLS1Protocol
3 http2.NextProtoTLS, "http/1.1"
3 nextProto
4 "acme-tls/1"
4 "h2", "http/1.1", tlsalpn01.ACMETLS1Protocol
4 "hq-29"
4 "mixin-quic-peer"
4 "pion-quic"
4 KQuicProxy
5 "SCION"
5 "http/1.1", "h2", "h3"
5 "relay"
5 acmez.ACMETLS1Protocol
6 "ftp"
8 "http2", "http/1.1"
12 "benchmark"
12 "crypto-setup"
12 "proto foo", "proto bar"
12 "proto1"
12 httpVersionToALPN[httpProxyVer]
15 httpVersionToAlpn[httpProxyVer]
16 "unhandled-proto"
18 "dns"
18 "protocol1"
18 "unhandled-proto", "tls-0.9"
25 NextProtoTLS, "foo", "bar"
27 "h3-29"
29 http2.NextProtoTLS
31 alpnProtocol
32 acme.ALPNProto
34 "http/1.1", acme.ALPNProto
35 acmeALPNProto
37 "quic-echo-example"
39 "http/1.1", "h2"
50 "foo", "bar", NextProtoTLS
57 "foo", "bar"
63 "foo"
66 alpn
75 NextProtoTLS
193 "http/1.1"
271 "h2"
353 "h2", "http/1.1"
Some of them are from gRPC, which as far as I know legitimately requires HTTP/2, so for those all is well.
Some though look like they might unintentionally break. We should figure out how common this is, weight it against the security benefit, and maybe proactively reach out.
- https://github.com/kubernetes-sigs/controller-runtime/blob/485a24a305332f2e9fbb3de8c26c7232303f7938/pkg/webhook/server.go#L201
- https://github.com/google/fleetspeak/blob/93b2b9a40808306722875abbd5434af4634c6531/fleetspeak/src/server/https/https.go#L127
- https://github.com/etcd-io/etcd/blob/ddc4f473c98d07c6a812b4d0f844e19c7216baf2/client/pkg/transport/listener.go#L497-L498
We should also make sure the HTTP/2 docs in Go have correct examples that include http/1.1.
/cc @golang/security
Activity
FiloSottile commentedon May 21, 2021
It looks like even x/net/http2 is doing this wrong
https://github.com/golang/net/blob/37e1c6afe02340126705deced573a85ab75209d7/http2/server.go#L262-L271
so we might need to just hardcode an exception such that if
h2is configured we keep ignoring the ALPN extension as long as it includeshttp/1.1, assuming a fallback is expected.gopherbot commentedon Jun 7, 2021
Change https://golang.org/cl/325611 mentions this issue:
http2: also set "http/1.1" ALPN in ConfigureServergopherbot commentedon Jun 7, 2021
Change https://golang.org/cl/325432 mentions this issue:
crypto/tls: let HTTP/1.1 clients connect to servers with NextProtos "h2"http2: also set "http/1.1" ALPN in ConfigureServer
howardjohn commentedon Oct 1, 2021
This broke us and has been preventing us from upgrade http://github.com/istio/istio to golang 1.17.
Our expectation is that we can have a server that will serve any explicitly configured ALPNs with that config (ie
h2will serve HTTP 2), and anything else will fall back to http 1.1. This was the behavior on all former go versions.dc00dc6 was an insufficient fix for us; we do not have
NextProtos: h2and want to allowhttp/1.1, we want to support ANY alpn.A few alternatives we have tried:
NextProtos. This breaks h2 negotiation.SupportedProtosinGetConfigForClient. This works on the serverside but breaks go clients (and possibly others) as the NegotiatedProtocol is not correntNextProtos(not really possible, since ideally we want all of them, but we can live with this), and then setupTLSNextProtosto serve http/1.1 for all those ALPNs. This seems to require completely reimplementing the HTTP/1.1 serving code which is highly undesirable.Is there any viable workarounds? Right now we are stuck.
http2: also set "http/1.1" ALPN in ConfigureServer
crypto/tls: let HTTP/1.1 clients connect to servers with NextProtos "h2"