From cfddd5f7d3235fee86a9768ed0edca2d9161a2c8 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 5 Mar 2021 12:23:33 -0800 Subject: [PATCH 1/2] xds: add env var protection for client-side security --- xds/internal/client/cds_test.go | 58 +++++++++++++++++++++++++++++++++ xds/internal/client/xds.go | 13 ++++++-- xds/internal/env/env.go | 16 ++++++--- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/xds/internal/client/cds_test.go b/xds/internal/client/cds_test.go index 6ad0b0fa88a..c5f1d76d32c 100644 --- a/xds/internal/client/cds_test.go +++ b/xds/internal/client/cds_test.go @@ -223,7 +223,65 @@ func (s) TestValidateCluster_Success(t *testing.T) { } } +func (s) TestValidateClusterWithSecurityConfig_EnvVarOff(t *testing.T) { + // Turn off the env var protection for client-side security. + origClientSideSecurityEnvVar := env.ClientSideSecuritySupport + env.ClientSideSecuritySupport = false + defer func() { env.ClientSideSecuritySupport = origClientSideSecurityEnvVar }() + + cluster := &v3clusterpb.Cluster{ + ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS}, + EdsClusterConfig: &v3clusterpb.Cluster_EdsClusterConfig{ + EdsConfig: &v3corepb.ConfigSource{ + ConfigSourceSpecifier: &v3corepb.ConfigSource_Ads{ + Ads: &v3corepb.AggregatedConfigSource{}, + }, + }, + ServiceName: serviceName, + }, + LbPolicy: v3clusterpb.Cluster_ROUND_ROBIN, + TransportSocket: &v3corepb.TransportSocket{ + Name: "envoy.transport_sockets.tls", + ConfigType: &v3corepb.TransportSocket_TypedConfig{ + TypedConfig: &anypb.Any{ + TypeUrl: version.V3UpstreamTLSContextURL, + Value: func() []byte { + tls := &v3tlspb.UpstreamTlsContext{ + CommonTlsContext: &v3tlspb.CommonTlsContext{ + ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextCertificateProviderInstance{ + ValidationContextCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{ + InstanceName: "rootInstance", + CertificateName: "rootCert", + }, + }, + }, + } + mtls, _ := proto.Marshal(tls) + return mtls + }(), + }, + }, + }, + } + wantUpdate := ClusterUpdate{ + ServiceName: serviceName, + EnableLRS: false, + } + gotUpdate, err := validateCluster(cluster) + if err != nil { + t.Errorf("validateCluster() failed: %v", err) + } + if diff := cmp.Diff(wantUpdate, gotUpdate); diff != "" { + t.Errorf("validateCluster() returned unexpected diff (-want, got):\n%s", diff) + } +} + func (s) TestValidateClusterWithSecurityConfig(t *testing.T) { + // Turn on the env var protection for client-side security. + origClientSideSecurityEnvVar := env.ClientSideSecuritySupport + env.ClientSideSecuritySupport = true + defer func() { env.ClientSideSecuritySupport = origClientSideSecurityEnvVar }() + const ( identityPluginInstance = "identityPluginInstance" identityCertName = "identityCert" diff --git a/xds/internal/client/xds.go b/xds/internal/client/xds.go index 8fc50ea3056..471400ce9e3 100644 --- a/xds/internal/client/xds.go +++ b/xds/internal/client/xds.go @@ -567,10 +567,17 @@ func validateCluster(cluster *v3clusterpb.Cluster) (ClusterUpdate, error) { return emptyUpdate, fmt.Errorf("unexpected lbPolicy %v in response: %+v", cluster.GetLbPolicy(), cluster) } - sc, err := securityConfigFromCluster(cluster) - if err != nil { - return emptyUpdate, err + // Process security configuration received from the control plane iff the + // corresponding environment variable is set. + var sc *SecurityConfig + if env.ClientSideSecuritySupport { + var err error + sc, err = securityConfigFromCluster(cluster) + if err != nil { + return emptyUpdate, err + } } + return ClusterUpdate{ ServiceName: cluster.GetEdsClusterConfig().GetServiceName(), EnableLRS: cluster.GetLrsServer().GetSelf() != nil, diff --git a/xds/internal/env/env.go b/xds/internal/env/env.go index 065ce614563..a28d741f356 100644 --- a/xds/internal/env/env.go +++ b/xds/internal/env/env.go @@ -37,10 +37,11 @@ const ( // and kept in variable BootstrapFileName. // // When both bootstrap FileName and FileContent are set, FileName is used. - BootstrapFileContentEnv = "GRPC_XDS_BOOTSTRAP_CONFIG" - circuitBreakingSupportEnv = "GRPC_XDS_EXPERIMENTAL_CIRCUIT_BREAKING" - timeoutSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_TIMEOUT" - faultInjectionSupportEnv = "GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION" + BootstrapFileContentEnv = "GRPC_XDS_BOOTSTRAP_CONFIG" + circuitBreakingSupportEnv = "GRPC_XDS_EXPERIMENTAL_CIRCUIT_BREAKING" + timeoutSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_TIMEOUT" + faultInjectionSupportEnv = "GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION" + clientSideSecuritySupportEnv = "GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT" ) var ( @@ -67,4 +68,11 @@ var ( // FaultInjectionSupport is used to control both fault injection and HTTP // filter support. FaultInjectionSupport = strings.EqualFold(os.Getenv(faultInjectionSupportEnv), "true") + // ClientSideSecuritySupport is used to control processing of security + // configuration on the client-side. + // + // Note that there is no env var protection for the server-side because we + // have a brand new API on the server-side and users explicitly need to use + // the new API to get security integration on the server. + ClientSideSecuritySupport = strings.EqualFold(os.Getenv(clientSideSecuritySupportEnv), "true") ) From 39899c8a442efa2fb4dc230f15f07ffd4c59e244 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 5 Mar 2021 15:52:01 -0800 Subject: [PATCH 2/2] review comment --- xds/internal/client/xds.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xds/internal/client/xds.go b/xds/internal/client/xds.go index 471400ce9e3..78dc139e8fe 100644 --- a/xds/internal/client/xds.go +++ b/xds/internal/client/xds.go @@ -572,8 +572,7 @@ func validateCluster(cluster *v3clusterpb.Cluster) (ClusterUpdate, error) { var sc *SecurityConfig if env.ClientSideSecuritySupport { var err error - sc, err = securityConfigFromCluster(cluster) - if err != nil { + if sc, err = securityConfigFromCluster(cluster); err != nil { return emptyUpdate, err } }