2005-05-17 15:24:12

by Michael Halcrow

Subject: [patch 1/7] BSD Secure Levels: printk overhaul

This is the first in a series of seven 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.

Note that these supersede the series of patches submitted on Feb 7th.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 14:52:41.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:23:15.000000000 -0500
@@ -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
@@ -138,8 +136,8 @@
*/
struct seclvl_attribute {
struct attribute attr;
- ssize_t(*show) (struct seclvl_obj *, char *);
- ssize_t(*store) (struct seclvl_obj *, const char *, size_t);
+ ssize_t(*show) (struct seclvl_obj *, char *);
+ ssize_t(*store) (struct seclvl_obj *, const char *, size_t);
};

/**
@@ -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,38 +683,40 @@
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");
- exit:
+ seclvl_printk(0, KERN_INFO "%s: seclvl: Successfully initialized.\n",
+ __FUNCTION__);
+ exit:
if (rc) {
printk(KERN_ERR "seclvl: Error during initialization: rc = "
"[%d]\n", 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__);
}
}

2005-05-17 15:26:30

by Michael Halcrow

Subject: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

This is the second in a series of seven 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. Chris Wright observed that
using filp as a holder addresses the fact that multiple processes can
share an fd.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 16:23:15.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:27:27.000000000 -0500
@@ -487,46 +487,34 @@
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)) {
+ if (bd_claim(bdev, filp)) {
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 = filp;
}
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 +522,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",
@@ -565,15 +553,23 @@
return 0;
}

-/* release busied block devices */
+/**
+ * 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);
+ if (dentry && (filp->f_mode & FMODE_WRITE)) {
+ struct inode *inode = dentry->d_inode;
+ if (inode && S_ISBLK(inode->i_mode)
+ && filp->f_security == filp) {
+ struct block_device *bdev = inode->i_bdev;
+ if (bdev) {
+ bd_release(bdev);
+ blkdev_put(bdev);
+ filp->f_security = NULL;
+ }
+ }
}
}

@@ -596,7 +592,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,
.file_free_security = seclvl_file_free_security,
.settime = seclvl_settime,

2005-05-17 15:27:21

by Michael Halcrow

Subject: [patch 3/7] BSD Secure Levels: allow suid and sgid on directories

This is the third in a series of seven 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 <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 16:27:27.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:28:19.000000000 -0500
@@ -540,7 +540,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 "
@@ -554,6 +558,36 @@
}

/**
+ * 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)
@@ -594,6 +628,8 @@
.capable = seclvl_capable,
.file_permission = seclvl_file_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,

2005-05-17 15:29:02

by Michael Halcrow

Subject: [patch 4/7] BSD Secure Levels: memory alloc failure check

This is the fourth in a series of seven 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 <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 16:28:19.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:28:29.000000000 -0500
@@ -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;
}

2005-05-17 15:30:55

by Michael Halcrow

Subject: [patch 5/7] BSD Secure Levels: allow setuid/setgid on root user processes

This is the fifth in a series of seven 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 <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 16:28:29.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:29:19.000000000 -0500
@@ -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);

2005-05-17 15:33:14

by Michael Halcrow

Subject: [patch 7/7] BSD Secure Levels: remove redundant ptrace check

This is the seventh in a series of seven 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 <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 16:31:36.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:33:01.000000000 -0500
@@ -396,23 +396,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.
*/
@@ -631,7 +614,6 @@
}

static struct security_operations seclvl_ops = {
- .ptrace = seclvl_ptrace,
.capable = seclvl_capable,
.file_permission = seclvl_file_permission,
.inode_setattr = seclvl_inode_setattr,

2005-05-17 15:35:12

by Michael Halcrow

Subject: [patch 6/7] BSD Secure Levels: trivial code and comment changes

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

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-16 16:29:19.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-16 16:31:36.000000000 -0500
@@ -2,12 +2,12 @@
* BSD Secure Levels LSM
*
* Maintainers:
- * Michael A. Halcrow <[email protected]>
- * Serge Hallyn <[email protected]>
+ * Michael A. Halcrow <[email protected]>
+ * Serge Hallyn <[email protected]>
*
* Copyright (c) 2001 WireX Communications, Inc <[email protected]>
* Copyright (c) 2001 Greg Kroah-Hartman <[email protected]>
- * Copyright (c) 2002 International Business Machines <[email protected]>
+ * Copyright (c) 2002 International Business Machines <[email protected]>
*
* 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

@@ -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 "
@@ -67,7 +69,6 @@
* not a good idea to use this parameter when loading seclvl from a
* script; use sha1_passwd instead.
*/
-
#define MAX_PASSWD_SIZE 32
static char passwd[MAX_PASSWD_SIZE];
module_param_string(passwd, passwd, sizeof(passwd), 0);
@@ -93,9 +94,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");

@@ -166,7 +167,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 +186,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 +213,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 +221,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,28 +273,28 @@
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);

-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)
{
- /* So just how good *is* your password? :-) */
char tmp[3];
int i = 0;
buff[0] = '\0';
- if (hideHash) {
- /* Security through obscurity */
+ if (hide_hash) {
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++;
}
@@ -325,8 +326,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__);
@@ -377,7 +378,7 @@
return rc;
}
for (i = 0; i < SHA1_DIGEST_SIZE; i++) {
- if (hashedPassword[i] != tmp[i]) {
+ if (hashed_password[i] != tmp[i]) {
return -EPERM;
}
}
@@ -387,7 +388,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);
@@ -431,7 +434,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);
@@ -612,7 +615,7 @@
}

/**
- * Cannot unmount in secure level 2
+ * Cannot unmount in secure level 2.
*/
static int seclvl_umount(struct vfsmount *mnt, int flags)
{
@@ -640,12 +643,12 @@
};

/**
- * Process the password-related module parameters
+ * 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 "
@@ -654,15 +657,15 @@
"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__);
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)) {
@@ -677,7 +680,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;
}
@@ -686,9 +689,9 @@
}

/**
- * Sysfs registrations
+ * Sysfs registrations.
*/
-static int doSysfsRegistrations(void)
+static int do_sysfs_registrations(void)
{
int rc = 0;
if ((rc = subsystem_register(&seclvl_subsys))) {
@@ -725,7 +728,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);
@@ -745,7 +748,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;
@@ -782,6 +785,6 @@
module_init(seclvl_init);
module_exit(seclvl_exit);

-MODULE_AUTHOR("Michael A. Halcrow <[email protected]>");
+MODULE_AUTHOR("Michael A. Halcrow <[email protected]>");
MODULE_DESCRIPTION("LSM implementation of the BSD Secure Levels");
MODULE_LICENSE("GPL");

2005-05-17 16:17:01

by Christoph Hellwig

Subject: Re: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

On Tue, May 17, 2005 at 10:25:46AM -0500, Michael Halcrow wrote:
> +/**
> + * 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)) {
> + if (bd_claim(bdev, filp)) {
> 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 = filp;
> }
> return 0;

While we're at it this code is crap before and after your patch. There's absolutely
no point at all to use open_by_devnum if you already have an inode or file that you
can get the struct block_device from easily.

2005-05-17 16:49:12

by Al Viro

Subject: Re: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

On Tue, May 17, 2005 at 05:09:00PM +0100, Christoph Hellwig wrote:
> On Tue, May 17, 2005 at 10:25:46AM -0500, Michael Halcrow wrote:
> > +/**
> > + * 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)) {
> > + if (bd_claim(bdev, filp)) {
> > 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 = filp;
> > }
> > return 0;
>
> While we're at it this code is crap before and after your patch. There's absolutely
> no point at all to use open_by_devnum if you already have an inode or file that you
> can get the struct block_device from easily.

It's worse than you think. No, they do *not* necessary have block_device
there. Guess what happens if some clown calls e.g. utime("/dev/sda", NULL)?
That's right, we go checking if we have write permissions on the file in
question. Which happens to be block device node. Which triggers a call
of that junk. At which point we
a) have caused open() on that device node, even though caller did
not ask for that and actually had not planned to do anything with actual
device.
b) have caused all subsequent permission() for MAY_WRITE fail for
that sucker [*] until somebody opens and closes device in question (for
read, obviously).
c) seclvl_bd_release() expects, for some reason, to be called when
task that had called seclvl_bd_claim() to be still alive. Use of current
in setting/checking ->i_security is a bad joke.
d) cargo-cult programming: ->f_dentry and ->f_dentry->d_inode are
*not* NULL, TYVM.

While we are at it... Guys, you do realize that registering an object
and then deciding to bail out of module_init requires unregistering it?

[*] that is, unless they happen to get the same address of local variable
when calling this Fine Piece Of Software - nice misuse of bd_claim() that
should've warned that something is not right here. As it is, just call
utime() several times in row and you've won a cookie - device that had been
opened that many times and will *never* get closed.

2005-05-17 16:57:41

by Al Viro

Subject: Re: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

On Tue, May 17, 2005 at 05:49:22PM +0100, Al Viro wrote:
> a) have caused open() on that device node, even though caller did
> not ask for that and actually had not planned to do anything with actual
> device.
> b) have caused all subsequent permission() for MAY_WRITE fail for
> that sucker [*] until somebody opens and closes device in question (for
> read, obviously).

Actually, that is correct only if that somebody gets the same task_struct
as deceased caller of utime()/whatever had triggered call of permission().
Due to (c) below...

> c) seclvl_bd_release() expects, for some reason, to be called when
> task that had called seclvl_bd_claim() to be still alive. Use of current
> in setting/checking ->i_security is a bad joke.

2005-05-17 17:24:20

by Alexey Dobriyan

Subject: Re: [patch 4/7] BSD Secure Levels: memory alloc failure check

On Tuesday 17 May 2005 19:27, Michael Halcrow wrote:
> It adds a check for a memory allocation failure
> condition.

And leaks tfm if such failure occurs.

> --- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c
> +++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c

> static int
> plaintext_to_sha1(unsigned char *hash, const char *plaintext, int len)
> {

tfm = crypto_alloc_tfm("sha1", 0);
if (tfm == NULL) {
seclvl_printk(0, KERN_ERR, "Failed to load transform for SHA1\n");
return -ENOSYS;
> }
> // 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;
> + }

2005-05-17 17:35:41

by dean gaudet

Subject: Re: [patch 1/7] BSD Secure Levels: printk overhaul

On Tue, 17 May 2005, Michael Halcrow wrote:

> This is the first in a series of seven 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.

it also changes the rate limiting from per message to global... so
one noisy message could shut off other non-noisy messages. was that
intentional?

-dean

> -#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);
> +}

2005-05-17 19:46:28

by Michael Halcrow

Subject: Re: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

Al -

Thank you for your feedback. I believe that most of your concerns are
addressed with this set of patches.

On Tue, May 17, 2005 at 05:49:22PM +0100, Al Viro wrote:
> On Tue, May 17, 2005 at 05:09:00PM +0100, Christoph Hellwig wrote:
> > On Tue, May 17, 2005 at 10:25:46AM -0500, Michael Halcrow wrote:
> > > +/**
> > > + * 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)) {
> > > + if (bd_claim(bdev, filp)) {
> > > 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 = filp;
> > > }
> > > return 0;
> >
> > While we're at it this code is crap before and after your patch.
> > There's absolutely no point at all to use open_by_devnum if you
> > already have an inode or file that you can get the struct
> > block_device from easily.
>
> It's worse than you think. No, they do *not* necessary have
> block_device there. Guess what happens if some clown calls
> e.g. utime("/dev/sda", NULL)? That's right, we go checking if we
> have write permissions on the file in question.

In my tests, utime() does not cause file_permission() to be called.

> Use of current in setting/checking ->i_security is a bad joke.

The patch fixes this:

- /* claimed, mark it to release on close */
- inode->i_security = current;
+ /* Claimed; mark it to release on close */
+ filp->f_security = filp
...

> d) cargo-cult programming: ->f_dentry and ->f_dentry->d_inode are
> *not* NULL, TYVM.

Exactly what code are you refering to here?

> While we are at it... Guys, you do realize that registering an
> object and then deciding to bail out of module_init requires
> unregistering it?

Thanks; this is fixed in the next round of patches.

Mike

2005-05-17 20:12:58

by Al Viro

Subject: Re: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

On Tue, May 17, 2005 at 02:46:17PM -0500, Michael Halcrow wrote:
> In my tests, utime() does not cause file_permission() to be called.

> > Use of current in setting/checking ->i_security is a bad joke.

OK, these applied to the original. Now to the patched version:

a) ->file_permission() is called on every write(2). So you get
an open() for each write(). And only one matching close() (aka. blkdev_put())
b) ->file_permission() is *not* called on mmap() path. So much for
protection, exclusion, etc.
c) you get bd_claim() on each write(); subsequent ones work just
fine, since you use the same holder. On close() you undo one of those.
Leaving the rest there, with holder pointing to freed-and-soon-to-be-reused
struct file.

> > d) cargo-cult programming: ->f_dentry and ->f_dentry->d_inode are
> > *not* NULL, TYVM.
>
> Exactly what code are you refering to here?

seclvl_file_free_security(), but I have to lift that objection in part - this
crap gets called from put_filp(), so ->f_dentry might be NULL. If it is not
NULL, though, we are guaranteed that ->f_dentry->d_inode will *not* be NULL.
BTW, all your checks in that path can be replaced with a single check for
f->f_security being non-NULL. Not that it helped, though - see (a) and (c)
above.

2005-05-19 01:30:02

by Dave Hansen

Subject: Re: [patch 1/7] BSD Secure Levels: printk overhaul

On Tue, 2005-05-17 at 10:23 -0500, Michael Halcrow wrote:
> struct seclvl_attribute {
> struct attribute attr;
> - ssize_t(*show) (struct seclvl_obj *, char *);
> - ssize_t(*store) (struct seclvl_obj *, const char *, size_t);
> + ssize_t(*show) (struct seclvl_obj *, char *);
> + ssize_t(*store) (struct seclvl_obj *, const char *, size_t);
> };

You've changed tabs to spaces.

> /**
> @@ -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);

Instead of changing each and every seclvl_printk() call to add
__FUNCTION__, why not do this:

+static void __seclvl_printk(int verb, const char *fmt, ...)
...

#define seclvl_printk(verb, fmt, arg...) \
__seclvl_printk(verb, __FUNCTION__ ": " fmt, arg)

It requires that the fmt be a string literal, but it saves a lot of code
duplication. I'm sure there are some more examples of this around as
well.

-- Dave

2005-05-19 10:40:35

by Bernd Petrovitsch

Subject: Re: [patch 1/7] BSD Secure Levels: printk overhaul

On Wed, 2005-05-18 at 18:29 -0700, Dave Hansen wrote:
> On Tue, 2005-05-17 at 10:23 -0500, Michael Halcrow wrote:
[...]
> > @@ -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);
>
> Instead of changing each and every seclvl_printk() call to add
> __FUNCTION__, why not do this:
>
> +static void __seclvl_printk(int verb, const char *fmt, ...)
> ...
>
> #define seclvl_printk(verb, fmt, arg...) \
> __seclvl_printk(verb, __FUNCTION__ ": " fmt, arg)
>
> It requires that the fmt be a string literal, but it saves a lot of code
> duplication. I'm sure there are some more examples of this around as

And it duplicates identical format strings in different functions
(besides violating another unwritten rule). What about:
---- snip ----
#define seclvl_printk(verb, fmt, arg...) \
__seclvl_printk((verb), "%s: " fmt, __FUNCTION__, arg)
---- snip ----

Bernd
--
Firmix Software GmbH http://www.firmix.at/
mobil: +43 664 4416156 fax: +43 1 7890849-55
Embedded Linux Development and Services

2005-05-19 20:55:51

by Michael Halcrow

Subject: Re: [updated patch 1/7] BSD Secure Levels: printk overhaul

On Thu, May 19, 2005 at 12:39:45PM +0200, Bernd Petrovitsch wrote:
> On Wed, 2005-05-18 at 18:29 -0700, Dave Hansen wrote:
> > On Tue, 2005-05-17 at 10:23 -0500, Michael Halcrow wrote:
> [...]
> > > @@ -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);
> >
> > Instead of changing each and every seclvl_printk() call to add
> > __FUNCTION__, why not do this:
> >
> > +static void __seclvl_printk(int verb, const char *fmt, ...)
> > ...
> >
> > #define seclvl_printk(verb, fmt, arg...) \
> > __seclvl_printk(verb, __FUNCTION__ ": " fmt, arg)
> >
> > It requires that the fmt be a string literal, but it saves a lot
> > of code
> > duplication. I'm sure there are some more examples of this around
> > as
>
> And it duplicates identical format strings in different functions
> (besides violating another unwritten rule). What about:
> ---- snip ----
> #define seclvl_printk(verb, fmt, arg...) \
> __seclvl_printk((verb), "%s: " fmt, __FUNCTION__, arg)
> ---- snip ----

More than one person has mentioned to me that global rate limiting
does not make as much sense as per message rate limiting. The
original macro ate up a bit of text area. The non-patched module
occupies 22701 bytes in my build. The recent patch, which rate-limits
globally, results in a module size of 18443. This patch, which
rate-limits on a per message basis, results in a module size of
20733.

Signed-off by: Michael Halcrow <[email protected]

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-19 15:44:25.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-19 15:49:02.000000000 -0500
@@ -102,21 +102,32 @@
#define MY_NAME "seclvl"

/**
- * This time-limits log writes to one per second.
+ * This time-limits log writes to one per second for every message
+ * type.
*/
-#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(unsigned long *_prior, int verb, const char *fmt, ...)
+{
+ va_list args;
+ va_start(args, fmt);
+ unsigned long _now = jiffies;
+ if (time_after(_now, (*_prior) + HZ)) {
+ vprintk(fmt, args);
+ }
+ *_prior = _now;
+ va_end(args);
+}
+
+/**
+ * Breaking the printk up into a macro and a function saves some text
+ * space.
+ */
+#define seclvl_printk(verb, type, fmt, arg...) \
+ if (verbosity >= verb ) { \
+ static unsigned long _prior; \
+ __seclvl_printk(&_prior, (verb), type "%s: " fmt, \
+ __FUNCTION__, ## arg); \
+ }

/**
* kobject stuff
@@ -711,7 +722,7 @@
goto exit;
}
seclvl_printk(0, KERN_INFO, "seclvl: Successfully initialized.\n");
- exit:
+ exit:
if (rc) {
printk(KERN_ERR "seclvl: Error during initialization: rc = "
"[%d]\n", rc);

2005-05-19 21:41:45

by Michael Halcrow

Subject: Re: [updated patch 1/7] BSD Secure Levels: printk overhaul

On Thu, May 19, 2005 at 01:58:06PM -0700, Andrew Morton wrote:
> Did anyone mention printk_ratelimit()?

Third time's a charm. :-)

I think this makes the most sense. Module size is 18284; messages are
globally limited, but the space savings is significant.

Signed-off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-19 15:49:51.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-19 16:33:20.000000000 -0500
@@ -102,21 +102,25 @@
#define MY_NAME "seclvl"

/**
- * This time-limits log writes to one per second.
+ * This time-limits log writes to one per second for every message
+ * type.
*/
-#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 && printk_ratelimit()) {
+ vprintk(fmt, args);
+ }
+ va_end(args);
+}
+
+/**
+ * Breaking the printk up into a macro and a function saves some text
+ * space.
+ */
+#define seclvl_printk(verb, type, fmt, arg...) \
+ __seclvl_printk((verb), type "%s: " fmt, __FUNCTION__, ## arg);

/**
* kobject stuff
@@ -711,7 +715,7 @@
goto exit;
}
seclvl_printk(0, KERN_INFO, "seclvl: Successfully initialized.\n");
- exit:
+ exit:
if (rc) {
printk(KERN_ERR "seclvl: Error during initialization: rc = "
"[%d]\n", rc);

2005-05-20 05:20:25

by Nguyen Anh Quynh

Subject: Re: [updated patch 1/7] BSD Secure Levels: printk overhaul

On 5/20/05, Michael Halcrow <[email protected]> wrote:
> On Thu, May 19, 2005 at 01:58:06PM -0700, Andrew Morton wrote:
> > Did anyone mention printk_ratelimit()?
>
> Third time's a charm. :-)
>
> I think this makes the most sense. Module size is 18284; messages are
> globally limited, but the space savings is significant.
>
> Signed-off by: Michael Halcrow <[email protected]>
>
> Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
> ===================================================================
> --- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-19 15:49:51.000000000 -0500
> +++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-19 16:33:20.000000000 -0500
> @@ -102,21 +102,25 @@
> #define MY_NAME "seclvl"
>
> /**
> - * This time-limits log writes to one per second.
> + * This time-limits log writes to one per second for every message
> + * type.
> */
> -#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 && printk_ratelimit()) {
> + vprintk(fmt, args);
> + }
> + va_end(args);
> +}
> +
> +/**
> + * Breaking the printk up into a macro and a function saves some text
> + * space.
> + */
> +#define seclvl_printk(verb, type, fmt, arg...) \
> + __seclvl_printk((verb), type "%s: " fmt, __FUNCTION__, ## arg);
>
> /**
> * kobject stuff
> @@ -711,7 +715,7 @@
> goto exit;
> }
> seclvl_printk(0, KERN_INFO, "seclvl: Successfully initialized.\n");
> - exit:
> + exit:
> if (rc) {
> printk(KERN_ERR "seclvl: Error during initialization: rc = "
> "[%d]\n", rc);
> -
> 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/
>

i disklike the fact that the rate limit is based on 1 sec. how about
finer-grain limit?

it is best to let user to config the litmit. 1 sec is too raw to some purpose.

regards,
aq

2005-05-20 15:04:24

by Michael Halcrow

Subject: [updated patch 2/7] BSD Secure Levels: bd_claim fixes

Christoph Hellwig wrote:
> There's absolutely no point at all to use open_by_devnum if you
> already have an inode or file that you can get the struct
> block_device from easily.

Al Viro wrote:
> OK, these applied to the original. Now to the patched version:
>
> a) ->file_permission() is called on every write(2). So you get an
> open() for each write(). And only one matching close()
> (aka. blkdev_put())
>
> b) ->file_permission() is *not* called on mmap() path. So much for
> protection, exclusion, etc.
>
> c) you get bd_claim() on each write(); subsequent ones work just
> fine, since you use the same holder. On close() you undo one of
> those. Leaving the rest there, with holder pointing to
> freed-and-soon-to-be-reused struct file.

This patch makes another attempt at enforcing the the ``no write to
mounted block device'' BSD Secure Levels rule, but as you can see,
it's not perfect. Namely, if you were to open and mmap a block device
for writing, and then if someone were to mount that block device, the
application with the mmap could still write to the file. Given the
current placement of the hooks, it is not obvious to us how this might
best be avoided (i.e., with minimum breakage for user apps).

Christoph/Al: Please look this over and let us know if you think this
patch correctly addresses all the pertinent issues.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-19 16:33:20.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:56.000000000 -0500
@@ -27,6 +27,7 @@
#include <linux/mount.h>
#include <linux/capability.h>
#include <linux/time.h>
+#include <linux/mman.h>
#include <linux/proc_fs.h>
#include <linux/kobject.h>
#include <linux/crypto.h>
@@ -489,35 +490,48 @@
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;
- bdev = open_by_devnum(dev, FMODE_WRITE);
- if (bdev) {
- if (bd_claim(bdev, &holder)) {
- blkdev_put(bdev);
+ if (filp->f_dentry->d_inode->i_bdev) {
+ if (bd_claim(filp->f_dentry->d_inode->i_bdev, filp)) {
return -EPERM;
}
- /* claimed, mark it to release on close */
- inode->i_security = current;
+ /* Claimed; mark it to release on close */
+ filp->f_security = filp;
}
return 0;
}

-/* release the blockdev if you claimed it */
-static void seclvl_bd_release(struct inode *inode)
+/**
+ * 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_file_permission(struct file *filp, int mask)
{
- 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;
+ if (filp->f_security != NULL || filp->f_dentry == NULL)
+ return 0;
+ 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, "Write to block device "
+ "denied in secure level [%d]\n", seclvl);
+ return -EPERM;
+ case 1:
+ if (seclvl_bd_claim(filp)) {
+ seclvl_printk(1, KERN_WARNING, "Write to "
+ "mounted block device denied in "
+ "secure level [%d]\n",
+ seclvl);
+ return -EPERM;
+ }
}
}
+ return 0;
}

/**
@@ -525,22 +539,37 @@
* 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_mmap(struct file *filp, unsigned long reqprot,
+ unsigned long prot, unsigned long flags)
{
- if (current->pid != 1 && S_ISBLK(inode->i_mode) && (mask & MAY_WRITE)) {
+ struct block_device *bdev = NULL;
+ /* This may be called for anonymous memory regions */
+ if (filp == NULL || filp->f_security != NULL
+ || filp->f_dentry == NULL) {
+ return 0;
+ }
+ if (current->pid != 1 && S_ISBLK(filp->f_dentry->d_inode->i_mode)
+ && (prot & PROT_WRITE)) {
switch (seclvl) {
case 2:
seclvl_printk(1, KERN_WARNING, "Write to block device "
"denied in secure level [%d]\n", 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);
- return -EPERM;
+ bdev = filp->f_dentry->d_inode->i_bdev;
+ if (bdev) {
+ if (bd_claim(bdev, filp)) {
+ /* Mounted block device */
+ seclvl_printk(1, KERN_WARNING,
+ "Write to mounted block "
+ "device denied in secure"
+ " level [%d]\n", seclvl);
+ return -EPERM;
+ } else {
+ /* No reason to hold the claim */
+ bd_release(bdev);
+ filp->f_security = filp;
+ }
}
}
}
@@ -566,15 +595,16 @@
return 0;
}

-/* release busied block devices */
+/**
+ * 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);
+ if (filp->f_security != NULL) {
+ struct block_device *bdev = filp->f_dentry->d_inode->i_bdev;
+ if (bdev)
+ bd_release(bdev);
+ filp->f_security = NULL;
}
}

@@ -597,7 +627,8 @@
static struct security_operations seclvl_ops = {
.ptrace = seclvl_ptrace,
.capable = seclvl_capable,
- .inode_permission = seclvl_inode_permission,
+ .file_permission = seclvl_file_permission,
+ .file_mmap = seclvl_file_mmap,
.inode_setattr = seclvl_inode_setattr,
.file_free_security = seclvl_file_free_security,
.settime = seclvl_settime,

2005-05-20 15:07:39

by Michael Halcrow

Subject: [updated patch 3/7] BSD Secure Levels: allow suid and sgid on directories

This patch is applies cleanly against the new printk() patch. It
allows setuid and setgid on directories. It also disallows the
creation of setuid/setgid executables via open or mknod.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-19 17:46:13.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:03.000000000 -0500
@@ -582,7 +582,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, "Attempt to "
@@ -596,6 +600,34 @@
}

/**
+ * 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, "Attempt to mknod with suid "
+ "or guid bit set in seclvl [%d]\n", 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, "Attempt to create inode with "
+ "suid or guid bit set in seclvl [%d]\n", seclvl);
+ return -EPERM;
+ }
+ return 0;
+}
+
+/**
* Release busied block devices.
*/
static void seclvl_file_free_security(struct file *filp)
@@ -630,6 +662,8 @@
.file_permission = seclvl_file_permission,
.file_mmap = seclvl_file_mmap,
.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,

2005-05-20 15:09:32

by Michael Halcrow

Subject: [updated patch 4/7] BSD Secure Levels: memory alloc failure check

This patch is applies cleanly against the new printk() patch. It adds
a check for a memory allocation failure condition.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-20 09:09:03.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:07.000000000 -0500
@@ -317,7 +317,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) {
@@ -334,16 +334,21 @@
}
// 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, "Out of memory\n" );
+ crypto_free_tfm(tfm);
+ 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;
}

2005-05-20 15:11:18

by Michael Halcrow

Subject: [updated patch 5/7] BSD Secure Levels: allow setuid/setgid on root user processes

This patch is applies cleanly against the new printk() patch. It
allows setuid and setgid on a process if the user is already root.
This allows non-root users to log in.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-20 09:09:07.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:13.000000000 -0500
@@ -447,12 +447,12 @@
"network administrative task while "
"in secure level [%d] denied\n", seclvl);
return -EPERM;
- } else if (cap == CAP_SETUID) {
+ } else if (cap == CAP_SETUID && current->uid != 0) {
seclvl_printk(1, KERN_WARNING, "Attempt to setuid "
"while in secure level [%d] denied\n",
seclvl);
return -EPERM;
- } else if (cap == CAP_SETGID) {
+ } else if (cap == CAP_SETGID && current->uid != 0) {
seclvl_printk(1, KERN_WARNING, "Attempt to setgid "
"while in secure level [%d] denied\n",
seclvl);

2005-05-20 15:15:16

by Michael Halcrow

Subject: [updated patch 6/7] BSD Secure Levels: trivial code and comment changes

This patch is applies cleanly against the new printk() patch. It
makes several trivial changes to make the code and comments
consistent. (Note that Lindent causes a whitespace issue w/ the
struct seclvl_attribute {} code.)

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-20 09:09:13.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:37.000000000 -0500
@@ -2,12 +2,12 @@
* BSD Secure Levels LSM
*
* Maintainers:
- * Michael A. Halcrow <[email protected]>
- * Serge Hallyn <[email protected]>
+ * Michael A. Halcrow <[email protected]>
+ * Serge Hallyn <[email protected]>
*
* Copyright (c) 2001 WireX Communications, Inc <[email protected]>
* Copyright (c) 2001 Greg Kroah-Hartman <[email protected]>
- * Copyright (c) 2002 International Business Machines <[email protected]>
+ * Copyright (c) 2002 International Business Machines <[email protected]>
*
* 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
@@ -31,9 +31,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

@@ -53,7 +53,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 "
@@ -68,7 +70,6 @@
* not a good idea to use this parameter when loading seclvl from a
* script; use sha1_passwd instead.
*/
-
#define MAX_PASSWD_SIZE 32
static char passwd[MAX_PASSWD_SIZE];
module_param_string(passwd, passwd, sizeof(passwd), 0);
@@ -94,17 +95,16 @@
"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");

#define MY_NAME "seclvl"

/**
- * This time-limits log writes to one per second for every message
- * type.
+ * This time-limits log writes.
*/
static void __seclvl_printk(int verb, const char *fmt, ...)
{
@@ -126,7 +126,6 @@
/**
* kobject stuff
*/
-
struct subsystem seclvl_subsys;

struct seclvl_obj {
@@ -173,7 +172,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,
@@ -192,7 +191,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;

@@ -219,7 +218,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)
{
@@ -227,7 +226,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.
@@ -279,28 +278,28 @@
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);

-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)
{
- /* So just how good *is* your password? :-) */
char tmp[3];
int i = 0;
buff[0] = '\0';
- if (hideHash) {
- /* Security through obscurity */
+ if (hide_hash) {
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++;
}
@@ -332,8 +331,8 @@
"Failed to load transform for SHA1\n");
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, "Out of memory\n" );
@@ -385,7 +384,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 +394,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);
@@ -437,7 +438,7 @@
"and/or APPEND extended attribute set "
"denied in seclvl [%d]\n", seclvl);
return -EPERM;
- } else if (cap == CAP_SYS_RAWIO) { // Somewhat broad...
+ } else if (cap == CAP_SYS_RAWIO) { /* Somewhat broad */
seclvl_printk(1, KERN_WARNING, "Attempt to perform "
"raw I/O while in secure level [%d] "
"denied\n", seclvl);
@@ -646,7 +647,7 @@
}

/**
- * Cannot unmount in secure level 2
+ * Cannot unmount in secure level 2.
*/
static int seclvl_umount(struct vfsmount *mnt, int flags)
{
@@ -675,12 +676,12 @@
};

/**
- * Process the password-related module parameters
+ * 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, "Error: Both "
@@ -689,13 +690,13 @@
"exclusive.\n");
return -EINVAL;
}
- if ((rc = plaintext_to_sha1(hashedPassword, passwd,
+ if ((rc = plaintext_to_sha1(hashed_password, passwd,
strlen(passwd)))) {
seclvl_printk(0, KERN_ERR, "Error: SHA1 support not "
"in kernel\n");
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
int i;
@@ -712,7 +713,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;
}
@@ -721,9 +722,9 @@
}

/**
- * Sysfs registrations
+ * Sysfs registrations.
*/
-static int doSysfsRegistrations(void)
+static int do_sysfs_registrations(void)
{
int rc = 0;
if ((rc = subsystem_register(&seclvl_subsys))) {
@@ -760,7 +761,7 @@
goto exit;
}
seclvl = initlvl;
- if ((rc = processPassword())) {
+ if ((rc = process_password())) {
seclvl_printk(0, KERN_ERR, "Error processing the password "
"module parameter(s): rc = [%d]\n", rc);
goto exit;
@@ -777,10 +778,10 @@
"registering with primary security "
"module.\n");
goto exit;
- } /* if primary module registered */
+ } /* if primary module registered */
secondary = 1;
- } /* if we registered ourselves with the security framework */
- if ((rc = doSysfsRegistrations())) {
+ } /* if we registered ourselves with the security framework */
+ if ((rc = do_sysfs_registrations())) {
seclvl_printk(0, KERN_ERR, "Error registering with sysfs\n");
goto exit;
}
@@ -816,6 +817,6 @@
module_init(seclvl_init);
module_exit(seclvl_exit);

-MODULE_AUTHOR("Michael A. Halcrow <[email protected]>");
+MODULE_AUTHOR("Michael A. Halcrow <[email protected]>");
MODULE_DESCRIPTION("LSM implementation of the BSD Secure Levels");
MODULE_LICENSE("GPL");

2005-05-20 15:15:55

by Michael Halcrow

Subject: [updated patch 7/7] BSD Secure Levels: remove redundant ptrace check

This patch is applies cleanly against the new printk() patch. It
removes the ptrace check because it is redundant with the check made
in kernel/ptrace.c.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-20 09:09:37.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:42.000000000 -0500
@@ -402,22 +402,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, "Attempt to ptrace "
- "the init process dissallowed in "
- "secure level %d\n", seclvl);
- return -EPERM;
- }
- }
- return 0;
-}
-
-/**
* Capability checks for seclvl. The majority of the policy
* enforcement for seclvl takes place here.
*/
@@ -663,7 +647,6 @@
}

static struct security_operations seclvl_ops = {
- .ptrace = seclvl_ptrace,
.capable = seclvl_capable,
.file_permission = seclvl_file_permission,
.file_mmap = seclvl_file_mmap,

2005-05-20 15:23:20

by Michael Halcrow

Subject: [patch 8/7] BSD Secure Levels: unregister on sysfs failure

As a feeble ode to Douglas Adams, this is the 8th in a series of 7
patches. It fixes the issue where the security ops were not being
released on sysfs registration failure.

Signed off by: Michael Halcrow <[email protected]>

Index: linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c
===================================================================
--- linux-2.6.12-rc4-mm2-seclvl.orig/security/seclvl.c 2005-05-20 09:09:42.000000000 -0500
+++ linux-2.6.12-rc4-mm2-seclvl/security/seclvl.c 2005-05-20 09:09:46.000000000 -0500
@@ -766,6 +766,7 @@
} /* if we registered ourselves with the security framework */
if ((rc = do_sysfs_registrations())) {
seclvl_printk(0, KERN_ERR, "Error registering with sysfs\n");
+ unregister_security(&seclvl_ops);
goto exit;
}
seclvl_printk(0, KERN_INFO, "seclvl: Successfully initialized.\n");