[libvirt] [PATCH v2 5/7] network: set firewalld zone of bridges to "libvirt" zone when appropriate
Daniel P. Berrangé
berrange at redhat.com
Fri Feb 1 13:24:52 UTC 2019
On Thu, Jan 31, 2019 at 08:24:56PM -0500, Laine Stump wrote:
> From: Laine Stump <laine at redhat.com>
>
> 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).
>
> Resolves: https://bugzilla.redhat.com/1638342
> Creates-and-Resolves: https://bugzilla.redhat.com/1650320
> Signed-off-by: Laine Stump <laine at laine.org>
> ---
>
> NB: I had considered that it might be useful to cache the results of
> checking the list of active zones, the firewalld version, or the
> firewalld backend, but in my tests of restarting libvirtd with 100
> active networks, the full startup time (from the beginning of
> "systemctl restart libvirtd.service" until successful return from a
> subsequent "virsh list") showed 42 seconds and a bit regardless of
> whether or not I made those checks. This tells me that the amount of
> time to be saved by caching the results of a single call vs calling
> once for each network are insignificant relative to everything else
> that is being done. Because any cached values would need to be stored
> in the network driver state object, and thus require acquiring the
> driver-wide lock to update at potentially very different times
> (e.g. in the response to a dbus message informing us that firewalld
> was restarted, as well as while starting a new network from an API
> call) I consider the chance of a bug in my code causing an obscure
> deadlock sometime in the future to be a much greater concern than
> maybe saving 1/10th of a second out of 42 (and lock contention might
> eliminate the gain anyway(), so I have left the code to retrieve the
> list of zones once for each network start.
>
> Changes in V2:
>
> * split off from Patch 5. This patch only sets the libvirt zone if it
> exists (and attempts to somewhat document the behavior in
> firewall.html), it doesn't install the libvirt zone.
>
> * check for existence of libvirt zone before attempting to set it.
>
> * if libvirt zone doesn't exist, only log an error in the case that
> the firewalld version is new enough to have an nftables backend, and
> the backend is actually set to nftables.
>
>
> 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</pre>
> </li>
> </ul>
>
> + <h3><a id="fw-firewalld-and-virtual-network-driver">firewalld and the virtual network driver</a>
> + </h3>
> + <p>
> + If <a href="https://firewalld.org">firewalld</a> 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.
> + </p>
> + <p>
> + 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).
> + </p>
> +
> <h3><a id="fw-network-filter-driver">The network filter driver</a>
> </h3>
> <p>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 e5e48c90f1..9d2e6877ae 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -27,6 +27,7 @@
> #include "virstring.h"
> #include "virlog.h"
> #include "virfirewall.h"
> +#include "virfirewalld.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -670,6 +671,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) {
The indentation here is odd - its 4 spaces too much for the
surrounding context.
With that fixed
Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list