[PATCH 1/2] libsepol: Only apply bounds checking to source types in rules

[PATCH 1/2] libsepol: Only apply bounds checking to source types in rules

[Stephen Smalley]

The current bounds checking of both source and target types
requires allowing any domain that has access to the child domain
to also have the same permissions to the parent, which is undesirable.
Drop the target bounds checking.

Making this change fully functional requires a corresponding kernel
change; this change only allows one to build policies that would
otherwise violate the bounds checking on target type.  The kernel
change is required to allow the permissions at runtime.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libsepol/src/hierarchy.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
index 6f73195..ba65978 100644
--- a/libsepol/src/hierarchy.c
+++ b/libsepol/src/hierarchy.c
@@ -329,32 +329,6 @@ static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
 			if (rc) goto exit;
 		}
 	}
-	if (ebitmap_get_bit(&p->attr_type_map[tgt - 1], child - 1)) {
-		avtab_key.target_type = parent;
-		ebitmap_for_each_bit(&p->attr_type_map[src - 1], tnode, i) {
-			if (!ebitmap_node_get_bit(tnode, i))
-				continue;
-			avtab_key.source_type = i + 1;
-			if (avtab_key.source_type == child) {
-				/* Checked above */
-				continue;
-			}
-			d = bounds_not_covered(global_avtab, cur_avtab,
-					       &avtab_key, data);
-			if (!d) continue;
-			td = p->type_val_to_struct[i];
-			if (td && td->bounds) {
-				avtab_key.source_type = td->bounds;
-				d = bounds_not_covered(global_avtab, cur_avtab,
-						       &avtab_key, data);
-				if (!d) continue;
-			}
-			(*numbad)++;
-			rc = bounds_add_bad(handle, i+1, child, class, d, bad);
-			if (rc) goto exit;
-		}
-	}
-
 exit:
 	return rc;
 }
-- 
2.5.5

[PATCH 2/2] libsepol: fix type bounds checking for attributes

[Stephen Smalley]

The type bounds / hierarchy checker was incorrectly handling
rules with attributes as the target type, causing it to trigger
violations if the parent type lacked a rule directly to the same
attribute even if the parent had all the permissions to the
types associated with that attribute.  Skip the degenerate
case and only check for the individual types.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libsepol/src/hierarchy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
index ba65978..79db3a6 100644
--- a/libsepol/src/hierarchy.c
+++ b/libsepol/src/hierarchy.c
@@ -318,6 +318,8 @@ static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
 					       &avtab_key, data);
 			if (!d) continue;
 			td = p->type_val_to_struct[i];
+			if (!td || td->flavor == TYPE_ATTRIB)
+				continue;
 			if (td && td->bounds) {
 				avtab_key.target_type = td->bounds;
 				d = bounds_not_covered(global_avtab, cur_avtab,
-- 
2.5.5

Re: [PATCH 2/2] libsepol: fix type bounds checking for attributes

[James Carter]

On 04/28/2016 04:02 PM, Stephen Smalley wrote:
> The type bounds / hierarchy checker was incorrectly handling
> rules with attributes as the target type, causing it to trigger
> violations if the parent type lacked a rule directly to the same
> attribute even if the parent had all the permissions to the
> types associated with that attribute.  Skip the degenerate
> case and only check for the individual types.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>   libsepol/src/hierarchy.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
> index ba65978..79db3a6 100644
> --- a/libsepol/src/hierarchy.c
> +++ b/libsepol/src/hierarchy.c
> @@ -318,6 +318,8 @@ static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
>   					       &avtab_key, data);
>   			if (!d) continue;
>   			td = p->type_val_to_struct[i];
> +			if (!td || td->flavor == TYPE_ATTRIB)
> +				continue;
>   			if (td && td->bounds) {
>   				avtab_key.target_type = td->bounds;
>   				d = bounds_not_covered(global_avtab, cur_avtab,
>

There shouldn't be any attributes here. CIL is wrongly including the attribute 
itself in the attr_type_map. Patch coming.

Jim

-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency

Re: [PATCH 1/2] libsepol: Only apply bounds checking to source types in rules

[James Carter]

On 04/28/2016 04:02 PM, Stephen Smalley wrote:
> The current bounds checking of both source and target types
> requires allowing any domain that has access to the child domain
> to also have the same permissions to the parent, which is undesirable.
> Drop the target bounds checking.
>
> Making this change fully functional requires a corresponding kernel
> change; this change only allows one to build policies that would
> otherwise violate the bounds checking on target type.  The kernel
> change is required to allow the permissions at runtime.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>   libsepol/src/hierarchy.c | 26 --------------------------
>   1 file changed, 26 deletions(-)
>
> diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
> index 6f73195..ba65978 100644
> --- a/libsepol/src/hierarchy.c
> +++ b/libsepol/src/hierarchy.c
> @@ -329,32 +329,6 @@ static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
>   			if (rc) goto exit;
>   		}
>   	}
> -	if (ebitmap_get_bit(&p->attr_type_map[tgt - 1], child - 1)) {
> -		avtab_key.target_type = parent;
> -		ebitmap_for_each_bit(&p->attr_type_map[src - 1], tnode, i) {
> -			if (!ebitmap_node_get_bit(tnode, i))
> -				continue;
> -			avtab_key.source_type = i + 1;
> -			if (avtab_key.source_type == child) {
> -				/* Checked above */
> -				continue;
> -			}
> -			d = bounds_not_covered(global_avtab, cur_avtab,
> -					       &avtab_key, data);
> -			if (!d) continue;
> -			td = p->type_val_to_struct[i];
> -			if (td && td->bounds) {
> -				avtab_key.source_type = td->bounds;
> -				d = bounds_not_covered(global_avtab, cur_avtab,
> -						       &avtab_key, data);
> -				if (!d) continue;
> -			}
> -			(*numbad)++;
> -			rc = bounds_add_bad(handle, i+1, child, class, d, bad);
> -			if (rc) goto exit;
> -		}
> -	}
> -
>   exit:
>   	return rc;
>   }
>

We also don't need to expand the tgt. I will send a new and improved patch.

Jim


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency