Upon VM bootstrapping (start,restore,incoming migration)
iptablesCreateBaseChainsFW is called and unconditionally deletes and
reinserts top-level firewall chain jumps (e.g. INPUT, FORWARD rules).
This briefly opens a hole in the firewall, allowing packets through
until the insertions complete.

This commit ensures that the base chains are only created once per layer
(IPV4/IPV6) and checks whether the expected rules already exist using
`iptables -C`. If they do, no delete/insert operations are performed.

This eliminates the short window where packets could bypass filters during
VM lifecycle operations.

Signed-off-by: Dion Bosschieter <dionbosschie...@gmail.com>
---
 src/nwfilter/nwfilter_ebiptables_driver.c | 79 ++++++++++++++---------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c 
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 067df6e612..42a0133159 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -131,6 +131,14 @@ static char chainprefixes_host_temp[3] = {
     0
 };
 
+typedef struct {
+    const char *chain;
+    const char *position;
+    const char *targetChain;
+} iptablesBaseChainFW;
+
+static bool baseChainFWDefined[VIR_FIREWALL_LAYER_LAST] = { false };
+
 static int
 printVar(virNWFilterVarCombIter *vars,
          char *buf, int bufsize,
@@ -403,38 +411,45 @@ static void
 iptablesCreateBaseChainsFW(virFirewall *fw,
                            virFirewallLayer layer)
 {
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-N", VIRT_IN_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-N", VIRT_OUT_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-N", VIRT_IN_POST_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-N", HOST_IN_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-D", "FORWARD", "-j", VIRT_IN_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-D", "FORWARD", "-j", VIRT_OUT_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-D", "FORWARD", "-j", VIRT_IN_POST_CHAIN, NULL);
-    virFirewallAddCmdFull(fw, layer,
-                          true, NULL, NULL,
-                          "-D", "INPUT", "-j", HOST_IN_CHAIN, NULL);
-    virFirewallAddCmd(fw, layer,
-                      "-I", "FORWARD", "1", "-j", VIRT_IN_CHAIN, NULL);
-    virFirewallAddCmd(fw, layer,
-                      "-I", "FORWARD", "2", "-j", VIRT_OUT_CHAIN, NULL);
-    virFirewallAddCmd(fw, layer,
-                      "-I", "FORWARD", "3", "-j", VIRT_IN_POST_CHAIN, NULL);
-    virFirewallAddCmd(fw, layer,
-                      "-I", "INPUT", "1", "-j", HOST_IN_CHAIN, NULL);
+    iptablesBaseChainFW fw_chains[] = {
+        {"FORWARD", "1", VIRT_IN_CHAIN},
+        {"FORWARD", "2", VIRT_OUT_CHAIN},
+        {"FORWARD", "3", VIRT_IN_POST_CHAIN},
+        {"INPUT", "1", HOST_IN_CHAIN},
+    };
+    size_t i;
+
+    // iptablesCreateBaseChainsFW already ran once for this layer,
+    // we don't have to recreate the base chains on every firewall update
+    if (baseChainFWDefined[layer])
+        return;
+
+    // set defined state so we skip the following logic next run
+    baseChainFWDefined[layer] = true;
+
+    virFirewallStartTransaction(fw, 0);
+
+    for (i = 0; i < G_N_ELEMENTS(fw_chains); i++)
+        virFirewallAddCmd(fw, layer,
+                      "-C", fw_chains[i].chain,
+                      "-j", fw_chains[i].targetChain, NULL);
+
+    if (virFirewallApply(fw) == 0)
+        // rules already in place
+        return;
+
+    for (i = 0; i < G_N_ELEMENTS(fw_chains); i++) {
+        virFirewallAddCmdFull(fw, layer,
+                              true, NULL, NULL,
+                              "-N", fw_chains[i].targetChain, NULL);
+        virFirewallAddCmdFull(fw, layer,
+                              true, NULL, NULL,
+                              "-D", fw_chains[i].chain, "-j",
+                              fw_chains[i].targetChain, NULL);
+        virFirewallAddCmd(fw, layer,
+                          "-I", fw_chains[i].chain, fw_chains[i].position,
+                          "-j", fw_chains[i].targetChain, NULL);
+    }
 }
 
 
-- 
2.39.3 (Apple Git-146)

Reply via email to