From bfed4b7cc3820ee3a74580aca55d5918bf05eef5 Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Thu, 7 May 2015 09:49:07 -0700 Subject: [PATCH] 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 --- api/server/server.go | 18 ++----- docker/daemon.go | 18 +++++-- docker/docker.go | 71 +++++++++---------------- docker/flags.go | 11 ++-- pkg/sockets/tcp_socket.go | 57 ++------------------ pkg/tlsconfig/config.go | 107 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 159 insertions(+), 123 deletions(-) create mode 100644 pkg/tlsconfig/config.go diff --git a/api/server/server.go b/api/server/server.go index d7ec2d47d7..d17288f2c8 100644 --- a/api/server/server.go +++ b/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 } diff --git a/docker/daemon.go b/docker/daemon.go index fc08bc9c26..0dbcd22c93 100644 --- a/docker/daemon.go +++ b/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) diff --git a/docker/docker.go b/docker/docker.go index 092e04fc83..f7fd62b6c5 100644 --- a/docker/docker.go +++ b/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) + var tlsConfig *tls.Config + if *flTls { + tlsOptions.InsecureSkipVerify = !*flTlsVerify + if !flag.IsSet("-tlscert") { + if _, err := os.Stat(tlsOptions.CertFile); os.IsNotExist(err) { + tlsOptions.CertFile = "" + } + } + if !flag.IsSet("-tlskey") { + if _, err := os.Stat(tlsOptions.KeyFile); os.IsNotExist(err) { + tlsOptions.KeyFile = "" + } + } + var err error + tlsConfig, err = tlsconfig.Client(tlsOptions) if err != nil { - fmt.Fprintf(os.Stderr, "Couldn't read ca cert %s: %s\n", *flCa, err) + fmt.Fprintln(stderr, err) os.Exit(1) } - 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) - } - 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) } + 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 { diff --git a/docker/flags.go b/docker/flags.go index cbdb6a859d..d860ec7e1f 100644 --- a/docker/flags.go +++ b/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() { diff --git a/pkg/sockets/tcp_socket.go b/pkg/sockets/tcp_socket.go index ac9edaebd1..658ee23dd7 100644 --- a/pkg/sockets/tcp_socket.go +++ b/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 -} diff --git a/pkg/tlsconfig/config.go b/pkg/tlsconfig/config.go new file mode 100644 index 0000000000..ee67f5cc91 --- /dev/null +++ b/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 +}