From fb3a3362a4a0a63bec94776b29ad6d3dd6d6a39c Mon Sep 17 00:00:00 2001
From: Michael Wilson <mike@mdwn.dev>
Date: Tue, 1 Nov 2022 11:38:50 -0400
Subject: [PATCH] Proritize HTTP/1.1 over HTTP/2. (#17886)

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 https://github.com/golang/go/issues/49918
is implemented, we may be able to revert this if enabling HTTP/2 websockets
fixes the issue on our end.
---
 lib/service/service.go                | 14 +++++++++++---
 lib/service/service_test.go           |  8 ++++----
 lib/srv/alpnproxy/common/protocols.go |  9 +++++++--
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/lib/service/service.go b/lib/service/service.go
index e5b47a98eb87c..5f33119d553e0 100644
--- a/lib/service/service.go
+++ b/lib/service/service.go
@@ -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"
@@ -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 {
diff --git a/lib/service/service_test.go b/lib/service/service_test.go
index e6dd3a84a6dd6..d94b65b32bee5 100644
--- a/lib/service/service_test.go
+++ b/lib/service/service_test.go
@@ -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",
@@ -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@",
diff --git a/lib/srv/alpnproxy/common/protocols.go b/lib/srv/alpnproxy/common/protocols.go
index e928a7df20bcc..5beab228030c1 100644
--- a/lib/srv/alpnproxy/common/protocols.go
+++ b/lib/srv/alpnproxy/common/protocols.go
@@ -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"
 
@@ -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,