1:18 a.m.
From: Thomas Woerner <twoerner(a)redhat.com>
(This is Thomas v3 version of 1/2 of the firewalld patches, modified
to check for firewall-cmd and firewalld state only once, rather than
every time an iptables rule is added or removed. It's not intended to
be pushed, because I'm still having issues with it, at least on my
machine. I'm mostly concerned with item (1) on the list below; the
others could be solved later or tolerated.)
* 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 8581aea..5dec3a2 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -257,7 +257,7 @@ Patches have also been contributed by:
Frido Roose <frido.roose(a)gmail.com>
Asad Saeed <asad.saeed(a)acidseed.com>
Sukadev Bhattiprolu <sukadev(a)linux.vnet.ibm.com>
-
+ Thomas Woerner <twoerner(a)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 ba5a3cd..0150f99 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1321,6 +1321,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@:>@]),
@@ -3028,6 +3044,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 67b955a..ea2fd88 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 b5f8056..6a94ecc 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -988,7 +988,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
@@ -998,7 +998,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 474bbfa..e3f0c1c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -62,6 +62,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"
@@ -249,6 +250,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:
*
@@ -257,6 +277,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;
@@ -323,6 +346,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 ca056b1..1a78f89 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 b23aca9..d8fdd3b 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",
--
1.7.11.4