Created attachment 156767 [details] patch net.link.tap.user_open does two thing: * prevents non-root users to open /dev/tapN * prevents non-root users to clone /dev/tapN The first function is performed by the node permissions, as set by the admin. There is no need for this additional way to limit users to open devices. The second function refers to the legacy cloning process. When some process attempts to open the non-existent device, devfs tries to call 'clone' functions of all available modules to see if they can auto-create such device. 'tapclone' is the relevant function in 'tap' module. There is another sysctl variable net.link.tap.devfs_cloning, that currently allows/disallows cloning for everybody, and then net.link.tap.user_open checks for PRIV_NET_IFCREATE when set. The new behavior is that net.link.tap.devfs_cloning also checks credentials based on PRIV_NET_IFCREATE, like this is currently for tunN. Practically speaking, net.link.tap.user_open is always in the way of every user process which needs to use /dev/tapN, while such limitation is not necessary at all. Ex. user being in the 'network' group should be entirely sufficient. Admin should set these permissions. Also, I doubt that PRIV_NET_IFCREATE even works properly, because I wasn't able to clone /dev/tapN even when my user is in wheel and network groups, and /dev is owned by root:wheel and has 0777 mask. I still got 'Permission denied'. I suggest to apply this patch to 11 (trunk) only, because it incurs an interface change. Also the following note should be added to the Change Log for 11.0: * net.link.tap.user_open sysctl variable is deprecated. Opening of /dev/tapN is now based on the node permissions and user credentials only. The meaning of net.link.tap.devfs_cloning has changed: when set to non-zero it allows /dev/tapN cloning to users with PRIV_NET_IFCREATE privilege.
I'm attempting to merge tun and tap, then I'll look at deprecating it.
CC'ing rwatson@- PRIV_NET_TAP is allocated and also used for allowing opening of tap devices, because this is historically a superuser-only privilege. It was added after the user_open sysctl and we currently honor PRIV_NET_TAP xor user_open; it seems like PRIV_NET_TAP should've pushed user_open towards deprecation in favor of MAC policy to more cleanly do the same thing. I'm not sure now what the correct behavior is- your point about groups is good, but do we want to (also, can we?) do away with PRIV_NET_TAP in favor of relying on group membership? > Also, I doubt that PRIV_NET_IFCREATE even works properly, because I wasn't able to clone /dev/tapN even when my user is in wheel and network groups, and /dev is owned by root:wheel and has 0777 mask. I still got 'Permission denied'. Basically all PRIV_* are only granted to root by default without a policy to grant them otherwise, so this is correct behavior.
While a MAC policy is a good idea, simple group policy is very easy to implement and I expect I'm not the only one using that in the wild (together with the sysctl). I would definitely like the sysctl to go away and group policy to "just work". I have no strong opinions on a MAC policy.
(In reply to Kyle Evans from comment #2) Yuri makes excellent points w.r.t. the sysctl, and I agree that we should just get rid of it (in 13 now). I'm less sure about getting rid of PRIV_NET_TAP, because creating the interface is distinct from opening (and using) it. We may still want to express letting users use an existing interface without immediately allowing them to create new ones.
(In reply to Kristof Provost from comment #4) PRIV_NET_TAP currently only applies to the opening of a tapN interface; PRIV_NET_IFCREATE for cloning. I'm also in favor of letting group policy dictate it, but PRIV_NET_TAP is the barrier when we remove the sysctl.
Created attachment 207504 [details] git(1) diff Attaching the route I'm planning to take after sorting out a method to mark this sysctl as deprecated beforehand in https://reviews.freebsd.org/D21662 -- rip out the sysctl and add an option, TAP_OPEN_PRIVILEGED (OFF by default), for letting interested parties re-enable super user policy on their kernel. This is a lightweight, low-maintenance option.
Can/should this be merged or is is considered breaking (and relnotes) ? If not set mfc-stable* to -
(In reply to Kubilay Kocak from comment #7) Half of it gets MFC'd- the half where the sysctl gets marked as deprecated to announce to anyone that views/tries to use it that it's deprecated.
(In reply to Kyle Evans from comment #8) Roger that, thank you for clarifying
A commit references this bug: Author: kevans Date: Mon Oct 21 14:38:12 UTC 2019 New revision: 353798 URL: https://svnweb.freebsd.org/changeset/base/353798 Log: tuntap(4): restrict scope of net.link.tap.user_open slightly net.link.tap.user_open has historically allowed non-root users to do devfs cloning and open /dev/tap* nodes based on permissions. Loosen this up to make it only allow users to do devfs cloning -- we no longer check it in tunopen. This allows tap devices to be created that can actually be opened by a user, rather than swiftly restricting them to root because the magic sysctl has not been set. The sysctl has not yet been completely deprecated, because more thought is needed for how to handle the devfs cloning case. There is not an easy suitable replacement for the sysctl there, and more care needs to be placed in determining whether that's OK or not. PR: 200185 Changes: head/UPDATING head/sys/net/if_tuntap.c
^Triage: committed back in 2019.