Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v9] Prioritize HTTP/1.1 over HTTP/2. #18005

Merged
merged 1 commit into from
Nov 1, 2022
Merged

[v9] Prioritize HTTP/1.1 over HTTP/2. #18005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions lib/service/service.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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