Skip to content

Commit

Permalink
Proritize HTTP/1.1 over HTTP/2.
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
Mike Wilson authored and mdwn committed Nov 1, 2022
1 parent 537c7e6 commit 15317ae
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
14 changes: 12 additions & 2 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,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 @@ -4108,10 +4109,19 @@ 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))
tlsConfig.NextProtos = apiutils.Deduplicate(tlsConfig.NextProtos)
}

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 @@ -526,9 +526,9 @@ func TestSetupProxyTLSConfig(t *testing.T) {
"teleport-sqlserver-ping",
"teleport-snowflake-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 @@ -62,7 +62,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 @@ -95,8 +94,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 to that fix will work.
ProtocolHTTP,
ProtocolHTTP2,
ProtocolProxySSH,
ProtocolReverseTunnel,
ProtocolAuth,
Expand Down

0 comments on commit 15317ae

Please sign in to comment.