[PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Florian Westphal]

Jozsef says:
 The correct behaviour is that if we have
 ipset create test1 hash:net,iface
 ipset add test1 0.0.0.0/0,eth0
 iptables -A INPUT -m set --match-set test1 src,src

 then the rule should match for any traffic coming in through eth0.

This removes the -EINVAL runtime test to make matching work
in case packet arrived via the specified interface.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1297092
Signed-off-by: Florian Westphal <fw@strlen.de>
---
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 43d8c98..f0f688d 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -164,8 +164,6 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	if (e.cidr == 0)
-		return -EINVAL;
 	if (adt == IPSET_TEST)
 		e.cidr = HOST_MASK;
 
@@ -377,8 +375,6 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
 	};
 	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
 
-	if (e.cidr == 0)
-		return -EINVAL;
 	if (adt == IPSET_TEST)
 		e.cidr = HOST_MASK;
 
-- 
2.4.10

Re: [PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Jozsef Kadlecsik]

Hi Florian,

On Tue, 12 Jan 2016, Florian Westphal wrote:

> Jozsef says:
>  The correct behaviour is that if we have
>  ipset create test1 hash:net,iface
>  ipset add test1 0.0.0.0/0,eth0
>  iptables -A INPUT -m set --match-set test1 src,src
> 
>  then the rule should match for any traffic coming in through eth0.
> 
> This removes the -EINVAL runtime test to make matching work
> in case packet arrived via the specified interface.

No, the patch actually would break the set type. In order to support /0 
prefixes, cidr + 1 is stored internally. Zero value means "empty 
slot/bucket".

> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1297092

>From the bugreport we neither know the kernel version nor the exact 
iptables command. It might be that the rule is added to a chain from where 
the input interface is not available.

Best regards,
Jozsef

> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
> index 43d8c98..f0f688d 100644
> --- a/net/netfilter/ipset/ip_set_hash_netiface.c
> +++ b/net/netfilter/ipset/ip_set_hash_netiface.c
> @@ -164,8 +164,6 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
>  	};
>  	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>  
> -	if (e.cidr == 0)
> -		return -EINVAL;
>  	if (adt == IPSET_TEST)
>  		e.cidr = HOST_MASK;
>  
> @@ -377,8 +375,6 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
>  	};
>  	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>  
> -	if (e.cidr == 0)
> -		return -EINVAL;
>  	if (adt == IPSET_TEST)
>  		e.cidr = HOST_MASK;
>  
> -- 
> 2.4.10
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Florian Westphal]

Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> wrote:
> Hi Florian,
> 
> On Tue, 12 Jan 2016, Florian Westphal wrote:
> 
> > Jozsef says:
> >  The correct behaviour is that if we have
> >  ipset create test1 hash:net,iface
> >  ipset add test1 0.0.0.0/0,eth0
> >  iptables -A INPUT -m set --match-set test1 src,src
> > 
> >  then the rule should match for any traffic coming in through eth0.
> > 
> > This removes the -EINVAL runtime test to make matching work
> > in case packet arrived via the specified interface.
> 
> No, the patch actually would break the set type. In order to support /0 
> prefixes, cidr + 1 is stored internally. Zero value means "empty 
> slot/bucket".

Hmm, but matching is broken currently, the rule quoted above never
matches.  And its exaclty because of if (e.cidr == 0) is true.

Tested with nf-next tree.
Before patch: never matches
After patch: matches for all packets from eth0

Re: [PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Jozsef Kadlecsik]

On Tue, 12 Jan 2016, Florian Westphal wrote:

> Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> wrote:
> > 
> > On Tue, 12 Jan 2016, Florian Westphal wrote:
> > 
> > > Jozsef says:
> > >  The correct behaviour is that if we have
> > >  ipset create test1 hash:net,iface
> > >  ipset add test1 0.0.0.0/0,eth0
> > >  iptables -A INPUT -m set --match-set test1 src,src
> > > 
> > >  then the rule should match for any traffic coming in through eth0.
> > > 
> > > This removes the -EINVAL runtime test to make matching work
> > > in case packet arrived via the specified interface.
> > 
> > No, the patch actually would break the set type. In order to support /0 
> > prefixes, cidr + 1 is stored internally. Zero value means "empty 
> > slot/bucket".
> 
> Hmm, but matching is broken currently, the rule quoted above never
> matches.  And its exaclty because of if (e.cidr == 0) is true.
> 
> Tested with nf-next tree.
> Before patch: never matches
> After patch: matches for all packets from eth0

I'll look into it, then something just resurfaces this way.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Jozsef Kadlecsik]

On Tue, 12 Jan 2016, Florian Westphal wrote:

> Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> wrote:
> > 
> > On Tue, 12 Jan 2016, Florian Westphal wrote:
> > 
> > > Jozsef says:
> > >  The correct behaviour is that if we have
> > >  ipset create test1 hash:net,iface
> > >  ipset add test1 0.0.0.0/0,eth0
> > >  iptables -A INPUT -m set --match-set test1 src,src
> > > 
> > >  then the rule should match for any traffic coming in through eth0.
> > > 
> > > This removes the -EINVAL runtime test to make matching work
> > > in case packet arrived via the specified interface.
> > 
> > No, the patch actually would break the set type. In order to support /0 
> > prefixes, cidr + 1 is stored internally. Zero value means "empty 
> > slot/bucket".
> 
> Hmm, but matching is broken currently, the rule quoted above never
> matches.  And its exaclty because of if (e.cidr == 0) is true.
> 
> Tested with nf-next tree.
> Before patch: never matches
> After patch: matches for all packets from eth0

I was wrong and the patch is correct, the removed condition is just 
unnecessary and breaks to match /0. I have already applied the patch in 
the ipset tree and added a new test to verify the case. Thanks, Florian!

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the patch to the nf tree, thanks!

On Tue, 12 Jan 2016, Florian Westphal wrote:

> Jozsef says:
>  The correct behaviour is that if we have
>  ipset create test1 hash:net,iface
>  ipset add test1 0.0.0.0/0,eth0
>  iptables -A INPUT -m set --match-set test1 src,src
> 
>  then the rule should match for any traffic coming in through eth0.
> 
> This removes the -EINVAL runtime test to make matching work
> in case packet arrived via the specified interface.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1297092
> Signed-off-by: Florian Westphal <fw@strlen.de>

Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Best regards,
Jozsef

> ---
> diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
> index 43d8c98..f0f688d 100644
> --- a/net/netfilter/ipset/ip_set_hash_netiface.c
> +++ b/net/netfilter/ipset/ip_set_hash_netiface.c
> @@ -164,8 +164,6 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
>  	};
>  	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>  
> -	if (e.cidr == 0)
> -		return -EINVAL;
>  	if (adt == IPSET_TEST)
>  		e.cidr = HOST_MASK;
>  
> @@ -377,8 +375,6 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
>  	};
>  	struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
>  
> -	if (e.cidr == 0)
> -		return -EINVAL;
>  	if (adt == IPSET_TEST)
>  		e.cidr = HOST_MASK;
>  
> -- 
> 2.4.10
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH nf] ipset: allow a 0 netmask with hash_netiface type

[Pablo Neira Ayuso]

On Wed, Jan 13, 2016 at 09:35:02AM +0100, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please apply the patch to the nf tree, thanks!
> 
> On Tue, 12 Jan 2016, Florian Westphal wrote:
> 
> > Jozsef says:
> >  The correct behaviour is that if we have
> >  ipset create test1 hash:net,iface
> >  ipset add test1 0.0.0.0/0,eth0
> >  iptables -A INPUT -m set --match-set test1 src,src
> > 
> >  then the rule should match for any traffic coming in through eth0.
> > 
> > This removes the -EINVAL runtime test to make matching work
> > in case packet arrived via the specified interface.
> > 
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1297092
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> 
> Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Applied, thanks!

This applies cleanly to 4.3 and 4.4, so will be requesting -stable for
these two.