From 12df37fdd0ef6866d46fda029650bd6a2d9bef3e Mon Sep 17 00:00:00 2001 From: Mohammad Banikazemi Date: Thu, 11 Jun 2015 18:12:00 -0700 Subject: [PATCH] Seperates the driver-specific and network-specific iptable operations for the bridge driver. Moves two config options, namely EnableIPTables and EnableUserlandProxy from networks to the driver. Closes #242 Signed-off-by: Mohammad Banikazemi --- libnetwork/api/api_test.go | 10 +++ libnetwork/drivers/bridge/bridge.go | 73 +++++++++++------ libnetwork/drivers/bridge/bridge_test.go | 78 +++++++++++++++---- libnetwork/drivers/bridge/link.go | 4 +- libnetwork/drivers/bridge/network_test.go | 16 ++++ .../drivers/bridge/port_mapping_test.go | 13 +++- libnetwork/drivers/bridge/setup_firewalld.go | 7 +- libnetwork/drivers/bridge/setup_ip_tables.go | 54 +++++++++++-- .../drivers/bridge/setup_ip_tables_test.go | 39 +++++++--- libnetwork/iptables/firewalld_test.go | 15 ++-- libnetwork/iptables/iptables.go | 59 ++++++++------ libnetwork/iptables/iptables_test.go | 36 +++++---- libnetwork/libnetwork_test.go | 3 +- libnetwork/portmapper/mapper.go | 8 +- libnetwork/portmapper/mapper_test.go | 7 +- 15 files changed, 303 insertions(+), 119 deletions(-) diff --git a/libnetwork/api/api_test.go b/libnetwork/api/api_test.go index f4e80df861..8f1c48f99e 100644 --- a/libnetwork/api/api_test.go +++ b/libnetwork/api/api_test.go @@ -1680,6 +1680,11 @@ func TestHttpHandlerUninit(t *testing.T) { t.Fatal(err) } + err = c.ConfigureNetworkDriver(bridgeNetType, nil) + if err != nil { + t.Fatal(err) + } + h := &httpHandler{c: c} h.initRouter() if h.r == nil { @@ -1777,6 +1782,11 @@ func TestEndToEnd(t *testing.T) { if err != nil { t.Fatal(err) } + err = c.ConfigureNetworkDriver(bridgeNetType, nil) + if err != nil { + t.Fatal(err) + } + handleRequest := NewHTTPHandler(c) ops := options.Generic{ diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 2e952dfb9b..7e9fcfa868 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -40,7 +40,9 @@ var ( // configuration info for the "bridge" driver. type configuration struct { - EnableIPForwarding bool + EnableIPForwarding bool + EnableIPTables bool + EnableUserlandProxy bool } // networkConfiguration for network specific configuration @@ -50,7 +52,6 @@ type networkConfiguration struct { FixedCIDR *net.IPNet FixedCIDRv6 *net.IPNet EnableIPv6 bool - EnableIPTables bool EnableIPMasquerade bool EnableICC bool Mtu int @@ -58,7 +59,6 @@ type networkConfiguration struct { DefaultGatewayIPv6 net.IP DefaultBindingIP net.IP AllowNonDefaultBridge bool - EnableUserlandProxy bool } // endpointConfiguration represents the user specified configuration for the sandbox endpoint @@ -91,13 +91,16 @@ type bridgeNetwork struct { config *networkConfiguration endpoints map[types.UUID]*bridgeEndpoint // key: endpoint id portMapper *portmapper.PortMapper + driver *driver // The network's driver sync.Mutex } type driver struct { - config *configuration - network *bridgeNetwork - networks map[types.UUID]*bridgeNetwork + config *configuration + network *bridgeNetwork + natChain *iptables.ChainInfo + filterChain *iptables.ChainInfo + networks map[types.UUID]*bridgeNetwork sync.Mutex } @@ -223,16 +226,6 @@ func (c *networkConfiguration) fromMap(data map[string]interface{}) error { } } - if i, ok := data["EnableIPTables"]; ok && i != nil { - if s, ok := i.(string); ok { - if c.EnableIPTables, err = strconv.ParseBool(s); err != nil { - return types.BadRequestErrorf("failed to parse EnableIPTables value: %s", err.Error()) - } - } else { - return types.BadRequestErrorf("invalid type for EnableIPTables value") - } - } - if i, ok := data["EnableIPMasquerade"]; ok && i != nil { if s, ok := i.(string); ok { if c.EnableIPMasquerade, err = strconv.ParseBool(s); err != nil { @@ -334,6 +327,25 @@ func (c *networkConfiguration) fromMap(data map[string]interface{}) error { return nil } +func (n *bridgeNetwork) getDriverChains() (*iptables.ChainInfo, *iptables.ChainInfo, error) { + n.Lock() + defer n.Unlock() + + if n.driver == nil { + return nil, nil, types.BadRequestErrorf("no driver found") + } + + return n.driver.natChain, n.driver.filterChain, nil +} + +func (n *bridgeNetwork) getNetworkBridgeName() string { + n.Lock() + config := n.config + n.Unlock() + + return config.BridgeName +} + func (n *bridgeNetwork) getEndpoint(eid types.UUID) (*bridgeEndpoint, error) { n.Lock() defer n.Unlock() @@ -418,6 +430,7 @@ func (c *networkConfiguration) conflictsWithNetworks(id types.UUID, others []*br func (d *driver) Config(option map[string]interface{}) error { var config *configuration + var err error d.Lock() defer d.Unlock() @@ -444,10 +457,19 @@ func (d *driver) Config(option map[string]interface{}) error { d.config = config } else { config = &configuration{} + d.config = config } if config.EnableIPForwarding { - return setupIPForwarding() + err = setupIPForwarding() + if err != nil { + return err + } + } + + if config.EnableIPTables { + d.natChain, d.filterChain, err = setupIPChains(config) + return err } return nil @@ -480,7 +502,6 @@ func parseNetworkGenericOptions(data interface{}) (*networkConfiguration, error) case map[string]interface{}: config = &networkConfiguration{ EnableICC: true, - EnableIPTables: true, EnableIPMasquerade: true, } err = config.fromMap(opt) @@ -578,6 +599,7 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err endpoints: make(map[types.UUID]*bridgeEndpoint), config: config, portMapper: portmapper.New(), + driver: d, } d.Lock() @@ -660,14 +682,14 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err {enableIPv6Forwarding, setupIPv6Forwarding}, // Setup Loopback Adresses Routing - {!config.EnableUserlandProxy, setupLoopbackAdressesRouting}, + {!d.config.EnableUserlandProxy, setupLoopbackAdressesRouting}, // Setup IPTables. - {config.EnableIPTables, network.setupIPTables}, + {d.config.EnableIPTables, network.setupIPTables}, //We want to track firewalld configuration so that //if it is started/reloaded, the rules can be applied correctly - {config.EnableIPTables, network.setupFirewalld}, + {d.config.EnableIPTables, network.setupFirewalld}, // Setup DefaultGatewayIPv4 {config.DefaultGatewayIPv4 != nil, setupGatewayIPv4}, @@ -676,10 +698,10 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err {config.DefaultGatewayIPv6 != nil, setupGatewayIPv6}, // Add inter-network communication rules. - {config.EnableIPTables, setupNetworkIsolationRules}, + {d.config.EnableIPTables, setupNetworkIsolationRules}, //Configure bridge networking filtering if ICC is off and IP tables are enabled - {!config.EnableICC && config.EnableIPTables, setupBridgeNetFiltering}, + {!config.EnableICC && d.config.EnableIPTables, setupBridgeNetFiltering}, } { if step.Condition { bridgeSetup.queueStep(step.Fn) @@ -838,6 +860,7 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn // Get the network handler and make sure it exists d.Lock() n, ok := d.networks[nid] + dconfig := d.config d.Unlock() if !ok { @@ -950,7 +973,7 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn return fmt.Errorf("adding interface %s to bridge %s failed: %v", hostIfName, config.BridgeName, err) } - if !config.EnableUserlandProxy { + if !dconfig.EnableUserlandProxy { err = setHairpinMode(host, true) if err != nil { return err @@ -1023,7 +1046,7 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn } // Program any required port mapping and store them in the endpoint - endpoint.portMapping, err = n.allocatePorts(epConfig, endpoint, config.DefaultBindingIP, config.EnableUserlandProxy) + endpoint.portMapping, err = n.allocatePorts(epConfig, endpoint, config.DefaultBindingIP, d.config.EnableUserlandProxy) if err != nil { return err } diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index ffd2c94c90..78eed8291c 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/drivers/bridge/bridge_test.go @@ -21,6 +21,7 @@ func TestCreateFullOptions(t *testing.T) { config := &configuration{ EnableIPForwarding: true, + EnableIPTables: true, } // Test this scenario: Default gw address does not belong to @@ -37,7 +38,6 @@ func TestCreateFullOptions(t *testing.T) { FixedCIDR: cnw, DefaultGatewayIPv4: gw, EnableIPv6: true, - EnableIPTables: true, } _, netConfig.FixedCIDRv6, _ = net.ParseCIDR("2001:db8::/48") genericOption := make(map[string]interface{}) @@ -71,9 +71,13 @@ func TestCreate(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() - config := &networkConfiguration{BridgeName: DefaultBridgeName} + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + netconfig := &networkConfiguration{BridgeName: DefaultBridgeName} genericOption := make(map[string]interface{}) - genericOption[netlabel.GenericData] = config + genericOption[netlabel.GenericData] = netconfig if err := d.CreateNetwork("dummy", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) @@ -100,9 +104,13 @@ func TestCreateFail(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() - config := &networkConfiguration{BridgeName: "dummy0"} + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + netconfig := &networkConfiguration{BridgeName: "dummy0"} genericOption := make(map[string]interface{}) - genericOption[netlabel.GenericData] = config + genericOption[netlabel.GenericData] = netconfig if err := d.CreateNetwork("dummy", genericOption); err == nil { t.Fatal("Bridge creation was expected to fail") @@ -114,20 +122,30 @@ func TestCreateMultipleNetworks(t *testing.T) { d := newDriver() dd, _ := d.(*driver) - config1 := &networkConfiguration{BridgeName: "net_test_1", AllowNonDefaultBridge: true, EnableIPTables: true} + config := &configuration{ + EnableIPTables: true, + } genericOption := make(map[string]interface{}) + genericOption[netlabel.GenericData] = config + + if err := d.Config(genericOption); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + config1 := &networkConfiguration{BridgeName: "net_test_1", AllowNonDefaultBridge: true} + genericOption = make(map[string]interface{}) genericOption[netlabel.GenericData] = config1 if err := d.CreateNetwork("1", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) } - config2 := &networkConfiguration{BridgeName: "net_test_2", AllowNonDefaultBridge: true, EnableIPTables: true} + config2 := &networkConfiguration{BridgeName: "net_test_2", AllowNonDefaultBridge: true} genericOption[netlabel.GenericData] = config2 if err := d.CreateNetwork("2", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) } - config3 := &networkConfiguration{BridgeName: "net_test_3", AllowNonDefaultBridge: true, EnableIPTables: true} + config3 := &networkConfiguration{BridgeName: "net_test_3", AllowNonDefaultBridge: true} genericOption[netlabel.GenericData] = config3 if err := d.CreateNetwork("3", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) @@ -136,7 +154,7 @@ func TestCreateMultipleNetworks(t *testing.T) { // Verify the network isolation rules are installed, each network subnet should appear 4 times verifyV4INCEntries(dd.networks, 4, t) - config4 := &networkConfiguration{BridgeName: "net_test_4", AllowNonDefaultBridge: true, EnableIPTables: true} + config4 := &networkConfiguration{BridgeName: "net_test_4", AllowNonDefaultBridge: true} genericOption[netlabel.GenericData] = config4 if err := d.CreateNetwork("4", genericOption); err != nil { t.Fatalf("Failed to create bridge: %v", err) @@ -278,15 +296,24 @@ func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) { d := newDriver() dd, _ := d.(*driver) - config := &networkConfiguration{ - BridgeName: DefaultBridgeName, + config := &configuration{ EnableIPTables: true, - EnableICC: false, EnableUserlandProxy: ulPxyEnabled, } genericOption := make(map[string]interface{}) genericOption[netlabel.GenericData] = config + if err := d.Config(genericOption); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + netconfig := &networkConfiguration{ + BridgeName: DefaultBridgeName, + EnableICC: false, + } + genericOption = make(map[string]interface{}) + genericOption[netlabel.GenericData] = netconfig + err := d.CreateNetwork("net1", genericOption) if err != nil { t.Fatalf("Failed to create bridge: %v", err) @@ -339,9 +366,13 @@ func TestCreateLinkWithOptions(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() - config := &networkConfiguration{BridgeName: DefaultBridgeName} + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + netconfig := &networkConfiguration{BridgeName: DefaultBridgeName} netOptions := make(map[string]interface{}) - netOptions[netlabel.GenericData] = config + netOptions[netlabel.GenericData] = netconfig err := d.CreateNetwork("net1", netOptions) if err != nil { @@ -395,14 +426,23 @@ func TestLinkContainers(t *testing.T) { d := newDriver() - config := &networkConfiguration{ - BridgeName: DefaultBridgeName, + config := &configuration{ EnableIPTables: true, - EnableICC: false, } genericOption := make(map[string]interface{}) genericOption[netlabel.GenericData] = config + if err := d.Config(genericOption); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + netconfig := &networkConfiguration{ + BridgeName: DefaultBridgeName, + EnableICC: false, + } + genericOption = make(map[string]interface{}) + genericOption[netlabel.GenericData] = netconfig + err := d.CreateNetwork("net1", genericOption) if err != nil { t.Fatalf("Failed to create bridge: %v", err) @@ -602,6 +642,10 @@ func TestSetDefaultGw(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + _, subnetv6, _ := net.ParseCIDR("2001:db8:ea9:9abc:b0c4::/80") gw4 := bridgeNetworks[0].IP.To4() gw4[3] = 254 diff --git a/libnetwork/drivers/bridge/link.go b/libnetwork/drivers/bridge/link.go index 894c5e772c..25877737ba 100644 --- a/libnetwork/drivers/bridge/link.go +++ b/libnetwork/drivers/bridge/link.go @@ -74,9 +74,9 @@ func linkContainers(action, parentIP, childIP string, ports []types.TransportPor return InvalidLinkIPAddrError(childIP) } - chain := iptables.Chain{Name: DockerChain, Bridge: bridge} + chain := iptables.ChainInfo{Name: DockerChain} for _, port := range ports { - err := chain.Link(nfAction, ip1, ip2, int(port.Port), port.Proto.String()) + err := chain.Link(nfAction, ip1, ip2, int(port.Port), port.Proto.String(), bridge) if !ignoreErrors && err != nil { return err } diff --git a/libnetwork/drivers/bridge/network_test.go b/libnetwork/drivers/bridge/network_test.go index 2a7a4690f7..13a996e310 100644 --- a/libnetwork/drivers/bridge/network_test.go +++ b/libnetwork/drivers/bridge/network_test.go @@ -14,6 +14,10 @@ func TestLinkCreate(t *testing.T) { d := newDriver() dr := d.(*driver) + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + mtu := 1490 config := &networkConfiguration{ BridgeName: DefaultBridgeName, @@ -108,6 +112,10 @@ func TestLinkCreateTwo(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + config := &networkConfiguration{ BridgeName: DefaultBridgeName, EnableIPv6: true} @@ -140,6 +148,10 @@ func TestLinkCreateNoEnableIPv6(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + config := &networkConfiguration{ BridgeName: DefaultBridgeName} genericOption := make(map[string]interface{}) @@ -170,6 +182,10 @@ func TestLinkDelete(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() + if err := d.Config(nil); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + config := &networkConfiguration{ BridgeName: DefaultBridgeName, EnableIPv6: true} diff --git a/libnetwork/drivers/bridge/port_mapping_test.go b/libnetwork/drivers/bridge/port_mapping_test.go index 52cb3eacb2..8b6887cb10 100644 --- a/libnetwork/drivers/bridge/port_mapping_test.go +++ b/libnetwork/drivers/bridge/port_mapping_test.go @@ -21,6 +21,16 @@ func TestPortMappingConfig(t *testing.T) { defer netutils.SetupTestNetNS(t)() d := newDriver() + config := &configuration{ + EnableIPTables: true, + } + genericOption := make(map[string]interface{}) + genericOption[netlabel.GenericData] = config + + if err := d.Config(genericOption); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + binding1 := types.PortBinding{Proto: types.UDP, Port: uint16(400), HostPort: uint16(54000)} binding2 := types.PortBinding{Proto: types.TCP, Port: uint16(500), HostPort: uint16(65000)} portBindings := []types.PortBinding{binding1, binding2} @@ -29,8 +39,7 @@ func TestPortMappingConfig(t *testing.T) { epOptions[netlabel.PortMap] = portBindings netConfig := &networkConfiguration{ - BridgeName: DefaultBridgeName, - EnableIPTables: true, + BridgeName: DefaultBridgeName, } netOptions := make(map[string]interface{}) netOptions[netlabel.GenericData] = netConfig diff --git a/libnetwork/drivers/bridge/setup_firewalld.go b/libnetwork/drivers/bridge/setup_firewalld.go index eeb7764801..fc45a7e983 100644 --- a/libnetwork/drivers/bridge/setup_firewalld.go +++ b/libnetwork/drivers/bridge/setup_firewalld.go @@ -3,8 +3,13 @@ package bridge import "github.com/docker/libnetwork/iptables" func (n *bridgeNetwork) setupFirewalld(config *networkConfiguration, i *bridgeInterface) error { + d := n.driver + d.Lock() + driverConfig := d.config + d.Unlock() + // Sanity check. - if config.EnableIPTables == false { + if driverConfig.EnableIPTables == false { return IPTableCfgError(config.BridgeName) } diff --git a/libnetwork/drivers/bridge/setup_ip_tables.go b/libnetwork/drivers/bridge/setup_ip_tables.go index 42290384d9..f597f67b7f 100644 --- a/libnetwork/drivers/bridge/setup_ip_tables.go +++ b/libnetwork/drivers/bridge/setup_ip_tables.go @@ -4,6 +4,7 @@ import ( "fmt" "net" + "github.com/Sirupsen/logrus" "github.com/docker/libnetwork/iptables" "github.com/docker/libnetwork/netutils" ) @@ -13,14 +14,48 @@ const ( DockerChain = "DOCKER" ) -func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInterface) error { +func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainInfo, error) { // Sanity check. if config.EnableIPTables == false { - return IPTableCfgError(config.BridgeName) + return nil, nil, fmt.Errorf("Cannot create new chains, EnableIPTable is disabled") } hairpinMode := !config.EnableUserlandProxy + natChain, err := iptables.NewChain(DockerChain, iptables.Nat, hairpinMode) + if err != nil { + return nil, nil, fmt.Errorf("Failed to create NAT chain: %s", err.Error()) + } + 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) + } + } + }() + + filterChain, err := iptables.NewChain(DockerChain, iptables.Filter, hairpinMode) + if err != nil { + return nil, nil, fmt.Errorf("Failed to create FILTER chain: %s", err.Error()) + } + + return natChain, filterChain, nil +} + +func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInterface) error { + d := n.driver + d.Lock() + driverConfig := d.config + d.Unlock() + + // Sanity check. + if driverConfig.EnableIPTables == false { + return fmt.Errorf("Cannot program chains, EnableIPTable is disabled") + } + + // Pickup this configuraton option from driver + hairpinMode := !driverConfig.EnableUserlandProxy + addrv4, _, err := netutils.GetIfaceAddr(config.BridgeName) if err != nil { return fmt.Errorf("Failed to setup IP tables, cannot acquire Interface address: %s", err.Error()) @@ -34,17 +69,22 @@ func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInt return fmt.Errorf("Failed to Setup IP tables: %s", err.Error()) } - _, err = iptables.NewChain(DockerChain, config.BridgeName, iptables.Nat, hairpinMode) + natChain, filterChain, err := n.getDriverChains() if err != nil { - return fmt.Errorf("Failed to create NAT chain: %s", err.Error()) + return fmt.Errorf("Failed to setup IP tables, cannot acquire chain info %s", err.Error()) } - chain, err := iptables.NewChain(DockerChain, config.BridgeName, iptables.Filter, hairpinMode) + err = iptables.ProgramChain(natChain, config.BridgeName, hairpinMode) if err != nil { - return fmt.Errorf("Failed to create FILTER chain: %s", err.Error()) + return fmt.Errorf("Failed to program NAT chain: %s", err.Error()) } - n.portMapper.SetIptablesChain(chain) + err = iptables.ProgramChain(filterChain, config.BridgeName, hairpinMode) + if err != nil { + return fmt.Errorf("Failed to program FILTER chain: %s", err.Error()) + } + + n.portMapper.SetIptablesChain(filterChain, n.getNetworkBridgeName()) return nil } diff --git a/libnetwork/drivers/bridge/setup_ip_tables_test.go b/libnetwork/drivers/bridge/setup_ip_tables_test.go index b371822304..c1f81ba04e 100644 --- a/libnetwork/drivers/bridge/setup_ip_tables_test.go +++ b/libnetwork/drivers/bridge/setup_ip_tables_test.go @@ -37,26 +37,32 @@ func TestProgramIPTable(t *testing.T) { } } -func TestSetupIPTables(t *testing.T) { +func TestSetupIPChains(t *testing.T) { // Create a test bridge with a basic bridge configuration (name + IPv4). defer netutils.SetupTestNetNS(t)() + + driverconfig := &configuration{ + EnableIPTables: true, + } + d := &driver{ + config: driverconfig, + } + assertChainConfig(d, t) + config := getBasicTestConfig() br := &bridgeInterface{} - createTestBridge(config, br, t) - // Modify iptables params in base configuration and apply them. - config.EnableIPTables = true - assertBridgeConfig(config, br, t) + assertBridgeConfig(config, br, d, t) config.EnableIPMasquerade = true - assertBridgeConfig(config, br, t) + assertBridgeConfig(config, br, d, t) config.EnableICC = true - assertBridgeConfig(config, br, t) + assertBridgeConfig(config, br, d, t) config.EnableIPMasquerade = false - assertBridgeConfig(config, br, t) + assertBridgeConfig(config, br, d, t) } func getBasicTestConfig() *networkConfiguration { @@ -94,9 +100,22 @@ func assertIPTableChainProgramming(rule iptRule, descr string, t *testing.T) { } } +// Assert function which create chains. +func assertChainConfig(d *driver, t *testing.T) { + var err error + + d.natChain, d.filterChain, err = setupIPChains(d.config) + if err != nil { + t.Fatal(err) + } +} + // Assert function which pushes chains based on bridge config parameters. -func assertBridgeConfig(config *networkConfiguration, br *bridgeInterface, t *testing.T) { - nw := bridgeNetwork{portMapper: portmapper.New()} +func assertBridgeConfig(config *networkConfiguration, br *bridgeInterface, d *driver, t *testing.T) { + nw := bridgeNetwork{portMapper: portmapper.New(), + config: config} + nw.driver = d + // Attempt programming of ip tables. err := nw.setupIPTables(config, br) if err != nil { diff --git a/libnetwork/iptables/firewalld_test.go b/libnetwork/iptables/firewalld_test.go index 547ba7e683..6607307564 100644 --- a/libnetwork/iptables/firewalld_test.go +++ b/libnetwork/iptables/firewalld_test.go @@ -17,9 +17,12 @@ func TestFirewalldInit(t *testing.T) { func TestReloaded(t *testing.T) { var err error - var fwdChain *Chain + var fwdChain *ChainInfo - fwdChain, err = NewChain("FWD", "lo", Filter, false) + fwdChain, err = NewChain("FWD", Filter, false) + bridgeName := "lo" + + err = ProgramChain(fwdChain, bridgeName, false) if err != nil { t.Fatal(err) } @@ -31,17 +34,17 @@ func TestReloaded(t *testing.T) { port := 1234 proto := "tcp" - err = fwdChain.Link(Append, ip1, ip2, port, proto) + err = fwdChain.Link(Append, ip1, ip2, port, proto, bridgeName) if err != nil { t.Fatal(err) } else { // to be re-called again later - OnReloaded(func() { fwdChain.Link(Append, ip1, ip2, port, proto) }) + OnReloaded(func() { fwdChain.Link(Append, ip1, ip2, port, proto, bridgeName) }) } rule1 := []string{ - "-i", fwdChain.Bridge, - "-o", fwdChain.Bridge, + "-i", bridgeName, + "-o", bridgeName, "-p", proto, "-s", ip1.String(), "-d", ip2.String(), diff --git a/libnetwork/iptables/iptables.go b/libnetwork/iptables/iptables.go index 707ddb7e59..4e24c3ac52 100644 --- a/libnetwork/iptables/iptables.go +++ b/libnetwork/iptables/iptables.go @@ -42,10 +42,9 @@ var ( ErrIptablesNotFound = errors.New("Iptables not found") ) -// Chain defines the iptables chain. -type Chain struct { +// ChainInfo defines the iptables chain. +type ChainInfo struct { Name string - Bridge string Table Table HairpinMode bool } @@ -74,14 +73,12 @@ func initCheck() error { } // NewChain adds a new chain to ip table. -func NewChain(name, bridge string, table Table, hairpinMode bool) (*Chain, error) { - c := &Chain{ +func NewChain(name string, table Table, hairpinMode bool) (*ChainInfo, error) { + c := &ChainInfo{ Name: name, - Bridge: bridge, Table: table, HairpinMode: hairpinMode, } - if string(c.Table) == "" { c.Table = Filter } @@ -94,8 +91,16 @@ func NewChain(name, bridge string, table Table, hairpinMode bool) (*Chain, error return nil, fmt.Errorf("Could not create %s/%s chain: %s", c.Table, c.Name, output) } } + return c, nil +} - switch table { +// ProgramChain is used to add rules to a chain +func ProgramChain(c *ChainInfo, bridgeName string, hairpinMode bool) error { + if c.Name == "" { + return fmt.Errorf("Could not program chain, missing chain name.") + } + + switch c.Table { case Nat: preroute := []string{ "-m", "addrtype", @@ -103,7 +108,7 @@ func NewChain(name, bridge string, table Table, hairpinMode bool) (*Chain, error "-j", c.Name} if !Exists(Nat, "PREROUTING", preroute...) { if err := c.Prerouting(Append, preroute...); err != nil { - return nil, fmt.Errorf("Failed to inject docker in PREROUTING chain: %s", err) + return fmt.Errorf("Failed to inject docker in PREROUTING chain: %s", err) } } output := []string{ @@ -115,28 +120,32 @@ func NewChain(name, bridge string, table Table, hairpinMode bool) (*Chain, error } if !Exists(Nat, "OUTPUT", output...) { if err := c.Output(Append, output...); err != nil { - return nil, fmt.Errorf("Failed to inject docker in OUTPUT chain: %s", err) + return fmt.Errorf("Failed to inject docker in OUTPUT chain: %s", err) } } case Filter: + if bridgeName == "" { + return fmt.Errorf("Could not program chain %s/%s, missing bridge name.", + c.Table, c.Name) + } link := []string{ - "-o", c.Bridge, + "-o", bridgeName, "-j", c.Name} if !Exists(Filter, "FORWARD", link...) { insert := append([]string{string(Insert), "FORWARD"}, link...) if output, err := Raw(insert...); err != nil { - return nil, err + return 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 fmt.Errorf("Could not create linking rule to %s/%s: %s", c.Table, c.Name, output) } } } - return c, nil + return nil } // RemoveExistingChain removes existing chain from the table. func RemoveExistingChain(name string, table Table) error { - c := &Chain{ + c := &ChainInfo{ Name: name, Table: table, } @@ -147,7 +156,7 @@ func RemoveExistingChain(name string, table Table) error { } // Forward adds 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 { +func (c *ChainInfo) Forward(action Action, ip net.IP, port int, proto, destAddr string, destPort int, bridgeName string) error { daddr := ip.String() if ip.IsUnspecified() { // iptables interprets "0.0.0.0" as "0.0.0.0/32", whereas we @@ -162,7 +171,7 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, destAddr stri "-j", "DNAT", "--to-destination", net.JoinHostPort(destAddr, strconv.Itoa(destPort))} if !c.HairpinMode { - args = append(args, "!", "-i", c.Bridge) + args = append(args, "!", "-i", bridgeName) } if output, err := Raw(args...); err != nil { return err @@ -171,8 +180,8 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, destAddr stri } if output, err := Raw("-t", string(Filter), string(action), c.Name, - "!", "-i", c.Bridge, - "-o", c.Bridge, + "!", "-i", bridgeName, + "-o", bridgeName, "-p", proto, "-d", destAddr, "--dport", strconv.Itoa(destPort), @@ -198,9 +207,9 @@ func (c *Chain) Forward(action Action, ip net.IP, port int, proto, destAddr stri // Link adds 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 { +func (c *ChainInfo) Link(action Action, ip1, ip2 net.IP, port int, proto string, bridgeName string) error { if output, err := Raw("-t", string(Filter), string(action), c.Name, - "-i", c.Bridge, "-o", c.Bridge, + "-i", bridgeName, "-o", bridgeName, "-p", proto, "-s", ip1.String(), "-d", ip2.String(), @@ -211,7 +220,7 @@ func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) err 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, + "-i", bridgeName, "-o", bridgeName, "-p", proto, "-s", ip2.String(), "-d", ip1.String(), @@ -225,7 +234,7 @@ func (c *Chain) Link(action Action, ip1, ip2 net.IP, port int, proto string) err } // Prerouting adds linking rule to nat/PREROUTING chain. -func (c *Chain) Prerouting(action Action, args ...string) error { +func (c *ChainInfo) Prerouting(action Action, args ...string) error { a := []string{"-t", string(Nat), string(action), "PREROUTING"} if len(args) > 0 { a = append(a, args...) @@ -239,7 +248,7 @@ func (c *Chain) Prerouting(action Action, args ...string) error { } // Output adds linking rule to an OUTPUT chain. -func (c *Chain) Output(action Action, args ...string) error { +func (c *ChainInfo) Output(action Action, args ...string) error { a := []string{"-t", string(c.Table), string(action), "OUTPUT"} if len(args) > 0 { a = append(a, args...) @@ -253,7 +262,7 @@ func (c *Chain) Output(action Action, args ...string) error { } // Remove removes the chain. -func (c *Chain) Remove() error { +func (c *ChainInfo) Remove() error { // Ignore errors - This could mean the chains were never set up if c.Table == Nat { c.Prerouting(Delete, "-m", "addrtype", "--dst-type", "LOCAL", "-j", c.Name) diff --git a/libnetwork/iptables/iptables_test.go b/libnetwork/iptables/iptables_test.go index 63d931c8ab..31331dcdbd 100644 --- a/libnetwork/iptables/iptables_test.go +++ b/libnetwork/iptables/iptables_test.go @@ -13,18 +13,22 @@ import ( const chainName = "DOCKEREST" -var natChain *Chain -var filterChain *Chain +var natChain *ChainInfo +var filterChain *ChainInfo +var bridgeName string func TestNewChain(t *testing.T) { var err error - natChain, err = NewChain(chainName, "lo", Nat, false) + bridgeName = "lo" + natChain, err = NewChain(chainName, Nat, false) + err = ProgramChain(natChain, bridgeName, false) if err != nil { t.Fatal(err) } - filterChain, err = NewChain(chainName, "lo", Filter, false) + filterChain, err = NewChain(chainName, Filter, false) + err = ProgramChain(filterChain, bridgeName, false) if err != nil { t.Fatal(err) } @@ -37,7 +41,8 @@ func TestForward(t *testing.T) { dstPort := 4321 proto := "tcp" - err := natChain.Forward(Insert, ip, port, proto, dstAddr, dstPort) + bridgeName := "lo" + err := natChain.Forward(Insert, ip, port, proto, dstAddr, dstPort, bridgeName) if err != nil { t.Fatal(err) } @@ -48,7 +53,7 @@ func TestForward(t *testing.T) { "--dport", strconv.Itoa(port), "-j", "DNAT", "--to-destination", dstAddr + ":" + strconv.Itoa(dstPort), - "!", "-i", natChain.Bridge, + "!", "-i", bridgeName, } if !Exists(natChain.Table, natChain.Name, dnatRule...) { @@ -56,8 +61,8 @@ func TestForward(t *testing.T) { } filterRule := []string{ - "!", "-i", filterChain.Bridge, - "-o", filterChain.Bridge, + "!", "-i", bridgeName, + "-o", bridgeName, "-d", dstAddr, "-p", proto, "--dport", strconv.Itoa(dstPort), @@ -84,19 +89,20 @@ func TestForward(t *testing.T) { func TestLink(t *testing.T) { var err error + bridgeName := "lo" 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) + err = filterChain.Link(Append, ip1, ip2, port, proto, bridgeName) if err != nil { t.Fatal(err) } rule1 := []string{ - "-i", filterChain.Bridge, - "-o", filterChain.Bridge, + "-i", bridgeName, + "-o", bridgeName, "-p", proto, "-s", ip1.String(), "-d", ip2.String(), @@ -108,8 +114,8 @@ func TestLink(t *testing.T) { } rule2 := []string{ - "-i", filterChain.Bridge, - "-o", filterChain.Bridge, + "-i", bridgeName, + "-o", bridgeName, "-p", proto, "-s", ip2.String(), "-d", ip1.String(), @@ -192,7 +198,7 @@ func RunConcurrencyTest(t *testing.T, allowXlock bool) { wg.Add(1) go func() { defer wg.Done() - err := natChain.Forward(Append, ip, port, proto, dstAddr, dstPort) + err := natChain.Forward(Append, ip, port, proto, dstAddr, dstPort, "lo") if err != nil { t.Fatal(err) } @@ -208,7 +214,7 @@ func TestCleanup(t *testing.T) { // Cleanup filter/FORWARD first otherwise output of iptables-save is dirty link := []string{"-t", string(filterChain.Table), string(Delete), "FORWARD", - "-o", filterChain.Bridge, + "-o", bridgeName, "-j", filterChain.Name} if _, err = Raw(link...); err != nil { t.Fatal(err) diff --git a/libnetwork/libnetwork_test.go b/libnetwork/libnetwork_test.go index 3bf62bf497..3d4a8b0eb3 100644 --- a/libnetwork/libnetwork_test.go +++ b/libnetwork/libnetwork_test.go @@ -251,10 +251,9 @@ func TestBridge(t *testing.T) { "FixedCIDR": cidr, "FixedCIDRv6": cidrv6, "EnableIPv6": true, - "EnableIPTables": true, - "EnableIPMasquerade": true, "EnableICC": true, "AllowNonDefaultBridge": true, + "EnableIPMasquerade": true, }, } diff --git a/libnetwork/portmapper/mapper.go b/libnetwork/portmapper/mapper.go index b928e3c619..48a19053ef 100644 --- a/libnetwork/portmapper/mapper.go +++ b/libnetwork/portmapper/mapper.go @@ -31,7 +31,8 @@ var ( // PortMapper manages the network address translation type PortMapper struct { - chain *iptables.Chain + chain *iptables.ChainInfo + bridgeName string // udp:ip:port currentMappings map[string]*mapping @@ -54,8 +55,9 @@ func NewWithPortAllocator(allocator *portallocator.PortAllocator) *PortMapper { } // SetIptablesChain sets the specified chain into portmapper -func (pm *PortMapper) SetIptablesChain(c *iptables.Chain) { +func (pm *PortMapper) SetIptablesChain(c *iptables.ChainInfo, bridgeName string) { pm.chain = c + pm.bridgeName = bridgeName } // Map maps the specified container transport address to the host's network address and transport port @@ -215,5 +217,5 @@ func (pm *PortMapper) forward(action iptables.Action, proto string, sourceIP net if pm.chain == nil { return nil } - return pm.chain.Forward(action, sourceIP, sourcePort, proto, containerIP, containerPort) + return pm.chain.Forward(action, sourceIP, sourcePort, proto, containerIP, containerPort, pm.bridgeName) } diff --git a/libnetwork/portmapper/mapper_test.go b/libnetwork/portmapper/mapper_test.go index 635723de8c..7e1c65d2c5 100644 --- a/libnetwork/portmapper/mapper_test.go +++ b/libnetwork/portmapper/mapper_test.go @@ -17,16 +17,15 @@ func init() { func TestSetIptablesChain(t *testing.T) { pm := New() - c := &iptables.Chain{ - Name: "TEST", - Bridge: "192.168.1.1", + c := &iptables.ChainInfo{ + Name: "TEST", } if pm.chain != nil { t.Fatal("chain should be nil at init") } - pm.SetIptablesChain(c) + pm.SetIptablesChain(c, "lo") if pm.chain == nil { t.Fatal("chain should not be nil after set") }