From c610bd34e3c5cefac541be99885d923281628cce Mon Sep 17 00:00:00 2001
From: Michael Wilson <mike@mdwn.dev>
Date: Tue, 1 Nov 2022 16:37:08 -0400
Subject: [PATCH] Prioritize HTTP/1.1 over HTTP/2. (#18005)

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           | 10 +++++-----
 lib/srv/alpnproxy/common/protocols.go |  8 +++++++-
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/lib/service/service.go b/lib/service/service.go
index 32d28a31ef91..7be2c01bc6fd 100644
--- a/lib/service/service.go
+++ b/lib/service/service.go
@@ -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"
@@ -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
diff --git a/lib/service/service_test.go b/lib/service/service_test.go
index 5a542e97acec..47da99438235 100644
--- a/lib/service/service_test.go
+++ b/lib/service/service_test.go
@@ -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,
@@ -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",
@@ -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",
diff --git a/lib/srv/alpnproxy/common/protocols.go b/lib/srv/alpnproxy/common/protocols.go
index ba34655299dc..466ae6e09928 100644
--- a/lib/srv/alpnproxy/common/protocols.go
+++ b/lib/srv/alpnproxy/common/protocols.go
@@ -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,