Possible mistake in commit 3ca459eaba1b ("tun: fix group permission check")

[stsp]

29.01.2025 01:59, Willem de Bruijn пишет:
> stsp wrote:
>> By doing that you indeed avoid
>> the problem of "completely
>> inaccessible tap". However, that
>> breaks my setup, as I really
>> intended to provide tap to the
>> owner and the unrelated group.
>> This is because, eg when setting
>> a CI job, you can add the needed
>> user to the needed group, but
>> you also need to re-login, which
>> is not always possible. :(
> Could you leave tun->owner unset?

That's exactly the problem: when
the user is not in the needed group,
then you need to unset _both_.
Unsetting only owner is not enough.
Adding the user to the group is not
enough because then you need to
re-login (bad for CI jobs).

I actually tried to address the
supplementary groups problem:
https://lore.kernel.org/lkml/20241108204102.1752206-1-stsp2@yandex.ru/T/
but nothing came out, so I have
to walk around multiple projects,
talking them into a new semantics
and representing the problems
like this one. If people instead
concentrate on solving the inability
to change the supplementary group
list, nothing like this would ever
happen. :)

>> Also completely ignoring group
>> when the user is set, is somewhat
>> questionable. At the very least,
>> perhaps then you need to explicitly
>> clear the group when the user
>> is set, to avoid the confusion.
>> Having "either user or group"
>> sounds like a sensible semantic,
>> but its a different semantic.
> True. I think that would have satisfied the intent of adding the
> group check at the time, and would have avoided this situation.
>
> But we indeed cannot retroactively restrict allowed behavior.
> As that will break users.
>
> Conversely, it might be that an existing user out there depends on
> the prior behavior that only a process that matches both user and
> group can use the device. Which might be reason for reverting the
> patch entirely.
But this is not an option too, let
me remind the previous situation:
1. If the user is in the group, then
    the group doesn't have any effect
    at all.
2. if the user is not in the group -
    no one can access the device.

"either-or" semantic is a direct fix
to that, as it represents case 1 and
fixes case 2. My semantic covers the
real-world situation of inability to
change the group list, but it needs
further tweaking and discussing.
Applying "either-or" may be feasible,
but the complete revert looks like
returning to a quite broken state.