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

grpcurl.go

@@ -20,6 +20,7 @@ import (
 	"slices"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/golang/protobuf/proto"   //lint:ignore SA1019 we have to import these because some of their types appear in exported API
 	"github.com/jhump/protoreflect/desc" //lint:ignore SA1019 same as above
@@ -733,23 +734,32 @@ func (c *errSignalingCreds) ClientHandshake(ctx context.Context, addr string, ra
 		c.writeResult(err)
 		return conn, auth, err
 	}
-	// Wrap the connection to capture post-handshake errors, e.g.:
+	// Wrap TLS connections to capture post-handshake errors. With TLS 1.3,
-	// - TLS 1.3 client cert rejection (server sends alert after handshake)
+	// client certificate rejection by the server happens after the client
-	// - Plaintext client to TLS server (server closes conn immediately)
+	// considers the handshake complete. The server's TLS alert surfaces on the
-	return &errSignalingConn{Conn: conn, writeResult: c.writeResult}, auth, nil
+	// 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.
+// report it via writeResult. This allows BlockingDial to surface post-handshake
+// errors.
 type errSignalingConn struct {
 	net.Conn
 	writeResult func(res interface{})
+	once        sync.Once
 }
 
 func (c *errSignalingConn) Read(b []byte) (int, error) {
 	n, err := c.Conn.Read(b)
 	if err != nil {
-		c.writeResult(err)
+		c.once.Do(func() {
+			c.writeResult(err)
+		})
 	}
 	return n, err
 }

tls_settings_test.go

@@ -171,17 +171,58 @@ func TestBrokenTLS_ClientPlainText(t *testing.T) {
 		t.Fatalf("failed to create server creds: %v", err)
 	}
 
-	// Plaintext client to TLS server: the server expects a TLS handshake,
+	// client connection (usually) succeeds since client is not waiting for TLS handshake
-	// gets an HTTP/2 preface instead, and closes the connection.
+	// (we try several times, but if we never get a connection and the error message is
-	e, err := createTestServerAndClient(serverCreds, nil)
+	// a known/expected possibility, we'll just bail)
-	if err == nil {
+	var e testEnv
-		e.Close()
+	failCount := 0
-		t.Fatal("expecting failure when connecting plaintext to TLS server")
+	for {
+		e, err = createTestServerAndClient(serverCreds, nil)
+		if err == nil {
+			// success!
+			defer e.Close()
+			break
+		}
+
+		if strings.Contains(err.Error(), "deadline exceeded") ||
+			strings.Contains(err.Error(), "use of closed network connection") {
+			// It is possible that the connection never becomes healthy:
+			//   1) grpc connects successfully
+			//   2) grpc client tries to send HTTP/2 preface and settings frame
+			//   3) server, expecting handshake, closes the connection
+			//   4) in the client, the write fails, so the connection never
+			//      becomes ready
+			// The client will attempt to reconnect on transient errors, so
+			// may eventually bump into the connect time limit. This used to
+			// result in a "deadline exceeded" error, but more recent versions
+			// of the grpc library report any underlying I/O error instead, so
+			// we also check for "use of closed network connection".
+			failCount++
+			if failCount > 5 {
+				return // bail...
+			}
+			// we'll try again
+
+		} else {
+			// some other error occurred, so we'll consider that a test failure
+			t.Fatalf("failed to setup server and client: %v", err)
+		}
 	}
-	if !strings.Contains(err.Error(), "EOF") &&
+
+	// but request fails because server closes connection upon seeing request
+	// bytes that are not a TLS handshake
+	cl := grpcurl_testing.NewTestServiceClient(e.cc)
+	_, err = cl.UnaryCall(context.Background(), &grpcurl_testing.SimpleRequest{})
+	if err == nil {
+		t.Fatal("expecting failure")
+	}
+	// various errors possible when server closes connection
+	if !strings.Contains(err.Error(), "transport is closing") &&
+		!strings.Contains(err.Error(), "connection is unavailable") &&
 		!strings.Contains(err.Error(), "use of closed network connection") &&
-		!strings.Contains(err.Error(), "connection reset by peer") {
+		!strings.Contains(err.Error(), "all SubConns are in TransientFailure") {
-		t.Fatalf("expecting connection closed error, got: %v", err)
+
+		t.Fatalf("expecting transport failure, got: %v", err)
 	}
 }