From da064c59cfc1f35f4638a9e05447c21fea88cb14 Mon Sep 17 00:00:00 2001

Message-Id: <da064c59cfc1f35f4638a9e05447c21fea88cb14@dist-git>

From: Laine Stump <laine@laine.org>

Date: Fri, 1 Feb 2019 20:29:28 -0500

Subject: [PATCH] util: move all firewalld-specific stuff into its own files

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

In preparation for adding several other firewalld-specific functions,

separate the code that's unique to firewalld from the more-generic

"firewall" file.

Signed-off-by: Laine Stump <laine@laine.org>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

(cherry picked from commit d8393b56e21708c219acc9bcd24a9c22ead4a3b4)

 Conflicts:

   src/util/virerror.c - ; added to end of MACRO in context upstream.

https://bugzilla.redhat.com/1650320

Signed-off-by: Laine Stump <laine@laine.org>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 include/libvirt/virterror.h |   1 +

 src/libvirt_private.syms    |   5 ++

 src/util/Makefile.inc.am    |   3 +

 src/util/virerror.c         |   3 +-

 src/util/virfirewall.c      |  86 +-------------------

 src/util/virfirewalld.c     | 151 ++++++++++++++++++++++++++++++++++++

 src/util/virfirewalld.h     |  33 ++++++++

 src/util/virfirewalldpriv.h |  30 +++++++

 src/util/virfirewallpriv.h  |   2 -

 tests/virfirewalltest.c     |   2 +

 10 files changed, 231 insertions(+), 85 deletions(-)

 create mode 100644 src/util/virfirewalld.c

 create mode 100644 src/util/virfirewalld.h

 create mode 100644 src/util/virfirewalldpriv.h

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h

index 57aadb8d16..30197adc3b 100644

--- a/include/libvirt/virterror.h

+++ b/include/libvirt/virterror.h

@@ -133,6 +133,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 86846f3b08..57948d8049 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -1903,6 +1903,11 @@ virFirewallStartRollback;

 virFirewallStartTransaction;

+# util/virfirewalld.h

+virFirewallDApplyRule;

+virFirewallDIsRegistered;

+

+

 # util/virfirmware.h

 virFirmwareFreeList;

 virFirmwareParse;

diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am

index a22265606c..2cef465208 100644

--- a/src/util/Makefile.inc.am

+++ b/src/util/Makefile.inc.am

@@ -57,6 +57,9 @@ UTIL_SOURCES = \

 	util/virfirewall.c \

 	util/virfirewall.h \

 	util/virfirewallpriv.h \

+	util/virfirewalld.c \

+	util/virfirewalld.h \

+	util/virfirewalldpriv.h \

 	util/virfirmware.c \

 	util/virfirmware.h \

 	util/virgettext.c \

diff --git a/src/util/virerror.c b/src/util/virerror.c

index f198f27957..1d46fcdc82 100644

--- a/src/util/virerror.c

+++ b/src/util/virerror.c

@@ -140,7 +140,8 @@ 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 10c370a2ae..4d084f6cbf 100644

--- a/src/util/virfirewall.c

+++ b/src/util/virfirewall.c

@@ -29,12 +29,12 @@

 #include "viralloc.h"

 #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"

@@ -51,11 +51,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;

@@ -158,7 +153,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 = virFirewallDIsRegistered();

         VIR_DEBUG("Firewalld is registered ? %d", rv);

         if (rv < 0) {

@@ -726,81 +721,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..f27ec9c124

--- /dev/null

+++ b/src/util/virfirewalld.c

@@ -0,0 +1,151 @@

+/*

+ * 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"

+#define LIBVIRT_VIRFIREWALLDPRIV_H_ALLOW

+#include "virfirewalldpriv.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");

+

+/* used to convert virFirewallLayer enum values to strings

+ * understood by the firewalld.direct "passthrough" method

+ */

+VIR_ENUM_DECL(virFirewallLayerFirewallD);

+VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST,

+              "eb",

+              "ipv4",

+              "ipv6",

+              );

+

+

+/**

+ * virFirewallDIsRegistered:

+ *

+ * Returns 0 if service is registered, -1 on fatal error, or -2 if service is not registered

+ */

+int

+virFirewallDIsRegistered(void)

+{

+    return virDBusIsServiceRegistered(VIR_FIREWALL_FIREWALLD_SERVICE);

+}

+

+

+/**

+ * virFirewallDApplyRule:

+ * @layer:        which layer to apply the rule to

+ * @args:         list of args to send to this layer's passthrough command.

+ * @argsLen:      number of items in @args

+ * @ignoreErrors: true to suppress logging of errors and return success

+ *                false to log errors and return actual status

+ * @output:       output of the direct passthrough command, if it was successful

+ */

+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..83fe1149cc

--- /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 virFirewallDIsRegistered(void);

+

+int virFirewallDApplyRule(virFirewallLayer layer,

+                          char **args, size_t argsLen,

+                          bool ignoreErrors,

+                          char **output);

+

+#endif /* LIBVIRT_VIRFIREWALLD_H */

diff --git a/src/util/virfirewalldpriv.h b/src/util/virfirewalldpriv.h

new file mode 100644

index 0000000000..6c03b467c9

--- /dev/null

+++ b/src/util/virfirewalldpriv.h

@@ -0,0 +1,30 @@

+/*

+ * virfirewalldpriv.h: private APIs for firewalld

+ *

+ * 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_VIRFIREWALLDPRIV_H_ALLOW

+# error "virfirewalldpriv.h may only be included by virfirewalld.c or test suites"

+#endif /* LIBVIRT_VIRFIREWALLDPRIV_H_ALLOW */

+

+#ifndef LIBVIRT_VIRFIREWALLDPRIV_H

+# define LIBVIRT_VIRFIREWALLDPRIV_H

+

+# define VIR_FIREWALL_FIREWALLD_SERVICE "org.fedoraproject.FirewallD1"

+

+#endif /* LIBVIRT_VIRFIREWALLDPRIV_H */

diff --git a/src/util/virfirewallpriv.h b/src/util/virfirewallpriv.h

index 130aaa1b73..bd6089bcc6 100644

--- a/src/util/virfirewallpriv.h

+++ b/src/util/virfirewallpriv.h

@@ -30,8 +30,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 e6d68745ae..397abfcc60 100644

--- a/tests/virfirewalltest.c

+++ b/tests/virfirewalltest.c

@@ -30,6 +30,8 @@

 # include "virbuffer.h"

 # include "vircommandpriv.h"

 # include "virfirewallpriv.h"

+# define LIBVIRT_VIRFIREWALLDPRIV_H_ALLOW

+# include "virfirewalldpriv.h"

 # include "virmock.h"

 # include "virdbuspriv.h"

-- 

2.20.1