From afa92a9af0f1a77ef25aab73b11aa855a1823666 Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Tue, 10 Mar 2015 10:22:29 -0400 Subject: [PATCH] Add warning for --dns flag set to localhost addresses. We should warn users who use the `--dns` command line option to point DNS to a localhost address, either IPv4 or IPv6. Unless they have specifically set up the container as a DNS server or are using --net=host (which is why this should be allowed, but warned on because those are pretty unique cases) a localhost address as a resolver will not reach what they might expect (e.g. expecting it will hit localhost on the Docker daemon/host). Added a test for the message, and fixed up tests to separate stdout and stderr that were using `--dns=127.0.0.1` to test the options. Docker-DCO-1.1-Signed-off-by: Phil Estes (github: estesp) --- api/client/commands.go | 13 +++++++++++++ integration-cli/docker_cli_run_test.go | 11 ++++++++--- pkg/networkfs/resolvconf/resolvconf.go | 19 ++++++++++++++----- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/api/client/commands.go b/api/client/commands.go index d6f19d934c..a6599d1c7f 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -37,6 +37,7 @@ import ( "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/homedir" flag "github.com/docker/docker/pkg/mflag" + "github.com/docker/docker/pkg/networkfs/resolvconf" "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/parsers/filters" "github.com/docker/docker/pkg/promise" @@ -2263,6 +2264,18 @@ func (cli *DockerCli) CmdRun(args ...string) error { if err != nil { utils.ReportError(cmd, err.Error(), true) } + + if len(hostConfig.Dns) > 0 { + // check the DNS settings passed via --dns against + // localhost regexp to warn if they are trying to + // set a DNS to a localhost address + for _, dnsIP := range hostConfig.Dns { + if resolvconf.IsLocalhost(dnsIP) { + fmt.Fprintf(cli.err, "WARNING: Localhost DNS setting (--dns=%s) may fail in containers.\n", dnsIP) + break + } + } + } if config.Image == "" { cmd.Usage() return nil diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 9869abe745..f348e54315 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -1463,11 +1463,16 @@ func TestRunDnsOptions(t *testing.T) { cmd := exec.Command(dockerBinary, "run", "--dns=127.0.0.1", "--dns-search=mydomain", "busybox", "cat", "/etc/resolv.conf") - out, _, err := runCommandWithOutput(cmd) + out, stderr, _, err := runCommandWithStdoutStderr(cmd) if err != nil { t.Fatal(err, out) } + // The client will get a warning on stderr when setting DNS to a localhost address; verify this: + if !strings.Contains(stderr, "Localhost DNS setting") { + t.Fatalf("Expected warning on stderr about localhost resolver, but got %q", stderr) + } + actual := strings.Replace(strings.Trim(out, "\r\n"), "\n", " ", -1) if actual != "nameserver 127.0.0.1 search mydomain" { t.Fatalf("expected 'nameserver 127.0.0.1 search mydomain', but says: %q", actual) @@ -1475,7 +1480,7 @@ func TestRunDnsOptions(t *testing.T) { cmd = exec.Command(dockerBinary, "run", "--dns=127.0.0.1", "--dns-search=.", "busybox", "cat", "/etc/resolv.conf") - out, _, err = runCommandWithOutput(cmd) + out, _, _, err = runCommandWithStdoutStderr(cmd) if err != nil { t.Fatal(err, out) } @@ -1502,7 +1507,7 @@ func TestRunDnsOptionsBasedOnHostResolvConf(t *testing.T) { var out string cmd := exec.Command(dockerBinary, "run", "--dns=127.0.0.1", "busybox", "cat", "/etc/resolv.conf") - if out, _, err = runCommandWithOutput(cmd); err != nil { + if out, _, _, err = runCommandWithStdoutStderr(cmd); err != nil { t.Fatal(err, out) } diff --git a/pkg/networkfs/resolvconf/resolvconf.go b/pkg/networkfs/resolvconf/resolvconf.go index d88074f598..61f92d9ae5 100644 --- a/pkg/networkfs/resolvconf/resolvconf.go +++ b/pkg/networkfs/resolvconf/resolvconf.go @@ -23,11 +23,13 @@ var ( // For readability and sufficiency for Docker purposes this seemed more reasonable than a // 1000+ character regexp with exact and complete IPv6 validation ipv6Address = `([0-9A-Fa-f]{0,4}:){2,7}([0-9A-Fa-f]{0,4})` + ipLocalhost = `((127\.([0-9]{1,3}.){2}[0-9]{1,3})|(::1))` - localhostRegexp = regexp.MustCompile(`(?m)^nameserver\s+((127\.([0-9]{1,3}.){2}[0-9]{1,3})|(::1))\s*\n*`) - nsIPv6Regexp = regexp.MustCompile(`(?m)^nameserver\s+` + ipv6Address + `\s*\n*`) - nsRegexp = regexp.MustCompile(`^\s*nameserver\s*((` + ipv4Address + `)|(` + ipv6Address + `))\s*$`) - searchRegexp = regexp.MustCompile(`^\s*search\s*(([^\s]+\s*)*)$`) + localhostIPRegexp = regexp.MustCompile(ipLocalhost) + localhostNSRegexp = regexp.MustCompile(`(?m)^nameserver\s+` + ipLocalhost + `\s*\n*`) + nsIPv6Regexp = regexp.MustCompile(`(?m)^nameserver\s+` + ipv6Address + `\s*\n*`) + nsRegexp = regexp.MustCompile(`^\s*nameserver\s*((` + ipv4Address + `)|(` + ipv6Address + `))\s*$`) + searchRegexp = regexp.MustCompile(`^\s*search\s*(([^\s]+\s*)*)$`) ) var lastModified struct { @@ -87,7 +89,7 @@ func GetLastModified() ([]byte, string) { // It also returns a boolean to notify the caller if changes were made at all func FilterResolvDns(resolvConf []byte, ipv6Enabled bool) ([]byte, bool) { changed := false - cleanedResolvConf := localhostRegexp.ReplaceAll(resolvConf, []byte{}) + cleanedResolvConf := localhostNSRegexp.ReplaceAll(resolvConf, []byte{}) // if IPv6 is not enabled, also clean out any IPv6 address nameserver if !ipv6Enabled { cleanedResolvConf = nsIPv6Regexp.ReplaceAll(cleanedResolvConf, []byte{}) @@ -124,6 +126,13 @@ func getLines(input []byte, commentMarker []byte) [][]byte { return output } +// returns true if the IP string matches the localhost IP regular expression. +// Used for determining if nameserver settings are being passed which are +// localhost addresses +func IsLocalhost(ip string) bool { + return localhostIPRegexp.MatchString(ip) +} + // GetNameservers returns nameservers (if any) listed in /etc/resolv.conf func GetNameservers(resolvConf []byte) []string { nameservers := []string{}