xds: server-side listener network filter validation (#4312)

internal/testutils/marshal_any.go

@@ -0,0 +1,36 @@
+/*
+ *
+ * Copyright 2021 gRPC authors.
+ *
+ * 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
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package testutils
+
+import (
+	"fmt"
+
+	"github.com/golang/protobuf/proto"
+	"github.com/golang/protobuf/ptypes"
+	"google.golang.org/protobuf/types/known/anypb"
+)
+
+// MarshalAny is a convenience function to marshal protobuf messages into any
+// protos. It will panic if the marshaling fails.
+func MarshalAny(m proto.Message) *anypb.Any {
+	a, err := ptypes.MarshalAny(m)
+	if err != nil {
+		panic(fmt.Sprintf("ptypes.MarshalAny(%+v) failed: %v", m, err))
+	}
+	return a
+}

xds/internal/client/filter_chain_test.go

@@ -31,6 +31,7 @@ import (
 	"google.golang.org/protobuf/types/known/anypb"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 
+	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/xds/internal/version"
 )
 
@@ -247,7 +248,7 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.UpstreamTlsContext{}),
+								TypedConfig: testutils.MarshalAny(&v3tlspb.UpstreamTlsContext{}),
 							},
 						},
 					},
@@ -282,7 +283,7 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{}),
+								TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{}),
 							},
 						},
 					},
@@ -298,7 +299,7 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+								TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 									CommonTlsContext: &v3tlspb.CommonTlsContext{
 										ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextSdsSecretConfig{
 											ValidationContextSdsSecretConfig: &v3tlspb.SdsSecretConfig{
@@ -322,7 +323,7 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+								TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 									RequireClientCertificate: &wrapperspb.BoolValue{Value: true},
 									CommonTlsContext: &v3tlspb.CommonTlsContext{
 										TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{
@@ -346,7 +347,7 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+								TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 									CommonTlsContext: &v3tlspb.CommonTlsContext{},
 								}),
 							},
@@ -413,7 +414,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+								TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 									CommonTlsContext: &v3tlspb.CommonTlsContext{
 										TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{
 											InstanceName:    "identityPluginInstance",
@@ -429,7 +430,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) {
 					TransportSocket: &v3corepb.TransportSocket{
 						Name: "envoy.transport_sockets.tls",
 						ConfigType: &v3corepb.TransportSocket_TypedConfig{
-							TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+							TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 								CommonTlsContext: &v3tlspb.CommonTlsContext{
 									TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{
 										InstanceName:    "defaultIdentityPluginInstance",
@@ -480,7 +481,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) {
 						TransportSocket: &v3corepb.TransportSocket{
 							Name: "envoy.transport_sockets.tls",
 							ConfigType: &v3corepb.TransportSocket_TypedConfig{
-								TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+								TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 									RequireClientCertificate: &wrapperspb.BoolValue{Value: true},
 									CommonTlsContext: &v3tlspb.CommonTlsContext{
 										TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{
@@ -504,7 +505,7 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) {
 					TransportSocket: &v3corepb.TransportSocket{
 						Name: "envoy.transport_sockets.tls",
 						ConfigType: &v3corepb.TransportSocket_TypedConfig{
-							TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+							TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 								RequireClientCertificate: &wrapperspb.BoolValue{Value: true},
 								CommonTlsContext: &v3tlspb.CommonTlsContext{
 									TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{
@@ -1101,7 +1102,7 @@ func TestLookup_Successes(t *testing.T) {
 				TransportSocket: &v3corepb.TransportSocket{
 					Name: "envoy.transport_sockets.tls",
 					ConfigType: &v3corepb.TransportSocket_TypedConfig{
-						TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+						TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 							CommonTlsContext: &v3tlspb.CommonTlsContext{
 								TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{InstanceName: "instance1"},
 							},
@@ -1115,7 +1116,7 @@ func TestLookup_Successes(t *testing.T) {
 			TransportSocket: &v3corepb.TransportSocket{
 				Name: "envoy.transport_sockets.tls",
 				ConfigType: &v3corepb.TransportSocket_TypedConfig{
-					TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+					TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 						CommonTlsContext: &v3tlspb.CommonTlsContext{
 							TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{InstanceName: "default"},
 						},
@@ -1434,7 +1435,7 @@ func transportSocketWithInstanceName(name string) *v3corepb.TransportSocket {
 	return &v3corepb.TransportSocket{
 		Name: "envoy.transport_sockets.tls",
 		ConfigType: &v3corepb.TransportSocket_TypedConfig{
-			TypedConfig: marshalAny(&v3tlspb.DownstreamTlsContext{
+			TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
 				CommonTlsContext: &v3tlspb.CommonTlsContext{
 					TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{InstanceName: name},
 				},

xds/internal/client/xds.go

@@ -270,6 +270,13 @@ func processServerSideListener(lis *v3listenerpb.Listener) (*ListenerUpdate, err
 			Port:    strconv.Itoa(int(sockAddr.GetPortValue())),
 		},
 	}
+	chains := lis.GetFilterChains()
+	if def := lis.GetDefaultFilterChain(); def != nil {
+		chains = append(chains, def)
+	}
+	if err := validateNetworkFilterChains(chains); err != nil {
+		return nil, err
+	}
 
 	fcMgr, err := NewFilterChainManager(lis)
 	if err != nil {
@@ -279,6 +286,61 @@ func processServerSideListener(lis *v3listenerpb.Listener) (*ListenerUpdate, err
 	return lu, nil
 }
 
+func validateNetworkFilterChains(filterChains []*v3listenerpb.FilterChain) error {
+	for _, filterChain := range filterChains {
+		seenNames := make(map[string]bool, len(filterChain.GetFilters()))
+		seenHCM := false
+		for _, filter := range filterChain.GetFilters() {
+			name := filter.GetName()
+			if name == "" {
+				return fmt.Errorf("filter chain {%+v} is missing name field in filter: {%+v}", filterChain, filter)
+			}
+			if seenNames[name] {
+				return fmt.Errorf("filter chain {%+v} has duplicate filter name %q", filterChain, name)
+			}
+			seenNames[name] = true
+
+			// Network filters have a oneof field named `config_type` where we
+			// only support `TypedConfig` variant.
+			switch typ := filter.GetConfigType().(type) {
+			case *v3listenerpb.Filter_TypedConfig:
+				// The typed_config field has an `anypb.Any` proto which could
+				// directly contain the serialized bytes of the actual filter
+				// configuration, or it could be encoded as a `TypedStruct`.
+				// TODO: Add support for `TypedStruct`.
+				tc := filter.GetTypedConfig()
+
+				// The only network filter that we currently support is the v3
+				// HttpConnectionManager. So, we can directly check the type_url
+				// and unmarshal the config.
+				// TODO: Implement a registry of supported network filters (like
+				// we have for HTTP filters), when we have to support network
+				// filters other than HttpConnectionManager.
+				if tc.GetTypeUrl() != version.V3HTTPConnManagerURL {
+					return fmt.Errorf("filter chain {%+v} has unsupported network filter %q in filter {%+v}", filterChain, tc.GetTypeUrl(), filter)
+				}
+				hcm := &v3httppb.HttpConnectionManager{}
+				if err := ptypes.UnmarshalAny(tc, hcm); err != nil {
+					return fmt.Errorf("filter chain {%+v} failed unmarshaling of network filter {%+v}: %v", filterChain, filter, err)
+				}
+				// We currently don't support HTTP filters on the server-side.
+				// We will be adding support for it in the future. So, we want
+				// to make sure that the http_filters configuration is valid.
+				if _, err := processHTTPFilters(hcm.GetHttpFilters(), true); err != nil {
+					return err
+				}
+				seenHCM = true
+			default:
+				return fmt.Errorf("filter chain {%+v} has unsupported config_type %T in filter %s", filterChain, typ, filter.GetName())
+			}
+		}
+		if !seenHCM {
+			return fmt.Errorf("filter chain {%+v} missing HttpConnectionManager filter", filterChain)
+		}
+	}
+	return nil
+}
+
 // UnmarshalRouteConfig processes resources received in an RDS response,
 // validates them, and transforms them into a native struct which contains only
 // fields we are interested in. The provided hostname determines the route

xds/internal/httpfilter/httpfilter.go

@@ -65,9 +65,6 @@ type ClientInterceptorBuilder interface {
 
 // ServerInterceptorBuilder constructs a Server Interceptor.  If this type is
 // implemented by a Filter, it is capable of working on a server.
-//
-// Server side filters are not currently supported, but this interface is
-// defined for clarity.
 type ServerInterceptorBuilder interface {
 	// BuildServerInterceptor uses the FilterConfigs produced above to produce
 	// an HTTP filter interceptor for servers.  config will always be non-nil,

xds/internal/httpfilter/router/router.go

@@ -73,7 +73,10 @@ func (builder) ParseFilterConfigOverride(override proto.Message) (httpfilter.Fil
 	return config{}, nil
 }
 
-var _ httpfilter.ClientInterceptorBuilder = builder{}
+var (
+	_ httpfilter.ClientInterceptorBuilder = builder{}
+	_ httpfilter.ServerInterceptorBuilder = builder{}
+)
 
 func (builder) BuildClientInterceptor(cfg, override httpfilter.FilterConfig) (iresolver.ClientInterceptor, error) {
 	if _, ok := cfg.(config); !ok {
@@ -88,6 +91,18 @@ func (builder) BuildClientInterceptor(cfg, override httpfilter.FilterConfig) (ir
 	return nil, nil
 }
 
+func (builder) BuildServerInterceptor(cfg, override httpfilter.FilterConfig) (iresolver.ServerInterceptor, error) {
+	if _, ok := cfg.(config); !ok {
+		return nil, fmt.Errorf("router: incorrect config type provided (%T): %v", cfg, cfg)
+	}
+	if override != nil {
+		return nil, fmt.Errorf("router: unexpected override configuration specified: %v", override)
+	}
+	// The gRPC router is currently unimplemented on the server side. So we
+	// return a nil HTTPFilter, which will not be invoked.
+	return nil, nil
+}
+
 // The gRPC router filter does not currently support any configuration.  Verify
 // type only.
 type config struct {

xds/internal/test/xds_server_integration_test.go

@@ -33,27 +33,29 @@ import (
 	"strconv"
 	"testing"
 
-	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
-	v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
-	v3tlspb "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
-	wrapperspb "github.com/golang/protobuf/ptypes/wrappers"
 	"github.com/google/uuid"
-	xds2 "google.golang.org/grpc/internal/xds"
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/anypb"
-
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/credentials/insecure"
-	xdscreds "google.golang.org/grpc/credentials/xds"
+	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/status"
-	testpb "google.golang.org/grpc/test/grpc_testing"
 	"google.golang.org/grpc/testdata"
 	"google.golang.org/grpc/xds"
-	"google.golang.org/grpc/xds/internal/testutils"
 	"google.golang.org/grpc/xds/internal/testutils/e2e"
 	"google.golang.org/grpc/xds/internal/version"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+
+	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+	v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
+	v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
+	v3tlspb "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
+	wrapperspb "github.com/golang/protobuf/ptypes/wrappers"
+	xdscreds "google.golang.org/grpc/credentials/xds"
+	xdsinternal "google.golang.org/grpc/internal/xds"
+	testpb "google.golang.org/grpc/test/grpc_testing"
+	xdstestutils "google.golang.org/grpc/xds/internal/testutils"
 )
 
 const (
@@ -151,8 +153,8 @@ func commonSetup(t *testing.T) (*e2e.ManagementServer, string, net.Listener, fun
 	cpc := e2e.DefaultFileWatcherConfig(path.Join(tmpdir, certFile), path.Join(tmpdir, keyFile), path.Join(tmpdir, rootFile))
 
 	// Create a bootstrap file in a temporary directory.
-	bootstrapCleanup, err := xds2.SetupBootstrapFile(xds2.BootstrapOptions{
+	bootstrapCleanup, err := xdsinternal.SetupBootstrapFile(xdsinternal.BootstrapOptions{
-		Version:                            xds2.TransportV3,
+		Version:                            xdsinternal.TransportV3,
 		NodeID:                             nodeID,
 		ServerURI:                          fs.Address,
 		CertificateProviders:               cpc,
@@ -175,7 +177,7 @@ func commonSetup(t *testing.T) (*e2e.ManagementServer, string, net.Listener, fun
 	testpb.RegisterTestServiceServer(server, &testService{})
 
 	// Create a local listener and pass it to Serve().
-	lis, err := testutils.LocalTCPListener()
+	lis, err := xdstestutils.LocalTCPListener()
 	if err != nil {
 		t.Fatalf("testutils.LocalTCPListener() failed: %v", err)
 	}
@@ -229,6 +231,14 @@ func listenerResourceWithoutSecurityConfig(t *testing.T, lis net.Listener) *v3li
 		FilterChains: []*v3listenerpb.FilterChain{
 			{
 				Name: "filter-chain-1",
+				Filters: []*v3listenerpb.Filter{
+					{
+						Name: "filter-1",
+						ConfigType: &v3listenerpb.Filter_TypedConfig{
+							TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{}),
+						},
+					},
+				},
 			},
 		},
 	}
@@ -271,6 +281,14 @@ func listenerResourceWithSecurityConfig(t *testing.T, lis net.Listener) *v3liste
 						},
 					},
 				},
+				Filters: []*v3listenerpb.Filter{
+					{
+						Name: "filter-1",
+						ConfigType: &v3listenerpb.Filter_TypedConfig{
+							TypedConfig: testutils.MarshalAny(&v3httppb.HttpConnectionManager{}),
+						},
+					},
+				},
 				TransportSocket: &v3corepb.TransportSocket{
 					Name: "envoy.transport_sockets.tls",
 					ConfigType: &v3corepb.TransportSocket_TypedConfig{