From 3d0ddef0eb94020d46cf7bf348e6f7aa751375e2 Mon Sep 17 00:00:00 2001
From: Tim Gross <tim@0x74696d.com>
Date: Mon, 4 Nov 2019 15:54:47 -0500
Subject: [PATCH] ensure iptables chain creation is idempotent

Concurrent use of the `portmap` and `firewall` plugins can result in
errors during iptables chain creation:

- The `portmap` plugin has a time-of-check-time-of-use race where it
  checks for existence of the chain but the operation isn't atomic.
- The `firewall` plugin doesn't check for existing chains and just
  returns an error.

This commit makes both operations idempotent by creating the chain and
then discarding the error if it's caused by the chain already existing.

Signed-off-by: Tim Gross <tim@0x74696d.com>
---
 plugins/meta/firewall/firewall_iptables_test.go |  7 +++++++
 plugins/meta/firewall/iptables.go               |  9 ++++++++-
 plugins/meta/portmap/chain.go                   | 11 ++++++++---
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/plugins/meta/firewall/firewall_iptables_test.go b/plugins/meta/firewall/firewall_iptables_test.go
index 6c7358f42..ac474dc84 100644
--- a/plugins/meta/firewall/firewall_iptables_test.go
+++ b/plugins/meta/firewall/firewall_iptables_test.go
@@ -270,6 +270,13 @@ var _ = Describe("firewall plugin iptables backend", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			validateFullRuleset(fullConf)
+
+			// ensure creation is idempotent
+			_, _, err = testutils.CmdAdd(targetNS.Path(), args.ContainerID, IFNAME, fullConf, func() error {
+				return cmdAdd(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+
 			return nil
 		})
 		Expect(err).NotTo(HaveOccurred())
diff --git a/plugins/meta/firewall/iptables.go b/plugins/meta/firewall/iptables.go
index faae35c61..f6e7fe2f9 100644
--- a/plugins/meta/firewall/iptables.go
+++ b/plugins/meta/firewall/iptables.go
@@ -43,7 +43,14 @@ func ensureChain(ipt *iptables.IPTables, table, chain string) error {
 		}
 	}
 
-	return ipt.NewChain(table, chain)
+	err = ipt.NewChain(table, chain)
+	if err != nil {
+		eerr, eok := err.(*iptables.Error)
+		if eok && eerr.ExitStatus() != 1 {
+			return err
+		}
+	}
+	return nil
 }
 
 func generateFilterRule(privChainName string) []string {
diff --git a/plugins/meta/portmap/chain.go b/plugins/meta/portmap/chain.go
index bca8214a9..7cb72c9d7 100644
--- a/plugins/meta/portmap/chain.go
+++ b/plugins/meta/portmap/chain.go
@@ -35,14 +35,19 @@ type chain struct {
 
 // setup idempotently creates the chain. It will not error if the chain exists.
 func (c *chain) setup(ipt *iptables.IPTables) error {
-	// create the chain
+
 	exists, err := chainExists(ipt, c.table, c.name)
 	if err != nil {
 		return err
 	}
+
 	if !exists {
-		if err := ipt.NewChain(c.table, c.name); err != nil {
-			return err
+		err := ipt.NewChain(c.table, c.name)
+		if err != nil {
+			eerr, eok := err.(*iptables.Error)
+			if eok && eerr.ExitStatus() != 1 {
+				return err
+			}
 		}
 	}