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

clientconn: override authority with address's ServerName, if set #3073

Merged
merged 1 commit into from Oct 8, 2019
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
8 changes: 7 additions & 1 deletion clientconn.go
Expand Up @@ -1135,10 +1135,16 @@ func (ac *addrConn) createTransport(addr resolver.Address, copts transport.Conne
onCloseCalled := make(chan struct{})
reconnect := grpcsync.NewEvent()

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

target := transport.TargetInfo{
Addr: addr.Addr,
Metadata: addr.Metadata,
Authority: ac.cc.authority,
Authority: authority,
}

once := sync.Once{}
Expand Down
9 changes: 8 additions & 1 deletion resolver/resolver.go
Expand Up @@ -86,9 +86,16 @@ type Address struct {
// Type is the type of this address.
Type AddressType
// ServerName is the name of this address.
// If non-empty, the ServerName is used as the transport certification authority for
// the address, instead of the hostname from the Dial target string. In most cases,
// this should not be set.
//
// e.g. if Type is GRPCLB, ServerName should be the name of the remote load
// If Type is GRPCLB, ServerName should be the name of the remote load
// balancer, not the name of the backend.
//
// WARNING: ServerName must only be populated with trusted values. It
// is insecure to populate it with data from untrusted inputs since untrusted
// values could be used to bypass the authority checks performed by TLS.
ServerName string
// Metadata is the information associated with Addr, which may be used
// to make load balancing decision.
Expand Down
43 changes: 43 additions & 0 deletions test/end2end_test.go
Expand Up @@ -4968,6 +4968,49 @@ func (s) TestCredsHandshakeAuthority(t *testing.T) {
}
}

// This test makes sure that the authority client handshake gets is the endpoint
// of the ServerName of the address when it is set.
func (s) TestCredsHandshakeServerNameAuthority(t *testing.T) {
const testAuthority = "test.auth.ori.ty"
const testServerName = "test.server.name"

lis, err := net.Listen("tcp", "localhost:0")
if err != nil {
t.Fatalf("Failed to listen: %v", err)
}
cred := &authorityCheckCreds{}
s := grpc.NewServer()
go s.Serve(lis)
defer s.Stop()

r, rcleanup := manual.GenerateAndRegisterManualResolver()
defer rcleanup()

cc, err := grpc.Dial(r.Scheme()+":///"+testAuthority, grpc.WithTransportCredentials(cred))
if err != nil {
t.Fatalf("grpc.Dial(%q) = %v", lis.Addr().String(), err)
}
defer cc.Close()
r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: lis.Addr().String(), ServerName: testServerName}}})

ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
for {
s := cc.GetState()
if s == connectivity.Ready {
break
}
if !cc.WaitForStateChange(ctx, s) {
// ctx got timeout or canceled.
t.Fatalf("ClientConn is not ready after 100 ms")
}
}

if cred.got != testServerName {
t.Fatalf("client creds got authority: %q, want: %q", cred.got, testAuthority)
}
}

type clientFailCreds struct{}

func (c *clientFailCreds) ServerHandshake(rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) {
Expand Down