network: *un*set the firewalld zone while shutting down a network

When a bridge device for a virtual network had been placed in a
firewalld zone while starting the network, then even after the network
is shut down and the bridge device is deleted, its name will still
show up in the list of interfaces for whichever zone it had been in,
and this setting will persist through the next time a device with the
same name is created (until a zone is once again explicitly set, or
the device is removed via a firewalld API call).

Usually this isn't a problem, but in the case of forward mode='open',
someone might start the network once with a zone specified, then
shut down the network, remove the zone from its config, and start it
again; in this case the bridge device would come up using the zone
from the previous time it was started.

The solution to this is to remove the interface from whatever zone it
is in as the network is being shut down. There is no downside to doing
this, since the device is going to be deleted anyway. Note that
forward mode='bridge' uses a bridge device that was created outside of
libvirt, and libvirt won't be deleting that bridge, so we take care to
not unset the zone in that case.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index af40e5dca3145e55a8d3fafab2972841bbc6af55..f15d16c2921d12362ba53c371555098f0b4fc380 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2451,6 +2451,7 @@ virFirewallDGetPolicies;
 virFirewallDGetVersion;
 virFirewallDGetZones;
 virFirewallDInterfaceSetZone;
+virFirewallDInterfaceUnsetZone;
 virFirewallDIsRegistered;
 virFirewallDPolicyExists;
 virFirewallDSynchronize;

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c9c6fcbccc7fabcdac9a88b33de14d31df5dbeaf..74ba59b4e98c589f44e920ab539fdd835e4faefe 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2127,6 +2127,8 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
         def->forward.type != VIR_NETWORK_FORWARD_OPEN)
         networkRemoveFirewallRules(obj);
 
+    networkUnsetBridgeZone(def);
+
     virNetworkObjUnrefMacMap(obj);
 
     ignore_value(virNetDevBridgeDelete(def->bridge));
@@ -2165,6 +2167,8 @@ networkShutdownNetworkVirtual(virNetworkObj *obj)
     if (def->forward.type != VIR_NETWORK_FORWARD_OPEN)
         networkRemoveFirewallRules(obj);
 
+    networkUnsetBridgeZone(def);
+
     ignore_value(virNetDevBridgeDelete(def->bridge));
 
     /* See if its still alive and really really kill it */

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index af758d4f3da9d31db8c76eb1657bac541bd40195..3b3608c08586e4a3b4ebcb7ae39971c43ae2f37f 100644 (file)
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -392,6 +392,20 @@ networkSetBridgeZone(virNetworkDef *def)
 }
 
 
+void
+networkUnsetBridgeZone(virNetworkDef *def)
+{
+    /* If there is a libvirt-managed bridge device remove it from any
+     * zone it had been placed in as a part of deleting the bridge.
+     * DO NOT CALL THIS FOR 'bridge' forward mode, since that
+     * bridge is not managed by libvirt.
+     */
+    if (def->bridge && def->forward.type != VIR_NETWORK_FORWARD_BRIDGE
+        && virFirewallDIsRegistered() == 0) {
+        virFirewallDInterfaceUnsetZone(def->bridge);
+    }
+}
+
 int
 networkAddFirewallRules(virNetworkDef *def,
                         virFirewallBackend firewallBackend,

diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index 20c7a2a595e7221c231b3830bb264424735d5841..831a5a5010200a63f2437289401ecafbfb78b01b 100644 (file)
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -51,6 +51,12 @@ networkSetBridgeZone(virNetworkDef *def)
 }
 
 
+void
+networkUnsetBridgeZone(virNetworkDef *def G_GNUC_UNUSED)
+{
+}
+
+
 int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED,
                             virFirewallBackend firewallBackend,
                             virFirewall **fwRemoval G_GNUC_UNUSED)

diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 02abdc197f76897d51e3a3e4d98174be72bc607c..a0291532a1b1aa02fe3458d665a24543134e4ef8 100644 (file)
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -38,4 +38,6 @@ int networkAddFirewallRules(virNetworkDef *def,
                             virFirewallBackend firewallBackend,
                             virFirewall **fwRemoval);
 
+void networkUnsetBridgeZone(virNetworkDef *def);
+
 void networkRemoveFirewallRules(virNetworkObj *obj);

diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index 827e201dbbe1be361e7e3158081cba617ef30716..4aec33ac45d2d97e43bf4d5aafca0fc21a0a3b43 100644 (file)
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -449,6 +449,29 @@ virFirewallDInterfaceSetZone(const char *iface,
 }
 
 
+int
+virFirewallDInterfaceUnsetZone(const char *iface)
+{
+    GDBusConnection *sysbus = virGDBusGetSystemBus();
+    g_autoptr(GVariant) message = NULL;
+
+    if (!sysbus)
+        return -1;
+
+    message = g_variant_new("(ss)", "", iface);
+
+    return virGDBusCallMethod(sysbus,
+                             NULL,
+                             NULL,
+                             NULL,
+                             VIR_FIREWALL_FIREWALLD_SERVICE,
+                             "/org/fedoraproject/FirewallD1",
+                             "org.fedoraproject.FirewallD1.zone",
+                             "removeInterface",
+                             message);
+}
+
+
 void
 virFirewallDSynchronize(void)
 {

diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h
index 0e94d3507b8d496801c6594046e1a2bc72740a89..0dbe66d435d355a30f6b09389247439d431479f3 100644 (file)
--- a/src/util/virfirewalld.h
+++ b/src/util/virfirewalld.h
@@ -46,4 +46,6 @@ int virFirewallDApplyRule(virFirewallLayer layer,
 int virFirewallDInterfaceSetZone(const char *iface,
                                  const char *zone);
 
+int virFirewallDInterfaceUnsetZone(const char *iface);
+
 void virFirewallDSynchronize(void);