The following issues are fixed in the patch below: - ebtables requires that some of the command line parameters are passed as hex numbers; so have those attributes call a function that prints 16 and 8 bit integers as hex nunbers. - ip6tables requires '--icmpv6-type' rather than '--icmp-type' - ebtables complains about protocol identifiers lower than 0x600, so already discard anything lower than 0x600 in the parser - make the protocol entry types more readable using a #define for its entries - continue parsing a filtering rule even if a faulty entry is encountered; return an error value at the end and let the caller decide what to do with the rule's object - fix an error message Signed-off-by: Stefan Berger --- src/conf/nwfilter_conf.c | 131 ++++++++---------------------- src/nwfilter/nwfilter_ebiptables_driver.c | 94 +++++++++++++++++---- 2 files changed, 116 insertions(+), 109 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 @@ -138,10 +138,11 @@ printVar(virConnectPtr conn, static int -printDataType(virConnectPtr conn, - virNWFilterHashTablePtr vars, - char *buf, int bufsize, - nwItemDescPtr item) +_printDataType(virConnectPtr conn, + virNWFilterHashTablePtr vars, + char *buf, int bufsize, + nwItemDescPtr item, + bool asHex) { int done; char *data; @@ -199,8 +200,18 @@ printDataType(virConnectPtr conn, virFormatMacAddr(item->u.macaddr.addr, buf); break; - case DATATYPE_UINT16: + case DATATYPE_IPV6MASK: + case DATATYPE_IPMASK: if (snprintf(buf, bufsize, "%d", + item->u.u8) >= bufsize) { + virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER, + _("Buffer too small for uint8 type")); + return 1; + } + break; + + case DATATYPE_UINT16: + if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d", item->u.u16) >= bufsize) { virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER, _("Buffer too small for uint16 type")); @@ -208,10 +219,8 @@ printDataType(virConnectPtr conn, } break; - case DATATYPE_IPV6MASK: - case DATATYPE_IPMASK: case DATATYPE_UINT8: - if (snprintf(buf, bufsize, "%d", + if (snprintf(buf, bufsize, asHex ? "0x%x" : "%d", item->u.u8) >= bufsize) { virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER, _("Buffer too small for uint8 type")); @@ -230,6 +239,26 @@ printDataType(virConnectPtr conn, } +static int +printDataType(virConnectPtr conn, + virNWFilterHashTablePtr vars, + char *buf, int bufsize, + nwItemDescPtr item) +{ + return _printDataType(conn, vars, buf, bufsize, item, 0); +} + + +static int +printDataTypeAsHex(virConnectPtr conn, + virNWFilterHashTablePtr vars, + char *buf, int bufsize, + nwItemDescPtr item) +{ + return _printDataType(conn, vars, buf, bufsize, item, 1); +} + + static void ebiptablesRuleInstFree(ebiptablesRuleInstPtr inst) { @@ -1270,6 +1299,12 @@ _iptablesCreateRuleInstance(virConnectPt goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) { + const char *parm; + if (rule->prtclType == VIR_NWFILTER_RULE_PROTOCOL_ICMP) + parm = "--icmp-type"; + else + parm = "--icmpv6-type"; + if (printDataType(conn, vars, number, sizeof(number), @@ -1277,8 +1312,9 @@ _iptablesCreateRuleInstance(virConnectPt goto err_exit; virBufferVSprintf(&buf, - " %s --icmp-type %s", + " %s %s %s", ENTRY_GET_NEG_SIGN(&rule->p.icmpHdrFilter.dataICMPType), + parm, number); if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPCode)) { @@ -1295,6 +1331,30 @@ _iptablesCreateRuleInstance(virConnectPt } break; + case VIR_NWFILTER_RULE_PROTOCOL_IGMP: + virBufferVSprintf(&buf, + CMD_DEF_PRE "%s -%%c %s %%s", + iptables_cmd, + chain); + + virBufferAddLit(&buf, " -p igmp"); + + if (iptablesHandleSrcMacAddr(conn, + &buf, + vars, + &rule->p.igmpHdrFilter.dataSrcMACAddr, + directionIn)) + goto err_exit; + + if (iptablesHandleIpHdr(conn, + &buf, + vars, + &rule->p.igmpHdrFilter.ipHdr, + directionIn)) + goto err_exit; + + break; + case VIR_NWFILTER_RULE_PROTOCOL_ALL: case VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6: virBufferVSprintf(&buf, @@ -1490,10 +1550,10 @@ ebtablesCreateRuleInstance(virConnectPtr goto err_exit; if (HAS_ENTRY_ITEM(&rule->p.ethHdrFilter.dataProtocolID)) { - if (printDataType(conn, - vars, - number, sizeof(number), - &rule->p.ethHdrFilter.dataProtocolID)) + if (printDataTypeAsHex(conn, + vars, + number, sizeof(number), + &rule->p.ethHdrFilter.dataProtocolID)) goto err_exit; virBufferVSprintf(&buf, " -p %s %s", @@ -1541,10 +1601,10 @@ ebtablesCreateRuleInstance(virConnectPtr } if (HAS_ENTRY_ITEM(&rule->p.arpHdrFilter.dataProtocolType)) { - if (printDataType(conn, - vars, - number, sizeof(number), - &rule->p.arpHdrFilter.dataProtocolType)) + if (printDataTypeAsHex(conn, + vars, + number, sizeof(number), + &rule->p.arpHdrFilter.dataProtocolType)) goto err_exit; virBufferVSprintf(&buf, " --arp-ptype %s %s", Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -409,6 +409,8 @@ checkMacProtocolID(enum attrDatatype dat res = -1; } else if (datatype == DATATYPE_UINT16) { res = (uint32_t)*(uint16_t *)value; + if (res < 0x600) + res = -1; } if (res != -1) { @@ -766,7 +768,7 @@ static const virXMLAttr2Struct ipv6Attri }, { .name = "protocol", - .datatype = DATATYPE_STRING, + .datatype = DATATYPE_STRING | DATATYPE_UINT8, .dataIdx = offsetof(virNWFilterRuleDef, p.ipv6HdrFilter.ipHdr.dataProtocolID), .validator= checkIPProtocolID, .formatter= formatIPProtocolID, @@ -1048,95 +1050,34 @@ struct _virAttributes { enum virNWFilterRuleProtocolType prtclType; }; +#define PROTOCOL_ENTRY(ID, ATT, PRTCLTYPE) \ + { .id = ID, .att = ATT, .prtclType = PRTCLTYPE } +#define PROTOCOL_ENTRY_LAST { .id = NULL } + static const virAttributes virAttr[] = { - { - .id = "arp", - .att = arpAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ARP, - }, { - .id = "mac", - .att = macAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_MAC, - }, { - .id = "ip", - .att = ipAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_IP, - }, { - .id = "ipv6", - .att = ipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_IPV6, - }, { - .id = "tcp", - .att = tcpAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_TCP, - }, { - .id = "udp", - .att = udpAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDP, - }, { - .id = "udplite", - .att = udpliteAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDPLITE, - }, { - .id = "esp", - .att = espAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ESP, - }, { - .id = "ah", - .att = ahAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_AH, - }, { - .id = "sctp", - .att = sctpAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_SCTP, - }, { - .id = "icmp", - .att = icmpAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ICMP, - }, { - .id = "all", // = 'any' - .att = allAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ALL, - }, { - .id = "igmp", - .att = igmpAttributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_IGMP, - }, { - .id = "tcp-ipv6", - .att = tcpipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6, - }, { - .id = "udp-ipv6", - .att = udpipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6, - }, { - .id = "udplite-ipv6", - .att = udpliteipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6, - }, { - .id = "esp-ipv6", - .att = espipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6, - }, { - .id = "ah-ipv6", - .att = ahipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6, - }, { - .id = "sctp-ipv6", - .att = sctpipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6, - }, { - .id = "icmpv6", - .att = icmpv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ICMPV6, - }, { - .id = "all-ipv6", // = 'any' - .att = allipv6Attributes, - .prtclType = VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6, - }, { - .id = NULL, - } + PROTOCOL_ENTRY("arp" , arpAttributes , VIR_NWFILTER_RULE_PROTOCOL_ARP), + PROTOCOL_ENTRY("mac" , macAttributes , VIR_NWFILTER_RULE_PROTOCOL_MAC), + PROTOCOL_ENTRY("ip" , ipAttributes , VIR_NWFILTER_RULE_PROTOCOL_IP), + PROTOCOL_ENTRY("ipv6" , ipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_IPV6), + PROTOCOL_ENTRY("tcp" , tcpAttributes , VIR_NWFILTER_RULE_PROTOCOL_TCP), + PROTOCOL_ENTRY("udp" , udpAttributes , VIR_NWFILTER_RULE_PROTOCOL_UDP), + PROTOCOL_ENTRY("udplite", udpliteAttributes, VIR_NWFILTER_RULE_PROTOCOL_UDPLITE), + PROTOCOL_ENTRY("esp" , espAttributes , VIR_NWFILTER_RULE_PROTOCOL_ESP), + PROTOCOL_ENTRY("ah" , ahAttributes , VIR_NWFILTER_RULE_PROTOCOL_AH), + PROTOCOL_ENTRY("sctp" , sctpAttributes , VIR_NWFILTER_RULE_PROTOCOL_SCTP), + PROTOCOL_ENTRY("icmp" , icmpAttributes , VIR_NWFILTER_RULE_PROTOCOL_ICMP), + PROTOCOL_ENTRY("all" , allAttributes , VIR_NWFILTER_RULE_PROTOCOL_ALL), + PROTOCOL_ENTRY("igmp" , igmpAttributes , VIR_NWFILTER_RULE_PROTOCOL_IGMP), + PROTOCOL_ENTRY("tcp-ipv6" , tcpipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6), + PROTOCOL_ENTRY("udp-ipv6" , udpipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6), + PROTOCOL_ENTRY("udplite-ipv6", udpliteipv6Attributes, VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6), + PROTOCOL_ENTRY("esp-ipv6" , espipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6), + PROTOCOL_ENTRY("ah-ipv6" , ahipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6), + PROTOCOL_ENTRY("sctp-ipv6" , sctpipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6), + PROTOCOL_ENTRY("icmpv6" , icmpv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_ICMPV6), + PROTOCOL_ENTRY("all-ipv6" , allipv6Attributes , VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6), + PROTOCOL_ENTRY_LAST }; @@ -1176,7 +1117,7 @@ virNWFilterRuleDetailsParse(virConnectPt virNWFilterRuleDefPtr nwf, const virXMLAttr2Struct *att) { - int rc = 0; + int rc = 0, g_rc = 0; int idx = 0; char *prop; int found = 0; @@ -1194,7 +1135,7 @@ virNWFilterRuleDetailsParse(virConnectPt VIR_FREE(match); match = NULL; - while (att[idx].name != NULL && rc == 0) { + while (att[idx].name != NULL) { prop = virXMLPropString(node, att[idx].name); item = (nwItemDesc *)((char *)nwf + att[idx].dataIdx); @@ -1390,10 +1331,16 @@ virNWFilterRuleDetailsParse(virConnectPt } VIR_FREE(prop); } + + if (rc) { + g_rc = rc; + rc = 0; + } + idx++; } - return rc; + return g_rc; } @@ -2178,7 +2125,7 @@ virNWFilterPoolObjAssignDef(virConnectPt if (virNWFilterDefLoopDetect(conn, pools, def)) { virNWFilterReportError(conn, VIR_ERR_INVALID_NWFILTER, - "%s", _("filter would introduce loop")); + "%s", _("filter would introduce a loop")); return NULL; }