Re: [PATCH] Version2 Smack: Simplified Mandatory Access Control Kernel

Thomas Bleher
2007-08-27T22:00:46Z

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

* Casey Schaufler <casey@schaufler-ca.com> [2007-08-27 22:51]:
>
> Smack is the Simplified Mandatory Access Control Kernel.
>
> Smack implements mandatory access control (MAC) using labels
> attached to tasks and data containers, including files, SVIPC,
> and other tasks. Smack is a kernel based scheme that requires
> an absolute minimum of application support and a very small
> amount of configuration data.
>
> Smack is implemented as a clean LSM. It requires no external
> code changes and the patch modifies only the Kconfig and Makefile
> in the security directory. Smack uses extended attributes and
> provides a set of general mount options, borrowing technics used
> elsewhere. Smack uses netlabel for CIPSO labeling. Smack provides
> a pseudo-filesystem smackfs that is used for manipulation of
> system Smack attributes.
>
> The patch, patches for ls and sshd, a README, a startup script,
> and x86 binaries for ls and sshd are also available on
>
> http:/www.schaufler-ca.com

I like the general idea of this LSM.
One question about your security model, though:
If I understand your LSM correctly, MAC override is based on
capabilities, specifically on CAP_LINUX_IMMUTABLE. So any root process
which doesn't explicitly give up this capability is effectively
unconfined, right?

If so, this is a serious limitation that should be mentioned in the
docs. File capabilities should alleviate this problem, but they first
need to be configured correctly...

Some other comments below:

> --- linux-2.6.22-base/security/smack/Kconfig 1969-12-31 16:00:00.000000000 -0800
> +++ linux-2.6.22-smack/security/smack/Kconfig 2007-08-22 02:18:55.000000000 -0700
> @@ -0,0 +1,10 @@
> +config SECURITY_SMACK
> + bool "Simplified Mandatory Access Control Kernel Support"
> + depends on NETLABEL && SECURITY_NETWORK
> + default n
> + help
> + This selects the Simplified Mandatory Access Control Kernel.
> + Smack is useful for sensitivity, integrity, and a variety
> + of other mandatory security schemes.
> + If you are unsure how to answer this question, answer N.

I think a link to further documentation would be nice.
Maybe you could include the README from your site in Documentation/?

BTW: Some documentation missing there:
* the various mount options
* how to enable cipso (Do I have to enable it explicitly? Most people
won't even know what cipso is)

> +/**
> + * smack_syslog - Smack approval on syslog
> + * @type: message type
> + *
> + * Require that the task has the floor label
> + *
> + * Returns 0 on success, error code otherwise.
> + */
> +static int smack_syslog(int type)
> +{
> + int rc;
> + smack_t *sp = current->security;
> +
> + rc = cap_syslog(type);
> + if (rc == 0)
> + if (*sp != SMK_FLOOR)
> + rc = -EACCES;
> +
> + return rc;
> +}

Can you explain why you limit syslog that way?

> +/**
> + * smackfs_follow_link - follow a smackfs symlink
^^^^^^^^^^^ should be put_link
> + * @dentry: unused
> + * @nd: name entry
> + * @ptr: unused
> + *
> + * free the buffer used in following the link.
> + */
> +static void smackfs_put_link(struct dentry *dentry, struct nameidata *nd, void *ptr)
> +{
> + kfree(nd_get_link(nd));
> +}

> + for (cp = data - 1; cp != NULL; cp = strchr(cp + 1, '\n')) {
> + if (*++cp == '\0')
> + break;
> + if (sscanf(cp, "%14s %30s\n", name, target) != 2) {
> + printk("%s:%d bad scan\n",
> + __func__, __LINE__);

Leftover debugging printk? Otherwise, a level would be nice.

> + break;
> + }
> + smk_add_symlink(name, target);
> + printk("%s:%d add %s -> %s\n",
> + __func__, __LINE__, name, target);

Ditto.

> + }

> +/**
> + * smackfs_follow_link - follow a smackfs symlink
> + * @dentry: name cache entry
> + * @nd: name entry
> + *
> + * A symlink on smackfs has unusual semantics.
> + *
> + * The Smack value of the task is appended to the link string.
> + * Thus, if a task labeled "Gentoo" does chdir("/smack/tmp")
> + * it will use "/moldy/Gentoo".
> + *
> + * The expected usage is the /tmp is a symlink to /smack/tmp
> + * which is itself a symlink to /moldy. /moldy should have a
> + * directory for each label in use to accomodate the value
> + * appended on the redirection.
> + *
> + * An interesting addition would be a file system that automatically
> + * creates directories as needed, at the appropriate label.
> + */

I'm not very versed in the VFS, so I don't think I understand the code
fully there, but shouldn't the above read /smack/links?

> + static struct tree_descr smack_files[] = {
> + [SMK_LOAD] = {"load", &smk_load_ops, S_IRUGO|S_IWUSR},
> + [SMK_LINKS] = {"links", &smk_links_ops, S_IRUGO|S_IWUSR},

I really don't understand your symlink code (not because it's bad, I just
don't know this area), so this is just a question:
Wouldn't S_IRUGO be enough for the links entry?

> + [SMK_CIPSO] = {"cipso", &smk_cipso_ops, S_IRUGO|S_IWUSR},
> + [SMK_DOI] = {"doi", &smk_doi_ops, S_IRUGO|S_IWUSR},
> + [SMK_DIRECT] = {"direct", &smk_direct_ops, S_IRUGO|S_IWUSR},
> + [SMK_AMBIENT] = {"ambient", &smk_ambient_ops,S_IRUGO|S_IWUSR},
> + [SMK_NLTYPE] = {"nltype", &smk_nltype_ops, S_IRUGO|S_IWUSR},
> + /* last one */ {""}
> + };
> +
> + /*
> + * There will be only one smackfs. Casey says so.
> + */
> + smk_sb = sb;
> +
> + rc = simple_fill_super(sb, SMACK_MAGIC, smack_files);
> + if (rc != 0) {
> + printk(KERN_ERR "%s failed %d while creating inodes\n",
> + __func__, rc);
> + return rc;
> + }
> +
> + root_inode = sb->s_root->d_inode;
> + root_inode->i_security = new_inode_smack(&smack_known_floor.smk_known);
> +
> + /*
> + * Create a directory for /smack/tmp
> + */
> + smk_add_symlink("tmp", SMK_TMPPATH_ROOT);

Ah, so you dynamically add symlinks here. Can the user do this, too? If
so, how? A little documentation might be nice.

Thomas

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