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.