netdev - Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

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:   Fri, 28 Jul 2017 12:52:17 -0700
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Hangbin Liu <liuhangbin@...il.com>,
        network dev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not
 null entry.

On Fri, Jul 28, 2017 at 10:39 AM, David Ahern <dsahern@...il.com> wrote:
> On 7/28/17 11:13 AM, Roopa Prabhu wrote:
>> for fibmatch, my original intent was to return with an error code.
>> This is similar
>> to the ipv4 behavior. One option is to keep the check in there and put
>> the 'fibmatch'
>> condition around it. But, i do want to make sure that for the fibmatch case,
>> it does not return an error directly on an existing prohibit route
>> entry in the fib.
>> This is probably doable by checking for appropriate
>> net->ipv6.ip6_prohibit_entry entries.
>>
>
> IPv4 does not have the notion of null_entry or prohibit route entries
> which makes IPv4 and IPv6 inconsistent - something we really need to be
> avoiding from a user experience.
>
> We have the following cases:
>
> # ip -4 rule  add to 172.16.60.0/24 prohibit
> # ip -4 route add prohibit 172.16.50.0/24
> # ip -6 rule  add to 6000::/120 prohibit
> # ip -6 route add prohibit 5000::/120
>
>
> Behavior before Roopa's patch set:
>   Rule match:
>     # ip ro get 172.16.60.1
>     RTNETLINK answers: Permission denied
>
>     # ip -6 ro get 6000::1
>     prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
> 4294967295  error -13 pref medium
>
>   Route match:
>     # ip ro get 172.16.50.1
>     RTNETLINK answers: Permission denied
>
>     # ip -6 ro get 5000::1
>     prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
> 1024  error -13 pref medium
>
>
> Behavior after Roopa's patch set:
>   Rule match:
>     # ip ro get 172.16.60.1
>     RTNETLINK answers: Permission denied
>
>     # ip -6 ro get 6000::1
>     RTNETLINK answers: Permission denied
>
>   Route match:
>     # ip ro get 172.16.50.1
>     RTNETLINK answers: Permission denied
>
>     # ip -6 ro get 5000::1
>     RTNETLINK answers: Permission denied
>
>
> So Roopa's fibmatch patches brings consistency between IPv4 and IPv6 at
> the cost of breaking backwards compatibility for IPv6 when the prohibit
> or blackhole routes are hit.
>
> If that is not acceptable, then let's wrap the change in 'if (fibmatch)'
> so that when fibmatch is requested we have consistency between IPv4 and
> IPv6 when it is set.


David, Thanks for listing all the cases and options.

for the route match fibmatch case, if a prohibit route entry exists
(added by user), I was hoping fibmatch can return that entry...

 # ip -6 ro get fibmatch 5000::1
    prohibit 5000::1 from :: dev lo

because the semantics of fibmatch is to return the matching route
entry if exists.
I am assuming that is possible with appropriate checks around the
dst.error check for fibmatch. what do you say ?
I need to verify if this can work for ipv4 the same way.