netdev - Re: [PATCH net-next] net: Enable neighbor sysctls that is save for userns root

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]

Message-ID: <8f5bb6e3-0ab4-c4a9-56a2-affe61223021@fb.com>
Date:   Fri, 10 Dec 2021 11:38:43 -0800
From:   Joanne Koong <joannekoong@...com>
To:     <cgel.zte@...il.com>, <davem@...emloft.net>
CC:     <kuba@...nel.org>, <ebiederm@...ssion.com>,
        <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
        <daniel@...earbox.net>, xu xin <xu.xin16@....com.cn>,
        Zeal Robot <zealci@....com.cn>
Subject: Re: [PATCH net-next] net: Enable neighbor sysctls that is save for
 userns root

On 12/8/21 12:58 AM, cgel.zte@...il.com wrote:

> From: xu xin <xu.xin16@....com.cn>
>
> Inside netns owned by non-init userns, sysctls about ARP/neighbor is
> currently not visible and configurable.
>
> For the attributes these sysctls correspond to, any modifications make
> effects on the performance of networking(ARP, especilly) only in the
> scope of netns, which does not affect other netns.
>
> Actually, some tools via netlink can modify these attribute. iproute2 is
> an example. see as follows:
>
> $ unshare -ur -n
> $ cat /proc/sys/net/ipv4/neigh/lo/retrans_time
> cat: can't open '/proc/sys/net/ipv4/neigh/lo/retrans_time': No such file
> or directory
> $ ip ntable show dev lo
> inet arp_cache
>      dev lo
>      refcnt 1 reachable 19494 base_reachable 30000 retrans 1000
>      gc_stale 60000 delay_probe 5000 queue 101
>      app_probes 0 ucast_probes 3 mcast_probes 3
>      anycast_delay 1000 proxy_delay 800 proxy_queue 64 locktime 1000
>
> inet6 ndisc_cache
>      dev lo
>      refcnt 1 reachable 42394 base_reachable 30000 retrans 1000
>      gc_stale 60000 delay_probe 5000 queue 101
>      app_probes 0 ucast_probes 3 mcast_probes 3
>      anycast_delay 1000 proxy_delay 800 proxy_queue 64 locktime 0
> $ ip ntable change name arp_cache dev <if> retrans 2000
> inet arp_cache
>      dev lo
>      refcnt 1 reachable 22917 base_reachable 30000 retrans 2000
>      gc_stale 60000 delay_probe 5000 queue 101
>      app_probes 0 ucast_probes 3 mcast_probes 3
>      anycast_delay 1000 proxy_delay 800 proxy_queue 64 locktime 1000
>
> inet6 ndisc_cache
>      dev lo
>      refcnt 1 reachable 35524 base_reachable 30000 retrans 1000
>      gc_stale 60000 delay_probe 5000 queue 101
>      app_probes 0 ucast_probes 3 mcast_probes 3
>      anycast_delay 1000 proxy_delay 800 proxy_queue 64 locktime 0
>
> Reported-by: Zeal Robot <zealci@....com.cn>
> Signed-off-by: xu xin <xu.xin16@....com.cn>
> ---
This LGTM. The neighbour sysctls are registered to the net namespace
associated with the neigh_parms. Any changes made to a net namespace
will be locally scoped to that net namespace (changes won't affect any other
net namespace).

There is also no possibility of a non-privileged user namespace messing 
up the
net namespace sysctls it shares with its parent user namespace. When a 
new user
namespace is created without unsharing the network namespace (eg calling
clone()  with CLONE_NEWUSER), the new user namespace shares its
parent's network namespace. Write access is protected by the mode set
in the sysctl ctl_table (and enforced by procfs). Here in the case of 
the neighbour
sysctls, 0644 is set for every sysctl; only the user owner has write access.


Acked-by: Joanne Koong <joannekoong@...com>
>   net/core/neighbour.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 0cdd4d9ad942..44d90cc341ea 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -3771,10 +3771,6 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
>   			neigh_proc_base_reachable_time;
>   	}
>   
> -	/* Don't export sysctls to unprivileged users */
> -	if (neigh_parms_net(p)->user_ns != &init_user_ns)
> -		t->neigh_vars[0].procname = NULL;
> -
>   	switch (neigh_parms_family(p)) {
>   	case AF_INET:
>   	      p_name = "ipv4";