Hi Stefano MithilMithil,
On Mon, 24 Feb 2020, Stefano Brivio wrote:
> In ip_set_match_extensions(), for sets with counters, we take care of
> updating counters themselves by calling ip_set_update_counter(), and of
> checking if the given comparison and values match, by calling
> ip_set_match_counter() if needed.
>
> However, if a given comparison on counters doesn't match the configured
> values, that doesn't mean the set entry itself isn't matching.
>
> This fix restores the behaviour we had before commit 4750005a85f7
> ("netfilter: ipset: Fix "don't update counters" mode when counters used
> at the matching"), without reintroducing the issue fixed there: back
> then, mtype_data_match() first updated counters in any case, and then
> took care of matching on counters.
>
> Now, if the IPSET_FLAG_SKIP_COUNTER_UPDATE flag is set,
> ip_set_update_counter() will anyway skip counter updates if desired.
>
> The issue observed is illustrated by this reproducer:
>
> ipset create c hash:ip counters
> ipset add c 192.0.2.1
> iptables -I INPUT -m set --match-set c src --bytes-gt 800 -j DROP
>
> if we now send packets from 192.0.2.1, bytes and packets counters
> for the entry as shown by 'ipset list' are always zero, and, no
> matter how many bytes we send, the rule will never match, because
> counters themselves are not updated.
Sorry, but I disagree. ipset behaves the same as iptables itself: the
counters are increased when the whole rule matches and that includes the
counter comparison as well. I think it's less counter-intuitive that one
can create never matching rules than to explain that "counter matching is
a non-match for the point of view of 'when the rule matches, update the
counter'".
What's really missing is a decrement-counters flag: that way one could
store different "quotas" for the elements in a set.
Best regards,
Jozsef
> Reported-by: Mithil Mhatre <mmhatre@xxxxxxxxxx>
> Fixes: 4750005a85f7 ("netfilter: ipset: Fix "don't update counters" mode when counters used at the matching")
> Signed-off-by: Stefano Brivio <sbrivio@xxxxxxxxxx>
> ---
> net/netfilter/ipset/ip_set_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index 69c107f9ba8d..b140e38d9333 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -649,13 +649,14 @@ ip_set_match_extensions(struct ip_set *set, const struct ip_set_ext *ext,
> if (SET_WITH_COUNTER(set)) {
> struct ip_set_counter *counter = ext_counter(data, set);
>
> + ip_set_update_counter(counter, ext, flags);
> +
> if (flags & IPSET_FLAG_MATCH_COUNTERS &&
> !(ip_set_match_counter(ip_set_get_packets(counter),
> mext->packets, mext->packets_op) &&
> ip_set_match_counter(ip_set_get_bytes(counter),
> mext->bytes, mext->bytes_op)))
> return false;
> - ip_set_update_counter(counter, ext, flags);
> }
> if (SET_WITH_SKBINFO(set))
> ip_set_get_skbinfo(ext_skbinfo(data, set),
> --
> 2.25.0
>
>
-
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
