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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 19 Mar 2021 22:24:04 -0600
From:   David Ahern <dsahern@...il.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...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 3/19/21 6:53 PM, Willem de Bruijn wrote:
> 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.
> 

sure a sysctl is definitely required for a feature like this.

>From my perspective to be useful the control needs to be per interface
(e.g., management interface vs dataplane devices) and that has a higher
cost. Add in the amount of information returned and we know from other
examples that some users will want to limit which data is returned and
that increases the number of sysctls per device.

On top of that there is the logic of resolving what is the right device
and its information to return. There is are multiple layers - nic port,
bond, vlan, bridge, vrf, macvlan - each of which might be relevant. The
RFC referenced unnumbered devices as the ingress device. It seems like a
means for leaking information which comes back to the sysctl for proper
controls.

At the end of the day, what is the value of this feature vs the other
ICMP probing set?