Merge pull request #545 from docker/use-go-connections

Use go-connections, and TLS flags for notary client

cmd/notary-server/main.go

@@ -21,6 +21,7 @@ import (
 	_ "github.com/go-sql-driver/mysql"
 	"golang.org/x/net/context"
 
+	"github.com/docker/go-connections/tlsconfig"
 	"github.com/docker/notary/server"
 	"github.com/docker/notary/utils"
 	"github.com/docker/notary/version"
@@ -63,39 +64,27 @@ func getAddrAndTLSConfig(configuration *viper.Viper) (string, *tls.Config, error
 		return "", nil, fmt.Errorf("http listen address required for server")
 	}
 
-	tlsOpts, err := utils.ParseServerTLS(configuration, false)
+	tlsConfig, err := utils.ParseServerTLS(configuration, false)
 	if err != nil {
 		return "", nil, fmt.Errorf(err.Error())
 	}
-	// do not support this yet since the client doesn't have client cert support
-	if tlsOpts != nil {
-		tlsOpts.ClientCAFile = ""
-		tlsConfig, err := utils.ConfigureServerTLS(tlsOpts)
-		if err != nil {
-			return "", nil, fmt.Errorf(
-				"unable to set up TLS for server: %s", err.Error())
-		}
 	return httpAddr, tlsConfig, nil
-	}
-	return httpAddr, nil, nil
 }
 
 // sets up TLS for the GRPC connection to notary-signer
 func grpcTLS(configuration *viper.Viper) (*tls.Config, error) {
 	rootCA := utils.GetPathRelativeToConfig(configuration, "trust_service.tls_ca_file")
-	serverName := configuration.GetString("trust_service.hostname")
 	clientCert := utils.GetPathRelativeToConfig(configuration, "trust_service.tls_client_cert")
 	clientKey := utils.GetPathRelativeToConfig(configuration, "trust_service.tls_client_key")
 
-	if (clientCert == "" && clientKey != "") || (clientCert != "" && clientKey == "") {
+	if clientCert == "" && clientKey != "" || clientCert != "" && clientKey == "" {
-		return nil, fmt.Errorf("Partial TLS configuration found. Either include both a client cert and client key file in the configuration, or include neither.")
+		return nil, fmt.Errorf("either pass both client key and cert, or neither")
 	}
 
-	tlsConfig, err := utils.ConfigureClientTLS(&utils.ClientTLSOpts{
+	tlsConfig, err := tlsconfig.Client(tlsconfig.Options{
-		RootCAFile:     rootCA,
+		CAFile:   rootCA,
-		ServerName:     serverName,
+		CertFile: clientCert,
-		ClientCertFile: clientCert,
+		KeyFile:  clientKey,
-		ClientKeyFile:  clientKey,
 	})
 	if err != nil {
 		return nil, fmt.Errorf(

cmd/notary-server/main_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -78,8 +79,7 @@ func TestGetAddrAndTLSConfigSuccessWithoutTLS(t *testing.T) {
 	assert.Nil(t, tlsConf)
 }
 
-// We don't support client CAs yet on notary server
+func TestGetAddrAndTLSConfigWithClientTLS(t *testing.T) {
-func TestGetAddrAndTLSConfigSkipClientTLS(t *testing.T) {
 	httpAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
 		"server": {
 			"http_addr": ":2345",
@@ -90,7 +90,7 @@ func TestGetAddrAndTLSConfigSkipClientTLS(t *testing.T) {
 	}`, Cert, Key, Root)))
 	assert.NoError(t, err)
 	assert.Equal(t, ":2345", httpAddr)
-	assert.Nil(t, tlsConf.ClientCAs)
+	assert.NotNil(t, tlsConf.ClientCAs)
 }
 
 // If neither "remote" nor "local" is passed for "trust_service.type", an
@@ -200,7 +200,7 @@ func TestGetTrustServiceTLSMissingCertOrKey(t *testing.T) {
 			fakeRegister)
 		assert.Error(t, err)
 		assert.True(t,
-			strings.Contains(err.Error(), "Partial TLS configuration found."))
+			strings.Contains(err.Error(), "either pass both client key and cert, or neither"))
 	}
 	// no health function ever registered
 	assert.Equal(t, 0, registerCalled)
@@ -233,7 +233,6 @@ func TestGetTrustServiceNoTLSConfig(t *testing.T) {
 	assert.NoError(t, err)
 	assert.IsType(t, &client.NotarySigner{}, trust)
 	assert.Equal(t, "ecdsa", algo)
-	assert.Equal(t, "notary-signer", tlsConfig.ServerName)
 	assert.Nil(t, tlsConfig.RootCAs)
 	assert.Nil(t, tlsConfig.Certificates)
 	// health function registered
@@ -267,8 +266,8 @@ func TestGetTrustServiceTLSSuccess(t *testing.T) {
 	assert.NoError(t, err)
 	assert.IsType(t, &client.NotarySigner{}, trust)
 	assert.Equal(t, "ecdsa", algo)
-	assert.Equal(t, "notary-signer", tlsConfig.ServerName)
+	assert.Len(t, tlsConfig.Certificates, 1)
-	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
+	assert.True(t, reflect.DeepEqual(keypair, tlsConfig.Certificates[0]))
 	// health function registered
 	assert.Equal(t, 1, registerCalled)
 }

cmd/notary-signer/main.go

@@ -154,11 +154,7 @@ func setupHTTPServer(httpAddr string, tlsConfig *tls.Config,
 }
 
 func getAddrAndTLSConfig(configuration *viper.Viper) (string, string, *tls.Config, error) {
-	tlsOpts, err := utils.ParseServerTLS(configuration, true)
+	tlsConfig, err := utils.ParseServerTLS(configuration, true)
-	if err != nil {
-		return "", "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error())
-	}
-	tlsConfig, err := utils.ConfigureServerTLS(tlsOpts)
 	if err != nil {
 		return "", "", nil, fmt.Errorf("unable to set up TLS: %s", err.Error())
 	}

cmd/notary/integration_test.go

@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"crypto/rand"
 	"io/ioutil"
+	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
@@ -56,11 +57,8 @@ func runCommand(t *testing.T, tempDir string, args ...string) (string, error) {
 	return string(output), retErr
 }
 
-// makes a testing notary-server
+func setupServerHandler(metaStore storage.MetaStore) http.Handler {
-func setupServer() *httptest.Server {
+	ctx := context.WithValue(context.Background(), "metaStore", metaStore)
-	// Set up server
-	ctx := context.WithValue(
-		context.Background(), "metaStore", storage.NewMemStorage())
 
 	ctx = context.WithValue(ctx, "keyAlgorithm", data.ECDSAKey)
 
@@ -72,7 +70,12 @@ func setupServer() *httptest.Server {
 
 	cryptoService := cryptoservice.NewCryptoService(
 		"", trustmanager.NewKeyMemoryStore(passphrase.ConstantRetriever("pass")))
-	return httptest.NewServer(server.RootHandler(nil, ctx, cryptoService))
+	return server.RootHandler(nil, ctx, cryptoService)
+}
+
+// makes a testing notary-server
+func setupServer() *httptest.Server {
+	return httptest.NewServer(setupServerHandler(storage.NewMemStorage()))
 }
 
 // Initializes a repo, adds a target, publishes the target, lists the target,

cmd/notary/main.go

@@ -41,6 +41,17 @@ func (u usageTemplate) ToCommand(run cobraRunE) *cobra.Command {
 	return &c
 }
 
+func pathRelativeToCwd(path string) string {
+	if path == "" || filepath.IsAbs(path) {
+		return path
+	}
+	cwd, err := os.Getwd()
+	if err != nil {
+		return filepath.Clean(path)
+	}
+	return filepath.Clean(filepath.Join(cwd, path))
+}
+
 type notaryCommander struct {
 	// this needs to be set
 	getRetriever func() passphrase.Retriever
@@ -51,6 +62,10 @@ type notaryCommander struct {
 	trustDir          string
 	configFile        string
 	remoteTrustServer string
+
+	tlsCAFile   string
+	tlsCertFile string
+	tlsKeyFile  string
 }
 
 func (n *notaryCommander) parseConfig() (*viper.Viper, error) {
@@ -96,7 +111,16 @@ func (n *notaryCommander) parseConfig() (*viper.Viper, error) {
 	// At this point we either have the default value or the one set by the config.
 	// Either way, some command-line flags have precedence and overwrites the value
 	if n.trustDir != "" {
-		config.Set("trust_dir", n.trustDir)
+		config.Set("trust_dir", pathRelativeToCwd(n.trustDir))
+	}
+	if n.tlsCAFile != "" {
+		config.Set("remote_server.root_ca", pathRelativeToCwd(n.tlsCAFile))
+	}
+	if n.tlsCertFile != "" {
+		config.Set("remote_server.tls_client_cert", pathRelativeToCwd(n.tlsCertFile))
+	}
+	if n.tlsKeyFile != "" {
+		config.Set("remote_server.tls_client_key", pathRelativeToCwd(n.tlsKeyFile))
 	}
 	if n.remoteTrustServer != "" {
 		config.Set("remote_server.url", n.remoteTrustServer)
@@ -118,8 +142,9 @@ func (n *notaryCommander) GetCommand() *cobra.Command {
 		Use:           "notary",
 		Short:         "Notary allows the creation of trusted collections.",
 		Long:          "Notary allows the creation and management of collections of signed targets, allowing the signing and validation of arbitrary content.",
-		SilenceUsage:  true,
+		SilenceUsage:  true, // we don't want to print out usage for EVERY error
-		SilenceErrors: true,
+		SilenceErrors: true, // we do our own error reporting with fatalf
+		Run:           func(cmd *cobra.Command, args []string) { cmd.Usage() },
 	}
 	notaryCmd.SetOutput(os.Stdout)
 	notaryCmd.AddCommand(&cobra.Command{
@@ -138,6 +163,9 @@ func (n *notaryCommander) GetCommand() *cobra.Command {
 	notaryCmd.PersistentFlags().BoolVarP(&n.verbose, "verbose", "v", false, "Verbose output")
 	notaryCmd.PersistentFlags().BoolVarP(&n.debug, "debug", "D", false, "Debug output")
 	notaryCmd.PersistentFlags().StringVarP(&n.remoteTrustServer, "server", "s", "", "Remote trust server location")
+	notaryCmd.PersistentFlags().StringVar(&n.tlsCAFile, "tlscacert", "", "Trust certs signed only by this CA")
+	notaryCmd.PersistentFlags().StringVar(&n.tlsCertFile, "tlscert", "", "Path to TLS certificate file")
+	notaryCmd.PersistentFlags().StringVar(&n.tlsKeyFile, "tlskey", "", "Path to TLS key file")
 
 	cmdKeyGenerator := &keyCommander{
 		configGetter: n.parseConfig,

cmd/notary/main_test.go

@@ -2,13 +2,18 @@ package main
 
 import (
 	"bytes"
+	"crypto/tls"
+	"fmt"
 	"io/ioutil"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/docker/go-connections/tlsconfig"
 	"github.com/docker/notary/passphrase"
+	"github.com/docker/notary/server/storage"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -131,8 +136,8 @@ func TestConfigParsingErrorsPropagatedByCommands(t *testing.T) {
 			strings.Fields(args)...))
 		err = cmd.Execute()
 
-		require.Error(t, err, "expected error when running %s", args)
+		require.Error(t, err, "expected error when running `notary %s`", args)
-		require.Contains(t, err.Error(), "error opening config file", "running %s", args)
+		require.Contains(t, err.Error(), "error opening config file", "running `notary %s`", args)
 		require.NotContains(t, b.String(), "Usage:")
 	}
 }
@@ -162,8 +167,205 @@ func TestInsufficientArgumentsReturnsErrorAndPrintsUsage(t *testing.T) {
 			[]string{"-c", filepath.Join(tempdir, "idonotexist.json"), "-d", tempdir}, arglist...))
 		err = cmd.Execute()
 
-		require.NotContains(t, err.Error(), "error opening config file", "running %s", invalid)
+		require.NotContains(t, err.Error(), "error opening config file", "running `notary %s`", invalid)
 		// it's a usage error, so the usage is printed
-		require.Contains(t, b.String(), "Usage:", "expected usage when running %s", invalid)
+		require.Contains(t, b.String(), "Usage:", "expected usage when running `notary %s`", invalid)
 	}
 }
+
+// The bare notary command and bare subcommands all print out usage
+func TestBareCommandPrintsUsageAndNoError(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "empty-dir")
+	require.NoError(t, err)
+	defer os.RemoveAll(tempdir)
+
+	// just the notary command
+	b := new(bytes.Buffer)
+	cmd := NewNotaryCommand()
+	cmd.SetOutput(b)
+
+	cmd.SetArgs([]string{"-c", filepath.Join(tempdir, "idonotexist.json")})
+	require.NoError(t, cmd.Execute(), "Expected no error from a help request")
+	// usage is printed
+	require.Contains(t, b.String(), "Usage:", "expected usage when running `notary`")
+
+	// notary key, notary cert, and notary delegation
+	for _, bareCommand := range []string{"key", "cert", "delegation"} {
+		b := new(bytes.Buffer)
+		cmd := NewNotaryCommand()
+		cmd.SetOutput(b)
+
+		cmd.SetArgs([]string{"-c", filepath.Join(tempdir, "idonotexist.json"), bareCommand})
+		require.NoError(t, cmd.Execute(), "Expected no error from a help request")
+		// usage is printed
+		require.Contains(t, b.String(), "Usage:", "expected usage when running `notary %s`", bareCommand)
+	}
+}
+
+type recordingMetaStore struct {
+	gotten []string
+	storage.MemStorage
+}
+
+// GetCurrent gets the metadata from the underlying MetaStore, but also records
+// that the metadata was requested
+func (r *recordingMetaStore) GetCurrent(gun, role string) (data []byte, err error) {
+	r.gotten = append(r.gotten, fmt.Sprintf("%s.%s", gun, role))
+	return r.MemStorage.GetCurrent(gun, role)
+}
+
+// GetChecksum gets the metadata from the underlying MetaStore, but also records
+// that the metadata was requested
+func (r *recordingMetaStore) GetChecksum(gun, role, checksum string) (data []byte, err error) {
+	r.gotten = append(r.gotten, fmt.Sprintf("%s.%s", gun, role))
+	return r.MemStorage.GetChecksum(gun, role, checksum)
+}
+
+// the config can provide all the TLS information necessary - the root ca file,
+// the tls client files - they are all relative to the directory of the config
+// file, and not the cwd
+func TestConfigFileTLSCannotBeRelativeToCWD(t *testing.T) {
+	// Set up server that with a self signed cert
+	var err error
+	// add a handler for getting the root
+	m := &recordingMetaStore{MemStorage: *storage.NewMemStorage()}
+	s := httptest.NewUnstartedServer(setupServerHandler(m))
+	s.TLS, err = tlsconfig.Server(tlsconfig.Options{
+		CertFile:   "../../fixtures/notary-server.crt",
+		KeyFile:    "../../fixtures/notary-server.key",
+		CAFile:     "../../fixtures/root-ca.crt",
+		ClientAuth: tls.RequireAndVerifyClientCert,
+	})
+	assert.NoError(t, err)
+	s.StartTLS()
+	defer s.Close()
+
+	// test that a config file with certs that are relative to the cwd fail
+	tempDir := tempDirWithConfig(t, fmt.Sprintf(`{
+		"remote_server": {
+			"url": "%s",
+			"root_ca": "../../fixtures/root-ca.crt",
+			"tls_client_cert": "../../fixtures/notary-server.crt",
+			"tls_client_key": "../../fixtures/notary-server.key"
+		}
+	}`, s.URL))
+	defer os.RemoveAll(tempDir)
+	configFile := filepath.Join(tempDir, "config.json")
+
+	// set a config file, so it doesn't check ~/.notary/config.json by default,
+	// and execute a random command so that the flags are parsed
+	cmd := NewNotaryCommand()
+	cmd.SetArgs([]string{"-c", configFile, "list", "repo"})
+	cmd.SetOutput(new(bytes.Buffer)) // eat the output
+	err = cmd.Execute()
+	assert.Error(t, err, "expected a failure due to TLS")
+	assert.Contains(t, err.Error(), "TLS", "should have been a TLS error")
+
+	// validate that we failed to connect and attempt any downloads at all
+	assert.Len(t, m.gotten, 0)
+}
+
+// the config can provide all the TLS information necessary - the root ca file,
+// the tls client files - they are all relative to the directory of the config
+// file, and not the cwd, or absolute paths
+func TestConfigFileTLSCanBeRelativeToConfigOrAbsolute(t *testing.T) {
+	// Set up server that with a self signed cert
+	var err error
+	// add a handler for getting the root
+	m := &recordingMetaStore{MemStorage: *storage.NewMemStorage()}
+	s := httptest.NewUnstartedServer(setupServerHandler(m))
+	s.TLS, err = tlsconfig.Server(tlsconfig.Options{
+		CertFile:   "../../fixtures/notary-server.crt",
+		KeyFile:    "../../fixtures/notary-server.key",
+		CAFile:     "../../fixtures/root-ca.crt",
+		ClientAuth: tls.RequireAndVerifyClientCert,
+	})
+	assert.NoError(t, err)
+	s.StartTLS()
+	defer s.Close()
+
+	tempDir, err := ioutil.TempDir("", "config-test")
+	assert.NoError(t, err)
+	defer os.RemoveAll(tempDir)
+	configFile, err := os.Create(filepath.Join(tempDir, "config.json"))
+	assert.NoError(t, err)
+	fmt.Fprintf(configFile, `{
+		"remote_server": {
+			"url": "%s",
+			"root_ca": "root-ca.crt",
+			"tls_client_cert": "%s",
+			"tls_client_key": "notary-server.key"
+		}
+	}`, s.URL, filepath.Join(tempDir, "notary-server.crt"))
+	configFile.Close()
+
+	// copy the certs to be relative to the config directory
+	for _, fname := range []string{"notary-server.crt", "notary-server.key", "root-ca.crt"} {
+		content, err := ioutil.ReadFile(filepath.Join("../../fixtures", fname))
+		assert.NoError(t, err)
+		assert.NoError(t, ioutil.WriteFile(filepath.Join(tempDir, fname), content, 0766))
+	}
+
+	// set a config file, so it doesn't check ~/.notary/config.json by default,
+	// and execute a random command so that the flags are parsed
+	cmd := NewNotaryCommand()
+	cmd.SetArgs([]string{"-c", configFile.Name(), "list", "repo"})
+	cmd.SetOutput(new(bytes.Buffer)) // eat the output
+	err = cmd.Execute()
+	assert.Error(t, err, "there was no repository, so list should have failed")
+	assert.NotContains(t, err.Error(), "TLS", "there was no TLS error though!")
+
+	// validate that we actually managed to connect and attempted to download the root though
+	assert.Len(t, m.gotten, 1)
+	assert.Equal(t, m.gotten[0], "repo.root")
+}
+
+// Whatever TLS config is in the config file can be overridden by the command line
+// TLS flags, which are relative to the CWD (not the config) or absolute
+func TestConfigFileOverridenByCmdLineFlags(t *testing.T) {
+	// Set up server that with a self signed cert
+	var err error
+	// add a handler for getting the root
+	m := &recordingMetaStore{MemStorage: *storage.NewMemStorage()}
+	s := httptest.NewUnstartedServer(setupServerHandler(m))
+	s.TLS, err = tlsconfig.Server(tlsconfig.Options{
+		CertFile:   "../../fixtures/notary-server.crt",
+		KeyFile:    "../../fixtures/notary-server.key",
+		CAFile:     "../../fixtures/root-ca.crt",
+		ClientAuth: tls.RequireAndVerifyClientCert,
+	})
+	assert.NoError(t, err)
+	s.StartTLS()
+	defer s.Close()
+
+	tempDir := tempDirWithConfig(t, fmt.Sprintf(`{
+		"remote_server": {
+			"url": "%s",
+			"root_ca": "nope",
+			"tls_client_cert": "nope",
+			"tls_client_key": "nope"
+		}
+	}`, s.URL))
+	defer os.RemoveAll(tempDir)
+	configFile := filepath.Join(tempDir, "config.json")
+
+	// set a config file, so it doesn't check ~/.notary/config.json by default,
+	// and execute a random command so that the flags are parsed
+	cwd, err := os.Getwd()
+	assert.NoError(t, err)
+
+	cmd := NewNotaryCommand()
+	cmd.SetArgs([]string{
+		"-c", configFile, "list", "repo",
+		"--tlscacert", "../../fixtures/root-ca.crt",
+		"--tlscert", filepath.Clean(filepath.Join(cwd, "../../fixtures/notary-server.crt")),
+		"--tlskey", "../../fixtures/notary-server.key"})
+	cmd.SetOutput(new(bytes.Buffer)) // eat the output
+	err = cmd.Execute()
+	assert.Error(t, err, "there was no repository, so list should have failed")
+	assert.NotContains(t, err.Error(), "TLS", "there was no TLS error though!")
+
+	// validate that we actually managed to connect and attempted to download the root though
+	assert.Len(t, m.gotten, 1)
+	assert.Equal(t, m.gotten[0], "repo.root")
+}

cmd/notary/tuf.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/distribution/registry/client/auth"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/docker/pkg/term"
+	"github.com/docker/go-connections/tlsconfig"
 	notaryclient "github.com/docker/notary/client"
 	"github.com/docker/notary/passphrase"
 	"github.com/docker/notary/tuf/data"
@@ -446,17 +447,26 @@ func (ps passwordStore) Basic(u *url.URL) (string, string) {
 func getTransport(config *viper.Viper, gun string, readOnly bool) (http.RoundTripper, error) {
 	// Attempt to get a root CA from the config file. Nil is the host defaults.
 	rootCAFile := utils.GetPathRelativeToConfig(config, "remote_server.root_ca")
+	clientCert := utils.GetPathRelativeToConfig(config, "remote_server.tls_client_cert")
+	clientKey := utils.GetPathRelativeToConfig(config, "remote_server.tls_client_key")
 
 	insecureSkipVerify := false
 	if config.IsSet("remote_server.skipTLSVerify") {
 		insecureSkipVerify = config.GetBool("remote_server.skipTLSVerify")
 	}
-	tlsConfig, err := utils.ConfigureClientTLS(&utils.ClientTLSOpts{
+
-		RootCAFile:         rootCAFile,
+	if clientCert == "" && clientKey != "" || clientCert != "" && clientKey == "" {
+		return nil, fmt.Errorf("either pass both client key and cert, or neither")
+	}
+
+	tlsConfig, err := tlsconfig.Client(tlsconfig.Options{
+		CAFile:             rootCAFile,
 		InsecureSkipVerify: insecureSkipVerify,
+		CertFile:           clientCert,
+		KeyFile:            clientKey,
 	})
 	if err != nil {
-		logrus.Fatal("Unable to configure TLS: ", err.Error())
+		return nil, fmt.Errorf("unable to configure TLS: %s", err.Error())
 	}
 
 	base := &http.Transport{

utils/configuration.go

@@ -3,6 +3,7 @@
 package utils
 
 import (
+	"crypto/tls"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -10,6 +11,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	bugsnag_hook "github.com/Sirupsen/logrus/hooks/bugsnag"
 	"github.com/bugsnag/bugsnag-go"
+	"github.com/docker/go-connections/tlsconfig"
 	"github.com/spf13/viper"
 )
 
@@ -35,40 +37,36 @@ func GetPathRelativeToConfig(configuration *viper.Viper, key string) string {
 	if p == "" || filepath.IsAbs(p) {
 		return p
 	}
-	return filepath.Join(filepath.Dir(configFile), p)
+	return filepath.Clean(filepath.Join(filepath.Dir(configFile), p))
 }
 
-// ParseServerTLS tries to parse out a valid ServerTLSOpts from a Viper:
+// ParseServerTLS tries to parse out valid server TLS options from a Viper.
-// - If TLS is required, both the cert and key must be provided
+// The cert/key files are relative to the config file used to populate the instance
-// - If TLS is not requried, either both the cert and key must be provided or
-//	 neither must be provided
-// The files are relative to the config file used to populate the instance
 // of viper.
-func ParseServerTLS(configuration *viper.Viper, tlsRequired bool) (*ServerTLSOpts, error) {
+func ParseServerTLS(configuration *viper.Viper, tlsRequired bool) (*tls.Config, error) {
 	//  unmarshalling into objects does not seem to pick up env vars
-	tlsOpts := ServerTLSOpts{
+	tlsOpts := tlsconfig.Options{
-		ServerCertFile: GetPathRelativeToConfig(configuration, "server.tls_cert_file"),
+		CertFile: GetPathRelativeToConfig(configuration, "server.tls_cert_file"),
-		ServerKeyFile:  GetPathRelativeToConfig(configuration, "server.tls_key_file"),
+		KeyFile:  GetPathRelativeToConfig(configuration, "server.tls_key_file"),
-		ClientCAFile:   GetPathRelativeToConfig(configuration, "server.client_ca_file"),
+		CAFile:   GetPathRelativeToConfig(configuration, "server.client_ca_file"),
+	}
+	if tlsOpts.CAFile != "" {
+		tlsOpts.ClientAuth = tls.RequireAndVerifyClientCert
 	}
 
-	cert, key := tlsOpts.ServerCertFile, tlsOpts.ServerKeyFile
+	if !tlsRequired {
-	if tlsRequired {
+		cert, key, ca := tlsOpts.CertFile, tlsOpts.KeyFile, tlsOpts.CAFile
-		if cert == "" || key == "" {
+		if cert == "" && key == "" && ca == "" {
-			return nil, fmt.Errorf("both the TLS certificate and key are mandatory")
-		}
-	} else {
-		if (cert == "" && key != "") || (cert != "" && key == "") {
-			return nil, fmt.Errorf(
-				"either include both a cert and key file, or neither to disable TLS")
-		}
-	}
-
-	if cert == "" && key == "" && tlsOpts.ClientCAFile == "" {
 			return nil, nil
 		}
 
-	return &tlsOpts, nil
+		if (cert == "" && key != "") || (cert != "" && key == "") || (cert == "" && key == "" && ca != "") {
+			return nil, fmt.Errorf(
+				"either include both a cert and key file, or no TLS information at all to disable TLS")
+		}
+	}
+
+	return tlsconfig.Server(tlsOpts)
 }
 
 // ParseLogLevel tries to parse out a log level from a Viper.  If there is no

utils/configuration_test.go

@@ -2,18 +2,28 @@ package utils
 
 import (
 	"bytes"
+	"crypto/tls"
 	"fmt"
 	"os"
+	"path/filepath"
+	"reflect"
 	"testing"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/bugsnag/bugsnag-go"
+	"github.com/docker/notary/trustmanager"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 )
 
 const envPrefix = "NOTARY_TESTING_ENV_PREFIX"
 
+const (
+	Cert = "../fixtures/notary-server.crt"
+	Key  = "../fixtures/notary-server.key"
+	Root = "../fixtures/root-ca.crt"
+)
+
 // initializes a viper object with test configuration
 func configure(jsonConfig string) *viper.Viper {
 	config := viper.New()
@@ -234,28 +244,27 @@ func TestParseStorageWithEnvironmentVariables(t *testing.T) {
 // If TLS is required and the parameters are missing, an error is returned
 func TestParseTLSNoTLSWhenRequired(t *testing.T) {
 	invalids := []string{
-		`{"server": {"tls_cert_file": "path/to/cert"}}`,
+		fmt.Sprintf(`{"server": {"tls_cert_file": "%s"}}`, Cert),
-		`{"server": {"tls_key_file": "path/to/key"}}`,
+		fmt.Sprintf(`{"server": {"tls_key_file": "%s"}}`, Key),
 	}
 	for _, configJSON := range invalids {
 		_, err := ParseServerTLS(configure(configJSON), true)
 		assert.Error(t, err)
-		assert.Contains(t, err.Error(),
+		assert.Contains(t, err.Error(), "no such file or directory")
-			"both the TLS certificate and key are mandatory")
 	}
 }
 
-// If TLS is not and the cert/key are partially provided, an error is returned
+// If TLS is not required and the cert/key are partially provided, an error is returned
 func TestParseTLSPartialTLS(t *testing.T) {
 	invalids := []string{
-		`{"server": {"tls_cert_file": "path/to/cert"}}`,
+		fmt.Sprintf(`{"server": {"tls_cert_file": "%s"}}`, Cert),
-		`{"server": {"tls_key_file": "path/to/key"}}`,
+		fmt.Sprintf(`{"server": {"tls_key_file": "%s"}}`, Key),
 	}
 	for _, configJSON := range invalids {
 		_, err := ParseServerTLS(configure(configJSON), false)
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(),
-			"either include both a cert and key file, or neither to disable TLS")
+			"either include both a cert and key file, or no TLS information at all to disable TLS")
 	}
 }
 
@@ -264,76 +273,95 @@ func TestParseTLSNoTLSNotRequired(t *testing.T) {
 		"server": {}
 	}`)
 
-	tlsOpts, err := ParseServerTLS(config, false)
+	tlsConfig, err := ParseServerTLS(config, false)
 	assert.NoError(t, err)
-	assert.Nil(t, tlsOpts)
+	assert.Nil(t, tlsConfig)
 }
 
 func TestParseTLSWithTLS(t *testing.T) {
-	config := configure(`{
+	config := configure(fmt.Sprintf(`{
 		"server": {
-			"tls_cert_file": "path/to/cert",
+			"tls_cert_file": "%s",
-			"tls_key_file": "path/to/key",
+			"tls_key_file": "%s",
-			"client_ca_file": "path/to/clientca"
+			"client_ca_file": "%s"
 		}
-	}`)
+	}`, Cert, Key, Root))
 
-	expected := ServerTLSOpts{
+	tlsConfig, err := ParseServerTLS(config, false)
-		ServerCertFile: "path/to/cert",
-		ServerKeyFile:  "path/to/key",
-		ClientCAFile:   "path/to/clientca",
-	}
-
-	tlsOpts, err := ParseServerTLS(config, false)
 	assert.NoError(t, err)
-	assert.Equal(t, expected, *tlsOpts)
+
+	expectedCert, err := tls.LoadX509KeyPair(Cert, Key)
+	assert.NoError(t, err)
+
+	expectedRoot, err := trustmanager.LoadCertFromFile(Root)
+	assert.NoError(t, err)
+
+	assert.Len(t, tlsConfig.Certificates, 1)
+	assert.True(t, reflect.DeepEqual(expectedCert, tlsConfig.Certificates[0]))
+
+	subjects := tlsConfig.ClientCAs.Subjects()
+	assert.Len(t, subjects, 1)
+	assert.True(t, bytes.Equal(expectedRoot.RawSubject, subjects[0]))
+	assert.Equal(t, tlsConfig.ClientAuth, tls.RequireAndVerifyClientCert)
 }
 
 func TestParseTLSWithTLSRelativeToConfigFile(t *testing.T) {
-	config := configure(`{
+	currDir, err := os.Getwd()
+	assert.NoError(t, err)
+
+	config := configure(fmt.Sprintf(`{
 		"server": {
-			"tls_cert_file": "path/to/cert",
+			"tls_cert_file": "%s",
-			"tls_key_file": "/abspath/to/key",
+			"tls_key_file": "%s",
 			"client_ca_file": ""
 		}
-	}`)
+	}`, Cert, filepath.Clean(filepath.Join(currDir, Key))))
-	config.SetConfigFile("/opt/me.json")
+	config.SetConfigFile(filepath.Join(currDir, "me.json"))
 
-	expected := ServerTLSOpts{
+	tlsConfig, err := ParseServerTLS(config, false)
-		ServerCertFile: "/opt/path/to/cert",
-		ServerKeyFile:  "/abspath/to/key",
-		ClientCAFile:   "",
-	}
-
-	tlsOpts, err := ParseServerTLS(config, false)
 	assert.NoError(t, err)
-	assert.Equal(t, expected, *tlsOpts)
+
+	expectedCert, err := tls.LoadX509KeyPair(Cert, Key)
+	assert.NoError(t, err)
+
+	assert.Len(t, tlsConfig.Certificates, 1)
+	assert.True(t, reflect.DeepEqual(expectedCert, tlsConfig.Certificates[0]))
+
+	assert.Nil(t, tlsConfig.ClientCAs)
+	assert.Equal(t, tlsConfig.ClientAuth, tls.NoClientCert)
 }
 
 func TestParseTLSWithEnvironmentVariables(t *testing.T) {
-	config := configure(`{
+	config := configure(fmt.Sprintf(`{
 		"server": {
-			"tls_cert_file": "path/to/cert",
+			"tls_cert_file": "%s",
 			"client_ca_file": "nosuchfile"
 		}
-	}`)
+	}`, Cert))
 
 	vars := map[string]string{
-		"SERVER_TLS_KEY_FILE":   "path/to/key",
+		"SERVER_TLS_KEY_FILE":   Key,
-		"SERVER_CLIENT_CA_FILE": "path/to/clientca",
+		"SERVER_CLIENT_CA_FILE": Root,
 	}
 	setupEnvironmentVariables(t, vars)
 	defer cleanupEnvironmentVariables(t, vars)
 
-	expected := ServerTLSOpts{
+	tlsConfig, err := ParseServerTLS(config, true)
-		ServerCertFile: "path/to/cert",
-		ServerKeyFile:  "path/to/key",
-		ClientCAFile:   "path/to/clientca",
-	}
-
-	tlsOpts, err := ParseServerTLS(config, true)
 	assert.NoError(t, err)
-	assert.Equal(t, expected, *tlsOpts)
+
+	expectedCert, err := tls.LoadX509KeyPair(Cert, Key)
+	assert.NoError(t, err)
+
+	expectedRoot, err := trustmanager.LoadCertFromFile(Root)
+	assert.NoError(t, err)
+
+	assert.Len(t, tlsConfig.Certificates, 1)
+	assert.True(t, reflect.DeepEqual(expectedCert, tlsConfig.Certificates[0]))
+
+	subjects := tlsConfig.ClientCAs.Subjects()
+	assert.Len(t, subjects, 1)
+	assert.True(t, bytes.Equal(expectedRoot.RawSubject, subjects[0]))
+	assert.Equal(t, tlsConfig.ClientAuth, tls.RequireAndVerifyClientCert)
 }
 
 func TestParseViperWithInvalidFile(t *testing.T) {

utils/tls_config.go

@@ -1,129 +0,0 @@
-package utils
-
-import (
-	"crypto/rand"
-	"crypto/tls"
-	"crypto/x509"
-	"fmt"
-	"io/ioutil"
-)
-
-// Client TLS cipher suites (dropping CBC ciphers for client preferred suite set)
-var clientCipherSuites = []uint16{
-	tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-	tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-}
-
-// Server TLS cipher suites
-var serverCipherSuites = append(clientCipherSuites, []uint16{
-	tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
-	tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
-	tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
-	tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
-	tls.TLS_RSA_WITH_AES_256_CBC_SHA,
-	tls.TLS_RSA_WITH_AES_128_CBC_SHA,
-}...)
-
-func poolFromFile(filename string) (*x509.CertPool, error) {
-	pemBytes, err := ioutil.ReadFile(filename)
-	if err != nil {
-		return nil, err
-	}
-	pool := x509.NewCertPool()
-	if ok := pool.AppendCertsFromPEM(pemBytes); !ok {
-		return nil, fmt.Errorf(
-			"Unable to parse certificates from %s", filename)
-	}
-	if len(pool.Subjects()) == 0 {
-		return nil, fmt.Errorf(
-			"No certificates parsed from %s", filename)
-	}
-	return pool, nil
-}
-
-// ServerTLSOpts generates a tls configuration for servers using the
-// provided parameters.
-type ServerTLSOpts struct {
-	ServerCertFile string
-	ServerKeyFile  string
-	ClientCAFile   string
-}
-
-// ConfigureServerTLS specifies a set of ciphersuites, the server cert and key,
-// and optionally client authentication.  Note that a tls configuration is
-// constructed that either requires and verifies client authentication or
-// doesn't deal with client certs at all. Nothing in the middle.
-//
-// Also note that if the client CA file contains invalid data, behavior is not
-// guaranteed.  Currently (as of Go 1.5.1) only the valid certificates up to
-// the bad data will be parsed and added the client CA pool.
-func ConfigureServerTLS(opts *ServerTLSOpts) (*tls.Config, error) {
-	keypair, err := tls.LoadX509KeyPair(
-		opts.ServerCertFile, opts.ServerKeyFile)
-	if err != nil {
-		return nil, err
-	}
-
-	tlsConfig := &tls.Config{
-		MinVersion:               tls.VersionTLS12,
-		PreferServerCipherSuites: true,
-		CipherSuites:             serverCipherSuites,
-		Certificates:             []tls.Certificate{keypair},
-		Rand:                     rand.Reader,
-	}
-
-	if opts.ClientCAFile != "" {
-		pool, err := poolFromFile(opts.ClientCAFile)
-		if err != nil {
-			return nil, err
-		}
-		tlsConfig.ClientCAs = pool
-		tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
-	}
-
-	return tlsConfig, nil
-}
-
-// ClientTLSOpts is a struct that contains options to pass to
-// ConfigureClientTLS
-type ClientTLSOpts struct {
-	RootCAFile         string
-	ServerName         string
-	InsecureSkipVerify bool
-	ClientCertFile     string
-	ClientKeyFile      string
-}
-
-// ConfigureClientTLS generates a tls configuration for clients using the
-// provided parameters.
-///
-// Note that if the root CA file contains invalid data, behavior is not
-// guaranteed.  Currently (as of Go 1.5.1) only the valid certificates up to
-// the bad data will be parsed and added the root CA pool.
-func ConfigureClientTLS(opts *ClientTLSOpts) (*tls.Config, error) {
-	tlsConfig := &tls.Config{
-		InsecureSkipVerify: opts.InsecureSkipVerify,
-		MinVersion:         tls.VersionTLS12,
-		CipherSuites:       clientCipherSuites,
-		ServerName:         opts.ServerName,
-	}
-
-	if opts.RootCAFile != "" {
-		pool, err := poolFromFile(opts.RootCAFile)
-		if err != nil {
-			return nil, err
-		}
-		tlsConfig.RootCAs = pool
-	}
-
-	if opts.ClientCertFile != "" || opts.ClientKeyFile != "" {
-		keypair, err := tls.LoadX509KeyPair(
-			opts.ClientCertFile, opts.ClientKeyFile)
-		if err != nil {
-			return nil, err
-		}
-		tlsConfig.Certificates = []tls.Certificate{keypair}
-	}
-
-	return tlsConfig, nil
-}

utils/tls_config_test.go

@@ -1,227 +0,0 @@
-package utils
-
-import (
-	"crypto"
-	"crypto/ecdsa"
-	"crypto/elliptic"
-	"crypto/rand"
-	"crypto/rsa"
-	"crypto/tls"
-	"io/ioutil"
-	"os"
-	"testing"
-
-	"github.com/docker/notary/cryptoservice"
-	"github.com/docker/notary/trustmanager"
-	"github.com/stretchr/testify/assert"
-)
-
-const (
-	ServerCert = "../fixtures/notary-server.crt"
-	ServerKey  = "../fixtures/notary-server.key"
-	RootCA     = "../fixtures/root-ca.crt"
-)
-
-// generates a multiple-certificate file with both RSA and ECDSA certs and
-// returns the filename so that cleanup can be deferred.
-func generateMultiCert(t *testing.T) string {
-	tempFile, err := ioutil.TempFile("/tmp", "cert-test")
-	defer tempFile.Close()
-	assert.NoError(t, err)
-
-	rsaKey, err := rsa.GenerateKey(rand.Reader, 2048)
-	assert.NoError(t, err)
-	ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
-	assert.NoError(t, err)
-
-	for _, key := range []crypto.Signer{rsaKey, ecKey} {
-		cert, err := cryptoservice.GenerateTestingCertificate(key, "gun")
-		assert.NoError(t, err)
-
-		pemBytes := trustmanager.CertToPEM(cert)
-		nBytes, err := tempFile.Write(pemBytes)
-		assert.NoError(t, err)
-		assert.Equal(t, nBytes, len(pemBytes))
-	}
-	return tempFile.Name()
-}
-
-// If the cert files and directory are provided but are invalid, an error is
-// returned.
-func TestConfigServerTLSFailsIfUnableToLoadCerts(t *testing.T) {
-	for i := 0; i < 3; i++ {
-		files := []string{ServerCert, ServerKey, RootCA}
-		files[i] = "not-real-file"
-
-		result, err := ConfigureServerTLS(&ServerTLSOpts{
-			ServerCertFile: files[0],
-			ServerKeyFile:  files[1],
-			ClientCAFile:   files[2],
-		})
-		assert.Nil(t, result)
-		assert.Error(t, err)
-	}
-}
-
-// If server cert and key are provided, and client auth is disabled, then
-// a valid tls.Config is returned with ClientAuth set to NoClientCert
-func TestConfigServerTLSServerCertsOnly(t *testing.T) {
-	keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey)
-	assert.NoError(t, err)
-
-	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
-		ServerCertFile: ServerCert,
-		ServerKeyFile:  ServerKey,
-	})
-	assert.NoError(t, err)
-	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
-	assert.True(t, tlsConfig.PreferServerCipherSuites)
-	assert.Equal(t, tls.NoClientCert, tlsConfig.ClientAuth)
-	assert.Nil(t, tlsConfig.ClientCAs)
-}
-
-// If a valid client cert file is provided, but it contains no client
-// certs, an error is returned.
-func TestConfigServerTLSWithEmptyCACertFile(t *testing.T) {
-	tempFile, err := ioutil.TempFile("/tmp", "cert-test")
-	assert.NoError(t, err)
-	defer os.RemoveAll(tempFile.Name())
-	tempFile.Close()
-
-	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
-		ServerCertFile: ServerCert,
-		ServerKeyFile:  ServerKey,
-		ClientCAFile:   tempFile.Name(),
-	})
-	assert.Nil(t, tlsConfig)
-	assert.Error(t, err)
-}
-
-// If server cert and key are provided, and client cert file is provided with
-// one cert, a valid tls.Config is returned with the clientCAs set to that
-// cert.  ClientAuth is set to RequireAndVerifyClientCert.
-func TestConfigServerTLSWithOneCACert(t *testing.T) {
-	keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey)
-	assert.NoError(t, err)
-
-	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
-		ServerCertFile: ServerCert,
-		ServerKeyFile:  ServerKey,
-		ClientCAFile:   RootCA,
-	})
-	assert.NoError(t, err)
-	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
-	assert.True(t, tlsConfig.PreferServerCipherSuites)
-	assert.Equal(t, tls.RequireAndVerifyClientCert, tlsConfig.ClientAuth)
-	assert.Len(t, tlsConfig.ClientCAs.Subjects(), 1)
-}
-
-// If server cert and key are provided, and client cert file is provided with
-// multiple certs, a valid tls.Config is returned with the clientCAs set to
-// the valid cert.  ClientAuth is set to RequireAndVerifyClientCert.
-func TestConfigServerTLSWithMultipleCACerts(t *testing.T) {
-	tempFilename := generateMultiCert(t)
-	defer os.RemoveAll(tempFilename)
-
-	keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey)
-	assert.NoError(t, err)
-
-	tlsConfig, err := ConfigureServerTLS(&ServerTLSOpts{
-		ServerCertFile: ServerCert,
-		ServerKeyFile:  ServerKey,
-		ClientCAFile:   tempFilename,
-	})
-	assert.NoError(t, err)
-	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
-	assert.True(t, tlsConfig.PreferServerCipherSuites)
-	assert.Equal(t, tls.RequireAndVerifyClientCert, tlsConfig.ClientAuth)
-	assert.Len(t, tlsConfig.ClientCAs.Subjects(), 2)
-}
-
-// The skipVerify boolean gets set on the tls.Config's InsecureSkipBoolean
-func TestConfigClientTLSNoVerify(t *testing.T) {
-	for _, skip := range []bool{true, false} {
-		tlsConfig, err := ConfigureClientTLS(
-			&ClientTLSOpts{InsecureSkipVerify: skip})
-		assert.NoError(t, err)
-		assert.Nil(t, tlsConfig.Certificates)
-		assert.Equal(t, skip, tlsConfig.InsecureSkipVerify)
-		assert.Equal(t, "", tlsConfig.ServerName)
-		assert.Nil(t, tlsConfig.RootCAs)
-	}
-}
-
-// The skipVerify boolean gets set on the tls.Config's InsecureSkipBoolean
-func TestConfigClientServerName(t *testing.T) {
-	for _, name := range []string{"", "myname"} {
-		tlsConfig, err := ConfigureClientTLS(&ClientTLSOpts{ServerName: name})
-		assert.NoError(t, err)
-		assert.Nil(t, tlsConfig.Certificates)
-		assert.Equal(t, false, tlsConfig.InsecureSkipVerify)
-		assert.Equal(t, name, tlsConfig.ServerName)
-		assert.Nil(t, tlsConfig.RootCAs)
-	}
-}
-
-// The RootCA is set if the file provided has a single CA cert.
-func TestConfigClientTLSRootCAFileWithOneCert(t *testing.T) {
-	tlsConfig, err := ConfigureClientTLS(&ClientTLSOpts{RootCAFile: RootCA})
-	assert.NoError(t, err)
-	assert.Nil(t, tlsConfig.Certificates)
-	assert.Equal(t, false, tlsConfig.InsecureSkipVerify)
-	assert.Equal(t, "", tlsConfig.ServerName)
-	assert.Len(t, tlsConfig.RootCAs.Subjects(), 1)
-}
-
-// If the root CA file provided has multiple CA certs, only the valid certs
-// are read.
-func TestConfigClientTLSRootCAFileMultipleCerts(t *testing.T) {
-	tempFilename := generateMultiCert(t)
-	defer os.RemoveAll(tempFilename)
-
-	tlsConfig, err := ConfigureClientTLS(
-		&ClientTLSOpts{RootCAFile: tempFilename})
-	assert.NoError(t, err)
-	assert.Nil(t, tlsConfig.Certificates)
-	assert.Equal(t, false, tlsConfig.InsecureSkipVerify)
-	assert.Equal(t, "", tlsConfig.ServerName)
-	assert.Len(t, tlsConfig.RootCAs.Subjects(), 2)
-}
-
-// An error is returned if a root CA is provided but the file doesn't exist.
-func TestConfigClientTLSNonexistentRootCAFile(t *testing.T) {
-	tlsConfig, err := ConfigureClientTLS(
-		&ClientTLSOpts{RootCAFile: "not-a-file"})
-	assert.Error(t, err)
-	assert.Nil(t, tlsConfig)
-}
-
-// An error is returned if either the client cert or the key are provided
-// but invalid or blank.
-func TestConfigClientTLSClientCertOrKeyInvalid(t *testing.T) {
-	for i := 0; i < 2; i++ {
-		for _, invalid := range []string{"not-a-file", ""} {
-			files := []string{ServerCert, ServerKey}
-			files[i] = invalid
-			tlsConfig, err := ConfigureClientTLS(&ClientTLSOpts{
-				ClientCertFile: files[0], ClientKeyFile: files[1]})
-			assert.Error(t, err)
-			assert.Nil(t, tlsConfig)
-		}
-	}
-}
-
-// The certificate is set if the client cert and client key are provided and
-// valid.
-func TestConfigClientTLSValidClientCertAndKey(t *testing.T) {
-	keypair, err := tls.LoadX509KeyPair(ServerCert, ServerKey)
-	assert.NoError(t, err)
-
-	tlsConfig, err := ConfigureClientTLS(&ClientTLSOpts{
-		ClientCertFile: ServerCert, ClientKeyFile: ServerKey})
-	assert.NoError(t, err)
-	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
-	assert.Equal(t, false, tlsConfig.InsecureSkipVerify)
-	assert.Equal(t, "", tlsConfig.ServerName)
-	assert.Nil(t, tlsConfig.RootCAs)
-}