From 0b341811fc7b82bdce253ebe04e5f5e2f859bf0f Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 3 Apr 2018 16:50:00 +0900 Subject: [PATCH] update libnetwork to improve scalabiltiy of bridge network isolation rules * libnetwork#2121: Retry other external DNS servers on ServFail * libnetwork#2125: Fix README flag and expose orphan network peers * libnetwork#2126: Adding goreport card * libnetwork#2130: Modify awk to use cut in check_ip_overlap * libnetwork#2117: [Carry 1534] Improve scalabiltiy of bridge network isolation rules Full changes: https://github.com/docker/libnetwork/compare/2bf63300c52f5ea61989f85c732f00097d746530...5c1218c956c99f3365711974e300087810c31379 Signed-off-by: Akihiro Suda Upstream-commit: b159da19734269c4a162763ebfa28dff07b703f3 Component: engine --- .../hack/dockerfile/install/proxy.installer | 2 +- components/engine/vendor.conf | 2 +- .../github.com/docker/libnetwork/README.md | 2 +- .../libnetwork/drivers/bridge/bridge.go | 61 ++++----- .../drivers/bridge/setup_ip_tables.go | 125 ++++++++++++------ .../docker/libnetwork/networkdb/networkdb.go | 2 +- .../networkdb/networkdbdiagnostic.go | 6 +- .../github.com/docker/libnetwork/resolver.go | 8 +- 8 files changed, 125 insertions(+), 83 deletions(-) diff --git a/components/engine/hack/dockerfile/install/proxy.installer b/components/engine/hack/dockerfile/install/proxy.installer index bc2f92a63a..d66a5ffe40 100755 --- a/components/engine/hack/dockerfile/install/proxy.installer +++ b/components/engine/hack/dockerfile/install/proxy.installer @@ -3,7 +3,7 @@ # LIBNETWORK_COMMIT is used to build the docker-userland-proxy binary. When # updating the binary version, consider updating github.com/docker/libnetwork # in vendor.conf accordingly -LIBNETWORK_COMMIT=1b91bc94094ecfdae41daa465cc0c8df37dfb3dd +LIBNETWORK_COMMIT=5c1218c956c99f3365711974e300087810c31379 install_proxy() { case "$1" in diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index f983706f6b..8715bead17 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -32,7 +32,7 @@ github.com/tonistiigi/fsutil dea3a0da73aee887fc02142d995be764106ac5e2 #get libnetwork packages # When updating, also update LIBNETWORK_COMMIT in hack/dockerfile/install/proxy accordingly -github.com/docker/libnetwork 2bf63300c52f5ea61989f85c732f00097d746530 +github.com/docker/libnetwork 5c1218c956c99f3365711974e300087810c31379 github.com/docker/go-events 9461782956ad83b30282bf90e31fa6a70c255ba9 github.com/armon/go-radix e39d623f12e8e41c7b5529e9a9dd67a1e2261f80 github.com/armon/go-metrics eb0af217e5e9747e41dd5303755356b62d28e3ec diff --git a/components/engine/vendor/github.com/docker/libnetwork/README.md b/components/engine/vendor/github.com/docker/libnetwork/README.md index 536f8aa2b3..981b62bb1c 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/README.md +++ b/components/engine/vendor/github.com/docker/libnetwork/README.md @@ -1,6 +1,6 @@ # libnetwork - networking for containers -[![Circle CI](https://circleci.com/gh/docker/libnetwork/tree/master.svg?style=svg)](https://circleci.com/gh/docker/libnetwork/tree/master) [![Coverage Status](https://coveralls.io/repos/docker/libnetwork/badge.svg)](https://coveralls.io/r/docker/libnetwork) [![GoDoc](https://godoc.org/github.com/docker/libnetwork?status.svg)](https://godoc.org/github.com/docker/libnetwork) +[![Circle CI](https://circleci.com/gh/docker/libnetwork/tree/master.svg?style=svg)](https://circleci.com/gh/docker/libnetwork/tree/master) [![Coverage Status](https://coveralls.io/repos/docker/libnetwork/badge.svg)](https://coveralls.io/r/docker/libnetwork) [![GoDoc](https://godoc.org/github.com/docker/libnetwork?status.svg)](https://godoc.org/github.com/docker/libnetwork) [![Go Report Card](https://goreportcard.com/badge/github.com/docker/libnetwork)](https://goreportcard.com/report/github.com/docker/libnetwork) Libnetwork provides a native Go implementation for connecting containers diff --git a/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/bridge.go b/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/bridge.go index 58dc825e9f..783d45c113 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/bridge.go +++ b/components/engine/vendor/github.com/docker/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,33 +312,18 @@ 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 - } - } - } - - return nil + // Install the rules to isolate this network against each of the other networks + return setINC(thisConfig.BridgeName, enable) } 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 +351,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 +370,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/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/setup_ip_tables.go b/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/setup_ip_tables.go index da654449d2..5865a18f18 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/setup_ip_tables.go +++ b/components/engine/vendor/github.com/docker/libnetwork/drivers/bridge/setup_ip_tables.go @@ -12,52 +12,85 @@ 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 %s: %v", DockerChain, err) } defer func() { if err != nil { if err := iptables.RemoveExistingChain(DockerChain, iptables.Nat); err != nil { - logrus.Warnf("failed on removing iptables NAT chain on cleanup: %v", err) + logrus.Warnf("failed on removing iptables NAT chain %s on cleanup: %v", DockerChain, err) } } }() 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 %s: %v", DockerChain, err) } defer func() { if err != nil { if err := iptables.RemoveExistingChain(DockerChain, iptables.Filter); err != nil { - logrus.Warnf("failed on removing iptables FILTER chain on cleanup: %v", err) + logrus.Warnf("failed on removing iptables FILTER chain %s on cleanup: %v", DockerChain, err) } } }() - 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) + } + defer func() { + if err != nil { + if err := iptables.RemoveExistingChain(IsolationChain1, iptables.Filter); err != nil { + logrus.Warnf("failed on removing iptables FILTER chain %s on cleanup: %v", IsolationChain1, 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) + } + defer func() { + if err != nil { + if err := iptables.RemoveExistingChain(IsolationChain2, iptables.Filter); err != nil { + logrus.Warnf("failed on removing iptables FILTER chain %s on cleanup: %v", IsolationChain2, err) + } + } + }() + + if err := iptables.AddReturnRule(IsolationChain1); err != nil { + return nil, nil, nil, nil, err } - if err := iptables.AddReturnRule(IsolationChain); err != nil { - return nil, nil, nil, err + if err := iptables.AddReturnRule(IsolationChain2); err != nil { + return nil, nil, nil, nil, err } - return natChain, filterChain, isolationChain, nil + return natChain, filterChain, isolationChain1, isolationChain2, nil } func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInterface) error { @@ -94,7 +127,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 +150,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 +278,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 +337,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/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdb.go b/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdb.go index d84f96267f..d1fa3b8d86 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdb.go +++ b/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdb.go @@ -313,7 +313,7 @@ func (nDB *NetworkDB) Peers(nid string) []PeerInfo { } else { // Added for testing purposes, this condition should never happen else mean that the network list // is out of sync with the node list - peers = append(peers, PeerInfo{}) + peers = append(peers, PeerInfo{Name: nodeName, IP: "unknown"}) } } return peers diff --git a/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdbdiagnostic.go b/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdbdiagnostic.go index 97516d4c7a..ffeb98d607 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdbdiagnostic.go +++ b/components/engine/vendor/github.com/docker/libnetwork/networkdb/networkdbdiagnostic.go @@ -84,7 +84,11 @@ func dbPeers(ctx interface{}, w http.ResponseWriter, r *http.Request) { peers := nDB.Peers(r.Form["nid"][0]) rsp := &diagnostic.TableObj{Length: len(peers)} for i, peerInfo := range peers { - rsp.Elements = append(rsp.Elements, &diagnostic.PeerEntryObj{Index: i, Name: peerInfo.Name, IP: peerInfo.IP}) + if peerInfo.IP == "unknown" { + rsp.Elements = append(rsp.Elements, &diagnostic.PeerEntryObj{Index: i, Name: "orphan-" + peerInfo.Name, IP: peerInfo.IP}) + } else { + rsp.Elements = append(rsp.Elements, &diagnostic.PeerEntryObj{Index: i, Name: peerInfo.Name, IP: peerInfo.IP}) + } } log.WithField("response", fmt.Sprintf("%+v", rsp)).Info("network peers done") diagnostic.HTTPReply(w, diagnostic.CommandSucceed(rsp), json) diff --git a/components/engine/vendor/github.com/docker/libnetwork/resolver.go b/components/engine/vendor/github.com/docker/libnetwork/resolver.go index 64c73e53b7..e420905350 100644 --- a/components/engine/vendor/github.com/docker/libnetwork/resolver.go +++ b/components/engine/vendor/github.com/docker/libnetwork/resolver.go @@ -452,7 +452,6 @@ func (r *resolver) ServeDNS(w dns.ResponseWriter, query *dns.Msg) { logrus.Warnf("[resolver] connect failed: %s", err) continue } - queryType := dns.TypeToString[query.Question[0].Qtype] logrus.Debugf("[resolver] query %s (%s) from %s, forwarding to %s:%s", name, queryType, extConn.LocalAddr().String(), proto, extDNS.IPStr) @@ -492,6 +491,11 @@ func (r *resolver) ServeDNS(w dns.ResponseWriter, query *dns.Msg) { } r.forwardQueryEnd() if resp != nil { + if resp.Rcode == dns.RcodeServerFailure { + // for Server Failure response, continue to the next external DNS server + logrus.Debugf("[resolver] external DNS %s:%s responded with ServFail for %q", proto, extDNS.IPStr, name) + continue + } answers := 0 for _, rr := range resp.Answer { h := rr.Header() @@ -511,10 +515,10 @@ func (r *resolver) ServeDNS(w dns.ResponseWriter, query *dns.Msg) { if resp.Answer == nil || answers == 0 { logrus.Debugf("[resolver] external DNS %s:%s did not return any %s records for %q", proto, extDNS.IPStr, queryType, name) } + resp.Compress = true } else { logrus.Debugf("[resolver] external DNS %s:%s returned empty response for %q", proto, extDNS.IPStr, name) } - resp.Compress = true break } if resp == nil {