[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] nwfilter: fix for directionality of ICMP traffic



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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]