On 6/14/24 16:43, Daniel P. Berrangé wrote: > There are two scenarios identified after the recent firewall backend > selection was introduced, which result in libvirtd failing to startup > due to an inability to find either iptables/nftables > > - On Linux if running unprivileged with $PATH lacking the dir > containing iptables/nftables > - On non-Linux where iptables/nftables never existed > > In the former case, it is preferrable to restore the behaviour whereby > the driver starts successfully. Users will get an error reported when > attempting to start any virtual network, due to the lack of permissions > needed to create bridge devices. This makes the missing firewall backend > irrelevant. > > In the latter case, the network driver calls the 'nop' platform > implementation which does not attempt to implement any firewall logic, > just allowing the network to start without firewall rules. > > To solve this are number of changes are required > > * Introduce VIR_FIREWALL_BACKEND_NONE, which does nothing except > report a fatal error from virFirewallApply(). This code path > is unreachable, since we'll never create a virFirewall > object with with VIR_FIREWALL_BACKEND_NONE, so the error reporting > is just a sanity check. > > * Ignore the compile time backend defaults and assume use of > the 'none' backend if running unprivileged. > > This fixes the first regression, avoiding the failure to start > libvirtd on Linux in unprivileged context, instead allowing use > of the driver and expecting a permission denied when creating a > bridge. > > * Reject the use of compile time backend defaults no non-Linux > and hardcode the 'none' backend. The non-Linux platforms have > no firewall implementation at all currently, so there's no > reason to permit the use of 'firewall_backend_priority' > meson option. > > This fixes the second regression, avoiding the failure to start > libvirtd on non-Linux hosts due to non-existant Linux binaries. > > * Change the Linux platform backend to raise an error if the > firewall backend is 'none'. Again this code path is unreachable > by default since we'll fail to create the bridge before getting > here, but if someone modified network.conf to request the 'none' > backend, this will stop further progress. > > * Change the nop platform backend to raise an error if the > firewall backend is 'iptables' or 'nftables'. Again this code > path is unreachable, since we should already have failed to > find the iptables/nftables binaries on non-Linux hosts, so > this is just a sanity check. > > * 'none' is not permited as a value in 'firewall_backend_priority' > meson option, since it is conceptually meaningless to ask for > that on Linux. > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > --- > > Changed in v2: > > - Fix build problems on FreeBSD (changes proposed by Roman) > > meson.build | 26 +++++++++++++++++++------- > meson_options.txt | 2 +- > src/network/bridge_driver_conf.c | 19 ++++++++++++++----- > src/network/bridge_driver_linux.c | 10 ++++++++++ > src/network/bridge_driver_nop.c | 15 ++++++++++++++- > src/util/virfirewall.c | 6 ++++++ > src/util/virfirewall.h | 1 + > 7 files changed, 65 insertions(+), 14 deletions(-) Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> Michal
