network: use firewalld instead of iptables, when available

* configure.ac, spec file: firewalld defaults to enabled if dbus is
  available, otherwise is disabled. If --with_firewalld is explicitly
  requested and dbus is not available, configure will fail.

* bridge_driver: add dbus filters to get the FirewallD1.Reloaded
  signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1.
  When these are encountered, reload all the iptables reuls of all
  libvirt's virtual networks (similar to what happens when libvirtd is
  restarted).

* iptables, ebtables: use firewall-cmd's direct passthrough interface
  when available, otherwise use iptables and ebtables commands. This
  decision is made once the first time libvirt calls
  iptables/ebtables, and that decision is maintained for the life of
  libvirtd.

* Note that the nwfilter part of this patch was separated out into
  another patch by Stefan in V2, so that needs to be revised and
  re-reviewed as well.

================

All the configure.ac and specfile changes are unchanged from Thomas'
V3.

V3 re-ran "firewall-cmd --state" every time a new rule was added,
which was extremely inefficient.  V4 uses VIR_ONCE_GLOBAL_INIT to set
up a one-time initialization function.

The VIR_ONCE_GLOBAL_INIT(x) macro references a static function called
vir(Ip|Eb)OnceInit(), which will then be called the first time that
the static function vir(Ip|Eb)TablesInitialize() is called (that
function is defined for you by the macro). This is
thread-safe, so there is no chance of any race.

IMPORTANT NOTE: I've left the VIR_DEBUG messages in these two init
functions (one for iptables, on for ebtables) as VIR_WARN so that I
don't have to turn on all the other debug message just to see
these. Even if this patch doesn't need any other modification, those
messages need to be changed to VIR_DEBUG before pushing.

This one-time initialization works well. However, I've encountered
problems with testing:

1) Whenever I have enabled the firewalld service, *all* attempts to
call firewall-cmd from within libvirtd end with firewall-cmd hanging
internally somewhere. This is *not* the case if firewall-cmd returns
non-0 in response to "firewall-cmd --state" (i.e. *that* command runs
and returns to libvirt successfully.)

2) If I start libvirtd while firewalld is stopped, then start
firewalld later, this triggers libvirtd to reload its iptables rules,
however it also spits out a *ton* of complaints about deletion failing
(I suppose because firewalld has nuked all of libvirt's rules). I
guess we need to suppress those messages (which is a more annoying
problem to fix than you might think, but that's another story).

3) I noticed a few times during this long line of errors that
firewalld made a complaint about "Resource Temporarily
unavailable. Having libvirtd access iptables commands directly at the
same time as firewalld is doing so is apparently problematic.

4) In general, I'm concerned about the "set it once and never change
it" method - if firewalld is disabled at libvirtd startup, causing
libvirtd to always use iptables/ebtables directly, this won't cause
*terrible* problems, but if libvirtd decides to use firewall-cmd and
firewalld is later disabled, libvirtd will not be able to recover.

diff --git a/AUTHORS b/AUTHORS
index 8581aea8a7f0354309674991d65e51d7891e4ed1..5dec3a25b16202cbe3148d929078eb9b5abc063a 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -257,7 +257,7 @@ Patches have also been contributed by:
   Frido Roose          <frido.roose@gmail.com>
   Asad Saeed           <asad.saeed@acidseed.com>
   Sukadev Bhattiprolu  <sukadev@linux.vnet.ibm.com>
-
+  Thomas Woerner       <twoerner@redhat.com>
   [....send patches to get your name here....]
 
 The libvirt Logo was designed by Diana Fong

diff --git a/configure.ac b/configure.ac
index e7d36bc6b05adf8d98af392dce2e7e8e14060077..2c33d77779fd9f8e4137a76915d0319fcd658dec 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -1327,6 +1327,22 @@ AM_CONDITIONAL([HAVE_POLKIT1], [test "x$with_polkit1" = "xyes"])
 AC_SUBST([POLKIT_CFLAGS])
 AC_SUBST([POLKIT_LIBS])
 
+dnl firewalld
+AC_ARG_WITH([firewalld],
+  AC_HELP_STRING([--with-firewalld], [enable firewalld support @<:@default=check@:>@]),
+  [],
+  [with_firewalld=check])
+if test "x$with_firewalld" = "xcheck" ; then
+   with_firewalld=$with_dbus
+fi
+if test "x$with_firewalld" == "xyes" ; then
+  if test "x$with_dbus" != "xyes" ; then
+     AC_MSG_ERROR([You must have dbus enabled for firewalld support])
+  fi
+  AC_DEFINE_UNQUOTED([HAVE_FIREWALLD], [1], [whether firewalld support is enabled])
+fi
+AM_CONDITIONAL([HAVE_FIREWALLD], [test "x$with_firewalld" != "xno"])
+
 dnl Avahi library
 AC_ARG_WITH([avahi],
   AC_HELP_STRING([--with-avahi], [use avahi to advertise remote daemon @<:@default=check@:>@]),
@@ -3063,6 +3079,7 @@ AC_MSG_NOTICE([ sanlock: $SANLOCK_CFLAGS $SANLOCK_LIBS])
 else
 AC_MSG_NOTICE([ sanlock: no])
 fi
+AC_MSG_NOTICE([firewalld: $with_firewalld])
 if test "$with_avahi" = "yes" ; then
 AC_MSG_NOTICE([   avahi: $AVAHI_CFLAGS $AVAHI_LIBS])
 else

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 67b955a0b6cfe1a823822066d4acb8bffea45de0..ea2fd883d98f6580d7cef3f2c261ae9fe2de4033 100644 (file)
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -106,6 +106,7 @@
 %define with_sanlock       0%{!?_without_sanlock:0}
 %define with_systemd       0%{!?_without_systemd:0}
 %define with_numad         0%{!?_without_numad:0}
+%define with_firewalld     0%{!?_without_firewalld:0}
 
 # Non-server/HV driver defaults which are always enabled
 %define with_python        0%{!?_without_python:1}
@@ -146,6 +147,11 @@
 %define with_systemd 1
 %endif
 
+# Fedora 18 / RHEL-7 are first where firewalld support is enabled
+%if 0%{?fedora} >= 17 || 0%{?rhel} >= 7
+%define with_firewalld 1
+%endif
+
 # RHEL-5 has restricted QEMU to x86_64 only and is too old for LXC
 %if 0%{?rhel} == 5
 %define with_qemu_tcg 0
@@ -1182,6 +1188,10 @@ of recent versions of Linux (and other OSes).
 %define _without_driver_modules --without-driver-modules
 %endif
 
+%if %{with_firewalld}
+%define _with_firewalld --with-firewalld
+%endif
+
 %define when  %(date +"%%F-%%T")
 %define where %(hostname)
 %define who   %{?packager}%{!?packager:Unknown}
@@ -1240,6 +1250,7 @@ autoreconf -if
            %{?_without_audit} \
            %{?_without_dtrace} \
            %{?_without_driver_modules} \
+           %{?_with_firewalld} \
            %{with_packager} \
            %{with_packager_version} \
            --with-qemu-user=%{qemu_user} \

diff --git a/src/Makefile.am b/src/Makefile.am
index 152ee5b7a47c1c4edb17ffacd3963327a71e3ce7..4bc7e2f1df5ee36a164ac99a68d91dcdb840a5b2 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -997,7 +997,7 @@ libvirt_driver_network_la_SOURCES =
 libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_network.la
-libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la $(LIBNL_LIBS)
+libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la $(LIBNL_LIBS) $(DBUS_LIBS)
 libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_network.la
@@ -1007,7 +1007,7 @@ endif
 
 libvirt_driver_network_impl_la_CFLAGS = \
                $(LIBNL_CFLAGS) \
-               -I$(top_srcdir)/src/conf $(AM_CFLAGS)
+               -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS)
 libvirt_driver_network_impl_la_SOURCES = $(NETWORK_DRIVER_SOURCES)
 endif
 EXTRA_DIST += network/default.xml

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index bff2d30366aaa8334432d23a2062700b771be73b..7144589b72eeb5e69d102ad1dc04f5783c9279dc 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -64,6 +64,7 @@
 #include "virnetdevbridge.h"
 #include "virnetdevtap.h"
 #include "virnetdevvportprofile.h"
+#include "virdbus.h"
 
 #define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
 #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -251,6 +252,25 @@ networkAutostartConfigs(struct network_driver *driver) {
     }
 }
 
+#if HAVE_FIREWALLD
+static DBusHandlerResult
+firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED,
+                             DBusMessage *message, void *user_data) {
+    struct network_driver *_driverState = user_data;
+
+    if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
+                               "NameOwnerChanged") ||
+        dbus_message_is_signal(message, "org.fedoraproject.FirewallD1",
+                               "Reloaded"))
+    {
+        VIR_DEBUG("Reload in bridge_driver because of firewalld.");
+        networkReloadIptablesRules(_driverState);
+    }
+
+    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+#endif
+
 /**
  * networkStartup:
  *
@@ -259,6 +279,9 @@ networkAutostartConfigs(struct network_driver *driver) {
 static int
 networkStartup(int privileged) {
     char *base = NULL;
+#ifdef HAVE_FIREWALLD
+    DBusConnection *sysbus = NULL;
+#endif
 
     if (VIR_ALLOC(driverState) < 0)
         goto error;
@@ -325,6 +348,32 @@ networkStartup(int privileged) {
 
     networkDriverUnlock(driverState);
 
+#ifdef HAVE_FIREWALLD
+    if (!(sysbus = virDBusGetSystemBus())) {
+        virErrorPtr err = virGetLastError();
+        VIR_WARN("DBus not available, disabling firewalld support "
+                 "in bridge_driver: %s", err->message);
+    } else {
+        /* add matches for
+         * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop
+         * Reloaded on org.fedoraproject.FirewallD1 for firewalld reload
+         */
+        dbus_bus_add_match(sysbus,
+                           "type='signal'"
+                           ",interface='"DBUS_INTERFACE_DBUS"'"
+                           ",member='NameOwnerChanged'"
+                           ",arg0='org.fedoraproject.FirewallD1'",
+                           NULL);
+        dbus_bus_add_match(sysbus,
+                           "type='signal'"
+                           ",interface='org.fedoraproject.FirewallD1'"
+                           ",member='Reloaded'",
+                           NULL);
+        dbus_connection_add_filter(sysbus, firewalld_dbus_filter_bridge,
+                                   driverState, NULL);
+    }
+#endif
+
     return 0;
 
 out_of_memory:

diff --git a/src/util/ebtables.c b/src/util/ebtables.c
index ca056b1fa33566ea33704788d44694f9e28e6465..1a78f891ffcd5ae12a8dd5a59b84299d603f1b0a 100644 (file)
--- a/src/util/ebtables.c
+++ b/src/util/ebtables.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2010 Red Hat, Inc.
+ * Copyright (C) 2007-2012 Red Hat, Inc.
  * Copyright (C) 2009 IBM Corp.
  *
  * This library is free software; you can redistribute it and/or
@@ -45,6 +45,38 @@
 #include "memory.h"
 #include "virterror_internal.h"
 #include "logging.h"
+#include "threads.h"
+
+#if HAVE_FIREWALLD
+static char *firewall_cmd_path = NULL;
+
+static int
+virEbTablesOnceInit(void)
+{
+    firewall_cmd_path = virFindFileInPath("firewall-cmd");
+    if (!firewall_cmd_path) {
+        VIR_WARN("firewall-cmd not found on system. "
+                 "firewalld support disabled for ebtables.");
+    } else {
+        virCommandPtr cmd = virCommandNew(firewall_cmd_path);
+        int status;
+
+        virCommandAddArgList(cmd, "--state", NULL);
+        if (virCommandRun(cmd, &status) < 0 || status != 0) {
+            VIR_WARN("firewall-cmd found but disabled for ebtables");
+            VIR_FREE(firewall_cmd_path);
+            firewall_cmd_path = NULL;
+        } else {
+            VIR_WARN("using firewalld for ebtables commands");
+        }
+        virCommandFree(cmd);
+    }
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virEbTables)
+
+#endif
 
 struct _ebtablesContext
 {
@@ -181,6 +213,12 @@ ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg, ...)
         2 + /*   --insert bar  */
         1;  /*   arg           */
 
+#if HAVE_FIREWALLD
+    virEbTablesInitialize();
+    if (firewall_cmd_path)
+        n += 3; /* --direct --passthrough eb */
+#endif
+
     va_start(args, arg);
     while (va_arg(args, const char *))
         n++;
@@ -192,6 +230,18 @@ ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg, ...)
 
     n = 0;
 
+#if HAVE_FIREWALLD
+    if (firewall_cmd_path) {
+        if (!(argv[n++] = strdup(firewall_cmd_path)))
+            goto error;
+        if (!(argv[n++] = strdup("--direct")))
+            goto error;
+        if (!(argv[n++] = strdup("--passthrough")))
+            goto error;
+        if (!(argv[n++] = strdup("eb")))
+            goto error;
+    } else
+#endif
     if (!(argv[n++] = strdup(EBTABLES_PATH)))
         goto error;
 

diff --git a/src/util/iptables.c b/src/util/iptables.c
index b23aca98dba347c7dc73493b9c4dee1ac18385c8..d8fdd3b425241b353fec606c3f1cdfc94264ee19 100644 (file)
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2011 Red Hat, Inc.
+ * Copyright (C) 2007-2012 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -43,6 +43,38 @@
 #include "memory.h"
 #include "virterror_internal.h"
 #include "logging.h"
+#include "threads.h"
+
+#if HAVE_FIREWALLD
+static char *firewall_cmd_path = NULL;
+
+static int
+virIpTablesOnceInit(void)
+{
+    firewall_cmd_path = virFindFileInPath("firewall-cmd");
+    if (!firewall_cmd_path) {
+        VIR_WARN("firewall-cmd not found on system. "
+                 "firewalld support disabled for iptables.");
+    } else {
+        virCommandPtr cmd = virCommandNew(firewall_cmd_path);
+        int status;
+
+        virCommandAddArgList(cmd, "--state", NULL);
+        if (virCommandRun(cmd, &status) < 0 || status != 0) {
+            VIR_WARN("firewall-cmd found but disabled for iptables");
+            VIR_FREE(firewall_cmd_path);
+            firewall_cmd_path = NULL;
+        } else {
+            VIR_WARN("using firewalld for iptables commands");
+        }
+        virCommandFree(cmd);
+    }
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virIpTables)
+
+#endif
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -101,11 +133,22 @@ iptablesAddRemoveRule(iptRules *rules, int family, int action,
 {
     va_list args;
     int ret;
-    virCommandPtr cmd;
+    virCommandPtr cmd = NULL;
     const char *s;
 
-    cmd = virCommandNew((family == AF_INET6)
+#if HAVE_FIREWALLD
+    virIpTablesInitialize();
+    if (firewall_cmd_path) {
+        cmd = virCommandNew(firewall_cmd_path);
+        virCommandAddArgList(cmd, "--direct", "--passthrough",
+                             (family == AF_INET6) ? "ipv6" : "ipv4", NULL);
+    }
+#endif
+
+    if (cmd == NULL) {
+        cmd = virCommandNew((family == AF_INET6)
                         ? IP6TABLES_PATH : IPTABLES_PATH);
+    }
 
     virCommandAddArgList(cmd, "--table", rules->table,
                          action == ADD ? "--insert" : "--delete",