Re: group ownership of tun devices -- nonfunctional?

Re: group ownership of tun devices -- nonfunctional?

[Bodo Eggert]

Mike Mohr <akihana@gmail.com> wrote:

(intentionally not snipping much)

> Per the post here:
> 
> http://lkml.org/lkml/2007/6/18/228
> 
> it appears that the group ownership patch has made it into .23.  I am
> using these patches, amongst which the kernel component appears to be
> identical:
> 
> http://sigxcpu.org/unsorted-patches/0001-allow-tun-ownership-by-group.patch
> http://sigxcpu.org/unsorted-patches/tunctl_gid.diff
> 
> I can create devices that are owned by my user account (tunctl -u
> `whoami` -t tap0) and it works fine.  However, if I use group
> permissions with -g it stops working.  In all cases, if I pass -g
> <group>, the interface is created correctly but it is unusable as a
> non-root user.
> 
> So my question is: am I doing something wrong?  If I am, I don't see
> it.  Assuming then that I am not doing anything wrong on my end, I
> assume then that there is something missing from the kernel patch I
> applied.  I read over it and I can't see any issues, especially
> considering that tunctl comes back without error (even with -g) and
> creates an interface.
> 
> Just wondering if this was an issue that should be looked into--


IMHO the check is broken:

+               if (((tun->owner != -1 &&
+                     current->euid != tun->owner) ||
+                    (tun->group != -1 &&
+                     current->egid != tun->group)) &&
+                    !capable(CAP_NET_ADMIN))
                        return -EPERM;

It should be something like:

+               if (!((tun->owner == tun->owner) ||
+                     (tun->group == tun->group) ||
+                     capable(CAP_NET_ADMIN)))
                        return -EPERM;

Please verify and forward to the maintainers if my guess appears to be correct.
-- 
Never stand when you can sit, never sit when you can lie down, never stay
awake when you can sleep.

Friß, Spammer: xxh@n.btxp.7eggert.dyndns.org

Re: group ownership of tun devices -- nonfunctional?

[Rene Herman]

On 08/19/2007 06:05 PM, Bodo Eggert wrote:

> IMHO the check is broken:
> 
> +               if (((tun->owner != -1 &&
> +                     current->euid != tun->owner) ||
> +                    (tun->group != -1 &&
> +                     current->egid != tun->group)) &&
> +                    !capable(CAP_NET_ADMIN))
>                         return -EPERM;
> 
> It should be something like:
> 
> +               if (!((tun->owner == tun->owner) ||
> +                     (tun->group == tun->group) ||

???

> +                     capable(CAP_NET_ADMIN)))
>                         return -EPERM;
> 
> Please verify and forward to the maintainers if my guess appears to be correct.

Rene.

Re: group ownership of tun devices -- nonfunctional?

[Bodo Eggert]

On Sun, 19 Aug 2007, Rene Herman wrote:

> On 08/19/2007 06:05 PM, Bodo Eggert wrote:
> 
> > IMHO the check is broken:
> > 
> > +               if (((tun->owner != -1 &&
> > +                     current->euid != tun->owner) ||
> > +                    (tun->group != -1 &&
> > +                     current->egid != tun->group)) &&
> > +                    !capable(CAP_NET_ADMIN))
> >                         return -EPERM;
> > 
> > It should be something like:
> > 
> > +               if (!((tun->owner == tun->owner) ||
> > +                     (tun->group == tun->group) ||
> 
> ???

Argh, I edited asuming the same order of variables. Substitute 
current->e{uid,gid} for one of the sides.

> > +                     capable(CAP_NET_ADMIN)))
> >                         return -EPERM;

The intended semantics is If the user is not
 * the allowed user
or
 * member of the allowed group
or
 * cabable of CAP_NET_ADMIN
then error out. I'm asuming  

Thinking about it, maybe you should check each group, not just the 
effective group. In that case, my change would be still wrong. However, 
I'm not going to fix this anytime soon.

-- 
Funny quotes:
15. I drive way too fast to worry about cholesterol.

Re: group ownership of tun devices -- nonfunctional?

[Rene Herman]

On 08/19/2007 11:42 PM, Bodo Eggert wrote:

> On Sun, 19 Aug 2007, Rene Herman wrote:
> 
>> On 08/19/2007 06:05 PM, Bodo Eggert wrote:
>>
>>> IMHO the check is broken:
>>>
>>> +               if (((tun->owner != -1 &&
>>> +                     current->euid != tun->owner) ||
>>> +                    (tun->group != -1 &&
>>> +                     current->egid != tun->group)) &&
>>> +                    !capable(CAP_NET_ADMIN))
>>>                         return -EPERM;
>>>
>>> It should be something like:
>>>
>>> +               if (!((tun->owner == tun->owner) ||
>>> +                     (tun->group == tun->group) ||
>> ???
> 
> Argh, I edited asuming the same order of variables. Substitute 
> current->e{uid,gid} for one of the sides.

Okay. Just had to ask. That looked so odd...

>>> +                     capable(CAP_NET_ADMIN)))
>>>                         return -EPERM;
> 
> The intended semantics is If the user is not
>  * the allowed user
> or
>  * member of the allowed group
> or
>  * cabable of CAP_NET_ADMIN
> then error out. I'm asuming

There is a short description of the desired semantics in the link that was 
posted:

http://lkml.org/lkml/2007/6/18/228

===
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.
===

Paraphrasing the original code above, it's saying:

if ((owner_is_set && does_not_match) || (group_is_set && does_not_match))
	bugger_off_unless(CAP_NET_ADMIN);

or reverting the logic:

if ((owner_is_unset || does_match) && (group_is_unset || does_match))
	good_to_go();

which probably matches the intention -- we're good to go only if the 
credentials that are set also match.

Rene.

Re: group ownership of tun devices -- nonfunctional?

[Bodo Eggert]

On Mon, 20 Aug 2007, Rene Herman wrote:
> On 08/19/2007 11:42 PM, Bodo Eggert wrote:

> > The intended [my me] semantics is If the user is not
> >  * the allowed user
> > or
> >  * member of the allowed group
> > or
> >  * cabable of CAP_NET_ADMIN
> > then error out. I'm asuming
> 
> There is a short description of the desired semantics in the link that was 
> posted:
> 
> http://lkml.org/lkml/2007/6/18/228
> 
> ===
> 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.
> ===
> 
> Paraphrasing the original code above, it's saying:
> 
> if ((owner_is_set && does_not_match) || (group_is_set && does_not_match))
> 	bugger_off_unless(CAP_NET_ADMIN);
> 
> or reverting the logic:
> 
> if ((owner_is_unset || does_match) && (group_is_unset || does_match))
> 	good_to_go();
> 
> which probably matches the intention -- we're good to go only if the 
> credentials that are set also match.

Maybe there are valid reasons to do it this way, but I think having it the 
way I described would be less confusing.

-- 
       ¤ Bill of Spammer-Rights ¤
1. We have the right to assassinate you.
2. You have the right to be assassinated.
3. You have the right to resist, but it is futile.

Re: group ownership of tun devices -- nonfunctional?

[Jeff Dike]

> I can create devices that are owned by my user account (tunctl -u
> `whoami` -t tap0) and it works fine.  However, if I use group
> permissions with -g it stops working.  In all cases, if I pass -g
> <group>, the interface is created correctly but it is unusable as a
> non-root user.

I can't reproduce this - it seems to work fine on -rc3-mm1:

As root:
	./tunctl -u user -g user -t tap1
	ifconfig tap1 192.168.0.130 up
	route add -host 192.168.0.131 dev tap1
	chmod 666 /dev/net/tun

As a normal user:
	./tunread # tunread source is below

As root again:
	ping 192.168.0.131

tunread output:
	0xffffffff 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0x0 0xffffffff 0xffffff9b 0x51 0xffffffb9 0xffffffd9 0x8 0x6 0x0 0x1 0x8 0x0 0x6 0x4 0x0 0x1 0x0 0xffffffff 0xffffff9b 0x51 0xffffffb9 0xffffffd9 0xffffffc0 0xffffffa8 0x0 0xffffff82 
0x0 0x0 0x0 0x0 0x0 0x0 0xffffffc0 0xffffffa8 0x0 0xffffff83 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0x0 0xffffffff 0xffffff9b 0x51 0xffffffb9 0xffffffd9 0x8 0x6 0x0 0x1 0x8 0x0 0x6 0x4 0x0 0x1 0x0 0xffffffff 0xffffff9b 0x51 0xffffffb9 0xffffffd9 0xffffffc0 0xffffffa8 0x0 0xffffff82 
0x0 0x0 0x0 0x0 0x0 0x0 0xffffffc0 0xffffffa8 0x0 0xffffff83 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0xffffffff 0x0 0xffffffff 0xffffff9b 0x51 0xffffffb9 0xffffffd9 0x8 0x6 0x0 0x1 0x8 0x0 0x6 0x4 0x0 0x1 0x0 0xffffffff 0xffffff9b 0x51 0xffffffb9 0xffffffd9 0xffffffc0 0xffffffa8 0x0 0xffffff82 

				Jeff

-- 
Work email - jdike at linux dot intel dot com

#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <sys/ioctl.h>
#define __KERNEL__
#include <linux/socket.h>
#include <linux/if.h>
#include <linux/if_tun.h>

int main(int argc, char **argv)
{
  char packet[1600];
  struct ifreq ifr;
  int fd, err, i, n;

  if((fd = open("/dev/net/tun", O_RDWR)) < 0){
    perror("Opening /dev/net/tun");
    exit(1);
  }

  memset(&ifr, 0, sizeof(ifr));

  /* Flags: IFF_TUN   - TUN device (no Ethernet headers) 
   *        IFF_TAP   - TAP device  
   *
   *        IFF_NO_PI - Do not provide packet information  
   */ 
  ifr.ifr_flags = IFF_TUN; 
  strncpy(ifr.ifr_name, "tap1", IFNAMSIZ);

  if((err = ioctl(fd, TUNSETIFF, (void *) &ifr)) < 0){
    perror("TUNSETIFF");
    exit(1);
  }

  while(1){
    n = read(fd, packet, sizeof(packet));
    if(n < 0){
      perror("read");
      exit(1);
    }
    else if(n == 0)
      break;

    for(i = 0; i < n; i++){
      printf("0x%x ", packet[i]);
      if((i % 32) == 31)
        printf("\n");
    }
  }
}

Re: group ownership of tun devices -- nonfunctional?

[Guido Guenther]

Hi,
On Wed, Aug 22, 2007 at 04:42:54PM -0400, Jeff Dike wrote:
> > I can create devices that are owned by my user account (tunctl -u
> > `whoami` -t tap0) and it works fine.  However, if I use group
> > permissions with -g it stops working.  In all cases, if I pass -g
> > <group>, the interface is created correctly but it is unusable as a
> > non-root user.
> 
> I can't reproduce this - it seems to work fine on -rc3-mm1:
Works fine here too. I had a similar report and there seems to be some
confusion since the group has to be the processes effective group not
any supplementary group the user happens to be in. Could that be the
problem here too?
Cheers,
 -- Guido

group ownership of tun devices -- nonfunctional?

[Mike Mohr]

Per the post here:

http://lkml.org/lkml/2007/6/18/228

it appears that the group ownership patch has made it into .23.  I am
using these patches, amongst which the kernel component appears to be
identical:

http://sigxcpu.org/unsorted-patches/0001-allow-tun-ownership-by-group.patch
http://sigxcpu.org/unsorted-patches/tunctl_gid.diff

I can create devices that are owned by my user account (tunctl -u
`whoami` -t tap0) and it works fine.  However, if I use group
permissions with -g it stops working.  In all cases, if I pass -g
<group>, the interface is created correctly but it is unusable as a
non-root user.

So my question is: am I doing something wrong?  If I am, I don't see
it.  Assuming then that I am not doing anything wrong on my end, I
assume then that there is something missing from the kernel patch I
applied.  I read over it and I can't see any issues, especially
considering that tunctl comes back without error (even with -g) and
creates an interface.

Just wondering if this was an issue that should be looked into--

Mike