From d01eb153035a8ad8d4317552e195782b74f5921d Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Fri, 14 Mar 2014 12:58:18 +0000 Subject: [PATCH] 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 --- src/nwfilter/nwfilter_ebiptables_driver.c | 113 ++++++++++++---------- tests/nwfilterebiptablestest.c | 76 +++++++++++++++ 2 files changed, 139 insertions(+), 50 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 293e585c30..f1bb097d89 100644 --- 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", + 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); - chain, macaddr_str, - CMD_STOPONERR(true)); + ebtablesLinkTmpRootChainFW(fw, true, ifname); + ebtablesRenameTmpRootChainFW(fw, true, ifname); - 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)); - - ebtablesLinkTmpRootChain(&buf, true, ifname); - ebtablesRenameTmpRootChain(&buf, 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 2cd93291cb..c03e79f092 100644 --- 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; }