Add support for TLS 1.3 (#564)

* Add support for TLS 1.3

This PR allows TLS 1.3, by removing the MaxVersion in the client config.

This would silently swallow errors, so e.g. a client without cert
dialing a server that requires client certs would lead to an error which
gets ignored, leading to retries until timeout.

In this PR, we wrap the connection and if an error occurs we send it to
the existing `result` channel.

I think this matches @jhump's comment in https://github.com/fullstorydev/grpcurl/issues/387#issuecomment-1517098394

 **Testing**

```console
 # Start the test server (in another tab)
go run ./internal/testing/cmd/testserver \
    -cert internal/testing/tls/server.crt \
    -key internal/testing/tls/server.key \
    -cacert internal/testing/tls/ca.crt \
    -requirecert -p 9999

 # Old behavior
$ grpcurl -cacert internal/testing/tls/ca.crt \
    localhost:9999 list
Failed to dial target host "localhost:9999": context deadline exceeded

 # New behavior
$ go run ./cmd/grpcurl -cacert internal/testing/tls/ca.crt \
    localhost:9999 list
Failed to dial target host "localhost:9999": remote error: tls: certificate required
exit status 1
```

The old behavior is to hang until we hit the deadline. The new behavior
is to return immediately with an error.

Fixes #563

* Implement review comments

Removing unnecessary sync.Once, as the channel will already drop repeated calls.

grpcurl.go

@@ -568,9 +568,6 @@ func ClientTLSConfig(insecureSkipVerify bool, cacertFile, clientCertFile, client
 // client certs. The serverCertFile and serverKeyFile must both not be blank.
 func ServerTransportCredentials(cacertFile, serverCertFile, serverKeyFile string, requireClientCerts bool) (credentials.TransportCredentials, error) {
 	var tlsConf tls.Config
-	// TODO(jh): Remove this line once https://github.com/golang/go/issues/28779 is fixed
-	// in Go tip. Until then, the recently merged TLS 1.3 support breaks the TLS tests.
-	tlsConf.MaxVersion = tls.VersionTLS12
 
 	// Load the server certificates from disk
 	certificate, err := tls.LoadX509KeyPair(serverCertFile, serverKeyFile)
@@ -734,6 +731,31 @@ func (c *errSignalingCreds) ClientHandshake(ctx context.Context, addr string, ra
 	conn, auth, err := c.TransportCredentials.ClientHandshake(ctx, addr, rawConn)
 	if err != nil {
 		c.writeResult(err)
+		return conn, auth, err
 	}
-	return conn, auth, err
+	// Wrap TLS connections to capture post-handshake errors. With TLS 1.3,
+	// client certificate rejection by the server happens after the client
+	// considers the handshake complete. The server's TLS alert surfaces on the
+	// first Read from the connection. Only TLS connections need this (plaintext
+	// connections don't have post-handshake alerts).
+	if _, isTLS := auth.(credentials.TLSInfo); isTLS {
+		conn = &errSignalingConn{Conn: conn, writeResult: c.writeResult}
+	}
+	return conn, auth, nil
+}
+
+// errSignalingConn wraps a net.Conn to capture the first read error and
+// report it via writeResult. This allows BlockingDial to surface post-handshake
+// errors.
+type errSignalingConn struct {
+	net.Conn
+	writeResult func(res interface{})
+}
+
+func (c *errSignalingConn) Read(b []byte) (int, error) {
+	n, err := c.Conn.Read(b)
+	if err != nil {
+		c.writeResult(err)
+	}
+	return n, err
 }

tls_settings_test.go

@@ -2,6 +2,7 @@ package grpcurl_test
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net"
 	"strings"
@@ -10,6 +11,7 @@ import (
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/peer"
 
 	. "github.com/fullstorydev/grpcurl"
 	grpcurl_testing "github.com/fullstorydev/grpcurl/internal/testing"
@@ -101,6 +103,68 @@ func TestRequireClientCertTLS(t *testing.T) {
 	simpleTest(t, e.cc)
 }
 
+func TestTLS12(t *testing.T) {
+	serverCreds, err := ServerTransportCredentials("", "internal/testing/tls/server.crt", "internal/testing/tls/server.key", false)
+	if err != nil {
+		t.Fatalf("failed to create server creds: %v", err)
+	}
+	tlsConf, err := ClientTLSConfig(false, "internal/testing/tls/ca.crt", "", "")
+	if err != nil {
+		t.Fatalf("failed to create client TLS config: %v", err)
+	}
+	tlsConf.MaxVersion = tls.VersionTLS12
+
+	e, err := createTestServerAndClient(serverCreds, credentials.NewTLS(tlsConf))
+	if err != nil {
+		t.Fatalf("failed to setup server and client: %v", err)
+	}
+	defer e.Close()
+
+	tlsVersion := negotiatedTLSVersion(t, e.cc)
+	if tlsVersion != tls.VersionTLS12 {
+		t.Errorf("expected TLS 1.2, got 0x%04x", tlsVersion)
+	}
+}
+
+func TestTLS13(t *testing.T) {
+	serverCreds, err := ServerTransportCredentials("", "internal/testing/tls/server.crt", "internal/testing/tls/server.key", false)
+	if err != nil {
+		t.Fatalf("failed to create server creds: %v", err)
+	}
+	clientCreds, err := ClientTransportCredentials(false, "internal/testing/tls/ca.crt", "", "")
+	if err != nil {
+		t.Fatalf("failed to create client creds: %v", err)
+	}
+
+	e, err := createTestServerAndClient(serverCreds, clientCreds)
+	if err != nil {
+		t.Fatalf("failed to setup server and client: %v", err)
+	}
+	defer e.Close()
+
+	tlsVersion := negotiatedTLSVersion(t, e.cc)
+	if tlsVersion != tls.VersionTLS13 {
+		t.Errorf("expected TLS 1.3, got 0x%04x", tlsVersion)
+	}
+}
+
+func negotiatedTLSVersion(t *testing.T, cc *grpc.ClientConn) uint16 {
+	t.Helper()
+	cl := grpcurl_testing.NewTestServiceClient(cc)
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	var p peer.Peer
+	_, err := cl.UnaryCall(ctx, &grpcurl_testing.SimpleRequest{}, grpc.WaitForReady(true), grpc.Peer(&p))
+	if err != nil {
+		t.Fatalf("RPC failed: %v", err)
+	}
+	tlsInfo, ok := p.AuthInfo.(credentials.TLSInfo)
+	if !ok {
+		t.Fatalf("expected TLS auth info, got %T", p.AuthInfo)
+	}
+	return tlsInfo.State.Version
+}
+
 func TestBrokenTLS_ClientPlainText(t *testing.T) {
 	serverCreds, err := ServerTransportCredentials("", "internal/testing/tls/server.crt", "internal/testing/tls/server.key", false)
 	if err != nil {
@@ -253,12 +317,14 @@ func TestBrokenTLS_ClientNotTrusted(t *testing.T) {
 		e.Close()
 		t.Fatal("expecting TLS failure setting up server and client")
 	}
-	// Check for either the old error (Go <=1.24) or the new one (Go 1.25+)
-	// Go 1.24: "bad certificate"
-	// Go 1.25: "handshake failure"
+	// The exact TLS alert varies by Go version and TLS version negotiated:
+	// - TLS 1.2: "bad certificate" (Go <=1.24) or "handshake failure" (Go 1.25+)
+	// - TLS 1.3: "certificate required" (server rejects after handshake)
 	errMsg := err.Error()
-	if !strings.Contains(errMsg, "bad certificate") && !strings.Contains(errMsg, "handshake failure") {
-		t.Fatalf("expecting a specific TLS certificate or handshake error, got: %v", err)
+	if !strings.Contains(errMsg, "bad certificate") &&
+		!strings.Contains(errMsg, "handshake failure") &&
+		!strings.Contains(errMsg, "certificate required") {
+		t.Fatalf("expecting a TLS certificate error, got: %v", err)
 	}
 }
 
@@ -297,12 +363,14 @@ func TestBrokenTLS_RequireClientCertButNonePresented(t *testing.T) {
 		e.Close()
 		t.Fatal("expecting TLS failure setting up server and client")
 	}
-	// Check for either the old error (Go <=1.24) or the new one (Go 1.25+)
-	// Go 1.24: "bad certificate"
-	// Go 1.25: "handshake failure"
+	// The exact TLS alert varies by Go version and TLS version negotiated:
+	// - TLS 1.2: "bad certificate" (Go <=1.24) or "handshake failure" (Go 1.25+)
+	// - TLS 1.3: "certificate required" (server rejects after handshake)
 	errMsg := err.Error()
-	if !strings.Contains(errMsg, "bad certificate") && !strings.Contains(errMsg, "handshake failure") {
-		t.Fatalf("expecting a specific TLS certificate or handshake error, got: %v", err)
+	if !strings.Contains(errMsg, "bad certificate") &&
+		!strings.Contains(errMsg, "handshake failure") &&
+		!strings.Contains(errMsg, "certificate required") {
+		t.Fatalf("expecting a TLS certificate error, got: %v", err)
 	}
 }