linux-kernel - Re: [PATCH net-next] tun: fix group permission check

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: <673bb45c6f64b_200fa9294ee@willemb.c.googlers.com.notmuch>
Date: Mon, 18 Nov 2024 16:40:44 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Stas Sergeev <stsp2@...dex.ru>, 
 linux-kernel@...r.kernel.org
Cc: Stas Sergeev <stsp2@...dex.ru>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Jason Wang <jasowang@...hat.com>, 
 Andrew Lunn <andrew+netdev@...n.ch>, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 netdev@...r.kernel.org, 
 agx@...xcpu.org, 
 jdike@...ux.intel.com
Subject: Re: [PATCH net-next] tun: fix group permission check

Willem de Bruijn wrote:
> Stas Sergeev wrote:
> > Currently tun checks the group permission even if the user have matched.
> > Besides going against the usual permission semantic, this has a
> > very interesting implication: if the tun group is not among the
> > supplementary groups of the tun user, then effectively no one can
> > access the tun device. CAP_SYS_ADMIN still can, but its the same as
> > not setting the tun ownership.
> > 
> > This patch relaxes the group checking so that either the user match
> > or the group match is enough. This avoids the situation when no one
> > can access the device even though the ownership is properly set.
> > 
> > Also I simplified the logic by removing the redundant inversions:
> > tun_not_capable() --> !tun_capable()
> > 
> > Signed-off-by: Stas Sergeev <stsp2@...dex.ru>
> 
> This behavior goes back through many patches to commit 8c644623fe7e:
> 
>     [NET]: Allow group ownership of TUN/TAP devices.
> 
>     Introduce a new syscall TUNSETGROUP for group ownership setting of tap
>     devices. The user now is allowed to send packages if either his euid or
>     his egid matches the one specified via tunctl (via -u or -g
>     respecitvely). If both, gid and uid, are set via tunctl, both have to
>     match.
> 
> The choice evidently was on purpose. Even if indeed non-standard.

I should clarify that I'm not against bringing this file in line with
normal user/group behavior.

Just want to give anyone a chance to speak up if they disagree and/or
recall why the code was originally written as it is.