diff --git a/clientconn.go b/clientconn.go index d2717fba5..3f762285d 100644 --- a/clientconn.go +++ b/clientconn.go @@ -1824,7 +1824,7 @@ func (cc *ClientConn) initAuthority() error { } 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 4710a32b1..dba11dfd3 100644 --- a/credentials/alts/alts_test.go +++ b/credentials/alts/alts_test.go @@ -153,9 +153,6 @@ func (s) TestInfo(t *testing.T) { // 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 a63ab606e..c8e337cdd 100644 --- a/credentials/credentials.go +++ b/credentials/credentials.go @@ -96,10 +96,11 @@ func (c CommonAuthInfo) GetCommonAuthInfo() CommonAuthInfo { 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 @@ type ProtocolInfo struct { // // 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 @@ type TransportCredentials interface { // 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 20f65f7bd..8277be7d6 100644 --- a/credentials/tls.go +++ b/credentials/tls.go @@ -110,14 +110,14 @@ func (c tlsCreds) Info() ProtocolInfo { 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 @@ func applyDefaults(c *tls.Config) *tls.Config { // 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 @@ func NewClientTLSFromCert(cp *x509.CertPool, serverNameOverride string) Transpor // 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 3d07c0b56..023fbd4d3 100644 --- a/credentials/xds/xds_client_test.go +++ b/credentials/xds/xds_client_test.go @@ -47,7 +47,7 @@ const ( 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 @@ func Test(t *testing.T) { // 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 ec0ca89cc..7a5ac2e7c 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -608,6 +608,8 @@ func WithChainStreamInterceptor(interceptors ...StreamClientInterceptor) DialOpt // 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 7b50b0a72..0e19c86a2 100644 --- a/experimental/credentials/tls.go +++ b/experimental/credentials/tls.go @@ -56,14 +56,14 @@ func (c tlsCreds) Info() credentials.ProtocolInfo { 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 b84ef26d4..8e6af9514 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -332,6 +332,11 @@ type AuthorityOverrider interface { // 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 18c4085fc..38b6650d2 100755 --- a/scripts/vet.sh +++ b/scripts/vet.sh @@ -179,6 +179,7 @@ NewSubConn is deprecated: 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 ddd67aa2d..c1f7a11ff 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -430,11 +430,10 @@ func (c advancedTLSCreds) Info() credentials.ProtocolInfo { 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 7e3280d3d..ef0348d41 100644 --- a/test/balancer_test.go +++ b/test/balancer_test.go @@ -154,6 +154,7 @@ func (s) TestCredsBundleFromBalancer(t *testing.T) { 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 bedafa5b7..6ef62d961 100644 --- a/test/creds_test.go +++ b/test/creds_test.go @@ -83,6 +83,7 @@ func (s) TestCredsBundleBoth(t *testing.T) { 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 @@ func (s) TestCredsBundleTransportCredentials(t *testing.T) { 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 {