specifying groups of types

specifying groups of types

[Russell Coker]

Following a discussion on IRC, it occurs to me that it would be handy to have 
the following in the policy language:
allow some_domain { file_type !shadow_t }:...

So we can specify everything in file_type except for shadow_t.

-- 
http://www.coker.com.au/selinux/   My NSA Security Enhanced Linux packages
http://www.coker.com.au/bonnie++/  Bonnie++ hard drive benchmark
http://www.coker.com.au/postal/    Postal SMTP/POP benchmark
http://www.coker.com.au/~russell/  My home page


--
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: specifying groups of types

[Stephen Smalley]

On Sat, 2003-10-11 at 00:35, Russell Coker wrote:
> Following a discussion on IRC, it occurs to me that it would be handy to have 
> the following in the policy language:
> allow some_domain { file_type !shadow_t }:...
> 
> So we can specify everything in file_type except for shadow_t.

Yes, although I'm not sure about the notation; might be better to
provide a set difference operator, e.g.
	file_type - shadow_t

Are you offering to implement this enhancement to checkpolicy?

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
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: specifying groups of types

[David Caplan]

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

Russell,

Here's a quick hack that appears to work.  It turns off the type (or 
list of types if used on an attribute) when building the bitmap of types 
for a rule.  The syntax is to use a '-' in front of a type or attribute 
name.

allow some_domain { file_type -shadow_t -null_device_t -exec_type}:...

The proper way to do this is in the yacc parsing section.  All I did was 
allow '-' as the first character of an identifier (policy_scan.l) and 
handle the subtraction of the type/attribute in 
policy_parse.y:set_types().  The danger is that types (and anything 
using the identifier definition) can be declared with '-' as the first 
character and cause problems.  The advantage, in theory, is that 
wherever a list of types/attributes is processed, the '-' notation can 
be used to turn off types.  So, you should also be able to do something 
like:

allow { auth -crond_t } file_type:...

Types/attributes are processed in order, and subsequent allow rules can 
also override the subtraction.

I'd recommend trying this out and if you find it useful change the parse 
rules.  I tested it on some real basic policy, so it may cause other 
unintended problems.  I'm throwing it out more as a starting point 
rather than something intended to be integrated into checkpolicy.

David

Stephen Smalley wrote:
> On Sat, 2003-10-11 at 00:35, Russell Coker wrote:
> 
>>Following a discussion on IRC, it occurs to me that it would be handy to have 
>>the following in the policy language:
>>allow some_domain { file_type !shadow_t }:...
>>
>>So we can specify everything in file_type except for shadow_t.
> 
> 
> Yes, although I'm not sure about the notation; might be better to
> provide a set difference operator, e.g.
> 	file_type - shadow_t
> 
> Are you offering to implement this enhancement to checkpolicy?
> 

-- 
__________________________________

David Caplan     410 290 1411 x105
dac@tresys.com
Tresys Technology, LLC
8840 Stanford Blvd., Suite 2100
Columbia, MD 21045

[-- Attachment #2: checkpolicy.patch --]
[-- Type: text/plain, Size: 2637 bytes --]

diff -ruN checkpolicy.old/policy_parse.y checkpolicy/policy_parse.y
--- checkpolicy.old/policy_parse.y	2003-10-15 07:15:18.431551648 -0400
+++ checkpolicy/policy_parse.y	2003-10-15 07:19:00.550784392 -0400
@@ -1,6 +1,10 @@
 
 /*
  * Author : Stephen Smalley, <sds@epoch.ncsc.mil> 
+ *
+ * Modified October 14, 2003 David Caplan, <dac@tresys.com>
+ * - allow exclusion of types and attributes in type/attribute lists
+ * 
  */
 
 /* FLASK */
@@ -1660,10 +1664,11 @@
 {
 	type_datum_t *t;
 	int i;
+	int add = TRUE;
 
 	if (strcmp(id, "*") == 0) {
 		/* set all types */
-		for (i = 0; i < policydbp->p_types.nprim; i++) 
+		for (i = 0; i < policydbp->p_types.nprim; i++)
 			ebitmap_set_bit(set, i, TRUE);
 		free(id);
 		return 0;
@@ -1674,14 +1679,27 @@
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (ebitmap_get_bit(set, i))
 				ebitmap_set_bit(set, i, FALSE);
-			else 
+			else
 				ebitmap_set_bit(set, i, TRUE);
 		}
 		free(id);
 		return 0;
 	}
 
-	t = hashtab_search(policydbp->p_types.table, id);
+	/* see if we want to exclude type/attribute */
+	if (id[0] == '-') {
+	         if (strlen(id) == 1) {
+		      sprintf(errormsg, "illegal identifier %s", id);
+		      yyerror(errormsg);
+		      free(id);
+		      return -1;
+	         }
+	         add = FALSE;
+	         t = hashtab_search(policydbp->p_types.table, id+1);
+	} else {
+	         t = hashtab_search(policydbp->p_types.table, id);
+	}
+
 	if (!t) {
 		sprintf(errormsg, "unknown type %s", id);
 		yyerror(errormsg);
@@ -1693,12 +1711,13 @@
 		/* set all types with this attribute */
 		for (i = ebitmap_startbit(&t->types); i < ebitmap_length(&t->types); i++) {
 			if (!ebitmap_get_bit(&t->types, i)) 
-				continue;		
-			ebitmap_set_bit(set, i, TRUE);
+				continue;
+			/* set or clear bit depending on add */
+			ebitmap_set_bit(set, i, add);
 		}
 	} else {
-		/* set one type */
-		ebitmap_set_bit(set, t->value - 1, TRUE);
+		/* set or clear (depending on add) one type */
+		ebitmap_set_bit(set, t->value - 1, add);
 	}
 
 	free(id);
diff -ruN checkpolicy.old/policy_scan.l checkpolicy/policy_scan.l
--- checkpolicy.old/policy_scan.l	2003-10-15 07:15:18.426552408 -0400
+++ checkpolicy/policy_scan.l	2003-10-15 07:10:39.149009048 -0400
@@ -127,7 +127,7 @@
 t2 |
 T2				{ return(T2); }
 "/"({letter}|{digit}|_|"."|"-"|"/")*	{ return(PATH); }
-{letter}({letter}|{digit}|_)*	{ return(IDENTIFIER); }
+({letter}|"-")({letter}|{digit}|_)*	{ return(IDENTIFIER); }
 {letter}({letter}|{digit}|_|"."|"-")*	{ return(USER_IDENTIFIER); }
 {digit}{digit}*                 { return(NUMBER); }
 #[^\n]*                         { /* delete comments */ }

Re: specifying groups of types

[Chris PeBenito]

Well I tried this out, and it worked, though I used simple examples:

portage.te:
create_dir_notdevfile(portage_t,{file_type -shadow_t})

admin_macros.te:
# Relabel most files.
allow $1_t {file_type -shadow_t}:dir { getattr read search relabelfrom relabelto };
allow $1_t {file_type -shadow_t}:file_class_set { getattr relabelfrom relabelto };

(where portage is Gentoo's packaging system)  I don't know anything
about yacc, so I can't comment on the implementation.  But the new
shadow assertions are what the first IRC discussion stemmed from.  They
were (correctly) not tripped by these statements, and sysadm_t can stat
but not read /etc/shadow.

On Tue, 2003-10-14 at 13:56, David Caplan wrote:
> I'd recommend trying this out and if you find it useful change the parse 
> rules.  I tested it on some real basic policy, so it may cause other 
> unintended problems.  I'm throwing it out more as a starting point 
> rather than something intended to be integrated into checkpolicy.

> Stephen Smalley wrote:
> > On Sat, 2003-10-11 at 00:35, Russell Coker wrote:
> > 
> >>Following a discussion on IRC, it occurs to me that it would be handy to have 
> >>the following in the policy language:
> >>allow some_domain { file_type !shadow_t }:...
> >>
> >>So we can specify everything in file_type except for shadow_t.
-- 
Chris PeBenito
<pebenito@gentoo.org>
Developer, SELinux
Hardened Gentoo Linux
 
Public Key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xE6AF9243
Key fingerprint = B0E6 877A 883F A57A 8E6A  CB00 BC8E E42D E6AF 9243

--
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: specifying groups of types

[Chris PeBenito]

Was there any resolution to this?  I think this would be useful for
checkpolicy to have, but it hasn't been merged (at least on the
sourceforge cvs).  I don't remember seeing any official response from
the NSA team about it nor any criticism/improvements of it.

On Tue, 2003-10-14 at 13:56, David Caplan wrote:
> Russell,
> 
> Here's a quick hack that appears to work.  It turns off the type (or 
> list of types if used on an attribute) when building the bitmap of types 
> for a rule.  The syntax is to use a '-' in front of a type or attribute 
> name.
> 
> allow some_domain { file_type -shadow_t -null_device_t -exec_type}:...
> 
> The proper way to do this is in the yacc parsing section.  All I did was 
> allow '-' as the first character of an identifier (policy_scan.l) and 
> handle the subtraction of the type/attribute in 
> policy_parse.y:set_types().  The danger is that types (and anything 
> using the identifier definition) can be declared with '-' as the first 
> character and cause problems.  The advantage, in theory, is that 
> wherever a list of types/attributes is processed, the '-' notation can 
> be used to turn off types.  So, you should also be able to do something 
> like:
> 
> allow { auth -crond_t } file_type:...
> 
> Types/attributes are processed in order, and subsequent allow rules can 
> also override the subtraction.
> 
> I'd recommend trying this out and if you find it useful change the parse 
> rules.  I tested it on some real basic policy, so it may cause other 
> unintended problems.  I'm throwing it out more as a starting point 
> rather than something intended to be integrated into checkpolicy.
> 
> David
> 
> Stephen Smalley wrote:
> > On Sat, 2003-10-11 at 00:35, Russell Coker wrote:
> > 
> >>Following a discussion on IRC, it occurs to me that it would be handy to have 
> >>the following in the policy language:
> >>allow some_domain { file_type !shadow_t }:...
> >>
> >>So we can specify everything in file_type except for shadow_t.
> > 
> > 
> > Yes, although I'm not sure about the notation; might be better to
> > provide a set difference operator, e.g.
> > 	file_type - shadow_t
> > 
> > Are you offering to implement this enhancement to checkpolicy?
> > 
-- 
Chris PeBenito
<pebenito@gentoo.org>
Developer, SELinux
Hardened Gentoo Linux
 
Public Key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xE6AF9243
Key fingerprint = B0E6 877A 883F A57A 8E6A  CB00 BC8E E42D E6AF 9243

--
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: specifying groups of types

[Stephen Smalley]

On Sun, 2003-11-02 at 12:43, Chris PeBenito wrote:
> Was there any resolution to this?  I think this would be useful for
> checkpolicy to have, but it hasn't been merged (at least on the
> sourceforge cvs).  I don't remember seeing any official response from
> the NSA team about it nor any criticism/improvements of it.

Dave correctly noted that the implementation was just an experimental
hack, not something for upstream inclusion, and explained what would
need to be done to implement it correctly.  No one has done that yet,
AFAIK.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
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: specifying groups of types

[David Caplan]

I didn't receive any comments on it.  I'd emphasize again that my 
solution was not a "proper" fix (it leaves open too many 
inconsistencies) and was not intended to be accepted as an official 
patch until it was cleaned up.  If no one comes up with a clean solution 
I'll fix mine when I have the chance.  Right now I'm trying to finish up 
the parsing code for our conditional policy modification among other things.

David

Chris PeBenito wrote:
> Was there any resolution to this?  I think this would be useful for
> checkpolicy to have, but it hasn't been merged (at least on the
> sourceforge cvs).  I don't remember seeing any official response from
> the NSA team about it nor any criticism/improvements of it.
> 
> On Tue, 2003-10-14 at 13:56, David Caplan wrote:
> 
>>Russell,
>>
>>Here's a quick hack that appears to work.  It turns off the type (or 
>>list of types if used on an attribute) when building the bitmap of types 
>>for a rule.  The syntax is to use a '-' in front of a type or attribute 
>>name.
>>
>>allow some_domain { file_type -shadow_t -null_device_t -exec_type}:...
>>
>>The proper way to do this is in the yacc parsing section.  All I did was 
>>allow '-' as the first character of an identifier (policy_scan.l) and 
>>handle the subtraction of the type/attribute in 
>>policy_parse.y:set_types().  The danger is that types (and anything 
>>using the identifier definition) can be declared with '-' as the first 
>>character and cause problems.  The advantage, in theory, is that 
>>wherever a list of types/attributes is processed, the '-' notation can 
>>be used to turn off types.  So, you should also be able to do something 
>>like:
>>
>>allow { auth -crond_t } file_type:...
>>
>>Types/attributes are processed in order, and subsequent allow rules can 
>>also override the subtraction.
>>
>>I'd recommend trying this out and if you find it useful change the parse 
>>rules.  I tested it on some real basic policy, so it may cause other 
>>unintended problems.  I'm throwing it out more as a starting point 
>>rather than something intended to be integrated into checkpolicy.
>>
>>David
>>
>>Stephen Smalley wrote:
>>
>>>On Sat, 2003-10-11 at 00:35, Russell Coker wrote:
>>>
>>>
>>>>Following a discussion on IRC, it occurs to me that it would be handy to have 
>>>>the following in the policy language:
>>>>allow some_domain { file_type !shadow_t }:...
>>>>
>>>>So we can specify everything in file_type except for shadow_t.
>>>
>>>
>>>Yes, although I'm not sure about the notation; might be better to
>>>provide a set difference operator, e.g.
>>>	file_type - shadow_t
>>>
>>>Are you offering to implement this enhancement to checkpolicy?
>>>


-- 
__________________________________

David Caplan     410 290 1411 x105
dac@tresys.com
Tresys Technology, LLC
8840 Stanford Blvd., Suite 2100
Columbia, MD 21045


--
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: specifying groups of types

[Stephen Smalley]

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

On Tue, 2003-10-14 at 14:56, David Caplan wrote:
> Here's a quick hack that appears to work.  It turns off the type (or 
> list of types if used on an attribute) when building the bitmap of types 
> for a rule.  The syntax is to use a '-' in front of a type or attribute 
> name.
> 
> allow some_domain { file_type -shadow_t -null_device_t -exec_type}:...
> 
> The proper way to do this is in the yacc parsing section.  All I did was 
> allow '-' as the first character of an identifier (policy_scan.l) and 
> handle the subtraction of the type/attribute in 
> policy_parse.y:set_types().  The danger is that types (and anything 
> using the identifier definition) can be declared with '-' as the first 
> character and cause problems.  The advantage, in theory, is that 
> wherever a list of types/attributes is processed, the '-' notation can 
> be used to turn off types.  So, you should also be able to do something 
> like:
> 
> allow { auth -crond_t } file_type:...
> 
> Types/attributes are processed in order, and subsequent allow rules can 
> also override the subtraction.
> 
> I'd recommend trying this out and if you find it useful change the parse 
> rules.  I tested it on some real basic policy, so it may cause other 
> unintended problems.  I'm throwing it out more as a starting point 
> rather than something intended to be integrated into checkpolicy.

The attached patch for checkpolicy-1.4 provides the same feature (and
syntax), but is implemented as a modification to the policy grammar
rather than a change to the identifier token definition.  It doesn't
address the use of types in constraint expressions, but should work for
the TE rules, including assertions.
   
-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

[-- Attachment #2: checkpolicy-excludetypes.patch --]
[-- Type: text/x-patch, Size: 4139 bytes --]

Index: checkpolicy/policy_parse.y
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/checkpolicy/policy_parse.y,v
retrieving revision 1.10
diff -u -r1.10 policy_parse.y
--- checkpolicy/policy_parse.y	9 Jan 2004 14:05:01 -0000	1.10
+++ checkpolicy/policy_parse.y	14 Jan 2004 21:08:32 -0000
@@ -520,6 +520,8 @@
 			| tilde nested_id_set
 	 		{ if (insert_id("~", 0)) return -1; 
 			  if (insert_separator(0)) return -1; }
+                        | identifier '-' { if (insert_id("-", 0)) return -1; } identifier 
+			{ if (insert_separator(0)) return -1; }
 			;
 tilde_push              : tilde
                         { if (insert_id("~", 1)) return -1; }
@@ -546,7 +548,7 @@
                         ;
 nested_id_list          : nested_id_element | nested_id_list nested_id_element
                         ;
-nested_id_element       : identifier | nested_id_set
+nested_id_element       : identifier | '-' { if (insert_id("-", 0)) return -1; } identifier | nested_id_set
                         ;
 identifier		: IDENTIFIER
 			{ if (insert_id(yytext,0)) return -1; }
@@ -1661,7 +1663,8 @@
 
 
 static int set_types(ebitmap_t *set,
-		     char *id)
+		     char *id,
+		     int *add)
 {
 	type_datum_t *t;
 	unsigned int i;
@@ -1686,6 +1689,12 @@
 		return 0;
 	}
 
+	if (strcmp(id, "-") == 0) {
+		*add = 0;
+		free(id);
+		return 0;
+	}	
+
 	t = hashtab_search(policydbp->p_types.table, id);
 	if (!t) {
 		sprintf(errormsg, "unknown type %s", id);
@@ -1695,18 +1704,19 @@
 	}
 
 	if (t->isattr) {
-		/* set all types with this attribute */
+		/* set or clear all types with this attribute */
 		for (i = ebitmap_startbit(&t->types); i < ebitmap_length(&t->types); i++) {
 			if (!ebitmap_get_bit(&t->types, i)) 
 				continue;		
-			ebitmap_set_bit(set, i, TRUE);
+			ebitmap_set_bit(set, i, *add);
 		}
 	} else {
-		/* set one type */
-		ebitmap_set_bit(set, t->value - 1, TRUE);
+		/* set or clear one type */
+		ebitmap_set_bit(set, t->value - 1, *add);
 	}
 
 	free(id);
+	*add = 1;
 	return 0;
 }
 
@@ -1720,7 +1730,7 @@
 	class_datum_t *cladatum;
 	ebitmap_t stypes, ttypes, tclasses;
 	__u32 newtype = 0;
-	int ret;
+	int ret, add = 1;
 	unsigned int i, j, k;
 
 	if (pass == 1) {
@@ -1740,12 +1750,12 @@
 	ebitmap_init(&tclasses);
 
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, id))
+		if (set_types(&stypes, id, &add))
 			return -1;
 	}
 
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, id))
+		if (set_types(&ttypes, id, &add))
 			return -1;
 	}
 
@@ -1967,7 +1977,7 @@
 	ebitmap_t stypes, ttypes, tclasses;
 	access_vector_t *avp;
 	unsigned int i, j, hiclass;
-	int self = 0;
+	int self = 0, add = 1;
 	te_assert_t *newassert;
 
 	if (pass == 1) {
@@ -1987,7 +1997,7 @@
 	ebitmap_init(&tclasses);
 
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, id))
+		if (set_types(&stypes, id, &add))
 			return -1;
 	}
 
@@ -1996,7 +2006,7 @@
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, id))
+		if (set_types(&ttypes, id, &add))
 			return -1;
 	}
 
@@ -2139,7 +2149,7 @@
 {
 	role_datum_t *role;
 	char *role_id, *id;
-	int ret;
+	int ret, add = 1;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2174,7 +2184,7 @@
 		free(role_id);
 
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&role->types, id))
+		if (set_types(&role->types, id, &add))
 			return -1;
 	}
 
@@ -2328,6 +2338,7 @@
 	ebitmap_t roles, types;
 	struct role_trans *tr = 0;
 	unsigned int i, j;
+	int add = 1;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2348,7 +2359,7 @@
 	}
 
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, id))
+		if (set_types(&types, id, &add))
 			return -1;
 	}
 
@@ -2589,6 +2600,7 @@
 	role_datum_t *role;
 	char *id;
 	__u32 val;
+	int add = 1;
 
 	if (pass == 1) {
 		if (expr_type == CEXPR_NAMES) {
@@ -2678,7 +2690,7 @@
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
-				if (set_types(&expr->names, id)) {
+				if (set_types(&expr->names, id, &add)) {
 					free(expr);
 					return 0;
 				}

Re: specifying groups of types

[Karl MacMillan]

On Thu, 2004-01-15 at 09:31, Stephen Smalley wrote:
> > allow { auth -crond_t } file_type:...
> > 
> > Types/attributes are processed in order, and subsequent allow rules can 
> > also override the subtraction.
> > 
> > I'd recommend trying this out and if you find it useful change the parse 
> > rules.  I tested it on some real basic policy, so it may cause other 
> > unintended problems.  I'm throwing it out more as a starting point 
> > rather than something intended to be integrated into checkpolicy.
> 
> The attached patch for checkpolicy-1.4 provides the same feature (and
> syntax), but is implemented as a modification to the policy grammar
> rather than a change to the identifier token definition.  It doesn't
> address the use of types in constraint expressions, but should work for
> the TE rules, including assertions.

I'm not certain that we want these to be order dependent. I can imagine
a policy writer doing

allow { attr_a -sysadm_t attr_b} . . . 

and being surprised that sysadm_t was allowed because it was in attr_b.

Karl
-- 
Karl MacMillan
Tresys Technology
kmacmillan@tresys.com
http://www.tresys.com
(410) 290-1411 x134


--
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: specifying groups of types

[Stephen Smalley]

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

On Fri, 2004-01-16 at 11:50, Karl MacMillan wrote:
> I'm not certain that we want these to be order dependent. I can imagine
> a policy writer doing
> 
> allow { attr_a -sysadm_t attr_b} . . . 
> 
> and being surprised that sysadm_t was allowed because it was in attr_b.

So, you want something like the attached (untested) patch?
Still wouldn't prevent one allow rule from adding back a
type excluded by another allow rule...

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
National Security Agency

[-- Attachment #2: checkpolicy-negset.patch --]
[-- Type: text/x-patch, Size: 5540 bytes --]

Index: checkpolicy/policy_parse.y
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/checkpolicy/policy_parse.y,v
retrieving revision 1.12
diff -u -r1.12 policy_parse.y
--- checkpolicy/policy_parse.y	16 Jan 2004 15:07:17 -0000	1.12
+++ checkpolicy/policy_parse.y	16 Jan 2004 18:11:50 -0000
@@ -1724,6 +1724,7 @@
 
 
 static int set_types(ebitmap_t *set,
+		     ebitmap_t *negset,
 		     char *id,
 		     int *add)
 {
@@ -1731,9 +1732,11 @@
 	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
-		/* set all types */
-		for (i = 0; i < policydbp->p_types.nprim; i++) 
-			ebitmap_set_bit(set, i, TRUE);
+		/* set all types not in negset */
+		for (i = 0; i < policydbp->p_types.nprim; i++) {
+			if (!ebitmap_get_bit(negset, i))
+				ebitmap_set_bit(set, i, TRUE);
+		}
 		free(id);
 		return 0;
 	}
@@ -1765,15 +1768,34 @@
 	}
 
 	if (t->isattr) {
-		/* set or clear all types with this attribute */
+		/* set or clear all types with this attribute,
+		   but do not set anything explicitly cleared previously */
 		for (i = ebitmap_startbit(&t->types); i < ebitmap_length(&t->types); i++) {
 			if (!ebitmap_get_bit(&t->types, i)) 
 				continue;		
-			ebitmap_set_bit(set, i, *add);
+			if (!(*add)) {
+				ebitmap_set_bit(set, i, FALSE);
+				ebitmap_set_bit(negset, i, TRUE);
+			} else if (!ebitmap_get_bit(negset, i)) {
+				ebitmap_set_bit(set, i, TRUE);
+			} else {
+				char *name = type_val_to_name(i+1);
+				sprintf(errormsg, "ignoring %s due to prior -%s", name, name);
+				yywarn(errormsg);
+			}
 		}
 	} else {
-		/* set or clear one type */
-		ebitmap_set_bit(set, t->value - 1, *add);
+		/* set or clear one type, but do not set anything
+		   explicitly cleared previously */	
+		if (!(*add)) {
+			ebitmap_set_bit(set, t->value - 1, FALSE);
+			ebitmap_set_bit(negset, t->value - 1, TRUE);
+		} else if (!ebitmap_get_bit(negset, t->value - 1)) {
+			ebitmap_set_bit(set, t->value - 1, TRUE);
+		} else {
+			sprintf(errormsg, "ignoring %s due to prior -%s", id, id);
+			yywarn(errormsg);
+		}
 	}
 
 	free(id);
@@ -1789,7 +1811,7 @@
 	avtab_datum_t avdatum, *avdatump;
 	type_datum_t *datum;
 	class_datum_t *cladatum;
-	ebitmap_t stypes, ttypes, tclasses;
+	ebitmap_t stypes, ttypes, tclasses, negset;
 	__u32 newtype = 0;
 	int ret, add = 1;
 	unsigned int i, j, k;
@@ -1810,15 +1832,19 @@
 	ebitmap_init(&ttypes);
 	ebitmap_init(&tclasses);
 
+	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, id, &add))
+		if (set_types(&stypes, &negset, id, &add))
 			return -1;
 	}
+	ebitmap_destroy(&negset);
 
+	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, id, &add))
+		if (set_types(&ttypes, &negset, id, &add))
 			return -1;
 	}
+	ebitmap_destroy(&negset);
 
 	while ((id = queue_remove(id_queue))) {
 		cladatum = hashtab_search(policydbp->p_classes.table, id);
@@ -2035,7 +2061,7 @@
 	char *id;
 	class_datum_t *cladatum;
 	perm_datum_t *perdatum;
-	ebitmap_t stypes, ttypes, tclasses;
+	ebitmap_t stypes, ttypes, tclasses, negset;
 	access_vector_t *avp;
 	unsigned int i, j, hiclass;
 	int self = 0, add = 1;
@@ -2057,19 +2083,23 @@
 	ebitmap_init(&ttypes);
 	ebitmap_init(&tclasses);
 
+	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, id, &add))
+		if (set_types(&stypes, &negset, id, &add))
 			return -1;
 	}
+	ebitmap_destroy(&negset);
 
+	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
 		if (strcmp(id, "self") == 0) {
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, id, &add))
+		if (set_types(&ttypes, &negset, id, &add))
 			return -1;
 	}
+	ebitmap_destroy(&negset);
 
 	hiclass = 0;
 	while ((id = queue_remove(id_queue))) {
@@ -2211,6 +2241,7 @@
 	role_datum_t *role;
 	char *role_id, *id;
 	int ret, add = 1;
+	ebitmap_t negset;
 
 	if (pass == 1) {
 		while ((id = queue_remove(id_queue))) 
@@ -2244,10 +2275,12 @@
 	} else
 		free(role_id);
 
+	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&role->types, id, &add))
+		if (set_types(&role->types, &negset, id, &add))
 			return -1;
 	}
+	ebitmap_destroy(&negset);
 
 	return 0;
 }
@@ -2396,7 +2429,7 @@
 {
 	char *id;
 	role_datum_t *role;
-	ebitmap_t roles, types;
+	ebitmap_t roles, types, negset;
 	struct role_trans *tr = 0;
 	unsigned int i, j;
 	int add = 1;
@@ -2419,10 +2452,12 @@
 			return -1;
 	}
 
+	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, id, &add))
+		if (set_types(&types, &negset, id, &add))
 			return -1;
 	}
+	ebitmap_destroy(&negset);
 
 	id = (char *) queue_remove(id_queue);
 	if (!id) {
@@ -2659,6 +2694,7 @@
 	struct constraint_expr *expr, *e1 = NULL, *e2;
 	user_datum_t *user;
 	role_datum_t *role;
+	ebitmap_t negset;
 	char *id;
 	__u32 val;
 	int add = 1;
@@ -2729,6 +2765,7 @@
 	case CEXPR_NAMES:
 		expr->attr = arg1;
 		expr->op = arg2;
+		ebitmap_init(&negset);
 		while ((id = (char *) queue_remove(id_queue))) {
 			if (expr->attr & CEXPR_USER) {
 				user = (user_datum_t *) hashtab_search(policydbp->p_users.table,
@@ -2751,7 +2788,7 @@
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
-				if (set_types(&expr->names, id, &add)) {
+				if (set_types(&expr->names, &negset, id, &add)) {
 					free(expr);
 					return 0;
 				}
@@ -2769,6 +2806,7 @@
 			}
 			free(id);
 		}
+		ebitmap_destroy(&negset);
 		return (uintptr_t)expr;
 	default:
 		yyerror("invalid constraint expression");

Re: specifying groups of types

[Karl MacMillan]

On Fri, 2004-01-16 at 13:17, Stephen Smalley wrote:
> On Fri, 2004-01-16 at 11:50, Karl MacMillan wrote:
> > I'm not certain that we want these to be order dependent. I can imagine
> > a policy writer doing
> > 
> > allow { attr_a -sysadm_t attr_b} . . . 
> > 
> > and being surprised that sysadm_t was allowed because it was in attr_b.
> 
> So, you want something like the attached (untested) patch?
> Still wouldn't prevent one allow rule from adding back a
> type excluded by another allow rule...

Exactly. I don't think that we want to prevent other rules from adding
back the type, just consider the list of types within a rule as a whole.
The patch looks correct to me at first glance, but I didn't test it
either. After testing, will this change go in? I'm working on supporting
this in our tools and need to know the final semantics.

Thanks,

Karl

-- 
Karl MacMillan
Tresys Technology
kmacmillan@tresys.com
http://www.tresys.com
(410) 290-1411 x134


--
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: specifying groups of types

[Stephen Smalley]

On Fri, 2004-01-16 at 13:36, Karl MacMillan wrote:
> Exactly. I don't think that we want to prevent other rules from adding
> back the type, just consider the list of types within a rule as a whole.
> The patch looks correct to me at first glance, but I didn't test it
> either. After testing, will this change go in? I'm working on supporting
> this in our tools and need to know the final semantics.

I would expect so, barring a significant objection to it.  Assuming that
it does go in, I expect we'll drop the warnings when a type is not added
due to a prior negation, as that may occur naturally due to combinations
of type attributes and types.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
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.

Trival relabel Problem

[Thomas DuBuisson]

I looked at the other relabel threads, they didn't seem to address my
issue.  But if I missed something, sorry if this is a repeat.  I am fairly
certain this is trivial but I am lost.

My problem:
-----------------------
root@blacktower:/etc/security/selinux/src/policy# execcon \
root:sysadm_r:sysadm_t make relabel

/usr/sbin/setfiles:  read 425 specifications
/usr/sbin/setfiles:  labeling files under /
/: Operation not supported
/usr/sbin/setfiles:  unable to obtain attribute for file /
/usr/sbin/setfiles:  error while labeling files under /
----------------------------------------------------

My setup
-----------------------------------
Enforcing: 0
Base System: Slackware 9.1
Kernel: 2.6.1-mm2
FS: reiserfs (I did edit the Makefile changing ext[23])
PAM.77
selinux-usr: 1.4
----------------------------------

So, any pointers on how to get this fs labeled?  What am I missing?

Thanks,
Thomas DuBuisson

--
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: Trival relabel Problem

[Stephen Smalley]

On Fri, 2004-01-16 at 16:03, Thomas DuBuisson wrote:
> I looked at the other relabel threads, they didn't seem to address my
> issue.  But if I missed something, sorry if this is a repeat.  I am fairly
> certain this is trivial but I am lost.
> 
> My problem:
> -----------------------
> root@blacktower:/etc/security/selinux/src/policy# execcon \
> root:sysadm_r:sysadm_t make relabel
> 
> /usr/sbin/setfiles:  read 425 specifications
> /usr/sbin/setfiles:  labeling files under /
> /: Operation not supported
> /usr/sbin/setfiles:  unable to obtain attribute for file /
> /usr/sbin/setfiles:  error while labeling files under /
> ----------------------------------------------------
> 
> My setup
> -----------------------------------
> Enforcing: 0
> Base System: Slackware 9.1
> Kernel: 2.6.1-mm2
> FS: reiserfs (I did edit the Makefile changing ext[23])
> PAM.77
> selinux-usr: 1.4
> ----------------------------------
> 
> So, any pointers on how to get this fs labeled?  What am I missing?

reiserfs doesn't support extended attributes unless you patch in
support, e.g. using Jeff Mahoney's patches from
ftp://ftp.suse.com/pub/people/jeffm/reiserfs/aclea/.  Hans has
previously indicated that he won't add new features to reiser3, so he
won't accept those patches into the mainline reiserfs.

-- 
Stephen Smalley <sds@epoch.ncsc.mil>
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.