From: David W. <dw...@in...> - 2000-08-22 17:39:27
|
What's the reason for requiring a daemon to be run as root? Wouldn't it be possible to simply use a /dev/tap{x] device directly from the UML? Then all root needs to do is: ifconfig tap0 $IPADDR pointopoint $UMLADDR chown $UMLUSER /dev/tap0 -- dwmw2 |
From: Lennert B. <bu...@gn...> - 2000-08-22 18:07:23
|
On Tue, 22 Aug 2000, David Woodhouse wrote: > What's the reason for requiring a daemon to be run as root? > > Wouldn't it be possible to simply use a /dev/tap{x] device directly from > the UML? Then all root needs to do is: > > ifconfig tap0 $IPADDR pointopoint $UMLADDR > chown $UMLUSER /dev/tap0 I think you have never tested something like this. greetings, Lennert |
From: David W. <dw...@in...> - 2000-08-22 18:08:40
|
bu...@gn... said: > I think you have never tested something like this. You're right. I was about to hack it up. Would you care to enlighten me about what I'll discover, and save me the trouble? :) -- dwmw2 |
From: Lennert B. <bu...@gn...> - 2000-08-22 18:46:20
|
On Tue, 22 Aug 2000, David Woodhouse wrote: > > I think you have never tested something like this. > > You're right. I was about to hack it up. Would you care to enlighten me > about what I'll discover, and save me the trouble? :) Hehehe.. if you be so kind, I cannot refuse! :-) [buytenh@mara buytenh]$ ls -al /dev/tap0 crw-rw-rw- 1 buytenh admins 36, 16 Aug 16 18:32 /dev/tap0 [buytenh@mara buytenh]$ cat /dev/tap0 cat: /dev/tap0: Operation not permitted [buytenh@mara buytenh]$ su Password: [root@mara buytenh]# cat /dev/tap0 FET@ s9^* !"#$%&'()*+,-./012345678PET@ s9 !"#$%&'()*+,-./01234567ET@ s9/ !"#$%&'()*+,-./01234567ET@ s9? !"#$%&'()*+,-./01234567 While writing a TCP implementation in userland and wanting to use this construction I encountered the same problem. Apparently it checks for some POSIX capability on open. It shouldn't be too hard to dike it out though, but that _will_ require modifications to each and every kernel you want to run it on. I've been looking at the present preferred way of communication (eth? devices), but I'm not too happy about those.. the level of isolation between virtual machines seems sub-optimal. I'm sure more thought will go into this on my part. For now I'm busy chasing a panic that won't reproduce when run under the debugger... :-) greetings, Lennert |
From: David W. <dw...@in...> - 2000-08-22 19:01:12
|
bu...@gn... said: > [buytenh@mara buytenh]$ ls -al /dev/tap0 > crw-rw-rw- 1 buytenh admins 36, 16 Aug 16 18:32 /dev/tap0 > [buytenh@mara buytenh]$ cat /dev/tap0 > cat: /dev/tap0: Operation not permitted Aha. I consider that a bug in the netlink implementation. We should make netlink_bind() bypass the capability check iff it's being invoked on behalf of the netlink chardevice. -- dwmw2 |
From: Jeff D. <jd...@ka...> - 2000-08-22 19:15:37
|
bu...@gn... said: > Apparently it checks for some POSIX capability on open. It shouldn't > be too hard to dike it out though, but that _will_ require > modifications to each and every kernel you want to run it on. The ethertap driver requires CAP_NET_ADMIN and ignores the bits on /dev/tap*. I think Alexey would accept a patch that changes this. > I've been looking at the present preferred way of communication (eth? > devices), but I'm not too happy about those.. the level of isolation > between virtual machines seems sub-optimal. Care to elaborate on that? > For now I'm busy chasing a panic that won't reproduce when run under > the debugger... :-) Yummy. My favorite kind of bug... Jeff |
From: Andi K. <ak...@su...> - 2000-08-22 19:22:10
|
On Tue, Aug 22, 2000 at 03:21:17PM -0500, Jeff Dike wrote: > bu...@gn... said: > > Apparently it checks for some POSIX capability on open. It shouldn't > > be too hard to dike it out though, but that _will_ require > > modifications to each and every kernel you want to run it on. > > The ethertap driver requires CAP_NET_ADMIN and ignores the bits on /dev/tap*. > I think Alexey would accept a patch that changes this. Using /dev/tap* is obsolete anyways, use PF_NETLINK sockets instead. -Andi |
From: David W. <dw...@in...> - 2000-08-22 19:28:01
|
ak...@su... said: > Using /dev/tap* is obsolete anyways, use PF_NETLINK sockets instead. Please demonstrate how to do so without root privileges - which _is_ possible with a fairly minor fix to the netlink chardevice. Unless PF_NETLINK provides all functionality that the chardevice can provide, it can hardly be said to obsolete it. -- dwmw2 |
From: David W. <dw...@in...> - 2000-08-22 19:45:04
|
jd...@ka... said: > The ethertap driver requires CAP_NET_ADMIN and ignores the bits on / > dev/tap*. I think Alexey would accept a patch that changes this. This patch does it. It looks like it's feasible to do it as non-root with PF_NETLINK sockets, but then it's available to _any_ user, which isn't much better. We need to be able to restrict it to a single user/group. Index: include/linux/netlink.h =================================================================== RCS file: /inst/cvs/linux/include/linux/netlink.h,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 netlink.h --- include/linux/netlink.h 2000/06/07 08:41:09 1.1.1.1 +++ include/linux/netlink.h 2000/08/22 19:08:11 @@ -141,6 +141,21 @@ return nlh; } +struct netlink_opt +{ + u32 pid; + unsigned groups; + u32 dst_pid; + unsigned dst_groups; + unsigned long state; + int netlink_dev; + int (*handler)(int unit, struct sk_buff *skb); + wait_queue_head_t wait; + struct netlink_callback *cb; + spinlock_t cb_lock; + void (*data_ready)(struct sock *sk, int bytes); +}; + #define NLMSG_PUT(skb, pid, seq, type, len) \ ({ if (skb_tailroom(skb) < (int)NLMSG_SPACE(len)) goto nlmsg_failure; \ __nlmsg_put(skb, pid, seq, type, len); }) Index: net/netlink/af_netlink.c =================================================================== RCS file: /inst/cvs/linux/net/netlink/af_netlink.c,v retrieving revision 1.4.2.10 diff -u -r1.4.2.10 af_netlink.c --- net/netlink/af_netlink.c 2000/07/06 15:28:51 1.4.2.10 +++ net/netlink/af_netlink.c 2000/08/22 19:31:08 @@ -46,20 +46,6 @@ #define BUG_TRAP(x) if (!(x)) { printk("Assertion (" #x ") failed at " __FILE__ "(%d):" __FUNCTION__ "\n", __LINE__); } -struct netlink_opt -{ - u32 pid; - unsigned groups; - u32 dst_pid; - unsigned dst_groups; - unsigned long state; - int (*handler)(int unit, struct sk_buff *skb); - wait_queue_head_t wait; - struct netlink_callback *cb; - spinlock_t cb_lock; - void (*data_ready)(struct sock *sk, int bytes); -}; - static struct sock *nl_table[MAX_LINKS]; static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait); @@ -309,7 +295,8 @@ return -EINVAL; /* Only superuser is allowed to listen multicasts */ - if (nladdr->nl_groups && !capable(CAP_NET_ADMIN)) + if (nladdr->nl_groups && !sk->protinfo.af_netlink->netlink_dev && + !capable(CAP_NET_ADMIN)) return -EPERM; if (sk->protinfo.af_netlink->pid) { @@ -563,7 +550,8 @@ return -EINVAL; dst_pid = addr->nl_pid; dst_groups = addr->nl_groups; - if (dst_groups && !capable(CAP_NET_ADMIN)) + if (dst_groups && !sk->protinfo.af_netlink->netlink_dev && + !capable(CAP_NET_ADMIN)) return -EPERM; } else { dst_pid = sk->protinfo.af_netlink->dst_pid; Index: net/netlink/netlink_dev.c =================================================================== RCS file: /inst/cvs/linux/net/netlink/netlink_dev.c,v retrieving revision 1.1.1.1.2.8 diff -u -r1.1.1.1.2.8 netlink_dev.c --- net/netlink/netlink_dev.c 2000/07/13 10:13:52 1.1.1.1.2.8 +++ net/netlink/netlink_dev.c 2000/08/22 19:16:24 @@ -27,6 +27,8 @@ #include <linux/init.h> #include <linux/devfs_fs_kernel.h> #include <linux/smp_lock.h> +#include <net/sock.h> +#include <linux/netlink.h> #include <asm/system.h> #include <asm/uaccess.h> @@ -123,6 +125,7 @@ memset(&nladdr, 0, sizeof(nladdr)); nladdr.nl_family = AF_NETLINK; nladdr.nl_groups = ~0; + sock->sk->protinfo.af_netlink->netlink_dev = 1; if ((err = sock->ops->bind(sock, (struct sockaddr*)&nladdr, sizeof(nladdr))) < 0) { sock_release(sock); goto out; -- dwmw2 |
From: <ku...@ms...> - 2000-08-23 17:55:07
|
Hello! > > The ethertap driver requires CAP_NET_ADMIN and ignores the bits on / > > dev/tap*. I think Alexey would accept a patch that changes this. Ethertap device has just been replaced. > This patch does it. It looks like it's feasible to do it as non-root with > PF_NETLINK sockets, but then it's available to _any_ user, which isn't much > better. We need to be able to restrict it to a single user/group. Netlink is broadcast media. Access control is made per message and depends on message kind. Any client knows who sent the packet. netlink_dev is open for removal rather than for improvements. User/group access to netlink is advertised (though it is _not_ more fine grain, but rther too coarce from netlink viewpoint, see above), but certainly not using character device interface. Using vfs facilities, probably. Alexey |
From: David W. <dw...@in...> - 2000-08-25 12:28:55
|
ku...@ms... said: > Ethertap device has just been replaced. > netlink_dev is open for removal rather than for improvements. OK. The new Ethertap in 2.4 looks far nicer. 2.2 is still a problem though. I'd like to see either the new Ethertap or this patch go in. Index: include/net/sock.h =================================================================== RCS file: /inst/cvs/linux/include/net/sock.h,v retrieving revision 1.8 diff -u -r1.8 sock.h --- include/net/sock.h 2000/06/07 09:55:39 1.8 +++ include/net/sock.h 2000/08/23 12:05:09 @@ -115,6 +115,7 @@ unsigned groups; pid_t dst_pid; unsigned dst_groups; + int netlink_dev; int (*handler)(int unit, struct sk_buff *skb); atomic_t locks; struct netlink_callback *cb; Index: net/netlink/af_netlink.c =================================================================== RCS file: /inst/cvs/linux/net/netlink/af_netlink.c,v retrieving revision 1.5 diff -u -r1.5 af_netlink.c --- net/netlink/af_netlink.c 2000/06/07 09:54:25 1.5 +++ net/netlink/af_netlink.c 2000/08/23 12:05:50 @@ -254,7 +254,8 @@ return -EINVAL; /* Only superuser is allowed to listen multicasts */ - if (nladdr->nl_groups && !capable(CAP_NET_ADMIN)) + if (nladdr->nl_groups && !sk->protinfo.af_netlink.netlink_dev && + !capable(CAP_NET_ADMIN)) return -EPERM; if (sk->protinfo.af_netlink.pid) { @@ -506,7 +507,8 @@ return -EINVAL; dst_pid = addr->nl_pid; dst_groups = addr->nl_groups; - if (dst_groups && !capable(CAP_NET_ADMIN)) + if (dst_groups && !sk->protinfo.af_netlink.netlink_dev && + !capable(CAP_NET_ADMIN)) return -EPERM; } else { dst_pid = sk->protinfo.af_netlink.dst_pid; Index: net/netlink/netlink_dev.c =================================================================== RCS file: /inst/cvs/linux/net/netlink/netlink_dev.c,v retrieving revision 1.2 diff -u -r1.2 netlink_dev.c --- net/netlink/netlink_dev.c 2000/06/07 09:54:25 1.2 +++ net/netlink/netlink_dev.c 2000/08/23 12:05:56 @@ -25,6 +25,7 @@ #include <linux/netlink.h> #include <linux/poll.h> #include <linux/init.h> +#include <net/sock.h> #include <asm/system.h> #include <asm/uaccess.h> @@ -134,6 +135,7 @@ memset(&nladdr, 0, sizeof(nladdr)); nladdr.nl_family = AF_NETLINK; nladdr.nl_groups = ~0; + sock->sk->protinfo.af_netlink.netlink_dev = 1; if ((err = sock->ops->bind(sock, (struct sockaddr*)&nladdr, sizeof(nladdr))) < 0) { sock_release(sock); goto out; -- dwmw2 |
From: Andi K. <ak...@su...> - 2000-08-25 12:35:17
|
On Fri, Aug 25, 2000 at 01:28:42PM +0100, David Woodhouse wrote: > > ku...@ms... said: > > Ethertap device has just been replaced. > > netlink_dev is open for removal rather than for improvements. > > OK. The new Ethertap in 2.4 looks far nicer. > > 2.2 is still a problem though. I'd like to see either the new Ethertap or > this patch go in. > > Index: include/net/sock.h > =================================================================== > RCS file: /inst/cvs/linux/include/net/sock.h,v > retrieving revision 1.8 > diff -u -r1.8 sock.h > --- include/net/sock.h 2000/06/07 09:55:39 1.8 > +++ include/net/sock.h 2000/08/23 12:05:09 > @@ -115,6 +115,7 @@ > unsigned groups; > pid_t dst_pid; > unsigned dst_groups; > + int netlink_dev; > int (*handler)(int unit, struct sk_buff *skb); > atomic_t locks; > struct netlink_callback *cb; > Index: net/netlink/af_netlink.c > =================================================================== > RCS file: /inst/cvs/linux/net/netlink/af_netlink.c,v > retrieving revision 1.5 > diff -u -r1.5 af_netlink.c > --- net/netlink/af_netlink.c 2000/06/07 09:54:25 1.5 > +++ net/netlink/af_netlink.c 2000/08/23 12:05:50 > @@ -254,7 +254,8 @@ > return -EINVAL; > > /* Only superuser is allowed to listen multicasts */ > - if (nladdr->nl_groups && !capable(CAP_NET_ADMIN)) > + if (nladdr->nl_groups && !sk->protinfo.af_netlink.netlink_dev && > + !capable(CAP_NET_ADMIN)) > return -EPERM; A nlsocket with opened character devices can still be accessed via sockets too, so you just opened a security hole. -Andi |
From: David W. <dw...@in...> - 2000-08-25 12:51:05
|
da...@re... said: > Looks fine, you'll want to resend this to Alan in a few weeks when he > starts up 2.2.18 :-) ak...@su... said: > A nlsocket with opened character devices can still be accessed via > sockets too, so you just opened a security hole. I don't comprehend. The flag is being set only in the struct sock which is used by the netlink_dev. It's not being set anywhere else. Only operations on that particular socket will bypass the capability check. Other users can create their own socket, which won't have the same flag set. How can they access the socket which is being used by the netlink_dev, without actually going through the permissions check required to open /dev/tap$n ? -- dwmw2 |
From: Andi K. <ak...@su...> - 2000-08-25 12:54:22
|
On Fri, Aug 25, 2000 at 01:50:56PM +0100, David Woodhouse wrote: > I don't comprehend. The flag is being set only in the struct sock > which is used by the netlink_dev. It's not being set anywhere else. Only > operations on that particular socket will bypass the capability check. > > Other users can create their own socket, which won't have the same flag > set. How can they access the socket which is being used by the netlink_dev, > without actually going through the permissions check required to open > /dev/tap$n ? Yes, I misread the code. There is no hole. -Andi |
From: David W. <dw...@in...> - 2000-08-27 16:53:36
|
On Fri, 25 Aug 2000, David Woodhouse wrote: > ku...@ms... said: > > Ethertap device has just been replaced. > > netlink_dev is open for removal rather than for improvements. > > OK. The new Ethertap in 2.4 looks far nicer. Upon further investigation, I'll revise that opinion. Regardless of being a cleaner implementation, the functionality isn't the same. I don't like the way that you have to ifconfig the tap interface after the UML has opened the character device. It means that the UML still has to have a suid helper. I preferred the old behaviour where the interface could be configured _once_ by root on the host, and the UML could open and close the device as many times at it wanted to without needing root to intervene again. -- dwmw2 |