Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Sep 15, 2021
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fabf975
xds: validations for security config, as specified in A29
easwars Sep 13, 2021
a9c022f
make vet happy
easwars Sep 13, 2021
9c43a04
fix error log
easwars Sep 14, 2021
1787418
fix error msg in test
easwars Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 240 additions & 0 deletions xds/internal/xdsclient/cds_test.go
View file
Open in desktop
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 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() succeded 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
Open in desktop
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() == true {
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
Open in desktop
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