diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 58dc825e9f..eaaf3516a8 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -137,15 +137,16 @@ type bridgeNetwork struct { } type driver struct { - config *configuration - network *bridgeNetwork - natChain *iptables.ChainInfo - filterChain *iptables.ChainInfo - isolationChain *iptables.ChainInfo - networks map[string]*bridgeNetwork - store datastore.DataStore - nlh *netlink.Handle - configNetwork sync.Mutex + config *configuration + network *bridgeNetwork + natChain *iptables.ChainInfo + filterChain *iptables.ChainInfo + isolationChain1 *iptables.ChainInfo + isolationChain2 *iptables.ChainInfo + networks map[string]*bridgeNetwork + store datastore.DataStore + nlh *netlink.Handle + configNetwork sync.Mutex sync.Mutex } @@ -266,15 +267,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() (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) { n.Lock() defer n.Unlock() if n.driver == nil { - return nil, nil, nil, types.BadRequestErrorf("no driver found") + return nil, nil, nil, nil, types.BadRequestErrorf("no driver found") } - return n.driver.natChain, n.driver.filterChain, n.driver.isolationChain, nil + return n.driver.natChain, n.driver.filterChain, n.driver.isolationChain1, n.driver.isolationChain2, nil } func (n *bridgeNetwork) getNetworkBridgeName() string { @@ -311,21 +312,9 @@ func (n *bridgeNetwork) isolateNetwork(others []*bridgeNetwork, enable bool) err return nil } - // Install the rules to isolate this networks against each of the other networks - for _, o := range others { - o.Lock() - otherConfig := o.config - o.Unlock() - - if otherConfig.Internal { - continue - } - - if thisConfig.BridgeName != otherConfig.BridgeName { - if err := setINC(thisConfig.BridgeName, otherConfig.BridgeName, enable); err != nil { - return err - } - } + // Install the rules to isolate this network against each of the other networks + if err := setINC(thisConfig.BridgeName, enable); err != nil { + return err } return nil @@ -333,11 +322,12 @@ func (n *bridgeNetwork) isolateNetwork(others []*bridgeNetwork, enable bool) err 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 + natChain *iptables.ChainInfo + filterChain *iptables.ChainInfo + isolationChain1 *iptables.ChainInfo + isolationChain2 *iptables.ChainInfo ) genericData, ok := option[netlabel.GenericData] @@ -365,7 +355,7 @@ func (d *driver) configure(option map[string]interface{}) error { } } removeIPChains() - natChain, filterChain, isolationChain, err = setupIPChains(config) + natChain, filterChain, isolationChain1, isolationChain2, err = setupIPChains(config) if err != nil { return err } @@ -384,7 +374,8 @@ func (d *driver) configure(option map[string]interface{}) error { d.Lock() d.natChain = natChain d.filterChain = filterChain - d.isolationChain = isolationChain + d.isolationChain1 = isolationChain1 + d.isolationChain2 = isolationChain2 d.config = config d.Unlock() diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index a9e49bd2c5..e2f073fef6 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/drivers/bridge/bridge_test.go @@ -425,14 +425,15 @@ func TestCreateMultipleNetworks(t *testing.T) { t.Fatalf("Failed to create bridge: %v", err) } + verifyV4INCEntries(d.networks, t) + config2 := &networkConfiguration{BridgeName: "net_test_2"} genericOption[netlabel.GenericData] = config2 if err := d.CreateNetwork("2", genericOption, nil, getIPv4Data(t, ""), nil); err != nil { t.Fatalf("Failed to create bridge: %v", err) } - // Verify the network isolation rules are installed, each network subnet should appear 2 times - verifyV4INCEntries(d.networks, 2, t) + verifyV4INCEntries(d.networks, t) config3 := &networkConfiguration{BridgeName: "net_test_3"} genericOption[netlabel.GenericData] = config3 @@ -440,8 +441,7 @@ func TestCreateMultipleNetworks(t *testing.T) { t.Fatalf("Failed to create bridge: %v", err) } - // Verify the network isolation rules are installed, each network subnet should appear 4 times - verifyV4INCEntries(d.networks, 6, t) + verifyV4INCEntries(d.networks, t) config4 := &networkConfiguration{BridgeName: "net_test_4"} genericOption[netlabel.GenericData] = config4 @@ -449,45 +449,43 @@ func TestCreateMultipleNetworks(t *testing.T) { t.Fatalf("Failed to create bridge: %v", err) } - // Now 6 times - verifyV4INCEntries(d.networks, 12, t) + verifyV4INCEntries(d.networks, t) d.DeleteNetwork("1") - verifyV4INCEntries(d.networks, 6, t) + verifyV4INCEntries(d.networks, t) d.DeleteNetwork("2") - verifyV4INCEntries(d.networks, 2, t) + verifyV4INCEntries(d.networks, t) d.DeleteNetwork("3") - verifyV4INCEntries(d.networks, 0, t) + verifyV4INCEntries(d.networks, t) d.DeleteNetwork("4") - verifyV4INCEntries(d.networks, 0, t) + verifyV4INCEntries(d.networks, t) } -func verifyV4INCEntries(networks map[string]*bridgeNetwork, numEntries int, t *testing.T) { - out, err := iptables.Raw("-nvL", IsolationChain) +// Verify the network isolation rules are installed for each network +func verifyV4INCEntries(networks map[string]*bridgeNetwork, t *testing.T) { + out1, err := iptables.Raw("-S", IsolationChain1) + if err != nil { + t.Fatal(err) + } + out2, err := iptables.Raw("-S", IsolationChain2) if err != nil { t.Fatal(err) } - found := 0 - for _, x := range networks { - for _, y := range networks { - if x == y { - continue - } - re := regexp.MustCompile(x.config.BridgeName + " " + y.config.BridgeName) - matches := re.FindAllString(string(out[:]), -1) - if len(matches) != 1 { - t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables:\n%s", string(out[:])) - } - found++ + for _, n := range networks { + re := regexp.MustCompile(fmt.Sprintf("-i %s ! -o %s -j %s", n.config.BridgeName, n.config.BridgeName, IsolationChain2)) + matches := re.FindAllString(string(out1[:]), -1) + if len(matches) != 1 { + t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables for network %s:\n%s.", n.id, string(out1[:])) + } + re = regexp.MustCompile(fmt.Sprintf("-o %s -j DROP", n.config.BridgeName)) + matches = re.FindAllString(string(out2[:]), -1) + if len(matches) != 1 { + t.Fatalf("Cannot find expected inter-network isolation rules in IP Tables for network %s:\n%s.", n.id, string(out2[:])) } - } - - if found != numEntries { - t.Fatalf("Cannot find expected number (%d) of inter-network isolation rules in IP Tables:\n%s\nFound %d", numEntries, string(out[:]), found) } } @@ -996,9 +994,9 @@ func TestCleanupIptableRules(t *testing.T) { bridgeChain := []iptables.ChainInfo{ {Name: DockerChain, Table: iptables.Nat}, {Name: DockerChain, Table: iptables.Filter}, - {Name: IsolationChain, Table: iptables.Filter}, + {Name: IsolationChain1, Table: iptables.Filter}, } - if _, _, _, err := setupIPChains(&configuration{EnableIPTables: true}); err != nil { + if _, _, _, _, err := setupIPChains(&configuration{EnableIPTables: true}); err != nil { t.Fatalf("Error setting up ip chains: %v", err) } for _, chainInfo := range bridgeChain { diff --git a/libnetwork/drivers/bridge/setup_ip_tables.go b/libnetwork/drivers/bridge/setup_ip_tables.go index da654449d2..97bf95d82b 100644 --- a/libnetwork/drivers/bridge/setup_ip_tables.go +++ b/libnetwork/drivers/bridge/setup_ip_tables.go @@ -12,21 +12,31 @@ import ( // DockerChain: DOCKER iptable chain name const ( - DockerChain = "DOCKER" - IsolationChain = "DOCKER-ISOLATION" + DockerChain = "DOCKER" + // Isolation between bridge networks is achieved in two stages by means + // of the following two chains in the filter table. The first chain matches + // on the source interface being a bridge network's bridge and the + // destination being a different interface. A positive match leads to the + // second isolation chain. No match returns to the parent chain. The second + // isolation chain matches on destination interface being a bridge network's + // bridge. A positive match identifies a packet originated from one bridge + // network's bridge destined to another bridge network's bridge and will + // result in the packet being dropped. No match returns to the parent chain. + IsolationChain1 = "DOCKER-ISOLATION-STAGE-1" + IsolationChain2 = "DOCKER-ISOLATION-STAGE-2" ) -func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) { +func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, *iptables.ChainInfo, error) { // Sanity check. if config.EnableIPTables == false { - return nil, nil, nil, errors.New("cannot create new chains, EnableIPTable is disabled") + return nil, nil, nil, nil, errors.New("cannot create new chains, EnableIPTable is disabled") } hairpinMode := !config.EnableUserlandProxy natChain, err := iptables.NewChain(DockerChain, iptables.Nat, hairpinMode) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to create NAT chain: %v", err) + return nil, nil, nil, nil, fmt.Errorf("failed to create NAT chain: %v", err) } defer func() { if err != nil { @@ -38,7 +48,7 @@ func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainI filterChain, err := iptables.NewChain(DockerChain, iptables.Filter, false) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to create FILTER chain: %v", err) + return nil, nil, nil, nil, fmt.Errorf("failed to create FILTER chain: %v", err) } defer func() { if err != nil { @@ -48,16 +58,25 @@ func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainI } }() - isolationChain, err := iptables.NewChain(IsolationChain, iptables.Filter, false) + isolationChain1, err := iptables.NewChain(IsolationChain1, iptables.Filter, false) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to create FILTER isolation chain: %v", err) + return nil, nil, nil, nil, fmt.Errorf("failed to create FILTER isolation chain: %v", err) } - if err := iptables.AddReturnRule(IsolationChain); err != nil { - return nil, nil, nil, err + isolationChain2, err := iptables.NewChain(IsolationChain2, iptables.Filter, false) + if err != nil { + return nil, nil, nil, nil, fmt.Errorf("failed to create FILTER isolation chain: %v", err) } - return natChain, filterChain, isolationChain, nil + if err := iptables.AddReturnRule(IsolationChain1); err != nil { + return nil, nil, nil, nil, err + } + + if err := iptables.AddReturnRule(IsolationChain2); err != nil { + return nil, nil, nil, nil, err + } + + return natChain, filterChain, isolationChain1, isolationChain2, nil } func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInterface) error { @@ -94,7 +113,7 @@ 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() + natChain, filterChain, _, _, err := n.getDriverChains() if err != nil { return fmt.Errorf("Failed to setup IP tables, cannot acquire chain info %s", err.Error()) } @@ -117,7 +136,7 @@ func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInt } d.Lock() - err = iptables.EnsureJumpRule("FORWARD", IsolationChain) + err = iptables.EnsureJumpRule("FORWARD", IsolationChain1) d.Unlock() if err != nil { return err @@ -245,42 +264,56 @@ func setIcc(bridgeIface string, iccEnable, insert bool) error { return nil } -// Control Inter Network Communication. Install/remove only if it is not/is present. -func setINC(iface1, iface2 string, enable bool) error { +// Control Inter Network Communication. Install[Remove] only if it is [not] present. +func setINC(iface string, enable bool) error { var ( - table = iptables.Filter - chain = IsolationChain - args = [2][]string{{"-i", iface1, "-o", iface2, "-j", "DROP"}, {"-i", iface2, "-o", iface1, "-j", "DROP"}} + action = iptables.Insert + actionMsg = "add" + chains = []string{IsolationChain1, IsolationChain2} + rules = [][]string{ + {"-i", iface, "!", "-o", iface, "-j", IsolationChain2}, + {"-o", iface, "-j", "DROP"}, + } ) - if enable { - for i := 0; i < 2; i++ { - if iptables.Exists(table, chain, args[i]...) { - continue - } - if err := iptables.RawCombinedOutput(append([]string{"-I", chain}, args[i]...)...); err != nil { - return fmt.Errorf("unable to add inter-network communication rule: %v", err) - } - } - } else { - for i := 0; i < 2; i++ { - if !iptables.Exists(table, chain, args[i]...) { - continue - } - if err := iptables.RawCombinedOutput(append([]string{"-D", chain}, args[i]...)...); err != nil { - return fmt.Errorf("unable to remove inter-network communication rule: %v", err) + if !enable { + action = iptables.Delete + actionMsg = "remove" + } + + for i, chain := range chains { + if err := iptables.ProgramRule(iptables.Filter, chain, action, rules[i]); err != nil { + msg := fmt.Sprintf("unable to %s inter-network communication rule: %v", actionMsg, err) + if enable { + if i == 1 { + // Rollback the rule installed on first chain + if err2 := iptables.ProgramRule(iptables.Filter, chains[0], iptables.Delete, rules[0]); err2 != nil { + logrus.Warn("Failed to rollback iptables rule after failure (%v): %v", err, err2) + } + } + return fmt.Errorf(msg) } + logrus.Warn(msg) } } return nil } +// Obsolete chain from previous docker versions +const oldIsolationChain = "DOCKER-ISOLATION" + func removeIPChains() { + // Remove obsolete rules from default chains + iptables.ProgramRule(iptables.Filter, "FORWARD", iptables.Delete, []string{"-j", oldIsolationChain}) + + // Remove chains for _, chainInfo := range []iptables.ChainInfo{ {Name: DockerChain, Table: iptables.Nat}, {Name: DockerChain, Table: iptables.Filter}, - {Name: IsolationChain, Table: iptables.Filter}, + {Name: IsolationChain1, Table: iptables.Filter}, + {Name: IsolationChain2, Table: iptables.Filter}, + {Name: oldIsolationChain, 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) @@ -290,8 +323,8 @@ func removeIPChains() { func setupInternalNetworkRules(bridgeIface string, addr net.Addr, icc, insert bool) error { var ( - inDropRule = iptRule{table: iptables.Filter, chain: IsolationChain, args: []string{"-i", bridgeIface, "!", "-d", addr.String(), "-j", "DROP"}} - outDropRule = iptRule{table: iptables.Filter, chain: IsolationChain, args: []string{"-o", bridgeIface, "!", "-s", addr.String(), "-j", "DROP"}} + inDropRule = iptRule{table: iptables.Filter, chain: IsolationChain1, args: []string{"-i", bridgeIface, "!", "-d", addr.String(), "-j", "DROP"}} + outDropRule = iptRule{table: iptables.Filter, chain: IsolationChain1, args: []string{"-o", bridgeIface, "!", "-s", addr.String(), "-j", "DROP"}} ) if err := programChainRule(inDropRule, "DROP INCOMING", insert); err != nil { return err diff --git a/libnetwork/drivers/bridge/setup_ip_tables_test.go b/libnetwork/drivers/bridge/setup_ip_tables_test.go index 0faaa01f59..ea8e5da16d 100644 --- a/libnetwork/drivers/bridge/setup_ip_tables_test.go +++ b/libnetwork/drivers/bridge/setup_ip_tables_test.go @@ -116,7 +116,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.natChain, d.filterChain, d.isolationChain1, d.isolationChain2, err = setupIPChains(d.config) if err != nil { t.Fatal(err) }