From bf156385a031c0cafa8a068f6fbfc41fc9823a99 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Tue, 14 Aug 2012 20:59:52 +0200 Subject: [PATCH] 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. --- AUTHORS | 2 +- configure.ac | 17 ++++++++++++ libvirt.spec.in | 11 ++++++++ src/Makefile.am | 4 +-- src/network/bridge_driver.c | 49 ++++++++++++++++++++++++++++++++++ src/util/ebtables.c | 52 ++++++++++++++++++++++++++++++++++++- src/util/iptables.c | 49 +++++++++++++++++++++++++++++++--- 7 files changed, 177 insertions(+), 7 deletions(-) diff --git a/AUTHORS b/AUTHORS index 8581aea8a7..5dec3a25b1 100644 --- a/AUTHORS +++ b/AUTHORS @@ -257,7 +257,7 @@ Patches have also been contributed by: Frido Roose Asad Saeed Sukadev Bhattiprolu - + Thomas Woerner [....send patches to get your name here....] The libvirt Logo was designed by Diana Fong diff --git a/configure.ac b/configure.ac index e7d36bc6b0..2c33d77779 100644 --- 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 67b955a0b6..ea2fd883d9 100644 --- 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 152ee5b7a4..4bc7e2f1df 100644 --- 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 bff2d30366..7144589b72 100644 --- 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 ca056b1fa3..1a78f891ff 100644 --- 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 b23aca98db..d8fdd3b425 100644 --- 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",