Re: kern/47749

[Sergio López <slp%sinrega.org@localhost>]

The following reply was made to PR kern/47749; it has been noted by GNATS.

From: =?ISO-8859-1?Q?Sergio_L=F3pez?= <slp%sinrega.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: martin%duskware.de@localhost, darrenr%NetBSD.org@localhost
Subject: Re: kern/47749
Date: Sat, 24 Aug 2013 11:03:31 +0200

 Apparently, IPF gets confused when trying to keep track of an
 ICMP_ECHOREPLY packet.
 
 The first time such a packet is found, fr_addstate from fr_scanlist
 (fil.c:2146) fails, and if no other rule matches, it returns the
 default "pass" value (without FR_KEEPSTATE flag), but leaves
 fin->fin_fr pointing to the rule (with FR_KEEPSTATE), and this
 structure is saved into frcache. So this first packet is considered to
 haven't match any rule, and unless IPF is in block by default, it
 passes the filter.
 
 But, when a second packet arrives, its "pass" value is taken from
 frcache, which contains the flags from the rule, including
 FR_KEEPSTATE. In consequence, fr_addstate fails again when called from
 fr_check (fil.c:2586), and this time the packet is marked as blocked.
 
 From an user's perspective, this can be avoided by not using "keep
 state" for icmp rules, or by doing so in both the input and output
 filters, as this will make all ICMP_ECHOREPLY packets to be matched by
 a previous ICMP_ECHO.
 
 But, as Martin said, I don't think IPF should behave this way. I think
 a sensible solution, without altering much of IPF behavior, could be
 something like this:
 
 --- fil.c.orig 2013-08-24 08:55:50.000000000 +0000
 +++ fil.c 2013-08-24 08:56:20.000000000 +0000
 @@ -2147,6 +2147,7 @@
   ATOMIC_INCL(frstats[out].fr_ads);
   } else {
   ATOMIC_INCL(frstats[out].fr_bads);
 + fin->fin_fr = NULL;
   pass = passo;
   continue;
   }
 
 This makes that both first and subsequent ICMP_ECHOREPLY packets
 without a related ICMP_ECHO, will not match against a rule with "keep
 state", much in the same way as (AFAIK) IPF from "-current" does.