[libvirt] [PATCH v2] nwfilters: support for TCP flags evaluation

This patch adds support for the evaluation of TCP flags in nwfilters. It adds documentation to the web page and extends the tests as well. Also, the nwfilter schema is extended. The following are some example for rules using the tcp flags: <rule action='accept' direction='in'> <tcp state='NONE' flags='SYN/ALL' dsptportstart='80'/> </rule> <rule action='drop' direction='in'> <tcp state='NONE' flags='SYN/ALL'/> </rule> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- docs/formatnwfilter.html.in | 10 ++ docs/schemas/nwfilter.rng | 16 ++++ src/conf/nwfilter_conf.c | 115 +++++++++++++++++++++++++++--- src/conf/nwfilter_conf.h | 9 ++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 9 ++ tests/nwfilterxml2xmlin/tcp-test.xml | 12 +++ tests/nwfilterxml2xmlout/tcp-test.xml | 12 +++ 8 files changed, 174 insertions(+), 10 deletions(-) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -5,7 +5,8 @@ * Copyright (C) 2006-2011 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * - * Copyright (C) 2010 IBM Corporation + * Copyright (C) 2010-2011 IBM Corporation + * Copyright (C) 2010-2011 Stefan Berger * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -726,17 +727,23 @@ printStringItems(virBufferPtr buf, const int32_t flags, const char *sep) { unsigned int i, c = 0; - int32_t last_attr = 0; + int32_t mask = 0x1; - for (i = 0; int_map[i].val; i++) { - if (last_attr != int_map[i].attr && - flags & int_map[i].attr) { - if (c >= 1) - virBufferVSprintf(buf, "%s", sep); - virBufferVSprintf(buf, "%s", int_map[i].val); - c++; + while (mask) { + if ((mask & flags)) { + for (i = 0; int_map[i].val; i++) { + if (mask == int_map[i].attr) { + if (c >= 1) + virBufferVSprintf(buf, "%s", sep); + virBufferVSprintf(buf, "%s", int_map[i].val); + c++; + } + } + flags ^= mask; + if (!flags) + break; } - last_attr = int_map[i].attr; + mask <<= 1; } return 0; @@ -799,6 +806,87 @@ stateFormatter(virBufferPtr buf, } + +static const struct int_map tcpFlags[] = { + INTMAP_ENTRY(0x1 , "FIN"), + INTMAP_ENTRY(0x2 , "SYN"), + INTMAP_ENTRY(0x4 , "RST"), + INTMAP_ENTRY(0x8 , "PSH"), + INTMAP_ENTRY(0x10, "ACK"), + INTMAP_ENTRY(0x20, "URG"), + INTMAP_ENTRY(0x3F, "ALL"), + INTMAP_ENTRY(0x0 , "NONE"), + INTMAP_ENTRY_LAST +}; + + +static bool +tcpFlagsValidator(enum attrDatatype datatype ATTRIBUTE_UNUSED, union data *val, + virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED, + nwItemDesc *item) +{ + bool rc = false; + char *s_mask = val->c; + char *sep = strchr(val->c, '/'); + char *s_flags; + int32_t mask = 0, flags = 0; + + if (!sep) + return false; + + s_flags = sep + 1; + + *sep = '\0'; + + if (!parseStringItems(tcpFlags, s_mask , &mask , ',') && + !parseStringItems(tcpFlags, s_flags, &flags, ',')) { + item->u.tcpFlags.mask = mask & 0x3f; + item->u.tcpFlags.flags = flags & 0x3f; + rc = true; + } + + *sep = '/'; + + return rc; +} + + +static void +printTCPFlags(virBufferPtr buf, uint8_t flags) +{ + if (flags == 0) + virBufferAddLit(buf, "NONE"); + else if (flags == 0x3f) + virBufferAddLit(buf, "ALL"); + else + printStringItems(buf, tcpFlags, flags, ","); +} + + +void +virNWFilterPrintTCPFlags(virBufferPtr buf, + uint8_t mask, char sep, uint8_t flags) +{ + printTCPFlags(buf, mask); + virBufferAddChar(buf, sep); + printTCPFlags(buf, flags); +} + + +static bool +tcpFlagsFormatter(virBufferPtr buf, + virNWFilterRuleDefPtr nwf ATTRIBUTE_UNUSED, + nwItemDesc *item) +{ + virNWFilterPrintTCPFlags(buf, + item->u.tcpFlags.mask, + '/', + item->u.tcpFlags.flags); + + return true; +} + + #define COMMON_MAC_PROPS(STRUCT) \ {\ .name = SRCMACADDR,\ @@ -1104,6 +1192,13 @@ static const virXMLAttr2Struct tcpAttrib .datatype = DATATYPE_UINT8 | DATATYPE_UINT8_HEX, .dataIdx = offsetof(virNWFilterRuleDef, p.tcpHdrFilter.dataTCPOption), }, + { + .name = "flags", + .datatype = DATATYPE_STRING, + .dataIdx = offsetof(virNWFilterRuleDef, p.tcpHdrFilter.dataTCPFlags), + .validator = tcpFlagsValidator, + .formatter = tcpFlagsFormatter, + }, COMMENT_PROP_IPHDR(tcpHdrFilter), { .name = NULL, Index: libvirt-acl/src/conf/nwfilter_conf.h =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.h +++ libvirt-acl/src/conf/nwfilter_conf.h @@ -122,6 +122,10 @@ struct _nwItemDesc { uint16_t u16; char protocolID[10]; char *string; + struct { + uint8_t mask; + uint8_t flags; + } tcpFlags; } u; }; @@ -242,6 +246,7 @@ struct _tcpHdrFilterDef { ipHdrDataDef ipHdr; portDataDef portData; nwItemDesc dataTCPOption; + nwItemDesc dataTCPFlags; }; @@ -667,6 +672,10 @@ void virNWFilterCallbackDriversLock(void void virNWFilterCallbackDriversUnlock(void); +void virNWFilterPrintTCPFlags(virBufferPtr buf, uint8_t mask, + char sep, uint8_t flags); + + VIR_ENUM_DECL(virNWFilterRuleAction); VIR_ENUM_DECL(virNWFilterRuleDirection); VIR_ENUM_DECL(virNWFilterRuleProtocol); 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 @@ -1204,6 +1204,15 @@ _iptablesCreateRuleInstance(int directio &prefix)) goto err_exit; + if (HAS_ENTRY_ITEM(&rule->p.tcpHdrFilter.dataTCPFlags)) { + virBufferVSprintf(&buf, " %s --tcp-flags ", + ENTRY_GET_NEG_SIGN(&rule->p.tcpHdrFilter.dataTCPFlags)); + virNWFilterPrintTCPFlags(&buf, + rule->p.tcpHdrFilter.dataTCPFlags.u.tcpFlags.mask, + ' ', + rule->p.tcpHdrFilter.dataTCPFlags.u.tcpFlags.flags); + } + if (iptablesHandlePortData(&buf, vars, &rule->p.tcpHdrFilter.portData, Index: libvirt-acl/docs/schemas/nwfilter.rng =================================================================== --- libvirt-acl.orig/docs/schemas/nwfilter.rng +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -81,6 +81,7 @@ <ref name="common-port-attributes"/> <ref name="common-ip-attributes-p1"/> <ref name="common-ip-attributes-p2"/> + <ref name="tcp-attributes"/> <ref name="comment-attribute"/> </element> </zeroOrMore> @@ -184,6 +185,7 @@ <ref name="common-port-attributes"/> <ref name="common-ipv6-attributes-p1"/> <ref name="common-ipv6-attributes-p2"/> + <ref name="tcp-attributes"/> <ref name="comment-attribute"/> </element> </zeroOrMore> @@ -606,6 +608,14 @@ </optional> </define> + <define name="tcp-attributes"> + <optional> + <attribute name="flags"> + <ref name="tcpflags-type"/> + </attribute> + </optional> + </define> + <!-- ################ type library ################ --> <define name="UUID"> @@ -872,4 +882,10 @@ <param name="pattern">((NEW|ESTABLISHED|RELATED|INVALID)(,(NEW|ESTABLISHED|RELATED|INVALID))*|NONE)</param> </data> </define> + + <define name='tcpflags-type'> + <data type="string"> + <param name="pattern">((SYN|ACK|URG|PSH|FIN|RST)(,(SYN|ACK|URG|PSH|FIN|RST))*|ALL|NONE)/((SYN|ACK|URG|PSH|FIN|RST)(,(SYN|ACK|URG|PSH|FIN|RST))*|ALL|NONE)</param> + </data> + </define> </grammar> Index: libvirt-acl/docs/formatnwfilter.html.in =================================================================== --- libvirt-acl.orig/docs/formatnwfilter.html.in +++ libvirt-acl/docs/formatnwfilter.html.in @@ -755,6 +755,11 @@ <td>STRING</td> <td>comma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE</td> </tr> + <tr> + <td>flags <span class="since">(Since 0.9.1)</span></td> + <td>STRING</td> + <td>TCP-only: format of mask/flags with mask and flags each being a comma separated list of SYN,ACK,URG,PSH,FIN,RST or NONE or ALL</td> + </tr> </table> <p> <br><br> @@ -1040,6 +1045,11 @@ <td>STRING</td> <td>comma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE</td> </tr> + <tr> + <td>flags <span class="since">(Since 0.9.1)</span></td> + <td>STRING</td> + <td>TCP-only: format of mask/flags with mask and flags each being a comma separated list of SYN,ACK,URG,PSH,FIN,RST or NONE or ALL</td> + </tr> </table> <p> <br><br> Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -683,6 +683,7 @@ virNWFilterObjRemove; virNWFilterObjSaveDef; virNWFilterObjUnlock; virNWFilterPrintStateMatchFlags; +virNWFilterPrintTCPFlags; virNWFilterRegisterCallbackDriver; virNWFilterRuleActionTypeToString; virNWFilterRuleProtocolTypeToString; Index: libvirt-acl/tests/nwfilterxml2xmlin/tcp-test.xml =================================================================== --- libvirt-acl.orig/tests/nwfilterxml2xmlin/tcp-test.xml +++ libvirt-acl/tests/nwfilterxml2xmlin/tcp-test.xml @@ -19,4 +19,16 @@ srcportstart='255' srcportend='256' dstportstart='65535' dstportend='65536'/> </rule> + <rule action='accept' direction='in'> + <tcp state='NONE' flags='SYN/ALL'/> + </rule> + <rule action='accept' direction='in'> + <tcp state='NONE' flags='SYN/SYN,ACK'/> + </rule> + <rule action='accept' direction='in'> + <tcp state='NONE' flags='RST/NONE'/> + </rule> + <rule action='accept' direction='in'> + <tcp state='NONE' flags='PSH/'/> + </rule> </filter> Index: libvirt-acl/tests/nwfilterxml2xmlout/tcp-test.xml =================================================================== --- libvirt-acl.orig/tests/nwfilterxml2xmlout/tcp-test.xml +++ libvirt-acl/tests/nwfilterxml2xmlout/tcp-test.xml @@ -9,4 +9,16 @@ <rule action='accept' direction='in' priority='500' statematch='false'> <tcp srcmacaddr='01:02:03:04:05:06' srcipaddr='10.1.2.3' srcipmask='32' dscp='63' srcportstart='255' srcportend='256' dstportstart='65535'/> </rule> + <rule action='accept' direction='in' priority='500'> + <tcp state='NONE' flags='SYN/ALL'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp state='NONE' flags='SYN/SYN,ACK'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp state='NONE' flags='RST/NONE'/> + </rule> + <rule action='accept' direction='in' priority='500'> + <tcp state='NONE' flags='PSH/NONE'/> + </rule> </filter>

On 04/07/2011 01:47 PM, Stefan Berger wrote:
This patch adds support for the evaluation of TCP flags in nwfilters.
It adds documentation to the web page and extends the tests as well. Also, the nwfilter schema is extended.
The following are some example for rules using the tcp flags:
<rule action='accept' direction='in'> <tcp state='NONE' flags='SYN/ALL' dsptportstart='80'/> </rule> <rule action='drop' direction='in'> <tcp state='NONE' flags='SYN/ALL'/> </rule>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
--- docs/formatnwfilter.html.in | 10 ++ docs/schemas/nwfilter.rng | 16 ++++ src/conf/nwfilter_conf.c | 115 +++++++++++++++++++++++++++--- src/conf/nwfilter_conf.h | 9 ++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 9 ++ tests/nwfilterxml2xmlin/tcp-test.xml | 12 +++ tests/nwfilterxml2xmlout/tcp-test.xml | 12 +++ 8 files changed, 174 insertions(+), 10 deletions(-)
ACK, looks reasonable to me, with one optimization nit fixed:
+ int32_t mask = 0x1;
- for (i = 0; int_map[i].val; i++) { - if (last_attr != int_map[i].attr && - flags & int_map[i].attr) { - if (c >= 1) - virBufferVSprintf(buf, "%s", sep); - virBufferVSprintf(buf, "%s", int_map[i].val); - c++; + while (mask) { + if ((mask & flags)) { + for (i = 0; int_map[i].val; i++) { + if (mask == int_map[i].attr) { + if (c >= 1) + virBufferVSprintf(buf, "%s", sep); + virBufferVSprintf(buf, "%s", int_map[i].val); + c++; + } + } + flags ^= mask; + if (!flags) + break;
This if condition should be after...
}
this brace; otherwise, if flags == 0 on entry, then you will (needlessly) iterate through all 32 bits of mask, rather than breaking on the first iteration. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 04/07/2011 05:47 PM, Eric Blake wrote:
On 04/07/2011 01:47 PM, Stefan Berger wrote:
This patch adds support for the evaluation of TCP flags in nwfilters.
It adds documentation to the web page and extends the tests as well. Also, the nwfilter schema is extended.
The following are some example for rules using the tcp flags:
<rule action='accept' direction='in'> <tcp state='NONE' flags='SYN/ALL' dsptportstart='80'/> </rule> <rule action='drop' direction='in'> <tcp state='NONE' flags='SYN/ALL'/> </rule>
Signed-off-by: Stefan Berger<stefanb@linux.vnet.ibm.com>
--- docs/formatnwfilter.html.in | 10 ++ docs/schemas/nwfilter.rng | 16 ++++ src/conf/nwfilter_conf.c | 115 +++++++++++++++++++++++++++--- src/conf/nwfilter_conf.h | 9 ++ src/libvirt_private.syms | 1 src/nwfilter/nwfilter_ebiptables_driver.c | 9 ++ tests/nwfilterxml2xmlin/tcp-test.xml | 12 +++ tests/nwfilterxml2xmlout/tcp-test.xml | 12 +++ 8 files changed, 174 insertions(+), 10 deletions(-) ACK, looks reasonable to me, with one optimization nit fixed:
+ int32_t mask = 0x1;
- for (i = 0; int_map[i].val; i++) { - if (last_attr != int_map[i].attr&& - flags& int_map[i].attr) { - if (c>= 1) - virBufferVSprintf(buf, "%s", sep); - virBufferVSprintf(buf, "%s", int_map[i].val); - c++; + while (mask) { + if ((mask& flags)) { + for (i = 0; int_map[i].val; i++) { + if (mask == int_map[i].attr) { + if (c>= 1) + virBufferVSprintf(buf, "%s", sep); + virBufferVSprintf(buf, "%s", int_map[i].val); + c++; + } + } + flags ^= mask; + if (!flags) + break; This if condition should be after...
}
this brace; otherwise, if flags == 0 on entry, then you will (needlessly) iterate through all 32 bits of mask, rather than breaking on the first iteration.
Excellent! I'll modify and push. Stefan
participants (2)
-
Eric Blake
-
Stefan Berger