From d8b768149b21db9c9e831f7b899d1018d0ad64b9 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Thu, 4 Apr 2024 09:19:45 +0100 Subject: [PATCH 1/4] Move dummy DNS server to integration/internal/network Signed-off-by: Rob Murray --- integration/internal/network/dns.go | 66 +++++++++++++++++++++ integration/networking/resolvconf_test.go | 70 ++--------------------- 2 files changed, 72 insertions(+), 64 deletions(-) create mode 100644 integration/internal/network/dns.go diff --git a/integration/internal/network/dns.go b/integration/internal/network/dns.go new file mode 100644 index 0000000000000..ab87b2b8b8ee5 --- /dev/null +++ b/integration/internal/network/dns.go @@ -0,0 +1,66 @@ +package network + +import ( + "net" + "os" + "testing" + + "github.com/miekg/dns" + "gotest.tools/v3/assert" +) + +const DNSRespAddr = "10.11.12.13" + +// WriteTempResolvConf writes a resolv.conf that only contains a single +// nameserver line, with address addr. +// It returns the name of the temp file. The temp file will be deleted +// automatically by a t.Cleanup(). +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() +} + +// StartDaftDNS starts and returns a really, really daft DNS server that only +// responds to type-A requests, and always with address dnsRespAddr. +// The DNS server will be stopped automatically by a t.Cleanup(). +func StartDaftDNS(t *testing.T, addr string) { + 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() + }() + + t.Cleanup(func() { server.Shutdown() }) +} diff --git a/integration/networking/resolvconf_test.go b/integration/networking/resolvconf_test.go index f776d7bd42980..f489c6a7f4786 100644 --- a/integration/networking/resolvconf_test.go +++ b/integration/networking/resolvconf_test.go @@ -1,8 +1,6 @@ package networking import ( - "net" - "os" "strings" "testing" @@ -10,29 +8,11 @@ import ( "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() -} - // Regression test for https://github.com/moby/moby/issues/46968 func TestResolvConfLocalhostIPv6(t *testing.T) { // No "/etc/resolv.conf" on Windows. @@ -40,7 +20,7 @@ func TestResolvConfLocalhostIPv6(t *testing.T) { ctx := setupTest(t) - tmpFileName := writeTempResolvConf(t, "127.0.0.53") + tmpFileName := network.WriteTempResolvConf(t, "127.0.0.53") d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName)) d.StartWithBusybox(ctx, t, "--experimental", "--ip6tables") @@ -81,43 +61,6 @@ options ndots:0 `)) } -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. @@ -128,11 +71,10 @@ func TestInternalNetworkDNS(t *testing.T) { ctx := setupTest(t) // Start a DNS server on the loopback interface. - server := startDaftDNS(t, "127.0.0.1") - defer server.Shutdown() + network.StartDaftDNS(t, "127.0.0.1") // Set up a temp resolv.conf pointing at that DNS server, and a daemon using it. - tmpFileName := writeTempResolvConf(t, "127.0.0.1") + tmpFileName := network.WriteTempResolvConf(t, "127.0.0.1") d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName)) d.StartWithBusybox(ctx, t, "--experimental", "--ip6tables") defer d.Stop(t) @@ -160,7 +102,7 @@ func TestInternalNetworkDNS(t *testing.T) { 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)) + assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr)) // Connect the container to the internal network as well. // External DNS should still be used. @@ -169,7 +111,7 @@ func TestInternalNetworkDNS(t *testing.T) { 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)) + assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr)) // Disconnect from the external network. // Expect no access to the external DNS. @@ -187,5 +129,5 @@ func TestInternalNetworkDNS(t *testing.T) { 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)) + assert.Check(t, is.Contains(res.Stdout(), network.DNSRespAddr)) } From 17b863154573d998394be336fe1487827071b019 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 3 Apr 2024 14:54:52 +0100 Subject: [PATCH 2/4] Enable DNS proxying for ipvlan-l3 The internal DNS resolver should only forward requests to external resolvers if the libnetwork.Sandbox served by the resolver has external network access (so, no forwarding for '--internal' networks). The test for external network access was whether the Sandbox had an Endpoint with a gateway configured. However, an ipvlan-l3 networks with external network access does not have a gateway, it has a default route bound to an interface. Also, we document that an ipvlan network with no parent interface is equivalent to a '--internal' network. But, in this case, an ipvlan-l2 network was configured with a gateway. So, DNS proxying would be enabled in the internal resolver (and, if the host's resolver was on a localhost address, requests to external resolvers from the host's network namespace would succeed). So, this change adjusts the test for enabling DNS proxying to include a check for '--internal' (as a shortcut) and, for non-internal networks, checks for a default route as well as a gateway. It also disables configuration of a gateway or a default route for an ipvlan Endpoint if no parent interface is specified. (Note if a parent interface with no external network is supplied as '-o parent=', the gateway/default route will still be set up and external DNS proxying will be enabled. The network must be configured as '--internal' to prevent that from happening.) Signed-off-by: Rob Murray --- integration/network/ipvlan/ipvlan_test.go | 196 ++++++++++++++---- libnetwork/drivers/ipvlan/ipvlan_joinleave.go | 4 + libnetwork/drivers/ipvlan/ipvlan_network.go | 1 + libnetwork/endpoint.go | 10 +- libnetwork/endpoint_info.go | 25 +++ libnetwork/sandbox.go | 15 ++ libnetwork/sandbox_dns_unix.go | 2 +- 7 files changed, 212 insertions(+), 41 deletions(-) diff --git a/integration/network/ipvlan/ipvlan_test.go b/integration/network/ipvlan/ipvlan_test.go index 37e3dd0b303bd..49e1f3155f0d8 100644 --- a/integration/network/ipvlan/ipvlan_test.go +++ b/integration/network/ipvlan/ipvlan_test.go @@ -4,12 +4,15 @@ package ipvlan // import "github.com/docker/docker/integration/network/ipvlan" import ( "context" + "fmt" "os" "os/exec" "strings" "sync" "testing" + "github.com/docker/docker/api/types" + containertypes "github.com/docker/docker/api/types/container" dclient "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" net "github.com/docker/docker/integration/internal/network" @@ -17,6 +20,7 @@ import ( "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" ) @@ -79,14 +83,20 @@ func TestDockerNetworkIpvlan(t *testing.T) { name: "L3InternalMode", test: testIpvlanL3InternalMode, }, { - name: "L2MultiSubnet", - test: testIpvlanL2MultiSubnet, + name: "L2MultiSubnetWithParent", + test: testIpvlanL2MultiSubnetWithParent, + }, { + name: "L2MultiSubnetNoParent", + test: testIpvlanL2MultiSubnetNoParent, }, { name: "L3MultiSubnet", test: testIpvlanL3MultiSubnet, }, { - name: "Addressing", - test: testIpvlanAddressing, + name: "L2Addressing", + test: testIpvlanL2Addressing, + }, { + name: "L3Addressing", + test: testIpvlanL3Addressing, }, } { @@ -225,10 +235,21 @@ func testIpvlanL3InternalMode(t *testing.T, ctx context.Context, client dclient. assert.NilError(t, err) } -func testIpvlanL2MultiSubnet(t *testing.T, ctx context.Context, client dclient.APIClient) { +func testIpvlanL2MultiSubnetWithParent(t *testing.T, ctx context.Context, client dclient.APIClient) { + const parentIfName = "di-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) + testIpvlanL2MultiSubnet(t, ctx, client, parentIfName) +} + +func testIpvlanL2MultiSubnetNoParent(t *testing.T, ctx context.Context, client dclient.APIClient) { + testIpvlanL2MultiSubnet(t, ctx, client, "") +} + +func testIpvlanL2MultiSubnet(t *testing.T, ctx context.Context, client dclient.APIClient, parent string) { netName := "dualstackl2" net.CreateNoError(ctx, t, client, netName, - net.WithIPvlan("", ""), + net.WithIPvlan(parent, ""), net.WithIPv6(), net.WithIPAM("172.28.200.0/24", ""), net.WithIPAM("172.28.202.0/24", "172.28.202.254"), @@ -250,11 +271,22 @@ func testIpvlanL2MultiSubnet(t *testing.T, ctx context.Context, client dclient.A ) c1, err := client.ContainerInspect(ctx, id1) assert.NilError(t, err) + if parent == "" { + // Inspect the v4 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks[netName].Gateway, "")) + // Inspect the v6 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks[netName].IPv6Gateway, "")) + } else { + // Inspect the v4 gateway to ensure the proper default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks[netName].Gateway, "172.28.200.1")) + // Inspect the v6 gateway to ensure the proper default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks[netName].IPv6Gateway, "2001:db8:abc8::1")) + } - // verify ipv4 connectivity to the explicit --ipv address second to first + // verify ipv4 connectivity to the explicit --ip address second to first _, err = container.Exec(ctx, client, id2, []string{"ping", "-c", "1", c1.NetworkSettings.Networks[netName].IPAddress}) assert.NilError(t, err) - // verify ipv6 connectivity to the explicit --ipv6 address second to first + // verify ipv6 connectivity to the explicit --ip6 address second to first _, err = container.Exec(ctx, client, id2, []string{"ping6", "-c", "1", c1.NetworkSettings.Networks[netName].GlobalIPv6Address}) assert.NilError(t, err) @@ -271,22 +303,24 @@ func testIpvlanL2MultiSubnet(t *testing.T, ctx context.Context, client dclient.A ) c3, err := client.ContainerInspect(ctx, id3) assert.NilError(t, err) + if parent == "" { + // Inspect the v4 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks[netName].Gateway, "")) + // Inspect the v6 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks[netName].IPv6Gateway, "")) + } else { + // Inspect the v4 gateway to ensure the proper explicitly assigned default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks[netName].Gateway, "172.28.202.254")) + // Inspect the v6 gateway to ensure the proper explicitly assigned default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks[netName].IPv6Gateway, "2001:db8:abc6::254")) + } - // verify ipv4 connectivity to the explicit --ipv address from third to fourth + // verify ipv4 connectivity to the explicit --ip address from third to fourth _, err = container.Exec(ctx, client, id4, []string{"ping", "-c", "1", c3.NetworkSettings.Networks[netName].IPAddress}) assert.NilError(t, err) - // verify ipv6 connectivity to the explicit --ipv6 address from third to fourth + // verify ipv6 connectivity to the explicit --ip6 address from third to fourth _, err = container.Exec(ctx, client, id4, []string{"ping6", "-c", "1", c3.NetworkSettings.Networks[netName].GlobalIPv6Address}) assert.NilError(t, err) - - // Inspect the v4 gateway to ensure the proper default GW was assigned - assert.Equal(t, c1.NetworkSettings.Networks[netName].Gateway, "172.28.200.1") - // Inspect the v6 gateway to ensure the proper default GW was assigned - assert.Equal(t, c1.NetworkSettings.Networks[netName].IPv6Gateway, "2001:db8:abc8::1") - // Inspect the v4 gateway to ensure the proper explicitly assigned default GW was assigned - assert.Equal(t, c3.NetworkSettings.Networks[netName].Gateway, "172.28.202.254") - // Inspect the v6 gateway to ensure the proper explicitly assigned default GW was assigned - assert.Equal(t, c3.NetworkSettings.Networks[netName].IPv6Gateway, "2001:db8:abc6::254") } func testIpvlanL3MultiSubnet(t *testing.T, ctx context.Context, client dclient.APIClient) { @@ -353,51 +387,61 @@ func testIpvlanL3MultiSubnet(t *testing.T, ctx context.Context, client dclient.A assert.Equal(t, c3.NetworkSettings.Networks[netName].IPv6Gateway, "") } -func testIpvlanAddressing(t *testing.T, ctx context.Context, client dclient.APIClient) { - // Verify ipvlan l2 mode sets the proper default gateway routes via netlink - // for either an explicitly set route by the user or inferred via default IPAM +// Verify ipvlan l2 mode sets the proper default gateway routes via netlink +// for either an explicitly set route by the user or inferred via default IPAM +func testIpvlanL2Addressing(t *testing.T, ctx context.Context, client dclient.APIClient) { + const parentIfName = "di-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) + netNameL2 := "dualstackl2" net.CreateNoError(ctx, t, client, netNameL2, - net.WithIPvlan("", "l2"), + net.WithIPvlan(parentIfName, "l2"), net.WithIPv6(), net.WithIPAM("172.28.140.0/24", "172.28.140.254"), net.WithIPAM("2001:db8:abcb::/64", ""), ) assert.Check(t, n.IsNetworkAvailable(ctx, client, netNameL2)) - id1 := container.Run(ctx, t, client, + id := container.Run(ctx, t, client, container.WithNetworkMode(netNameL2), ) // Validate ipvlan l2 mode defaults gateway sets the default IPAM next-hop inferred from the subnet - result, err := container.Exec(ctx, client, id1, []string{"ip", "route"}) + result, err := container.Exec(ctx, client, id, []string{"ip", "route"}) assert.NilError(t, err) - assert.Check(t, strings.Contains(result.Combined(), "default via 172.28.140.254 dev eth0")) + assert.Check(t, is.Contains(result.Combined(), "default via 172.28.140.254 dev eth0")) // Validate ipvlan l2 mode sets the v6 gateway to the user specified default gateway/next-hop - result, err = container.Exec(ctx, client, id1, []string{"ip", "-6", "route"}) + result, err = container.Exec(ctx, client, id, []string{"ip", "-6", "route"}) assert.NilError(t, err) - assert.Check(t, strings.Contains(result.Combined(), "default via 2001:db8:abcb::1 dev eth0")) + assert.Check(t, is.Contains(result.Combined(), "default via 2001:db8:abcb::1 dev eth0")) +} + +// Validate ipvlan l3 mode sets the v4 gateway to dev eth0 and disregards any explicit or inferred next-hops +func testIpvlanL3Addressing(t *testing.T, ctx context.Context, client dclient.APIClient) { + const parentIfName = "di-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) - // Validate ipvlan l3 mode sets the v4 gateway to dev eth0 and disregards any explicit or inferred next-hops netNameL3 := "dualstackl3" net.CreateNoError(ctx, t, client, netNameL3, - net.WithIPvlan("", "l3"), + net.WithIPvlan(parentIfName, "l3"), net.WithIPv6(), net.WithIPAM("172.28.160.0/24", "172.28.160.254"), net.WithIPAM("2001:db8:abcd::/64", "2001:db8:abcd::254"), ) assert.Check(t, n.IsNetworkAvailable(ctx, client, netNameL3)) - id2 := container.Run(ctx, t, client, + id := container.Run(ctx, t, client, container.WithNetworkMode(netNameL3), ) // Validate ipvlan l3 mode sets the v4 gateway to dev eth0 and disregards any explicit or inferred next-hops - result, err = container.Exec(ctx, client, id2, []string{"ip", "route"}) + result, err := container.Exec(ctx, client, id, []string{"ip", "route"}) assert.NilError(t, err) - assert.Check(t, strings.Contains(result.Combined(), "default dev eth0")) + assert.Check(t, is.Contains(result.Combined(), "default dev eth0")) // Validate ipvlan l3 mode sets the v6 gateway to dev eth0 and disregards any explicit or inferred next-hops - result, err = container.Exec(ctx, client, id2, []string{"ip", "-6", "route"}) + result, err = container.Exec(ctx, client, id, []string{"ip", "-6", "route"}) assert.NilError(t, err) - assert.Check(t, strings.Contains(result.Combined(), "default dev eth0")) + assert.Check(t, is.Contains(result.Combined(), "default dev eth0")) } var ( @@ -420,3 +464,85 @@ func ipvlanKernelSupport(t *testing.T) bool { return ipvlanSupported } + +// TestIPVlanDNS checks whether DNS is forwarded, for combinations of l2/l3 mode, +// with/without a parent interface, and with '--internal'. Note that, there's no +// attempt here to give the ipvlan network external connectivity - when this test +// supplies a parent interface, it's a dummy. External DNS lookups only work +// because the daemon is configured to see a host resolver on a loopback +// interface, so the external DNS lookup happens in the host's namespace. The +// test is checking that an automatically configured dummy interface causes the +// network to behave as if it was '--internal'. Regression test for +// https://github.com/moby/moby/issues/47662 +func TestIPVlanDNS(t *testing.T) { + skip.If(t, testEnv.IsRootless, "rootless mode has different view of network") + + ctx := testutil.StartSpan(baseContext, t) + + net.StartDaftDNS(t, "127.0.0.1") + + tmpFileName := net.WriteTempResolvConf(t, "127.0.0.1") + d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName)) + d.StartWithBusybox(ctx, t) + t.Cleanup(func() { d.Stop(t) }) + c := d.NewClientT(t) + + const parentIfName = "di-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) + + const netName = "ipvlan-dns-net" + + testcases := []struct { + name string + parent string + internal bool + expDNS bool + }{ + { + name: "with parent", + parent: parentIfName, + // External DNS should be used (even though the network has no external connectivity). + expDNS: true, + }, + { + name: "no parent", + // External DNS should not be used, equivalent to '--internal'. + }, + { + name: "with parent, internal", + parent: parentIfName, + internal: true, + // External DNS should not be used. + }, + } + + for _, mode := range []string{"l2", "l3"} { + for _, tc := range testcases { + name := fmt.Sprintf("Mode=%v/HasParent=%v/Internal=%v", mode, tc.parent != "", tc.internal) + t.Run(name, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + createOpts := []func(*types.NetworkCreate){ + net.WithIPvlan(tc.parent, mode), + } + if tc.internal { + createOpts = append(createOpts, net.WithInternal()) + } + net.CreateNoError(ctx, t, c, netName, createOpts...) + defer c.NetworkRemove(ctx, netName) + + ctrId := container.Run(ctx, t, c, container.WithNetworkMode(netName)) + defer c.ContainerRemove(ctx, ctrId, containertypes.RemoveOptions{Force: true}) + res, err := container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"}) + assert.NilError(t, err) + if tc.expDNS { + assert.Check(t, is.Equal(res.ExitCode, 0)) + assert.Check(t, is.Contains(res.Stdout(), net.DNSRespAddr)) + } else { + assert.Check(t, is.Equal(res.ExitCode, 1)) + assert.Check(t, is.Contains(res.Stdout(), "SERVFAIL")) + } + }) + } + } +} diff --git a/libnetwork/drivers/ipvlan/ipvlan_joinleave.go b/libnetwork/drivers/ipvlan/ipvlan_joinleave.go index f00e279969369..59e27bbbf5dea 100644 --- a/libnetwork/drivers/ipvlan/ipvlan_joinleave.go +++ b/libnetwork/drivers/ipvlan/ipvlan_joinleave.go @@ -122,6 +122,10 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, log.G(context.TODO()).Debugf("Ipvlan Endpoint Joined with IPv6_Addr: %s IpVlan_Mode: %s, Parent: %s", ep.addrv6.IP.String(), n.config.IpvlanMode, n.config.Parent) } + // If n.config.Internal was set locally by the driver because there's no parent + // interface, libnetwork doesn't know the network is internal. So, stop it from + // adding a gateway endpoint. + jinfo.DisableGatewayService() } iNames := jinfo.InterfaceName() err = iNames.SetNames(vethName, containerVethPrefix) diff --git a/libnetwork/drivers/ipvlan/ipvlan_network.go b/libnetwork/drivers/ipvlan/ipvlan_network.go index 11bde5a3fd2dc..0ef0d12469ee4 100644 --- a/libnetwork/drivers/ipvlan/ipvlan_network.go +++ b/libnetwork/drivers/ipvlan/ipvlan_network.go @@ -41,6 +41,7 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // if parent interface not specified, create a dummy type link to use named dummy+net_id if config.Parent == "" { config.Parent = getDummyName(stringid.TruncateID(config.ID)) + config.Internal = true } foundExisting, err := d.createNetwork(config) if err != nil { diff --git a/libnetwork/endpoint.go b/libnetwork/endpoint.go index 9c1b7eee15e80..f0f92e28d495f 100644 --- a/libnetwork/endpoint.go +++ b/libnetwork/endpoint.go @@ -569,12 +569,12 @@ func (ep *Endpoint) sbJoin(sb *Sandbox, options ...EndpointOption) (err error) { return sb.setupDefaultGW() } - currentExtEp := sb.getGatewayEndpoint() // Enable upstream forwarding if the sandbox gained external connectivity. if sb.resolver != nil { - sb.resolver.SetForwardingPolicy(currentExtEp != nil) + sb.resolver.SetForwardingPolicy(sb.hasExternalAccess()) } + currentExtEp := sb.getGatewayEndpoint() moveExtConn := currentExtEp != extEp if moveExtConn { if extEp != nil { @@ -767,13 +767,13 @@ func (ep *Endpoint) sbLeave(sb *Sandbox, force bool) error { return sb.setupDefaultGW() } - // New endpoint providing external connectivity for the sandbox - extEp = sb.getGatewayEndpoint() // Disable upstream forwarding if the sandbox lost external connectivity. if sb.resolver != nil { - sb.resolver.SetForwardingPolicy(extEp != nil) + sb.resolver.SetForwardingPolicy(sb.hasExternalAccess()) } + // New endpoint providing external connectivity for the sandbox + extEp = sb.getGatewayEndpoint() if moveExtConn && extEp != nil { log.G(context.TODO()).Debugf("Programming external connectivity on endpoint %s (%s)", extEp.Name(), extEp.ID()) extN, err := extEp.getNetworkFromStore() diff --git a/libnetwork/endpoint_info.go b/libnetwork/endpoint_info.go index 3938b41342082..31b421d2cff1a 100644 --- a/libnetwork/endpoint_info.go +++ b/libnetwork/endpoint_info.go @@ -382,6 +382,31 @@ func (ep *Endpoint) SetGatewayIPv6(gw6 net.IP) error { return nil } +// hasGatewayOrDefaultRoute returns true if ep has a gateway, or a route to '0.0.0.0'/'::'. +func (ep *Endpoint) hasGatewayOrDefaultRoute() bool { + ep.mu.Lock() + defer ep.mu.Unlock() + + if ep.joinInfo != nil { + if len(ep.joinInfo.gw) > 0 { + return true + } + for _, route := range ep.joinInfo.StaticRoutes { + if route.Destination.IP.IsUnspecified() && net.IP(route.Destination.Mask).IsUnspecified() { + return true + } + } + } + if ep.iface != nil { + for _, route := range ep.iface.routes { + if route.IP.IsUnspecified() && net.IP(route.Mask).IsUnspecified() { + return true + } + } + } + return false +} + func (ep *Endpoint) retrieveFromStore() (*Endpoint, error) { n, err := ep.getNetworkFromStore() if err != nil { diff --git a/libnetwork/sandbox.go b/libnetwork/sandbox.go index b7c242ec21d75..94c460e0eaad8 100644 --- a/libnetwork/sandbox.go +++ b/libnetwork/sandbox.go @@ -507,6 +507,21 @@ func (sb *Sandbox) resolveName(ctx context.Context, nameOrAlias string, networkN return nil, ipv6Miss } +// hasExternalAccess returns true if any of sb's Endpoints appear to have external +// network access. +func (sb *Sandbox) hasExternalAccess() bool { + for _, ep := range sb.Endpoints() { + nw := ep.getNetwork() + if nw.Internal() || nw.Type() == "null" || nw.Type() == "host" { + continue + } + if ep.hasGatewayOrDefaultRoute() { + return true + } + } + return false +} + // EnableService makes a managed container's service available by adding the // endpoint to the service load balancer and service discovery. func (sb *Sandbox) EnableService() (err error) { diff --git a/libnetwork/sandbox_dns_unix.go b/libnetwork/sandbox_dns_unix.go index 2dea55a9b09eb..72a2e4b523f35 100644 --- a/libnetwork/sandbox_dns_unix.go +++ b/libnetwork/sandbox_dns_unix.go @@ -48,7 +48,7 @@ func (sb *Sandbox) startResolver(restore bool) { // 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.getGatewayEndpoint() != nil, sb) + sb.resolver = NewResolver(resolverIPSandbox, sb.hasExternalAccess(), sb) defer func() { if err != nil { sb.resolver = nil From cd7240f6d94359b873fd8f7db02495a8322c0b2d Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 3 Apr 2024 19:06:46 +0100 Subject: [PATCH 3/4] Stop macvlan with no parent from using ext-dns We document that an macvlan network with no parent interface is equivalent to a '--internal' network. But, in this case, an macvlan network was still configured with a gateway. So, DNS proxying would be enabled in the internal resolver (and, if the host's resolver was on a localhost address, requests to external resolvers from the host's network namespace would succeed). This change disables configuration of a gateway for a macvlan Endpoint if no parent interface is specified. (Note if a parent interface with no external network is supplied as '-o parent=', the gateway will still be set up. Documentation will need to be updated to note that '--internal' should be used to prevent DNS request forwarding in this case.) Signed-off-by: Rob Murray --- integration/network/macvlan/macvlan_test.go | 148 +++++++++++++++--- .../drivers/macvlan/macvlan_joinleave.go | 4 + libnetwork/drivers/macvlan/macvlan_network.go | 1 + 3 files changed, 135 insertions(+), 18 deletions(-) diff --git a/integration/network/macvlan/macvlan_test.go b/integration/network/macvlan/macvlan_test.go index 5b0a174eecce5..ee510124f4f0b 100644 --- a/integration/network/macvlan/macvlan_test.go +++ b/integration/network/macvlan/macvlan_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" + "github.com/docker/docker/api/types" + containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" net "github.com/docker/docker/integration/internal/network" @@ -14,6 +16,7 @@ import ( "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" ) @@ -66,8 +69,11 @@ func TestDockerNetworkMacvlan(t *testing.T) { name: "InternalMode", test: testMacvlanInternalMode, }, { - name: "MultiSubnet", - test: testMacvlanMultiSubnet, + name: "MultiSubnetWithParent", + test: testMacvlanMultiSubnetWithParent, + }, { + name: "MultiSubnetNoParent", + test: testMacvlanMultiSubnetNoParent, }, { name: "Addressing", test: testMacvlanAddressing, @@ -173,10 +179,21 @@ func testMacvlanInternalMode(t *testing.T, ctx context.Context, client client.AP assert.Check(t, err == nil) } -func testMacvlanMultiSubnet(t *testing.T, ctx context.Context, client client.APIClient) { +func testMacvlanMultiSubnetWithParent(t *testing.T, ctx context.Context, client client.APIClient) { + const parentIfName = "dm-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) + testMacvlanMultiSubnet(t, ctx, client, parentIfName) +} + +func testMacvlanMultiSubnetNoParent(t *testing.T, ctx context.Context, client client.APIClient) { + testMacvlanMultiSubnet(t, ctx, client, "") +} + +func testMacvlanMultiSubnet(t *testing.T, ctx context.Context, client client.APIClient, parent string) { netName := "dualstackbridge" net.CreateNoError(ctx, t, client, netName, - net.WithMacvlan(""), + net.WithMacvlan(parent), net.WithIPv6(), net.WithIPAM("172.28.100.0/24", ""), net.WithIPAM("172.28.102.0/24", "172.28.102.254"), @@ -199,11 +216,22 @@ func testMacvlanMultiSubnet(t *testing.T, ctx context.Context, client client.API ) c1, err := client.ContainerInspect(ctx, id1) assert.NilError(t, err) + if parent == "" { + // Inspect the v4 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks["dualstackbridge"].Gateway, "")) + // Inspect the v6 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks["dualstackbridge"].IPv6Gateway, "")) + } else { + // Inspect the v4 gateway to ensure the proper default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks["dualstackbridge"].Gateway, "172.28.100.1")) + // Inspect the v6 gateway to ensure the proper default GW was assigned + assert.Check(t, is.Equal(c1.NetworkSettings.Networks["dualstackbridge"].IPv6Gateway, "2001:db8:abc2::1")) + } - // verify ipv4 connectivity to the explicit --ipv address second to first + // verify ipv4 connectivity to the explicit --ip address second to first _, err = container.Exec(ctx, client, id2, []string{"ping", "-c", "1", c1.NetworkSettings.Networks["dualstackbridge"].IPAddress}) assert.NilError(t, err) - // verify ipv6 connectivity to the explicit --ipv6 address second to first + // verify ipv6 connectivity to the explicit --ip6 address second to first _, err = container.Exec(ctx, client, id2, []string{"ping6", "-c", "1", c1.NetworkSettings.Networks["dualstackbridge"].GlobalIPv6Address}) assert.NilError(t, err) @@ -220,29 +248,35 @@ func testMacvlanMultiSubnet(t *testing.T, ctx context.Context, client client.API ) c3, err := client.ContainerInspect(ctx, id3) assert.NilError(t, err) + if parent == "" { + // Inspect the v4 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks["dualstackbridge"].Gateway, "")) + // Inspect the v6 gateway to ensure no default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks["dualstackbridge"].IPv6Gateway, "")) + } else { + // Inspect the v4 gateway to ensure the proper explicitly assigned default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks["dualstackbridge"].Gateway, "172.28.102.254")) + // Inspect the v6 gateway to ensure the proper explicitly assigned default GW was assigned + assert.Check(t, is.Equal(c3.NetworkSettings.Networks["dualstackbridge"].IPv6Gateway, "2001:db8:abc4::254")) + } - // verify ipv4 connectivity to the explicit --ipv address from third to fourth + // verify ipv4 connectivity to the explicit --ip address from third to fourth _, err = container.Exec(ctx, client, id4, []string{"ping", "-c", "1", c3.NetworkSettings.Networks["dualstackbridge"].IPAddress}) assert.NilError(t, err) - // verify ipv6 connectivity to the explicit --ipv6 address from third to fourth + // verify ipv6 connectivity to the explicit --ip6 address from third to fourth _, err = container.Exec(ctx, client, id4, []string{"ping6", "-c", "1", c3.NetworkSettings.Networks["dualstackbridge"].GlobalIPv6Address}) assert.NilError(t, err) - - // Inspect the v4 gateway to ensure the proper default GW was assigned - assert.Equal(t, c1.NetworkSettings.Networks["dualstackbridge"].Gateway, "172.28.100.1") - // Inspect the v6 gateway to ensure the proper default GW was assigned - assert.Equal(t, c1.NetworkSettings.Networks["dualstackbridge"].IPv6Gateway, "2001:db8:abc2::1") - // Inspect the v4 gateway to ensure the proper explicitly assigned default GW was assigned - assert.Equal(t, c3.NetworkSettings.Networks["dualstackbridge"].Gateway, "172.28.102.254") - // Inspect the v6 gateway to ensure the proper explicitly assigned default GW was assigned - assert.Equal(t, c3.NetworkSettings.Networks["dualstackbridge"].IPv6Gateway, "2001:db8:abc4::254") } func testMacvlanAddressing(t *testing.T, ctx context.Context, client client.APIClient) { + const parentIfName = "dm-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) + // Ensure the default gateways, next-hops and default dev devices are properly set netName := "dualstackbridge" net.CreateNoError(ctx, t, client, netName, - net.WithMacvlan(""), + net.WithMacvlan(parentIfName), net.WithIPv6(), net.WithOption("macvlan_mode", "bridge"), net.WithIPAM("172.28.130.0/24", ""), @@ -263,3 +297,81 @@ func testMacvlanAddressing(t *testing.T, ctx context.Context, client client.APIC assert.NilError(t, err) assert.Check(t, strings.Contains(result.Combined(), "default via 2001:db8:abca::254 dev eth0")) } + +// TestMACVlanDNS checks whether DNS is forwarded, with/without a parent +// interface, and with '--internal'. Note that there's no attempt here to give +// the macvlan network external connectivity - when this test supplies a parent +// interface, it's a dummy. External DNS lookups only work because the daemon is +// configured to see a host resolver on a loopback interface, so the external DNS +// lookup happens in the host's namespace. The test is checking that an +// automatically configured dummy interface causes the network to behave as if it +// was '--internal'. +func TestMACVlanDNS(t *testing.T) { + skip.If(t, testEnv.IsRootless, "rootless mode has different view of network") + + ctx := testutil.StartSpan(baseContext, t) + + net.StartDaftDNS(t, "127.0.0.1") + + tmpFileName := net.WriteTempResolvConf(t, "127.0.0.1") + d := daemon.New(t, daemon.WithEnvVars("DOCKER_TEST_RESOLV_CONF_PATH="+tmpFileName)) + d.StartWithBusybox(ctx, t) + t.Cleanup(func() { d.Stop(t) }) + c := d.NewClientT(t) + + const parentIfName = "dm-dummy0" + n.CreateMasterDummy(ctx, t, parentIfName) + defer n.DeleteInterface(ctx, t, parentIfName) + + const netName = "macvlan-dns-net" + + testcases := []struct { + name string + parent string + internal bool + expDNS bool + }{ + { + name: "with parent", + parent: parentIfName, + // External DNS should be used (even though the network has no external connectivity). + expDNS: true, + }, + { + name: "no parent", + // External DNS should not be used, equivalent to '--internal'. + }, + { + name: "with parent, internal", + parent: parentIfName, + internal: true, + expDNS: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + createOpts := []func(*types.NetworkCreate){ + net.WithMacvlan(tc.parent), + } + if tc.internal { + createOpts = append(createOpts, net.WithInternal()) + } + net.CreateNoError(ctx, t, c, netName, createOpts...) + defer c.NetworkRemove(ctx, netName) + + ctrId := container.Run(ctx, t, c, container.WithNetworkMode(netName)) + defer c.ContainerRemove(ctx, ctrId, containertypes.RemoveOptions{Force: true}) + res, err := container.Exec(ctx, c, ctrId, []string{"nslookup", "test.example"}) + assert.NilError(t, err) + if tc.expDNS { + assert.Check(t, is.Equal(res.ExitCode, 0)) + assert.Check(t, is.Contains(res.Stdout(), net.DNSRespAddr)) + } else { + assert.Check(t, is.Equal(res.ExitCode, 1)) + assert.Check(t, is.Contains(res.Stdout(), "SERVFAIL")) + } + }) + } +} diff --git a/libnetwork/drivers/macvlan/macvlan_joinleave.go b/libnetwork/drivers/macvlan/macvlan_joinleave.go index ed32ddc739242..95a35bbe03a7b 100644 --- a/libnetwork/drivers/macvlan/macvlan_joinleave.go +++ b/libnetwork/drivers/macvlan/macvlan_joinleave.go @@ -83,6 +83,10 @@ func (d *driver) Join(nid, eid string, sboxKey string, jinfo driverapi.JoinInfo, log.G(context.TODO()).Debugf("Macvlan Endpoint Joined with IPv6_Addr: %s MacVlan_Mode: %s, Parent: %s", ep.addrv6.IP.String(), n.config.MacvlanMode, n.config.Parent) } + // If n.config.Internal was set locally by the driver because there's no parent + // interface, libnetwork doesn't know the network is internal. So, stop it from + // adding a gateway endpoint. + jinfo.DisableGatewayService() } iNames := jinfo.InterfaceName() err = iNames.SetNames(vethName, containerVethPrefix) diff --git a/libnetwork/drivers/macvlan/macvlan_network.go b/libnetwork/drivers/macvlan/macvlan_network.go index 77aa323ecba94..75313dd834563 100644 --- a/libnetwork/drivers/macvlan/macvlan_network.go +++ b/libnetwork/drivers/macvlan/macvlan_network.go @@ -31,6 +31,7 @@ func (d *driver) CreateNetwork(nid string, option map[string]interface{}, nInfo // if parent interface not specified, create a dummy type link to use named dummy+net_id if config.Parent == "" { config.Parent = getDummyName(stringid.TruncateID(config.ID)) + config.Internal = true } foundExisting, err := d.createNetwork(config) if err != nil { From 9954d7c6bd2023a85ca3e5daa9aafb4a6adbce93 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Thu, 4 Apr 2024 11:45:18 +0100 Subject: [PATCH 4/4] Run ipvlan tests even if 'modprobe ipvlan' fails This reverts commit a77e147d322c153ae1c2ae0ee45c1835c109e231. The ipvlan integration tests have been skipped in CI because of a check intended to ensure the kernel has ipvlan support - which failed, but seems to be unnecessary (probably because kernels have moved on). Signed-off-by: Rob Murray --- integration/network/ipvlan/ipvlan_test.go | 28 ++--------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/integration/network/ipvlan/ipvlan_test.go b/integration/network/ipvlan/ipvlan_test.go index 49e1f3155f0d8..e4d4aad04b1b1 100644 --- a/integration/network/ipvlan/ipvlan_test.go +++ b/integration/network/ipvlan/ipvlan_test.go @@ -5,10 +5,7 @@ package ipvlan // import "github.com/docker/docker/integration/network/ipvlan" import ( "context" "fmt" - "os" - "os/exec" "strings" - "sync" "testing" "github.com/docker/docker/api/types" @@ -27,7 +24,7 @@ import ( func TestDockerNetworkIpvlanPersistance(t *testing.T) { // verify the driver automatically provisions the 802.1q link (di-dummy0.70) skip.If(t, testEnv.IsRemoteDaemon) - skip.If(t, !ipvlanKernelSupport(t), "Kernel doesn't support ipvlan") + skip.If(t, testEnv.IsRootless, "rootless mode has different view of network") ctx := testutil.StartSpan(baseContext, t) @@ -56,7 +53,7 @@ func TestDockerNetworkIpvlanPersistance(t *testing.T) { func TestDockerNetworkIpvlan(t *testing.T) { skip.If(t, testEnv.IsRemoteDaemon) - skip.If(t, !ipvlanKernelSupport(t), "Kernel doesn't support ipvlan") + skip.If(t, testEnv.IsRootless, "rootless mode has different view of network") ctx := testutil.StartSpan(baseContext, t) @@ -444,27 +441,6 @@ func testIpvlanL3Addressing(t *testing.T, ctx context.Context, client dclient.AP assert.Check(t, is.Contains(result.Combined(), "default dev eth0")) } -var ( - once sync.Once - ipvlanSupported bool -) - -// figure out if ipvlan is supported by the kernel -func ipvlanKernelSupport(t *testing.T) bool { - once.Do(func() { - // this may have the side effect of enabling the ipvlan module - exec.Command("modprobe", "ipvlan").Run() - _, err := os.Stat("/sys/module/ipvlan") - if err == nil { - ipvlanSupported = true - } else if !os.IsNotExist(err) { - t.Logf("WARNING: ipvlanKernelSupport: stat failed: %v\n", err) - } - }) - - return ipvlanSupported -} - // TestIPVlanDNS checks whether DNS is forwarded, for combinations of l2/l3 mode, // with/without a parent interface, and with '--internal'. Note that, there's no // attempt here to give the ipvlan network external connectivity - when this test