fix various problems with iptables.Exists

This modifies iptables.Exists so that it must be called with an explicit
table and chain.  This allows us (a) to generate an appropriate command
line for "iptables -C", which was not previously possible, and (b) it
allows us to limit our strings.Contains() search to just the table and
chain in question, preventing erroneous matches against unrelated rules.

Resolves #10781

Signed-off-by: Lars Kellogg-Stedman <lars@redhat.com>

iptables/iptables.go

@@ -21,6 +21,7 @@ const (
 	Insert Action = "-I"
 	Nat    Table  = "nat"
 	Filter Table  = "filter"
+	Mangle Table  = "mangle"
 )
 
 var (
@@ -82,7 +83,7 @@ func NewChain(name, bridge string, table Table) (*Chain, error) {
 		preroute := []string{
 			"-m", "addrtype",
 			"--dst-type", "LOCAL"}
-		if !Exists(preroute...) {
+		if !Exists(Nat, "PREROUTING", preroute...) {
 			if err := c.Prerouting(Append, preroute...); err != nil {
 				return nil, fmt.Errorf("Failed to inject docker in PREROUTING chain: %s", err)
 			}
@@ -91,17 +92,17 @@ func NewChain(name, bridge string, table Table) (*Chain, error) {
 			"-m", "addrtype",
 			"--dst-type", "LOCAL",
 			"!", "--dst", "127.0.0.0/8"}
-		if !Exists(output...) {
+		if !Exists(Nat, "OUTPUT", output...) {
 			if err := c.Output(Append, output...); err != nil {
 				return nil, fmt.Errorf("Failed to inject docker in OUTPUT chain: %s", err)
 			}
 		}
 	case Filter:
-		link := []string{"FORWARD",
+		link := []string{
 			"-o", c.Bridge,
 			"-j", c.Name}
-		if !Exists(link...) {
+		if !Exists(Filter, "FORWARD", link...) {
-			insert := append([]string{string(Insert)}, link...)
+			insert := append([]string{string(Insert), "FORWARD"}, link...)
 			if output, err := Raw(insert...); err != nil {
 				return nil, err
 			} else if len(output) != 0 {
@@ -242,19 +243,25 @@ func (c *Chain) Remove() error {
 }
 
 // Check if a rule exists
-func Exists(args ...string) bool {
+func Exists(table Table, chain string, rule ...string) bool {
+	if string(table) == "" {
+		table = Filter
+	}
+
 	// iptables -C, --check option was added in v.1.4.11
 	// http://ftp.netfilter.org/pub/iptables/changes-iptables-1.4.11.txt
 
 	// try -C
 	// if exit status is 0 then return true, the rule exists
-	if _, err := Raw(append([]string{"-C"}, args...)...); err == nil {
+	if _, err := Raw(append([]string{
+		"-t", string(table), "-C", chain}, rule...)...); err == nil {
 		return true
 	}
 
-	// parse iptables-save for the rule
+	// parse "iptables -S" for the rule (this checks rules in a specific chain
-	rule := strings.Replace(strings.Join(args, " "), "-t nat ", "", -1)
+	// in a specific table)
-	existingRules, _ := exec.Command("iptables-save").Output()
+	rule_string := strings.Join(rule, " ")
+	existingRules, _ := exec.Command("iptables", "-t", string(table), "-S", chain).Output()
 
 	// regex to replace ips in rule
 	// because MASQUERADE rule will not be exactly what was passed
@@ -262,7 +269,7 @@ func Exists(args ...string) bool {
 
 	return strings.Contains(
 		re.ReplaceAllString(string(existingRules), "?"),
-		re.ReplaceAllString(rule, "?"),
+		re.ReplaceAllString(rule_string, "?"),
 	)
 }
 

iptables/iptables_test.go

@@ -39,8 +39,7 @@ func TestForward(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	dnatRule := []string{natChain.Name,
+	dnatRule := []string{
-		"-t", string(natChain.Table),
 		"!", "-i", filterChain.Bridge,
 		"-d", ip.String(),
 		"-p", proto,
@@ -49,12 +48,11 @@ func TestForward(t *testing.T) {
 		"--to-destination", dstAddr + ":" + strconv.Itoa(dstPort),
 	}
 
-	if !Exists(dnatRule...) {
+	if !Exists(natChain.Table, natChain.Name, dnatRule...) {
 		t.Fatalf("DNAT rule does not exist")
 	}
 
-	filterRule := []string{filterChain.Name,
+	filterRule := []string{
-		"-t", string(filterChain.Table),
 		"!", "-i", filterChain.Bridge,
 		"-o", filterChain.Bridge,
 		"-d", dstAddr,
@@ -63,12 +61,11 @@ func TestForward(t *testing.T) {
 		"-j", "ACCEPT",
 	}
 
-	if !Exists(filterRule...) {
+	if !Exists(filterChain.Table, filterChain.Name, filterRule...) {
 		t.Fatalf("filter rule does not exist")
 	}
 
-	masqRule := []string{"POSTROUTING",
+	masqRule := []string{
-		"-t", string(natChain.Table),
 		"-d", dstAddr,
 		"-s", dstAddr,
 		"-p", proto,
@@ -76,7 +73,7 @@ func TestForward(t *testing.T) {
 		"-j", "MASQUERADE",
 	}
 
-	if !Exists(masqRule...) {
+	if !Exists(natChain.Table, "POSTROUTING", masqRule...) {
 		t.Fatalf("MASQUERADE rule does not exist")
 	}
 }
@@ -94,8 +91,7 @@ func TestLink(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	rule1 := []string{filterChain.Name,
+	rule1 := []string{
-		"-t", string(filterChain.Table),
 		"-i", filterChain.Bridge,
 		"-o", filterChain.Bridge,
 		"-p", proto,
@@ -104,12 +100,11 @@ func TestLink(t *testing.T) {
 		"--dport", strconv.Itoa(port),
 		"-j", "ACCEPT"}
 
-	if !Exists(rule1...) {
+	if !Exists(filterChain.Table, filterChain.Name, rule1...) {
 		t.Fatalf("rule1 does not exist")
 	}
 
-	rule2 := []string{filterChain.Name,
+	rule2 := []string{
-		"-t", string(filterChain.Table),
 		"-i", filterChain.Bridge,
 		"-o", filterChain.Bridge,
 		"-p", proto,
@@ -118,7 +113,7 @@ func TestLink(t *testing.T) {
 		"--sport", strconv.Itoa(port),
 		"-j", "ACCEPT"}
 
-	if !Exists(rule2...) {
+	if !Exists(filterChain.Table, filterChain.Name, rule2...) {
 		t.Fatalf("rule2 does not exist")
 	}
 }
@@ -133,17 +128,16 @@ func TestPrerouting(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	rule := []string{"PREROUTING",
+	rule := []string{
-		"-t", string(Nat),
 		"-j", natChain.Name}
 
 	rule = append(rule, args...)
 
-	if !Exists(rule...) {
+	if !Exists(natChain.Table, "PREROUTING", rule...) {
 		t.Fatalf("rule does not exist")
 	}
 
-	delRule := append([]string{"-D"}, rule...)
+	delRule := append([]string{"-D", "PREROUTING", "-t", string(Nat)}, rule...)
 	if _, err = Raw(delRule...); err != nil {
 		t.Fatal(err)
 	}
@@ -159,17 +153,17 @@ func TestOutput(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	rule := []string{"OUTPUT",
+	rule := []string{
-		"-t", string(natChain.Table),
 		"-j", natChain.Name}
 
 	rule = append(rule, args...)
 
-	if !Exists(rule...) {
+	if !Exists(natChain.Table, "OUTPUT", rule...) {
 		t.Fatalf("rule does not exist")
 	}
 
-	delRule := append([]string{"-D"}, rule...)
+	delRule := append([]string{"-D", "OUTPUT", "-t",
+		string(natChain.Table)}, rule...)
 	if _, err = Raw(delRule...); err != nil {
 		t.Fatal(err)
 	}