Skip to content

Commit

Permalink
Prioritize 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
mdwn authored and Mike Wilson committed Nov 1, 2022
1 parent 4223704 commit 5acce2d
Show file tree
Hide file tree
Showing 3 changed files with 23 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 @@ -86,6 +86,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 @@ -3551,10 +3552,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))
}

// Go 1.17 introduced strict ALPN https://golang.org/doc/go1.17#ALPN If a client protocol is not recognized
Expand Down
10 changes: 5 additions & 5 deletions lib/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
Expand Down Expand Up @@ -487,9 +487,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",
"teleport-mysql",
Expand All @@ -505,9 +505,9 @@ func TestSetupProxyTLSConfig(t *testing.T) {
name: "ACME disabled",
acmeEnabled: false,
wantNextProtos: []string{
// Ensure h2 has precedence over http/1.1.
"h2",
// Ensure http/1.1 has precedence over http2.
"http/1.1",
"h2",
"teleport-postgres",
"teleport-mysql",
"teleport-mongodb",
Expand Down
8 changes: 7 additions & 1 deletion lib/srv/alpnproxy/common/protocols.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,14 @@ const (

// SupportedProtocols is the list of supported ALPN protocols.
var SupportedProtocols = []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,
ProtocolPostgres,
ProtocolMySQL,
ProtocolMongoDB,
Expand Down

0 comments on commit 5acce2d

Please sign in to comment.