From bb0d32f07806ec84fc7a7bfeff73a5c3b53555bb Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Tue, 19 Dec 2023 16:30:43 -0800 Subject: [PATCH] xds: don't fail channel/server startup when xds creds is specified, but bootstrap is missing certificate providers (#6848) --- internal/testutils/xds/e2e/setup_certs.go | 26 +- .../xds_client_certificate_providers_test.go | 362 +++++++++++++ .../xds_server_certificate_providers_test.go | 493 ++++++++++++++++++ test/xds/xds_server_integration_test.go | 28 +- .../balancer/cdsbalancer/cdsbalancer.go | 13 +- xds/internal/resolver/xds_resolver.go | 18 - xds/internal/resolver/xds_resolver_test.go | 92 +--- xds/internal/server/conn_wrapper.go | 13 +- xds/internal/server/listener_wrapper.go | 5 - xds/server.go | 27 - xds/server_test.go | 22 - 11 files changed, 929 insertions(+), 170 deletions(-) create mode 100644 test/xds/xds_client_certificate_providers_test.go create mode 100644 test/xds/xds_server_certificate_providers_test.go diff --git a/internal/testutils/xds/e2e/setup_certs.go b/internal/testutils/xds/e2e/setup_certs.go index 799e18564879..dea392162595 100644 --- a/internal/testutils/xds/e2e/setup_certs.go +++ b/internal/testutils/xds/e2e/setup_certs.go @@ -87,7 +87,7 @@ func CreateClientTLSCredentials(t *testing.T) credentials.TransportCredentials { } roots := x509.NewCertPool() if !roots.AppendCertsFromPEM(b) { - t.Fatal("failed to append certificates") + t.Fatal("Failed to append certificates") } return credentials.NewTLS(&tls.Config{ Certificates: []tls.Certificate{cert}, @@ -95,3 +95,27 @@ func CreateClientTLSCredentials(t *testing.T) credentials.TransportCredentials { ServerName: "x.test.example.com", }) } + +// CreateServerTLSCredentials creates server-side TLS transport credentials +// using certificate and key files from testdata/x509 directory. +func CreateServerTLSCredentials(t *testing.T) credentials.TransportCredentials { + t.Helper() + + cert, err := tls.LoadX509KeyPair(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem")) + if err != nil { + t.Fatalf("tls.LoadX509KeyPair(x509/server1_cert.pem, x509/server1_key.pem) failed: %v", err) + } + b, err := os.ReadFile(testdata.Path("x509/client_ca_cert.pem")) + if err != nil { + t.Fatalf("os.ReadFile(x509/client_ca_cert.pem) failed: %v", err) + } + ca := x509.NewCertPool() + if !ca.AppendCertsFromPEM(b) { + t.Fatal("Failed to append certificates") + } + return credentials.NewTLS(&tls.Config{ + ClientAuth: tls.RequireAndVerifyClientCert, + Certificates: []tls.Certificate{cert}, + ClientCAs: ca, + }) +} diff --git a/test/xds/xds_client_certificate_providers_test.go b/test/xds/xds_client_certificate_providers_test.go new file mode 100644 index 000000000000..a2979ca1beae --- /dev/null +++ b/test/xds/xds_client_certificate_providers_test.go @@ -0,0 +1,362 @@ +/* + * + * Copyright 2023 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 xds_test + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/google/uuid" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" + xdscreds "google.golang.org/grpc/credentials/xds" + "google.golang.org/grpc/internal" + "google.golang.org/grpc/internal/stubserver" + "google.golang.org/grpc/internal/testutils" + "google.golang.org/grpc/internal/testutils/xds/bootstrap" + "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/peer" + "google.golang.org/grpc/resolver" + "google.golang.org/grpc/status" + + v3clusterpb "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + v3endpointpb "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" + v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + v3tlspb "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" + testgrpc "google.golang.org/grpc/interop/grpc_testing" + testpb "google.golang.org/grpc/interop/grpc_testing" +) + +// Tests the case where the bootstrap configuration contains no certificate +// providers, and xDS credentials with an insecure fallback is specified at dial +// time. The management server is configured to return client side xDS resources +// with no security configuration. The test verifies that the gRPC client is +// able to make RPCs to the backend which is configured to accept plaintext +// connections. This ensures that the insecure fallback credentials are getting +// used on the client. +func (s) TestClientSideXDS_WithNoCertificateProvidersInBootstrap_Success(t *testing.T) { + // Spin up an xDS management server. + mgmtServer, err := e2e.StartManagementServer(e2e.ManagementServerOptions{}) + if err != nil { + t.Fatalf("Failed to start management server: %v", err) + } + defer mgmtServer.Stop() + + // Create bootstrap configuration with no certificate providers. + nodeID := uuid.New().String() + bs, err := bootstrap.Contents(bootstrap.Options{ + NodeID: nodeID, + ServerURI: mgmtServer.Address, + }) + if err != nil { + t.Fatalf("Failed to create bootstrap configuration: %v", err) + } + + // Create an xDS resolver with the above bootstrap configuration. + newResolver := internal.NewXDSResolverWithConfigForTesting + if newResolver == nil { + t.Fatal("internal.NewXDSResolverWithConfigForTesting is unset") + } + resolverBuilder, err := newResolver.(func([]byte) (resolver.Builder, error))(bs) + if err != nil { + t.Fatalf("Failed to create xDS resolver for testing: %v", err) + } + + // Spin up a test backend. + server := stubserver.StartTestService(t, nil) + defer server.Stop() + + // Configure client side xDS resources on the management server, with no + // security configuration in the Cluster resource. + const serviceName = "my-service-client-side-xds" + resources := e2e.DefaultClientResources(e2e.ResourceParams{ + DialTarget: serviceName, + NodeID: nodeID, + Host: "localhost", + Port: testutils.ParsePort(t, server.Address), + SecLevel: e2e.SecurityLevelNone, + }) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := mgmtServer.Update(ctx, resources); err != nil { + t.Fatal(err) + } + + // Create client-side xDS credentials with an insecure fallback. + creds, err := xdscreds.NewClientCredentials(xdscreds.ClientOptions{FallbackCreds: insecure.NewCredentials()}) + if err != nil { + t.Fatal(err) + } + + // Create a ClientConn and make a successful RPC. + cc, err := grpc.Dial(fmt.Sprintf("xds:///%s", serviceName), grpc.WithTransportCredentials(creds), grpc.WithResolvers(resolverBuilder)) + if err != nil { + t.Fatalf("failed to dial local test server: %v", err) + } + defer cc.Close() + + client := testgrpc.NewTestServiceClient(cc) + if _, err := client.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); err != nil { + t.Fatalf("EmptyCall() failed: %v", err) + } +} + +// Tests the case where the bootstrap configuration contains no certificate +// providers, and xDS credentials with an insecure fallback is specified at dial +// time. The management server is configured to return client side xDS resources +// with an mTLS security configuration. The test verifies that the gRPC client +// moves to TRANSIENT_FAILURE and rpcs fail with the expected error code and +// string. This ensures that when the certificate provider instance name +// specified in the security configuration is not present in the bootstrap, +// channel creation does not fail, but it moves to TRANSIENT_FAILURE and +// subsequent rpcs fail. +func (s) TestClientSideXDS_WithNoCertificateProvidersInBootstrap_Failure(t *testing.T) { + // Spin up an xDS management server. + mgmtServer, err := e2e.StartManagementServer(e2e.ManagementServerOptions{}) + if err != nil { + t.Fatalf("Failed to start management server: %v", err) + } + defer mgmtServer.Stop() + + // Create bootstrap configuration with no certificate providers. + nodeID := uuid.New().String() + bs, err := bootstrap.Contents(bootstrap.Options{ + NodeID: nodeID, + ServerURI: mgmtServer.Address, + }) + if err != nil { + t.Fatalf("Failed to create bootstrap configuration: %v", err) + } + + // Create an xDS resolver with the above bootstrap configuration. + newResolver := internal.NewXDSResolverWithConfigForTesting + if newResolver == nil { + t.Fatal("internal.NewXDSResolverWithConfigForTesting is unset") + } + resolverBuilder, err := newResolver.(func([]byte) (resolver.Builder, error))(bs) + if err != nil { + t.Fatalf("Failed to create xDS resolver for testing: %v", err) + } + + // Spin up a test backend. + server := stubserver.StartTestService(t, nil) + defer server.Stop() + + // Configure client side xDS resources on the management server, with mTLS + // security configuration in the Cluster resource. + const serviceName = "my-service-client-side-xds" + const clusterName = "cluster-" + serviceName + const endpointsName = "endpoints-" + serviceName + resources := e2e.DefaultClientResources(e2e.ResourceParams{ + DialTarget: serviceName, + NodeID: nodeID, + Host: "localhost", + Port: testutils.ParsePort(t, server.Address), + SecLevel: e2e.SecurityLevelNone, + }) + resources.Clusters = []*v3clusterpb.Cluster{e2e.DefaultCluster(clusterName, endpointsName, e2e.SecurityLevelMTLS)} + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := mgmtServer.Update(ctx, resources); err != nil { + t.Fatal(err) + } + + // Create client-side xDS credentials with an insecure fallback. + creds, err := xdscreds.NewClientCredentials(xdscreds.ClientOptions{FallbackCreds: insecure.NewCredentials()}) + if err != nil { + t.Fatal(err) + } + + // Create a ClientConn and ensure that it moves to TRANSIENT_FAILURE. + cc, err := grpc.Dial(fmt.Sprintf("xds:///%s", serviceName), grpc.WithTransportCredentials(creds), grpc.WithResolvers(resolverBuilder)) + if err != nil { + t.Fatalf("failed to dial local test server: %v", err) + } + defer cc.Close() + testutils.AwaitState(ctx, t, cc, connectivity.TransientFailure) + + // Make an RPC and ensure that expected error is returned. + wantErr := fmt.Sprintf("identitiy certificate provider instance name %q missing in bootstrap configuration", e2e.ClientSideCertProviderInstance) + client := testgrpc.NewTestServiceClient(cc) + if _, err := client.EmptyCall(ctx, &testpb.Empty{}); status.Code(err) != codes.Unavailable || !strings.Contains(err.Error(), wantErr) { + t.Fatalf("EmptyCall() failed: %v, wantCode: %s, wantErr: %s", err, codes.Unavailable, wantErr) + } +} + +// Tests the case where the bootstrap configuration contains one certificate +// provider, and xDS credentials with an insecure fallback is specified at dial +// time. The management server responds with three clusters: +// 1. contains valid security configuration pointing to the certificate provider +// instance specified in the bootstrap +// 2. contains no security configuration, hence should use insecure fallback +// 3. contains invalid security configuration pointing to a non-existent +// certificate provider instance +// +// The test verifies that RPCs to the first two clusters succeed, while RPCs to +// the third cluster fails with an appropriate code and error message. +func (s) TestClientSideXDS_WithValidAndInvalidSecurityConfiguration(t *testing.T) { + // Spin up an xDS management server. This uses a bootstrap config with a + // certificate provider instance name e2e.ClientSideCertProviderInstance. + mgmtServer, nodeID, _, resolver, cleanup := e2e.SetupManagementServer(t, e2e.ManagementServerOptions{AllowResourceSubset: true}) + defer cleanup() + + // Create test backends for all three clusters + // backend1 configured with TLS creds, represents cluster1 + // backend2 configured with insecure creds, represents cluster2 + // backend3 configured with insecure creds, represents cluster3 + creds := e2e.CreateServerTLSCredentials(t) + server1 := stubserver.StartTestService(t, nil, grpc.Creds(creds)) + defer server1.Stop() + server2 := stubserver.StartTestService(t, nil) + defer server2.Stop() + server3 := stubserver.StartTestService(t, nil) + defer server3.Stop() + + // Configure client side xDS resources on the management server. + const serviceName = "my-service-client-side-xds" + const routeConfigName = "route-" + serviceName + const clusterName1 = "cluster1-" + serviceName + const clusterName2 = "cluster2-" + serviceName + const clusterName3 = "cluster3-" + serviceName + const endpointsName1 = "endpoints1-" + serviceName + const endpointsName2 = "endpoints2-" + serviceName + const endpointsName3 = "endpoints3-" + serviceName + listeners := []*v3listenerpb.Listener{e2e.DefaultClientListener(serviceName, routeConfigName)} + // Route configuration: + // - "/grpc.testing.TestService/EmptyCall" --> cluster1 + // - "/grpc.testing.TestService/UnaryCall" --> cluster2 + // - "/grpc.testing.TestService/FullDuplexCall" --> cluster3 + routes := []*v3routepb.RouteConfiguration{{ + Name: routeConfigName, + VirtualHosts: []*v3routepb.VirtualHost{{ + Domains: []string{serviceName}, + Routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/grpc.testing.TestService/EmptyCall"}}, + Action: &v3routepb.Route_Route{Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName1}, + }}, + }, + { + Match: &v3routepb.RouteMatch{PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/grpc.testing.TestService/UnaryCall"}}, + Action: &v3routepb.Route_Route{Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName2}, + }}, + }, + { + Match: &v3routepb.RouteMatch{PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/grpc.testing.TestService/FullDuplexCall"}}, + Action: &v3routepb.Route_Route{Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName3}, + }}, + }, + }, + }}, + }} + // Clusters: + // - cluster1 with cert provider name e2e.ClientSideCertProviderInstance. + // - cluster2 with no security configuration. + // - cluster3 with non-existent cert provider name. + clusters := []*v3clusterpb.Cluster{ + e2e.DefaultCluster(clusterName1, endpointsName1, e2e.SecurityLevelMTLS), + e2e.DefaultCluster(clusterName2, endpointsName2, e2e.SecurityLevelNone), + func() *v3clusterpb.Cluster { + cluster3 := e2e.DefaultCluster(clusterName3, endpointsName3, e2e.SecurityLevelMTLS) + cluster3.TransportSocket = &v3corepb.TransportSocket{ + Name: "envoy.transport_sockets.tls", + ConfigType: &v3corepb.TransportSocket_TypedConfig{ + TypedConfig: testutils.MarshalAny(t, &v3tlspb.UpstreamTlsContext{ + CommonTlsContext: &v3tlspb.CommonTlsContext{ + ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextCertificateProviderInstance{ + ValidationContextCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{ + InstanceName: "non-existent-certificate-provider-instance-name", + }, + }, + TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{ + InstanceName: "non-existent-certificate-provider-instance-name", + }, + }, + }), + }, + } + return cluster3 + }(), + } + // Endpoints for each of the above clusters with backends created earlier. + endpoints := []*v3endpointpb.ClusterLoadAssignment{ + e2e.DefaultEndpoint(endpointsName1, "localhost", []uint32{testutils.ParsePort(t, server1.Address)}), + e2e.DefaultEndpoint(endpointsName2, "localhost", []uint32{testutils.ParsePort(t, server2.Address)}), + } + resources := e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: listeners, + Routes: routes, + Clusters: clusters, + Endpoints: endpoints, + SkipValidation: true, + } + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := mgmtServer.Update(ctx, resources); err != nil { + t.Fatal(err) + } + + // Create client-side xDS credentials with an insecure fallback. + creds, err := xdscreds.NewClientCredentials(xdscreds.ClientOptions{FallbackCreds: insecure.NewCredentials()}) + if err != nil { + t.Fatal(err) + } + + // Create a ClientConn. + cc, err := grpc.Dial(fmt.Sprintf("xds:///%s", serviceName), grpc.WithTransportCredentials(creds), grpc.WithResolvers(resolver)) + if err != nil { + t.Fatalf("failed to dial local test server: %v", err) + } + defer cc.Close() + + // Make an RPC to be routed to cluster1 and verify that it succeeds. + client := testgrpc.NewTestServiceClient(cc) + peer := &peer.Peer{} + if _, err := client.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true), grpc.Peer(peer)); err != nil { + t.Fatalf("EmptyCall() failed: %v", err) + } + if got, want := peer.Addr.String(), server1.Address; got != want { + t.Errorf("EmptyCall() routed to %q, want to be routed to: %q", got, want) + + } + + // Make an RPC to be routed to cluster2 and verify that it succeeds. + if _, err := client.UnaryCall(ctx, &testpb.SimpleRequest{}, grpc.Peer(peer)); err != nil { + t.Fatalf("UnaryCall() failed: %v", err) + } + if got, want := peer.Addr.String(), server2.Address; got != want { + t.Errorf("EmptyCall() routed to %q, want to be routed to: %q", got, want) + } + + // Make an RPC to be routed to cluster3 and verify that it fails. + const wantErr = `identitiy certificate provider instance name "non-existent-certificate-provider-instance-name" missing in bootstrap configuration` + if _, err := client.FullDuplexCall(ctx); status.Code(err) != codes.Unavailable || !strings.Contains(err.Error(), wantErr) { + t.Fatalf("FullDuplexCall failed: %v, wantCode: %s, wantErr: %s", err, codes.Unavailable, wantErr) + } +} diff --git a/test/xds/xds_server_certificate_providers_test.go b/test/xds/xds_server_certificate_providers_test.go new file mode 100644 index 000000000000..c0fc333c21a6 --- /dev/null +++ b/test/xds/xds_server_certificate_providers_test.go @@ -0,0 +1,493 @@ +/* + * + * Copyright 2023 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 xds_test + +import ( + "context" + "fmt" + "net" + "strconv" + "testing" + "time" + + "github.com/google/uuid" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" + xdscreds "google.golang.org/grpc/credentials/xds" + "google.golang.org/grpc/internal/testutils" + "google.golang.org/grpc/internal/testutils/xds/bootstrap" + "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/status" + "google.golang.org/grpc/xds" + "google.golang.org/protobuf/types/known/wrapperspb" + + v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/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" + v3discoverypb "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" + testgrpc "google.golang.org/grpc/interop/grpc_testing" + testpb "google.golang.org/grpc/interop/grpc_testing" +) + +// Tests the case where the bootstrap configuration contains no certificate +// providers, and xDS credentials with an insecure fallback is specified at +// server creation time. The management server is configured to return a +// server-side xDS Listener resource with no security configuration. The test +// verifies that a gRPC client configured with insecure credentials is able to +// make RPCs to the backend. This ensures that the insecure fallback +// credentials are getting used on the server. +func (s) TestServerSideXDS_WithNoCertificateProvidersInBootstrap_Success(t *testing.T) { + // Spin up an xDS management server. + mgmtServer, err := e2e.StartManagementServer(e2e.ManagementServerOptions{AllowResourceSubset: true}) + if err != nil { + t.Fatalf("Failed to start management server: %v", err) + } + defer mgmtServer.Stop() + + // Create bootstrap configuration with no certificate providers. + nodeID := uuid.New().String() + bs, err := bootstrap.Contents(bootstrap.Options{ + NodeID: nodeID, + ServerURI: mgmtServer.Address, + ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, + }) + if err != nil { + t.Fatalf("Failed to create bootstrap configuration: %v", err) + } + + // Spin up an xDS-enabled gRPC server that uses xDS credentials with + // insecure fallback, and the above bootstrap configuration. + lis, cleanup := setupGRPCServer(t, bs) + defer cleanup() + + // Create an inbound xDS listener resource for the server side that does not + // contain any security configuration. This should force the server-side + // xdsCredentials to use fallback. + host, port, err := hostPortFromListener(lis) + if err != nil { + t.Fatalf("Failed to retrieve host and port of server: %v", err) + } + resources := e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: []*v3listenerpb.Listener{e2e.DefaultServerListener(host, port, e2e.SecurityLevelNone, "routeName")}, + SkipValidation: true, + } + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := mgmtServer.Update(ctx, resources); err != nil { + t.Fatal(err) + } + + // Create a client that uses insecure creds and verify RPCs. + cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("Failed to dial local test server: %v", err) + } + defer cc.Close() + + client := testgrpc.NewTestServiceClient(cc) + if _, err := client.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); err != nil { + t.Fatalf("EmptyCall() failed: %v", err) + } +} + +// Tests the case where the bootstrap configuration contains no certificate +// providers, and xDS credentials with an insecure fallback is specified at +// server creation time. The management server is configured to return a +// server-side xDS Listener resource with mTLS security configuration. The xDS +// client is expected to NACK this resource because the certificate provider +// instance name specified in the Listener resource will not be present in the +// bootstrap file. The test verifies that server creation does not fail and that +// the xDS-enabled gRPC server does not enter "serving" mode. +func (s) TestServerSideXDS_WithNoCertificateProvidersInBootstrap_Failure(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + // Spin up an xDS management server that pushes on a channel when it + // receives a NACK for an LDS response. + nackCh := make(chan struct{}, 1) + mgmtServer, err := e2e.StartManagementServer(e2e.ManagementServerOptions{ + OnStreamRequest: func(_ int64, req *v3discoverypb.DiscoveryRequest) error { + if req.GetTypeUrl() != "type.googleapis.com/envoy.config.listener.v3.Listener" { + return nil + } + if req.GetErrorDetail() == nil { + return nil + } + select { + case nackCh <- struct{}{}: + case <-ctx.Done(): + } + return nil + }, + AllowResourceSubset: true, + }) + if err != nil { + t.Fatalf("Failed to start management server: %v", err) + } + defer mgmtServer.Stop() + + // Create bootstrap configuration with no certificate providers. + nodeID := uuid.New().String() + bs, err := bootstrap.Contents(bootstrap.Options{ + NodeID: nodeID, + ServerURI: mgmtServer.Address, + ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, + }) + if err != nil { + t.Fatalf("Failed to create bootstrap configuration: %v", err) + } + + // Configure xDS credentials with an insecure fallback to be used on the + // server-side. + creds, err := xdscreds.NewServerCredentials(xdscreds.ServerOptions{FallbackCreds: insecure.NewCredentials()}) + if err != nil { + t.Fatal(err) + } + + // Initialize an xDS-enabled gRPC server and register the stubServer on it. + // Pass it a mode change server option that pushes on a channel the mode + // changes to "not serving". + servingModeCh := make(chan struct{}) + modeChangeOpt := xds.ServingModeCallback(func(addr net.Addr, args xds.ServingModeChangeArgs) { + if args.Mode == connectivity.ServingModeServing { + close(servingModeCh) + } + }) + server, err := xds.NewGRPCServer(grpc.Creds(creds), modeChangeOpt, xds.BootstrapContentsForTesting(bs)) + if err != nil { + t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) + } + testgrpc.RegisterTestServiceServer(server, &testService{}) + defer server.Stop() + + // Create a local listener and pass it to Serve(). + lis, err := testutils.LocalTCPListener() + if err != nil { + t.Fatalf("testutils.LocalTCPListener() failed: %v", err) + } + + go func() { + if err := server.Serve(lis); err != nil { + t.Errorf("Serve() failed: %v", err) + } + }() + + // Create an inbound xDS listener resource for the server side that contains + // mTLS security configuration. Since the received certificate provider + // instance name would be missing in the bootstrap configuration, this + // resource is expected to NACKed by the xDS client. + host, port, err := hostPortFromListener(lis) + if err != nil { + t.Fatalf("Failed to retrieve host and port of server: %v", err) + } + resources := e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: []*v3listenerpb.Listener{e2e.DefaultServerListener(host, port, e2e.SecurityLevelMTLS, "routeName")}, + SkipValidation: true, + } + if err := mgmtServer.Update(ctx, resources); err != nil { + t.Fatal(err) + } + + // Wait for the NACK from the xDS client. + select { + case <-nackCh: + case <-ctx.Done(): + t.Fatal("Timeout when waiting for an NACK from the xDS client for the LDS response") + } + + // Wait a short duration and ensure that the server does not enter "serving" + // mode. + select { + case <-time.After(2 * defaultTestShortTimeout): + case <-servingModeCh: + t.Fatal("Server changed to serving mode when not expected to") + } + + // Create a client that uses insecure creds and verify that RPCs don't + // succeed. + cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("Failed to dial local test server: %v", err) + } + defer cc.Close() + + client := testgrpc.NewTestServiceClient(cc) + sCtx, sCancel := context.WithTimeout(ctx, defaultTestShortTimeout) + defer sCancel() + if _, err := client.EmptyCall(sCtx, &testpb.Empty{}); status.Code(err) != codes.DeadlineExceeded { + t.Fatalf("EmptyCall() failed: %v, wantCode: %s", err, codes.DeadlineExceeded) + } +} + +// Tests the case where the bootstrap configuration contains one certificate +// provider, and xDS credentials with an insecure fallback is specified at +// server creation time. Two listeners are configured on the xDS-enabled gRPC +// server. The management server responds with two listener resources: +// 1. contains valid security configuration pointing to the certificate provider +// instance specified in the bootstrap +// 2. contains invalid security configuration pointing to a non-existent +// certificate provider instance +// +// The test verifies that an RPC to the first listener succeeds, while the +// second listener never moves to "serving" mode and RPCs to it fail. +func (s) TestServerSideXDS_WithValidAndInvalidSecurityConfiguration(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + // Spin up an xDS management server that pushes on a channel when it + // receives a NACK for an LDS response. + nackCh := make(chan struct{}, 1) + mgmtServer, nodeID, bs, _, cleanup := e2e.SetupManagementServer(t, e2e.ManagementServerOptions{ + OnStreamRequest: func(_ int64, req *v3discoverypb.DiscoveryRequest) error { + if req.GetTypeUrl() != "type.googleapis.com/envoy.config.listener.v3.Listener" { + return nil + } + if req.GetErrorDetail() == nil { + return nil + } + select { + case nackCh <- struct{}{}: + case <-ctx.Done(): + } + return nil + }, + AllowResourceSubset: true, + }) + defer cleanup() + + // Create two local listeners. + lis1, err := testutils.LocalTCPListener() + if err != nil { + t.Fatalf("testutils.LocalTCPListener() failed: %v", err) + } + lis2, err := testutils.LocalTCPListener() + if err != nil { + t.Fatalf("testutils.LocalTCPListener() failed: %v", err) + } + + // Create an xDS-enabled grpc server that is configured to use xDS + // credentials, and register the test service on it. Configure a mode change + // option that closes a channel when listener2 enter serving mode. + creds, err := xdscreds.NewServerCredentials(xdscreds.ServerOptions{FallbackCreds: insecure.NewCredentials()}) + if err != nil { + t.Fatal(err) + } + servingModeCh := make(chan struct{}) + modeChangeOpt := xds.ServingModeCallback(func(addr net.Addr, args xds.ServingModeChangeArgs) { + if addr.String() == lis2.Addr().String() { + if args.Mode == connectivity.ServingModeServing { + close(servingModeCh) + } + } + }) + server, err := xds.NewGRPCServer(grpc.Creds(creds), modeChangeOpt, xds.BootstrapContentsForTesting(bs)) + if err != nil { + t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) + } + testgrpc.RegisterTestServiceServer(server, &testService{}) + defer server.Stop() + + go func() { + if err := server.Serve(lis1); err != nil { + t.Errorf("Serve() failed: %v", err) + } + }() + go func() { + if err := server.Serve(lis2); err != nil { + t.Errorf("Serve() failed: %v", err) + } + }() + + // Create inbound xDS listener resources for the server side that contains + // mTLS security configuration. + // lis1 --> security configuration pointing to a valid cert provider + // lis2 --> security configuration pointing to a non-existent cert provider + host1, port1, err := hostPortFromListener(lis1) + if err != nil { + t.Fatalf("Failed to retrieve host and port of server: %v", err) + } + resource1 := e2e.DefaultServerListener(host1, port1, e2e.SecurityLevelMTLS, "routeName") + host2, port2, err := hostPortFromListener(lis2) + if err != nil { + t.Fatalf("Failed to retrieve host and port of server: %v", err) + } + hcm := &v3httppb.HttpConnectionManager{ + RouteSpecifier: &v3httppb.HttpConnectionManager_RouteConfig{ + RouteConfig: &v3routepb.RouteConfiguration{ + Name: "routeName", + VirtualHosts: []*v3routepb.VirtualHost{{ + Domains: []string{"*"}, + Routes: []*v3routepb.Route{{ + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/"}, + }, + Action: &v3routepb.Route_NonForwardingAction{}, + }}}}}, + }, + HttpFilters: []*v3httppb.HttpFilter{e2e.RouterHTTPFilter}, + } + ts := &v3corepb.TransportSocket{ + Name: "envoy.transport_sockets.tls", + ConfigType: &v3corepb.TransportSocket_TypedConfig{ + TypedConfig: testutils.MarshalAny(t, &v3tlspb.DownstreamTlsContext{ + RequireClientCertificate: &wrapperspb.BoolValue{Value: true}, + CommonTlsContext: &v3tlspb.CommonTlsContext{ + TlsCertificateCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{ + InstanceName: "non-existent-certificate-provider", + }, + ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextCertificateProviderInstance{ + ValidationContextCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{ + InstanceName: "non-existent-certificate-provider", + }, + }, + }, + }), + }, + } + resource2 := &v3listenerpb.Listener{ + Name: fmt.Sprintf(e2e.ServerListenerResourceNameTemplate, net.JoinHostPort(host2, strconv.Itoa(int(port2)))), + Address: &v3corepb.Address{ + Address: &v3corepb.Address_SocketAddress{ + SocketAddress: &v3corepb.SocketAddress{ + Address: host2, + PortSpecifier: &v3corepb.SocketAddress_PortValue{ + PortValue: port2, + }, + }, + }, + }, + FilterChains: []*v3listenerpb.FilterChain{ + { + Name: "v4-wildcard", + FilterChainMatch: &v3listenerpb.FilterChainMatch{ + PrefixRanges: []*v3corepb.CidrRange{ + { + AddressPrefix: "0.0.0.0", + PrefixLen: &wrapperspb.UInt32Value{ + Value: uint32(0), + }, + }, + }, + SourceType: v3listenerpb.FilterChainMatch_SAME_IP_OR_LOOPBACK, + SourcePrefixRanges: []*v3corepb.CidrRange{ + { + AddressPrefix: "0.0.0.0", + PrefixLen: &wrapperspb.UInt32Value{ + Value: uint32(0), + }, + }, + }, + }, + Filters: []*v3listenerpb.Filter{ + { + Name: "filter-1", + ConfigType: &v3listenerpb.Filter_TypedConfig{TypedConfig: testutils.MarshalAny(t, hcm)}, + }, + }, + TransportSocket: ts, + }, + { + Name: "v6-wildcard", + FilterChainMatch: &v3listenerpb.FilterChainMatch{ + PrefixRanges: []*v3corepb.CidrRange{ + { + AddressPrefix: "::", + PrefixLen: &wrapperspb.UInt32Value{ + Value: uint32(0), + }, + }, + }, + SourceType: v3listenerpb.FilterChainMatch_SAME_IP_OR_LOOPBACK, + SourcePrefixRanges: []*v3corepb.CidrRange{ + { + AddressPrefix: "::", + PrefixLen: &wrapperspb.UInt32Value{ + Value: uint32(0), + }, + }, + }, + }, + Filters: []*v3listenerpb.Filter{ + { + Name: "filter-1", + ConfigType: &v3listenerpb.Filter_TypedConfig{TypedConfig: testutils.MarshalAny(t, hcm)}, + }, + }, + TransportSocket: ts, + }, + }, + } + resources := e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: []*v3listenerpb.Listener{resource1, resource2}, + SkipValidation: true, + } + if err := mgmtServer.Update(ctx, resources); err != nil { + t.Fatal(err) + } + + // Create a client that uses TLS creds and verify RPCs to listener1. + clientCreds := e2e.CreateClientTLSCredentials(t) + cc1, err := grpc.Dial(lis1.Addr().String(), grpc.WithTransportCredentials(clientCreds)) + if err != nil { + t.Fatalf("Failed to dial local test server: %v", err) + } + defer cc1.Close() + + client1 := testgrpc.NewTestServiceClient(cc1) + if _, err := client1.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); err != nil { + t.Fatalf("EmptyCall() failed: %v", err) + } + + // Wait for the NACK from the xDS client. + select { + case <-nackCh: + case <-ctx.Done(): + t.Fatal("Timeout when waiting for an NACK from the xDS client for the LDS response") + } + + // Wait a short duration and ensure that the server does not enter "serving" + // mode. + select { + case <-time.After(2 * defaultTestShortTimeout): + case <-servingModeCh: + t.Fatal("Server changed to serving mode when not expected to") + } + + // Create a client that uses insecure creds and verify that RPCs don't + // succeed to listener2. + cc2, err := grpc.Dial(lis2.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("Failed to dial local test server: %v", err) + } + defer cc2.Close() + + client2 := testgrpc.NewTestServiceClient(cc2) + sCtx, sCancel := context.WithTimeout(ctx, defaultTestShortTimeout) + defer sCancel() + if _, err := client2.EmptyCall(sCtx, &testpb.Empty{}); status.Code(err) != codes.DeadlineExceeded { + t.Fatalf("EmptyCall() failed: %v, wantCode: %s", err, codes.DeadlineExceeded) + } +} diff --git a/test/xds/xds_server_integration_test.go b/test/xds/xds_server_integration_test.go index 59d10e244b96..eaeeef13f814 100644 --- a/test/xds/xds_server_integration_test.go +++ b/test/xds/xds_server_integration_test.go @@ -50,6 +50,20 @@ func (*testService) UnaryCall(context.Context, *testpb.SimpleRequest) (*testpb.S return &testpb.SimpleResponse{}, nil } +func testModeChangeServerOption(t *testing.T) grpc.ServerOption { + // Create a server option to get notified about serving mode changes. We don't + // do anything other than throwing a log entry here. But this is required, + // since the server code emits a log entry at the default level (which is + // ERROR) if no callback is registered for serving mode changes. Our + // testLogger fails the test if there is any log entry at ERROR level. It does + // provide an ExpectError() method, but that takes a string and it would be + // painful to construct the exact error message expected here. Instead this + // works just fine. + return xds.ServingModeCallback(func(addr net.Addr, args xds.ServingModeChangeArgs) { + t.Logf("Serving mode for listener %q changed to %q, err: %v", addr.String(), args.Mode, args.Err) + }) +} + // setupGRPCServer performs the following: // - spin up an xDS-enabled gRPC server, configure it with xdsCredentials and // register the test service on it @@ -69,20 +83,8 @@ func setupGRPCServer(t *testing.T, bootstrapContents []byte) (net.Listener, func t.Fatal(err) } - // Create a server option to get notified about serving mode changes. We don't - // do anything other than throwing a log entry here. But this is required, - // since the server code emits a log entry at the default level (which is - // ERROR) if no callback is registered for serving mode changes. Our - // testLogger fails the test if there is any log entry at ERROR level. It does - // provide an ExpectError() method, but that takes a string and it would be - // painful to construct the exact error message expected here. Instead this - // works just fine. - modeChangeOpt := xds.ServingModeCallback(func(addr net.Addr, args xds.ServingModeChangeArgs) { - t.Logf("Serving mode for listener %q changed to %q, err: %v", addr.String(), args.Mode, args.Err) - }) - // Initialize an xDS-enabled gRPC server and register the stubServer on it. - server, err := xds.NewGRPCServer(grpc.Creds(creds), modeChangeOpt, xds.BootstrapContentsForTesting(bootstrapContents)) + server, err := xds.NewGRPCServer(grpc.Creds(creds), testModeChangeServerOption(t), xds.BootstrapContentsForTesting(bootstrapContents)) if err != nil { t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) } diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer.go b/xds/internal/balancer/cdsbalancer/cdsbalancer.go index 9eb27aa1d62a..b0f38aff8ee8 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer.go @@ -206,16 +206,8 @@ func (b *cdsBalancer) handleSecurityConfig(config *xdsresource.SecurityConfig) e } - bc := b.xdsClient.BootstrapConfig() - if bc == nil || bc.CertProviderConfigs == nil { - // Bootstrap did not find any certificate provider configs, but the user - // has specified xdsCredentials and the management server has sent down - // security configuration. - return fmt.Errorf("xds: certificate_providers config missing in bootstrap file") - } - cpc := bc.CertProviderConfigs - // A root provider is required whether we are using TLS or mTLS. + cpc := b.xdsClient.BootstrapConfig().CertProviderConfigs rootProvider, err := buildProvider(cpc, config.RootInstanceName, config.RootCertName, false, true) if err != nil { return err @@ -248,6 +240,9 @@ func (b *cdsBalancer) handleSecurityConfig(config *xdsresource.SecurityConfig) e func buildProviderFunc(configs map[string]*certprovider.BuildableConfig, instanceName, certName string, wantIdentity, wantRoot bool) (certprovider.Provider, error) { cfg, ok := configs[instanceName] if !ok { + // Defensive programming. If a resource received from the management + // server contains a certificate provider instance name that is not + // found in the bootstrap, the resource is NACKed by the xDS client. return nil, fmt.Errorf("certificate provider instance %q not found in bootstrap file", instanceName) } provider, err := cfg.Build(certprovider.BuildOptions{ diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go index a517aa2e4e55..d1b3195dd1fe 100644 --- a/xds/internal/resolver/xds_resolver.go +++ b/xds/internal/resolver/xds_resolver.go @@ -25,7 +25,6 @@ import ( "strings" "sync/atomic" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/grpcrand" @@ -142,23 +141,6 @@ func (r *xdsResolver) sanityChecksOnBootstrapConfig(target resolver.Target, opts return "", fmt.Errorf("xds: bootstrap configuration is empty") } - // If xDS credentials were specified by the user, but the bootstrap config - // does not contain any certificate providers, it is better to fail right - // now rather than failing when attempting to create certificate providers - // after receiving an CDS response with security configuration. - var creds credentials.TransportCredentials - switch { - case opts.DialCreds != nil: - creds = opts.DialCreds - case opts.CredsBundle != nil: - creds = opts.CredsBundle.TransportCredentials() - } - if xc, ok := creds.(interface{ UsesXDS() bool }); ok && xc.UsesXDS() { - if len(bootstrapConfig.CertProviderConfigs) == 0 { - return "", fmt.Errorf("xds: use of xDS credentials is specified, but certificate_providers config missing in bootstrap file") - } - } - // Find the client listener template to use from the bootstrap config: // - If authority is not set in the target, use the top level template // - If authority is set, use the template from the authority map. diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index 4030e4031f09..bb108fa69052 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -31,9 +31,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/uuid" "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" "google.golang.org/grpc/internal/grpcsync" iresolver "google.golang.org/grpc/internal/resolver" @@ -86,71 +83,36 @@ func (s) TestResolverBuilder_ClientCreationFails_NoBootstap(t *testing.T) { } } -// Tests the resolver builder's Build() method with different xDS bootstrap -// configurations. -func (s) TestResolverBuilder_DifferentBootstrapConfigs(t *testing.T) { - tests := []struct { - name string - target resolver.Target - buildOpts resolver.BuildOptions - bootstrapOpts xdsbootstrap.Options - wantErr string - }{ - { - name: "authority not defined in bootstrap", - target: resolver.Target{URL: *testutils.MustParseURL("xds://non-existing-authority/target")}, - bootstrapOpts: xdsbootstrap.Options{ - NodeID: "node-id", - ServerURI: "dummy-management-server", - }, - wantErr: `authority "non-existing-authority" specified in dial target "xds://non-existing-authority/target" is not found in the bootstrap file`, - }, - { - name: "xDS creds specified without certificate providers in bootstrap", - target: resolver.Target{URL: *testutils.MustParseURL("xds:///target")}, - buildOpts: resolver.BuildOptions{ - DialCreds: func() credentials.TransportCredentials { - creds, err := xdscreds.NewClientCredentials(xdscreds.ClientOptions{FallbackCreds: insecure.NewCredentials()}) - if err != nil { - t.Fatalf("xds.NewClientCredentials() failed: %v", err) - } - return creds - }(), - }, - bootstrapOpts: xdsbootstrap.Options{ - NodeID: "node-id", - ServerURI: "dummy-management-server", - }, - wantErr: `use of xDS credentials is specified, but certificate_providers config missing in bootstrap file`, - }, +// Tests the case where the specified dial target contains an authority that is +// not specified in the bootstrap file. Verifies that the resolver.Build method +// fails with the expected error string. +func (s) TestResolverBuilder_AuthorityNotDefinedInBootstrap(t *testing.T) { + bootstrapCleanup, err := xdsbootstrap.CreateFile(xdsbootstrap.Options{ + NodeID: "node-id", + ServerURI: "dummy-management-server", + }) + if err != nil { + t.Fatal(err) } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - // Create a bootstrap file and set the env var. - bootstrapCleanup, err := xdsbootstrap.CreateFile(test.bootstrapOpts) - if err != nil { - t.Fatal(err) - } - defer bootstrapCleanup() + defer bootstrapCleanup() - builder := resolver.Get(xdsresolver.Scheme) - if builder == nil { - t.Fatalf("Scheme %q is not registered", xdsresolver.Scheme) - } + builder := resolver.Get(xdsresolver.Scheme) + if builder == nil { + t.Fatalf("Scheme %q is not registered", xdsresolver.Scheme) + } - r, err := builder.Build(test.target, &testutils.ResolverClientConn{Logger: t}, test.buildOpts) - if gotErr, wantErr := err != nil, test.wantErr != ""; gotErr != wantErr { - t.Fatalf("xds Resolver Build(%v) returned err: %v, wantErr: %v", test.target, err, test.wantErr) - } - if test.wantErr != "" && !strings.Contains(err.Error(), test.wantErr) { - t.Fatalf("xds Resolver Build(%v) returned err: %v, wantErr: %v", test.target, err, test.wantErr) - } - if err != nil { - // This is the case where we expect an error and got it. - return - } - r.Close() - }) + target := resolver.Target{URL: *testutils.MustParseURL("xds://non-existing-authority/target")} + const wantErr = `authority "non-existing-authority" specified in dial target "xds://non-existing-authority/target" is not found in the bootstrap file` + + r, err := builder.Build(target, &testutils.ResolverClientConn{Logger: t}, resolver.BuildOptions{}) + if r != nil { + r.Close() + } + if err == nil { + t.Fatalf("xds Resolver Build(%v) succeeded for target with authority not specified in bootstrap", target) + } + if !strings.Contains(err.Error(), wantErr) { + t.Fatalf("xds Resolver Build(%v) returned err: %v, wantErr: %v", target, err, wantErr) } } diff --git a/xds/internal/server/conn_wrapper.go b/xds/internal/server/conn_wrapper.go index c622455455f6..97f22ce13941 100644 --- a/xds/internal/server/conn_wrapper.go +++ b/xds/internal/server/conn_wrapper.go @@ -19,7 +19,6 @@ package server import ( - "errors" "fmt" "net" "sync" @@ -92,15 +91,6 @@ func (c *connWrapper) GetDeadline() time.Time { // configuration for this connection. This method is invoked by the // ServerHandshake() method of the XdsCredentials. func (c *connWrapper) XDSHandshakeInfo() (*xdsinternal.HandshakeInfo, error) { - // Ideally this should never happen, since xdsCredentials are the only ones - // which will invoke this method at handshake time. But to be on the safe - // side, we avoid acting on the security configuration received from the - // control plane when the user has not configured the use of xDS - // credentials, by checking the value of this flag. - if !c.parent.xdsCredsInUse { - return nil, errors.New("user has not configured xDS credentials") - } - if c.filterChain.SecurityCfg == nil { // If the security config is empty, this means that the control plane // did not provide any security configuration and therefore we should @@ -145,6 +135,9 @@ func (c *connWrapper) Close() error { func buildProviderFunc(configs map[string]*certprovider.BuildableConfig, instanceName, certName string, wantIdentity, wantRoot bool) (certprovider.Provider, error) { cfg, ok := configs[instanceName] if !ok { + // Defensive programming. If a resource received from the management + // server contains a certificate provider instance name that is not + // found in the bootstrap, the resource is NACKed by the xDS client. return nil, fmt.Errorf("certificate provider instance %q not found in bootstrap file", instanceName) } provider, err := cfg.Build(certprovider.BuildOptions{ diff --git a/xds/internal/server/listener_wrapper.go b/xds/internal/server/listener_wrapper.go index 6ad159b4716d..0d1743196258 100644 --- a/xds/internal/server/listener_wrapper.go +++ b/xds/internal/server/listener_wrapper.go @@ -78,9 +78,6 @@ type ListenerWrapperParams struct { Listener net.Listener // ListenerResourceName is the xDS Listener resource to request. ListenerResourceName string - // XDSCredsInUse specifies whether or not the user expressed interest to - // receive security configuration from the control plane. - XDSCredsInUse bool // XDSClient provides the functionality from the XDSClient required here. XDSClient XDSClient // ModeCallback is the callback to invoke when the serving mode changes. @@ -99,7 +96,6 @@ func NewListenerWrapper(params ListenerWrapperParams) (net.Listener, <-chan stru lw := &listenerWrapper{ Listener: params.Listener, name: params.ListenerResourceName, - xdsCredsInUse: params.XDSCredsInUse, xdsC: params.XDSClient, modeCallback: params.ModeCallback, drainCallback: params.DrainCallback, @@ -135,7 +131,6 @@ type listenerWrapper struct { logger *internalgrpclog.PrefixLogger name string - xdsCredsInUse bool xdsC XDSClient cancelWatch func() modeCallback ServingModeCallback diff --git a/xds/server.go b/xds/server.go index 2396c3d9ccda..276c8d549e10 100644 --- a/xds/server.go +++ b/xds/server.go @@ -27,7 +27,6 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/connectivity" - "google.golang.org/grpc/credentials" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/buffer" @@ -54,7 +53,6 @@ var ( return grpc.NewServer(opts...) } - grpcGetServerCreds = internal.GetServerCredentials.(func(*grpc.Server) credentials.TransportCredentials) drainServerTransports = internal.DrainServerTransports.(func(*grpc.Server, string)) logger = grpclog.Component("xds") ) @@ -77,7 +75,6 @@ type GRPCServer struct { gs grpcServer quit *grpcsync.Event logger *internalgrpclog.PrefixLogger - xdsCredsInUse bool opts *serverOptions xdsC xdsclient.XDSClient xdsClientClose func() @@ -98,18 +95,6 @@ func NewGRPCServer(opts ...grpc.ServerOption) (*GRPCServer, error) { } s.handleServerOptions(opts) - // We type assert our underlying gRPC server to the real grpc.Server here - // before trying to retrieve the configured credentials. This approach - // avoids performing the same type assertion in the grpc package which - // provides the implementation for internal.GetServerCredentials, and allows - // us to use a fake gRPC server in tests. - if gs, ok := s.gs.(*grpc.Server); ok { - creds := grpcGetServerCreds(gs) - if xc, ok := creds.(interface{ UsesXDS() bool }); ok && xc.UsesXDS() { - s.xdsCredsInUse = true - } - } - // Initializing the xDS client upfront (instead of at serving time) // simplifies the code by eliminating the need for a mutex to protect the // xdsC and xdsClientClose fields. @@ -134,22 +119,11 @@ func NewGRPCServer(opts ...grpc.ServerOption) (*GRPCServer, error) { return nil, errors.New("missing server_listener_resource_name_template in the bootstrap configuration") } - // If xds credentials were specified by the user, but bootstrap configs do - // not contain any certificate provider configuration, it is better to fail - // right now rather than failing when attempting to create certificate - // providers after receiving an LDS response with security configuration. - if s.xdsCredsInUse { - if len(cfg.CertProviderConfigs) == 0 { - xdsClientClose() - return nil, fmt.Errorf("xds credentials are passed to the user, but certificate_providers config is missing in the bootstrap configuration") - } - } s.xdsC = xdsClient s.xdsClientClose = xdsClientClose s.logger = internalgrpclog.NewPrefixLogger(logger, fmt.Sprintf(serverPrefix, s)) s.logger.Infof("Created xds.GRPCServer") - s.logger.Infof("xDS credentials in use: %v", s.xdsCredsInUse) return s, nil } @@ -235,7 +209,6 @@ func (s *GRPCServer) Serve(lis net.Listener) error { lw, goodUpdateCh := server.NewListenerWrapper(server.ListenerWrapperParams{ Listener: lis, ListenerResourceName: name, - XDSCredsInUse: s.xdsCredsInUse, XDSClient: s.xdsC, ModeCallback: func(addr net.Addr, mode connectivity.ServingMode, err error) { modeUpdateCh.Put(&modeChangeArgs{ diff --git a/xds/server_test.go b/xds/server_test.go index 02dba9876807..19eb3e498967 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -165,10 +165,6 @@ func (s) TestNewServer_Success(t *testing.T) { t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) } defer s.Stop() - - if s.xdsCredsInUse != test.wantXDSCredsInUse { - t.Fatalf("xdsCredsInUse is %v, want %v", s.xdsCredsInUse, test.wantXDSCredsInUse) - } }) } } @@ -197,24 +193,6 @@ func (s) TestNewServer_Failure(t *testing.T) { }, wantErr: "xDS client creation failed", }, - { - desc: "certificate provider config is missing", - serverOpts: []grpc.ServerOption{ - grpc.Creds(xdsCreds), - func() grpc.ServerOption { - bs, err := bootstrap.Contents(bootstrap.Options{ - NodeID: uuid.New().String(), - ServerURI: nonExistentManagementServer, - ServerListenerResourceNameTemplate: e2e.ServerListenerResourceNameTemplate, - }) - if err != nil { - t.Errorf("Failed to create bootstrap configuration: %v", err) - } - return BootstrapContentsForTesting(bs) - }(), - }, - wantErr: "certificate_providers config is missing", - }, { desc: "server_listener_resource_name_template is missing", serverOpts: []grpc.ServerOption{