Avoid Egress's SNAT rules being applied to unintended traffic (antrea…

…-io#2047) Local out packets destined for Pods have "0x1/0x1" mark. Add a condition that restricts the out interface to Egress's SNAT rule to filter out those packets.
antoninbas · Apr 8, 2021 · 3643815 · 3643815
1 parent eb21c41
commit 3643815
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 5 deletions.
diff --git a/pkg/agent/route/route_linux.go b/pkg/agent/route/route_linux.go
@@ -360,6 +360,7 @@ func (c *Client) restoreIptablesData(podCIDR *net.IPNet, podIPSet string, snatMa
 		writeLine(iptablesData, []string{
 			"-A", antreaPostRoutingChain,
 			"-m", "comment", "--comment", `"Antrea: SNAT Pod to external packets"`,
+			"!", "-o", c.nodeConfig.GatewayConfig.Name,
 			"-m", "mark", "--mark", fmt.Sprintf("%#08x/%#08x", snatMark, types.SNATIPMarkMask),
 			"-j", iptables.SNATTarget, "--to", snatIP.String(),
 		}...)
@@ -683,9 +684,12 @@ func (c *Client) UnMigrateRoutesFromGw(route *net.IPNet, linkName string) error
 	return nil
 }
 
-func snatRuleSpec(snatIP net.IP, snatMark uint32) []string {
+func (c *Client) snatRuleSpec(snatIP net.IP, snatMark uint32) []string {
 	return []string{
 		"-m", "comment", "--comment", "Antrea: SNAT Pod to external packets",
+		// The condition is needed to prevent the rule from being applied to local out packets destined for Pods, which
+		// have "0x1/0x1" mark.
+		"!", "-o", c.nodeConfig.GatewayConfig.Name,
 		"-m", "mark", "--mark", fmt.Sprintf("%#08x/%#08x", snatMark, types.SNATIPMarkMask),
 		"-j", iptables.SNATTarget, "--to", snatIP.String(),
 	}
@@ -697,7 +701,7 @@ func (c *Client) AddSNATRule(snatIP net.IP, mark uint32) error {
 		protocol = iptables.ProtocolIPv6
 	}
 	c.markToSNATIP.Store(mark, snatIP)
-	return c.ipt.InsertRule(protocol, iptables.NATTable, antreaPostRoutingChain, snatRuleSpec(snatIP, mark))
+	return c.ipt.InsertRule(protocol, iptables.NATTable, antreaPostRoutingChain, c.snatRuleSpec(snatIP, mark))
 }
 
 func (c *Client) DeleteSNATRule(mark uint32) error {
@@ -708,5 +712,5 @@ func (c *Client) DeleteSNATRule(mark uint32) error {
 	}
 	c.markToSNATIP.Delete(mark)
 	snatIP := value.(net.IP)
-	return c.ipt.DeleteRule(iptables.NATTable, antreaPostRoutingChain, snatRuleSpec(snatIP, mark))
+	return c.ipt.DeleteRule(iptables.NATTable, antreaPostRoutingChain, c.snatRuleSpec(snatIP, mark))
 }
diff --git a/test/integration/agent/route_test.go b/test/integration/agent/route_test.go
@@ -262,7 +262,7 @@ func TestIpTablesSync(t *testing.T) {
 	}{
 		{Table: "raw", Cmd: "-A", Chain: "OUTPUT", RuleSpec: "-m comment --comment \"Antrea: jump to Antrea output rules\" -j ANTREA-OUTPUT"},
 		{Table: "filter", Cmd: "-A", Chain: "ANTREA-FORWARD", RuleSpec: "-i antrea-gw0 -m comment --comment \"Antrea: accept packets from local Pods\" -j ACCEPT"},
-		{Table: "nat", Cmd: "-A", Chain: "ANTREA-POSTROUTING", RuleSpec: fmt.Sprintf("-m comment --comment \"Antrea: SNAT Pod to external packets\" -m mark --mark %#x/0xff -j SNAT --to-source %s", mark, snatIP)},
+		{Table: "nat", Cmd: "-A", Chain: "ANTREA-POSTROUTING", RuleSpec: fmt.Sprintf("! -o antrea-gw0 -m comment --comment \"Antrea: SNAT Pod to external packets\" -m mark --mark %#x/0xff -j SNAT --to-source %s", mark, snatIP)},
 	}
 	// we delete some rules, start the sync goroutine, wait for sync operation to restore them.
 	for _, tc := range tcs {
@@ -306,7 +306,7 @@ func TestAddAndDeleteSNATRule(t *testing.T) {
 
 	snatIP := net.ParseIP("1.1.1.1")
 	mark := uint32(1)
-	expectedRule := fmt.Sprintf("-m comment --comment \"Antrea: SNAT Pod to external packets\" -m mark --mark %#x/0xff -j SNAT --to-source %s", mark, snatIP)
+	expectedRule := fmt.Sprintf("! -o antrea-gw0 -m comment --comment \"Antrea: SNAT Pod to external packets\" -m mark --mark %#x/0xff -j SNAT --to-source %s", mark, snatIP)
 
 	assert.NoError(t, routeClient.AddSNATRule(snatIP, mark))
 	saveCmd := fmt.Sprintf("iptables-save -t nat | grep ANTREA-POSTROUTING")