Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
* Michael Halcrow ([EMAIL PROTECTED]) wrote:
> [...]. This occurs because the bd_release function will
> bd_release(bdev) and set inode->i_security to NULL on the close(fd1).
> Hence, we want to place the control at the level of the file struct,
> not the inode.
This is basically what I was referring to pre-merge. And it is still
not fully sufficient. Multiple processes can share an fd. So the test
against current is broken. Also well-behaved apps that are already
using O_EXCL will break. Using filp as the holder is sufficient to fix
both of these issues. Here's a 3.5/8 that will fix this. 6/8 no longer
applies cleanly with this change.
Signed-off-by: Chris Wright <[EMAIL PROTECTED]>
--- a/security/seclvl.c~bd_claim2005-02-08 15:05:09.0 -0800
+++ b/security/seclvl.c 2005-02-08 15:05:17.0 -0800
@@ -492,17 +492,16 @@
*/
static int seclvl_bd_claim(struct file * filp)
{
- int holder;
struct block_device *bdev = NULL;
dev_t dev = filp->f_dentry->d_inode->i_rdev;
bdev = open_by_devnum(dev, FMODE_WRITE);
if (bdev) {
- if (bd_claim(bdev, )) {
+ if (bd_claim(bdev, filp)) {
blkdev_put(bdev);
return -EPERM;
}
/* Claimed; mark it to release on close */
- filp->f_security = current;
+ filp->f_security = filp;
}
return 0;
}
@@ -597,7 +596,7 @@
if (dentry && (filp->f_mode & FMODE_WRITE)) {
struct inode * inode = dentry->d_inode;
if (inode && S_ISBLK(inode->i_mode)
- && filp->f_security == current) {
+ && filp->f_security == filp) {
struct block_device *bdev = inode->i_bdev;
if (bdev) {
bd_release(bdev);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
Quoting [EMAIL PROTECTED] ([EMAIL PROTECTED]): > On Tue, 08 Feb 2005 11:24:50 CST, Michael Halcrow said: > > > While the program is waiting for a keystroke, mount the block device. > > Enter a keystroke. The result without the patch is 1, which is a > > security violation. This occurs because the bd_release function will > > bd_release(bdev) and set inode->i_security to NULL on the close(fd1). > > Sounds like a bug, not a feature. Should it be zeroing out inode->i_security > for an inode with a non-zero reference count? Valdis, inode->i_security is no longer used after the patch. Does your question still apply with the proposed patch, %s/inode->i_security/file->f_security/? Nevertheless, note that the thing being enforced is "no simultaneous write access to a block device and mount of that block device." The file->f_security is just used as a flag to seclvl that when this file is closed, we can bd_release the device to allow a mount or another open(O_RDWR) of the file. So references to the inode don't matter, provided the other references are read accesses. Which they have to be, since otherwise the seclvl_bd_claim() would have failed on the second open(O_RDWR) call. I hope I'm at least remotely answering your question :) -serge - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Tue, 08 Feb 2005 11:24:50 CST, Michael Halcrow said: > While the program is waiting for a keystroke, mount the block device. > Enter a keystroke. The result without the patch is 1, which is a > security violation. This occurs because the bd_release function will > bd_release(bdev) and set inode->i_security to NULL on the close(fd1). Sounds like a bug, not a feature. Should it be zeroing out inode->i_security for an inode with a non-zero reference count? pgprTLY1VZm0q.pgp Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, Feb 07, 2005 at 02:26:03PM -0800, Chris Wright wrote:
> * Michael Halcrow ([EMAIL PROTECTED]) wrote:
> > This is the third in a series of eight patches to the BSD Secure
> > Levels LSM. It moves the claim on the block device from the inode
> > struct to the file struct in order to address a potential
> > circumvention of the control via hard links to block devices. Thanks
> > to Serge Hallyn for pointing this out.
>
> Hard links still point to same inode, what's the issue that this
> addresses?
Actually, it turns out that hard links have nothing to do with the
vulnerability that this patch addresses:
#include
#include
#include
#include
int main()
{
int fd1, fd2;
int rc;
fd1 = open( "/dev/device", O_RDONLY );
fd2 = open( "/dev/device", O_RDWR );
close(fd1);
getchar();
rc = write( fd2, "0", 1 );
printf( "write result: [%d]\n", rc );
close( fd2 );
return 0;
}
While the program is waiting for a keystroke, mount the block device.
Enter a keystroke. The result without the patch is 1, which is a
security violation. This occurs because the bd_release function will
bd_release(bdev) and set inode->i_security to NULL on the close(fd1).
Hence, we want to place the control at the level of the file struct,
not the inode.
Mike
.___.
Michael A. Halcrow
Security Software Engineer, IBM Linux Technology Center
GnuPG Fingerprint: 05B5 08A8 713A 64C1 D35D 2371 2D3C FDDA 3EB6 601D
The hokey pokey... What if that's really what it's all about?
signature.asc
Description: Digital signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
>The attack is to hardlink some tempfile name to some file you want >over-written. This usually involves just a little bit of work, such as >recognizing that a given root cronjob uses an unsafe predictable filename >in /tmp (look at the Bugtraq or Full-Disclosure archives, there's plenty). >Then you set a little program that sleep()s till a few seconds before >the cronjob runs, does a getpid(), and then sprays hardlinks into the >next 15 or 20 things that mktemp() will generate... Got it. Very good -- now I see. Thanks for the explanation. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
The attack is to hardlink some tempfile name to some file you want over-written. This usually involves just a little bit of work, such as recognizing that a given root cronjob uses an unsafe predictable filename in /tmp (look at the Bugtraq or Full-Disclosure archives, there's plenty). Then you set a little program that sleep()s till a few seconds before the cronjob runs, does a getpid(), and then sprays hardlinks into the next 15 or 20 things that mktemp() will generate... Got it. Very good -- now I see. Thanks for the explanation. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, Feb 07, 2005 at 02:26:03PM -0800, Chris Wright wrote:
* Michael Halcrow ([EMAIL PROTECTED]) wrote:
This is the third in a series of eight patches to the BSD Secure
Levels LSM. It moves the claim on the block device from the inode
struct to the file struct in order to address a potential
circumvention of the control via hard links to block devices. Thanks
to Serge Hallyn for pointing this out.
Hard links still point to same inode, what's the issue that this
addresses?
Actually, it turns out that hard links have nothing to do with the
vulnerability that this patch addresses:
#include sys/types.h
#include sys/stat.h
#include fcntl.h
#include stdio.h
int main()
{
int fd1, fd2;
int rc;
fd1 = open( /dev/device, O_RDONLY );
fd2 = open( /dev/device, O_RDWR );
close(fd1);
getchar();
rc = write( fd2, 0, 1 );
printf( write result: [%d]\n, rc );
close( fd2 );
return 0;
}
While the program is waiting for a keystroke, mount the block device.
Enter a keystroke. The result without the patch is 1, which is a
security violation. This occurs because the bd_release function will
bd_release(bdev) and set inode-i_security to NULL on the close(fd1).
Hence, we want to place the control at the level of the file struct,
not the inode.
Mike
.___.
Michael A. Halcrow
Security Software Engineer, IBM Linux Technology Center
GnuPG Fingerprint: 05B5 08A8 713A 64C1 D35D 2371 2D3C FDDA 3EB6 601D
The hokey pokey... What if that's really what it's all about?
signature.asc
Description: Digital signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Tue, 08 Feb 2005 11:24:50 CST, Michael Halcrow said: While the program is waiting for a keystroke, mount the block device. Enter a keystroke. The result without the patch is 1, which is a security violation. This occurs because the bd_release function will bd_release(bdev) and set inode-i_security to NULL on the close(fd1). Sounds like a bug, not a feature. Should it be zeroing out inode-i_security for an inode with a non-zero reference count? pgprTLY1VZm0q.pgp Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
Quoting [EMAIL PROTECTED] ([EMAIL PROTECTED]): On Tue, 08 Feb 2005 11:24:50 CST, Michael Halcrow said: While the program is waiting for a keystroke, mount the block device. Enter a keystroke. The result without the patch is 1, which is a security violation. This occurs because the bd_release function will bd_release(bdev) and set inode-i_security to NULL on the close(fd1). Sounds like a bug, not a feature. Should it be zeroing out inode-i_security for an inode with a non-zero reference count? Valdis, inode-i_security is no longer used after the patch. Does your question still apply with the proposed patch, %s/inode-i_security/file-f_security/? Nevertheless, note that the thing being enforced is no simultaneous write access to a block device and mount of that block device. The file-f_security is just used as a flag to seclvl that when this file is closed, we can bd_release the device to allow a mount or another open(O_RDWR) of the file. So references to the inode don't matter, provided the other references are read accesses. Which they have to be, since otherwise the seclvl_bd_claim() would have failed on the second open(O_RDWR) call. I hope I'm at least remotely answering your question :) -serge - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
* Michael Halcrow ([EMAIL PROTECTED]) wrote:
[...]. This occurs because the bd_release function will
bd_release(bdev) and set inode-i_security to NULL on the close(fd1).
Hence, we want to place the control at the level of the file struct,
not the inode.
This is basically what I was referring to pre-merge. And it is still
not fully sufficient. Multiple processes can share an fd. So the test
against current is broken. Also well-behaved apps that are already
using O_EXCL will break. Using filp as the holder is sufficient to fix
both of these issues. Here's a 3.5/8 that will fix this. 6/8 no longer
applies cleanly with this change.
Signed-off-by: Chris Wright [EMAIL PROTECTED]
--- a/security/seclvl.c~bd_claim2005-02-08 15:05:09.0 -0800
+++ b/security/seclvl.c 2005-02-08 15:05:17.0 -0800
@@ -492,17 +492,16 @@
*/
static int seclvl_bd_claim(struct file * filp)
{
- int holder;
struct block_device *bdev = NULL;
dev_t dev = filp-f_dentry-d_inode-i_rdev;
bdev = open_by_devnum(dev, FMODE_WRITE);
if (bdev) {
- if (bd_claim(bdev, holder)) {
+ if (bd_claim(bdev, filp)) {
blkdev_put(bdev);
return -EPERM;
}
/* Claimed; mark it to release on close */
- filp-f_security = current;
+ filp-f_security = filp;
}
return 0;
}
@@ -597,7 +596,7 @@
if (dentry (filp-f_mode FMODE_WRITE)) {
struct inode * inode = dentry-d_inode;
if (inode S_ISBLK(inode-i_mode)
-filp-f_security == current) {
+filp-f_security == filp) {
struct block_device *bdev = inode-i_bdev;
if (bdev) {
bd_release(bdev);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, 07 Feb 2005 18:20:36 PST, Chris Wright said:
> * [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote:
> > open("/tmp/sh-thd-1107848098", O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_LARGEFILE,
0600) = 3
>
> O_EXCL
>
> > Wow - if my /tmp was on the same partition, and I'd hard-linked that
> > file to /etc/passwd, it would be toast now if root had run it.
>
> So, in fact, it wouldn't ;-)
Well.. Yeah. bash gets it right, a lot of programs botch it. ;)
pgpW8EazQy2Vi.pgp
Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
* [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote:
> open("/tmp/sh-thd-1107848098", O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_LARGEFILE,
> 0600) = 3
O_EXCL
> Wow - if my /tmp was on the same partition, and I'd hard-linked that
> file to /etc/passwd, it would be toast now if root had run it.
So, in fact, it wouldn't ;-)
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Tue, 08 Feb 2005 01:48:40 GMT, David Wagner said:
> How would /etc/passwd get clobbered? Are you thinking that a tmp
> cleaner run by cron might delete /tmp/whatever (i.e., delete the hardlink
> you created above)? But deleting /tmp/whatever is safe; it doesn't affect
> /etc/passwd. I'm guessing I'm probably missing something.
The attack is to hardlink some tempfile name to some file you want over-written.
This usually involves just a little bit of work, such as recognizing that a
given
root cronjob uses an unsafe predictable filename in /tmp (look at the Bugtraq or
Full-Disclosure archives, there's plenty). Then you set a little program that
sleep()s till a few seconds before the cronjob runs, does a getpid(), and then
sprays hardlinks into the next 15 or 20 things that mktemp() will generate...
Consider how bash implements "here" scripts:
#!/bin/bash
echo << EOF
some trash
EOF
Now let's look at the strace (snipped for brevity..)
statfs("/tmp", {f_type="EXT2_SUPER_MAGIC", f_bsize=1024, f_blocks=253871,
f_bfree=213773, f_bavail=200666, f_files=65536, f_ffree=65445, f_fsid={0, 0},
f_namelen=255, f_frsize=1024}) = 0
time(NULL) = 1107828098
open("/tmp/sh-thd-1107848098", O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_LARGEFILE,
0600) = 3
dup(3) = 4
fcntl64(4, F_GETFL) = 0x8001 (flags O_WRONLY|O_LARGEFILE)
fstat64(4, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7d71000
_llseek(4, 0, [0], SEEK_CUR)= 0
write(4, "some trash\n", 11)= 11
close(4)= 0
munmap(0xb7d71000, 4096)= 0
open("/tmp/sh-thd-1107848098", O_RDONLY|O_LARGEFILE) = 4
close(3)= 0
unlink("/tmp/sh-thd-1107848098")= 0
fcntl64(0, F_GETFD) = 0
fcntl64(0, F_DUPFD, 10) = 10
fcntl64(0, F_GETFD) = 0
fcntl64(10, F_SETFD, FD_CLOEXEC)= 0
dup2(4, 0) = 0
close(4)= 0
Wow - if my /tmp was on the same partition, and I'd hard-linked that
file to /etc/passwd, it would be toast now if root had run it.
You usually can't control what gets written - but often it's sufficient for the
attacker to simply get a file clobbered
pgp1unSohNbRA.pgp
Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
>For those systems that have everything on one big partition, you can often >do stuff like: > >ln /etc/passwd /tmp/ > >and wait for /etc/passwd to get clobbered by a cron job run by root... How would /etc/passwd get clobbered? Are you thinking that a tmp cleaner run by cron might delete /tmp/whatever (i.e., delete the hardlink you created above)? But deleting /tmp/whatever is safe; it doesn't affect /etc/passwd. I'm guessing I'm probably missing something. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, 07 Feb 2005 14:26:03 PST, Chris Wright said: > * Michael Halcrow ([EMAIL PROTECTED]) wrote: > > This is the third in a series of eight patches to the BSD Secure > > Levels LSM. It moves the claim on the block device from the inode > > struct to the file struct in order to address a potential > > circumvention of the control via hard links to block devices. Thanks > > to Serge Hallyn for pointing this out. > > Hard links still point to same inode, what's the issue that this > addresses? Ignore that last - I thought it was the "filesystem linking permissions" thread rather than the BSD Secure linking permissions thread. ;) pgpHd0UzzrMjl.pgp Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, 07 Feb 2005 14:26:03 PST, Chris Wright said: > Hard links still point to same inode, what's the issue that this > addresses? For those systems that have everything on one big partition, you can often do stuff like: ln /etc/passwd /tmp/ and wait for /etc/passwd to get clobbered by a cron job run by root... pgpv1juO6RgIl.pgp Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
* Michael Halcrow ([EMAIL PROTECTED]) wrote: > This is the third in a series of eight patches to the BSD Secure > Levels LSM. It moves the claim on the block device from the inode > struct to the file struct in order to address a potential > circumvention of the control via hard links to block devices. Thanks > to Serge Hallyn for pointing this out. Hard links still point to same inode, what's the issue that this addresses? thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
This is the third in a series of eight patches to the BSD Secure
Levels LSM. It moves the claim on the block device from the inode
struct to the file struct in order to address a potential
circumvention of the control via hard links to block devices. Thanks
to Serge Hallyn for pointing this out.
Signed off by: Michael Halcrow <[EMAIL PROTECTED]>
Index: linux-2.6.11-rc2-mm1-modules/security/seclvl.c
===
--- linux-2.6.11-rc2-mm1-modules.orig/security/seclvl.c 2005-02-03
15:36:43.925683472 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c 2005-02-03
16:41:55.075098384 -0600
@@ -487,46 +487,35 @@
return 0;
}
-/* claim the blockdev to exclude mounters, release on file close */
-static int seclvl_bd_claim(struct inode *inode)
+/**
+ * Claim the blockdev to exclude mounters; release on file close.
+ */
+static int seclvl_bd_claim(struct file * filp)
{
int holder;
struct block_device *bdev = NULL;
- dev_t dev = inode->i_rdev;
+ dev_t dev = filp->f_dentry->d_inode->i_rdev;
bdev = open_by_devnum(dev, FMODE_WRITE);
if (bdev) {
if (bd_claim(bdev, )) {
blkdev_put(bdev);
return -EPERM;
}
- /* claimed, mark it to release on close */
- inode->i_security = current;
+ /* Claimed; mark it to release on close */
+ filp->f_security = current;
}
return 0;
}
-/* release the blockdev if you claimed it */
-static void seclvl_bd_release(struct inode *inode)
-{
- if (inode && S_ISBLK(inode->i_mode) && inode->i_security == current) {
- struct block_device *bdev = inode->i_bdev;
- if (bdev) {
- bd_release(bdev);
- blkdev_put(bdev);
- inode->i_security = NULL;
- }
- }
-}
-
/**
* Security for writes to block devices is regulated by this seclvl
* function. Deny all writes to block devices in seclvl 2. In
* seclvl 1, we only deny writes to *mounted* block devices.
*/
-static int
-seclvl_inode_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int seclvl_file_permission(struct file * filp, int mask)
{
- if (current->pid != 1 && S_ISBLK(inode->i_mode) && (mask & MAY_WRITE)) {
+ if (current->pid != 1 && S_ISBLK(filp->f_dentry->d_inode->i_mode)
+ && (mask & MAY_WRITE)) {
switch (seclvl) {
case 2:
seclvl_printk(1, KERN_WARNING "%s: Write to block "
@@ -534,7 +523,7 @@
__FUNCTION__, seclvl);
return -EPERM;
case 1:
- if (seclvl_bd_claim(inode)) {
+ if (seclvl_bd_claim(filp)) {
seclvl_printk(1, KERN_WARNING "%s: Write to "
"mounted block device denied in "
"secure level [%d]\n",
@@ -549,7 +538,7 @@
/**
* The SUID and SGID bits cannot be set in seclvl >= 1
*/
-static int seclvl_inode_setattr(struct dentry *dentry, struct iattr *iattr)
+static int seclvl_inode_setattr(struct dentry * dentry, struct iattr * iattr)
{
if (seclvl > 0) {
if (dentry && dentry->d_inode
@@ -599,15 +588,23 @@
return 0;
}
-/* release busied block devices */
-static void seclvl_file_free_security(struct file *filp)
+/**
+ * Release busied block devices.
+ */
+static void seclvl_file_free_security(struct file * filp)
{
- struct dentry *dentry = filp->f_dentry;
- struct inode *inode = NULL;
-
- if (dentry) {
- inode = dentry->d_inode;
- seclvl_bd_release(inode);
+ struct dentry * dentry = filp->f_dentry;
+ if (dentry && (filp->f_mode & FMODE_WRITE)) {
+ struct inode * inode = dentry->d_inode;
+ if (inode && S_ISBLK(inode->i_mode)
+ && filp->f_security == current) {
+ struct block_device *bdev = inode->i_bdev;
+ if (bdev) {
+ bd_release(bdev);
+ blkdev_put(bdev);
+ filp->f_security = NULL;
+ }
+ }
}
}
@@ -630,7 +627,7 @@
static struct security_operations seclvl_ops = {
.ptrace = seclvl_ptrace,
.capable = seclvl_capable,
- .inode_permission = seclvl_inode_permission,
+ .file_permission = seclvl_file_permission,
.inode_setattr = seclvl_inode_setattr,
.inode_mknod = seclvl_inode_mknod,
.inode_create = seclvl_inode_create,
[PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
This is the third in a series of eight patches to the BSD Secure
Levels LSM. It moves the claim on the block device from the inode
struct to the file struct in order to address a potential
circumvention of the control via hard links to block devices. Thanks
to Serge Hallyn for pointing this out.
Signed off by: Michael Halcrow [EMAIL PROTECTED]
Index: linux-2.6.11-rc2-mm1-modules/security/seclvl.c
===
--- linux-2.6.11-rc2-mm1-modules.orig/security/seclvl.c 2005-02-03
15:36:43.925683472 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c 2005-02-03
16:41:55.075098384 -0600
@@ -487,46 +487,35 @@
return 0;
}
-/* claim the blockdev to exclude mounters, release on file close */
-static int seclvl_bd_claim(struct inode *inode)
+/**
+ * Claim the blockdev to exclude mounters; release on file close.
+ */
+static int seclvl_bd_claim(struct file * filp)
{
int holder;
struct block_device *bdev = NULL;
- dev_t dev = inode-i_rdev;
+ dev_t dev = filp-f_dentry-d_inode-i_rdev;
bdev = open_by_devnum(dev, FMODE_WRITE);
if (bdev) {
if (bd_claim(bdev, holder)) {
blkdev_put(bdev);
return -EPERM;
}
- /* claimed, mark it to release on close */
- inode-i_security = current;
+ /* Claimed; mark it to release on close */
+ filp-f_security = current;
}
return 0;
}
-/* release the blockdev if you claimed it */
-static void seclvl_bd_release(struct inode *inode)
-{
- if (inode S_ISBLK(inode-i_mode) inode-i_security == current) {
- struct block_device *bdev = inode-i_bdev;
- if (bdev) {
- bd_release(bdev);
- blkdev_put(bdev);
- inode-i_security = NULL;
- }
- }
-}
-
/**
* Security for writes to block devices is regulated by this seclvl
* function. Deny all writes to block devices in seclvl 2. In
* seclvl 1, we only deny writes to *mounted* block devices.
*/
-static int
-seclvl_inode_permission(struct inode *inode, int mask, struct nameidata *nd)
+static int seclvl_file_permission(struct file * filp, int mask)
{
- if (current-pid != 1 S_ISBLK(inode-i_mode) (mask MAY_WRITE)) {
+ if (current-pid != 1 S_ISBLK(filp-f_dentry-d_inode-i_mode)
+(mask MAY_WRITE)) {
switch (seclvl) {
case 2:
seclvl_printk(1, KERN_WARNING %s: Write to block
@@ -534,7 +523,7 @@
__FUNCTION__, seclvl);
return -EPERM;
case 1:
- if (seclvl_bd_claim(inode)) {
+ if (seclvl_bd_claim(filp)) {
seclvl_printk(1, KERN_WARNING %s: Write to
mounted block device denied in
secure level [%d]\n,
@@ -549,7 +538,7 @@
/**
* The SUID and SGID bits cannot be set in seclvl = 1
*/
-static int seclvl_inode_setattr(struct dentry *dentry, struct iattr *iattr)
+static int seclvl_inode_setattr(struct dentry * dentry, struct iattr * iattr)
{
if (seclvl 0) {
if (dentry dentry-d_inode
@@ -599,15 +588,23 @@
return 0;
}
-/* release busied block devices */
-static void seclvl_file_free_security(struct file *filp)
+/**
+ * Release busied block devices.
+ */
+static void seclvl_file_free_security(struct file * filp)
{
- struct dentry *dentry = filp-f_dentry;
- struct inode *inode = NULL;
-
- if (dentry) {
- inode = dentry-d_inode;
- seclvl_bd_release(inode);
+ struct dentry * dentry = filp-f_dentry;
+ if (dentry (filp-f_mode FMODE_WRITE)) {
+ struct inode * inode = dentry-d_inode;
+ if (inode S_ISBLK(inode-i_mode)
+filp-f_security == current) {
+ struct block_device *bdev = inode-i_bdev;
+ if (bdev) {
+ bd_release(bdev);
+ blkdev_put(bdev);
+ filp-f_security = NULL;
+ }
+ }
}
}
@@ -630,7 +627,7 @@
static struct security_operations seclvl_ops = {
.ptrace = seclvl_ptrace,
.capable = seclvl_capable,
- .inode_permission = seclvl_inode_permission,
+ .file_permission = seclvl_file_permission,
.inode_setattr = seclvl_inode_setattr,
.inode_mknod = seclvl_inode_mknod,
.inode_create = seclvl_inode_create,
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
* Michael Halcrow ([EMAIL PROTECTED]) wrote: This is the third in a series of eight patches to the BSD Secure Levels LSM. It moves the claim on the block device from the inode struct to the file struct in order to address a potential circumvention of the control via hard links to block devices. Thanks to Serge Hallyn for pointing this out. Hard links still point to same inode, what's the issue that this addresses? thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, 07 Feb 2005 14:26:03 PST, Chris Wright said: Hard links still point to same inode, what's the issue that this addresses? For those systems that have everything on one big partition, you can often do stuff like: ln /etc/passwd /tmp/filename_generated_by_mktemp and wait for /etc/passwd to get clobbered by a cron job run by root... pgpv1juO6RgIl.pgp Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, 07 Feb 2005 14:26:03 PST, Chris Wright said: * Michael Halcrow ([EMAIL PROTECTED]) wrote: This is the third in a series of eight patches to the BSD Secure Levels LSM. It moves the claim on the block device from the inode struct to the file struct in order to address a potential circumvention of the control via hard links to block devices. Thanks to Serge Hallyn for pointing this out. Hard links still point to same inode, what's the issue that this addresses? Ignore that last - I thought it was the filesystem linking permissions thread rather than the BSD Secure linking permissions thread. ;) pgpHd0UzzrMjl.pgp Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
For those systems that have everything on one big partition, you can often do stuff like: ln /etc/passwd /tmp/filename_generated_by_mktemp and wait for /etc/passwd to get clobbered by a cron job run by root... How would /etc/passwd get clobbered? Are you thinking that a tmp cleaner run by cron might delete /tmp/whatever (i.e., delete the hardlink you created above)? But deleting /tmp/whatever is safe; it doesn't affect /etc/passwd. I'm guessing I'm probably missing something. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Tue, 08 Feb 2005 01:48:40 GMT, David Wagner said:
How would /etc/passwd get clobbered? Are you thinking that a tmp
cleaner run by cron might delete /tmp/whatever (i.e., delete the hardlink
you created above)? But deleting /tmp/whatever is safe; it doesn't affect
/etc/passwd. I'm guessing I'm probably missing something.
The attack is to hardlink some tempfile name to some file you want over-written.
This usually involves just a little bit of work, such as recognizing that a
given
root cronjob uses an unsafe predictable filename in /tmp (look at the Bugtraq or
Full-Disclosure archives, there's plenty). Then you set a little program that
sleep()s till a few seconds before the cronjob runs, does a getpid(), and then
sprays hardlinks into the next 15 or 20 things that mktemp() will generate...
Consider how bash implements here scripts:
#!/bin/bash
echo EOF
some trash
EOF
Now let's look at the strace (snipped for brevity..)
statfs(/tmp, {f_type=EXT2_SUPER_MAGIC, f_bsize=1024, f_blocks=253871,
f_bfree=213773, f_bavail=200666, f_files=65536, f_ffree=65445, f_fsid={0, 0},
f_namelen=255, f_frsize=1024}) = 0
time(NULL) = 1107828098
open(/tmp/sh-thd-1107848098, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_LARGEFILE,
0600) = 3
dup(3) = 4
fcntl64(4, F_GETFL) = 0x8001 (flags O_WRONLY|O_LARGEFILE)
fstat64(4, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7d71000
_llseek(4, 0, [0], SEEK_CUR)= 0
write(4, some trash\n, 11)= 11
close(4)= 0
munmap(0xb7d71000, 4096)= 0
open(/tmp/sh-thd-1107848098, O_RDONLY|O_LARGEFILE) = 4
close(3)= 0
unlink(/tmp/sh-thd-1107848098)= 0
fcntl64(0, F_GETFD) = 0
fcntl64(0, F_DUPFD, 10) = 10
fcntl64(0, F_GETFD) = 0
fcntl64(10, F_SETFD, FD_CLOEXEC)= 0
dup2(4, 0) = 0
close(4)= 0
Wow - if my /tmp was on the same partition, and I'd hard-linked that
file to /etc/passwd, it would be toast now if root had run it.
You usually can't control what gets written - but often it's sufficient for the
attacker to simply get a file clobbered
pgp1unSohNbRA.pgp
Description: PGP signature
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
* [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote: open(/tmp/sh-thd-1107848098, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_LARGEFILE, 0600) = 3 O_EXCL Wow - if my /tmp was on the same partition, and I'd hard-linked that file to /etc/passwd, it would be toast now if root had run it. So, in fact, it wouldn't ;-) thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)
On Mon, 07 Feb 2005 18:20:36 PST, Chris Wright said: * [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote: open(/tmp/sh-thd-1107848098, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_LARGEFILE, 0600) = 3 O_EXCL Wow - if my /tmp was on the same partition, and I'd hard-linked that file to /etc/passwd, it would be toast now if root had run it. So, in fact, it wouldn't ;-) Well.. Yeah. bash gets it right, a lot of programs botch it. ;) pgpW8EazQy2Vi.pgp Description: PGP signature
