[PATCH 7/8] util: synchronize with firewalld before we start calling iptables directly

[Daniel Henrique Barboza]

On 11/24/20 12:30 AM, Laine Stump wrote:
> 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 cmmands sent to firewalld are completed synchronously, and

s/cmmands/commands

> 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).
> 
> Signed-off-by: Laine Stump <laine at 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 5684cd3316..2cd046df8b 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2148,6 +2148,7 @@ virFileCacheSetPriv;
>   # util/virfirewall.h
>   virFirewallAddRuleFull;
>   virFirewallApply;
> +virFirewallBackendSynchronize;
>   virFirewallFree;
>   virFirewallNew;
>   virFirewallRemoveRule;
> diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
> index 0b022b14af..307825331d 100644
> --- a/src/util/virfirewall.c
> +++ b/src/util/virfirewall.c
> @@ -649,6 +649,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 9cfbc9f2aa..5fbb77fd5b 100644
> --- a/src/util/viriptables.c
> +++ b/src/util/viriptables.c
> @@ -150,6 +150,13 @@ iptablesSetupPrivateChains(virFirewallLayer layer)
>       };
>       size_t i;
>   
> +    /* 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++)
>