Browse Source

Merge pull request #7003 from porjo/6034-fowardChain

Move per-container forward rules to DOCKER chain
Alexander Morozov 10 years ago
parent
commit
04117e4021

+ 31 - 25
daemon/networkdriver/bridge/driver.go

@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"io/ioutil"
 	"net"
 	"net"
 	"os"
 	"os"
-	"strconv"
 	"sync"
 	"sync"
 
 
 	log "github.com/Sirupsen/logrus"
 	log "github.com/Sirupsen/logrus"
@@ -144,12 +143,16 @@ func InitDriver(job *engine.Job) engine.Status {
 	}
 	}
 
 
 	// We can always try removing the iptables
 	// 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)
 		return job.Error(err)
 	}
 	}
 
 
 	if enableIPTables {
 	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 {
 		if err != nil {
 			return job.Error(err)
 			return job.Error(err)
 		}
 		}
@@ -501,35 +504,38 @@ func AllocatePort(job *engine.Job) engine.Status {
 func LinkContainers(job *engine.Job) engine.Status {
 func LinkContainers(job *engine.Job) engine.Status {
 	var (
 	var (
 		action       = job.Args[0]
 		action       = job.Args[0]
+		nfAction     iptables.Action
 		childIP      = job.Getenv("ChildIP")
 		childIP      = job.Getenv("ChildIP")
 		parentIP     = job.Getenv("ParentIP")
 		parentIP     = job.Getenv("ParentIP")
 		ignoreErrors = job.GetenvBool("IgnoreErrors")
 		ignoreErrors = job.GetenvBool("IgnoreErrors")
 		ports        = job.GetenvList("Ports")
 		ports        = job.GetenvList("Ports")
 	)
 	)
-	for _, value := range ports {
-		port := nat.Port(value)
-		if output, err := iptables.Raw(action, "FORWARD",
-			"-i", bridgeIface, "-o", bridgeIface,
-			"-p", port.Proto(),
-			"-s", parentIP,
-			"--dport", strconv.Itoa(port.Int()),
-			"-d", childIP,
-			"-j", "ACCEPT"); !ignoreErrors && err != nil {
-			return job.Error(err)
-		} else if len(output) != 0 {
-			return job.Errorf("Error toggle iptables forward: %s", output)
-		}
 
 
-		if output, err := iptables.Raw(action, "FORWARD",
-			"-i", bridgeIface, "-o", bridgeIface,
-			"-p", port.Proto(),
-			"-s", childIP,
-			"--sport", strconv.Itoa(port.Int()),
-			"-d", parentIP,
-			"-j", "ACCEPT"); !ignoreErrors && err != nil {
+	switch action {
+	case "-A":
+		nfAction = iptables.Append
+	case "-I":
+		nfAction = iptables.Insert
+	case "-D":
+		nfAction = iptables.Delete
+	default:
+		return job.Errorf("Invalid action '%s' specified", action)
+	}
+
+	ip1 := net.ParseIP(parentIP)
+	if ip1 == nil {
+		return job.Errorf("parent IP '%s' is invalid", parentIP)
+	}
+	ip2 := net.ParseIP(childIP)
+	if ip2 == nil {
+		return job.Errorf("child IP '%s' is invalid", childIP)
+	}
+
+	chain := iptables.Chain{Name: "DOCKER", Bridge: bridgeIface}
+	for _, p := range ports {
+		port := nat.Port(p)
+		if err := chain.Link(nfAction, ip1, ip2, port.Int(), port.Proto()); !ignoreErrors && err != nil {
 			return job.Error(err)
 			return job.Error(err)
-		} else if len(output) != 0 {
-			return job.Errorf("Error toggle iptables forward: %s", output)
 		}
 		}
 	}
 	}
 	return engine.StatusOK
 	return engine.StatusOK

+ 41 - 0
daemon/networkdriver/bridge/driver_test.go

@@ -7,6 +7,7 @@ import (
 
 
 	"github.com/docker/docker/daemon/networkdriver/portmapper"
 	"github.com/docker/docker/daemon/networkdriver/portmapper"
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/engine"
+	"github.com/docker/docker/pkg/iptables"
 )
 )
 
 
 func init() {
 func init() {
@@ -118,3 +119,43 @@ func TestMacAddrGeneration(t *testing.T) {
 		t.Fatal("Non-unique MAC address")
 		t.Fatal("Non-unique MAC address")
 	}
 	}
 }
 }
+
+func TestLinkContainers(t *testing.T) {
+	eng := engine.New()
+	eng.Logging = false
+
+	// Init driver
+	job := eng.Job("initdriver")
+	if res := InitDriver(job); res != engine.StatusOK {
+		t.Fatal("Failed to initialize network driver")
+	}
+
+	// Allocate interface
+	job = eng.Job("allocate_interface", "container_id")
+	if res := Allocate(job); res != engine.StatusOK {
+		t.Fatal("Failed to allocate network interface")
+	}
+
+	job.Args[0] = "-I"
+
+	job.Setenv("ChildIP", "172.17.0.2")
+	job.Setenv("ParentIP", "172.17.0.1")
+	job.SetenvBool("IgnoreErrors", false)
+	job.SetenvList("Ports", []string{"1234"})
+
+	bridgeIface = "lo"
+	_, err := iptables.NewChain("DOCKER", bridgeIface, iptables.Filter)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if res := LinkContainers(job); res != engine.StatusOK {
+		t.Fatalf("LinkContainers failed")
+	}
+
+	// flush rules
+	if _, err = iptables.Raw([]string{"-F", "DOCKER"}...); err != nil {
+		t.Fatal(err)
+	}
+
+}

+ 1 - 1
daemon/networkdriver/portmapper/mapper.go

@@ -93,7 +93,7 @@ func Map(container net.Addr, hostIP net.IP, hostPort int) (host net.Addr, err er
 	}
 	}
 
 
 	containerIP, containerPort := getIPAndPort(m.container)
 	containerIP, containerPort := getIPAndPort(m.container)
-	if err := forward(iptables.Add, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort); err != nil {
+	if err := forward(iptables.Append, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort); err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
 
 

+ 44 - 23
docs/sources/articles/networking.md

@@ -184,30 +184,46 @@ running.  The options then modify this default configuration.
 
 
 <a name="the-world"></a>
 <a name="the-world"></a>
 
 
-Whether a container can talk to the world is governed by one main factor.
+Whether a container can talk to the world is governed by two factors.
 
 
-Is the host machine willing to forward IP packets?  This is governed
-by the `ip_forward` system parameter.  Packets can only pass between
-containers if this parameter is `1`.  Usually you will simply leave
-the Docker server at its default setting `--ip-forward=true` and
-Docker will go set `ip_forward` to `1` for you when the server
-starts up.  To check the setting or turn it on manually:
-
-    # Usually not necessary: turning on forwarding,
-    # on the host where your Docker server is running
+1.  Is the host machine willing to forward IP packets?  This is governed
+    by the `ip_forward` system parameter.  Packets can only pass between
+    containers if this parameter is `1`.  Usually you will simply leave
+    the Docker server at its default setting `--ip-forward=true` and
+    Docker will go set `ip_forward` to `1` for you when the server
+    starts up.  To check the setting or turn it on manually:
 
 
+    ```
     $ cat /proc/sys/net/ipv4/ip_forward
     $ cat /proc/sys/net/ipv4/ip_forward
     0
     0
-    $ sudo echo 1 > /proc/sys/net/ipv4/ip_forward
+    $ echo 1 > /proc/sys/net/ipv4/ip_forward
     $ cat /proc/sys/net/ipv4/ip_forward
     $ cat /proc/sys/net/ipv4/ip_forward
     1
     1
+    ```
+
+    Many using Docker will want `ip_forward` to be on, to at
+    least make communication *possible* between containers and
+    the wider world.
+
+    May also be needed for inter-container communication if you are
+    in a multiple bridge setup.
+
+2.  Do your `iptables` allow this particular connection? Docker will
+    never make changes to your system `iptables` rules if you set
+    `--iptables=false` when the daemon starts.  Otherwise the Docker
+    server will append forwarding rules to the `DOCKER` filter chain.
+
+Docker will not delete or modify any pre-existing rules from the `DOCKER`
+filter chain. This allows the user to create in advance any rules required
+to further restrict access to the containers.
 
 
-Many using Docker will want `ip_forward` to be on, to at
-least make communication *possible* between containers and
-the wider world.
+Docker's forward rules permit all external source IPs by default. To allow
+only a specific IP or network to access the containers, insert a negated
+rule at the top of the `DOCKER` filter chain. For example, to restrict
+external access such that *only* source IP 8.8.8.8 can access the
+containers, the following rule could be added:
 
 
-May also be needed for inter-container communication if you are
-in a multiple bridge setup.
+    $ iptables -I DOCKER -i ext_if ! -s 8.8.8.8 -j DROP
 
 
 ## Communication between containers
 ## Communication between containers
 
 
@@ -222,12 +238,12 @@ system level, by two factors.
     between them.  See the later sections of this document for other
     between them.  See the later sections of this document for other
     possible topologies.
     possible topologies.
 
 
-2.  Do your `iptables` allow this particular connection to be made?
-    Docker will never make changes to your system `iptables` rules if
-    you set `--iptables=false` when the daemon starts.  Otherwise the
-    Docker server will add a default rule to the `FORWARD` chain with a
-    blanket `ACCEPT` policy if you retain the default `--icc=true`, or
-    else will set the policy to `DROP` if `--icc=false`.
+2.  Do your `iptables` allow this particular connection? Docker will never
+    make changes to your system `iptables` rules if you set
+    `--iptables=false` when the daemon starts.  Otherwise the Docker server
+    will add a default rule to the `FORWARD` chain with a blanket `ACCEPT`
+    policy if you retain the default `--icc=true`, or else will set the
+    policy to `DROP` if `--icc=false`.
 
 
 It is a strategic question whether to leave `--icc=true` or change it to
 It is a strategic question whether to leave `--icc=true` or change it to
 `--icc=false` (on Ubuntu, by editing the `DOCKER_OPTS` variable in
 `--icc=false` (on Ubuntu, by editing the `DOCKER_OPTS` variable in
@@ -267,6 +283,7 @@ the `FORWARD` chain has a default policy of `ACCEPT` or `DROP`:
     ...
     ...
     Chain FORWARD (policy ACCEPT)
     Chain FORWARD (policy ACCEPT)
     target     prot opt source               destination
     target     prot opt source               destination
+    DOCKER     all  --  0.0.0.0/0            0.0.0.0/0
     DROP       all  --  0.0.0.0/0            0.0.0.0/0
     DROP       all  --  0.0.0.0/0            0.0.0.0/0
     ...
     ...
 
 
@@ -278,9 +295,13 @@ the `FORWARD` chain has a default policy of `ACCEPT` or `DROP`:
     ...
     ...
     Chain FORWARD (policy ACCEPT)
     Chain FORWARD (policy ACCEPT)
     target     prot opt source               destination
     target     prot opt source               destination
+    DOCKER     all  --  0.0.0.0/0            0.0.0.0/0
+    DROP       all  --  0.0.0.0/0            0.0.0.0/0
+
+    Chain DOCKER (1 references)
+    target     prot opt source               destination
     ACCEPT     tcp  --  172.17.0.2           172.17.0.3           tcp spt:80
     ACCEPT     tcp  --  172.17.0.2           172.17.0.3           tcp spt:80
     ACCEPT     tcp  --  172.17.0.3           172.17.0.2           tcp dpt:80
     ACCEPT     tcp  --  172.17.0.3           172.17.0.2           tcp dpt:80
-    DROP       all  --  0.0.0.0/0            0.0.0.0/0
 
 
 > **Note**:
 > **Note**:
 > Docker is careful that its host-wide `iptables` rules fully expose
 > Docker is careful that its host-wide `iptables` rules fully expose

+ 2 - 2
integration-cli/docker_cli_links_test.go

@@ -83,8 +83,8 @@ func TestLinksIpTablesRulesWhenLinkAndUnlink(t *testing.T) {
 	childIP := findContainerIP(t, "child")
 	childIP := findContainerIP(t, "child")
 	parentIP := findContainerIP(t, "parent")
 	parentIP := findContainerIP(t, "parent")
 
 
-	sourceRule := []string{"FORWARD", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", childIP, "--sport", "80", "-d", parentIP, "-j", "ACCEPT"}
-	destinationRule := []string{"FORWARD", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", parentIP, "--dport", "80", "-d", childIP, "-j", "ACCEPT"}
+	sourceRule := []string{"DOCKER", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", childIP, "--sport", "80", "-d", parentIP, "-j", "ACCEPT"}
+	destinationRule := []string{"DOCKER", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-s", parentIP, "--dport", "80", "-d", childIP, "-j", "ACCEPT"}
 	if !iptables.Exists(sourceRule...) || !iptables.Exists(destinationRule...) {
 	if !iptables.Exists(sourceRule...) || !iptables.Exists(destinationRule...) {
 		t.Fatal("Iptables rules not found")
 		t.Fatal("Iptables rules not found")
 	}
 	}

+ 1 - 1
integration-cli/docker_cli_run_test.go

@@ -2029,7 +2029,7 @@ func TestRunDeallocatePortOnMissingIptablesRule(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatal(err)
 		t.Fatal(err)
 	}
 	}
-	iptCmd := exec.Command("iptables", "-D", "FORWARD", "-d", fmt.Sprintf("%s/32", ip),
+	iptCmd := exec.Command("iptables", "-D", "DOCKER", "-d", fmt.Sprintf("%s/32", ip),
 		"!", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-m", "tcp", "--dport", "23", "-j", "ACCEPT")
 		"!", "-i", "docker0", "-o", "docker0", "-p", "tcp", "-m", "tcp", "--dport", "23", "-j", "ACCEPT")
 	out, _, err = runCommandWithOutput(iptCmd)
 	out, _, err = runCommandWithOutput(iptCmd)
 	if err != nil {
 	if err != nil {

+ 3 - 1
links/links.go

@@ -138,7 +138,8 @@ func (l *Link) getDefaultPort() *nat.Port {
 }
 }
 
 
 func (l *Link) Enable() error {
 func (l *Link) Enable() error {
-	if err := l.toggle("-I", false); err != nil {
+	// -A == iptables append flag
+	if err := l.toggle("-A", false); err != nil {
 		return err
 		return err
 	}
 	}
 	l.IsEnabled = true
 	l.IsEnabled = true
@@ -148,6 +149,7 @@ func (l *Link) Enable() error {
 func (l *Link) Disable() {
 func (l *Link) Disable() {
 	// We do not care about errors here because the link may not
 	// We do not care about errors here because the link may not
 	// exist in iptables
 	// exist in iptables
+	// -D == iptables delete flag
 	l.toggle("-D", true)
 	l.toggle("-D", true)
 
 
 	l.IsEnabled = false
 	l.IsEnabled = false

+ 122 - 40
pkg/iptables/iptables.go

@@ -13,14 +13,17 @@ import (
 )
 )
 
 
 type Action string
 type Action string
+type Table string
 
 
 const (
 const (
-	Add    Action = "-A"
+	Append Action = "-A"
 	Delete Action = "-D"
 	Delete Action = "-D"
+	Insert Action = "-I"
+	Nat    Table  = "nat"
+	Filter Table  = "filter"
 )
 )
 
 
 var (
 var (
-	nat                 = []string{"-t", "nat"}
 	supportsXlock       = false
 	supportsXlock       = false
 	ErrIptablesNotFound = errors.New("Iptables not found")
 	ErrIptablesNotFound = errors.New("Iptables not found")
 )
 )
@@ -28,6 +31,7 @@ var (
 type Chain struct {
 type Chain struct {
 	Name   string
 	Name   string
 	Bridge string
 	Bridge string
+	Table  Table
 }
 }
 
 
 type ChainError struct {
 type ChainError struct {
@@ -43,34 +47,74 @@ func init() {
 	supportsXlock = exec.Command("iptables", "--wait", "-L", "-n").Run() == nil
 	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,
 		Name:   name,
 		Bridge: bridge,
 		Bridge: bridge,
+		Table:  table,
+	}
+
+	if string(c.Table) == "" {
+		c.Table = Filter
 	}
 	}
 
 
-	if err := chain.Prerouting(Add, "-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(Add, "-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()
 }
 }
 
 
-func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr string, dest_port int) error {
+// 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, destAddr string, destPort int) error {
 	daddr := ip.String()
 	daddr := ip.String()
 	if ip.IsUnspecified() {
 	if ip.IsUnspecified() {
 		// iptables interprets "0.0.0.0" as "0.0.0.0/32", whereas we
 		// iptables interprets "0.0.0.0" as "0.0.0.0/32", whereas we
@@ -78,39 +122,75 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, dest_addr str
 		// value" by both iptables and ip6tables.
 		// value" by both iptables and ip6tables.
 		daddr = "0/0"
 		daddr = "0/0"
 	}
 	}
-	if output, err := Raw("-t", "nat", fmt.Sprint(action), c.Name,
+	if output, err := Raw("-t", string(Nat), string(action), c.Name,
 		"-p", proto,
 		"-p", proto,
 		"-d", daddr,
 		"-d", daddr,
 		"--dport", strconv.Itoa(port),
 		"--dport", strconv.Itoa(port),
 		"!", "-i", c.Bridge,
 		"!", "-i", c.Bridge,
 		"-j", "DNAT",
 		"-j", "DNAT",
-		"--to-destination", net.JoinHostPort(dest_addr, strconv.Itoa(dest_port))); err != nil {
+		"--to-destination", net.JoinHostPort(destAddr, strconv.Itoa(destPort))); err != nil {
 		return err
 		return err
 	} else if len(output) != 0 {
 	} else if len(output) != 0 {
 		return &ChainError{Chain: "FORWARD", Output: output}
 		return &ChainError{Chain: "FORWARD", Output: output}
 	}
 	}
 
 
-	fAction := action
-	if fAction == Add {
-		fAction = "-I"
-	}
-	if output, err := Raw(string(fAction), "FORWARD",
+	if output, err := Raw("-t", string(Filter), string(action), c.Name,
 		"!", "-i", c.Bridge,
 		"!", "-i", c.Bridge,
 		"-o", c.Bridge,
 		"-o", c.Bridge,
 		"-p", proto,
 		"-p", proto,
-		"-d", dest_addr,
-		"--dport", strconv.Itoa(dest_port),
+		"-d", destAddr,
+		"--dport", strconv.Itoa(destPort),
 		"-j", "ACCEPT"); err != nil {
 		"-j", "ACCEPT"); err != nil {
 		return err
 		return err
 	} else if len(output) != 0 {
 	} else if len(output) != 0 {
 		return &ChainError{Chain: "FORWARD", Output: output}
 		return &ChainError{Chain: "FORWARD", Output: output}
 	}
 	}
 
 
+	if output, err := Raw("-t", string(Nat), string(action), "POSTROUTING",
+		"-p", proto,
+		"-s", destAddr,
+		"-d", destAddr,
+		"--dport", strconv.Itoa(destPort),
+		"-j", "MASQUERADE"); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return &ChainError{Chain: "FORWARD", Output: output}
+	}
+
+	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 output, err := Raw("-t", string(Filter), string(action), c.Name,
+		"-i", c.Bridge, "-o", c.Bridge,
+		"-p", proto,
+		"-s", ip1.String(),
+		"-d", ip2.String(),
+		"--dport", strconv.Itoa(port),
+		"-j", "ACCEPT"); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return fmt.Errorf("Error iptables forward: %s", output)
+	}
+	if output, err := Raw("-t", string(Filter), string(action), c.Name,
+		"-i", c.Bridge, "-o", c.Bridge,
+		"-p", proto,
+		"-s", ip2.String(),
+		"-d", ip1.String(),
+		"--sport", strconv.Itoa(port),
+		"-j", "ACCEPT"); err != nil {
+		return err
+	} else if len(output) != 0 {
+		return fmt.Errorf("Error iptables forward: %s", output)
+	}
 	return nil
 	return nil
 }
 }
 
 
+// Add linking rule to nat/PREROUTING chain.
 func (c *Chain) Prerouting(action Action, args ...string) error {
 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 {
 	if len(args) > 0 {
 		a = append(a, args...)
 		a = append(a, args...)
 	}
 	}
@@ -122,8 +202,9 @@ func (c *Chain) Prerouting(action Action, args ...string) error {
 	return nil
 	return nil
 }
 }
 
 
+// Add linking rule to an OUTPUT chain
 func (c *Chain) Output(action Action, args ...string) error {
 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 {
 	if len(args) > 0 {
 		a = append(a, args...)
 		a = append(a, args...)
 	}
 	}
@@ -137,20 +218,20 @@ func (c *Chain) Output(action Action, args ...string) error {
 
 
 func (c *Chain) Remove() error {
 func (c *Chain) Remove() error {
 	// Ignore errors - This could mean the chains were never set up
 	// 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)
-
-	Raw("-t", "nat", "-F", c.Name)
-	Raw("-t", "nat", "-X", c.Name)
+	if c.Table == Nat {
+		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)
+	}
+	Raw("-t", string(c.Table), "-F", c.Name)
+	Raw("-t", string(c.Table), "-X", c.Name)
 	return nil
 	return nil
 }
 }
 
 
-// Check if an existing rule exists
+// Check if a rule exists
 func Exists(args ...string) bool {
 func Exists(args ...string) bool {
 	// iptables -C, --check option was added in v.1.4.11
 	// iptables -C, --check option was added in v.1.4.11
 	// http://ftp.netfilter.org/pub/iptables/changes-iptables-1.4.11.txt
 	// http://ftp.netfilter.org/pub/iptables/changes-iptables-1.4.11.txt
@@ -175,6 +256,7 @@ func Exists(args ...string) bool {
 	)
 	)
 }
 }
 
 
+// Call 'iptables' system command, passing supplied arguments
 func Raw(args ...string) ([]byte, error) {
 func Raw(args ...string) ([]byte, error) {
 	path, err := exec.LookPath("iptables")
 	path, err := exec.LookPath("iptables")
 	if err != nil {
 	if err != nil {

+ 204 - 0
pkg/iptables/iptables_test.go

@@ -0,0 +1,204 @@
+package iptables
+
+import (
+	"net"
+	"os/exec"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+const chainName = "DOCKERTEST"
+
+var natChain *Chain
+var filterChain *Chain
+
+func TestNewChain(t *testing.T) {
+	var err error
+
+	natChain, err = NewChain(chainName, "lo", Nat)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	filterChain, err = NewChain(chainName, "lo", Filter)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestForward(t *testing.T) {
+	ip := net.ParseIP("192.168.1.1")
+	port := 1234
+	dstAddr := "172.17.0.1"
+	dstPort := 4321
+	proto := "tcp"
+
+	err := natChain.Forward(Insert, ip, port, proto, dstAddr, dstPort)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dnatRule := []string{natChain.Name,
+		"-t", string(natChain.Table),
+		"!", "-i", filterChain.Bridge,
+		"-d", ip.String(),
+		"-p", proto,
+		"--dport", strconv.Itoa(port),
+		"-j", "DNAT",
+		"--to-destination", dstAddr + ":" + strconv.Itoa(dstPort),
+	}
+
+	if !Exists(dnatRule...) {
+		t.Fatalf("DNAT rule does not exist")
+	}
+
+	filterRule := []string{filterChain.Name,
+		"-t", string(filterChain.Table),
+		"!", "-i", filterChain.Bridge,
+		"-o", filterChain.Bridge,
+		"-d", dstAddr,
+		"-p", proto,
+		"--dport", strconv.Itoa(dstPort),
+		"-j", "ACCEPT",
+	}
+
+	if !Exists(filterRule...) {
+		t.Fatalf("filter rule does not exist")
+	}
+
+	masqRule := []string{"POSTROUTING",
+		"-t", string(natChain.Table),
+		"-d", dstAddr,
+		"-s", dstAddr,
+		"-p", proto,
+		"--dport", strconv.Itoa(dstPort),
+		"-j", "MASQUERADE",
+	}
+
+	if !Exists(masqRule...) {
+		t.Fatalf("MASQUERADE rule does not exist")
+	}
+}
+
+func TestLink(t *testing.T) {
+	var err error
+
+	ip1 := net.ParseIP("192.168.1.1")
+	ip2 := net.ParseIP("192.168.1.2")
+	port := 1234
+	proto := "tcp"
+
+	err = filterChain.Link(Append, ip1, ip2, port, proto)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rule1 := []string{filterChain.Name,
+		"-t", string(filterChain.Table),
+		"-i", filterChain.Bridge,
+		"-o", filterChain.Bridge,
+		"-p", proto,
+		"-s", ip1.String(),
+		"-d", ip2.String(),
+		"--dport", strconv.Itoa(port),
+		"-j", "ACCEPT"}
+
+	if !Exists(rule1...) {
+		t.Fatalf("rule1 does not exist")
+	}
+
+	rule2 := []string{filterChain.Name,
+		"-t", string(filterChain.Table),
+		"-i", filterChain.Bridge,
+		"-o", filterChain.Bridge,
+		"-p", proto,
+		"-s", ip2.String(),
+		"-d", ip1.String(),
+		"--sport", strconv.Itoa(port),
+		"-j", "ACCEPT"}
+
+	if !Exists(rule2...) {
+		t.Fatalf("rule2 does not exist")
+	}
+}
+
+func TestPrerouting(t *testing.T) {
+	args := []string{
+		"-i", "lo",
+		"-d", "192.168.1.1"}
+
+	err := natChain.Prerouting(Insert, args...)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rule := []string{"PREROUTING",
+		"-t", string(Nat),
+		"-j", natChain.Name}
+
+	rule = append(rule, args...)
+
+	if !Exists(rule...) {
+		t.Fatalf("rule does not exist")
+	}
+
+	delRule := append([]string{"-D"}, rule...)
+	if _, err = Raw(delRule...); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestOutput(t *testing.T) {
+	args := []string{
+		"-o", "lo",
+		"-d", "192.168.1.1"}
+
+	err := natChain.Output(Insert, args...)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rule := []string{"OUTPUT",
+		"-t", string(natChain.Table),
+		"-j", natChain.Name}
+
+	rule = append(rule, args...)
+
+	if !Exists(rule...) {
+		t.Fatalf("rule does not exist")
+	}
+
+	delRule := append([]string{"-D"}, rule...)
+	if _, err = Raw(delRule...); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestCleanup(t *testing.T) {
+	var err error
+	var rules []byte
+
+	// Cleanup filter/FORWARD first otherwise output of iptables-save is dirty
+	link := []string{"-t", string(filterChain.Table),
+		string(Delete), "FORWARD",
+		"-o", filterChain.Bridge,
+		"-j", filterChain.Name}
+	if _, err = Raw(link...); err != nil {
+		t.Fatal(err)
+	}
+	filterChain.Remove()
+
+	err = RemoveExistingChain(chainName, Nat)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rules, err = exec.Command("iptables-save").Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(string(rules), chainName) {
+		t.Fatalf("Removing chain failed. %s found in iptables-save", chainName)
+	}
+}