From 6de74c60a4c93ec2ce98d0f9915c02a970400f29 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 7 Nov 2016 18:15:26 -0800 Subject: [PATCH] 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 | 2 +- README.md | 4 ++-- scripts/consul-builder/Dockerfile | 2 +- tlsutil/config.go | 37 +++++++++++++++++++++++++++++-- tlsutil/config_test.go | 7 +++--- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3111b9ee1d..41378f4ca7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - - 1.6.3 + - 1.7.3 branches: only: diff --git a/README.md b/README.md index 793194d7bd..6196e79db3 100644 --- a/README.md +++ b/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. diff --git a/scripts/consul-builder/Dockerfile b/scripts/consul-builder/Dockerfile index 3c6b90c2b8..04e20fe088 100644 --- a/scripts/consul-builder/Dockerfile +++ b/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 \ diff --git a/tlsutil/config.go b/tlsutil/config.go index 105934d3a8..57c9867483 100644 --- a/tlsutil/config.go +++ b/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 { diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index cbd127ac89..2bf413672b 100644 --- a/tlsutil/config_test.go +++ b/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 }