[libvirt] [PATCH V6 00/11] Make inner workings of nwfilters more flexible + extensions
The following series of patches re-does some of the inner workings of nwfilters with the goal to enable users to write filters that have other than the system-known chains supported right now ('root','arp','rarp','ipv4' and 'ipv6'). Ideally users should be able to provide a chain name in the chains XML attribute and either be able to jump to it as an 'action' or have the chain created automatically as it is the case right now for those chains enumerated before. The latter is now added in this patch series as well. I am first introducing internal priorities for the chains mentioned above so that their creation can be made more flexible -- currently their creation and the order in which they are accessed is hardcoded. This largely does away with the hardcoded stuff. All assigned priorities have negative values. Later on the priorities for the chains are made accessible via an XML attribute. Further, filters will be automatically accessed from the (ebtables) interface 'root' chain using the prefix of the name of the chain. As an example, the following filter will be accessed from the root chain for 'arp' packets since its name 'arp-xyz' has the prefix 'arp'. <filter name='test-arp-xyz' chain='arp-xyz' priority='-650'> <uuid>94abeecc-c956-0ac8-1f49-a06ee8995688</uuid> <rule action='accept' direction='out' priority='100'> <arp opcode='Request_Reverse' arpsrcmacaddr='$MAC' arpdstmacaddr='$MAC' arpsrcipaddr='0.0.0.0' arpdstipaddr='0.0.0.0'/> </rule> <rule action='accept' direction='inout' priority='500'/> </filter> In another step the priorities of rules is extended to also allow negativ values. This then allows the creation of rules and chains in the interface 'root' chain to be mixed so that the following layout becomes possible: Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT -p IPv4 -j I-vnet0-ipv4 -p ARP -j I-vnet0-arp -p ARP -j ACCEPT -p 0x8035 -j I-vnet0-rarp -p 0x835 -j ACCEPT -j DROP In the above list of rules the '-p ARP -j ACCEPT' can now be found in between the 'jumps' to protocol-specific chains, which allows for more efficient rule evaluation. I did testing with the test cases in libvirt-tck as well as those in the tests/ directory and did not see any regressions. v6: - addresses Eric Blake's comments on patches 2, 6, 7, 9 + added documentation patch (11) v5: - addressed Eric Blake's comments v4: - assign priority to chain according to name prefix - change default (internal) priorities of chains (by +200) - fix memory leak Regards, Stefan
Add a function to the virHashTable for getting an array of the hash table's key-value pairs and have the keys (optionally) sorted. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v5: - follwed Eric Blake's suggestions: - added better description to new function - return array of key-value pairs rather than only keys --- src/libvirt_private.syms | 2 ++ src/util/hash.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/hash.h | 22 ++++++++++++++++++++++ 3 files changed, 69 insertions(+) Index: libvirt-acl/src/util/hash.c =================================================================== --- libvirt-acl.orig/src/util/hash.c +++ libvirt-acl/src/util/hash.c @@ -618,3 +618,48 @@ void *virHashSearch(virHashTablePtr tabl return NULL; } + +struct getKeysIter +{ + virHashKeyValuePair *sortArray; + unsigned arrayIdx; +}; + +static void virHashGetKeysIterator(void *payload, + const void *key, void *data) +{ + struct getKeysIter *iter = data; + + iter->sortArray[iter->arrayIdx].key = key; + iter->sortArray[iter->arrayIdx].value = payload; + + iter->arrayIdx++; +} + +typedef int (*qsort_comp)(const void *, const void *); + +virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table, + virHashKeyComparator compar) +{ + int numElems = virHashSize(table); + struct getKeysIter iter = { + .arrayIdx = 0, + .sortArray = NULL, + }; + + if (numElems < 0) + return NULL; + + if (VIR_ALLOC_N(iter.sortArray, numElems + 1)) { + virReportOOMError(); + return NULL; + } + + virHashForEach(table, virHashGetKeysIterator, &iter); + + if (compar) + qsort(&iter.sortArray[0], numElems, sizeof(iter.sortArray[0]), + (qsort_comp)compar); + + return iter.sortArray; +} Index: libvirt-acl/src/util/hash.h =================================================================== --- libvirt-acl.orig/src/util/hash.h +++ libvirt-acl/src/util/hash.h @@ -130,6 +130,28 @@ void *virHashLookup(virHashTablePtr tabl */ void *virHashSteal(virHashTablePtr table, const void *name); +/* + * Get the hash table's key/value pairs and have them optionally sorted. + * The returned array contains virHashSize() elements. Additionally, + * an empty element has been added to the end of the array (with key == NULL) + * to indicate the end of the array. + * The key/value pairs are only valid as long as the underlying hash + * table is not modified, i.e., no keys are removed or inserted, and + * the hash table is not deleted. + * The caller must only free the returned array using VIR_FREE(). + * The caller must make copies of all returned keys and values if they are + * to be used somewhere else. + */ +typedef struct _virHashKeyValuePair virHashKeyValuePair; +typedef virHashKeyValuePair *virHashKeyValuePairPtr; +struct _virHashKeyValuePair { + const void *key; + const void *value; +}; +typedef int (*virHashKeyComparator)(const virHashKeyValuePairPtr, + const virHashKeyValuePairPtr); +virHashKeyValuePairPtr virHashGetItems(virHashTablePtr table, + virHashKeyComparator compar); /* * Iterators Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -559,6 +559,7 @@ virHashAddEntry; virHashCreate; virHashForEach; virHashFree; +virHashGetItems; virHashLookup; virHashRemoveEntry; virHashRemoveSet; @@ -566,6 +567,7 @@ virHashSearch; virHashSize; virHashSteal; virHashTableSize; +virHashUpdateEntry; # hooks.h
For better handling of the sorting of chains introduce an internally used priority. Use a lookup table to store the priorities. For now their actual values do not matter just that the values cause the chains to be properly sorted through changes in the following patches. However, the values are chosen as negative so that once they are sorted along with filtering rules (whose priority may only be positive for now) they will always be instantiated before them (lower values cause instantiation before higher values). This is done to maintain backwards compatibility. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v3: - increased filter priorities to have more room before them --- src/conf/nwfilter_conf.c | 14 ++++++++++++++ src/conf/nwfilter_conf.h | 12 ++++++++++++ src/nwfilter/nwfilter_ebiptables_driver.c | 4 ++++ src/nwfilter/nwfilter_ebiptables_driver.h | 1 + 4 files changed, 31 insertions(+) Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -357,8 +357,18 @@ enum virNWFilterEbtablesTableType { }; +# define MIN_RULE_PRIORITY 0 # define MAX_RULE_PRIORITY 1000 +# define NWFILTER_MIN_FILTER_PRIORITY -1000 +# define NWFILTER_MAX_FILTER_PRIORITY MAX_RULE_PRIORITY + +# define NWFILTER_ROOT_FILTER_PRI 0 +# define NWFILTER_IPV4_FILTER_PRI -700 +# define NWFILTER_IPV6_FILTER_PRI -600 +# define NWFILTER_ARP_FILTER_PRI -500 +# define NWFILTER_RARP_FILTER_PRI -400 + enum virNWFilterRuleFlags { RULE_FLAG_NO_STATEMATCH = (1 << 0), RULE_FLAG_STATE_NEW = (1 << 1), @@ -436,6 +446,7 @@ enum virNWFilterChainSuffixType { VIR_NWFILTER_CHAINSUFFIX_LAST, }; +typedef int32_t virNWFilterChainPriority; typedef struct _virNWFilterDef virNWFilterDef; typedef virNWFilterDef *virNWFilterDefPtr; @@ -445,6 +456,7 @@ struct _virNWFilterDef { unsigned char uuid[VIR_UUID_BUFLEN]; int chainsuffix; /*enum virNWFilterChainSuffixType */ + virNWFilterChainPriority chainPriority; int nentries; virNWFilterEntryPtr *filterEntries; Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -124,6 +124,14 @@ struct int_map { #define INTMAP_ENTRY(ATT, VAL) { .attr = ATT, .val = VAL } #define INTMAP_ENTRY_LAST { .val = NULL } +static const struct int_map chain_priorities[] = { + INTMAP_ENTRY(NWFILTER_ROOT_FILTER_PRI, "root"), + INTMAP_ENTRY(NWFILTER_IPV4_FILTER_PRI, "ipv4"), + INTMAP_ENTRY(NWFILTER_IPV6_FILTER_PRI, "ipv6"), + INTMAP_ENTRY(NWFILTER_ARP_FILTER_PRI , "arp" ), + INTMAP_ENTRY(NWFILTER_RARP_FILTER_PRI, "rarp"), + INTMAP_ENTRY_LAST, +}; /* * only one filter update allowed @@ -2028,6 +2036,12 @@ virNWFilterDefParseXML(xmlXPathContextPt _("unknown chain suffix '%s'"), chain); goto cleanup; } + /* assign an implicit priority -- support XML attribute later */ + if (!intMapGetByString(chain_priorities, chain, 0, + &ret->chainPriority)) { + ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + + NWFILTER_MIN_FILTER_PRIORITY) / 2; + } } uuid = virXPathString("string(./uuid)", ctxt); Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -36,6 +36,7 @@ typedef ebiptablesRuleInst *ebiptablesRu struct _ebiptablesRuleInst { char *commandTemplate; enum virNWFilterChainSuffixType neededProtocolChain; + virNWFilterChainPriority chainPriority; char chainprefix; /* I for incoming, O for outgoing */ unsigned int priority; enum RuleType ruleType; Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -328,6 +328,7 @@ static int ebiptablesAddRuleInst(virNWFilterRuleInstPtr res, char *commandTemplate, enum virNWFilterChainSuffixType neededChain, + virNWFilterChainPriority chainPriority, char chainprefix, unsigned int priority, enum RuleType ruleType) @@ -341,6 +342,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns inst->commandTemplate = commandTemplate; inst->neededProtocolChain = neededChain; + inst->chainPriority = chainPriority; inst->chainprefix = chainprefix; inst->priority = priority; inst->ruleType = ruleType; @@ -1589,6 +1591,7 @@ _iptablesCreateRuleInstance(int directio return ebiptablesAddRuleInst(res, virBufferContentAndReset(final), nwfilter->chainsuffix, + nwfilter->chainPriority, '\0', rule->priority, (isIPv6) ? RT_IP6TABLES : RT_IPTABLES); @@ -2338,6 +2341,7 @@ ebtablesCreateRuleInstance(char chainPre return ebiptablesAddRuleInst(res, virBufferContentAndReset(&buf), nwfilter->chainsuffix, + nwfilter->chainPriority, chainPrefix, rule->priority, RT_EBTABLES);
Use the previously introduced chain priorities to sort the chains for access from an interface's 'root' table and have them created in the proper order. This gets rid of a lot of code that was previously creating the chains in a more hardcoded way. To determine what protocol a filter is used for evaluation do prefix- matching, i.e., the filter 'arp' is used to filter for the 'arp' protocol, 'ipv4' for the 'ipv4' protocol and 'arp-xyz' will also be used to filter for the 'arp' protocol following the prefix 'arp' in its name. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v5: - followed Eric Blake's suggestions: - return -1 as error code - add comments as appropriate - other style fixes --- src/nwfilter/nwfilter_ebiptables_driver.c | 134 ++++++++++++++++++++++-------- 1 file changed, 102 insertions(+), 32 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -140,6 +140,11 @@ enum l3_proto_idx { #define USHORTMAP_ENTRY_IDX(IDX, ATT, VAL) [IDX] = { .attr = ATT, .val = VAL } +/* A lookup table for translating ethernet protocol IDs to human readable + * strings. None of the human readable strings must be found as a prefix + * in another entry here (example 'ab' would be found in 'abc') to allow + * for prefix matching. + */ static const struct ushort_map l3_protocols[] = { USHORTMAP_ENTRY_IDX(L3_PROTO_IPV4_IDX, ETHERTYPE_IP , "ipv4"), USHORTMAP_ENTRY_IDX(L3_PROTO_IPV6_IDX, ETHERTYPE_IPV6 , "ipv6"), @@ -2662,6 +2667,7 @@ ebtablesCreateTmpSubChain(virBufferPtr b int incoming, const char *ifname, enum l3_proto_idx protoidx, + const char *filtername, int stopOnError) { char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; @@ -2669,7 +2675,8 @@ ebtablesCreateTmpSubChain(virBufferPtr b : CHAINPREFIX_HOST_OUT_TEMP; PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname); - PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val); + PRINT_CHAIN(chain, chainPrefix, ifname, + (filtername) ? filtername : l3_protocols[protoidx].val); virBufferAsprintf(buf, CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR @@ -3176,6 +3183,14 @@ ebiptablesRuleOrderSort(const void *a, c return ((*insta)->priority - (*instb)->priority); } +static int +ebiptablesFilterOrderSort(const virHashKeyValuePairPtr a, + const virHashKeyValuePairPtr b) +{ + /* elements' values has been limited to range [-1000, 1000] */ + return *(virNWFilterChainPriority *)a->value - + *(virNWFilterChainPriority *)b->value; +} static void iptablesCheckBridgeNFCallEnabled(bool isIPv6) @@ -3215,6 +3230,54 @@ iptablesCheckBridgeNFCallEnabled(bool is } } +/* + * Given a filtername determine the protocol it is used for evaluating + * We do prefix-matching to determine the protocol. + */ +static enum l3_proto_idx +ebtablesGetProtoIdxByFiltername(const char *filtername) +{ + enum l3_proto_idx idx; + + for (idx = 0; idx < L3_PROTO_LAST_IDX; idx++) { + if (STRPREFIX(filtername, l3_protocols[idx].val)) { + return idx; + } + } + + return -1; +} + +static int +ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, + const char *ifname, + virHashTablePtr chains, int direction) +{ + int rc = 0, i; + virHashKeyValuePairPtr filter_names; + + if (ebtablesCreateTmpRootChain(buf, direction, ifname, 1) < 0) + return -1; + + filter_names = virHashGetItems(chains, + ebiptablesFilterOrderSort); + if (filter_names == NULL) + return -1; + + for (i = 0; filter_names[i].key; i++) { + enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername( + filter_names[i].key); + if ((int)idx < 0) + continue; + rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx, + filter_names[i].key, 1); + if (rc < 0) + break; + } + + VIR_FREE(filter_names); + return rc; +} static int ebiptablesApplyNewRules(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -3225,24 +3288,43 @@ ebiptablesApplyNewRules(virConnectPtr co int i; int cli_status; ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst; - int chains_in = 0, chains_out = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; + virHashTablePtr chains_in_set = virHashCreate(10, NULL); + virHashTablePtr chains_out_set = virHashCreate(10, NULL); bool haveIptables = false; bool haveIp6tables = false; + if (!chains_in_set || !chains_out_set) { + virReportOOMError(); + goto exit_free_sets; + } + if (nruleInstances > 1 && inst) qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort); + /* scan the rules to see which chains need to be created */ for (i = 0; i < nruleInstances; i++) { sa_assert (inst); if (inst[i]->ruleType == RT_EBTABLES) { - if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) - chains_in |= (1 << inst[i]->neededProtocolChain); - else - chains_out |= (1 << inst[i]->neededProtocolChain); + const char *name = virNWFilterChainSuffixTypeToString( + inst[i]->neededProtocolChain); + if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) { + if (virHashUpdateEntry(chains_in_set, name, + &inst[i]->chainPriority)) { + virReportOOMError(); + goto exit_free_sets; + } + } else { + if (virHashUpdateEntry(chains_out_set, name, + &inst[i]->chainPriority)) { + virReportOOMError(); + goto exit_free_sets; + } + } } } + /* cleanup whatever may exist */ if (ebtables_cmd_path) { ebtablesUnlinkTmpRootChain(&buf, 1, ifname); ebtablesUnlinkTmpRootChain(&buf, 0, ifname); @@ -3252,30 +3334,11 @@ ebiptablesApplyNewRules(virConnectPtr co ebiptablesExecCLI(&buf, &cli_status); } - if (chains_in != 0) - ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); - if (chains_out != 0) - ebtablesCreateTmpRootChain(&buf, 0, ifname, 1); - - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV4_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv4)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV4_IDX, 1); - - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv6)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_IPV6_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_IPv6)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_IPV6_IDX, 1); - - /* keep arp,rarp as last */ - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_ARP_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_ARP)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_ARP_IDX, 1); - if (chains_in & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP)) - ebtablesCreateTmpSubChain(&buf, 1, ifname, L3_PROTO_RARP_IDX, 1); - if (chains_out & (1 << VIR_NWFILTER_CHAINSUFFIX_RARP)) - ebtablesCreateTmpSubChain(&buf, 0, ifname, L3_PROTO_RARP_IDX, 1); + /* create needed chains */ + if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1) || + ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0)) { + goto tear_down_tmpebchains; + } if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) goto tear_down_tmpebchains; @@ -3365,14 +3428,17 @@ ebiptablesApplyNewRules(virConnectPtr co iptablesCheckBridgeNFCallEnabled(true); } - if (chains_in != 0) + if (virHashSize(chains_in_set) != 0) ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); - if (chains_out != 0) + if (virHashSize(chains_out_set) != 0) ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) goto tear_down_ebsubchains_and_unlink; + virHashFree(chains_in_set); + virHashFree(chains_out_set); + return 0; tear_down_ebsubchains_and_unlink: @@ -3407,6 +3473,10 @@ tear_down_tmpebchains: "interface %s."), ifname); +exit_free_sets: + virHashFree(chains_in_set); + virHashFree(chains_out_set); + return 1; }
Use scripts for the renaming and cleaning up of chains. This allows us to get rid of some of the code that is only capable of renaming and removing chains whose names are hardcoded. A shell function 'collect_chains' is introduced that is given the name of an ebtables chain and then recursively determines the names of all chains that are accessed from this chain and its sub-chains using 'jumps'. The resulting list of chain names is then used to delete all the found chains by first flushing and then deleting them. The same function is also used for renaming temporary filters to their final names. I tested this with the bash and dash as script interpreters. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v5: - followng Eric Blake's suggestion on a major overhaul of the embedded scripts v2: - setting IFS to intended value; works with bash and dash (phew!) --- src/nwfilter/nwfilter_ebiptables_driver.c | 217 +++++++++++++++++------------- 1 file changed, 129 insertions(+), 88 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -92,6 +92,59 @@ static char *gawk_cmd_path; #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ snprintf(buf, sizeof(buf), "%c-%s-%s", prefix, ifname, suffix) +/* The collect_chains() script recursively determines all names + * of ebtables (nat) chains that are 'children' of a given 'root' chain. + * The typical output of an ebtables call is as follows: + * + * #> ebtables -t nat -L libvirt-I-tck-test205002 + * Bridge table: nat + * + * Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT + * -p IPv4 -j I-tck-test205002-ipv4 + * -p ARP -j I-tck-test205002-arp + * -p 0x8035 -j I-tck-test205002-rarp + * -p 0x835 -j ACCEPT + * -j DROP + */ +static const char ebtables_script_func_collect_chains[] = + "collect_chains()\n" + "{\n" + " for tmp2 in $*; do\n" + " for tmp in $(%s -t %s -L $tmp2 | \\\n" + " sed -n \"/Bridge chain/,\\$ s/.*-j \\\\([%s]-.*\\\\)/\\\\1/p\");\n" + " do\n" + " echo $tmp\n" + " collect_chains $tmp\n" + " done\n" + " done\n" + "}\n"; + +static const char ebiptables_script_func_rm_chains[] = + "rm_chains()\n" + "{\n" + " for tmp in $*; do %s -t %s -F $tmp; done\n" + " for tmp in $*; do %s -t %s -X $tmp; done\n" + "}\n"; + +static const char ebiptables_script_func_rename_chains[] = + "rename_chains()\n" + "{\n" + " for tmp in $*; do\n" + " case $tmp in\n" + " %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n" + " %c*) %s -t %s -E $tmp %c${tmp#?} ;;\n" + " esac\n" + " done\n" + "}\n"; + +static const char ebiptables_script_set_ifs[] = + "tmp='\n'\n" + "IFS=' ''\t'$tmp\n"; + +#define NWFILTER_FUNC_COLLECT_CHAINS ebtables_script_func_collect_chains +#define NWFILTER_FUNC_RM_CHAINS ebiptables_script_func_rm_chains +#define NWFILTER_FUNC_RENAME_CHAINS ebiptables_script_func_rename_chains +#define NWFILTER_FUNC_SET_IFS ebiptables_script_set_ifs #define VIRT_IN_CHAIN "libvirt-in" #define VIRT_OUT_CHAIN "libvirt-out" @@ -2698,95 +2751,66 @@ ebtablesCreateTmpSubChain(virBufferPtr b return 0; } - static int -_ebtablesRemoveSubChain(virBufferPtr buf, - int incoming, - const char *ifname, - enum l3_proto_idx protoidx, - int isTempChain) -{ - char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; - char chainPrefix; - - if (isTempChain) { - chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; - } else { - chainPrefix =(incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; - } - - PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname); - PRINT_CHAIN(chain, chainPrefix, ifname, l3_protocols[protoidx].val); - - virBufferAsprintf(buf, - "%s -t %s -D %s -p 0x%x -j %s" CMD_SEPARATOR - "%s -t %s -F %s" CMD_SEPARATOR - "%s -t %s -X %s" CMD_SEPARATOR, +_ebtablesRemoveSubChains(virBufferPtr buf, + const char *ifname, + const char *chains) +{ + char rootchain[MAX_CHAINNAME_LENGTH]; + unsigned i; + + virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains); + virBufferAsprintf(buf, NWFILTER_FUNC_RM_CHAINS, ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, - rootchain, l3_protocols[protoidx].attr, chain, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE); - ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain, + virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS); + virBufferAddLit(buf, "a=\"$(collect_chains "); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, "%s ", rootchain); + } + virBufferAddLit(buf, ")\"\n"); - ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, + "%s -t %s -F %s\n", + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, + rootchain); + } + virBufferAddLit(buf, "rm_chains $a\n"); return 0; } - -static int -ebtablesRemoveSubChain(virBufferPtr buf, - int incoming, - const char *ifname, - enum l3_proto_idx protoidx) -{ - return _ebtablesRemoveSubChain(buf, - incoming, ifname, protoidx, 0); -} - - static int ebtablesRemoveSubChains(virBufferPtr buf, const char *ifname) { - enum l3_proto_idx i; - - for (i = 0; i < L3_PROTO_LAST_IDX; i++) { - ebtablesRemoveSubChain(buf, 1, ifname, i); - ebtablesRemoveSubChain(buf, 0, ifname, i); - } - - return 0; -} - + char chains[3] = { + CHAINPREFIX_HOST_IN, + CHAINPREFIX_HOST_OUT, + 0 + }; -static int -ebtablesRemoveTmpSubChain(virBufferPtr buf, - int incoming, - const char *ifname, - enum l3_proto_idx protoidx) -{ - return _ebtablesRemoveSubChain(buf, - incoming, ifname, protoidx, 1); + return _ebtablesRemoveSubChains(buf, ifname, chains); } - static int ebtablesRemoveTmpSubChains(virBufferPtr buf, const char *ifname) { - enum l3_proto_idx i; - - for (i = 0; i < L3_PROTO_LAST_IDX; i++) { - ebtablesRemoveTmpSubChain(buf, 1, ifname, i); - ebtablesRemoveTmpSubChain(buf, 0, ifname, i); - } + char chains[3] = { + CHAINPREFIX_HOST_IN_TEMP, + CHAINPREFIX_HOST_OUT_TEMP, + 0 + }; - return 0; + return _ebtablesRemoveSubChains(buf, ifname, chains); } - static int ebtablesRenameTmpSubChain(virBufferPtr buf, int incoming, @@ -2813,31 +2837,51 @@ ebtablesRenameTmpSubChain(virBufferPtr b return 0; } - static int -ebtablesRenameTmpSubChains(virBufferPtr buf, +ebtablesRenameTmpRootChain(virBufferPtr buf, + int incoming, const char *ifname) { - enum l3_proto_idx i; + return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); +} + +static int +ebtablesRenameTmpSubAndRootChains(virBufferPtr buf, + const char *ifname) +{ + char rootchain[MAX_CHAINNAME_LENGTH]; + unsigned i; + char chains[3] = { + CHAINPREFIX_HOST_IN_TEMP, + CHAINPREFIX_HOST_OUT_TEMP, + 0}; + + virBufferAsprintf(buf, NWFILTER_FUNC_COLLECT_CHAINS, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chains); + virBufferAsprintf(buf, NWFILTER_FUNC_RENAME_CHAINS, + CHAINPREFIX_HOST_IN_TEMP, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, + CHAINPREFIX_HOST_IN, + CHAINPREFIX_HOST_OUT_TEMP, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, + CHAINPREFIX_HOST_OUT); - for (i = 0; i < L3_PROTO_LAST_IDX; i++) { - ebtablesRenameTmpSubChain (buf, 1, ifname, l3_protocols[i].val); - ebtablesRenameTmpSubChain (buf, 0, ifname, l3_protocols[i].val); + virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS); + virBufferAddLit(buf, "a=\"$(collect_chains "); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, "%s ", rootchain); } + virBufferAddLit(buf, ")\"\n"); - return 0; -} + virBufferAddLit(buf, "rename_chains $a\n"); + ebtablesRenameTmpRootChain(buf, 1, ifname); + ebtablesRenameTmpRootChain(buf, 0, ifname); -static int -ebtablesRenameTmpRootChain(virBufferPtr buf, - int incoming, - const char *ifname) -{ - return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); + return 0; } - static void ebiptablesInstCommand(virBufferPtr buf, const char *templ, char cmd, int pos, @@ -3546,9 +3590,7 @@ ebiptablesTearOldRules(virConnectPtr con ebtablesRemoveRootChain(&buf, 1, ifname); ebtablesRemoveRootChain(&buf, 0, ifname); - ebtablesRenameTmpSubChains(&buf, ifname); - ebtablesRenameTmpRootChain(&buf, 1, ifname); - ebtablesRenameTmpRootChain(&buf, 0, ifname); + ebtablesRenameTmpSubAndRootChains(&buf, ifname); ebiptablesExecCLI(&buf, &cli_status); } @@ -3633,12 +3675,11 @@ ebiptablesAllTeardown(const char *ifname ebtablesUnlinkRootChain(&buf, 1, ifname); ebtablesUnlinkRootChain(&buf, 0, ifname); + ebtablesRemoveSubChains(&buf, ifname); + ebtablesRemoveRootChain(&buf, 1, ifname); ebtablesRemoveRootChain(&buf, 0, ifname); - - ebtablesRemoveSubChains(&buf, ifname); } - ebiptablesExecCLI(&buf, &cli_status); return 0;
On 11/18/2011 06:32 AM, Stefan Berger wrote:
Use scripts for the renaming and cleaning up of chains. This allows us to get rid of some of the code that is only capable of renaming and removing chains whose names are hardcoded.
+static const char ebtables_script_func_collect_chains[] = + "collect_chains()\n" + "{\n" + " for tmp2 in $*; do\n" + " for tmp in $(%s -t %s -L $tmp2 | \\\n" + " sed -n \"/Bridge chain/,\\$ s/.*-j \\\\([%s]-.*\\\\)/\\\\1/p\");\n"
No need to change this unless you want, but this could be written in fewer bytes as: " sed -n '/Bridge chain/,$ s/.*-j \\([%s]-.*\\)/\\1/p')\n"
+ virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS); + virBufferAddLit(buf, "a=\"$(collect_chains "); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, "%s ", rootchain); + } + virBufferAddLit(buf, ")\"\n");
As written, you generate: a="$(collect_chains a b )" with an odd space before ). I also think the name $a is rather terse; it might be better to use something a bit less cryptic. What you have works, but you could also make this tweak if you'd like: virBufferAddLit(buf, "chains=\"$(collect_chains"); for ... virBufferAsprintf(buf, " %s", rootchain); virBufferAddLit(buf, ")\"\n"); ... virBufferAddLit(buf, "rm_chains $chains\n"); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/18/2011 11:01 AM, Eric Blake wrote:
On 11/18/2011 06:32 AM, Stefan Berger wrote:
Use scripts for the renaming and cleaning up of chains. This allows us to get rid of some of the code that is only capable of renaming and removing chains whose names are hardcoded.
+static const char ebtables_script_func_collect_chains[] = + "collect_chains()\n" + "{\n" + " for tmp2 in $*; do\n" + " for tmp in $(%s -t %s -L $tmp2 | \\\n" + " sed -n \"/Bridge chain/,\\$ s/.*-j \\\\([%s]-.*\\\\)/\\\\1/p\");\n" No need to change this unless you want, but this could be written in fewer bytes as:
" sed -n '/Bridge chain/,$ s/.*-j \\([%s]-.*\\)/\\1/p')\n"
+ virBufferAsprintf(buf, NWFILTER_FUNC_SET_IFS); + virBufferAddLit(buf, "a=\"$(collect_chains "); + for (i = 0; chains[i] != 0; i++) { + PRINT_ROOT_CHAIN(rootchain, chains[i], ifname); + virBufferAsprintf(buf, "%s ", rootchain); + } + virBufferAddLit(buf, ")\"\n"); As written, you generate: a="$(collect_chains a b )" with an odd space before ). I also think the name $a is rather terse; it might be better to use something a bit less cryptic. What you have works, but you could also make this tweak if you'd like:
virBufferAddLit(buf, "chains=\"$(collect_chains"); for ... virBufferAsprintf(buf, " %s", rootchain); virBufferAddLit(buf, ")\"\n"); ... virBufferAddLit(buf, "rm_chains $chains\n");
Fixed this part.
Use the name of the chain rather than its type index (enum). This pushes the later enablement of chains with user-given names into the XML parser. For now we still only allow those names that are well known ('root', 'arp', 'rarp', 'ipv4' and 'ipv6'). Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/conf/nwfilter_conf.c | 16 ++++++++++++---- src/conf/nwfilter_conf.h | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 13 +++++++------ src/nwfilter/nwfilter_ebiptables_driver.h | 2 +- 4 files changed, 21 insertions(+), 12 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -309,6 +309,7 @@ virNWFilterDefFree(virNWFilterDefPtr def virNWFilterEntryFree(def->filterEntries[i]); VIR_FREE(def->filterEntries); + VIR_FREE(def->chainsuffix); VIR_FREE(def); } @@ -2027,21 +2028,28 @@ virNWFilterDefParseXML(xmlXPathContextPt goto cleanup; } - ret->chainsuffix = VIR_NWFILTER_CHAINSUFFIX_ROOT; chain = virXPathString("string(./@chain)", ctxt); if (chain) { - if ((ret->chainsuffix = - virNWFilterChainSuffixTypeFromString(chain)) < 0) { + if (virNWFilterChainSuffixTypeFromString(chain) < 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("unknown chain suffix '%s'"), chain); goto cleanup; } + ret->chainsuffix = chain; /* assign an implicit priority -- support XML attribute later */ if (!intMapGetByString(chain_priorities, chain, 0, &ret->chainPriority)) { ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + NWFILTER_MIN_FILTER_PRIORITY) / 2; } + chain = NULL; + } else { + ret->chainsuffix = strdup(virNWFilterChainSuffixTypeToString( + VIR_NWFILTER_CHAINSUFFIX_ROOT)); + if (ret->chainsuffix == NULL) { + virReportOOMError(); + goto cleanup; + } } uuid = virXPathString("string(./uuid)", ctxt); @@ -2843,7 +2851,7 @@ virNWFilterDefFormat(virNWFilterDefPtr d virBufferAsprintf(&buf, "<filter name='%s' chain='%s'", def->name, - virNWFilterChainSuffixTypeToString(def->chainsuffix)); + def->chainsuffix); virBufferAddLit(&buf, ">\n"); virUUIDFormat(def->uuid, uuid); Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -455,7 +455,7 @@ struct _virNWFilterDef { char *name; unsigned char uuid[VIR_UUID_BUFLEN]; - int chainsuffix; /*enum virNWFilterChainSuffixType */ + char *chainsuffix; virNWFilterChainPriority chainPriority; int nentries; Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -385,7 +385,7 @@ ebiptablesRuleInstFree(ebiptablesRuleIns static int ebiptablesAddRuleInst(virNWFilterRuleInstPtr res, char *commandTemplate, - enum virNWFilterChainSuffixType neededChain, + const char *neededChain, virNWFilterChainPriority chainPriority, char chainprefix, unsigned int priority, @@ -1961,11 +1961,13 @@ ebtablesCreateRuleInstance(char chainPre goto err_exit; } - if (nwfilter->chainsuffix == VIR_NWFILTER_CHAINSUFFIX_ROOT) + if (STREQ(nwfilter->chainsuffix, + virNWFilterChainSuffixTypeToString( + VIR_NWFILTER_CHAINSUFFIX_ROOT))) PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); else PRINT_CHAIN(chain, chainPrefix, ifname, - virNWFilterChainSuffixTypeToString(nwfilter->chainsuffix)); + nwfilter->chainsuffix); switch (rule->prtclType) { @@ -2532,7 +2534,7 @@ ebiptablesDisplayRuleInstance(virConnect ebiptablesRuleInstPtr inst = (ebiptablesRuleInstPtr)_inst; VIR_INFO("Command Template: '%s', Needed protocol: '%s'", inst->commandTemplate, - virNWFilterChainSuffixTypeToString(inst->neededProtocolChain)); + inst->neededProtocolChain); return 0; } @@ -3350,8 +3352,7 @@ ebiptablesApplyNewRules(virConnectPtr co for (i = 0; i < nruleInstances; i++) { sa_assert (inst); if (inst[i]->ruleType == RT_EBTABLES) { - const char *name = virNWFilterChainSuffixTypeToString( - inst[i]->neededProtocolChain); + const char *name = inst[i]->neededProtocolChain; if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) { if (virHashUpdateEntry(chains_in_set, name, &inst[i]->chainPriority)) { Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -35,7 +35,7 @@ typedef struct _ebiptablesRuleInst ebipt typedef ebiptablesRuleInst *ebiptablesRuleInstPtr; struct _ebiptablesRuleInst { char *commandTemplate; - enum virNWFilterChainSuffixType neededProtocolChain; + const char *neededProtocolChain; virNWFilterChainPriority chainPriority; char chainprefix; /* I for incoming, O for outgoing */ unsigned int priority;
This patch extends the filter XML to support priorities of chains in the XML. An example would be: <filter name='allow-arpxyz' chain='arp-xyz' priority='200'> [...] </filter> The permitted values for priorities are [-1000, 1000]. By setting the priority of a chain the order in which it is accessed from the interface root chain can be influenced. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v6: - addressed Eric Blake's comments --- docs/schemas/nwfilter.rng | 7 ++++++- src/conf/nwfilter_conf.c | 42 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 6 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -2014,7 +2014,9 @@ virNWFilterDefParseXML(xmlXPathContextPt xmlNodePtr curr = ctxt->node; char *uuid = NULL; char *chain = NULL; + char *chain_pri_s = NULL; virNWFilterEntryPtr entry; + int chain_priority; if (VIR_ALLOC(ret) < 0) { virReportOOMError(); @@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt goto cleanup; } + chain_pri_s = virXPathString("string(./@priority)", ctxt); + if (chain_pri_s) { + if (virStrToLong_i(chain_pri_s, NULL, 10, &chain_priority) < 0) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Could not parse chain priority '%s'"), + chain_pri_s); + goto cleanup; + } + if (chain_priority < NWFILTER_MIN_FILTER_PRIORITY || + chain_priority > NWFILTER_MAX_FILTER_PRIORITY) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Priority '%d' is outside valid " + "range of [%d,%d]"), + chain_priority, + NWFILTER_MIN_FILTER_PRIORITY, + NWFILTER_MAX_FILTER_PRIORITY); + goto cleanup; + } + } + chain = virXPathString("string(./@chain)", ctxt); if (chain) { if (virNWFilterChainSuffixTypeFromString(chain) < 0) { @@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt goto cleanup; } ret->chainsuffix = chain; - /* assign an implicit priority -- support XML attribute later */ - if (!intMapGetByString(chain_priorities, chain, 0, - &ret->chainPriority)) { - ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + - NWFILTER_MIN_FILTER_PRIORITY) / 2; + + if (chain_pri_s) { + ret->chainPriority = chain_priority; + } else { + /* assign default priority if none can be found via lookup */ + if (!intMapGetByString(chain_priorities, chain, 0, + &ret->chainPriority)) { + ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + + NWFILTER_MIN_FILTER_PRIORITY) / 2; + } } chain = NULL; } else { @@ -2097,6 +2124,7 @@ virNWFilterDefParseXML(xmlXPathContextPt } VIR_FREE(chain); + VIR_FREE(chain_pri_s); return ret; @@ -2104,6 +2132,7 @@ virNWFilterDefParseXML(xmlXPathContextPt virNWFilterDefFree(ret); VIR_FREE(chain); VIR_FREE(uuid); + VIR_FREE(chain_pri_s); return NULL; } @@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d virBufferAsprintf(&buf, "<filter name='%s' chain='%s'", def->name, def->chainsuffix); + if (def->chainPriority != 0) + virBufferAsprintf(&buf, " priority='%d'", + def->chainPriority); virBufferAddLit(&buf, ">\n"); virUUIDFormat(def->uuid, uuid); Index: libvirt-acl/docs/schemas/nwfilter.rng =================================================================== --- libvirt-acl.orig/docs/schemas/nwfilter.rng +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -293,6 +293,11 @@ </choice> </attribute> </optional> + <optional> + <attribute name="priority"> + <ref name='priority-type'/> + </attribute> + </optional> </define> <define name="filterref-node-attributes"> @@ -881,7 +886,7 @@ <define name='priority-type'> <data type="int"> - <param name="minInclusive">0</param> + <param name="minInclusive">-1000</param> <param name="maxInclusive">1000</param> </data> </define>
This patch enables chains that have a known prefix in their name. Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All prefixes are also protocols that can be evaluated on the ebtables level. Following the prefix they will be automatically connected to an interface's 'root' chain and jumped into following the protocol they evaluate, i.e., a table 'arp-xyz' will be accessed from the root table using ebtables -t nat -A <iface root table> -p arp -j I-<ifname>-arp-xyz thus generating a 'root' chain like this one here: Bridge chain: libvirt-O-vnet0, entries: 5, policy: ACCEPT -p IPv4 -j O-vnet0-ipv4 -p ARP -j O-vnet0-arp -p 0x8035 -j O-vnet0-rarp -p ARP -j O-vnet0-arp-xyz -j DROP where the chain 'arp-xyz' is accessed for filtering of ARP packets. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v5: - addressed Eric Blake's comments v3: - assign a priority to filters that have an allowed prefix, e.g., assign the arp chain priority to a filter arp-xyz unless user provided a priority in the XML --- docs/schemas/nwfilter.rng | 16 ++++++-- src/conf/nwfilter_conf.c | 89 +++++++++++++++++++++++++++++++++++++++++++--- src/conf/nwfilter_conf.h | 3 + 3 files changed, 99 insertions(+), 9 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -2007,6 +2007,84 @@ err_exit: goto cleanup; } +static bool +virNWFilterIsValidChainName(const char *chainname) +{ + if (strlen(chainname) > MAX_CHAIN_SUFFIX_SIZE) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Name of chain is longer than " + "%u characters"), + MAX_CHAIN_SUFFIX_SIZE); + return false; + } + + if (chainname[strspn(chainname, VALID_CHAINNAME)] != 0) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Chain name contains illegal characters")); + return false; + } + + return true; +} + +/* + * Test whether the name of the chain is supported. + * It current has to have a prefix of either one of the strings found in + * virNWFilterChainSuffixTypeToString(). + */ +static const char * +virNWFilterIsAllowedChain(const char *chainname) +{ + enum virNWFilterChainSuffixType i; + const char *name, *msg; + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool printed = false; + + if (!virNWFilterIsValidChainName(chainname)) + return NULL; + + for (i = 0; i < VIR_NWFILTER_CHAINSUFFIX_LAST; i++) { + name = virNWFilterChainSuffixTypeToString(i); + if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT) { + /* allow 'root' as a complete name but not as a prefix */ + if (STREQ(chainname, name)) + return name; + if (STRPREFIX(chainname, name)) + return NULL; + } + if (STRPREFIX(chainname, name)) + return name; + } + + virBufferAsprintf(&buf, + _("Invalid chain name '%s'. Please use a chain name " + "called '%s' or any of the following prefixes: "), + virNWFilterChainSuffixTypeToString( + VIR_NWFILTER_CHAINSUFFIX_ROOT), + chainname); + for (i = 0; i < VIR_NWFILTER_CHAINSUFFIX_LAST; i++) { + if (i == VIR_NWFILTER_CHAINSUFFIX_ROOT) + continue; + if (printed) + virBufferAddLit(&buf, ", "); + virBufferAdd(&buf, virNWFilterChainSuffixTypeToString(i), -1); + printed = true; + } + + if (virBufferError(&buf)) { + virReportOOMError(); + virBufferFreeAndReset(&buf); + goto err_exit; + } + + msg = virBufferContentAndReset(&buf); + + virNWFilterReportError(VIR_ERR_INVALID_ARG, "%s", msg); + VIR_FREE(msg); + +err_exit: + return NULL; +} static virNWFilterDefPtr virNWFilterDefParseXML(xmlXPathContextPtr ctxt) { @@ -2017,6 +2095,7 @@ virNWFilterDefParseXML(xmlXPathContextPt char *chain_pri_s = NULL; virNWFilterEntryPtr entry; int chain_priority; + const char *name_prefix; if (VIR_ALLOC(ret) < 0) { virReportOOMError(); @@ -2052,19 +2131,19 @@ virNWFilterDefParseXML(xmlXPathContextPt chain = virXPathString("string(./@chain)", ctxt); if (chain) { - if (virNWFilterChainSuffixTypeFromString(chain) < 0) { - virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown chain suffix '%s'"), chain); + name_prefix = virNWFilterIsAllowedChain(chain); + if (name_prefix == NULL) goto cleanup; - } ret->chainsuffix = chain; if (chain_pri_s) { ret->chainPriority = chain_priority; } else { /* assign default priority if none can be found via lookup */ - if (!intMapGetByString(chain_priorities, chain, 0, + if (!name_prefix || + !intMapGetByString(chain_priorities, name_prefix, 0, &ret->chainPriority)) { + /* assign default chain priority */ ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY + NWFILTER_MIN_FILTER_PRIORITY) / 2; } Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -446,6 +446,9 @@ enum virNWFilterChainSuffixType { VIR_NWFILTER_CHAINSUFFIX_LAST, }; +# define VALID_CHAINNAME \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:-" + typedef int32_t virNWFilterChainPriority; typedef struct _virNWFilterDef virNWFilterDef; Index: libvirt-acl/docs/schemas/nwfilter.rng =================================================================== --- libvirt-acl.orig/docs/schemas/nwfilter.rng +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -286,10 +286,18 @@ <attribute name="chain"> <choice> <value>root</value> - <value>arp</value> - <value>rarp</value> - <value>ipv4</value> - <value>ipv6</value> + <data type="string"> + <param name="pattern">arp[a-zA-Z0-9_\.:-]{0,9}</param> + </data> + <data type="string"> + <param name="pattern">rarp[a-zA-Z0-9_\.:-]{0,8}*</param> + </data> + <data type="string"> + <param name="pattern">ipv4[a-zA-Z0-9_\.:-]{0,8}*</param> + </data> + <data type="string"> + <param name="pattern">ipv6[a-zA-Z0-9_\.:-]{0,8}*</param> + </data> </choice> </attribute> </optional>
On 11/18/2011 06:32 AM, Stefan Berger wrote:
This patch enables chains that have a known prefix in their name. Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All prefixes are also protocols that can be evaluated on the ebtables level.
+ + if (chainname[strspn(chainname, VALID_CHAINNAME)] != 0) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Chain name contains illegal characters"));
s/illegal/invalid/ - we aren't breaking laws :)
+++ libvirt-acl/docs/schemas/nwfilter.rng @@ -286,10 +286,18 @@ <attribute name="chain"> <choice> <value>root</value> - <value>arp</value> - <value>rarp</value> - <value>ipv4</value> - <value>ipv6</value> + <data type="string"> + <param name="pattern">arp[a-zA-Z0-9_\.:-]{0,9}</param> + </data> + <data type="string"> + <param name="pattern">rarp[a-zA-Z0-9_\.:-]{0,8}*</param> + </data> + <data type="string"> + <param name="pattern">ipv4[a-zA-Z0-9_\.:-]{0,8}*</param> + </data> + <data type="string"> + <param name="pattern">ipv6[a-zA-Z0-9_\.:-]{0,8}*</param>
Drop the * on the last three patterns. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/18/2011 11:01 AM, Eric Blake wrote:
On 11/18/2011 06:32 AM, Stefan Berger wrote:
This patch enables chains that have a known prefix in their name. Known prefixes are: 'ipv4', 'ipv6', 'arp', 'rarp'. All prefixes are also protocols that can be evaluated on the ebtables level.
+ + if (chainname[strspn(chainname, VALID_CHAINNAME)] != 0) { + virNWFilterReportError(VIR_ERR_INVALID_ARG, + _("Chain name contains illegal characters")); s/illegal/invalid/ - we aren't breaking laws :) right... +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -286,10 +286,18 @@ <attribute name="chain"> <choice> <value>root</value> -<value>arp</value> -<value>rarp</value> -<value>ipv4</value> -<value>ipv6</value> +<data type="string"> +<param name="pattern">arp[a-zA-Z0-9_\.:-]{0,9}</param> +</data> +<data type="string"> +<param name="pattern">rarp[a-zA-Z0-9_\.:-]{0,8}*</param> +</data> +<data type="string"> +<param name="pattern">ipv4[a-zA-Z0-9_\.:-]{0,8}*</param> +</data> +<data type="string"> +<param name="pattern">ipv6[a-zA-Z0-9_\.:-]{0,8}*</param> Drop the * on the last three patterns.
Fixed
So far rules' priorities have only been valid in the range [0,1000]. Now I am extending their priority into the range [-1000, 1000] for subsequently being able to sort rules and the access of (jumps into) chains following priorities. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/conf/nwfilter_conf.c | 7 ++++--- src/conf/nwfilter_conf.h | 6 ++++-- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.h | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -1897,7 +1897,7 @@ virNWFilterRuleParse(xmlNodePtr node) char *statematch; int found; int found_i = 0; - unsigned int priority; + int priority; xmlNodePtr cur; virNWFilterRuleDefPtr ret; @@ -1943,8 +1943,9 @@ virNWFilterRuleParse(xmlNodePtr node) ret->priority = MAX_RULE_PRIORITY / 2; if (prio) { - if (virStrToLong_ui(prio, NULL, 10, &priority) >= 0) { - if (priority <= MAX_RULE_PRIORITY) + if (virStrToLong_i(prio, NULL, 10, &priority) >= 0) { + if (priority <= MAX_RULE_PRIORITY && + priority >= MIN_RULE_PRIORITY) ret->priority = priority; } } Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -357,7 +357,7 @@ enum virNWFilterEbtablesTableType { }; -# define MIN_RULE_PRIORITY 0 +# define MIN_RULE_PRIORITY -1000 # define MAX_RULE_PRIORITY 1000 # define NWFILTER_MIN_FILTER_PRIORITY -1000 @@ -389,10 +389,12 @@ enum virNWFilterRuleFlags { void virNWFilterPrintStateMatchFlags(virBufferPtr buf, const char *prefix, int32_t flags, bool disp_none); +typedef int32_t virNWFilterRulePriority; + typedef struct _virNWFilterRuleDef virNWFilterRuleDef; typedef virNWFilterRuleDef *virNWFilterRuleDefPtr; struct _virNWFilterRuleDef { - unsigned int priority; + virNWFilterRulePriority priority; enum virNWFilterRuleFlags flags; int action; /*enum virNWFilterRuleActionType*/ int tt; /*enum virNWFilterRuleDirectionType*/ Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.h @@ -38,7 +38,7 @@ struct _ebiptablesRuleInst { const char *neededProtocolChain; virNWFilterChainPriority chainPriority; char chainprefix; /* I for incoming, O for outgoing */ - unsigned int priority; + virNWFilterRulePriority priority; enum RuleType ruleType; }; Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -388,7 +388,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns const char *neededChain, virNWFilterChainPriority chainPriority, char chainprefix, - unsigned int priority, + virNWFilterRulePriority priority, enum RuleType ruleType) { ebiptablesRuleInstPtr inst;
The previous patch extends the priority of filtering rules into negative numbers. We now use this possibility to interleave the jumping into chains with filtering rules to for example create the 'root' table of an interface with the following sequence of rules: Bridge chain: libvirt-I-vnet0, entries: 6, policy: ACCEPT -p IPv4 -j I-vnet0-ipv4 -p ARP -j I-vnet0-arp -p ARP -j ACCEPT -p 0x8035 -j I-vnet0-rarp -p 0x835 -j ACCEPT -j DROP The '-p ARP -j ACCEPT' rule now appears between the jumps. Since the 'arp' chain has been assigned priority -700 and the 'rarp' chain -600, the above ordering can now be achieved with the following rule: <rule action='accept' direction='out' priority='-650'> <mac protocolid='arp'/> </rule> This patch now sorts the commands generating the above shown jumps into chains and interleaves their execution with those for generating rules. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v6: - addressed Eric Blake's comments v3: - fix memory leak --- src/nwfilter/nwfilter_ebiptables_driver.c | 115 ++++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 12 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2718,13 +2718,18 @@ ebtablesUnlinkTmpRootChain(virBufferPtr static int -ebtablesCreateTmpSubChain(virBufferPtr buf, +ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, + int *nRuleInstances, int incoming, const char *ifname, enum l3_proto_idx protoidx, const char *filtername, - int stopOnError) + int stopOnError, + virNWFilterChainPriority priority) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + ebiptablesRuleInstPtr tmp = *inst; + size_t count = *nRuleInstances; char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP : CHAINPREFIX_HOST_OUT_TEMP; @@ -2733,15 +2738,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b PRINT_CHAIN(chain, chainPrefix, ifname, (filtername) ? filtername : l3_protocols[protoidx].val); - virBufferAsprintf(buf, + virBufferAsprintf(&buf, + CMD_DEF("%s -t %s -F %s") CMD_SEPARATOR + CMD_EXEC + CMD_DEF("%s -t %s -X %s") CMD_SEPARATOR + CMD_EXEC CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR CMD_EXEC "%s" - CMD_DEF("%s -t %s -A %s -p 0x%x -j %s") CMD_SEPARATOR + CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s") + CMD_SEPARATOR CMD_EXEC "%s", ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain, + ebtables_cmd_path, EBTABLES_DEFAULT_TABLE, chain, CMD_STOPONERR(stopOnError), @@ -2750,6 +2762,22 @@ ebtablesCreateTmpSubChain(virBufferPtr b CMD_STOPONERR(stopOnError)); + if (virBufferError(&buf) || + VIR_EXPAND_N(tmp, count, 1) < 0) { + virReportOOMError(); + virBufferFreeAndReset(&buf); + return -1; + } + + *nRuleInstances = count; + *inst = tmp; + + tmp[*nRuleInstances - 1].priority = priority; + tmp[*nRuleInstances - 1].commandTemplate = + virBufferContentAndReset(&buf); + tmp[*nRuleInstances - 1].neededProtocolChain = + virNWFilterChainSuffixTypeToString(VIR_NWFILTER_CHAINSUFFIX_ROOT); + return 0; } @@ -3224,9 +3252,35 @@ static int ebtablesCleanAll(const char * static int ebiptablesRuleOrderSort(const void *a, const void *b) { + const ebiptablesRuleInstPtr insta = (const ebiptablesRuleInstPtr)a; + const ebiptablesRuleInstPtr instb = (const ebiptablesRuleInstPtr)b; + const char *root = virNWFilterChainSuffixTypeToString( + VIR_NWFILTER_CHAINSUFFIX_ROOT); + bool root_a = STREQ(insta->neededProtocolChain, root); + bool root_b = STREQ(instb->neededProtocolChain, root); + + /* ensure root chain commands appear before all others since + we will need them to create the child chains */ + if (root_a) { + if (root_b) { + goto normal; + } + return -1; /* a before b */ + } + if (root_b) { + return 1; /* b before a */ + } +normal: + /* priorities are limited to range [-1000, 1000] */ + return (insta->priority - instb->priority); +} + +static int +ebiptablesRuleOrderSortPtr(const void *a, const void *b) +{ const ebiptablesRuleInstPtr *insta = a; const ebiptablesRuleInstPtr *instb = b; - return ((*insta)->priority - (*instb)->priority); + return ebiptablesRuleOrderSort(*insta, *instb); } static int @@ -3297,10 +3351,13 @@ ebtablesGetProtoIdxByFiltername(const ch static int ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, const char *ifname, - virHashTablePtr chains, int direction) + virHashTablePtr chains, int direction, + ebiptablesRuleInstPtr *inst, + int *nRuleInstances) { int rc = 0, i; virHashKeyValuePairPtr filter_names; + const virNWFilterChainPriority *priority; if (ebtablesCreateTmpRootChain(buf, direction, ifname, 1) < 0) return -1; @@ -3315,8 +3372,11 @@ ebtablesCreateTmpRootAndSubChains(virBuf filter_names[i].key); if ((int)idx < 0) continue; - rc = ebtablesCreateTmpSubChain(buf, direction, ifname, idx, - filter_names[i].key, 1); + priority = (const virNWFilterChainPriority *)filter_names[i].value; + rc = ebtablesCreateTmpSubChain(inst, nRuleInstances, + direction, ifname, idx, + filter_names[i].key, 1, + *priority); if (rc < 0) break; } @@ -3331,7 +3391,7 @@ ebiptablesApplyNewRules(virConnectPtr co int nruleInstances, void **_inst) { - int i; + int i, j; int cli_status; ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3339,6 +3399,8 @@ ebiptablesApplyNewRules(virConnectPtr co virHashTablePtr chains_out_set = virHashCreate(10, NULL); bool haveIptables = false; bool haveIp6tables = false; + ebiptablesRuleInstPtr ebtChains = NULL; + int nEbtChains = 0; if (!chains_in_set || !chains_out_set) { virReportOOMError(); @@ -3346,7 +3408,8 @@ ebiptablesApplyNewRules(virConnectPtr co } if (nruleInstances > 1 && inst) - qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort); + qsort(inst, nruleInstances, sizeof(inst[0]), + ebiptablesRuleOrderSortPtr); /* scan the rules to see which chains need to be created */ for (i = 0; i < nruleInstances; i++) { @@ -3380,18 +3443,33 @@ ebiptablesApplyNewRules(virConnectPtr co } /* create needed chains */ - if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1) || - ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0)) { + if (ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set , 1, + &ebtChains, &nEbtChains) || + ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0, + &ebtChains, &nEbtChains)) { goto tear_down_tmpebchains; } + if (nEbtChains > 0) + qsort(&ebtChains[0], nEbtChains, sizeof(ebtChains[0]), + ebiptablesRuleOrderSort); + if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) goto tear_down_tmpebchains; + /* process ebtables commands; interleave commands from filters with + commands for creating and connecting ebtables chains */ + j = 0; for (i = 0; i < nruleInstances; i++) { sa_assert (inst); switch (inst[i]->ruleType) { case RT_EBTABLES: + while (j < nEbtChains && + ebtChains[j].priority <= inst[i]->priority) { + ebiptablesInstCommand(&buf, + ebtChains[j++].commandTemplate, + 'A', -1, 1); + } ebiptablesInstCommand(&buf, inst[i]->commandTemplate, 'A', -1, 1); @@ -3405,6 +3483,11 @@ ebiptablesApplyNewRules(virConnectPtr co } } + while (j < nEbtChains) + ebiptablesInstCommand(&buf, + ebtChains[j++].commandTemplate, + 'A', -1, 1); + if (ebiptablesExecCLI(&buf, &cli_status) || cli_status != 0) goto tear_down_tmpebchains; @@ -3484,6 +3567,10 @@ ebiptablesApplyNewRules(virConnectPtr co virHashFree(chains_in_set); virHashFree(chains_out_set); + for (i = 0; i < nEbtChains; i++) + VIR_FREE(ebtChains[i].commandTemplate); + VIR_FREE(ebtChains); + return 0; tear_down_ebsubchains_and_unlink: @@ -3522,6 +3609,10 @@ exit_free_sets: virHashFree(chains_in_set); virHashFree(chains_out_set); + for (i = 0; i < nEbtChains; i++) + VIR_FREE(ebtChains[i].commandTemplate); + VIR_FREE(ebtChains); + return 1; }
Add test case for the chain names with known prefixes and the chain priority. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- tests/nwfilterxml2xmlin/chain_prefixtest1.xml | 37 +++++++++++++++++++++++++ tests/nwfilterxml2xmlout/chain_prefixtest1.xml | 21 ++++++++++++++ tests/nwfilterxml2xmltest.c | 2 + 3 files changed, 60 insertions(+) Index: libvirt-acl/tests/nwfilterxml2xmlin/chain_prefixtest1.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/chain_prefixtest1.xml @@ -0,0 +1,37 @@ +<filter name='testcase' chain='arp-testme' priority='-650'> + <uuid>e5700920-a333-4c05-8016-b669e46b7599</uuid> + <rule action='accept' direction='out'> + <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + protocolid='arp' + dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff' + hwtype='12' + protocoltype='34' + opcode='Request' + arpsrcmacaddr='1:2:3:4:5:6' + arpdstmacaddr='a:b:c:d:e:f'/> + </rule> + + <rule action='accept' direction='out'> + <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='1' hwtype='255' protocoltype='255'/> + </rule> + + <rule action='accept' direction='out'> + <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='11' hwtype='256' protocoltype='256'/> + </rule> + + <rule action='accept' direction='out'> + <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='65535' hwtype='65535' protocoltype='65535' /> + </rule> + + <rule action='accept' direction='out'> + <arp srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff' + opcode='65536' hwtype='65536' protocoltype='65536' /> + </rule> + + <rule action='accept' direction='in'> + <arp gratuitous='true'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmlout/chain_prefixtest1.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/chain_prefixtest1.xml @@ -0,0 +1,21 @@ +<filter name='testcase' chain='arp-testme' priority='-650'> + <uuid>e5700920-a333-4c05-8016-b669e46b7599</uuid> + <rule action='accept' direction='out' priority='500'> + <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' dstmacaddr='aa:bb:cc:dd:ee:ff' dstmacmask='ff:ff:ff:ff:ff:ff' hwtype='12' protocoltype='34' opcode='Request' arpsrcmacaddr='01:02:03:04:05:06' arpdstmacaddr='0a:0b:0c:0d:0e:0f'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' hwtype='255' protocoltype='255' opcode='Request'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' hwtype='256' protocoltype='256' opcode='11'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff' hwtype='65535' protocoltype='65535' opcode='65535'/> + </rule> + <rule action='accept' direction='out' priority='500'> + <arp srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <arp gratuitous='true'/> + </rule> +</filter> Index: libvirt-acl/tests/nwfilterxml2xmltest.c =================================================================== --- libvirt-acl.orig/tests/nwfilterxml2xmltest.c +++ libvirt-acl/tests/nwfilterxml2xmltest.c @@ -148,6 +148,8 @@ mymain(void) DO_TEST("example-1", false); DO_TEST("example-2", false); + DO_TEST("chain_prefixtest1", true); /* derived from arp-test */ + return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); }
This patch adds several aspects of documentation about the network filtering system: - chains, chains' priorities and chains' default priorities - talks about lists of elements, i.e., a variable assigned multiple values (part of already ACK-ed series) - already mentions the vlan, stp and mac chains added later on (https://www.redhat.com/archives/libvir-list/2011-October/msg01238.html) - mentions limitations of vlan filtering (when sent by VM) on Linux systems --- docs/formatnwfilter.html.in | 227 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 189 insertions(+), 38 deletions(-) Index: libvirt-acl/docs/formatnwfilter.html.in =================================================================== --- libvirt-acl.orig/docs/formatnwfilter.html.in +++ libvirt-acl/docs/formatnwfilter.html.in @@ -109,40 +109,48 @@ <br/><br/> </p> - <h3><a name="nwfconceptsvars">Usage of variables in filters</a></h3> + <h3><a name="nwfconceptschains">Filtering chains</a></h3> <p> - - Two variables names have so far been reserved for usage by the - network traffic filtering subsystem: <code>MAC</code> and - <code>IP</code>. - <br/><br/> - <code>MAC</code> is the MAC address of the - network interface. A filtering rule that references this variable - will automatically be instantiated with the MAC address of the - interface. This works without the user having to explicitly provide - the MAC parameter. Even though it is possible to specify the MAC - parameter similar to the IP parameter above, it is discouraged - since libvirt knows what MAC address an interface will be using. - <br/><br/> - The parameter <code>IP</code> represents the IP address - that the operating system inside the virtual machine is expected - to use on the given interface. The <code>IP</code> parameter - is special in so far as the libvirt daemon will try to determine - the IP address (and thus the IP parameter's value) that is being - used on an interface if the parameter - is not explicitly provided but referenced. - For current limitations on IP address detection, consult the - <a href="#nwflimits">section on limitations</a> on how to use this - feature and what to expect when using it. - <br/><br/> - The following is the XML description of the network filer - <code>no-arp-spoofing</code>. It serves as an example for - a network filter XML referencing the <code>MAC</code> and - <code>IP</code> parameters. This particular filter is referenced by the - <code>clean-traffic</code> filter. + Filtering rules are organized in filter chains. These chains can be + thought of as having a tree structure with packet + filtering rules as entries in individual chains (branches). <br> + Packets start their filter evaluation in the <code>root</code> chain + and can then continue their evaluation in other chains, return from + those chains back into the <code>root</code> chain or be + dropped or accepted by a filtering rule in one of the traversed chains. + <br/> + Libvirt's network filtering system automatically creates individual + <code>root</code> chains for every virtual machine's network interface + on which the user chooses to activate traffic filtering. + The user may write filtering rules that are either directly instantiated + in the <code>root</code> chain or may create protocol-specific + filtering chains for efficient evaluation of protocol-specific rules. + The following chains exist: + </p> + <ul> + <li>root</li> + <li>mac <span class="since">(since 0.9.8)</span></li> + <li>stp (spanning tree protocol) + <span class="since">(since 0.9.8)</span></li> + <li>vlan (802.1Q) <span class="since">(since 0.9.8)</span></li> + <li>arp, rarp</li> + <li>ip</li> + <li>ipv6</li> + </ul> + <p> + <span class="since">Since 0.9.8</span> multiple chains evaluating the + <code>mac</code>, <code>stp</code>, <code>vlan</code>, + <code>arp</code> or <code>rarp</code> protocol can be created using + the protocol name only as a prefix in the chain's name. This for + examples allows chains with names <code>arp-xyz</code> or + <code>arp-test</code> to be specified and have ARP protocol packets + evaluated in those chains. + <br/><br/> + The following filter shows an example of filtering ARP traffic + in the <code>arp</code> chain. </p> <pre> -<filter name='no-arp-spoofing' chain='arp'> +<filter name='no-arp-spoofing' chain='arp' priority='-500'> <uuid>f88f1932-debf-4aa1-9fbe-f10d3aa4bc95</uuid> <rule action='drop' direction='out' priority='300'> <mac match='no' srcmacaddr='$MAC'/> @@ -169,8 +177,93 @@ <rule action='drop' direction='inout' priority='1000'/> </filter> </pre> - <p> + The consequence of putting ARP-specific rules in the <code>arp</code> + chain, rather than for example in the <code>root</code> chain, is that + packets for any other protocol than ARP do not need to be evaluated by + ARP protocol-specific rules. This improves the efficiency + of the traffic filtering. However, one must then pay attention to only + put filtering rules for the given protocol into the chain since + any other rules will not be evaluated, i.e., an IPv4 rule will not + be evaluated in the ARP chain since no IPv4 protocol packets will + traverse the ARP chain. + <br/><br/> + </p> + <h3><a name="nwfconceptschainpriorities">Filtering chain priorities</a></h3> + <p> + All chains are connected to the <code>root</code> chain. The order in + which those chains are accessed is influenced by the priority of the + chain. The following table shows the chains that can be assigned a + priority and their default priorities. + </p> + <table class="top_table"> + <tr> + <th> Chain (prefix) </th> + <th> Default priority </th> + </tr> + <tr> + <td>stp</td><td>-810</td> + </tr> + <tr> + <td>mac</td><td>-800</td> + </tr> + <tr> + <td>vlan</td><td>-750</td> + </tr> + <tr> + <td>ipv4</td><td>-700</td> + </tr> + <tr> + <td>ipv6</td><td>-600</td> + </tr> + <tr> + <td>arp</td><td>-500</td> + </tr> + <tr> + <td>rarp</td><td>-400</td> + </tr> + </table> + <p> + A chain with a lower priority value is accessed before one with a + higher value. + <br><br> + <span class="since">Since 0.9.8</span> the above listed chains + can be assigned custom priorities by writing a value in the + range [-1000, 1000] into the priority (XML) attribute in the filter + node. The above example filter shows the default priority of -500 + for <code>arp</code> chains. + </p> + <h3><a name="nwfconceptsvars">Usage of variables in filters</a></h3> + <p> + + Two variables names have so far been reserved for usage by the + network traffic filtering subsystem: <code>MAC</code> and + <code>IP</code>. + <br/><br/> + <code>MAC</code> is the MAC address of the + network interface. A filtering rule that references this variable + will automatically be instantiated with the MAC address of the + interface. This works without the user having to explicitly provide + the MAC parameter. Even though it is possible to specify the MAC + parameter similar to the IP parameter above, it is discouraged + since libvirt knows what MAC address an interface will be using. + <br/><br/> + The parameter <code>IP</code> represents the IP address + that the operating system inside the virtual machine is expected + to use on the given interface. The <code>IP</code> parameter + is special in so far as the libvirt daemon will try to determine + the IP address (and thus the IP parameter's value) that is being + used on an interface if the parameter + is not explicitly provided but referenced. + For current limitations on IP address detection, consult the + <a href="#nwflimits">section on limitations</a> on how to use this + feature and what to expect when using it. + <br/><br/> + The above-shown network filer <code>no-arp-spoofing</code> + is an example of + a network filter XML referencing the <code>MAC</code> and + <code>IP</code> variables. + <br/><br/> Note that referenced variables are always prefixed with the $ (dollar) sign. The format of the value of a variable must be of the type expected by the filter attribute in the @@ -182,7 +275,38 @@ interface from attaching when hotplugging is used. The types that are expected for each XML attribute are shown below. + <br/><br/> + <span class="since">Since 0.9.8</span> variables can contain lists of + elements, e.g., the variable <code>IP</code> can contain multiple IP + addresses that are valid on a particular interface. The notation for + providing multiple elements for the IP variable is: </p> +<pre> + ... + <devices> + <interface type='bridge'> + <mac address='00:16:3e:5d:c7:9e'/> + <filterref filter='clean-traffic'> + <parameter name='IP' value='10.0.0.1'/> + <parameter name='IP' value='10.0.0.2'/> + <parameter name='IP' value='10.0.0.3'/> + </filterref> + </interface> + </devices> + ...</pre> + <p> + This then allows filters to enable multiple IP addresses + per interface. Therefore, with the list + of IP address shown above, the following rule will create 3 + individual filtering rules, one for each IP address. + </p> +<pre> + ... + <rule action='accept' direction='in' priority='500'> + <tcp srpipaddr='$IP'/> + </rule> + ... +</pre> <h2><a name="nwfelems">Element and attribute overview</a></h2> @@ -280,10 +404,21 @@ <li> priority -- optional; the priority of the rule controls the order in which the rule will be instantiated relative to other rules. - Rules with lower value will be instantiated and therefore evaluated - before rules with higher value. - Valid values are in the range of 0 to 1000. If this attribute is not - provided, the value 500 will automatically be assigned. + Rules with lower value will be instantiated before rules with higher + values. + Valid values are in the range of 0 to 1000. + <span class="since">Since 0.9.8</span> this has been extended to cover + the range of -1000 to 1000. If this attribute is not + provided, priority 500 will automatically be assigned. + <br> + Note that filtering rules in the <code>root</code> chain are sorted + with filters connected to the <code>root</code> chain following + their priorities. This allows to interleave filtering rules with + access to filter chains. + (See also section on + <a href="#nwfconceptschainpriorities"> + filtering chain priorities + </a>.) </li> <li> statematch -- optional; possible values are '0' or 'false' to @@ -1431,6 +1566,8 @@ </p> <ul> <li>mac</li> + <li>stp (spanning tree protocol)</li> + <li>vlan (802.1Q)</li> <li>arp, rarp</li> <li>ip</li> <li>ipv6</li> @@ -1444,13 +1581,14 @@ filter subsystem first passes through the filtering support implemented by ebtables and only then through iptables or ip6tables filters. If a filter tree has rules with the protocols <code>mac</code>, + <code>stp</code>, <code>vlan</code> <code>arp</code>, <code>rarp</code>, <code>ip</code>, or <code>ipv6</code> ebtables rules will automatically be instantiated. <br/> The role of the <code>chain</code> attribute in the network filter XML is that internally a new user-defined ebtables table is created that then for example receives all <code>arp</code> traffic coming - from or going to a virtual machine, if the chain <code>arp</code> + from or going to a virtual machine if the chain <code>arp</code> has been specified. Further, a rule is generated in an interface's <code>root</code> chain that directs all ipv4 traffic into the user-defined chain. Therefore, all ARP traffic rules should then be @@ -1458,6 +1596,12 @@ into user-defined tables is only supported with filtering on the ebtables layer. <br/> + <span class="since">Since 0.9.8</span> multiple chains for the same + protocol can be created. For this the name of the chain must have + a prefix of one of the previously enumerated protocols. To create an + additional chain for handling of ARP traffic, a chain with name + <code>arp-test</code> can be specified. + <br/> As an example, it is possible to filter on UDP traffic by source and destination ports using the <code>ip</code> protocol filter and specifying attributes for the @@ -1803,6 +1947,13 @@ 0.8.1 or later in order not to lose the network traffic filters associated with an interface. </p> - + <h3><a name="nwflimitsvlan">VLAN filtering on Linux</a></h3> + <p> + VLAN (802.1Q) packets, if sent by a virtual machine, cannot be filtered + with rules for protocol IDs <code>arp</code>, <code>rarp</code>, + <code>ipv4</code> and <code>ipv6</code> but only + with protocol IDs <code>mac</code> and <code>vlan</code>. Therefore, + the example filter <code>clean-traffic</code> will not work as expected. + </p> </body> </html>
On 11/18/2011 06:32 AM, Stefan Berger wrote:
This patch adds several aspects of documentation about the network filtering system:
- chains, chains' priorities and chains' default priorities - talks about lists of elements, i.e., a variable assigned multiple values (part of already ACK-ed series) - already mentions the vlan, stp and mac chains added later on (https://www.redhat.com/archives/libvir-list/2011-October/msg01238.html) - mentions limitations of vlan filtering (when sent by VM) on Linux systems
Thanks for shuffling this work in sooner. Guess that means we're committing to adding some of the other series in short order :)
+ Filtering rules are organized in filter chains. These chains can be + thought of as having a tree structure with packet + filtering rules as entries in individual chains (branches). <br> + Packets start their filter evaluation in the <code>root</code> chain + and can then continue their evaluation in other chains, return from + those chains back into the <code>root</code> chain or be + dropped or accepted by a filtering rule in one of the traversed chains. + <br/> + Libvirt's network filtering system automatically creates individual
I don't know if the convention is to use </p><p> instead of <br/> between paragraphs; I'm not too fussed, though, as the rendered page still looked okay to me.
+ <ul> + <li>root</li> + <li>mac <span class="since">(since 0.9.8)</span></li> + <li>stp (spanning tree protocol) + <span class="since">(since 0.9.8)</span></li> + <li>vlan (802.1Q) <span class="since">(since 0.9.8)</span></li> + <li>arp, rarp</li> + <li>ip</li>
Is this right? My recollection of the code was that your prefix lookup had ipv4 and ipv6, not ip and ipv6, given that I had you add a comment about none of the prefixes being subsumed by another entry in the table. On the other hand, using 'ip' as short for 'ipv4' is nice. Is there more code work to do on this front? And if it does work as 'ip' vs. 'ipv6', we probably ought to list this line as <li>ip (IPv4)</li>.
@@ -1431,6 +1566,8 @@ </p> <ul> <li>mac</li> + <li>stp (spanning tree protocol)</li> + <li>vlan (802.1Q)</li> <li>arp, rarp</li> <li>ip</li> <li>ipv6</li>
Hmm, we already have another table with just 'ip'. Okay, then, what you have is okay to commit as-is, and any further tweaks (such as if we add code to explicitly allow 'ipv4' as an alias for 'ip') can come later with the code changes. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/18/2011 11:01 AM, Eric Blake wrote:
On 11/18/2011 06:32 AM, Stefan Berger wrote:
This patch adds several aspects of documentation about the network filtering system:
- chains, chains' priorities and chains' default priorities - talks about lists of elements, i.e., a variable assigned multiple values (part of already ACK-ed series) - already mentions the vlan, stp and mac chains added later on (https://www.redhat.com/archives/libvir-list/2011-October/msg01238.html) - mentions limitations of vlan filtering (when sent by VM) on Linux systems Thanks for shuffling this work in sooner. Guess that means we're committing to adding some of the other series in short order :) Adding stp, vlan and mac should be 'easy' -- more or less 'mechanical' + Filtering rules are organized in filter chains. These chains can be + thought of as having a tree structure with packet + filtering rules as entries in individual chains (branches).<br> + Packets start their filter evaluation in the<code>root</code> chain + and can then continue their evaluation in other chains, return from + those chains back into the<code>root</code> chain or be + dropped or accepted by a filtering rule in one of the traversed chains. +<br/> + Libvirt's network filtering system automatically creates individual I don't know if the convention is to use</p><p> instead of<br/> between paragraphs; I'm not too fussed, though, as the rendered page still looked okay to me.
+<ul> +<li>root</li> +<li>mac<span class="since">(since 0.9.8)</span></li> +<li>stp (spanning tree protocol) +<span class="since">(since 0.9.8)</span></li> +<li>vlan (802.1Q)<span class="since">(since 0.9.8)</span></li> +<li>arp, rarp</li> +<li>ip</li> Is this right? My recollection of the code was that your prefix lookup had ipv4 and ipv6, not ip and ipv6, given that I had you add a comment Good catch! It's supposed to be 'ipv4' in the name of the chain. I may later on try to add an alias 'ip'...
about none of the prefixes being subsumed by another entry in the table. On the other hand, using 'ip' as short for 'ipv4' is nice. Is there more code work to do on this front? And if it does work as 'ip' vs. On this 'particular' front, I would say 'no'. There are other aspects that I have done work, though... 'ipv6', we probably ought to list this line as<li>ip (IPv4)</li>.
@@ -1431,6 +1566,8 @@ </p> <ul> <li>mac</li> +<li>stp (spanning tree protocol)</li> +<li>vlan (802.1Q)</li> <li>arp, rarp</li> <li>ip</li> <li>ipv6</li> Hmm, we already have another table with just 'ip'. Okay, then, what you have is okay to commit as-is, and any further tweaks (such as if we add code to explicitly allow 'ipv4' as an alias for 'ip') can come later with the code changes.
I fixed this typo. The above table is a c&p of this one... Stefan
ACK.
On 11/18/2011 06:32 AM, Stefan Berger wrote:
The following series of patches re-does some of the inner workings of nwfilters with the goal to enable users to write filters that have other than the system-known chains supported right now ('root','arp','rarp','ipv4' and 'ipv6'). Ideally users should be able to provide a chain name in the chains XML attribute and either be able to jump to it as an 'action' or have the chain created automatically as it is the case right now for those chains enumerated before. The latter is now added in this patch series as well.
ACK series, except that you need to fix nits in 7/11, and may want to tweak a couple of other places where I replied on the patch. Fix the nits and push; I don't think we need a v7. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/18/2011 11:02 AM, Eric Blake wrote:
On 11/18/2011 06:32 AM, Stefan Berger wrote:
The following series of patches re-does some of the inner workings of nwfilters with the goal to enable users to write filters that have other than the system-known chains supported right now ('root','arp','rarp','ipv4' and 'ipv6'). Ideally users should be able to provide a chain name in the chains XML attribute and either be able to jump to it as an 'action' or have the chain created automatically as it is the case right now for those chains enumerated before. The latter is now added in this patch series as well. ACK series, except that you need to fix nits in 7/11, and may want to tweak a couple of other places where I replied on the patch.
Fix the nits and push; I don't think we need a v7.
Pushed now with the tweaks applied.
participants (2)
-
Eric Blake -
Stefan Berger