Merge pull request moby#50646 from robmry/nftables_no_enable_ip_forwarding

nftables: never enable IP forwarding on the host

Author: robmry

contrib/check-config.sh

@@ -128,6 +128,16 @@ check_device() {
 	fi
 }
 
+check_sysctl() {
+	val=$(sysctl -n $1)
+	want=$2
+	if [ "$val" = "$want" ]; then
+		wrap_good "sysctl $1" "enabled"
+	else
+		wrap_bad "sysctl $1" "disabled"
+	fi
+}
+
 if [ ! -e "$CONFIG" ]; then
 	wrap_warning "warning: $CONFIG does not exist, searching other paths for kernel config ..."
 	for tryConfig in $possibleConfigs; do
@@ -343,6 +353,10 @@ if ! is_set EXT4_FS || ! is_set EXT4_FS_POSIX_ACL || ! is_set EXT4_FS_SECURITY;
 fi
 
 echo '- Network Drivers:'
+echo "  - \"$(wrap_color 'bridge' blue)\":"
+check_sysctl net.ipv4.ip_forward 1 | sed 's/^/    - /'
+check_sysctl net.ipv6.conf.all.forwarding 1 | sed 's/^/    - /'
+check_sysctl net.ipv6.conf.default.forwarding 1 | sed 's/^/    - /'
 echo "  - \"$(wrap_color 'overlay' blue)\":"
 check_flags VXLAN BRIDGE_VLAN_FILTERING | sed 's/^/    /'
 echo '      Optional (for encrypted networks):'

contrib/dockerd-rootless.sh

@@ -199,5 +199,12 @@ else
 		mount_directory /etc/ssl "--rbind"
 	fi
 
+	# When running with --firewall-backend=nftables, IP forwarding needs to be enabled
+	# because the daemon won't enable it. IP forwarding is harmless in the rootless
+	# netns, there's only a single external interface and only Docker uses the netns.
+	# So, always enable IPv4 and IPv6 forwarding.
+	sysctl -w net.ipv4.ip_forward=1
+	sysctl -w net.ipv6.conf.all.forwarding=1
+
 	exec "$dockerd" "$@"
 fi

daemon/libnetwork/drivers/bridge/bridge_linux.go

@@ -556,7 +556,7 @@ var newFirewaller = func(ctx context.Context, config firewaller.Config) (firewal
 		// cleaner can't clean up network or port-specific rules that may have been added
 		// to iptables built-in chains. So, if cleanup is needed, give the cleaner to
 		// the nftabler. Then, it'll use it to delete old rules as networks are restored.
-		fw.(firewaller.FirewallCleanerSetter).SetFirewallCleaner(iptabler.NewCleaner(ctx, config))
+		fw.SetFirewallCleaner(iptabler.NewCleaner(ctx, config))
 		return fw, nil
 	}
 
@@ -880,14 +880,28 @@ func (d *driver) createNetwork(ctx context.Context, config *networkConfiguration
 			config.EnableIPv4 && d.config.EnableIPForwarding,
 			"setupIPv4Forwarding",
 			func(*networkConfiguration, *bridgeInterface) error {
-				return setupIPv4Forwarding(d.firewaller, d.config.EnableIPTables && !d.config.DisableFilterForwardDrop)
+				ffd, ok := d.firewaller.(filterForwardDropper)
+				if !ok {
+					// The firewaller can't drop non-Docker forwarding. It's up to the user to enable
+					// forwarding on their host, and configure their firewall appropriately.
+					return checkIPv4Forwarding()
+				}
+				// Enable forwarding and set a default-drop forwarding policy if necessary.
+				return setupIPv4Forwarding(ffd, d.config.EnableIPTables && !d.config.DisableFilterForwardDrop)
 			},
 		},
 		{
 			config.EnableIPv6 && d.config.EnableIPForwarding,
 			"setupIPv6Forwarding",
 			func(*networkConfiguration, *bridgeInterface) error {
-				return setupIPv6Forwarding(d.firewaller, d.config.EnableIP6Tables && !d.config.DisableFilterForwardDrop)
+				ffd, ok := d.firewaller.(filterForwardDropper)
+				if !ok {
+					// The firewaller can't drop non-Docker forwarding. It's up to the user to enable
+					// forwarding on their host, and configure their firewall appropriately.
+					return checkIPv6Forwarding()
+				}
+				// Enable forwarding and set a default-drop forwarding policy if necessary.
+				return setupIPv6Forwarding(ffd, d.config.EnableIP6Tables && !d.config.DisableFilterForwardDrop)
 			},
 		},
 

daemon/libnetwork/drivers/bridge/bridge_store.go

@@ -13,7 +13,7 @@ import (
 	"github.com/containerd/log"
 	"github.com/moby/moby/v2/daemon/internal/otelutil"
 	"github.com/moby/moby/v2/daemon/libnetwork/datastore"
-	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/firewaller"
+	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/nftabler"
 	"github.com/moby/moby/v2/daemon/libnetwork/portmapperapi"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 	"go.opentelemetry.io/otel"
@@ -43,8 +43,8 @@ func (d *driver) initStore() error {
 
 	// If there's a firewall cleaner, it's done its job by cleaning up rules
 	// belonging to the restored networks. So, drop it.
-	if fcs, ok := d.firewaller.(firewaller.FirewallCleanerSetter); ok {
-		fcs.SetFirewallCleaner(nil)
+	if nft, ok := d.firewaller.(*nftabler.Nftabler); ok {
+		nft.SetFirewallCleaner(nil)
 	}
 
 	return nil

daemon/libnetwork/drivers/bridge/internal/firewaller/firewaller.go

@@ -76,9 +76,6 @@ type Firewaller interface {
 	// NewNetwork returns an object that can be used to add published ports and legacy
 	// links for a bridge network.
 	NewNetwork(ctx context.Context, nc NetworkConfig) (Network, error)
-	// FilterForwardDrop sets the default policy of the FORWARD chain in the filter
-	// table to DROP.
-	FilterForwardDrop(ctx context.Context, ipv IPVersion) error
 }
 
 // Network can be used to manipulate firewall rules for a bridge network.
@@ -110,12 +107,6 @@ type Network interface {
 	DelLink(ctx context.Context, parentIP, childIP netip.Addr, ports []types.TransportPort)
 }
 
-// FirewallCleanerSetter is an optional interface for a Firewaller.
-type FirewallCleanerSetter interface {
-	// SetFirewallCleaner replaces the FirewallCleaner (possibly with 'nil').
-	SetFirewallCleaner(FirewallCleaner)
-}
-
 // FirewallCleaner is used to delete rules created by previous incarnations of
 // the daemon. On startup, once a Firewaller implementation has been selected, if
 // rules may have been left behind by a different Firewaller implementation, get

daemon/libnetwork/drivers/bridge/internal/firewaller/stub.go

@@ -15,7 +15,6 @@ import (
 type StubFirewaller struct {
 	Config
 	Networks map[string]*StubFirewallerNetwork
-	FFD      map[IPVersion]bool // filter forward drop
 }
 
 func NewStubFirewaller(config Config) *StubFirewaller {
@@ -24,7 +23,6 @@ func NewStubFirewaller(config Config) *StubFirewaller {
 		// A real Firewaller shouldn't hold on to its own networks, the bridge driver is doing that.
 		// But, for unit tests cross-checking the driver, this is useful.
 		Networks: make(map[string]*StubFirewallerNetwork),
-		FFD:      make(map[IPVersion]bool),
 	}
 }
 
@@ -41,11 +39,6 @@ func (fw *StubFirewaller) NewNetwork(_ context.Context, nc NetworkConfig) (Netwo
 	return nw, nil
 }
 
-func (fw *StubFirewaller) FilterForwardDrop(_ context.Context, ipv IPVersion) error {
-	fw.FFD[ipv] = true
-	return nil
-}
-
 type stubFirewallerLink struct {
 	parentIP netip.Addr
 	childIP  netip.Addr

daemon/libnetwork/drivers/bridge/internal/iptabler/cleaner.go

@@ -17,7 +17,7 @@ type iptablesCleaner struct {
 }
 
 // NewCleaner checks for iptables rules left behind by an old daemon that was using
-// the iptabler.
+// the Iptabler.
 //
 // If there are old rules present, it deletes as much as possible straight away
 // (user-defined chains and jumps from the built-in chains).
@@ -46,6 +46,13 @@ func NewCleaner(ctx context.Context, config firewaller.Config) firewaller.Firewa
 		_ = t.DeleteJumpRule(iptables.Filter, "FORWARD", DockerForwardChain)
 		_ = deleteLegacyTopLevelRules(ctx, t, ipv)
 		removeIPChains(ctx, ipv)
+		// The iptables chains will no longer have Docker's ACCEPT rules. So, if the
+		// filter-FORWARD chain has policy DROP (possibly set by Docker when it enabled
+		// IP forwarding), packets accepted by nftables chains will still be processed by
+		// iptables and dropped. It's the user's responsibility to sort that out.
+		if t.HasPolicy("filter", "FORWARD", iptables.Drop) {
+			log.G(ctx).WithField("ipv", ipv).Warn("Network traffic for published ports may be dropped, iptables chain FORWARD has policy DROP.")
+		}
 		return true
 	}
 	cleaned4 := clean(iptables.IPv4, config.IPv4)
@@ -62,7 +69,7 @@ func (ic iptablesCleaner) DelNetwork(ctx context.Context, nc firewaller.NetworkC
 	}
 	n := network{
 		config: nc,
-		ipt:    &iptabler{config: ic.config},
+		ipt:    &Iptabler{config: ic.config},
 	}
 	if ic.config.IPv4 && nc.Config4.Prefix.IsValid() {
 		_ = deleteLegacyFilterRules(iptables.IPv4, nc.IfName)
@@ -77,7 +84,7 @@ func (ic iptablesCleaner) DelNetwork(ctx context.Context, nc firewaller.NetworkC
 func (ic iptablesCleaner) DelEndpoint(ctx context.Context, nc firewaller.NetworkConfig, epIPv4, epIPv6 netip.Addr) {
 	n := network{
 		config: nc,
-		ipt:    &iptabler{config: ic.config},
+		ipt:    &Iptabler{config: ic.config},
 	}
 	if n.ipt.config.IPv4 && epIPv4.IsValid() {
 		_ = n.filterDirectAccess(ctx, iptables.IPv4, n.config.Config4, epIPv4, false)
@@ -90,7 +97,7 @@ func (ic iptablesCleaner) DelEndpoint(ctx context.Context, nc firewaller.Network
 func (ic iptablesCleaner) DelPorts(ctx context.Context, nc firewaller.NetworkConfig, pbs []types.PortBinding) {
 	n := network{
 		config: nc,
-		ipt:    &iptabler{config: ic.config},
+		ipt:    &Iptabler{config: ic.config},
 	}
 	_ = n.DelPorts(ctx, pbs)
 }

daemon/libnetwork/drivers/bridge/internal/iptabler/iptabler.go

@@ -35,12 +35,12 @@ const (
 	isolationChain2 = "DOCKER-ISOLATION-STAGE-2"
 )
 
-type iptabler struct {
+type Iptabler struct {
 	config firewaller.Config
 }
 
-func NewIptabler(ctx context.Context, config firewaller.Config) (firewaller.Firewaller, error) {
-	ipt := &iptabler{config: config}
+func NewIptabler(ctx context.Context, config firewaller.Config) (*Iptabler, error) {
+	ipt := &Iptabler{config: config}
 
 	if ipt.config.IPv4 {
 		removeIPChains(ctx, iptables.IPv4)
@@ -91,7 +91,8 @@ func NewIptabler(ctx context.Context, config firewaller.Config) (firewaller.Fire
 	return ipt, nil
 }
 
-func (ipt *iptabler) FilterForwardDrop(ctx context.Context, ipv firewaller.IPVersion) error {
+// FilterForwardDrop sets the default policy of the FORWARD chain in the filter table to DROP.
+func (ipt *Iptabler) FilterForwardDrop(ctx context.Context, ipv firewaller.IPVersion) error {
 	var iptv iptables.IPVersion
 	switch ipv {
 	case firewaller.IPv4:

daemon/libnetwork/drivers/bridge/internal/iptabler/network.go

@@ -22,11 +22,11 @@ type (
 
 type network struct {
 	config     firewaller.NetworkConfig
-	ipt        *iptabler
+	ipt        *Iptabler
 	cleanFuncs iptablesCleanFuncs
 }
 
-func (ipt *iptabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) {
+func (ipt *Iptabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) {
 	n := &network{
 		ipt:    ipt,
 		config: nc,

daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go

@@ -11,7 +11,7 @@ import (
 	"github.com/moby/moby/v2/daemon/libnetwork/internal/nftables"
 )
 
-// Cleanup deletes all rules created by nftabler; it's intended to be used
+// Cleanup deletes all rules created by Nftabler; it's intended to be used
 // during startup, to clean up rules created by an old incarnation of the daemon
 // after switching to a different Firewaller implementation.
 func Cleanup(ctx context.Context, config firewaller.Config) {
@@ -31,6 +31,6 @@ func Cleanup(ctx context.Context, config firewaller.Config) {
 	}
 }
 
-func (nft *nftabler) SetFirewallCleaner(fc firewaller.FirewallCleaner) {
+func (nft *Nftabler) SetFirewallCleaner(fc firewaller.FirewallCleaner) {
 	nft.cleaner = fc
 }