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]

Message-ID: <CAM_iQpVobEXqE-CWVpQU4b8jnOrzq9EEfszj+SnyqX-RZ+B6Bw@mail.gmail.com>
Date:   Mon, 31 Jul 2017 11:37:44 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Roopa Prabhu <roopa@...ulusnetworks.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
>

There must be a reason why we allocate prohibit entries
dynamically for IPv6 despite we already have a (relatively)
static one.

>From this point of view, we need to dump them, that is,
restore the behavior before Roopa's patch.


>
> 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.
>

There are already many differences between IPv4 and
IPv6 behaviors, I don't see why this one is so special
that we have to make it consistent.