Windows DNS resolver forwarding

Make the internal DNS resolver for Windows containers forward requests
to upsteam DNS servers when it cannot respond itself, rather than
returning SERVFAIL.

Windows containers are normally configured with the internal resolver
first for service discovery (container name lookup), then external
resolvers from '--dns' or the host's networking configuration.

When a tool like ping gets a SERVFAIL from the internal resolver, it
tries the other nameservers. But, nslookup does not, and with this
change it does not need to.

The internal resolver learns external server addresses from the
container's HNSEndpoint configuration, so it will use the same DNS
servers as processes in the container.

The internal resolver for Windows containers listens on the network's
gateway address, and each container may have a different set of external
DNS servers. So, the resolver uses the source address of the DNS request
to select external resolvers.

On Windows, daemon.json feature option 'windows-no-dns-proxy' can be used
to prevent the internal resolver from forwarding requests (restoring the
old behaviour).

Signed-off-by: Rob Murray <rob.murray@docker.com>

daemon/config/config_linux.go

@@ -196,6 +196,10 @@ func (conf *Config) ValidatePlatformConfig() error {
 		return errors.Wrap(err, "invalid fixed-cidr-v6")
 	}
 
+	if _, ok := conf.Features["windows-dns-proxy"]; ok {
+		return errors.New("feature option 'windows-dns-proxy' is only available on Windows")
+	}
+
 	return verifyDefaultCgroupNsMode(conf.CgroupNamespaceMode)
 }
 

daemon/container_operations.go

@@ -54,7 +54,8 @@ func (daemon *Daemon) buildSandboxOptions(cfg *config.Config, container *contain
 		sboxOptions = append(sboxOptions, libnetwork.OptionUseExternalKey())
 	}
 
-	if err := setupPathsAndSandboxOptions(container, cfg, &sboxOptions); err != nil {
+	// Add platform-specific Sandbox options.
+	if err := buildSandboxPlatformOptions(container, cfg, &sboxOptions); err != nil {
 		return nil, err
 	}
 

daemon/container_operations_unix.go

@@ -417,7 +417,7 @@ func serviceDiscoveryOnDefaultNetwork() bool {
 	return false
 }
 
-func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
+func buildSandboxPlatformOptions(container *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
 	var err error
 	var originResolvConfPath string
 
@@ -481,6 +481,7 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
 		return err
 	}
 	*sboxOptions = append(*sboxOptions, libnetwork.OptionResolvConfPath(container.ResolvConfPath))
+
 	return nil
 }
 

daemon/container_operations_windows.go

@@ -163,7 +163,13 @@ func serviceDiscoveryOnDefaultNetwork() bool {
 	return true
 }
 
-func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
+func buildSandboxPlatformOptions(container *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
+	// By default, the Windows internal resolver does not forward requests to
+	// external resolvers - but forwarding can be enabled using feature flag
+	// "windows-dns-proxy":true.
+	if doproxy, exists := cfg.Features["windows-dns-proxy"]; !exists || !doproxy {
+		*sboxOptions = append(*sboxOptions, libnetwork.OptionDNSNoProxy())
+	}
 	return nil
 }
 

integration/networking/resolvconf_test.go

@@ -1,8 +1,10 @@
 package networking
 
 import (
+	"context"
 	"strings"
 	"testing"
+	"time"
 
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/integration/internal/container"
@@ -131,3 +133,27 @@ func TestInternalNetworkDNS(t *testing.T) {
 	assert.Check(t, is.Equal(res.ExitCode, 0))
 	assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr))
 }
+
+// TestNslookupWindows checks that nslookup gets results from external DNS.
+// Regression test for https://github.com/moby/moby/issues/46792
+func TestNslookupWindows(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType != "windows")
+
+	ctx := setupTest(t)
+	c := testEnv.APIClient()
+
+	attachCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
+	defer cancel()
+	res := container.RunAttach(attachCtx, t, c,
+		container.WithCmd("nslookup", "docker.com"),
+	)
+	defer c.ContainerRemove(ctx, res.ContainerID, containertypes.RemoveOptions{Force: true})
+
+	assert.Check(t, is.Equal(res.ExitCode, 0))
+	// Current default is to not-forward requests to external servers, which
+	// can only be changed in daemon.json using feature flag "windows-dns-proxy".
+	// So, expect the lookup to fail...
+	assert.Check(t, is.Contains(res.Stderr.String(), "Server failed"))
+	// When the default behaviour is changed, nslookup should succeed...
+	//assert.Check(t, is.Contains(res.Stdout.String(), "Addresses:"))
+}

libnetwork/endpoint.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 
 	"github.com/containerd/log"
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/internal/sliceutil"
 	"github.com/docker/docker/libnetwork/datastore"
 	"github.com/docker/docker/libnetwork/ipamapi"
@@ -543,6 +544,10 @@ func (ep *Endpoint) sbJoin(sb *Sandbox, options ...EndpointOption) (err error) {
 		return err
 	}
 
+	if err = addEpToResolver(context.TODO(), n.Name(), ep.Name(), &sb.config, ep.iface, n.Resolvers()); err != nil {
+		return errdefs.System(err)
+	}
+
 	if err = n.getController().updateToStore(ep); err != nil {
 		return err
 	}
@@ -745,6 +750,10 @@ func (ep *Endpoint) sbLeave(sb *Sandbox, force bool) error {
 		log.G(context.TODO()).Warnf("Failed to clean up service info on container %s disconnect: %v", ep.name, err)
 	}
 
+	if err := deleteEpFromResolver(ep.Name(), ep.iface, n.Resolvers()); err != nil {
+		log.G(context.TODO()).Warnf("Failed to clean up resolver info on container %s disconnect: %v", ep.name, err)
+	}
+
 	if err := sb.clearNetworkResources(ep); err != nil {
 		log.G(context.TODO()).Warnf("Failed to clean up network resources on container %s disconnect: %v", ep.name, err)
 	}

libnetwork/network.go

@@ -192,7 +192,6 @@ type Network struct {
 	dbExists         bool
 	persist          bool
 	drvOnce          *sync.Once
-	resolverOnce     sync.Once //nolint:nolintlint,unused // only used on windows
 	resolver         []*Resolver
 	internal         bool
 	attachable       bool
@@ -204,6 +203,7 @@ type Network struct {
 	configFrom       string
 	loadBalancerIP   net.IP
 	loadBalancerMode string
+	platformNetwork  //nolint:nolintlint,unused // only populated on windows
 	mu               sync.Mutex
 }
 
@@ -244,6 +244,13 @@ func (n *Network) Type() string {
 	return n.networkType
 }
 
+func (n *Network) Resolvers() []*Resolver {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+
+	return n.resolver
+}
+
 func (n *Network) Key() []string {
 	n.mu.Lock()
 	defer n.mu.Unlock()
@@ -2097,10 +2104,6 @@ func (n *Network) ResolveService(ctx context.Context, name string) ([]*net.SRV,
 	return srv, ip
 }
 
-func (n *Network) ExecFunc(f func()) error {
-	return types.NotImplementedErrorf("ExecFunc not supported by network")
-}
-
 func (n *Network) NdotsSet() bool {
 	return false
 }

libnetwork/network_unix.go

@@ -2,13 +2,33 @@
 
 package libnetwork
 
-import "github.com/docker/docker/libnetwork/ipamapi"
+import (
+	"context"
+
+	"github.com/docker/docker/libnetwork/ipamapi"
+)
+
+type platformNetwork struct{} //nolint:nolintlint,unused // only populated on windows
 
 // Stub implementations for DNS related functions
 
 func (n *Network) startResolver() {
 }
 
+func addEpToResolver(
+	ctx context.Context,
+	netName, epName string,
+	config *containerConfig,
+	epIface *EndpointInterface,
+	resolvers []*Resolver,
+) error {
+	return nil
+}
+
+func deleteEpFromResolver(epName string, epIface *EndpointInterface, resolvers []*Resolver) error {
+	return nil
+}
+
 func defaultIpamForNetworkType(networkType string) string {
 	return ipamapi.DefaultIPAM
 }