Merge pull request #925 from aboch/ex

Fix iptables.Exists logic

Author: mavenugo

iptables/iptables.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"os/exec"
+	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -36,6 +37,7 @@ const (
 var (
 	iptablesPath  string
 	supportsXlock = false
+	supportsCOpt  = false
 	// used to lock iptables commands if xtables lock is not supported
 	bestEffortLock sync.Mutex
 	// ErrIptablesNotFound is returned when the rule is not found.
@@ -60,14 +62,19 @@ func (e ChainError) Error() string {
 }
 
 func initCheck() error {
-
 	if iptablesPath == "" {
 		path, err := exec.LookPath("iptables")
 		if err != nil {
 			return ErrIptablesNotFound
 		}
 		iptablesPath = path
 		supportsXlock = exec.Command(iptablesPath, "--wait", "-L", "-n").Run() == nil
+		mj, mn, mc, err := GetVersion()
+		if err != nil {
+			logrus.Warnf("Failed to read iptables version: %v", err)
+			return nil
+		}
+		supportsCOpt = supportsCOption(mj, mn, mc)
 	}
 	return nil
 }
@@ -299,20 +306,19 @@ func Exists(table Table, chain string, rule ...string) bool {
 		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{
-		"-t", string(table), "-C", chain}, rule...)...); err == nil {
-		return true
+	if supportsCOpt {
+		// if exit status is 0 then return true, the rule exists
+		_, err := Raw(append([]string{"-t", string(table), "-C", chain}, rule...)...)
+		return err == nil
 	}
 
-	// parse "iptables -S" for the rule (this checks rules in a specific chain
-	// in a specific table)
-	ruleString := strings.Join(rule, " ")
-	ruleString = chain + " " + ruleString
+	// parse "iptables -S" for the rule (it checks rules in a specific chain
+	// in a specific table and it is very unreliable)
+	return existsRaw(table, chain, rule...)
+}
+
+func existsRaw(table Table, chain string, rule ...string) bool {
+	ruleString := fmt.Sprintf("%s %s\n", chain, strings.Join(rule, " "))
 	existingRules, _ := exec.Command(iptablesPath, "-t", string(table), "-S", chain).Output()
 
 	return strings.Contains(string(existingRules), ruleString)
@@ -380,3 +386,25 @@ func ExistChain(chain string, table Table) bool {
 	}
 	return false
 }
+
+// GetVersion reads the iptables version numbers
+func GetVersion() (major, minor, micro int, err error) {
+	out, err := Raw("--version")
+	if err == nil {
+		major, minor, micro = parseVersionNumbers(string(out))
+	}
+	return
+}
+
+func parseVersionNumbers(input string) (major, minor, micro int) {
+	re := regexp.MustCompile(`v\d*.\d*.\d*`)
+	line := re.FindString(input)
+	fmt.Sscanf(line, "v%d.%d.%d", &major, &minor, &micro)
+	return
+}
+
+// iptables -C, --check option was added in v.1.4.11
+// http://ftp.netfilter.org/pub/iptables/changes-iptables-1.4.11.txt
+func supportsCOption(mj, mn, mc int) bool {
+	return mj > 1 || (mj == 1 && (mn > 4 || (mn == 4 && mc >= 11)))
+}

iptables/iptables_test.go

@@ -234,3 +234,79 @@ func TestCleanup(t *testing.T) {
 		t.Fatalf("Removing chain failed. %s found in iptables-save", chainName)
 	}
 }
+
+func TestExistsRaw(t *testing.T) {
+	testChain1 := "ABCD"
+	testChain2 := "EFGH"
+
+	_, err := NewChain(testChain1, Filter, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		RemoveExistingChain(testChain1, Filter)
+	}()
+
+	_, err = NewChain(testChain2, Filter, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		RemoveExistingChain(testChain2, Filter)
+	}()
+
+	// Test detection over full and truncated rule string
+	input := []struct{ rule []string }{
+		{[]string{"-s", "172.8.9.9/32", "-j", "ACCEPT"}},
+		{[]string{"-d", "172.8.9.0/24", "-j", "DROP"}},
+		{[]string{"-s", "172.0.3.0/24", "-d", "172.17.0.0/24", "-p", "tcp", "-m", "tcp", "--dport", "80", "-j", testChain2}},
+		{[]string{"-j", "RETURN"}},
+	}
+
+	for i, r := range input {
+		ruleAdd := append([]string{"-t", string(Filter), "-A", testChain1}, r.rule...)
+		err = RawCombinedOutput(ruleAdd...)
+		if err != nil {
+			t.Fatalf("i=%d, err: %v", i, err)
+		}
+		if !existsRaw(Filter, testChain1, r.rule...) {
+			t.Fatalf("Failed to detect rule. i=%d", i)
+		}
+		// Truncate the rule
+		trg := r.rule[len(r.rule)-1]
+		trg = trg[:len(trg)-2]
+		r.rule[len(r.rule)-1] = trg
+		if existsRaw(Filter, testChain1, r.rule...) {
+			t.Fatalf("Invalid detection. i=%d", i)
+		}
+	}
+}
+
+func TestGetVersion(t *testing.T) {
+	mj, mn, mc := parseVersionNumbers("iptables v1.4.19.1-alpha")
+	if mj != 1 || mn != 4 || mc != 19 {
+		t.Fatalf("Failed to parse version numbers")
+	}
+}
+
+func TestSupportsCOption(t *testing.T) {
+	input := []struct {
+		mj int
+		mn int
+		mc int
+		ok bool
+	}{
+		{1, 4, 11, true},
+		{1, 4, 12, true},
+		{1, 5, 0, true},
+		{0, 4, 11, false},
+		{0, 5, 12, false},
+		{1, 3, 12, false},
+		{1, 4, 10, false},
+	}
+	for ind, inp := range input {
+		if inp.ok != supportsCOption(inp.mj, inp.mn, inp.mc) {
+			t.Fatalf("Incorrect check: %d", ind)
+		}
+	}
+}