diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d14853cfd0..3c175786fe 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1685,6 +1685,7 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(networkGetDriver()); VIR_LOCK_GUARD lock = virObjectLockGuard(obj); virNetworkDef *def = virNetworkObjGetDef(obj); + virFirewall *fwRemoval = NULL; if (virNetworkObjIsActive(obj)) { switch ((virNetworkForwardType) def->forward.type) { @@ -1696,8 +1697,9 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, * network type, forward='open', doesn't need this because it * has no iptables rules. */ - networkRemoveFirewallRules(def, cfg->firewallBackend); - ignore_value(networkAddFirewallRules(def, cfg->firewallBackend)); + networkRemoveFirewallRules(obj); + ignore_value(networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval)); + virNetworkObjSetFwRemoval(obj, fwRemoval); break; case VIR_NETWORK_FORWARD_OPEN: @@ -1904,6 +1906,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, bool devOnline = false; bool firewalRulesAdded = false; virSocketAddr *dnsServer = NULL; + virFirewall *fwRemoval = NULL; /* Check to see if any network IP collides with an existing route */ if (networkCheckRouteCollision(def) < 0) @@ -1949,9 +1952,11 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, /* Add "once per network" rules */ if (def->forward.type != VIR_NETWORK_FORWARD_OPEN && - networkAddFirewallRules(def, cfg->firewallBackend) < 0) + networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval) < 0) { goto error; + } + virNetworkObjSetFwRemoval(obj, fwRemoval); firewalRulesAdded = true; for (i = 0; (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i)); i++) { @@ -2067,7 +2072,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, if (firewalRulesAdded && def->forward.type != VIR_NETWORK_FORWARD_OPEN) - networkRemoveFirewallRules(def, cfg->firewallBackend); + networkRemoveFirewallRules(obj); virNetworkObjUnrefMacMap(obj); @@ -2079,8 +2084,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, static int -networkShutdownNetworkVirtual(virNetworkObj *obj, - virNetworkDriverConfig *cfg) +networkShutdownNetworkVirtual(virNetworkObj *obj) { virNetworkDef *def = virNetworkObjGetDef(obj); pid_t dnsmasqPid; @@ -2106,7 +2110,7 @@ networkShutdownNetworkVirtual(virNetworkObj *obj, ignore_value(virNetDevSetOnline(def->bridge, false)); if (def->forward.type != VIR_NETWORK_FORWARD_OPEN) - networkRemoveFirewallRules(def, cfg->firewallBackend); + networkRemoveFirewallRules(obj); ignore_value(virNetDevBridgeDelete(def->bridge)); @@ -2410,7 +2414,7 @@ networkShutdownNetwork(virNetworkDriverState *driver, case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_OPEN: - ret = networkShutdownNetworkVirtual(obj, cfg); + ret = networkShutdownNetworkVirtual(obj); break; case VIR_NETWORK_FORWARD_BRIDGE: @@ -3215,6 +3219,8 @@ networkUpdate(virNetworkPtr net, virNetworkIPDef *ipdef; bool oldDhcpActive = false; bool needFirewallRefresh = false; + virFirewall *fwRemoval = NULL; + virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE | @@ -3261,7 +3267,7 @@ networkUpdate(virNetworkPtr net, * old rules (and remember to load new ones after the * update). */ - networkRemoveFirewallRules(def, cfg->firewallBackend); + networkRemoveFirewallRules(obj); needFirewallRefresh = true; break; default: @@ -3288,16 +3294,21 @@ networkUpdate(virNetworkPtr net, if (virNetworkObjUpdate(obj, command, section, parentIndex, xml, network_driver->xmlopt, flags) < 0) { - if (needFirewallRefresh) - ignore_value(networkAddFirewallRules(def, cfg->firewallBackend)); + if (needFirewallRefresh) { + ignore_value(networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval)); + virNetworkObjSetFwRemoval(obj, fwRemoval); + } goto cleanup; } /* @def is replaced */ def = virNetworkObjGetDef(obj); - if (needFirewallRefresh && networkAddFirewallRules(def, cfg->firewallBackend) < 0) + if (needFirewallRefresh && + networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval) < 0) { goto cleanup; + } + virNetworkObjSetFwRemoval(obj, fwRemoval); if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { /* save updated persistent config to disk */ diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 988362abef..b488600989 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -325,7 +325,8 @@ int networkCheckRouteCollision(virNetworkDef *def) int networkAddFirewallRules(virNetworkDef *def, - virFirewallBackend firewallBackend) + virFirewallBackend firewallBackend, + virFirewall **fwRemoval) { networkSetupPrivateChains(firewallBackend, false); @@ -411,13 +412,28 @@ networkAddFirewallRules(virNetworkDef *def, } } - return iptablesAddFirewallRules(def); + return iptablesAddFirewallRules(def, fwRemoval); } void -networkRemoveFirewallRules(virNetworkDef *def, - virFirewallBackend firewallBackend G_GNUC_UNUSED) +networkRemoveFirewallRules(virNetworkObj *obj) { - iptablesRemoveFirewallRules(def); + virFirewall *fw; + + if ((fw = virNetworkObjGetFwRemoval(obj)) == NULL) { + /* No information about firewall rules in the network status, + * so we assume the old iptables-based rules from 10.2.0 and + * earlier. + */ + VIR_DEBUG("No firewall info in network status, assuming old-style iptables"); + iptablesRemoveFirewallRules(virNetworkObjGetDef(obj)); + return; + } + + /* fwRemoval info was stored in the network status, so use that to + * remove the firewall + */ + VIR_DEBUG("Removing firewall rules with commands saved in network status"); + virFirewallApply(fw); } diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index 7d9a061e50..537b9234f8 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -37,12 +37,12 @@ int networkCheckRouteCollision(virNetworkDef *def G_GNUC_UNUSED) } int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED, - virFirewallBackend firewallBackend G_GNUC_UNUSED) + virFirewallBackend firewallBackend G_GNUC_UNUSED, + virFirewall **fwRemoval G_GNUC_UNUSED) { return 0; } -void networkRemoveFirewallRules(virNetworkDef *def G_GNUC_UNUSED, - virFirewallBackend firewallBackend G_GNUC_UNUSED) +void networkRemoveFirewallRules(virNetworkObj *obj G_GNUC_UNUSED) { } diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index 7443c3129f..cd2e3fa7b5 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -33,7 +33,7 @@ void networkPostReloadFirewallRules(bool startup); int networkCheckRouteCollision(virNetworkDef *def); int networkAddFirewallRules(virNetworkDef *def, - virFirewallBackend firewallBackend); + virFirewallBackend firewallBackend, + virFirewall **fwRemoval); -void networkRemoveFirewallRules(virNetworkDef *def, - virFirewallBackend firewallBackend); +void networkRemoveFirewallRules(virNetworkObj *obj); diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c index 467d43c1e9..f774176b3d 100644 --- a/src/network/network_iptables.c +++ b/src/network/network_iptables.c @@ -1591,9 +1591,19 @@ iptablesRemoveIPSpecificFirewallRules(virFirewall *fw, } -/* Add all rules for all ip addresses (and general rules) on a network */ +/* iptablesAddFirewallrules: + * + * @def - the network that needs an iptables firewall added + * @fwRemoval - if this is not NULL, it points to a pointer + * that should be filled in with a virFirewall object containing + * all the commands needed to remove this firewall at a later time. + * + * Add all rules for all ip addresses (and general rules) on a + * network, and optionally return a virFirewall object containing all + * the rules needed to later remove the firewall that has been added. +*/ int -iptablesAddFirewallRules(virNetworkDef *def) +iptablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval) { size_t i; virNetworkIPDef *ipdef; @@ -1614,10 +1624,35 @@ iptablesAddFirewallRules(virNetworkDef *def) VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK)); iptablesAddChecksumFirewallRules(fw, def); - return virFirewallApply(fw); + if (virFirewallApply(fw) < 0) + return -1; + + if (fwRemoval) { + /* caller wants us to create a virFirewall object that can be + * applied to undo everything that was just done by * virFirewallApply() + */ + + if (virFirewallNewFromRollback(fw, fwRemoval) < 0) + return -1; + } + + return 0; } -/* Remove all rules for all ip addresses (and general rules) on a network */ +/* iptablesRemoveFirewallRules: + * + * @def - the network that needs its iptables firewall rules removed + * + * Remove all rules for all ip addresses (and general rules) on a + * network that is being shut down. + * + * This function assumes the set of iptables rules that were added by + * all versions of libvirt prior to 10.4.0; any libvirt of that + * release or newer may or may not have this same set of rules, and + * should be using the list of commands saved in NetworkObj::fwRemoval + * ( element in the network status XML) to remove the + * network's firewall rules. + */ void iptablesRemoveFirewallRules(virNetworkDef *def) { diff --git a/src/network/network_iptables.h b/src/network/network_iptables.h index cdc143f154..d6ffc15bb0 100644 --- a/src/network/network_iptables.h +++ b/src/network/network_iptables.h @@ -23,7 +23,7 @@ #include "virfirewall.h" #include "network_conf.h" -int iptablesAddFirewallRules(virNetworkDef *def); +int iptablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval); void iptablesRemoveFirewallRules(virNetworkDef *def); diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index 3a9f409e2a..082979e5dc 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -98,7 +98,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(def = virNetworkDefParse(NULL, xml, NULL, false))) return -1; - if (networkAddFirewallRules(def, VIR_FIREWALL_BACKEND_IPTABLES) < 0) + if (networkAddFirewallRules(def, VIR_FIREWALL_BACKEND_IPTABLES, NULL) < 0) return -1; actual = actualargv = virBufferContentAndReset(&buf);