Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v3 1/6] rtnetlink: allow RTM_SETLINK to reference other
 namespaces
To:     =?UTF-8?B?TWFoZXNoIEJhbmRld2FyICjgpK7gpLngpYfgpLYg4KSs4KSC4KSh4KWH4KS1?=
         =?UTF-8?B?4KS+4KSwKQ==?= <maheshb@google.com>
Cc:     nicolas.dichtel@6wind.com, linux-netdev <netdev@vger.kernel.org>,
        linux-kernel@vger.kernel.org, David Miller <davem@davemloft.net>
References: <20191107132755.8517-1-jonas@norrbonn.se>
 <20191107132755.8517-2-jonas@norrbonn.se>
 <CAF2d9jjRLZ07Qx0NJ9fi1iUpHn+qYEJ+cacKgBmeZ2FvZLObEQ@mail.gmail.com>
From:   Jonas Bonn <jonas@norrbonn.se>
Message-ID: <fff51fa7-5c42-7fa7-6208-d911b18bd91e@norrbonn.se>
Date:   Fri, 8 Nov 2019 09:20:40 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.2
MIME-Version: 1.0
In-Reply-To: <CAF2d9jjRLZ07Qx0NJ9fi1iUpHn+qYEJ+cacKgBmeZ2FvZLObEQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Mahesh,

On 07/11/2019 21:36, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Thu, Nov 7, 2019 at 5:30 AM Jonas Bonn <jonas@norrbonn.se> wrote:
>>
>>
>> +       /* A hack to preserve kernel<->userspace interface.
>> +        * It was previously allowed to pass the IFLA_TARGET_NETNSID
>> +        * attribute as a way to _set_ the network namespace.  In this
>> +        * case, the device interface was assumed to be in the  _current_
>> +        * namespace.
>> +        * If the device cannot be found in the target namespace then we
>> +        * assume that the request is to set the device in the current
>> +        * namespace and thus we attempt to find the device there.
>> +        */
> Could this bypasses the ns_capable() check? i.e. if the target is
> "foo" but your current ns is bar. The process may be "capable" is foo
> but the interface is not found in foo but present in bar and ends up
> modifying it (especially when you are not capable in bar)?

I don't think so.  There was never any capable-check for the "current" 
namespace so there's no change in that regard.

I do think there is an issue with this hack that I can't see any 
workaround for.  If the user specifies an interface (by name or index) 
for another namespace that doesn't exist, there's a potential problem if 
that name/index happens to exist in the "current" namespace.  In that 
case, one many end up inadvertently modifying the interface in the 
current namespace.  I don't see how to avoid that while maintaining the 
backwards compatibility.

My absolute preference would be to drop this compat-hack altogether. 
iproute2 doesn't use a bare TARGET_NETNSID in this manner (for changing 
namespaces) and I didn't find any other users by a quick search of other 
prominent Netlink users:  systemd, network-manager, connman.  This 
compat-hack is there for the _potential ab-user_ of the interface, not 
for any known such.

> 
>> +       if (!dev && tgt_net) {
>> +               net = sock_net(skb->sk);
>> +               if (ifm->ifi_index > 0)
>> +                       dev = __dev_get_by_index(net, ifm->ifi_index);
>> +               else if (tb[IFLA_IFNAME])
>> +                       dev = __dev_get_by_name(net, ifname);
>> +       }


/Jonas