From: David Woodhouse Twenty years ago I made creating a tuntap device require CAP_NET_ADMIN in commit ca6bb5d7ab22 ("[NET]: Require CAP_NET_ADMIN to create tuntap devices.") The idea was to allow the permissions on /dev/net/tun to be relaxed to 0666 and thus allow owners of tun devices assigned with TUNSETOWNER to actually open it and get at their devices. What we missed at the time was that persistent devices with *no* owner were already essentially world-consumable, and anyone who could open the device node could also attach to those. Even then, using persistent devices with no explicitly set owner — on the (false) presumption that CAP_NET_ADMIN would be required to open them — wasn't an egregious userspace bug. There are cases where it does make sense to separate the creation/configuration of the netdevice, from the process which shovels packets into it. Since then, TUNSETGROUP was added a year later in commit 8c644623fe7e ("[NET]: Allow group ownership of TUN/TAP devices."). Its commit message noted that if both uid and gid are set, both have to match (in itself a bizarre choice) — but still not that with *neither* set, no permission at all was required. Then in 2012 commit c8d68e6be1c3 ("tuntap: multiqueue support") allowed new queues to be attached to a device from the moment of its creation, with no way for userspace to close the race between creation and a subsequent TUNSET{OWNER,GROUP}. In 2024, commit 3ca459eaba1bf ("tun: fix group permission check") tried to fix things to be saner: only require the user *or* the group to match, in line with standard POSIX permissions, and require CAP_NET_ADMIN if neither the user nor group authorised the access (although it put the CAP_NET_ADMIN check first, and would wrongly have reported the capability as having been used even when it was not). That fix was reverted in commit a70c7b3cbc06 ("tun: revert fix group permission check") due to allegedly causing a regression in an selinux test suite which *expected* the attachment to succeed when authorised by none of user, group, nor CAP_NET_ADMIN: https://lore.kernel.org/netdev/CAFqZXNtkCBT4f+PwyVRmQGoT3p1eVa01fCG_aNtpt6dakXncUg@mail.gmail.com/ We should not break UAPI. But we don't have to consider an selinux test case which deliberately exercises a security flaw to be UAPI worth preserving either. To which end, I have fixed it in selinux-testsuite commit b2f894f0016b ("tests/tun_tap: set an owner on the device for the relabel tests"). So: fix the lack of CAP_NET_ADMIN check on devices which *haven't* been assigned an owner or group, while leaving the weird tun ownership model as it was (user *and* group have to match, if both are set). If someone still wants to fix the latter (which wasn't actually the reason the previous attempt was reverted), they can do it under separate cover. Permit attaching to an existing device only if an owner and/or group was set and the caller matches all that were set, or if the caller has CAP_NET_ADMIN. The only behavioural change is that a device with no owner or group now requires CAP_NET_ADMIN to attach — precisely the loophole we want to close. And put the CAP_NET_ADMIN check in the right place this time. Fixes: ca6bb5d7ab22 ("[NET]: Require CAP_NET_ADMIN to create tuntap devices.") Reported-by: Andrew Lutomirski Signed-off-by: David Woodhouse Cc: stable@vger.kernel.org --- drivers/net/tun.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 5a302709a68a..0002e1598e69 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -513,14 +513,18 @@ static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb, return ret; } -static inline bool tun_not_capable(struct tun_struct *tun) +static inline bool tun_permitted(struct tun_struct *tun) { const struct cred *cred = current_cred(); struct net *net = dev_net(tun->dev); - return ((uid_valid(tun->owner) && !uid_eq(cred->euid, tun->owner)) || - (gid_valid(tun->group) && !in_egroup_p(tun->group))) && - !ns_capable(net->user_ns, CAP_NET_ADMIN); + /* If both uid and gid are set, *both* must match */ + if ((uid_valid(tun->owner) || gid_valid(tun->group)) && + (!uid_valid(tun->owner) || uid_eq(cred->euid, tun->owner)) && + (!gid_valid(tun->group) || in_egroup_p(tun->group))) + return true; + + return ns_capable(net->user_ns, CAP_NET_ADMIN); } static void tun_set_real_num_queues(struct tun_struct *tun) @@ -2872,7 +2876,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) !!(tun->flags & IFF_MULTI_QUEUE)) return -EINVAL; - if (tun_not_capable(tun)) + if (!tun_permitted(tun)) return -EPERM; err = security_tun_dev_open(tun->security); if (err < 0) -- 2.43.0