crypto/tls: enforce that either ServerName or InsecureSkipVerify be g…

…iven.

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

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)

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},