netdev - Re: [PATCH v3] icmp: support rfc5837

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 19 Mar 2021 20:53:28 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Ishaan Gandhi <ishaangandhi@...il.com>,
        David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH v3] icmp: support rfc5837

On Fri, Mar 19, 2021 at 7:54 PM David Ahern <dsahern@...il.com> wrote:
>
> On 3/19/21 10:11 AM, Ishaan Gandhi wrote:
> > Thank you. Would it be better to do instead:
> >
> > +     if_index = skb->skb_iif;
> >
> > or
> >
> > +     if_index = ip_version == 4 ? inet_iif(skb) : skb->skb_iif;
> >
>
> If the packet comes in via an interface assigned to a VRF, skb_iif is
> most likely the VRF index which is not what you want.
>
> The general problem of relying on skb_iif was discussed on v1 and v2 of
> your patch. Returning an iif that is a VRF, as an example, leaks
> information about the networking configuration of the device which from
> a quick reading of the RFC is not the intention.
>
> Further, the Security Considerations section recommends controls on what
> information can be returned where you have added a single sysctl that
> determines if all information or none is returned. Further, it is not a
> a per-device control but a global one that applies to all net devices -
> though multiple entries per netdevice has a noticeable cost in memory at
> scale.
>
> In the end it seems to me the cost benefit is not there for a feature
> like this.

The sysctl was my suggestion. The detailed filtering suggested in the
RFC would add more complexity, not helping that cost benefit analysis.
I cared mostly about being able to disable this feature outright as it has
obvious risks.

But perhaps that is overly simplistic. The RFC suggests deciding trusted
recipients based on destination address. With a sysctl this feature can be
only deployed when all possible recipients are trusted, i.e., on an isolated
network. That is highly limiting.

Perhaps a per-netns trusted subnet prefix?

The root admin should always be able to override and disable this outright.