xt_recent broken in kernel 3.19.0

xt_recent broken in kernel 3.19.0

[Chris Vine]

With kernel 3.19.0, the following iptables rule, where SSH_TRIES is set
to 4:

  iptables -D SSH_CHAIN -m conntrack --ctstate NEW \
    -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount $SSH_TRIES -j DROP

generates this error message in syslog:

  kernel: xt_recent: hitcount (4) is larger than packets to be
  remembered (4) for table DEFAULT

and the rule fails to install in the table.  No error is generated with
kernel 3.18.6.

Chris

Re: xt_recent broken in kernel 3.19.0 + PATCH

[Chris Vine]

On Wed, 11 Feb 2015 09:28:34 +0000
Chris Vine <chris@cvine.freeserve.co.uk> wrote:
> With kernel 3.19.0, the following iptables rule, where SSH_TRIES is
> set to 4:
> 
>   iptables -D SSH_CHAIN -m conntrack --ctstate NEW \
>     -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount
> $SSH_TRIES -j DROP
> 
> generates this error message in syslog:
> 
>   kernel: xt_recent: hitcount (4) is larger than packets to be
>   remembered (4) for table DEFAULT
> 
> and the rule fails to install in the table.  No error is generated
> with kernel 3.18.6.

The rule provoking this should of course have been the one appending
the rule, which is:

  iptables -A SSH_CHAIN -m conntrack --ctstate NEW \
    -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount $SSH_TRIES -j DROP

On looking at the code, the changes in the 3.19 kernel seem not to have
been tested and there is an off-by-one error.  The patch below restores
behaviour to be identical to that found in the 3.18 kernel.

Chris

--- linux-3.19.0/net/netfilter/xt_recent.c~	2015-02-10 09:18:44.657376355 +0000
+++ linux-3.19.0/net/netfilter/xt_recent.c	2015-02-11 17:58:33.311608835 +0000
@@ -378,7 +378,7 @@
 	mutex_lock(&recent_mutex);
 	t = recent_table_lookup(recent_net, info->name);
 	if (t != NULL) {
-		if (info->hit_count > t->nstamps_max_mask) {
+		if (info->hit_count > t->nstamps_max_mask + 1) {
 			pr_info("hitcount (%u) is larger than packets to be remembered (%u) for table %s\n",
 				info->hit_count, t->nstamps_max_mask + 1,
 				info->name);

Re: xt_recent broken in kernel 3.19.0 + PATCH

[Cong Wang]

(Cc'ing netdev and netfilter-devel lists)

On Wed, Feb 11, 2015 at 10:31 AM, Chris Vine
<chris@cvine.freeserve.co.uk> wrote:
> On Wed, 11 Feb 2015 09:28:34 +0000
> Chris Vine <chris@cvine.freeserve.co.uk> wrote:
>> With kernel 3.19.0, the following iptables rule, where SSH_TRIES is
>> set to 4:
>>
>>   iptables -D SSH_CHAIN -m conntrack --ctstate NEW \
>>     -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount
>> $SSH_TRIES -j DROP
>>
>> generates this error message in syslog:
>>
>>   kernel: xt_recent: hitcount (4) is larger than packets to be
>>   remembered (4) for table DEFAULT
>>
>> and the rule fails to install in the table.  No error is generated
>> with kernel 3.18.6.
>
> The rule provoking this should of course have been the one appending
> the rule, which is:
>
>   iptables -A SSH_CHAIN -m conntrack --ctstate NEW \
>     -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount $SSH_TRIES -j DROP
>
> On looking at the code, the changes in the 3.19 kernel seem not to have
> been tested and there is an off-by-one error.  The patch below restores
> behaviour to be identical to that found in the 3.18 kernel.
>
> Chris
>
> --- linux-3.19.0/net/netfilter/xt_recent.c~     2015-02-10 09:18:44.657376355 +0000
> +++ linux-3.19.0/net/netfilter/xt_recent.c      2015-02-11 17:58:33.311608835 +0000
> @@ -378,7 +378,7 @@
>         mutex_lock(&recent_mutex);
>         t = recent_table_lookup(recent_net, info->name);
>         if (t != NULL) {
> -               if (info->hit_count > t->nstamps_max_mask) {
> +               if (info->hit_count > t->nstamps_max_mask + 1) {
>                         pr_info("hitcount (%u) is larger than packets to be remembered (%u) for table %s\n",
>                                 info->hit_count, t->nstamps_max_mask + 1,
>                                 info->name);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: xt_recent broken in kernel 3.19.0 + PATCH

[Florian Westphal]

Cong Wang <xiyou.wangcong@gmail.com> wrote:
> (Cc'ing netdev and netfilter-devel lists)

Thanks for forwarding.

> > Chris Vine <chris@cvine.freeserve.co.uk> wrote:
> >>   iptables -D SSH_CHAIN -m conntrack --ctstate NEW \
> >>     -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount
> >> $SSH_TRIES -j DROP
> > --- linux-3.19.0/net/netfilter/xt_recent.c~     2015-02-10 09:18:44.657376355 +0000
> > +++ linux-3.19.0/net/netfilter/xt_recent.c      2015-02-11 17:58:33.311608835 +0000
> > @@ -378,7 +378,7 @@
> >         mutex_lock(&recent_mutex);
> >         t = recent_table_lookup(recent_net, info->name);
> >         if (t != NULL) {
> > -               if (info->hit_count > t->nstamps_max_mask) {
> > +               if (info->hit_count > t->nstamps_max_mask + 1) {

Looks good. Chris, could you formally submit this patch to
netfilter-devel@vger.kernel.org?

Thanks!

Re: xt_recent broken in kernel 3.19.0 + PATCH

[Chris Vine]

On Thu, 12 Feb 2015 09:35:33 +0100
Florian Westphal <fw@strlen.de> wrote:
> Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > (Cc'ing netdev and netfilter-devel lists)
> 
> Thanks for forwarding.
> 
> > > Chris Vine <chris@cvine.freeserve.co.uk> wrote:
> > >>   iptables -D SSH_CHAIN -m conntrack --ctstate NEW \
> > >>     -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount
> > >> $SSH_TRIES -j DROP
> > > --- linux-3.19.0/net/netfilter/xt_recent.c~     2015-02-10
> > > 09:18:44.657376355 +0000 +++
> > > linux-3.19.0/net/netfilter/xt_recent.c      2015-02-11
> > > 17:58:33.311608835 +0000 @@ -378,7 +378,7 @@
> > > mutex_lock(&recent_mutex); t = recent_table_lookup(recent_net,
> > > info->name); if (t != NULL) {
> > > -               if (info->hit_count > t->nstamps_max_mask) {
> > > +               if (info->hit_count > t->nstamps_max_mask + 1) {
> 
> Looks good. Chris, could you formally submit this patch to
> netfilter-devel@vger.kernel.org?
> 
> Thanks!

Done.

Chris

Re: xt_recent broken in kernel 3.19.0 + PATCH

[Chris Vine]

On Thu, 12 Feb 2015 10:26:16 +0000
Chris Vine <chris@cvine.freeserve.co.uk> wrote:
> On Thu, 12 Feb 2015 09:35:33 +0100
> Florian Westphal <fw@strlen.de> wrote:
> > Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > (Cc'ing netdev and netfilter-devel lists)
> > 
> > Thanks for forwarding.
> > 
> > > > Chris Vine <chris@cvine.freeserve.co.uk> wrote:
> > > >>   iptables -D SSH_CHAIN -m conntrack --ctstate NEW \
> > > >>     -m recent --update --seconds $SSH_LOGIN_PERIOD --hitcount
> > > >> $SSH_TRIES -j DROP
> > > > --- linux-3.19.0/net/netfilter/xt_recent.c~     2015-02-10
> > > > 09:18:44.657376355 +0000 +++
> > > > linux-3.19.0/net/netfilter/xt_recent.c      2015-02-11
> > > > 17:58:33.311608835 +0000 @@ -378,7 +378,7 @@
> > > > mutex_lock(&recent_mutex); t = recent_table_lookup(recent_net,
> > > > info->name); if (t != NULL) {
> > > > -               if (info->hit_count > t->nstamps_max_mask) {
> > > > +               if (info->hit_count > t->nstamps_max_mask + 1) {
> > 
> > Looks good. Chris, could you formally submit this patch to
> > netfilter-devel@vger.kernel.org?
> > 
> > Thanks!
> 
> Done.

On further testing I see that that patch only solves the problem if
SSH_TRIES is set to a power of two boundary.  You still get an error
loading the rule if it is anything else.  I think there is something
wrong with the nstamp_mask heuristic which is used here.

Chris

Re: xt_recent broken in kernel 3.19.0 + PATCH

[Chris Vine]

On Thu, 12 Feb 2015 10:54:17 +0000
Chris Vine <chris@cvine.freeserve.co.uk> wrote:
[snip]
> On further testing I see that that patch only solves the problem if
> SSH_TRIES is set to a power of two boundary.  You still get an error
> loading the rule if it is anything else.  I think there is something
> wrong with the nstamp_mask heuristic which is used here.

I now find that that is not right either.  I had to rmmod xt_recent to
get it to drop its previous setting.  With that done, the patch does
indeed seem to work for all values of SSH_TRIES.

Chris