credentials: fix behavior of grpc.WithAuthority and credential handshake precedence (#8488)
diff --git a/clientconn.go b/clientconn.go
index d2717fb..3f76228 100644
--- a/clientconn.go
+++ b/clientconn.go
@@ -1824,7 +1824,7 @@
} else if auth, ok := cc.resolverBuilder.(resolver.AuthorityOverrider); ok {
cc.authority = auth.OverrideAuthority(cc.parsedTarget)
} else if strings.HasPrefix(endpoint, ":") {
- cc.authority = "localhost" + endpoint
+ cc.authority = "localhost" + encodeAuthority(endpoint)
} else {
cc.authority = encodeAuthority(endpoint)
}
diff --git a/credentials/alts/alts_test.go b/credentials/alts/alts_test.go
index 4710a32..dba11df 100644
--- a/credentials/alts/alts_test.go
+++ b/credentials/alts/alts_test.go
@@ -153,9 +153,6 @@
// use NewServerCreds and not NewClientCreds.
c := NewServerCreds(DefaultServerOptions())
info := c.Info()
- if got, want := info.ProtocolVersion, ""; got != want {
- t.Errorf("info.ProtocolVersion=%v, want %v", got, want)
- }
if got, want := info.SecurityProtocol, "alts"; got != want {
t.Errorf("info.SecurityProtocol=%v, want %v", got, want)
}
diff --git a/credentials/credentials.go b/credentials/credentials.go
index a63ab60..c8e337c 100644
--- a/credentials/credentials.go
+++ b/credentials/credentials.go
@@ -96,10 +96,11 @@
return c
}
-// ProtocolInfo provides information regarding the gRPC wire protocol version,
-// security protocol, security protocol version in use, server name, etc.
+// ProtocolInfo provides static information regarding transport credentials.
type ProtocolInfo struct {
// ProtocolVersion is the gRPC wire protocol version.
+ //
+ // Deprecated: this is unused by gRPC.
ProtocolVersion string
// SecurityProtocol is the security protocol in use.
SecurityProtocol string
@@ -109,7 +110,16 @@
//
// Deprecated: please use Peer.AuthInfo.
SecurityVersion string
- // ServerName is the user-configured server name.
+ // ServerName is the user-configured server name. If set, this overrides
+ // the default :authority header used for all RPCs on the channel using the
+ // containing credentials, unless grpc.WithAuthority is set on the channel,
+ // in which case that setting will take precedence.
+ //
+ // This must be a valid `:authority` header according to
+ // [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986#section-3.2).
+ //
+ // Deprecated: Users should use grpc.WithAuthority to override the authority
+ // on a channel instead of configuring the credentials.
ServerName string
}
@@ -173,12 +183,17 @@
// Clone makes a copy of this TransportCredentials.
Clone() TransportCredentials
// OverrideServerName specifies the value used for the following:
+ //
// - verifying the hostname on the returned certificates
// - as SNI in the client's handshake to support virtual hosting
// - as the value for `:authority` header at stream creation time
//
- // Deprecated: use grpc.WithAuthority instead. Will be supported
- // throughout 1.x.
+ // The provided string should be a valid `:authority` header according to
+ // [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986#section-3.2).
+ //
+ // Deprecated: this method is unused by gRPC. Users should use
+ // grpc.WithAuthority to override the authority on a channel instead of
+ // configuring the credentials.
OverrideServerName(string) error
}
diff --git a/credentials/tls.go b/credentials/tls.go
index 20f65f7..8277be7 100644
--- a/credentials/tls.go
+++ b/credentials/tls.go
@@ -110,14 +110,14 @@
func (c *tlsCreds) ClientHandshake(ctx context.Context, authority string, rawConn net.Conn) (_ net.Conn, _ AuthInfo, err error) {
// use local cfg to avoid clobbering ServerName if using multiple endpoints
cfg := credinternal.CloneTLSConfig(c.config)
- if cfg.ServerName == "" {
- serverName, _, err := net.SplitHostPort(authority)
- if err != nil {
- // If the authority had no host port or if the authority cannot be parsed, use it as-is.
- serverName = authority
- }
- cfg.ServerName = serverName
+
+ serverName, _, err := net.SplitHostPort(authority)
+ if err != nil {
+ // If the authority had no host port or if the authority cannot be parsed, use it as-is.
+ serverName = authority
}
+ cfg.ServerName = serverName
+
conn := tls.Client(rawConn, cfg)
errChannel := make(chan error, 1)
go func() {
@@ -259,9 +259,11 @@
// certificates to establish the identity of the client need to be included in
// the credentials (eg: for mTLS), use NewTLS instead, where a complete
// tls.Config can be specified.
-// serverNameOverride is for testing only. If set to a non empty string,
-// it will override the virtual host name of authority (e.g. :authority header
-// field) in requests.
+//
+// serverNameOverride is for testing only. If set to a non empty string, it will
+// override the virtual host name of authority (e.g. :authority header field) in
+// requests. Users should use grpc.WithAuthority passed to grpc.NewClient to
+// override the authority of the client instead.
func NewClientTLSFromCert(cp *x509.CertPool, serverNameOverride string) TransportCredentials {
return NewTLS(&tls.Config{ServerName: serverNameOverride, RootCAs: cp})
}
@@ -271,9 +273,11 @@
// certificates to establish the identity of the client need to be included in
// the credentials (eg: for mTLS), use NewTLS instead, where a complete
// tls.Config can be specified.
-// serverNameOverride is for testing only. If set to a non empty string,
-// it will override the virtual host name of authority (e.g. :authority header
-// field) in requests.
+//
+// serverNameOverride is for testing only. If set to a non empty string, it will
+// override the virtual host name of authority (e.g. :authority header field) in
+// requests. Users should use grpc.WithAuthority passed to grpc.NewClient to
+// override the authority of the client instead.
func NewClientTLSFromFile(certFile, serverNameOverride string) (TransportCredentials, error) {
b, err := os.ReadFile(certFile)
if err != nil {
diff --git a/credentials/xds/xds_client_test.go b/credentials/xds/xds_client_test.go
index 3d07c0b..023fbd4 100644
--- a/credentials/xds/xds_client_test.go
+++ b/credentials/xds/xds_client_test.go
@@ -47,7 +47,7 @@
defaultTestTimeout = 1 * time.Second
defaultTestShortTimeout = 10 * time.Millisecond
defaultTestCertSAN = "abc.test.example.com"
- authority = "authority"
+ authority = "x.test.example.com"
)
type s struct {
@@ -61,7 +61,7 @@
// Helper function to create a real TLS client credentials which is used as
// fallback credentials from multiple tests.
func makeFallbackClientCreds(t *testing.T) credentials.TransportCredentials {
- creds, err := credentials.NewClientTLSFromFile(testdata.Path("x509/server_ca_cert.pem"), "x.test.example.com")
+ creds, err := credentials.NewClientTLSFromFile(testdata.Path("x509/server_ca_cert.pem"), "")
if err != nil {
t.Fatal(err)
}
diff --git a/dialoptions.go b/dialoptions.go
index ec0ca89..7a5ac2e 100644
--- a/dialoptions.go
+++ b/dialoptions.go
@@ -608,6 +608,8 @@
// WithAuthority returns a DialOption that specifies the value to be used as the
// :authority pseudo-header and as the server name in authentication handshake.
+// This overrides all other ways of setting authority on the channel, but can be
+// overridden per-call by using grpc.CallAuthority.
func WithAuthority(a string) DialOption {
return newFuncDialOption(func(o *dialOptions) {
o.authority = a
diff --git a/experimental/credentials/tls.go b/experimental/credentials/tls.go
index 7b50b0a..0e19c86 100644
--- a/experimental/credentials/tls.go
+++ b/experimental/credentials/tls.go
@@ -56,14 +56,14 @@
func (c *tlsCreds) ClientHandshake(ctx context.Context, authority string, rawConn net.Conn) (_ net.Conn, _ credentials.AuthInfo, err error) {
// use local cfg to avoid clobbering ServerName if using multiple endpoints
cfg := cloneTLSConfig(c.config)
- if cfg.ServerName == "" {
- serverName, _, err := net.SplitHostPort(authority)
- if err != nil {
- // If the authority had no host port or if the authority cannot be parsed, use it as-is.
- serverName = authority
- }
- cfg.ServerName = serverName
+
+ serverName, _, err := net.SplitHostPort(authority)
+ if err != nil {
+ // If the authority had no host port or if the authority cannot be parsed, use it as-is.
+ serverName = authority
}
+ cfg.ServerName = serverName
+
conn := tls.Client(rawConn, cfg)
errChannel := make(chan error, 1)
go func() {
diff --git a/resolver/resolver.go b/resolver/resolver.go
index b84ef26..8e6af95 100644
--- a/resolver/resolver.go
+++ b/resolver/resolver.go
@@ -332,6 +332,11 @@
// OverrideAuthority returns the authority to use for a ClientConn with the
// given target. The implementation must generate it without blocking,
// typically in line, and must keep it unchanged.
+ //
+ // The returned string must be a valid ":authority" header value, i.e. be
+ // encoded according to
+ // [RFC3986](https://datatracker.ietf.org/doc/html/rfc3986#section-3.2) as
+ // necessary.
OverrideAuthority(Target) string
}
diff --git a/scripts/vet.sh b/scripts/vet.sh
index 18c4085..38b6650 100755
--- a/scripts/vet.sh
+++ b/scripts/vet.sh
@@ -179,6 +179,7 @@
OverrideServerName is deprecated:
RemoveSubConn is deprecated:
SecurityVersion is deprecated:
+.ServerName is deprecated:
stats.PickerUpdated is deprecated:
Target is deprecated: Use the Target field in the BuildOptions instead.
UpdateAddresses is deprecated:
diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go
index ddd67aa..c1f7a11 100644
--- a/security/advancedtls/advancedtls.go
+++ b/security/advancedtls/advancedtls.go
@@ -430,11 +430,10 @@
func (c *advancedTLSCreds) ClientHandshake(ctx context.Context, authority string, rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) {
// Use local cfg to avoid clobbering ServerName if using multiple endpoints.
cfg := credinternal.CloneTLSConfig(c.config)
- // We return the full authority name to users if ServerName is empty without
- // stripping the trailing port.
- if cfg.ServerName == "" {
- cfg.ServerName = authority
- }
+ // We return the full authority name to users without stripping the trailing
+ // port.
+ cfg.ServerName = authority
+
peerVerifiedChains := CertificateChains{}
cfg.VerifyPeerCertificate = buildVerifyFunc(c, cfg.ServerName, rawConn, &peerVerifiedChains)
conn := tls.Client(rawConn, cfg)
diff --git a/test/balancer_test.go b/test/balancer_test.go
index 7e3280d..ef0348d 100644
--- a/test/balancer_test.go
+++ b/test/balancer_test.go
@@ -154,6 +154,7 @@
te.tapHandle = authHandle
te.customDialOptions = []grpc.DialOption{
grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, testBalancerName)),
+ grpc.WithAuthority("x.test.example.com"),
}
creds, err := credentials.NewServerTLSFromFile(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem"))
if err != nil {
diff --git a/test/creds_test.go b/test/creds_test.go
index bedafa5..6ef62d9 100644
--- a/test/creds_test.go
+++ b/test/creds_test.go
@@ -83,6 +83,7 @@
te.tapHandle = authHandle
te.customDialOptions = []grpc.DialOption{
grpc.WithCredentialsBundle(&testCredsBundle{t: t}),
+ grpc.WithAuthority("x.test.example.com"),
}
creds, err := credentials.NewServerTLSFromFile(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem"))
if err != nil {
@@ -107,6 +108,7 @@
te := newTest(t, env{name: "creds-bundle", network: "tcp", security: "empty"})
te.customDialOptions = []grpc.DialOption{
grpc.WithCredentialsBundle(&testCredsBundle{t: t, mode: bundleTLSOnly}),
+ grpc.WithAuthority("x.test.example.com"),
}
creds, err := credentials.NewServerTLSFromFile(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem"))
if err != nil {
