Re: [PATCH 1/4] network: firewalld: convert to policies

On 5/12/22 12:53 PM, Eric Garver wrote: > On Wed, May 11, 2022 at 05:15:25PM +0100, Daniel P. Berrangé wrote: > > On Wed, May 11, 2022 at 11:41:52AM -0400, Eric Garver wrote: > > > Convert the existing behavior into policies. > > > > Has this split of .zone vs .policy been something firewalld > > always supported, or is it a "new" feature for some value > > of "new" ? > > Policies are new in firewalld-0.9.0. > https://firewalld.org/2020/09/policy-objects-introduction > > Policies supplement zones. They do not split or replace them. > > > Essentially wonder if this has any historical back compat > > implications for libvirt, given the platforms we target > > (2 most recent major releases of all distros, so RHEL >= 8 > > and equiv). > > The original zone definition requires firewalld >= 0.7.0. So the > versions we need to worry about with this change are 0.7.z through > 0.8.z. > > At least these distributions (probably non-exhaustive list) have a > firewalld version in that range: > > Ubuntu: > - focal (20.04 LTS) has 0.8.2 > - this is 3 major releases ago, but 2 LTS releases ago Okay, so until we drop support for Ubuntu 20.04 I guess we need to maintain the existing "standalone" libvirt zone (i.e. doesn't use a policy file, and assumes the implied passage of inbound traffic due to the "default accept" behavior of the zone).

We still have a (very outdated and now pointless! note to self: send a patch to remove it!) firewalld version check for a "firewalld < 0.6.0 with nftables backend", so we can easily add a check for firewalld < 0.9.0, use the standalone single libvirt zone in that case, and use separate libvirt-(nat|routed|isolated?) zones when >= 0.9.0. When Ubuntu 20.04 drops from our supported distros, then we can just remove that check along with the standalone "libvirt" zone.

The only quirk here is that anyone who has copied the existing libvirt zone into /etc/firewalld/zones will be surprised when their modified zone is no longer used (as a result of us switching the default zone from "libvirt" to "libvirt-routed" or "libvirt-nat"). If only I'd had the foresight to install separately-named zones from the start :-/ (of course that would have *it's own* set of problems in the case of a zonefile update like this, so I guess I shouldn't beat myself up too much).

Oh, but wait! I just realized - won't an older firewalld complain about a zonefile that references a policy even if no interfaces ever use that zone?

If so, then the packaging for Ubuntu 20.04 may need to only install the old zone file, which would render all the version-check stuff outlined above to be pointless. An associated problem is that the files that create the ubuntu packages are not a part of libvirt's git tree, so making this all work would require updating the out-of-tree build config for a two year old distro (not that they would ever release an official new libvirt version for it, but if we say we support it, that implies that somebody somewhere might want to build an updated libvirt for it). (sigh. Why is nothing ever just simple and straightforward?...) > > -- > > The below distributions should be "good to go": > > RHEL/Fedora: > - RHEL-8 and RHEL-9 have >= 0.9.0. > - f34 and later have >= 0.9.0. > > Debian: > - stable (11, bullseye) has 0.9.2. > - oldstable (10, buster) has 0.6.3 > - defaults to iptables backend [1] so even the original zone is not > necessary > > Ubuntu: > - jammy (22.04 LTS) has 1.1.1 > - impish (21.10) has 0.9.3 repeat sigh. I will never not be annoyed by childish sounding version names :-/ version.numbers++ (/me hangs head in embarrassment at the Fedora "Beefy Miracle" debacle) > > SUSE: > - 15 SP4 has 0.9.3 > - 12 SP5 has 0.4.3.3 (too old to care) > > Note: I didn't investigate rolling release distributions, e.g. Arch, > Gentoo I think by definition those *should* just have the latest version (or close) of every package, shouldn't they?

> > [1]: https://salsa.debian.org/utopia-team/firewalld/-/blob/17fc3126d6eab159f6c... > > > > > > > This commit has no functional changes. > > > > > > Signed-off-by: Eric Garver <eric(a)garver.life&gt; > > > --- > > > src/network/libvirt-nat-out.policy | 12 ++++++++++++ > > > src/network/libvirt-to-host.policy | 20 ++++++++++++++++++++ > > > src/network/libvirt.zone | 23 +++++------------------ > > > src/network/meson.build | 10 ++++++++++ > > > 4 files changed, 47 insertions(+), 18 deletions(-) > > > create mode 100644 src/network/libvirt-nat-out.policy > > > create mode 100644 src/network/libvirt-to-host.policy > > > > > > diff --git a/src/network/libvirt-nat-out.policy b/src/network/libvirt-nat-out.policy > > > new file mode 100644 > > > index 000000000000..7d1cf6dfb4c4 > > > --- /dev/null > > > +++ b/src/network/libvirt-nat-out.policy > > > @@ -0,0 +1,12 @@ > > > +<?xml version="1.0" encoding="utf-8"?> > > > +<policy target="ACCEPT"> > > > + <short>libvirt-nat-out</short> > > > + > > > + <description> > > > + This policy is used to allow NAT virtual machine traffic to the > > > + rest of the network. > > > + </description> > > > + > > > + <ingress-zone name="libvirt" /> > > > + <egress-zone name="ANY" /> > > > +</policy> > > > diff --git a/src/network/libvirt-to-host.policy b/src/network/libvirt-to-host.policy > > > new file mode 100644 > > > index 000000000000..045b35d58d0d > > > --- /dev/null > > > +++ b/src/network/libvirt-to-host.policy > > > @@ -0,0 +1,20 @@ > > > +<?xml version="1.0" encoding="utf-8"?> > > > +<policy target="REJECT"> > > > + <short>libvirt-to-host</short> > > > + > > > + <description> > > > + This policy is used to filter traffic from virtual machines to the > > > + host. > > > + </description> > > > + > > > + <ingress-zone name="libvirt" /> > > > + <egress-zone name="HOST" /> > > > + > > > + <protocol value='icmp'/> > > > + <protocol value='ipv6-icmp'/> > > > + <service name='dhcp'/> > > > + <service name='dhcpv6'/> > > > + <service name='dns'/> > > > + <service name='ssh'/> > > > + <service name='tftp'/> > > > +</policy> > > > diff --git a/src/network/libvirt.zone b/src/network/libvirt.zone > > > index b1e84b52ecc9..4c5639d8a84f 100644 > > > --- a/src/network/libvirt.zone > > > +++ b/src/network/libvirt.zone > > > @@ -1,25 +1,12 @@ > > > <?xml version="1.0" encoding="utf-8"?> > > > -<zone target="ACCEPT"> > > > +<zone> > > > <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. > > > + 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> > > > -<protocol value='icmp'/> > > > -<protocol value='ipv6-icmp'/> > > > -<service name='dhcp'/> > > > -<service name='dhcpv6'/> > > > -<service name='dns'/> > > > -<service name='ssh'/> > > > -<service name='tftp'/> > > > + <forward /> > > > </zone> > > > diff --git a/src/network/meson.build b/src/network/meson.build > > > index b5eff0c3ab6b..3dd342639a46 100644 > > > --- a/src/network/meson.build > > > +++ b/src/network/meson.build > > > @@ -100,5 +100,15 @@ if conf.has('WITH_NETWORK') > > > install_dir: prefix / 'lib' / 'firewalld' / 'zones', > > > rename: [ 'libvirt.xml' ], > > > ) > > > + install_data( > > > + 'libvirt-to-host.policy', > > > + install_dir: prefix / 'lib' / 'firewalld' / 'policies', > > > + rename: [ 'libvirt-to-host.xml' ], > > > + ) > > > + install_data( > > > + 'libvirt-nat-out.policy', > > > + install_dir: prefix / 'lib' / 'firewalld' / 'policies', > > > + rename: [ 'libvirt-nat-out.xml' ], > > > + ) > > > endif > > > endif > > > -- > > > 2.33.0 > > > > > > > With 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 :| > > >