Create DOCKER forward chain on driver init

Docker-DCO-1.1-Signed-off-by: Ian Bishop <ianbishop@pace7.com> (github: porjo)

daemon/networkdriver/bridge/driver.go

@@ -145,12 +145,16 @@ func InitDriver(job *engine.Job) engine.Status {
 	}
 
 	// We can always try removing the iptables
-	if err := iptables.RemoveExistingChain("DOCKER"); err != nil {
+	if err := iptables.RemoveExistingChain("DOCKER", iptables.Nat); err != nil {
 		return job.Error(err)
 	}
 
 	if enableIPTables {
-		chain, err := iptables.NewChain("DOCKER", bridgeIface)
+		_, err := iptables.NewChain("DOCKER", bridgeIface, iptables.Nat)
+		if err != nil {
+			return job.Error(err)
+		}
+		chain, err := iptables.NewChain("DOCKER", bridgeIface, iptables.Filter)
 		if err != nil {
 			return job.Error(err)
 		}

pkg/iptables/iptables.go

@@ -13,15 +13,17 @@ import (
 )
 
 type Action string
+type Table string
 
 const (
 	Append Action = "-A"
 	Delete Action = "-D"
 	Insert Action = "-I"
+	Nat    Table  = "nat"
+	Filter Table  = "filter"
 )
 
 var (
-	nat                 = []string{"-t", "nat"}
 	supportsXlock       = false
 	ErrIptablesNotFound = errors.New("Iptables not found")
 )
@@ -29,6 +31,7 @@ var (
 type Chain struct {
 	Name   string
 	Bridge string
+	Table  Table
 }
 
 type ChainError struct {
@@ -44,33 +47,73 @@ func init() {
 	supportsXlock = exec.Command("iptables", "--wait", "-L", "-n").Run() == nil
 }
 
-func NewChain(name, bridge string) (*Chain, error) {
-	if output, err := Raw("-t", "nat", "-N", name); err != nil {
-		return nil, err
-	} else if len(output) != 0 {
-		return nil, fmt.Errorf("Error creating new iptables chain: %s", output)
-	}
-	chain := &Chain{
+func NewChain(name, bridge string, table Table) (*Chain, error) {
+	c := &Chain{
 		Name:   name,
 		Bridge: bridge,
+		Table:  table,
+	}
+
+	if string(c.Table) == "" {
+		c.Table = Filter
 	}
 
-	if err := chain.Prerouting(Append, "-m", "addrtype", "--dst-type", "LOCAL"); err != nil {
-		return nil, fmt.Errorf("Failed to inject docker in PREROUTING chain: %s", err)
+	// Add chain if it doesn't exist
+	if _, err := Raw("-t", string(c.Table), "-n", "-L", c.Name); err != nil {
+		if output, err := Raw("-t", string(c.Table), "-N", c.Name); err != nil {
+			return nil, err
+		} else if len(output) != 0 {
+			return nil, fmt.Errorf("Could not create %s/%s chain: %s", c.Table, c.Name, output)
+		}
 	}
-	if err := chain.Output(Append, "-m", "addrtype", "--dst-type", "LOCAL", "!", "--dst", "127.0.0.0/8"); err != nil {
-		return nil, fmt.Errorf("Failed to inject docker in OUTPUT chain: %s", err)
+
+	switch table {
+	case Nat:
+		preroute := []string{
+			"-m", "addrtype",
+			"--dst-type", "LOCAL"}
+		if !Exists(preroute...) {
+			if err := c.Prerouting(Append, preroute...); err != nil {
+				return nil, fmt.Errorf("Failed to inject docker in PREROUTING chain: %s", err)
+			}
+		}
+		output := []string{
+			"-m", "addrtype",
+			"--dst-type", "LOCAL",
+			"!", "--dst", "127.0.0.0/8"}
+		if !Exists(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",
+			"-o", c.Bridge,
+			"-j", c.Name}
+		if !Exists(link...) {
+			insert := append([]string{string(Insert)}, link...)
+			if output, err := Raw(insert...); err != nil {
+				return nil, err
+			} else if len(output) != 0 {
+				return nil, fmt.Errorf("Could not create linking rule to %s/%s: %s", c.Table, c.Name, output)
+			}
+		}
 	}
-	return chain, nil
+	return c, nil
 }
 
-func RemoveExistingChain(name string) error {
-	chain := &Chain{
-		Name: name,
+func RemoveExistingChain(name string, table Table) error {
+	c := &Chain{
+		Name:  name,
+		Table: table,
+	}
+	if string(c.Table) == "" {
+		c.Table = Filter
 	}
-	return chain.Remove()
+	return c.Remove()
 }
 
+// Add forwarding rule to 'filter' table and corresponding nat rule to 'nat' table
 func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr string, dest_port int) error {
 	daddr := ip.String()
 	if ip.IsUnspecified() {
@@ -79,7 +122,7 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr str
 		// value" by both iptables and ip6tables.
 		daddr = "0/0"
 	}
-	if output, err := Raw("-t", "nat", string(action), c.Name,
+	if output, err := Raw("-t", string(Nat), string(action), c.Name,
 		"-p", proto,
 		"-d", daddr,
 		"--dport", strconv.Itoa(port),
@@ -91,13 +134,7 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr str
 		return &ChainError{Chain: "FORWARD", Output: output}
 	}
 
-	if action != Delete {
-		if err := c.createForwardChain(); err != nil {
-			return err
-		}
-	}
-
-	if output, err := Raw(string(action), c.Name,
+	if output, err := Raw("-t", string(Filter), string(action), c.Name,
 		"!", "-i", c.Bridge,
 		"-o", c.Bridge,
 		"-p", proto,
@@ -112,13 +149,10 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr str
 	return nil
 }
 
+// Add reciprocal ACCEPT rule for two supplied IP addresses.
+// Traffic is allowed from ip1 to ip2 and vice-versa
 func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) error {
-	if action != Delete {
-		if err := c.createForwardChain(); err != nil {
-			return err
-		}
-	}
-	if output, err := Raw(string(action), c.Name,
+	if output, err := Raw("-t", string(Filter), string(action), c.Name,
 		"-i", c.Bridge, "-o", c.Bridge,
 		"-p", proto,
 		"-s", ip1.String(),
@@ -127,10 +161,9 @@ func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) err
 		"-j", "ACCEPT"); err != nil {
 		return err
 	} else if len(output) != 0 {
-		return fmt.Errorf("Error toggle iptables forward: %s", output)
+		return fmt.Errorf("Error iptables forward: %s", output)
 	}
-
-	if output, err := Raw(string(action), c.Name,
+	if output, err := Raw("-t", string(Filter), string(action), c.Name,
 		"-i", c.Bridge, "-o", c.Bridge,
 		"-p", proto,
 		"-s", ip2.String(),
@@ -139,14 +172,14 @@ func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) err
 		"-j", "ACCEPT"); err != nil {
 		return err
 	} else if len(output) != 0 {
-		return fmt.Errorf("Error toggle iptables forward: %s", output)
+		return fmt.Errorf("Error iptables forward: %s", output)
 	}
-
 	return nil
 }
 
+// Add linking rule to nat/PREROUTING chain.
 func (c *Chain) Prerouting(action Action, args ...string) error {
-	a := append(nat, fmt.Sprint(action), "PREROUTING")
+	a := []string{"-t", string(Nat), string(action), "PREROUTING"}
 	if len(args) > 0 {
 		a = append(a, args...)
 	}
@@ -158,8 +191,9 @@ func (c *Chain) Prerouting(action Action, args ...string) error {
 	return nil
 }
 
+// Add linking rule to an OUTPUT chain
 func (c *Chain) Output(action Action, args ...string) error {
-	a := append(nat, fmt.Sprint(action), "OUTPUT")
+	a := []string{"-t", string(c.Table), string(action), "OUTPUT"}
 	if len(args) > 0 {
 		a = append(a, args...)
 	}
@@ -172,21 +206,22 @@ func (c *Chain) Output(action Action, args ...string) error {
 }
 
 func (c *Chain) Remove() error {
-	// Ignore errors - This could mean the chains were never set up
-	c.Prerouting(Delete, "-m", "addrtype", "--dst-type", "LOCAL")
-	c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "!", "--dst", "127.0.0.0/8")
-	c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL") // Created in versions <= 0.1.6
-
-	c.Prerouting(Delete)
-	c.Output(Delete)
+	if c.Table == Nat {
+		// Ignore errors - This could mean the chains were never set up
+		c.Prerouting(Delete, "-m", "addrtype", "--dst-type", "LOCAL")
+		c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "!", "--dst", "127.0.0.0/8")
+		c.Output(Delete, "-m", "addrtype", "--dst-type", "LOCAL") // Created in versions <= 0.1.6
 
-	Raw("-t", "nat", "-F", c.Name)
-	Raw("-t", "nat", "-X", c.Name)
+		c.Prerouting(Delete)
+		c.Output(Delete)
 
+		Raw("-t", string(Nat), "-F", c.Name)
+		Raw("-t", string(Nat), "-X", c.Name)
+	}
 	return nil
 }
 
-// Check if an existing rule exists
+// Check if a rule exists
 func Exists(args ...string) bool {
 	// iptables -C, --check option was added in v.1.4.11
 	// http://ftp.netfilter.org/pub/iptables/changes-iptables-1.4.11.txt
@@ -211,6 +246,7 @@ func Exists(args ...string) bool {
 	)
 }
 
+// Call 'iptables' system command, passing supplied arguments
 func Raw(args ...string) ([]byte, error) {
 	path, err := exec.LookPath("iptables")
 	if err != nil {
@@ -235,28 +271,3 @@ func Raw(args ...string) ([]byte, error) {
 
 	return output, err
 }
-
-func (c *Chain) createForwardChain() error {
-	// Add chain if doesn't exist
-	if _, err := Raw("-n", "-L", c.Name); err != nil {
-		output, err := Raw("-N", c.Name)
-		if err != nil {
-			return err
-		} else if len(output) != 0 {
-			return fmt.Errorf("Error iptables forward: %s", output)
-		}
-	}
-	// Add linking rule if it doesn't exist
-	if !Exists("FORWARD",
-		"-o", c.Bridge,
-		"-j", c.Name) {
-		if output2, err := Raw(string(Insert), "FORWARD",
-			"-o", c.Bridge,
-			"-j", c.Name); err != nil {
-			return err
-		} else if len(output2) != 0 {
-			return fmt.Errorf("Error iptables forward: %s", output2)
-		}
-	}
-	return nil
-}