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.

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

Add auto-sni support #38238

Merged
merged 3 commits into from Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions pilot/pkg/networking/core/v1alpha3/cluster_builder.go
Expand Up @@ -1047,6 +1047,9 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
CommonTlsContext: defaultUpstreamCommonTLSContext(),
Sni: tls.Sni,
}
// Set auto-sni if sni field is not explicitly set
cb.setAutoSni(c, tls)

// Use subject alt names specified in service entry if TLS settings does not have subject alt names.
if opts.serviceRegistry == provider.External && len(tls.SubjectAltNames) == 0 {
tls.SubjectAltNames = opts.serviceAccounts
Expand Down Expand Up @@ -1083,6 +1086,10 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
CommonTlsContext: defaultUpstreamCommonTLSContext(),
Sni: tls.Sni,
}

// Set auto-sni if sni field is not explicitly set
cb.setAutoSni(c, tls)

// Use subject alt names specified in service entry if TLS settings does not have subject alt names.
if opts.serviceRegistry == provider.External && len(tls.SubjectAltNames) == 0 {
tls.SubjectAltNames = opts.serviceAccounts
Expand Down Expand Up @@ -1130,6 +1137,18 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
return tlsContext, nil
}

func (cb *ClusterBuilder) setAutoSni(mc *MutableCluster, tls *networking.ClientTLSSettings) {
if mc != nil && features.VerifyCertAtClient && len(tls.Sni) == 0 {
if mc.httpProtocolOptions == nil {
mc.httpProtocolOptions = &http.HttpProtocolOptions{}
}
if mc.httpProtocolOptions.UpstreamHttpProtocolOptions == nil {
mc.httpProtocolOptions.UpstreamHttpProtocolOptions = &core.UpstreamHttpProtocolOptions{}
}
mc.httpProtocolOptions.UpstreamHttpProtocolOptions.AutoSni = true
}
}

func (cb *ClusterBuilder) setUseDownstreamProtocol(mc *MutableCluster) {
if mc.httpProtocolOptions == nil {
mc.httpProtocolOptions = &http.HttpProtocolOptions{}
Expand Down
71 changes: 65 additions & 6 deletions pilot/pkg/networking/core/v1alpha3/cluster_builder_test.go
Expand Up @@ -1774,12 +1774,13 @@ func TestBuildUpstreamClusterTLSContext(t *testing.T) {
credentialName := "some-fake-credential"

testCases := []struct {
name string
opts *buildClusterOpts
tls *networking.ClientTLSSettings
h2 bool
router bool
result expectedResult
name string
opts *buildClusterOpts
tls *networking.ClientTLSSettings
h2 bool
router bool
result expectedResult
enableVerifyCertAtClient bool
}{
{
name: "tls mode disabled",
Expand Down Expand Up @@ -1967,6 +1968,56 @@ func TestBuildUpstreamClusterTLSContext(t *testing.T) {
err: nil,
},
},
{
name: "tls mode SIMPLE, with VerifyCert enabled and no sni specified in tls",
opts: &buildClusterOpts{
mutable: newTestCluster(),
},
tls: &networking.ClientTLSSettings{
Mode: networking.ClientTLSSettings_SIMPLE,
SubjectAltNames: []string{"SAN"},
},
result: expectedResult{
tlsContext: &tls.UpstreamTlsContext{
CommonTlsContext: &tls.CommonTlsContext{
TlsParams: &tls.TlsParameters{
// if not specified, envoy use TLSv1_2 as default for client.
TlsMaximumProtocolVersion: tls.TlsParameters_TLSv1_3,
TlsMinimumProtocolVersion: tls.TlsParameters_TLSv1_2,
},
ValidationContextType: &tls.CommonTlsContext_ValidationContext{},
},
},
err: nil,
},
enableVerifyCertAtClient: true,
},
{
name: "tls mode SIMPLE, with VerifyCert enabled and sni specified in tls",
opts: &buildClusterOpts{
mutable: newTestCluster(),
},
tls: &networking.ClientTLSSettings{
Mode: networking.ClientTLSSettings_SIMPLE,
SubjectAltNames: []string{"SAN"},
Sni: "some-sni.com",
},
result: expectedResult{
tlsContext: &tls.UpstreamTlsContext{
CommonTlsContext: &tls.CommonTlsContext{
TlsParams: &tls.TlsParameters{
// if not specified, envoy use TLSv1_2 as default for client.
TlsMaximumProtocolVersion: tls.TlsParameters_TLSv1_3,
TlsMinimumProtocolVersion: tls.TlsParameters_TLSv1_2,
},
ValidationContextType: &tls.CommonTlsContext_ValidationContext{},
},
Sni: "some-sni.com",
},
err: nil,
},
enableVerifyCertAtClient: true,
},
{
name: "tls mode SIMPLE, with certs specified in tls",
opts: &buildClusterOpts{
Expand Down Expand Up @@ -2512,6 +2563,7 @@ func TestBuildUpstreamClusterTLSContext(t *testing.T) {
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
features.VerifyCertAtClient = tc.enableVerifyCertAtClient
var proxy *model.Proxy
if tc.router {
proxy = newGatewayProxy()
Expand All @@ -2528,6 +2580,13 @@ func TestBuildUpstreamClusterTLSContext(t *testing.T) {
} else if diff := cmp.Diff(tc.result.tlsContext, ret, protocmp.Transform()); diff != "" {
t.Errorf("got diff: `%v", diff)
}
if tc.enableVerifyCertAtClient {
if len(tc.tls.Sni) == 0 {
assert.Equal(t, tc.opts.mutable.httpProtocolOptions.UpstreamHttpProtocolOptions.AutoSni, true)
} else if tc.opts.mutable.httpProtocolOptions != nil {
t.Errorf("expecting nil httpProtocolOptions but got %v", tc.opts.mutable.httpProtocolOptions)
}
}
})
}
}
Expand Down
9 changes: 9 additions & 0 deletions releasenotes/notes/auto-sni-support.yaml
@@ -0,0 +1,9 @@
apiVersion: release-notes/v2
kind: feature
area: traffic-management
releaseNotes:
- |
**Added** the ability to configure automatically set SNI when `DestinationRules`
do not specify it and `VERIFY_CLIENT_AT_CERT` is enabled.
docs:
- https://docs.google.com/document/d/1pTUl-Ng3nXAWJb7UGJtalftznpxQEfID/