[libvirt] [PATCH 4/5] network: regain guest network connectivity after firewalld switch to nftables

Laine Stump laine at laine.org
Tue Jan 15 17:43:05 UTC 2019


On 1/15/19 11:39 AM, Daniel P. Berrangé wrote:
> On Wed, Jan 09, 2019 at 09:57:36PM -0500, Laine Stump wrote:
>> From: Laine Stump <laine at 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")


Depending on the firewalld version won't give us correct results, since 
the patches to support rule priorities could be / will be backported to 
older firewalld versions on some distros.


Of course we could make failure to set the zone to "libvirt" a fatal 
error - that is really the most reliable proxy we have for the question 
"Does this firewalld support rule priorities?" That's a bit draconian 
though, since it means that "new" libvirt will fail to start networks on 
all distros that haven't yet updated their firewalld version (or 
backported the necessary patches). In particular, if they haven't 
updated firewalld, but they're still using the iptables backend anyway, 
we shouldn't be whining (although for consistency we should still set 
the zone to "libvirt" when possible, even if the backend is iptables,)


>
>> 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


Yep,

>
>> +    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 :-)


Yeah, the examples I saw had everything in a single line, and that made 
me think maybe something in firewalld requires it to be a single line, 
so I avoided breaking it up. I guess I *could* go to the horrible great 
trouble of actually trying it out with line breaks and see if it's 
accepted :-)


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


1) see the comment above about the unreliability of version checks.


2) The version / build of firewalld present at build time is really 
irrelevant. As a matter of fact, we currently don't require firewalld to 
be present *at all* at build time (or at runtime - we just use it if 
it's there and active, otherwise we go directly to iptables/ebtables).


3) I actually *want* the bug reported to distro maintainers, to push 
them to update their firewalld sooner rather than later.


On the other hand, it's much nicer if the error is reported only when 
someone tries to configure things in a manner that won't work, i.e. 
"backend==nftables, but firewalld doesn't support rule priorities".


So here's an idea - how about if we do this:


Once when libvirtd is started:

   retrieve a list of zones and check for presence of the libvirt zone. 
Save this somewhere.

Each time a network is started:

if (libvirt zone doesn't exist) {

     if (firewalldbackend == nftables) {

        Error "Hey you!! Change the firewalld backend to nftables or 
update firewalld!!"

     }

} else {

     put the bridge in the libvirt zone (regardless of nftables/iptables)

}


This way if firewalld is too old to support rich rule priorities, but 
the backend is iptables, the network silently functions as it always has 
(i.e. in the default zone). *AND* as long as firewalld is new enough to 
support rule priorities, behavior will be consistent no matter what the 
backend is set to. But if a distro or user tries to set the nftables 
backend (meaning that networking can't work without rule priorities) and 
firewalld doesn't support rule priorities, *then* it fails to start the 
network and logs an error.


(in the meantime, we haven't relied on version number checks, and a 
system with old firewalld can still build a libvirt that supports new 
firewalld)


Does this sound reasonable?





More information about the libvir-list mailing list