From cd7240f6d94359b873fd8f7db02495a8322c0b2d Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 3 Apr 2024 19:06:46 +0100 Subject: [PATCH] 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 5b0a174eec..ee510124f4 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 ed32ddc739..95a35bbe03 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 77aa323ecb..75313dd834 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 {