Skip to content

Commit

Permalink
xds: validations for security config, as specified in A29 (#4762)
Browse files Browse the repository at this point in the history 
* xds: validations for security config, as specified in A29

* make vet happy

* fix error log

* fix error msg in test
  • Loading branch information
@easwars
easwars authored Sep 15, 2021
1 parent 4f093b9 commit 7cf9689
Show file tree
Hide file tree
Showing 4 changed files with 378 additions and 18 deletions.
240 changes: 240 additions & 0 deletions xds/internal/xdsclient/cds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package xdsclient

import (
"regexp"
"strings"
"testing"

v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2"
Expand Down Expand Up @@ -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"
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
11 changes: 11 additions & 0 deletions xds/internal/xdsclient/filter_chain.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
63 changes: 63 additions & 0 deletions xds/internal/xdsclient/filter_chain_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand Down
Loading

0 comments on commit 7cf9689

Please sign in to comment.