From 4d8cff7bd496a01da4dfbc5df5c678cac3c5cfbd Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Tue, 4 Mar 2025 16:00:35 +0000 Subject: [PATCH] Don't skip DNAT for a routed network without userland-proxy If the userland-proxy is running, packets from one bridge network addressed to the host port are not DNAT'd - so that docker-proxy can pick them up, and therefore the packet bypasses the network isolation rules. Without the userland-proxy, there's no way for a packet from one bridge network to bypass the network isolation rules. So, in this case, DNAT is not skipped - and that at-least allows packets originating from the network that published the port to access the host port. Commit 0546d90 improved support for routed mode networks (allowing nat-mode networks access to containers in routed-mode networks, as well as just remote access). That commit changed the "SKIP DNAT" logic, making sure DNAT was skipped for a routed-mode network if the userland-proxy was enabled (so, containers in routed mode networks could access ports published by other networks). But, it still skipped DNAT for a routed mode network if the userland proxy was disabled - packets from the routed mode network aimed at any other network would be dropped by the network isolation rules anyway, and containers in a routed mode network don't need access to ports published from that network (because, by definition, there can't be any). However, network isolation rules can be worked-around with a rule in the DOCKER-USER chain, but the SKIP DNAT rule is harder to deal with. So, for routed-mode, only skip DNAT if the userland-proxy is enabled (just like nat-mode networks). Signed-off-by: Rob Murray --- integration/networking/bridge_linux_test.go | 119 ++++++++++++++++++ .../drivers/bridge/setup_ip_tables_linux.go | 35 ++++-- 2 files changed, 143 insertions(+), 11 deletions(-) diff --git a/integration/networking/bridge_linux_test.go b/integration/networking/bridge_linux_test.go index 34f6f26c0f..0462db4c01 100644 --- a/integration/networking/bridge_linux_test.go +++ b/integration/networking/bridge_linux_test.go @@ -494,6 +494,125 @@ func TestBridgeINCRouted(t *testing.T) { } } +// TestRoutedAccessToPublishedPort checks that: +// - with docker-proxy enabled, a container in a gw-mode=routed network can access a port +// published to the host by a container in a gw-mode=nat network. +// - if the proxy is disabled, those packets are dropped by the network isolation rules +// - working around those INC rules by adding a rule to DOCKER-USER enables access to the +// published port (so, packets from the mode-routed network are still DNAT'd). +// +// Regression test for https://github.com/moby/moby/issues/49509 +func TestRoutedAccessToPublishedPort(t *testing.T) { + skip.If(t, testEnv.IsRootless, "Published port not accessible from rootless netns") + + ctx := setupTest(t) + + testcases := []struct { + name string + userlandProxy bool + skipINC bool + expResponse bool + }{ + { + name: "proxy=true/skipICC=false", + userlandProxy: true, + expResponse: true, + }, + { + name: "proxy=false/skipICC=false", + }, + { + name: "proxy=false/skipICC=true", + skipINC: true, + expResponse: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + d := daemon.New(t) + d.StartWithBusybox(ctx, t, "--ipv6", "--userland-proxy="+strconv.FormatBool(tc.userlandProxy)) + defer d.Stop(t) + + c := d.NewClientT(t) + defer c.Close() + + const natNetName = "tnet-nat" + const natBridgeName = "br-nat" + network.CreateNoError(ctx, t, c, natNetName, + network.WithDriver("bridge"), + network.WithIPv6(), + network.WithOption(bridge.BridgeName, natBridgeName), + ) + defer network.RemoveNoError(ctx, t, c, natNetName) + + ctrId := container.Run(ctx, t, c, + container.WithNetworkMode(natNetName), + container.WithName("ctr-nat"), + container.WithExposedPorts("80/tcp"), + container.WithPortMap(nat.PortMap{"80/tcp": {nat.PortBinding{HostPort: "8080"}}}), + container.WithCmd("httpd", "-f"), + ) + defer c.ContainerRemove(ctx, ctrId, containertypes.RemoveOptions{Force: true}) + + const routedNetName = "tnet-routed" + network.CreateNoError(ctx, t, c, routedNetName, + network.WithDriver("bridge"), + network.WithIPv6(), + network.WithOption(bridge.BridgeName, "br-routed"), + network.WithOption(bridge.IPv4GatewayMode, "routed"), + network.WithOption(bridge.IPv6GatewayMode, "routed"), + ) + defer network.RemoveNoError(ctx, t, c, routedNetName) + + // With docker-proxy disabled, a container can't normally access a port published + // from a container in a different bridge network. But, users can add rules to + // the DOCKER-USER chain to get around that limitation of docker's iptables rules. + // Do that here, if the test requires it. + if tc.skipINC { + for _, ipv := range []iptables.IPVersion{iptables.IPv4, iptables.IPv6} { + rule := iptables.Rule{ + IPVer: ipv, Table: iptables.Filter, Chain: "DOCKER-USER", + Args: []string{"-o", natBridgeName, "-j", "ACCEPT"}, + } + err := rule.Insert() + assert.NilError(t, err) + defer func() { + if err := rule.Delete(); err != nil { + t.Errorf("Failed to delete %s DOCKER-USER rule: %v", ipv, err) + } + }() + } + } + + // Use the default bridge addresses as host addresses (like "host-gateway", but + // there's no way to tell wget to prefer ipv4/ipv6 transport, so just use the + // addresses directly). + insp, err := c.NetworkInspect(ctx, "bridge", networktypes.InspectOptions{}) + assert.NilError(t, err) + for _, ipamCfg := range insp.IPAM.Config { + ipv := "ipv4" + if strings.Contains(ipamCfg.Gateway, ":") { + ipv = "ipv6" + } + t.Run(ipv, func(t *testing.T) { + url := "http://" + net.JoinHostPort(ipamCfg.Gateway, "8080") + res := container.RunAttach(ctx, t, c, + container.WithNetworkMode(routedNetName), + container.WithCmd("wget", "-O-", "-T3", url), + ) + if tc.expResponse { + // 404 Not Found means the server responded, but it's got nothing to serve. + assert.Check(t, is.Contains(res.Stderr.String(), "404 Not Found"), "url: %s", url) + } else { + assert.Check(t, is.Contains(res.Stderr.String(), "download timed out"), "url: %s", url) + } + }) + } + }) + } +} + func TestDefaultBridgeIPv6(t *testing.T) { ctx := setupTest(t) diff --git a/libnetwork/drivers/bridge/setup_ip_tables_linux.go b/libnetwork/drivers/bridge/setup_ip_tables_linux.go index cdc661b0ac..4803f4ef53 100644 --- a/libnetwork/drivers/bridge/setup_ip_tables_linux.go +++ b/libnetwork/drivers/bridge/setup_ip_tables_linux.go @@ -457,18 +457,31 @@ func setupNonInternalNetworkRules(ipVer iptables.IPVersion, config *networkConfi hpNatRule := iptables.Rule{IPVer: ipVer, Table: iptables.Nat, Chain: "POSTROUTING", Args: hpNatArgs} // Set NAT. - if nat && config.EnableIPMasquerade { - if err := programChainRule(natRule, "NAT", enable); err != nil { - return err + if config.EnableIPMasquerade { + if nat { + if err := programChainRule(natRule, "NAT", enable); err != nil { + return err + } } - } - if !nat || (config.EnableIPMasquerade && !hairpin) { - skipDNAT := iptables.Rule{IPVer: ipVer, Table: iptables.Nat, Chain: DockerChain, Args: []string{ - "-i", config.BridgeName, - "-j", "RETURN", - }} - if err := programChainRule(skipDNAT, "SKIP DNAT", enable); err != nil { - return err + // If the userland proxy is running (!hairpin), skip DNAT for packets originating from + // this new network. Then, the proxy can pick up the packet from the host address the dest + // port is published to. Otherwise, if the packet is DNAT'd, it's forwarded straight to the + // target network, and will be dropped by network isolation rules if it didn't originate in + // the same bridge network. (So, with the proxy enabled, this skip allows a container in one + // network to reach a port published by a container in another bridge network.) + // + // If the userland proxy is disabled, don't skip, so packets will be DNAT'd. That will + // enable access to ports published by containers in the same network. But, the INC rules + // will block access to that published port from containers in other networks. (However, + // users may add a rule to DOCKER-USER to work around the INC rules if needed.) + if !hairpin { + skipDNAT := iptables.Rule{IPVer: ipVer, Table: iptables.Nat, Chain: DockerChain, Args: []string{ + "-i", config.BridgeName, + "-j", "RETURN", + }} + if err := programChainRule(skipDNAT, "SKIP DNAT", enable); err != nil { + return err + } } }