[PATCH nft] nftables: xt: fix misprint in nft_xt_compatible_revision

[PATCH nft] nftables: xt: fix misprint in nft_xt_compatible_revision

[Pavel Tikhomirov]

The rev variable is used here instead of opt obviously by mistake.
Please see iptables:nft_compatible_revision() for an example how it
should be.

This breaks revision compatibility checks completely when reading
compat-target rules from nft utility. That's why nftables can't work on
"old" kernels which don't support new revisons. That's a problem for
containers.

E.g.: 0 and 1 is supported but not 2:
https://git.sw.ru/projects/VZS/repos/vzkernel/browse/net/netfilter/xt_nat.c#111

Reproduce of the problem on Virtuozzo 7 kernel
3.10.0-1160.11.1.vz7.172.18 in centos 8 container:

  iptables-nft -t nat -N TEST
  iptables-nft -t nat -A TEST -j DNAT --to-destination 172.19.0.2
  nft list ruleset > nft.ruleset
  nft -f - < nft.ruleset
  #/dev/stdin:19:67-81: Error: Range has zero or negative size
  #		meta l4proto tcp tcp dport 81 counter packets 0 bytes 0 dnat to 3.0.0.0-0.0.0.0
  #		                                                                ^^^^^^^^^^^^^^^

  nft -v
  #nftables v0.9.3 (Topsy)
  iptables-nft -v
  #iptables v1.8.7 (nf_tables)

Kernel returns ip range in rev 0 format:

  crash> p *((struct nf_nat_ipv4_multi_range_compat *) 0xffff8ca2fabb3068)
  $5 = {
    rangesize = 1,
    range = {{
        flags = 3,
        min_ip = 33559468,
        max_ip = 33559468,

But nft reads this as rev 2 format (nf_nat_range2) which does not have
rangesize, and thus flugs 3 is treated as ip 3.0.0.0, which is wrong and
can't be restored later.

(Should probably be the same on Centos 7 kernel 3.10.0-1160.11.1)

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 src/xt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/xt.c b/src/xt.c
index f39acf30..789de992 100644
--- a/src/xt.c
+++ b/src/xt.c
@@ -321,7 +321,7 @@ static int nft_xt_compatible_revision(const char *name, uint8_t rev, int opt)
 	struct nfgenmsg *nfg;
 	int ret = 0;
 
-	switch (rev) {
+	switch (opt) {
 	case IPT_SO_GET_REVISION_MATCH:
 		family = NFPROTO_IPV4;
 		type = 0;
-- 
2.29.2

Re: [PATCH nft] nftables: xt: fix misprint in nft_xt_compatible_revision

[Pavel Tikhomirov]

Florian, sorry for miscopying your name in CC from git...

On 3/9/21 6:09 PM, Pavel Tikhomirov wrote:
> The rev variable is used here instead of opt obviously by mistake.
> Please see iptables:nft_compatible_revision() for an example how it
> should be.
> 
> This breaks revision compatibility checks completely when reading
> compat-target rules from nft utility. That's why nftables can't work on
> "old" kernels which don't support new revisons. That's a problem for
> containers.
> 
> E.g.: 0 and 1 is supported but not 2:
> https://git.sw.ru/projects/VZS/repos/vzkernel/browse/net/netfilter/xt_nat.c#111
> 
> Reproduce of the problem on Virtuozzo 7 kernel
> 3.10.0-1160.11.1.vz7.172.18 in centos 8 container:
> 
>    iptables-nft -t nat -N TEST
>    iptables-nft -t nat -A TEST -j DNAT --to-destination 172.19.0.2
>    nft list ruleset > nft.ruleset
>    nft -f - < nft.ruleset
>    #/dev/stdin:19:67-81: Error: Range has zero or negative size
>    #		meta l4proto tcp tcp dport 81 counter packets 0 bytes 0 dnat to 3.0.0.0-0.0.0.0
>    #		                                                                ^^^^^^^^^^^^^^^
> 
>    nft -v
>    #nftables v0.9.3 (Topsy)
>    iptables-nft -v
>    #iptables v1.8.7 (nf_tables)
> 
> Kernel returns ip range in rev 0 format:
> 
>    crash> p *((struct nf_nat_ipv4_multi_range_compat *) 0xffff8ca2fabb3068)
>    $5 = {
>      rangesize = 1,
>      range = {{
>          flags = 3,
>          min_ip = 33559468,
>          max_ip = 33559468,
> 
> But nft reads this as rev 2 format (nf_nat_range2) which does not have
> rangesize, and thus flugs 3 is treated as ip 3.0.0.0, which is wrong and
> can't be restored later.
> 
> (Should probably be the same on Centos 7 kernel 3.10.0-1160.11.1)
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>   src/xt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/xt.c b/src/xt.c
> index f39acf30..789de992 100644
> --- a/src/xt.c
> +++ b/src/xt.c
> @@ -321,7 +321,7 @@ static int nft_xt_compatible_revision(const char *name, uint8_t rev, int opt)
>   	struct nfgenmsg *nfg;
>   	int ret = 0;
>   
> -	switch (rev) {
> +	switch (opt) {
>   	case IPT_SO_GET_REVISION_MATCH:
>   		family = NFPROTO_IPV4;
>   		type = 0;
> 

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.

Re: [PATCH nft] nftables: xt: fix misprint in nft_xt_compatible_revision

[Pablo Neira Ayuso]

On Tue, Mar 09, 2021 at 06:09:15PM +0300, Pavel Tikhomirov wrote:
> The rev variable is used here instead of opt obviously by mistake.
> Please see iptables:nft_compatible_revision() for an example how it
> should be.
> 
> This breaks revision compatibility checks completely when reading
> compat-target rules from nft utility. That's why nftables can't work on
> "old" kernels which don't support new revisons. That's a problem for
> containers.

Applied, thanks.