network: support setting firewallBackend from network.conf

It still can have only one useful value ("iptables"), but once a 2nd
value is supported, it will be selectable by setting
"firewall_backend=nftables" in /etc/libvirt/network.conf.

If firewall_backend isn't set in network.conf, then libvirt will check
to see if FIREWALL_BACKEND_DEFAULT_1 is available and, if so, set
that. (Since FIREWALL_BACKEND_DEFAULT_1 is currently "iptables", this
means checking to see it the iptables binary is present on the
system).  If the default backend isn't available, that is considered a
fatal error (since no networks can be started anyway), so an error is
logged and startup of the network driver fails.

NB: network.conf is itself created from network.conf.in at build time,
and the advertised default setting of firewall_backend (in a commented
out line) is set from the meson_options.txt setting
"firewall_backend_default_1". This way the conf file will have correct
information no matter what ordering is chosen for default backend at
build time (as more backends are added, settings will be added for
"firewall_backend_default_n", and those will be settable in
meson_options.txt and on the meson commandline to change the ordering
of the auto-detection when no backend is set in network.conf).

virNetworkLoadDriverConfig() may look more complicated than necessary,
but as additional backends are added, it will be easier to add checks
for those backends (and to re-order the checks based on builders'
preferences).

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/meson.build b/meson.build
index 063233e05eead222faa8198d2768abccb7817039..af3719a73444d8319ad7dc7fad3dafd8105f45ba 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -1638,6 +1638,10 @@ endif
 
 if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
   conf.set('WITH_NETWORK', 1)
+  firewall_backend_default_1 = get_option('firewall_backend_default_1')
+  firewall_backend_default_conf = firewall_backend_default_1
+  firewall_backend_default_1 = 'VIR_FIREWALL_BACKEND_' + firewall_backend_default_1.to_upper()
+  conf.set('FIREWALL_BACKEND_DEFAULT_1', firewall_backend_default_1)
 elif get_option('driver_network').enabled()
   error('libvirtd must be enabled to build the network driver')
 endif

diff --git a/meson_options.txt b/meson_options.txt
index 6258e50c9199a1938987fbe3f8a7af909d9e5a4a..41342793bc325f5b5f800ff84aaf30894558150f 100644 (file)
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -115,6 +115,7 @@ option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for st
 option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support')
 # dep:firewalld
 option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone')
+option('firewall_backend_default_1', type: 'string', value: 'iptables', description: 'first firewall backend to try when none is specified')
 option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate')
 option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install')
 option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.')

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d7c1ef172dde693388581076c20b367b1cf89fe6..d14853cfd00483b285ce67ce3a0387f39837d234 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1682,6 +1682,7 @@ static int
 networkReloadFirewallRulesHelper(virNetworkObj *obj,
                                  void *opaque G_GNUC_UNUSED)
 {
+    g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(networkGetDriver());
     VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
     virNetworkDef *def = virNetworkObjGetDef(obj);
 
@@ -1695,8 +1696,8 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj,
              * network type, forward='open', doesn't need this because it
              * has no iptables rules.
              */
-            networkRemoveFirewallRules(def);
-            ignore_value(networkAddFirewallRules(def));
+            networkRemoveFirewallRules(def, cfg->firewallBackend);
+            ignore_value(networkAddFirewallRules(def, cfg->firewallBackend));
             break;
 
         case VIR_NETWORK_FORWARD_OPEN:
@@ -1948,7 +1949,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
 
     /* Add "once per network" rules */
     if (def->forward.type != VIR_NETWORK_FORWARD_OPEN &&
-        networkAddFirewallRules(def) < 0)
+        networkAddFirewallRules(def, cfg->firewallBackend) < 0)
         goto error;
 
     firewalRulesAdded = true;
@@ -2066,7 +2067,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
 
     if (firewalRulesAdded &&
         def->forward.type != VIR_NETWORK_FORWARD_OPEN)
-        networkRemoveFirewallRules(def);
+        networkRemoveFirewallRules(def, cfg->firewallBackend);
 
     virNetworkObjUnrefMacMap(obj);
 
@@ -2078,7 +2079,8 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
 
 
 static int
-networkShutdownNetworkVirtual(virNetworkObj *obj)
+networkShutdownNetworkVirtual(virNetworkObj *obj,
+                              virNetworkDriverConfig *cfg)
 {
     virNetworkDef *def = virNetworkObjGetDef(obj);
     pid_t dnsmasqPid;
@@ -2104,7 +2106,7 @@ networkShutdownNetworkVirtual(virNetworkObj *obj)
     ignore_value(virNetDevSetOnline(def->bridge, false));
 
     if (def->forward.type != VIR_NETWORK_FORWARD_OPEN)
-        networkRemoveFirewallRules(def);
+        networkRemoveFirewallRules(def, cfg->firewallBackend);
 
     ignore_value(virNetDevBridgeDelete(def->bridge));
 
@@ -2408,7 +2410,7 @@ networkShutdownNetwork(virNetworkDriverState *driver,
     case VIR_NETWORK_FORWARD_NAT:
     case VIR_NETWORK_FORWARD_ROUTE:
     case VIR_NETWORK_FORWARD_OPEN:
-        ret = networkShutdownNetworkVirtual(obj);
+        ret = networkShutdownNetworkVirtual(obj, cfg);
         break;
 
     case VIR_NETWORK_FORWARD_BRIDGE:
@@ -3259,7 +3261,7 @@ networkUpdate(virNetworkPtr net,
                  * old rules (and remember to load new ones after the
                  * update).
                  */
-                networkRemoveFirewallRules(def);
+                networkRemoveFirewallRules(def, cfg->firewallBackend);
                 needFirewallRefresh = true;
                 break;
             default:
@@ -3287,14 +3289,14 @@ networkUpdate(virNetworkPtr net,
                             parentIndex, xml,
                             network_driver->xmlopt, flags) < 0) {
         if (needFirewallRefresh)
-            ignore_value(networkAddFirewallRules(def));
+            ignore_value(networkAddFirewallRules(def, cfg->firewallBackend));
         goto cleanup;
     }
 
     /* @def is replaced */
     def = virNetworkObjGetDef(obj);
 
-    if (needFirewallRefresh && networkAddFirewallRules(def) < 0)
+    if (needFirewallRefresh && networkAddFirewallRules(def, cfg->firewallBackend) < 0)
         goto cleanup;
 
     if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) {

diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c
index a2edafa837fcaa9e517925351d54d87871fef298..f63a9780960f32312add90a7f75fb6773131e806 100644 (file)
--- a/src/network/bridge_driver_conf.c
+++ b/src/network/bridge_driver_conf.c
@@ -25,6 +25,7 @@
 #include "datatypes.h"
 #include "virlog.h"
 #include "virerror.h"
+#include "virfile.h"
 #include "virutil.h"
 #include "bridge_driver_conf.h"
 
@@ -32,7 +33,6 @@
 
 VIR_LOG_INIT("network.bridge_driver");
 
-
 static virClass *virNetworkDriverConfigClass;
 static void virNetworkDriverConfigDispose(void *obj);
 
@@ -62,18 +62,76 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
                            const char *filename)
 {
     g_autoptr(virConf) conf = NULL;
+    g_autofree char *fwBackendStr = NULL;
+    bool fwBackendSelected = false;
+    size_t i;
+    int fwBackends[] = { FIREWALL_BACKEND_DEFAULT_1 };
+    G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST);
+    int nFwBackends = G_N_ELEMENTS(fwBackends);
+
+    if (access(filename, R_OK) == 0) {
+
+        conf = virConfReadFile(filename, 0);
+        if (!conf)
+            return -1;
+
+        /* use virConfGetValue*(conf, ...) functions to read any settings into cfg */
+
+        if (virConfGetValueString(conf, "firewall_backend", &fwBackendStr) < 0)
+            return -1;
+
+        if (fwBackendStr) {
+            fwBackends[0] = virFirewallBackendTypeFromString(fwBackendStr);
+            nFwBackends = 1;
+
+            if (fwBackends[0] < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("unrecognized 'firewall_backend = '%1$s' set in network driver config file %2$s"),
+                               fwBackendStr, filename);
+                return -1;
+            }
+            VIR_INFO("firewall_backend setting requested from config file %s: '%s'",
+                     virFirewallBackendTypeToString(fwBackends[0]), filename);
+        }
+    }
+
+    for (i = 0; i < nFwBackends && !fwBackendSelected; i++) {
+
+        switch ((virFirewallBackend)fwBackends[i]) {
+        case VIR_FIREWALL_BACKEND_IPTABLES: {
+            g_autofree char *iptablesInPath = virFindFileInPath(IPTABLES);
+
+            if (iptablesInPath)
+                fwBackendSelected = true;
+            break;
+        }
+        case VIR_FIREWALL_BACKEND_LAST:
+            virReportEnumRangeError(virFirewallBackend, fwBackends[i]);
+            return -1;
+        }
+
+        if (fwBackendSelected)
+            cfg->firewallBackend = fwBackends[i];
+    }
 
-    /* if file doesn't exist or is unreadable, ignore the "error" */
-    if (access(filename, R_OK) == -1)
+    if (fwBackendSelected) {
+        VIR_INFO("using firewall_backend: '%s'",
+                 virFirewallBackendTypeToString(cfg->firewallBackend));
         return 0;
 
-    conf = virConfReadFile(filename, 0);
-    if (!conf)
-        return -1;
+    } else if (fwBackendStr) {
 
-    /* use virConfGetValue*(conf, ...) functions to read any settings into cfg */
+        /* the explicitly requested backend wasn't found - this is a failure */
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("requested firewall_backend '%1$s' is not available"),
+                       fwBackendStr);
+        return -1;
 
-    return 0;
+    } else {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("could not find a usable firewall backend"));
+        return -1;
+    }
 }
 
 

diff --git a/src/network/bridge_driver_conf.h b/src/network/bridge_driver_conf.h
index 426c16198d8a546feef689f847484140508fd647..8f221f391eb73346156d9ec09b1e7f535b7010f1 100644 (file)
--- a/src/network/bridge_driver_conf.h
+++ b/src/network/bridge_driver_conf.h
@@ -26,6 +26,7 @@
 #include "virdnsmasq.h"
 #include "virnetworkobj.h"
 #include "object_event.h"
+#include "virfirewall.h"
 
 typedef struct _virNetworkDriverConfig virNetworkDriverConfig;
 struct _virNetworkDriverConfig {
@@ -37,6 +38,8 @@ struct _virNetworkDriverConfig {
     char *stateDir;
     char *pidDir;
     char *dnsmasqStateDir;
+
+    virFirewallBackend firewallBackend;
 };
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetworkDriverConfig, virObjectUnref);

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 4914d5c903f168fe757df92a6ef97120dd57ccb5..c2ef27f25128999f79e578cfc70d3b27318a0291 100644 (file)
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -303,7 +303,8 @@ int networkCheckRouteCollision(virNetworkDef *def)
 
 
 int
-networkAddFirewallRules(virNetworkDef *def)
+networkAddFirewallRules(virNetworkDef *def,
+                        virFirewallBackend firewallBackend G_GNUC_UNUSED)
 {
     if (virOnce(&createdOnce, networkSetupPrivateChains) < 0)
         return -1;
@@ -394,7 +395,8 @@ networkAddFirewallRules(virNetworkDef *def)
 
 
 void
-networkRemoveFirewallRules(virNetworkDef *def)
+networkRemoveFirewallRules(virNetworkDef *def,
+                           virFirewallBackend firewallBackend G_GNUC_UNUSED)
 {
     iptablesRemoveFirewallRules(def);
 }

diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index 6eee6043e6820df18c813b1d4533c15b86f5ca84..7d9a061e50757854c70cf05ca1b0a3554bf55690 100644 (file)
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -36,11 +36,13 @@ int networkCheckRouteCollision(virNetworkDef *def G_GNUC_UNUSED)
     return 0;
 }
 
-int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED)
+int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED,
+                            virFirewallBackend firewallBackend G_GNUC_UNUSED)
 {
     return 0;
 }
 
-void networkRemoveFirewallRules(virNetworkDef *def G_GNUC_UNUSED)
+void networkRemoveFirewallRules(virNetworkDef *def G_GNUC_UNUSED,
+                               virFirewallBackend firewallBackend G_GNUC_UNUSED)
 {
 }

diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index b720d343beb88cca1d9e56ef2ad2f6994b071afb..7443c3129ff0e7c3d60282e1314ce543fadd496c 100644 (file)
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -32,6 +32,8 @@ void networkPostReloadFirewallRules(bool startup);
 
 int networkCheckRouteCollision(virNetworkDef *def);
 
-int networkAddFirewallRules(virNetworkDef *def);
+int networkAddFirewallRules(virNetworkDef *def,
+                            virFirewallBackend firewallBackend);
 
-void networkRemoveFirewallRules(virNetworkDef *def);
+void networkRemoveFirewallRules(virNetworkDef *def,
+                                virFirewallBackend firewallBackend);

diff --git a/src/network/libvirtd_network.aug b/src/network/libvirtd_network.aug
index ae153d96a1d58a7e321a205f6c2e7be9f6ad0226..5d6d72dd929ac277da735d53143a09433312e98a 100644 (file)
--- a/src/network/libvirtd_network.aug
+++ b/src/network/libvirtd_network.aug
@@ -22,11 +22,14 @@ module Libvirtd_network =
    let int_entry       (kw:string) = [ key kw . value_sep . int_val ]
    let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
 
+   let firewall_backend_entry = str_entry "firewall_backend"
+
    (* Each entry in the config is one of the following *)
+   let entry = firewall_backend_entry
    let comment = [ label "#comment" . del /#[ \t]*/ "# " .  store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ]
    let empty = [ label "#empty" . eol ]
 
-   let record = indent . eol
+   let record = indent . entry . eol
 
    let lns = ( record | comment | empty ) *
 

diff --git a/src/network/meson.build b/src/network/meson.build
index 03364358627687b6df2adda04e67d32ea0cb0185..c1934d2e689d5131189a2c33ab3980d5c46902a3 100644 (file)
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -49,16 +49,34 @@ if conf.has('WITH_NETWORK')
     ],
   }
 
+  network_options_conf = configuration_data({
+    'FIREWALL_BACKEND': firewall_backend_default_conf,
+  })
+
   network_conf = configure_file(
     input: 'network.conf.in',
     output: 'network.conf',
-    configuration: configmake_conf,
+    configuration: network_options_conf,
   )
+
+  network_options_hack_conf = configuration_data({
+    'FIREWALL_BACKEND': firewall_backend_default_conf,
+    # This hack is necessary because the output file is going to be
+    # used as input for another configure_file() call later, which
+    # will take care of substituting @CONFIG@ with useful data
+    'CONFIG': '@CONFIG@',
+  })
+  test_libvirtd_network_aug_tmp = configure_file(
+    input: 'test_libvirtd_network.aug.in',
+    output: 'test_libvirtd_network.aug.tmp',
+    configuration: network_options_hack_conf,
+  )
+
   virt_conf_files += network_conf
   virt_aug_files += files('libvirtd_network.aug')
   virt_test_aug_files += {
     'name': 'test_libvirtd_network.aug',
-    'aug': files('test_libvirtd_network.aug.in'),
+    'aug': test_libvirtd_network_aug_tmp,
     'conf': network_conf,
     'test_name': 'libvirtd_network',
     'test_srcdir': meson.current_source_dir(),

diff --git a/src/network/network.conf.in b/src/network/network.conf.in
index 5c84003f6da664d8669577ea5588ed692edc7eff..ec75e125d8c1419ca58dfee60d483e819a109870 100644 (file)
--- a/src/network/network.conf.in
+++ b/src/network/network.conf.in
@@ -1,3 +1,11 @@
 # Master configuration file for the network driver.
 # All settings described here are optional - if omitted, sensible
 # defaults are used.
+
+# firewall_backend:
+#
+#   determines which subsystem to use to setup firewall packet
+#   filtering rules for virtual networks. Currently the only supported
+#   selection is "iptables".
+#
+#firewall_backend = "@FIREWALL_BACKEND@"

diff --git a/src/network/test_libvirtd_network.aug.in b/src/network/test_libvirtd_network.aug.in
index ffdca520ce2667556de52362edd2c9d165ffe4ae..9e29a9192f764c21d6a8b02f1255d04076711f2c 100644 (file)
--- a/src/network/test_libvirtd_network.aug.in
+++ b/src/network/test_libvirtd_network.aug.in
@@ -1,2 +1,5 @@
 module Test_libvirtd_network =
   @CONFIG@
+
+  test Libvirtd_network.lns get conf =
+{ "firewall_backend" = "@FIREWALL_BACKEND@" }

diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
index cb66a26294df543bdd4eb124e43a3753ea507059..3a9f409e2a1d200df39a60061c0e710255d51054 100644 (file)
--- a/tests/networkxml2firewalltest.c
+++ b/tests/networkxml2firewalltest.c
@@ -98,7 +98,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
     if (!(def = virNetworkDefParse(NULL, xml, NULL, false)))
         return -1;
 
-    if (networkAddFirewallRules(def) < 0)
+    if (networkAddFirewallRules(def, VIR_FIREWALL_BACKEND_IPTABLES) < 0)
         return -1;
 
     actual = actualargv = virBufferContentAndReset(&buf);