diff mbox series

[nf,v2] netfilter: nft_tunnel: Fix don't convert tun id to host endian

Message ID 1564040633-25728-1-git-send-email-wenxu@ucloud.cn
State Not Applicable
Delegated to: Pablo Neira
Headers show
Series [nf,v2] netfilter: nft_tunnel: Fix don't convert tun id to host endian | expand

Commit Message

wenxu July 25, 2019, 7:43 a.m. UTC
From: wenxu <wenxu@ucloud.cn>

In the action store tun_id to reg in a host endian. But the
nft_cmp action get the user data in a net endian which lead
match failed.

nft --debug=netlink add rule netdev firewall aclin ip daddr 10.0.0.7
tunnel tun_id 1000 fwd to eth0

the expr tunnel tun_id 1000 --> [ cmp eq reg 1 0xe8030000 ]:
the cmp expr set the tun_id 1000 in network endian.

So the tun_id should be store as network endian. Which is the
same as nft_payload match. 

[ meta load protocol => reg 1 ]
[ cmp eq reg 1 0x00000008 ]
[ payload load 4b @ network header + 16 => reg 1 ]
[ cmp eq reg 1 0x0700000a ]
[ tunnel load id => reg 1 ]
[ cmp eq reg 1 0xe8030000 ]
[ immediate reg 1 0x0000000f ]
[ fwd sreg_dev 1 ]

Fixes: aaecfdb5c5dd ("netfilter: nf_tables: match on tunnel metadata")
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 net/netfilter/nft_tunnel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Pablo Neira Ayuso July 25, 2019, 8:31 a.m. UTC | #1
On Thu, Jul 25, 2019 at 03:43:53PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> In the action store tun_id to reg in a host endian.

This is correct.

> But the nft_cmp action get the user data in a net endian which lead
> match failed.
> 
> nft --debug=netlink add rule netdev firewall aclin ip daddr 10.0.0.7
> tunnel tun_id 1000 fwd to eth0
> 
> the expr tunnel tun_id 1000 --> [ cmp eq reg 1 0xe8030000 ]:
> the cmp expr set the tun_id 1000 in network endian.
> 
> So the tun_id should be store as network endian. Which is the
> same as nft_payload match. 
> 
> [ meta load protocol => reg 1 ]
> [ cmp eq reg 1 0x00000008 ]
> [ payload load 4b @ network header + 16 => reg 1 ]
> [ cmp eq reg 1 0x0700000a ]
> [ tunnel load id => reg 1 ]
> [ cmp eq reg 1 0xe8030000 ]
> [ immediate reg 1 0x0000000f ]
> [ fwd sreg_dev 1 ]
> 
> Fixes: aaecfdb5c5dd ("netfilter: nf_tables: match on tunnel metadata")
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  net/netfilter/nft_tunnel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> index 3d4c2ae..c9f4585 100644
> --- a/net/netfilter/nft_tunnel.c
> +++ b/net/netfilter/nft_tunnel.c
> @@ -53,7 +53,7 @@ static void nft_tunnel_get_eval(const struct nft_expr *expr,
>  		     !(tun_info->mode & IP_TUNNEL_INFO_TX)) ||
>  		    (priv->mode == NFT_TUNNEL_MODE_TX &&
>  		     (tun_info->mode & IP_TUNNEL_INFO_TX)))
> -			*dest = ntohl(tunnel_id_to_key32(tun_info->key.tun_id));
> +			*dest = tunnel_id_to_key32(tun_info->key.tun_id);

Data is _never_ stored in network byteorder in registers.
wenxu July 25, 2019, 9:27 a.m. UTC | #2
sorry for this.

nftables should be set the token byetorder as BYTEORDER_HOST_ENDIAN


BR

wenxu

On 7/25/2019 4:31 PM, Pablo Neira Ayuso wrote:
> On Thu, Jul 25, 2019 at 03:43:53PM +0800, wenxu@ucloud.cn wrote:
>> From: wenxu <wenxu@ucloud.cn>
>>
>> In the action store tun_id to reg in a host endian.
> This is correct.
>
>> But the nft_cmp action get the user data in a net endian which lead
>> match failed.
>>
>> nft --debug=netlink add rule netdev firewall aclin ip daddr 10.0.0.7
>> tunnel tun_id 1000 fwd to eth0
>>
>> the expr tunnel tun_id 1000 --> [ cmp eq reg 1 0xe8030000 ]:
>> the cmp expr set the tun_id 1000 in network endian.
>>
>> So the tun_id should be store as network endian. Which is the
>> same as nft_payload match. 
>>
>> [ meta load protocol => reg 1 ]
>> [ cmp eq reg 1 0x00000008 ]
>> [ payload load 4b @ network header + 16 => reg 1 ]
>> [ cmp eq reg 1 0x0700000a ]
>> [ tunnel load id => reg 1 ]
>> [ cmp eq reg 1 0xe8030000 ]
>> [ immediate reg 1 0x0000000f ]
>> [ fwd sreg_dev 1 ]
>>
>> Fixes: aaecfdb5c5dd ("netfilter: nf_tables: match on tunnel metadata")
>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>> ---
>>  net/netfilter/nft_tunnel.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
>> index 3d4c2ae..c9f4585 100644
>> --- a/net/netfilter/nft_tunnel.c
>> +++ b/net/netfilter/nft_tunnel.c
>> @@ -53,7 +53,7 @@ static void nft_tunnel_get_eval(const struct nft_expr *expr,
>>  		     !(tun_info->mode & IP_TUNNEL_INFO_TX)) ||
>>  		    (priv->mode == NFT_TUNNEL_MODE_TX &&
>>  		     (tun_info->mode & IP_TUNNEL_INFO_TX)))
>> -			*dest = ntohl(tunnel_id_to_key32(tun_info->key.tun_id));
>> +			*dest = tunnel_id_to_key32(tun_info->key.tun_id);
> Data is _never_ stored in network byteorder in registers.
>
diff mbox series

Patch

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index 3d4c2ae..c9f4585 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -53,7 +53,7 @@  static void nft_tunnel_get_eval(const struct nft_expr *expr,
 		     !(tun_info->mode & IP_TUNNEL_INFO_TX)) ||
 		    (priv->mode == NFT_TUNNEL_MODE_TX &&
 		     (tun_info->mode & IP_TUNNEL_INFO_TX)))
-			*dest = ntohl(tunnel_id_to_key32(tun_info->key.tun_id));
+			*dest = tunnel_id_to_key32(tun_info->key.tun_id);
 		else
 			regs->verdict.code = NFT_BREAK;
 		break;