netdev - Re: [PATCH iproute2] ip route: broken logic when using default word and family not specified

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]

Message-ID: <20171216132115.7d924d8d@xeon-e3>
Date:   Sat, 16 Dec 2017 13:21:15 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Alexander Zubkov <zubkov318@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] ip route: broken logic when using default word
 and family not specified

On Sat, 18 Nov 2017 14:12:48 +0100
Alexander Zubkov <zubkov318@...il.com> wrote:

> Hello,
> 
> I have found odd behaviour when using "ip route list" (and other bound
> commands) with prefix "default".
> 
> When family not specified, its value is completely ignored and "ip
> route list default" shows all inet4 prefixes. Same do "ip route list
> exact default" and "ip route list match default". Examples are at the
> end of the message.
> 
> When family is specified, the behaviour changes and default works as
> expected (=0.0.0.0/0 for inet4 and =::/0 for inet6). The above
> commands all shows only default prefix in the output and only "root
> default" shows all prefixes.
> 
> I tried to dig into the code and found that when default is using with
> unspecified family - the resulting structures filter.[mr]dst will
> actually become all-zeroes as in the case when nothing is specified.
> 
> I propose to change this in such a way (see attached patch). When
> default prefix is parsed, the flag PREFIXLEN_SPECIFIED is attached to
> it too, like for prefixes with "/<masklen>". It seems logical to me,
> because "/0" is really implied by "default" and even directly set up
> in the code:
> 
> dst->bitlen = 0;
> 
> Then during filtering there is additional logic for unspecified family
> and specified prefix.
> 
> With this patch ip route list commands shown above are working as
> expected. And it also works with unspecified table when routes are
> printed from different families.
> 
> Examples after applying the patch:
> 
> # ./ip route list
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0 proto kernel scope link src 192.168.0.1
> 192.168.1.0/24 via 192.168.0.3 dev eth0
> # ./ip -6 route list
> fe80::/64 dev eth0 proto kernel metric 256 pref medium
> fe80:1::/64 via fe80::3 dev eth0 metric 1024 pref medium
> default via fe80::2 dev eth0 metric 1024 pref medium
> # ./ip route list default
> default via 192.168.0.2 dev eth0
> # ./ip route list exact default
> default via 192.168.0.2 dev eth0
> # ./ip route list match default
> default via 192.168.0.2 dev eth0
> # ./ip route list root default
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0 proto kernel scope link src 192.168.0.1
> 192.168.1.0/24 via 192.168.0.3 dev eth0
> # ./ip route list default table all
> default via 192.168.0.2 dev eth0
> unreachable default dev lo proto kernel metric 4294967295 error -101 pref medium
> default via fe80::2 dev eth0 metric 1024 pref medium
> unreachable default dev lo proto kernel metric 4294967295 error -101 pref medium
> unreachable default dev lo proto kernel metric 4294967295 error -101 pref medium
> # ./ip route list root default table all
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0 proto kernel scope link src 192.168.0.1
> ...
> ff00::/8 dev eth0 table local metric 256 pref medium
> unreachable default dev lo proto kernel metric 4294967295 error -101 pref medium
> 
> And before patch:
> 
> # ip route list default
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0  proto kernel  scope link  src 192.168.0.1
> 192.168.1.0/24 via 192.168.0.3 dev eth0
> # ip route list match default
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0  proto kernel  scope link  src 192.168.0.1
> 192.168.1.0/24 via 192.168.0.3 dev eth0
> # ip route list exact default
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0  proto kernel  scope link  src 192.168.0.1
> 192.168.1.0/24 via 192.168.0.3 dev eth0
> # ip -4 route list exact default
> default via 192.168.0.2 dev eth0
> # ip -6 route list exact default
> default via fe80::2 dev eth0  metric 1024
> # ip route list exact default table all
> default via 192.168.0.2 dev eth0
> 192.168.0.0/24 dev eth0  proto kernel  scope link  src 192.168.0.1
> ...
> local fe80::3c09:19ff:feee:9866 dev lo  table local  proto none  metric 0
> ff00::/8 dev eth0  table local  metric 256
> unreachable default dev lo  table unspec  proto kernel  metric
> 4294967295  error -101

I think this is fine, and see no problem.
The patch is missing 'Signed-off-by' line which is required for iproute2;
the overall rules for iproute2 are (mostly) the same as the kernel.

Please resubmit with Signed-off-by