External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 05:35:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

network: use previously saved list of firewall removal commands

Browse Source 
When destroying a network, the network driver has always assumed that
it knew what firewall rules had been added as the network was
started. This was usually correct - I only recall one time in the past
that the firewall rules added by libvirt were changed. But if the
exact rules used for a network *were* ever changed from one
build/version of libvirt to another, then we would end up attempting
to remove rules that hadn't been added, and could possibly *not*
remove rules that had been added.

The solution to this to not make such brash assumptions about the
past, but instead to save (in the network status object at network
start time) a list of all the rules needed to remove the rules that
were added for the network, and then use that saved list during
network destroy to remove exactly what was previous added.

Beyond making net-destroy more precise, there are other benefits:

1) We can change the details of the rules we add for networks from one
build/release of libvirt to another and painlessly upgrade.

2) The user can switch from one firewall backend to another by simply
changing the setting in network.conf and restarting
libvirtd/virtnetworkd.

In both cases, the restarted libvirtd/virtnetworkd will remove all the
rules that had been previously added (based on the network status),
and then add new rules (saving the new removal commands back into the
network status)

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Laine Stump 2024-04-19 22:19:42 -04:00
parent 0fa79844a1
commit 97061d576b
7 changed files with 91 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
src/network/bridge_driver.c
View File

@ -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 */

26
src/network/bridge_driver_linux.c
View File

@ -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);
}

6
src/network/bridge_driver_nop.c
View File

@ -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)
{
}

6
src/network/bridge_driver_platform.h
View File

@ -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);

43
src/network/network_iptables.c
View File

@ -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
* (<fwRemoval> element in the network status XML) to remove the
* network's firewall rules.
*/
void
iptablesRemoveFirewallRules(virNetworkDef *def)
{

2
src/network/network_iptables.h
View File

@ -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);

2
tests/networkxml2firewalltest.c
View File

@ -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);