Skip to content

Commit

Permalink
Revert "Internal resolver for default bridge network"
Browse files Browse the repository at this point in the history
This reverts commit 18f4f77.

Because buildkit doesn't run an internal resolver, and it bases its
/etc/resolv.conf on the host's ... when buildkit is run in a container
that has 'nameserver 127.0.0.11', its build containers will use Google's
DNS servers as a fallback (unless the build container uses host
networking).

Before, when the 127.0.0.11 resolver was not used for the default network,
the buildkit container would have inherited a site-local nameserver. So,
the build containers it created would also have inherited that DNS
server - and they'd be able to resolve site-local hostnames.

By replacing the site-local nameserver with Google's, we broke access
to local DNS and its hostnames.

Signed-off-by: Rob Murray <[email protected]>
  • Loading branch information
robmry committed Jun 17, 2024
1 parent 6d35673 commit 74d77d8
Show file tree
Hide file tree
Showing 13 changed files with 227 additions and 96 deletions.
10 changes: 9 additions & 1 deletion daemon/config/config_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ type Config struct {
NoNewPrivileges bool `json:"no-new-privileges,omitempty"`
IpcMode string `json:"default-ipc-mode,omitempty"`
CgroupNamespaceMode string `json:"default-cgroupns-mode,omitempty"`
Rootless bool `json:"rootless,omitempty"`
// ResolvConf is the path to the configuration of the host resolver
ResolvConf string `json:"resolv-conf,omitempty"`
Rootless bool `json:"rootless,omitempty"`
}

// GetExecRoot returns the user configured Exec-root
Expand Down Expand Up @@ -134,6 +136,12 @@ func (conf *Config) LookupInitPath() (string, error) {
return exec.LookPath(binary)
}

// GetResolvConf returns the appropriate resolv.conf
// Check setupResolvConf on how this is selected
func (conf *Config) GetResolvConf() string {
return conf.ResolvConf
}

// IsSwarmCompatible defines if swarm mode can be enabled in this config
func (conf *Config) IsSwarmCompatible() error {
if conf.LiveRestoreEnabled {
Expand Down
45 changes: 37 additions & 8 deletions daemon/container_operations_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,21 +419,50 @@ func serviceDiscoveryOnDefaultNetwork() bool {

func buildSandboxPlatformOptions(ctr *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
var err error
var originResolvConfPath string

// In host-mode networking, the container does not have its own networking
// namespace, so `/etc/hosts` should be the same as on the host itself. Setting
// OptionOriginHostsPath means the container will get a copy of '/etc/hosts' from
// the host filesystem.
// Note that containers with "container" networking have been handled in
// "initializeNetworking()", so do not have to be accounted for here.
if ctr.HostConfig.NetworkMode.IsHost() {
// Set the correct paths for /etc/hosts and /etc/resolv.conf, based on the
// networking-mode of the container. Note that containers with "container"
// networking are already handled in "initializeNetworking()" before we reach
// this function, so do not have to be accounted for here.
switch {
case ctr.HostConfig.NetworkMode.IsHost():
// In host-mode networking, the container does not have its own networking
// namespace, so both `/etc/hosts` and `/etc/resolv.conf` should be the same
// as on the host itself. The container gets a copy of these files.
*sboxOptions = append(
*sboxOptions,
libnetwork.OptionOriginHostsPath("/etc/hosts"),
)
originResolvConfPath = "/etc/resolv.conf"
case ctr.HostConfig.NetworkMode.IsUserDefined():
// The container uses a user-defined network. We use the embedded DNS
// server for container name resolution and to act as a DNS forwarder
// for external DNS resolution.
// We parse the DNS server(s) that are defined in /etc/resolv.conf on
// the host, which may be a local DNS server (for example, if DNSMasq or
// systemd-resolvd are in use). The embedded DNS server forwards DNS
// resolution to the DNS server configured on the host, which in itself
// may act as a forwarder for external DNS servers.
// If systemd-resolvd is used, the "upstream" DNS servers can be found in
// /run/systemd/resolve/resolv.conf. We do not query those DNS servers
// directly, as they can be dynamically reconfigured.
originResolvConfPath = "/etc/resolv.conf"
default:
// For other situations, such as the default bridge network, container
// discovery / name resolution is handled through /etc/hosts, and no
// embedded DNS server is available. Without the embedded DNS, we
// cannot use local DNS servers on the host (for example, if DNSMasq or
// systemd-resolvd is used). If systemd-resolvd is used, we try to
// determine the external DNS servers that are used on the host.
// This situation is not ideal, because DNS servers configured in the
// container are not updated after the container is created, but the
// DNS servers on the host can be dynamically updated.
//
// Copy the host's resolv.conf for the container (/run/systemd/resolve/resolv.conf or /etc/resolv.conf)
originResolvConfPath = cfg.GetResolvConf()
}

originResolvConfPath := "/etc/resolv.conf"
// Allow tests to point at their own resolv.conf file.
if envPath := os.Getenv("DOCKER_TEST_RESOLV_CONF_PATH"); envPath != "" {
log.G(context.TODO()).Infof("Using OriginResolvConfPath from env: %s", envPath)
Expand Down
3 changes: 3 additions & 0 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,9 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S
// Do we have a disabled network?
config.DisableBridge = isBridgeNetworkDisabled(config)

// Setup the resolv.conf
setupResolvConf(config)

idMapping, err := setupRemappedRoot(config)
if err != nil {
return nil, err
Expand Down
13 changes: 13 additions & 0 deletions daemon/daemon_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/containerd/log"
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/libnetwork/ns"
"github.com/docker/docker/libnetwork/resolvconf"
"github.com/moby/sys/mount"
"github.com/moby/sys/mountinfo"
"github.com/pkg/errors"
Expand Down Expand Up @@ -135,6 +136,18 @@ func shouldUnmountRoot(root string, info *mountinfo.Info) bool {
return hasMountInfoOption(info.Optional, sharedPropagationOption)
}

// setupResolvConf sets the appropriate resolv.conf file if not specified
// When systemd-resolved is running the default /etc/resolv.conf points to
// localhost. In this case fetch the alternative config file that is in a
// different path so that containers can use it
// In all the other cases fallback to the default one
func setupResolvConf(config *config.Config) {
if config.ResolvConf != "" {
return
}
config.ResolvConf = resolvconf.Path()
}

// ifaceAddrs returns the IPv4 and IPv6 addresses assigned to the network
// interface with name linkName.
//
Expand Down
2 changes: 2 additions & 0 deletions daemon/daemon_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ func checkSystem() error {
return errors.New("the Docker daemon is not supported on this platform")
}

func setupResolvConf(_ *interface{}) {}

func getSysInfo(_ *Daemon) *sysinfo.SysInfo {
return sysinfo.New()
}
2 changes: 2 additions & 0 deletions daemon/daemon_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,8 @@ func (daemon *Daemon) setupSeccompProfile(*config.Config) error {
return nil
}

func setupResolvConf(config *config.Config) {}

func getSysInfo(*config.Config) *sysinfo.SysInfo {
return sysinfo.New()
}
Expand Down
3 changes: 1 addition & 2 deletions daemon/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,7 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
createOptions = append(createOptions, libnetwork.CreateOptionService(svcCfg.Name, svcCfg.ID, vip, portConfigs, svcCfg.Aliases[nwID]))
}

// Don't run an internal DNS resolver for host/container/none networks.
if nm := containertypes.NetworkMode(nwName); nm.IsHost() || nm.IsContainer() || nm.IsNone() {
if !containertypes.NetworkMode(nwName).IsUserDefined() {
createOptions = append(createOptions, libnetwork.CreateOptionDisableResolution())
}

Expand Down
138 changes: 123 additions & 15 deletions integration-cli/docker_cli_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/docker/docker/integration-cli/cli/build"
"github.com/docker/docker/integration-cli/daemon"
"github.com/docker/docker/internal/testutils/specialimage"
"github.com/docker/docker/libnetwork/resolvconf"
"github.com/docker/docker/pkg/stringid"
"github.com/docker/docker/runconfig"
"github.com/docker/docker/testutil"
Expand Down Expand Up @@ -1248,8 +1249,45 @@ func (s *DockerCLIRunSuite) TestRunDisallowBindMountingRootToRoot(c *testing.T)
}
}

// Verify that a container gets default DNS when only localhost resolvers exist
func (s *DockerCLIRunSuite) TestRunDNSDefaultOptions(c *testing.T) {
// Not applicable on Windows as this is testing Unix specific functionality
testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)

// preserve original resolv.conf for restoring after test
origResolvConf, err := os.ReadFile("/etc/resolv.conf")
if os.IsNotExist(err) {
c.Fatalf("/etc/resolv.conf does not exist")
}
// defer restored original conf
defer func() {
if err := os.WriteFile("/etc/resolv.conf", origResolvConf, 0o644); err != nil {
c.Fatal(err)
}
}()

// test 3 cases: standard IPv4 localhost, commented out localhost, and IPv6 localhost
// 2 are removed from the file at container start, and the 3rd (commented out) one is ignored by
// GetNameservers(), leading to a replacement of nameservers with the default set
tmpResolvConf := []byte("nameserver 127.0.0.1\n#nameserver 127.0.2.1\nnameserver ::1")
if err := os.WriteFile("/etc/resolv.conf", tmpResolvConf, 0o644); err != nil {
c.Fatal(err)
}

actual := cli.DockerCmd(c, "run", "busybox", "cat", "/etc/resolv.conf").Combined()
actual = regexp.MustCompile("(?m)^#.*$").ReplaceAllString(actual, "")
actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ")
// NOTE: if we ever change the defaults from google dns, this will break
expected := "nameserver 8.8.8.8 nameserver 8.8.4.4"

if actual != expected {
c.Fatalf("expected resolv.conf be: %q, but was: %q", expected, actual)
}
}

func (s *DockerCLIRunSuite) TestRunDNSOptions(c *testing.T) {
// Not applicable on Windows as Windows does not support the Unix-specific functionality of resolv.conf.
// Not applicable on Windows as Windows does not support --dns*, or
// the Unix-specific functionality of resolv.conf.
testRequires(c, DaemonIsLinux)
result := cli.DockerCmd(c, "run", "--dns=127.0.0.1", "--dns-search=mydomain", "--dns-opt=ndots:9", "busybox", "cat", "/etc/resolv.conf")

Expand All @@ -1260,22 +1298,16 @@ func (s *DockerCLIRunSuite) TestRunDNSOptions(c *testing.T) {

actual := regexp.MustCompile("(?m)^#.*$").ReplaceAllString(result.Stdout(), "")
actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ")
if actual != "nameserver 127.0.0.11 search mydomain options ndots:9" {
c.Fatalf("nameserver 127.0.0.11 expected 'search mydomain options ndots:9', but says: %q", actual)
}
if !strings.Contains(result.Stdout(), "ExtServers: [127.0.0.1]") {
c.Fatalf("expected 'ExtServers: [127.0.0.1]' was not found in %q", result.Stdout())
if actual != "nameserver 127.0.0.1 search mydomain options ndots:9" {
c.Fatalf("nameserver 127.0.0.1 expected 'search mydomain options ndots:9', but says: %q", actual)
}

out := cli.DockerCmd(c, "run", "--dns=1.1.1.1", "--dns-search=.", "--dns-opt=ndots:3", "busybox", "cat", "/etc/resolv.conf").Combined()

actual = regexp.MustCompile("(?m)^#.*$").ReplaceAllString(out, "")
actual = strings.ReplaceAll(strings.Trim(strings.Trim(actual, "\r\n"), " "), "\n", " ")
if actual != "nameserver 127.0.0.11 options ndots:3" {
c.Fatalf("expected 'nameserver 127.0.0.11 options ndots:3', but says: %q", actual)
}
if !strings.Contains(out, "ExtServers: [1.1.1.1]") {
c.Fatalf("expected 'ExtServers: [1.1.1.1]' was not found in %q", out)
if actual != "nameserver 1.1.1.1 options ndots:3" {
c.Fatalf("expected 'nameserver 1.1.1.1 options ndots:3', but says: %q", actual)
}
}

Expand All @@ -1285,11 +1317,87 @@ func (s *DockerCLIRunSuite) TestRunDNSRepeatOptions(c *testing.T) {

actual := regexp.MustCompile("(?m)^#.*$").ReplaceAllString(out, "")
actual = strings.ReplaceAll(strings.Trim(actual, "\r\n"), "\n", " ")
if actual != "nameserver 127.0.0.11 search mydomain mydomain2 options ndots:9 timeout:3" {
c.Fatalf("expected 'nameserver 127.0.0.11 search mydomain mydomain2 options ndots:9 timeout:3', but says: %q", actual)
if actual != "nameserver 1.1.1.1 nameserver 2.2.2.2 search mydomain mydomain2 options ndots:9 timeout:3" {
c.Fatalf("expected 'nameserver 1.1.1.1 nameserver 2.2.2.2 search mydomain mydomain2 options ndots:9 timeout:3', but says: %q", actual)
}
}

func (s *DockerCLIRunSuite) TestRunDNSOptionsBasedOnHostResolvConf(c *testing.T) {
// Not applicable on Windows as testing Unix specific functionality
testRequires(c, testEnv.IsLocalDaemon, DaemonIsLinux)

origResolvConf, err := os.ReadFile("/etc/resolv.conf")
if os.IsNotExist(err) {
c.Fatalf("/etc/resolv.conf does not exist")
}

hostNameservers := resolvconf.GetNameservers(origResolvConf, resolvconf.IP)
hostSearch := resolvconf.GetSearchDomains(origResolvConf)

out := cli.DockerCmd(c, "run", "--dns=127.0.0.1", "busybox", "cat", "/etc/resolv.conf").Combined()

if actualNameservers := resolvconf.GetNameservers([]byte(out), resolvconf.IP); actualNameservers[0] != "127.0.0.1" {
c.Fatalf("expected '127.0.0.1', but says: %q", actualNameservers[0])
}
if !strings.Contains(out, "ExtServers: [1.1.1.1 2.2.2.2]") {
c.Fatalf("expected 'ExtServers: [1.1.1.1 2.2.2.2]' was not found in %q", out)

actualSearch := resolvconf.GetSearchDomains([]byte(out))
if len(actualSearch) != len(hostSearch) {
c.Fatalf("expected %q search domain(s), but it has: %q", len(hostSearch), len(actualSearch))
}
for i := range actualSearch {
if actualSearch[i] != hostSearch[i] {
c.Fatalf("expected %q domain, but says: %q", actualSearch[i], hostSearch[i])
}
}

out = cli.DockerCmd(c, "run", "--dns-search=mydomain", "busybox", "cat", "/etc/resolv.conf").Combined()

actualNameservers := resolvconf.GetNameservers([]byte(out), resolvconf.IP)
if len(actualNameservers) != len(hostNameservers) {
c.Fatalf("expected %q nameserver(s), but it has: %q", len(hostNameservers), len(actualNameservers))
}
for i := range actualNameservers {
if actualNameservers[i] != hostNameservers[i] {
c.Fatalf("expected %q nameserver, but says: %q", actualNameservers[i], hostNameservers[i])
}
}

if actualSearch = resolvconf.GetSearchDomains([]byte(out)); actualSearch[0] != "mydomain" {
c.Fatalf("expected 'mydomain', but says: %q", actualSearch[0])
}

// test with file
tmpResolvConf := []byte("search example.com\nnameserver 12.34.56.78\nnameserver 127.0.0.1")
if err := os.WriteFile("/etc/resolv.conf", tmpResolvConf, 0o644); err != nil {
c.Fatal(err)
}
// put the old resolvconf back
defer func() {
if err := os.WriteFile("/etc/resolv.conf", origResolvConf, 0o644); err != nil {
c.Fatal(err)
}
}()

resolvConf, err := os.ReadFile("/etc/resolv.conf")
if os.IsNotExist(err) {
c.Fatalf("/etc/resolv.conf does not exist")
}

hostSearch = resolvconf.GetSearchDomains(resolvConf)

out = cli.DockerCmd(c, "run", "busybox", "cat", "/etc/resolv.conf").Combined()
if actualNameservers = resolvconf.GetNameservers([]byte(out), resolvconf.IP); actualNameservers[0] != "12.34.56.78" || len(actualNameservers) != 1 {
c.Fatalf("expected '12.34.56.78', but has: %v", actualNameservers)
}

actualSearch = resolvconf.GetSearchDomains([]byte(out))
if len(actualSearch) != len(hostSearch) {
c.Fatalf("expected %q search domain(s), but it has: %q", len(hostSearch), len(actualSearch))
}
for i := range actualSearch {
if actualSearch[i] != hostSearch[i] {
c.Fatalf("expected %q domain, but says: %q", actualSearch[i], hostSearch[i])
}
}
}

Expand Down
4 changes: 1 addition & 3 deletions integration-cli/docker_cli_swarm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -974,16 +974,14 @@ func (s *DockerSwarmSuite) TestDNSConfig(c *testing.T) {
id := strings.TrimSpace(out)

// Compare against expected output.
expectedOutput1 := "nameserver 127.0.0.11"
expectedOutput1 := "nameserver 1.2.3.4"
expectedOutput2 := "search example.com"
expectedOutput3 := "options timeout:3"
expectedOutput4 := "ExtServers: [1.2.3.4]"
out, err = d.Cmd("exec", id, "cat", "/etc/resolv.conf")
assert.NilError(c, err, out)
assert.Assert(c, strings.Contains(out, expectedOutput1), "Expected '%s', but got %q", expectedOutput1, out)
assert.Assert(c, strings.Contains(out, expectedOutput2), "Expected '%s', but got %q", expectedOutput2, out)
assert.Assert(c, strings.Contains(out, expectedOutput3), "Expected '%s', but got %q", expectedOutput3, out)
assert.Assert(c, strings.Contains(out, expectedOutput4), "Expected '%s', but got %q", expectedOutput4, out)
}

func (s *DockerSwarmSuite) TestDNSConfigUpdate(c *testing.T) {
Expand Down
Loading

0 comments on commit 74d77d8

Please sign in to comment.