Re: [PATCH] ipset: Update byte and packet counters regardless of whether they match

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

 



On Tue, 25 Feb 2020, Stefano Brivio wrote:

> > The logic could be changed in the user rules from
> > 
> > iptables -I INPUT -m set --match-set c src --bytes-gt 800 -j DROP
> > 
> > to
> > 
> > iptables -I INPUT -m set --match-set c src --bytes-lt 800 -j ACCEPT
> > [ otherwise DROP ]
> > 
> > but of course it might be not so simple, depending on how the rules are 
> > built up.
> 
> Yes, it would work, unless the user actually wants to check with the
> same counter how many bytes are sent "in excess".

You mean the counters are still updated whenever the element is matched in 
the set and then one could check how many bytes were sent over the 
threshold just by listing the set elements.

> Now, I see the conceptual problem about matching: if the rule isn't 
> matching, and counters count matched packets, counters shouldn't 
> increase. But still, I think there are a number of facts to be 
> considered:
> 
> - the man page says (and has said for a number of years):
> 
> 	If the packet is matched an element in the set, match only if
> 	the byte counter of the element is greater than the given value
> 	as well.
> 
>   which actually makes the problem undecidable: matching depends on
>   matching itself. Trying some "common sense" interpretation, I would
>   read this as:
> 
> 	If the packet matches an *element* in the set, this *rule* will
> 	match only if the byte counter of the element is greater than
> 	the given value.
> 
>   that is, by separating the meaning of "element matching" from "rule
>   matching", this starts making sense.

Yes, you are right. Sometimes I think I'm far from the best at writing 
documentation... So I'm going to update the manpage with your sentence.
 
> - I spent the past two hours trying to think of an actual case that was
>   affected by 4750005a85f7, *other than the "main" bug it fixes*, that
>   is, "! --update-counters" was ignored altogether, and I couldn't.
> 
>   Even if we had a --bytes-lt option, it would be counter-intuitive,
>   because the counter would be updated until bytes are less than the
>   threshold, and then the rule would stop matching, meaning that the
>   user most probably thinks:
> 
> 	"Drop matching packets as long as less than 800 bytes are sent"
> 
>   and what happens is:
> 
> 	"Count and drop matching packets until 800 bytes are sent, then
> 	stop dropping and counting them"

Again, yes, that's what would happen.

>   The only "functional" case I can think of is something like
>   --bytes-lt 800 -j ACCEPT. User probably thinks:
> 
> 	"Don't let more than 800 bytes go through"
> 
>   and what happens is:
> 
> 	"Let up to 800 bytes, or 799 bytes plus one packet, go through,
> 	counting the bytes in packets that were let through"
> 
>   which isn't much different from the expectation.
> 
> - and then,
> 
> > > Other than this, I'm a bit confused. How could --packets-gt and
> > > --bytes-gt be used, if counters don't increase as long as the rule
> > > doesn't match?  
> > 
> > I almost added to my previous mail that the 'ge' and 'gt' matches are not 
> > really useful at the moment...
> 
> ...yes, I can't think of any other use for those either.

Those could really be useful if the counters could be decremented. 
Otherwise I think the counter matching in the sets is not as useful as it 
seems to be.
 
> > > > What's really missing is a decrement-counters flag: that way one could 
> > > > store different "quotas" for the elements in a set.  
> > > 
> > > I see, that would work as well.  
> > 
> > The other possibility is to force counter update. I.e. instead of
> > 
> > iptables -I INPUT -m set --match-set c src --bytes-gt 800 -j DROP
> > 
> > something like
> > 
> > iptables -I INPUT -m set --match-set c src --update-counters \
> > 	--bytes-gt 800 -j DROP
> > 
> > but that also requires some internal changes to store a new flag, because 
> > at the moment only "! --update-counters" is supported. So there'd be then 
> > a fine-grained control over how the counters are updated:
> > 
> > - no --update-counters flag: update counters only if the whole rule 
> >   matches, including the counter matches
> > - --update-counters flag: update counters if counter matching is false
> 
> ...this should probably be "in any case", also if it's true.

Yes, but now I don't really like the name itself: --force-update-counters
or something like that would be more clear.

> > - ! --update-counters flag: don't update counters
> 
> I think that would fix the issue as well, I'm just struggling to find a
> sensible use case for the "no --update-counters" case -- especially one
> where there would be a substantial issue with the change I proposed.

The no update counter flag was introduced to handle when one needs to 
match in the same set multiple times, i.e. there are multiple rules with 
the same set. Like you need to match in the raw/mangle/filter tables as 
well. Unfortunately I can't recall the usercase.

Best regards,
Jozsef
-
E-mail  : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxx
PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics
          H-1525 Budapest 114, POB. 49, Hungary



[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux