From dc8cf11686c075166a3029e974a6caeefe521d75 Mon Sep 17 00:00:00 2001

Message-Id: <dc8cf11686c075166a3029e974a6caeefe521d75@dist-git>

From: Laine Stump <laine@redhat.com>

Date: Fri, 15 Jan 2021 22:51:50 -0500

Subject: [PATCH] util: synchronize with firewalld before we start calling

 iptables directly

When it is starting up, firewalld will delete all existing iptables

rules and chains before adding its own rules. If libvirtd were to try

to directly add iptables rules during the time before firewalld has

finished initializing, firewalld would end up deleting the rules that

libvirtd has just added.

Currently this isn't a problem, since libvirtd only adds iptables

rules via the firewalld "passthrough command" API, and so firewalld is

able to properly serialize everything. However, we will soon be

changing libvirtd to add its iptables and ebtables rules by directly

calling iptables/ebtables rather than via firewalld, thus removing the

serialization of libvirtd adding rules vs. firewalld deleting rules.

This will especially apparent (if we don't fix it in advance, as this

patch does) when libvirtd is responding to the dbus NameOwnerChanged

event, which is used to learn when firewalld has been restarted. In

that case, dbus sends the event before firewalld has been able to

complete its initialization, so when libvirt responds to the event by

adding back its iptables rules (with direct calls to

/usr/bin/iptables), some of those rules are added before firewalld has

a chance to do its "remove everything" startup protocol. The usual

result of this is that libvirt will successfully add its private

chains (e.g. LIBVIRT_INP, etc), but then fail when it tries to add a

rule jumping to one of those chains (because in the interim, firewalld

has deleted the new chains).

The solution is for libvirt to preface it's direct calling to iptables

with a iptables command sent via firewalld's passthrough command

API. Since commands sent to firewalld are completed synchronously, and

since firewalld won't service them until it has completed its own

initialization, this will assure that by the time libvirt starts

calling iptables to add rules, that firewalld will not be following up

by deleting any of those rules.

To minimize the amount of extra overhead, we request the simplest

iptables command possible: "iptables -V" (and aside from logging a

debug message, we ignore the result, for good measure).

(This patch is being done *before* the patch that switches to calling

iptables directly, so that everything will function properly with any

fractional part of the series applied).

https://bugzilla.redhat.com/1607929

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

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

(cherry picked from commit 070690538a1ed301b004c542d94b13ee9bffc9d6)

Conflicts: src/util/viriptables.c:

    one line of code in context moved during g_autoptr conversion.

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

Message-Id: <20210116035151.1066734-8-laine@redhat.com>

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/libvirt_private.syms |  1 +

 src/util/virfirewall.c   | 30 ++++++++++++++++++++++++++++++

 src/util/virfirewall.h   |  2 ++

 src/util/viriptables.c   |  7 +++++++

 4 files changed, 40 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index edc53ce899..9d87e2a27b 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -2080,6 +2080,7 @@ virFileCacheSetPriv;

 # util/virfirewall.h

 virFirewallAddRuleFull;

 virFirewallApply;

+virFirewallBackendSynchronize;

 virFirewallFree;

 virFirewallNew;

 virFirewallRemoveRule;

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

index 520d515c11..66d20d3f17 100644

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

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

@@ -653,6 +653,36 @@ virFirewallApplyRuleFirewallD(virFirewallRulePtr rule,

     return virFirewallDApplyRule(rule->layer, rule->args, rule->argsLen, ignoreErrors, output);

 }

+

+void

+virFirewallBackendSynchronize(void)

+{

+    const char *arg = "-V";

+    g_autofree char *output = NULL;

+

+    switch (currentBackend) {

+    case VIR_FIREWALL_BACKEND_DIRECT:

+        /* nobody to synchronize with */

+        break;

+    case VIR_FIREWALL_BACKEND_FIREWALLD:

+        /* Send a simple rule via firewalld's passthrough iptables

+         * command so that we'll be sure firewalld has fully

+         * initialized and caught up with its internal queue of

+         * iptables commands. Waiting for this will prevent our own

+         * directly-executed iptables commands from being run while

+         * firewalld is still initializing.

+         */

+        ignore_value(virFirewallDApplyRule(VIR_FIREWALL_LAYER_IPV4,

+                                           (char **)&arg, 1, true, &output));

+        VIR_DEBUG("Result of 'iptables -V' via firewalld: %s", NULLSTR(output));

+        break;

+    case VIR_FIREWALL_BACKEND_AUTOMATIC:

+    case VIR_FIREWALL_BACKEND_LAST:

+        break;

+    }

+}

+

+

 static int

 virFirewallApplyRule(virFirewallPtr firewall,

                      virFirewallRulePtr rule,

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

index fda3cdec01..3db0864380 100644

--- a/src/util/virfirewall.h

+++ b/src/util/virfirewall.h

@@ -111,4 +111,6 @@ void virFirewallStartRollback(virFirewallPtr firewall,

 int virFirewallApply(virFirewallPtr firewall);

+void virFirewallBackendSynchronize(void);

+

 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virFirewall, virFirewallFree);

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

index 6b3a025880..41544b7f36 100644

--- a/src/util/viriptables.c

+++ b/src/util/viriptables.c

@@ -154,6 +154,13 @@ iptablesSetupPrivateChains(virFirewallLayer layer)

     fw = virFirewallNew();

+    /* When the backend is firewalld, we need to make sure that

+     * firewalld has been fully started and completed its

+     * initialization, otherwise firewalld might delete our rules soon

+     * after we add them!

+     */

+    virFirewallBackendSynchronize();

+

     virFirewallStartTransaction(fw, 0);

     for (i = 0; i < G_N_ELEMENTS(data); i++)

-- 

2.30.0