[PATCH] BSD Secure Levels: printk overhaul, 2.6.11-rc2-mm1 (1/8)

[PATCH] BSD Secure Levels: printk overhaul, 2.6.11-rc2-mm1 (1/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 271 bytes --]

This is the first in a series of eight patches to the BSD Secure
Levels LSM.  It overhauls the printk mechanism in order to reduce the
unnecessary usage of the .text area.  Thanks to Brad Spengler for the
suggestion.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_printk.patch --]
[-- Type: text/plain, Size: 14302 bytes --]

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 14:55:44.799527472 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 14:56:18.527400056 -0600
@@ -101,22 +101,20 @@
 
 #define MY_NAME "seclvl"
 
-/**
- * This time-limits log writes to one per second.
- */
-#define seclvl_printk(verb, type, fmt, arg...)			\
-	do {							\
-		if (verbosity >= verb) {			\
-			static unsigned long _prior;		\
-			unsigned long _now = jiffies;		\
-			if ((_now - _prior) > HZ) {		\
-				printk(type "%s: %s: " fmt,	\
-					MY_NAME, __FUNCTION__ ,	\
-					## arg);		\
-				_prior = _now;			\
-			}					\
-		}						\
-	} while (0)
+static void seclvl_printk( int verb, const char * fmt, ... )
+{
+	va_list args;
+	va_start( args, fmt );
+	if (verbosity >= verb) {
+		static unsigned long _prior;
+		unsigned long _now = jiffies;
+		if ((_now - _prior) > HZ) {
+			vprintk( fmt, args );
+		}
+		_prior = _now;
+	}
+	va_end( args );
+}
 
 /**
  * kobject stuff
@@ -198,15 +196,15 @@
 static int seclvl_sanity(int reqlvl)
 {
 	if ((reqlvl < -1) || (reqlvl > 2)) {
-		seclvl_printk(1, KERN_WARNING, "Attempt to set seclvl out of "
-			      "range: [%d]\n", reqlvl);
+		seclvl_printk(1, KERN_WARNING "%s: Attempt to set seclvl out "
+			      "of range: [%d]\n", __FUNCTION__, reqlvl);
 		return -EINVAL;
 	}
 	if ((seclvl == 0) && (reqlvl == -1))
 		return 0;
 	if (reqlvl < seclvl) {
-		seclvl_printk(1, KERN_WARNING, "Attempt to lower seclvl to "
-			      "[%d]\n", reqlvl);
+		seclvl_printk(1, KERN_WARNING "%s: Attempt to lower seclvl to "
+			      "[%d]\n", __FUNCTION__, reqlvl);
 		return -EPERM;
 	}
 	return 0;
@@ -230,18 +228,18 @@
 static int do_seclvl_advance(int newlvl)
 {
 	if (newlvl <= seclvl) {
-		seclvl_printk(1, KERN_WARNING, "Cannot advance to seclvl "
-			      "[%d]\n", newlvl);
+		seclvl_printk(1, KERN_WARNING "%s: Cannot advance to seclvl "
+			      "[%d]\n", __FUNCTION__, newlvl);
 		return -EINVAL;
 	}
 	if (newlvl > 2) {
-		seclvl_printk(1, KERN_WARNING, "Cannot advance to seclvl "
-			      "[%d]\n", newlvl);
+		seclvl_printk(1, KERN_WARNING "%s: Cannot advance to seclvl "
+			      "[%d]\n", __FUNCTION__, newlvl);
 		return -EINVAL;
 	}
 	if (seclvl == -1) {
-		seclvl_printk(1, KERN_WARNING, "Not allowed to advance to "
-			      "seclvl [%d]\n", seclvl);
+		seclvl_printk(1, KERN_WARNING "%s: Not allowed to advance to "
+			      "seclvl [%d]\n", __FUNCTION__, seclvl);
 		return -EPERM;
 	}
 	seclvl = newlvl;
@@ -257,19 +255,19 @@
 {
 	unsigned long val;
 	if (count > 2 || (count == 2 && buff[1] != '\n')) {
-		seclvl_printk(1, KERN_WARNING, "Invalid value passed to "
-			      "seclvl: [%s]\n", buff);
+		seclvl_printk(1, KERN_WARNING "%s: Invalid value passed to "
+			      "seclvl: [%s]\n", __FUNCTION__, buff);
 		return -EINVAL;
 	}
 	val = buff[0] - 48;
 	if (seclvl_sanity(val)) {
-		seclvl_printk(1, KERN_WARNING, "Illegal secure level "
-			      "requested: [%d]\n", (int)val);
+		seclvl_printk(1, KERN_WARNING "%s: Illegal secure level "
+			      "requested: [%d]\n", __FUNCTION__, (int)val);
 		return -EPERM;
 	}
 	if (do_seclvl_advance(val)) {
-		seclvl_printk(0, KERN_ERR, "Failure advancing security level "
-			      "to %lu\n", val);
+		seclvl_printk(0, KERN_ERR "%s: Failure advancing security "
+			      "level to [%lu]\n", __FUNCTION__, val);
 	}
 	return count;
 }
@@ -316,15 +314,15 @@
 	struct crypto_tfm *tfm;
 	struct scatterlist sg[1];
 	if (len > PAGE_SIZE) {
-		seclvl_printk(0, KERN_ERR, "Plaintext password too large (%d "
-			      "characters).  Largest possible is %lu "
-			      "bytes.\n", len, PAGE_SIZE);
+		seclvl_printk(0, KERN_ERR "%s: Plaintext password too large "
+			      "(%d characters).  Largest possible is %lu "
+			      "bytes.\n", __FUNCTION__, len, PAGE_SIZE);
 		return -ENOMEM;
 	}
 	tfm = crypto_alloc_tfm("sha1", 0);
 	if (tfm == NULL) {
-		seclvl_printk(0, KERN_ERR,
-			      "Failed to load transform for SHA1\n");
+		seclvl_printk(0, KERN_ERR "%s: Failed to load transform for "
+			      "SHA1\n", __FUNCTION__);
 		return -ENOSYS;
 	}
 	// Just get a new page; don't play around with page boundaries
@@ -354,13 +352,13 @@
 	int rc;
 	int len;
 	if (!*passwd && !*sha1_passwd) {
-		seclvl_printk(0, KERN_ERR, "Attempt to password-unlock the "
+		seclvl_printk(0, KERN_ERR "%s: Attempt to password-unlock the "
 			      "seclvl module, but neither a plain text "
 			      "password nor a SHA1 hashed password was "
 			      "passed in as a module parameter!  This is a "
 			      "bug, since it should not be possible to be in "
 			      "this part of the module; please tell a "
-			      "maintainer about this event.\n");
+			      "maintainer about this event.\n", __FUNCTION__);
 		return -EINVAL;
 	}
 	len = strlen(buff);
@@ -370,8 +368,8 @@
 	}
 	/* Hash the password, then compare the hashed values */
 	if ((rc = plaintext_to_sha1(tmp, buff, len))) {
-		seclvl_printk(0, KERN_ERR, "Error hashing password: rc = "
-			      "[%d]\n", rc);
+		seclvl_printk(0, KERN_ERR "%s: Error hashing password: rc = "
+			      "[%d]\n", __FUNCTION__, rc);
 		return rc;
 	}
 	for (i = 0; i < SHA1_DIGEST_SIZE; i++) {
@@ -379,8 +377,8 @@
 			return -EPERM;
 		}
 	}
-	seclvl_printk(0, KERN_INFO,
-		      "Password accepted; seclvl reduced to 0.\n");
+	seclvl_printk(0, KERN_INFO "%s: Password accepted; seclvl reduced to "
+		      "0.\n", __FUNCTION__);
 	seclvl = 0;
 	return count;
 }
@@ -397,9 +395,10 @@
 {
 	if (seclvl >= 0) {
 		if (child->pid == 1) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to ptrace "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to ptrace "
 				      "the init process dissallowed in "
-				      "secure level %d\n", seclvl);
+				      "secure level %d\n", __FUNCTION__,
+				      seclvl);
 			return -EPERM;
 		}
 	}
@@ -421,35 +420,38 @@
 		/* fall through */
 	case 1:
 		if (cap == CAP_LINUX_IMMUTABLE) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to modify "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to modify "
 				      "the IMMUTABLE and/or APPEND extended "
 				      "attribute on a file with the IMMUTABLE "
 				      "and/or APPEND extended attribute set "
-				      "denied in seclvl [%d]\n", seclvl);
+				      "denied in seclvl [%d]\n", __FUNCTION__,
+				      seclvl);
 			return -EPERM;
 		} else if (cap == CAP_SYS_RAWIO) {	// Somewhat broad...
-			seclvl_printk(1, KERN_WARNING, "Attempt to perform "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to perform "
 				      "raw I/O while in secure level [%d] "
-				      "denied\n", seclvl);
+				      "denied\n", __FUNCTION__, seclvl);
 			return -EPERM;
 		} else if (cap == CAP_NET_ADMIN) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to perform "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to perform "
 				      "network administrative task while "
-				      "in secure level [%d] denied\n", seclvl);
+				      "in secure level [%d] denied\n",
+				      __FUNCTION__, seclvl);
 			return -EPERM;
 		} else if (cap == CAP_SETUID) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to setuid "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to setuid "
 				      "while in secure level [%d] denied\n",
-				      seclvl);
+				      __FUNCTION__, seclvl);
 			return -EPERM;
 		} else if (cap == CAP_SETGID) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to setgid "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to setgid "
 				      "while in secure level [%d] denied\n",
-				      seclvl);
+				      __FUNCTION__, seclvl);
 		} else if (cap == CAP_SYS_MODULE) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to perform "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to perform "
 				      "a module operation while in secure "
-				      "level [%d] denied\n", seclvl);
+				      "level [%d] denied\n",
+				      __FUNCTION__, seclvl);
 			return -EPERM;
 		}
 		break;
@@ -459,7 +461,7 @@
 	/* from dummy.c */
 	if (cap_is_fs_cap(cap) ? tsk->fsuid == 0 : tsk->euid == 0)
 		return 0;	/* capability granted */
-	seclvl_printk(1, KERN_WARNING, "Capability denied\n");
+	seclvl_printk(1, KERN_WARNING "%s: Capability denied\n", __FUNCTION__);
 	return -EPERM;		/* capability denied */
 }
 
@@ -473,11 +475,11 @@
 		now = current_kernel_time();
 		if (tv->tv_sec < now.tv_sec ||
 		    (tv->tv_sec == now.tv_sec && tv->tv_nsec < now.tv_nsec)) {
-			seclvl_printk(1, KERN_WARNING, "Attempt to decrement "
-				      "time in secure level %d denied: "
-				      "current->pid = [%d], "
+			seclvl_printk(1, KERN_WARNING "%s: Attempt to "
+				      "decrement time in secure level %d "
+				      "denied: current->pid = [%d], "
 				      "current->group_leader->pid = [%d]\n",
-				      seclvl, current->pid,
+				      __FUNCTION__, seclvl, current->pid,
 				      current->group_leader->pid);
 			return -EPERM;
 		}		/* if attempt to decrement time */
@@ -527,15 +529,16 @@
 	if (current->pid != 1 && S_ISBLK(inode->i_mode) && (mask & MAY_WRITE)) {
 		switch (seclvl) {
 		case 2:
-			seclvl_printk(1, KERN_WARNING, "Write to block device "
-				      "denied in secure level [%d]\n", seclvl);
+			seclvl_printk(1, KERN_WARNING "%s: Write to block "
+				      "device denied in secure level [%d]\n",
+				      __FUNCTION__, seclvl);
 			return -EPERM;
 		case 1:
 			if (seclvl_bd_claim(inode)) {
-				seclvl_printk(1, KERN_WARNING,
-					      "Write to mounted block device "
-					      "denied in secure level [%d]\n",
-					      seclvl);
+				seclvl_printk(1, KERN_WARNING "%s: Write to "
+					      "mounted block device denied in "
+					      "secure level [%d]\n",
+					      __FUNCTION__, seclvl);
 				return -EPERM;
 			}
 		}
@@ -552,10 +555,10 @@
 		if (iattr->ia_valid & ATTR_MODE)
 			if (iattr->ia_mode & S_ISUID ||
 			    iattr->ia_mode & S_ISGID) {
-				seclvl_printk(1, KERN_WARNING, "Attempt to "
+				seclvl_printk(1, KERN_WARNING "%s: Attempt to "
 					      "modify SUID or SGID bit "
 					      "denied in seclvl [%d]\n",
-					      seclvl);
+					      __FUNCTION__, seclvl);
 				return -EPERM;
 			}
 	}
@@ -583,8 +586,8 @@
 		return 0;
 	}
 	if (seclvl == 2) {
-		seclvl_printk(1, KERN_WARNING, "Attempt to unmount in secure "
-			      "level %d\n", seclvl);
+		seclvl_printk(1, KERN_WARNING "%s: Attempt to unmount in "
+			      "secure level [%d]\n", __FUNCTION__, seclvl);
 		return -EPERM;
 	}
 	return 0;
@@ -609,16 +612,16 @@
 	hashedPassword[0] = '\0';
 	if (*passwd) {
 		if (*sha1_passwd) {
-			seclvl_printk(0, KERN_ERR, "Error: Both "
+			seclvl_printk(0, KERN_ERR "%s: Error: Both "
 				      "passwd and sha1_passwd "
 				      "were set, but they are mutually "
-				      "exclusive.\n");
+				      "exclusive.\n", __FUNCTION__);
 			return -EINVAL;
 		}
 		if ((rc = plaintext_to_sha1(hashedPassword, passwd,
 					    strlen(passwd)))) {
-			seclvl_printk(0, KERN_ERR, "Error: SHA1 support not "
-				      "in kernel\n");
+			seclvl_printk(0, KERN_ERR "%s: Error: SHA1 support "
+				      "not in kernel\n", __FUNCTION__);
 			return rc;
 		}
 		/* All static data goes to the BSS, which zero's the
@@ -627,10 +630,10 @@
 		int i;
 		i = strlen(sha1_passwd);
 		if (i != (SHA1_DIGEST_SIZE * 2)) {
-			seclvl_printk(0, KERN_ERR, "Received [%d] bytes; "
+			seclvl_printk(0, KERN_ERR "%s: Received [%d] bytes; "
 				      "expected [%d] for the hexadecimal "
 				      "representation of the SHA1 hash of "
-				      "the password.\n",
+				      "the password.\n", __FUNCTION__,
 				      i, (SHA1_DIGEST_SIZE * 2));
 			return -EINVAL;
 		}
@@ -653,8 +656,8 @@
 {
 	int rc = 0;
 	if ((rc = subsystem_register(&seclvl_subsys))) {
-		seclvl_printk(0, KERN_WARNING,
-			      "Error [%d] registering seclvl subsystem\n", rc);
+		seclvl_printk(0, KERN_WARNING "Error [%d] registering seclvl "
+			      "subsystem\n", __FUNCTION__, rc);
 		return rc;
 	}
 	sysfs_create_file(&seclvl_subsys.kset.kobj, &sysfs_attr_seclvl.attr);
@@ -680,37 +683,39 @@
 	sysfs_attr_seclvl.attr.owner = THIS_MODULE;
 	sysfs_attr_passwd.attr.owner = THIS_MODULE;
 	if (initlvl < -1 || initlvl > 2) {
-		seclvl_printk(0, KERN_ERR, "Error: bad initial securelevel "
-			      "[%d].\n", initlvl);
+		seclvl_printk(0, KERN_ERR "%s: Error: bad initial securelevel "
+			      "[%d].\n", __FUNCTION__, initlvl);
 		rc = -EINVAL;
 		goto exit;
 	}
 	seclvl = initlvl;
 	if ((rc = processPassword())) {
-		seclvl_printk(0, KERN_ERR, "Error processing the password "
-			      "module parameter(s): rc = [%d]\n", rc);
+		seclvl_printk(0, KERN_ERR "%s: Error processing the password "
+			      "module parameter(s): rc = [%d]\n", __FUNCTION__,
+			      rc);
 		goto exit;
 	}
 	/* register ourselves with the security framework */
 	if (register_security(&seclvl_ops)) {
-		seclvl_printk(0, KERN_ERR,
-			      "seclvl: Failure registering with the "
-			      "kernel.\n");
+		seclvl_printk(0, KERN_ERR "%s: seclvl: Failure registering "
+			      "with the kernel.\n", __FUNCTION__);
 		/* try registering with primary module */
 		rc = mod_reg_security(MY_NAME, &seclvl_ops);
 		if (rc) {
-			seclvl_printk(0, KERN_ERR, "seclvl: Failure "
+			seclvl_printk(0, KERN_ERR "%s: seclvl: Failure "
 				      "registering with primary security "
-				      "module.\n");
+				      "module.\n", __FUNCTION__);
 			goto exit;
 		}		/* if primary module registered */
 		secondary = 1;
 	}			/* if we registered ourselves with the security framework */
 	if ((rc = doSysfsRegistrations())) {
-		seclvl_printk(0, KERN_ERR, "Error registering with sysfs\n");
+		seclvl_printk(0, KERN_ERR "%s: Error registering with sysfs\n",
+			      __FUNCTION__);
 		goto exit;
 	}
-	seclvl_printk(0, KERN_INFO, "seclvl: Successfully initialized.\n");
+	seclvl_printk(0, KERN_INFO "%s: seclvl: Successfully initialized.\n",
+		      __FUNCTION__);
  exit:
 	if (rc) {
 		printk(KERN_ERR "seclvl: Error during initialization: rc = "
@@ -733,9 +738,8 @@
 	if (secondary == 1) {
 		mod_unreg_security(MY_NAME, &seclvl_ops);
 	} else if (unregister_security(&seclvl_ops)) {
-		seclvl_printk(0, KERN_INFO,
-			      "seclvl: Failure unregistering with the "
-			      "kernel\n");
+		seclvl_printk(0, KERN_INFO "%s: seclvl: Failure unregistering "
+			      "with the kernel\n", __FUNCTION__);
 	}
 }
 

[PATCH] BSD Secure Levels: suid/sgid on directories; open/mknod issue, 2.6.11-rc2-mm1 (2/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

This is the second in a series of eight patches to the BSD Secure
Levels LSM.  It allows setuid and setgid on directories.  It also
disallows the creation of setuid/setgid executables via open or mknod.
Thanks to Brad Spengler for the suggestion.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_suid_and_guid.patch --]
[-- Type: text/plain, Size: 2028 bytes --]

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:14:54.907684456 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 15:36:43.925683472 -0600
@@ -552,7 +552,11 @@
 static int seclvl_inode_setattr(struct dentry *dentry, struct iattr *iattr)
 {
 	if (seclvl > 0) {
-		if (iattr->ia_valid & ATTR_MODE)
+		if (dentry && dentry->d_inode
+		    && S_ISDIR(dentry->d_inode->i_mode)) {
+			return 0;
+		}
+		if (iattr && iattr->ia_valid & ATTR_MODE)
 			if (iattr->ia_mode & S_ISUID ||
 			    iattr->ia_mode & S_ISGID) {
 				seclvl_printk(1, KERN_WARNING "%s: Attempt to "
@@ -565,6 +569,36 @@
 	return 0;
 }
 
+/**
+ * Prevent an end-run around the inode_setattr control.
+ */
+static int seclvl_inode_mknod (struct inode * inode, struct dentry * dentry,
+			       int mode, dev_t dev)
+{
+	if (seclvl > 0 && (mode & 02000 || mode & 04000)) {
+		seclvl_printk(1, KERN_WARNING "%s: Attempt to mknod with suid "
+			      "or guid bit set in seclvl [%d]\n", __FUNCTION__,
+			      seclvl );
+		return -EPERM;
+	}
+        return 0;
+}
+
+/**
+ * Prevent an end-run around the inode_setattr control.
+ */
+static int
+seclvl_inode_create (struct inode * inode, struct dentry * dentry, int mask)
+{
+	if (seclvl > 0 && (mask & 02000 || mask & 04000)) {
+		seclvl_printk(1, KERN_WARNING "%s: Attempt to "
+			      "create inode with suid or guid bit set in "
+			      "seclvl [%d]\n", __FUNCTION__, seclvl );
+		return -EPERM;
+	}
+        return 0;
+}
+
 /* release busied block devices */
 static void seclvl_file_free_security(struct file *filp)
 {
@@ -598,6 +632,8 @@
 	.capable = seclvl_capable,
 	.inode_permission = seclvl_inode_permission,
 	.inode_setattr = seclvl_inode_setattr,
+	.inode_mknod = seclvl_inode_mknod,
+	.inode_create = seclvl_inode_create,
 	.file_free_security = seclvl_file_free_security,
 	.settime = seclvl_settime,
 	.sb_umount = seclvl_umount,

[PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

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 <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_bd_claim.patch --]
[-- Type: text/plain, Size: 3656 bytes --]

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,

[PATCH] BSD Secure Levels: memory alloc failure check, 2.6.11-rc2-mm1 (4/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

This is the fourth in a series of eight patches to the BSD Secure
Levels LSM.  It adds a check for a memory allocation failure
condition.  Thanks to Vesa-Matti J Kari for pointing out this problem.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_mem_alloc_check.patch --]
[-- Type: text/plain, Size: 1289 bytes --]

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:37:26.231252048 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 15:39:35.786556648 -0600
@@ -310,7 +310,7 @@
 static int
 plaintext_to_sha1(unsigned char *hash, const char *plaintext, int len)
 {
-	char *pgVirtAddr;
+	char *pg_virt_addr;
 	struct crypto_tfm *tfm;
 	struct scatterlist sg[1];
 	if (len > PAGE_SIZE) {
@@ -327,16 +327,20 @@
 	}
 	// Just get a new page; don't play around with page boundaries
 	// and scatterlists.
-	pgVirtAddr = (char *)__get_free_page(GFP_KERNEL);
-	sg[0].page = virt_to_page(pgVirtAddr);
+	pg_virt_addr = (char *)__get_free_page(GFP_KERNEL);
+	if (!pg_virt_addr) {
+		seclvl_printk(0, KERN_ERR "%s: Out of memory\n", __FUNCTION__);
+		return -ENOMEM;
+	}	
+	sg[0].page = virt_to_page(pg_virt_addr);
 	sg[0].offset = 0;
 	sg[0].length = len;
-	strncpy(pgVirtAddr, plaintext, len);
+	strncpy(pg_virt_addr, plaintext, len);
 	crypto_digest_init(tfm);
 	crypto_digest_update(tfm, sg, 1);
 	crypto_digest_final(tfm, hash);
 	crypto_free_tfm(tfm);
-	free_page((unsigned long)pgVirtAddr);
+	free_page((unsigned long)pg_virt_addr);
 	return 0;
 }
 

[PATCH] BSD Secure Levels: allow setuid/setgid on process if root, 2.6.11-rc2-mm1 (5/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

This is the fifth in a series of eight patches to the BSD Secure
Levels LSM.  It allows setuid and setgid on a process if the user is
already root.  This allows non-root users to log in.  Thanks to Serge
Hallyn for the suggestion.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_setuid_and_setgid.patch --]
[-- Type: text/plain, Size: 907 bytes --]

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:39:35.786556648 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 15:41:46.043754544 -0600
@@ -442,12 +442,12 @@
 				      "in secure level [%d] denied\n",
 				      __FUNCTION__, seclvl);
 			return -EPERM;
-		} else if (cap == CAP_SETUID) {
+		} else if (cap == CAP_SETUID && current->uid != 0) {
 			seclvl_printk(1, KERN_WARNING "%s: Attempt to setuid "
 				      "while in secure level [%d] denied\n",
 				      __FUNCTION__, seclvl);
 			return -EPERM;
-		} else if (cap == CAP_SETGID) {
+		} else if (cap == CAP_SETGID && current->uid != 0) {
 			seclvl_printk(1, KERN_WARNING "%s: Attempt to setgid "
 				      "while in secure level [%d] denied\n",
 				      __FUNCTION__, seclvl);

[PATCH] BSD Secure Levels: nits, 2.6.11-rc2-mm1 (6/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]

This is the sixth in a series of eight patches to the BSD Secure
Levels LSM.  It makes several trivial changes to make the code
consistent.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_nits.patch --]
[-- Type: text/plain, Size: 8761 bytes --]

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:41:46.043754544 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 15:47:52.249082872 -0600
@@ -2,12 +2,12 @@
  * BSD Secure Levels LSM
  *
  * Maintainers:
- *	Michael A. Halcrow <mike@halcrow.us>
- *	Serge Hallyn <hallyn@cs.wm.edu>
+ *	Michael A. Halcrow <mhalcrow@us.ibm.com>
+ *	Serge Hallyn <serue@us.ibm.com>
  *
  * Copyright (c) 2001 WireX Communications, Inc <chris@wirex.com>
  * Copyright (c) 2001 Greg Kroah-Hartman <greg@kroah.com>
- * Copyright (c) 2002 International Business Machines <robb@austin.ibm.com>
+ * Copyright (c) 2002 International Business Machines <mhalcrow@us.ibm.com>
  *
  *	This program is free software; you can redistribute it and/or modify
  *	it under the terms of the GNU General Public License as published by
@@ -30,9 +30,9 @@
 #include <linux/proc_fs.h>
 #include <linux/kobject.h>
 #include <linux/crypto.h>
-#include <asm/scatterlist.h>
 #include <linux/gfp.h>
 #include <linux/sysfs.h>
+#include <asm/scatterlist.h>
 
 #define SHA1_DIGEST_SIZE 20
 
@@ -93,9 +93,9 @@
 		 "sets seclvl=0 when plaintext password is written to "
 		 "(sysfs mount point)/seclvl/passwd\n");
 
-static int hideHash = 1;
-module_param(hideHash, int, 0);
-MODULE_PARM_DESC(hideHash, "When set to 0, reading seclvl/passwd from sysfs "
+static int hide_hash = 1;
+module_param(hide_hash, int, 0);
+MODULE_PARM_DESC(hide_hash, "When set to 0, reading seclvl/passwd from sysfs "
 		 "will return the SHA1-hashed value of the password that "
 		 "lowers the secure level to 0.\n");
 
@@ -123,7 +123,7 @@
 struct subsystem seclvl_subsys;
 
 struct seclvl_obj {
-	char *name;
+	char * name;
 	struct list_head slot_list;
 	struct kobject kobj;
 };
@@ -147,20 +147,20 @@
  * unique for "passwd" and "seclvl".
  */
 static ssize_t
-seclvl_attr_store(struct kobject *kobj,
-		  struct attribute *attr, const char *buf, size_t len)
+seclvl_attr_store(struct kobject * kobj,
+		  struct attribute * attr, const char * buf, size_t len)
 {
-	struct seclvl_obj *obj = container_of(kobj, struct seclvl_obj, kobj);
-	struct seclvl_attribute *attribute =
+	struct seclvl_obj * obj = container_of(kobj, struct seclvl_obj, kobj);
+	struct seclvl_attribute * attribute =
 	    container_of(attr, struct seclvl_attribute, attr);
 	return (attribute->store ? attribute->store(obj, buf, len) : 0);
 }
 
 static ssize_t
-seclvl_attr_show(struct kobject *kobj, struct attribute *attr, char *buf)
+seclvl_attr_show(struct kobject * kobj, struct attribute * attr, char * buf)
 {
-	struct seclvl_obj *obj = container_of(kobj, struct seclvl_obj, kobj);
-	struct seclvl_attribute *attribute =
+	struct seclvl_obj * obj = container_of(kobj, struct seclvl_obj, kobj);
+	struct seclvl_attribute * attribute =
 	    container_of(attr, struct seclvl_attribute, attr);
 	return (attribute->show ? attribute->show(obj, buf) : 0);
 }
@@ -214,7 +214,7 @@
  * Called whenever the user reads the sysfs handle to this kernel
  * object
  */
-static ssize_t seclvl_read_file(struct seclvl_obj *obj, char *buff)
+static ssize_t seclvl_read_file(struct seclvl_obj * obj, char * buff)
 {
 	return snprintf(buff, PAGE_SIZE, "%d\n", seclvl);
 }
@@ -251,7 +251,7 @@
  * object (seclvl/seclvl).  It expects a single-digit number.
  */
 static ssize_t
-seclvl_write_file(struct seclvl_obj *obj, const char *buff, size_t count)
+seclvl_write_file(struct seclvl_obj * obj, const char * buff, size_t count)
 {
 	unsigned long val;
 	if (count > 2 || (count == 2 && buff[1] != '\n')) {
@@ -277,23 +277,23 @@
 __ATTR(seclvl, (S_IFREG | S_IRUGO | S_IWUSR), seclvl_read_file,
        seclvl_write_file);
 
-static unsigned char hashedPassword[SHA1_DIGEST_SIZE];
+static unsigned char hashed_password[SHA1_DIGEST_SIZE];
 
 /**
  * Called whenever the user reads the sysfs passwd handle.
  */
-static ssize_t seclvl_read_passwd(struct seclvl_obj *obj, char *buff)
+static ssize_t seclvl_read_passwd(struct seclvl_obj * obj, char * buff)
 {
 	/* So just how good *is* your password? :-) */
 	char tmp[3];
 	int i = 0;
 	buff[0] = '\0';
-	if (hideHash) {
+	if (hide_hash) {
 		/* Security through obscurity */
 		return 0;
 	}
 	while (i < SHA1_DIGEST_SIZE) {
-		snprintf(tmp, 3, "%02x", hashedPassword[i]);
+		snprintf(tmp, 3, "%02x", hashed_password[i]);
 		strncat(buff, tmp, 2);
 		i++;
 	}
@@ -308,10 +308,10 @@
  * people...
  */
 static int
-plaintext_to_sha1(unsigned char *hash, const char *plaintext, int len)
+plaintext_to_sha1(unsigned char * hash, const char * plaintext, int len)
 {
-	char *pg_virt_addr;
-	struct crypto_tfm *tfm;
+	char * pg_virt_addr;
+	struct crypto_tfm * tfm;
 	struct scatterlist sg[1];
 	if (len > PAGE_SIZE) {
 		seclvl_printk(0, KERN_ERR "%s: Plaintext password too large "
@@ -349,7 +349,7 @@
  * object.  It hashes the password and compares the hashed results.
  */
 static ssize_t
-seclvl_write_passwd(struct seclvl_obj *obj, const char *buff, size_t count)
+seclvl_write_passwd(struct seclvl_obj * obj, const char * buff, size_t count)
 {
 	int i;
 	unsigned char tmp[SHA1_DIGEST_SIZE];
@@ -377,7 +377,7 @@
 		return rc;
 	}
 	for (i = 0; i < SHA1_DIGEST_SIZE; i++) {
-		if (hashedPassword[i] != tmp[i]) {
+		if (hashed_password[i] != tmp[i]) {
 			return -EPERM;
 		}
 	}
@@ -395,7 +395,8 @@
 /**
  * Explicitely disallow ptrace'ing the init process.
  */
-static int seclvl_ptrace(struct task_struct *parent, struct task_struct *child)
+static int
+seclvl_ptrace(struct task_struct * parent, struct task_struct * child)
 {
 	if (seclvl >= 0) {
 		if (child->pid == 1) {
@@ -413,7 +414,7 @@
  * Capability checks for seclvl.  The majority of the policy
  * enforcement for seclvl takes place here.
  */
-static int seclvl_capable(struct task_struct *tsk, int cap)
+static int seclvl_capable(struct task_struct * tsk, int cap)
 {
 	/* init can do anything it wants */
 	if (tsk->pid == 1)
@@ -472,7 +473,7 @@
 /**
  * Disallow reversing the clock in seclvl > 1
  */
-static int seclvl_settime(struct timespec *tv, struct timezone *tz)
+static int seclvl_settime(struct timespec * tv, struct timezone * tz)
 {
 	struct timespec now;
 	if (seclvl > 1) {
@@ -497,7 +498,7 @@
 static int seclvl_bd_claim(struct file * filp)
 {
 	int holder;
-	struct block_device *bdev = NULL;
+	struct block_device * bdev = NULL;
 	dev_t dev = filp->f_dentry->d_inode->i_rdev;
 	bdev = open_by_devnum(dev, FMODE_WRITE);
 	if (bdev) {
@@ -615,7 +616,7 @@
 /**
  * Cannot unmount in secure level 2
  */
-static int seclvl_umount(struct vfsmount *mnt, int flags)
+static int seclvl_umount(struct vfsmount * mnt, int flags)
 {
 	if (current->pid == 1) {
 		return 0;
@@ -643,10 +644,10 @@
 /**
  * Process the password-related module parameters
  */
-static int processPassword(void)
+static int process_password(void)
 {
 	int rc = 0;
-	hashedPassword[0] = '\0';
+	hashed_password[0] = '\0';
 	if (*passwd) {
 		if (*sha1_passwd) {
 			seclvl_printk(0, KERN_ERR "%s: Error: Both "
@@ -655,7 +656,7 @@
 				      "exclusive.\n", __FUNCTION__);
 			return -EINVAL;
 		}
-		if ((rc = plaintext_to_sha1(hashedPassword, passwd,
+		if ((rc = plaintext_to_sha1(hashed_password, passwd,
 					    strlen(passwd)))) {
 			seclvl_printk(0, KERN_ERR "%s: Error: SHA1 support "
 				      "not in kernel\n", __FUNCTION__);
@@ -678,7 +679,7 @@
 			unsigned char tmp;
 			tmp = sha1_passwd[i + 2];
 			sha1_passwd[i + 2] = '\0';
-			hashedPassword[i / 2] = (unsigned char)
+			hashed_password[i / 2] = (unsigned char)
 			    simple_strtol(&sha1_passwd[i], NULL, 16);
 			sha1_passwd[i + 2] = tmp;
 		}
@@ -689,7 +690,7 @@
 /**
  * Sysfs registrations
  */
-static int doSysfsRegistrations(void)
+static int do_sysfs_registrations(void)
 {
 	int rc = 0;
 	if ((rc = subsystem_register(&seclvl_subsys))) {
@@ -726,7 +727,7 @@
 		goto exit;
 	}
 	seclvl = initlvl;
-	if ((rc = processPassword())) {
+	if ((rc = process_password())) {
 		seclvl_printk(0, KERN_ERR "%s: Error processing the password "
 			      "module parameter(s): rc = [%d]\n", __FUNCTION__,
 			      rc);
@@ -746,7 +747,7 @@
 		}		/* if primary module registered */
 		secondary = 1;
 	}			/* if we registered ourselves with the security framework */
-	if ((rc = doSysfsRegistrations())) {
+	if ((rc = do_sysfs_registrations())) {
 		seclvl_printk(0, KERN_ERR "%s: Error registering with sysfs\n",
 			      __FUNCTION__);
 		goto exit;
@@ -783,6 +784,6 @@
 module_init(seclvl_init);
 module_exit(seclvl_exit);
 
-MODULE_AUTHOR("Michael A. Halcrow <mike@halcrow.us>");
+MODULE_AUTHOR("Michael A. Halcrow <mhalcrow@us.ibm.com>");
 MODULE_DESCRIPTION("LSM implementation of the BSD Secure Levels");
 MODULE_LICENSE("GPL");

[PATCH] BSD Secure Levels: comment cleanups, 2.6.11-rc2-mm1 (7/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 223 bytes --]

This is the seventh in a series of eight patches to the BSD Secure
Levels LSM.  It makes several trivial changes to comments in order to
make the code look more pretty.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_comment_cleanups.patch --]
[-- Type: text/plain, Size: 4612 bytes --]

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:47:52.249082872 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 15:54:35.055846936 -0600
@@ -52,7 +52,9 @@
 module_param(initlvl, int, 0);
 MODULE_PARM_DESC(initlvl, "Initial secure level (defaults to 1)");
 
-/* Module parameter that defines the verbosity level */
+/**
+ * Module parameter that defines the verbosity level.
+ */
 static int verbosity;
 module_param(verbosity, int, 0);
 MODULE_PARM_DESC(verbosity, "Initial verbosity level (0 or 1; defaults to "
@@ -166,7 +168,7 @@
 }
 
 /**
- * Callback function pointers for show and store
+ * Callback function pointers for show and store.
  */
 static struct sysfs_ops seclvlfs_sysfs_ops = {
 	.show = seclvl_attr_show,
@@ -185,7 +187,7 @@
 static int seclvl;
 
 /**
- * flag to keep track of how we were registered
+ * Flag to keep track of how we were registered.
  */
 static int secondary;
 
@@ -212,7 +214,7 @@
 
 /**
  * Called whenever the user reads the sysfs handle to this kernel
- * object
+ * object.
  */
 static ssize_t seclvl_read_file(struct seclvl_obj * obj, char * buff)
 {
@@ -220,7 +222,7 @@
 }
 
 /**
- * security level advancement rules:
+ * Security level advancement rules:
  *   Valid levels are -1 through 2, inclusive.
  *   From -1, stuck.  [ in case compiled into kernel ]
  *   From 0 or above, can only increment.
@@ -272,7 +274,9 @@
 	return count;
 }
 
-/* Generate sysfs_attr_seclvl */
+/**
+ * Generate sysfs_attr_seclvl.
+ */
 static struct seclvl_attribute sysfs_attr_seclvl =
 __ATTR(seclvl, (S_IFREG | S_IRUGO | S_IWUSR), seclvl_read_file,
        seclvl_write_file);
@@ -284,12 +288,10 @@
  */
 static ssize_t seclvl_read_passwd(struct seclvl_obj * obj, char * buff)
 {
-	/* So just how good *is* your password? :-) */
 	char tmp[3];
 	int i = 0;
 	buff[0] = '\0';
 	if (hide_hash) {
-		/* Security through obscurity */
 		return 0;
 	}
 	while (i < SHA1_DIGEST_SIZE) {
@@ -325,8 +327,8 @@
 			      "SHA1\n", __FUNCTION__);
 		return -ENOSYS;
 	}
-	// Just get a new page; don't play around with page boundaries
-	// and scatterlists.
+	/* Just get a new page; don't play around with page boundaries
+	   and scatterlists. */
 	pg_virt_addr = (char *)__get_free_page(GFP_KERNEL);
 	if (!pg_virt_addr) {
 		seclvl_printk(0, KERN_ERR "%s: Out of memory\n", __FUNCTION__);
@@ -387,7 +389,9 @@
 	return count;
 }
 
-/* Generate sysfs_attr_passwd */
+/**
+ * Generate sysfs_attr_passwd.
+ */
 static struct seclvl_attribute sysfs_attr_passwd =
 __ATTR(passwd, (S_IFREG | S_IRUGO | S_IWUSR), seclvl_read_passwd,
        seclvl_write_passwd);
@@ -432,7 +436,7 @@
 				      "denied in seclvl [%d]\n", __FUNCTION__,
 				      seclvl);
 			return -EPERM;
-		} else if (cap == CAP_SYS_RAWIO) {	// Somewhat broad...
+		} else if (cap == CAP_SYS_RAWIO) { /* Somewhat broad */
 			seclvl_printk(1, KERN_WARNING "%s: Attempt to perform "
 				      "raw I/O while in secure level [%d] "
 				      "denied\n", __FUNCTION__, seclvl);
@@ -487,8 +491,8 @@
 				      __FUNCTION__, seclvl, current->pid,
 				      current->group_leader->pid);
 			return -EPERM;
-		}		/* if attempt to decrement time */
-	}			/* if seclvl > 1 */
+		}
+	}
 	return 0;
 }
 
@@ -614,7 +618,7 @@
 }
 
 /**
- * Cannot unmount in secure level 2
+ * Cannot unmount in secure level 2.
  */
 static int seclvl_umount(struct vfsmount * mnt, int flags)
 {
@@ -642,7 +646,7 @@
 };
 
 /**
- * Process the password-related module parameters
+ * Process the password-related module parameters.
  */
 static int process_password(void)
 {
@@ -662,9 +666,9 @@
 				      "not in kernel\n", __FUNCTION__);
 			return rc;
 		}
-		/* All static data goes to the BSS, which zero's the
+		/* All static data goes to the BSS, which wipes the
 		 * plaintext password out for us. */
-	} else if (*sha1_passwd) {	// Base 16
+	} else if (*sha1_passwd) { /* Base 16 */
 		int i;
 		i = strlen(sha1_passwd);
 		if (i != (SHA1_DIGEST_SIZE * 2)) {
@@ -688,7 +692,7 @@
 }
 
 /**
- * Sysfs registrations
+ * Sysfs registrations.
  */
 static int do_sysfs_registrations(void)
 {
@@ -744,9 +748,9 @@
 				      "registering with primary security "
 				      "module.\n", __FUNCTION__);
 			goto exit;
-		}		/* if primary module registered */
+		}
 		secondary = 1;
-	}			/* if we registered ourselves with the security framework */
+	}
 	if ((rc = do_sysfs_registrations())) {
 		seclvl_printk(0, KERN_ERR "%s: Error registering with sysfs\n",
 			      __FUNCTION__);

[PATCH] BSD Secure Levels: remove ptrace, 2.6.11-rc2-mm1 (8/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

This is the eighth in a series of eight patches to the BSD Secure
Levels LSM.  It removes the ptrace check because it is redundant with
the check made in kernel/ptrace.c.  Thanks for Brad Spengler for this
suggestion.

Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

[-- Attachment #2: seclvl_remove_ptrace.patch --]
[-- Type: text/plain, Size: 1102 bytes --]

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:54:35.055846936 -0600
+++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 15:55:29.109629512 -0600
@@ -397,24 +397,6 @@
        seclvl_write_passwd);
 
 /**
- * Explicitely disallow ptrace'ing the init process.
- */
-static int
-seclvl_ptrace(struct task_struct * parent, struct task_struct * child)
-{
-	if (seclvl >= 0) {
-		if (child->pid == 1) {
-			seclvl_printk(1, KERN_WARNING "%s: Attempt to ptrace "
-				      "the init process dissallowed in "
-				      "secure level %d\n", __FUNCTION__,
-				      seclvl);
-			return -EPERM;
-		}
-	}
-	return 0;
-}
-
-/**
  * Capability checks for seclvl.  The majority of the policy
  * enforcement for seclvl takes place here.
  */
@@ -634,7 +616,6 @@
 }
 
 static struct security_operations seclvl_ops = {
-	.ptrace = seclvl_ptrace,
 	.capable = seclvl_capable,
 	.file_permission = seclvl_file_permission,
 	.inode_setattr = seclvl_inode_setattr,

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Chris Wright]

* Michael Halcrow (mhalcrow@us.ibm.com) 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

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 644 bytes --]

On Mon, 07 Feb 2005 14:26:03 PST, Chris Wright said:
> * Michael Halcrow (mhalcrow@us.ibm.com) 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. ;)


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[David Wagner]

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

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2393 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Chris Wright]

* Valdis.Kletnieks@vt.edu (Valdis.Kletnieks@vt.edu) 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

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 460 bytes --]

On Mon, 07 Feb 2005 18:20:36 PST, Chris Wright said:
> * Valdis.Kletnieks@vt.edu (Valdis.Kletnieks@vt.edu) 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. ;)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[David Wagner]

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

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Michael Halcrow]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

On Mon, Feb 07, 2005 at 02:26:03PM -0800, Chris Wright wrote:
> * Michael Halcrow (mhalcrow@us.ibm.com) 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? 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Serge E. Hallyn]

Quoting Valdis.Kletnieks@vt.edu (Valdis.Kletnieks@vt.edu):
> 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

Re: [PATCH] BSD Secure Levels: claim block dev in file struct rather than inode struct, 2.6.11-rc2-mm1 (3/8)

[Chris Wright]

* Michael Halcrow (mhalcrow@us.ibm.com) 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 <chrisw@osdl.org>

--- a/security/seclvl.c~bd_claim	2005-02-08 15:05:09.000000000 -0800
+++ b/security/seclvl.c	2005-02-08 15:05:17.000000000 -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);

Re: [PATCH] BSD Secure Levels: nits, 2.6.11-rc2-mm1 (6/8)

[Chris Wright]

* Michael Halcrow (mhalcrow@us.ibm.com) wrote:
> This is the sixth in a series of eight patches to the BSD Secure
> Levels LSM.  It makes several trivial changes to make the code
> consistent.

These are inconsistent with CodingStyle.  I'd drop this, and go the
other way (patch is smaller) ala Lindent.

>  struct seclvl_obj {
> -	char *name;
> +	char * name;

This is opposite of typical style.

> -seclvl_attr_store(struct kobject *kobj,
> -		  struct attribute *attr, const char *buf, size_t len)
> +seclvl_attr_store(struct kobject * kobj,
> +		  struct attribute * attr, const char * buf, size_t len)

same here...etc.

Lindent nearly undoes all these changes.  If we're going to reformat
code, I'd prefer to see it done via Lindent.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] BSD Secure Levels: printk overhaul, 2.6.11-rc2-mm1 (1/8)

[Matt Mackall]

On Mon, Feb 07, 2005 at 01:21:08PM -0600, Michael Halcrow wrote:
> This is the first in a series of eight patches to the BSD Secure
> Levels LSM.  It overhauls the printk mechanism in order to reduce the
> unnecessary usage of the .text area.  Thanks to Brad Spengler for the
> suggestion.
> 
> Signed off by: Michael Halcrow <mhalcrow@us.ibm.com>

> 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 14:55:44.799527472 -0600
> +++ linux-2.6.11-rc2-mm1-modules/security/seclvl.c	2005-02-03 14:56:18.527400056 -0600
> @@ -101,22 +101,20 @@
>  
>  #define MY_NAME "seclvl"
>  
> -/**
> - * This time-limits log writes to one per second.
> - */
> -#define seclvl_printk(verb, type, fmt, arg...)			\
> -	do {							\
> -		if (verbosity >= verb) {			\
> -			static unsigned long _prior;		\
> -			unsigned long _now = jiffies;		\
> -			if ((_now - _prior) > HZ) {		\
> -				printk(type "%s: %s: " fmt,	\
> -					MY_NAME, __FUNCTION__ ,	\
> -					## arg);		\
> -				_prior = _now;			\
> -			}					\
> -		}						\
> -	} while (0)
> +static void seclvl_printk( int verb, const char * fmt, ... )
> +{
> +	va_list args;
> +	va_start( args, fmt );
> +	if (verbosity >= verb) {
> +		static unsigned long _prior;
> +		unsigned long _now = jiffies;
> +		if ((_now - _prior) > HZ) {
> +			vprintk( fmt, args );
> +		}
> +		_prior = _now;
> +	}
> +	va_end( args );
> +}

This could be done with a seclvl_printk macro wrapping a
__seclvl_printk function that provides __FUNCTION__, leaving the
callers the same.

-- 
Mathematics is the supreme nostalgia of our time.