*: add file-based lock for old iptables versions

iptables <1.4.20 does not support blocking (the `--wait` option) to
prevent concurrent invocations from interrupting each other. To work
around this, this patch adds a file-based lock (flock), using the same
file that xtables now uses [1] (`/run/xtables.lock`).

Note that this follows a similar behaviour to xtables in that it's
best-effort only: if the lock can't be acquired for some reason,
operations proceed anyway.

h/t to @janeczku and @adfernandes for original fix suggestions

[1]: http://git.netfilter.org/iptables/commit/?id=aa562a660d1555b13cffbac1e744033e91f82707

Author: jonboulle

iptables/iptables.go

@@ -40,41 +40,53 @@ func (e *Error) Error() string {
 }
 
 type IPTables struct {
-	path string
+	path     string
+	hasCheck bool
+	hasWait  bool
+
+	fmu *fileLock
 }
 
 func New() (*IPTables, error) {
 	path, err := exec.LookPath("iptables")
 	if err != nil {
 		return nil, err
 	}
-
-	return &IPTables{path}, nil
-}
-
-// Exists checks if given rulespec in specified table/chain exists
-func (ipt *IPTables) Exists(table, chain string, rulespec ...string) (bool, error) {
-	checkPresent, err := getIptablesHasCheckCommand()
+	checkPresent, waitPresent, err := getIptablesCommandSupport()
 	if err != nil {
-		log.Printf("Error checking iptables version, assuming version at least 1.4.11: %v", err)
+		log.Printf("Error checking iptables version, assuming version at least 1.4.20: %v", err)
 		checkPresent = true
+		waitPresent = true
 	}
+	ipt := IPTables{
+		path:     path,
+		hasCheck: checkPresent,
+		hasWait:  waitPresent,
+	}
+	if !waitPresent {
+		ipt.fmu, err = newXtablesFileLock()
+		if err != nil {
+			return nil, err
+		}
+	}
+	return &ipt, nil
+}
 
-	if !checkPresent {
+// Exists checks if given rulespec in specified table/chain exists
+func (ipt *IPTables) Exists(table, chain string, rulespec ...string) (bool, error) {
+	if !ipt.hasCheck {
 		cmd := append([]string{"-A", chain}, rulespec...)
 		return existsForOldIpTables(table, strings.Join(cmd, " "))
-	} else {
-		cmd := append([]string{"-t", table, "-C", chain}, rulespec...)
-		err := ipt.run(cmd...)
-
-		switch {
-		case err == nil:
-			return true, nil
-		case err.(*Error).ExitStatus() == 1:
-			return false, nil
-		default:
-			return false, err
-		}
+	}
+	cmd := append([]string{"-t", table, "-C", chain}, rulespec...)
+	err := ipt.run(cmd...)
+	switch {
+	case err == nil:
+		return true, nil
+	case err.(*Error).ExitStatus() == 1:
+		return false, nil
+	default:
+		return false, err
 	}
 }
 
@@ -113,9 +125,21 @@ func (ipt *IPTables) Delete(table, chain string, rulespec ...string) error {
 // List rules in specified table/chain
 func (ipt *IPTables) List(table, chain string) ([]string, error) {
 	var stdout, stderr bytes.Buffer
+	args := []string{ipt.path, "-t", table, "-S", chain}
+
+	if ipt.hasWait {
+		args = append(args, "--wait")
+	} else {
+		ul, err := ipt.fmu.tryLock()
+		if err != nil {
+			return nil, err
+		}
+		defer ul.Unlock()
+	}
+
 	cmd := exec.Cmd{
 		Path:   ipt.path,
-		Args:   []string{ipt.path, "--wait", "-t", table, "-S", chain},
+		Args:   args,
 		Stdout: &stdout,
 		Stderr: &stderr,
 	}
@@ -136,8 +160,8 @@ func (ipt *IPTables) NewChain(table, chain string) error {
 	return ipt.run("-t", table, "-N", chain)
 }
 
-// ClearChain flushed (deletes all rules) in the specifed table/chain.
-// If the chain does not exist, new one will be created
+// ClearChain flushed (deletes all rules) in the specified table/chain.
+// If the chain does not exist, a new one will be created
 func (ipt *IPTables) ClearChain(table, chain string) error {
 	err := ipt.NewChain(table, chain)
 
@@ -160,7 +184,16 @@ func (ipt *IPTables) DeleteChain(table, chain string) error {
 
 func (ipt *IPTables) run(args ...string) error {
 	var stderr bytes.Buffer
-	args = append([]string{"--wait"}, args...)
+	if ipt.hasWait {
+		args = append([]string{"--wait"}, args...)
+	} else {
+		ul, err := ipt.fmu.tryLock()
+		if err != nil {
+			return err
+		}
+		defer ul.Unlock()
+	}
+
 	cmd := exec.Cmd{
 		Path:   ipt.path,
 		Args:   append([]string{ipt.path}, args...),
@@ -174,19 +207,19 @@ func (ipt *IPTables) run(args ...string) error {
 	return nil
 }
 
-// Checks if iptables has the "-C" flag
-func getIptablesHasCheckCommand() (bool, error) {
+// Checks if iptables has the "-C" and "--wait" flag
+func getIptablesCommandSupport() (bool, bool, error) {
 	vstring, err := getIptablesVersionString()
 	if err != nil {
-		return false, err
+		return false, false, err
 	}
 
 	v1, v2, v3, err := extractIptablesVersion(vstring)
 	if err != nil {
-		return false, err
+		return false, false, err
 	}
 
-	return iptablesHasCheckCommand(v1, v2, v3), nil
+	return iptablesHasCheckCommand(v1, v2, v3), iptablesHasWaitCommand(v1, v2, v3), nil
 }
 
 // getIptablesVersion returns the first three components of the iptables version.
@@ -242,6 +275,20 @@ func iptablesHasCheckCommand(v1 int, v2 int, v3 int) bool {
 	return false
 }
 
+// Checks if an iptables version is after 1.4.20, when --wait was added
+func iptablesHasWaitCommand(v1 int, v2 int, v3 int) bool {
+	if v1 > 1 {
+		return true
+	}
+	if v1 == 1 && v2 > 4 {
+		return true
+	}
+	if v1 == 1 && v2 == 4 && v3 >= 20 {
+		return true
+	}
+	return false
+}
+
 // Checks if a rule specification exists for a table
 func existsForOldIpTables(table string, ruleSpec string) (bool, error) {
 	cmd := exec.Command("iptables", "-t", table, "-S")

iptables/lock.go

@@ -0,0 +1,84 @@
+// Copyright 2015 CoreOS, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package iptables
+
+import (
+	"os"
+	"sync"
+	"syscall"
+)
+
+const (
+	// In earlier versions of iptables, the xtables lock was implemented
+	// via a Unix socket, but now flock is used via this lockfile:
+	// http://git.netfilter.org/iptables/commit/?id=aa562a660d1555b13cffbac1e744033e91f82707
+	// Note the LSB-conforming "/run" directory does not exist on old
+	// distributions, so assume "/var" is symlinked
+	xtablesLockFilePath = "/var/run/xtables.lock"
+
+	defaultFilePerm = 0600
+)
+
+type Unlocker interface {
+	Unlock() error
+}
+
+type nopUnlocker struct{}
+
+func (_ nopUnlocker) Unlock() error { return nil }
+
+type fileLock struct {
+	// mu is used to protect against concurrent invocations from within this process
+	mu sync.Mutex
+	fd int
+}
+
+// tryLock takes an exclusive lock on the xtables lock file without blocking.
+// This is best-effort only: if the exclusive lock would block (i.e. because
+// another process already holds it), no error is returned. Otherwise, any
+// error encountered during the locking operation is returned.
+// The returned Unlocker should be used to release the lock when the caller is
+// done invoking iptables commands.
+func (l *fileLock) tryLock() (Unlocker, error) {
+	l.mu.Lock()
+	err := syscall.Flock(l.fd, syscall.LOCK_EX|syscall.LOCK_NB)
+	switch err {
+	case syscall.EWOULDBLOCK:
+		l.mu.Unlock()
+		return nopUnlocker{}, nil
+	case nil:
+		return l, nil
+	default:
+		l.mu.Unlock()
+		return nil, err
+	}
+}
+
+// Unlock closes the underlying file, which implicitly unlocks it as well. It
+// also unlocks the associated mutex.
+func (l *fileLock) Unlock() error {
+	defer l.mu.Unlock()
+	return syscall.Close(l.fd)
+}
+
+// newXtablesFileLock opens a new lock on the xtables lockfile without
+// acquiring the lock
+func newXtablesFileLock() (*fileLock, error) {
+	fd, err := syscall.Open(xtablesLockFilePath, os.O_CREATE, defaultFilePerm)
+	if err != nil {
+		return nil, err
+	}
+	return &fileLock{fd: fd}, nil
+}