334 lines
14 KiB
Diff
334 lines
14 KiB
Diff
|
From 383ace4feee4ae610917edf8a583257a67a7d52c Mon Sep 17 00:00:00 2001
|
||
|
Message-ID: <383ace4feee4ae610917edf8a583257a67a7d52c.1728560653.git.jdenemar@redhat.com>
|
||
|
From: Laine Stump <laine@redhat.com>
|
||
|
Date: Fri, 4 Oct 2024 17:17:59 -0400
|
||
|
Subject: [PATCH] network: call network(Add|Remove)FirewallRules() for forward
|
||
|
mode='open'
|
||
|
|
||
|
Previously networkAddFirewallRules() and networkRemoveFirewallRules()
|
||
|
were only called if the forward mode was none, 'route', or 'nat', so
|
||
|
those functions didn't check the forward mode. Although their current
|
||
|
contents shouldn't be executed for forward mode='open', soon they will
|
||
|
have extra functionality that should be executed for all the current
|
||
|
forward modes and also mode='open'.
|
||
|
|
||
|
This patch modifies all places either of the functions are called to
|
||
|
make sure they are called for mode='open' in addition to current modes
|
||
|
(by either adding 'case ..._OPEN:' to the case of a switch statement,
|
||
|
or just removing an 'if (mode != ...OPEN)' around the calls; to
|
||
|
balance out for that, it puts the entirety of the contents of both
|
||
|
functions inside if (mode != ...OPEN) to retain current behavior. (an
|
||
|
upcoming patch will add code outside that if clause).
|
||
|
|
||
|
debug log messages were also added to make it easier to test that the
|
||
|
right thing is being done in all cases.
|
||
|
|
||
|
Signed-off-by: Laine Stump <laine@redhat.com>
|
||
|
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
|
||
|
(cherry picked from commit d552d810b97d478675eac830164349d8a1a35e63)
|
||
|
|
||
|
https://issues.redhat.com/browse/RHEL-61752
|
||
|
|
||
|
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
|
||
|
---
|
||
|
src/network/bridge_driver.c | 26 ++---
|
||
|
src/network/bridge_driver_linux.c | 175 +++++++++++++++++-------------
|
||
|
2 files changed, 110 insertions(+), 91 deletions(-)
|
||
|
|
||
|
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
|
||
|
index fe053f423a..f604b2695c 100644
|
||
|
--- a/src/network/bridge_driver.c
|
||
|
+++ b/src/network/bridge_driver.c
|
||
|
@@ -1735,10 +1735,15 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj,
|
||
|
case VIR_NETWORK_FORWARD_NONE:
|
||
|
case VIR_NETWORK_FORWARD_NAT:
|
||
|
case VIR_NETWORK_FORWARD_ROUTE:
|
||
|
- /* Only three of the L3 network types that are configured by
|
||
|
- * libvirt need to have iptables rules reloaded. The 4th L3
|
||
|
- * network type, forward='open', doesn't need this because it
|
||
|
- * has no iptables rules.
|
||
|
+ case VIR_NETWORK_FORWARD_OPEN:
|
||
|
+ /* even 'open' forward type networks need to call
|
||
|
+ * networkAdd/RemoveFirewallRules() in spite of the fact
|
||
|
+ * that, by definition, libvirt doesn't add any firewall
|
||
|
+ * rules for those networks.. This is because libvirt
|
||
|
+ * *does* support explicitly naming (in the config) a
|
||
|
+ * firewalld zone the network's bridge should be added to,
|
||
|
+ * and this functionality is also handled by
|
||
|
+ * networkAdd/RemoveFirewallRules()
|
||
|
*/
|
||
|
networkRemoveFirewallRules(obj);
|
||
|
ignore_value(networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval));
|
||
|
@@ -1746,7 +1751,6 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj,
|
||
|
saveStatus = true;
|
||
|
break;
|
||
|
|
||
|
- case VIR_NETWORK_FORWARD_OPEN:
|
||
|
case VIR_NETWORK_FORWARD_BRIDGE:
|
||
|
case VIR_NETWORK_FORWARD_PRIVATE:
|
||
|
case VIR_NETWORK_FORWARD_VEPA:
|
||
|
@@ -2000,10 +2004,8 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
|
||
|
goto error;
|
||
|
|
||
|
/* Add "once per network" rules */
|
||
|
- if (def->forward.type != VIR_NETWORK_FORWARD_OPEN &&
|
||
|
- networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval) < 0) {
|
||
|
+ if (networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval) < 0)
|
||
|
goto error;
|
||
|
- }
|
||
|
|
||
|
virNetworkObjSetFwRemoval(obj, fwRemoval);
|
||
|
firewalRulesAdded = true;
|
||
|
@@ -2119,8 +2121,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
|
||
|
if (devOnline)
|
||
|
ignore_value(virNetDevSetOnline(def->bridge, false));
|
||
|
|
||
|
- if (firewalRulesAdded &&
|
||
|
- def->forward.type != VIR_NETWORK_FORWARD_OPEN)
|
||
|
+ if (firewalRulesAdded)
|
||
|
networkRemoveFirewallRules(obj);
|
||
|
|
||
|
virNetworkObjUnrefMacMap(obj);
|
||
|
@@ -2158,8 +2159,7 @@ networkShutdownNetworkVirtual(virNetworkObj *obj)
|
||
|
|
||
|
ignore_value(virNetDevSetOnline(def->bridge, false));
|
||
|
|
||
|
- if (def->forward.type != VIR_NETWORK_FORWARD_OPEN)
|
||
|
- networkRemoveFirewallRules(obj);
|
||
|
+ networkRemoveFirewallRules(obj);
|
||
|
|
||
|
ignore_value(virNetDevBridgeDelete(def->bridge));
|
||
|
|
||
|
@@ -3307,6 +3307,7 @@ networkUpdate(virNetworkPtr net,
|
||
|
case VIR_NETWORK_FORWARD_NONE:
|
||
|
case VIR_NETWORK_FORWARD_NAT:
|
||
|
case VIR_NETWORK_FORWARD_ROUTE:
|
||
|
+ case VIR_NETWORK_FORWARD_OPEN:
|
||
|
switch (section) {
|
||
|
case VIR_NETWORK_SECTION_FORWARD:
|
||
|
case VIR_NETWORK_SECTION_FORWARD_INTERFACE:
|
||
|
@@ -3325,7 +3326,6 @@ networkUpdate(virNetworkPtr net,
|
||
|
}
|
||
|
break;
|
||
|
|
||
|
- case VIR_NETWORK_FORWARD_OPEN:
|
||
|
case VIR_NETWORK_FORWARD_BRIDGE:
|
||
|
case VIR_NETWORK_FORWARD_PRIVATE:
|
||
|
case VIR_NETWORK_FORWARD_VEPA:
|
||
|
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
|
||
|
index 5981e3bd19..31feec9c9f 100644
|
||
|
--- a/src/network/bridge_driver_linux.c
|
||
|
+++ b/src/network/bridge_driver_linux.c
|
||
|
@@ -337,90 +337,101 @@ networkAddFirewallRules(virNetworkDef *def,
|
||
|
virFirewallBackend firewallBackend,
|
||
|
virFirewall **fwRemoval)
|
||
|
{
|
||
|
+ if (def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
|
||
|
|
||
|
- networkSetupPrivateChains(firewallBackend, false);
|
||
|
+ VIR_DEBUG("No firewall rules to add for mode='open' network '%s'", def->name);
|
||
|
|
||
|
- if (errInitV4 &&
|
||
|
- (virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
|
||
|
- virNetworkDefGetRouteByIndex(def, AF_INET, 0))) {
|
||
|
- virSetError(errInitV4);
|
||
|
- return -1;
|
||
|
- }
|
||
|
+ } else {
|
||
|
|
||
|
- if (errInitV6 &&
|
||
|
- (virNetworkDefGetIPByIndex(def, AF_INET6, 0) ||
|
||
|
- virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
|
||
|
- def->ipv6nogw)) {
|
||
|
- virSetError(errInitV6);
|
||
|
- return -1;
|
||
|
- }
|
||
|
+ VIR_DEBUG("Adding firewall rules for mode='%s' network '%s' using %s",
|
||
|
+ virNetworkForwardTypeToString(def->forward.type),
|
||
|
+ def->name,
|
||
|
+ virFirewallBackendTypeToString(firewallBackend));
|
||
|
|
||
|
- if (def->bridgeZone) {
|
||
|
+ networkSetupPrivateChains(firewallBackend, false);
|
||
|
|
||
|
- /* if a firewalld zone has been specified, fail/log an error
|
||
|
- * if we can't honor it
|
||
|
- */
|
||
|
- if (virFirewallDIsRegistered() < 0) {
|
||
|
- virReportError(VIR_ERR_INTERNAL_ERROR,
|
||
|
- _("zone %1$s requested for network %2$s but firewalld is not active"),
|
||
|
- def->bridgeZone, def->name);
|
||
|
+ if (errInitV4 &&
|
||
|
+ (virNetworkDefGetIPByIndex(def, AF_INET, 0) ||
|
||
|
+ virNetworkDefGetRouteByIndex(def, AF_INET, 0))) {
|
||
|
+ virSetError(errInitV4);
|
||
|
return -1;
|
||
|
}
|
||
|
|
||
|
- if (virFirewallDInterfaceSetZone(def->bridge, def->bridgeZone) < 0)
|
||
|
+ if (errInitV6 &&
|
||
|
+ (virNetworkDefGetIPByIndex(def, AF_INET6, 0) ||
|
||
|
+ virNetworkDefGetRouteByIndex(def, AF_INET6, 0) ||
|
||
|
+ def->ipv6nogw)) {
|
||
|
+ virSetError(errInitV6);
|
||
|
return -1;
|
||
|
+ }
|
||
|
|
||
|
- } else {
|
||
|
+ if (def->bridgeZone) {
|
||
|
|
||
|
- /* if firewalld is active, try to set the "libvirt" zone. This is
|
||
|
- * desirable (for consistency) if firewalld is using the iptables
|
||
|
- * backend, but is necessary (for basic network connectivity) if
|
||
|
- * firewalld is using the nftables backend
|
||
|
- */
|
||
|
- if (virFirewallDIsRegistered() == 0) {
|
||
|
-
|
||
|
- /* if the "libvirt" zone exists, then set it. If not, and
|
||
|
- * if firewalld is using the nftables backend, then we
|
||
|
- * need to log an error because the combination of
|
||
|
- * nftables + default zone means that traffic cannot be
|
||
|
- * forwarded (and even DHCP and DNS from guest to host
|
||
|
- * will probably no be permitted by the default zone
|
||
|
- *
|
||
|
- * Routed networks use a different zone and policy which we also
|
||
|
- * need to verify exist. Probing for the policy guarantees the
|
||
|
- * running firewalld has support for policies (firewalld >= 0.9.0).
|
||
|
+ /* if a firewalld zone has been specified, fail/log an error
|
||
|
+ * if we can't honor it
|
||
|
*/
|
||
|
- if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE &&
|
||
|
- virFirewallDPolicyExists("libvirt-routed-out") &&
|
||
|
- virFirewallDZoneExists("libvirt-routed")) {
|
||
|
- if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
|
||
|
- return -1;
|
||
|
- } else if (virFirewallDZoneExists("libvirt")) {
|
||
|
- if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
|
||
|
- return -1;
|
||
|
- } else {
|
||
|
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||
|
- _("firewalld can't find the 'libvirt' zone that should have been installed with libvirt"));
|
||
|
+ if (virFirewallDIsRegistered() < 0) {
|
||
|
+ virReportError(VIR_ERR_INTERNAL_ERROR,
|
||
|
+ _("zone %1$s requested for network %2$s but firewalld is not active"),
|
||
|
+ def->bridgeZone, def->name);
|
||
|
return -1;
|
||
|
}
|
||
|
+
|
||
|
+ if (virFirewallDInterfaceSetZone(def->bridge, def->bridgeZone) < 0)
|
||
|
+ return -1;
|
||
|
+
|
||
|
+ } else {
|
||
|
+
|
||
|
+ /* if firewalld is active, try to set the "libvirt" zone. This is
|
||
|
+ * desirable (for consistency) if firewalld is using the iptables
|
||
|
+ * backend, but is necessary (for basic network connectivity) if
|
||
|
+ * firewalld is using the nftables backend
|
||
|
+ */
|
||
|
+ if (virFirewallDIsRegistered() == 0) {
|
||
|
+
|
||
|
+ /* if the "libvirt" zone exists, then set it. If not, and
|
||
|
+ * if firewalld is using the nftables backend, then we
|
||
|
+ * need to log an error because the combination of
|
||
|
+ * nftables + default zone means that traffic cannot be
|
||
|
+ * forwarded (and even DHCP and DNS from guest to host
|
||
|
+ * will probably no be permitted by the default zone
|
||
|
+ *
|
||
|
+ * Routed networks use a different zone and policy which we also
|
||
|
+ * need to verify exist. Probing for the policy guarantees the
|
||
|
+ * running firewalld has support for policies (firewalld >= 0.9.0).
|
||
|
+ */
|
||
|
+ if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE &&
|
||
|
+ virFirewallDPolicyExists("libvirt-routed-out") &&
|
||
|
+ virFirewallDZoneExists("libvirt-routed")) {
|
||
|
+ if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0)
|
||
|
+ return -1;
|
||
|
+ } else if (virFirewallDZoneExists("libvirt")) {
|
||
|
+ if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)
|
||
|
+ return -1;
|
||
|
+ } else {
|
||
|
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||
|
+ _("firewalld can't find the 'libvirt' zone that should have been installed with libvirt"));
|
||
|
+ return -1;
|
||
|
+ }
|
||
|
+ }
|
||
|
}
|
||
|
- }
|
||
|
|
||
|
- switch (firewallBackend) {
|
||
|
- case VIR_FIREWALL_BACKEND_NONE:
|
||
|
- virReportError(VIR_ERR_NO_SUPPORT, "%s",
|
||
|
- _("No firewall backend is available"));
|
||
|
- return -1;
|
||
|
+ switch (firewallBackend) {
|
||
|
+ case VIR_FIREWALL_BACKEND_NONE:
|
||
|
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
|
||
|
+ _("No firewall backend is available"));
|
||
|
+ return -1;
|
||
|
|
||
|
- case VIR_FIREWALL_BACKEND_IPTABLES:
|
||
|
- return iptablesAddFirewallRules(def, fwRemoval);
|
||
|
+ case VIR_FIREWALL_BACKEND_IPTABLES:
|
||
|
+ return iptablesAddFirewallRules(def, fwRemoval);
|
||
|
|
||
|
- case VIR_FIREWALL_BACKEND_NFTABLES:
|
||
|
- return nftablesAddFirewallRules(def, fwRemoval);
|
||
|
+ case VIR_FIREWALL_BACKEND_NFTABLES:
|
||
|
+ return nftablesAddFirewallRules(def, fwRemoval);
|
||
|
|
||
|
- case VIR_FIREWALL_BACKEND_LAST:
|
||
|
- virReportEnumRangeError(virFirewallBackend, firewallBackend);
|
||
|
- return -1;
|
||
|
+ case VIR_FIREWALL_BACKEND_LAST:
|
||
|
+ virReportEnumRangeError(virFirewallBackend, firewallBackend);
|
||
|
+ return -1;
|
||
|
+ }
|
||
|
}
|
||
|
return 0;
|
||
|
}
|
||
|
@@ -429,21 +440,29 @@ networkAddFirewallRules(virNetworkDef *def,
|
||
|
void
|
||
|
networkRemoveFirewallRules(virNetworkObj *obj)
|
||
|
{
|
||
|
+ virNetworkDef *def = virNetworkObjGetDef(obj);
|
||
|
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.
|
||
|
+ if (def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
|
||
|
+
|
||
|
+ VIR_DEBUG("No firewall rules to remove for mode='open' network '%s'", def->name);
|
||
|
+
|
||
|
+ } else {
|
||
|
+
|
||
|
+ 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 status of network '%s', assuming old-style iptables", def->name);
|
||
|
+ iptablesRemoveFirewallRules(def);
|
||
|
+ return;
|
||
|
+ }
|
||
|
+
|
||
|
+ /* fwRemoval info was stored in the network status, so use that to
|
||
|
+ * remove the firewall
|
||
|
*/
|
||
|
- VIR_DEBUG("No firewall info in network status, assuming old-style iptables");
|
||
|
- iptablesRemoveFirewallRules(virNetworkObjGetDef(obj));
|
||
|
- return;
|
||
|
+ VIR_DEBUG("Removing firewall rules of network '%s' using commands saved in status", def->name);
|
||
|
+ virFirewallApply(fw);
|
||
|
}
|
||
|
-
|
||
|
- /* 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);
|
||
|
}
|
||
|
--
|
||
|
2.47.0
|