Adjust filter iptables chains

- In filter "FORWARD" chain send all docker bridges ingressing packets to DOCKER chain
- In DOCKER chain first jump to a new DOCKER-EXPOSED chain which will contain the
  ACCEPT rules for container's exposed ports. Then return to caller.
- In DOCKER chain second jump to DOCKER-ISOLATION chain which contains the DROP rules
  for inter-network communications and internal networks. Then return to caller.
- The rest of DOCKER chain rules are ACCEPT and DROP for ICC true/false.
  Then return to caller.
- Change the nat Docker chain name to DOCKER-EXPOSED because of the way
  port mapper programs the "forward" rule

Signed-off-by: Alessandro Boch <aboch@docker.com>

drivers/bridge/bridge.go

@@ -112,14 +112,19 @@ type bridgeNetwork struct {
 	sync.Mutex
 }
 
+type iptablesChains struct {
+	nat       *iptables.ChainInfo
+	filter    *iptables.ChainInfo
+	exposed   *iptables.ChainInfo
+	isolation *iptables.ChainInfo
+}
+
 type driver struct {
-	config         *configuration
-	network        *bridgeNetwork
-	natChain       *iptables.ChainInfo
-	filterChain    *iptables.ChainInfo
-	isolationChain *iptables.ChainInfo
-	networks       map[string]*bridgeNetwork
-	store          datastore.DataStore
+	config   *configuration
+	network  *bridgeNetwork
+	chains   iptablesChains
+	networks map[string]*bridgeNetwork
+	store    datastore.DataStore
 	sync.Mutex
 }
 
@@ -252,15 +257,15 @@ func (n *bridgeNetwork) registerIptCleanFunc(clean iptableCleanFunc) {
 	n.iptCleanFuncs = append(n.iptCleanFuncs, clean)
 }
 
-func (n *bridgeNetwork) getDriverChains() (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) {
+func (n *bridgeNetwork) getDriverChains() (*iptablesChains, error) {
 	n.Lock()
 	defer n.Unlock()
 
 	if n.driver == nil {
-		return nil, nil, nil, types.BadRequestErrorf("no driver found")
+		return nil, types.BadRequestErrorf("no driver found")
 	}
 
-	return n.driver.natChain, n.driver.filterChain, n.driver.isolationChain, nil
+	return &n.driver.chains, nil
 }
 
 func (n *bridgeNetwork) getNetworkBridgeName() string {
@@ -354,11 +359,8 @@ func (c *networkConfiguration) conflictsWithNetworks(id string, others []*bridge
 
 func (d *driver) configure(option map[string]interface{}) error {
 	var (
-		config         *configuration
-		err            error
-		natChain       *iptables.ChainInfo
-		filterChain    *iptables.ChainInfo
-		isolationChain *iptables.ChainInfo
+		config *configuration
+		err    error
 	)
 
 	genericData, ok := option[netlabel.GenericData]
@@ -379,29 +381,22 @@ func (d *driver) configure(option map[string]interface{}) error {
 		return &ErrInvalidDriverConfig{}
 	}
 
-	if config.EnableIPForwarding {
-		err = setupIPForwarding()
-		if err != nil {
-			return err
-		}
+	d.Lock()
+	d.config = config
+	d.Unlock()
+
+	err = d.setupIPForwarding()
+	if err != nil {
+		return err
 	}
 
-	if config.EnableIPTables {
-		removeIPChains()
-		natChain, filterChain, isolationChain, err = setupIPChains(config)
-		if err != nil {
-			return err
-		}
-		// Make sure on firewall reload, first thing being re-played is chains creation
-		iptables.OnReloaded(func() { logrus.Debugf("Recreating iptables chains on firewall reload"); setupIPChains(config) })
+	err = d.setupIPChains()
+	if err != nil {
+		return err
 	}
 
-	d.Lock()
-	d.natChain = natChain
-	d.filterChain = filterChain
-	d.isolationChain = isolationChain
-	d.config = config
-	d.Unlock()
+	// Make sure on firewall reload, first thing being re-played is chains creation
+	iptables.OnReloaded(func() { logrus.Debugf("Recreating iptables chains on firewall reload"); d.setupIPChains() })
 
 	err = d.initStore(option)
 	if err != nil {

drivers/bridge/bridge_test.go

@@ -811,13 +811,18 @@ func TestSetDefaultGw(t *testing.T) {
 
 func TestCleanupIptableRules(t *testing.T) {
 	defer testutils.SetupTestOSContext(t)()
+
+	d := newDriver()
+	d.config = &configuration{EnableIPTables: true}
+
 	bridgeChain := []iptables.ChainInfo{
-		{Name: DockerChain, Table: iptables.Nat},
+		{Name: ExposedChain, Table: iptables.Nat},
 		{Name: DockerChain, Table: iptables.Filter},
+		{Name: ExposedChain, Table: iptables.Filter},
 		{Name: IsolationChain, Table: iptables.Filter},
 	}
-	if _, _, _, err := setupIPChains(&configuration{EnableIPTables: true}); err != nil {
-		t.Fatalf("Error setting up ip chains: %v", err)
+	if err := d.setupIPChains(); err != nil {
+		t.Fatal(err)
 	}
 	for _, chainInfo := range bridgeChain {
 		if !iptables.ExistChain(chainInfo.Name, chainInfo.Table) {

drivers/bridge/port_mapping.go

@@ -84,6 +84,10 @@ func (n *bridgeNetwork) allocatePort(bnd *types.PortBinding, containerIP, defHos
 		return err
 	}
 
+	if err := ensureReturnRule(ExposedChain); err != nil {
+		logrus.Warnf("Failed to ensure return rule as last in %s chain: %v", ExposedChain, err)
+	}
+
 	// Save the host port (regardless it was or not specified in the binding)
 	switch netAddr := host.(type) {
 	case *net.TCPAddr:

drivers/bridge/setup_ip_forwarding.go

@@ -10,7 +10,11 @@ const (
 	ipv4ForwardConfPerm = 0644
 )
 
-func setupIPForwarding() error {
+func (d *driver) setupIPForwarding() error {
+	if !d.config.EnableIPForwarding {
+		return nil
+	}
+
 	// Get current IPv4 forward setup
 	ipv4ForwardData, err := ioutil.ReadFile(ipv4ForwardConf)
 	if err != nil {

drivers/bridge/setup_ip_forwarding_test.go

@@ -17,7 +17,8 @@ func TestSetupIPForwarding(t *testing.T) {
 	}
 
 	// Set IP Forwarding
-	if err := setupIPForwarding(); err != nil {
+	d := &driver{config: &configuration{EnableIPForwarding: true}}
+	if err := d.setupIPForwarding(); err != nil {
 		t.Fatalf("Failed to setup IP forwarding: %v", err)
 	}
 

drivers/bridge/setup_ip_tables.go

@@ -13,31 +13,35 @@ import (
 const (
 	DockerChain    = "DOCKER"
 	IsolationChain = "DOCKER-ISOLATION"
+	ExposedChain   = "DOCKER-EXPOSED"
 )
 
-func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) {
-	// Sanity check.
-	if config.EnableIPTables == false {
-		return nil, nil, nil, fmt.Errorf("cannot create new chains, EnableIPTable is disabled")
+func (d *driver) setupIPChains() error {
+	var err error
+
+	if d.config.EnableIPTables == false {
+		return nil
 	}
 
-	hairpinMode := !config.EnableUserlandProxy
+	removeIPChains()
 
-	natChain, err := iptables.NewChain(DockerChain, iptables.Nat, hairpinMode)
+	hairpinMode := !d.config.EnableUserlandProxy
+
+	d.chains.nat, err = iptables.NewChain(ExposedChain, iptables.Nat, hairpinMode)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to create NAT chain: %v", err)
+		return fmt.Errorf("failed to create NAT chain: %v", err)
 	}
 	defer func() {
 		if err != nil {
-			if err := iptables.RemoveExistingChain(DockerChain, iptables.Nat); err != nil {
+			if err := iptables.RemoveExistingChain(ExposedChain, iptables.Nat); err != nil {
 				logrus.Warnf("failed on removing iptables NAT chain on cleanup: %v", err)
 			}
 		}
 	}()
 
-	filterChain, err := iptables.NewChain(DockerChain, iptables.Filter, false)
+	d.chains.filter, err = iptables.NewChain(DockerChain, iptables.Filter, false)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to create FILTER chain: %v", err)
+		return fmt.Errorf("failed to create FILTER chain: %v", err)
 	}
 	defer func() {
 		if err != nil {
@@ -46,17 +50,49 @@ func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainI
 			}
 		}
 	}()
+	if err := ensureReturnRule(DockerChain); err != nil {
+		return err
+	}
 
-	isolationChain, err := iptables.NewChain(IsolationChain, iptables.Filter, false)
+	d.chains.exposed, err = iptables.NewChain(ExposedChain, iptables.Filter, false)
 	if err != nil {
-		return nil, nil, nil, fmt.Errorf("failed to create FILTER isolation chain: %v", err)
+		return fmt.Errorf("failed to create FILTER chain: %v", err)
+	}
+	defer func() {
+		if err != nil {
+			if err := iptables.RemoveExistingChain(ExposedChain, iptables.Filter); err != nil {
+				logrus.Warnf("failed on removing iptables FILTER chain on cleanup: %v", err)
+			}
+		}
+	}()
+	if err := ensureReturnRule(ExposedChain); err != nil {
+		return err
 	}
 
-	if err := addReturnRule(IsolationChain); err != nil {
-		return nil, nil, nil, err
+	d.chains.isolation, err = iptables.NewChain(IsolationChain, iptables.Filter, false)
+	if err != nil {
+		return fmt.Errorf("failed to create FILTER isolation chain: %v", err)
+	}
+	defer func() {
+		if err != nil {
+			if err := iptables.RemoveExistingChain(IsolationChain, iptables.Filter); err != nil {
+				logrus.Warnf("failed on removing iptables FILTER chain on cleanup: %v", err)
+			}
+		}
+	}()
+	if err = ensureReturnRule(IsolationChain); err != nil {
+		return err
+	}
+
+	if err := ensureJumpRule(DockerChain, IsolationChain); err != nil {
+		return err
+	}
+
+	if err := ensureJumpRule(DockerChain, ExposedChain); err != nil {
+		return err
 	}
 
-	return natChain, filterChain, isolationChain, nil
+	return nil
 }
 
 func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInterface) error {
@@ -96,32 +132,31 @@ func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInt
 		n.registerIptCleanFunc(func() error {
 			return setupIPTablesInternal(config.BridgeName, maskedAddrv4, config.EnableICC, config.EnableIPMasquerade, hairpinMode, false)
 		})
-		natChain, filterChain, _, err := n.getDriverChains()
+		chains, err := n.getDriverChains()
 		if err != nil {
 			return fmt.Errorf("Failed to setup IP tables, cannot acquire chain info %s", err.Error())
 		}
 
-		err = iptables.ProgramChain(natChain, config.BridgeName, hairpinMode, true)
+		err = iptables.ProgramChain(chains.nat, config.BridgeName, hairpinMode, true)
 		if err != nil {
 			return fmt.Errorf("Failed to program NAT chain: %s", err.Error())
 		}
 
-		err = iptables.ProgramChain(filterChain, config.BridgeName, hairpinMode, true)
+		err = iptables.ProgramChain(chains.filter, config.BridgeName, hairpinMode, true)
 		if err != nil {
 			return fmt.Errorf("Failed to program FILTER chain: %s", err.Error())
 		}
 
 		n.registerIptCleanFunc(func() error {
-			return iptables.ProgramChain(filterChain, config.BridgeName, hairpinMode, false)
+			return iptables.ProgramChain(chains.filter, config.BridgeName, hairpinMode, false)
 		})
 
-		n.portMapper.SetIptablesChain(natChain, n.getNetworkBridgeName())
+		n.portMapper.SetIptablesChain(chains.exposed, n.getNetworkBridgeName())
 	}
 
-	if err := ensureJumpRule("FORWARD", IsolationChain); err != nil {
+	if err := ensureReturnRule(DockerChain); err != nil {
 		return err
 	}
-
 	return nil
 }
 
@@ -138,9 +173,9 @@ func setupIPTablesInternal(bridgeIface string, addr net.Addr, icc, ipmasq, hairp
 		address   = addr.String()
 		natRule   = iptRule{table: iptables.Nat, chain: "POSTROUTING", preArgs: []string{"-t", "nat"}, args: []string{"-s", address, "!", "-o", bridgeIface, "-j", "MASQUERADE"}}
 		hpNatRule = iptRule{table: iptables.Nat, chain: "POSTROUTING", preArgs: []string{"-t", "nat"}, args: []string{"-m", "addrtype", "--src-type", "LOCAL", "-o", bridgeIface, "-j", "MASQUERADE"}}
-		skipDNAT  = iptRule{table: iptables.Nat, chain: DockerChain, preArgs: []string{"-t", "nat"}, args: []string{"-i", bridgeIface, "-j", "RETURN"}}
-		outRule   = iptRule{table: iptables.Filter, chain: "FORWARD", args: []string{"-i", bridgeIface, "!", "-o", bridgeIface, "-j", "ACCEPT"}}
-		inRule    = iptRule{table: iptables.Filter, chain: "FORWARD", args: []string{"-o", bridgeIface, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"}}
+		skipDNAT  = iptRule{table: iptables.Nat, chain: ExposedChain, preArgs: []string{"-t", "nat"}, args: []string{"-i", bridgeIface, "-j", "RETURN"}}
+		outRule   = iptRule{table: iptables.Filter, chain: DockerChain, args: []string{"-i", bridgeIface, "!", "-o", bridgeIface, "-j", "ACCEPT"}}
+		inRule    = iptRule{table: iptables.Filter, chain: DockerChain, args: []string{"-o", bridgeIface, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"}}
 	)
 
 	// Set NAT.
@@ -191,7 +226,7 @@ func programChainRule(rule iptRule, ruleDescr string, insert bool) error {
 
 	if insert {
 		condition = !doesExist
-		prefix = []string{"-I", rule.chain}
+		prefix = []string{"-A", rule.chain}
 		operation = "enable"
 	} else {
 		condition = doesExist
@@ -214,7 +249,7 @@ func programChainRule(rule iptRule, ruleDescr string, insert bool) error {
 func setIcc(bridgeIface string, iccEnable, insert bool) error {
 	var (
 		table      = iptables.Filter
-		chain      = "FORWARD"
+		chain      = DockerChain
 		args       = []string{"-i", bridgeIface, "-o", bridgeIface, "-j"}
 		acceptArgs = append(args, "ACCEPT")
 		dropArgs   = append(args, "DROP")
@@ -233,7 +268,7 @@ func setIcc(bridgeIface string, iccEnable, insert bool) error {
 			iptables.Raw(append([]string{"-D", chain}, dropArgs...)...)
 
 			if !iptables.Exists(table, chain, acceptArgs...) {
-				if err := iptables.RawCombinedOutput(append([]string{"-I", chain}, acceptArgs...)...); err != nil {
+				if err := iptables.RawCombinedOutput(append([]string{"-A", chain}, acceptArgs...)...); err != nil {
 					return fmt.Errorf("Unable to allow intercontainer communication: %s", err.Error())
 				}
 			}
@@ -285,17 +320,21 @@ func setINC(iface1, iface2 string, enable bool) error {
 	return nil
 }
 
-func addReturnRule(chain string) error {
+// Ensures the return rule is at the bottom of the chain
+func ensureReturnRule(chain string) error {
 	var (
 		table = iptables.Filter
 		args  = []string{"-j", "RETURN"}
 	)
 
 	if iptables.Exists(table, chain, args...) {
-		return nil
+		err := iptables.RawCombinedOutput(append([]string{"-D", chain}, args...)...)
+		if err != nil {
+			return fmt.Errorf("unable to remove return rule in %s chain: %s", chain, err.Error())
+		}
 	}
 
-	err := iptables.RawCombinedOutput(append([]string{"-I", chain}, args...)...)
+	err := iptables.RawCombinedOutput(append([]string{"-A", chain}, args...)...)
 	if err != nil {
 		return fmt.Errorf("unable to add return rule in %s chain: %s", chain, err.Error())
 	}
@@ -327,12 +366,21 @@ func ensureJumpRule(fromChain, toChain string) error {
 
 func removeIPChains() {
 	for _, chainInfo := range []iptables.ChainInfo{
-		{Name: DockerChain, Table: iptables.Nat},
+		{Name: ExposedChain, Table: iptables.Nat},
 		{Name: DockerChain, Table: iptables.Filter},
 		{Name: IsolationChain, Table: iptables.Filter},
+		{Name: ExposedChain, Table: iptables.Filter},
 	} {
 		if err := chainInfo.Remove(); err != nil {
-			logrus.Warnf("Failed to remove existing iptables entries in table %s chain %s : %v", chainInfo.Table, chainInfo.Name, err)
+			logrus.Warnf("Failed to remove existing iptables entries in table %s chain %s: %v", chainInfo.Table, chainInfo.Name, err)
+		}
+	}
+	// Also remove docker 1.9 & 1.10 `jump to isolation chain` rule from FORWARD chain
+	var args = []string{"-j", IsolationChain}
+	if iptables.Exists(iptables.Filter, "FORWARD", args...) {
+		err := iptables.RawCombinedOutput(append([]string{"-D", "FORWARD"}, args...)...)
+		if err != nil {
+			logrus.Warnf("Unable to remove old jump rule in FORWARD chain: %s", err.Error())
 		}
 	}
 }

drivers/bridge/setup_ip_tables_test.go

@@ -104,7 +104,7 @@ func assertIPTableChainProgramming(rule iptRule, descr string, t *testing.T) {
 func assertChainConfig(d *driver, t *testing.T) {
 	var err error
 
-	d.natChain, d.filterChain, d.isolationChain, err = setupIPChains(d.config)
+	d.setupIPChains()
 	if err != nil {
 		t.Fatal(err)
 	}