Skip to content

Commit

Permalink
crypto/tls: enforce that either ServerName or InsecureSkipVerify be g…
Browse files Browse the repository at this point in the history
…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 golang#7342.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/67010043
  • Loading branch information
agl committed Feb 21, 2014
1 parent dbe3452 commit 2680804
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions handshake_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down

0 comments on commit 2680804

Please sign in to comment.