On Mon, Apr 23, 2012 at 11:11:51PM +0200, Thomas Woerner wrote:
Add support for firewalld
* bridge_driver, nwfilter_driver: new dbus filters to get FirewallD1.Reloaded
signal and DBus.NameOwnerChanged on org.fedoraproject.FirewallD1
* iptables, ebtables, nwfilter_ebiptables_driver: use firewall-cmd direct
passthrough interface
---
configure.ac | 9 ++++++
src/Makefile.am | 8 ++---
src/network/bridge_driver.c | 50 +++++++++++++++++++++++++++++
src/nwfilter/nwfilter_driver.c | 49 ++++++++++++++++++++++++++++
src/nwfilter/nwfilter_ebiptables_driver.c | 27 ++++++++++++++++
src/util/ebtables.c | 32 ++++++++++++++++++
src/util/iptables.c | 25 ++++++++++++---
7 files changed, 192 insertions(+), 8 deletions(-)
diff --git a/configure.ac b/configure.ac
index 89fe818..41d9371 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1191,6 +1191,15 @@ 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]))
+if test "x$with_firewalld" = "xyes" ; then
+ AC_DEFINE_UNQUOTED([HAVE_FIREWALLD], [1], [whether firewalld support is enabled])
+fi
+AM_CONDITIONAL([HAVE_FIREWALLD], [test "x$with_firewalld" =
"xyes"])
+
+
dnl Avahi library
AC_ARG_WITH([avahi],
AC_HELP_STRING([--with-avahi], [use avahi to advertise remote daemon
@<:@default=check@:>@]),
To go along with this, you should also modify the libvirt.spec.in file in
GIT, to add logic to turn on firewalld on Fedora >= 17 + RHEL >= 7 only,
forcably disabling elsewhere.
diff --git a/src/network/bridge_driver.c
b/src/network/bridge_driver.c
index d82212f..094bbae 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -63,6 +63,11 @@
#include "virnetdevbridge.h"
#include "virnetdevtap.h"
+#if HAVE_FIREWALLD
+#include "virdbus.h"
+#include "logging.h"
+#endif
While technically correct, this is overkill - just let them be
included unconditionally.
+
#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -253,6 +258,24 @@ 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 = (struct network_driver *) user_data;
This cast is not required, since void * auto-casts to anything in C.
+
+ 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:
*
@@ -262,6 +285,9 @@ static int
networkStartup(int privileged) {
uid_t uid = geteuid();
char *base = NULL;
+#ifdef HAVE_FIREWALLD
+ DBusConnection *sysbus = NULL;
+#endif
if (VIR_ALLOC(driverState) < 0)
goto error;
@@ -326,6 +352,30 @@ 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, (void
*)driverState, NULL);
No need to cast to void * here either.
+ }
+#endif
+
return 0;
out_of_memory:
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index ffb4b5d..da9f4a0 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -27,6 +27,11 @@
#include <config.h>
+#if HAVE_FIREWALLD
+#include "virdbus.h"
+#include "logging.h"
+#endif
As before can make this unconditional
+
#include "internal.h"
#include "virterror_internal.h"
@@ -47,6 +52,8 @@ static virNWFilterDriverStatePtr driverState;
static int nwfilterDriverShutdown(void);
+static int nwfilterDriverReload(void);
+
static void nwfilterDriverLock(virNWFilterDriverStatePtr driver)
{
virMutexLock(&driver->lock);
@@ -57,6 +64,22 @@ static void nwfilterDriverUnlock(virNWFilterDriverStatePtr driver)
}
+#if HAVE_FIREWALLD
+static DBusHandlerResult
+firewalld_dbus_filter_nwfilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage
*message, void *user_data ATTRIBUTE_UNUSED) {
+ if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
+ "NameOwnerChanged") ||
+ dbus_message_is_signal(message, "org.fedoraproject.FirewallD1",
+ "Reloaded"))
+ {
+ VIR_DEBUG("Reload in nwfilter_driver because of firewalld.");
+ nwfilterDriverReload();
+ }
+
+ return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+#endif
+
/**
* virNWFilterStartup:
*
@@ -66,6 +89,32 @@ static int
nwfilterDriverStartup(int privileged) {
char *base = NULL;
+#ifdef HAVE_FIREWALLD
+ DBusConnection *sysbus = NULL;
+
+ if (!(sysbus = virDBusGetSystemBus())) {
+ virErrorPtr err = virGetLastError();
+ VIR_WARN("DBus not available, disabling firewalld support in
nwfilter_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_nwfilter, NULL, NULL);
+ }
+#endif
+
if (virNWFilterLearnInit() < 0)
return -1;
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 8e4436f..0cd047a 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -4051,6 +4051,7 @@ ebiptablesDriverInit(bool privileged)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *errmsg = NULL;
+ char *firewall_cmd_path = NULL;
if (!privileged)
return 0;
@@ -4061,6 +4062,30 @@ ebiptablesDriverInit(bool privileged)
gawk_cmd_path = virFindFileInPath("gawk");
grep_cmd_path = virFindFileInPath("grep");
+ firewall_cmd_path = virFindFileInPath("firewall-cmd");
+ if (firewall_cmd_path) {
+ virBufferAsprintf(&buf, "IPT=%s\n", firewall_cmd_path);
+ /* basic probing */
+ virBufferAsprintf(&buf,
+ CMD_DEF("$IPT --state") CMD_SEPARATOR
+ CMD_EXEC
+ "%s",
+ CMD_STOPONERR(1));
+
+ if (ebiptablesExecCLI(&buf, NULL, NULL) >= 0) {
+ VIR_DEBUG("Using firewall-cmd in nwfilter_ebiptables_driver.");
+ ebtables_cmd_path = NULL;
+ iptables_cmd_path = NULL;
+ ip6tables_cmd_path = NULL;
+ virAsprintf(&ebtables_cmd_path, "%s --direct --passthrough
eb", firewall_cmd_path);
+ virAsprintf(&iptables_cmd_path, "%s --direct --passthrough
ipv4", firewall_cmd_path);
+ virAsprintf(&ip6tables_cmd_path, "%s --direct --passthrough
ipv6", firewall_cmd_path);
Need to check for & report OOM in virAsprintf. I'm surprised we haven't
actually annotated it with ATTRIBUTE_RETURN_CHECK to catch this
+ }
+ VIR_FREE(firewall_cmd_path);
+ }
+ if (ebtables_cmd_path == NULL || iptables_cmd_path == NULL ||
+ ip6tables_cmd_path == NULL) {
+
ebtables_cmd_path = virFindFileInPath("ebtables");
if (ebtables_cmd_path) {
NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
@@ -4099,6 +4124,8 @@ ebiptablesDriverInit(bool privileged)
VIR_WARN("Could not find 'iptables' executable");
}
+ }
+
ip6tables_cmd_path = virFindFileInPath("ip6tables");
if (ip6tables_cmd_path) {
NWFILTER_SET_IP6TABLES_SHELLVAR(&buf);
diff --git a/src/util/ebtables.c b/src/util/ebtables.c
index dcb3eb9..b7773ea 100644
--- a/src/util/ebtables.c
+++ b/src/util/ebtables.c
@@ -176,11 +176,34 @@ ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg,
...)
const char *s;
int n, command_idx;
+#if HAVE_FIREWALLD
+ int ret;
+ char *firewall_cmd_path = NULL;
+ virCommandPtr cmd = NULL;
+
+ firewall_cmd_path = virFindFileInPath("firewall-cmd");
+ if (firewall_cmd_path) {
+ cmd = virCommandNew(firewall_cmd_path);
+ virCommandAddArgList(cmd, "--state", NULL);
+ ret = virCommandRun(cmd, NULL);
+ if (ret != 0) {
+ VIR_FREE(firewall_cmd_path);
+ firewall_cmd_path = NULL;
+ }
+ virCommandFree(cmd);
+ }
+#endif
+
n = 1 + /* /sbin/ebtables */
2 + /* --table foo */
2 + /* --insert bar */
1; /* arg */
+#if HAVE_FIREWALLD
+ if (firewall_cmd_path)
+ n += 3; /* --direct --passthrough eb */
+#endif
+
va_start(args, arg);
while (va_arg(args, const char *))
n++;
@@ -192,6 +215,15 @@ 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;
+ argv[n++] = strdup("--direct");
+ argv[n++] = strdup("--passthrough");
+ argv[n++] = strdup("eb");
Need to check OOM on all 4 of thoses strdup()'s not just the first
+ } else
+#endif
if (!(argv[n++] = strdup(EBTABLES_PATH)))
goto error;
Not your fault, but this is a nice reminder to us that we need to convert
this code to virCommand.
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 3023900..0cb3293 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -104,11 +104,28 @@ 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)
- ? IP6TABLES_PATH : IPTABLES_PATH);
+ char *firewall_cmd_path = NULL;
+
+#if HAVE_FIREWALLD
+ firewall_cmd_path = virFindFileInPath("firewall-cmd");
+ if (firewall_cmd_path) {
+ cmd = virCommandNew(firewall_cmd_path);
+ virCommandAddArgList(cmd, "--state", NULL);
+ ret = virCommandRun(cmd, NULL);
+ if (ret == 0) {
+ cmd = virCommandNew(firewall_cmd_path);
+ virCommandAddArgList(cmd, "--direct", "--passthrough",
+ (family == AF_INET6) ? "ipv6" :
"ipv4", NULL);
+ } else
+ cmd = NULL;
+ VIR_FREE(firewall_cmd_path);
+ }
+ if (!cmd)
+#endif
+ cmd = virCommandNew((family == AF_INET6)
+ ? IP6TABLES_PATH : IPTABLES_PATH);
virCommandAddArgList(cmd, "--table", rules->table,
action == ADD ? "--insert" : "--delete",
--
Regards,
Daniel
--
|:
http://berrange.com -o-
http://www.flickr.com/photos/dberrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|:
http://entangle-photo.org -o-
http://live.gnome.org/gtk-vnc :|