[libvirt] [PATCH] Add support for firewalld

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@:>@]), diff --git a/src/Makefile.am b/src/Makefile.am index e48dfa5..e60a8af 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -941,9 +941,9 @@ noinst_LTLIBRARIES += libvirt_driver_network.la #libvirt_la_BUILT_LIBADD += libvirt_driver_network.la endif libvirt_driver_network_la_CFLAGS = \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) + -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS) if WITH_DRIVER_MODULES -libvirt_driver_network_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_network_la_LIBADD = ../gnulib/lib/libgnu.la $(DBUS_LIBS) libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif libvirt_driver_network_la_SOURCES = $(NETWORK_DRIVER_SOURCES) @@ -1086,9 +1086,9 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_nwfilter.la noinst_LTLIBRARIES += libvirt_driver_nwfilter.la endif libvirt_driver_nwfilter_la_CFLAGS = $(LIBPCAP_CFLAGS) \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) + -I$(top_srcdir)/src/conf $(AM_CFLAGS) $(DBUS_CFLAGS) libvirt_driver_nwfilter_la_LDFLAGS = $(LD_AMFLAGS) -libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) +libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) $(DBUS_LIBS) if WITH_DRIVER_MODULES libvirt_driver_nwfilter_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_nwfilter_la_LDFLAGS += -module -avoid-version 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 + #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; + + 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); + } +#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 + #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); + } + 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"); + } else +#endif if (!(argv[n++] = strdup(EBTABLES_PATH))) goto error; 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", -- 1.7.10

On 04/23/2012 05:11 PM, 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
Great! After fixing some problems I at least got the nwfilter part to work and the TCK test suite passed without a problem. I didn't test firewalld shutdown and restart interaction so far. I would like to massage the nwfilter part a little bit more tomorrow and send you a patch you can then merge with this one. Thanks! Stefan

On 04/23/2012 05:11 PM, 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
After some more massaging of the nwfilter code, my suggestion would now be to split this patch up into two parts, one touching the nwfilter driver, the other (1st) part for the rest. I did a lot of changes in the nwfilter driver that I can send you and you may want to merge or I can merge it with your nwfilter-related code changes. It seems to be working when using the firewall-cmd, but unfortunately running the TCK test suite for example is like 8 times slower when using firewalld. Also the VM startup times have significantly increased. :-(( Is this scheduled to be included in the next libvirt release ? I guess architecturally it also is needed for FC 17, so is the plan then to include the latest version of libvirt with firewalld support in FC17? Stefan

On Tue, Apr 24, 2012 at 10:20:32AM -0400, Stefan Berger wrote:
On 04/23/2012 05:11 PM, 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
After some more massaging of the nwfilter code, my suggestion would now be to split this patch up into two parts, one touching the nwfilter driver, the other (1st) part for the rest. I did a lot of changes in the nwfilter driver that I can send you and you may want to merge or I can merge it with your nwfilter-related code changes.
It seems to be working when using the firewall-cmd, but unfortunately running the TCK test suite for example is like 8 times slower when using firewalld. Also the VM startup times have significantly increased. :-((
I wonder if that would be improved by making DBus calls directly to firewalld, instead of invoking firewalld-cmd all the time. The latter is unquestionably inefficient compared to DBus calls, but it'd be interesting to know if that's really what's causing the x8 slowdown.
Is this scheduled to be included in the next libvirt release ? I guess architecturally it also is needed for FC 17, so is the plan then to include the latest version of libvirt with firewalld support in FC17?
The libvirt in Fedora 17 is frozen at this point. So if we did include this, it'd be cherry-picking backports. 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 :|

On 04/24/2012 11:27 AM, Daniel P. Berrange wrote:
On Tue, Apr 24, 2012 at 10:20:32AM -0400, Stefan Berger wrote:
On 04/23/2012 05:11 PM, 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 After some more massaging of the nwfilter code, my suggestion would now be to split this patch up into two parts, one touching the nwfilter driver, the other (1st) part for the rest. I did a lot of changes in the nwfilter driver that I can send you and you may want to merge or I can merge it with your nwfilter-related code changes.
It seems to be working when using the firewall-cmd, but unfortunately running the TCK test suite for example is like 8 times slower when using firewalld. Also the VM startup times have significantly increased. :-(( I wonder if that would be improved by making DBus calls directly to firewalld, instead of invoking firewalld-cmd all the time. The latter is unquestionably inefficient compared to DBus calls, but it'd be interesting to know if that's really what's causing the x8 slowdown.
That would a bigger code change to go directly through DBus. I am currently accumulating CLI commands to execute and then run them in a batch. For comparison: time firewall-cmd --direct --passthrough eb -t nat -L [...] real 0m0.102s user 0m0.075s sys 0m0.013s versus time ebtables -t nat -L [...] real 0m0.003s user 0m0.000s sys 0m0.002s Well, I guess it adds up. Stefan

On Tue, Apr 24, 2012 at 12:01:38PM -0400, Stefan Berger wrote:
On 04/24/2012 11:27 AM, Daniel P. Berrange wrote:
On Tue, Apr 24, 2012 at 10:20:32AM -0400, Stefan Berger wrote:
On 04/23/2012 05:11 PM, 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 After some more massaging of the nwfilter code, my suggestion would now be to split this patch up into two parts, one touching the nwfilter driver, the other (1st) part for the rest. I did a lot of changes in the nwfilter driver that I can send you and you may want to merge or I can merge it with your nwfilter-related code changes.
It seems to be working when using the firewall-cmd, but unfortunately running the TCK test suite for example is like 8 times slower when using firewalld. Also the VM startup times have significantly increased. :-(( I wonder if that would be improved by making DBus calls directly to firewalld, instead of invoking firewalld-cmd all the time. The latter is unquestionably inefficient compared to DBus calls, but it'd be interesting to know if that's really what's causing the x8 slowdown.
That would a bigger code change to go directly through DBus. I am currently accumulating CLI commands to execute and then run them in a batch.
For comparison:
time firewall-cmd --direct --passthrough eb -t nat -L [...] real 0m0.102s user 0m0.075s sys 0m0.013s
versus
time ebtables -t nat -L [...] real 0m0.003s user 0m0.000s sys 0m0.002s
Well, I guess it adds up.
Yeah the DBus connection handshake being repeated soo many times, causing many many context switches for each single rule to be added. I wonder if firewall-cmd could be extended to allow multiple rules to be specified at once. It'd just need some kind of character to be designated as the separator for each rule. 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 :|

On 04/24/2012 12:11 PM, Daniel P. Berrange wrote:
On Tue, Apr 24, 2012 at 12:01:38PM -0400, Stefan Berger wrote:
Yeah the DBus connection handshake being repeated soo many times, causing many many context switches for each single rule to be added.
I wonder if firewall-cmd could be extended to allow multiple rules to be specified at once. It'd just need some kind of character to be designated as the separator for each rule.
Certainly this would be helpful for some applications. For nwfilter also this feature would require a bigger change since different form of formatting for the batching would be required. Also sometimes nwfilter needs control over the return status of a particular command while at other times commands can run without looking at their return status. Stefan

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 :|
participants (3)
-
Daniel P. Berrange
-
Stefan Berger
-
Thomas Woerner