Skip to content

Commit

Permalink
Merge pull request moby#47744 from robmry/47716_no_dns_req_to_self
Browse files Browse the repository at this point in the history
Do not forward DNS requests to self.
  • Loading branch information
akerouanton authored May 7, 2024
2 parents da3f60b + 8750614 commit 4d525c9
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 8 deletions.
7 changes: 7 additions & 0 deletions integration/internal/container/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ func WithNetworkMode(mode string) func(*TestContainerConfig) {
}
}

// WithDNS sets external DNS servers for the container
func WithDNS(dns []string) func(*TestContainerConfig) {
return func(c *TestContainerConfig) {
c.HostConfig.DNS = append([]string(nil), dns...)
}
}

// WithSysctls sets sysctl options for the container
func WithSysctls(sysctls map[string]string) func(*TestContainerConfig) {
return func(c *TestContainerConfig) {
Expand Down
58 changes: 58 additions & 0 deletions integration/network/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/docker/docker/integration/internal/network"
"github.com/docker/docker/testutil"
"github.com/docker/docker/testutil/daemon"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
"gotest.tools/v3/poll"
"gotest.tools/v3/skip"
)
Expand All @@ -33,3 +35,59 @@ func TestDaemonDNSFallback(t *testing.T) {

poll.WaitOn(t, container.IsSuccessful(ctx, c, cid), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(10*time.Second))
}

// Check that, when the internal DNS server's address is supplied as an external
// DNS server, the daemon doesn't start talking to itself.
func TestIntDNSAsExtDNS(t *testing.T) {
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot start daemon on Windows test run")
skip.If(t, testEnv.IsRemoteDaemon, "cannot start daemon on remote test run")

ctx := setupTest(t)

d := daemon.New(t)
d.StartWithBusybox(ctx, t)
defer d.Stop(t)

c := d.NewClientT(t)
defer c.Close()

testcases := []struct {
name string
extServers []string
expExitCode int
expStdout string
}{
{
name: "only self",
extServers: []string{"127.0.0.11"},
expExitCode: 1,
expStdout: "SERVFAIL",
},
{
name: "self then ext",
extServers: []string{"127.0.0.11", "8.8.8.8"},
expExitCode: 0,
expStdout: "Non-authoritative answer",
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
ctx := testutil.StartSpan(ctx, t)

const netName = "testnet"
network.CreateNoError(ctx, t, c, netName)
defer network.RemoveNoError(ctx, t, c, netName)

res := container.RunAttach(ctx, t, c,
container.WithNetworkMode(netName),
container.WithDNS(tc.extServers),
container.WithCmd("nslookup", "docker.com"),
)
defer c.ContainerRemove(ctx, res.ContainerID, containertypes.RemoveOptions{Force: true})

assert.Check(t, is.Equal(res.ExitCode, tc.expExitCode))
assert.Check(t, is.Contains(res.Stdout.String(), tc.expStdout))
})
}
}
27 changes: 19 additions & 8 deletions libnetwork/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,7 @@ func (r *Resolver) Stop() {
// when forwarding queries, unless SetExtServersForSrc has configured servers
// for the DNS client making the request.
func (r *Resolver) SetExtServers(extDNS []extDNSEntry) {
l := len(extDNS)
if l > maxExtDNS {
l = maxExtDNS
}
for i := 0; i < l; i++ {
r.extDNSList[i] = extDNS[i]
}
copy(r.extDNSList[:], r.filterExtServers(extDNS))
}

// SetForwardingPolicy re-configures the embedded DNS resolver to either enable or disable forwarding DNS queries to
Expand All @@ -244,7 +238,7 @@ func (r *Resolver) SetForwardingPolicy(policy bool) {
// in preference to servers set by SetExtServers. Supplying a nil or empty extDNS
// deletes nameservers for srcAddr.
func (r *Resolver) SetExtServersForSrc(srcAddr netip.Addr, extDNS []extDNSEntry) error {
r.ipToExtDNS.set(srcAddr, extDNS)
r.ipToExtDNS.set(srcAddr, r.filterExtServers(extDNS))
return nil
}

Expand All @@ -258,6 +252,23 @@ func (r *Resolver) ResolverOptions() []string {
return []string{"ndots:0"}
}

// filterExtServers removes the resolver's own address from extDNS if present,
// and returns the result.
func (r *Resolver) filterExtServers(extDNS []extDNSEntry) []extDNSEntry {
result := make([]extDNSEntry, 0, len(extDNS))
for _, e := range extDNS {
if !e.HostLoopback {
if ra, _ := netip.ParseAddr(e.IPStr); ra == r.listenAddress {
log.G(context.TODO()).Infof("[resolver] not using own address (%s) as an external DNS server",
r.listenAddress)
continue
}
}
result = append(result, e)
}
return result
}

//nolint:gosec // The RNG is not used in a security-sensitive context.
var (
shuffleRNG = rand.New(rand.NewSource(time.Now().Unix()))
Expand Down

0 comments on commit 4d525c9

Please sign in to comment.