On Tue, Sep 21, 2010 at 06:32:50PM -0700, Matthew Dempsky wrote: > /dev/tun* are already owned by root and mode 0600 by default, so it > seems redundant to check suser() in tunopen(). > > ok?
I object to this patch as is. FreeBSD has a sysctl to disable the super user check for tap devices (the equivalent of OpenBSD's tun device with the link0 flag set), off by default: http://svn.freebsd.org/viewvc/base/head/sys/net/if_tap.c?revision=205222&view=markup SYSCTL_INT(_net_link_tap, OID_AUTO, user_open, CTLFLAG_RW, &tapuopen, 0, "Allow user to open /dev/tap (based on node permissions)"); I suppose that's a bit safer than relying on device file perms alone, in case the perms get changed accidentally or via some attack vector. A tap device can inject arbitrary packets on the local network, like a raw socket. In case you didn't know, there already is a way for non-privileged users to open a tun/tap device if the admin allows sudo -C. See the qemu port's README.OpenBSD for an example. If a variant of this diff gets committed, you should adjust the EPERM error documentation in the tun(4) man page, too. > Index: if_tun.c > =================================================================== > RCS file: /cvs/src/sys/net/if_tun.c,v > retrieving revision 1.107 > diff -u -p -r1.107 if_tun.c > --- if_tun.c 8 Jul 2010 08:40:29 -0000 1.107 > +++ if_tun.c 22 Sep 2010 01:27:22 -0000 > @@ -343,9 +343,6 @@ tunopen(dev_t dev, int flag, int mode, s > struct ifnet *ifp; > int error, s; > > - if ((error = suser(p, 0)) != 0) > - return (error); > - > if ((tp = tun_lookup(minor(dev))) == NULL) { /* create on demand */ > char xname[IFNAMSIZ];
