Support nftables+firewalld

Signed-off-by: Rob Murray <rob.murray@docker.com>

daemon/libnetwork/controller_linux.go

@@ -17,18 +17,18 @@ import (
 
 // FirewallBackend returns the name of the firewall backend for "docker info".
 func (c *Controller) FirewallBackend() *system.FirewallInfo {
+	var info system.FirewallInfo
+	info.Driver = "iptables"
 	if nftables.Enabled() {
-		return &system.FirewallInfo{Driver: "nftables"}
+		info.Driver = "nftables"
 	}
 	if iptables.UsingFirewalld() {
-		info := &system.FirewallInfo{Driver: "iptables+firewalld"}
-		reloadedAt := iptables.FirewalldReloadedAt()
-		if !reloadedAt.IsZero() {
-			info.Info = append(info.Info, [2]string{"ReloadedAt", reloadedAt.Format(time.RFC3339)})
+		info.Driver += "+firewalld"
+		if reloadedAt := iptables.FirewalldReloadedAt(); !reloadedAt.IsZero() {
+			info.Info = [][2]string{{"ReloadedAt", reloadedAt.Format(time.RFC3339)}}
 		}
-		return info
 	}
-	return &system.FirewallInfo{Driver: "iptables"}
+	return &info
 }
 
 // enabledIptablesVersions returns the iptables versions that are enabled

daemon/libnetwork/drivers/bridge/bridge_linux.go

@@ -409,7 +409,7 @@ func parseErr(label, value, errString string) error {
 	return types.InvalidParameterErrorf("failed to parse %s value: %v (%s)", label, value, errString)
 }
 
-func (n *bridgeNetwork) newFirewallerNetwork(ctx context.Context) (firewaller.Network, error) {
+func (n *bridgeNetwork) newFirewallerNetwork(ctx context.Context) (_ firewaller.Network, retErr error) {
 	config4, err := makeNetworkConfigFam(n.config.HostIPv4, n.bridge.bridgeIPv4, n.gwMode(firewaller.IPv4))
 	if err != nil {
 		return nil, err
@@ -418,6 +418,18 @@ func (n *bridgeNetwork) newFirewallerNetwork(ctx context.Context) (firewaller.Ne
 	if err != nil {
 		return nil, err
 	}
+
+	if err := iptables.AddInterfaceFirewalld(n.config.BridgeName); err != nil {
+		return nil, err
+	}
+	defer func() {
+		if retErr != nil {
+			if err := iptables.DelInterfaceFirewalld(n.config.BridgeName); err != nil {
+				log.G(ctx).WithError(err).Errorf("failed to delete network level rules following error")
+			}
+		}
+	}()
+
 	return n.driver.firewaller.NewNetwork(ctx, firewaller.NetworkConfig{
 		IfName:                n.config.BridgeName,
 		Internal:              n.config.Internal,
@@ -1041,6 +1053,9 @@ func (d *driver) deleteNetwork(nid string) error {
 	if err := n.firewallerNetwork.DelNetworkLevelRules(context.TODO()); err != nil {
 		log.G(context.TODO()).WithError(err).Warnf("Failed to clean iptables rules for bridge network")
 	}
+	if err := iptables.DelInterfaceFirewalld(n.config.BridgeName); err != nil {
+		log.G(context.TODO()).WithError(err).Warnf("Failed to clean firewalld rules for bridge network")
+	}
 
 	return d.storeDelete(config)
 }
@@ -1750,6 +1765,14 @@ func (d *driver) handleFirewalldReloadNw(nid string) {
 	// gateway network. So, this is a no-op for networks that aren't providing endpoints
 	// with the gateway.
 	nw.reapplyPerPortIptables()
+
+	if err := iptables.AddInterfaceFirewalld(nw.config.BridgeName); err != nil {
+		log.G(context.Background()).WithFields(log.Fields{
+			"error":  err,
+			"nid":    nw.id,
+			"bridge": nw.config.BridgeName,
+		}).Error("Failed to add interface to docker zone on firewalld reload")
+	}
 }
 
 func LegacyContainerLinkOptions(parentEndpoints, childEndpoints []string) map[string]interface{} {

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

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"net/netip"
 
-	cerrdefs "github.com/containerd/errdefs"
 	"github.com/containerd/log"
 	"github.com/docker/docker/daemon/libnetwork/drivers/bridge/internal/firewaller"
 	"github.com/docker/docker/daemon/libnetwork/iptables"
@@ -96,16 +95,6 @@ func (n *network) setupIPTables(ctx context.Context, ipVersion iptables.IPVersio
 			return n.setupNonInternalNetworkRules(ctx, ipVersion, config, false)
 		})
 
-		if err := iptables.AddInterfaceFirewalld(n.config.IfName); err != nil {
-			return err
-		}
-		n.registerCleanFunc(func() error {
-			if err := iptables.DelInterfaceFirewalld(n.config.IfName); err != nil && !cerrdefs.IsNotFound(err) {
-				return err
-			}
-			return nil
-		})
-
 		if err := deleteLegacyFilterRules(ipVersion, n.config.IfName); err != nil {
 			return fmt.Errorf("failed to delete legacy rules in filter-FORWARD: %w", err)
 		}
@@ -430,17 +419,6 @@ func setupInternalNetworkRules(ctx context.Context, bridgeIface string, prefix n
 	var version iptables.IPVersion
 	var inDropRule, outDropRule iptables.Rule
 
-	// Either add or remove the interface from the firewalld zone, if firewalld is running.
-	if insert {
-		if err := iptables.AddInterfaceFirewalld(bridgeIface); err != nil {
-			return err
-		}
-	} else {
-		if err := iptables.DelInterfaceFirewalld(bridgeIface); err != nil && !cerrdefs.IsNotFound(err) {
-			return err
-		}
-	}
-
 	if prefix.Addr().Is4() {
 		version = iptables.IPv4
 		inDropRule = iptables.Rule{

daemon/libnetwork/firewall_linux.go

@@ -19,10 +19,6 @@ func (c *Controller) selectFirewallBackend() error {
 	}
 	// If configured to use nftables, but it can't be initialised, return an error.
 	if c.cfg.FirewallBackend == "nftables" {
-		// Don't try to enable nftables if firewalld is running.
-		if iptables.UsingFirewalld() {
-			return errors.New("firewall-backend is set to nftables, but firewalld is running")
-		}
 		if err := nftables.Enable(); err != nil {
 			return fmt.Errorf("firewall-backend is set to nftables: %v", err)
 		}

integration/daemon/daemon_test.go

@@ -734,7 +734,7 @@ func testLiveRestoreVolumeReferences(t *testing.T) {
 
 func testLiveRestoreUserChainsSetup(t *testing.T) {
 	skip.If(t, testEnv.IsRootless(), "rootless daemon uses it's own network namespace")
-	skip.If(t, testEnv.FirewallBackendDriver() == "nftables", "nftables enabled, skipping iptables test")
+	skip.If(t, strings.HasPrefix(testEnv.FirewallBackendDriver(), "nftables"), "nftables enabled, skipping iptables test")
 
 	t.Parallel()
 	ctx := testutil.StartSpan(baseContext, t)

integration/network/bridge/bridge_linux_test.go

@@ -607,8 +607,12 @@ func TestFirewalldReloadNoZombies(t *testing.T) {
 		}
 	}()
 
-	iptablesSave := icmd.Command("iptables-save")
-	resBeforeDel := icmd.RunCmd(iptablesSave)
+	saveCmd := []string{"iptables-save"}
+	if strings.HasPrefix(d.FirewallBackendDriver(t), "nftables") {
+		saveCmd = []string{"nft", "list ruleset"}
+	}
+	saveRules := icmd.Command(saveCmd[0], saveCmd[1:]...)
+	resBeforeDel := icmd.RunCmd(saveRules)
 	assert.NilError(t, resBeforeDel.Error)
 	assert.Check(t, strings.Contains(resBeforeDel.Combined(), bridgeName),
 		"With container: expected rules for %s in: %s", bridgeName, resBeforeDel.Combined())
@@ -619,7 +623,7 @@ func TestFirewalldReloadNoZombies(t *testing.T) {
 	removed = true
 
 	// Check the network does not appear in iptables rules.
-	resAfterDel := icmd.RunCmd(iptablesSave)
+	resAfterDel := icmd.RunCmd(saveRules)
 	assert.NilError(t, resAfterDel.Error)
 	assert.Check(t, !strings.Contains(resAfterDel.Combined(), bridgeName),
 		"After deletes: did not expect rules for %s in: %s", bridgeName, resAfterDel.Combined())
@@ -628,7 +632,7 @@ func TestFirewalldReloadNoZombies(t *testing.T) {
 	networking.FirewalldReload(t, d)
 
 	// Check that rules for the deleted container/network have not reappeared.
-	resAfterReload := icmd.RunCmd(iptablesSave)
+	resAfterReload := icmd.RunCmd(saveRules)
 	assert.NilError(t, resAfterReload.Error)
 	assert.Check(t, !strings.Contains(resAfterReload.Combined(), bridgeName),
 		"After deletes: did not expect rules for %s in: %s", bridgeName, resAfterReload.Combined())

integration/network/network_linux_test.go

@@ -93,7 +93,7 @@ func TestHostIPv4BridgeLabel(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Assert(t, len(out.IPAM.Config) > 0)
 	// Make sure the SNAT rule exists
-	if testEnv.FirewallBackendDriver() == "nftables" {
+	if strings.HasPrefix(testEnv.FirewallBackendDriver(), "nftables") {
 		chain := testutil.RunCommand(ctx, "nft", "--stateless", "list", "chain", "ip", "docker-bridges", "nat-postrouting-out__hostIPv4Bridge").Combined()
 		exp := fmt.Sprintf(`oifname != "hostIPv4Bridge" ip saddr %s counter snat to %s comment "SNAT"`,
 			out.IPAM.Config[0].Subnet, ipv4SNATAddr)

integration/networking/bridge_linux_test.go

@@ -1145,7 +1145,7 @@ func TestNoIP6Tables(t *testing.T) {
 			defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{Force: true})
 
 			var cmd *exec.Cmd
-			if d.FirewallBackendDriver(t) == "nftables" {
+			if strings.HasPrefix(d.FirewallBackendDriver(t), "nftables") {
 				cmd = exec.Command("nft", "list", "table", "ip6", "docker-bridges")
 			} else {
 				cmd = exec.Command("/usr/sbin/ip6tables-save")
@@ -1302,6 +1302,7 @@ func TestContainerDisabledIPv6(t *testing.T) {
 	ctx := setupTest(t)
 	d := daemon.New(t)
 	d.StartWithBusybox(ctx, t)
+	defer d.Stop(t)
 
 	c := d.NewClientT(t)
 	defer c.Close()

integration/networking/firewall_linux_test.go

@@ -20,8 +20,9 @@ func TestInfoFirewallBackend(t *testing.T) {
 	expDriver := defaultFirewallBackend
 	if val := os.Getenv("DOCKER_FIREWALL_BACKEND"); val != "" {
 		expDriver = val
-	} else if !testEnv.IsRootless() && networking.FirewalldRunning() {
-		expDriver = "iptables+firewalld"
+	}
+	if !testEnv.IsRootless() && networking.FirewalldRunning() {
+		expDriver += "+firewalld"
 	}
 	info, err := c.Info(ctx)
 	assert.NilError(t, err)

internal/testutils/networking/firewall.go

@@ -32,11 +32,11 @@ var rePolicy = lazyregexp.New("policy ([A-Za-z]+)")
 // behaviour.
 func SetFilterForwardPolicies(t *testing.T, firewallBackend string, policy string) {
 	t.Helper()
-	if strings.Contains(firewallBackend, "iptables") {
+	if strings.HasPrefix(firewallBackend, "iptables") {
 		setIptablesFFP(t, policy)
 		return
 	}
-	if firewallBackend == "nftables" {
+	if strings.HasPrefix(firewallBackend, "nftables") {
 		setNftablesFFP(t, policy)
 		return
 	}