libnet: Don't forward to upstream resolvers on internal nw

This commit makes sure the embedded resolver doesn't try to forward to
upstream servers when a container is only attached to an internal
network.

Co-authored-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Rob Murray <rob.murray@docker.com>
(cherry picked from commit 790c3039d0)
Signed-off-by: Cory Snider <csnider@mirantis.com>

daemon/container_operations_unix.go

@@ -384,6 +384,7 @@ func serviceDiscoveryOnDefaultNetwork() bool {
 
 func (daemon *Daemon) setupPathsAndSandboxOptions(container *container.Container, sboxOptions *[]libnetwork.SandboxOption) error {
 	var err error
+	var originResolvConfPath string
 
 	// Set the correct paths for /etc/hosts and /etc/resolv.conf, based on the
 	// networking-mode of the container. Note that containers with "container"
@@ -397,8 +398,8 @@ func (daemon *Daemon) setupPathsAndSandboxOptions(container *container.Container
 		*sboxOptions = append(
 			*sboxOptions,
 			libnetwork.OptionOriginHostsPath("/etc/hosts"),
-			libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf"),
 		)
+		originResolvConfPath = "/etc/resolv.conf"
 	case container.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
@@ -411,10 +412,7 @@ func (daemon *Daemon) setupPathsAndSandboxOptions(container *container.Container
 		// 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.
-		*sboxOptions = append(
-			*sboxOptions,
-			libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf"),
-		)
+		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
@@ -427,12 +425,16 @@ func (daemon *Daemon) setupPathsAndSandboxOptions(container *container.Container
 		// 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)
-		*sboxOptions = append(
-			*sboxOptions,
-			libnetwork.OptionOriginResolvConfPath(daemon.configStore.GetResolvConf()),
-		)
+		originResolvConfPath = daemon.configStore.GetResolvConf()
 	}
 
+	// Allow tests to point at their own resolv.conf file.
+	if envPath := os.Getenv("DOCKER_TEST_RESOLV_CONF_PATH"); envPath != "" {
+		logrus.Infof("Using OriginResolvConfPath from env: %s", envPath)
+		originResolvConfPath = envPath
+	}
+	*sboxOptions = append(*sboxOptions, libnetwork.OptionOriginResolvConfPath(originResolvConfPath))
+
 	container.HostsPath, err = container.GetRootResourcePath("hosts")
 	if err != nil {
 		return err

integration/internal/network/network.go

@@ -33,3 +33,10 @@ func CreateNoError(ctx context.Context, t *testing.T, client client.APIClient, n
 	assert.NilError(t, err)
 	return name
 }
+
+func RemoveNoError(ctx context.Context, t *testing.T, apiClient client.APIClient, name string) {
+	t.Helper()
+
+	err := apiClient.NetworkRemove(ctx, name)
+	assert.NilError(t, err)
+}

integration/networking/resolvconf_test.go

@@ -0,0 +1,142 @@
+package networking
+
+import (
+	"net"
+	"os"
+	"testing"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/integration/internal/container"
+	"github.com/docker/docker/integration/internal/network"
+	"github.com/docker/docker/testutil/daemon"
+	"github.com/miekg/dns"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+	"gotest.tools/v3/skip"
+)
+
+// writeTempResolvConf writes a resolv.conf that only contains a single
+// nameserver line, with address addr.
+// It returns the name of the temp file.
+func writeTempResolvConf(t *testing.T, addr string) string {
+	t.Helper()
+	// Not using t.TempDir() here because in rootless mode, while the temporary
+	// directory gets mode 0777, it's a subdir of an 0700 directory owned by root.
+	// So, it's not accessible by the daemon.
+	f, err := os.CreateTemp("", "resolv.conf")
+	assert.NilError(t, err)
+	t.Cleanup(func() { os.Remove(f.Name()) })
+	err = f.Chmod(0644)
+	assert.NilError(t, err)
+	f.Write([]byte("nameserver " + addr + "\n"))
+	return f.Name()
+}
+
+const dnsRespAddr = "10.11.12.13"
+
+// startDaftDNS starts and returns a really, really daft DNS server that only
+// responds to type-A requests, and always with address dnsRespAddr.
+func startDaftDNS(t *testing.T, addr string) *dns.Server {
+	serveDNS := func(w dns.ResponseWriter, query *dns.Msg) {
+		if query.Question[0].Qtype == dns.TypeA {
+			resp := &dns.Msg{}
+			resp.SetReply(query)
+			answer := &dns.A{
+				Hdr: dns.RR_Header{
+					Name:   query.Question[0].Name,
+					Rrtype: dns.TypeA,
+					Class:  dns.ClassINET,
+					Ttl:    600,
+				},
+			}
+			answer.A = net.ParseIP(dnsRespAddr)
+			resp.Answer = append(resp.Answer, answer)
+			_ = w.WriteMsg(resp)
+		}
+	}
+
+	conn, err := net.ListenUDP("udp", &net.UDPAddr{
+		IP:   net.ParseIP(addr),
+		Port: 53,
+	})
+	assert.NilError(t, err)
+
+	server := &dns.Server{Handler: dns.HandlerFunc(serveDNS), PacketConn: conn}
+	go func() {
+		_ = server.ActivateAndServe()
+	}()
+
+	return server
+}
+
+// Check that when a container is connected to an internal network, DNS
+// requests sent to daemon's internal DNS resolver are not forwarded to
+// an upstream resolver listening on a localhost address.
+// (Assumes the host does not already have a DNS server on 127.0.0.1.)
+func TestInternalNetworkDNS(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "No resolv.conf on Windows")
+	skip.If(t, testEnv.IsRootless, "Can't use resolver on host in rootless mode")
+	ctx := setupTest(t)
+
+	// Start a DNS server on the loopback interface.
+	server := startDaftDNS(t, "127.0.0.1")
+	defer server.Shutdown()
+
+	// Set up a temp resolv.conf pointing at that DNS server, and a daemon using it.
+	tmpFileName := writeTempResolvConf(t, "127.0.0.1")
+	d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName))
+	d.StartWithBusybox(t, "--experimental", "--ip6tables")
+	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)
+
+	extNetName := "extnet"
+	network.CreateNoError(ctx, t, c, extNetName,
+		network.WithDriver("bridge"),
+	)
+	defer network.RemoveNoError(ctx, t, c, extNetName)
+
+	// Create a container, initially with external connectivity.
+	// Expect the external DNS server to respond to a request from the container.
+	ctrId := container.Run(ctx, t, c, container.WithNetworkMode(extNetName))
+	defer c.ContainerRemove(ctx, ctrId, types.ContainerRemoveOptions{Force: true})
+	res, err := container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
+	assert.NilError(t, err)
+	assert.Check(t, is.Equal(res.ExitCode, 0))
+	assert.Check(t, is.Contains(res.Stdout(), dnsRespAddr))
+
+	// Connect the container to the internal network as well.
+	// External DNS should still be used.
+	err = c.NetworkConnect(ctx, intNetName, ctrId, nil)
+	assert.NilError(t, err)
+	res, err = container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
+	assert.NilError(t, err)
+	assert.Check(t, is.Equal(res.ExitCode, 0))
+	assert.Check(t, is.Contains(res.Stdout(), dnsRespAddr))
+
+	// Disconnect from the external network.
+	// Expect no access to the external DNS.
+	err = c.NetworkDisconnect(ctx, extNetName, ctrId, true)
+	assert.NilError(t, err)
+	res, err = container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
+	assert.NilError(t, err)
+	assert.Check(t, is.Equal(res.ExitCode, 1))
+	assert.Check(t, is.Contains(res.Stdout(), "SERVFAIL"))
+
+	// Reconnect the external network.
+	// Check that the external DNS server is used again.
+	err = c.NetworkConnect(ctx, extNetName, ctrId, nil)
+	assert.NilError(t, err)
+	res, err = container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
+	assert.NilError(t, err)
+	assert.Check(t, is.Equal(res.ExitCode, 0))
+	assert.Check(t, is.Contains(res.Stdout(), dnsRespAddr))
+}

libnetwork/default_gateway.go

@@ -186,3 +186,27 @@ func (sb *sandbox) getGatewayEndpoint() *endpoint {
 	}
 	return nil
 }
+
+// hasExternalConnectivity returns true if the sandbox is connected to any
+// endpoint which provides external connectivity.
+//
+// This function is only necessary on branches without
+// https://github.com/moby/moby/pull/46603. With that PR applied, this function
+// would be equivalent to sb.getGatewayEndpoint() != nil.
+func (sb *sandbox) hasExternalConnectivity() bool {
+	for _, ep := range sb.getConnectedEndpoints() {
+		n := ep.getNetwork()
+		switch n.Type() {
+		case "null", "host":
+			continue
+		case "bridge":
+			if n.Internal() {
+				continue
+			}
+		}
+		if len(ep.Gateway()) != 0 {
+			return true
+		}
+	}
+	return false
+}

libnetwork/endpoint.go

@@ -544,8 +544,12 @@ func (ep *endpoint) sbJoin(sb *sandbox, options ...EndpointOption) (err error) {
 		return sb.setupDefaultGW()
 	}
 
-	moveExtConn := sb.getGatewayEndpoint() != extEp
+	// Enable upstream forwarding if the sandbox gained external connectivity.
+	if sb.resolver != nil {
+		sb.resolver.SetForwardingPolicy(sb.hasExternalConnectivity())
+	}
 
+	moveExtConn := sb.getGatewayEndpoint() != extEp
 	if moveExtConn {
 		if extEp != nil {
 			logrus.Debugf("Revoking external connectivity on endpoint %s (%s)", extEp.Name(), extEp.ID())
@@ -777,6 +781,11 @@ func (ep *endpoint) sbLeave(sb *sandbox, force bool, options ...EndpointOption)
 		return sb.setupDefaultGW()
 	}
 
+	// Disable upstream forwarding if the sandbox lost external connectivity.
+	if sb.resolver != nil {
+		sb.resolver.SetForwardingPolicy(sb.hasExternalConnectivity())
+	}
+
 	// New endpoint providing external connectivity for the sandbox
 	extEp = sb.getGatewayEndpoint()
 	if moveExtConn && extEp != nil {

libnetwork/resolver.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/docker/docker/libnetwork/types"
@@ -30,6 +31,9 @@ type Resolver interface {
 	// SetExtServers configures the external nameservers the resolver
 	// should use to forward queries
 	SetExtServers([]extDNSEntry)
+	// SetForwardingPolicy re-configures the embedded DNS resolver to either
+	// enable or disable forwarding DNS queries to external servers.
+	SetForwardingPolicy(policy bool)
 	// ResolverOptions returns resolv.conf options that should be set
 	ResolverOptions() []string
 }
@@ -89,21 +93,23 @@ type resolver struct {
 	tStamp        time.Time
 	queryLock     sync.Mutex
 	listenAddress string
-	proxyDNS      bool
+	proxyDNS      atomic.Bool
 	resolverKey   string
 	startCh       chan struct{}
 }
 
 // NewResolver creates a new instance of the Resolver
 func NewResolver(address string, proxyDNS bool, resolverKey string, backend DNSBackend) Resolver {
-	return &resolver{
+	r := &resolver{
 		backend:       backend,
-		proxyDNS:      proxyDNS,
 		listenAddress: address,
 		resolverKey:   resolverKey,
 		err:           fmt.Errorf("setup not done yet"),
 		startCh:       make(chan struct{}, 1),
 	}
+	r.proxyDNS.Store(proxyDNS)
+
+	return r
 }
 
 func (r *resolver) SetupFunc(port int) func() {
@@ -196,6 +202,10 @@ func (r *resolver) SetExtServers(extDNS []extDNSEntry) {
 	}
 }
 
+func (r *resolver) SetForwardingPolicy(policy bool) {
+	r.proxyDNS.Store(policy)
+}
+
 func (r *resolver) NameServer() string {
 	return r.listenAddress
 }
@@ -407,7 +417,7 @@ func (r *resolver) ServeDNS(w dns.ResponseWriter, query *dns.Msg) {
 	if resp == nil {
 		// If the backend doesn't support proxying dns request
 		// fail the response
-		if !r.proxyDNS {
+		if !r.proxyDNS.Load() {
 			resp = new(dns.Msg)
 			resp.SetRcode(query, dns.RcodeServerFailure)
 			if err := w.WriteMsg(resp); err != nil {

libnetwork/sandbox_dns_unix.go

@@ -27,7 +27,11 @@ const (
 func (sb *sandbox) startResolver(restore bool) {
 	sb.resolverOnce.Do(func() {
 		var err error
-		sb.resolver = NewResolver(resolverIPSandbox, true, sb.Key(), sb)
+		// The resolver is started with proxyDNS=false if the sandbox does not currently
+		// have a gateway. So, if the Sandbox is only connected to an 'internal' network,
+		// it will not forward DNS requests to external resolvers. The resolver's
+		// proxyDNS setting is then updated as network Endpoints are added/removed.
+		sb.resolver = NewResolver(resolverIPSandbox, sb.hasExternalConnectivity(), sb.Key(), sb)
 		defer func() {
 			if err != nil {
 				sb.resolver = nil