Merge pull request #46043 from thaJeztah/cleanup_iptables_the_firewall_strikes_back

libnetwork/iptables: cleaning up: "there's more where that came from"

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/scope"
 	"github.com/docker/docker/libnetwork/types"
@@ -670,7 +669,14 @@ addToStore:
 		arrangeIngressFilterRule()
 		c.mu.Unlock()
 	}
-	arrangeUserFilterRule()
+
+	// Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6)
+	// that's enabled in the controller's configuration.
+	for _, ipVersion := range c.enabledIptablesVersions() {
+		if err := setupUserChain(ipVersion); err != nil {
+			log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name)
+		}
+	}
 
 	return nw, nil
 }
@@ -1137,41 +1143,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

@@ -2,6 +2,7 @@ package libnetwork
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/containerd/containerd/log"
 	"github.com/docker/docker/libnetwork/iptables"
@@ -16,47 +17,38 @@ 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.
 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)
-			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)
-			return
+	for _, ipVersion := range ctrl.enabledIptablesVersions() {
+		if err := setupUserChain(ipVersion); err != nil {
+			log.G(context.TODO()).WithError(err).Warn("arrangeUserFilterRule")
 		}
+	}
+}
 
-		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)
-		}
+// setupUserChain sets up the DOCKER-USER chain for the given [iptables.IPVersion].
+//
+// 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 setupUserChain(ipVersion iptables.IPVersion) error {
+	ipt := iptables.GetIptable(ipVersion)
+	if _, err := ipt.NewChain(userChain, iptables.Filter, false); err != nil {
+		return fmt.Errorf("failed to create %s %v chain: %v", userChain, ipVersion, err)
+	}
+	if err := ipt.AddReturnRule(userChain); err != nil {
+		return fmt.Errorf("failed to add the RETURN rule for %s %v: %w", userChain, ipVersion, err)
+	}
+	if err := ipt.EnsureJumpRule("FORWARD", userChain); err != nil {
+		return fmt.Errorf("failed to ensure the jump rule for %s %v: %w", userChain, ipVersion, err)
 	}
+	return nil
 }

libnetwork/firewall_others.go

@@ -4,3 +4,4 @@ package libnetwork
 
 func setupArrangeUserFilterRule(c *Controller) {}
 func arrangeUserFilterRule()                   {}
+func setupUserChain(ipVersion any) error       { return nil }

libnetwork/iptables/conntrack.go

@@ -14,19 +14,20 @@ import (
 	"github.com/vishvananda/netlink"
 )
 
-// ErrConntrackNotConfigurable means that conntrack module is not loaded or does not have the netlink module loaded
-var ErrConntrackNotConfigurable = errors.New("conntrack is not available")
-
-// IsConntrackProgrammable returns true if the handle supports the NETLINK_NETFILTER and the base modules are loaded
-func IsConntrackProgrammable(nlh *netlink.Handle) bool {
-	return nlh.SupportsNetlinkFamily(syscall.NETLINK_NETFILTER)
+// checkConntrackProgrammable checks if the handle supports the
+// NETLINK_NETFILTER and the base modules are loaded.
+func checkConntrackProgrammable(nlh *netlink.Handle) error {
+	if !nlh.SupportsNetlinkFamily(syscall.NETLINK_NETFILTER) {
+		return errors.New("conntrack is not available")
+	}
+	return nil
 }
 
 // DeleteConntrackEntries deletes all the conntrack connections on the host for the specified IP
 // Returns the number of flows deleted for IPv4, IPv6 else error
 func DeleteConntrackEntries(nlh *netlink.Handle, ipv4List []net.IP, ipv6List []net.IP) (uint, uint, error) {
-	if !IsConntrackProgrammable(nlh) {
-		return 0, 0, ErrConntrackNotConfigurable
+	if err := checkConntrackProgrammable(nlh); err != nil {
+		return 0, 0, err
 	}
 
 	var totalIPv4FlowPurged uint
@@ -54,8 +55,8 @@ func DeleteConntrackEntries(nlh *netlink.Handle, ipv4List []net.IP, ipv6List []n
 }
 
 func DeleteConntrackEntriesByPort(nlh *netlink.Handle, proto types.Protocol, ports []uint16) error {
-	if !IsConntrackProgrammable(nlh) {
-		return ErrConntrackNotConfigurable
+	if err := checkConntrackProgrammable(nlh); err != nil {
+		return err
 	}
 
 	var totalIPv4FlowPurged uint