Skip to content

Commit 71849e8

Browse files
xds: change clusterimpl to add SNI to handshake info (#9016)
This PR is the final PR for implementation of [gRFC A101](https://github.com/grpc/proposal/blob/master/A101-SNI-setting-and-SNI-SAN-validation.md) This PR does the following : - Change NewSubconn in ClusterImpl to add the hostname to attribute of the address so that it can be propagated to the ClientHandshake and retrieved there to decide the SNI which can be either DNS Hostname or endpoint Hostname if `AutoHostSni` is set , or the SNI received from control plane. This is done because each endpoint can have a different hostname. It is not a config that can be used across all endpoints of the cluster , so it cannot be set in handshake info. - Add `AutoHostSni` filed to the handshake info. - Adds a functions to set and get hostname from address attributes. As of now, there was a function to get the Hostname from address balancer.Attributes and set in endpoint atrributes. - Adds E2E tests to verify the complete SNI setting and validation flow. - Fix comments that mention CDS balancer creates handshake info. Note: We will turn the environment variable to true only after inter-op tests pass. RELEASE NOTES: - xds: add SNI support and SAN validation behind GRPC_EXPERIMENTAL_XDS_SNI ([gRFC A101](https://github.com/grpc/proposal/blob/master/A101-SNI-setting-and-SNI-SAN-validation.md))
1 parent 5fdb6d0 commit 71849e8

8 files changed

Lines changed: 1039 additions & 56 deletions

File tree

credentials/xds/xds.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,14 @@ func (c *credsImpl) ClientHandshake(ctx context.Context, authority string, rawCo
9595
return nil, nil, errors.New("ClientHandshake() is not supported for server credentials")
9696
}
9797

98-
// The CDS balancer constructs a new HandshakeInfo using a call to
98+
// The clusterimpl balancer constructs a new HandshakeInfo using a call to
9999
// NewHandshakeInfo(), and then adds it to the attributes field of the
100100
// resolver.Address when handling calls to NewSubConn(). The transport layer
101101
// takes care of shipping these attributes in the context to this handshake
102102
// function. We first read the credentials.ClientHandshakeInfo type from the
103-
// context, which contains the attributes added by the CDS balancer. We then
104-
// read the HandshakeInfo from the attributes to get to the actual data that
105-
// we need here for the handshake.
103+
// context, which contains the attributes added by the clusterimpl balancer.
104+
// We then read the HandshakeInfo from the attributes to get to the actual
105+
// data that we need here for the handshake.
106106
chi := credentials.ClientHandshakeInfoFromContext(ctx)
107107
// If there are no attributes in the received context or the attributes does
108108
// not contain a HandshakeInfo, it could either mean that the user did not
@@ -133,7 +133,8 @@ func (c *credsImpl) ClientHandshake(ctx context.Context, authority string, rawCo
133133
// 4. Key usage to match whether client/server usage.
134134
// 5. A `VerifyPeerCertificate` function which performs normal peer
135135
// cert verification using configured roots, and the custom SAN checks.
136-
cfg, err := hi.ClientSideTLSConfig(ctx)
136+
hostname := xdsinternal.Hostname(chi.Attributes)
137+
cfg, err := hi.ClientSideTLSConfig(ctx, hostname)
137138
if err != nil {
138139
return nil, nil, err
139140
}

credentials/xds/xds_client_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func newTestContextWithHandshakeInfo(parent context.Context, root, identity cert
229229
sms = []matcher.StringMatcher{matcher.NewExactStringMatcher(sanExactMatch, false)}
230230
}
231231
var hiPtr atomic.Pointer[xdsinternal.HandshakeInfo]
232-
info := xdsinternal.NewHandshakeInfo(root, identity, sms, false, sni, validateSANUsingSNI)
232+
info := xdsinternal.NewHandshakeInfo(root, identity, sms, false, sni, validateSANUsingSNI, false)
233233
hiPtr.Store(info)
234234
addr := xdsinternal.SetHandshakeInfo(resolver.Address{}, &hiPtr)
235235

@@ -618,7 +618,7 @@ func (s) TestClientCredsProviderSwitch(t *testing.T) {
618618
// Create a root provider which will fail the handshake because it does not
619619
// use the correct trust roots.
620620
root1 := makeRootProvider(t, "x509/client_ca_cert.pem")
621-
handshakeInfo := xdsinternal.NewHandshakeInfo(root1, nil, []matcher.StringMatcher{matcher.NewExactStringMatcher(defaultTestCertSAN, false)}, false, "", false)
621+
handshakeInfo := xdsinternal.NewHandshakeInfo(root1, nil, []matcher.StringMatcher{matcher.NewExactStringMatcher(defaultTestCertSAN, false)}, false, "", false, false)
622622
// We need to repeat most of what newTestContextWithHandshakeInfo() does
623623
// here because we need access to the underlying HandshakeInfo so that we
624624
// can update it before the next call to ClientHandshake().
@@ -645,7 +645,7 @@ func (s) TestClientCredsProviderSwitch(t *testing.T) {
645645
// Create a new root provider which uses the correct trust roots. And update
646646
// the HandshakeInfo with the new provider.
647647
root2 := makeRootProvider(t, "x509/server_ca_cert.pem")
648-
handshakeInfo = xdsinternal.NewHandshakeInfo(root2, nil, []matcher.StringMatcher{matcher.NewExactStringMatcher(defaultTestCertSAN, false)}, false, "", false)
648+
handshakeInfo = xdsinternal.NewHandshakeInfo(root2, nil, []matcher.StringMatcher{matcher.NewExactStringMatcher(defaultTestCertSAN, false)}, false, "", false, false)
649649
// Update the existing pointer, which address attribute will continue to
650650
// point to.
651651
hiPtr.Store(handshakeInfo)

credentials/xds/xds_server_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (s) TestServerCredsInvalidHandshakeInfo(t *testing.T) {
123123
t.Fatalf("NewServerCredentials(%v) failed: %v", opts, err)
124124
}
125125

126-
info := xdsinternal.NewHandshakeInfo(&fakeProvider{}, nil, nil, false, "", false)
126+
info := xdsinternal.NewHandshakeInfo(&fakeProvider{}, nil, nil, false, "", false, false)
127127
conn := newWrappedConn(nil, info, time.Time{})
128128
if _, _, err := creds.ServerHandshake(conn); err == nil {
129129
t.Fatal("ServerHandshake succeeded without identity certificate provider in HandshakeInfo")
@@ -159,7 +159,7 @@ func (s) TestServerCredsProviderFailure(t *testing.T) {
159159
}
160160
for _, test := range tests {
161161
t.Run(test.desc, func(t *testing.T) {
162-
info := xdsinternal.NewHandshakeInfo(test.rootProvider, test.identityProvider, nil, false, "", false)
162+
info := xdsinternal.NewHandshakeInfo(test.rootProvider, test.identityProvider, nil, false, "", false, false)
163163
conn := newWrappedConn(nil, info, time.Time{})
164164
if _, _, err := creds.ServerHandshake(conn); err == nil || !strings.Contains(err.Error(), test.wantErr) {
165165
t.Fatalf("ServerHandshake() returned error: %q, wantErr: %q", err, test.wantErr)
@@ -235,7 +235,7 @@ func (s) TestServerCredsHandshakeTimeout(t *testing.T) {
235235
// Create a test server which uses the xDS server credentials created above
236236
// to perform TLS handshake on incoming connections.
237237
ts := newTestServerWithHandshakeFunc(ctx, func(rawConn net.Conn) handshakeResult {
238-
hi := xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/client_ca_cert.pem"), makeIdentityProvider(t, "x509/server2_cert.pem", "x509/server2_key.pem"), nil, true, "", false)
238+
hi := xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/client_ca_cert.pem"), makeIdentityProvider(t, "x509/server2_cert.pem", "x509/server2_key.pem"), nil, true, "", false, false)
239239

240240
// Create a wrapped conn which can return the HandshakeInfo created
241241
// above with a very small deadline.
@@ -287,7 +287,7 @@ func (s) TestServerCredsHandshakeFailure(t *testing.T) {
287287
ts := newTestServerWithHandshakeFunc(ctx, func(rawConn net.Conn) handshakeResult {
288288
// Create a HandshakeInfo which has a root provider which does not match
289289
// the certificate sent by the client.
290-
hi := xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/server_ca_cert.pem"), makeIdentityProvider(t, "x509/client2_cert.pem", "x509/client2_key.pem"), nil, true, "", false)
290+
hi := xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/server_ca_cert.pem"), makeIdentityProvider(t, "x509/client2_cert.pem", "x509/client2_key.pem"), nil, true, "", false, false)
291291

292292
// Create a wrapped conn which can return the HandshakeInfo and
293293
// configured deadline to the xDS credentials' ServerHandshake()
@@ -368,7 +368,7 @@ func (s) TestServerCredsHandshakeSuccess(t *testing.T) {
368368
// created above to perform TLS handshake on incoming connections.
369369
ts := newTestServerWithHandshakeFunc(ctx, func(rawConn net.Conn) handshakeResult {
370370
// Create a HandshakeInfo with information from the test table.
371-
hi := xdsinternal.NewHandshakeInfo(test.rootProvider, test.identityProvider, nil, test.requireClientCert, "", false)
371+
hi := xdsinternal.NewHandshakeInfo(test.rootProvider, test.identityProvider, nil, test.requireClientCert, "", false, false)
372372

373373
// Create a wrapped conn which can return the HandshakeInfo and
374374
// configured deadline to the xDS credentials' ServerHandshake()
@@ -448,7 +448,7 @@ func (s) TestServerCredsProviderSwitch(t *testing.T) {
448448
if cnt == 1 {
449449
// Create a HandshakeInfo which has a root provider which does not match
450450
// the certificate sent by the client.
451-
hi = xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/server_ca_cert.pem"), makeIdentityProvider(t, "x509/client2_cert.pem", "x509/client2_key.pem"), nil, true, "", false)
451+
hi = xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/server_ca_cert.pem"), makeIdentityProvider(t, "x509/client2_cert.pem", "x509/client2_key.pem"), nil, true, "", false, false)
452452

453453
// Create a wrapped conn which can return the HandshakeInfo and
454454
// configured deadline to the xDS credentials' ServerHandshake()
@@ -462,7 +462,7 @@ func (s) TestServerCredsProviderSwitch(t *testing.T) {
462462
return handshakeResult{}
463463
}
464464

465-
hi = xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/client_ca_cert.pem"), makeIdentityProvider(t, "x509/server1_cert.pem", "x509/server1_key.pem"), nil, true, "", false)
465+
hi = xdsinternal.NewHandshakeInfo(makeRootProvider(t, "x509/client_ca_cert.pem"), makeIdentityProvider(t, "x509/server1_cert.pem", "x509/server1_key.pem"), nil, true, "", false, false)
466466

467467
// Create a wrapped conn which can return the HandshakeInfo and
468468
// configured deadline to the xDS credentials' ServerHandshake()

internal/credentials/xds/handshake_info.go

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,27 @@ func init() {
4545
// the Attributes field of resolver.Address.
4646
type handshakeAttrKey struct{}
4747

48+
// hostnameKey is the type used as the key to store the hostname in the
49+
// Attributes field of resolver.Address.
50+
type hostnameKey struct{}
51+
52+
// SetAddressHostname returns a copy of addr in which the Attributes field is
53+
// updated with the provided hostname.
54+
func SetAddressHostname(addr resolver.Address, hostname string) resolver.Address {
55+
addr.Attributes = addr.Attributes.WithValue(hostnameKey{}, hostname)
56+
return addr
57+
}
58+
59+
// Hostname returns the endpoint hostname stored in attr.
60+
func Hostname(attr *attributes.Attributes) string {
61+
if attr == nil {
62+
return ""
63+
}
64+
v := attr.Value(hostnameKey{})
65+
hn, _ := v.(string)
66+
return hn
67+
}
68+
4869
// Equal reports whether the handshake info structs are identical.
4970
func (hi *HandshakeInfo) Equal(other *HandshakeInfo) bool {
5071
if hi == nil && other == nil {
@@ -58,6 +79,7 @@ func (hi *HandshakeInfo) Equal(other *HandshakeInfo) bool {
5879
hi.requireClientCert != other.requireClientCert ||
5980
hi.sni != other.sni ||
6081
hi.validateSANUsingSNI != other.validateSANUsingSNI ||
82+
hi.useAutoHostSNI != other.useAutoHostSNI ||
6183
len(hi.sanMatchers) != len(other.sanMatchers) {
6284
return false
6385
}
@@ -95,18 +117,20 @@ type HandshakeInfo struct {
95117
requireClientCert bool // Only on server side.
96118
sni string // Only on client side, used for Server Name Indication in TLS handshake.
97119
validateSANUsingSNI bool // Only on client side, indicates whether to perform validation of SANs based on SNI value.
120+
useAutoHostSNI bool // Only on client side, indicates whether to use endpoint hostname as SNI.
98121
}
99122

100123
// NewHandshakeInfo returns a new handshake info configured with the provided
101124
// options.
102-
func NewHandshakeInfo(rootProvider certprovider.Provider, identityProvider certprovider.Provider, sanMatchers []matcher.StringMatcher, requireClientCert bool, sni string, validateSANUsingSNI bool) *HandshakeInfo {
125+
func NewHandshakeInfo(rootProvider certprovider.Provider, identityProvider certprovider.Provider, sanMatchers []matcher.StringMatcher, requireClientCert bool, sni string, validateSANUsingSNI bool, useAutoHostSNI bool) *HandshakeInfo {
103126
return &HandshakeInfo{
104127
rootProvider: rootProvider,
105128
identityProvider: identityProvider,
106129
sanMatchers: sanMatchers,
107130
requireClientCert: requireClientCert,
108131
sni: sni,
109132
validateSANUsingSNI: validateSANUsingSNI,
133+
useAutoHostSNI: useAutoHostSNI,
110134
}
111135
}
112136

@@ -127,7 +151,13 @@ func (hi *HandshakeInfo) GetSANMatchersForTesting() []matcher.StringMatcher {
127151

128152
// ClientSideTLSConfig constructs a tls.Config to be used in a client-side
129153
// handshake based on the contents of the HandshakeInfo.
130-
func (hi *HandshakeInfo) ClientSideTLSConfig(ctx context.Context) (*tls.Config, error) {
154+
//
155+
// hostname is passed as a parameter here instead of being part of the
156+
// HandshakeInfo because HandshakeInfo contains cluster-level security
157+
// configuration that applies to all endpoints in the cluster, while hostname is
158+
// specific to each endpoint. This allows sharing a single HandshakeInfo
159+
// instance across multiple endpoints in the same cluster.
160+
func (hi *HandshakeInfo) ClientSideTLSConfig(ctx context.Context, hostname string) (*tls.Config, error) {
131161
// On the client side, rootProvider is mandatory. IdentityProvider is
132162
// optional based on whether the client is doing TLS or mTLS.
133163
if hi.rootProvider == nil {
@@ -152,7 +182,17 @@ func (hi *HandshakeInfo) ClientSideTLSConfig(ctx context.Context) (*tls.Config,
152182
return nil, fmt.Errorf("xds: fetching trusted roots from CertificateProvider failed: %v", err)
153183
}
154184
cfg.RootCAs = km.Roots
155-
cfg.VerifyPeerCertificate = hi.buildVerifyFunc(km, true)
185+
186+
// If AutoHostSNI is true, and the endpoint hostname is present, we use the
187+
// endpoint hostname as the SNI value and also for SAN validation.
188+
// Otherwise, we use the SNI value from HandshakeInfo (which is configured
189+
// by the control plane) and validating SANs based on that.
190+
sni := hi.sni
191+
if hi.useAutoHostSNI && hostname != "" {
192+
sni = hostname
193+
}
194+
195+
cfg.VerifyPeerCertificate = hi.buildVerifyFunc(km, true, sni)
156196

157197
if idProv != nil {
158198
km, err := idProv.KeyMaterial(ctx)
@@ -162,13 +202,13 @@ func (hi *HandshakeInfo) ClientSideTLSConfig(ctx context.Context) (*tls.Config,
162202
cfg.Certificates = km.Certs
163203
}
164204

165-
if envconfig.XDSSNIEnabled && hi.sni != "" {
166-
cfg.ServerName = hi.sni
205+
if envconfig.XDSSNIEnabled && sni != "" {
206+
cfg.ServerName = sni
167207
}
168208
return cfg, nil
169209
}
170210

171-
func (hi *HandshakeInfo) buildVerifyFunc(km *certprovider.KeyMaterial, isClient bool) func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
211+
func (hi *HandshakeInfo) buildVerifyFunc(km *certprovider.KeyMaterial, isClient bool, sni string) func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
172212
return func(rawCerts [][]byte, _ [][]*x509.Certificate) error {
173213
// Parse all raw certificates presented by the peer.
174214
var certs []*x509.Certificate
@@ -216,14 +256,14 @@ func (hi *HandshakeInfo) buildVerifyFunc(km *certprovider.KeyMaterial, isClient
216256
// non-empty, validate only DNS SANs against the SNI. Otherwise, fallback to
217257
// validating all received SANs against the control plane provided SAN
218258
// matchers.
219-
if envconfig.XDSSNIEnabled && hi.validateSANUsingSNI && hi.sni != "" {
259+
if envconfig.XDSSNIEnabled && hi.validateSANUsingSNI && sni != "" {
220260
// Verify SAN of leaf certificate with SNI using exact DNS matcher.
221261
for _, san := range certs[0].DNSNames {
222-
if dnsMatch(hi.sni, san) {
262+
if dnsMatch(sni, san) {
223263
return nil
224264
}
225265
}
226-
return fmt.Errorf("xds: received DNS SANs: %v do not match the SNI: %v", certs[0].DNSNames, hi.sni)
266+
return fmt.Errorf("xds: received DNS SANs: %v do not match the SNI: %s", certs[0].DNSNames, sni)
227267
}
228268
// The SANs sent by the xDS control plane are encoded as SPIFFE IDs. We need to
229269
// only look at the SANs on the leaf cert.
@@ -272,7 +312,7 @@ func (hi *HandshakeInfo) ServerSideTLSConfig(ctx context.Context) (*tls.Config,
272312
// dropped to tls.RequireAnyClientCert so that custom verification
273313
// to use SPIFFE Bundles is done.
274314
cfg.ClientAuth = tls.RequireAnyClientCert
275-
cfg.VerifyPeerCertificate = hi.buildVerifyFunc(km, false)
315+
cfg.VerifyPeerCertificate = hi.buildVerifyFunc(km, false, "")
276316
} else {
277317
cfg.ClientCAs = km.Roots
278318
}

0 commit comments

Comments
 (0)