Date:   Fri, 20 Mar 2020 09:21:52 -0600
From:   David Ahern <dsahern@...il.com>
To:     Vincent Bernat <vincent@...nat.ch>,
        David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, kuba@...nel.org, edumazet@...gle.com,
        kafai@...com
Subject: Re: [RFC PATCH net-next v1] net: core: enable SO_BINDTODEVICE for
 non-root users

[ sorry for the delay; out of commission for a few days ]

On 3/16/20 7:58 AM, Vincent Bernat wrote:
>  ❦ 16 mars 2020 02:13 -07, David Miller:
> 
>>> As a reminder, there are currently 3 APIs to specify a preferred device
>>> association which influences route lookups:
>>>
>>> 1. SO_BINDTODEVICE - sets sk_bound_dev_if and is the strongest binding
>>> (ie., can not be overridden),
>>>
>>> 2. IP_UNICAST_IF / IPV6_UNICAST_IF - sets uc_index / ucast_oif and is
>>> sticky for a socket, and
>>>
>>> 3. IP_PKTINFO / IPV6_PKTINFO - which is per message.
>>>
>>> The first, SO_BINDTODEVICE, requires root privileges. The last 2 do not
>>> require root privileges but only apply to raw and UDP sockets making TCP
>>> the outlier.
>>>
>>> Further, a downside to the last 2 is that they work for sendmsg only;
>>> there is no way to definitively match a response to the sending socket.
>>> The key point is that UDP and raw have multiple non-root APIs to dictate
>>> a preferred device for sending messages.
>>>
>>> Vincent's patch simplifies things quite a bit - allowing consistency
>>> across the protocols and directions - but without overriding any
>>> administrator settings (e.g., inherited bindings via ebpf programs).
>>
>> Understood, but I still wonder if this mis-match of privilege
>> requirements was by design or unintentional.

IP_UNICAST_IF and IPV6_UNICAST_IF were added for wine (76e21053b5bf3 and
c4062dfc425e9) specifically for use by non-root processes. It is not
clear in the commit message why only sendmsg was needed and why only
udp/raw. I have not touched wine in 15+ years, so no easy way for me to
look into how the response side worked. My guess is just relying on the
socket matching when sk_bound_dev_if is not set.

>>
>> Allowing arbitrary users to specify SO_BINDTODEVICE has broad and far
>> reaching consequences, so at a minimum if we are going to remove the
>> restriction we should at least discuss the implications.

certainly. I brought up this need for non-privileged binding at netconf
a few years ago.[0] My thought at that time was to make IP_UNICAST_IF /
IPV6_UNICAST_IF work for TCP. What I like about Vincent's proposed
change is that it checks for an existing setting and does not override.

As I mentioned before, UDP and RAW both have APIs to get around the root
requirement for sends and responses can match sockets for non-VRF
scenarios. So, the biggest change for this patch is to TCP (and VRF use
cases).

wrt to TCP, allowing the setting for daemons narrows the scope of
bind(), so that should be ok. The daemon could, with additional work,
just ignore connection requests it does not believe come through the
preferred device, though routing changes could move an established
connection to a different interface. Overall, this should not be an
impact to servers.

TCP clients are the biggest question mark wrt to evading an admin's
routing setup. I can't say I can make a persuasive argument here beyond
consistency - a concern for TCP connections but not UDP.

> 
> Without VRF, it's hard to build a case where a process could "evade" its
> setup using SO_BINDTODEVICE. It could be used to use alternative routing
> table when ip rules are using interfaces (which is not uncommon), but I
> think almost all such setups are using this to have some isolation in
> routing (not for local processes), something that could be replaced by
> VRF.
> 
> With VRF, removing the restriction allows a process to have more
> possibilities than previously if it has not been restricted. This is my
> use case: it is actually difficult to use VRF for anything else than
> routing because local processes may want to receive connections in a VRF
> and forward them to another one.
> 
> In summary, unless I am missing something, the main implication is when
> using VRF. Without VRF, no real change.
> 
> An alternative would be to use a sysctl to decide the behaviour.

That is consistent with other l3mdev/vrf features.

[0] https://lwn.net/Articles/719393/

Powered by blists - more mailing lists