diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 842e7f4eb8..275d9a7262 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "fmt" + "net" "os" "path/filepath" "runtime" @@ -380,8 +381,16 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { conf.Debug = opts.Debug conf.Hosts = opts.Hosts conf.LogLevel = opts.LogLevel - conf.TLS = opts.TLS - conf.TLSVerify = opts.TLSVerify + + if opts.flags.Changed(FlagTLS) { + conf.TLS = &opts.TLS + } + if opts.flags.Changed(FlagTLSVerify) { + conf.TLSVerify = &opts.TLSVerify + v := true + conf.TLS = &v + } + conf.CommonTLSOptions = config.CommonTLSOptions{} if opts.TLSOptions != nil { @@ -409,6 +418,7 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { return nil, errors.Wrapf(err, "unable to configure the Docker daemon with file %s", opts.configFile) } } + // the merged configuration can be nil if the config file didn't exist. // leave the current configuration as it is if when that happens. if c != nil { @@ -434,7 +444,12 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) { // 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 conf.IsValueSet(FlagTLSVerify) { - conf.TLS = true + v := true + conf.TLS = &v + } + + if conf.TLSVerify == nil && conf.TLS != nil { + conf.TLSVerify = conf.TLS } return conf, nil @@ -548,7 +563,7 @@ func newAPIServerConfig(cli *DaemonCli) (*apiserver.Config, error) { CorsHeaders: cli.Config.CorsHeaders, } - if cli.Config.TLS { + if cli.Config.TLS != nil && *cli.Config.TLS { tlsOptions := tlsconfig.Options{ CAFile: cli.Config.CommonTLSOptions.CAFile, CertFile: cli.Config.CommonTLSOptions.CertFile, @@ -556,7 +571,7 @@ func newAPIServerConfig(cli *DaemonCli) (*apiserver.Config, error) { ExclusiveRootPools: true, } - if cli.Config.TLSVerify { + if cli.Config.TLSVerify == nil || *cli.Config.TLSVerify { // server requires and verifies client's certificate tlsOptions.ClientAuth = tls.RequireAndVerifyClientCert } @@ -574,13 +589,43 @@ func newAPIServerConfig(cli *DaemonCli) (*apiserver.Config, error) { return serverConfig, nil } +// checkTLSAuthOK checks basically for an explicitly disabled TLS/TLSVerify +// Going forward we do not want to support a scenario where dockerd listens +// on TCP without either TLS client auth (or an explicit opt-in to disable it) +func checkTLSAuthOK(c *config.Config) bool { + if c.TLS == nil { + // Either TLS is enabled by default, in which case TLS verification should be enabled by default, or explicitly disabled + // Or TLS is disabled by default... in any of these cases, we can just take the default value as to how to proceed + return DefaultTLSValue + } + + if !*c.TLS { + // TLS is explicitly disabled, which is supported + return true + } + + if c.TLSVerify == nil { + // this actually shouldn't happen since we set TLSVerify on the config object anyway + // But in case it does get here, be cautious and assume this is not supported. + return false + } + + // Either TLSVerify is explicitly enabled or disabled, both cases are supported + return true +} + func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, error) { var hosts []string seen := make(map[string]struct{}, len(cli.Config.Hosts)) + useTLS := DefaultTLSValue + if cli.Config.TLS != nil { + useTLS = *cli.Config.TLS + } + for i := 0; i < len(cli.Config.Hosts); i++ { var err error - if cli.Config.Hosts[i], err = dopts.ParseHost(cli.Config.TLS, honorXDG, cli.Config.Hosts[i]); err != nil { + if cli.Config.Hosts[i], err = dopts.ParseHost(useTLS, honorXDG, cli.Config.Hosts[i]); err != nil { return nil, errors.Wrapf(err, "error parsing -H %s", cli.Config.Hosts[i]) } if _, ok := seen[cli.Config.Hosts[i]]; ok { @@ -598,8 +643,43 @@ func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, er addr := protoAddrParts[1] // It's a bad idea to bind to TCP without tlsverify. - if proto == "tcp" && (serverConfig.TLSConfig == nil || serverConfig.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 [!]") + authEnabled := serverConfig.TLSConfig != nil && serverConfig.TLSConfig.ClientAuth == tls.RequireAndVerifyClientCert + if proto == "tcp" && !authEnabled { + logrus.WithField("host", protoAddr).Warn("Binding to IP address without --tlsverify is insecure and gives root access on this machine to everyone who has access to your network.") + logrus.WithField("host", protoAddr).Warn("Binding to an IP address, even on localhost, can also give access to scripts run in a browser. Be safe out there!") + time.Sleep(time.Second) + + // If TLSVerify is explicitly set to false we'll take that as "Please let me shoot myself in the foot" + // We do not want to continue to support a default mode where tls verification is disabled, so we do some extra warnings here and eventually remove support + if !checkTLSAuthOK(cli.Config) { + ipAddr, _, err := net.SplitHostPort(addr) + if err != nil { + return nil, errors.Wrap(err, "error parsing tcp address") + } + + // shortcut all this extra stuff for literal "localhost" + // -H supports specifying hostnames, since we want to bypass this on loopback interfaces we'll look it up here. + if ipAddr != "localhost" { + ip := net.ParseIP(ipAddr) + if ip == nil { + ipA, err := net.ResolveIPAddr("ip", ipAddr) + if err != nil { + logrus.WithError(err).WithField("host", ipAddr).Error("Error looking up specified host address") + } + if ipA != nil { + ip = ipA.IP + } + } + if ip == nil || !ip.IsLoopback() { + logrus.WithField("host", protoAddr).Warn("Binding to an IP address without --tlsverify is deprecated. Startup is intentionally being slowed down to show this message") + logrus.WithField("host", protoAddr).Warn("Please consider generating tls certificates with client validation to prevent exposing unauthenticated root access to your network") + logrus.WithField("host", protoAddr).Warnf("You can override this by explicitly specifying '--%s=false' or '--%s=false'", FlagTLS, FlagTLSVerify) + logrus.WithField("host", protoAddr).Warnf("Support for listening on TCP without authentication or explicit intent to run without authentication will be removed in the next release") + + time.Sleep(15 * time.Second) + } + } + } } ls, err := listeners.Init(proto, addr, serverConfig.SocketGroup, serverConfig.TLSConfig) if err != nil { diff --git a/cmd/dockerd/daemon_test.go b/cmd/dockerd/daemon_test.go index c4dcda87be..f2fff3728f 100644 --- a/cmd/dockerd/daemon_test.go +++ b/cmd/dockerd/daemon_test.go @@ -112,7 +112,7 @@ func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) { loadedConfig, err := loadDaemonCliConfig(opts) assert.NilError(t, err) assert.Assert(t, loadedConfig != nil) - assert.Check(t, is.Equal(loadedConfig.TLS, true)) + assert.Check(t, is.Equal(*loadedConfig.TLS, true)) } func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) { @@ -125,7 +125,7 @@ func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) { loadedConfig, err := loadDaemonCliConfig(opts) assert.NilError(t, err) assert.Assert(t, loadedConfig != nil) - assert.Check(t, loadedConfig.TLS) + assert.Check(t, *loadedConfig.TLS) } func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) { @@ -138,7 +138,7 @@ func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) { loadedConfig, err := loadDaemonCliConfig(opts) assert.NilError(t, err) assert.Assert(t, loadedConfig != nil) - assert.Check(t, !loadedConfig.TLS) + assert.Check(t, loadedConfig.TLS == nil) } func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) { diff --git a/cmd/dockerd/options.go b/cmd/dockerd/options.go index f6ea21a6dc..e8e8fd41e8 100644 --- a/cmd/dockerd/options.go +++ b/cmd/dockerd/options.go @@ -20,6 +20,10 @@ const ( DefaultCertFile = "cert.pem" // FlagTLSVerify is the flag name for the TLS verification option FlagTLSVerify = "tlsverify" + // FlagTLS is the flag name for the TLS option + FlagTLS = "tls" + // DefaultTLSValue is the default value used for setting the tls option for tcp connections + DefaultTLSValue = false ) var ( @@ -56,8 +60,8 @@ func (o *daemonOptions) InstallFlags(flags *pflag.FlagSet) { flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode") flags.StringVarP(&o.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`) - flags.BoolVar(&o.TLS, "tls", false, "Use TLS; implied by --tlsverify") - flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote") + flags.BoolVar(&o.TLS, FlagTLS, DefaultTLSValue, "Use TLS; implied by --tlsverify") + flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify || DefaultTLSValue, "Use TLS and verify the remote") // TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file") @@ -86,6 +90,11 @@ func (o *daemonOptions) SetDefaultOptions(flags *pflag.FlagSet) { o.TLS = true } + if o.TLS && !flags.Changed(FlagTLSVerify) { + // Enable tls verification unless explicitly disabled + o.TLSVerify = true + } + if !o.TLS { o.TLSOptions = nil } else { diff --git a/daemon/config/config.go b/daemon/config/config.go index bcecd36356..4990727597 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -205,8 +205,8 @@ type CommonConfig struct { Debug bool `json:"debug,omitempty"` Hosts []string `json:"hosts,omitempty"` LogLevel string `json:"log-level,omitempty"` - TLS bool `json:"tls,omitempty"` - TLSVerify bool `json:"tlsverify,omitempty"` + TLS *bool `json:"tls,omitempty"` + TLSVerify *bool `json:"tlsverify,omitempty"` // Embedded structs that allow config // deserialization without the full struct. diff --git a/daemon/info.go b/daemon/info.go index b47cb7283d..610d7fe2ec 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -219,11 +219,11 @@ func (daemon *Daemon) fillAPIInfo(v *types.Info) { if proto != "tcp" { continue } - if !cfg.TLS { + if cfg.TLS == nil || !*cfg.TLS { v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: API is accessible on http://%s without encryption.%s", addr, warn)) continue } - if !cfg.TLSVerify { + if cfg.TLSVerify == nil || !*cfg.TLSVerify { v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: API is accessible on https://%s without TLS client verification.%s", addr, warn)) continue } diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index c3c28b4c3c..d5fa41c29e 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -528,21 +528,26 @@ func (s *DockerDaemonSuite) TestDaemonFlagDebugLogLevelFatal(c *testing.T) { } func (s *DockerDaemonSuite) TestDaemonAllocatesListeningPort(c *testing.T) { - listeningPorts := [][]string{ + type listener struct { + daemon string + client string + port string + } + listeningPorts := []listener{ {"0.0.0.0", "0.0.0.0", "5678"}, {"127.0.0.1", "127.0.0.1", "1234"}, {"localhost", "127.0.0.1", "1235"}, } cmdArgs := make([]string, 0, len(listeningPorts)*2) - for _, hostDirective := range listeningPorts { - cmdArgs = append(cmdArgs, "--host", fmt.Sprintf("tcp://%s:%s", hostDirective[0], hostDirective[2])) + for _, l := range listeningPorts { + cmdArgs = append(cmdArgs, "--tls=false", "--host", fmt.Sprintf("tcp://%s:%s", l.daemon, l.port)) } s.d.StartWithBusybox(c, cmdArgs...) - for _, hostDirective := range listeningPorts { - output, err := s.d.Cmd("run", "-p", fmt.Sprintf("%s:%s:80", hostDirective[1], hostDirective[2]), "busybox", "true") + for _, l := range listeningPorts { + output, err := s.d.Cmd("run", "-p", fmt.Sprintf("%s:%s:80", l.client, l.port), "busybox", "true") if err == nil { c.Fatalf("Container should not start, expected port already allocated error: %q", output) } else if !strings.Contains(output, "port is already allocated") {