From 17b863154573d998394be336fe1487827071b019 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 3 Apr 2024 14:54:52 +0100 Subject: [PATCH] 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 37e3dd0b30..49e1f3155f 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 f00e279969..59e27bbbf5 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 11bde5a3fd..0ef0d12469 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 9c1b7eee15..f0f92e28d4 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 3938b41342..31b421d2cf 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 b7c242ec21..94c460e0ea 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 2dea55a9b0..72a2e4b523 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