Merge pull request moby#48854 from robmry/12632_noproxy_masquerade

Only masquerade access to own published ports for userland-proxy=false
thaJeztah · Nov 15, 2024 · 4c19680 · 4c19680
2 parents cd8d2c5 + bf251c3
commit 4c19680
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 6 deletions.
diff --git a/integration/network/bridge/iptablesdoc/generated/usernet-portmap-noicc.md b/integration/network/bridge/iptablesdoc/generated/usernet-portmap-noicc.md
@@ -111,7 +111,6 @@ And the corresponding nat table:
     num   pkts bytes target     prot opt in     out     source               destination         
     1        0     0 MASQUERADE  0    --  *      !bridge1  192.0.2.0/24         0.0.0.0/0           
     2        0     0 MASQUERADE  0    --  *      !docker0  172.17.0.0/16        0.0.0.0/0           
-    3        0     0 MASQUERADE  6    --  *      *       192.0.2.2            192.0.2.2            tcp dpt:80
 
     Chain DOCKER (2 references)
     num   pkts bytes target     prot opt in     out     source               destination         
@@ -132,7 +131,6 @@ And the corresponding nat table:
     -A OUTPUT ! -d 127.0.0.0/8 -m addrtype --dst-type LOCAL -j DOCKER
     -A POSTROUTING -s 192.0.2.0/24 ! -o bridge1 -j MASQUERADE
     -A POSTROUTING -s 172.17.0.0/16 ! -o docker0 -j MASQUERADE
-    -A POSTROUTING -s 192.0.2.2/32 -d 192.0.2.2/32 -p tcp -m tcp --dport 80 -j MASQUERADE
     -A DOCKER -i bridge1 -j RETURN
     -A DOCKER -i docker0 -j RETURN
     -A DOCKER ! -i bridge1 -p tcp -m tcp --dport 8080 -j DNAT --to-destination 192.0.2.2:80

diff --git a/integration/network/bridge/iptablesdoc/generated/usernet-portmap-noproxy.md b/integration/network/bridge/iptablesdoc/generated/usernet-portmap-noproxy.md
@@ -138,6 +138,8 @@ Differences from [running with the proxy][0] are:
     [ProgramChain][1].
   - The "SKIP DNAT" RETURN rule for packets routed to the bridge is omitted from
     the DOCKER chain [setupIPTablesInternal][2].
+  - A MASQUERADE rule is added for packets sent from the container to one of its
+    own published ports on the host.
   - A MASQUERADE rule for packets from a LOCAL source address is included in
     POSTROUTING [setupIPTablesInternal][3].
   - In the DOCKER chain's DNAT rule, there's no destination bridge [setPerPortNAT][4].

diff --git a/integration/network/bridge/iptablesdoc/generated/usernet-portmap.md b/integration/network/bridge/iptablesdoc/generated/usernet-portmap.md
@@ -123,7 +123,6 @@ And the corresponding nat table:
     num   pkts bytes target     prot opt in     out     source               destination         
     1        0     0 MASQUERADE  0    --  *      !bridge1  192.0.2.0/24         0.0.0.0/0           
     2        0     0 MASQUERADE  0    --  *      !docker0  172.17.0.0/16        0.0.0.0/0           
-    3        0     0 MASQUERADE  6    --  *      *       192.0.2.2            192.0.2.2            tcp dpt:80
 
     Chain DOCKER (2 references)
     num   pkts bytes target     prot opt in     out     source               destination         
@@ -144,7 +143,6 @@ And the corresponding nat table:
     -A OUTPUT ! -d 127.0.0.0/8 -m addrtype --dst-type LOCAL -j DOCKER
     -A POSTROUTING -s 192.0.2.0/24 ! -o bridge1 -j MASQUERADE
     -A POSTROUTING -s 172.17.0.0/16 ! -o docker0 -j MASQUERADE
-    -A POSTROUTING -s 192.0.2.2/32 -d 192.0.2.2/32 -p tcp -m tcp --dport 80 -j MASQUERADE
     -A DOCKER -i bridge1 -j RETURN
     -A DOCKER -i docker0 -j RETURN
     -A DOCKER ! -i bridge1 -p tcp -m tcp --dport 8080 -j DNAT --to-destination 192.0.2.2:80

diff --git a/integration/network/bridge/iptablesdoc/templates/usernet-portmap-noproxy.md b/integration/network/bridge/iptablesdoc/templates/usernet-portmap-noproxy.md
@@ -36,6 +36,8 @@ Differences from [running with the proxy][0] are:
     [ProgramChain][1].
   - The "SKIP DNAT" RETURN rule for packets routed to the bridge is omitted from
     the DOCKER chain [setupIPTablesInternal][2].
+  - A MASQUERADE rule is added for packets sent from the container to one of its
+    own published ports on the host.
   - A MASQUERADE rule for packets from a LOCAL source address is included in
     POSTROUTING [setupIPTablesInternal][3].
   - In the DOCKER chain's DNAT rule, there's no destination bridge [setPerPortNAT][4].

diff --git a/integration/networking/port_mapping_linux_test.go b/integration/networking/port_mapping_linux_test.go
@@ -459,6 +459,13 @@ func TestAccessPublishedPortFromRemoteHost(t *testing.T) {
 	}
 }
 
+// TestAccessPublishedPortFromCtr checks that a container's published ports can
+// be reached from the container that published the ports, and a neighbouring
+// container on the same network. It runs in three modes:
+//
+// - userland proxy enabled (default behaviour).
+// - proxy disabled (https://github.com/moby/moby/issues/12632)
+// - proxy disabled, 'bridge-nf-call-iptables=0' (https://github.com/moby/moby/issues/48664)
 func TestAccessPublishedPortFromCtr(t *testing.T) {
 	// This test makes changes to the host's "/proc/sys/net/bridge/bridge-nf-call-iptables",
 	// which would have no effect on rootlesskit's netns.
@@ -543,6 +550,12 @@ func TestAccessPublishedPortFromCtr(t *testing.T) {
 			)
 			defer c.ContainerRemove(ctx, res.ContainerID, containertypes.RemoveOptions{Force: true})
 			assert.Check(t, is.Contains(res.Stderr.String(), "404 Not Found"))
+
+			// Also check that the container can reach its own published port.
+			clientCtx2, cancel2 := context.WithTimeout(ctx, 5*time.Second)
+			defer cancel2()
+			execRes := container.ExecT(clientCtx2, t, c, serverId, []string{"wget", "http://" + net.JoinHostPort(hostAddr, hostPort)})
+			assert.Check(t, is.Contains(execRes.Stderr(), "404 Not Found"))
 		})
 	}
 }

diff --git a/libnetwork/drivers/bridge/port_mapping_linux.go b/libnetwork/drivers/bridge/port_mapping_linux.go
@@ -821,7 +821,7 @@ func setPerPortNAT(b portBinding, ipv iptables.IPVersion, proxyPath string, brid
 		"-j", "MASQUERADE",
 	}
 	rule = iptRule{ipv: ipv, table: iptables.Nat, chain: "POSTROUTING", args: args}
-	if err := appendOrDelChainRule(rule, "MASQUERADE", enable); err != nil {
+	if err := appendOrDelChainRule(rule, "MASQUERADE", hairpinMode && enable); err != nil {
 		return err
 	}
 

diff --git a/libnetwork/drivers/bridge/port_mapping_linux_test.go b/libnetwork/drivers/bridge/port_mapping_linux_test.go
@@ -904,7 +904,7 @@ func TestAddPortMappings(t *testing.T) {
 				masqRule := fmt.Sprintf("-s %s -d %s -p %s -m %s --dport %d -j MASQUERADE",
 					addrM, addrM, expPB.Proto, expPB.Proto, expPB.Port)
 				ir := iptRule{ipv: ipv, table: iptables.Nat, chain: "POSTROUTING", args: strings.Split(masqRule, " ")}
-				if disableNAT {
+				if disableNAT || tc.proxyPath != "" {
 					assert.Check(t, !ir.Exists(), fmt.Sprintf("unexpected rule %s", ir))
 				} else {
 					assert.Check(t, ir.Exists(), fmt.Sprintf("expected rule %s", ir))