[PATCH] disallow * and ~ in rules

[PATCH] disallow * and ~ in rules

[Joshua Brindle]

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

The attached patch disallows * and ~ in certain kinds of rules, the list
of where they are allowed and where they are not follows. I'm very
willing to discuss any ideas or arguments as to why these should or
shouldn't be in the list they are in.


* and ~ allowed:
range_trans (I was hoping TCS had an opinion on this)
neverallow, dontaudit, auditallow


* and ~ not allowed:
allow rules
type_transition, type_member, type_change
role declarations (to add types to a role)
role transitions


Joshua Brindle
Tresys Technology

[-- Attachment #2: no-star-comp-allow.patch --]
[-- Type: text/x-patch, Size: 4545 bytes --]

Index: policy_parse.y
===================================================================
RCS file: /cvsroot/selinux/nsa/selinux-usr/checkpolicy/policy_parse.y,v
retrieving revision 1.31
diff -u -u -p -r1.31 policy_parse.y
--- policy_parse.y	13 May 2005 19:53:31 -0000	1.31
+++ policy_parse.y	23 Jun 2005 14:49:29 -0000
@@ -1781,12 +1781,17 @@ static char *type_val_to_name(unsigned i
 static int set_types(ebitmap_t *set,
 		     ebitmap_t *negset,
 		     char *id,
-		     int *add)
+		     int *add, 
+		     char starallowed)
 {
 	type_datum_t *t;
 	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
+		if (!starallowed) {
+			yyerror("* not allowed in this type of rule");
+			return -1;
+		}
 		/* set all types not in negset */
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (!ebitmap_get_bit(negset, i))
@@ -1797,6 +1802,10 @@ static int set_types(ebitmap_t *set,
 	}
 
 	if (strcmp(id, "~") == 0) {
+		if (!starallowed) {
+			yyerror("~ not allowed in this type of rule");
+			return -1;
+		}
 		/* complement the set */
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (ebitmap_get_bit(set, i))
@@ -1893,14 +1902,14 @@ static int define_compute_type(int which
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2033,14 +2042,14 @@ static cond_av_list_t *define_cond_compu
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, 0))
 			return  COND_ERR;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, 0))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2468,7 +2477,7 @@ static cond_av_list_t *define_cond_te_av
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, which == AVTAB_ALLOWED? 0 : 1 ))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2479,7 +2488,7 @@ static cond_av_list_t *define_cond_te_av
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, which == AVTAB_ALLOWED? 0 : 1 ))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2646,7 +2655,7 @@ static int define_te_avtab(int which)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, which == AVTAB_ALLOWED? 0 : 1 ))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2657,7 +2666,7 @@ static int define_te_avtab(int which)
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, which == AVTAB_ALLOWED? 0 : 1 ))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2853,7 +2862,7 @@ static int define_role_types(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&role->types, &negset, id, &add))
+		if (set_types(&role->types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -3068,7 +3077,7 @@ static int define_role_trans(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, &negset, id, &add))
+		if (set_types(&types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -3493,7 +3502,7 @@ static uintptr_t
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
-				if (set_types(&expr->names, &negset, id, &add)) {
+				if (set_types(&expr->names, &negset, id, &add, 1)) {
 					free(expr);
 					return 0;
 				}
@@ -4839,14 +4848,14 @@ static int define_range_trans(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&doms, &negset, id, &add))
+		if (set_types(&doms, &negset, id, &add, 1))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, &negset, id, &add))
+		if (set_types(&types, &negset, id, &add, 1))
 			return -1;
 	}
 	ebitmap_destroy(&negset);

Re: [PATCH] disallow * and ~ in rules

[Stephen Smalley]

On Thu, 2005-06-23 at 12:17 -0400, Joshua Brindle wrote:
> The attached patch disallows * and ~ in certain kinds of rules, the list
> of where they are allowed and where they are not follows. I'm very
> willing to discuss any ideas or arguments as to why these should or
> shouldn't be in the list they are in.
> 
> 
> * and ~ allowed:
> range_trans (I was hoping TCS had an opinion on this)
> neverallow, dontaudit, auditallow
> 
> 
> * and ~ not allowed:
> allow rules
> type_transition, type_member, type_change
> role declarations (to add types to a role)
> role transitions

The patch only appears to alter their use in type sets, nothing else
(e.g. they can still be used for permission sets and role sets).  Is
that your intent?

For type sets, I'd favor greater consistency, e.g. not allow their use
in any type set except neverallow rules and only in them because they
are used to catch ill-formed policies.  Otherwise, type set exclusion
(e.g. domain -foo) is always preferable to set complement (e.g. ~foo),
as the latter will include unrelated types (e.g. file types) that are
not possible in that field, and a type attribute like domain or
file_type should always be used rather than a * for the same reason.
Looks like the FC4 strict and targeted policies are clean in this
respect, not sure about everything in the example 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: [PATCH] disallow * and ~ in rules

[Joshua Brindle]

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

On Thu, 2005-06-23 at 13:54 -0400, Stephen Smalley wrote:
> On Thu, 2005-06-23 at 12:17 -0400, Joshua Brindle wrote:
> > The attached patch disallows * and ~ in certain kinds of rules, the list
> > of where they are allowed and where they are not follows. I'm very
> > willing to discuss any ideas or arguments as to why these should or
> > shouldn't be in the list they are in.
> > 
> > 
> > * and ~ allowed:
> > range_trans (I was hoping TCS had an opinion on this)
> > neverallow, dontaudit, auditallow
> > 
> > 
> > * and ~ not allowed:
> > allow rules
> > type_transition, type_member, type_change
> > role declarations (to add types to a role)
> > role transitions
> 
> The patch only appears to alter their use in type sets, nothing else
> (e.g. they can still be used for permission sets and role sets).  Is
> that your intent?
> 
This is definitely up for debate. The major problems with type sets was
that using * and ~ in types would add rules when types were added, even
without assigning attributes or writing explicit rules. Permissions are
something different because they are mostly static and it's fairly well
known what you are saying you want when you use * for permissions.

Roles are interesting and I hadn't really thought about them. The
attached patch disallows * and ~ in role transitions, role allows and
user declarations in the role sets.

> For type sets, I'd favor greater consistency, e.g. not allow their use
> in any type set except neverallow rules and only in them because they
> are used to catch ill-formed policies.  Otherwise, type set exclusion
> (e.g. domain -foo) is always preferable to set complement (e.g. ~foo),
> as the latter will include unrelated types (e.g. file types) that are
> not possible in that field, and a type attribute like domain or
> file_type should always be used rather than a * for the same reason.
> Looks like the FC4 strict and targeted policies are clean in this
> respect, not sure about everything in the example policy.

Ok, I wasn't really sure if * or ~ would be useful in range_trans so I
left that for an MLS user to say something. The attached patch disallows
it.

I agree that neverallow has very legitimate reasons for * but dontaudit
and auditallow are a little less clear. Perhaps auditallow should allow
*, it's conceivable that someone would want to audit all access to an
object (though in the current policies and reference policy * would have
the same net effect as domain with fewer avtab entries). This may not be
the case for all policies though. The attached patch disallows it in all
rule types but neverallow. 

I forgot to mention constraints, which currently allow * and ~, any
thoughts on this?


Joshua 

[-- Attachment #2: no-star-comp-allow.patch --]
[-- Type: text/x-patch, Size: 5975 bytes --]

Index: policy_parse.y
===================================================================
RCS file: /cvsroot/selinux/nsa/selinux-usr/checkpolicy/policy_parse.y,v
retrieving revision 1.31
diff -u -u -p -r1.31 policy_parse.y
--- policy_parse.y	13 May 2005 19:53:31 -0000	1.31
+++ policy_parse.y	23 Jun 2005 18:44:30 -0000
@@ -1781,12 +1781,17 @@ static char *type_val_to_name(unsigned i
 static int set_types(ebitmap_t *set,
 		     ebitmap_t *negset,
 		     char *id,
-		     int *add)
+		     int *add, 
+		     char starallowed)
 {
 	type_datum_t *t;
 	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
+		if (!starallowed) {
+			yyerror("* not allowed in this type of rule");
+			return -1;
+		}
 		/* set all types not in negset */
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (!ebitmap_get_bit(negset, i))
@@ -1797,6 +1802,10 @@ static int set_types(ebitmap_t *set,
 	}
 
 	if (strcmp(id, "~") == 0) {
+		if (!starallowed) {
+			yyerror("~ not allowed in this type of rule");
+			return -1;
+		}
 		/* complement the set */
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (ebitmap_get_bit(set, i))
@@ -1893,14 +1902,14 @@ static int define_compute_type(int which
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2033,14 +2042,14 @@ static cond_av_list_t *define_cond_compu
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, 0))
 			return  COND_ERR;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, 0))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2468,7 +2477,7 @@ static cond_av_list_t *define_cond_te_av
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2479,7 +2488,7 @@ static cond_av_list_t *define_cond_te_av
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2646,7 +2655,7 @@ static int define_te_avtab(int which)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2657,7 +2666,7 @@ static int define_te_avtab(int which)
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2853,7 +2862,7 @@ static int define_role_types(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&role->types, &negset, id, &add))
+		if (set_types(&role->types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -3002,26 +3011,17 @@ static int set_roles(ebitmap_t *set,
 		     char *id)
 {
 	role_datum_t *r;
-	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
-		/* set all roles */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) 
-			ebitmap_set_bit(set, i, TRUE);
 		free(id);
-		return 0;
+		yyerror("* is not allowed for role sets");
+		return -1;
 	}
 
 	if (strcmp(id, "~") == 0) {
-		/* complement the set */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) {
-			if (ebitmap_get_bit(set, i))
-				ebitmap_set_bit(set, i, FALSE);
-			else 
-				ebitmap_set_bit(set, i, TRUE);
-		}
 		free(id);
-		return 0;
+		yyerror("~ is not allowed for role sets");
+		return -1;
 	}
 
 	r = hashtab_search(policydbp->p_roles.table, id);
@@ -3068,7 +3068,7 @@ static int define_role_trans(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, &negset, id, &add))
+		if (set_types(&types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -3493,7 +3493,7 @@ static uintptr_t
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
-				if (set_types(&expr->names, &negset, id, &add)) {
+				if (set_types(&expr->names, &negset, id, &add, 1)) {
 					free(expr);
 					return 0;
 				}
@@ -3862,23 +3862,15 @@ static int set_user_roles(ebitmap_t *set
 	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
-		/* set all roles */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) 
-			ebitmap_set_bit(set, i, TRUE);
 		free(id);
-		return 0;
+		yyerror("* not allowed in user declarations");
+		return -1;
 	}
 
 	if (strcmp(id, "~") == 0) {
-		/* complement the set */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) {
-			if (ebitmap_get_bit(set, i))
-				ebitmap_set_bit(set, i, FALSE);
-			else 
-				ebitmap_set_bit(set, i, TRUE);
-		}
 		free(id);
-		return 0;
+		yyerror("~ not allowed in user declarations");
+		return -1;
 	}
 
 	r = hashtab_search(policydbp->p_roles.table, id);
@@ -4839,14 +4831,14 @@ static int define_range_trans(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&doms, &negset, id, &add))
+		if (set_types(&doms, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, &negset, id, &add))
+		if (set_types(&types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);

Re: [PATCH] disallow * and ~ in rules

[Stephen Smalley]

On Thu, 2005-06-23 at 14:47 -0400, Joshua Brindle wrote:
> I agree that neverallow has very legitimate reasons for * but dontaudit
> and auditallow are a little less clear. Perhaps auditallow should allow
> *, it's conceivable that someone would want to audit all access to an
> object (though in the current policies and reference policy * would have
> the same net effect as domain with fewer avtab entries). This may not be
> the case for all policies though. The attached patch disallows it in all
> rule types but neverallow. 

Yes, I think requiring people to use an attribute like domain or
file_type is preferable anyway; otherwise you end up with massive
explosion in the set of rules for a lot of types that can't possibly be
used in that manner anyway.

> I forgot to mention constraints, which currently allow * and ~, any
> thoughts on this?

I'd exclude * and ~ entirely in type sets except for neverallow.  In
every other case, we should be using type attributes (like domain or
file_type) and type set exclusion rather than * or ~.   For permission
sets, I think they are useful.  For role sets, I'm not sure - we don't
have a parallel to attributes for roles, so there is no easy way to say
all roles or all roles except X,Y,Z in any other way.

-- 
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: [PATCH] disallow * and ~ in rules

[Joshua Brindle]

On Thursday 23 June 2005 15:00, Stephen Smalley wrote:
> On Thu, 2005-06-23 at 14:47 -0400, Joshua Brindle wrote:
> > I agree that neverallow has very legitimate reasons for * but dontaudit
> > and auditallow are a little less clear. Perhaps auditallow should allow
> > *, it's conceivable that someone would want to audit all access to an
> > object (though in the current policies and reference policy * would have
> > the same net effect as domain with fewer avtab entries). This may not be
> > the case for all policies though. The attached patch disallows it in all
> > rule types but neverallow.
>
> Yes, I think requiring people to use an attribute like domain or
> file_type is preferable anyway; otherwise you end up with massive
> explosion in the set of rules for a lot of types that can't possibly be
> used in that manner anyway.
>

agreed.

> > I forgot to mention constraints, which currently allow * and ~, any
> > thoughts on this?
>
> I'd exclude * and ~ entirely in type sets except for neverallow.  In
> every other case, we should be using type attributes (like domain or
> file_type) and type set exclusion rather than * or ~.   For permission
> sets, I think they are useful.  For role sets, I'm not sure - we don't
> have a parallel to attributes for roles, so there is no easy way to say
> all roles or all roles except X,Y,Z in any other way.

in the constraint case * seems to be entirely unnecessary. However I'm not 
convinced that a compliment would never be useful in a constraint. 

As for roles, it certainly isn't an issue now but one can easily concieve a 
policy that creates a role for each user on the system. Then something like 
allow system_r * would actually make sense (err, more sense than now) but 
still isn't the best way, which would be adding the allow when the role is 
created. I don't think it's a problem to remove * and ~ from role sets, at 
least not yet.

Joshua

--
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: [PATCH] disallow * and ~ in rules

[Stephen Smalley]

On Thu, 2005-06-23 at 15:29 -0400, Joshua Brindle wrote:
> in the constraint case * seems to be entirely unnecessary. However I'm not 
> convinced that a compliment would never be useful in a constraint. 

I'm not sure why you'd ever use set complement (~b) rather than type set
exclusion (a -b).

> As for roles, it certainly isn't an issue now but one can easily concieve a 
> policy that creates a role for each user on the system. Then something like 
> allow system_r * would actually make sense (err, more sense than now) but 
> still isn't the best way, which would be adding the allow when the role is 
> created. I don't think it's a problem to remove * and ~ from role sets, at 
> least not yet.

If we do retain them (or later restore them), it occurs to me that they
need to exclude the implicitly defined object_r; otherwise, they are
useless.

-- 
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: [PATCH] disallow * and ~ in rules

[Joshua Brindle]

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

On Thursday 23 June 2005 16:19, you wrote:
> On Thu, 2005-06-23 at 15:29 -0400, Joshua Brindle wrote:
> > in the constraint case * seems to be entirely unnecessary. However I'm
> > not convinced that a compliment would never be useful in a constraint.
>
> I'm not sure why you'd ever use set complement (~b) rather than type set
> exclusion (a -b).
>
> > As for roles, it certainly isn't an issue now but one can easily concieve
> > a policy that creates a role for each user on the system. Then something
> > like allow system_r * would actually make sense (err, more sense than
> > now) but still isn't the best way, which would be adding the allow when
> > the role is created. I don't think it's a problem to remove * and ~ from
> > role sets, at least not yet.
>
> If we do retain them (or later restore them), it occurs to me that they
> need to exclude the implicitly defined object_r; otherwise, they are
> useless.

Fair enough, attached is hopefully the final patch, * and ~ disabled in all 
type sets other than neverallow and removed from role sets entirely.

Joshua

[-- Attachment #2: no-star-comp-allow.patch --]
[-- Type: text/x-diff, Size: 5975 bytes --]

Index: policy_parse.y
===================================================================
RCS file: /cvsroot/selinux/nsa/selinux-usr/checkpolicy/policy_parse.y,v
retrieving revision 1.31
diff -u -u -p -r1.31 policy_parse.y
--- policy_parse.y	13 May 2005 19:53:31 -0000	1.31
+++ policy_parse.y	23 Jun 2005 18:44:30 -0000
@@ -1781,12 +1781,17 @@ static char *type_val_to_name(unsigned i
 static int set_types(ebitmap_t *set,
 		     ebitmap_t *negset,
 		     char *id,
-		     int *add)
+		     int *add, 
+		     char starallowed)
 {
 	type_datum_t *t;
 	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
+		if (!starallowed) {
+			yyerror("* not allowed in this type of rule");
+			return -1;
+		}
 		/* set all types not in negset */
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (!ebitmap_get_bit(negset, i))
@@ -1797,6 +1802,10 @@ static int set_types(ebitmap_t *set,
 	}
 
 	if (strcmp(id, "~") == 0) {
+		if (!starallowed) {
+			yyerror("~ not allowed in this type of rule");
+			return -1;
+		}
 		/* complement the set */
 		for (i = 0; i < policydbp->p_types.nprim; i++) {
 			if (ebitmap_get_bit(set, i))
@@ -1893,14 +1902,14 @@ static int define_compute_type(int which
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2033,14 +2042,14 @@ static cond_av_list_t *define_cond_compu
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, 0))
 			return  COND_ERR;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, 0))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2468,7 +2477,7 @@ static cond_av_list_t *define_cond_te_av
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2479,7 +2488,7 @@ static cond_av_list_t *define_cond_te_av
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return COND_ERR;
 	}
 	ebitmap_destroy(&negset);
@@ -2646,7 +2655,7 @@ static int define_te_avtab(int which)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&stypes, &negset, id, &add))
+		if (set_types(&stypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2657,7 +2666,7 @@ static int define_te_avtab(int which)
 			self = 1;
 			continue;
 		}
-		if (set_types(&ttypes, &negset, id, &add))
+		if (set_types(&ttypes, &negset, id, &add, which == -AVTAB_ALLOWED? 1 : 0 ))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -2853,7 +2862,7 @@ static int define_role_types(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&role->types, &negset, id, &add))
+		if (set_types(&role->types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -3002,26 +3011,17 @@ static int set_roles(ebitmap_t *set,
 		     char *id)
 {
 	role_datum_t *r;
-	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
-		/* set all roles */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) 
-			ebitmap_set_bit(set, i, TRUE);
 		free(id);
-		return 0;
+		yyerror("* is not allowed for role sets");
+		return -1;
 	}
 
 	if (strcmp(id, "~") == 0) {
-		/* complement the set */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) {
-			if (ebitmap_get_bit(set, i))
-				ebitmap_set_bit(set, i, FALSE);
-			else 
-				ebitmap_set_bit(set, i, TRUE);
-		}
 		free(id);
-		return 0;
+		yyerror("~ is not allowed for role sets");
+		return -1;
 	}
 
 	r = hashtab_search(policydbp->p_roles.table, id);
@@ -3068,7 +3068,7 @@ static int define_role_trans(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, &negset, id, &add))
+		if (set_types(&types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
@@ -3493,7 +3493,7 @@ static uintptr_t
 				}
 				val = role->value;
 			} else if (expr->attr & CEXPR_TYPE) {
-				if (set_types(&expr->names, &negset, id, &add)) {
+				if (set_types(&expr->names, &negset, id, &add, 0)) {
 					free(expr);
 					return 0;
 				}
@@ -3862,23 +3862,15 @@ static int set_user_roles(ebitmap_t *set
 	unsigned int i;
 
 	if (strcmp(id, "*") == 0) {
-		/* set all roles */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) 
-			ebitmap_set_bit(set, i, TRUE);
 		free(id);
-		return 0;
+		yyerror("* not allowed in user declarations");
+		return -1;
 	}
 
 	if (strcmp(id, "~") == 0) {
-		/* complement the set */
-		for (i = 0; i < policydbp->p_roles.nprim; i++) {
-			if (ebitmap_get_bit(set, i))
-				ebitmap_set_bit(set, i, FALSE);
-			else 
-				ebitmap_set_bit(set, i, TRUE);
-		}
 		free(id);
-		return 0;
+		yyerror("~ not allowed in user declarations");
+		return -1;
 	}
 
 	r = hashtab_search(policydbp->p_roles.table, id);
@@ -4839,14 +4831,14 @@ static int define_range_trans(void)
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&doms, &negset, id, &add))
+		if (set_types(&doms, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);
 
 	ebitmap_init(&negset);
 	while ((id = queue_remove(id_queue))) {
-		if (set_types(&types, &negset, id, &add))
+		if (set_types(&types, &negset, id, &add, 0))
 			return -1;
 	}
 	ebitmap_destroy(&negset);

Re: [PATCH] disallow * and ~ in rules

[Stephen Smalley]

On Thu, 2005-06-23 at 16:36 -0400, Joshua Brindle wrote:
> Fair enough, attached is hopefully the final patch, * and ~ disabled in all 
> type sets other than neverallow and removed from role sets entirely.

Thanks, merged (checkpolicy 1.25.1).

-- 
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: [PATCH] disallow * and ~ in rules

[Russell Coker]

On Friday 24 June 2005 05:00, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Thu, 2005-06-23 at 14:47 -0400, Joshua Brindle wrote:
> > I agree that neverallow has very legitimate reasons for * but dontaudit
> > and auditallow are a little less clear. Perhaps auditallow should allow
> > *, it's conceivable that someone would want to audit all access to an
> > object (though in the current policies and reference policy * would have
> > the same net effect as domain with fewer avtab entries). This may not be
> > the case for all policies though. The attached patch disallows it in all
> > rule types but neverallow.
>
> Yes, I think requiring people to use an attribute like domain or
> file_type is preferable anyway; otherwise you end up with massive
> explosion in the set of rules for a lot of types that can't possibly be
> used in that manner anyway.

I agree for dontaudit, but disagree for auditallow.

Sometimes when debugging policy issues I want to see all the accesses to an 
object.  Writing rules that cover everything can be a drag, and running apol 
also takes time.  It's a lot easier to just do: auditallow * foo_t:file *;

> sets, I think they are useful.  For role sets, I'm not sure - we don't
> have a parallel to attributes for roles, so there is no easy way to say
> all roles or all roles except X,Y,Z in any other way.

Having role attributes would be handy.

The in_user_role() macro is a gross hack, role attributes would remove the 
need for it.

-- 
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: [PATCH] disallow * and ~ in rules

[Stephen Smalley]

On Fri, 2005-06-24 at 16:29 +1000, Russell Coker wrote:
> I agree for dontaudit, but disagree for auditallow.
> 
> Sometimes when debugging policy issues I want to see all the accesses to an 
> object.  Writing rules that cover everything can be a drag, and running apol 
> also takes time.  It's a lot easier to just do: auditallow * foo_t:file *;

But that can just as easily be written as:
	auditallow domain foo_t:file *;
with no loss in what it truly provides (and definite improvement in the
size of the resulting policy).  Note that we aren't eliminating use of *
in permission sets, just in type sets and role sets.  The problem with *
in type sets is that you never truly want all types (except in
assertions checking for policy errors); you only want "all process
types, i.e. domain" or "all file types, i.e. file_type", etc.

> Having role attributes would be handy.
> 
> The in_user_role() macro is a gross hack, role attributes would remove the 
> need for it.

Hmm...need a list somewhere that tracks all requests for improvements to
the policy language...

-- 
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: [PATCH] disallow * and ~ in rules

[Russell Coker]

On Friday 24 June 2005 21:35, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Fri, 2005-06-24 at 16:29 +1000, Russell Coker wrote:
> > I agree for dontaudit, but disagree for auditallow.
> >
> > Sometimes when debugging policy issues I want to see all the accesses to
> > an object.  Writing rules that cover everything can be a drag, and
> > running apol also takes time.  It's a lot easier to just do: auditallow *
> > foo_t:file *;
>
> But that can just as easily be written as:
> 	auditallow domain foo_t:file *;

I just did a quick test and found a .3% size difference.  I'm surprised, I had 
expected that as auditallow doesn't permit an operation the rules would not 
result in anything in the binary policy for an operation which is not 
permitted.  Does the current behavior really make sense?

> > Having role attributes would be handy.
> >
> > The in_user_role() macro is a gross hack, role attributes would remove
> > the need for it.
>
> Hmm...need a list somewhere that tracks all requests for improvements to
> the policy language...

I think that Tresys have an internal list.  When I was at the Symposium they 
were telling me about their work to implement a feature I had forgotten about 
requesting.

-- 
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: [PATCH] disallow * and ~ in rules

[Stephen Smalley]

On Fri, 2005-06-24 at 23:24 +1000, Russell Coker wrote:
> I just did a quick test and found a .3% size difference.  I'm surprised, I had 
> expected that as auditallow doesn't permit an operation the rules would not 
> result in anything in the binary policy for an operation which is not 
> permitted.  Does the current behavior really make sense?

checkpolicy isn't that smart, at least not presently ;)
Same core logic is shared for allow/auditallow/auditdeny/dontaudit, see
te_avtab_helper() in policy_parse.y.  Yes, we could have it optimize
away auditallow entries for which no corresponding allow entry exists.

-- 
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: [PATCH] disallow * and ~ in rules

[Karl MacMillan]

> -----Original Message-----
> From: owner-selinux@tycho.nsa.gov [mailto:owner-selinux@tycho.nsa.gov] On
> Behalf Of Stephen Smalley
> Sent: Friday, June 24, 2005 7:35 AM
> To: russell@coker.com.au
> Cc: Joshua Brindle; selinux
> Subject: Re: [PATCH] disallow * and ~ in rules
> 
> On Fri, 2005-06-24 at 16:29 +1000, Russell Coker wrote:
> > I agree for dontaudit, but disagree for auditallow.
> >
> > Sometimes when debugging policy issues I want to see all the accesses to an
> > object.  Writing rules that cover everything can be a drag, and running apol
> > also takes time.  It's a lot easier to just do: auditallow * foo_t:file *;
> 
> But that can just as easily be written as:
> 	auditallow domain foo_t:file *;
> with no loss in what it truly provides (and definite improvement in the
> size of the resulting policy).  Note that we aren't eliminating use of *
> in permission sets, just in type sets and role sets.  The problem with *
> in type sets is that you never truly want all types (except in
> assertions checking for policy errors); you only want "all process
> types, i.e. domain" or "all file types, i.e. file_type", etc.
> 
> > Having role attributes would be handy.
> >
> > The in_user_role() macro is a gross hack, role attributes would remove the
> > need for it.
> 
> Hmm...need a list somewhere that tracks all requests for improvements to
> the policy language...
> 

We can put this on the policy server sf site as long as everyone knows that we
can't address all of these requests as part of that project.

Karl

---
Karl MacMillan
Tresys Technology
http://www.tresys.com
(410) 290-1411 ext 134

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



--
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: [PATCH] disallow * and ~ in rules

[Joshua Brindle]

<snip>
> >
> > Hmm...need a list somewhere that tracks all requests for improvements to
> > the policy language...
>
> We can put this on the policy server sf site as long as everyone knows that
> we can't address all of these requests as part of that project.
>
http://sepolicy-server.sourceforge.net/index.php?page=compiler-wishlist

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