nftables bug: Only the first two elements of a map are used for NAT

nftables bug: Only the first two elements of a map are used for NAT

[Simon Hanisch]

[-- Attachment #1.1: Type: text/plain, Size: 1547 bytes --]

Hi,

we want to use a map for a large NAT setup, mapping subnets to a single
IP. Thats why we wanted to use a map, but only the first two elements of
the map are used for the NAT.
I added two config examples to reproduce the bug. In the first example
the NAT works fine for the network 100.64.15.0/24, in the second it does
not. The only difference is the order of the map elements.
We have build nft from the current master branch, last commit is
97a2a5bde2f03e33315eab4b76a9e69770b99351.



Working for 100.64.15.0/24

#!/usr/sbin/nft
add chain nat postrouting { type nat hook postrouting priority 100 ;}
add chain nat prerouting { type nat hook prerouting priority 0 ;}
add map nat subnettoip { type ipv4_addr: ipv4_addr ; flags interval ; }
add rule ip nat postrouting snat ip saddr map @subnettoip;
add element nat subnettoip { 100.64.13.0/24 : 192.168.0.32 }
add element nat subnettoip { 100.64.15.0/24 : 192.168.0.34 }
add element nat subnettoip { 100.64.14.0/24 : 192.168.0.33 }


Not working for 100.64.15.0/24

#!/usr/sbin/nft
add chain nat postrouting { type nat hook postrouting priority 100 ;}
add chain nat prerouting { type nat hook prerouting priority 0 ;}
add map nat subnettoip { type ipv4_addr: ipv4_addr ; flags interval ; }
add rule ip nat postrouting snat ip saddr map @subnettoip;
add element nat subnettoip { 100.64.13.0/24 : 192.168.0.32 }
add element nat subnettoip { 100.64.14.0/24 : 192.168.0.33 }
add element nat subnettoip { 100.64.15.0/24 : 192.168.0.34 }


Greetings,
Simon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: nftables bug: Only the first two elements of a map are used for NAT

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]

Hi Simon,

On Wed, Feb 08, 2017 at 07:11:17PM +0100, Simon Hanisch wrote:
> Hi,
> 
> we want to use a map for a large NAT setup, mapping subnets to a single
> IP. Thats why we wanted to use a map, but only the first two elements of
> the map are used for the NAT.
> I added two config examples to reproduce the bug. In the first example
> the NAT works fine for the network 100.64.15.0/24, in the second it does
> not. The only difference is the order of the map elements.
> We have build nft from the current master branch, last commit is
> 97a2a5bde2f03e33315eab4b76a9e69770b99351.
> 
> 
> 
> Working for 100.64.15.0/24
> 
> #!/usr/sbin/nft
> add chain nat postrouting { type nat hook postrouting priority 100 ;}
> add chain nat prerouting { type nat hook prerouting priority 0 ;}
> add map nat subnettoip { type ipv4_addr: ipv4_addr ; flags interval ; }
> add rule ip nat postrouting snat ip saddr map @subnettoip;
> add element nat subnettoip { 100.64.13.0/24 : 192.168.0.32 }
> add element nat subnettoip { 100.64.15.0/24 : 192.168.0.34 }
> add element nat subnettoip { 100.64.14.0/24 : 192.168.0.33 }
> 
> 
> Not working for 100.64.15.0/24
> 
> #!/usr/sbin/nft
> add chain nat postrouting { type nat hook postrouting priority 100 ;}
> add chain nat prerouting { type nat hook prerouting priority 0 ;}
> add map nat subnettoip { type ipv4_addr: ipv4_addr ; flags interval ; }
> add rule ip nat postrouting snat ip saddr map @subnettoip;
> add element nat subnettoip { 100.64.13.0/24 : 192.168.0.32 }
> add element nat subnettoip { 100.64.14.0/24 : 192.168.0.33 }
> add element nat subnettoip { 100.64.15.0/24 : 192.168.0.34 }

Thanks for the report, I can indeed reproduce it here.

I made a wrong assumption on an earlier patchset that aimed to fix
this. Would you please give a try to this patch to see if this solves
the problem for you?

Thanks.

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 825 bytes --]

diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 71e8fb886a73..78dfbf9588b3 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -60,11 +60,10 @@ static bool nft_rbtree_lookup(const struct net *net, const struct nft_set *set,
 		d = memcmp(this, key, set->klen);
 		if (d < 0) {
 			parent = parent->rb_left;
-			/* In case of adjacent ranges, we always see the high
-			 * part of the range in first place, before the low one.
-			 * So don't update interval if the keys are equal.
-			 */
-			if (interval && nft_rbtree_equal(set, this, interval))
+			if (interval &&
+			    nft_rbtree_equal(set, this, interval) &&
+			    nft_rbtree_interval_end(this) &&
+			    !nft_rbtree_interval_end(interval))
 				continue;
 			interval = rbe;
 		} else if (d > 0)