Re: [PATCH net v3] net: require CAP_NET_ADMIN in the device netns for tunnel changelink

[Paolo Abeni <pabeni@redhat.com>]

On 6/4/26 2:50 PM, Maoyi Xie wrote:
> A tunnel changelink mutates the tunnel state in the device's creation
> netns. After an IFLA_NET_NS_FD migration that creation netns differs
> from the caller's netns. The rtnl changelink path only checks
> CAP_NET_ADMIN against the caller's netns, so a caller with caps only in
> its current netns can rewrite a tunnel that lives in the creation netns.
> They pick the endpoint addresses. Commit 8b484efd5cb4 ("ip6: vti: Use
> ip6_tnl.net in vti6_siocdevprivate().") added the same check on the
> ioctl path. This adds it on the RTM_NEWLINK path.
> 
> Gate each tunnel changelink on ns_capable against the creation netns, at
> the top of the op before any attribute is parsed or applied. The ipv4
> types need it there because the parsers can update live tunnel fields
> before ip_tunnel_changelink() runs, for example ipgre_netlink_parms()
> sets t->collect_md. The check is skipped when the creation netns equals
> the device's current netns (net_eq), where the existing CAP_NET_ADMIN
> check already applies and no extra LSM hook is wanted.
> 
> The newlink path has long checked the capability in the link netns. The
> changelink path never did.
> 
> Reported-by: Xiao Liang <shaw.leon@gmail.com>
> Closes: https://lore.kernel.org/netdev/CABAhCOSzP1vaThGV35_VnsRCb=87_CPjPVsTHbq905k8A+BuUg@mail.gmail.com/
> Fixes: d0f418516022 ("net, ip_tunnel: fix namespaces move")
> Fixes: 5311a69aaca3 ("net, ip6_tunnel: fix namespaces move")
> Fixes: 690afc165bb3 ("net: ip6_gre: fix moving ip6gre between namespaces")
> Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
> Fixes: 11b326fb0a37 ("ip6: vti: Use ip6_tnl.net in vti6_changelink().")
> Cc: stable@vger.kernel.org
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>

Since the fix is not centralized in a single place, I think it would be
better to split this in a series addressing each tunnel individually.

> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 169e2921a851..02328c9a3c07 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1457,6 +1457,10 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
>  	__u32 fwmark = t->fwmark;
>  	int err;
>  
> +	if (!net_eq(t->net, dev_net(dev)) &&
> +	    !ns_capable(t->net->user_ns, CAP_NET_ADMIN))

the above checks are replicated several times. I think it would be
better to place them in a new helper.

/P