Merge pull request #47593 from corhere/backport-23.0/libnet-resolver-…

…nxdomain

[23.0 backport] libnet: Don't forward to upstream resolvers on internal nw

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,11 +425,15 @@ 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 {

integration/internal/container/container.go

@@ -1,14 +1,18 @@
 package container
 
 import (
+	"bytes"
 	"context"
+	"errors"
 	"runtime"
+	"sync"
 	"testing"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/client"
+	"github.com/docker/docker/pkg/stdcopy"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"gotest.tools/v3/assert"
 )
@@ -71,3 +75,75 @@ func Run(ctx context.Context, t *testing.T, client client.APIClient, ops ...func
 
 	return id
 }
+
+type RunResult struct {
+	ContainerID string
+	ExitCode    int
+	Stdout      *bytes.Buffer
+	Stderr      *bytes.Buffer
+}
+
+func RunAttach(ctx context.Context, t *testing.T, client client.APIClient, ops ...func(config *TestContainerConfig)) RunResult {
+	t.Helper()
+
+	ops = append(ops, func(c *TestContainerConfig) {
+		c.Config.AttachStdout = true
+		c.Config.AttachStderr = true
+	})
+	id := Create(ctx, t, client, ops...)
+
+	aresp, err := client.ContainerAttach(ctx, id, types.ContainerAttachOptions{
+		Stream: true,
+		Stdout: true,
+		Stderr: true,
+	})
+	assert.NilError(t, err)
+
+	err = client.ContainerStart(ctx, id, types.ContainerStartOptions{})
+	assert.NilError(t, err)
+
+	s, err := demultiplexStreams(ctx, aresp)
+	if !errors.Is(err, context.DeadlineExceeded) && !errors.Is(err, context.Canceled) {
+		assert.NilError(t, err)
+	}
+
+	// Inspect to get the exit code. A new context is used here to make sure that if the context passed as argument as
+	// reached timeout during the demultiplexStream call, we still return a RunResult.
+	resp, err := client.ContainerInspect(context.Background(), id)
+	assert.NilError(t, err)
+
+	return RunResult{ContainerID: id, ExitCode: resp.State.ExitCode, Stdout: &s.stdout, Stderr: &s.stderr}
+}
+
+type streams struct {
+	stdout, stderr bytes.Buffer
+}
+
+// demultiplexStreams starts a goroutine to demultiplex stdout and stderr from the types.HijackedResponse resp and
+// waits until either multiplexed stream reaches EOF or the context expires. It unconditionally closes resp and waits
+// until the demultiplexing goroutine has finished its work before returning.
+func demultiplexStreams(ctx context.Context, resp types.HijackedResponse) (streams, error) {
+	var s streams
+	outputDone := make(chan error, 1)
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		_, err := stdcopy.StdCopy(&s.stdout, &s.stderr, resp.Reader)
+		outputDone <- err
+		wg.Done()
+	}()
+
+	var err error
+	select {
+	case copyErr := <-outputDone:
+		err = copyErr
+		break
+	case <-ctx.Done():
+		err = ctx.Err()
+	}
+
+	resp.Close()
+	wg.Wait()
+	return s, 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/main_test.go

@@ -0,0 +1,34 @@
+package networking
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/docker/docker/testutil/environment"
+)
+
+var testEnv *environment.Execution
+
+func TestMain(m *testing.M) {
+	var err error
+	testEnv, err = environment.New()
+	if err != nil {
+		panic(err)
+	}
+
+	err = environment.EnsureFrozenImagesLinux(testEnv)
+	if err != nil {
+		panic(err)
+	}
+
+	testEnv.Print()
+	code := m.Run()
+	os.Exit(code)
+}
+
+func setupTest(t *testing.T) context.Context {
+	environment.ProtectAll(t, testEnv)
+	t.Cleanup(func() { testEnv.Clean(t) })
+	return context.Background()
+}

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 {