[PATCH 0/4] network: firewalld: fix routed network

This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function. New firewalld policies are added. This is done to use common rules between NAT and routed networks. Policies have been supported since firewalld 0.9.0. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=2055706 [2]: https://github.com/firewalld/firewalld/issues/177 Eric Garver (4): network: firewalld: convert to policies network: firewalld: add zone for routed networks network: firewalld: add policies for routed networks network: firewalld: add support for routed networks src/network/bridge_driver_linux.c | 6 +++++- src/network/libvirt-nat-out.policy | 12 ++++++++++++ src/network/libvirt-routed-in.policy | 11 +++++++++++ src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/libvirt-routed.zone | 12 ++++++++++++ src/network/libvirt-to-host.policy | 21 +++++++++++++++++++++ src/network/libvirt.zone | 23 +++++------------------ src/network/meson.build | 25 +++++++++++++++++++++++++ 8 files changed, 103 insertions(+), 19 deletions(-) create mode 100644 src/network/libvirt-nat-out.policy create mode 100644 src/network/libvirt-routed-in.policy create mode 100644 src/network/libvirt-routed-out.policy create mode 100644 src/network/libvirt-routed.zone create mode 100644 src/network/libvirt-to-host.policy -- 2.33.0

Convert the existing behavior into policies. This commit has no functional changes. Signed-off-by: Eric Garver <eric@garver.life> --- 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

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" ? 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).
This commit has no functional changes.
Signed-off-by: Eric Garver <eric@garver.life> --- 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 :|

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 -- 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 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 [1]: https://salsa.debian.org/utopia-team/firewalld/-/blob/17fc3126d6eab159f6c703...
This commit has no functional changes.
Signed-off-by: Eric Garver <eric@garver.life> --- 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 :|

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/17fc3126d6eab159f6c703...
This commit has no functional changes.
Signed-off-by: Eric Garver <eric@garver.life> --- 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 :|

On Thu, May 12, 2022 at 01:53:00PM -0400, Laine Stump wrote:
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).
Seems like that may be the case.
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.
That may work. I was worried about shipping policy files and the running firewalld not understanding them, but it would simply not load them!
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).
You are right. The user modified config will always be a problem. They won't get future zone/policy updates - those in /etc/firewalld override the defaults in /usr/lib/firewalld. This issue exists today without this series though. The NAT network could continue to use "libvirt" zone as-is. The routed network could use the new policies only if firewalld > 0.9.0 by adding the interface to the "libvirt-routed" zone. It would to fallback to the "libvirt" zone otherwise and routed would still work. After the distributions catch up, then we could transition the "libvirt" zone to policies as I've done here. But that's optional.
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?
Nope. Zones can't reference a policy. Polices reference zones.
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?
Right. I checked Arch and Gentoo. They're both 1.1.1 (the latest release).
[1]: https://salsa.debian.org/utopia-team/firewalld/-/blob/17fc3126d6eab159f6c703...
This commit has no functional changes.
Signed-off-by: Eric Garver <eric@garver.life> --- 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 :|

On Wed, May 11, 2022 at 11:41:52AM -0400, Eric Garver wrote:
Convert the existing behavior into policies.
This commit has no functional changes.
Signed-off-by: Eric Garver <eric@garver.life> --- 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>
The 'libvirt' zone contains the host bridge device (virbr0, IP address 192.168.122.1). The VMs are implicitly in 'libvirt' zone because their TAP devices are members of virbr0. So this policy allows traffic originating in the VMs to forwarded to any other zone. IIUC, it would also allow traffic originating from the host via the virbr0 device to forward to any other zone. In practice the only traffic originating on the host from virbr0, should be traffic destined for the guest. Is it possible to force traffic on host destined for the LAN to originate in virbr0, and forward with NAT, instead of just originating in eth0 and get onto the LAN without forwarding ? If it is possible, do we actually care ?
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>
So this applies to traffic originating in the VM, and destined for the host. I'm fuzzy on exactly what "HOST" expands to in our context. IIUC 'policy' rules apply to traffic transitting between zones, but we shouldn't have any zone transits involved. virbr0 is in the 'libvirt' zones and VM TAP devs are part of virbr0. So traffic VM -> host should be entirely within the libvirt zone, never transitting zones. Does this use of "HOST", for example allow traffic from the VMs to connect to the host via its public IP address on eth0, instead of via its private IP on virbr0 which is what we want ?
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 />
What does '<forward/>' do - i'm not actually finding it mentioned in the zone docs at https://firewalld.org/documentation/man-pages/firewalld.zone.html
</zone>
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 :|

I'm adding this text here in hopes that Mimecast no longer thinks this email is s-p-a-m. My replies are inline below. :) On Thu, May 12, 2022 at 07:35:03PM +0100, Daniel P. Berrangé wrote:
On Wed, May 11, 2022 at 11:41:52AM -0400, Eric Garver wrote:
Convert the existing behavior into policies.
This commit has no functional changes.
Signed-off-by: Eric Garver <eric@garver.life> --- 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>
The 'libvirt' zone contains the host bridge device (virbr0, IP address 192.168.122.1). The VMs are implicitly in 'libvirt' zone because their TAP devices are members of virbr0.
So this policy allows traffic originating in the VMs to forwarded to any other zone.
IIUC, it would also allow traffic originating from the host via the virbr0 device to forward to any other zone. In practice the only traffic originating on the host from virbr0, should be traffic destined for the guest.
No. That's OUTPUT traffic and would require a policy with ingress-zones=HOST. ANY excludes HOST (INPUT, OUTPUT).
Is it possible to force traffic on host destined for the LAN to originate in virbr0, and forward with NAT, instead of just originating in eth0 and get onto the LAN without forwarding ? If it is possible, do we actually care ?
I don't think it's possible. But I'm not certain. The traffic should be OUTPUT.
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>
So this applies to traffic originating in the VM, and destined for the host. I'm fuzzy on exactly what "HOST" expands to in our context.
That's correct. If egress-zones=HOST, then it's equivalent to INPUT. If ingress-zones=HOST, then it's equivalent to OUTPUT.
IIUC 'policy' rules apply to traffic transitting between zones, but we shouldn't have any zone transits involved.
libvirt zone --> public zone (uplink) That should occur for the default config as "public" is the default zone for unassigned traffic.
virbr0 is in the 'libvirt' zones and VM TAP devs are part of virbr0. So traffic VM -> host should be entirely within the libvirt zone, never transitting zones.
Right. The above policy is libvirt --> HOST. Internally the services/ports/etc added to a zone are implemented as a policy identical to above. They're functionally equivalent. The real reason I moved these rules into a policy is so they could be common with the routed network.
Does this use of "HOST", for example allow traffic from the VMs to connect to the host via its public IP address on eth0, instead of via its private IP on virbr0 which is what we want ?
I think so, but a quick look at the iptables rules generated by libvirt look like they also allow it. Chain INPUT (policy ACCEPT 471K packets, 200M bytes) pkts bytes target prot opt in out source destination 472K 200M LIBVIRT_INP all -- any any anywhere anywhere LIBVIRT_INP is non-terminal, so they would be accepted by the INPUT policy (see the counters?).
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 />
What does '<forward/>' do - i'm not actually finding it mentioned in the zone docs at
It's INTRA-zone forwarding. Equivalent to LIBVIRT_FWX. https://firewalld.org/2020/04/intra-zone-forwarding i.e. forward between interfaces in the same zone. iptables equivalent generated by libvirt: Chain LIBVIRT_FWX (1 references) pkts bytes target prot opt in out source destination 0 0 ACCEPT all -- virbr0 virbr0 anywhere anywhere
https://firewalld.org/documentation/man-pages/firewalld.zone.html
</zone>
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 :|

This zone will be used for the routed network by default. Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed.zone | 12 ++++++++++++ src/network/meson.build | 5 +++++ 2 files changed, 17 insertions(+) create mode 100644 src/network/libvirt-routed.zone diff --git a/src/network/libvirt-routed.zone b/src/network/libvirt-routed.zone new file mode 100644 index 000000000000..9cc6cacc2f8a --- /dev/null +++ b/src/network/libvirt-routed.zone @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="utf-8"?> +<zone> + <short>libvirt-routed</short> + + <description> + This zone is intended to be used only by routed libvirt virtual networks - + libvirt will add the bridge devices for all new virtual networks to this + zone by default. + </description> + + <forward /> +</zone> diff --git a/src/network/meson.build b/src/network/meson.build index 3dd342639a46..cd52e2a54c28 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -100,6 +100,11 @@ if conf.has('WITH_NETWORK') install_dir: prefix / 'lib' / 'firewalld' / 'zones', rename: [ 'libvirt.xml' ], ) + install_data( + 'libvirt-routed.zone', + install_dir: prefix / 'lib' / 'firewalld' / 'zones', + rename: [ 'libvirt-routed.xml' ], + ) install_data( 'libvirt-to-host.policy', install_dir: prefix / 'lib' / 'firewalld' / 'policies', -- 2.33.0

Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed-in.policy | 11 +++++++++++ src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/meson.build | 10 ++++++++++ 3 files changed, 33 insertions(+) create mode 100644 src/network/libvirt-routed-in.policy create mode 100644 src/network/libvirt-routed-out.policy diff --git a/src/network/libvirt-routed-in.policy b/src/network/libvirt-routed-in.policy new file mode 100644 index 000000000000..baf8822d747c --- /dev/null +++ b/src/network/libvirt-routed-in.policy @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed traffic to the virtual machines. + </description> + + <ingress-zone name="ANY" /> + <egress-zone name="libvirt-routed" /> +</policy> diff --git a/src/network/libvirt-routed-out.policy b/src/network/libvirt-routed-out.policy new file mode 100644 index 000000000000..efa0030569d6 --- /dev/null +++ b/src/network/libvirt-routed-out.policy @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed virtual machine traffic to the rest of + the network. + </description> + + <ingress-zone name="libvirt-routed" /> + <egress-zone name="ANY" /> +</policy> diff --git a/src/network/meson.build b/src/network/meson.build index cd52e2a54c28..20e411c7eaff 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -115,5 +115,15 @@ if conf.has('WITH_NETWORK') install_dir: prefix / 'lib' / 'firewalld' / 'policies', rename: [ 'libvirt-nat-out.xml' ], ) + install_data( + 'libvirt-routed-out.policy', + install_dir: prefix / 'lib' / 'firewalld' / 'policies', + rename: [ 'libvirt-routed-out.xml' ], + ) + install_data( + 'libvirt-routed-in.policy', + install_dir: prefix / 'lib' / 'firewalld' / 'policies', + rename: [ 'libvirt-routed-in.xml' ], + ) endif endif -- 2.33.0

On Wed, May 11, 2022 at 11:41:54AM -0400, Eric Garver wrote:
Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed-in.policy | 11 +++++++++++ src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/meson.build | 10 ++++++++++ 3 files changed, 33 insertions(+) create mode 100644 src/network/libvirt-routed-in.policy create mode 100644 src/network/libvirt-routed-out.policy
diff --git a/src/network/libvirt-routed-in.policy b/src/network/libvirt-routed-in.policy new file mode 100644 index 000000000000..baf8822d747c --- /dev/null +++ b/src/network/libvirt-routed-in.policy @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed traffic to the virtual machines. + </description> + + <ingress-zone name="ANY" /> + <egress-zone name="libvirt-routed" /> +</policy>
Same as the NAT version of the policy so makes sense.
diff --git a/src/network/libvirt-routed-out.policy b/src/network/libvirt-routed-out.policy new file mode 100644 index 000000000000..efa0030569d6 --- /dev/null +++ b/src/network/libvirt-routed-out.policy @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed virtual machine traffic to the rest of + the network. + </description> + + <ingress-zone name="libvirt-routed" /> + <egress-zone name="ANY" /> +</policy>
This is much more permissive than what I expected. Doesn't this allow the VMs to have unrestricted access to anything on the host ? At a libvirt POV, the NAT and routed zones should be identical, with the only difference being whether masquerading is applied. In terms of VM -> host, we still only want to allow the small set of services, dns, dhcp, ssh AFAIK. 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 :|

On Thu, May 12, 2022 at 07:42:43PM +0100, Daniel P. Berrangé wrote:
On Wed, May 11, 2022 at 11:41:54AM -0400, Eric Garver wrote:
Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed-in.policy | 11 +++++++++++ src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/meson.build | 10 ++++++++++ 3 files changed, 33 insertions(+) create mode 100644 src/network/libvirt-routed-in.policy create mode 100644 src/network/libvirt-routed-out.policy
diff --git a/src/network/libvirt-routed-in.policy b/src/network/libvirt-routed-in.policy new file mode 100644 index 000000000000..baf8822d747c --- /dev/null +++ b/src/network/libvirt-routed-in.policy @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed traffic to the virtual machines. + </description> + + <ingress-zone name="ANY" /> + <egress-zone name="libvirt-routed" /> +</policy>
Same as the NAT version of the policy so makes sense.
diff --git a/src/network/libvirt-routed-out.policy b/src/network/libvirt-routed-out.policy new file mode 100644 index 000000000000..efa0030569d6 --- /dev/null +++ b/src/network/libvirt-routed-out.policy @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed virtual machine traffic to the rest of + the network. + </description> + + <ingress-zone name="libvirt-routed" /> + <egress-zone name="ANY" /> +</policy>
This is much more permissive than what I expected. Doesn't this allow the VMs to have unrestricted access to anything on the host ?
No. ANY means any zone. i.e. FORWARD. There is another symbolic zone, HOST, that is used for INPUT.
At a libvirt POV, the NAT and routed zones should be identical, with the only difference being whether masquerading is applied.
I think the additional difference is that routed allows connections originating from outside (world -> libvirt) to the VMs. There is no "in" policy for NAT for the same reason - they should always be denied. In both NAT and routed, connections originating from VMs allow the return path implicitly via conntrack state.
In terms of VM -> host, we still only want to allow the small set of services, dns, dhcp, ssh AFAIK.
Right, that's covered by the libvirt-to-host policy and is common between the NAT and routed networks.
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 :|

Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/meson.build | 5 +++++ 2 files changed, 17 insertions(+) create mode 100644 src/network/libvirt-routed-out.policy diff --git a/src/network/libvirt-routed-out.policy b/src/network/libvirt-routed-out.policy new file mode 100644 index 000000000000..efa0030569d6 --- /dev/null +++ b/src/network/libvirt-routed-out.policy @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed virtual machine traffic to the rest of + the network. + </description> + + <ingress-zone name="libvirt-routed" /> + <egress-zone name="ANY" /> +</policy> diff --git a/src/network/meson.build b/src/network/meson.build index cd52e2a54c28..36d9b51a2cf9 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -115,5 +115,10 @@ if conf.has('WITH_NETWORK') install_dir: prefix / 'lib' / 'firewalld' / 'policies', rename: [ 'libvirt-nat-out.xml' ], ) + install_data( + 'libvirt-routed-out.policy', + install_dir: prefix / 'lib' / 'firewalld' / 'policies', + rename: [ 'libvirt-routed-out.xml' ], + ) endif endif -- 2.33.0

On Wed, May 11, 2022 at 11:41:55AM -0400, Eric Garver wrote:
Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/meson.build | 5 +++++ 2 files changed, 17 insertions(+) create mode 100644 src/network/libvirt-routed-out.policy
I guess this patch was a mistake, since there's already another PATCH 3 in this series, which appears to be a superset of this one.
diff --git a/src/network/libvirt-routed-out.policy b/src/network/libvirt-routed-out.policy new file mode 100644 index 000000000000..efa0030569d6 --- /dev/null +++ b/src/network/libvirt-routed-out.policy @@ -0,0 +1,12 @@ +<?xml version="1.0" encoding="utf-8"?> +<policy target="ACCEPT"> + <short>libvirt-routed-out</short> + + <description> + This policy is used to allow routed virtual machine traffic to the rest of + the network. + </description> + + <ingress-zone name="libvirt-routed" /> + <egress-zone name="ANY" /> +</policy> diff --git a/src/network/meson.build b/src/network/meson.build index cd52e2a54c28..36d9b51a2cf9 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -115,5 +115,10 @@ if conf.has('WITH_NETWORK') install_dir: prefix / 'lib' / 'firewalld' / 'policies', rename: [ 'libvirt-nat-out.xml' ], ) + install_data( + 'libvirt-routed-out.policy', + install_dir: prefix / 'lib' / 'firewalld' / 'policies', + rename: [ 'libvirt-routed-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 :|

On Thu, May 12, 2022 at 07:37:30PM +0100, Daniel P. Berrangé wrote:
On Wed, May 11, 2022 at 11:41:55AM -0400, Eric Garver wrote:
Signed-off-by: Eric Garver <eric@garver.life> --- src/network/libvirt-routed-out.policy | 12 ++++++++++++ src/network/meson.build | 5 +++++ 2 files changed, 17 insertions(+) create mode 100644 src/network/libvirt-routed-out.policy
I guess this patch was a mistake, since there's already another PATCH 3 in this series, which appears to be a superset of this one.
Yes. Sorry. I must have had a stale 0003 laying around. Please ignore this patch. :)

Signed-off-by: Eric Garver <eric@garver.life> --- src/network/bridge_driver_linux.c | 6 +++++- src/network/libvirt-to-host.policy | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 98d2a33a1da0..2c8e43b427cb 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -859,7 +859,11 @@ int networkAddFirewallRules(virNetworkDef *def) * forwarded (and even DHCP and DNS from guest to host * will probably no be permitted by the default zone */ - if (virFirewallDZoneExists("libvirt")) { + if (def->forward.type == VIR_NETWORK_FORWARD_ROUTE && + virFirewallDZoneExists("libvirt-routed")) { + if (virFirewallDInterfaceSetZone(def->bridge, "libvirt-routed") < 0) + return -1; + } else if (virFirewallDZoneExists("libvirt")) { if (virFirewallDInterfaceSetZone(def->bridge, "libvirt") < 0) return -1; } else { diff --git a/src/network/libvirt-to-host.policy b/src/network/libvirt-to-host.policy index 045b35d58d0d..9ec489dc57b5 100644 --- a/src/network/libvirt-to-host.policy +++ b/src/network/libvirt-to-host.policy @@ -8,6 +8,7 @@ </description> <ingress-zone name="libvirt" /> + <ingress-zone name="libvirt-routed" /> <egress-zone name="HOST" /> <protocol value='icmp'/> -- 2.33.0

On Wed, May 11, 2022 at 11:41:51AM -0400, Eric Garver wrote:
This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function.
New firewalld policies are added. This is done to use common rules between NAT and routed networks. Policies have been supported since firewalld 0.9.0.
For those following along, there's a helpful description of policies here, specifically explaining how its useful to the libvirt scenario: https://firewalld.org/2020/09/policy-objects-introduction 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 :|

On 5/12/22 2:00 PM, Daniel P. Berrangé wrote:
On Wed, May 11, 2022 at 11:41:51AM -0400, Eric Garver wrote:
This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function.
New firewalld policies are added. This is done to use common rules between NAT and routed networks. Policies have been supported since firewalld 0.9.0.
For those following along, there's a helpful description of policies here, specifically explaining how its useful to the libvirt scenario:
...and for some further context that is probably only documented in the discussions that we had with Eric and some other people back in 2018 or so: Once firewalld switches to its native-nftables backend, all of its own rules go into a separate nftables table, while libvirt's rules go into the iptables-compatibility table called "filter". In order for a packet to be accepted and forwarded, it must be accepted by *all* tables. (with iptables, both firewalld and libvirt use the "filter" table, and it is enough for the rules of one or the other to accept a packet). At the time libvirt added support for the firewalld nftables backend, there was no way to explicitly specify "allow forwarded traffic" in a zone, and if the zone was "default REJECT" then all forwarded traffic would be rejected. In order for our traffic to be accepted, we had to make the "libvirt zone" (which is itself a part of *firewalld's* rules, not libvirt's rules!) "default ACCEPT", and then use an at-the-time new feature of firewalld that allowed us to specify higher priority ACCEPT rules for the traffic we wanted accepted, then a lower priority "REJECT ALL" rule (which would reject all traffic on the *INPUT* chain, but not on the FORWARD chain), and then the "default ACCEPT" rule would implicitly add rules that accepted any forwarded traffic. Yes, in restrospect it sounds fragile. And at the time it sounded fragile as well. Unfortunately it was the only way to make things work. In the ensuing years, firewalld has added explicit support for accepting/rejecting traffic on the FORWARD and OUTPUT chains, but as a part of this, that implicit "default ACCEPT" of forwarded traffic has been removed. And *that* is what necessitates Eric's new zone/policy files! Whew!

On Thu, May 12, 2022 at 07:00:09PM +0100, Daniel P. Berrangé wrote:
On Wed, May 11, 2022 at 11:41:51AM -0400, Eric Garver wrote:
This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function.
New firewalld policies are added. This is done to use common rules between NAT and routed networks. Policies have been supported since firewalld 0.9.0.
For those following along, there's a helpful description of policies here, specifically explaining how its useful to the libvirt scenario:
In reviewing these patches I've come to realize I'm still not confident I'm understanding the interaction between traffic we're managing at the firewalld zones/policies. For illustration let me assume the following setup: [ * Remote host on LAN (remote host IP 10.0.0.2) * eth0 public facing ethernet on the LAN (local host IP 10.0.0.5) * virbr0 isolated bridge device (local host IP 192.168.122.1) * vnet0 TAP device for a guest (guest IP 192.168.122.5) Remote host Local host +----------+ LAN +----------+ IP forward +---------------+ | 10.0.0.2 | -------- | 10.0.0.5 | --------------| 192.168.122.1 | | eth0 | | eth0 | w/ NAT | virbr0 | +----------+ +----------+ +---------------+ | | bridge port | +---------------+ | 192.168.122.5 | | host: vnet0 | | guest: eth0 | +---------------+ IIUC zones are * 'libvirt' containing 'virbr0' * 'FedoraWorkstation' containing 'eth0' Is 'vnet0' in a zone or not ? Traffic flows * LAN Remote host (10.0.0.2) -> local host (10.0.0.5) Normal traffic nothing to do with libvirt Rules in <zone> FedoraWorkstation apply * LAN Remote host (10.0.0.2) -> guest (192.168.122.5) IP layer forwarding via eth0 (with conntrack match for NAT zone) ingress=FedoraWorkstation egress=libvirt Rules in <policy> libvirt-host-in apply ? * Local host (192.168.122.1) -> guest (192.168.122.5) Rules in <zone> libvirt apply ? * Local host (10.0.0.5) -> guest (192.168.122.5) NB, shouldn't happen as traffic should have originated from 192.168.122.1 instead. ingress=FedoraWorkstation egress=libvirt Rules in <policy> libvirt-host-in apply ? * Guest (192.168.122.5) -> Local host (192.168.122.1) Rules in <zone> libvirt apply ? Need to allow dhcp, dns, ssh. Feels like this should still be rules in the <zone> ? * Guest (192.168.122.5) -> Local host (10.0.0.5) NB, shouldn't happen as guest generally won't be aware of host's eth0 IP address. ingress=libvirt egress=FedoraWorkstation Rules in <policy> libvirt-nat-out apply ? Should not allow anything special related to virt, as dhcp/dns stuff should only be serviced from virbr0. So the libvirt-nat-out policy feels wrong for this case. * Guest (192.168.122.5) -> LAN remote host (10.0.0.2) ingress=libvirt egress=FedoraWorkstation Rules in <policy> libvirt-nat-out apply ? Need to allow all traffic Is the above right, or any I getting mixed up somewhere ? 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 :|

On Thu, May 12, 2022 at 08:04:28PM +0100, Daniel P. Berrangé wrote:
On Thu, May 12, 2022 at 07:00:09PM +0100, Daniel P. Berrangé wrote:
On Wed, May 11, 2022 at 11:41:51AM -0400, Eric Garver wrote:
This series fixes routed networks when a newer firewalld (>= 1.0.0) is present [1]. Firewalld 1.0.0 included a change that disallows implicit forwarding between zones [2]. libvirt was relying on this behavior to allow routed networks to function.
New firewalld policies are added. This is done to use common rules between NAT and routed networks. Policies have been supported since firewalld 0.9.0.
For those following along, there's a helpful description of policies here, specifically explaining how its useful to the libvirt scenario:
In reviewing these patches I've come to realize I'm still not confident I'm understanding the interaction between traffic we're managing at the firewalld zones/policies.
It's confusing because it's a combination of iptables (libvirt) and firewalld (nftables). And they filter independently. Think of it as having to pass through two firewalls. Hopefully I got it all correct below.
For illustration let me assume the following setup: [ * Remote host on LAN (remote host IP 10.0.0.2)
* eth0 public facing ethernet on the LAN (local host IP 10.0.0.5)
* virbr0 isolated bridge device (local host IP 192.168.122.1)
* vnet0 TAP device for a guest (guest IP 192.168.122.5)
Remote host Local host
+----------+ LAN +----------+ IP forward +---------------+ | 10.0.0.2 | -------- | 10.0.0.5 | --------------| 192.168.122.1 | | eth0 | | eth0 | w/ NAT | virbr0 | +----------+ +----------+ +---------------+ | | bridge port | +---------------+ | 192.168.122.5 | | host: vnet0 | | guest: eth0 | +---------------+
IIUC zones are
* 'libvirt' containing 'virbr0' * 'FedoraWorkstation' containing 'eth0'
Is 'vnet0' in a zone or not ?
No. Only the bridge interface is added to the zone. The vnet* interfaces don't have addresses.
Traffic flows
* LAN Remote host (10.0.0.2) -> local host (10.0.0.5)
Normal traffic nothing to do with libvirt
Rules in <zone> FedoraWorkstation apply
True.
* LAN Remote host (10.0.0.2) -> guest (192.168.122.5)
IP layer forwarding via eth0 (with conntrack match for NAT zone)
ingress=FedoraWorkstation egress=libvirt
Rules in <policy> libvirt-host-in apply ?
False. There are no explicit firewalld rules for this. Existing connections would be implicitly allowed by a top-level "ct state" match in FORWARD.
* Local host (192.168.122.1) -> guest (192.168.122.5)
Rules in <zone> libvirt apply ?
False. No rules explicit rules apply. Firewalld allows outbound by default.
* Local host (10.0.0.5) -> guest (192.168.122.5)
NB, shouldn't happen as traffic should have originated from 192.168.122.1 instead.
ingress=FedoraWorkstation egress=libvirt
Rules in <policy> libvirt-host-in apply ?
False. There are no explicit firewalld rules for this. New connections would be denied. Existing (originating from VM) would be allowed.
* Guest (192.168.122.5) -> Local host (192.168.122.1)
Rules in <zone> libvirt apply ?
Need to allow dhcp, dns, ssh. Feels like this should still be rules in the <zone> ?
True. This is handled by the current zone definition. This series moves them into libvirt-to-host. You used the name libvirt-host-in, which may be a better name for the policy. :)
* Guest (192.168.122.5) -> Local host (10.0.0.5)
NB, shouldn't happen as guest generally won't be aware of host's eth0 IP address.
ingress=libvirt egress=FedoraWorkstation
Rules in <policy> libvirt-nat-out apply ?
Should not allow anything special related to virt, as dhcp/dns stuff should only be serviced from virbr0. So the libvirt-nat-out policy feels wrong for this case.
False. I think this is still considered INPUT traffic since it's going to the local network stack. So the "libvirt" zone and libvirt-to-host would apply. Would be ingress=libvirt egress=HOST
* Guest (192.168.122.5) -> LAN remote host (10.0.0.2)
ingress=libvirt egress=FedoraWorkstation
Rules in <policy> libvirt-nat-out apply ?
Need to allow all traffic
True.
Is the above right, or any I getting mixed up somewhere ?
Answered all inline.
participants (3)
-
Daniel P. Berrangé
-
Eric Garver
-
Laine Stump