[RFC] Ability to allow undefined permissions and classes -v2

[RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

This an obviously incomplete second attempt at creating a mechanism to
allow permissions and classes which are defined in the kernel but not in
the policy being loaded.  It has a /selinux tunable which is used to
enable or disable the feature.  The real submission will not have this
and will instead be passed to the kernel much the same way as we pass
the MLS flag today.  So do not comment on the /selinux tunable portion
of the patch.  I also have not implemented the 3 state system (allow
unknown, deny unknown, deny policy with unknown) which was discussed,
but will do that if this method seems reasonable.

I started off doing as Stephen suggested and padding the avtab entries
with permissions which were unknown to the kernel.  But upon (completing
this implementation and then) re-reading the whole thread Stephen also
mentioned the case where NO TE rules existed but we still would want to
give a 'default' avd.allow which included undefined perms.  And when I
went back to look at this I threw out all of my previous padding at load
time.  Adding checks into context_struct_compute_av to see 'did we find
any avtab entry' appeared to me that it would take more work than simply
making the 'default' .allow created by context_struct_compute_av include
all the undefined perms.  In doing this I was able to stop padding avtab
entries on load.  Permission check time now has a single new if
statement and simple |=.  Neither of which do I guess will have
meaningful performance impact.

I do still have a problem.  I don't really understand the 'inherits'
ideas and so I'm not sure how to pad my p->undefined_perms[tclass] with
permissions which are missing in policy due to an incorrect or missing
inherits statement.  If this approach seems good and reasonable I will
try to better understand how I can glean those bits from
validate_classes() and I will also move this to use config flags and
have multiple options on how to handle policies.

Right now to make changes you need to:

echo 1 > /selinux/allow_unknown
semodule -R

-Eric

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index cd79c63..364d872 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -670,6 +670,8 @@ void policydb_destroy(struct policydb *p)
 	}
 	kfree(p->type_attr_map);
 
+	kfree(p->undefined_perms);
+
 	return;
 }
 
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 8319d5f..257b836 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -242,6 +242,8 @@ struct policydb {
 	struct ebitmap *type_attr_map;
 
 	unsigned int policyvers;
+
+	u32 *undefined_perms;
 };
 
 extern void policydb_destroy(struct policydb *p);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ca9154d..e6a60a5 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -306,22 +306,30 @@ static int context_struct_compute_av(struct context *scontext,
 		    tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
 			tclass = SECCLASS_NETLINK_SOCKET;
 
-	if (!tclass || tclass > policydb.p_classes.nprim) {
-		printk(KERN_ERR "security_compute_av:  unrecognized class %d\n",
-		       tclass);
-		return -EINVAL;
-	}
-	tclass_datum = policydb.class_val_to_struct[tclass - 1];
-
 	/*
 	 * Initialize the access vectors to the default values.
+	 * This will pad the allow for undefined perms if appropriete.
 	 */
-	avd->allowed = 0;
+	if(selinux_allow_unknown)
+		avd->allowed = policydb.undefined_perms[tclass];
+	else
+		avd->allowed = 0;
 	avd->decided = 0xffffffff;
 	avd->auditallow = 0;
 	avd->auditdeny = 0xffffffff;
 	avd->seqno = latest_granting;
 
+	if (!tclass || tclass > policydb.p_classes.nprim) {
+		if(selinux_allow_unknown && tclass) {
+			avd->allowed = 0xffffffff;
+			return 0;
+		}
+		else {
+			return -EINVAL;
+		}
+	}
+	tclass_datum = policydb.class_val_to_struct[tclass - 1];
+
 	/*
 	 * If a specific type enforcement rule was defined for
 	 * this permission check, then use it.
@@ -1037,6 +1045,7 @@ int security_change_sid(u32 ssid,
 static int validate_classes(struct policydb *p)
 {
 	int i, j;
+	int rc = 0;
 	struct class_datum *cladatum;
 	struct perm_datum *perdatum;
 	u32 nprim, tmp, common_pts_len, perm_val, pol_val;
@@ -1045,6 +1054,19 @@ static int validate_classes(struct policydb *p)
 	const char *def_class, *def_perm, *pol_class;
 	struct symtab *perms;
 
+	if (selinux_allow_unknown) {
+		int num_classes;
+		if (kdefs->cts_len > p->p_classes.nprim)
+			num_classes = kdefs->cts_len;
+		else
+			num_classes = p->p_classes.nprim;
+		p->undefined_perms = kzalloc(sizeof(u32)*num_classes, GFP_KERNEL);
+		if (!p->undefined_perms) {
+			rc = -ENOMEM;
+			goto out;
+		}
+	}
+
 	for (i = 1; i < kdefs->cts_len; i++) {
 		def_class = kdefs->class_to_string[i];
 		if (i > p->p_classes.nprim) {
@@ -1058,7 +1080,8 @@ static int validate_classes(struct policydb *p)
 			printk(KERN_ERR
 			       "security:  class %d is incorrect, found %s but should be %s\n",
 			       i, pol_class, def_class);
-			return -EINVAL;
+			rc = -EINVAL;
+			goto out;
 		}
 	}
 	for (i = 0; i < kdefs->av_pts_len; i++) {
@@ -1076,6 +1099,8 @@ static int validate_classes(struct policydb *p)
 			printk(KERN_INFO
 			       "security:  permission %s in class %s not defined in policy\n",
 			       def_perm, pol_class);
+			if (selinux_allow_unknown)
+				p->undefined_perms[class_val] |= perm_val;
 			continue;
 		}
 		perdatum = hashtab_search(perms->table, def_perm);
@@ -1083,14 +1108,16 @@ static int validate_classes(struct policydb *p)
 			printk(KERN_ERR
 			       "security:  permission %s in class %s not found in policy\n",
 			       def_perm, pol_class);
-			return -EINVAL;
+			rc = -EINVAL;
+			goto out;
 		}
 		pol_val = 1 << (perdatum->value - 1);
 		if (pol_val != perm_val) {
 			printk(KERN_ERR
 			       "security:  permission %s in class %s has incorrect value\n",
 			       def_perm, pol_class);
-			return -EINVAL;
+			rc = -EINVAL;
+			goto out;
 		}
 	}
 	for (i = 0; i < kdefs->av_inherit_len; i++) {
@@ -1104,7 +1131,8 @@ static int validate_classes(struct policydb *p)
 			printk(KERN_ERR
 			       "security:  class %s should have an inherits clause but does not\n",
 			       pol_class);
-			return -EINVAL;
+			rc = -EINVAL;
+			goto out;
 		}
 		tmp = kdefs->av_inherit[i].common_base;
 		common_pts_len = 0;
@@ -1126,17 +1154,24 @@ static int validate_classes(struct policydb *p)
 				printk(KERN_ERR
 				       "security:  permission %s in class %s not found in policy\n",
 				       def_perm, pol_class);
-				return -EINVAL;
+				rc = -EINVAL;
+				goto out;
 			}
 			if (perdatum->value != j + 1) {
 				printk(KERN_ERR
 				       "security:  permission %s in class %s has incorrect value\n",
 				       def_perm, pol_class);
-				return -EINVAL;
+				rc = -EINVAL;
+				goto out;
 			}
 		}
 	}
-	return 0;
+out:
+	if (rc && selinux_allow_unknown) {
+		kfree(p->undefined_perms);
+		p->undefined_perms = NULL;
+	}
+	return rc;
 }
 
 /* Clone the SID into the new SID table. */
@@ -1268,6 +1303,9 @@ int security_load_policy(void *data, size_t len)
 
 	LOAD_LOCK;
 
+	/* pay not attention to me as I am part of the /selinux madness!! */
+	selinux_allow_unknown = selinux_allow_unknown_sysctl_value;
+
 	if (!ss_initialized) {
 		avtab_cache_init();
 		if (policydb_read(&policydb, fp)) {
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65fb5e8..10af3d0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -86,6 +86,9 @@ extern unsigned int policydb_loaded_version;
 extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
 extern int selinux_compat_net;
 
+int selinux_allow_unknown = 0;
+int selinux_allow_unknown_sysctl_value = 0;
+
 #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
 int selinux_enforcing = 0;
 
diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index 6ed10c3..1b02289 100644
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -18,6 +18,9 @@
 #include "av_permissions.h"
 #include "security.h"
 
+extern int selinux_allow_unknown;
+extern int selinux_allow_unknown_sysctl_value;
+
 #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
 extern int selinux_enforcing;
 #else
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c8bf6e1..67c7810 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -100,6 +100,7 @@ enum sel_inos {
 	SEL_MEMBER,	/* compute polyinstantiation membership decision */
 	SEL_CHECKREQPROT, /* check requested protection, not kernel-applied one */
 	SEL_COMPAT_NET,	/* whether to use old compat network packet controls */
+	SEL_UNKNOWN,	/* allows unknown perms and classes */
 };
 
 #define TMPBUFLEN	12
@@ -113,6 +114,16 @@ static ssize_t sel_read_enforce(struct file *filp, char __user *buf,
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
 }
 
+static ssize_t sel_read_unknown(struct file *filp, char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	char tmpbuf[TMPBUFLEN];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, TMPBUFLEN, "%d", selinux_allow_unknown_sysctl_value);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
 #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
 static ssize_t sel_write_enforce(struct file * file, const char __user * buf,
 				 size_t count, loff_t *ppos)
@@ -161,11 +172,55 @@ out:
 #define sel_write_enforce NULL
 #endif
 
+static ssize_t sel_write_unknown(struct file * file, const char __user * buf,
+				 size_t count, loff_t *ppos)
+
+{
+	char *page;
+	ssize_t length;
+	int new_value;
+
+	if (count >= PAGE_SIZE)
+		return -ENOMEM;
+	if (*ppos != 0) {
+		/* No partial writes. */
+		return -EINVAL;
+	}
+	page = (char*)get_zeroed_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
+	length = -EFAULT;
+	if (copy_from_user(page, buf, count))
+		goto out;
+
+	length = -EINVAL;
+	if (sscanf(page, "%d", &new_value) != 1)
+		goto out;
+
+	if (new_value != selinux_allow_unknown_sysctl_value) {
+		length = 0;
+		audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_STATUS,
+			"allow_unknown=%d old_allow_unknown%d auid=%u", new_value, 
+			selinux_allow_unknown_sysctl_value,
+			audit_get_loginuid(current->audit_context));
+		selinux_allow_unknown_sysctl_value = new_value;
+	}
+	length = count;
+out:
+	free_page((unsigned long) page);
+	return length;
+}
+
 static struct file_operations sel_enforce_ops = {
 	.read		= sel_read_enforce,
 	.write		= sel_write_enforce,
 };
 
+static struct file_operations sel_unknown_ops = {
+	.read		= sel_read_unknown,
+	.write		= sel_write_unknown,
+};
+
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 static ssize_t sel_write_disable(struct file * file, const char __user * buf,
 				 size_t count, loff_t *ppos)
@@ -1283,6 +1338,7 @@ static int sel_fill_super(struct super_block * sb, void * data, int silent)
 		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
 		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
 		[SEL_COMPAT_NET] = {"compat_net", &sel_compat_net_ops, S_IRUGO|S_IWUSR},
+		[SEL_UNKNOWN] = {"allow_unknown", &sel_unknown_ops, S_IRUGO|S_IWUSR},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);



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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Mon, 2007-02-12 at 14:43 -0500, Eric Paris wrote:
> I started off doing as Stephen suggested and padding the avtab entries
> with permissions which were unknown to the kernel.  But upon (completing
> this implementation and then) re-reading the whole thread Stephen also
> mentioned the case where NO TE rules existed but we still would want to
> give a 'default' avd.allow which included undefined perms.  And when I
> went back to look at this I threw out all of my previous padding at load
> time.  Adding checks into context_struct_compute_av to see 'did we find
> any avtab entry' appeared to me that it would take more work than simply
> making the 'default' .allow created by context_struct_compute_av include
> all the undefined perms.  In doing this I was able to stop padding avtab
> entries on load.  Permission check time now has a single new if
> statement and simple |=.  Neither of which do I guess will have
> meaningful performance impact.

Fair enough.

> I do still have a problem.  I don't really understand the 'inherits'
> ideas and so I'm not sure how to pad my p->undefined_perms[tclass] with
> permissions which are missing in policy due to an incorrect or missing
> inherits statement.  If this approach seems good and reasonable I will
> try to better understand how I can glean those bits from
> validate_classes() and I will also move this to use config flags and
> have multiple options on how to handle policies.

Missing or incorrect inherits clause should remain fatal, I'd say - it
is too substantive a change to the class definition to expect a running
kernel to adapt.  Classes that inherit a common definition pick up their
initial set of permission bits from that common definition, such that
those bits are uniform across those classes and can be checked by a
single permission check (e.g. for common file or socket checks).  But as
soon as you remove or change them, in addition to removing all of those
inherited permissions, you will have disturbed the values of any
subsequent class-specific permissions.

> 
> Right now to make changes you need to:
> 
> echo 1 > /selinux/allow_unknown
> semodule -R
> 
> -Eric
> 

> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index ca9154d..e6a60a5 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -306,22 +306,30 @@ static int context_struct_compute_av(struct context *scontext,
>  		    tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
>  			tclass = SECCLASS_NETLINK_SOCKET;
>  
> -	if (!tclass || tclass > policydb.p_classes.nprim) {
> -		printk(KERN_ERR "security_compute_av:  unrecognized class %d\n",
> -		       tclass);
> -		return -EINVAL;
> -	}
> -	tclass_datum = policydb.class_val_to_struct[tclass - 1];
> -
>  	/*
>  	 * Initialize the access vectors to the default values.
> +	 * This will pad the allow for undefined perms if appropriete.
>  	 */
> -	avd->allowed = 0;
> +	if(selinux_allow_unknown)
> +		avd->allowed = policydb.undefined_perms[tclass];

You moved the bounds checks on tclass down further, but here you use
tclass as an index.  Also, why [tclass] vs. [tclass-1] as in other code?
Aren't you wasting undefined_perms[0]?

> +	else
> +		avd->allowed = 0;
>  	avd->decided = 0xffffffff;
>  	avd->auditallow = 0;
>  	avd->auditdeny = 0xffffffff;
>  	avd->seqno = latest_granting;
>  
> +	if (!tclass || tclass > policydb.p_classes.nprim) {
> +		if(selinux_allow_unknown && tclass) {

Awkward - split up the original test.

> +			avd->allowed = 0xffffffff;
> +			return 0;
> +		}
> +		else {
> +			return -EINVAL;
> +		}
> +	}
> +	tclass_datum = policydb.class_val_to_struct[tclass - 1];
> +
>  	/*
>  	 * If a specific type enforcement rule was defined for
>  	 * this permission check, then use it.
> @@ -1037,6 +1045,7 @@ int security_change_sid(u32 ssid,
>  static int validate_classes(struct policydb *p)
>  {
>  	int i, j;
> +	int rc = 0;
>  	struct class_datum *cladatum;
>  	struct perm_datum *perdatum;
>  	u32 nprim, tmp, common_pts_len, perm_val, pol_val;
> @@ -1045,6 +1054,19 @@ static int validate_classes(struct policydb *p)
>  	const char *def_class, *def_perm, *pol_class;
>  	struct symtab *perms;
>  
> +	if (selinux_allow_unknown) {
> +		int num_classes;

u32

> +		if (kdefs->cts_len > p->p_classes.nprim)
> +			num_classes = kdefs->cts_len;
> +		else
> +			num_classes = p->p_classes.nprim;
> +		p->undefined_perms = kzalloc(sizeof(u32)*num_classes, GFP_KERNEL);
> +		if (!p->undefined_perms) {
> +			rc = -ENOMEM;
> +			goto out;
> +		}
> +	}
> +
>  	for (i = 1; i < kdefs->cts_len; i++) {
>  		def_class = kdefs->class_to_string[i];
>  		if (i > p->p_classes.nprim) {
> @@ -1058,7 +1080,8 @@ static int validate_classes(struct policydb *p)
>  			printk(KERN_ERR
>  			       "security:  class %d is incorrect, found %s but should be %s\n",
>  			       i, pol_class, def_class);
> -			return -EINVAL;
> +			rc = -EINVAL;
> +			goto out;
>  		}
>  	}
>  	for (i = 0; i < kdefs->av_pts_len; i++) {
> @@ -1076,6 +1099,8 @@ static int validate_classes(struct policydb *p)
>  			printk(KERN_INFO
>  			       "security:  permission %s in class %s not defined in policy\n",
>  			       def_perm, pol_class);
> +			if (selinux_allow_unknown)
> +				p->undefined_perms[class_val] |= perm_val;
>  			continue;
>  		}
>  		perdatum = hashtab_search(perms->table, def_perm);
> @@ -1083,14 +1108,16 @@ static int validate_classes(struct policydb *p)
>  			printk(KERN_ERR
>  			       "security:  permission %s in class %s not found in policy\n",
>  			       def_perm, pol_class);
> -			return -EINVAL;
> +			rc = -EINVAL;
> +			goto out;

I would have thought that this also would have turned into a non-fatal
error if (selinux_allow_unknown).  No?


> @@ -1126,17 +1154,24 @@ static int validate_classes(struct policydb *p)
>  				printk(KERN_ERR
>  				       "security:  permission %s in class %s not found in policy\n",
>  				       def_perm, pol_class);
> -				return -EINVAL;
> +				rc = -EINVAL;
> +				goto out;

Ditto.

> +out:
> +	if (rc && selinux_allow_unknown) {
> +		kfree(p->undefined_perms);
> +		p->undefined_perms = NULL;

Is this necessary - it would be caught when the policydb is destroyed on
the final exit path of the caller, right?  Once we link something into a
policydb, we generally don't free it except upon policydb_destroy.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

On Tue, 2007-02-13 at 07:27 -0500, Stephen Smalley wrote:
> On Mon, 2007-02-12 at 14:43 -0500, Eric Paris wrote:
> > I do still have a problem.  I don't really understand the 'inherits'
> > ideas and so I'm not sure how to pad my p->undefined_perms[tclass] with
> > permissions which are missing in policy due to an incorrect or missing
> > inherits statement.  If this approach seems good and reasonable I will
> > try to better understand how I can glean those bits from
> > validate_classes() and I will also move this to use config flags and
> > have multiple options on how to handle policies.
> 
> Missing or incorrect inherits clause should remain fatal, I'd say - it
> is too substantive a change to the class definition to expect a running
> kernel to adapt.  Classes that inherit a common definition pick up their
> initial set of permission bits from that common definition, such that
> those bits are uniform across those classes and can be checked by a
> single permission check (e.g. for common file or socket checks).  But as
> soon as you remove or change them, in addition to removing all of those
> inherited permissions, you will have disturbed the values of any
> subsequent class-specific permissions.

ATM one of the inherits checking paths is non-fatal in validate_classes
I'll spend some time to understand it and either make it fatal or figure
out how to fill my table correctly

> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index ca9154d..e6a60a5 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -306,22 +306,30 @@ static int context_struct_compute_av(struct context *scontext,
> >  		    tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
> >  			tclass = SECCLASS_NETLINK_SOCKET;
> >  
> > -	if (!tclass || tclass > policydb.p_classes.nprim) {
> > -		printk(KERN_ERR "security_compute_av:  unrecognized class %d\n",
> > -		       tclass);
> > -		return -EINVAL;
> > -	}
> > -	tclass_datum = policydb.class_val_to_struct[tclass - 1];
> > -
> >  	/*
> >  	 * Initialize the access vectors to the default values.
> > +	 * This will pad the allow for undefined perms if appropriete.
> >  	 */
> > -	avd->allowed = 0;
> > +	if(selinux_allow_unknown)
> > +		avd->allowed = policydb.undefined_perms[tclass];
> 
> You moved the bounds checks on tclass down further, but here you use
> tclass as an index.  Also, why [tclass] vs. [tclass-1] as in other code?
> Aren't you wasting undefined_perms[0]?

I'll work a little on the bounds checking but only the it(!tclass) part
is even relevant since the padding array is large enough even if tclass
is greater than the # of kernel classes.  I'll look at tclass vs. tclass
-1.  I'm surprised I didn't get a notice I was writing outside of my
malloc'd area now that I think about it....

> > @@ -1045,6 +1054,19 @@ static int validate_classes(struct policydb *p)
> >  	const char *def_class, *def_perm, *pol_class;
> >  	struct symtab *perms;
> >  
> > +	if (selinux_allow_unknown) {
> > +		int num_classes;
> 
> u32

u16?  But irrelevant. 

> > +		if (kdefs->cts_len > p->p_classes.nprim)
> > +			num_classes = kdefs->cts_len;
> > +		else
> > +			num_classes = p->p_classes.nprim;
> > +		p->undefined_perms = kzalloc(sizeof(u32)*num_classes, GFP_KERNEL);
> > +		if (!p->undefined_perms) {
> > +			rc = -ENOMEM;
> > +			goto out;
> > +		}
> > +	}
> > +
> >  	for (i = 1; i < kdefs->cts_len; i++) {
> >  		def_class = kdefs->class_to_string[i];
> >  		if (i > p->p_classes.nprim) {

This is a vestige of the original av padding where I had to be concerned
that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
only use the array when making permission checks I can assume that I
will never get a class from the check greater than the number of kernel
classes defined.  So all that can be simplified to just
p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);

> > @@ -1083,14 +1108,16 @@ static int validate_classes(struct policydb *p)
> >  			printk(KERN_ERR
> >  			       "security:  permission %s in class %s not found in policy\n",
> >  			       def_perm, pol_class);
> > -			return -EINVAL;
> > +			rc = -EINVAL;
> > +			goto out;
> 
> I would have thought that this also would have turned into a non-fatal
> error if (selinux_allow_unknown).  No?

Yup, can fix.

> > @@ -1126,17 +1154,24 @@ static int validate_classes(struct policydb *p)
> >  				printk(KERN_ERR
> >  				       "security:  permission %s in class %s not found in policy\n",
> >  				       def_perm, pol_class);
> > -				return -EINVAL;
> > +				rc = -EINVAL;
> > +				goto out;
> 
> Ditto.

Part of the inherits stuff that I still don't fully understand.  If made
non-fatal I have to fill my table properly.

> 
> > +out:
> > +	if (rc && selinux_allow_unknown) {
> > +		kfree(p->undefined_perms);
> > +		p->undefined_perms = NULL;
> 
> Is this necessary - it would be caught when the policydb is destroyed on
> the final exit path of the caller, right?  Once we link something into a
> policydb, we generally don't free it except upon policydb_destroy.

Another vestige of original implementation before I realized that I
could (and needed to) carry this information inside the policydb.
Removing this also allows me to revert all of the goto additions.

As I oh so slowly work through this I see that this patch is going to
end up being about 10 lines!!

-Eric


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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> On Tue, 2007-02-13 at 07:27 -0500, Stephen Smalley wrote:
> > You moved the bounds checks on tclass down further, but here you use
> > tclass as an index.  Also, why [tclass] vs. [tclass-1] as in other code?
> > Aren't you wasting undefined_perms[0]?
> 
> I'll work a little on the bounds checking but only the it(!tclass) part
> is even relevant since the padding array is large enough even if tclass
> is greater than the # of kernel classes.

The entire bounds check is either relevant together or not at all; you
either check that the inputs to the function are within the
policy-defined range before using them as indices or you treat that as a
kernel bug (in which case you BUG_ON it).  We have seen cases where
completely out of range classes have been passed in as a result of a
kernel bug elsewhere.

> > > @@ -1045,6 +1054,19 @@ static int validate_classes(struct policydb *p)
> > >  	const char *def_class, *def_perm, *pol_class;
> > >  	struct symtab *perms;
> > >  
> > > +	if (selinux_allow_unknown) {
> > > +		int num_classes;
> > 
> > u32
> 
> u16?  But irrelevant. 

You were assigning u32 values to it (from kdefs->cts_len or
p->p_classes.nprim).

> This is a vestige of the original av padding where I had to be concerned
> that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
> only use the array when making permission checks I can assume that I
> will never get a class from the check greater than the number of kernel
> classes defined.  So all that can be simplified to just
> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);

No - security_compute_av can also be called via selinuxfs to check
userland security classes (from libselinux for userspace object
managers).

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > On Tue, 2007-02-13 at 07:27 -0500, Stephen Smalley wrote:
> > > You moved the bounds checks on tclass down further, but here you use
> > > tclass as an index.  Also, why [tclass] vs. [tclass-1] as in other code?
> > > Aren't you wasting undefined_perms[0]?
> > 
> > I'll work a little on the bounds checking but only the it(!tclass) part
> > is even relevant since the padding array is large enough even if tclass
> > is greater than the # of kernel classes.
> 
> The entire bounds check is either relevant together or not at all; you
> either check that the inputs to the function are within the
> policy-defined range before using them as indices or you treat that as a
> kernel bug (in which case you BUG_ON it).  We have seen cases where
> completely out of range classes have been passed in as a result of a
> kernel bug elsewhere.

Plus, there is the case where this code is called via selinuxfs, so
BUG_ON isn't an option.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > On Tue, 2007-02-13 at 07:27 -0500, Stephen Smalley wrote:
> > > You moved the bounds checks on tclass down further, but here you use
> > > tclass as an index.  Also, why [tclass] vs. [tclass-1] as in other code?
> > > Aren't you wasting undefined_perms[0]?
> > 
> > I'll work a little on the bounds checking but only the it(!tclass) part
> > is even relevant since the padding array is large enough even if tclass
> > is greater than the # of kernel classes.
> 
> The entire bounds check is either relevant together or not at all; you
> either check that the inputs to the function are within the
> policy-defined range before using them as indices or you treat that as a
> kernel bug (in which case you BUG_ON it).  We have seen cases where
> completely out of range classes have been passed in as a result of a
> kernel bug elsewhere.
> 
> > > > @@ -1045,6 +1054,19 @@ static int validate_classes(struct policydb *p)
> > > >  	const char *def_class, *def_perm, *pol_class;
> > > >  	struct symtab *perms;
> > > >  
> > > > +	if (selinux_allow_unknown) {
> > > > +		int num_classes;
> > > 
> > > u32
> > 
> > u16?  But irrelevant. 
> 
> You were assigning u32 values to it (from kdefs->cts_len or
> p->p_classes.nprim).
> 
> > This is a vestige of the original av padding where I had to be concerned
> > that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
> > only use the array when making permission checks I can assume that I
> > will never get a class from the check greater than the number of kernel
> > classes defined.  So all that can be simplified to just
> > p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
> 
> No - security_compute_av can also be called via selinuxfs to check
> userland security classes (from libselinux for userspace object
> managers).

Also, keep in mind that the plan is to strip the userland class and
permission definitions out of the kernel headers so that it stops
validating them and they can change without affecting the kernel.  Just
requires an update to the scripts that generate those headers to split
the output to different sets of headers for kernel vs. libselinux based
on the already existing "userspace" comment marker in the
security_classes file.  That is allegedly already done for refpolicy,
although I don't see it yet in the public svn, at least on the trunk.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Tue, 2007-02-13 at 12:49 -0500, Stephen Smalley wrote:
> On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> > On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > > On Tue, 2007-02-13 at 07:27 -0500, Stephen Smalley wrote:
> > > > You moved the bounds checks on tclass down further, but here you use
> > > > tclass as an index.  Also, why [tclass] vs. [tclass-1] as in other code?
> > > > Aren't you wasting undefined_perms[0]?
> > > 
> > > I'll work a little on the bounds checking but only the it(!tclass) part
> > > is even relevant since the padding array is large enough even if tclass
> > > is greater than the # of kernel classes.
> > 
> > The entire bounds check is either relevant together or not at all; you
> > either check that the inputs to the function are within the
> > policy-defined range before using them as indices or you treat that as a
> > kernel bug (in which case you BUG_ON it).  We have seen cases where
> > completely out of range classes have been passed in as a result of a
> > kernel bug elsewhere.
> > 
> > > > > @@ -1045,6 +1054,19 @@ static int validate_classes(struct policydb *p)
> > > > >  	const char *def_class, *def_perm, *pol_class;
> > > > >  	struct symtab *perms;
> > > > >  
> > > > > +	if (selinux_allow_unknown) {
> > > > > +		int num_classes;
> > > > 
> > > > u32
> > > 
> > > u16?  But irrelevant. 
> > 
> > You were assigning u32 values to it (from kdefs->cts_len or
> > p->p_classes.nprim).
> > 
> > > This is a vestige of the original av padding where I had to be concerned
> > > that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
> > > only use the array when making permission checks I can assume that I
> > > will never get a class from the check greater than the number of kernel
> > > classes defined.  So all that can be simplified to just
> > > p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
> > 
> > No - security_compute_av can also be called via selinuxfs to check
> > userland security classes (from libselinux for userspace object
> > managers).
> 
> Also, keep in mind that the plan is to strip the userland class and
> permission definitions out of the kernel headers so that it stops
> validating them and they can change without affecting the kernel.  Just
> requires an update to the scripts that generate those headers to split
> the output to different sets of headers for kernel vs. libselinux based
> on the already existing "userspace" comment marker in the
> security_classes file.  That is allegedly already done for refpolicy,
> although I don't see it yet in the public svn, at least on the trunk.

Ah, it looks like there is a flask-headers-2121 branch there.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
[snip snip]
> > This is a vestige of the original av padding where I had to be concerned
> > that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
> > only use the array when making permission checks I can assume that I
> > will never get a class from the check greater than the number of kernel
> > classes defined.  So all that can be simplified to just
> > p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
> 
> No - security_compute_av can also be called via selinuxfs to check
> userland security classes (from libselinux for userspace object
> managers).

I'm starting to wonder if the entire way I am constructing and using
using my table might be wrong.  Right now I am finding the permissions
which are defined in the kernel but are not defined in the policy,
storing just those select permissions, and then adding those to the
allow rules.  This works very well when I only consider the kernel and
don't think about things in userspace.

But now I ask the question, do we/I want to extend this methodology to
userspace classes and permissions?  Obviously I wouldn't be able to
build the 'unknown perms' table for userspace since they are completely
unknown!  Would it be better if I changed my approach to

Now:
avd.allowed |= p->undefined_perms[tclass-1]

Possible
avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)

It's not quite as directed.  Instead of only allowing the select
permissions which we know are missing from policy we instead shotgun
approach and allow everything not explicitly defined.  Maybe this allows
buggy code to get allows which would have been caught and things like
that, but at least it could work for userspace classes and perms.  Seems
to me like this undefined perms problem will be worse in userspace than
it is in the kernel.

Thoughts?  Or should I just stick with the ideas behind the patch below?

-Eric

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index cd79c63..364d872 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -670,6 +670,8 @@ void policydb_destroy(struct policydb *p)
 	}
 	kfree(p->type_attr_map);
 
+	kfree(p->undefined_perms);
+
 	return;
 }
 
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 8319d5f..257b836 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -242,6 +242,8 @@ struct policydb {
 	struct ebitmap *type_attr_map;
 
 	unsigned int policyvers;
+
+	u32 *undefined_perms;
 };
 
 extern void policydb_destroy(struct policydb *p);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ca9154d..a511d75 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -306,22 +306,26 @@ static int context_struct_compute_av(struct context *scontext,
 		    tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
 			tclass = SECCLASS_NETLINK_SOCKET;
 
-	if (!tclass || tclass > policydb.p_classes.nprim) {
-		printk(KERN_ERR "security_compute_av:  unrecognized class %d\n",
-		       tclass);
-		return -EINVAL;
-	}
-	tclass_datum = policydb.class_val_to_struct[tclass - 1];
-
 	/*
 	 * Initialize the access vectors to the default values.
+	 * This will pad the allow for undefined perms if appropriete.
 	 */
-	avd->allowed = 0;
 	avd->decided = 0xffffffff;
 	avd->auditallow = 0;
 	avd->auditdeny = 0xffffffff;
 	avd->seqno = latest_granting;
 
+	if (tclass > policydb.p_classes.nprim && selinux_allow_unknwon) {
+		avd->allowed = 0xffffffff;
+		return 0;
+	} else if (!tclass || tclass > policydb.p_classes.nprim)
+		return -EINVAL;
+	else if (selinux_allow_unknown)
+		avd->allowed = policydb.undefined_perms[tclass - 1];
+	else
+		avd->allowed = 0;
+	tclass_datum = policydb.class_val_to_struct[tclass - 1];
+
 	/*
 	 * If a specific type enforcement rule was defined for
 	 * this permission check, then use it.
@@ -1045,6 +1049,17 @@ static int validate_classes(struct policydb *p)
 	const char *def_class, *def_perm, *pol_class;
 	struct symtab *perms;
 
+	if (selinux_allow_unknown) {
+		u32 num_classes;
+		if (kdefs->cts_len > p->p_classes.nprim)
+			num_classes = kdefs->cts_len;
+		else
+			num_classes = p->p_classes.nprim;
+		p->undefined_perms = kzalloc(sizeof(u32)*num_classes, GFP_KERNEL);
+		if (!p->undefined_perms)
+			return -ENOMEM;
+	}
+
 	for (i = 1; i < kdefs->cts_len; i++) {
 		def_class = kdefs->class_to_string[i];
 		if (i > p->p_classes.nprim) {
@@ -1076,14 +1091,18 @@ static int validate_classes(struct policydb *p)
 			printk(KERN_INFO
 			       "security:  permission %s in class %s not defined in policy\n",
 			       def_perm, pol_class);
+			if (selinux_allow_unknown)
+				p->undefined_perms[class_val-1] |= perm_val;
 			continue;
 		}
 		perdatum = hashtab_search(perms->table, def_perm);
 		if (perdatum == NULL) {
-			printk(KERN_ERR
+			printk(KERN_INFO
 			       "security:  permission %s in class %s not found in policy\n",
 			       def_perm, pol_class);
-			return -EINVAL;
+			if (selinux_allow_unknown)
+				p->undefined_perms[class_val-1] |= perm_val;
+			continue;
 		}
 		pol_val = 1 << (perdatum->value - 1);
 		if (pol_val != perm_val) {
@@ -1119,14 +1138,18 @@ static int validate_classes(struct policydb *p)
 				printk(KERN_INFO
 				       "security:  permission %s in class %s not defined in policy\n",
 				       def_perm, pol_class);
+				if (selinux_allow_unknown)
+					p->undefined_perms[class_val-1] |= (1 << j);
 				continue;
 			}
 			perdatum = hashtab_search(perms->table, def_perm);
 			if (perdatum == NULL) {
-				printk(KERN_ERR
+				printk(KERN_INFO
 				       "security:  permission %s in class %s not found in policy\n",
 				       def_perm, pol_class);
-				return -EINVAL;
+				if (selinux_allow_unknown)
+					p->undefined_perms[class_val-1] |= (1 << j);
+				continue;
 			}
 			if (perdatum->value != j + 1) {
 				printk(KERN_ERR
@@ -1268,6 +1291,9 @@ int security_load_policy(void *data, size_t len)
 
 	LOAD_LOCK;
 
+	/* pay not attention to me as I am part of the /selinux madness!! */
+	selinux_allow_unknown = selinux_allow_unknown_sysctl_value;
+
 	if (!ss_initialized) {
 		avtab_cache_init();
 		if (policydb_read(&policydb, fp)) {
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65fb5e8..10af3d0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -86,6 +86,9 @@ extern unsigned int policydb_loaded_version;
 extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
 extern int selinux_compat_net;
 
+int selinux_allow_unknown = 0;
+int selinux_allow_unknown_sysctl_value = 0;
+
 #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
 int selinux_enforcing = 0;
 
diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index 6ed10c3..1b02289 100644
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -18,6 +18,9 @@
 #include "av_permissions.h"
 #include "security.h"
 
+extern int selinux_allow_unknown;
+extern int selinux_allow_unknown_sysctl_value;
+
 #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
 extern int selinux_enforcing;
 #else
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c8bf6e1..67c7810 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -100,6 +100,7 @@ enum sel_inos {
 	SEL_MEMBER,	/* compute polyinstantiation membership decision */
 	SEL_CHECKREQPROT, /* check requested protection, not kernel-applied one */
 	SEL_COMPAT_NET,	/* whether to use old compat network packet controls */
+	SEL_UNKNOWN,	/* allows unknown perms and classes */
 };
 
 #define TMPBUFLEN	12
@@ -113,6 +114,16 @@ static ssize_t sel_read_enforce(struct file *filp, char __user *buf,
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
 }
 
+static ssize_t sel_read_unknown(struct file *filp, char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	char tmpbuf[TMPBUFLEN];
+	ssize_t length;
+
+	length = scnprintf(tmpbuf, TMPBUFLEN, "%d", selinux_allow_unknown_sysctl_value);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
 #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
 static ssize_t sel_write_enforce(struct file * file, const char __user * buf,
 				 size_t count, loff_t *ppos)
@@ -161,11 +172,55 @@ out:
 #define sel_write_enforce NULL
 #endif
 
+static ssize_t sel_write_unknown(struct file * file, const char __user * buf,
+				 size_t count, loff_t *ppos)
+
+{
+	char *page;
+	ssize_t length;
+	int new_value;
+
+	if (count >= PAGE_SIZE)
+		return -ENOMEM;
+	if (*ppos != 0) {
+		/* No partial writes. */
+		return -EINVAL;
+	}
+	page = (char*)get_zeroed_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
+	length = -EFAULT;
+	if (copy_from_user(page, buf, count))
+		goto out;
+
+	length = -EINVAL;
+	if (sscanf(page, "%d", &new_value) != 1)
+		goto out;
+
+	if (new_value != selinux_allow_unknown_sysctl_value) {
+		length = 0;
+		audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_STATUS,
+			"allow_unknown=%d old_allow_unknown%d auid=%u", new_value, 
+			selinux_allow_unknown_sysctl_value,
+			audit_get_loginuid(current->audit_context));
+		selinux_allow_unknown_sysctl_value = new_value;
+	}
+	length = count;
+out:
+	free_page((unsigned long) page);
+	return length;
+}
+
 static struct file_operations sel_enforce_ops = {
 	.read		= sel_read_enforce,
 	.write		= sel_write_enforce,
 };
 
+static struct file_operations sel_unknown_ops = {
+	.read		= sel_read_unknown,
+	.write		= sel_write_unknown,
+};
+
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 static ssize_t sel_write_disable(struct file * file, const char __user * buf,
 				 size_t count, loff_t *ppos)
@@ -1283,6 +1338,7 @@ static int sel_fill_super(struct super_block * sb, void * data, int silent)
 		[SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
 		[SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
 		[SEL_COMPAT_NET] = {"compat_net", &sel_compat_net_ops, S_IRUGO|S_IWUSR},
+		[SEL_UNKNOWN] = {"allow_unknown", &sel_unknown_ops, S_IRUGO|S_IWUSR},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);



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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Chad Sellers]

On 2/13/07 2:36 PM, "Eric Paris" <eparis@redhat.com> wrote:

> On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
>> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> [snip snip]
>>> This is a vestige of the original av padding where I had to be concerned
>>> that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
>>> only use the array when making permission checks I can assume that I
>>> will never get a class from the check greater than the number of kernel
>>> classes defined.  So all that can be simplified to just
>>> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
>> 
>> No - security_compute_av can also be called via selinuxfs to check
>> userland security classes (from libselinux for userspace object
>> managers).
> 
> I'm starting to wonder if the entire way I am constructing and using
> using my table might be wrong.  Right now I am finding the permissions
> which are defined in the kernel but are not defined in the policy,
> storing just those select permissions, and then adding those to the
> allow rules.  This works very well when I only consider the kernel and
> don't think about things in userspace.
> 
> But now I ask the question, do we/I want to extend this methodology to
> userspace classes and permissions?  Obviously I wouldn't be able to
> build the 'unknown perms' table for userspace since they are completely
> unknown!  Would it be better if I changed my approach to
> 
> Now:
> avd.allowed |= p->undefined_perms[tclass-1]
> 
> Possible
> avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)
> 
Don't you still have the same problem of needing to know what classes and
permissions are defined in the userspace object manager? You still have to
fill out defined_perms for these classes (or you'll just return 0xffffffff
for all userspace classes).

> It's not quite as directed.  Instead of only allowing the select
> permissions which we know are missing from policy we instead shotgun
> approach and allow everything not explicitly defined.  Maybe this allows
> buggy code to get allows which would have been caught and things like
> that, but at least it could work for userspace classes and perms.  Seems
> to me like this undefined perms problem will be worse in userspace than
> it is in the kernel.
> 
> Thoughts?  Or should I just stick with the ideas behind the patch below?
> 
> -Eric
> 
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index cd79c63..364d872 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -670,6 +670,8 @@ void policydb_destroy(struct policydb *p)
> }
> kfree(p->type_attr_map);
>  
> + kfree(p->undefined_perms);
> +
> return;
>  }
>  
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 8319d5f..257b836 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -242,6 +242,8 @@ struct policydb {
> struct ebitmap *type_attr_map;
>  
> unsigned int policyvers;
> +
> + u32 *undefined_perms;
>  };
>  
>  extern void policydb_destroy(struct policydb *p);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index ca9154d..a511d75 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -306,22 +306,26 @@ static int context_struct_compute_av(struct context
> *scontext,
>    tclass <= SECCLASS_NETLINK_DNRT_SOCKET)
> tclass = SECCLASS_NETLINK_SOCKET;
>  
> - if (!tclass || tclass > policydb.p_classes.nprim) {
> -  printk(KERN_ERR "security_compute_av:  unrecognized class %d\n",
> -         tclass);
> -  return -EINVAL;
> - }
> - tclass_datum = policydb.class_val_to_struct[tclass - 1];
> -
> /*
> * Initialize the access vectors to the default values.
> +  * This will pad the allow for undefined perms if appropriete.
> */
> - avd->allowed = 0;
> avd->decided = 0xffffffff;
> avd->auditallow = 0;
> avd->auditdeny = 0xffffffff;
> avd->seqno = latest_granting;
>  
> + if (tclass > policydb.p_classes.nprim && selinux_allow_unknwon) {
> +  avd->allowed = 0xffffffff;
> +  return 0;
> + } else if (!tclass || tclass > policydb.p_classes.nprim)
> +  return -EINVAL;
> + else if (selinux_allow_unknown)
> +  avd->allowed = policydb.undefined_perms[tclass - 1];
> + else
> +  avd->allowed = 0;
> + tclass_datum = policydb.class_val_to_struct[tclass - 1];
> +
> /*
> * If a specific type enforcement rule was defined for
> * this permission check, then use it.
> @@ -1045,6 +1049,17 @@ static int validate_classes(struct policydb *p)
> const char *def_class, *def_perm, *pol_class;
> struct symtab *perms;
>  
> + if (selinux_allow_unknown) {
> +  u32 num_classes;
> +  if (kdefs->cts_len > p->p_classes.nprim)
> +   num_classes = kdefs->cts_len;
> +  else
> +   num_classes = p->p_classes.nprim;
> +  p->undefined_perms = kzalloc(sizeof(u32)*num_classes, GFP_KERNEL);
> +  if (!p->undefined_perms)
> +   return -ENOMEM;
> + }
> +
> for (i = 1; i < kdefs->cts_len; i++) {
> def_class = kdefs->class_to_string[i];
> if (i > p->p_classes.nprim) {
> @@ -1076,14 +1091,18 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
>       "security:  permission %s in class %s not defined in policy\n",
>       def_perm, pol_class);
> +   if (selinux_allow_unknown)
> +    p->undefined_perms[class_val-1] |= perm_val;
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> -   printk(KERN_ERR
> +   printk(KERN_INFO
>       "security:  permission %s in class %s not found in policy\n",
>       def_perm, pol_class);
> -   return -EINVAL;
> +   if (selinux_allow_unknown)
> +    p->undefined_perms[class_val-1] |= perm_val;
> +   continue;
> }
> pol_val = 1 << (perdatum->value - 1);
> if (pol_val != perm_val) {
> @@ -1119,14 +1138,18 @@ static int validate_classes(struct policydb *p)
> printk(KERN_INFO
>       "security:  permission %s in class %s not defined in policy\n",
>       def_perm, pol_class);
> +    if (selinux_allow_unknown)
> +     p->undefined_perms[class_val-1] |= (1 << j);
> continue;
> }
> perdatum = hashtab_search(perms->table, def_perm);
> if (perdatum == NULL) {
> -    printk(KERN_ERR
> +    printk(KERN_INFO
>       "security:  permission %s in class %s not found in policy\n",
>       def_perm, pol_class);
> -    return -EINVAL;
> +    if (selinux_allow_unknown)
> +     p->undefined_perms[class_val-1] |= (1 << j);
> +    continue;
> }
> if (perdatum->value != j + 1) {
> printk(KERN_ERR
> @@ -1268,6 +1291,9 @@ int security_load_policy(void *data, size_t len)
>  
> LOAD_LOCK;
>  
> + /* pay not attention to me as I am part of the /selinux madness!! */
> + selinux_allow_unknown = selinux_allow_unknown_sysctl_value;
> +
> if (!ss_initialized) {
> avtab_cache_init();
> if (policydb_read(&policydb, fp)) {
> diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 65fb5e8..10af3d0 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -86,6 +86,9 @@ extern unsigned int policydb_loaded_version;
>  extern int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm);
>  extern int selinux_compat_net;
>  
> +int selinux_allow_unknown = 0;
> +int selinux_allow_unknown_sysctl_value = 0;
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
>  int selinux_enforcing = 0;
>  
> diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
> index 6ed10c3..1b02289 100644
> --- a/security/selinux/include/avc.h
> +++ b/security/selinux/include/avc.h
> @@ -18,6 +18,9 @@
>  #include "av_permissions.h"
>  #include "security.h"
>  
> +extern int selinux_allow_unknown;
> +extern int selinux_allow_unknown_sysctl_value;
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
>  extern int selinux_enforcing;
>  #else
> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index c8bf6e1..67c7810 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -100,6 +100,7 @@ enum sel_inos {
> SEL_MEMBER, /* compute polyinstantiation membership decision */
> SEL_CHECKREQPROT, /* check requested protection, not kernel-applied one */
> SEL_COMPAT_NET, /* whether to use old compat network packet controls */
> + SEL_UNKNOWN, /* allows unknown perms and classes */
>  };
>  
>  #define TMPBUFLEN 12
> @@ -113,6 +114,16 @@ static ssize_t sel_read_enforce(struct file *filp, char
> __user *buf,
> return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
>  }
>  
> +static ssize_t sel_read_unknown(struct file *filp, char __user *buf,
> +    size_t count, loff_t *ppos)
> +{
> + char tmpbuf[TMPBUFLEN];
> + ssize_t length;
> +
> + length = scnprintf(tmpbuf, TMPBUFLEN, "%d",
> selinux_allow_unknown_sysctl_value);
> + return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
> +}
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
>  static ssize_t sel_write_enforce(struct file * file, const char __user * buf,
> size_t count, loff_t *ppos)
> @@ -161,11 +172,55 @@ out:
>  #define sel_write_enforce NULL
>  #endif
>  
> +static ssize_t sel_write_unknown(struct file * file, const char __user * buf,
> +     size_t count, loff_t *ppos)
> +
> +{
> + char *page;
> + ssize_t length;
> + int new_value;
> +
> + if (count >= PAGE_SIZE)
> +  return -ENOMEM;
> + if (*ppos != 0) {
> +  /* No partial writes. */
> +  return -EINVAL;
> + }
> + page = (char*)get_zeroed_page(GFP_KERNEL);
> + if (!page)
> +  return -ENOMEM;
> + length = -EFAULT;
> + if (copy_from_user(page, buf, count))
> +  goto out;
> +
> + length = -EINVAL;
> + if (sscanf(page, "%d", &new_value) != 1)
> +  goto out;
> +
> + if (new_value != selinux_allow_unknown_sysctl_value) {
> +  length = 0;
> +  audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_STATUS,
> +   "allow_unknown=%d old_allow_unknown%d auid=%u", new_value,
> +   selinux_allow_unknown_sysctl_value,
> +   audit_get_loginuid(current->audit_context));
> +  selinux_allow_unknown_sysctl_value = new_value;
> + }
> + length = count;
> +out:
> + free_page((unsigned long) page);
> + return length;
> +}
> +
>  static struct file_operations sel_enforce_ops = {
> .read  = sel_read_enforce,
> .write  = sel_write_enforce,
>  };
>  
> +static struct file_operations sel_unknown_ops = {
> + .read  = sel_read_unknown,
> + .write  = sel_write_unknown,
> +};
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DISABLE
>  static ssize_t sel_write_disable(struct file * file, const char __user * buf,
> size_t count, loff_t *ppos)
> @@ -1283,6 +1338,7 @@ static int sel_fill_super(struct super_block * sb, void
> * data, int silent)
> [SEL_MEMBER] = {"member", &transaction_ops, S_IRUGO|S_IWUGO},
> [SEL_CHECKREQPROT] = {"checkreqprot", &sel_checkreqprot_ops, S_IRUGO|S_IWUSR},
> [SEL_COMPAT_NET] = {"compat_net", &sel_compat_net_ops, S_IRUGO|S_IWUSR},
> +  [SEL_UNKNOWN] = {"allow_unknown", &sel_unknown_ops, S_IRUGO|S_IWUSR},
> /* last one */ {""}
> };
> ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
> 
> 
> 
> --
> 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.


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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

On Wed, 2007-02-14 at 13:10 -0500, Chad Sellers wrote:
> On 2/13/07 2:36 PM, "Eric Paris" <eparis@redhat.com> wrote:
> 
> > On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> >> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > [snip snip]
> >>> This is a vestige of the original av padding where I had to be concerned
> >>> that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
> >>> only use the array when making permission checks I can assume that I
> >>> will never get a class from the check greater than the number of kernel
> >>> classes defined.  So all that can be simplified to just
> >>> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
> >> 
> >> No - security_compute_av can also be called via selinuxfs to check
> >> userland security classes (from libselinux for userspace object
> >> managers).
> > 
> > I'm starting to wonder if the entire way I am constructing and using
> > using my table might be wrong.  Right now I am finding the permissions
> > which are defined in the kernel but are not defined in the policy,
> > storing just those select permissions, and then adding those to the
> > allow rules.  This works very well when I only consider the kernel and
> > don't think about things in userspace.
> > 
> > But now I ask the question, do we/I want to extend this methodology to
> > userspace classes and permissions?  Obviously I wouldn't be able to
> > build the 'unknown perms' table for userspace since they are completely
> > unknown!  Would it be better if I changed my approach to
> > 
> > Now:
> > avd.allowed |= p->undefined_perms[tclass-1]
> > 
> > Possible
> > avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)
> > 
> Don't you still have the same problem of needing to know what classes and
> permissions are defined in the userspace object manager? You still have to
> fill out defined_perms for these classes (or you'll just return 0xffffffff
> for all userspace classes).

That is my point of question and lack of understanding.  Will the policy
which is passed to the kernel have all of the policy for all of the
system or only that for the kernel?  The way I see both cases:

All of the policy for the whole system:
- My ^ 0xffffffff method will work just fine since the policy knows what
is defined by policy and we can simply allow everything else.
- This would be the only reasonable way the kernel could answer
through /selinux for userapace objects.  If the kernel doesn't know
about the userspace objects or permissions how can it answer yes or no
with any meaning?

Only the kernel policy is loaded into the kernel:
- Either method works fine although the more directed method might help
to catch accidental bugs in the kernel which would be missed by the
shotgun method.
- Obviously userspace can't ask the kernel to make decisions since it
doesn't know, so it seems I can be directed or just allow everything.

So really I just need some ejumakashun on the mythic userspace security
server and just what information will be available to the kernel when it
is passed the policy.  And just what decision context_struct_compute_av
might be ask to decide.

-Eric


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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Joshua Brindle]

> From: Eric Paris [mailto:eparis@redhat.com] 
> 
> On Wed, 2007-02-14 at 13:10 -0500, Chad Sellers wrote:
> > On 2/13/07 2:36 PM, "Eric Paris" <eparis@redhat.com> wrote:
> > 
> > > On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> > >> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > > [snip snip]
> > >>> This is a vestige of the original av padding where I had to be 
> > >>> concerned that p->p_classes.nprim was larger than 
> kdefs->cts_len.  
> > >>> Since now I only use the array when making permission 
> checks I can 
> > >>> assume that I will never get a class from the check 
> greater than 
> > >>> the number of kernel classes defined.  So all that can be 
> > >>> simplified to just
> > >>> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, 
> > >>> p->GFP_KERNEL);
> > >> 
> > >> No - security_compute_av can also be called via 
> selinuxfs to check 
> > >> userland security classes (from libselinux for userspace object 
> > >> managers).
> > > 
> > > I'm starting to wonder if the entire way I am 
> constructing and using 
> > > using my table might be wrong.  Right now I am finding the 
> > > permissions which are defined in the kernel but are not 
> defined in 
> > > the policy, storing just those select permissions, and 
> then adding 
> > > those to the allow rules.  This works very well when I 
> only consider 
> > > the kernel and don't think about things in userspace.
> > > 
> > > But now I ask the question, do we/I want to extend this 
> methodology 
> > > to userspace classes and permissions?  Obviously I 
> wouldn't be able 
> > > to build the 'unknown perms' table for userspace since they are 
> > > completely unknown!  Would it be better if I changed my 
> approach to
> > > 
> > > Now:
> > > avd.allowed |= p->undefined_perms[tclass-1]
> > > 
> > > Possible
> > > avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)
> > > 
> > Don't you still have the same problem of needing to know 
> what classes 
> > and permissions are defined in the userspace object 
> manager? You still 
> > have to fill out defined_perms for these classes (or you'll just 
> > return 0xffffffff for all userspace classes).
> 
> That is my point of question and lack of understanding.  Will 
> the policy which is passed to the kernel have all of the 
> policy for all of the system or only that for the kernel?  
> The way I see both cases:
> 

Only for the kernel.

> All of the policy for the whole system:
> - My ^ 0xffffffff method will work just fine since the policy 
> knows what is defined by policy and we can simply allow 
> everything else.
> - This would be the only reasonable way the kernel could 
> answer through /selinux for userapace objects.  If the kernel 
> doesn't know about the userspace objects or permissions how 
> can it answer yes or no with any meaning?
> 

It answers no if it doesn't know about it because selinux is deny by
default, no means it isn't allowed.

> Only the kernel policy is loaded into the kernel:
> - Either method works fine although the more directed method 
> might help to catch accidental bugs in the kernel which would 
> be missed by the shotgun method.
> - Obviously userspace can't ask the kernel to make decisions 
> since it doesn't know, so it seems I can be directed or just 
> allow everything.
> 

No, if userspace does manage to ask the kernel about something (on
accident or otherwise) it must be denied by default, SELinux is a deny
by default mechanism. The routing mechanism is in userland (in
libselinux) so the kernel doesn't know to point the requester somewhere
else for an answer, it must simply answer denied.

> So really I just need some ejumakashun on the mythic 
> userspace security server and just what information will be 
> available to the kernel when it is passed the policy.  And 
> just what decision context_struct_compute_av might be ask to decide.
> 

We haven't totally decided but I think its safe to assume that the
kernel won't have policy for userspace object classes at all and may not
even know of the existance of said classes. The userspace requests would
be routed to a userspace security server and the kernel would only
answer questions for object classes it controls (or maybe not even
those).


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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> > From: Eric Paris [mailto:eparis@redhat.com] 
> > All of the policy for the whole system:
> > - My ^ 0xffffffff method will work just fine since the policy 
> > knows what is defined by policy and we can simply allow 
> > everything else.
> > - This would be the only reasonable way the kernel could 
> > answer through /selinux for userapace objects.  If the kernel 
> > doesn't know about the userspace objects or permissions how 
> > can it answer yes or no with any meaning?
> > 
> 
> It answers no if it doesn't know about it because selinux is deny by
> default, no means it isn't allowed.

The whole point of this exercise is to make it possible for the policy
author to make that rule a little less rigid.  But your point is taken
and I think we can make what you want work.

> > Only the kernel policy is loaded into the kernel:
> > - Either method works fine although the more directed method 
> > might help to catch accidental bugs in the kernel which would 
> > be missed by the shotgun method.
> > - Obviously userspace can't ask the kernel to make decisions 
> > since it doesn't know, so it seems I can be directed or just 
> > allow everything.
> > 
> 
> No, if userspace does manage to ask the kernel about something (on
> accident or otherwise) it must be denied by default, SELinux is a deny
> by default mechanism. The routing mechanism is in userland (in
> libselinux) so the kernel doesn't know to point the requester somewhere
> else for an answer, it must simply answer denied.

Hmmmmm, right now I say that any tclass in an access decision which is
larger than the highest class defined in policy is 'unknown' and so I
allow it.  If I want to try to make this methodology even more kernel
specific I will leave my current permissions table and I guess I also
need to define a bit map and fill it during validate_classes based on
the appearance of the class both in the loaded policy and in the kernel
pre-definitions...

> > So really I just need some ejumakashun on the mythic 
> > userspace security server and just what information will be 
> > available to the kernel when it is passed the policy.  And 
> > just what decision context_struct_compute_av might be ask to decide.
> > 
> 
> We haven't totally decided but I think its safe to assume that the
> kernel won't have policy for userspace object classes at all and may not
> even know of the existance of said classes. The userspace requests would
> be routed to a userspace security server and the kernel would only
> answer questions for object classes it controls (or maybe not even
> those).

Super kernel specific work coming up.

-Eric


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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> > From: Eric Paris [mailto:eparis@redhat.com] 
> > 
> > On Wed, 2007-02-14 at 13:10 -0500, Chad Sellers wrote:
> > > On 2/13/07 2:36 PM, "Eric Paris" <eparis@redhat.com> wrote:
> > > 
> > > > On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> > > >> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > > > [snip snip]
> > > >>> This is a vestige of the original av padding where I had to be 
> > > >>> concerned that p->p_classes.nprim was larger than 
> > kdefs->cts_len.  
> > > >>> Since now I only use the array when making permission 
> > checks I can 
> > > >>> assume that I will never get a class from the check 
> > greater than 
> > > >>> the number of kernel classes defined.  So all that can be 
> > > >>> simplified to just
> > > >>> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, 
> > > >>> p->GFP_KERNEL);
> > > >> 
> > > >> No - security_compute_av can also be called via 
> > selinuxfs to check 
> > > >> userland security classes (from libselinux for userspace object 
> > > >> managers).
> > > > 
> > > > I'm starting to wonder if the entire way I am 
> > constructing and using 
> > > > using my table might be wrong.  Right now I am finding the 
> > > > permissions which are defined in the kernel but are not 
> > defined in 
> > > > the policy, storing just those select permissions, and 
> > then adding 
> > > > those to the allow rules.  This works very well when I 
> > only consider 
> > > > the kernel and don't think about things in userspace.
> > > > 
> > > > But now I ask the question, do we/I want to extend this 
> > methodology 
> > > > to userspace classes and permissions?  Obviously I 
> > wouldn't be able 
> > > > to build the 'unknown perms' table for userspace since they are 
> > > > completely unknown!  Would it be better if I changed my 
> > approach to
> > > > 
> > > > Now:
> > > > avd.allowed |= p->undefined_perms[tclass-1]
> > > > 
> > > > Possible
> > > > avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)
> > > > 
> > > Don't you still have the same problem of needing to know 
> > what classes 
> > > and permissions are defined in the userspace object 
> > manager? You still 
> > > have to fill out defined_perms for these classes (or you'll just 
> > > return 0xffffffff for all userspace classes).
> > 
> > That is my point of question and lack of understanding.  Will 
> > the policy which is passed to the kernel have all of the 
> > policy for all of the system or only that for the kernel?  
> > The way I see both cases:
> > 
> 
> Only for the kernel.

Actually, I thought you had found that the userspace security server
isn't much of a win (and a definite cost) for current usage by e.g. the
X server.  Which would suggest that we aren't likely to evict userspace
policy from the kernel any time soon.

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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Thu, 2007-02-15 at 13:33 -0500, Stephen Smalley wrote:
> On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> > > From: Eric Paris [mailto:eparis@redhat.com] 
> > > 
> > > On Wed, 2007-02-14 at 13:10 -0500, Chad Sellers wrote:
> > > > On 2/13/07 2:36 PM, "Eric Paris" <eparis@redhat.com> wrote:
> > > > 
> > > > > On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> > > > >> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> > > > > [snip snip]
> > > > >>> This is a vestige of the original av padding where I had to be 
> > > > >>> concerned that p->p_classes.nprim was larger than 
> > > kdefs->cts_len.  
> > > > >>> Since now I only use the array when making permission 
> > > checks I can 
> > > > >>> assume that I will never get a class from the check 
> > > greater than 
> > > > >>> the number of kernel classes defined.  So all that can be 
> > > > >>> simplified to just
> > > > >>> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, 
> > > > >>> p->GFP_KERNEL);
> > > > >> 
> > > > >> No - security_compute_av can also be called via 
> > > selinuxfs to check 
> > > > >> userland security classes (from libselinux for userspace object 
> > > > >> managers).
> > > > > 
> > > > > I'm starting to wonder if the entire way I am 
> > > constructing and using 
> > > > > using my table might be wrong.  Right now I am finding the 
> > > > > permissions which are defined in the kernel but are not 
> > > defined in 
> > > > > the policy, storing just those select permissions, and 
> > > then adding 
> > > > > those to the allow rules.  This works very well when I 
> > > only consider 
> > > > > the kernel and don't think about things in userspace.
> > > > > 
> > > > > But now I ask the question, do we/I want to extend this 
> > > methodology 
> > > > > to userspace classes and permissions?  Obviously I 
> > > wouldn't be able 
> > > > > to build the 'unknown perms' table for userspace since they are 
> > > > > completely unknown!  Would it be better if I changed my 
> > > approach to
> > > > > 
> > > > > Now:
> > > > > avd.allowed |= p->undefined_perms[tclass-1]
> > > > > 
> > > > > Possible
> > > > > avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)
> > > > > 
> > > > Don't you still have the same problem of needing to know 
> > > what classes 
> > > > and permissions are defined in the userspace object 
> > > manager? You still 
> > > > have to fill out defined_perms for these classes (or you'll just 
> > > > return 0xffffffff for all userspace classes).
> > > 
> > > That is my point of question and lack of understanding.  Will 
> > > the policy which is passed to the kernel have all of the 
> > > policy for all of the system or only that for the kernel?  
> > > The way I see both cases:
> > > 
> > 
> > Only for the kernel.
> 
> Actually, I thought you had found that the userspace security server
> isn't much of a win (and a definite cost) for current usage by e.g. the
> X server.  Which would suggest that we aren't likely to evict userspace
> policy from the kernel any time soon.

Just to clarify, I only mean the policy itself - the symbol definitions
for userspace classes and permissions should be removed from the kernel
headers ASAP, as it never needs to refer to them and we don't want to
impose validity checks on them within the kernel.

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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

On Thu, 2007-02-15 at 13:46 -0500, Stephen Smalley wrote:
> On Thu, 2007-02-15 at 13:33 -0500, Stephen Smalley wrote:
> > On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> > > > From: Eric Paris [mailto:eparis@redhat.com] 

> > > > That is my point of question and lack of understanding.  Will 
> > > > the policy which is passed to the kernel have all of the 
> > > > policy for all of the system or only that for the kernel?  
> > > > The way I see both cases:
> > > > 
> > > 
> > > Only for the kernel.
> > 
> > Actually, I thought you had found that the userspace security server
> > isn't much of a win (and a definite cost) for current usage by e.g. the
> > X server.  Which would suggest that we aren't likely to evict userspace
> > policy from the kernel any time soon.
> 
> Just to clarify, I only mean the policy itself - the symbol definitions
> for userspace classes and permissions should be removed from the kernel
> headers ASAP, as it never needs to refer to them and we don't want to
> impose validity checks on them within the kernel.

Ok so then I'm a little confused about the best way to proceed.  My
current method determines 'unknown' by walking the kernel header
definitions and then finding the differences in the policy definitions.
If I continue to use that method all kernel security decisions will be
affected by the new allow unknown stuff and that makes me happy.  But
all userspace decisions won't be affected at all.  A security decision
based on a class not defined in policy and not defined in the kernel
headers will be 'denied.'  Same goes for a permission check not defined
in policy or in the kernel headers.  Is this the desired behavior?  Or
should unknown be 'everything the policy doesn't know irregardless of
the kernel headers?'

Basically, should I walk the policy and find out everything it knows
nothing about or should I continue to just walk the kernel headers and
and only allow 'unknown' things that aren't quite the same between the
kernel headers and the policy?

-Eric


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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Chad Sellers]

On 2/15/07 2:05 PM, "Eric Paris" <eparis@redhat.com> wrote:

> On Thu, 2007-02-15 at 13:46 -0500, Stephen Smalley wrote:
>> On Thu, 2007-02-15 at 13:33 -0500, Stephen Smalley wrote:
>>> On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
>>>>> From: Eric Paris [mailto:eparis@redhat.com]
> 
>>>>> That is my point of question and lack of understanding.  Will
>>>>> the policy which is passed to the kernel have all of the
>>>>> policy for all of the system or only that for the kernel?
>>>>> The way I see both cases:
>>>>> 
>>>> 
>>>> Only for the kernel.
>>> 
>>> Actually, I thought you had found that the userspace security server
>>> isn't much of a win (and a definite cost) for current usage by e.g. the
>>> X server.  Which would suggest that we aren't likely to evict userspace
>>> policy from the kernel any time soon.
>> 
>> Just to clarify, I only mean the policy itself - the symbol definitions
>> for userspace classes and permissions should be removed from the kernel
>> headers ASAP, as it never needs to refer to them and we don't want to
>> impose validity checks on them within the kernel.
> 
> Ok so then I'm a little confused about the best way to proceed.  My
> current method determines 'unknown' by walking the kernel header
> definitions and then finding the differences in the policy definitions.
> If I continue to use that method all kernel security decisions will be
> affected by the new allow unknown stuff and that makes me happy.  But
> all userspace decisions won't be affected at all.  A security decision
> based on a class not defined in policy and not defined in the kernel
> headers will be 'denied.'  Same goes for a permission check not defined
> in policy or in the kernel headers.  Is this the desired behavior?  Or
> should unknown be 'everything the policy doesn't know irregardless of
> the kernel headers?'
> 
> Basically, should I walk the policy and find out everything it knows
> nothing about or should I continue to just walk the kernel headers and
> and only allow 'unknown' things that aren't quite the same between the
> kernel headers and the policy?
> 
Continue to walk the kernel headers. This way you'll allow permission checks
for things the kernel thinks should be there but aren't. Userspace will get
denials for missing things, as the kernel headers won't have userspace
permissions any more.

Plus I'm not sure how you would "walk the policy and find out everything it
knows nothing about," given that you have no way of knowing what it should
know about other than the kernel headers.

Chad Sellers 


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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Thu, 2007-02-15 at 14:05 -0500, Eric Paris wrote:
> On Thu, 2007-02-15 at 13:46 -0500, Stephen Smalley wrote:
> > On Thu, 2007-02-15 at 13:33 -0500, Stephen Smalley wrote:
> > > On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> > > > > From: Eric Paris [mailto:eparis@redhat.com] 
> 
> > > > > That is my point of question and lack of understanding.  Will 
> > > > > the policy which is passed to the kernel have all of the 
> > > > > policy for all of the system or only that for the kernel?  
> > > > > The way I see both cases:
> > > > > 
> > > > 
> > > > Only for the kernel.
> > > 
> > > Actually, I thought you had found that the userspace security server
> > > isn't much of a win (and a definite cost) for current usage by e.g. the
> > > X server.  Which would suggest that we aren't likely to evict userspace
> > > policy from the kernel any time soon.
> > 
> > Just to clarify, I only mean the policy itself - the symbol definitions
> > for userspace classes and permissions should be removed from the kernel
> > headers ASAP, as it never needs to refer to them and we don't want to
> > impose validity checks on them within the kernel.
> 
> Ok so then I'm a little confused about the best way to proceed.  My
> current method determines 'unknown' by walking the kernel header
> definitions and then finding the differences in the policy definitions.
> If I continue to use that method all kernel security decisions will be
> affected by the new allow unknown stuff and that makes me happy.  But
> all userspace decisions won't be affected at all.  A security decision
> based on a class not defined in policy and not defined in the kernel
> headers will be 'denied.'  Same goes for a permission check not defined
> in policy or in the kernel headers.  Is this the desired behavior?  Or
> should unknown be 'everything the policy doesn't know irregardless of
> the kernel headers?'

I think we'd need to provide some similar logic in the userspace object
managers for handling unknown userspace classes and permissions that
they use.  Only problem there of course is that presently they don't
ever see the full policy, so they can't build the table in the same way.
  
> Basically, should I walk the policy and find out everything it knows
> nothing about or should I continue to just walk the kernel headers and
> and only allow 'unknown' things that aren't quite the same between the
> kernel headers and the policy?

I'd stay with using the kernel headers as the basis.  And I think you'll
want to disable this allow_unknown behavior when called from selinuxfs,
so create a separate security_compute_av_user() function to be called by
selinuxfs, and pass a flag down to the internal helper.

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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Thu, 2007-02-15 at 14:27 -0500, Stephen Smalley wrote:
> On Thu, 2007-02-15 at 14:05 -0500, Eric Paris wrote:
> > On Thu, 2007-02-15 at 13:46 -0500, Stephen Smalley wrote:
> > > On Thu, 2007-02-15 at 13:33 -0500, Stephen Smalley wrote:
> > > > On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> > > > > > From: Eric Paris [mailto:eparis@redhat.com] 
> > 
> > > > > > That is my point of question and lack of understanding.  Will 
> > > > > > the policy which is passed to the kernel have all of the 
> > > > > > policy for all of the system or only that for the kernel?  
> > > > > > The way I see both cases:
> > > > > > 
> > > > > 
> > > > > Only for the kernel.
> > > > 
> > > > Actually, I thought you had found that the userspace security server
> > > > isn't much of a win (and a definite cost) for current usage by e.g. the
> > > > X server.  Which would suggest that we aren't likely to evict userspace
> > > > policy from the kernel any time soon.
> > > 
> > > Just to clarify, I only mean the policy itself - the symbol definitions
> > > for userspace classes and permissions should be removed from the kernel
> > > headers ASAP, as it never needs to refer to them and we don't want to
> > > impose validity checks on them within the kernel.
> > 
> > Ok so then I'm a little confused about the best way to proceed.  My
> > current method determines 'unknown' by walking the kernel header
> > definitions and then finding the differences in the policy definitions.
> > If I continue to use that method all kernel security decisions will be
> > affected by the new allow unknown stuff and that makes me happy.  But
> > all userspace decisions won't be affected at all.  A security decision
> > based on a class not defined in policy and not defined in the kernel
> > headers will be 'denied.'  Same goes for a permission check not defined
> > in policy or in the kernel headers.  Is this the desired behavior?  Or
> > should unknown be 'everything the policy doesn't know irregardless of
> > the kernel headers?'
> 
> I think we'd need to provide some similar logic in the userspace object
> managers for handling unknown userspace classes and permissions that
> they use.  Only problem there of course is that presently they don't
> ever see the full policy, so they can't build the table in the same way.

But that isn't too different than the dynamic discovery of object
classes problem, previously discussed, so if we solve that one (either
via exporting class/perm info via selinuxfs or via an auxiliary file
generated from policy that is readable by all object managers), then
that should be sufficient.

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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Eric Paris]

On Thu, 2007-02-15 at 14:27 -0500, Stephen Smalley wrote:
> On Thu, 2007-02-15 at 14:05 -0500, Eric Paris wrote:

>   
> > Basically, should I walk the policy and find out everything it knows
> > nothing about or should I continue to just walk the kernel headers and
> > and only allow 'unknown' things that aren't quite the same between the
> > kernel headers and the policy?
> 
> I'd stay with using the kernel headers as the basis.  And I think you'll
> want to disable this allow_unknown behavior when called from selinuxfs,
> so create a separate security_compute_av_user() function to be called by
> selinuxfs, and pass a flag down to the internal helper.
> 

Sounds good, will do.  I'll admit though i'm surprised the selinuxfs
access interface uses security_compute_av rather than
avc_has_perm_noaudit.  Out of curiosity what is the reason for not
wanting to cache those decisions?

-Eric


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

RE: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Thu, 2007-02-15 at 14:44 -0500, Eric Paris wrote:
> On Thu, 2007-02-15 at 14:27 -0500, Stephen Smalley wrote:
> > On Thu, 2007-02-15 at 14:05 -0500, Eric Paris wrote:
> 
> >   
> > > Basically, should I walk the policy and find out everything it knows
> > > nothing about or should I continue to just walk the kernel headers and
> > > and only allow 'unknown' things that aren't quite the same between the
> > > kernel headers and the policy?
> > 
> > I'd stay with using the kernel headers as the basis.  And I think you'll
> > want to disable this allow_unknown behavior when called from selinuxfs,
> > so create a separate security_compute_av_user() function to be called by
> > selinuxfs, and pass a flag down to the internal helper.
> > 
> 
> Sounds good, will do.  I'll admit though i'm surprised the selinuxfs
> access interface uses security_compute_av rather than
> avc_has_perm_noaudit.  Out of curiosity what is the reason for not
> wanting to cache those decisions?

It doesn't make any sense to cache them in the kernel, as 1) you have
already paid the cost of calling the kernel, and 2) the kernel itself
won't use them so you are polluting its cache.  They are cached by the
userspace AVC in libselinux.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Karl MacMillan]

Stephen Smalley wrote:
> On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
>>> From: Eric Paris [mailto:eparis@redhat.com] 
>>> That is my point of question and lack of understanding.  Will 
>>> the policy which is passed to the kernel have all of the 
>>> policy for all of the system or only that for the kernel?  
>>> The way I see both cases:
>>>
>> Only for the kernel.
> 
> Actually, I thought you had found that the userspace security server
> isn't much of a win (and a definite cost) for current usage by e.g. the
> X server.  Which would suggest that we aren't likely to evict userspace
> policy from the kernel any time soon.
> 

And the cost is not just performance (which is a large cost - the number 
of context switches will always make it lose). There are:

- Problems synchronizing policy change (which is particularly bad when 
contexts are invalidated). This includes boolean changes.

- Increased user-visible complexity in libselinux (including a new 
configuration file for routing).

- Potentially overlapping object classes (e.g., for something like 
samba, should answers about "file" come from the kernel or the userspace 
security server).

- Potential for the death of the userspace security server (including 
things like the OOM killer) or extreme performance problems under heavy 
swapping.

The only potential win is reduced kernel memory usage for policy, but 
the total memory footprint for kernel + userspace security server is 
likely much higher and it is not clear that the ability to swap portions 
of the userspace security server is desirable. I think we will get 
further by reducing the kernel policy memory footprint.

Even when you want to disable kernel enforcement for some object classes 
I think that the userspace security server loses (this is something I 
have suggested to allow userspace apps like DBUS to rely entirely on 
SELinux policy instead of SELinux + custom security policy). In those 
scenarios you are likely still going to want labeling to happen for 
processes, files, etc. even if many of the access checks are disabled. 
That means there will still need to be a policy loaded into the kernel.

I don't see any compelling use for the userspace security server and 
don't think we should make kernel changes with the assumption that it 
will be present in the future.

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.

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Thu, 2007-02-15 at 14:03 -0500, Karl MacMillan wrote:
> Stephen Smalley wrote:
> > On Wed, 2007-02-14 at 14:22 -0500, Joshua Brindle wrote:
> >>> From: Eric Paris [mailto:eparis@redhat.com] 
> >>> That is my point of question and lack of understanding.  Will 
> >>> the policy which is passed to the kernel have all of the 
> >>> policy for all of the system or only that for the kernel?  
> >>> The way I see both cases:
> >>>
> >> Only for the kernel.
> > 
> > Actually, I thought you had found that the userspace security server
> > isn't much of a win (and a definite cost) for current usage by e.g. the
> > X server.  Which would suggest that we aren't likely to evict userspace
> > policy from the kernel any time soon.
> > 
> 
> And the cost is not just performance (which is a large cost - the number 
> of context switches will always make it lose). There are:
> 
> - Problems synchronizing policy change (which is particularly bad when 
> contexts are invalidated). This includes boolean changes.
> 
> - Increased user-visible complexity in libselinux (including a new 
> configuration file for routing).
> 
> - Potentially overlapping object classes (e.g., for something like 
> samba, should answers about "file" come from the kernel or the userspace 
> security server).
> 
> - Potential for the death of the userspace security server (including 
> things like the OOM killer) or extreme performance problems under heavy 
> swapping.
> 
> The only potential win is reduced kernel memory usage for policy, but 
> the total memory footprint for kernel + userspace security server is 
> likely much higher and it is not clear that the ability to swap portions 
> of the userspace security server is desirable. I think we will get 
> further by reducing the kernel policy memory footprint.
> 
> Even when you want to disable kernel enforcement for some object classes 
> I think that the userspace security server loses (this is something I 
> have suggested to allow userspace apps like DBUS to rely entirely on 
> SELinux policy instead of SELinux + custom security policy). In those 
> scenarios you are likely still going to want labeling to happen for 
> processes, files, etc. even if many of the access checks are disabled. 
> That means there will still need to be a policy loaded into the kernel.
> 
> I don't see any compelling use for the userspace security server and 
> don't think we should make kernel changes with the assumption that it 
> will be present in the future.

The userspace security server still has potential utility for higher
level application policies that are orthogonal to the local kernel's
policy, like the RAdAC work.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Joshua Brindle]

Eric Paris wrote:
> On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
>   
>> On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
>>     
> [snip snip]
>   
>>> This is a vestige of the original av padding where I had to be concerned
>>> that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
>>> only use the array when making permission checks I can assume that I
>>> will never get a class from the check greater than the number of kernel
>>> classes defined.  So all that can be simplified to just
>>> p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
>>>       
>> No - security_compute_av can also be called via selinuxfs to check
>> userland security classes (from libselinux for userspace object
>> managers).
>>     
>
> I'm starting to wonder if the entire way I am constructing and using
> using my table might be wrong.  Right now I am finding the permissions
> which are defined in the kernel but are not defined in the policy,
> storing just those select permissions, and then adding those to the
> allow rules.  This works very well when I only consider the kernel and
> don't think about things in userspace.
>
>   
What happens when we start using a userspace security server and the 
kernel policy doesn't have any userspace object classes? If the avc 
routing gets changed the policy suddenly turns to default allow, not 
good. We have to be careful here, chances are we'll remove user object 
classes from kernel headers and that would make the result acceptable 
but in the mean time this puts the policy in a bad state.


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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Eamon Walsh]

Joshua Brindle wrote:
> What happens when we start using a userspace security server and the 
> kernel policy doesn't have any userspace object classes? If the avc 
> routing gets changed the policy suddenly turns to default allow, not 
> good. We have to be careful here, chances are we'll remove user object 
> classes from kernel headers and that would make the result acceptable 
> but in the mean time this puts the policy in a bad state.

It would be nice if there were string-to-number or number-to-number 
lookup functions for the class and permission values so that userspace 
object managers didn't have to hardcode the values from the header files.

-- 
Eamon Walsh <ewalsh@tycho.nsa.gov>
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.

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Thu, 2007-02-15 at 13:30 -0500, Eamon Walsh wrote:
> Joshua Brindle wrote:
> > What happens when we start using a userspace security server and the 
> > kernel policy doesn't have any userspace object classes? If the avc 
> > routing gets changed the policy suddenly turns to default allow, not 
> > good. We have to be careful here, chances are we'll remove user object 
> > classes from kernel headers and that would make the result acceptable 
> > but in the mean time this puts the policy in a bad state.
> 
> It would be nice if there were string-to-number or number-to-number 
> lookup functions for the class and permission values so that userspace 
> object managers didn't have to hardcode the values from the header files.

That was supposed to be done by now (c.f. dynamic discovery of object
classes).  Look up of userspace class and permission names via a
selinuxfs node or an auxiliary policy file that contained just those
mappings from the generated policy.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Stephen Smalley]

On Tue, 2007-02-13 at 14:36 -0500, Eric Paris wrote:
> On Tue, 2007-02-13 at 12:40 -0500, Stephen Smalley wrote:
> > On Tue, 2007-02-13 at 12:28 -0500, Eric Paris wrote:
> [snip snip]
> > > This is a vestige of the original av padding where I had to be concerned
> > > that p->p_classes.nprim was larger than kdefs->cts_len.  Since now I
> > > only use the array when making permission checks I can assume that I
> > > will never get a class from the check greater than the number of kernel
> > > classes defined.  So all that can be simplified to just
> > > p->undefined_perms = kzalloc(sizeof(u32)*kdefs->cts_len, GFP_KERNEL);
> > 
> > No - security_compute_av can also be called via selinuxfs to check
> > userland security classes (from libselinux for userspace object
> > managers).
> 
> I'm starting to wonder if the entire way I am constructing and using
> using my table might be wrong.  Right now I am finding the permissions
> which are defined in the kernel but are not defined in the policy,
> storing just those select permissions, and then adding those to the
> allow rules.  This works very well when I only consider the kernel and
> don't think about things in userspace.
> 
> But now I ask the question, do we/I want to extend this methodology to
> userspace classes and permissions?  Obviously I wouldn't be able to
> build the 'unknown perms' table for userspace since they are completely
> unknown!  Would it be better if I changed my approach to
> 
> Now:
> avd.allowed |= p->undefined_perms[tclass-1]
> 
> Possible
> avd.allowed |= (p->defined_perms[tclass-1]  ^ 0xffffffff)
> 
> It's not quite as directed.  Instead of only allowing the select
> permissions which we know are missing from policy we instead shotgun
> approach and allow everything not explicitly defined.  Maybe this allows
> buggy code to get allows which would have been caught and things like
> that, but at least it could work for userspace classes and perms.  Seems
> to me like this undefined perms problem will be worse in userspace than
> it is in the kernel.
> 
> Thoughts?  Or should I just stick with the ideas behind the patch below?

Keep it simple and directed.

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

Re: [RFC] Ability to allow undefined permissions and classes -v2

[Chad Sellers]

On 2/13/07 12:28 PM, "Eric Paris" <eparis@redhat.com> wrote:

> On Tue, 2007-02-13 at 07:27 -0500, Stephen Smalley wrote:
>> On Mon, 2007-02-12 at 14:43 -0500, Eric Paris wrote:
>>> I do still have a problem.  I don't really understand the 'inherits'
>>> ideas and so I'm not sure how to pad my p->undefined_perms[tclass] with
>>> permissions which are missing in policy due to an incorrect or missing
>>> inherits statement.  If this approach seems good and reasonable I will
>>> try to better understand how I can glean those bits from
>>> validate_classes() and I will also move this to use config flags and
>>> have multiple options on how to handle policies.
>> 
>> Missing or incorrect inherits clause should remain fatal, I'd say - it
>> is too substantive a change to the class definition to expect a running
>> kernel to adapt.  Classes that inherit a common definition pick up their
>> initial set of permission bits from that common definition, such that
>> those bits are uniform across those classes and can be checked by a
>> single permission check (e.g. for common file or socket checks).  But as
>> soon as you remove or change them, in addition to removing all of those
>> inherited permissions, you will have disturbed the values of any
>> subsequent class-specific permissions.
> 
> ATM one of the inherits checking paths is non-fatal in validate_classes
> I'll spend some time to understand it and either make it fatal or figure
> out how to fill my table correctly
>
This non-fatal path covers the simple case where there is an extra
permission in the kernel defined common that is not in the policy defined
common inherited by class foo. Note that the policy will still fail to load
if the class has any additional defined permissions, as those additional
permission values will be disturbed so the check on those permissions will
fail. So, the only corner case policy that could pass this non-fatal path
would be one in which all classes inheriting the common in question did not
add any additional permissions of their own.

Chad Sellers


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