Skip to content

Commit

Permalink
Proritize HTTP/1.1 over HTTP/2. (#17886)
Browse files Browse the repository at this point in the history
Due to a bug in Chrome, secure websockets in Teleport work intermittently on
the Chrome browser when using HTTP/2. Prioritizing HTTP/1.1 fixes this issue,
though the reasons for this are not 100% clear. When or if
https://bugs.chromium.org/p/chromium/issues/detail?id=1379017 is resolved, we
should be able to revert this. When golang/go#49918
is implemented, we may be able to revert this if enabling HTTP/2 websockets
fixes the issue on our end.
  • Loading branch information
mdwn authored Nov 1, 2022
1 parent 0673d9e commit fb3a336
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 9 deletions.
14 changes: 11 additions & 3 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ import (
"github.com/gravitational/teleport/lib/srv"
"github.com/gravitational/teleport/lib/srv/alpnproxy"
alpnproxyauth "github.com/gravitational/teleport/lib/srv/alpnproxy/auth"
"github.com/gravitational/teleport/lib/srv/alpnproxy/common"
alpncommon "github.com/gravitational/teleport/lib/srv/alpnproxy/common"
"github.com/gravitational/teleport/lib/srv/app"
"github.com/gravitational/teleport/lib/srv/db"
Expand Down Expand Up @@ -4082,10 +4083,17 @@ func (process *TeleportProcess) setupProxyTLSConfig(conn *Connector, tsrv revers
if acmeCfg.URI != "" {
m.Client = &acme.Client{DirectoryURL: acmeCfg.URI}
}
tlsConfig = m.TLSConfig()
// We have to duplicate the behavior of `m.TLSConfig()` here because
// http/1.1 needs to take precedence over h2 due to
// https://bugs.chromium.org/p/chromium/issues/detail?id=1379017#c5 in Chrome.
tlsConfig = &tls.Config{
GetCertificate: m.GetCertificate,
NextProtos: []string{
string(common.ProtocolHTTP), string(common.ProtocolHTTP2), // enable HTTP/2
acme.ALPNProto, // enable tls-alpn ACME challenges
},
}
utils.SetupTLSConfig(tlsConfig, cfg.CipherSuites)

tlsConfig.NextProtos = apiutils.Deduplicate(append(tlsConfig.NextProtos, acme.ALPNProto))
}

for _, pair := range process.Config.Proxy.KeyPairs {
Expand Down
8 changes: 4 additions & 4 deletions lib/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,9 @@ func TestSetupProxyTLSConfig(t *testing.T) {
name: "ACME enabled, teleport ALPN protocols should be appended",
acmeEnabled: true,
wantNextProtos: []string{
// Ensure h2 has precedence over http/1.1.
"h2",
// Ensure http/1.1 has precedence over http2.
"http/1.1",
"h2",
"acme-tls/1",
"teleport-postgres-ping",
"teleport-mysql-ping",
Expand Down Expand Up @@ -529,9 +529,9 @@ func TestSetupProxyTLSConfig(t *testing.T) {
"teleport-snowflake-ping",
"teleport-cassandra-ping",
"teleport-elasticsearch-ping",
// Ensure h2 has precedence over http/1.1.
"h2",
// Ensure http/1.1 has precedence over http2.
"http/1.1",
"h2",
"teleport-proxy-ssh",
"teleport-reversetunnel",
"teleport-auth@",
Expand Down
9 changes: 7 additions & 2 deletions lib/srv/alpnproxy/common/protocols.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ const (
// be included in the list of ALPN header for the proxy server to handle the connection properly.
ProtocolReverseTunnelV2 Protocol = "teleport-reversetunnelv2"

// ProtocolHTTP is TLS ALPN protocol value used to indicate HTTP2 protocol
// ProtocolHTTP is TLS ALPN protocol value used to indicate HTTP 1.1 protocol
ProtocolHTTP Protocol = "http/1.1"

Expand Down Expand Up @@ -98,8 +97,14 @@ const (
var SupportedProtocols = append(
ProtocolsWithPing(ProtocolsWithPingSupport...),
append([]Protocol{
ProtocolHTTP2,
// HTTP needs to be prioritized over HTTP2 due to a bug in Chrome:
// https://bugs.chromium.org/p/chromium/issues/detail?id=1379017
// If Chrome resolves this, we can switch the prioritization. We may
// also be able to get around this if https://github.com/golang/go/issues/49918
// is implemented and we can enable HTTP2 websockets on our end, but
// it's less clear this will actually fix the issue.
ProtocolHTTP,
ProtocolHTTP2,
ProtocolProxySSH,
ProtocolReverseTunnel,
ProtocolAuth,
Expand Down

0 comments on commit fb3a336

Please sign in to comment.