diff --git a/libnetwork/controller.go b/libnetwork/controller.go index 5a499aa428..dad968edce 100644 --- a/libnetwork/controller.go +++ b/libnetwork/controller.go @@ -67,6 +67,7 @@ import ( "github.com/docker/libnetwork/hostdiscovery" "github.com/docker/libnetwork/ipamapi" "github.com/docker/libnetwork/netlabel" + "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/osl" "github.com/docker/libnetwork/types" "github.com/pkg/errors" @@ -252,6 +253,7 @@ func New(cfgOptions ...config.Option) (NetworkController, error) { return nil, err } + setupArrangeUserFilterRule(c) return c, nil } @@ -909,8 +911,7 @@ addToStore: arrangeIngressFilterRule() c.Unlock() } - - c.arrangeUserFilterRule() + arrangeUserFilterRule() return network, nil } @@ -1363,3 +1364,27 @@ func (c *controller) IsDiagnosticEnabled() bool { defer c.Unlock() return c.DiagnosticServer.IsDiagnosticEnabled() } + +func (c *controller) iptablesEnabled() bool { + c.Lock() + defer c.Unlock() + + if c.cfg == nil { + return false + } + // parse map cfg["bridge"]["generic"]["EnableIPTable"] + cfgBridge, ok := c.cfg.Daemon.DriverCfg["bridge"].(map[string]interface{}) + if !ok { + return false + } + cfgGeneric, ok := cfgBridge[netlabel.GenericData].(options.Generic) + if !ok { + return false + } + enabled, ok := cfgGeneric["EnableIPTables"].(bool) + if !ok { + // unless user explicitly stated, assume iptable is enabled + enabled = true + } + return enabled +} diff --git a/libnetwork/firewall_linux.go b/libnetwork/firewall_linux.go index 54f9621f81..26ee91346e 100644 --- a/libnetwork/firewall_linux.go +++ b/libnetwork/firewall_linux.go @@ -7,21 +7,25 @@ import ( const userChain = "DOCKER-USER" -func (c *controller) arrangeUserFilterRule() { - c.Lock() - arrangeUserFilterRule() - c.Unlock() - iptables.OnReloaded(func() { - c.Lock() - arrangeUserFilterRule() - c.Unlock() - }) +var ( + ctrl *controller = nil +) + +func setupArrangeUserFilterRule(c *controller) { + ctrl = c + iptables.OnReloaded(arrangeUserFilterRule) } // This chain allow users to configure firewall policies in a way that persists // docker operations/restarts. Docker will not delete or modify any pre-existing // rules from the DOCKER-USER filter chain. +// Note once DOCKER-USER chain is created, docker engine does not remove it when +// IPTableForwarding is disabled, because it contains rules configured by user that +// are beyond docker engine's control. func arrangeUserFilterRule() { + if ctrl == nil || !ctrl.iptablesEnabled() { + return + } _, err := iptables.NewChain(userChain, iptables.Filter, false) if err != nil { logrus.Warnf("Failed to create %s chain: %v", userChain, err) diff --git a/libnetwork/firewall_others.go b/libnetwork/firewall_others.go index 901f568fed..4f72ae9df3 100644 --- a/libnetwork/firewall_others.go +++ b/libnetwork/firewall_others.go @@ -2,5 +2,5 @@ package libnetwork -func (c *controller) arrangeUserFilterRule() { -} +func setupArrangeUserFilterRule(c *controller) {} +func arrangeUserFilterRule() {} diff --git a/libnetwork/firewall_test.go b/libnetwork/firewall_test.go new file mode 100644 index 0000000000..65a12f5a48 --- /dev/null +++ b/libnetwork/firewall_test.go @@ -0,0 +1,97 @@ +package libnetwork + +import ( + "fmt" + "gotest.tools/assert" + "strings" + "testing" + + "github.com/docker/libnetwork/iptables" + "github.com/docker/libnetwork/netlabel" + "github.com/docker/libnetwork/options" +) + +const ( + fwdChainName = "FORWARD" + usrChainName = userChain +) + +func TestUserChain(t *testing.T) { + nc, err := New() + assert.NilError(t, err) + + tests := []struct { + iptables bool + insert bool // insert other rules to FORWARD + fwdChain []string + userChain []string + }{ + { + iptables: false, + insert: false, + fwdChain: []string{"-P FORWARD ACCEPT"}, + }, + { + iptables: true, + insert: false, + fwdChain: []string{"-P FORWARD ACCEPT", "-A FORWARD -j DOCKER-USER"}, + userChain: []string{"-N DOCKER-USER", "-A DOCKER-USER -j RETURN"}, + }, + { + iptables: true, + insert: true, + fwdChain: []string{"-P FORWARD ACCEPT", "-A FORWARD -j DOCKER-USER", "-A FORWARD -j DROP"}, + userChain: []string{"-N DOCKER-USER", "-A DOCKER-USER -j RETURN"}, + }, + } + + resetIptables(t) + for _, tc := range tests { + tc := tc + t.Run(fmt.Sprintf("iptables=%v,insert=%v", tc.iptables, tc.insert), func(t *testing.T) { + c := nc.(*controller) + c.cfg.Daemon.DriverCfg["bridge"] = map[string]interface{}{ + netlabel.GenericData: options.Generic{ + "EnableIPTables": tc.iptables, + }, + } + + // init. condition, FORWARD chain empty DOCKER-USER not exist + assert.DeepEqual(t, getRules(t, fwdChainName), []string{"-P FORWARD ACCEPT"}) + + if tc.insert { + _, err = iptables.Raw("-A", fwdChainName, "-j", "DROP") + assert.NilError(t, err) + } + arrangeUserFilterRule() + + assert.DeepEqual(t, getRules(t, fwdChainName), tc.fwdChain) + if tc.userChain != nil { + assert.DeepEqual(t, getRules(t, usrChainName), tc.userChain) + } else { + _, err := iptables.Raw("-S", usrChainName) + assert.Assert(t, err != nil, "chain %v: created unexpectedly", usrChainName) + } + }) + resetIptables(t) + } +} + +func getRules(t *testing.T, chain string) []string { + t.Helper() + output, err := iptables.Raw("-S", chain) + assert.NilError(t, err, "chain %s: failed to get rules", chain) + + rules := strings.Split(string(output), "\n") + if len(rules) > 0 { + rules = rules[:len(rules)-1] + } + return rules +} + +func resetIptables(t *testing.T) { + t.Helper() + _, err := iptables.Raw("-F", fwdChainName) + assert.NilError(t, err) + _ = iptables.RemoveExistingChain(usrChainName, "") +}