Browse code

Sterner warnings for unathenticated tcp

People keep doing this and getting pwned because they accidentally left
it exposed to the internet.

The warning about doing this has been there forever.
This introduces a sleep after warning.
To disable the extra sleep users must explicitly specify `--tls=false`
or `--tlsverify=false`

Warning also specifies this sleep will be removed in the next release
where the flag will be required if running unauthenticated.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2020/07/29 08:01:08
Showing 6 changed files
... ...
@@ -4,6 +4,7 @@ import (
4 4
 	"context"
5 5
 	"crypto/tls"
6 6
 	"fmt"
7
+	"net"
7 8
 	"os"
8 9
 	"path/filepath"
9 10
 	"runtime"
... ...
@@ -380,8 +381,16 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
380 380
 	conf.Debug = opts.Debug
381 381
 	conf.Hosts = opts.Hosts
382 382
 	conf.LogLevel = opts.LogLevel
383
-	conf.TLS = opts.TLS
384
-	conf.TLSVerify = opts.TLSVerify
383
+
384
+	if opts.flags.Changed(FlagTLS) {
385
+		conf.TLS = &opts.TLS
386
+	}
387
+	if opts.flags.Changed(FlagTLSVerify) {
388
+		conf.TLSVerify = &opts.TLSVerify
389
+		v := true
390
+		conf.TLS = &v
391
+	}
392
+
385 393
 	conf.CommonTLSOptions = config.CommonTLSOptions{}
386 394
 
387 395
 	if opts.TLSOptions != nil {
... ...
@@ -409,6 +418,7 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
409 409
 				return nil, errors.Wrapf(err, "unable to configure the Docker daemon with file %s", opts.configFile)
410 410
 			}
411 411
 		}
412
+
412 413
 		// the merged configuration can be nil if the config file didn't exist.
413 414
 		// leave the current configuration as it is if when that happens.
414 415
 		if c != nil {
... ...
@@ -434,7 +444,12 @@ func loadDaemonCliConfig(opts *daemonOptions) (*config.Config, error) {
434 434
 	// Regardless of whether the user sets it to true or false, if they
435 435
 	// specify TLSVerify at all then we need to turn on TLS
436 436
 	if conf.IsValueSet(FlagTLSVerify) {
437
-		conf.TLS = true
437
+		v := true
438
+		conf.TLS = &v
439
+	}
440
+
441
+	if conf.TLSVerify == nil && conf.TLS != nil {
442
+		conf.TLSVerify = conf.TLS
438 443
 	}
439 444
 
440 445
 	return conf, nil
... ...
@@ -548,7 +563,7 @@ func newAPIServerConfig(cli *DaemonCli) (*apiserver.Config, error) {
548 548
 		CorsHeaders: cli.Config.CorsHeaders,
549 549
 	}
550 550
 
551
-	if cli.Config.TLS {
551
+	if cli.Config.TLS != nil && *cli.Config.TLS {
552 552
 		tlsOptions := tlsconfig.Options{
553 553
 			CAFile:             cli.Config.CommonTLSOptions.CAFile,
554 554
 			CertFile:           cli.Config.CommonTLSOptions.CertFile,
... ...
@@ -556,7 +571,7 @@ func newAPIServerConfig(cli *DaemonCli) (*apiserver.Config, error) {
556 556
 			ExclusiveRootPools: true,
557 557
 		}
558 558
 
559
-		if cli.Config.TLSVerify {
559
+		if cli.Config.TLSVerify == nil || *cli.Config.TLSVerify {
560 560
 			// server requires and verifies client's certificate
561 561
 			tlsOptions.ClientAuth = tls.RequireAndVerifyClientCert
562 562
 		}
... ...
@@ -574,13 +589,43 @@ func newAPIServerConfig(cli *DaemonCli) (*apiserver.Config, error) {
574 574
 	return serverConfig, nil
575 575
 }
576 576
 
577
+// checkTLSAuthOK checks basically for an explicitly disabled TLS/TLSVerify
578
+// Going forward we do not want to support a scenario where dockerd listens
579
+//   on TCP without either TLS client auth (or an explicit opt-in to disable it)
580
+func checkTLSAuthOK(c *config.Config) bool {
581
+	if c.TLS == nil {
582
+		// Either TLS is enabled by default, in which case TLS verification should be enabled by default, or explicitly disabled
583
+		// Or TLS is disabled by default... in any of these cases, we can just take the default value as to how to proceed
584
+		return DefaultTLSValue
585
+	}
586
+
587
+	if !*c.TLS {
588
+		// TLS is explicitly disabled, which is supported
589
+		return true
590
+	}
591
+
592
+	if c.TLSVerify == nil {
593
+		// this actually shouldn't happen since we set TLSVerify on the config object anyway
594
+		// But in case it does get here, be cautious and assume this is not supported.
595
+		return false
596
+	}
597
+
598
+	// Either TLSVerify is explicitly enabled or disabled, both cases are supported
599
+	return true
600
+}
601
+
577 602
 func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, error) {
578 603
 	var hosts []string
579 604
 	seen := make(map[string]struct{}, len(cli.Config.Hosts))
580 605
 
606
+	useTLS := DefaultTLSValue
607
+	if cli.Config.TLS != nil {
608
+		useTLS = *cli.Config.TLS
609
+	}
610
+
581 611
 	for i := 0; i < len(cli.Config.Hosts); i++ {
582 612
 		var err error
583
-		if cli.Config.Hosts[i], err = dopts.ParseHost(cli.Config.TLS, honorXDG, cli.Config.Hosts[i]); err != nil {
613
+		if cli.Config.Hosts[i], err = dopts.ParseHost(useTLS, honorXDG, cli.Config.Hosts[i]); err != nil {
584 614
 			return nil, errors.Wrapf(err, "error parsing -H %s", cli.Config.Hosts[i])
585 615
 		}
586 616
 		if _, ok := seen[cli.Config.Hosts[i]]; ok {
... ...
@@ -598,8 +643,43 @@ func loadListeners(cli *DaemonCli, serverConfig *apiserver.Config) ([]string, er
598 598
 		addr := protoAddrParts[1]
599 599
 
600 600
 		// It's a bad idea to bind to TCP without tlsverify.
601
-		if proto == "tcp" && (serverConfig.TLSConfig == nil || serverConfig.TLSConfig.ClientAuth != tls.RequireAndVerifyClientCert) {
602
-			logrus.Warn("[!] DON'T BIND ON ANY IP ADDRESS WITHOUT setting --tlsverify IF YOU DON'T KNOW WHAT YOU'RE DOING [!]")
601
+		authEnabled := serverConfig.TLSConfig != nil && serverConfig.TLSConfig.ClientAuth == tls.RequireAndVerifyClientCert
602
+		if proto == "tcp" && !authEnabled {
603
+			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.")
604
+			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!")
605
+			time.Sleep(time.Second)
606
+
607
+			// If TLSVerify is explicitly set to false we'll take that as "Please let me shoot myself in the foot"
608
+			// 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
609
+			if !checkTLSAuthOK(cli.Config) {
610
+				ipAddr, _, err := net.SplitHostPort(addr)
611
+				if err != nil {
612
+					return nil, errors.Wrap(err, "error parsing tcp address")
613
+				}
614
+
615
+				// shortcut all this extra stuff for literal "localhost"
616
+				// -H supports specifying hostnames, since we want to bypass this on loopback interfaces we'll look it up here.
617
+				if ipAddr != "localhost" {
618
+					ip := net.ParseIP(ipAddr)
619
+					if ip == nil {
620
+						ipA, err := net.ResolveIPAddr("ip", ipAddr)
621
+						if err != nil {
622
+							logrus.WithError(err).WithField("host", ipAddr).Error("Error looking up specified host address")
623
+						}
624
+						if ipA != nil {
625
+							ip = ipA.IP
626
+						}
627
+					}
628
+					if ip == nil || !ip.IsLoopback() {
629
+						logrus.WithField("host", protoAddr).Warn("Binding to an IP address without --tlsverify is deprecated. Startup is intentionally being slowed down to show this message")
630
+						logrus.WithField("host", protoAddr).Warn("Please consider generating tls certificates with client validation to prevent exposing unauthenticated root access to your network")
631
+						logrus.WithField("host", protoAddr).Warnf("You can override this by explicitly specifying '--%s=false' or '--%s=false'", FlagTLS, FlagTLSVerify)
632
+						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")
633
+
634
+						time.Sleep(15 * time.Second)
635
+					}
636
+				}
637
+			}
603 638
 		}
604 639
 		ls, err := listeners.Init(proto, addr, serverConfig.SocketGroup, serverConfig.TLSConfig)
605 640
 		if err != nil {
... ...
@@ -112,7 +112,7 @@ func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) {
112 112
 	loadedConfig, err := loadDaemonCliConfig(opts)
113 113
 	assert.NilError(t, err)
114 114
 	assert.Assert(t, loadedConfig != nil)
115
-	assert.Check(t, is.Equal(loadedConfig.TLS, true))
115
+	assert.Check(t, is.Equal(*loadedConfig.TLS, true))
116 116
 }
117 117
 
118 118
 func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) {
... ...
@@ -125,7 +125,7 @@ func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) {
125 125
 	loadedConfig, err := loadDaemonCliConfig(opts)
126 126
 	assert.NilError(t, err)
127 127
 	assert.Assert(t, loadedConfig != nil)
128
-	assert.Check(t, loadedConfig.TLS)
128
+	assert.Check(t, *loadedConfig.TLS)
129 129
 }
130 130
 
131 131
 func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) {
... ...
@@ -138,7 +138,7 @@ func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) {
138 138
 	loadedConfig, err := loadDaemonCliConfig(opts)
139 139
 	assert.NilError(t, err)
140 140
 	assert.Assert(t, loadedConfig != nil)
141
-	assert.Check(t, !loadedConfig.TLS)
141
+	assert.Check(t, loadedConfig.TLS == nil)
142 142
 }
143 143
 
144 144
 func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) {
... ...
@@ -20,6 +20,10 @@ const (
20 20
 	DefaultCertFile = "cert.pem"
21 21
 	// FlagTLSVerify is the flag name for the TLS verification option
22 22
 	FlagTLSVerify = "tlsverify"
23
+	// FlagTLS is the flag name for the TLS option
24
+	FlagTLS = "tls"
25
+	// DefaultTLSValue is the default value used for setting the tls option for tcp connections
26
+	DefaultTLSValue = false
23 27
 )
24 28
 
25 29
 var (
... ...
@@ -56,8 +60,8 @@ func (o *daemonOptions) InstallFlags(flags *pflag.FlagSet) {
56 56
 
57 57
 	flags.BoolVarP(&o.Debug, "debug", "D", false, "Enable debug mode")
58 58
 	flags.StringVarP(&o.LogLevel, "log-level", "l", "info", `Set the logging level ("debug"|"info"|"warn"|"error"|"fatal")`)
59
-	flags.BoolVar(&o.TLS, "tls", false, "Use TLS; implied by --tlsverify")
60
-	flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify, "Use TLS and verify the remote")
59
+	flags.BoolVar(&o.TLS, FlagTLS, DefaultTLSValue, "Use TLS; implied by --tlsverify")
60
+	flags.BoolVar(&o.TLSVerify, FlagTLSVerify, dockerTLSVerify || DefaultTLSValue, "Use TLS and verify the remote")
61 61
 
62 62
 	// TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file")
63 63
 
... ...
@@ -86,6 +90,11 @@ func (o *daemonOptions) SetDefaultOptions(flags *pflag.FlagSet) {
86 86
 		o.TLS = true
87 87
 	}
88 88
 
89
+	if o.TLS && !flags.Changed(FlagTLSVerify) {
90
+		// Enable tls verification unless explicitly disabled
91
+		o.TLSVerify = true
92
+	}
93
+
89 94
 	if !o.TLS {
90 95
 		o.TLSOptions = nil
91 96
 	} else {
... ...
@@ -205,8 +205,8 @@ type CommonConfig struct {
205 205
 	Debug     bool     `json:"debug,omitempty"`
206 206
 	Hosts     []string `json:"hosts,omitempty"`
207 207
 	LogLevel  string   `json:"log-level,omitempty"`
208
-	TLS       bool     `json:"tls,omitempty"`
209
-	TLSVerify bool     `json:"tlsverify,omitempty"`
208
+	TLS       *bool    `json:"tls,omitempty"`
209
+	TLSVerify *bool    `json:"tlsverify,omitempty"`
210 210
 
211 211
 	// Embedded structs that allow config
212 212
 	// deserialization without the full struct.
... ...
@@ -219,11 +219,11 @@ func (daemon *Daemon) fillAPIInfo(v *types.Info) {
219 219
 		if proto != "tcp" {
220 220
 			continue
221 221
 		}
222
-		if !cfg.TLS {
222
+		if cfg.TLS == nil || !*cfg.TLS {
223 223
 			v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: API is accessible on http://%s without encryption.%s", addr, warn))
224 224
 			continue
225 225
 		}
226
-		if !cfg.TLSVerify {
226
+		if cfg.TLSVerify == nil || !*cfg.TLSVerify {
227 227
 			v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: API is accessible on https://%s without TLS client verification.%s", addr, warn))
228 228
 			continue
229 229
 		}
... ...
@@ -528,21 +528,26 @@ func (s *DockerDaemonSuite) TestDaemonFlagDebugLogLevelFatal(c *testing.T) {
528 528
 }
529 529
 
530 530
 func (s *DockerDaemonSuite) TestDaemonAllocatesListeningPort(c *testing.T) {
531
-	listeningPorts := [][]string{
531
+	type listener struct {
532
+		daemon string
533
+		client string
534
+		port   string
535
+	}
536
+	listeningPorts := []listener{
532 537
 		{"0.0.0.0", "0.0.0.0", "5678"},
533 538
 		{"127.0.0.1", "127.0.0.1", "1234"},
534 539
 		{"localhost", "127.0.0.1", "1235"},
535 540
 	}
536 541
 
537 542
 	cmdArgs := make([]string, 0, len(listeningPorts)*2)
538
-	for _, hostDirective := range listeningPorts {
539
-		cmdArgs = append(cmdArgs, "--host", fmt.Sprintf("tcp://%s:%s", hostDirective[0], hostDirective[2]))
543
+	for _, l := range listeningPorts {
544
+		cmdArgs = append(cmdArgs, "--tls=false", "--host", fmt.Sprintf("tcp://%s:%s", l.daemon, l.port))
540 545
 	}
541 546
 
542 547
 	s.d.StartWithBusybox(c, cmdArgs...)
543 548
 
544
-	for _, hostDirective := range listeningPorts {
545
-		output, err := s.d.Cmd("run", "-p", fmt.Sprintf("%s:%s:80", hostDirective[1], hostDirective[2]), "busybox", "true")
549
+	for _, l := range listeningPorts {
550
+		output, err := s.d.Cmd("run", "-p", fmt.Sprintf("%s:%s:80", l.client, l.port), "busybox", "true")
546 551
 		if err == nil {
547 552
 			c.Fatalf("Container should not start, expected port already allocated error: %q", output)
548 553
 		} else if !strings.Contains(output, "port is already allocated") {