[PATCH v3 12/27] network: support setting firewallBackend from network.conf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 the iptables binary is present on the system and set
firewallBackend to iptables - if no iptables binary is found, 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". This way the conf file will have correct
information no matter what default backend is chosen at build time.

Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
---
Changes from V2:

 * make failure to find iptables during network driver init fatal

 * recognize firewall_backend setting in meson_options.txt and
   propagate it through to:

    * meson-config.h (along with a numeric constant
      FIREWALL_BACKEND_DEFAULT_IPTABLES, which can be used for
      conditional compilation)
    * network.conf - default setting and comments use @FIREWALL_BACKEND@
    * test_libvirtd_network.aug.in so that default firewall backend
      for testing is set properly (this required calisthenics similar to
      qemu_user_group_hack_conf in the qemu driver's meson.build
      (see commit 64a7b8203bf)

 meson.build                              |  5 +++
 meson_options.txt                        |  1 +
 src/network/bridge_driver.c              | 22 ++++++-----
 src/network/bridge_driver_conf.c         | 50 ++++++++++++++++++++++++
 src/network/bridge_driver_conf.h         |  3 ++
 src/network/bridge_driver_linux.c        |  6 ++-
 src/network/bridge_driver_nop.c          |  6 ++-
 src/network/bridge_driver_platform.h     |  6 ++-
 src/network/libvirtd_network.aug         |  5 ++-
 src/network/meson.build                  | 22 ++++++++++-
 src/network/network.conf.in              |  8 ++++
 src/network/test_libvirtd_network.aug.in |  3 ++
 tests/networkxml2firewalltest.c          |  2 +-
 13 files changed, 119 insertions(+), 20 deletions(-)

diff --git a/meson.build b/meson.build
index 1518afa1cb..02340c09dd 100644
--- a/meson.build
+++ b/meson.build
@@ -1618,6 +1618,11 @@ endif
 
 if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
   conf.set('WITH_NETWORK', 1)
+  firewall_backend = get_option('firewall_backend')
+  conf.set_quoted('FIREWALL_BACKEND', firewall_backend)
+  if firewall_backend == 'iptables'
+    conf.set('FIREWALL_BACKEND_DEFAULT_IPTABLES', 1)
+  endif
 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 ed91d97abf..367629f5dc 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -98,6 +98,7 @@ option('chrdev_lock_files', type: 'string', value: '', description: 'location fo
 option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for static probing')
 option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support')
 option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone')
+option('firewall_backend', type: 'string', value: 'iptables', description: 'which firewall backend to use by default 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 d89700c6ee..38e4ab84ad 100644
--- 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;
@@ -2064,7 +2065,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
 
     if (firewalRulesAdded &&
         def->forward.type != VIR_NETWORK_FORWARD_OPEN)
-        networkRemoveFirewallRules(def);
+        networkRemoveFirewallRules(def, cfg->firewallBackend);
 
     virNetworkObjUnrefMacMap(obj);
 
@@ -2076,7 +2077,8 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
 
 
 static int
-networkShutdownNetworkVirtual(virNetworkObj *obj)
+networkShutdownNetworkVirtual(virNetworkObj *obj,
+                              virNetworkDriverConfig *cfg)
 {
     virNetworkDef *def = virNetworkObjGetDef(obj);
     pid_t dnsmasqPid;
@@ -2102,7 +2104,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));
 
@@ -2406,7 +2408,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:
@@ -3257,7 +3259,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:
@@ -3285,14 +3287,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 a2edafa837..25cbbf8933 100644
--- 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"
 
@@ -62,6 +63,7 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
                            const char *filename)
 {
     g_autoptr(virConf) conf = NULL;
+    g_autofree char *firewallBackendStr = NULL;
 
     /* if file doesn't exist or is unreadable, ignore the "error" */
     if (access(filename, R_OK) == -1)
@@ -73,6 +75,54 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
 
     /* use virConfGetValue*(conf, ...) functions to read any settings into cfg */
 
+    if (virConfGetValueString(conf, "firewall_backend", &firewallBackendStr) < 0)
+        return -1;
+
+    if (firewallBackendStr) {
+        int backend = virFirewallBackendTypeFromString(firewallBackendStr);
+
+        if (backend < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("unknown value for 'firewall_backend' in network.conf: '%1$s'"),
+                           firewallBackendStr);
+            return -1;
+        }
+
+        cfg->firewallBackend = backend;
+        VIR_INFO("using firewall_backend setting from network.conf: '%s'",
+                 virFirewallBackendTypeToString(cfg->firewallBackend));
+
+    } else {
+
+        /* no .conf setting, so see what this host supports by looking
+         * for binaries used by the backends, and set accordingly.
+         */
+        g_autofree char *iptablesInPath = NULL;
+        bool binaryFound = false;
+
+        /* virFindFileInPath() uses g_find_program_in_path(),
+         * which allows absolute paths, and verifies that
+         * the file is executable.
+        */
+#if defined(FIREWALL_BACKEND_DEFAULT_IPTABLES)
+        if ((iptablesInPath = virFindFileInPath(IPTABLES))) {
+            cfg->firewallBackend = VIR_FIREWALL_BACKEND_IPTABLES;
+            binaryFound = true;
+        }
+#else
+# error "No usable firewall_backend was set in build options"
+#endif
+
+        if (!binaryFound) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("firewall_backend not set, and no usable backend auto-detected"));
+            return -1;
+        }
+
+        VIR_INFO("using auto-detected firewall_backend: '%s'",
+                 virFirewallBackendTypeToString(cfg->firewallBackend));
+    }
+
     return 0;
 }
 
diff --git a/src/network/bridge_driver_conf.h b/src/network/bridge_driver_conf.h
index 426c16198d..8f221f391e 100644
--- 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 4914d5c903..c2ef27f251 100644
--- 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 6eee6043e6..7d9a061e50 100644
--- 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 b720d343be..7443c3129f 100644
--- 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 ae153d96a1..5d6d72dd92 100644
--- 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 0336435862..c79a36da22 100644
--- 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,
+  })
+
   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,
+    # 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 5c84003f6d..ec75e125d8 100644
--- 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 ffdca520ce..9e29a9192f 100644
--- 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 cb66a26294..3a9f409e2a 100644
--- 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);
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux