crypto/tls: enforce that either ServerName or InsecureSkipVerify be given.

crypto/tls has two functions for creating a client connection: Dial,
which most users are expected to use, and Client, which is the
lower-level API.

Dial does what you expect: it gives you a secure connection to the host
that you specify and the majority of users of crypto/tls appear to work
fine with it.

Client gives more control but needs more care. Specifically, if it
wasn't given a server name in the tls.Config then it didn't check that
the server's certificates match any hostname - because it doesn't have
one to check against. It was assumed that users of the low-level API
call VerifyHostname on the certificate themselves if they didn't supply
a hostname.

A review of the uses of Client both within Google and in a couple of
external libraries has shown that nearly all of them got this wrong.

Thus, this change enforces that either a ServerName or
InsecureSkipVerify is given. This does not affect tls.Dial.

See discussion at https://groups.google.com/d/msg/golang-nuts/4vnt7NdLvVU/b1SJ4u0ikb0J.

Fixes #7342.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/67010043

diff --git a/doc/go1.3.txt b/doc/go1.3.txt
index 613d261974a7f49378d0e8750a8a55b43d979605..fda35a7e3ff8a7fe14594fc9637bd3ceb4035fe5 100644 (file)
--- a/doc/go1.3.txt
+++ b/doc/go1.3.txt
@@ -9,3 +9,4 @@ misc/benchcmp has been replaced by go tool benchcmp (CL 47980043)
 cmd/go, go/build: support .m files (CL 60590044)
 unicode: upgrade from Unicode 6.2.0 to 6.3.0 (CL 65400044)
 runtime/debug: add SetPanicOnFault (CL 66590044)
+crypto/tls: ServerName or InsecureSkipVerify (CL 67010043)

diff --git a/src/pkg/crypto/tls/handshake_client.go b/src/pkg/crypto/tls/handshake_client.go
index fd1303eebb947ff7d3a35ad46530c44f6460a05c..0d8d9a1d46f9c3de9a835df076b8789d5f1d492f 100644 (file)
--- a/src/pkg/crypto/tls/handshake_client.go
+++ b/src/pkg/crypto/tls/handshake_client.go
@@ -33,6 +33,10 @@ func (c *Conn) clientHandshake() error {
                c.config = defaultConfig()
        }
 
+       if len(c.config.ServerName) == 0 && !c.config.InsecureSkipVerify {
+               return errors.New("tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config")
+       }
+
        hello := &clientHelloMsg{
                vers:                c.config.maxVersion(),
                compressionMethods:  []uint8{compressionNone},