netdev - Re: [PATCH iproute2 v2] iproute: lwtunnel set TUNNEL_KEY on LWTUNNEL_IP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20181220201554.609843c0@xeon-e3>
Date:   Thu, 20 Dec 2018 20:15:54 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     wenxu@...oud.cn
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2 v2] iproute: lwtunnel set TUNNEL_KEY on
 LWTUNNEL_IP_FLAGS

On Thu, 20 Dec 2018 08:17:27 +0800
wenxu@...oud.cn wrote:

> From: wenxu <wenxu@...oud.cn>
> 
> ip l add dev tun type gretap external
> ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 dev gretap
> 
> For gretap example when the command set the id but don't set the
> TUNNEL_KEY flags. There is no key field in the send packet
> 
> Signed-off-by: wenxu <wenxu@...oud.cn>
> ---
>  include/uapi/linux/if_tunnel.h |   20 ++++++++++++++++++++
>  ip/iproute_lwtunnel.c          |   11 ++++++++++-
>  2 files changed, 30 insertions(+), 1 deletions(-)
> 
> diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
> index ecdc766..c7f0a5e 100644
> --- a/include/uapi/linux/if_tunnel.h
> +++ b/include/uapi/linux/if_tunnel.h

Already picked up header from kernel update

> diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
> index aee18ac..c659035 100644
> --- a/ip/iproute_lwtunnel.c
> +++ b/ip/iproute_lwtunnel.c
> @@ -18,6 +18,7 @@
>  #include <string.h>
>  #include <linux/ila.h>
>  #include <linux/lwtunnel.h>
> +#include <linux/if_tunnel.h>
>  #include <linux/mpls_iptunnel.h>
>  #include <errno.h>
>  
> @@ -31,7 +32,6 @@
>  #include <linux/seg6_iptunnel.h>
>  #include <linux/seg6_hmac.h>
>  #include <linux/seg6_local.h>
> -#include <net/if.h>
>  
>  static const char *format_encap_type(int type)
>  {
> @@ -780,12 +780,14 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
>  	char **argv = *argvp;
>  	int argc = *argcp;
>  	int ret = 0;
> +	__u16 flags = 0;
>  
>  	while (argc > 0) {
>  		if (strcmp(*argv, "id") == 0) {
>  			__u64 id;
>  
>  			NEXT_ARG();
> +			flags |= TUNNEL_KEY;

How is this backwards compatible with older kernels? You are always setting
flag. The design of the kernel attribute may be broken and have to be
reverted.

Old iproute2 has to work on 4.21 and new iproute2 has to work on old kernels
(even 2.6.32).


A bigger problem is you added a way to set the option without a corresponding way
to display the current settings. You need to look for and decode the netlink
attribute. And once again the API has to work backwards and forwards.