Re: [PATCH net-next] tun: fix group permission check
From: Willem de Bruijn
Date: Mon Nov 18 2024 - 16:40:58 EST
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@xxxxxxxxx>
>
> 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.