What version of gRPC are you using?

    sum = "h1:fPVVDxY9w++VjTZsYvXWqEf9Rqar/e+9zYfxKK+W+YU=",
    version = "v1.50.0",

What version of Go are you using (go version)?

go version go1.19.2 linux/amd64

What operating system (Linux, Windows, …) and version?

$ uname -r
5.11.0-051100-generic
$ cat /etc/os-release
NAME="Ubuntu"
VERSION="18.04.6 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.6 LTS"
VERSION_ID="18.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=bionic
UBUNTU_CODENAME=bionic

What did you do?

I tried to have a grpc server return different certificates based on server name (SNI). I use grpc_cli as the client. I ran
into some TLS problem and narrowed it down to the following. Everything is local. The only tls config set by me is
the server-side cert whose CN and SAN are both localhost.

As you can see, the same tls configuration is used whether or not it's from Certificates or GetConfigForClient.

func buildTLSConfigFromKeyPair(certPEM string, keyPEM string) (tls.Config, error) {
        cert, err := tls.X509KeyPair([]byte(certPEM), []byte(keyPEM))
        if err != nil {
                return tls.Config{}, err
        }

        tlsConfig := tls.Config{
                Certificates:             []tls.Certificate{cert},
        }

        // Make a copy of tlsConfig to start with.
        sniConfig := tlsConfig
        sniConfig.GetConfigForClient = func(helloInfo *tls.ClientHelloInfo) (*tls.Config, error) {
                if helloInfo.ServerName == "localhost" {
                        return &tlsConfig, nil
                }
                return nil, nil
        }
        return sniConfig, nil
}

The problem is that when I changed the code so that GetConfigClient is not invoked, grpc_cli doesn't return an error.
However, when GetConfigClient is invoked, the following error was returned.

Error code: 14, message: failed to connect to all addresses, debug info: {"created":"@1669180327.495657482","description":"Failed to pick subchannel","file":"/home/$USER/grpc/src/core/ext/filters/client_channel/client_channel.cc","file_line":3218,"referenced_errors":[{"created":"@1669180327.495654668","description":"failed to connect to all addresses","file":"/home/$USER/grpc/src/core/lib/transport/error_utils.cc","file_line":165,"grpc_status":14}]}

Turning on tracing, it comes down to.

Cannot check peer: missing selected ALPN property.

After some digging, I figured that when the supposed same tls config is passed to NewTLS before being passing to a grpc server,
the following happens. So "h2" is appeneded to NextProtos. That certainly doesn't happen when the config is returned from
GetConfigForClient. After adding "h2" to NextProtos manually in GetConfigForClient (NextProtos: []string{"h2"}), grpc_cli no longer returns errors.

func NewTLS(c *tls.Config) TransportCredentials {
	tc := &tlsCreds{credinternal.CloneTLSConfig(c)}
	tc.config.NextProtos = credinternal.AppendH2ToNextProtos(tc.config.NextProtos)
	return tc
}

What did you expect to see?

I expected the same behavior with the same tls.Config, no matter whether it's configured via grpc Server or GetConfigForClient.

What did you see instead?

Inconsistent behavior that was hard to debug caused by undocumented treatment by NewTLS().
I feel there was probably an issue reported so the special treatment was added.
I feel maybe the same treatment can be added to readClientHello (https://cs.opensource.google/go/go/+/refs/tags/go1.19.3:src/crypto/tls/handshake_server.go;l=130;drc=9efc5a5237c12b382b0383bd9fa3944c700bc899;bpv=0;bpt=1).

I feel at least the special treatment and the rationale can be documented for NewTLS() publicly.
Adding a warning to the commentary on GetConfigForClient() will also help.