netdev - Re: [PATCH net-next] veth: report NEWLINK event when moving the peer device in a new namespace

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:   Mon, 3 Sep 2018 20:54:59 -0600
From:   David Ahern <dsahern@...il.com>
To:     Thomas Haller <thaller@...hat.com>,
        Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] veth: report NEWLINK event when moving the peer
 device in a new namespace

On 9/3/18 3:10 AM, Thomas Haller wrote:
> Hi,
> 
> On Sat, 2018-09-01 at 17:45 -0600, David Ahern wrote:
>> On 9/1/18 3:05 AM, Lorenzo Bianconi wrote:
>>>
>>> I was thinking about the commit 38e01b30563a and then I realized I
>>> misread the code
>>> yesterday. The commit 38e01b30563a provides all relevant info but
>>> it
>>> emits the event
>>> for veth1 (the device moved in the new namespace).
>>> An userspace application will not receive that message if it
>>> filters
>>> events for just
>>> a specific device (veth0 in this case) despite that some device
>>> properties have changed
>>> (since veth0 and veth1 are paired devices). To fix that behavior in
>>> veth_notify routine
>>> I emits a RTM_NEWLINK event for veth0.
>>
>> Userspace is managing a veth a pair. It moves one of them to a new
>> namespace and decides to filter messages related to that device
>> before
>> the move. If it did not filter the DELLINK message it would get the
>> information you want. That to me is a userspace problem. What am I
>> missing?
> 
> The userspace component which moves the veth peer is likely not the
> same that is listening to these events.
> 
>> Fundamentally, your proposal means 3 messages when moving a device to
>> a
>> new namespace which is what I think is unnecessary.
> 
> You are correct, the necessary information is signaled in this case.
> 
> Usually, one can manage the information about one link by considering
> only RTM_NEWLINK/RTM_DELLINK message for that link. That seems
> desirable behavior of the netlink API, as it simplifies user space
> implementations.
> 
> For example, libnl3's cache for links doesn't properly handles this,
> and you end up with invalid data in the cache. You may call that a
> userspace problem and bug. But does it really need to be that
> complicated?
> 
> FWIW, a similar thing was also NACK'ed earlier:
>   http://lists.openwall.net/netdev/2016/06/12/8

And from what I can see, I agree with that NACK; a 3rd message is not
necessary. Userspace has the ability to learn what it needs and should
be looking at the existing options that bleed data across namespaces.

> 
> 
> 
> Paolo and Lorenzo pointed out another issue when moving the peer to a
> third namespace:
> 
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> 
> ip netns del ns1
> ip netns del ns2
> ip netns del ns3
> ip netns add ns1
> ip netns add ns2
> ip netns add ns3
> 
> 
> ip -n ns1 link add dev veth0 type veth peer name veth1
> ip -n ns1 monitor link &
> 
> ip -n ns1 link set veth1 netns ns2
> # Deleted 9: veth1@...h0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default 
> #    link/ether 86:79:1d:c6:45:bc brd ff:ff:ff:ff:ff:ff new-netnsid 0 new-ifindex 9
> 
> ip -n ns2 link set veth1 netns ns3
> # no event.


ip li add veth1 type veth peer name veth2
ip netns add foo
ip netns add bar
--> start ip monitor here; see below

ip li set veth2 netns foo
ip -netns foo li set veth2 netns bar


>From init_net:
$ ip monitor all-nsid
[nsid current]Deleted inet veth2
[nsid current]Deleted inet6 veth2
[nsid current]nsid 0 (iproute2 netns name: foo)
[nsid current]Deleted 16: veth2@...h1: <BROADCAST,MULTICAST,M-DOWN> mtu
1500 qdisc noop state DOWN group default
    link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff new-netns foo
new-ifindex 16
[nsid current]inet if16 forwarding on rp_filter off mc_forwarding off
proxy_neigh off ignore_routes_with_linkdown off
[nsid current]inet6 if16 forwarding off mc_forwarding off proxy_neigh
off ignore_routes_with_linkdown off
[nsid current]nsid 0 (iproute2 netns name: foo)
[nsid current]16: veth2@...7: <BROADCAST,MULTICAST> mtu 1500 qdisc noop
state DOWN group default
    link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff link-netnsid 0
[nsid current]Deleted inet veth2
[nsid current]Deleted inet6 veth2
[nsid current]nsid 1
[nsid current]Deleted 16: veth2@...7: <BROADCAST,MULTICAST> mtu 1500
qdisc noop state DOWN group default
    link/ether 0a:28:87:f9:17:7c brd ff:ff:ff:ff:ff:ff link-netnsid 0
new-netnsid 1 new-ifindex 16