grpc
/
grpc-go
mirror of https://github.com/grpc/grpc-go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

xds: validations for security config, as specified in A29 (#4762) 

* xds: validations for security config, as specified in A29

* make vet happy

* fix error log

* fix error msg in test
This commit is contained in:
Easwar Swaminathan 2021-09-15 15:38:01 -07:00 committed by GitHub
parent 4f093b9a5a
commit 7cf9689be2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 377 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

240
xds/internal/xdsclient/cds_test.go
View File

@ -20,6 +20,7 @@ package xdsclient
import (
"regexp"
"strings"
"testing"
v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2"
@ -480,6 +481,176 @@ func (s) TestValidateClusterWithSecurityConfig_EnvVarOff(t *testing.T) {
}
}
func (s) TestSecurityConfigFromCommonTLSContextUsingNewFields_ErrorCases(t *testing.T) {
tests := []struct {
name string
common *v3tlspb.CommonTlsContext
server bool
wantErr string
}{
{
name: "unsupported-tls_certificates-field-for-identity-certs",
common: &v3tlspb.CommonTlsContext{
TlsCertificates: []*v3tlspb.TlsCertificate{
{CertificateChain: &v3corepb.DataSource{}},
},
},
wantErr: "unsupported field tls_certificates is set in CommonTlsContext message",
},
{
name: "unsupported-tls_certificates_sds_secret_configs-field-for-identity-certs",
common: &v3tlspb.CommonTlsContext{
TlsCertificateSdsSecretConfigs: []*v3tlspb.SdsSecretConfig{
{Name: "sds-secrets-config"},
},
},
wantErr: "unsupported field tls_certificate_sds_secret_configs is set in CommonTlsContext message",
},
{
name: "unsupported-sds-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextSdsSecretConfig{
ValidationContextSdsSecretConfig: &v3tlspb.SdsSecretConfig{
Name: "foo-sds-secret",
},
},
},
wantErr: "validation context contains unexpected type",
},
{
name: "missing-ca_certificate_provider_instance-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{},
},
},
wantErr: "expected field ca_certificate_provider_instance is missing in CommonTlsContext message",
},
{
name: "unsupported-field-verify_certificate_spki-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
VerifyCertificateSpki: []string{"spki"},
},
},
},
wantErr: "unsupported verify_certificate_spki field in CommonTlsContext message",
},
{
name: "unsupported-field-verify_certificate_hash-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
VerifyCertificateHash: []string{"hash"},
},
},
},
wantErr: "unsupported verify_certificate_hash field in CommonTlsContext message",
},
{
name: "unsupported-field-require_signed_certificate_timestamp-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
RequireSignedCertificateTimestamp: &wrapperspb.BoolValue{Value: true},
},
},
},
wantErr: "unsupported require_sugned_ceritificate_timestamp field in CommonTlsContext message",
},
{
name: "unsupported-field-crl-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
Crl: &v3corepb.DataSource{},
},
},
},
wantErr: "unsupported crl field in CommonTlsContext message",
},
{
name: "unsupported-field-custom_validator_config-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
CustomValidatorConfig: &v3corepb.TypedExtensionConfig{},
},
},
},
wantErr: "unsupported custom_validator_config field in CommonTlsContext message",
},
{
name: "invalid-match_subject_alt_names-field-in-validation-context",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
MatchSubjectAltNames: []*v3matcherpb.StringMatcher{
{MatchPattern: &v3matcherpb.StringMatcher_Prefix{Prefix: ""}},
},
},
},
},
wantErr: "empty prefix is not allowed in StringMatcher",
},
{
name: "unsupported-field-matching-subject-alt-names-in-validation-context-of-server",
common: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContext{
ValidationContext: &v3tlspb.CertificateValidationContext{
CaCertificateProviderInstance: &v3tlspb.CertificateProviderPluginInstance{
InstanceName: "rootPluginInstance",
CertificateName: "rootCertName",
},
MatchSubjectAltNames: []*v3matcherpb.StringMatcher{
{MatchPattern: &v3matcherpb.StringMatcher_Prefix{Prefix: "sanPrefix"}},
},
},
},
},
server: true,
wantErr: "match_subject_alt_names field in validation context is not supported on the server",
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
_, err := securityConfigFromCommonTLSContextUsingNewFields(test.common, test.server)
if err == nil {
t.Fatal("securityConfigFromCommonTLSContextUsingNewFields() succeeded when expected to fail")
}
if !strings.Contains(err.Error(), test.wantErr) {
t.Fatalf("securityConfigFromCommonTLSContextUsingNewFields() returned err: %v, wantErr: %v", err, test.wantErr)
}
})
}
}
func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
const (
identityPluginInstance = "identityPluginInstance"
@ -503,6 +674,25 @@ func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
wantUpdate ClusterUpdate
wantErr bool
}{
{
name: "transport-socket-matches",
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,
TransportSocketMatches: []*v3clusterpb.Cluster_TransportSocketMatch{
{Name: "transport-socket-match-1"},
},
},
wantErr: true,
},
{
name: "transport-socket-unsupported-name",
cluster: &v3clusterpb.Cluster{
@ -574,6 +764,56 @@ func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
},
wantErr: true,
},
{
name: "transport-socket-unsupported-tls-params-field",
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{
ConfigType: &v3corepb.TransportSocket_TypedConfig{
TypedConfig: testutils.MarshalAny(&v3tlspb.UpstreamTlsContext{
CommonTlsContext: &v3tlspb.CommonTlsContext{
TlsParams: &v3tlspb.TlsParameters{},
},
}),
},
},
},
wantErr: true,
},
{
name: "transport-socket-unsupported-custom-handshaker-field",
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{
ConfigType: &v3corepb.TransportSocket_TypedConfig{
TypedConfig: testutils.MarshalAny(&v3tlspb.UpstreamTlsContext{
CommonTlsContext: &v3tlspb.CommonTlsContext{
CustomHandshaker: &v3corepb.TypedExtensionConfig{},
},
}),
},
},
},
wantErr: true,
},
{
name: "transport-socket-unsupported-validation-context",
cluster: &v3clusterpb.Cluster{

11
xds/internal/xdsclient/filter_chain.go
View File

@ -519,6 +519,17 @@ func (fci *FilterChainManager) filterChainFromProto(fc *v3listenerpb.FilterChain
if err := proto.Unmarshal(any.GetValue(), downstreamCtx); err != nil {
return nil, fmt.Errorf("failed to unmarshal DownstreamTlsContext in LDS response: %v", err)
}
if downstreamCtx.GetRequireSni().GetValue() {
return nil, fmt.Errorf("require_sni field set to true in DownstreamTlsContext message: %v", downstreamCtx)
}
if downstreamCtx.GetOcspStaplePolicy() != v3tlspb.DownstreamTlsContext_LENIENT_STAPLING {
return nil, fmt.Errorf("ocsp_staple_policy field set to unsupported value in DownstreamTlsContext message: %v", downstreamCtx)
}
// The following fields from `DownstreamTlsContext` are ignore:
// - disable_stateless_session_resumption
// - session_ticket_keys
// - session_ticket_keys_sds_secret_config
// - session_timeout
if downstreamCtx.GetCommonTlsContext() == nil {
return nil, errors.New("DownstreamTlsContext in LDS response does not contain a CommonTlsContext")
}

63
xds/internal/xdsclient/filter_chain_test.go
View File

@ -372,6 +372,44 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
},
wantErr: "DownstreamTlsContext in LDS response does not contain a CommonTlsContext",
},
{
desc: "require_sni-set-to-true-in-downstreamTlsContext",
lis: &v3listenerpb.Listener{
FilterChains: []*v3listenerpb.FilterChain{
{
TransportSocket: &v3corepb.TransportSocket{
Name: "envoy.transport_sockets.tls",
ConfigType: &v3corepb.TransportSocket_TypedConfig{
TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
RequireSni: &wrapperspb.BoolValue{Value: true},
}),
},
},
Filters: emptyValidNetworkFilters,
},
},
},
wantErr: "require_sni field set to true in DownstreamTlsContext message",
},
{
desc: "unsupported-ocsp_staple_policy-in-downstreamTlsContext",
lis: &v3listenerpb.Listener{
FilterChains: []*v3listenerpb.FilterChain{
{
TransportSocket: &v3corepb.TransportSocket{
Name: "envoy.transport_sockets.tls",
ConfigType: &v3corepb.TransportSocket_TypedConfig{
TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
OcspStaplePolicy: v3tlspb.DownstreamTlsContext_STRICT_STAPLING,
}),
},
},
Filters: emptyValidNetworkFilters,
},
},
},
wantErr: "ocsp_staple_policy field set to unsupported value in DownstreamTlsContext message",
},
{
desc: "unsupported validation context in transport socket",
lis: &v3listenerpb.Listener{
@ -397,6 +435,31 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) {
},
wantErr: "validation context contains unexpected type",
},
{
desc: "unsupported match_subject_alt_names field in transport socket",
lis: &v3listenerpb.Listener{
FilterChains: []*v3listenerpb.FilterChain{
{
TransportSocket: &v3corepb.TransportSocket{
Name: "envoy.transport_sockets.tls",
ConfigType: &v3corepb.TransportSocket_TypedConfig{
TypedConfig: testutils.MarshalAny(&v3tlspb.DownstreamTlsContext{
CommonTlsContext: &v3tlspb.CommonTlsContext{
ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextSdsSecretConfig{
ValidationContextSdsSecretConfig: &v3tlspb.SdsSecretConfig{
Name: "foo-sds-secret",
},
},
},
}),
},
},
Filters: emptyValidNetworkFilters,
},
},
},
wantErr: "validation context contains unexpected type",
},
{
desc: "no root certificate provider with require_client_cert",
lis: &v3listenerpb.Listener{

80
xds/internal/xdsclient/xds.go
View File

@ -794,6 +794,9 @@ func dnsHostNameFromCluster(cluster *v3clusterpb.Cluster) (string, error) {
// securityConfigFromCluster extracts the relevant security configuration from
// the received Cluster resource.
func securityConfigFromCluster(cluster *v3clusterpb.Cluster) (*SecurityConfig, error) {
if tsm := cluster.GetTransportSocketMatches(); len(tsm) != 0 {
return nil, fmt.Errorf("unsupport transport_socket_matches field is non-empty: %+v", tsm)
}
// The Cluster resource contains a `transport_socket` field, which contains
// a oneof `typed_config` field of type `protobuf.Any`. The any proto
// contains a marshaled representation of an `UpstreamTlsContext` message.
@ -812,6 +815,10 @@ func securityConfigFromCluster(cluster *v3clusterpb.Cluster) (*SecurityConfig, e
if err := proto.Unmarshal(any.GetValue(), upstreamCtx); err != nil {
return nil, fmt.Errorf("failed to unmarshal UpstreamTlsContext in CDS response: %v", err)
}
// The following fields from `UpstreamTlsContext` are ignored:
// - sni
// - allow_renegotiation
// - max_session_keys
if upstreamCtx.GetCommonTlsContext() == nil {
return nil, errors.New("UpstreamTlsContext in CDS response does not contain a CommonTlsContext")
}
@ -820,16 +827,22 @@ func securityConfigFromCluster(cluster *v3clusterpb.Cluster) (*SecurityConfig, e
}
// common is expected to be not nil.
// The `alpn_protocols` field is ignored.
func securityConfigFromCommonTLSContext(common *v3tlspb.CommonTlsContext, server bool) (*SecurityConfig, error) {
sc, err := securityConfigFromCommonTLSContextUsingNewFields(common)
if err != nil {
return nil, err
if common.GetTlsParams() != nil {
return nil, fmt.Errorf("unsupported tls_params field in CommonTlsContext message: %+v", common)
}
if common.GetCustomHandshaker() != nil {
return nil, fmt.Errorf("unsupported custom_handshaker field in CommonTlsContext message: %+v", common)
}
// For now, if we can't get a valid security config from the new fields, we
// fallback to the old deprecated fields.
// TODO: Drop support for deprecated fields. NACK if err != nil here.
sc, _ := securityConfigFromCommonTLSContextUsingNewFields(common, server)
if sc == nil || sc.Equal(&SecurityConfig{}) {
// If we can't get a valid security config from the new fields, we
// fallback to the old deprecated fields.
// TODO(easwars): Remove this once TD starts populating the new fields.
sc, err = securityConfigFromCommonTLSContextWithDeprecatedFields(common)
var err error
sc, err = securityConfigFromCommonTLSContextWithDeprecatedFields(common, server)
if err != nil {
return nil, err
}
@ -850,7 +863,7 @@ func securityConfigFromCommonTLSContext(common *v3tlspb.CommonTlsContext, server
return sc, nil
}
func securityConfigFromCommonTLSContextWithDeprecatedFields(common *v3tlspb.CommonTlsContext) (*SecurityConfig, error) {
func securityConfigFromCommonTLSContextWithDeprecatedFields(common *v3tlspb.CommonTlsContext, server bool) (*SecurityConfig, error) {
// The `CommonTlsContext` contains a
// `tls_certificate_certificate_provider_instance` field of type
// `CertificateProviderInstance`, which contains the provider instance name
@ -883,6 +896,9 @@ func securityConfigFromCommonTLSContextWithDeprecatedFields(common *v3tlspb.Comm
matchers = append(matchers, matcher)
}
}
if server && len(matchers) != 0 {
return nil, fmt.Errorf("match_subject_alt_names field in validation context is not supported on the server: %v", common)
}
sc.SubjectAltNameMatchers = matchers
if pi := combined.GetValidationContextCertificateProviderInstance(); pi != nil {
sc.RootInstanceName = pi.GetInstanceName()
@ -909,15 +925,20 @@ func securityConfigFromCommonTLSContextWithDeprecatedFields(common *v3tlspb.Comm
//
// This helper function attempts to fetch security configuration from the
// `CertificateProviderPluginInstance` message, given a CommonTlsContext.
func securityConfigFromCommonTLSContextUsingNewFields(common *v3tlspb.CommonTlsContext) (*SecurityConfig, error) {
func securityConfigFromCommonTLSContextUsingNewFields(common *v3tlspb.CommonTlsContext, server bool) (*SecurityConfig, error) {
// The `tls_certificate_provider_instance` field of type
// `CertificateProviderPluginInstance` is used to fetch the identity
// certificate provider.
sc := &SecurityConfig{}
if identity := common.GetTlsCertificateProviderInstance(); identity != nil {
sc.IdentityInstanceName = identity.GetInstanceName()
sc.IdentityCertName = identity.GetCertificateName()
identity := common.GetTlsCertificateProviderInstance()
if identity == nil && len(common.GetTlsCertificates()) != 0 {
return nil, fmt.Errorf("expected field tls_certificate_provider_instance is not set, while unsupported field tls_certificates is set in CommonTlsContext message: %+v", common)
}
if identity == nil && common.GetTlsCertificateSdsSecretConfigs() != nil {
return nil, fmt.Errorf("expected field tls_certificate_provider_instance is not set, while unsupported field tls_certificate_sds_secret_configs is set in CommonTlsContext message: %+v", common)
}
sc.IdentityInstanceName = identity.GetInstanceName()
sc.IdentityCertName = identity.GetCertificateName()
// The `CommonTlsContext` contains a oneof field `validation_context_type`,
// which contains the `CertificateValidationContext` message in one of the
@ -943,7 +964,7 @@ func securityConfigFromCommonTLSContextUsingNewFields(common *v3tlspb.CommonTlsC
// - certificate_name
// - this is an opaque name passed to the certificate provider
var validationCtx *v3tlspb.CertificateValidationContext
switch common.GetValidationContextType().(type) {
switch typ := common.GetValidationContextType().(type) {
case *v3tlspb.CommonTlsContext_ValidationContext:
validationCtx = common.GetValidationContext()
case *v3tlspb.CommonTlsContext_CombinedValidationContext:
@ -951,11 +972,33 @@ func securityConfigFromCommonTLSContextUsingNewFields(common *v3tlspb.CommonTlsC
case nil:
// It is valid for the validation context to be nil on the server side.
return sc, nil
default:
return nil, fmt.Errorf("validation context contains unexpected type: %T", typ)
}
if validationCtx == nil || validationCtx.GetCaCertificateProviderInstance() == nil {
// Bail out if the `CertificateProviderPluginInstance` message is not
// found through one of the way detailed above.
return nil, nil
// If we get here, it means that the `CertificateValidationContext` message
// was found through one of the supported ways. It is an error if the
// validation context is specified, but it does not contain the
// ca_certificate_provider_instance field which contains information about
// the certificate provider to be used for the root certificates.
if validationCtx.GetCaCertificateProviderInstance() == nil {
return nil, fmt.Errorf("expected field ca_certificate_provider_instance is missing in CommonTlsContext message: %+v", common)
}
// The following fields are ignored:
// - trusted_ca
// - watched_directory
// - allow_expired_certificate
// - trust_chain_verification
switch {
case len(validationCtx.GetVerifyCertificateSpki()) != 0:
return nil, fmt.Errorf("unsupported verify_certificate_spki field in CommonTlsContext message: %+v", common)
case len(validationCtx.GetVerifyCertificateHash()) != 0:
return nil, fmt.Errorf("unsupported verify_certificate_hash field in CommonTlsContext message: %+v", common)
case validationCtx.GetRequireSignedCertificateTimestamp().GetValue():
return nil, fmt.Errorf("unsupported require_sugned_ceritificate_timestamp field in CommonTlsContext message: %+v", common)
case validationCtx.GetCrl() != nil:
return nil, fmt.Errorf("unsupported crl field in CommonTlsContext message: %+v", common)
case validationCtx.GetCustomValidatorConfig() != nil:
return nil, fmt.Errorf("unsupported custom_validator_config field in CommonTlsContext message: %+v", common)
}
if rootProvider := validationCtx.GetCaCertificateProviderInstance(); rootProvider != nil {
@ -970,6 +1013,9 @@ func securityConfigFromCommonTLSContextUsingNewFields(common *v3tlspb.CommonTlsC
}
matchers = append(matchers, matcher)
}
if server && len(matchers) != 0 {
return nil, fmt.Errorf("match_subject_alt_names field in validation context is not supported on the server: %v", common)
}
sc.SubjectAltNameMatchers = matchers
return sc, nil
}