Since I'm going to be adding at least one more firewalld-specific function, this seems like a good time to separate the code that's unique to firewalld from the more-generic "firewall" file. Signed-off-by: Laine Stump <laine@xxxxxxxxx> --- include/libvirt/virterror.h | 1 + src/libvirt_private.syms | 3 + src/util/Makefile.inc.am | 2 + src/util/virerror.c | 1 + src/util/virfirewall.c | 86 ++---------------------- src/util/virfirewalld.c | 128 ++++++++++++++++++++++++++++++++++++ src/util/virfirewalld.h | 33 ++++++++++ src/util/virfirewallpriv.h | 2 - tests/virfirewalltest.c | 1 + 9 files changed, 173 insertions(+), 84 deletions(-) create mode 100644 src/util/virfirewalld.c create mode 100644 src/util/virfirewalld.h diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index fbbe2d5624..3c19ff5e2e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -131,6 +131,7 @@ typedef enum { VIR_FROM_PERF = 65, /* Error from perf */ VIR_FROM_LIBSSH = 66, /* Error from libssh connection transport */ VIR_FROM_RESCTRL = 67, /* Error from resource control */ + VIR_FROM_FIREWALLD = 68, /* Error from firewalld */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_DOMAIN_LAST diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3d6306809..583868f422 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1918,6 +1918,9 @@ virFirewallSetLockOverride; virFirewallStartRollback; virFirewallStartTransaction; +# util/virfirewalld.h +virFirewallDApplyRule; +virFirewallDStatus; # util/virfirmware.h virFirmwareFreeList; diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index 4295babac3..0295a1c7d0 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -64,6 +64,8 @@ UTIL_SOURCES = \ util/virfirewall.c \ util/virfirewall.h \ util/virfirewallpriv.h \ + util/virfirewalld.c \ + util/virfirewalld.h \ util/virfirmware.c \ util/virfirmware.h \ util/virgettext.c \ diff --git a/src/util/virerror.c b/src/util/virerror.c index 61b47d2be0..ae1efa72d8 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -138,6 +138,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST, "Perf", /* 65 */ "Libssh transport layer", "Resource control", + "FirewallD", ) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 5a0cf95a44..d5d647fcc4 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -24,12 +24,12 @@ #define LIBVIRT_VIRFIREWALLPRIV_H_ALLOW #include "virfirewallpriv.h" +#include "virfirewalld.h" #include "virerror.h" #include "virutil.h" #include "virstring.h" #include "vircommand.h" #include "virlog.h" -#include "virdbus.h" #include "virfile.h" #include "virthread.h" @@ -46,11 +46,6 @@ VIR_ENUM_IMPL(virFirewallLayerCommand, VIR_FIREWALL_LAYER_LAST, IPTABLES_PATH, IP6TABLES_PATH); -VIR_ENUM_DECL(virFirewallLayerFirewallD) -VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST, - "eb", "ipv4", "ipv6") - - struct _virFirewallRule { virFirewallLayer layer; @@ -152,7 +147,7 @@ virFirewallValidateBackend(virFirewallBackend backend) VIR_DEBUG("Validating backend %d", backend); if (backend == VIR_FIREWALL_BACKEND_AUTOMATIC || backend == VIR_FIREWALL_BACKEND_FIREWALLD) { - int rv = virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE); + int rv = virFirewallDStatus(); VIR_DEBUG("Firewalld is registered ? %d", rv); if (rv < 0) { @@ -712,81 +707,8 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule, bool ignoreErrors, char **output) { - const char *ipv = virFirewallLayerFirewallDTypeToString(rule->layer); - DBusConnection *sysbus = virDBusGetSystemBus(); - DBusMessage *reply = NULL; - virError error; - int ret = -1; - - if (!sysbus) - return -1; - - memset(&error, 0, sizeof(error)); - - if (!ipv) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown firewall layer %d"), - rule->layer); - goto cleanup; - } - - if (virDBusCallMethod(sysbus, - &reply, - &error, - VIR_FIREWALL_FIREWALLD_SERVICE, - "/org/fedoraproject/FirewallD1", - "org.fedoraproject.FirewallD1.direct", - "passthrough", - "sa&s", - ipv, - (int)rule->argsLen, - rule->args) < 0) - goto cleanup; - - if (error.level == VIR_ERR_ERROR) { - /* - * As of firewalld-0.3.9.3-1.fc20.noarch the name and - * message fields in the error look like - * - * name="org.freedesktop.DBus.Python.dbus.exceptions.DBusException" - * message="COMMAND_FAILED: '/sbin/iptables --table filter --delete - * INPUT --in-interface virbr0 --protocol udp --destination-port 53 - * --jump ACCEPT' failed: iptables: Bad rule (does a matching rule - * exist in that chain?)." - * - * We'd like to only ignore DBus errors precisely related to the failure - * of iptables/ebtables commands. A well designed DBus interface would - * return specific named exceptions not the top level generic python dbus - * exception name. With this current scheme our only option is todo a - * sub-string match for 'COMMAND_FAILED' on the message. eg like - * - * if (ignoreErrors && - * STREQ(error.name, - * "org.freedesktop.DBus.Python.dbus.exceptions.DBusException") && - * STRPREFIX(error.message, "COMMAND_FAILED")) - * ... - * - * But this risks our error detecting code being broken if firewalld changes - * ever alter the message string, so we're avoiding doing that. - */ - if (ignoreErrors) { - VIR_DEBUG("Ignoring error '%s': '%s'", - error.str1, error.message); - } else { - virReportErrorObject(&error); - goto cleanup; - } - } else { - if (virDBusMessageRead(reply, "s", output) < 0) - goto cleanup; - } - - ret = 0; - - cleanup: - virResetError(&error); - virDBusMessageUnref(reply); - return ret; + /* wrapper necessary because virFirewallRule is a private struct */ + return virFirewallDApplyRule(rule->layer, rule->args, rule->argsLen, ignoreErrors, output); } static int diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c new file mode 100644 index 0000000000..0dc2b3de08 --- /dev/null +++ b/src/util/virfirewalld.c @@ -0,0 +1,128 @@ +/* + * virfirewalld.c: support for firewalld (https://firewalld.org) + * + * Copyright (C) 2019 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include <stdarg.h> + +#include "virfirewall.h" +#include "virfirewalld.h" +#include "virerror.h" +#include "virutil.h" +#include "virlog.h" +#include "virdbus.h" + +#define VIR_FROM_THIS VIR_FROM_FIREWALLD + +VIR_LOG_INIT("util.firewalld"); + +VIR_ENUM_DECL(virFirewallLayerFirewallD) +VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST, + "eb", "ipv4", "ipv6") + +int +virFirewallDStatus(void) +{ + return virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE); +} + + +int +virFirewallDApplyRule(virFirewallLayer layer, + char **args, size_t argsLen, + bool ignoreErrors, + char **output) +{ + const char *ipv = virFirewallLayerFirewallDTypeToString(layer); + DBusConnection *sysbus = virDBusGetSystemBus(); + DBusMessage *reply = NULL; + virError error; + int ret = -1; + + if (!sysbus) + return -1; + + memset(&error, 0, sizeof(error)); + + if (!ipv) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown firewall layer %d"), + layer); + goto cleanup; + } + + if (virDBusCallMethod(sysbus, + &reply, + &error, + VIR_FIREWALL_FIREWALLD_SERVICE, + "/org/fedoraproject/FirewallD1", + "org.fedoraproject.FirewallD1.direct", + "passthrough", + "sa&s", + ipv, + (int)argsLen, + args) < 0) + goto cleanup; + + if (error.level == VIR_ERR_ERROR) { + /* + * As of firewalld-0.3.9.3-1.fc20.noarch the name and + * message fields in the error look like + * + * name="org.freedesktop.DBus.Python.dbus.exceptions.DBusException" + * message="COMMAND_FAILED: '/sbin/iptables --table filter --delete + * INPUT --in-interface virbr0 --protocol udp --destination-port 53 + * --jump ACCEPT' failed: iptables: Bad rule (does a matching rule + * exist in that chain?)." + * + * We'd like to only ignore DBus errors precisely related to the failure + * of iptables/ebtables commands. A well designed DBus interface would + * return specific named exceptions not the top level generic python dbus + * exception name. With this current scheme our only option is todo a + * sub-string match for 'COMMAND_FAILED' on the message. eg like + * + * if (ignoreErrors && + * STREQ(error.name, + * "org.freedesktop.DBus.Python.dbus.exceptions.DBusException") && + * STRPREFIX(error.message, "COMMAND_FAILED")) + * ... + * + * But this risks our error detecting code being broken if firewalld changes + * ever alter the message string, so we're avoiding doing that. + */ + if (ignoreErrors) { + VIR_DEBUG("Ignoring error '%s': '%s'", + error.str1, error.message); + } else { + virReportErrorObject(&error); + goto cleanup; + } + } else { + if (virDBusMessageRead(reply, "s", output) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + virResetError(&error); + virDBusMessageUnref(reply); + return ret; +} diff --git a/src/util/virfirewalld.h b/src/util/virfirewalld.h new file mode 100644 index 0000000000..c1c929399a --- /dev/null +++ b/src/util/virfirewalld.h @@ -0,0 +1,33 @@ +/* + * virfirewalld.h: support for firewalld (https://firewalld.org) + * + * Copyright (C) 2019 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#ifndef LIBVIRT_VIRFIREWALLD_H +# define LIBVIRT_VIRFIREWALLD_H + +# define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1" + +int virFirewallDStatus(void); + +int virFirewallDApplyRule(virFirewallLayer layer, + char **args, size_t argsLen, + bool ignoreErrors, + char **output); + +#endif /* LIBVIRT_VIRFIREWALLD_H */ diff --git a/src/util/virfirewallpriv.h b/src/util/virfirewallpriv.h index efa94a7da4..7c31d0680d 100644 --- a/src/util/virfirewallpriv.h +++ b/src/util/virfirewallpriv.h @@ -27,8 +27,6 @@ # include "virfirewall.h" -# define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1" - typedef enum { VIR_FIREWALL_BACKEND_AUTOMATIC, VIR_FIREWALL_BACKEND_DIRECT, diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index 63b9ced820..573ab1f9cd 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -27,6 +27,7 @@ # include "vircommandpriv.h" # define LIBVIRT_VIRFIREWALLPRIV_H_ALLOW # include "virfirewallpriv.h" +# include "virfirewalld.h" # include "virmock.h" # define LIBVIRT_VIRDBUSPRIV_H_ALLOW # include "virdbuspriv.h" -- 2.20.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list