Skip to content

Commit

Permalink
libnet: Don't forward to upstream resolvers on internal nw
Browse files Browse the repository at this point in the history
This commit makes sure the embedded resolver doesn't try to forward to
upstream servers when a container is only attached to an internal
network.

Co-authored-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Rob Murray <rob.murray@docker.com>
(cherry picked from commit 790c303)
Signed-off-by: Cory Snider <csnider@mirantis.com>
  • Loading branch information
akerouanton authored and corhere committed Mar 19, 2024
1 parent a379e02 commit f4657ea
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 15 deletions.
20 changes: 11 additions & 9 deletions daemon/container_operations_unix.go
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions integration/internal/network/network.go
Expand Up @@ -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)
}
142 changes: 142 additions & 0 deletions 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))
}
24 changes: 24 additions & 0 deletions libnetwork/default_gateway.go
Expand Up @@ -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
}
11 changes: 10 additions & 1 deletion libnetwork/endpoint.go
Expand Up @@ -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())
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 14 additions & 4 deletions libnetwork/resolver.go
Expand Up @@ -6,6 +6,7 @@ import (
"net"
"strings"
"sync"
"sync/atomic"
"time"

"github.com/docker/docker/libnetwork/types"
Expand All @@ -30,6 +31,9 @@ type Resolver interface {
// SetExtServers configures the external nameservers the resolver
// should use to forward queries
SetExtServers([]extDNSEntry)
// SetForwardingPolicy re-configures the embedded DNS resolver to either
// enable or disable forwarding DNS queries to external servers.
SetForwardingPolicy(policy bool)
// ResolverOptions returns resolv.conf options that should be set
ResolverOptions() []string
}
Expand Down Expand Up @@ -89,21 +93,23 @@ type resolver struct {
tStamp time.Time
queryLock sync.Mutex
listenAddress string
proxyDNS bool
proxyDNS atomic.Bool
resolverKey string
startCh chan struct{}
}

// NewResolver creates a new instance of the Resolver
func NewResolver(address string, proxyDNS bool, resolverKey string, backend DNSBackend) Resolver {
return &resolver{
r := &resolver{
backend: backend,
proxyDNS: proxyDNS,
listenAddress: address,
resolverKey: resolverKey,
err: fmt.Errorf("setup not done yet"),
startCh: make(chan struct{}, 1),
}
r.proxyDNS.Store(proxyDNS)

return r
}

func (r *resolver) SetupFunc(port int) func() {
Expand Down Expand Up @@ -196,6 +202,10 @@ func (r *resolver) SetExtServers(extDNS []extDNSEntry) {
}
}

func (r *resolver) SetForwardingPolicy(policy bool) {
r.proxyDNS.Store(policy)
}

func (r *resolver) NameServer() string {
return r.listenAddress
}
Expand Down Expand Up @@ -407,7 +417,7 @@ func (r *resolver) ServeDNS(w dns.ResponseWriter, query *dns.Msg) {
if resp == nil {
// If the backend doesn't support proxying dns request
// fail the response
if !r.proxyDNS {
if !r.proxyDNS.Load() {
resp = new(dns.Msg)
resp.SetRcode(query, dns.RcodeServerFailure)
if err := w.WriteMsg(resp); err != nil {
Expand Down
6 changes: 5 additions & 1 deletion libnetwork/sandbox_dns_unix.go
Expand Up @@ -27,7 +27,11 @@ const (
func (sb *sandbox) startResolver(restore bool) {
sb.resolverOnce.Do(func() {
var err error
sb.resolver = NewResolver(resolverIPSandbox, true, sb.Key(), 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.hasExternalConnectivity(), sb.Key(), sb)
defer func() {
if err != nil {
sb.resolver = nil
Expand Down

0 comments on commit f4657ea

Please sign in to comment.