Skip to content

Commit

Permalink
grpc: better RFC 3986 compliant target parsing (#4817)
Browse files Browse the repository at this point in the history
  • Loading branch information
@easwars
easwars committed Oct 14, 2021
1 parent 45097a8 commit aaff9e7
Show file tree
Hide file tree
Showing 13 changed files with 314 additions and 244 deletions.
184 changes: 144 additions & 40 deletions clientconn.go
View file
Expand Up @@ -23,6 +23,7 @@ import (
"errors"
"fmt"
"math"
"net/url"
"reflect"
"strings"
"sync"
Expand All @@ -37,7 +38,6 @@ import (
"google.golang.org/grpc/internal/backoff"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/grpc/internal/grpcsync"
"google.golang.org/grpc/internal/grpcutil"
iresolver "google.golang.org/grpc/internal/resolver"
"google.golang.org/grpc/internal/transport"
"google.golang.org/grpc/keepalive"
Expand Down Expand Up @@ -248,38 +248,15 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
}

// Determine the resolver to use.
cc.parsedTarget = grpcutil.ParseTarget(cc.target, cc.dopts.copts.Dialer != nil)
channelz.Infof(logger, cc.channelzID, "parsed scheme: %q", cc.parsedTarget.Scheme)
resolverBuilder := cc.getResolver(cc.parsedTarget.Scheme)
if resolverBuilder == nil {
// If resolver builder is still nil, the parsed target's scheme is
// not registered. Fallback to default resolver and set Endpoint to
// the original target.
channelz.Infof(logger, cc.channelzID, "scheme %q not registered, fallback to default scheme", cc.parsedTarget.Scheme)
cc.parsedTarget = resolver.Target{
Scheme: resolver.GetDefaultScheme(),
Endpoint: target,
}
resolverBuilder = cc.getResolver(cc.parsedTarget.Scheme)
if resolverBuilder == nil {
return nil, fmt.Errorf("could not get resolver for default scheme: %q", cc.parsedTarget.Scheme)
}
resolverBuilder, err := cc.parseTargetAndFindResolver()
if err != nil {
return nil, err
}

creds := cc.dopts.copts.TransportCredentials
if creds != nil && creds.Info().ServerName != "" {
cc.authority = creds.Info().ServerName
} else if cc.dopts.insecure && cc.dopts.authority != "" {
cc.authority = cc.dopts.authority
} else if strings.HasPrefix(cc.target, "unix:") || strings.HasPrefix(cc.target, "unix-abstract:") {
cc.authority = "localhost"
} else if strings.HasPrefix(cc.parsedTarget.Endpoint, ":") {
cc.authority = "localhost" + cc.parsedTarget.Endpoint
} else {
// Use endpoint from "scheme://authority/endpoint" as the default
// authority for ClientConn.
cc.authority = cc.parsedTarget.Endpoint
cc.authority, err = determineAuthority(cc.parsedTarget.Endpoint, cc.target, cc.dopts)
if err != nil {
return nil, err
}
channelz.Infof(logger, cc.channelzID, "Channel authority set to %q", cc.authority)

if cc.dopts.scChan != nil && !scSet {
// Blocking wait for the initial service config.
Expand Down Expand Up @@ -902,10 +879,7 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool {
// ac.state is Ready, try to find the connected address.
var curAddrFound bool
for _, a := range addrs {
// a.ServerName takes precedent over ClientConn authority, if present.
if a.ServerName == "" {
a.ServerName = ac.cc.authority
}
a.ServerName = ac.cc.getServerName(a)
if reflect.DeepEqual(ac.curAddr, a) {
curAddrFound = true
break
Expand All @@ -919,6 +893,26 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool {
return curAddrFound
}

// getServerName determines the serverName to be used in the connection
// handshake. The default value for the serverName is the authority on the
// ClientConn, which either comes from the user's dial target or through an
// authority override specified using the WithAuthority dial option. Name
// resolvers can specify a per-address override for the serverName through the
// resolver.Address.ServerName field which is used only if the WithAuthority
// dial option was not used. The rationale is that per-address authority
// overrides specified by the name resolver can represent a security risk, while
// an override specified by the user is more dependable since they probably know
// what they are doing.
func (cc *ClientConn) getServerName(addr resolver.Address) string {
if cc.dopts.authority != "" {
return cc.dopts.authority
}
if addr.ServerName != "" {
return addr.ServerName
}
return cc.authority
}

func getMethodConfig(sc *ServiceConfig, method string) MethodConfig {
if sc == nil {
return MethodConfig{}
Expand Down Expand Up @@ -1275,11 +1269,7 @@ func (ac *addrConn) createTransport(addr resolver.Address, copts transport.Conne
prefaceReceived := grpcsync.NewEvent()
connClosed := grpcsync.NewEvent()

// addr.ServerName takes precedent over ClientConn authority, if present.
if addr.ServerName == "" {
addr.ServerName = ac.cc.authority
}

addr.ServerName = ac.cc.getServerName(addr)
hctx, hcancel := context.WithCancel(ac.ctx)
hcStarted := false // protected by ac.mu

Expand Down Expand Up @@ -1621,3 +1611,117 @@ func (cc *ClientConn) connectionError() error {
defer cc.lceMu.Unlock()
return cc.lastConnectionError
}

func (cc *ClientConn) parseTargetAndFindResolver() (resolver.Builder, error) {
channelz.Infof(logger, cc.channelzID, "original dial target is: %q", cc.target)

var rb resolver.Builder
parsedTarget, err := parseTarget(cc.target)
if err != nil {
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", cc.target, err)
} else {
channelz.Infof(logger, cc.channelzID, "parsed dial target is: %+v", parsedTarget)
rb = cc.getResolver(parsedTarget.Scheme)
if rb != nil {
cc.parsedTarget = parsedTarget
return rb, nil
}
}

// We are here because the user's dial target did not contain a scheme or
// specified an unregistered scheme. We should fallback to the default
// scheme, except when a custom dialer is specified in which case, we should
// always use passthrough scheme.
defScheme := resolver.GetDefaultScheme()
if cc.dopts.copts.Dialer != nil {
defScheme = "passthrough"
}
channelz.Infof(logger, cc.channelzID, "fallback to scheme %q", defScheme)
canonicalTarget := defScheme + ":///" + cc.target

parsedTarget, err = parseTarget(canonicalTarget)
if err != nil {
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", canonicalTarget, err)
return nil, err
}
channelz.Infof(logger, cc.channelzID, "parsed dial target is: %+v", parsedTarget)
rb = cc.getResolver(parsedTarget.Scheme)
if rb == nil {
return nil, fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.Scheme)
}
cc.parsedTarget = parsedTarget
return rb, nil
}

// parseTarget uses RFC 3986 semantics to parse the given target into a
// resolver.Target struct containing scheme, authority and endpoint. Query
// params are stripped from the endpoint.
func parseTarget(target string) (resolver.Target, error) {
u, err := url.Parse(target)
if err != nil {
return resolver.Target{}, err
}
// For targets of the form "[scheme]://[authority]/endpoint, the endpoint
// value returned from url.Parse() contains a leading "/". Although this is
// in accordance with RFC 3986, we do not want to break existing resolver
// implementations which expect the endpoint without the leading "/". So, we
// end up stripping the leading "/" here. But this will result in an
// incorrect parsing for something like "unix:///path/to/socket". Since we
// own the "unix" resolver, we can workaround in the unix resolver by using
// the `URL` field instead of the `Endpoint` field.
endpoint := u.Path
if endpoint == "" {
endpoint = u.Opaque
}
endpoint = strings.TrimPrefix(endpoint, "/")
return resolver.Target{
Scheme: u.Scheme,
Authority: u.Host,
Endpoint: endpoint,
URL: *u,
}, nil
}

// Determine channel authority. The order of precedence is as follows:
// - user specified authority override using `WithAuthority` dial option
// - creds' notion of server name for the authentication handshake
// - endpoint from dial target of the form "scheme://[authority]/endpoint"
func determineAuthority(endpoint, target string, dopts dialOptions) (string, error) {
// Historically, we had two options for users to specify the serverName or
// authority for a channel. One was through the transport credentials
// (either in its constructor, or through the OverrideServerName() method).
// The other option (for cases where WithInsecure() dial option was used)
// was to use the WithAuthority() dial option.
//
// A few things have changed since:
// - `insecure` package with an implementation of the `TransportCredentials`
// interface for the insecure case
// - WithAuthority() dial option support for secure credentials
authorityFromCreds := ""
if creds := dopts.copts.TransportCredentials; creds != nil && creds.Info().ServerName != "" {
authorityFromCreds = creds.Info().ServerName
}
authorityFromDialOption := dopts.authority
if (authorityFromCreds != "" && authorityFromDialOption != "") && authorityFromCreds != authorityFromDialOption {
return "", fmt.Errorf("ClientConn's authority from transport creds %q and dial option %q don't match", authorityFromCreds, authorityFromDialOption)
}

switch {
case authorityFromDialOption != "":
return authorityFromDialOption, nil
case authorityFromCreds != "":
return authorityFromCreds, nil
case strings.HasPrefix(target, "unix:") || strings.HasPrefix(target, "unix-abstract:"):
// TODO: remove when the unix resolver implements optional interface to
// return channel authority.
return "localhost", nil
case strings.HasPrefix(endpoint, ":"):
return "localhost" + endpoint, nil
default:
// TODO: Define an optional interface on the resolver builder to return
// the channel authority given the user's dial target. For resolvers
// which don't implement this interface, we will use the endpoint from
// "scheme://authority/endpoint" as the default authority.
return endpoint, nil
}
}
20 changes: 16 additions & 4 deletions clientconn_authority_test.go
View file
Expand Up @@ -59,10 +59,9 @@ func (s) TestClientConnAuthority(t *testing.T) {
wantAuthority: "authority-override",
},
{
name: "override-via-creds-and-WithAuthority",
target: "Non-Existent.Server:8080",
// WithAuthority override works only for insecure creds.
opts: []DialOption{WithTransportCredentials(creds), WithAuthority("authority-override")},
name: "override-via-creds-and-WithAuthority",
target: "Non-Existent.Server:8080",
opts: []DialOption{WithTransportCredentials(creds), WithAuthority(serverNameOverride)},
wantAuthority: serverNameOverride,
},
{
Expand Down Expand Up @@ -120,3 +119,16 @@ func (s) TestClientConnAuthority(t *testing.T) {
})
}
}

func (s) TestClientConnAuthority_CredsAndDialOptionMismatch(t *testing.T) {
serverNameOverride := "over.write.server.name"
creds, err := credentials.NewClientTLSFromFile(testdata.Path("x509/server_ca_cert.pem"), serverNameOverride)
if err != nil {
t.Fatalf("credentials.NewClientTLSFromFile(_, %q) failed: %v", err, serverNameOverride)
}
opts := []DialOption{WithTransportCredentials(creds), WithAuthority("authority-override")}
if cc, err := Dial("Non-Existent.Server:8000", opts...); err == nil {
cc.Close()
t.Fatal("grpc.Dial() succeeded when expected to fail")
}
}

0 comments on commit aaff9e7

Please sign in to comment.