From 43d691afc9420b9d5893ba13eeb3d992d324f0b5 Mon Sep 17 00:00:00 2001
From: David Boslee <david@goteleport.com>
Date: Wed, 18 May 2022 10:25:23 -0600
Subject: [PATCH] Ensure h2 has precedence over http/1.1

This fixes an issue where if ACME is disabled the next protos in the tls
config is set to nil. This leads to protocol negotiation not taking
place and causes some clients to fallback to http/1.1.
---
 lib/service/service.go                | 10 +++++-----
 lib/service/service_test.go           | 16 ++++++++++++++--
 lib/srv/alpnproxy/common/protocols.go |  6 ++----
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/lib/service/service.go b/lib/service/service.go
index fca3ea37c42e3..6fc1eaa4709ec 100644
--- a/lib/service/service.go
+++ b/lib/service/service.go
@@ -3417,13 +3417,13 @@ func (process *TeleportProcess) setupProxyTLSConfig(conn *Connector, tsrv revers
 		tlsConfig = m.TLSConfig()
 		utils.SetupTLSConfig(tlsConfig, cfg.CipherSuites)
 
-		// If ACME protocol was enabled add all known TLS Routing protocols.
-		// Go 1.17 introduced strict ALPN https://golang.org/doc/go1.17#ALPN If a client protocol is not recognized
-		// the TLS handshake will fail.
-		supportedProtocols := alpncommon.ProtocolsToString(alpncommon.SupportedProtocols)
-		tlsConfig.NextProtos = apiutils.Deduplicate(append(tlsConfig.NextProtos, supportedProtocols...))
+		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
+	// the TLS handshake will fail.
+	tlsConfig.NextProtos = apiutils.Deduplicate(append(tlsConfig.NextProtos, alpncommon.ProtocolsToString(alpncommon.SupportedProtocols)...))
+
 	for _, pair := range process.Config.Proxy.KeyPairs {
 		process.Config.Log.Infof("Loading TLS certificate %v and key %v.", pair.Certificate, pair.PrivateKey)
 
diff --git a/lib/service/service_test.go b/lib/service/service_test.go
index 5702ede20c149..fc1e4ab76f77d 100644
--- a/lib/service/service_test.go
+++ b/lib/service/service_test.go
@@ -487,6 +487,7 @@ 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",
 				"http/1.1",
 				"acme-tls/1",
@@ -503,8 +504,19 @@ func TestSetupProxyTLSConfig(t *testing.T) {
 		{
 			name:        "ACME disabled",
 			acmeEnabled: false,
-			// If server NextProtos list is empty server allows for connection with any protocol.
-			wantNextProtos: nil,
+			wantNextProtos: []string{
+				// Ensure h2 has precedence over http/1.1.
+				"h2",
+				"http/1.1",
+				"teleport-postgres",
+				"teleport-mysql",
+				"teleport-mongodb",
+				"teleport-redis",
+				"teleport-sqlserver",
+				"teleport-proxy-ssh",
+				"teleport-reversetunnel",
+				"teleport-auth@",
+			},
 		},
 	}
 
diff --git a/lib/srv/alpnproxy/common/protocols.go b/lib/srv/alpnproxy/common/protocols.go
index 9f2c579b28652..ba34655299dc2 100644
--- a/lib/srv/alpnproxy/common/protocols.go
+++ b/lib/srv/alpnproxy/common/protocols.go
@@ -18,7 +18,6 @@ package common
 
 import (
 	"github.com/gravitational/trace"
-	"golang.org/x/crypto/acme"
 
 	"github.com/gravitational/teleport/lib/defaults"
 )
@@ -71,7 +70,8 @@ const (
 
 // SupportedProtocols is the list of supported ALPN protocols.
 var SupportedProtocols = []Protocol{
-	acme.ALPNProto,
+	ProtocolHTTP2,
+	ProtocolHTTP,
 	ProtocolPostgres,
 	ProtocolMySQL,
 	ProtocolMongoDB,
@@ -79,8 +79,6 @@ var SupportedProtocols = []Protocol{
 	ProtocolSQLServer,
 	ProtocolProxySSH,
 	ProtocolReverseTunnel,
-	ProtocolHTTP,
-	ProtocolHTTP2,
 	ProtocolAuth,
 }