Forward DNS requests into --internal networks

A recent change to prevent containers only connected to --internal networks from communicating with external DNS servers inadvertently prevented the daemon's internal DNS server from forwarding requests within an internal network to a containerised DNS server. Relax the check, so that only requests that need to be forwarded from the host's network namespace are dropped. External DNS servers remain unreachable from the internal network. Signed-off-by: Rob Murray <rob.murray@docker.com>
ggrighi15 · May 10, 2024 · 77a47db · 77a47db
1 parent 75821a7
commit 77a47db
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 11 deletions.
diff --git a/integration/networking/resolvconf_test.go b/integration/networking/resolvconf_test.go
@@ -2,11 +2,14 @@ package networking
 
 import (
 	"context"
+	"os"
+	"path"
 	"strings"
 	"testing"
 	"time"
 
 	containertypes "github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/integration/internal/network"
 	"github.com/docker/docker/testutil/daemon"
@@ -134,6 +137,59 @@ func TestInternalNetworkDNS(t *testing.T) {
 	assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr))
 }
 
+// Check that '--dns' can be used to name a server inside a '--internal' network.
+// Regression test for https://github.com/moby/moby/issues/47822
+func TestInternalNetworkLocalDNS(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "No internal networks on Windows")
+	skip.If(t, testEnv.IsRootless, "Can't write an accessible dnsd.conf in rootless mode")
+	ctx := setupTest(t)
+
+	d := daemon.New(t)
+	d.StartWithBusybox(ctx, t)
+	defer d.Stop(t)
+
+	c := d.NewClientT(t)
+	defer c.Close()
+
+	intNetName := "intnet"
+	network.CreateNoError(ctx, t, c, intNetName,
+		network.WithDriver("bridge"),
+		network.WithInternal(),
+	)
+	defer network.RemoveNoError(ctx, t, c, intNetName)
+
+	// Write a config file for busybox's dnsd.
+	td := t.TempDir()
+	fname := path.Join(td, "dnsd.conf")
+	err := os.WriteFile(fname, []byte("foo.example 192.0.2.42\n"), 0644)
+	assert.NilError(t, err)
+
+	// Start a DNS server on the internal network.
+	serverId := container.Run(ctx, t, c,
+		container.WithNetworkMode(intNetName),
+		container.WithMount(mount.Mount{
+			Type:   mount.TypeBind,
+			Source: fname,
+			Target: "/etc/dnsd.conf",
+		}),
+		container.WithCmd("dnsd"),
+	)
+	defer c.ContainerRemove(ctx, serverId, containertypes.RemoveOptions{Force: true})
+
+	// Get the DNS server's address.
+	inspect := container.Inspect(ctx, t, c, serverId)
+	serverIP := inspect.NetworkSettings.Networks[intNetName].IPAddress
+
+	// Query the internal network's DNS server (via the daemon's internal DNS server).
+	res := container.RunAttach(ctx, t, c,
+		container.WithNetworkMode(intNetName),
+		container.WithDNS([]string{serverIP}),
+		container.WithCmd("nslookup", "-type=A", "foo.example"),
+	)
+	defer c.ContainerRemove(ctx, res.ContainerID, containertypes.RemoveOptions{Force: true})
+	assert.Check(t, is.Contains(res.Stdout.String(), "192.0.2.42"))
+}
+
 // TestNslookupWindows checks that nslookup gets results from external DNS.
 // Regression test for https://github.com/moby/moby/issues/46792
 func TestNslookupWindows(t *testing.T) {

diff --git a/libnetwork/resolver.go b/libnetwork/resolver.go
@@ -486,17 +486,15 @@ func (r *Resolver) serveDNS(w dns.ResponseWriter, query *dns.Msg) {
 		return
 	}
 
-	if r.proxyDNS.Load() {
-		// If the user sets ndots > 0 explicitly and the query is
-		// in the root domain don't forward it out. We will return
-		// failure and let the client retry with the search domain
-		// attached.
-		if (queryType == dns.TypeA || queryType == dns.TypeAAAA) && r.backend.NdotsSet() &&
-			!strings.Contains(strings.TrimSuffix(queryName, "."), ".") {
-			resp = createRespMsg(query)
-		} else {
-			resp = r.forwardExtDNS(ctx, w.LocalAddr().Network(), w.RemoteAddr(), query)
-		}
+	// If the user sets ndots > 0 explicitly and the query is
+	// in the root domain don't forward it out. We will return
+	// failure and let the client retry with the search domain
+	// attached.
+	if (queryType == dns.TypeA || queryType == dns.TypeAAAA) && r.backend.NdotsSet() &&
+		!strings.Contains(strings.TrimSuffix(queryName, "."), ".") {
+		resp = createRespMsg(query)
+	} else {
+		resp = r.forwardExtDNS(ctx, w.LocalAddr().Network(), w.RemoteAddr(), query)
 	}
 
 	if resp == nil {
@@ -541,10 +539,18 @@ func (r *Resolver) forwardExtDNS(ctx context.Context, proto string, remoteAddr n
 	ctx, span := otel.Tracer("").Start(ctx, "resolver.forwardExtDNS")
 	defer span.End()
 
+	proxyDNS := r.proxyDNS.Load()
 	for _, extDNS := range r.extDNS(netiputil.AddrPortFromNet(remoteAddr)) {
 		if extDNS.IPStr == "" {
 			break
 		}
+		// If proxyDNS is false, do not forward the request from the host's namespace
+		// (don't access an external DNS server from an internal network). But, it is
+		// safe to make the request from the container's network namespace - it'll fail
+		// if the DNS server is not accessible, but the server may be on-net.
+		if !proxyDNS && extDNS.HostLoopback {
+			continue
+		}
 
 		// limits the number of outstanding concurrent queries.
 		ctx, cancel := context.WithTimeout(ctx, extIOTimeout)