From 8cc240a1652a465727d5b66d9fb6a5fa71656dba Mon Sep 17 00:00:00 2001

Message-Id: <8cc240a1652a465727d5b66d9fb6a5fa71656dba@dist-git>

From: Laine Stump <laine@redhat.com>

Date: Fri, 1 Feb 2019 20:29:31 -0500

Subject: [PATCH] network: set firewalld zone of bridges to "libvirt" zone when

 appropriate

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

This patch restores broken guest network connectivity after a host

firewalld is switched to using an nftables backend. It does this by

adding libvirt networks' bridge interfaces to the new "libvirt" zone

in firewalld.

After this patch, the bridge interface of any network created by

libvirt (when firewalld is active) will be added to the firewalld

zone called "libvirt" if it exists (regardless of the firewalld

backend setting). This behavior does *not* depend on whether or not

libvirt has installed the libvirt zone file (set with

"--with[out]-firewalld-zone" during the configure phase of the package

build).

If the libvirt zone doesn't exist (either because the package was

configured to not install it, or possibly it was installed, but

firewalld doesn't support rule priorities, resulting in a parse

error), the bridge will remain in firewalld's default zone, which

could be innocuous (in the case that the firewalld backend is

iptables, guest networking will still function properly with the

bridge in the default zone), or it could be disastrous (if the

firewalld backend is nftables, we can be assured that guest networking

will fail). In order to be unobtrusive in the former case, and

informative in the latter, when the libvirt zone doesn't exist we

then check the firewalld version to see if it's new enough to support

the nftables backend, and then if the backend is actually set to

nftables, before logging an error (and failing the net-start

operation, since the network couldn't possibly work anyway).

When the libvirt zone is used, network behavior is *slightly*

different from behavior of previous libvirt. In the past, libvirt

network behavior would be affected by the configuration of firewalld's

default zone (usually "public"), but now it is affected only by the

"libvirt" zone), and thus almost surely warrants a release note for

any distro upgrading to libvirt 5.1 or above. Although it's

unfortunate that we have to deal with a mandatory behavior change, the

architecture of multiple hooks makes it impossible to *not* change

behavior in some way, and the new behavior is arguably better (since

it will now be possible to manage access to the host from virtual

machines vs from public interfaces separately).

Creates-and-Resolves: https://bugzilla.redhat.com/1650320

Resolves: https://bugzilla.redhat.com/1638342

Signed-off-by: Laine Stump <laine@laine.org>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

(cherry picked from commit ae05211a360077f56883cd0a6c0f82ed57f746cb)

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 docs/firewall.html.in             | 33 +++++++++++++++++++++

 src/network/bridge_driver_linux.c | 48 +++++++++++++++++++++++++++++++

 2 files changed, 81 insertions(+)

diff --git a/docs/firewall.html.in b/docs/firewall.html.in

index 0a50687c26..5d584e582e 100644

--- a/docs/firewall.html.in

+++ b/docs/firewall.html.in

@@ -129,6 +129,39 @@ MASQUERADE all  --  *      *       192.168.122.0/24    !192.168.122.0/24

+    
firewalld and the virtual network driver

+    

+    

+      If firewalld is active on

+      the host, libvirt will attempt to place the bridge interface of

+      a libvirt virtual network into the firewalld zone named

+      "libvirt" (thus making all guest->host traffic on that network

+      subject to the rules of the "libvirt" zone). This is done

+      because, if firewalld is using its nftables backend (available

+      since firewalld 0.6.0) the default firewalld zone (which would

+      be used if libvirt didn't explicitly set the zone) prevents

+      forwarding traffic from guests through the bridge, as well as

+      preventing DHCP, DNS, and most other traffic from guests to

+      host. The zone named "libvirt" is installed into the firewalld

+      configuration by libvirt (not by firewalld), and allows

+      forwarded traffic through the bridge as well as DHCP, DNS, TFTP,

+      and SSH traffic to the host - depending on firewalld's backend

+      this will be implemented via either iptables or nftables

+      rules. libvirt's own rules outlined above will *always* be

+      iptables rules regardless of which backend is in use by

+      firewalld.

+    

+    

+      NB: Prior to libvirt 5.1.0, the firewalld "libvirt" zone did not

+      exist, and prior to firewalld 0.7.0 a feature crucial to making

+      the "libvirt" zone operate properly (rich rule priority

+      settings) was not implemented in firewalld. In cases where one

+      or the other of the two packages is missing the necessary

+      functionality, it's still possible to have functional guest

+      networking by setting the firewalld backend to "iptables" (in

+      firewalld prior to 0.6.0, this was the only backend available).

+    

+

     
The network filter driver

     
This driver provides a fully configurable network filtering capability

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c

index 3effcdce22..823d5a9742 100644

--- a/src/network/bridge_driver_linux.c

+++ b/src/network/bridge_driver_linux.c

@@ -29,6 +29,7 @@

 #include "virstring.h"

 #include "virlog.h"

 #include "virfirewall.h"

+#include "virfirewalld.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

@@ -641,6 +642,53 @@ int networkAddFirewallRules(virNetworkDefPtr def)

     virFirewallPtr fw = NULL;

     int ret = -1;

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

+         */

+        if (virFirewallDZoneExists("libvirt")) {

+            if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0)

+                goto cleanup;

+        } else {

+            unsigned long version;

+            int vresult = virFirewallDGetVersion(&version);

+

+            if (vresult < 0)

+                goto cleanup;

+

+            /* Support for nftables backend was added in firewalld

+             * 0.6.0. Support for rule priorities (required by the

+             * 'libvirt' zone, which should be installed by a

+             * libvirt package, *not* by firewalld) was not added

+             * until firewalld 0.7.0 (unless it was backported).

+             */

+            if (version >= 6000 &&

+                virFirewallDGetBackend() == VIR_FIREWALLD_BACKEND_NFTABLES) {

+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                               _("firewalld is set to use the nftables "

+                                 "backend, but the required firewalld "

+                                 "'libvirt' zone is missing. Either set "

+                                 "the firewalld backend to 'iptables', or "

+                                 "ensure that firewalld has a 'libvirt' "

+                                 "zone by upgrading firewalld to a "

+                                 "version supporting rule priorities "

+                                 "(0.7.0+) and/or rebuilding "

+                                 "libvirt with --with-firewalld-zone"));

+                goto cleanup;

+            }

+        }

+    }

+

     fw = virFirewallNew();

     virFirewallStartTransaction(fw, 0);

-- 

2.20.1