libnetwork: Controller: combine iptablesEnabled and ip6tablesEnabled

These functions were mostly identical, except for iptables being enabled
by default (unless explicitly disabled by config).

Rewrite the function to a enabledIptablesVersions, which returns the list
of iptables-versions that are enabled for the controller. This prevents
having to acquire a lock twice, and simplifies arrangeUserFilterRule, which
can now just iterate over the enabled versions.

Also moving this function to a linux-only file, as other platforms don't have
the iptables types defined.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

libnetwork/controller.go

@@ -64,7 +64,6 @@ import (
 	"github.com/docker/docker/libnetwork/drvregistry"
 	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/netlabel"
-	"github.com/docker/docker/libnetwork/options"
 	"github.com/docker/docker/libnetwork/osl"
 	"github.com/docker/docker/libnetwork/types"
 	"github.com/docker/docker/pkg/plugingetter"
@@ -1135,41 +1134,3 @@ func (c *Controller) IsDiagnosticEnabled() bool {
 	defer c.mu.Unlock()
 	return c.DiagnosticServer.IsDiagnosticEnabled()
 }
-
-func (c *Controller) iptablesEnabled() bool {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	if c.cfg == nil {
-		return false
-	}
-	// parse map cfg["bridge"]["generic"]["EnableIPTable"]
-	cfgBridge := c.cfg.DriverConfig("bridge")
-	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
-}
-
-func (c *Controller) ip6tablesEnabled() bool {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	if c.cfg == nil {
-		return false
-	}
-	// parse map cfg["bridge"]["generic"]["EnableIP6Table"]
-	cfgBridge := c.cfg.DriverConfig("bridge")
-	cfgGeneric, ok := cfgBridge[netlabel.GenericData].(options.Generic)
-	if !ok {
-		return false
-	}
-	enabled, _ := cfgGeneric["EnableIP6Tables"].(bool)
-	return enabled
-}

libnetwork/controller_linux.go

@@ -0,0 +1,33 @@
+package libnetwork
+
+import (
+	"github.com/docker/docker/libnetwork/iptables"
+	"github.com/docker/docker/libnetwork/netlabel"
+	"github.com/docker/docker/libnetwork/options"
+)
+
+// enabledIptablesVersions returns the iptables versions that are enabled
+// for the controller.
+func (c *Controller) enabledIptablesVersions() []iptables.IPVersion {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.cfg == nil {
+		return nil
+	}
+	// parse map cfg["bridge"]["generic"]["EnableIPTable"]
+	cfgBridge := c.cfg.DriverConfig("bridge")
+	cfgGeneric, ok := cfgBridge[netlabel.GenericData].(options.Generic)
+	if !ok {
+		return nil
+	}
+
+	var versions []iptables.IPVersion
+	if enabled, ok := cfgGeneric["EnableIPTables"].(bool); enabled || !ok {
+		// iptables is enabled unless user explicitly disabled it
+		versions = append(versions, iptables.IPv4)
+	}
+	if enabled, _ := cfgGeneric["EnableIP6Tables"].(bool); enabled {
+		versions = append(versions, iptables.IPv6)
+	}
+	return versions
+}

libnetwork/controller_others.go

@@ -0,0 +1,8 @@
+//go:build !linux
+
+package libnetwork
+
+// enabledIptablesVersions is a no-op on non-Linux systems.
+func (c *Controller) enabledIptablesVersions() []any {
+	return nil
+}

libnetwork/firewall_linux.go

@@ -16,47 +16,33 @@ func setupArrangeUserFilterRule(c *Controller) {
 	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.
+// arrangeUserFilterRule sets up the DOCKER-USER chain for each iptables version
+// (IPv4, IPv6) that's enabled in the controller's configuration.
+//
+// This chain allows users to configure firewall policies in a way that
+// persist daemon operations/restarts. The daemon does not delete or modify
+// any pre-existing rules from the DOCKER-USER filter chain.
+//
+// Once the DOCKER-USER chain is created, the daemon does not remove it when
+// IPTableForwarding is disabled, because it contains rules configured by user
+// that are beyond the daemon's control.
 func arrangeUserFilterRule() {
 	if ctrl == nil {
 		return
 	}
 
-	conds := []struct {
-		ipVer iptables.IPVersion
-		cond  bool
-	}{
-		{ipVer: iptables.IPv4, cond: ctrl.iptablesEnabled()},
-		{ipVer: iptables.IPv6, cond: ctrl.ip6tablesEnabled()},
-	}
-
-	for _, ipVerCond := range conds {
-		cond := ipVerCond.cond
-		if !cond {
-			continue
-		}
-
-		ipVer := ipVerCond.ipVer
-		iptable := iptables.GetIptable(ipVer)
-		_, err := iptable.NewChain(userChain, iptables.Filter, false)
-		if err != nil {
-			log.G(context.TODO()).WithError(err).Warnf("Failed to create %s %v chain", userChain, ipVer)
+	for _, ipVersion := range ctrl.enabledIptablesVersions() {
+		ipt := iptables.GetIptable(ipVersion)
+		if _, err := ipt.NewChain(userChain, iptables.Filter, false); err != nil {
+			log.G(context.TODO()).WithError(err).Warnf("Failed to create %s %v chain", userChain, ipVersion)
 			return
 		}
-
-		if err = iptable.AddReturnRule(userChain); err != nil {
-			log.G(context.TODO()).WithError(err).Warnf("Failed to add the RETURN rule for %s %v", userChain, ipVer)
+		if err := ipt.AddReturnRule(userChain); err != nil {
+			log.G(context.TODO()).WithError(err).Warnf("Failed to add the RETURN rule for %s %v", userChain, ipVersion)
 			return
 		}
-
-		err = iptable.EnsureJumpRule("FORWARD", userChain)
-		if err != nil {
-			log.G(context.TODO()).WithError(err).Warnf("Failed to ensure the jump rule for %s %v", userChain, ipVer)
+		if err := ipt.EnsureJumpRule("FORWARD", userChain); err != nil {
+			log.G(context.TODO()).WithError(err).Warnf("Failed to ensure the jump rule for %s %v", userChain, ipVersion)
 		}
 	}
 }