From a527e5a546526117a0f3d7a33f3fcb8f4cd87c72 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Wed, 12 Feb 2025 18:19:04 +0000 Subject: [PATCH] Restore iptables for current networks on firewalld reload Using iptables.OnReloaded to restore individual per-network rules on firewalld reload means rules for deleted networks pop back in to existence (because there was no way to delete the callbacks on network-delete). So, on firewalld reload, walk over current networks and ask them to restore their iptables rules. Signed-off-by: Rob Murray --- libnetwork/drivers/bridge/bridge_linux.go | 84 ++++++++++++++++++-- libnetwork/drivers/bridge/setup_firewalld.go | 15 ---- 2 files changed, 77 insertions(+), 22 deletions(-) delete mode 100644 libnetwork/drivers/bridge/setup_firewalld.go diff --git a/libnetwork/drivers/bridge/bridge_linux.go b/libnetwork/drivers/bridge/bridge_linux.go index 24fd34471d..6b8b691cc0 100644 --- a/libnetwork/drivers/bridge/bridge_linux.go +++ b/libnetwork/drivers/bridge/bridge_linux.go @@ -544,6 +544,7 @@ func (d *driver) configure(option map[string]interface{}) error { }) } } + iptables.OnReloaded(d.handleFirewalldReload) var pdc portDriverClient if config.Rootless { @@ -899,13 +900,6 @@ func (d *driver) createNetwork(config *networkConfiguration) (err error) { // Setup Loopback Addresses Routing {!d.config.EnableUserlandProxy, setupLoopbackAddressesRouting}, - // We want to track firewalld configuration so that - // if it is started/reloaded, the rules can be applied correctly - { - (config.EnableIPv4 && d.config.EnableIPTables) || (config.EnableIPv6 && d.config.EnableIP6Tables), - network.setupFirewalld, - }, - // Setup DefaultGatewayIPv4 {config.DefaultGatewayIPv4 != nil, setupGatewayIPv4}, @@ -1475,6 +1469,11 @@ func (d *driver) ProgramExternalConnectivity(ctx context.Context, nid, eid strin attribute.String("eid", eid))) defer span.End() + // Make sure the network isn't deleted, or in the middle of a firewalld reload, while + // updating its iptables rules. + d.configNetwork.Lock() + defer d.configNetwork.Unlock() + network, err := d.getNetwork(nid) if err != nil { return err @@ -1535,6 +1534,11 @@ func (d *driver) ProgramExternalConnectivity(ctx context.Context, nid, eid strin } func (d *driver) RevokeExternalConnectivity(nid, eid string) error { + // Make sure this function isn't deleting iptables rules while handleFirewalldReloadNw + // is restoring those same rules. + d.configNetwork.Lock() + defer d.configNetwork.Unlock() + network, err := d.getNetwork(nid) if err != nil { return err @@ -1568,6 +1572,72 @@ func (d *driver) RevokeExternalConnectivity(nid, eid string) error { return nil } +func (d *driver) handleFirewalldReload() { + if !d.config.EnableIPTables && !d.config.EnableIP6Tables { + return + } + + d.Lock() + nids := make([]string, 0, len(d.networks)) + for _, nw := range d.networks { + nids = append(nids, nw.id) + } + d.Unlock() + + for _, nid := range nids { + d.handleFirewalldReloadNw(nid) + } +} + +func (d *driver) handleFirewalldReloadNw(nid string) { + d.Lock() + defer d.Unlock() + + if !d.config.EnableIPTables && !d.config.EnableIP6Tables { + return + } + + // Make sure the network isn't being deleted, and ProgramExternalConnectivity/RevokeExternalConnectivity + // aren't modifying iptables rules, while restoring the rules. + d.configNetwork.Lock() + defer d.configNetwork.Unlock() + + nw, ok := d.networks[nid] + if !ok { + // Network deleted since the reload started, not an error. + return + } + + if err := nw.iptablesNetwork.reapplyNetworkLevelRules(); err != nil { + log.G(context.Background()).WithFields(log.Fields{ + "nid": nw.id, + "error": err, + }).Error("Failed to re-create packet filter on firewalld reload") + } + + // Re-add legacy links - only added during ProgramExternalConnectivity, but legacy + // links are default-bridge-only, and it's not possible to connect a container to + // the default bridge and a user-defined network. So, the default bridge is always + // the gateway and, if there are legacy links configured they need to be set up. + if !nw.config.EnableICC { + for _, ep := range nw.endpoints { + if err := d.link(nw, ep, true); err != nil { + log.G(context.Background()).WithFields(log.Fields{ + "nid": nw.id, + "eid": ep.id, + "error": err, + }).Error("Failed to re-create link on firewalld reload") + } + } + } + + // Set up per-port rules. These are also only set up during ProgramExternalConnectivity + // but the network's port bindings are only configured when it's acting as the + // gateway network. So, this is a no-op for networks that aren't providing endpoints + // with the gateway. + nw.reapplyPerPortIptables() +} + func LegacyContainerLinkOptions(parentEndpoints, childEndpoints []string) map[string]interface{} { return options.Generic{ netlabel.GenericData: options.Generic{ diff --git a/libnetwork/drivers/bridge/setup_firewalld.go b/libnetwork/drivers/bridge/setup_firewalld.go deleted file mode 100644 index f17a23d465..0000000000 --- a/libnetwork/drivers/bridge/setup_firewalld.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build linux - -package bridge - -import ( - "github.com/docker/docker/libnetwork/iptables" -) - -func (n *bridgeNetwork) setupFirewalld(config *networkConfiguration, i *bridgeInterface) error { - // FIXME(robmry) - these reload functions aren't deleted when the network is deleted. - // So, a firewalld reload leads to creation of zombie rules belonging to those networks. - iptables.OnReloaded(func() { n.iptablesNetwork.reapplyNetworkLevelRules() }) - iptables.OnReloaded(n.reapplyPerPortIptables) - return nil -}