Skip to content

Commit e63daec

Browse files
authored
Merge pull request #47589 from vvoland/v25.0-47538
[25.0 backport] libnet: Don't forward to upstream resolvers on internal nw
2 parents 817bccb + a987bc5 commit e63daec

File tree

5 files changed

+182
-18
lines changed

5 files changed

+182
-18
lines changed

daemon/container_operations_unix.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,7 @@ func serviceDiscoveryOnDefaultNetwork() bool {
380380

381381
func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Config, sboxOptions *[]libnetwork.SandboxOption) error {
382382
var err error
383+
var originResolvConfPath string
383384

384385
// Set the correct paths for /etc/hosts and /etc/resolv.conf, based on the
385386
// networking-mode of the container. Note that containers with "container"
@@ -393,8 +394,8 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
393394
*sboxOptions = append(
394395
*sboxOptions,
395396
libnetwork.OptionOriginHostsPath("/etc/hosts"),
396-
libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf"),
397397
)
398+
originResolvConfPath = "/etc/resolv.conf"
398399
case container.HostConfig.NetworkMode.IsUserDefined():
399400
// The container uses a user-defined network. We use the embedded DNS
400401
// server for container name resolution and to act as a DNS forwarder
@@ -407,10 +408,7 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
407408
// If systemd-resolvd is used, the "upstream" DNS servers can be found in
408409
// /run/systemd/resolve/resolv.conf. We do not query those DNS servers
409410
// directly, as they can be dynamically reconfigured.
410-
*sboxOptions = append(
411-
*sboxOptions,
412-
libnetwork.OptionOriginResolvConfPath("/etc/resolv.conf"),
413-
)
411+
originResolvConfPath = "/etc/resolv.conf"
414412
default:
415413
// For other situations, such as the default bridge network, container
416414
// discovery / name resolution is handled through /etc/hosts, and no
@@ -423,11 +421,15 @@ func setupPathsAndSandboxOptions(container *container.Container, cfg *config.Con
423421
// DNS servers on the host can be dynamically updated.
424422
//
425423
// Copy the host's resolv.conf for the container (/run/systemd/resolve/resolv.conf or /etc/resolv.conf)
426-
*sboxOptions = append(
427-
*sboxOptions,
428-
libnetwork.OptionOriginResolvConfPath(cfg.GetResolvConf()),
429-
)
424+
originResolvConfPath = cfg.GetResolvConf()
425+
}
426+
427+
// Allow tests to point at their own resolv.conf file.
428+
if envPath := os.Getenv("DOCKER_TEST_RESOLV_CONF_PATH"); envPath != "" {
429+
log.G(context.TODO()).Infof("Using OriginResolvConfPath from env: %s", envPath)
430+
originResolvConfPath = envPath
430431
}
432+
*sboxOptions = append(*sboxOptions, libnetwork.OptionOriginResolvConfPath(originResolvConfPath))
431433

432434
container.HostsPath, err = container.GetRootResourcePath("hosts")
433435
if err != nil {
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package networking
2+
3+
import (
4+
"net"
5+
"os"
6+
"testing"
7+
8+
containertypes "github.com/docker/docker/api/types/container"
9+
"github.com/docker/docker/integration/internal/container"
10+
"github.com/docker/docker/integration/internal/network"
11+
"github.com/docker/docker/testutil/daemon"
12+
"github.com/miekg/dns"
13+
"gotest.tools/v3/assert"
14+
is "gotest.tools/v3/assert/cmp"
15+
"gotest.tools/v3/skip"
16+
)
17+
18+
// writeTempResolvConf writes a resolv.conf that only contains a single
19+
// nameserver line, with address addr.
20+
// It returns the name of the temp file.
21+
func writeTempResolvConf(t *testing.T, addr string) string {
22+
t.Helper()
23+
// Not using t.TempDir() here because in rootless mode, while the temporary
24+
// directory gets mode 0777, it's a subdir of an 0700 directory owned by root.
25+
// So, it's not accessible by the daemon.
26+
f, err := os.CreateTemp("", "resolv.conf")
27+
assert.NilError(t, err)
28+
t.Cleanup(func() { os.Remove(f.Name()) })
29+
err = f.Chmod(0644)
30+
assert.NilError(t, err)
31+
f.Write([]byte("nameserver " + addr + "\n"))
32+
return f.Name()
33+
}
34+
35+
const dnsRespAddr = "10.11.12.13"
36+
37+
// startDaftDNS starts and returns a really, really daft DNS server that only
38+
// responds to type-A requests, and always with address dnsRespAddr.
39+
func startDaftDNS(t *testing.T, addr string) *dns.Server {
40+
serveDNS := func(w dns.ResponseWriter, query *dns.Msg) {
41+
if query.Question[0].Qtype == dns.TypeA {
42+
resp := &dns.Msg{}
43+
resp.SetReply(query)
44+
answer := &dns.A{
45+
Hdr: dns.RR_Header{
46+
Name: query.Question[0].Name,
47+
Rrtype: dns.TypeA,
48+
Class: dns.ClassINET,
49+
Ttl: 600,
50+
},
51+
}
52+
answer.A = net.ParseIP(dnsRespAddr)
53+
resp.Answer = append(resp.Answer, answer)
54+
_ = w.WriteMsg(resp)
55+
}
56+
}
57+
58+
conn, err := net.ListenUDP("udp", &net.UDPAddr{
59+
IP: net.ParseIP(addr),
60+
Port: 53,
61+
})
62+
assert.NilError(t, err)
63+
64+
server := &dns.Server{Handler: dns.HandlerFunc(serveDNS), PacketConn: conn}
65+
go func() {
66+
_ = server.ActivateAndServe()
67+
}()
68+
69+
return server
70+
}
71+
72+
// Check that when a container is connected to an internal network, DNS
73+
// requests sent to daemon's internal DNS resolver are not forwarded to
74+
// an upstream resolver listening on a localhost address.
75+
// (Assumes the host does not already have a DNS server on 127.0.0.1.)
76+
func TestInternalNetworkDNS(t *testing.T) {
77+
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "No resolv.conf on Windows")
78+
skip.If(t, testEnv.IsRootless, "Can't use resolver on host in rootless mode")
79+
ctx := setupTest(t)
80+
81+
// Start a DNS server on the loopback interface.
82+
server := startDaftDNS(t, "127.0.0.1")
83+
defer server.Shutdown()
84+
85+
// Set up a temp resolv.conf pointing at that DNS server, and a daemon using it.
86+
tmpFileName := writeTempResolvConf(t, "127.0.0.1")
87+
d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName))
88+
d.StartWithBusybox(ctx, t, "--experimental", "--ip6tables")
89+
defer d.Stop(t)
90+
91+
c := d.NewClientT(t)
92+
defer c.Close()
93+
94+
intNetName := "intnet"
95+
network.CreateNoError(ctx, t, c, intNetName,
96+
network.WithDriver("bridge"),
97+
network.WithInternal(),
98+
)
99+
defer network.RemoveNoError(ctx, t, c, intNetName)
100+
101+
extNetName := "extnet"
102+
network.CreateNoError(ctx, t, c, extNetName,
103+
network.WithDriver("bridge"),
104+
)
105+
defer network.RemoveNoError(ctx, t, c, extNetName)
106+
107+
// Create a container, initially with external connectivity.
108+
// Expect the external DNS server to respond to a request from the container.
109+
ctrId := container.Run(ctx, t, c, container.WithNetworkMode(extNetName))
110+
defer c.ContainerRemove(ctx, ctrId, containertypes.RemoveOptions{Force: true})
111+
res, err := container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
112+
assert.NilError(t, err)
113+
assert.Check(t, is.Equal(res.ExitCode, 0))
114+
assert.Check(t, is.Contains(res.Stdout(), dnsRespAddr))
115+
116+
// Connect the container to the internal network as well.
117+
// External DNS should still be used.
118+
err = c.NetworkConnect(ctx, intNetName, ctrId, nil)
119+
assert.NilError(t, err)
120+
res, err = container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
121+
assert.NilError(t, err)
122+
assert.Check(t, is.Equal(res.ExitCode, 0))
123+
assert.Check(t, is.Contains(res.Stdout(), dnsRespAddr))
124+
125+
// Disconnect from the external network.
126+
// Expect no access to the external DNS.
127+
err = c.NetworkDisconnect(ctx, extNetName, ctrId, true)
128+
assert.NilError(t, err)
129+
res, err = container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
130+
assert.NilError(t, err)
131+
assert.Check(t, is.Equal(res.ExitCode, 1))
132+
assert.Check(t, is.Contains(res.Stdout(), "SERVFAIL"))
133+
134+
// Reconnect the external network.
135+
// Check that the external DNS server is used again.
136+
err = c.NetworkConnect(ctx, extNetName, ctrId, nil)
137+
assert.NilError(t, err)
138+
res, err = container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"})
139+
assert.NilError(t, err)
140+
assert.Check(t, is.Equal(res.ExitCode, 0))
141+
assert.Check(t, is.Contains(res.Stdout(), dnsRespAddr))
142+
}

libnetwork/endpoint.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,8 +538,13 @@ func (ep *Endpoint) sbJoin(sb *Sandbox, options ...EndpointOption) (err error) {
538538
return sb.setupDefaultGW()
539539
}
540540

541-
moveExtConn := sb.getGatewayEndpoint() != extEp
541+
currentExtEp := sb.getGatewayEndpoint()
542+
// Enable upstream forwarding if the sandbox gained external connectivity.
543+
if sb.resolver != nil {
544+
sb.resolver.SetForwardingPolicy(currentExtEp != nil)
545+
}
542546

547+
moveExtConn := currentExtEp != extEp
543548
if moveExtConn {
544549
if extEp != nil {
545550
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)
735740

736741
// New endpoint providing external connectivity for the sandbox
737742
extEp = sb.getGatewayEndpoint()
743+
// Disable upstream forwarding if the sandbox lost external connectivity.
744+
if sb.resolver != nil {
745+
sb.resolver.SetForwardingPolicy(extEp != nil)
746+
}
747+
738748
if moveExtConn && extEp != nil {
739749
log.G(context.TODO()).Debugf("Programming external connectivity on endpoint %s (%s)", extEp.Name(), extEp.ID())
740750
extN, err := extEp.getNetworkFromStore()

libnetwork/resolver.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strconv"
1010
"strings"
1111
"sync"
12+
"sync/atomic"
1213
"time"
1314

1415
"github.com/containerd/log"
@@ -75,7 +76,7 @@ type Resolver struct {
7576
tcpListen *net.TCPListener
7677
err error
7778
listenAddress string
78-
proxyDNS bool
79+
proxyDNS atomic.Bool
7980
startCh chan struct{}
8081
logger *log.Entry
8182

@@ -85,15 +86,17 @@ type Resolver struct {
8586

8687
// NewResolver creates a new instance of the Resolver
8788
func NewResolver(address string, proxyDNS bool, backend DNSBackend) *Resolver {
88-
return &Resolver{
89+
r := &Resolver{
8990
backend: backend,
90-
proxyDNS: proxyDNS,
9191
listenAddress: address,
9292
err: fmt.Errorf("setup not done yet"),
9393
startCh: make(chan struct{}, 1),
9494
fwdSem: semaphore.NewWeighted(maxConcurrent),
9595
logInverval: rate.Sometimes{Interval: logInterval},
9696
}
97+
r.proxyDNS.Store(proxyDNS)
98+
99+
return r
97100
}
98101

99102
func (r *Resolver) log(ctx context.Context) *log.Entry {
@@ -194,6 +197,12 @@ func (r *Resolver) SetExtServers(extDNS []extDNSEntry) {
194197
}
195198
}
196199

200+
// SetForwardingPolicy re-configures the embedded DNS resolver to either enable or disable forwarding DNS queries to
201+
// external servers.
202+
func (r *Resolver) SetForwardingPolicy(policy bool) {
203+
r.proxyDNS.Store(policy)
204+
}
205+
197206
// NameServer returns the IP of the DNS resolver for the containers.
198207
func (r *Resolver) NameServer() string {
199208
return r.listenAddress
@@ -421,7 +430,7 @@ func (r *Resolver) serveDNS(w dns.ResponseWriter, query *dns.Msg) {
421430
return
422431
}
423432

424-
if r.proxyDNS {
433+
if r.proxyDNS.Load() {
425434
// If the user sets ndots > 0 explicitly and the query is
426435
// in the root domain don't forward it out. We will return
427436
// failure and let the client retry with the search domain

libnetwork/sandbox_dns_unix.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@ const (
3030
func (sb *Sandbox) startResolver(restore bool) {
3131
sb.resolverOnce.Do(func() {
3232
var err error
33-
// The embedded resolver is always started with proxyDNS set as true, even when the sandbox is only attached to
34-
// an internal network. This way, it's the driver responsibility to make sure `connect` syscall fails fast when
35-
// no external connectivity is available (eg. by not setting a default gateway).
36-
sb.resolver = NewResolver(resolverIPSandbox, true, sb)
33+
// The resolver is started with proxyDNS=false if the sandbox does not currently
34+
// have a gateway. So, if the Sandbox is only connected to an 'internal' network,
35+
// it will not forward DNS requests to external resolvers. The resolver's
36+
// proxyDNS setting is then updated as network Endpoints are added/removed.
37+
sb.resolver = NewResolver(resolverIPSandbox, sb.getGatewayEndpoint() != nil, sb)
3738
defer func() {
3839
if err != nil {
3940
sb.resolver = nil

0 commit comments

Comments
 (0)