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

[Paolo Abeni <pabeni@redhat.com>]

On 11/18/24 22:40, Willem de Bruijn wrote:
> 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@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.
> 
> 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.

I think we can't accept a behaviour changing patch this late in the
cycle. If an agreement is reached it should be reposted after the merge
window.

/P