netdev - Re: [PATCH iproute2 2/2] ip tunnel: warn when changing IPv6 tunnel without tunnel name

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Jul 2019 15:14:51 -0700
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Andrea Claudi <aclaudi@...hat.com>
Cc:     linux-netdev <netdev@...r.kernel.org>, stephen@...workplumber.org,
        dsahern@...nel.org
Subject: Re: [PATCH iproute2 2/2] ip tunnel: warn when changing IPv6 tunnel
 without tunnel name

On Tue, Jul 9, 2019 at 6:16 AM Andrea Claudi <aclaudi@...hat.com> wrote:
>
> Tunnel change fails if a tunnel name is not specified while using
> 'ip -6 tunnel change'. However, no warning message is printed and
> no error code is returned.
>
> $ ip -6 tunnel add ip6tnl1 mode ip6gre local fd::1 remote fd::2 tos inherit ttl 127 encaplimit none dev dummy0
> $ ip -6 tunnel change dev dummy0 local 2001:1234::1 remote 2001:1234::2
> $ ip -6 tunnel show ip6tnl1
> ip6tnl1: gre/ipv6 remote fd::2 local fd::1 dev dummy0 encaplimit none hoplimit 127 tclass inherit flowlabel 0x00000 (flowinfo 0x00000000)
>
> This commit checks if tunnel interface name is equal to an empty
> string: in this case, it prints a warning message to the user.
> It intentionally avoids to return an error to not break existing
> script setup.
>
> This is the output after this commit:
> $ ip -6 tunnel add ip6tnl1 mode ip6gre local fd::1 remote fd::2 tos inherit ttl 127 encaplimit none dev dummy0
> $ ip -6 tunnel change dev dummy0 local 2001:1234::1 remote 2001:1234::2
> Tunnel interface name not specified
> $ ip -6 tunnel show ip6tnl1
> ip6tnl1: gre/ipv6 remote fd::2 local fd::1 dev dummy0 encaplimit none hoplimit 127 tclass inherit flowlabel 0x00000 (flowinfo 0x00000000)
>
> Reviewed-by: Matteo Croce <mcroce@...hat.com>
> Signed-off-by: Andrea Claudi <aclaudi@...hat.com>

I tried your patch and the commands that I posted in my (previous) patch.

Here is the output after reverting my patch and applying your patch

<show command>
------------------------
vm0:/tmp# ./ip -6 tunnel add ip6tnl1 mode ip6gre local fd::1 remote
fd::2 tos inherit ttl 127 encaplimit none
vm0:/tmp# ./ip -6 tunnel show dev ip6tnl1
vm0:/tmp# echo $?
0

here the output is NULL and return code is 0. This is wrong and I
would expect to see the tunnel info (as displayed in 'ip -6 tunnel
show ip6tnl1')

<change command>
lpaa10:/tmp# ip -6 tunnel change dev ip6tnl1 local 2001:1234::1 remote
2001:1234::2 encaplimit none ttl 127 tos inherit allow-localremote
lpaa10:/tmp# echo $?
0
lpaa10:/tmp# ip -6 tunnel show dev ip6tnl1
lpaa10:/tmp# ip -6 tunnel show ip6tnl1
ip6tnl1: gre/ipv6 remote fd::2 local fd::1 encaplimit none hoplimit
127 tclass inherit flowlabel 0x00000 (flowinfo 0x00000000)

the change command appeared to be successful but change wasn't applied
(expecting the allow-localremote to be present on the tunnel).
---
>  ip/ip6tunnel.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/ip/ip6tunnel.c b/ip/ip6tunnel.c
> index 999408ed801b1..e3da11eb4518e 100644
> --- a/ip/ip6tunnel.c
> +++ b/ip/ip6tunnel.c
> @@ -386,6 +386,9 @@ static int do_add(int cmd, int argc, char **argv)
>         if (parse_args(argc, argv, cmd, &p) < 0)
>                 return -1;
>
> +       if (!*p.name)
> +               fprintf(stderr, "Tunnel interface name not specified\n");
> +
>         if (p.proto == IPPROTO_GRE)
>                 basedev = "ip6gre0";
>         else if (p.i_flags & VTI_ISVTI)
> --
> 2.20.1
>