Upgrades to Go 1.7 and fixes vet finding and TLS behavior change. (#2281)

* Upgrades to Go 1.7 and fixes vet finding and TLS behavior change.

* Fixes unit tests in a better manner by closing the client connection on errors.

We traced through and realized that https://github.com/golang/go/issues/15709
causes the output from the client to get buffered, which cuts off the alert
feedback due to the flush() call getting bypassed by the error return.

.travis.yml

@@ -1,7 +1,7 @@
 language: go
 
 go:
-  - 1.6.3
+  - 1.7.3
 
 branches:
   only:

README.md

@@ -42,7 +42,7 @@ https://www.consul.io/docs
 ## Developing Consul
 
 If you wish to work on Consul itself, you'll first need [Go](https://golang.org)
-installed (version 1.6+ is _required_). Make sure you have Go properly installed,
+installed (version 1.7+ is _required_). Make sure you have Go properly installed,
 including setting up your [GOPATH](https://golang.org/doc/code.html#GOPATH).
 
 Next, clone this repository into `$GOPATH/src/github.com/hashicorp/consul` and
@@ -64,7 +64,7 @@ format the code according to Go standards.
 
 ### Building Consul on Windows
 
-Make sure Go 1.6+ is installed on your system and that the Go command is in your
+Make sure Go 1.7+ is installed on your system and that the Go command is in your
 %PATH%.
 
 For building Consul on Windows, you also need to have MinGW installed.

scripts/consul-builder/Dockerfile

@@ -1,6 +1,6 @@
 FROM ubuntu:trusty
 
-ENV GOVERSION 1.6.3
+ENV GOVERSION 1.7.3
 
 RUN apt-get update -y && \
     apt-get install --no-install-recommends -y -q \

tlsutil/config.go

@@ -143,6 +143,39 @@ func (c *Config) OutgoingTLSConfig() (*tls.Config, error) {
 	return tlsConfig, nil
 }
 
+// Clone returns a copy of c. Only the exported fields are copied. This
+// was copied from https://golang.org/src/crypto/tls/common.go since that
+// isn't exported and Go 1.7's vet uncovered an unsafe copy of a mutex in
+// here.
+//
+// TODO (slackpad) - This can be removed once we move to Go 1.8, see
+// https://github.com/golang/go/commit/d24f446 for details.
+func clone(c *tls.Config) *tls.Config {
+	return &tls.Config{
+		Rand:                        c.Rand,
+		Time:                        c.Time,
+		Certificates:                c.Certificates,
+		NameToCertificate:           c.NameToCertificate,
+		GetCertificate:              c.GetCertificate,
+		RootCAs:                     c.RootCAs,
+		NextProtos:                  c.NextProtos,
+		ServerName:                  c.ServerName,
+		ClientAuth:                  c.ClientAuth,
+		ClientCAs:                   c.ClientCAs,
+		InsecureSkipVerify:          c.InsecureSkipVerify,
+		CipherSuites:                c.CipherSuites,
+		PreferServerCipherSuites:    c.PreferServerCipherSuites,
+		SessionTicketsDisabled:      c.SessionTicketsDisabled,
+		SessionTicketKey:            c.SessionTicketKey,
+		ClientSessionCache:          c.ClientSessionCache,
+		MinVersion:                  c.MinVersion,
+		MaxVersion:                  c.MaxVersion,
+		CurvePreferences:            c.CurvePreferences,
+		DynamicRecordSizingDisabled: c.DynamicRecordSizingDisabled,
+		Renegotiation:               c.Renegotiation,
+	}
+}
+
 // OutgoingTLSWrapper returns a a DCWrapper based on the OutgoingTLS
 // configuration. If hostname verification is on, the wrapper
 // will properly generate the dynamic server name for verification.
@@ -164,9 +197,9 @@ func (c *Config) OutgoingTLSWrapper() (DCWrapper, error) {
 	// Generate the wrapper based on hostname verification
 	if c.VerifyServerHostname {
 		wrapper := func(dc string, conn net.Conn) (net.Conn, error) {
-			conf := *tlsConfig
+			conf := clone(tlsConfig)
 			conf.ServerName = "server." + dc + "." + domain
-			return WrapTLSClient(conn, &conf)
+			return WrapTLSClient(conn, conf)
 		}
 		return wrapper, nil
 	} else {

tlsutil/config_test.go

@@ -207,6 +207,7 @@ func startTLSServer(config *Config) (net.Conn, chan error) {
 			errc <- err
 		}
 		close(errc)
+
 		// Because net.Pipe() is unbuffered, if both sides
 		// Close() simultaneously, we will deadlock as they
 		// both send an alert and then block. So we make the
@@ -276,12 +277,11 @@ func TestConfig_outgoingWrapper_BadDC(t *testing.T) {
 	if err != nil {
 		t.Fatalf("wrapTLS err: %v", err)
 	}
-	defer tlsClient.Close()
 	err = tlsClient.(*tls.Conn).Handshake()
-
 	if _, ok := err.(x509.HostnameError); !ok {
 		t.Fatalf("should get hostname err: %v", err)
 	}
+	tlsClient.Close()
 
 	<-errc
 }
@@ -309,12 +309,11 @@ func TestConfig_outgoingWrapper_BadCert(t *testing.T) {
 	if err != nil {
 		t.Fatalf("wrapTLS err: %v", err)
 	}
-	defer tlsClient.Close()
 	err = tlsClient.(*tls.Conn).Handshake()
-
 	if _, ok := err.(x509.HostnameError); !ok {
 		t.Fatalf("should get hostname err: %v", err)
 	}
+	tlsClient.Close()
 
 	<-errc
 }