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

Commit cbc2a71c2 makes `connect` syscall fail fast when a container is
only attached to an internal network. Thanks to that, if such a
container tries to resolve an "external" domain, the embedded resolver
returns an error immediately instead of waiting for a timeout.

This commit makes sure the embedded resolver doesn't even try to forward
to upstream servers.

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: Paweł Gronowski <pawel.gronowski@docker.com>

integration/networking/resolvconf_test.go

@@ -0,0 +1,142 @@
+package networking
+
+import (
+	"net"
+	"os"
+	"testing"
+
+	containertypes "github.com/docker/docker/api/types/container"
+	"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(ctx, 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, containertypes.RemoveOptions{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/endpoint.go

@@ -538,8 +538,13 @@ func (ep *Endpoint) sbJoin(sb *Sandbox, options ...EndpointOption) (err error) {
 		return sb.setupDefaultGW()
 	}
 
-	moveExtConn := sb.getGatewayEndpoint() != extEp
+	currentExtEp := sb.getGatewayEndpoint()
+	// Enable upstream forwarding if the sandbox gained external connectivity.
+	if sb.resolver != nil {
+		sb.resolver.SetForwardingPolicy(currentExtEp != nil)
+	}
 
+	moveExtConn := currentExtEp != extEp
 	if moveExtConn {
 		if extEp != nil {
 			log.G(context.TODO()).Debugf("Revoking external connectivity on endpoint %s (%s)", extEp.Name(), extEp.ID())
@@ -735,6 +740,11 @@ func (ep *Endpoint) sbLeave(sb *Sandbox, force bool, options ...EndpointOption)
 
 	// New endpoint providing external connectivity for the sandbox
 	extEp = sb.getGatewayEndpoint()
+	// Disable upstream forwarding if the sandbox lost external connectivity.
+	if sb.resolver != nil {
+		sb.resolver.SetForwardingPolicy(extEp != nil)
+	}
+
 	if moveExtConn && extEp != nil {
 		log.G(context.TODO()).Debugf("Programming external connectivity on endpoint %s (%s)", extEp.Name(), extEp.ID())
 		extN, err := extEp.getNetworkFromStore()

libnetwork/resolver.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/containerd/log"
@@ -75,7 +76,7 @@ type Resolver struct {
 	tcpListen     *net.TCPListener
 	err           error
 	listenAddress string
-	proxyDNS      bool
+	proxyDNS      atomic.Bool
 	startCh       chan struct{}
 	logger        *log.Entry
 
@@ -85,15 +86,17 @@ type Resolver struct {
 
 // NewResolver creates a new instance of the Resolver
 func NewResolver(address string, proxyDNS bool, backend DNSBackend) *Resolver {
-	return &Resolver{
+	r := &Resolver{
 		backend:       backend,
-		proxyDNS:      proxyDNS,
 		listenAddress: address,
 		err:           fmt.Errorf("setup not done yet"),
 		startCh:       make(chan struct{}, 1),
 		fwdSem:        semaphore.NewWeighted(maxConcurrent),
 		logInverval:   rate.Sometimes{Interval: logInterval},
 	}
+	r.proxyDNS.Store(proxyDNS)
+
+	return r
 }
 
 func (r *Resolver) log(ctx context.Context) *log.Entry {
@@ -194,6 +197,12 @@ func (r *Resolver) SetExtServers(extDNS []extDNSEntry) {
 	}
 }
 
+// SetForwardingPolicy re-configures the embedded DNS resolver to either enable or disable forwarding DNS queries to
+// external servers.
+func (r *Resolver) SetForwardingPolicy(policy bool) {
+	r.proxyDNS.Store(policy)
+}
+
 // NameServer returns the IP of the DNS resolver for the containers.
 func (r *Resolver) NameServer() string {
 	return r.listenAddress
@@ -421,7 +430,7 @@ func (r *Resolver) serveDNS(w dns.ResponseWriter, query *dns.Msg) {
 		return
 	}
 
-	if r.proxyDNS {
+	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

libnetwork/sandbox_dns_unix.go

@@ -30,10 +30,11 @@ const (
 func (sb *Sandbox) startResolver(restore bool) {
 	sb.resolverOnce.Do(func() {
 		var err error
-		// The embedded resolver is always started with proxyDNS set as true, even when the sandbox is only attached to
-		// an internal network. This way, it's the driver responsibility to make sure `connect` syscall fails fast when
-		// no external connectivity is available (eg. by not setting a default gateway).
-		sb.resolver = NewResolver(resolverIPSandbox, true, 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.getGatewayEndpoint() != nil, sb)
 		defer func() {
 			if err != nil {
 				sb.resolver = nil