d/libn/iptables: restore rules on firewalld stop

When firewalld is stopped with CleanupOnExit=yes (the default), it
deletes the iptables rules, including the daemon's rules for
masquerading, published ports and network isolation. The daemon does
not restore them until firewalld is started again or the daemon is
restarted.

firewalld deletes the rules before it releases its D-Bus name. Run the
reload callbacks when firewalld releases its name, as well as when it
acquires it. By then, UsingFirewalld() reports false and the callbacks
restore the rules using iptables directly.

Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit c4beb7e348)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon/libnetwork/iptables/firewalld.go

@@ -145,28 +145,17 @@ func signalHandler() {
 
 func dbusConnectionChanged(args []any) {
 	name := args[0].(string)
-	oldOwner := args[1].(string)
-	newOwner := args[2].(string)
 
 	if name != dbusInterface {
 		return
 	}
 
-	if newOwner != "" {
-		connectionEstablished()
-	} else if oldOwner != "" {
-		connectionLost()
-	}
-}
-
-func connectionEstablished() {
+	// With CleanupOnExit=yes, firewalld deletes the iptables rules before it
+	// releases its D-Bus name. Restore our iptables rules when firewalld
+	// starts or exits.
 	reloaded()
 }
 
-func connectionLost() {
-	// Doesn't do anything for now. Libvirt also doesn't react to this.
-}
-
 // call all callbacks
 func reloaded() {
 	firewalldReloadMu.Lock()

daemon/libnetwork/iptables/firewalld_test.go

@@ -89,6 +89,35 @@ func TestReloaded(t *testing.T) {
 	}
 }
 
+func TestDBusConnectionChanged(t *testing.T) {
+	saved := onReloaded
+	t.Cleanup(func() { onReloaded = saved })
+
+	tests := []struct {
+		name               string
+		service            string
+		oldOwner, newOwner string
+		expCalls           int
+	}{
+		{name: "firewalld started", service: dbusInterface, newOwner: ":1.42", expCalls: 1},
+		{name: "firewalld stopped", service: dbusInterface, oldOwner: ":1.42", expCalls: 1},
+		{name: "other service stopped", service: "org.example.Other", oldOwner: ":1.43", expCalls: 0},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			onReloaded = nil
+			var calls int
+			OnReloaded(func() { calls++ })
+			before := FirewalldReloadedAt()
+
+			dbusConnectionChanged([]any{tc.service, tc.oldOwner, tc.newOwner})
+
+			assert.Check(t, is.Equal(calls, tc.expCalls))
+			assert.Check(t, FirewalldReloadedAt().After(before) == (tc.expCalls > 0))
+		})
+	}
+}
+
 func TestPassthrough(t *testing.T) {
 	skipIfNoFirewalld(t)
 	rule1 := []string{

integration/networking/firewall_linux_test.go

@@ -1,15 +1,26 @@
 package networking
 
 import (
+	"context"
 	"os"
+	"slices"
+	"strings"
 	"testing"
+	"time"
 
+	networktypes "github.com/moby/moby/api/types/network"
 	"github.com/moby/moby/client"
+	"github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge"
+	"github.com/moby/moby/v2/integration/internal/container"
+	"github.com/moby/moby/v2/integration/internal/network"
 	"github.com/moby/moby/v2/integration/internal/testutils/networking"
 	"github.com/moby/moby/v2/internal/testutil/daemon"
 	"github.com/moby/moby/v2/internal/testutil/request"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
+	"gotest.tools/v3/icmd"
+	"gotest.tools/v3/poll"
+	"gotest.tools/v3/skip"
 )
 
 const defaultFirewallBackend = "iptables"
@@ -50,3 +61,120 @@ func TestInfoFirewallBackend(t *testing.T) {
 		assert.Check(t, is.Nil(info148.FirewallBackend))
 	})
 }
+
+// TestFirewalldStop checks that the daemon restores its iptables rules when
+// firewalld deletes them on stopping, and uses firewalld again when it is
+// started.
+func TestFirewalldStop(t *testing.T) {
+	skip.If(t, testEnv.IsRootless, "rootless mode does not use firewalld")
+	skip.If(t, testEnv.FirewallBackendDriver() != "iptables+firewalld", "test is for the iptables backend with firewalld")
+	ctx := setupTest(t)
+
+	d := daemon.New(t)
+	d.StartWithBusybox(ctx, t)
+	t.Cleanup(func() { d.Stop(t) })
+	c := d.NewClientT(t)
+	t.Cleanup(func() { c.Close() })
+
+	const bridgeName = "br-fwstop"
+	want := setupPublishedPort(ctx, t, c, bridgeName)
+
+	t.Cleanup(func() { startFirewalld(t) })
+	if !firewalldDeletesRules(t, func() { stopFirewalld(t) }) {
+		t.Skip("stopping firewalld does not delete iptables rules")
+	}
+	waitForRules(t, "after firewalld stopped", bridgeName, want)
+	assert.Check(t, is.Equal(d.FirewallBackendDriver(t), "iptables"))
+
+	deleted := firewalldDeletesRules(t, func() { startFirewalld(t) })
+	if deleted {
+		waitForRules(t, "after firewalld started", bridgeName, want)
+	}
+	assert.Check(t, is.Equal(d.FirewallBackendDriver(t), "iptables+firewalld"))
+}
+
+// setupPublishedPort creates a bridge network with the given bridge name, and
+// a container on it with a published port. It returns the iptables rules which
+// refer to the bridge.
+func setupPublishedPort(ctx context.Context, t *testing.T, c client.APIClient, bridgeName string) []string {
+	t.Helper()
+	const netName = "testnet"
+	network.CreateNoError(ctx, t, c, netName,
+		network.WithDriver("bridge"),
+		network.WithOption(bridge.BridgeName, bridgeName),
+	)
+	t.Cleanup(func() { network.RemoveNoError(ctx, t, c, netName) })
+	ctrID := container.Run(ctx, t, c,
+		container.WithNetworkMode(netName),
+		container.WithExposedPorts("80/tcp"),
+		container.WithPortMap(networktypes.PortMap{
+			networktypes.MustParsePort("80/tcp"): {{HostPort: "8080"}},
+		}),
+	)
+	t.Cleanup(func() { c.ContainerRemove(ctx, ctrID, client.ContainerRemoveOptions{Force: true}) })
+
+	rules := bridgeRules(t, bridgeName)
+	assert.Assert(t, len(rules) > 0, "no iptables rules refer to %s", bridgeName)
+	return rules
+}
+
+// bridgeRules returns the iptables rules which refer to bridgeName.
+func bridgeRules(t *testing.T, bridgeName string) []string {
+	t.Helper()
+	var rules []string
+	for _, cmd := range []string{"iptables-save", "ip6tables-save"} {
+		res := icmd.RunCommand(cmd)
+		res.Assert(t, icmd.Success)
+		for _, l := range strings.Split(res.Stdout(), "\n") {
+			if strings.Contains(l, bridgeName) {
+				rules = append(rules, cmd+": "+l)
+			}
+		}
+	}
+	return rules
+}
+
+// waitForRules waits until every rule in want is present.
+func waitForRules(t *testing.T, when, bridgeName string, want []string) {
+	t.Helper()
+	poll.WaitOn(t, func(poll.LogT) poll.Result {
+		got := bridgeRules(t, bridgeName)
+		var missing []string
+		for _, r := range want {
+			if !slices.Contains(got, r) {
+				missing = append(missing, r)
+			}
+		}
+		if len(missing) > 0 {
+			return poll.Continue("%s, rules are missing:\n%s", when, strings.Join(missing, "\n"))
+		}
+		return poll.Success()
+	}, poll.WithTimeout(30*time.Second))
+}
+
+// firewalldDeletesRules reports whether f deletes an iptables chain that the
+// test adds before calling it.
+func firewalldDeletesRules(t *testing.T, f func()) bool {
+	t.Helper()
+	const markerChain = "TEST-FIREWALLD-MARKER"
+	icmd.RunCommand("iptables", "-N", markerChain).Assert(t, icmd.Success)
+	t.Cleanup(func() { icmd.RunCommand("iptables", "-X", markerChain) })
+	f()
+	return icmd.RunCommand("iptables", "-S", markerChain).ExitCode != 0
+}
+
+func stopFirewalld(t *testing.T) {
+	t.Helper()
+	icmd.RunCommand("systemctl", "stop", "firewalld").Assert(t, icmd.Success)
+}
+
+func startFirewalld(t *testing.T) {
+	t.Helper()
+	icmd.RunCommand("systemctl", "start", "firewalld").Assert(t, icmd.Success)
+	poll.WaitOn(t, func(poll.LogT) poll.Result {
+		if !networking.FirewalldRunning() {
+			return poll.Continue("firewalld is not running")
+		}
+		return poll.Success()
+	}, poll.WithTimeout(30*time.Second))
+}