2/2 rootcontext= option to label root inode when mounting (take 3)

PATCH In review Eric Paris 2 participants 4 messages Jul 5, 2006 SELinux security module

Share & tools

Eric Paris

author

2006-07-05 17:47 UTC #0 lore raw

PATCH 1/1 2/2 rootcontext= option to label root inode when mounting (take 3)

The second patch will introduce a new rootcontext= option to FS mounting. This option will allow you to explicitly label the root inode of a FS being mounted before that FS or inode because visable to userspace. This was found to be useful for things like stateless linux, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190001

This should address all of the concerns with the original patch. Take 3.

 security/selinux/hooks.c |   66 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 57 insertions(+), 9 deletions(-)

security/selinux/hooks.c renamed from security/selinux/hooks.c.withdecouple +57 -9

@@ -320,12 +320,14 @@ enum {
 	Opt_context = 1,
 	Opt_fscontext = 2,
 	Opt_defcontext = 4,
+	Opt_rootcontext = 8,
 };
 
 static match_table_t tokens = {
 	{Opt_context, "context=%s"},
 	{Opt_fscontext, "fscontext=%s"},
 	{Opt_defcontext, "defcontext=%s"},
+	{Opt_rootcontext, "rootcontext=%s"},
 };
 
 #define SEL_MOUNT_FAIL_MSG "SELinux:  duplicate or incompatible mount options\n"

@@ -346,10 +348,25 @@ static int may_context_mount_sb_relabel(
 	return rc;
 }
 
+static int may_context_mount_inode_relabel(u32 sid, 
+			struct superblock_security_struct *sbsec,
+			struct task_security_struct *tsec)
+{
+	int rc;
+	rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM,
+			  FILESYSTEM__RELABELFROM, NULL);
+	if (rc)
+		return rc;
+
+	rc = avc_has_perm(sid, sbsec->sid, SECCLASS_FILESYSTEM,
+			  FILESYSTEM__ASSOCIATE, NULL);
+	return rc;
+}
+
 static int try_context_mount(struct super_block *sb, void *data)
 {
 	char *context = NULL, *defcontext = NULL;
-	char *fscontext = NULL;
+	char *fscontext = NULL, *rootcontext = NULL;
 	const char *name;
 	u32 sid;
 	int alloc = 0, rc = 0, seen = 0;

@@ -423,6 +440,22 @@ static int try_context_mount(struct supe
 				seen |= Opt_fscontext;
 				break;
 
+			case Opt_rootcontext:
+				if (seen & Opt_rootcontext) {
+					rc = -EINVAL;
+					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+					goto out_free;
+				}
+				rootcontext = match_strdup(&args[0]);
+				if (!rootcontext) {
+					rc = -ENOMEM;
+					goto out_free;
+				}
+				if (!alloc)
+					alloc = 1;
+				seen |= Opt_rootcontext;
+				break;
+
 			case Opt_defcontext:
 				if (sbsec->behavior != SECURITY_FS_USE_XATTR) {
 					rc = -EINVAL;

@@ -501,6 +534,25 @@ static int try_context_mount(struct supe
 		sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
 	}
 
+	if (rootcontext) {
+		struct inode *inode = sb->s_root->d_inode;
+		struct inode_security_struct *isec = inode->i_security;
+		rc = security_context_to_sid(rootcontext, strlen(rootcontext), &sid);
+		if (rc) {
+			printk(KERN_WARNING "SELinux: security_context_to_sid"
+			       "(%s) failed for (dev %s, type %s) errno=%d\n",
+			       rootcontext, sb->s_id, name, rc);
+			goto out_free;
+		}
+
+		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
+		if (rc)
+			goto out_free;
+
+		isec->sid = sid;
+		isec->initialized = 1;
+	}
+
 	if (defcontext) {
 		rc = security_context_to_sid(defcontext, strlen(defcontext), &sid);
 		if (rc) {

@@ -513,13 +565,7 @@ static int try_context_mount(struct supe
 		if (sid == sbsec->def_sid)
 			goto out_free;
 
-		rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM,
-				  FILESYSTEM__RELABELFROM, NULL);
-		if (rc)
-			goto out_free;
-
-		rc = avc_has_perm(sid, sbsec->sid, SECCLASS_FILESYSTEM,
-				  FILESYSTEM__ASSOCIATE, NULL);
+		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
 		if (rc)
 			goto out_free;

@@ -531,6 +577,7 @@ out_free:
 		kfree(context);
 		kfree(defcontext);
 		kfree(fscontext);
+		kfree(rootcontext);
 	}
 out:
 	return rc;

@@ -1882,7 +1929,8 @@ static inline int selinux_option(char *o
 {
 	return (match_prefix("context=", sizeof("context=")-1, option, len) ||
 	        match_prefix("fscontext=", sizeof("fscontext=")-1, option, len) ||
-	        match_prefix("defcontext=", sizeof("defcontext=")-1, option, len));
+	        match_prefix("defcontext=", sizeof("defcontext=")-1, option, len) ||
+		match_prefix("rootcontext=", sizeof("rootcontext=")-1, option, len));
 }
 
 static inline void take_option(char **to, char *from, int *first, int len)

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Stephen Smalley

Acked-by on v1 1/1

2006-07-05 18:16 UTC #1 lore raw

Quoted 10 lines · Eric Paris

The second patch will introduce a new rootcontext= option to FS mounting. This option will allow you to explicitly label the root inode of a FS being mounted before that FS or inode because visable to userspace. This was found to be useful for things like stateless linux, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190001
This should address all of the concerns with the original patch. Take 3.
 security/selinux/hooks.c |   66 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 57 insertions(+), 9 deletions(-)

Acked-by Stephen Smalley <[email protected]>

Quoted 135 lines

security/selinux/hooks.c renamed from security/selinux/hooks.c.withdecouple +57 ~~-10~~

@@ -320,12 +320,14 @@ enum {
 	Opt_context = 1,
 	Opt_fscontext = 2,
 	Opt_defcontext = 4,
+	Opt_rootcontext = 8,
 };
 
 static match_table_t tokens = {
 	{Opt_context, "context=%s"},
 	{Opt_fscontext, "fscontext=%s"},
 	{Opt_defcontext, "defcontext=%s"},
+	{Opt_rootcontext, "rootcontext=%s"},
 };
 
 #define SEL_MOUNT_FAIL_MSG "SELinux:  duplicate or incompatible mount options\n"

@@ -346,10 +348,25 @@ static int may_context_mount_sb_relabel(
 	return rc;
 }
 
+static int may_context_mount_inode_relabel(u32 sid, 
+			struct superblock_security_struct *sbsec,
+			struct task_security_struct *tsec)
+{
+	int rc;
+	rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM,
+			  FILESYSTEM__RELABELFROM, NULL);
+	if (rc)
+		return rc;
+
+	rc = avc_has_perm(sid, sbsec->sid, SECCLASS_FILESYSTEM,
+			  FILESYSTEM__ASSOCIATE, NULL);
+	return rc;
+}
+
 static int try_context_mount(struct super_block *sb, void *data)
 {
 	char *context = NULL, *defcontext = NULL;
-	char *fscontext = NULL;
+	char *fscontext = NULL, *rootcontext = NULL;
 	const char *name;
 	u32 sid;
 	int alloc = 0, rc = 0, seen = 0;

@@ -423,6 +440,22 @@ static int try_context_mount(struct supe
 				seen |= Opt_fscontext;
 				break;
 
+			case Opt_rootcontext:
+				if (seen & Opt_rootcontext) {
+					rc = -EINVAL;
+					printk(KERN_WARNING SEL_MOUNT_FAIL_MSG);
+					goto out_free;
+				}
+				rootcontext = match_strdup(&args[0]);
+				if (!rootcontext) {
+					rc = -ENOMEM;
+					goto out_free;
+				}
+				if (!alloc)
+					alloc = 1;
+				seen |= Opt_rootcontext;
+				break;
+
 			case Opt_defcontext:
 				if (sbsec->behavior != SECURITY_FS_USE_XATTR) {
 					rc = -EINVAL;

@@ -501,6 +534,25 @@ static int try_context_mount(struct supe
 		sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
 	}
 
+	if (rootcontext) {
+		struct inode *inode = sb->s_root->d_inode;
+		struct inode_security_struct *isec = inode->i_security;
+		rc = security_context_to_sid(rootcontext, strlen(rootcontext), &sid);
+		if (rc) {
+			printk(KERN_WARNING "SELinux: security_context_to_sid"
+			       "(%s) failed for (dev %s, type %s) errno=%d\n",
+			       rootcontext, sb->s_id, name, rc);
+			goto out_free;
+		}
+
+		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
+		if (rc)
+			goto out_free;
+
+		isec->sid = sid;
+		isec->initialized = 1;
+	}
+
 	if (defcontext) {
 		rc = security_context_to_sid(defcontext, strlen(defcontext), &sid);
 		if (rc) {

@@ -513,13 +565,7 @@ static int try_context_mount(struct supe
 		if (sid == sbsec->def_sid)
 			goto out_free;
 
-		rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM,
-				  FILESYSTEM__RELABELFROM, NULL);
-		if (rc)
-			goto out_free;
-
-		rc = avc_has_perm(sid, sbsec->sid, SECCLASS_FILESYSTEM,
-				  FILESYSTEM__ASSOCIATE, NULL);
+		rc = may_context_mount_inode_relabel(sid, sbsec, tsec);
 		if (rc)
 			goto out_free;

@@ -531,6 +577,7 @@ out_free:
 		kfree(context);
 		kfree(defcontext);
 		kfree(fscontext);
+		kfree(rootcontext);
 	}
 out:
 	return rc;

@@ -1882,7 +1929,8 @@ static inline int selinux_option(char *o
 {
 	return (match_prefix("context=", sizeof("context=")-1, option, len) ||
 	        match_prefix("fscontext=", sizeof("fscontext=")-1, option, len) ||
-	        match_prefix("defcontext=", sizeof("defcontext=")-1, option, len));
+	        match_prefix("defcontext=", sizeof("defcontext=")-1, option, len) ||
+		match_prefix("rootcontext=", sizeof("rootcontext=")-1, option, len));
 }
 
 static inline void take_option(char **to, char *from, int *first, int len)
 
 
 
--

This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Signature · 8 lines

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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.

Eric Paris

author

2006-07-05 19:43 UTC #2 lore raw

Re: [PATCH] 2/2 rootcontext=, are RELABELFROM and ASSOCIATE enough in may_context_mount_inode_relabel?

Quoted 6 lines

security/selinux/hooks.c renamed from security/selinux/hooks.c.withdecouple +6 -0

+static int may_context_mount_inode_relabel(u32 sid, 
+			struct superblock_security_struct *sbsec,
+			struct task_security_struct *tsec)
+{
+	int rc;
+	rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM,

Quoted 8 lines

security/selinux/hooks.c static int may_context_mount_sb_relabel( · line 364
Quoted 8 lines
security/selinux/hooks.c renamed from security/selinux/hooks.c.withdecouple +8 -0
+			  FILESYSTEM__RELABELFROM, NULL);
+	if (rc)
+		return rc;
+
+	rc = avc_has_perm(sid, sbsec->sid, SECCLASS_FILESYSTEM,
+			  FILESYSTEM__ASSOCIATE, NULL);
+	return rc;
+}

Not trying to bomb my own ack'd patch, but I do question why we don't check RELABEL_TO for the new context we set either from rootcontext or defcontext. Wouldn't there be cases where we pass associate but wouldn't want to allow relabel_to? Does associate inherently mean that we are allowed to relabel to that type? I've been trying to think of a way to exploit this for some usable purpose and admittedly I can't think of one, but that doesn't mean that it isn't there...

on another note in may_context_mount_sb_relabel, when I set both fscontext and context shouldn't we be checking the associate permission between the context= (all files on the fs) and the context set to the sb from fscontext?

should we check all 3 perms for all 4 context mount options? My patch doesn't change what we have today, but I'm not convinced they are complete....

-Eric

Stephen Smalley

2006-07-05 20:36 UTC #3 lore raw

Re: [PATCH] 2/2 rootcontext=, are RELABELFROM and ASSOCIATE enough in may_context_mount_inode_relabel?

Quoted 15 lines · Eric Paris

Quoted 6 lines
Quoted 6 lines
security/selinux/hooks.c renamed from security/selinux/hooks.c.withdecouple +6 -0
+static int may_context_mount_inode_relabel(u32 sid, 
+			struct superblock_security_struct *sbsec,
+			struct task_security_struct *tsec)
+{
+	int rc;
+	rc = avc_has_perm(tsec->sid, sbsec->sid, SECCLASS_FILESYSTEM,
Quoted 8 lines
security/selinux/hooks.c static int may_context_mount_sb_relabel( · line 364
Quoted 8 lines
security/selinux/hooks.c renamed from security/selinux/hooks.c.withdecouple +8 -0
+			  FILESYSTEM__RELABELFROM, NULL);
+	if (rc)
+		return rc;
+
+	rc = avc_has_perm(sid, sbsec->sid, SECCLASS_FILESYSTEM,
+			  FILESYSTEM__ASSOCIATE, NULL);
+	return rc;
+}

Quoted 7 lines

Not trying to bomb my own ack'd patch, but I do question why we don't check RELABEL_TO for the new context we set either from rootcontext or defcontext. Wouldn't there be cases where we pass associate but wouldn't want to allow relabel_to? Does associate inherently mean that we are allowed to relabel to that type? I've been trying to think of a way to exploit this for some usable purpose and admittedly I can't think of one, but that doesn't mean that it isn't there...

First, you need to distinguish between the file relabeling checks (of selinux_inode_setxattr) and the context mount checks. Don't confuse file relabelfrom/relabelto with filesystem relabelfrom/relabelto.

Second, keep in mind that there is some overloading going on here, as we didn't originally have context mount options and when they were introduced, they leveraged the old permissions for relabeling of superblock security contexts that used to be supported when SELinux had its own persistent label mapping on the filesystem (prior to migration to using xattrs).

The filesystem relabelto check is expected to occur against a filesystem/superblock SID, not an inode SID, and represents the relabeling of the superblock to a new value. That is why we don't presently apply it to defcontext= (which doesn't actually relabel any object with an existing label - it just defines the default label for inodes without an xattr). Given the relabelfrom check against the superblock SID and the associate check, we can at least establish a transitive relationship between the process and the defcontext SID (or the rootcontext SID in your case). If we needed something finer-grained, we could introduce a further check there, but that should really be a separate permission check against the inode's security class, not the filesystem relabelto check. But we don't yet have an example needing that granularity.

Quoted 4 lines

on another note in may_context_mount_sb_relabel, when I set both fscontext and context shouldn't we be checking the associate permission between the context= (all files on the fs) and the context set to the sb from fscontext?

Hmmm...I suppose in that case, the context= value is not being used as a superblock security context, and thus you should be applying the associate check instead of the relabelto check for it.

Quoted 3 lines

should we check all 3 perms for all 4 context mount options? My patch doesn't change what we have today, but I'm not convinced they are complete....

Ideally, we'd only check filesystem class permissions against superblock SIDs (as the target SID).

Signature · 8 lines

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 [email protected] with
the words "unsubscribe selinux" without quotes as the message.