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

2007-08-30 Thread Casey Schaufler

--- Jan Engelhardt <[EMAIL PROTECTED]> wrote:

> 
> On Aug 27 2007 23:59, Thomas Bleher wrote:
> 
> >> +   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.
> 
> Or even pr_debug.
> 
> (pr_debug("%s...

I'm taking out the printk.

> >> +  static struct tree_descr smack_files[] = {
> 
> Hey, can we make this const, if it does not change at runtime?

simple_fill_super() is unhappy with a const.
 

Casey Schaufler
[EMAIL PROTECTED]
-
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

2007-08-30 Thread Jan Engelhardt

On Aug 27 2007 23:59, Thomas Bleher wrote:

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

Or even pr_debug.

(pr_debug("%s...

>> +static struct tree_descr smack_files[] = {

Hey, can we make this const, if it does not change at runtime?


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

2007-08-30 Thread Jan Engelhardt

On Aug 27 2007 23:59, Thomas Bleher wrote:

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

Or even pr_debug.

(pr_debug(%s...

 +static struct tree_descr smack_files[] = {

Hey, can we make this const, if it does not change at runtime?


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

2007-08-30 Thread Casey Schaufler

--- Jan Engelhardt [EMAIL PROTECTED] wrote:

 
 On Aug 27 2007 23:59, Thomas Bleher wrote:
 
  +   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.
 
 Or even pr_debug.
 
 (pr_debug(%s...

I'm taking out the printk.

  +  static struct tree_descr smack_files[] = {
 
 Hey, can we make this const, if it does not change at runtime?

simple_fill_super() is unhappy with a const.
 

Casey Schaufler
[EMAIL PROTECTED]
-
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

2007-08-28 Thread Casey Schaufler

--- Thomas Bleher <[EMAIL PROTECTED]> wrote:

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

Thank you.

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

That is correct. This is consistant with the traditional and current
linux mainstream model.

> If so, this is a serious limitation that should be mentioned in the
> docs.

The superuser has been an issue since the first Security Expert
looked at the Unix kernel in the 1970's. I am not out to solve
that issue as Linux has the capability mechanism in place to do
that, and active development (as you point out) is in progress
to make that mechanism useful.

> File capabilities should alleviate this problem, but they first
> need to be configured correctly...

They need to get upstream, too.

Yes, program capability setting is an issue. I have done it before
on a Unix system and while it is a pain in the bum it's not actually
that hard when you have an audit trail and the source. It's the same
technic that the SELinux folks use with audit2allow, but directed at
a smaller set of choices.

> Some other comments below:
> 
> > --- linux-2.6.22-base/security/smack/Kconfig1969-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/?

An idea with merit. I'll do it.

> BTW: Some documentation missing there:
> * the various mount options
> * how to enable cipso

I'll add those

> (Do I have to enable it explicitly? Most people
>   won't even know what cipso is)

Nope. CIPSO is native to Smack. The touchy bit is turning it off
when talking off the box. So far it looks like most of the world
is sufficiently ignorant of CIPSO that it gets ignored enough for
communication to occur at the ambient label.

> > +/**
> > + * 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?

Indeed I can. In the end the syslog data is going to get put into
a file and may get displayed on consoles. You can argue that the
right thing to do is make the Smack label on those files and devices
hat ("^") and allow anyone (Smackwise) write to the syslog. That
disallows casual viewing of the syslog, and everyone would hate that.

It would make sense to allow this if the caller has appropriate
privilege. I think that I will make that change.
 
> > +/**
> > + * smackfs_follow_link - follow a smackfs symlink
>   ^^^ should be put_link

Whoops.

> > + * @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 

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

2007-08-28 Thread Casey Schaufler

--- Thomas Bleher [EMAIL PROTECTED] wrote:

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

Thank you.

 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?

That is correct. This is consistant with the traditional and current
linux mainstream model.

 If so, this is a serious limitation that should be mentioned in the
 docs.

The superuser has been an issue since the first Security Expert
looked at the Unix kernel in the 1970's. I am not out to solve
that issue as Linux has the capability mechanism in place to do
that, and active development (as you point out) is in progress
to make that mechanism useful.

 File capabilities should alleviate this problem, but they first
 need to be configured correctly...

They need to get upstream, too.

Yes, program capability setting is an issue. I have done it before
on a Unix system and while it is a pain in the bum it's not actually
that hard when you have an audit trail and the source. It's the same
technic that the SELinux folks use with audit2allow, but directed at
a smaller set of choices.

 Some other comments below:
 
  --- linux-2.6.22-base/security/smack/Kconfig1969-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/?

An idea with merit. I'll do it.

 BTW: Some documentation missing there:
 * the various mount options
 * how to enable cipso

I'll add those

 (Do I have to enable it explicitly? Most people
   won't even know what cipso is)

Nope. CIPSO is native to Smack. The touchy bit is turning it off
when talking off the box. So far it looks like most of the world
is sufficiently ignorant of CIPSO that it gets ignored enough for
communication to occur at the ambient label.

  +/**
  + * 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?

Indeed I can. In the end the syslog data is going to get put into
a file and may get displayed on consoles. You can argue that the
right thing to do is make the Smack label on those files and devices
hat (^) and allow anyone (Smackwise) write to the syslog. That
disallows casual viewing of the syslog, and everyone would hate that.

It would make sense to allow this if the caller has appropriate
privilege. I think that I will make that change.
 
  +/**
  + * smackfs_follow_link - follow a smackfs symlink
   ^^^ should be put_link

Whoops.

  + * @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, 

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

2007-08-27 Thread Thomas Bleher
* 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

2007-08-27 Thread Thomas Bleher
* 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