Refactor TLS code with a new `tlsconfig` package

This patch creates a new `tlsconfig` package to handle creation of
secure-enough TLS configurations for clients and servers.

The package was created by refactoring TLS code in the client and the
daemon. After this patch, it is expected that all code creating TLS
configurations use this `tlsconfig` package for greater security,
consistency and readability.

On the server side, this fixes a bug where --tlsverify was not taken
into account. Now, if specified, it will require the client to
authenticate.

Signed-off-by: Tibor Vass <tibor@docker.com>

api/server/server.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"crypto/tls"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -44,11 +45,7 @@ type ServerConfig struct {
 	CorsHeaders string
 	Version     string
 	SocketGroup string
-	Tls         bool
-	TlsVerify   bool
-	TlsCa       string
-	TlsCert     string
-	TlsKey      string
+	TLSConfig   *tls.Config
 }
 
 type Server struct {
@@ -1429,22 +1426,15 @@ func (s *Server) ping(version version.Version, w http.ResponseWriter, r *http.Re
 }
 
 func (s *Server) initTcpSocket(addr string) (l net.Listener, err error) {
-	if !s.cfg.TlsVerify {
+	if s.cfg.TLSConfig == nil || s.cfg.TLSConfig.ClientAuth != tls.RequireAndVerifyClientCert {
 		logrus.Warn("/!\\ DON'T BIND ON ANY IP ADDRESS WITHOUT setting -tlsverify IF YOU DON'T KNOW WHAT YOU'RE DOING /!\\")
 	}
-
-	var c *sockets.TlsConfig
-	if s.cfg.Tls || s.cfg.TlsVerify {
-		c = sockets.NewTlsConfig(s.cfg.TlsCert, s.cfg.TlsKey, s.cfg.TlsCa, s.cfg.TlsVerify)
-	}
-
-	if l, err = sockets.NewTcpSocket(addr, c, s.start); err != nil {
+	if l, err = sockets.NewTcpSocket(addr, s.cfg.TLSConfig, s.start); err != nil {
 		return nil, err
 	}
 	if err := allocateDaemonPort(addr); err != nil {
 		return nil, err
 	}
-
 	return
 }
 

docker/daemon.go

@@ -3,6 +3,7 @@
 package main
 
 import (
+	"crypto/tls"
 	"fmt"
 	"io"
 	"os"
@@ -21,6 +22,7 @@ import (
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/timeutils"
+	"github.com/docker/docker/pkg/tlsconfig"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/utils"
 )
@@ -112,11 +114,17 @@ func mainDaemon() {
 		CorsHeaders: daemonCfg.CorsHeaders,
 		Version:     dockerversion.VERSION,
 		SocketGroup: daemonCfg.SocketGroup,
-		Tls:         *flTls,
-		TlsVerify:   *flTlsVerify,
-		TlsCa:       *flCa,
-		TlsCert:     *flCert,
-		TlsKey:      *flKey,
+	}
+
+	if *flTls {
+		if *flTlsVerify {
+			tlsOptions.ClientAuth = tls.RequireAndVerifyClientCert
+		}
+		tlsConfig, err := tlsconfig.Server(tlsOptions)
+		if err != nil {
+			logrus.Fatal(err)
+		}
+		serverConfig.TLSConfig = tlsConfig
 	}
 
 	api := apiserver.New(serverConfig)

docker/docker.go

@@ -2,9 +2,7 @@ package main
 
 import (
 	"crypto/tls"
-	"crypto/x509"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"runtime"
 	"strings"
@@ -16,6 +14,7 @@ import (
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/docker/pkg/term"
+	"github.com/docker/docker/pkg/tlsconfig"
 	"github.com/docker/docker/utils"
 )
 
@@ -85,6 +84,12 @@ func main() {
 
 	setDefaultConfFlag(flTrustKey, defaultTrustKeyFile)
 
+	// Regardless of whether the user sets it to true or false, if they
+	// specify --tlsverify at all then we need to turn on tls
+	if flag.IsSet("-tlsverify") {
+		*flTls = true
+	}
+
 	if *flDaemon {
 		if *flHelp {
 			flag.Usage()
@@ -94,59 +99,35 @@ func main() {
 		return
 	}
 
+	// From here on, we assume we're a client, not a server.
+
 	if len(flHosts) > 1 {
 		fmt.Fprintf(os.Stderr, "Please specify only one -H")
 		os.Exit(0)
 	}
 	protoAddrParts := strings.SplitN(flHosts[0], "://", 2)
 
-	var (
-		cli       *client.DockerCli
-		tlsConfig tls.Config
-	)
-	tlsConfig.InsecureSkipVerify = true
-
-	// Regardless of whether the user sets it to true or false, if they
-	// specify --tlsverify at all then we need to turn on tls
-	if flag.IsSet("-tlsverify") {
-		*flTls = true
-	}
-
-	// If we should verify the server, we need to load a trusted ca
-	if *flTlsVerify {
-		certPool := x509.NewCertPool()
-		file, err := ioutil.ReadFile(*flCa)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Couldn't read ca cert %s: %s\n", *flCa, err)
-			os.Exit(1)
+	var tlsConfig *tls.Config
+	if *flTls {
+		tlsOptions.InsecureSkipVerify = !*flTlsVerify
+		if !flag.IsSet("-tlscert") {
+			if _, err := os.Stat(tlsOptions.CertFile); os.IsNotExist(err) {
+				tlsOptions.CertFile = ""
+			}
 		}
-		certPool.AppendCertsFromPEM(file)
-		tlsConfig.RootCAs = certPool
-		tlsConfig.InsecureSkipVerify = false
-	}
-
-	// If tls is enabled, try to load and send client certificates
-	if *flTls || *flTlsVerify {
-		_, errCert := os.Stat(*flCert)
-		_, errKey := os.Stat(*flKey)
-		if errCert == nil && errKey == nil {
-			*flTls = true
-			cert, err := tls.LoadX509KeyPair(*flCert, *flKey)
-			if err != nil {
-				fmt.Fprintf(os.Stderr, "Couldn't load X509 key pair: %q. Make sure the key is encrypted\n", err)
-				os.Exit(1)
+		if !flag.IsSet("-tlskey") {
+			if _, err := os.Stat(tlsOptions.KeyFile); os.IsNotExist(err) {
+				tlsOptions.KeyFile = ""
 			}
-			tlsConfig.Certificates = []tls.Certificate{cert}
 		}
-		// Avoid fallback to SSL protocols < TLS1.0
-		tlsConfig.MinVersion = tls.VersionTLS10
-	}
-
-	if *flTls || *flTlsVerify {
-		cli = client.NewDockerCli(stdin, stdout, stderr, *flTrustKey, protoAddrParts[0], protoAddrParts[1], &tlsConfig)
-	} else {
-		cli = client.NewDockerCli(stdin, stdout, stderr, *flTrustKey, protoAddrParts[0], protoAddrParts[1], nil)
+		var err error
+		tlsConfig, err = tlsconfig.Client(tlsOptions)
+		if err != nil {
+			fmt.Fprintln(stderr, err)
+			os.Exit(1)
+		}
 	}
+	cli := client.NewDockerCli(stdin, stdout, stderr, *flTrustKey, protoAddrParts[0], protoAddrParts[1], tlsConfig)
 
 	if err := cli.Cmd(flag.Args()...); err != nil {
 		if sterr, ok := err.(client.StatusError); ok {

docker/flags.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/homedir"
 	flag "github.com/docker/docker/pkg/mflag"
+	"github.com/docker/docker/pkg/tlsconfig"
 )
 
 type command struct {
@@ -94,10 +95,8 @@ var (
 	flTlsVerify = flag.Bool([]string{"-tlsverify"}, dockerTlsVerify, "Use TLS and verify the remote")
 
 	// these are initialized in init() below since their default values depend on dockerCertPath which isn't fully initialized until init() runs
+	tlsOptions tlsconfig.Options
 	flTrustKey *string
-	flCa       *string
-	flCert     *string
-	flKey      *string
 	flHosts    []string
 )
 
@@ -116,9 +115,9 @@ func init() {
 	// TODO use flag flag.String([]string{"i", "-identity"}, "", "Path to libtrust key file")
 	flTrustKey = &placeholderTrustKey
 
-	flCa = flag.String([]string{"-tlscacert"}, filepath.Join(dockerCertPath, defaultCaFile), "Trust certs signed only by this CA")
-	flCert = flag.String([]string{"-tlscert"}, filepath.Join(dockerCertPath, defaultCertFile), "Path to TLS certificate file")
-	flKey = flag.String([]string{"-tlskey"}, filepath.Join(dockerCertPath, defaultKeyFile), "Path to TLS key file")
+	flag.StringVar(&tlsOptions.CAFile, []string{"-tlscacert"}, filepath.Join(dockerCertPath, defaultCaFile), "Trust certs signed only by this CA")
+	flag.StringVar(&tlsOptions.CertFile, []string{"-tlscert"}, filepath.Join(dockerCertPath, defaultCertFile), "Path to TLS certificate file")
+	flag.StringVar(&tlsOptions.KeyFile, []string{"-tlskey"}, filepath.Join(dockerCertPath, defaultKeyFile), "Path to TLS key file")
 	opts.HostListVar(&flHosts, []string{"H", "-host"}, "Daemon socket(s) to connect to")
 
 	flag.Usage = func() {

pkg/sockets/tcp_socket.go

@@ -2,68 +2,19 @@ package sockets
 
 import (
 	"crypto/tls"
-	"crypto/x509"
-	"fmt"
-	"io/ioutil"
 	"net"
-	"os"
 
 	"github.com/docker/docker/pkg/listenbuffer"
 )
 
-type TlsConfig struct {
-	CA          string
-	Certificate string
-	Key         string
-	Verify      bool
-}
-
-func NewTlsConfig(tlsCert, tlsKey, tlsCA string, verify bool) *TlsConfig {
-	return &TlsConfig{
-		Verify:      verify,
-		Certificate: tlsCert,
-		Key:         tlsKey,
-		CA:          tlsCA,
-	}
-}
-
-func NewTcpSocket(addr string, config *TlsConfig, activate <-chan struct{}) (net.Listener, error) {
+func NewTcpSocket(addr string, tlsConfig *tls.Config, activate <-chan struct{}) (net.Listener, error) {
 	l, err := listenbuffer.NewListenBuffer("tcp", addr, activate)
 	if err != nil {
 		return nil, err
 	}
-	if config != nil {
-		if l, err = setupTls(l, config); err != nil {
-			return nil, err
-		}
+	if tlsConfig != nil {
+		tlsConfig.NextProtos = []string{"http/1.1"}
+		l = tls.NewListener(l, tlsConfig)
 	}
 	return l, nil
 }
-
-func setupTls(l net.Listener, config *TlsConfig) (net.Listener, error) {
-	tlsCert, err := tls.LoadX509KeyPair(config.Certificate, config.Key)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil, fmt.Errorf("Could not load X509 key pair (%s, %s): %v", config.Certificate, config.Key, err)
-		}
-		return nil, fmt.Errorf("Error reading X509 key pair (%s, %s): %q. Make sure the key is encrypted.",
-			config.Certificate, config.Key, err)
-	}
-	tlsConfig := &tls.Config{
-		NextProtos:   []string{"http/1.1"},
-		Certificates: []tls.Certificate{tlsCert},
-		// Avoid fallback on insecure SSL protocols
-		MinVersion: tls.VersionTLS10,
-	}
-	if config.CA != "" {
-		certPool := x509.NewCertPool()
-		file, err := ioutil.ReadFile(config.CA)
-		if err != nil {
-			return nil, fmt.Errorf("Could not read CA certificate: %v", err)
-		}
-		certPool.AppendCertsFromPEM(file)
-		tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
-		tlsConfig.ClientCAs = certPool
-	}
-	return tls.NewListener(l, tlsConfig), nil
-}

pkg/tlsconfig/config.go

@@ -0,0 +1,107 @@
+// Package tlsconfig provides primitives to retrieve secure-enough TLS configurations for both clients and servers.
+//
+// As a reminder from https://golang.org/pkg/crypto/tls/#Config:
+//	A Config structure is used to configure a TLS client or server. After one has been passed to a TLS function it must not be modified.
+//	A Config may be reused; the tls package will also not modify it.
+package tlsconfig
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+	"io/ioutil"
+	"os"
+
+	"github.com/Sirupsen/logrus"
+)
+
+// Options represents the information needed to create client and server TLS configurations.
+type Options struct {
+	InsecureSkipVerify bool
+	ClientAuth         tls.ClientAuthType
+	CAFile             string
+	CertFile           string
+	KeyFile            string
+}
+
+// Default is a secure-enough TLS configuration.
+var Default = tls.Config{
+	// Avoid fallback to SSL protocols < TLS1.0
+	MinVersion:               tls.VersionTLS10,
+	PreferServerCipherSuites: true,
+	CipherSuites: []uint16{
+		tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+		tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+		tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_RSA_WITH_AES_128_CBC_SHA,
+		tls.TLS_RSA_WITH_AES_256_CBC_SHA,
+	},
+}
+
+// certPool returns an X.509 certificate pool from `caFile`, the certificate file.
+func certPool(caFile string) (*x509.CertPool, error) {
+	// If we should verify the server, we need to load a trusted ca
+	certPool := x509.NewCertPool()
+	pem, err := ioutil.ReadFile(caFile)
+	if err != nil {
+		return nil, fmt.Errorf("Could not read CA certificate %s: %v", caFile, err)
+	}
+	if !certPool.AppendCertsFromPEM(pem) {
+		return nil, fmt.Errorf("failed to append certificates from PEM file: %s", caFile)
+	}
+	s := certPool.Subjects()
+	subjects := make([]string, len(s))
+	for i, subject := range s {
+		subjects[i] = string(subject)
+	}
+	logrus.Debugf("Trusting certs with subjects: %v", subjects)
+	return certPool, nil
+}
+
+// Client returns a TLS configuration meant to be used by a client.
+func Client(options Options) (*tls.Config, error) {
+	tlsConfig := Default
+	tlsConfig.InsecureSkipVerify = options.InsecureSkipVerify
+	if !options.InsecureSkipVerify {
+		CAs, err := certPool(options.CAFile)
+		if err != nil {
+			return nil, err
+		}
+		tlsConfig.RootCAs = CAs
+	}
+
+	if options.CertFile != "" && options.KeyFile != "" {
+		tlsCert, err := tls.LoadX509KeyPair(options.CertFile, options.KeyFile)
+		if err != nil {
+			return nil, fmt.Errorf("Could not load X509 key pair: %v. Make sure the key is not encrypted", err)
+		}
+		tlsConfig.Certificates = []tls.Certificate{tlsCert}
+	}
+
+	return &tlsConfig, nil
+}
+
+// Server returns a TLS configuration meant to be used by a server.
+func Server(options Options) (*tls.Config, error) {
+	tlsConfig := Default
+	tlsConfig.ClientAuth = options.ClientAuth
+	tlsCert, err := tls.LoadX509KeyPair(options.CertFile, options.KeyFile)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, fmt.Errorf("Could not load X509 key pair (%s, %s): %v", options.CertFile, options.KeyFile, err)
+		}
+		return nil, fmt.Errorf("Error reading X509 key pair (%s, %s): %v. Make sure the key is not encrypted.", options.CertFile, options.KeyFile, err)
+	}
+	tlsConfig.Certificates = []tls.Certificate{tlsCert}
+	if options.ClientAuth >= tls.VerifyClientCertIfGiven {
+		CAs, err := certPool(options.CAFile)
+		if err != nil {
+			return nil, err
+		}
+		tlsConfig.ClientCAs = CAs
+	}
+	return &tlsConfig, nil
+}