disable http/2 for websockets (#2399)

config/envoyconfig/clusters.go

@@ -40,15 +40,15 @@ func (b *Builder) BuildClusters(ctx context.Context, cfg *config.Config) ([]*env
 		return nil, err
 	}
 
-	controlGRPC, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-control-plane-grpc", []*url.URL{grpcURL}, true)
+	controlGRPC, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-control-plane-grpc", []*url.URL{grpcURL}, upstreamProtocolHTTP2)
 	if err != nil {
 		return nil, err
 	}
-	controlHTTP, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-control-plane-http", []*url.URL{httpURL}, false)
+	controlHTTP, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-control-plane-http", []*url.URL{httpURL}, upstreamProtocolAuto)
 	if err != nil {
 		return nil, err
 	}
-	authZ, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-authorize", authzURLs, true)
+	authZ, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-authorize", authzURLs, upstreamProtocolHTTP2)
 	if err != nil {
 		return nil, err
 	}
@@ -99,7 +99,7 @@ func (b *Builder) buildInternalCluster(
 	options *config.Options,
 	name string,
 	dsts []*url.URL,
-	forceHTTP2 bool,
+	upstreamProtocol upstreamProtocolConfig,
 ) (*envoy_config_cluster_v3.Cluster, error) {
 	cluster := newDefaultEnvoyClusterConfig()
 	cluster.DnsLookupFamily = config.GetEnvoyDNSLookupFamily(options.DNSLookupFamily)
@@ -111,7 +111,7 @@ func (b *Builder) buildInternalCluster(
 		}
 		endpoints = append(endpoints, NewEndpoint(dst, ts, 1))
 	}
-	if err := b.buildCluster(cluster, name, endpoints, forceHTTP2); err != nil {
+	if err := b.buildCluster(cluster, name, endpoints, upstreamProtocol); err != nil {
 		return nil, err
 	}
 
@@ -124,8 +124,14 @@ func (b *Builder) buildPolicyCluster(ctx context.Context, options *config.Option
 
 	cluster.AltStatName = getClusterStatsName(policy)
 
+	upstreamProtocol := upstreamProtocolAuto
+	if policy.AllowWebsockets {
+		// #2388, force http/1 when using web sockets
+		upstreamProtocol = upstreamProtocolHTTP1
+	}
+
 	name := getClusterID(policy)
-	endpoints, err := b.buildPolicyEndpoints(ctx, options, policy)
+	endpoints, err := b.buildPolicyEndpoints(ctx, options, policy, upstreamProtocol)
 	if err != nil {
 		return nil, err
 	}
@@ -138,17 +144,22 @@ func (b *Builder) buildPolicyCluster(ctx context.Context, options *config.Option
 		cluster.DnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
 	}
 
-	if err := b.buildCluster(cluster, name, endpoints, false); err != nil {
+	if err := b.buildCluster(cluster, name, endpoints, upstreamProtocol); err != nil {
 		return nil, err
 	}
 
 	return cluster, nil
 }
 
-func (b *Builder) buildPolicyEndpoints(ctx context.Context, options *config.Options, policy *config.Policy) ([]Endpoint, error) {
+func (b *Builder) buildPolicyEndpoints(
+	ctx context.Context,
+	options *config.Options,
+	policy *config.Policy,
+	upstreamProtocol upstreamProtocolConfig,
+) ([]Endpoint, error) {
 	var endpoints []Endpoint
 	for _, dst := range policy.To {
-		ts, err := b.buildPolicyTransportSocket(ctx, options, policy, dst.URL)
+		ts, err := b.buildPolicyTransportSocket(ctx, options, policy, dst.URL, upstreamProtocol)
 		if err != nil {
 			return nil, err
 		}
@@ -157,7 +168,11 @@ func (b *Builder) buildPolicyEndpoints(ctx context.Context, options *config.Opti
 	return endpoints, nil
 }
 
-func (b *Builder) buildInternalTransportSocket(ctx context.Context, options *config.Options, endpoint *url.URL) (*envoy_config_core_v3.TransportSocket, error) {
+func (b *Builder) buildInternalTransportSocket(
+	ctx context.Context,
+	options *config.Options,
+	endpoint *url.URL,
+) (*envoy_config_core_v3.TransportSocket, error) {
 	if endpoint.Scheme != "https" {
 		return nil, nil
 	}
@@ -201,6 +216,7 @@ func (b *Builder) buildPolicyTransportSocket(
 	options *config.Options,
 	policy *config.Policy,
 	dst url.URL,
+	upstreamProtocol upstreamProtocolConfig,
 ) (*envoy_config_core_v3.TransportSocket, error) {
 	if dst.Scheme != "https" {
 		return nil, nil
@@ -211,6 +227,16 @@ func (b *Builder) buildPolicyTransportSocket(
 		return nil, err
 	}
 
+	var alpn []string
+	switch upstreamProtocol {
+	case upstreamProtocolAuto:
+		alpn = []string{"h2", "http/1.1"}
+	case upstreamProtocolHTTP2:
+		alpn = []string{"h2"}
+	default:
+		alpn = []string{"http/1.1"}
+	}
+
 	sni := dst.Hostname()
 	if policy.TLSServerName != "" {
 		sni = policy.TLSServerName
@@ -241,7 +267,7 @@ func (b *Builder) buildPolicyTransportSocket(
 					"P-521",
 				},
 			},
-			AlpnProtocols: []string{"h2", "http/1.1"},
+			AlpnProtocols: alpn,
 			ValidationContextType: &envoy_extensions_transport_sockets_tls_v3.CommonTlsContext_ValidationContext{
 				ValidationContext: vc,
 			},
@@ -307,7 +333,7 @@ func (b *Builder) buildCluster(
 	cluster *envoy_config_cluster_v3.Cluster,
 	name string,
 	endpoints []Endpoint,
-	forceHTTP2 bool,
+	upstreamProtocol upstreamProtocolConfig,
 ) error {
 	if len(endpoints) == 0 {
 		return errNoEndpoints
@@ -339,7 +365,7 @@ func (b *Builder) buildCluster(
 	}
 
 	cluster.TypedExtensionProtocolOptions = map[string]*anypb.Any{
-		"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": marshalAny(buildUpstreamProtocolOptions(endpoints, forceHTTP2)),
+		"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": marshalAny(buildUpstreamProtocolOptions(endpoints, upstreamProtocol)),
 	}
 
 	cluster.ClusterDiscoveryType = getClusterDiscoveryType(lbEndpoints)

config/envoyconfig/clusters_test.go

@@ -37,14 +37,14 @@ func Test_buildPolicyTransportSocket(t *testing.T) {
 	t.Run("insecure", func(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "http://example.com"),
-		}, *mustParseURL(t, "http://example.com"))
+		}, *mustParseURL(t, "http://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		assert.Nil(t, ts)
 	})
 	t.Run("host as sni", func(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "https://example.com"),
-		}, *mustParseURL(t, "https://example.com"))
+		}, *mustParseURL(t, "https://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -95,7 +95,7 @@ func Test_buildPolicyTransportSocket(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o1, &config.Policy{
 			To:            mustParseWeightedURLs(t, "https://example.com"),
 			TLSServerName: "use-this-name.example.com",
-		}, *mustParseURL(t, "https://example.com"))
+		}, *mustParseURL(t, "https://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -146,7 +146,7 @@ func Test_buildPolicyTransportSocket(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o1, &config.Policy{
 			To:            mustParseWeightedURLs(t, "https://example.com"),
 			TLSSkipVerify: true,
-		}, *mustParseURL(t, "https://example.com"))
+		}, *mustParseURL(t, "https://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -198,7 +198,7 @@ func Test_buildPolicyTransportSocket(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o1, &config.Policy{
 			To:          mustParseWeightedURLs(t, "https://example.com"),
 			TLSCustomCA: base64.StdEncoding.EncodeToString([]byte{0, 0, 0, 0}),
-		}, *mustParseURL(t, "https://example.com"))
+		}, *mustParseURL(t, "https://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -248,7 +248,7 @@ func Test_buildPolicyTransportSocket(t *testing.T) {
 	t.Run("options custom ca", func(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o2, &config.Policy{
 			To: mustParseWeightedURLs(t, "https://example.com"),
-		}, *mustParseURL(t, "https://example.com"))
+		}, *mustParseURL(t, "https://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -300,7 +300,7 @@ func Test_buildPolicyTransportSocket(t *testing.T) {
 		ts, err := b.buildPolicyTransportSocket(ctx, o1, &config.Policy{
 			To:                mustParseWeightedURLs(t, "https://example.com"),
 			ClientCertificate: clientCert,
-		}, *mustParseURL(t, "https://example.com"))
+		}, *mustParseURL(t, "https://example.com"), upstreamProtocolAuto)
 		require.NoError(t, err)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -366,11 +366,11 @@ func Test_buildCluster(t *testing.T) {
 	t.Run("insecure", func(t *testing.T) {
 		endpoints, err := b.buildPolicyEndpoints(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "http://example.com", "http://1.2.3.4"),
-		})
+		}, upstreamProtocolAuto)
 		require.NoError(t, err)
 		cluster := newDefaultEnvoyClusterConfig()
 		cluster.DnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
-		err = b.buildCluster(cluster, "example", endpoints, true)
+		err = b.buildCluster(cluster, "example", endpoints, upstreamProtocolHTTP2)
 		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -428,10 +428,10 @@ func Test_buildCluster(t *testing.T) {
 				"https://example.com",
 				"https://example.com",
 			),
-		})
+		}, upstreamProtocolAuto)
 		require.NoError(t, err)
 		cluster := newDefaultEnvoyClusterConfig()
-		err = b.buildCluster(cluster, "example", endpoints, true)
+		err = b.buildCluster(cluster, "example", endpoints, upstreamProtocolHTTP2)
 		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -589,10 +589,10 @@ func Test_buildCluster(t *testing.T) {
 	t.Run("ip addresses", func(t *testing.T) {
 		endpoints, err := b.buildPolicyEndpoints(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "http://127.0.0.1", "http://127.0.0.2"),
-		})
+		}, upstreamProtocolAuto)
 		require.NoError(t, err)
 		cluster := newDefaultEnvoyClusterConfig()
-		err = b.buildCluster(cluster, "example", endpoints, true)
+		err = b.buildCluster(cluster, "example", endpoints, upstreamProtocolHTTP2)
 		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -646,10 +646,10 @@ func Test_buildCluster(t *testing.T) {
 	t.Run("weights", func(t *testing.T) {
 		endpoints, err := b.buildPolicyEndpoints(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "http://127.0.0.1:8080,1", "http://127.0.0.2,2"),
-		})
+		}, upstreamProtocolAuto)
 		require.NoError(t, err)
 		cluster := newDefaultEnvoyClusterConfig()
-		err = b.buildCluster(cluster, "example", endpoints, true)
+		err = b.buildCluster(cluster, "example", endpoints, upstreamProtocolHTTP2)
 		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -705,10 +705,10 @@ func Test_buildCluster(t *testing.T) {
 	t.Run("localhost", func(t *testing.T) {
 		endpoints, err := b.buildPolicyEndpoints(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "http://localhost"),
-		})
+		}, upstreamProtocolAuto)
 		require.NoError(t, err)
 		cluster := newDefaultEnvoyClusterConfig()
-		err = b.buildCluster(cluster, "example", endpoints, true)
+		err = b.buildCluster(cluster, "example", endpoints, upstreamProtocolHTTP2)
 		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
@@ -752,7 +752,7 @@ func Test_buildCluster(t *testing.T) {
 	t.Run("outlier", func(t *testing.T) {
 		endpoints, err := b.buildPolicyEndpoints(ctx, o1, &config.Policy{
 			To: mustParseWeightedURLs(t, "http://example.com"),
-		})
+		}, upstreamProtocolAuto)
 		require.NoError(t, err)
 		cluster := newDefaultEnvoyClusterConfig()
 		cluster.DnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
@@ -760,7 +760,7 @@ func Test_buildCluster(t *testing.T) {
 			EnforcingConsecutive_5Xx:       wrapperspb.UInt32(17),
 			SplitExternalLocalOriginErrors: true,
 		}
-		err = b.buildCluster(cluster, "example", endpoints, true)
+		err = b.buildCluster(cluster, "example", endpoints, upstreamProtocolHTTP2)
 		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{

config/envoyconfig/protocols.go

@@ -6,6 +6,14 @@ import (
 	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
+type upstreamProtocolConfig byte
+
+const (
+	upstreamProtocolAuto upstreamProtocolConfig = iota
+	upstreamProtocolHTTP2
+	upstreamProtocolHTTP1
+)
+
 // recommended defaults: https://www.envoyproxy.io/docs/envoy/latest/configuration/best_practices/edge
 const (
 	connectionBufferLimit            uint32 = 32 * 1024
@@ -21,9 +29,9 @@ var http2ProtocolOptions = &envoy_config_core_v3.Http2ProtocolOptions{
 	InitialConnectionWindowSize: wrapperspb.UInt32(initialConnectionWindowSizeLimit),
 }
 
-func buildUpstreamProtocolOptions(endpoints []Endpoint, forceHTTP2 bool) *envoy_extensions_upstreams_http_v3.HttpProtocolOptions {
-	// if forcing http/2, use that explicitly
-	if forceHTTP2 {
+func buildUpstreamProtocolOptions(endpoints []Endpoint, upstreamProtocol upstreamProtocolConfig) *envoy_extensions_upstreams_http_v3.HttpProtocolOptions {
+	switch upstreamProtocol {
+	case upstreamProtocolHTTP2:
 		return &envoy_extensions_upstreams_http_v3.HttpProtocolOptions{
 			UpstreamProtocolOptions: &envoy_extensions_upstreams_http_v3.HttpProtocolOptions_ExplicitHttpConfig_{
 				ExplicitHttpConfig: &envoy_extensions_upstreams_http_v3.HttpProtocolOptions_ExplicitHttpConfig{
@@ -33,23 +41,25 @@ func buildUpstreamProtocolOptions(endpoints []Endpoint, forceHTTP2 bool) *envoy_
 				},
 			},
 		}
-	}
-
-	// when using TLS use ALPN auto config
-	tlsCount := 0
-	for _, e := range endpoints {
-		if e.transportSocket != nil {
-			tlsCount++
+	case upstreamProtocolAuto:
+		// when using TLS use ALPN auto config
+		tlsCount := 0
+		for _, e := range endpoints {
+			if e.transportSocket != nil {
+				tlsCount++
+			}
 		}
-	}
-	if tlsCount > 0 && tlsCount == len(endpoints) {
-		return &envoy_extensions_upstreams_http_v3.HttpProtocolOptions{
-			UpstreamProtocolOptions: &envoy_extensions_upstreams_http_v3.HttpProtocolOptions_AutoConfig{
-				AutoConfig: &envoy_extensions_upstreams_http_v3.HttpProtocolOptions_AutoHttpConfig{
-					Http2ProtocolOptions: http2ProtocolOptions,
+		if tlsCount > 0 && tlsCount == len(endpoints) {
+			return &envoy_extensions_upstreams_http_v3.HttpProtocolOptions{
+				UpstreamProtocolOptions: &envoy_extensions_upstreams_http_v3.HttpProtocolOptions_AutoConfig{
+					AutoConfig: &envoy_extensions_upstreams_http_v3.HttpProtocolOptions_AutoHttpConfig{
+						Http2ProtocolOptions: http2ProtocolOptions,
+					},
 				},
-			},
+			}
 		}
+		fallthrough
+	default:
 	}
 
 	// otherwise only use http/1.1