Changes from V1 to V2 of this patch - I had reversed the logic thinking that icmp type 0 is a echo request,but it's reply -- needed to reverse the logic - Found that ebtables takes the --ip-tos argument only as a hex number This patch enables the skipping of some of the ICMP traffic rules on the iptables level under certain circumstances so that the following filter properly enables unidirectional pings: <filter name='testcase'> <uuid>d6b1a2af-def6-2898-9f8d-4a74e3c39558</uuid> <!-- allow incoming ICMP Echo Request --> <rule action='accept' direction='in' priority='500'> <icmp type='8'/> </rule> <!-- allow outgoing ICMP Echo Reply --> <rule action='accept' direction='out' priority='500'> <icmp type='0'/> </rule> <!-- drop all other ICMP traffic --> <rule action='drop' direction='inout' priority='600'> <icmp/> </rule> </filter> Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com&gt; --- src/nwfilter/nwfilter_ebiptables_driver.c | 108 +++++++++++++++++------------- 1 file changed, 64 insertions(+), 44 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 @@ -1022,6 +1022,12 @@ err_exit: * @ifname : The name of the interface to apply the rule to * @vars : A map containing the variables to resolve * @res : The data structure to store the result(s) into + * @match : optional string for state match + * @accept_target : where to jump to on accepted traffic, i.e., "RETURN" + * "ACCEPT" + * @isIPv6 : Whether this is an IPv6 rule + * @maySkipICMP : whether this rule may under certain circumstances skip + * the ICMP rule from being created * * Convert a single rule into its representation for later instantiation * @@ -1039,7 +1045,8 @@ _iptablesCreateRuleInstance(int directio virNWFilterRuleInstPtr res, const char *match, const char *accept_target, - bool isIPv6) + bool isIPv6, + bool maySkipICMP) { char chain[MAX_CHAINNAME_LENGTH]; char number[20]; @@ -1265,6 +1272,10 @@ _iptablesCreateRuleInstance(int directio if (HAS_ENTRY_ITEM(&rule->p.icmpHdrFilter.dataICMPType)) { const char *parm; + + if (maySkipICMP) + goto exit_no_error; + if (rule->prtclType == VIR_NWFILTER_RULE_PROTOCOL_ICMP) parm = "--icmp-type"; else @@ -1386,6 +1397,10 @@ err_exit: return -1; +exit_no_error: + virBufferFreeAndReset(&buf); + + return 0; } @@ -1401,15 +1416,19 @@ iptablesCreateRuleInstance(virNWFilterDe int directionIn = 0; char chainPrefix[2]; int needState = 1; + bool maySkipICMP, inout = false; if ((rule->tt == VIR_NWFILTER_RULE_DIRECTION_IN) || (rule->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT)) { directionIn = 1; needState = 0; + inout = (rule->tt == VIR_NWFILTER_RULE_DIRECTION_INOUT); } chainPrefix[0] = 'F'; + maySkipICMP = directionIn || inout; + chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP; rc = _iptablesCreateRuleInstance(directionIn, chainPrefix, @@ -1421,10 +1440,14 @@ iptablesCreateRuleInstance(virNWFilterDe needState ? MATCH_STATE_OUT : NULL, "RETURN", - isIPv6); + isIPv6, + maySkipICMP); if (rc) return rc; + + maySkipICMP = !directionIn || inout; + chainPrefix[1] = CHAINPREFIX_HOST_OUT_TEMP; rc = _iptablesCreateRuleInstance(!directionIn, chainPrefix, @@ -1436,10 +1459,13 @@ iptablesCreateRuleInstance(virNWFilterDe needState ? MATCH_STATE_IN : NULL, "ACCEPT", - isIPv6); + isIPv6, + maySkipICMP); if (rc) return rc; + maySkipICMP = directionIn; + chainPrefix[0] = 'H'; chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP; rc = _iptablesCreateRuleInstance(directionIn, @@ -1451,9 +1477,8 @@ iptablesCreateRuleInstance(virNWFilterDe res, NULL, "ACCEPT", - isIPv6); - if (rc) - return rc; + isIPv6, + maySkipICMP); return rc; } @@ -1750,9 +1775,9 @@ ebtablesCreateRuleInstance(char chainPre } if (HAS_ENTRY_ITEM(&rule->p.ipHdrFilter.ipHdr.dataDSCP)) { - if (printDataType(vars, - number, sizeof(number), - &rule->p.ipHdrFilter.ipHdr.dataDSCP)) + if (printDataTypeAsHex(vars, + number, sizeof(number), + &rule->p.ipHdrFilter.ipHdr.dataDSCP)) goto err_exit; virBufferVSprintf(&buf,