crypto/tls: Dial to TLS endpoint requiring ClientAuth does not error

[domodwyer]

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

$ go1.13beta1 version
go version go1.13beta1 darwin/amd64

$ go version
go version go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?

Error behaviour has changed on the latest 1.13 beta.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"
GOBIN="/Users/dom/Documents/Go/bin"
GOCACHE="/Users/dom/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/dom/Documents/Go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.6/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.6/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/y3/yw3qvdwx24b77glqt8ktxpcm0000gn/T/go-build730621054=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package tlsconfig

import (
	"crypto/tls"
	"crypto/x509"
	"io/ioutil"
	"os"
	"testing"
)

var (
	ca = []byte(`
-----BEGIN CERTIFICATE-----
MIIDXDCCAkSgAwIBAgIJAID95CGdQ6gJMA0GCSqGSIb3DQEBCwUAMCMxITAfBgNV
BAMMGExhIGbDoWJyaWNhIGRlIHBsw6F0YW5vczAeFw0xOTAyMTExMzM4MDRaFw0y
OTAyMDgxMzM4MDRaMCMxITAfBgNVBAMMGExhIGbDoWJyaWNhIGRlIHBsw6F0YW5v
czCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALJw+Mt286mv9LK7RYld
jOSDCycYtLMPwoMj73f05nW8JqLJiovqGHuU+8nAXIqoelTpqcAk/uoeAZ8BJW3J
s8Yw67StaELh49MabgkDfbbSkQgBe1uiXZvmqvUf51T1u7XWeHUkD6YGF0ZcZJtq
Jp77jA0+EViklZs9qnVEVS33OcCtJnatJcpjipMI6JVvvtFkSMkqIwhzNQKRyiBE
9N/sSQbe1CdIUb2jVm1f2n/V/Gyc/LUMByusACf10aMun3p9G9NZizk7+yiW0gbT
iQXDflgRoDvSucOatmSBwFEUgnXdhwyFekoqZnbI9y6GZD05G1I2H/oUQjVoDp8c
QqsCAwEAAaOBkjCBjzAdBgNVHQ4EFgQU13AdzmoQXb1wbBuAMwPFn3Ke4HkwUwYD
VR0jBEwwSoAU13AdzmoQXb1wbBuAMwPFn3Ke4HmhJ6QlMCMxITAfBgNVBAMMGExh
IGbDoWJyaWNhIGRlIHBsw6F0YW5vc4IJAID95CGdQ6gJMAwGA1UdEwQFMAMBAf8w
CwYDVR0PBAQDAgEGMA0GCSqGSIb3DQEBCwUAA4IBAQBflIQjGHS7WJcmDM07lEly
+g0i+XRwG8sMHKVUKUaVDD+hU2pZ4Jg389CHWkiqwD75f+hlXCgxVhdqDlJ+3hTW
fvhvQyfpez8dqpvp/GYuqQYf7EgSmg2t9ZeV1NkMaYJWQhnC753n1FYMOgWwFa/r
jnVxOec480pm9fJKvv38lKXr0FvZ3Lehxs9TxMKJzWCiSzSdcCqjX0sMsYzM0Chm
X8j4bmUCNf3qlN4FcI28zfXLuRXcu6YT/Cf0Ug/YwlMtWc0MLgzDUVkQxTuZeF4D
sJmfHZzXWGz0I5i/lZKWfXBODGyO8n3MPn7+nO1gAObW4piS/tQgULUVuUD7MUZr
-----END CERTIFICATE-----`)

	serverChain = append([]byte(`
-----BEGIN CERTIFICATE-----
MIIDeDCCAmCgAwIBAgIQJBFlQJ4DYSOjNvLV4fOUmTANBgkqhkiG9w0BAQsFADAj
MSEwHwYDVQQDDBhMYSBmw6FicmljYSBkZSBwbMOhdGFub3MwHhcNMTkwMjExMTQz
MTU1WhcNMjIwMTI2MTQzMTU1WjAVMRMwEQYDVQQDDApUZXN0U2VydmVyMIIBIjAN
BgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtN8+2vSTpeKdrhzJPVy0Zxh2vj/j
Rpq8gWu/qtB/2WOWnGLNigTF66dlAmrd8q9OuGKuDMOLuv0WGuRZSW8eYrkauKm+
0bZYthOk83d7UUc+SYFFcvlGFg5ElNRZw6kND5yQkIV3kYV/Lcf0XJnfB6EYuirZ
lQimV3gswZVbKNexszjK35GF+sszilv0TIZc6BvvMqwji6hzsDZEW03hew7sb4qt
pH0yydy3lF0P6OIe/aFR9cj2XUmd/ls2peY91Xl2OQLpzktgScClL3NArojdsUzG
81N+NuFfApn5IccDmDPEUM6UZrjXFHSLWCIb33bQy3oFmegEIxZedlTq/QIDAQAB
o4G1MIGyMAkGA1UdEwQCMAAwHQYDVR0OBBYEFIU7TsRegsqLfe95CX5rFRiHJTTw
MFMGA1UdIwRMMEqAFNdwHc5qEF29cGwbgDMDxZ9ynuB5oSekJTAjMSEwHwYDVQQD
DBhMYSBmw6FicmljYSBkZSBwbMOhdGFub3OCCQCA/eQhnUOoCTATBgNVHSUEDDAK
BggrBgEFBQcDATALBgNVHQ8EBAMCBaAwDwYDVR0RBAgwBocEfwAAATANBgkqhkiG
9w0BAQsFAAOCAQEAkXjG01tItzgFWtJ8ZTPKwLf92kd4KG67VaDx+3gnqmcQjQzt
zHY7chmaarTJ9pCV+wbTaRWl5JJIUMKI90p5sLJnYD6Qwo/PgGhaM3/tI17VrtAx
Qa9nIbthK2icqMAmn9iIvcjp+M7SgXDPwAFXKq836nlEtAR3xKW0puYa6rbhW1Pd
sBR/SbF0CXphWV/nURwWhsgddcj8lFzXUJB5KzNarBECClS3ePmUyS3uu+Mzh1qJ
+soc+b0xmcmT+3SMxmNWhCCNd+cUWEfxkPKKNoBg90quix0M3zrwsGVh+GmLo0yv
fnhr/VHlGLRi4ZrRY0hYKNQHGvHWEa+0irxAMQ==
-----END CERTIFICATE-----`), ca...)

	serverKey = []byte(`
-----BEGIN PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC03z7a9JOl4p2u
HMk9XLRnGHa+P+NGmryBa7+q0H/ZY5acYs2KBMXrp2UCat3yr064Yq4Mw4u6/RYa
5FlJbx5iuRq4qb7Rtli2E6Tzd3tRRz5JgUVy+UYWDkSU1FnDqQ0PnJCQhXeRhX8t
x/Rcmd8HoRi6KtmVCKZXeCzBlVso17GzOMrfkYX6yzOKW/RMhlzoG+8yrCOLqHOw
NkRbTeF7Duxviq2kfTLJ3LeUXQ/o4h79oVH1yPZdSZ3+Wzal5j3VeXY5AunOS2BJ
wKUvc0CuiN2xTMbzU3424V8CmfkhxwOYM8RQzpRmuNcUdItYIhvfdtDLegWZ6AQj
Fl52VOr9AgMBAAECggEAPO6QQDbwnouvTv6HlNJsO+bz6begGyL5qifgU+0VhOiV
zm1CjOJ6wp6L2rqhtqX0QQ2NUON3aTDlh32xzplOhTeSlu8oR4HEdOI9SX/Q3VrA
0wZnnQn44GlCFVlMPCvxKHc4BFfSJgXxCD06Rw/XILzWlbDlx904HHNxsG1eXm1p
1UYJekBSG4niobJcgMgZ6igb5SH8tA1IMEd2fd77aM0CBzxdXVpbkBR2Kza6sx5W
sGUBDE7U7PDd+GOtYHBSWBMk7GoENyB5Rh/HiZjno9OhNdiWNr+6mZluB5JqVs6R
7qGCBy4E818DSWI4qFETHdi8kVN+YpgVMr3LMn/viQKBgQDtQD6ZPiW16Or0aAHG
LiCwl5ArFwWmr46zkfkmw/5yQnDiqjHFIfQ2ap5joqWcZeEoGH9Rf45aiZJFVQBj
4cYEiMmu7WeQ3b8lUeh3hg2YeWBpdnN47qfoGPXUkTp3vnkYK1DBIgZNDvuESxZG
YKyS5bEAevee+SmgS/Jqa9H9cwKBgQDDKmptXa8bgprnsAjlJe1UWZz5CZwH3Ptk
LD/1hoy2TXX0kmqYe5THeCxIZis+DzOXbzZwaQ7hWtOU8TnrLzMvQON7VA96adlt
SNwf6r4pL0yegyLRZnEtU6bylG6UjWJ1NZtqaz1L5SfaWovoaDbncW/7GanCLtmG
ejpPRORZzwKBgQDRxyiSn10Ax/5YNU68STUmcB1NvIGGrVxkcwH2wP5PUWg3Q33W
bPte0k9SkhIVhA/eQCrziPH6JAJtw4cgyhUpFTofrlMQyMGm4hHG/YUv+MLC/bWE
jKFym/9iATfNgWHZeRNicg9YO5MxmqMLf5eYG/iyEAi7TFz+G6kxaDy4+QKBgQC7
DDziQqaf7CXAphNwY1A5xoVOnWogkyeRE2PdUi4sTsMdOvLU36RxJHj0jo8lNHaS
zbDUPaxYSa99EVEcWalpwXwEaEVJYodTWUA3isjkOgPp6+8D2HXiJklcNuxgjbzu
bwlcDhm6Uo9Gk/+BJ9hvK2ZSt37er/4lB6I49OMSOwKBgC8XCM8NdDFwiGAt2CQu
3bN7TzZ+Hki5OBpVUkWwRZSr624lw0xH3eraSV+7k9huKjqBbNj8AiB7UAMGRG11
6xFyo3/1mtmA+W6rHR8A/cL6+kKrQjAB0T8v7vo93I3qzUMpoIEm2knpvnF9KN5m
Qg/+I5OqmVFypMgbfjdvJZ0b
-----END PRIVATE KEY-----`)
)

func TestTLSDial(t *testing.T) {
	// Get a temporary file path for the unix socket to test the TLS dial
	tmpfile, err := ioutil.TempFile("", "storageos")
	if err != nil {
		t.Fatalf("failed to create temporary file: %v", err)
	}
	socketPath := tmpfile.Name()
	os.Remove(socketPath)

	// Start a TLS listener
	l, err := tls.Listen("unix", socketPath, getServerConfig(t))
	if err != nil {
		t.Fatalf("failed to bind unix socket %q: %v", socketPath, err)
	}

	// Start a background thread reading the conn
	go func() {
		for {
			conn, err := l.Accept()
			if err != nil {
				return
			}
			conn.Write([]byte("hola"))
		}
	}()

	// Dial to the TLS unix socket
	c, err := tls.Dial("unix", socketPath, getClientConfig(t))
	if err != nil {
		// Go 1.12.6 fails here
		t.Fatalf("dial: %v", err)
	}

	// Try and read from the connection
	buf := make([]byte, 128)
	n, err := c.Read(buf)
	if err != nil {
		// BUG? Go 1.13beta1 fails here
		t.Fatalf("read: %v", err)
	}
	t.Logf("got %v", string(buf[0:n]))
	c.Close()
}

// getClientConfig returns a tls.Config configured to accept certificates from
// the CA root but DOES NOT provide a client certificate.
func getClientConfig(t *testing.T) *tls.Config {
	t.Helper()

	roots := x509.NewCertPool()
	if !roots.AppendCertsFromPEM(ca) {
		t.Fatal("failed to add CA to root pool")
	}

	// NO client certs provided, so mTLS auth will fail
	return &tls.Config{
		MinVersion: tls.VersionTLS12,
		RootCAs:    roots,
		ServerName: "127.0.0.1",
	}
}

// getServerConfig returns a tls.Config configured to present a server chain
// (server+ca cert) and REQUIRES a client to present a certificate signed the
// same CA (mTLS auth).
func getServerConfig(t *testing.T) *tls.Config {
	t.Helper()

	serverCert, err := tls.X509KeyPair(serverChain, serverKey)
	if err != nil {
		t.Fatal(err)
	}

	roots := x509.NewCertPool()
	if !roots.AppendCertsFromPEM(ca) {
		t.Fatal("failed to add CA to root pool")
	}

	return &tls.Config{
		MinVersion: tls.VersionTLS12,
		Certificates: []tls.Certificate{
			serverCert,
		},
		ClientCAs:  roots,
		ClientAuth: tls.RequireAndVerifyClientCert,
	}
}

When connecting to a TLS endpoint that requires a client certificate and the client does not provide one, the tls.Dial() succeeds, but the socket read fails - this differs to previous releases.

What did you expect to see?

In 1.12.7 the tls.Dial() fails and execution never gets to the socket read:

--- FAIL: TestTLSDial (0.01s)
    bug_test.go:118: dial: remote error: tls: bad certificate
FAIL
exit status 1
FAIL	code.storageos.net/storageos/c2/config/tlsconfig	0.020s

What did you see instead?

In 1.13beta1 the Dial() returns a nil error, but reading from the socket returns tls: bad certificate instead:

--- FAIL: TestTLSDial (0.01s)
    bug_test.go:126: read: remote error: tls: bad certificate
FAIL
exit status 1
FAIL	code.storageos.net/storageos/c2/config/tlsconfig	0.021s

Not a big problem, but tripped our CI tests expecting the dial to fail.

[lucianoq]

Same on Linux.

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

$ go version
go version go1.12.7 linux/amd64

$ go1.13beta1 version
go version go1.13beta1 linux/amd64

What operating system and processor architecture are you using (go env)?

go env Output

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/luciano/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/luciano/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/golang"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build108933780=/tmp/go-build -gno-record-gcc-switches"

Output

$ go test   
--- FAIL: TestTLSDial (0.00s)
    main_test.go:118: dial: remote error: tls: bad certificate
FAIL
exit status 1

$ go1.13beta1 test
--- FAIL: TestTLSDial (0.00s)
    main_test.go:126: read: remote error: tls: bad certificate
FAIL
exit status 1

[tmthrgd]

This is a consequence of how TLS 1.3 was designed, see: https://golang.org/doc/go1.12#tls_1_3.

In TLS 1.3 the client is the last one to speak in the handshake, so if it causes an error to occur on the server, it will be returned on the client by the first Read, not by Handshake. For example, that will be the case if the server rejects the client certificate. Similarly, session tickets are now post-handshake messages, so are only received by the client upon its first Read.

[FiloSottile]

Indeed, this is unfortunate, but a core property of TLS 1.3 which is necessary for its performance improvements.