Browse Source

Merge pull request #43060 from akerouanton/fix-42127

Check iptables options before looking for ip6tables binary
Bjorn Neergaard 2 years ago
parent
commit
f5106148e3
2 changed files with 24 additions and 104 deletions
  1. 22 73
      libnetwork/iptables/iptables.go
  2. 2 31
      libnetwork/iptables/iptables_test.go

+ 22 - 73
libnetwork/iptables/iptables.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"fmt"
 	"net"
 	"net"
 	"os/exec"
 	"os/exec"
-	"regexp"
 	"strconv"
 	"strconv"
 	"strings"
 	"strings"
 	"sync"
 	"sync"
@@ -57,7 +56,6 @@ var (
 	iptablesPath  string
 	iptablesPath  string
 	ip6tablesPath string
 	ip6tablesPath string
 	supportsXlock = false
 	supportsXlock = false
-	supportsCOpt  = false
 	xLockWaitMsg  = "Another app is currently holding the xtables lock"
 	xLockWaitMsg  = "Another app is currently holding the xtables lock"
 	// used to lock iptables commands if xtables lock is not supported
 	// used to lock iptables commands if xtables lock is not supported
 	bestEffortLock sync.Mutex
 	bestEffortLock sync.Mutex
@@ -89,19 +87,27 @@ func (e ChainError) Error() string {
 	return fmt.Sprintf("Error iptables %s: %s", e.Chain, string(e.Output))
 	return fmt.Sprintf("Error iptables %s: %s", e.Chain, string(e.Output))
 }
 }
 
 
-func probe() {
+func detectIptables() {
 	path, err := exec.LookPath("iptables")
 	path, err := exec.LookPath("iptables")
 	if err != nil {
 	if err != nil {
-		logrus.Warnf("Failed to find iptables: %v", err)
+		logrus.WithError(err).Warnf("failed to find iptables")
 		return
 		return
 	}
 	}
-	if out, err := exec.Command(path, "--wait", "-t", "nat", "-L", "-n").CombinedOutput(); err != nil {
-		logrus.Warnf("Running iptables --wait -t nat -L -n failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)
+	iptablesPath = path
+
+	// The --wait flag was added in iptables v1.6.0.
+	// TODO remove this check once we drop support for CentOS/RHEL 7, which uses an older version of iptables
+	if out, err := exec.Command(path, "--wait", "-L", "-n").CombinedOutput(); err != nil {
+		logrus.WithError(err).Infof("unable to detect if iptables supports xlock: 'iptables --wait -L -n': `%s`", strings.TrimSpace(string(out)))
+	} else {
+		supportsXlock = true
 	}
 	}
-	_, err = exec.LookPath("ip6tables")
+
+	path, err = exec.LookPath("ip6tables")
 	if err != nil {
 	if err != nil {
-		logrus.Warnf("Failed to find ip6tables: %v", err)
-		return
+		logrus.WithError(err).Warnf("unable to find ip6tables")
+	} else {
+		ip6tablesPath = path
 	}
 	}
 }
 }
 
 
@@ -113,32 +119,11 @@ func initFirewalld() {
 		return
 		return
 	}
 	}
 	if err := FirewalldInit(); err != nil {
 	if err := FirewalldInit(); err != nil {
-		logrus.Debugf("Fail to initialize firewalld: %v, using raw iptables instead", err)
-	}
-}
-
-func detectIptables() {
-	path, err := exec.LookPath("iptables")
-	if err != nil {
-		return
-	}
-	iptablesPath = path
-	path, err = exec.LookPath("ip6tables")
-	if err != nil {
-		return
+		logrus.WithError(err).Debugf("unable to initialize firewalld; using raw iptables instead")
 	}
 	}
-	ip6tablesPath = 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
-	}
-	supportsCOpt = supportsCOption(mj, mn, mc)
 }
 }
 
 
 func initDependencies() {
 func initDependencies() {
-	probe()
 	initFirewalld()
 	initFirewalld()
 	detectIptables()
 	detectIptables()
 }
 }
@@ -476,26 +461,9 @@ func (iptable IPTable) exists(native bool, table Table, chain string, rule ...st
 		return false
 		return false
 	}
 	}
 
 
-	if supportsCOpt {
-		// if exit status is 0 then return true, the rule exists
-		_, err := f(append([]string{"-t", string(table), "-C", chain}, rule...)...)
-		return err == nil
-	}
-
-	// parse "iptables -S" for the rule (it checks rules in a specific chain
-	// in a specific table and it is very unreliable)
-	return iptable.existsRaw(table, chain, rule...)
-}
-
-func (iptable IPTable) existsRaw(table Table, chain string, rule ...string) bool {
-	path := iptablesPath
-	if iptable.Version == IPv6 {
-		path = ip6tablesPath
-	}
-	ruleString := fmt.Sprintf("%s %s\n", chain, strings.Join(rule, " "))
-	existingRules, _ := exec.Command(path, "-t", string(table), "-S", chain).Output()
-
-	return strings.Contains(string(existingRules), ruleString)
+	// if exit status is 0 then return true, the rule exists
+	_, err := f(append([]string{"-t", string(table), "-C", chain}, rule...)...)
+	return err == nil
 }
 }
 
 
 // Maximum duration that an iptables operation can take
 // Maximum duration that an iptables operation can take
@@ -549,6 +517,9 @@ func (iptable IPTable) raw(args ...string) ([]byte, error) {
 	path := iptablesPath
 	path := iptablesPath
 	commandName := "iptables"
 	commandName := "iptables"
 	if iptable.Version == IPv6 {
 	if iptable.Version == IPv6 {
+		if ip6tablesPath == "" {
+			return nil, fmt.Errorf("ip6tables is missing")
+		}
 		path = ip6tablesPath
 		path = ip6tablesPath
 		commandName = "ip6tables"
 		commandName = "ip6tables"
 	}
 	}
@@ -590,15 +561,6 @@ func (iptable IPTable) ExistChain(chain string, table Table) bool {
 	return false
 	return false
 }
 }
 
 
-// GetVersion reads the iptables version numbers during initialization
-func GetVersion() (major, minor, micro int, err error) {
-	out, err := exec.Command(iptablesPath, "--version").CombinedOutput()
-	if err == nil {
-		major, minor, micro = parseVersionNumbers(string(out))
-	}
-	return
-}
-
 // SetDefaultPolicy sets the passed default policy for the table/chain
 // SetDefaultPolicy sets the passed default policy for the table/chain
 func (iptable IPTable) SetDefaultPolicy(table Table, chain string, policy Policy) error {
 func (iptable IPTable) SetDefaultPolicy(table Table, chain string, policy Policy) error {
 	if err := iptable.RawCombinedOutput("-t", string(table), "-P", chain, string(policy)); err != nil {
 	if err := iptable.RawCombinedOutput("-t", string(table), "-P", chain, string(policy)); err != nil {
@@ -607,19 +569,6 @@ func (iptable IPTable) SetDefaultPolicy(table Table, chain string, policy Policy
 	return nil
 	return nil
 }
 }
 
 
-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)))
-}
-
 // AddReturnRule adds a return rule for the chain in the filter table
 // AddReturnRule adds a return rule for the chain in the filter table
 func (iptable IPTable) AddReturnRule(chain string) error {
 func (iptable IPTable) AddReturnRule(chain string) error {
 	var (
 	var (

+ 2 - 31
libnetwork/iptables/iptables_test.go

@@ -287,44 +287,15 @@ func TestExistsRaw(t *testing.T) {
 		if err != nil {
 		if err != nil {
 			t.Fatalf("i=%d, err: %v", i, err)
 			t.Fatalf("i=%d, err: %v", i, err)
 		}
 		}
-		if !iptable.existsRaw(Filter, testChain1, r.rule...) {
+		if !iptable.exists(true, Filter, testChain1, r.rule...) {
 			t.Fatalf("Failed to detect rule. i=%d", i)
 			t.Fatalf("Failed to detect rule. i=%d", i)
 		}
 		}
 		// Truncate the rule
 		// Truncate the rule
 		trg := r.rule[len(r.rule)-1]
 		trg := r.rule[len(r.rule)-1]
 		trg = trg[:len(trg)-2]
 		trg = trg[:len(trg)-2]
 		r.rule[len(r.rule)-1] = trg
 		r.rule[len(r.rule)-1] = trg
-		if iptable.existsRaw(Filter, testChain1, r.rule...) {
+		if iptable.exists(true, Filter, testChain1, r.rule...) {
 			t.Fatalf("Invalid detection. i=%d", i)
 			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.Fatal("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)
-		}
-	}
-}