On Wed, Apr 07, 2010 at 11:44:01AM -0400, Stefan Berger wrote:

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='0'/> </rule> <!-- allow outgoing ICMP Echo Reply --> <rule action='accept' direction='out' priority='500'> <icmp type='8'/> </rule> <!-- drop all other ICMP traffic --> <rule action='drop' direction='inout' priority='600'> <icmp/> </rule> </filter>

Signed-off-by: Stefan Berger <stefanb@us.ibm.com>

--- 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; }

ACK, looks fine, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/