On Wed, Jan 09, 2019 at 09:57:36PM -0500, Laine Stump wrote:
From: Laine Stump <laine(a)redhat.com>
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
by the same kernel hook.
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 local host. 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 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.
2) 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
is rejected by the public 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
that 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).
Putting each network's bridge in a new zone called "libvirt" which has
a default policy of accept will allow the forwarded traffic to pass,
but the same default accept policy that fixes forwarded traffic also
causes *all* traffic from guest to host to be accepted. To solve this
new problem, we 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).
Ok, so we depend on newer firewalld. Any older version using nftables
will still be broken.
I think it would be quite desirable if we queried the firewalld version
and backend and reported a fatal error to the user if they try to
start a virtual network when we know it will be broken.
You can get version from /org/fedoraproject/FirewallD1
org.freedesktop.Properties.Get("org.fedoraproject.FirewallD1",
"version")
And backend from /org/fedoraproject/FirewallD1/config
org.freedesktop.Properties.Get("org.fedoraproject.FirewallD1.config",
"FirewallBackend")
diff --git a/src/network/bridge_driver_linux.c
b/src/network/bridge_driver_linux.c
index dd08222653..a32f19bcf0 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
@@ -638,6 +639,14 @@ int networkAddFirewallRules(virNetworkDefPtr def)
virFirewallPtr fw = NULL;
int ret = -1;
+
+ /* if firewalld is active, try to set the default "libvirt" zone,
+ * but ignore failure, since the version of firewalld on the host
+ * may have failed to load the libvirt zone
+ */
Ah, I was wondering when you checked the version. This is the place
where I think we ought to raise an error
+ if (virFirewallDStatus() >= 0)
+ ignore_value(virFirewallDInterfaceSetZone(def->bridge, "libvirt"));
+
fw = virFirewallNew();
virFirewallStartTransaction(fw, 0);
diff --git a/src/network/libvirt.zone b/src/network/libvirt.zone
new file mode 100644
index 0000000000..1750ba2f06
--- /dev/null
+++ b/src/network/libvirt.zone
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+<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>
Line wrapping :-)
+
+<rule priority='127'>
+ <reject/>
+</rule>
+<service name='dhcp'/>
+<service name='dhcpv6'/>
+<service name='dns'/>
+<service name='ssh'/>
+<service name='tftp'/>
+</zone>
I wonder if we should have an configure time check so that we do
*not* install this file if built agains an old firewalld version.
By unconditionally installing it we're probably causing firewalld
to emit an error message in syslog which some sysadmin is almost
certainly to report a bug about
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 :|