Skip to content

Commit

Permalink
Use IPVS for nating instead of iptables
Browse files Browse the repository at this point in the history 
Using iptables for setting up nat rules has a nasty tendancy to conflict
with user-defined firewall rules, and tend to be invisible to tools like
UFW.
Meanwhile using ipvs for nat is fairly natural and does not require
mucking around in iptables rules.

The one thing we do need to continue using iptables for here is nating
against localhost, however this shouldn't intefere with user-defined
rules unless they are denying localhost (not likely)... and still much
better than having public interfaces here.

Fixes (after re-vendor) moby/moby#4737
Fixes (after re-vendor)  moby/moby#22054

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
@cpuguy83
cpuguy83 committed Nov 5, 2016
1 parent 11cf223 commit 6bcf354
Show file tree
Hide file tree
Showing 8 changed files with 309 additions and 73 deletions.
13 changes: 8 additions & 5 deletions drivers/bridge/bridge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ type driver struct {
natChain *iptables.ChainInfo
filterChain *iptables.ChainInfo
isolationChain *iptables.ChainInfo
mangleChain *iptables.ChainInfo
networks map[string]*bridgeNetwork
store datastore.DataStore
nlh *netlink.Handle
Expand Down Expand Up @@ -253,15 +254,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.isolationChain, n.driver.mangleChain, nil
}

func (n *bridgeNetwork) getNetworkBridgeName() string {
Expand Down Expand Up @@ -360,6 +361,7 @@ func (d *driver) configure(option map[string]interface{}) error {
natChain *iptables.ChainInfo
filterChain *iptables.ChainInfo
isolationChain *iptables.ChainInfo
mangleChain *iptables.ChainInfo
)

genericData, ok := option[netlabel.GenericData]
Expand Down Expand Up @@ -394,7 +396,7 @@ func (d *driver) configure(option map[string]interface{}) error {
}
}
removeIPChains()
natChain, filterChain, isolationChain, err = setupIPChains(config)
natChain, filterChain, isolationChain, mangleChain, err = setupIPChains(config)
if err != nil {
return err
}
Expand All @@ -406,6 +408,7 @@ func (d *driver) configure(option map[string]interface{}) error {
d.natChain = natChain
d.filterChain = filterChain
d.isolationChain = isolationChain
d.mangleChain = mangleChain
d.config = config
d.Unlock()

Expand Down Expand Up @@ -587,7 +590,7 @@ func (d *driver) CreateNetwork(id string, option map[string]interface{}, nInfo d
}
d.Unlock()

// Parse and validate the config. It should not be conflict with existing networks' config
// Parse and validate the config. It should not conflict with existing networks' config
config, err := parseNetworkOptions(id, option)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion drivers/bridge/bridge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ func TestCleanupIptableRules(t *testing.T) {
{Name: DockerChain, Table: iptables.Filter},
{Name: IsolationChain, 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 {
Expand Down
50 changes: 36 additions & 14 deletions drivers/bridge/setup_ip_tables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ const (
IsolationChain = "DOCKER-ISOLATION"
)

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, fmt.Errorf("cannot create new chains, EnableIPTable is disabled")
return nil, nil, 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, 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 {
Expand All @@ -36,7 +36,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 {
Expand All @@ -48,14 +48,26 @@ func setupIPChains(config *configuration) (*iptables.ChainInfo, *iptables.ChainI

isolationChain, err := iptables.NewChain(IsolationChain, 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(IsolationChain, iptables.Filter); err != nil {
logrus.Warnf("failed on removing iptables NAT chain on cleanup: %v", err)
}
}
}()

if err := addReturnRule(IsolationChain); err != nil {
return nil, nil, nil, err
return nil, nil, nil, nil, err
}

return natChain, filterChain, isolationChain, nil
mangleChain, err := iptables.NewChain(DockerChain, iptables.Mangle, false)
if err != nil {
return nil, nil, nil, nil, err
}

return natChain, filterChain, isolationChain, mangleChain, nil
}

func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInterface) error {
Expand Down Expand Up @@ -92,7 +104,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, _, mangleChain, err := n.getDriverChains()
if err != nil {
return fmt.Errorf("Failed to setup IP tables, cannot acquire chain info %s", err.Error())
}
Expand All @@ -102,6 +114,11 @@ func (n *bridgeNetwork) setupIPTables(config *networkConfiguration, i *bridgeInt
return fmt.Errorf("Failed to program NAT chain: %s", err.Error())
}

err = iptables.ProgramChain(mangleChain, 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)
if err != nil {
return fmt.Errorf("Failed to program FILTER chain: %s", err.Error())
Expand Down Expand Up @@ -131,12 +148,13 @@ type iptRule struct {
func setupIPTablesInternal(bridgeIface string, addr net.Addr, icc, ipmasq, hairpin, enable bool) error {

var (
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"}}
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"}}
skipMangle = iptRule{table: iptables.Mangle, chain: DockerChain, preArgs: []string{"-t", "mangle"}, 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"}}
)

// Set NAT.
Expand All @@ -150,6 +168,9 @@ func setupIPTablesInternal(bridgeIface string, addr net.Addr, icc, ipmasq, hairp
if err := programChainRule(skipDNAT, "SKIP DNAT", enable); err != nil {
return err
}
if err := programChainRule(skipMangle, "SKIP MANGLE", enable); err != nil {
return err
}
}

// In hairpin mode, masquerade traffic from localhost
Expand Down Expand Up @@ -324,6 +345,7 @@ func ensureJumpRule(fromChain, toChain string) error {
func removeIPChains() {
for _, chainInfo := range []iptables.ChainInfo{
{Name: DockerChain, Table: iptables.Nat},
{Name: DockerChain, Table: iptables.Mangle},
{Name: DockerChain, Table: iptables.Filter},
{Name: IsolationChain, Table: iptables.Filter},
} {
Expand Down
2 changes: 1 addition & 1 deletion drivers/bridge/setup_ip_tables_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.isolationChain, d.mangleChain, err = setupIPChains(d.config)
if err != nil {
t.Fatal(err)
}
Expand Down
3 changes: 3 additions & 0 deletions iptables/firewalld_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ func TestReloaded(t *testing.T) {
var fwdChain *ChainInfo

fwdChain, err = NewChain("FWD", Filter, false)
if err != nil {
t.Fatal(err)
}
bridgeName := "lo"

err = ProgramChain(fwdChain, bridgeName, false, true)
Expand Down
Loading

0 comments on commit 6bcf354

Please sign in to comment.