From f89135129d722dca4e5eb7dbcc6845ab757f2e08 Mon Sep 17 00:00:00 2001

Message-Id: <f89135129d722dca4e5eb7dbcc6845ab757f2e08@dist-git>

From: Laine Stump <laine@laine.org>

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

Subject: [PATCH] configure: selectively install a firewalld 'libvirt' zone

MIME-Version: 1.0

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

Content-Transfer-Encoding: 8bit

In the past (when both libvirt and firewalld used iptables), if either

libvirt's rules *OR* firewalld's rules accepted a packet, it would

be accepted. This was because libvirt and firewalld rules were

processed during the same kernel hook, and a single ACCEPT result

would terminate the rule traversal and cause the packet to be

accepted.

But now firewalld can use nftables for its backend, while libvirt's

firewall rules are still using iptables; iptables rules are still

processed, but at a different time during packet processing

(i.e. during a different hook) than the firewalld nftables rules. The

result is that a packet must be accepted by *BOTH* the libvirt

iptables rules *AND* the firewalld nftable rules in order to be

accepted.

This causes pain because

1) libvirt always adds rules to permit DNS and DHCP (and sometimes

TFTP) from guests to the host network's bridge interface. But

libvirt's bridges are in firewalld's "default" zone (which is usually

the zone called "public"). The public zone allows ssh, but doesn't

allow DNS, DHCP, or TFTP. So even though libvirt's rules allow the

DHCP and DNS traffic, the firewalld rules (now processed during a

different hook) dont, thus guests connected to libvirt's bridges can't

acquire an IP address from DHCP, nor can they make DNS queries to the

DNS server libvirt has setup on the host. (This could be solved by

modifying the default firewalld zone to allow DNS and DHCP, but that

would open *all* interfaces in the default zone to those services,

which is most likely not what the host's admin wants.)

2) Even though libvirt adds iptables rules to allow forwarded traffic

to pass the iptables hook, firewalld's higher level "rich rules" don't

yet have the ability to configure the acceptance of forwarded traffic

(traffic that is going somewhere beyond the host), so any traffic that

needs to be forwarded from guests to the network beyond the host is

rejected during the nftables hook by the default zone's "default

reject" policy (which rejects all traffic in the zone not specifically

allowed by the rules in the zone, whether that traffic is destined to

be forwarded or locally received by the host).

libvirt can't send "direct" nftables rules (firewalld only supports

direct/passthrough rules for iptables), so we can't solve this problem

by just sending explicit nftables rules instead of explicit iptables

rules (which, if it could be done, would place libvirt's rules in the

same hook as firewalld's native rules, and thus eliminate the need for

packets to be accepted by both libvirt's and firewalld's own rules).

However, we can take advantage of a quirk in firewalld zones that have

a default policy of "accept" (meaning any packet that doesn't match a

specific rule in the zone will be *accepted*) - this default accept will

also accept forwarded traffic (not just traffic destined for the host).

Of course we don't want to modify firewalld's default zone in that

way, because that would affect the filtering of traffic coming into

the host from other interfaces using that zone. Instead, we will

create a new zone called "libvirt". The libvirt zone will have a

default policy of accept so that forwarded traffic can pass and list

specific services that will be allowed into the host from guests (DNS,

DHCP, SSH, and TFTP).

But the same default accept policy that fixes forwarded traffic also

causes *all* traffic from guest to host to be accepted. To close this

new hole, the libvirt zone can take advantage of a new feature in

firewalld (currently slated for firewalld-0.7.0) - priorities for rich

rules - to add a low priority rule that rejects all local traffic (but

leaves alone all forwarded traffic).

So, our new zone will start with a list of services that are allowed

(dhcp, dns, tftp, and ssh to start, but configurable via any firewalld

management application, or direct editing of the zone file in

/etc/firewalld/zones/libvirt.xml), followed by a low priority

<reject/> rule (to reject all other traffic from guest to host), and

finally with a default policy of accept (to allow forwarded traffic).

This patch only creates the zonefile for the new zone, and implements

a configure.ac option to selectively enable/disable installation of

the new zone. A separate patch contains the necessary code to actually

place bridge interfaces in the libvirt zone.

Why do we need a configure option to disable installation of the new

libvirt zone? It uses a new firewalld attribute that sets the priority

of a rich rule; this feature first appears in firewalld-0.7.0 (unless

it has been backported to am earlier firewalld by a downstream

maintainer). If the file were installed on a system with firewalld

that didn't support rule priorities, firewalld would log an error

every time it restarted, causing confusion and lots of extra bug

reports.

So we add two new configure.ac switches to avoid polluting the system

logs with this error on systems that don't support rule priorities -

"--with-firewalld-zone" and "--without-firewalld-zone". A package

builder can use these to include/exclude the libvirt zone file in the

installation. If firewalld is enabled (--with-firewalld), the default

is --with-firewalld-zone, but it can be disabled during configure

(using --without-firewalld-zone). Targets that are using a firewalld

version too old to support the rule priority setting in the libvirt

zone file can simply add --without-firewalld-zone to their configure

commandline.

These switches only affect whether or not the libvirt zone file is

*installed* in /usr/lib/firewalld/zones, but have no effect on whether

or not libvirt looks for a zone called libvirt and tries to use it.

NB: firewalld zones can only be added to the permanent config of

firewalld, and won't be loaded/enabled until firewalld is restarted,

so at package install/upgrade time we have to restart firewalld. For

rpm-based distros, this is done in the libvirt.spec file by calling

the %firewalld_restart rpm macro, which is a part of the

firewalld-filesystem package. (For distros that don't use rpm

packages, the command "firewalld-cmd --reload" will have the same

effect).

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

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

(cherry picked from commit 3b71f2e42dc6c5453d09136578bfb868874da088)

https://bugzilla.redhat.com/1650320

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

---

 configure.ac                |  3 +++

 libvirt.spec.in             | 31 +++++++++++++++++++++++++

 m4/virt-firewalld-zone.m4   | 45 +++++++++++++++++++++++++++++++++++++

 src/network/Makefile.inc.am | 10 ++++++++-

 src/network/libvirt.zone    | 23 +++++++++++++++++++

 5 files changed, 111 insertions(+), 1 deletion(-)

 create mode 100644 m4/virt-firewalld-zone.m4

 create mode 100644 src/network/libvirt.zone

diff --git a/configure.ac b/configure.ac

index e25bf0a6ec..3da26484d0 100644

--- a/configure.ac

+++ b/configure.ac

@@ -247,6 +247,7 @@ LIBVIRT_ARG_CAPNG

 LIBVIRT_ARG_CURL

 LIBVIRT_ARG_DBUS

 LIBVIRT_ARG_FIREWALLD

+LIBVIRT_ARG_FIREWALLD_ZONE

 LIBVIRT_ARG_FUSE

 LIBVIRT_ARG_GLUSTER

 LIBVIRT_ARG_HAL

@@ -286,6 +287,7 @@ LIBVIRT_CHECK_DBUS

 LIBVIRT_CHECK_DEVMAPPER

 LIBVIRT_CHECK_DLOPEN

 LIBVIRT_CHECK_FIREWALLD

+LIBVIRT_CHECK_FIREWALLD_ZONE

 LIBVIRT_CHECK_FUSE

 LIBVIRT_CHECK_GLUSTER

 LIBVIRT_CHECK_GNUTLS

@@ -959,6 +961,7 @@ LIBVIRT_RESULT_CURL

 LIBVIRT_RESULT_DBUS

 LIBVIRT_RESULT_DLOPEN

 LIBVIRT_RESULT_FIREWALLD

+LIBVIRT_RESULT_FIREWALLD_ZONE

 LIBVIRT_RESULT_FUSE

 LIBVIRT_RESULT_GLUSTER

 LIBVIRT_RESULT_GNUTLS

diff --git a/m4/virt-firewalld-zone.m4 b/m4/virt-firewalld-zone.m4

new file mode 100644

index 0000000000..b67d1a0b2f

--- /dev/null

+++ b/m4/virt-firewalld-zone.m4

@@ -0,0 +1,45 @@

+dnl firewalld_zone check - whether or not to install the firewall "libvirt" zone

+dnl

+dnl Copyright (C) 2019 Red Hat, Inc.

+dnl

+dnl This library is free software; you can redistribute it and/or

+dnl modify it under the terms of the GNU Lesser General Public

+dnl License as published by the Free Software Foundation; either

+dnl version 2.1 of the License, or (at your option) any later version.

+dnl

+dnl This library is distributed in the hope that it will be useful,

+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of

+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

+dnl Lesser General Public License for more details.

+dnl

+dnl You should have received a copy of the GNU Lesser General Public

+dnl License along with this library.  If not, see

+dnl <http://www.gnu.org/licenses/>.

+dnl

+

+AC_DEFUN([LIBVIRT_ARG_FIREWALLD_ZONE], [

+  LIBVIRT_ARG_WITH([FIREWALLD_ZONE], [Whether to install firewalld libvirt zone], [check])

+])

+

+AC_DEFUN([LIBVIRT_CHECK_FIREWALLD_ZONE], [

+  AC_REQUIRE([LIBVIRT_CHECK_FIREWALLD])

+  AC_MSG_CHECKING([for whether to install firewalld libvirt zone])

+

+  if test "x$with_firewalld_zone" = "xcheck" ; then

+    with_firewalld_zone=$with_firewalld

+  fi

+

+  if test "x$with_firewalld_zone" = "xyes" ; then

+    if test "x$with_firewalld" != "xyes" ; then

+      AC_MSG_ERROR([You must have firewalld support enabled to enable firewalld-zone])

+    fi

+    AC_DEFINE_UNQUOTED([WITH_FIREWALLD_ZONE], [1], [whether firewalld libvirt zone is installed])

+  fi

+

+  AM_CONDITIONAL([WITH_FIREWALLD_ZONE], [test "x$with_firewalld_zone" != "xno"])

+  AC_MSG_RESULT($with_firewalld_zone)

+])

+

+AC_DEFUN([LIBVIRT_RESULT_FIREWALLD_ZONE], [

+  LIBVIRT_RESULT([firewalld-zone], [$with_firewalld_zone])

+])

diff --git a/src/network/Makefile.inc.am b/src/network/Makefile.inc.am

index 508c8c0422..cbaaa7ea68 100644

--- a/src/network/Makefile.inc.am

+++ b/src/network/Makefile.inc.am

@@ -87,6 +87,11 @@ install-data-network:

 	( cd $(DESTDIR)$(confdir)/qemu/networks/autostart && \

 	  rm -f default.xml && \

 	  $(LN_S) ../default.xml default.xml )

+if WITH_FIREWALLD_ZONE

+	$(MKDIR_P) "$(DESTDIR)$(prefix)/lib/firewalld/zones"

+	$(INSTALL_DATA) $(srcdir)/network/libvirt.zone \

+	  $(DESTDIR)$(prefix)/lib/firewalld/zones/libvirt.xml

+endif WITH_FIREWALLD_ZONE

 uninstall-data-network:

 	rm -f $(DESTDIR)$(confdir)/qemu/networks/autostart/default.xml

@@ -95,10 +100,13 @@ uninstall-data-network:

 	rmdir "$(DESTDIR)$(confdir)/qemu/networks" || :

 	rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/network" ||:

 	rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/network" ||:

+if WITH_FIREWALLD_ZONE

+	rm -f  $(DESTDIR)$(prefix)/lib/firewalld/zones/libvirt.xml

+endif WITH_FIREWALLD_ZONE

 endif WITH_NETWORK

-EXTRA_DIST += network/default.xml

+EXTRA_DIST += network/default.xml network/libvirt.zone

 .PHONY: \

 	install-data-network \

diff --git a/src/network/libvirt.zone b/src/network/libvirt.zone

new file mode 100644

index 0000000000..bf81db1b6e

--- /dev/null

+++ b/src/network/libvirt.zone

@@ -0,0 +1,23 @@

+

+<zone target="ACCEPT">

+  <short>libvirt</short>

+

+  <description>

+    The default policy of "ACCEPT" allows all packets to/from

+    interfaces in the zone to be forwarded, while the (*low priority*)

+    reject rule blocks any traffic destined for the host, except those

+    services explicitly listed (that list can be modified as required

+    by the local admin). This zone is intended to be used only by

+    libvirt virtual networks - libvirt will add the bridge devices for

+    all new virtual networks to this zone by default.

+  </description>

+

+<rule priority='32767'>

+  <reject/>

+</rule>

+<service name='dhcp'/>

+<service name='dhcpv6'/>

+<service name='dns'/>

+<service name='ssh'/>

+<service name='tftp'/>

+</zone>

-- 

2.20.1