check capabilities in open()
in [Kernel]
|
Prev: [PATCH] Staging: wlan-ng: fix style in cfg80211.c
Next: [PATCH] watchdog: Add watchdog driver for OCTEON SOCs (v2).
From: Vasiliy Kulikov on 24 Jul 2010 12:10 Hi, I've found that some drivers check process capabilities via capable() in open(), not in ioctl()/write()/etc. I cannot find answer in POSIX, but IMO process expects that file descriptors of priviledged user and file descriptors of the same file/device are the same in priviledge aspect. Driver should deny/allow open() and deny/allow ioctl() based on user priviledges. The path how the process gained this fd doesn't matter. So I think these 2 examples should be equal: 1) root process opened the file and then dropped its priviledges 2) nonroot process opened the file Currently gained fds are different in priviledge aspect. If you think these are bugs, I can move capable() checking down to ioctl()/write()/read()/etc. This is the full list of such drivers: drivers/staging/comedi/comedi_fops.c drivers/oprofile/event_buffer.c drivers/s390/char/vmcp.c drivers/s390/char/zcore.c drivers/net/ppp_generic.c drivers/scsi/3w-sas.c drivers/scsi/pmcraid.c drivers/scsi/megaraid.c drivers/scsi/megaraid/megaraid_sas.c drivers/scsi/megaraid/megaraid_mm.c drivers/char/mem.c drivers/char/tty_io.c drivers/char/agp/frontend.c drivers/char/apm-emulation.c This is coccinelle script to find that: @ r1 @ identifier fops; identifier openx; @@ struct file_operations fops = { .... ..open = openx, .... }; @@ identifier r1.openx; @@ openx(...) { .... *capable(...) .... } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Al Viro on 24 Jul 2010 14:30 On Sat, Jul 24, 2010 at 08:07:01PM +0400, Vasiliy Kulikov wrote: > Hi, > > I've found that some drivers check process capabilities via capable() in > open(), not in ioctl()/write()/etc. > > I cannot find answer in POSIX, but IMO process expects that file > descriptors of priviledged user and file descriptors of the same > file/device are the same in priviledge aspect. Driver should deny/allow > open() and deny/allow ioctl() based on user priviledges. The path how > the process gained this fd doesn't matter. > > So I think these 2 examples should be equal: > > 1) root process opened the file and then dropped its priviledges > > 2) nonroot process opened the file They most certainly should _not_. Consider the following mechanism: process A authenticates itself to process B B is convinced to open a file that wouldn't be readable for A. B passes descriptor to A. A reads from it. You are breaking that. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Vasiliy Kulikov on 25 Jul 2010 01:50 On Sat, Jul 24, 2010 at 19:23 +0100, Al Viro wrote: > On Sat, Jul 24, 2010 at 08:07:01PM +0400, Vasiliy Kulikov wrote: > > Hi, > > > > I've found that some drivers check process capabilities via capable() in > > open(), not in ioctl()/write()/etc. > > > > I cannot find answer in POSIX, but IMO process expects that file > > descriptors of priviledged user and file descriptors of the same > > file/device are the same in priviledge aspect. Driver should deny/allow > > open() and deny/allow ioctl() based on user priviledges. The path how > > the process gained this fd doesn't matter. > > > > So I think these 2 examples should be equal: > > > > 1) root process opened the file and then dropped its priviledges > > > > 2) nonroot process opened the file > > They most certainly should _not_. Consider the following mechanism: > process A authenticates itself to process B > B is convinced to open a file that wouldn't be readable for A. > B passes descriptor to A. > A reads from it. > You are breaking that. No, I mean that if driver allowed process to open the file, gained fd should be the same. I say that if process A has _opened_ file, its fd should be the same that convinced from B. In your example and current implementation process A allowed to open file, but it is not the same if B opens file and passes fd to A. Example from drivers/char/apm-emulation.c: static int apm_open(struct inode * inode, struct file * filp) { ... as->suser = capable(CAP_SYS_ADMIN); ... } apm_ioctl(struct file *filp, u_int cmd, u_long arg) { ... if (!as->suser || !as->writer) return -EPERM; ... } Root can open apm file (as->suser would be true), pass it to unpriviledged process and it would be able to suspend the system (as->suser would be still true). Unpriviledged process can also open apm file (as->suser would be 0), but would not be able to suspend the system. Also patalogical case :) unpriviledged process passes fd to root process and root process cannot suspend the system. Btw, the list of such drivers is much smaller, some of them just return -EPERM and open() fails, it is OK. I'll resend more precise list soon. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ted Ts'o on 26 Jul 2010 07:30 On Sun, Jul 25, 2010 at 09:45:11AM +0400, Vasiliy Kulikov wrote: > > Root can open apm file (as->suser would be true), pass it to > unpriviledged process and it would be able to suspend the system > (as->suser would be still true). The flip side of this --- and why these devices were deliberately coded that way, is a setuid root program can open the apm file, and then drop its root privileges for safety, so that if there is a buffer overrun in the program, the attacker doesn't get root privileges. This is quite common in Unix/Linux implementation pattern; let a setuid program do wha it needs to do as root in terms of opening specific file descriptors, and then have it drop its privileges. So the way they are written is quite correct. And it's consistent with a device file which is owned by root, mode 600. A setuid root program can open the device, but once it is opened, the file descriptor continues to have access to the program even if its privileges are dropped, or the file descriptor is passed to another program. (This is also sometimes done, deliberately; for example the original Berkely lpr/lpd program was written where the user would run lpr, and pass a file descriptor to the lpd daemon, which was not running as root, but as an unprivileged system process. This allowed the lpd daemon to have access to the file, even though it might not be allowed to open a file that was mode 600.) The reason why the apm device needed to sample the suser() bit is that it can be opened by root and non-root processes, but it wanted to extend the Unix/Linux paradigm that privileges are tested at open() time. So this is a not a bug, but quite deliberately, by design. Regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Vasiliy Kulikov on 26 Jul 2010 13:00
On Mon, Jul 26, 2010 at 07:23 -0400, Ted Ts'o wrote: > The reason why the apm device needed to sample the suser() bit is that > it can be opened by root and non-root processes, but it wanted to > extend the Unix/Linux paradigm that privileges are tested at open() > time. Yes, it's exactly that I mean, check at open() time and grand high or less priviledges. > > So this is a not a bug, but quite deliberately, by design. If it is explicitly designed to check UID at open() time and to have 2 kinds of file descriptors - priviledged and nonpriviledged, I'm fine with this. I wanted kernel community to draw attention because this moment was not obviously for me and I thought it was a design flaw. Now I'm pleased with your explanation, thank you. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |