ipfw: ouch!, skip past end of rules, denying packet

[Oleg Bulyzhin]

On Thu, 6 May 2004, Luigi Rizzo wrote:

> On Fri, May 07, 2004 at 01:35:11AM +0400, Oleg Bulyzhin wrote:
> ...
> > i see.
> > There is a little bug (i'll PR it as soon i'll get enough time), you can
> > try attached patch(built on RELENG_4).
>
> very interesting that you found out what the bug was -- i
> couldn't realize it myself. Thanks!
>
> However, i believe the fix is incorrect and in principle can
> still trigger the problem (which is innocuous).
>
> The bug your patch addresses is the following:
> when a packet is stored in a pipe, dummynet keeps a pointer
> to the matching rule so if one_pass=0 it can restart processing
> from the following one.
>
> if the matching rule goes away while a packet is queued,
> my intention was to use the default rule as the next one,
> but i mistakenly used the default rule as the _matching_ one.
>
> Your patch replaces the matching rule with the next one,
> which however might still end up being the default rule;
> so it does not fix the proble, plus, it might completely
> subvert the packet's flow.

I've used dn_pkt.rule cause dn_pkt structure has no separate pointer for
'next rule', perhaps we should add it?
To my mind problem is not log pollution with 'skip past' messages but dropping
packets which _should_ be further processed. If we have no default_to_accept
option in kernel and our next rule is default rule - packed should be dropped.
If we are here (ip_fw2.c:1452):	----------------
						|
                if (fw_one_pass)		|
                        return 0;		|
						|
                f = args->rule->next_rule;  <---
                if (f == NULL)
                        f = lookup_next_rule(args->rule);

it means we got a packet which passed all rules up to pipe/queue rule and which
was not dropped inside dummynet (i.e. packet already passed rule which was
deleted).
Why we should just drop such 'die hard' packet? ;))

>
> I believe a proper fix is right before the main loop in
> ipfw_chk(), check if we enter with a NULL rule pointer and use the
> default rule instead.

Hmm... consider following ruleset:

net.inet.ip.fw.one_pass=0
ipfw pipe 1 config bw 1Mbit/s queue 8Kbytes

10	pipe 1 ip from any to 10.0.0.2 out
20	count ip from any to 10.0.0.2 out
65535	allow ip from any to any

i.e. we are limiting user's bandwidth and want to know how much was downloaded
through traffic shaper.
Then user pay us for 10Mbit/s bandwidth:

ipfw pipe 2 config bw 10Mbit/s queue 16Kbytes
ipfw set disable 30
ipfw add 10 set 30 pipe 2 ip from any to 10.0.0.2 out
ipfw set swap 30 0
#### NB! ####
ipfw delete set 30

There are 2 cases:
1) if we delete temporary set 30 as fast as possible:
   With current code up to 8Kbytes data will be dropped. With your suggestion
   default rule will be used (packets will be passed in our case) but
   rule 20 will not catch em.

2) if we delete temporary set 30 after 5 seconds (i.e. when queue became empty):
   packets will be passed & counted by rule 20

>
> Alternatively, if we believe that when a rule goes away
> we should also drop queued packets because the resulting
> behaviour would be unpredictable or unsafe, then what happens
> now is basically correct (not by design, just pure luck),
> and we just need to remove the 'ouch...' message

Well, i think packets should not be dropped until we delete corresponding pipe
or queue.

>
> 	cheers
> 	luigi
>

-- 
SY, Oleg.

================================================================
=== Oleg Bulyzhin -- OBUL-RIPN -- OBUL-RIPE -- oleg at rinet.ru ===
================================================================