Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel
* Christoph Hellwig <[EMAIL PROTECTED]> [2007-10-02 10:14]: > On Sun, Sep 30, 2007 at 01:16:18AM -0700, Andrew Morton wrote: > > reviewed the August thread from your version 1 submission and the message I > > take away is that the code has been well-received and looks good when > > considered on its own merits, but selinux could probably be configured to > > do something sufficiently similar. > > > > I'd have trouble declaring that "but" to be a reason to not merge smack. > > I'm more thinking "let's merge it and see if people use it". > > I'm not sure this was discussed on the list, but as long as Casey doesn't > get rid of the magic symlinks (smackfs_follow_link), there's a clear NACK > from the VFS perspective. Any rationale for this NACK? Caseys patch doesn't add something crazy like "make symlinks depend on environment variables"; also, we already have something similar in-tree: /proc/self. Thomas - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel
* Christoph Hellwig [EMAIL PROTECTED] [2007-10-02 10:14]: On Sun, Sep 30, 2007 at 01:16:18AM -0700, Andrew Morton wrote: reviewed the August thread from your version 1 submission and the message I take away is that the code has been well-received and looks good when considered on its own merits, but selinux could probably be configured to do something sufficiently similar. I'd have trouble declaring that but to be a reason to not merge smack. I'm more thinking let's merge it and see if people use it. I'm not sure this was discussed on the list, but as long as Casey doesn't get rid of the magic symlinks (smackfs_follow_link), there's a clear NACK from the VFS perspective. Any rationale for this NACK? Caseys patch doesn't add something crazy like make symlinks depend on environment variables; also, we already have something similar in-tree: /proc/self. Thomas - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Version2 Smack: Simplified Mandatory Access Control Kernel
* Casey Schaufler <[EMAIL PROTECTED]> [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.0
> -0800
> +++ linux-2.6.22-smack/security/smack/Kconfig 2007-08-22 02:18:55.0
> -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", _load_ops, S_IRUGO|S_IWUSR},
> + [SMK_LINKS] = {"links", _links_ops, S_IRUGO|S_IWUSR},
I really don't understand your symlink code (not
Re: [PATCH] Version2 Smack: Simplified Mandatory Access Control Kernel
* Casey Schaufler [EMAIL PROTECTED] [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.0
-0800
+++ linux-2.6.22-smack/security/smack/Kconfig 2007-08-22 02:18:55.0
-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
