From 2cfe2d5bcebd64bf004bea4972c4eb589acfb53e Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Wed, 21 Oct 2026 14:12:46 -0400 Subject: [PATCH] d/libn/iptables: use firewalld when started later The daemon uses firewalld only if firewalld is running when the daemon starts. If firewalld is started later, the daemon does not react to it starting or reloading. On firewalld configurations which delete other applications' iptables rules when firewalld starts, the daemon's rules stay deleted. The daemon's bridges are not added to the docker zone. Keep the D-Bus connection when firewalld is not running at startup. When firewalld acquires its D-Bus name, set up the docker zone and forwarding policy before running the reload callbacks. Signed-off-by: Cory Snider (cherry picked from commit a57bf8e1358fc7b1ac4b71ab9451c26bf3fecacf) Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/iptables/firewalld.go | 49 ++++++++++++------- daemon/libnetwork/iptables/firewalld_test.go | 27 +++++++++- integration/networking/firewall_linux_test.go | 39 +++++++++++++++ 3 files changed, 95 insertions(+), 20 deletions(-) diff --git a/daemon/libnetwork/iptables/firewalld.go b/daemon/libnetwork/iptables/firewalld.go index 4d18c4d779..236a8899ca 100644 --- a/daemon/libnetwork/iptables/firewalld.go +++ b/daemon/libnetwork/iptables/firewalld.go @@ -88,29 +88,33 @@ func firewalldInit() error { return err } firewalldRunning.Store(running) + // Keep the connection when firewalld is not running, to start using + // firewalld if it is started later. + go signalHandler() if !running { - connection.sysconn.Close() - connection = nil + return nil } - if connection != nil { - go signalHandler() - zoneAdded, err := setupDockerZone() - if err != nil { + return setupFirewalld() +} + +// setupFirewalld creates the docker zone and forwarding policy in firewalld, +// if they do not already exist. +func setupFirewalld() error { + zoneAdded, err := setupDockerZone() + if err != nil { + return err + } + policyAdded, policyAddErr := setupDockerForwardingPolicy() + if policyAddErr != nil { + // Log the error, but still reload firewalld if necessary. + log.G(context.TODO()).WithError(policyAddErr).Warnf("Firewalld: failed to add policy %s", dockerFwdPolicy) + } + if zoneAdded || policyAdded { + // Reload for changes to take effect. + if err := connection.sysObj.Call(dbusInterface+".reload", 0).Err; err != nil { return err } - policyAdded, policyAddErr := setupDockerForwardingPolicy() - if policyAddErr != nil { - // Log the error, but still reload firewalld if necessary. - log.G(context.TODO()).WithError(policyAddErr).Warnf("Firewalld: failed to add policy %s", dockerFwdPolicy) - } - if zoneAdded || policyAdded { - // Reload for changes to take effect. - if err := connection.sysObj.Call(dbusInterface+".reload", 0).Err; err != nil { - return err - } - } } - return nil } @@ -186,6 +190,15 @@ func dbusConnectionChanged(args []any) { running := newOwner != "" firewalldRunning.Store(running) + if running { + // firewalld may have been started for the first time since the daemon + // started, or its permanent configuration may have changed. + // firewalld does not handle method calls until it has finished starting, + // so we can set up the docker zone as soon as it owns its name. + if err := setupFirewalld(); err != nil { + log.G(context.TODO()).WithError(err).Error("Firewalld: failed to set up docker zone") + } + } // With CleanupOnExit=yes, firewalld deletes the iptables rules before it // releases its D-Bus name. Restore our iptables rules when firewalld // starts or exits. diff --git a/daemon/libnetwork/iptables/firewalld_test.go b/daemon/libnetwork/iptables/firewalld_test.go index 254010e1ce..9905262579 100644 --- a/daemon/libnetwork/iptables/firewalld_test.go +++ b/daemon/libnetwork/iptables/firewalld_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net/netip" + "slices" "strconv" "testing" @@ -89,9 +90,21 @@ func TestReloaded(t *testing.T) { } } +// fakeFirewalldObject records the methods called on it, and fails every call. +type fakeFirewalldObject struct { + dbus.BusObject + methods []string +} + +func (o *fakeFirewalldObject) Call(method string, _ dbus.Flags, _ ...any) *dbus.Call { + o.methods = append(o.methods, method) + return &dbus.Call{Err: errors.New("fake firewalld")} +} + func TestDBusConnectionChanged(t *testing.T) { - savedCallbacks, savedRunning := onReloaded, firewalldRunning.Load() + savedConn, savedCallbacks, savedRunning := connection, onReloaded, firewalldRunning.Load() t.Cleanup(func() { + connection = savedConn onReloaded = savedCallbacks firewalldRunning.Store(savedRunning) }) @@ -102,20 +115,25 @@ func TestDBusConnectionChanged(t *testing.T) { wasRunning bool expRunning bool expCalls int + expSetup bool }{ - {name: "firewalld started", args: []any{dbusInterface, "", ":1.42"}, expRunning: true, expCalls: 1}, + {name: "firewalld started", args: []any{dbusInterface, "", ":1.42"}, expRunning: true, expCalls: 1, expSetup: true}, {name: "firewalld stopped", args: []any{dbusInterface, ":1.42", ""}, wasRunning: true, expRunning: false, expCalls: 1}, {name: "other service stopped", args: []any{"org.example.Other", ":1.43", ""}, wasRunning: true, expRunning: true, expCalls: 0}, {name: "malformed signal", args: []any{dbusInterface, ":1.42"}, wasRunning: true, expRunning: true, expCalls: 0}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + fw := &fakeFirewalldObject{} + connection = &Conn{sysObj: fw, sysConfObj: fw} onReloaded = nil var calls int var runningInCallback bool + var methodsBeforeCallback []string OnReloaded(func() { calls++ runningInCallback = firewalldRunning.Load() + methodsBeforeCallback = slices.Clone(fw.methods) }) firewalldRunning.Store(tc.wasRunning) before := FirewalldReloadedAt() @@ -127,6 +145,11 @@ func TestDBusConnectionChanged(t *testing.T) { if tc.expCalls > 0 { assert.Check(t, is.Equal(runningInCallback, tc.expRunning), "callbacks must see the new state") } + if tc.expSetup { + assert.Check(t, is.Contains(methodsBeforeCallback, dbusInterface+".zone.getZones"), "the docker zone must be set up before the callbacks run") + } else { + assert.Check(t, is.Len(fw.methods, 0)) + } assert.Check(t, FirewalldReloadedAt().After(before) == (tc.expCalls > 0)) }) } diff --git a/integration/networking/firewall_linux_test.go b/integration/networking/firewall_linux_test.go index f51673c4f2..9b543040d9 100644 --- a/integration/networking/firewall_linux_test.go +++ b/integration/networking/firewall_linux_test.go @@ -93,6 +93,45 @@ func TestFirewalldStop(t *testing.T) { assert.Check(t, is.Equal(d.FirewallBackendDriver(t), "iptables+firewalld")) } +// TestFirewalldStartedAfterDaemon checks that the daemon starts using firewalld +// when firewalld is started after the daemon. +func TestFirewalldStartedAfterDaemon(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) + + t.Cleanup(func() { startFirewalld(t) }) + stopFirewalld(t) + + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + t.Cleanup(func() { d.Stop(t) }) + c := d.NewClientT(t) + t.Cleanup(func() { c.Close() }) + assert.Check(t, is.Equal(d.FirewallBackendDriver(t), "iptables")) + + const bridgeName = "br-fwlate" + want := setupPublishedPort(ctx, t, c, bridgeName) + + deleted := firewalldDeletesRules(t, func() { startFirewalld(t) }) + poll.WaitOn(t, func(poll.LogT) poll.Result { + if drv := d.FirewallBackendDriver(t); drv != "iptables+firewalld" { + return poll.Continue("firewall backend driver is %q", drv) + } + return poll.Success() + }) + poll.WaitOn(t, func(poll.LogT) poll.Result { + res := icmd.RunCommand("firewall-cmd", "--zone=docker", "--list-interfaces") + if !slices.Contains(strings.Fields(res.Stdout()), bridgeName) { + return poll.Continue("%s is not in the docker zone: %s", bridgeName, res.Combined()) + } + return poll.Success() + }) + if deleted { + waitForRules(t, "after firewalld started", bridgeName, want) + } +} + // 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.