netdev - tun: group check overrides user, is this intentional?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <1f9e278a-6590-4ede-a74a-3923ebe4d154@yandex.ru>
Date: Fri, 15 Nov 2024 15:52:36 +0300
From: stsp <stsp2@...dex.ru>
To: netdev <netdev@...r.kernel.org>
Cc: Linux kernel <linux-kernel@...r.kernel.org>,
 "Eric W. Biederman" <ebiederm@...ssion.com>,
 "Andrew Bird (Sphere Systems)" <ajb@...eresystems.co.uk>
Subject: tun: group check overrides user, is this intentional?

Hello.

I've found that the user that is an owner
of tun device, can't access it if it is not
in the tun's group.
I.e. the following command:
$ sudo tunctl -u stas -g root

is insufficient for the user "stas"
to access tun, unless he has group
"root" in his supplementary list.
This is somewhat very strange, as
the group check usually enlarges
the scope, not restricts it.

I am going to send the patch below,
but I'd like to ask if maybe this is
intentional?

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9a0f6eb32016..d35b6a48d138 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -574,14 +574,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_capable(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 (ns_capable(net->user_ns, CAP_NET_ADMIN))
+               return 1;
+       if (uid_valid(tun->owner) && uid_eq(cred->euid, tun->owner))
+               return 1;
+       if (gid_valid(tun->group) && in_egroup_p(tun->group))
+               return 1;
+       return 0;
  }

  static void tun_set_real_num_queues(struct tun_struct *tun)
@@ -2778,7 +2782,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_capable(tun))
                         return -EPERM;
                 err = security_tun_dev_open(tun->security);
                 if (err < 0)