All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] SELINUX: new permission controlling the ability to set suid
@ 2010-04-22 20:46 Eric Paris
  2010-04-22 21:35 ` Stephen Smalley
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Paris @ 2010-04-22 20:46 UTC (permalink / raw)
  To: selinux; +Cc: jmorris, sds

This patch adds a new permission, setsuid, to the file class.  This
permission is checked any time a file has its attributes modified
(setattr) and the setuid or setgid bits are involved.  The purpose for
this change is to further allow selinux to confine administrative
duties.

This permission is needed to both set the setuid bit and to add more
permissions to a file which already has setuid bit.  This is also checked
when an acl is modified on a file containing the suid or sgid bit.  We
do not know if the acl is more or less restrictive, so we always check
the permission on acl changes.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 security/selinux/hooks.c            |   38 ++++++++++++++++++++++++++++++-----
 security/selinux/include/classmap.h |    2 +-
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 26b0a8a..bab6061 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -76,6 +76,7 @@
 #include <linux/selinux.h>
 #include <linux/mutex.h>
 #include <linux/posix-timers.h>
+#include <linux/posix_acl_xattr.h>
 #include <linux/syslog.h>
 
 #include "avc.h"
@@ -2728,6 +2729,9 @@ static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
 {
 	const struct cred *cred = current_cred();
 	unsigned int ia_valid = iattr->ia_valid;
+	unsigned int mode = dentry->d_inode->i_mode;
+	unsigned int ia_mode = iattr->ia_mode;
+	u32 av = 0;
 
 	/* ATTR_FORCE is just used for ATTR_KILL_S[UG]ID. */
 	if (ia_valid & ATTR_FORCE) {
@@ -2737,11 +2741,28 @@ static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
 			return 0;
 	}
 
-	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
-			ATTR_ATIME_SET | ATTR_MTIME_SET | ATTR_TIMES_SET))
-		return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR);
+	/*
+	 * are we changing mode?
+	 * is this a regular file?
+	 * do resulting iattr->i_mode have the suid/gid bits set?
+	 */
+	if ((iattr->ia_valid & ATTR_MODE) &&
+	    (S_ISREG(mode)) &&
+	    (ia_mode & (S_ISUID | S_ISGID))) {
+		/* calculate what bits are being added (if any) */
+		unsigned int new_bits = ((mode ^ ia_mode) & ia_mode);
+		if (new_bits)
+			av |= FILE__SETSUID;
+	}
+
+	if (iattr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
+			       ATTR_ATIME_SET | ATTR_MTIME_SET))
+		av |= FILE__SETATTR;
+	else
+		av |= FILE__WRITE;
+
+	return dentry_has_perm(cred, NULL, dentry, av);
 
-	return dentry_has_perm(cred, NULL, dentry, FILE__WRITE);
 }
 
 static int selinux_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
@@ -2754,6 +2775,7 @@ static int selinux_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
 static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name)
 {
 	const struct cred *cred = current_cred();
+	u32 av = FILE__SETATTR;
 
 	if (!strncmp(name, XATTR_SECURITY_PREFIX,
 		     sizeof XATTR_SECURITY_PREFIX - 1)) {
@@ -2765,11 +2787,17 @@ static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name)
 			   Restrict to administrator. */
 			return -EPERM;
 		}
+	} else if ((dentry->d_inode->i_mode & (S_ISUID | S_ISGID)) &&
+		   (!strncmp(name, POSIX_ACL_XATTR_ACCESS,
+			     sizeof(POSIX_ACL_XATTR_ACCESS) - 1))) {
+			/* changing access acl on a file with setuid/segid
+			 * requires FILE_SETSUID */
+			av |= FILE__SETSUID;
 	}
 
 	/* Not an attribute we recognize, so just check the
 	   ordinary setattr permission. */
-	return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR);
+	return dentry_has_perm(cred, NULL, dentry, av);
 }
 
 static int selinux_inode_setxattr(struct dentry *dentry, const char *name,
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index b4c9eb4..503b23e 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -44,7 +44,7 @@ struct security_class_mapping secclass_map[] = {
 	    "quotaget", NULL } },
 	{ "file",
 	  { COMMON_FILE_PERMS,
-	    "execute_no_trans", "entrypoint", NULL } },
+	    "execute_no_trans", "entrypoint", "setsuid", NULL } },
 	{ "dir",
 	  { COMMON_FILE_PERMS, "add_name", "remove_name",
 	    "reparent", "search", "rmdir", NULL } },


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-22 20:46 [PATCH] SELINUX: new permission controlling the ability to set suid Eric Paris
@ 2010-04-22 21:35 ` Stephen Smalley
  2010-04-23 12:03   ` Daniel J Walsh
  0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2010-04-22 21:35 UTC (permalink / raw)
  To: Eric Paris; +Cc: selinux, jmorris

On Thu, 2010-04-22 at 16:46 -0400, Eric Paris wrote:
> This patch adds a new permission, setsuid, to the file class.  This
> permission is checked any time a file has its attributes modified
> (setattr) and the setuid or setgid bits are involved.  The purpose for
> this change is to further allow selinux to confine administrative
> duties.
> 
> This permission is needed to both set the setuid bit and to add more
> permissions to a file which already has setuid bit.  This is also checked
> when an acl is modified on a file containing the suid or sgid bit.  We
> do not know if the acl is more or less restrictive, so we always check
> the permission on acl changes.

Can you help us all remember why you want this permission?  Specific use
case example, what threat you are mitigating, why it is sufficiently
important to justify an entirely new permission dedicated to it.

If we add this permission for this purpose, why wouldn't we also add
distinct permissions for other DAC purposes, e.g. suid-root vs.
suid-nonroot, suid vs sgid, sticky bit, setting vs clearing, making a
file executable, ...?  Where does it stop?

I'm also not keen on overloading it to also cover other changes to the
mode/ACL.  Presumably that is to constrain extending execute access to
an existing suid file more widely than originally set?  Write access
should kill suid/sgid bits.  Read access is perhaps a concern, but
relying on unreadable executables isn't a good idea.

Can you write a description of this permission and how it differs from
setattr permission in a way that will make sense to a typical Linux
admin?  

> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  security/selinux/hooks.c            |   38 ++++++++++++++++++++++++++++++-----
>  security/selinux/include/classmap.h |    2 +-
>  2 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 26b0a8a..bab6061 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -76,6 +76,7 @@
>  #include <linux/selinux.h>
>  #include <linux/mutex.h>
>  #include <linux/posix-timers.h>
> +#include <linux/posix_acl_xattr.h>
>  #include <linux/syslog.h>
>  
>  #include "avc.h"
> @@ -2728,6 +2729,9 @@ static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
>  {
>  	const struct cred *cred = current_cred();
>  	unsigned int ia_valid = iattr->ia_valid;
> +	unsigned int mode = dentry->d_inode->i_mode;
> +	unsigned int ia_mode = iattr->ia_mode;
> +	u32 av = 0;
>  
>  	/* ATTR_FORCE is just used for ATTR_KILL_S[UG]ID. */
>  	if (ia_valid & ATTR_FORCE) {
> @@ -2737,11 +2741,28 @@ static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
>  			return 0;
>  	}
>  
> -	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
> -			ATTR_ATIME_SET | ATTR_MTIME_SET | ATTR_TIMES_SET))
> -		return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR);
> +	/*
> +	 * are we changing mode?
> +	 * is this a regular file?
> +	 * do resulting iattr->i_mode have the suid/gid bits set?
> +	 */
> +	if ((iattr->ia_valid & ATTR_MODE) &&
> +	    (S_ISREG(mode)) &&
> +	    (ia_mode & (S_ISUID | S_ISGID))) {
> +		/* calculate what bits are being added (if any) */
> +		unsigned int new_bits = ((mode ^ ia_mode) & ia_mode);
> +		if (new_bits)
> +			av |= FILE__SETSUID;
> +	}
> +
> +	if (iattr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
> +			       ATTR_ATIME_SET | ATTR_MTIME_SET))

Logic changed in the ATTR_TIMES_SET case?

> +		av |= FILE__SETATTR;
> +	else
> +		av |= FILE__WRITE;
> +
> +	return dentry_has_perm(cred, NULL, dentry, av);
>  
> -	return dentry_has_perm(cred, NULL, dentry, FILE__WRITE);
>  }
>  
>  static int selinux_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
> @@ -2754,6 +2775,7 @@ static int selinux_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
>  static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name)
>  {
>  	const struct cred *cred = current_cred();
> +	u32 av = FILE__SETATTR;
>  
>  	if (!strncmp(name, XATTR_SECURITY_PREFIX,
>  		     sizeof XATTR_SECURITY_PREFIX - 1)) {
> @@ -2765,11 +2787,17 @@ static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name)
>  			   Restrict to administrator. */
>  			return -EPERM;
>  		}
> +	} else if ((dentry->d_inode->i_mode & (S_ISUID | S_ISGID)) &&
> +		   (!strncmp(name, POSIX_ACL_XATTR_ACCESS,
> +			     sizeof(POSIX_ACL_XATTR_ACCESS) - 1))) {
> +			/* changing access acl on a file with setuid/segid
> +			 * requires FILE_SETSUID */
> +			av |= FILE__SETSUID;
>  	}
>  
>  	/* Not an attribute we recognize, so just check the
>  	   ordinary setattr permission. */
> -	return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR);
> +	return dentry_has_perm(cred, NULL, dentry, av);
>  }
>  
>  static int selinux_inode_setxattr(struct dentry *dentry, const char *name,
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index b4c9eb4..503b23e 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -44,7 +44,7 @@ struct security_class_mapping secclass_map[] = {
>  	    "quotaget", NULL } },
>  	{ "file",
>  	  { COMMON_FILE_PERMS,
> -	    "execute_no_trans", "entrypoint", NULL } },
> +	    "execute_no_trans", "entrypoint", "setsuid", NULL } },
>  	{ "dir",
>  	  { COMMON_FILE_PERMS, "add_name", "remove_name",
>  	    "reparent", "search", "rmdir", NULL } },
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-22 21:35 ` Stephen Smalley
@ 2010-04-23 12:03   ` Daniel J Walsh
  2010-04-23 13:51     ` Stephen Smalley
  2010-04-26  6:18     ` Michal Svoboda
  0 siblings, 2 replies; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-23 12:03 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Eric Paris, selinux, jmorris

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/22/2010 05:35 PM, Stephen Smalley wrote:
> On Thu, 2010-04-22 at 16:46 -0400, Eric Paris wrote:
>> This patch adds a new permission, setsuid, to the file class.  This
>> permission is checked any time a file has its attributes modified
>> (setattr) and the setuid or setgid bits are involved.  The purpose for
>> this change is to further allow selinux to confine administrative
>> duties.
>>
>> This permission is needed to both set the setuid bit and to add more
>> permissions to a file which already has setuid bit.  This is also checked
>> when an acl is modified on a file containing the suid or sgid bit.  We
>> do not know if the acl is more or less restrictive, so we always check
>> the permission on acl changes.
> 
> Can you help us all remember why you want this permission?  Specific use
> case example, what threat you are mitigating, why it is sufficiently
> important to justify an entirely new permission dedicated to it.
> 
> If we add this permission for this purpose, why wouldn't we also add
> distinct permissions for other DAC purposes, e.g. suid-root vs.
> suid-nonroot, suid vs sgid, sticky bit, setting vs clearing, making a
> file executable, ...?  Where does it stop?
> 
> I'm also not keen on overloading it to also cover other changes to the
> mode/ACL.  Presumably that is to constrain extending execute access to
> an existing suid file more widely than originally set?  Write access
> should kill suid/sgid bits.  Read access is perhaps a concern, but
> relying on unreadable executables isn't a good idea.
> 
> Can you write a description of this permission and how it differs from
> setattr permission in a way that will make sense to a typical Linux
> admin?  
> 
>> Signed-off-by: Eric Paris <eparis@redhat.com>

My original thinking on this was a way to separate out Confined Root
Admin from user.

One possible use case would be.  I want to allow a user to login as
unconfined_t and only be able to become root as webadm_t through sudo.

If webadm_t has setattr on /var/www, he can cp /bin/sh /var/www/sh,
chcon 4755 /var/www/sh, exit webadm_t and as unconfined_t become root
using /var/www/sh.

I think your other examples are valid, but I believe setuid/setgid
applications have always been considered much more of a problem then the
others.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvRjK4ACgkQrlYvE4MpobOw6wCeLY0Yn5eLQu9hibHrTYLAqVex
IMIAn2P7S1lobPS21eAjOL/dBwua5R0K
=K0Xx
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-23 12:03   ` Daniel J Walsh
@ 2010-04-23 13:51     ` Stephen Smalley
  2010-04-26  6:18     ` Michal Svoboda
  1 sibling, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2010-04-23 13:51 UTC (permalink / raw)
  To: Daniel J Walsh; +Cc: Eric Paris, selinux, jmorris

On Fri, 2010-04-23 at 08:03 -0400, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 04/22/2010 05:35 PM, Stephen Smalley wrote:
> > On Thu, 2010-04-22 at 16:46 -0400, Eric Paris wrote:
> >> This patch adds a new permission, setsuid, to the file class.  This
> >> permission is checked any time a file has its attributes modified
> >> (setattr) and the setuid or setgid bits are involved.  The purpose for
> >> this change is to further allow selinux to confine administrative
> >> duties.
> >>
> >> This permission is needed to both set the setuid bit and to add more
> >> permissions to a file which already has setuid bit.  This is also checked
> >> when an acl is modified on a file containing the suid or sgid bit.  We
> >> do not know if the acl is more or less restrictive, so we always check
> >> the permission on acl changes.
> > 
> > Can you help us all remember why you want this permission?  Specific use
> > case example, what threat you are mitigating, why it is sufficiently
> > important to justify an entirely new permission dedicated to it.
> > 
> > If we add this permission for this purpose, why wouldn't we also add
> > distinct permissions for other DAC purposes, e.g. suid-root vs.
> > suid-nonroot, suid vs sgid, sticky bit, setting vs clearing, making a
> > file executable, ...?  Where does it stop?
> > 
> > I'm also not keen on overloading it to also cover other changes to the
> > mode/ACL.  Presumably that is to constrain extending execute access to
> > an existing suid file more widely than originally set?  Write access
> > should kill suid/sgid bits.  Read access is perhaps a concern, but
> > relying on unreadable executables isn't a good idea.
> > 
> > Can you write a description of this permission and how it differs from
> > setattr permission in a way that will make sense to a typical Linux
> > admin?  
> > 
> >> Signed-off-by: Eric Paris <eparis@redhat.com>
> 
> My original thinking on this was a way to separate out Confined Root
> Admin from user.
> 
> One possible use case would be.  I want to allow a user to login as
> unconfined_t and only be able to become root as webadm_t through sudo.
> 
> If webadm_t has setattr on /var/www, he can cp /bin/sh /var/www/sh,
> chcon 4755 /var/www/sh, exit webadm_t and as unconfined_t become root
> using /var/www/sh.
> 
> I think your other examples are valid, but I believe setuid/setgid
> applications have always been considered much more of a problem then the
> others.

I dug up the old threads on this topic:
http://marc.info/?t=119005242700001&r=1&w=2
http://marc.info/?t=122425833800011&r=1&w=2

It sounds like your only concern is being able to prevent the creation
of new suid (or perhaps even only suid-root) executables?  If so, then
the permission check could be much more tailored to that purpose so that
you only ever have to allow it in that situation and no other.

If we are going to revisit selinux_inode_setattr(), it would be nice to
systematically rework it to address any and all known concerns with it
rather than approaching it piecemeal.  I think I've said that before.

For example:
- Do we want to distinguish setting DAC protection attributes
(owner/group/mode/ACL) from setting other attributes?
- Do we want to distinguish the truncate and utimes(NULL) case from just
'write' permission (so that 'open' permission is more useful)?  For the
original discussion that led to changing those two cases to check
'write' instead of 'setattr' permission, see:
http://marc.info/?t=105166906900001&r=1&w=2

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-23 12:03   ` Daniel J Walsh
  2010-04-23 13:51     ` Stephen Smalley
@ 2010-04-26  6:18     ` Michal Svoboda
  2010-04-26 12:52       ` Daniel J Walsh
  1 sibling, 1 reply; 19+ messages in thread
From: Michal Svoboda @ 2010-04-26  6:18 UTC (permalink / raw)
  To: selinux

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

Daniel J Walsh wrote:
> One possible use case would be.  I want to allow a user to login as
> unconfined_t and only be able to become root as webadm_t through sudo.
> 
> If webadm_t has setattr on /var/www, he can cp /bin/sh /var/www/sh,
> chcon 4755 /var/www/sh, exit webadm_t and as unconfined_t become root
> using /var/www/sh.

Isn't this just a side effect of the 'unconfined' philosophy? I've
always been taught (and taught others) that with proper MAC controls you
can have as many setuid shells as you like.

You already give all your trust to the user by giving him unconfined.
Placing setuid controls in place is curing only (one of many) symptoms,
not the cause.

Michal Svoboda


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-26  6:18     ` Michal Svoboda
@ 2010-04-26 12:52       ` Daniel J Walsh
  2010-04-26 14:39         ` Michal Svoboda
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-26 12:52 UTC (permalink / raw)
  To: selinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/26/2010 02:18 AM, Michal Svoboda wrote:
> Daniel J Walsh wrote:
>> One possible use case would be.  I want to allow a user to login as
>> unconfined_t and only be able to become root as webadm_t through sudo.
>>
>> If webadm_t has setattr on /var/www, he can cp /bin/sh /var/www/sh,
>> chcon 4755 /var/www/sh, exit webadm_t and as unconfined_t become root
>> using /var/www/sh.
> 
> Isn't this just a side effect of the 'unconfined' philosophy? I've
> always been taught (and taught others) that with proper MAC controls you
> can have as many setuid shells as you like.
> 
> You already give all your trust to the user by giving him unconfined.
> Placing setuid controls in place is curing only (one of many) symptoms,
> not the cause.
> 
> Michal Svoboda
> 
First my example was sort of a gross oversimplification.  It would not
only effect unconfined_t but any other domain that could use the setuid
bit to gain additional privs.

unconfined_t to a user means, give him all the power of a normal user
with SELinux disabled.  You are still protected by DAC.  I would argue
that you want to make sure there are limited setuid apps around when
running with unconfined_t.  But if you give him unconfined_t and "chcon
4755"  as a confined user running as root, then you make it easy for him
to become unconfined_t running as UID=0.

If we want people to experiment with confined admins, allow unconfined_t
- -> sudo_exec_t -> confined_admin_t is a good thing.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvVjHsACgkQrlYvE4MpobOm9ACfZfmZfoTmD2In2wSC5+asiQUU
AmEAnjgC7RlRt2xtdUAm/t7gzYHMqBG9
=miW8
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-26 12:52       ` Daniel J Walsh
@ 2010-04-26 14:39         ` Michal Svoboda
  2010-04-26 15:19           ` Daniel J Walsh
  0 siblings, 1 reply; 19+ messages in thread
From: Michal Svoboda @ 2010-04-26 14:39 UTC (permalink / raw)
  To: selinux

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

Daniel J Walsh wrote:
> First my example was sort of a gross oversimplification.  It would not
> only effect unconfined_t but any other domain that could use the
> setuid bit to gain additional privs.

If a domain can use uid=0 to get additional privileges then the MAC
policy has holes... like cheese and relies on DAC to plumb them. That
significantly reduces the utility of the MAC.

> unconfined_t to a user means, give him all the power of a normal user
> with SELinux disabled.  You are still protected by DAC.  I would argue
> that you want to make sure there are limited setuid apps around when
> running with unconfined_t.  But if you give him unconfined_t and
> "chcon 4755"  as a confined user running as root, then you make it
> easy for him to become unconfined_t running as UID=0.

The problem with this reasoning is thus:

1) you give the user unconfined_t, well, okay, sad but perhaps necessary
2) you give the user root
3) you wonder that the user has root & unconfined

I know that the root in (2) is 'bundled in' a MAC restricted domain but
since root is a DAC concept it propagates ('leaks') back to the DAC
world.

You now want the MAC to stop the leak, but IMO that is a futile attempt.
It would take some serious analysis to find out how many ways 'root' can
leak out. I bet suid is not the only one. How about screwing up the web
server so that it runs as root and listens for your commands.

> If we want people to experiment with confined admins, allow unconfined_t
> - -> sudo_exec_t -> confined_admin_t is a good thing.

People can experiment with confined admins as they like, but they should
note that as long as they have an unconfined element in the game the
system simply won't be secure. Default allow has never worked.

Simply put I don't like this from systemic sense, but perhaps the demand
is so high that you should go ahead (with a big fat red warning).


Michal Svoboda


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-26 14:39         ` Michal Svoboda
@ 2010-04-26 15:19           ` Daniel J Walsh
  2010-04-28 14:32             ` Karl MacMillan
  2010-04-28 16:18             ` Michal Svoboda
  0 siblings, 2 replies; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-26 15:19 UTC (permalink / raw)
  To: selinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/26/2010 10:39 AM, Michal Svoboda wrote:
> Daniel J Walsh wrote:
>> First my example was sort of a gross oversimplification.  It would not
>> only effect unconfined_t but any other domain that could use the
>> setuid bit to gain additional privs.
> 
> If a domain can use uid=0 to get additional privileges then the MAC
> policy has holes... like cheese and relies on DAC to plumb them. That
> significantly reduces the utility of the MAC.
>
You have to look at this domain by domain, and the goal of users with
MAC is to "target" certain domains.  If we went full lock down on every
domain then we would not have > 70% of the fedora community running with
SELinux enabeled in enforcing mode.

>> unconfined_t to a user means, give him all the power of a normal user
>> with SELinux disabled.  You are still protected by DAC.  I would argue
>> that you want to make sure there are limited setuid apps around when
>> running with unconfined_t.  But if you give him unconfined_t and
>> "chcon 4755"  as a confined user running as root, then you make it
>> easy for him to become unconfined_t running as UID=0.
> 
> The problem with this reasoning is thus:
> 
> 1) you give the user unconfined_t, well, okay, sad but perhaps necessary
> 2) you give the user root
> 3) you wonder that the user has root & unconfined
> 
> I know that the root in (2) is 'bundled in' a MAC restricted domain but
> since root is a DAC concept it propagates ('leaks') back to the DAC
> world.
> 
> You now want the MAC to stop the leak, but IMO that is a futile attempt.
> It would take some serious analysis to find out how many ways 'root' can
> leak out. I bet suid is not the only one. How about screwing up the web
> server so that it runs as root and listens for your commands.
> 
We are building higher fences around the prison.  I am not going for the
perfect, since this is the enemy of the good.  We have lots of domains
that require DAC privs.  It is the combination of DAC and MAC that make
SELinux powerful.  Are goal is not to remove DAC checks in any way.
>> If we want people to experiment with confined admins, allow unconfined_t
>> - -> sudo_exec_t -> confined_admin_t is a good thing.
> 
> People can experiment with confined admins as they like, but they should
> note that as long as they have an unconfined element in the game the
> system simply won't be secure. Default allow has never worked.
> 
This is not default allow.  It is DAC + MAC as opposed to the way most
people run, which is just DAC. I am trying to make setattr check better.

DAC + sudo versus DAC + MAC + SUDO.
> Simply put I don't like this from systemic sense, but perhaps the demand
> is so high that you should go ahead (with a big fat red warning).
> 
> 
> Michal Svoboda
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvVrxUACgkQrlYvE4MpobOxgACfX4LLT9tm0uhXE6uoTv1LuHRw
eTwAoNtDnyTC3O+XIIODLAF8dYE5AWei
=Kg4/
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-26 15:19           ` Daniel J Walsh
@ 2010-04-28 14:32             ` Karl MacMillan
  2010-04-28 15:39               ` Daniel J Walsh
  2010-04-28 16:18             ` Michal Svoboda
  1 sibling, 1 reply; 19+ messages in thread
From: Karl MacMillan @ 2010-04-28 14:32 UTC (permalink / raw)
  To: Daniel J Walsh; +Cc: selinux

On Mon, Apr 26, 2010 at 11:19 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 04/26/2010 10:39 AM, Michal Svoboda wrote:
>> Daniel J Walsh wrote:
>>> First my example was sort of a gross oversimplification.  It would not
>>> only effect unconfined_t but any other domain that could use the
>>> setuid bit to gain additional privs.
>>
>> If a domain can use uid=0 to get additional privileges then the MAC
>> policy has holes... like cheese and relies on DAC to plumb them. That
>> significantly reduces the utility of the MAC.
>>
> You have to look at this domain by domain, and the goal of users with
> MAC is to "target" certain domains.  If we went full lock down on every
> domain then we would not have > 70% of the fedora community running with
> SELinux enabeled in enforcing mode.
>
>>> unconfined_t to a user means, give him all the power of a normal user
>>> with SELinux disabled.  You are still protected by DAC.  I would argue
>>> that you want to make sure there are limited setuid apps around when
>>> running with unconfined_t.  But if you give him unconfined_t and
>>> "chcon 4755"  as a confined user running as root, then you make it
>>> easy for him to become unconfined_t running as UID=0.
>>
>> The problem with this reasoning is thus:
>>
>> 1) you give the user unconfined_t, well, okay, sad but perhaps necessary
>> 2) you give the user root
>> 3) you wonder that the user has root & unconfined
>>
>> I know that the root in (2) is 'bundled in' a MAC restricted domain but
>> since root is a DAC concept it propagates ('leaks') back to the DAC
>> world.
>>
>> You now want the MAC to stop the leak, but IMO that is a futile attempt.
>> It would take some serious analysis to find out how many ways 'root' can
>> leak out. I bet suid is not the only one. How about screwing up the web
>> server so that it runs as root and listens for your commands.
>>
> We are building higher fences around the prison.  I am not going for the
> perfect, since this is the enemy of the good.  We have lots of domains
> that require DAC privs.  It is the combination of DAC and MAC that make
> SELinux powerful.  Are goal is not to remove DAC checks in any way.
>>> If we want people to experiment with confined admins, allow unconfined_t
>>> - -> sudo_exec_t -> confined_admin_t is a good thing.
>>
>> People can experiment with confined admins as they like, but they should
>> note that as long as they have an unconfined element in the game the
>> system simply won't be secure. Default allow has never worked.
>>
> This is not default allow.  It is DAC + MAC as opposed to the way most
> people run, which is just DAC. I am trying to make setattr check better.
>
> DAC + sudo versus DAC + MAC + SUDO.

I thought that the intent of the current MAC / DAC interaction was
that capabilities are used to decompose root and MAC can restrict
capabilities on processes to add extra DAC protections. Now, I'll
admit a good deal of ignorance here, but is there a reason that we
can't just write policy using that mechanism to accomplish what you
are after? If we prevented confined admin domains with root from
having the needed capability to setuid files isn't that enough? Or if
the right capability doesn't exist, can't you add a new capability?

Karl


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 14:32             ` Karl MacMillan
@ 2010-04-28 15:39               ` Daniel J Walsh
  2010-04-28 17:57                 ` Karl MacMillan
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-28 15:39 UTC (permalink / raw)
  To: Karl MacMillan; +Cc: selinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 10:32 AM, Karl MacMillan wrote:
> On Mon, Apr 26, 2010 at 11:19 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 04/26/2010 10:39 AM, Michal Svoboda wrote:
>>> Daniel J Walsh wrote:
>>>> First my example was sort of a gross oversimplification.  It would not
>>>> only effect unconfined_t but any other domain that could use the
>>>> setuid bit to gain additional privs.
>>>
>>> If a domain can use uid=0 to get additional privileges then the MAC
>>> policy has holes... like cheese and relies on DAC to plumb them. That
>>> significantly reduces the utility of the MAC.
>>>
>> You have to look at this domain by domain, and the goal of users with
>> MAC is to "target" certain domains.  If we went full lock down on every
>> domain then we would not have > 70% of the fedora community running with
>> SELinux enabeled in enforcing mode.
>>
>>>> unconfined_t to a user means, give him all the power of a normal user
>>>> with SELinux disabled.  You are still protected by DAC.  I would argue
>>>> that you want to make sure there are limited setuid apps around when
>>>> running with unconfined_t.  But if you give him unconfined_t and
>>>> "chcon 4755"  as a confined user running as root, then you make it
>>>> easy for him to become unconfined_t running as UID=0.
>>>
>>> The problem with this reasoning is thus:
>>>
>>> 1) you give the user unconfined_t, well, okay, sad but perhaps necessary
>>> 2) you give the user root
>>> 3) you wonder that the user has root & unconfined
>>>
>>> I know that the root in (2) is 'bundled in' a MAC restricted domain but
>>> since root is a DAC concept it propagates ('leaks') back to the DAC
>>> world.
>>>
>>> You now want the MAC to stop the leak, but IMO that is a futile attempt.
>>> It would take some serious analysis to find out how many ways 'root' can
>>> leak out. I bet suid is not the only one. How about screwing up the web
>>> server so that it runs as root and listens for your commands.
>>>
>> We are building higher fences around the prison.  I am not going for the
>> perfect, since this is the enemy of the good.  We have lots of domains
>> that require DAC privs.  It is the combination of DAC and MAC that make
>> SELinux powerful.  Are goal is not to remove DAC checks in any way.
>>>> If we want people to experiment with confined admins, allow unconfined_t
>>>> - -> sudo_exec_t -> confined_admin_t is a good thing.
>>>
>>> People can experiment with confined admins as they like, but they should
>>> note that as long as they have an unconfined element in the game the
>>> system simply won't be secure. Default allow has never worked.
>>>
>> This is not default allow.  It is DAC + MAC as opposed to the way most
>> people run, which is just DAC. I am trying to make setattr check better.
>>
>> DAC + sudo versus DAC + MAC + SUDO.
> 
> I thought that the intent of the current MAC / DAC interaction was
> that capabilities are used to decompose root and MAC can restrict
> capabilities on processes to add extra DAC protections. Now, I'll
> admit a good deal of ignorance here, but is there a reason that we
> can't just write policy using that mechanism to accomplish what you
> are after? If we prevented confined admin domains with root from
> having the needed capability to setuid files isn't that enough? Or if
> the right capability doesn't exist, can't you add a new capability?
> 
> Karl
Write now the ability to setattr on a file gives you the ability to
chmod 4755 EXE on types you control.

But we want to allow chmod 755 EXE but prevent chmod 4755.  Eric is
adding a Access check for this.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYVrUACgkQrlYvE4MpobPCUwCfVKETZQvQKRSvmDpUyBGxFovk
bIEAnjDYGP2oyTxMG8P5xPYOT/WyW31p
=PSKp
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-26 15:19           ` Daniel J Walsh
  2010-04-28 14:32             ` Karl MacMillan
@ 2010-04-28 16:18             ` Michal Svoboda
  2010-04-28 17:32               ` Daniel J Walsh
  1 sibling, 1 reply; 19+ messages in thread
From: Michal Svoboda @ 2010-04-28 16:18 UTC (permalink / raw)
  To: selinux

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

Daniel J Walsh wrote:
> If we went full lock down on every domain then we would not have > 70%
> of the fedora community running with SELinux enabeled in enforcing
> mode.

I hear you and I still believe this was/is a smart choice, but I think
with your proposal you're not adhering to its spirit.

If unconfined means equal to DAC then the unconfined user should be able
to become unconfined root even if via seteuid backdoor. If you want to
prevent this then you're confining the user. (You're now _targeting_
that ability.)

So here's an idea: why not just make an unconfined_user_t that would be
stripped of root powers so that even if it becomes euid 0 he could not 
exercise them. Then just control the ways of unconfined_user_t becoming
unconfined_admin_t (for example, type transition on trusted seteuid
executable program files).

Seems to me much simpler and much more bulletbroof than removing _one_
possible way of many by what you proposed - that is confining an already
confined admin, which is only very remotely responsible for what you
want to avoid.

> This is not default allow.  It is DAC + MAC as opposed to the way most
> people run, which is just DAC. I am trying to make setattr check better.

Note that from MAC viewpoint, DAC is remarkably similar to default allow.


Michal Svoboda

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 16:18             ` Michal Svoboda
@ 2010-04-28 17:32               ` Daniel J Walsh
  2010-04-28 18:54                 ` Michal Svoboda
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-28 17:32 UTC (permalink / raw)
  To: selinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 12:18 PM, Michal Svoboda wrote:
> Daniel J Walsh wrote:
>> If we went full lock down on every domain then we would not have > 70%
>> of the fedora community running with SELinux enabeled in enforcing
>> mode.
> 
> I hear you and I still believe this was/is a smart choice, but I think
> with your proposal you're not adhering to its spirit.
> 
> If unconfined means equal to DAC then the unconfined user should be able
> to become unconfined root even if via seteuid backdoor. If you want to
> prevent this then you're confining the user. (You're now _targeting_
> that ability.)
> 
> So here's an idea: why not just make an unconfined_user_t that would be
> stripped of root powers so that even if it becomes euid 0 he could not 
> exercise them. Then just control the ways of unconfined_user_t becoming
> unconfined_admin_t (for example, type transition on trusted seteuid
> executable program files).
> 
> Seems to me much simpler and much more bulletbroof than removing _one_
> possible way of many by what you proposed - that is confining an already
> confined admin, which is only very remotely responsible for what you
> want to avoid.
> 
>> This is not default allow.  It is DAC + MAC as opposed to the way most
>> people run, which is just DAC. I am trying to make setattr check better.
> 
> Note that from MAC viewpoint, DAC is remarkably similar to default allow.
> 
> 
> Michal Svoboda


Well in a way this is what staff_t is, a user which can run most apps
without a problem. but when it runs an app that requires capabilities,
it needs to transition to another domain.  staff_t is what I run on my
laptop.  The problem is staff_t < unconfined_t in that it can not run
apps that require capabilities that someone has not written policy for.

Admin installs a third party app that requires setuid/setgid or some
other priv, now he needs to write policy to transition his staff_t to
thirdparty_t.  In my scenario, unconfined_t will be able to run the
third party app, and will be able to becom confinedadmin_t for some sudo
jobs.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYcToACgkQrlYvE4MpobOlJQCeMYi4JDYBIdlo5hYeA2WZGEPT
NvAAoKta0qd51FFAGJWhB40r1KPQNmTB
=xSvm
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 15:39               ` Daniel J Walsh
@ 2010-04-28 17:57                 ` Karl MacMillan
  2010-04-28 18:07                   ` Stephen Smalley
  0 siblings, 1 reply; 19+ messages in thread
From: Karl MacMillan @ 2010-04-28 17:57 UTC (permalink / raw)
  To: Daniel J Walsh; +Cc: selinux

On Wed, Apr 28, 2010 at 11:39 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>>> This is not default allow.  It is DAC + MAC as opposed to the way most
>>> people run, which is just DAC. I am trying to make setattr check better.
>>>
>>> DAC + sudo versus DAC + MAC + SUDO.
>>
>> I thought that the intent of the current MAC / DAC interaction was
>> that capabilities are used to decompose root and MAC can restrict
>> capabilities on processes to add extra DAC protections. Now, I'll
>> admit a good deal of ignorance here, but is there a reason that we
>> can't just write policy using that mechanism to accomplish what you
>> are after? If we prevented confined admin domains with root from
>> having the needed capability to setuid files isn't that enough? Or if
>> the right capability doesn't exist, can't you add a new capability?
>>
>> Karl
> Write now the ability to setattr on a file gives you the ability to
> chmod 4755 EXE on types you control.
>
> But we want to allow chmod 755 EXE but prevent chmod 4755.  Eric is
> adding a Access check for this.

I understand that, but I (and I think others) are concerned about
directly adding permissions for what is essentially DAC policy. I was
wondering why the current strategy of mitigating DAC with SELinux
through capabilities is not workable in this case. That has the
additional benefit of allowing non-SELinux systems to benefit as well
if new capabilities are needed.

Karl

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.14 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvYVrUACgkQrlYvE4MpobPCUwCfVKETZQvQKRSvmDpUyBGxFovk
> bIEAnjDYGP2oyTxMG8P5xPYOT/WyW31p
> =PSKp
> -----END PGP SIGNATURE-----
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 17:57                 ` Karl MacMillan
@ 2010-04-28 18:07                   ` Stephen Smalley
  2010-04-28 18:31                     ` Karl MacMillan
  0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2010-04-28 18:07 UTC (permalink / raw)
  To: Karl MacMillan; +Cc: Daniel J Walsh, selinux

On Wed, 2010-04-28 at 13:57 -0400, Karl MacMillan wrote:
> On Wed, Apr 28, 2010 at 11:39 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> >>> This is not default allow.  It is DAC + MAC as opposed to the way most
> >>> people run, which is just DAC. I am trying to make setattr check better.
> >>>
> >>> DAC + sudo versus DAC + MAC + SUDO.
> >>
> >> I thought that the intent of the current MAC / DAC interaction was
> >> that capabilities are used to decompose root and MAC can restrict
> >> capabilities on processes to add extra DAC protections. Now, I'll
> >> admit a good deal of ignorance here, but is there a reason that we
> >> can't just write policy using that mechanism to accomplish what you
> >> are after? If we prevented confined admin domains with root from
> >> having the needed capability to setuid files isn't that enough? Or if
> >> the right capability doesn't exist, can't you add a new capability?
> >>
> >> Karl
> > Write now the ability to setattr on a file gives you the ability to
> > chmod 4755 EXE on types you control.
> >
> > But we want to allow chmod 755 EXE but prevent chmod 4755.  Eric is
> > adding a Access check for this.
> 
> I understand that, but I (and I think others) are concerned about
> directly adding permissions for what is essentially DAC policy. I was
> wondering why the current strategy of mitigating DAC with SELinux
> through capabilities is not workable in this case. That has the
> additional benefit of allowing non-SELinux systems to benefit as well
> if new capabilities are needed.

Setting the suid bit on a file is not a privileged operation on
Unix/Linux, so there is no capability check on it and you can't add a
capability check to it without breaking Unix/Linux compatibility.  Any
user is free to create his own "entrypoints" to his own uid/gid in this
manner at present.  Dan wants to be able to prevent that, particularly
since he is trying to give a non-root user the ability to run confined
root shells.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 18:07                   ` Stephen Smalley
@ 2010-04-28 18:31                     ` Karl MacMillan
  2010-04-28 18:41                       ` Daniel J Walsh
  2010-04-28 18:45                       ` Stephen Smalley
  0 siblings, 2 replies; 19+ messages in thread
From: Karl MacMillan @ 2010-04-28 18:31 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Daniel J Walsh, selinux

On Wed, Apr 28, 2010 at 2:07 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2010-04-28 at 13:57 -0400, Karl MacMillan wrote:
>> On Wed, Apr 28, 2010 at 11:39 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>> >>> This is not default allow.  It is DAC + MAC as opposed to the way most
>> >>> people run, which is just DAC. I am trying to make setattr check better.
>> >>>
>> >>> DAC + sudo versus DAC + MAC + SUDO.
>> >>
>> >> I thought that the intent of the current MAC / DAC interaction was
>> >> that capabilities are used to decompose root and MAC can restrict
>> >> capabilities on processes to add extra DAC protections. Now, I'll
>> >> admit a good deal of ignorance here, but is there a reason that we
>> >> can't just write policy using that mechanism to accomplish what you
>> >> are after? If we prevented confined admin domains with root from
>> >> having the needed capability to setuid files isn't that enough? Or if
>> >> the right capability doesn't exist, can't you add a new capability?
>> >>
>> >> Karl
>> > Write now the ability to setattr on a file gives you the ability to
>> > chmod 4755 EXE on types you control.
>> >
>> > But we want to allow chmod 755 EXE but prevent chmod 4755.  Eric is
>> > adding a Access check for this.
>>
>> I understand that, but I (and I think others) are concerned about
>> directly adding permissions for what is essentially DAC policy. I was
>> wondering why the current strategy of mitigating DAC with SELinux
>> through capabilities is not workable in this case. That has the
>> additional benefit of allowing non-SELinux systems to benefit as well
>> if new capabilities are needed.
>
> Setting the suid bit on a file is not a privileged operation on
> Unix/Linux, so there is no capability check on it and you can't add a
> capability check to it without breaking Unix/Linux compatibility.  Any
> user is free to create his own "entrypoints" to his own uid/gid in this
> manner at present.  Dan wants to be able to prevent that, particularly
> since he is trying to give a non-root user the ability to run confined
> root shells.
>

Ok - thanks for the explanation. You make it sound almost as if DAC is
discretionary.

Next stupid question then - is disallowing setattr for these shells
either not sufficient to achieve the desired security or not workable
in practice? I know the goal is to allow chmod but just not allow
creation of root owned setuid files, but is allowing chmod really
necessary for these limited use shells? Again, I'm just trying to
avoid the slippery slope of adding fine-grained control over DAC to
SELinux.

Karl

> --
> Stephen Smalley
> National Security Agency
>
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 18:31                     ` Karl MacMillan
@ 2010-04-28 18:41                       ` Daniel J Walsh
  2010-04-28 18:45                       ` Stephen Smalley
  1 sibling, 0 replies; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-28 18:41 UTC (permalink / raw)
  To: Karl MacMillan; +Cc: Stephen Smalley, selinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 02:31 PM, Karl MacMillan wrote:
> On Wed, Apr 28, 2010 at 2:07 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> On Wed, 2010-04-28 at 13:57 -0400, Karl MacMillan wrote:
>>> On Wed, Apr 28, 2010 at 11:39 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
>>>>>> This is not default allow.  It is DAC + MAC as opposed to the way most
>>>>>> people run, which is just DAC. I am trying to make setattr check better.
>>>>>>
>>>>>> DAC + sudo versus DAC + MAC + SUDO.
>>>>>
>>>>> I thought that the intent of the current MAC / DAC interaction was
>>>>> that capabilities are used to decompose root and MAC can restrict
>>>>> capabilities on processes to add extra DAC protections. Now, I'll
>>>>> admit a good deal of ignorance here, but is there a reason that we
>>>>> can't just write policy using that mechanism to accomplish what you
>>>>> are after? If we prevented confined admin domains with root from
>>>>> having the needed capability to setuid files isn't that enough? Or if
>>>>> the right capability doesn't exist, can't you add a new capability?
>>>>>
>>>>> Karl
>>>> Write now the ability to setattr on a file gives you the ability to
>>>> chmod 4755 EXE on types you control.
>>>>
>>>> But we want to allow chmod 755 EXE but prevent chmod 4755.  Eric is
>>>> adding a Access check for this.
>>>
>>> I understand that, but I (and I think others) are concerned about
>>> directly adding permissions for what is essentially DAC policy. I was
>>> wondering why the current strategy of mitigating DAC with SELinux
>>> through capabilities is not workable in this case. That has the
>>> additional benefit of allowing non-SELinux systems to benefit as well
>>> if new capabilities are needed.
>>
>> Setting the suid bit on a file is not a privileged operation on
>> Unix/Linux, so there is no capability check on it and you can't add a
>> capability check to it without breaking Unix/Linux compatibility.  Any
>> user is free to create his own "entrypoints" to his own uid/gid in this
>> manner at present.  Dan wants to be able to prevent that, particularly
>> since he is trying to give a non-root user the ability to run confined
>> root shells.
>>
> 
> Ok - thanks for the explanation. You make it sound almost as if DAC is
> discretionary.
> 
> Next stupid question then - is disallowing setattr for these shells
> either not sufficient to achieve the desired security or not workable
> in practice? I know the goal is to allow chmod but just not allow
> creation of root owned setuid files, but is allowing chmod really
> necessary for these limited use shells? Again, I'm just trying to
> avoid the slippery slope of adding fine-grained control over DAC to
> SELinux.
> 
> Karl

My use case was of the web administrator creating cgi scripts.  Being
able to copy files into a directory and then chown and chmod would be
the use case.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYgWoACgkQrlYvE4MpobOzTACfWUOhYXlms/NIaeLQJfEpkQIq
m6cAoJNCJCb55Xkt/nW85A5CeXdhKB5R
=j1PM
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 18:31                     ` Karl MacMillan
  2010-04-28 18:41                       ` Daniel J Walsh
@ 2010-04-28 18:45                       ` Stephen Smalley
  1 sibling, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2010-04-28 18:45 UTC (permalink / raw)
  To: Karl MacMillan; +Cc: Daniel J Walsh, selinux

On Wed, 2010-04-28 at 14:31 -0400, Karl MacMillan wrote:
> On Wed, Apr 28, 2010 at 2:07 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > On Wed, 2010-04-28 at 13:57 -0400, Karl MacMillan wrote:
> >> On Wed, Apr 28, 2010 at 11:39 AM, Daniel J Walsh <dwalsh@redhat.com> wrote:
> >> >>> This is not default allow.  It is DAC + MAC as opposed to the way most
> >> >>> people run, which is just DAC. I am trying to make setattr check better.
> >> >>>
> >> >>> DAC + sudo versus DAC + MAC + SUDO.
> >> >>
> >> >> I thought that the intent of the current MAC / DAC interaction was
> >> >> that capabilities are used to decompose root and MAC can restrict
> >> >> capabilities on processes to add extra DAC protections. Now, I'll
> >> >> admit a good deal of ignorance here, but is there a reason that we
> >> >> can't just write policy using that mechanism to accomplish what you
> >> >> are after? If we prevented confined admin domains with root from
> >> >> having the needed capability to setuid files isn't that enough? Or if
> >> >> the right capability doesn't exist, can't you add a new capability?
> >> >>
> >> >> Karl
> >> > Write now the ability to setattr on a file gives you the ability to
> >> > chmod 4755 EXE on types you control.
> >> >
> >> > But we want to allow chmod 755 EXE but prevent chmod 4755.  Eric is
> >> > adding a Access check for this.
> >>
> >> I understand that, but I (and I think others) are concerned about
> >> directly adding permissions for what is essentially DAC policy. I was
> >> wondering why the current strategy of mitigating DAC with SELinux
> >> through capabilities is not workable in this case. That has the
> >> additional benefit of allowing non-SELinux systems to benefit as well
> >> if new capabilities are needed.
> >
> > Setting the suid bit on a file is not a privileged operation on
> > Unix/Linux, so there is no capability check on it and you can't add a
> > capability check to it without breaking Unix/Linux compatibility.  Any
> > user is free to create his own "entrypoints" to his own uid/gid in this
> > manner at present.  Dan wants to be able to prevent that, particularly
> > since he is trying to give a non-root user the ability to run confined
> > root shells.
> >
> 
> Ok - thanks for the explanation. You make it sound almost as if DAC is
> discretionary.
> 
> Next stupid question then - is disallowing setattr for these shells
> either not sufficient to achieve the desired security or not workable
> in practice? I know the goal is to allow chmod but just not allow
> creation of root owned setuid files, but is allowing chmod really
> necessary for these limited use shells? Again, I'm just trying to
> avoid the slippery slope of adding fine-grained control over DAC to
> SELinux.

Dan's use model appears to be enabling unconfined users to assume roles
via sudo for things like web administration.  So you sudo -r webadm_r,
becoming both uid 0 and webadm_r:webadm_t, and then you can go
change /var/www at will, including setting DAC modes via chmod but are
otherwise restricted based on webadm_t.  But he doesn't want you to be
able to leave an entrypoint executable into uid 0 in /var/www that can
later be executed from unconfined_t.

I'm not so fond of the scenario.  But I can understand wanting to
control the ability to create new entrypoints to uids/gids, and I don't
mind splitting up setattr further as long as we come up with something
rational and don't keep adding ad-hoc permissions every so often.  So I
want to identify now all the possible permissions of interest, as I
mentioned in my email last Friday.  And I want to fix an existing
problem we have with the utimes(NULL) and truncate() checking while
we're at it.  I won't ack new setattr permissions until we've fixed
that.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 17:32               ` Daniel J Walsh
@ 2010-04-28 18:54                 ` Michal Svoboda
  2010-04-28 19:02                   ` Daniel J Walsh
  0 siblings, 1 reply; 19+ messages in thread
From: Michal Svoboda @ 2010-04-28 18:54 UTC (permalink / raw)
  To: selinux

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

Daniel J Walsh wrote:
> Admin installs a third party app that requires setuid/setgid or some
> other priv, now he needs to write policy to transition his staff_t to
> thirdparty_t.  In my scenario, unconfined_t will be able to run the
> third party app, and will be able to becom confinedadmin_t for some sudo
> jobs.

The admin will have a choice to either write that policy or keep the
users unconfined while sacrificing some security (that setuid example
AND a lot of others) or to give users two roles for this n that.

Isn't this feasible?


Michal Svoboda


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] SELINUX: new permission controlling the ability to set suid
  2010-04-28 18:54                 ` Michal Svoboda
@ 2010-04-28 19:02                   ` Daniel J Walsh
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel J Walsh @ 2010-04-28 19:02 UTC (permalink / raw)
  To: selinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 02:54 PM, Michal Svoboda wrote:
> Daniel J Walsh wrote:
>> Admin installs a third party app that requires setuid/setgid or some
>> other priv, now he needs to write policy to transition his staff_t to
>> thirdparty_t.  In my scenario, unconfined_t will be able to run the
>> third party app, and will be able to becom confinedadmin_t for some sudo
>> jobs.
> 
> The admin will have a choice to either write that policy or keep the
> users unconfined while sacrificing some security (that setuid example
> AND a lot of others) or to give users two roles for this n that.
> 
> Isn't this feasible?
> 
> 
> Michal Svoboda
> 
Feasable yes and for those uses we have staff_t.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvYhkMACgkQrlYvE4MpobOSJACgtibXHeEjgLkYwn7CdAxVcZbb
Sb4AoJkMtbz7/q4PTjZlBGG1MeIwhJIs
=uaA/
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-04-28 19:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-22 20:46 [PATCH] SELINUX: new permission controlling the ability to set suid Eric Paris
2010-04-22 21:35 ` Stephen Smalley
2010-04-23 12:03   ` Daniel J Walsh
2010-04-23 13:51     ` Stephen Smalley
2010-04-26  6:18     ` Michal Svoboda
2010-04-26 12:52       ` Daniel J Walsh
2010-04-26 14:39         ` Michal Svoboda
2010-04-26 15:19           ` Daniel J Walsh
2010-04-28 14:32             ` Karl MacMillan
2010-04-28 15:39               ` Daniel J Walsh
2010-04-28 17:57                 ` Karl MacMillan
2010-04-28 18:07                   ` Stephen Smalley
2010-04-28 18:31                     ` Karl MacMillan
2010-04-28 18:41                       ` Daniel J Walsh
2010-04-28 18:45                       ` Stephen Smalley
2010-04-28 16:18             ` Michal Svoboda
2010-04-28 17:32               ` Daniel J Walsh
2010-04-28 18:54                 ` Michal Svoboda
2010-04-28 19:02                   ` Daniel J Walsh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.