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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

stsp wrote:
> 17.11.2024 18:04, Willem de Bruijn пишет:
> > 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@yandex.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.
> 
> So what would you suggest?
> Added Guido Guenther <agx@sigxcpu.org> to CC
> for an opinion.
> The main problem here is that by
> setting user and group properly, you
> end up with device inaccessible by
> anyone, unless the user belongs to
> the tun group. I don't think someone
> wants to set up inaccessible devices,
> so this property doesn't seem useful.
> OTOH if the user does have that group
> in his list, then, AFAICT, adding such
> group to tun changes nothing: neither
> limits nor extends the scope.
> If you had group already set and you
> set also user, then you limit the scope,
> but its the same as just setting user alone.
> So I really can't think of any valid usage
> scenario of setting both tun user and tun
> group.

Understood. If no one comments before the window reopens, I think it's
fine to just resubmit.