Convert nwfilter ebtablesApplyBasicRules to virFirewall

Convert the nwfilter ebtablesApplyBasicRules method to use the
virFirewall object APIs instead of creating shell scripts
using virBuffer APIs. This provides a performance improvement
through allowing direct use of firewalld dbus APIs and will
facilitate automated testing.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 293e585c30b0b6f64c038d809b375c54044da88d..f1bb097d89e20c019e2900cbab0a71eff758079c 100644 (file)
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -2896,6 +2896,21 @@ ebtablesCreateTmpRootChain(virBufferPtr buf,
 }
 
 
+static void
+ebtablesCreateTmpRootChainFW(virFirewallPtr fw,
+                             int incoming, const char *ifname)
+{
+    char chain[MAX_CHAINNAME_LENGTH];
+    char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
+                                  : CHAINPREFIX_HOST_OUT_TEMP;
+
+    PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-N", chain, NULL);
+}
+
+
 static void
 ebtablesLinkTmpRootChain(virBufferPtr buf,
                          bool incoming, const char *ifname)
@@ -2919,6 +2934,24 @@ ebtablesLinkTmpRootChain(virBufferPtr buf,
 }
 
 
+static void
+ebtablesLinkTmpRootChainFW(virFirewallPtr fw,
+                           int incoming, const char *ifname)
+{
+    char chain[MAX_CHAINNAME_LENGTH];
+    char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP
+                                : CHAINPREFIX_HOST_OUT_TEMP;
+
+    PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
+
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A",
+                       incoming ? EBTABLES_CHAIN_INCOMING : EBTABLES_CHAIN_OUTGOING,
+                       incoming ? "-i" : "-o",
+                       ifname, "-j", chain, NULL);
+}
+
+
 static void
 _ebtablesRemoveRootChain(virBufferPtr buf,
                          bool incoming, const char *ifname,
@@ -3436,74 +3469,54 @@ static int
 ebtablesApplyBasicRules(const char *ifname,
                         const virMacAddr *macaddr)
 {
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    virFirewallPtr fw = virFirewallNew();
     char chain[MAX_CHAINNAME_LENGTH];
     char chainPrefix = CHAINPREFIX_HOST_IN_TEMP;
     char macaddr_str[VIR_MAC_STRING_BUFLEN];
 
-    if (!ebtables_cmd_path) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("cannot create rules since ebtables tool is "
-                         "missing."));
-        return -1;
-    }
-
     virMacAddrFormat(macaddr, macaddr_str);
 
-    ebiptablesAllTeardown(ifname);
+    if (ebiptablesAllTeardown(ifname) < 0)
+        goto error;
 
-    NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
+    virFirewallStartTransaction(fw, 0);
 
-    ebtablesCreateTmpRootChain(&buf, true, ifname);
+    ebtablesCreateTmpRootChainFW(fw, true, ifname);
 
     PRINT_ROOT_CHAIN(chain, chainPrefix, ifname);
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s -s ! %s -j DROP") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
-
-                      chain, macaddr_str,
-                      CMD_STOPONERR(true));
-
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s -p IPv4 -j ACCEPT") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
-
-                      chain,
-                      CMD_STOPONERR(true));
-
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s -p ARP -j ACCEPT") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
-
-                      chain,
-                      CMD_STOPONERR(true));
-
-    virBufferAsprintf(&buf,
-                      CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR
-                      CMD_EXEC
-                      "%s",
-
-                      chain,
-                      CMD_STOPONERR(true));
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain,
+                       "-s", "!", macaddr_str,
+                       "-j", "DROP", NULL);
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain,
+                       "-p", "IPv4",
+                       "-j", "ACCEPT", NULL);
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain,
+                       "-p", "ARP",
+                       "-j", "ACCEPT", NULL);
+    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
+                       "-t", "nat", "-A", chain,
+                       "-j", "DROP", NULL);
 
-    ebtablesLinkTmpRootChain(&buf, true, ifname);
-    ebtablesRenameTmpRootChain(&buf, true, ifname);
+    ebtablesLinkTmpRootChainFW(fw, true, ifname);
+    ebtablesRenameTmpRootChainFW(fw, true, ifname);
 
-    if (ebiptablesExecCLI(&buf, false, NULL) < 0)
+    virMutexLock(&execCLIMutex);
+    if (virFirewallApply(fw) < 0) {
+        virMutexUnlock(&execCLIMutex);
         goto tear_down_tmpebchains;
+    }
+    virMutexUnlock(&execCLIMutex);
 
+    virFirewallFree(fw);
     return 0;
 
  tear_down_tmpebchains:
     ebtablesCleanAll(ifname);
-
-    virReportError(VIR_ERR_BUILD_FIREWALL,
-                   "%s",
-                   _("Some rules could not be created."));
-
+ error:
+    virFirewallFree(fw);
     return -1;
 }
 

diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index 2cd93291cbf2997d63001cb47b7b1e1919cb3988..c03e79f09223099d53c3b70d023c3e88ead31491 100644 (file)
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -277,6 +277,77 @@ testNWFilterEBIPTablesTearNewRules(const void *opaque ATTRIBUTE_UNUSED)
 }
 
 
+static int
+testNWFilterEBIPTablesApplyBasicRules(const void *opaque ATTRIBUTE_UNUSED)
+{
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    const char *expected =
+        "iptables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
+        "iptables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
+        "iptables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
+        "iptables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HI-vnet0\n"
+        "iptables -D libvirt-in-post -m physdev --physdev-in vnet0 -j ACCEPT\n"
+        "iptables -F FO-vnet0\n"
+        "iptables -X FO-vnet0\n"
+        "iptables -F FI-vnet0\n"
+        "iptables -X FI-vnet0\n"
+        "iptables -F HI-vnet0\n"
+        "iptables -X HI-vnet0\n"
+        "ip6tables -D libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet0 -g FO-vnet0\n"
+        "ip6tables -D libvirt-out -m physdev --physdev-out vnet0 -g FO-vnet0\n"
+        "ip6tables -D libvirt-in -m physdev --physdev-in vnet0 -g FI-vnet0\n"
+        "ip6tables -D libvirt-host-in -m physdev --physdev-in vnet0 -g HI-vnet0\n"
+        "ip6tables -D libvirt-in-post -m physdev --physdev-in vnet0 -j ACCEPT\n"
+        "ip6tables -F FO-vnet0\n"
+        "ip6tables -X FO-vnet0\n"
+        "ip6tables -F FI-vnet0\n"
+        "ip6tables -X FI-vnet0\n"
+        "ip6tables -F HI-vnet0\n"
+        "ip6tables -X HI-vnet0\n"
+        "ebtables -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0\n"
+        "ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0\n"
+        "ebtables -t nat -L libvirt-I-vnet0\n"
+        "ebtables -t nat -L libvirt-O-vnet0\n"
+        "ebtables -t nat -F libvirt-I-vnet0\n"
+        "ebtables -t nat -X libvirt-I-vnet0\n"
+        "ebtables -t nat -F libvirt-O-vnet0\n"
+        "ebtables -t nat -X libvirt-O-vnet0\n"
+        "ebtables -t nat -N libvirt-J-vnet0\n"
+        "ebtables -t nat -A libvirt-J-vnet0 -s '!' 10:20:30:40:50:60 -j DROP\n"
+        "ebtables -t nat -A libvirt-J-vnet0 -p IPv4 -j ACCEPT\n"
+        "ebtables -t nat -A libvirt-J-vnet0 -p ARP -j ACCEPT\n"
+        "ebtables -t nat -A libvirt-J-vnet0 -j DROP\n"
+        "ebtables -t nat -A PREROUTING -i vnet0 -j libvirt-J-vnet0\n"
+        "ebtables -t nat -E libvirt-J-vnet0 libvirt-I-vnet0\n";
+    char *actual = NULL;
+    int ret = -1;
+    virMacAddr mac = { .addr = { 0x10, 0x20, 0x30, 0x40, 0x50, 0x60 } };
+
+    virCommandSetDryRun(&buf, NULL, NULL);
+
+    if (ebiptables_driver.applyBasicRules("vnet0", &mac) < 0)
+        goto cleanup;
+
+    if (virBufferError(&buf))
+        goto cleanup;
+
+    actual = virBufferContentAndReset(&buf);
+    virtTestClearCommandPath(actual);
+
+    if (STRNEQ_NULLABLE(actual, expected)) {
+        virtTestDifference(stderr, actual, expected);
+        goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virCommandSetDryRun(NULL, NULL, NULL);
+    virBufferFreeAndReset(&buf);
+    VIR_FREE(actual);
+    return ret;
+}
+
+
 static int
 mymain(void)
 {
@@ -307,6 +378,11 @@ mymain(void)
                     NULL) < 0)
         ret = -1;
 
+    if (virtTestRun("ebiptablesApplyBasicRules",
+                    testNWFilterEBIPTablesApplyBasicRules,
+                    NULL) < 0)
+        ret = -1;
+
  cleanup:
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }