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]

Date:   Sun, 17 Dec 2017 13:06:53 +0100
From:   Alexander Zubkov <zubkov318@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] ip route: broken logic when using default word
 and family not specified

OK. I have found the recommendations for kernel developers and
resubmited both of my patches according to their guidelines (I hope).
I have used my other e-mail because I think gmail is not good with
text email format.

On Sat, Dec 16, 2017 at 10:21 PM, Stephen Hemminger
<stephen@...workplumber.org> wrote:
> 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