All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking
@ 2015-06-17 19:58 James Carter
  2015-06-17 19:58 ` [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any() James Carter
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

V2 fixes the following issues:
 - Patch 02/10 uses ebitmap_cpy() instead of individual ebitmap_set_bit() calls.
 - Patch 03/10 just the offending permissions are reported during neverallow checking.
 - Patch 03/10 checks and handles failure on ebitmap_and() calls.
 - Patch 03/10 removes some of the duplicated logic between checking and reporting.
 - Patch 05/10 uses the newly added ebitmap_match_any() function instead of its own.
 - Patch 05/10 ignores macros and abstract blocks when walking the AST. If in the 
future there are other users of cil_find.c, then we can make this functionality 
configureable then.

This patch set refactors libsepol's neverallow and bounds checking, refactors CIL's
neverallow checking, and adds bounds checking to CIL.

It also results in improved memory usage and speed. With a test policy.conf derived
from Refpolicy with typebounds statements added (but no violations of neverallow
rules or type bounds) the following improvements are seen:
Before: 9,605M of memory and 22.5 sec.
Now   :   125M of memory and  2.1 sec.
(Memory usage as reported by valgrind)

In addition to adding bounds checking to CIL. This patch set also improves the error
reporting for neverallow rule violations and CIL will now report all violations
instead of stopping at the first one.

James Carter (10):
  libsepol: Add new ebitmap function named ebitmap_match_any()
  libsepol: Treat types like an attribute in the attr_type_map.
  libsepol: Refactored neverallow checking.
  libsepol: Refactored bounds (hierarchy) checking code
  libsepol/cil: Add function to search the CIL AST for an AV rule.
  libsepol/cil: Refactored CIL neverallow checking and reporting.
  libsepol/cil: Track number of classes and number of types and
    attributes.
  libsepol/cil: Add CIL bounds checking and reporting.
  secilc: Add a CIL policy file to test neverallow checking.
  secilc: Add a CIL policy file to test bounds checking.

 libsepol/cil/src/cil.c                      |    3 +-
 libsepol/cil/src/cil_binary.c               |  699 ++++++++++++++-----
 libsepol/cil/src/cil_binary.h               |    8 +-
 libsepol/cil/src/cil_find.c                 |  305 ++++++++
 libsepol/cil/src/cil_find.h                 |   39 ++
 libsepol/cil/src/cil_internal.h             |    2 +
 libsepol/cil/src/cil_post.c                 |   26 +-
 libsepol/include/sepol/policydb/ebitmap.h   |    1 +
 libsepol/include/sepol/policydb/hierarchy.h |   11 +
 libsepol/include/sepol/policydb/policydb.h  |    2 +-
 libsepol/src/assertion.c                    |  272 +++++---
 libsepol/src/ebitmap.c                      |   22 +
 libsepol/src/expand.c                       |   24 +-
 libsepol/src/hierarchy.c                    | 1000 +++++++++++++++++----------
 libsepol/src/policydb.c                     |    4 +
 secilc/test/bounds.cil                      |  241 +++++++
 secilc/test/neverallow.cil                  |   79 +++
 17 files changed, 2080 insertions(+), 658 deletions(-)
 create mode 100644 libsepol/cil/src/cil_find.c
 create mode 100644 libsepol/cil/src/cil_find.h
 create mode 100644 secilc/test/bounds.cil
 create mode 100644 secilc/test/neverallow.cil

-- 
1.9.3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any()
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-18 13:23   ` Stephen Smalley
  2015-06-17 19:58 ` [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map James Carter
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

This function returns true if there is a common bit that is set
in both bitmaps.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/include/sepol/policydb/ebitmap.h |  1 +
 libsepol/src/ebitmap.c                    | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
index 801438c..7b3508d 100644
--- a/libsepol/include/sepol/policydb/ebitmap.h
+++ b/libsepol/include/sepol/policydb/ebitmap.h
@@ -86,6 +86,7 @@ extern unsigned int ebitmap_cardinality(ebitmap_t *e1);
 extern int ebitmap_hamming_distance(ebitmap_t * e1, ebitmap_t * e2);
 extern int ebitmap_cpy(ebitmap_t * dst, const ebitmap_t * src);
 extern int ebitmap_contains(const ebitmap_t * e1, const ebitmap_t * e2);
+extern int ebitmap_match_any(const ebitmap_t *e1, const ebitmap_t *e2);
 extern int ebitmap_get_bit(const ebitmap_t * e, unsigned int bit);
 extern int ebitmap_set_bit(ebitmap_t * e, unsigned int bit, int value);
 extern void ebitmap_destroy(ebitmap_t * e);
diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
index be6b591..58f2fc4 100644
--- a/libsepol/src/ebitmap.c
+++ b/libsepol/src/ebitmap.c
@@ -224,6 +224,28 @@ int ebitmap_contains(const ebitmap_t * e1, const ebitmap_t * e2)
 	return 1;
 }
 
+int ebitmap_match_any(const ebitmap_t *e1, const ebitmap_t *e2)
+{
+	ebitmap_node_t *n1 = e1->node;
+	ebitmap_node_t *n2 = e2->node;
+
+	while (n1 && n2) {
+		if (n1->startbit < n2->startbit) {
+			n1 = n1->next;
+		} else if (n2->startbit < n1->startbit) {
+			n2 = n2->next;
+		} else {
+			if (n1->map & n2->map) {
+				return 1;
+			}
+			n1 = n1->next;
+			n2 = n2->next;
+		}
+	}
+
+	return 0;
+}
+
 int ebitmap_get_bit(const ebitmap_t * e, unsigned int bit)
 {
 	ebitmap_node_t *n;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
  2015-06-17 19:58 ` [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any() James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-18 13:41   ` Stephen Smalley
  2015-06-17 19:58 ` [PATCH 03/10 v2] libsepol: Refactored neverallow checking James Carter
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

Types are treated as attributes that contain only themselves. This
is how types are already treated in the type_attr_map.

Treating types this way makes finding rules that apply to a given
type much easier.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/src/expand.c   | 24 ++++++++++++++++--------
 libsepol/src/policydb.c |  4 ++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 478eaff..85fbe0f 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -2311,25 +2311,33 @@ static int type_attr_map(hashtab_key_t key
 	policydb_t *p = state->out;
 	unsigned int i;
 	ebitmap_node_t *tnode;
+	int value;
 
 	type = (type_datum_t *) datum;
+	value = type->s.value;
+
 	if (type->flavor == TYPE_ATTRIB) {
-		if (ebitmap_cpy(&p->attr_type_map[type->s.value - 1],
-				&type->types)) {
-			ERR(state->handle, "Out of memory!");
-			return -1;
+		if (ebitmap_cpy(&p->attr_type_map[value - 1], &type->types)) {
+			goto out;
 		}
 		ebitmap_for_each_bit(&type->types, tnode, i) {
 			if (!ebitmap_node_get_bit(tnode, i))
 				continue;
-			if (ebitmap_set_bit(&p->type_attr_map[i],
-					    type->s.value - 1, 1)) {
-				ERR(state->handle, "Out of memory!");
-				return -1;
+			if (ebitmap_set_bit(&p->type_attr_map[i], value - 1, 1)) {
+				goto out;
 			}
 		}
+	} else {
+		if (ebitmap_set_bit(&p->attr_type_map[value - 1], value - 1, 1)) {
+			goto out;
+		}
 	}
+
 	return 0;
+
+out:
+	ERR(state->handle, "Out of memory!");
+	return -1;
 }
 
 /* converts typeset using typemap and expands into ebitmap_t types using the attributes in the passed in policy.
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 1677eb5..670aef8 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -3936,6 +3936,10 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose)
 			/* add the type itself as the degenerate case */
 			if (ebitmap_set_bit(&p->type_attr_map[i], i, 1))
 				goto bad;
+			if (p->type_val_to_struct[i]->flavor != TYPE_ATTRIB) {
+				if (ebitmap_set_bit(&p->attr_type_map[i], i, 1))
+					goto bad;
+			}
 		}
 	}
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 03/10 v2] libsepol: Refactored neverallow checking.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
  2015-06-17 19:58 ` [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any() James Carter
  2015-06-17 19:58 ` [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-17 19:58 ` [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code James Carter
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

Instead of creating an expanded avtab, generating all of the avtab
keys corresponding to a neverallow rule and searching for a match,
walk the nodes in the avtab and use the attr_type_map and ebitmap
functions to find matching rules.

Memory usage is reduced from 370M to 125M and time is reduced from
14 sec to 2 sec. (Bounds checking commented out in both cases.)

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/include/sepol/policydb/policydb.h |   2 +-
 libsepol/src/assertion.c                   | 272 ++++++++++++++++++++---------
 2 files changed, 193 insertions(+), 81 deletions(-)

diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h
index 78b09c4..5da016b 100644
--- a/libsepol/include/sepol/policydb/policydb.h
+++ b/libsepol/include/sepol/policydb/policydb.h
@@ -649,7 +649,7 @@ extern void level_datum_init(level_datum_t * x);
 extern void level_datum_destroy(level_datum_t * x);
 extern void cat_datum_init(cat_datum_t * x);
 extern void cat_datum_destroy(cat_datum_t * x);
-
+extern int check_assertion(policydb_t *p, avrule_t *avrule);
 extern int check_assertions(sepol_handle_t * handle,
 			    policydb_t * p, avrule_t * avrules);
 
diff --git a/libsepol/src/assertion.c b/libsepol/src/assertion.c
index c335968..67a5ec4 100644
--- a/libsepol/src/assertion.c
+++ b/libsepol/src/assertion.c
@@ -27,11 +27,16 @@
 
 #include "debug.h"
 
-static void report_failure(sepol_handle_t *handle, policydb_t *p,
-			   const avrule_t * avrule,
+struct avtab_match_args {
+	sepol_handle_t *handle;
+	policydb_t *p;
+	avrule_t *avrule;
+	unsigned long errors;
+};
+
+static void report_failure(sepol_handle_t *handle, policydb_t *p, const avrule_t *avrule,
 			   unsigned int stype, unsigned int ttype,
-			   const class_perm_node_t *curperm,
-			   const avtab_ptr_t node)
+			   const class_perm_node_t *curperm, uint32_t perms)
 {
 	if (avrule->source_filename) {
 		ERR(handle, "neverallow on line %lu of %s (or line %lu of policy.conf) violated by allow %s %s:%s {%s };",
@@ -39,69 +44,208 @@ static void report_failure(sepol_handle_t *handle, policydb_t *p,
 		    p->p_type_val_to_name[stype],
 		    p->p_type_val_to_name[ttype],
 		    p->p_class_val_to_name[curperm->tclass - 1],
-		    sepol_av_to_string(p, curperm->tclass,
-				       node->datum.data & curperm->data));
+		    sepol_av_to_string(p, curperm->tclass, perms));
 	} else if (avrule->line) {
 		ERR(handle, "neverallow on line %lu violated by allow %s %s:%s {%s };",
 		    avrule->line, p->p_type_val_to_name[stype],
 		    p->p_type_val_to_name[ttype],
 		    p->p_class_val_to_name[curperm->tclass - 1],
-		    sepol_av_to_string(p, curperm->tclass,
-				       node->datum.data & curperm->data));
+		    sepol_av_to_string(p, curperm->tclass, perms));
 	} else {
 		ERR(handle, "neverallow violated by allow %s %s:%s {%s };",
 		    p->p_type_val_to_name[stype],
 		    p->p_type_val_to_name[ttype],
 		    p->p_class_val_to_name[curperm->tclass - 1],
-		    sepol_av_to_string(p, curperm->tclass,
-				       node->datum.data & curperm->data));
+		    sepol_av_to_string(p, curperm->tclass, perms));
 	}
 }
 
-static unsigned long check_assertion_helper(sepol_handle_t * handle,
-				  policydb_t * p,
-				  avtab_t * te_avtab, avtab_t * te_cond_avtab,
-				  unsigned int stype, unsigned int ttype,
-				  const avrule_t * avrule)
+static int match_any_class_permissions(class_perm_node_t *cp, uint32_t class, uint32_t data)
 {
-	avtab_key_t avkey;
-	avtab_ptr_t node;
-	class_perm_node_t *curperm;
-	unsigned long errors = 0;
+	for (; cp; cp = cp->next) {
+		if ((cp->tclass == class) && (cp->data & data)) {
+			break;
+		}
+	}
+	if (!cp)
+		return 0;
 
-	for (curperm = avrule->perms; curperm != NULL; curperm = curperm->next) {
-		avkey.source_type = stype + 1;
-		avkey.target_type = ttype + 1;
-		avkey.target_class = curperm->tclass;
-		avkey.specified = AVTAB_ALLOWED;
-		for (node = avtab_search_node(te_avtab, &avkey);
-		     node != NULL;
-		     node = avtab_search_node_next(node, avkey.specified)) {
-			if (node->datum.data & curperm->data) {
-				report_failure(handle, p, avrule, stype, ttype, curperm, node);
-				errors++;
-			}
+	return 1;
+}
+
+
+static int report_assertion_avtab_matches(avtab_key_t *k, avtab_datum_t *d, void *args)
+{
+	int rc = 0;
+	struct avtab_match_args *a = (struct avtab_match_args *)args;
+	sepol_handle_t *handle = a->handle;
+	policydb_t *p = a->p;
+	avrule_t *avrule = a->avrule;
+	class_perm_node_t *cp;
+	uint32_t perms;
+	ebitmap_t src_matches, tgt_matches, matches;
+	ebitmap_node_t *snode, *tnode;
+	unsigned int i, j;
+
+	if (k->specified != AVTAB_ALLOWED)
+		return 0;
+
+	if (!match_any_class_permissions(avrule->perms, k->target_class, d->data))
+		return 0;
+
+	ebitmap_init(&src_matches);
+	ebitmap_init(&tgt_matches);
+	ebitmap_init(&matches);
+
+	rc = ebitmap_and(&src_matches, &avrule->stypes.types,
+			 &p->attr_type_map[k->source_type - 1]);
+	if (rc)
+		goto oom;
+
+	if (ebitmap_length(&src_matches) == 0)
+		goto exit;
+
+	if (avrule->flags == RULE_SELF) {
+		rc = ebitmap_and(&matches, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1]);
+		if (rc)
+			goto oom;
+		rc = ebitmap_and(&tgt_matches, &avrule->stypes.types, &matches);
+		if (rc)
+			goto oom;
+	} else {
+		rc = ebitmap_and(&tgt_matches, &avrule->ttypes.types, &p->attr_type_map[k->target_type -1]);
+		if (rc)
+			goto oom;
+	}
+
+	if (ebitmap_length(&tgt_matches) == 0)
+		goto exit;
+
+	for (cp = avrule->perms; cp; cp = cp->next) {
+		perms = cp->data & d->data;
+		if ((cp->tclass != k->target_class) || !perms) {
+			continue;
 		}
-		for (node = avtab_search_node(te_cond_avtab, &avkey);
-		     node != NULL;
-		     node = avtab_search_node_next(node, avkey.specified)) {
-			if (node->datum.data & curperm->data) {
-				report_failure(handle, p, avrule, stype, ttype, curperm, node);
-				errors++;
+
+		ebitmap_for_each_bit(&src_matches, snode, i) {
+			if (!ebitmap_node_get_bit(snode, i))
+				continue;
+			ebitmap_for_each_bit(&tgt_matches, tnode, j) {
+				if (!ebitmap_node_get_bit(tnode, j))
+					continue;
+				a->errors++;
+				report_failure(handle, p, avrule, i, j, cp, perms);
 			}
 		}
 	}
 
-	return errors;
+	goto exit;
+
+oom:
+	ERR(NULL, "Out of memory - unable to check neverallows");
+	
+exit:
+	ebitmap_destroy(&src_matches);
+	ebitmap_destroy(&tgt_matches);
+	ebitmap_destroy(&matches);
+	return rc;
+}
+
+int report_assertion_failures(sepol_handle_t *handle, policydb_t *p, avrule_t *avrule)
+{
+	int rc;
+	struct avtab_match_args args;
+
+	args.handle = handle;
+	args.p = p;
+	args.avrule = avrule;
+	args.errors = 0;
+
+	rc = avtab_map(&p->te_avtab, report_assertion_avtab_matches, &args);
+	if (rc)
+		goto oom;
+
+	rc = avtab_map(&p->te_cond_avtab, report_assertion_avtab_matches, &args);
+	if (rc)
+		goto oom;
+
+	return args.errors;
+
+oom:
+	return rc;
+}
+
+static int check_assertion_avtab_match(avtab_key_t *k, avtab_datum_t *d, void *args)
+{
+	int rc;
+	struct avtab_match_args *a = (struct avtab_match_args *)args;
+	policydb_t *p = a->p;
+	avrule_t *avrule = a->avrule;
+
+	if (k->specified != AVTAB_ALLOWED)
+		goto exit;
+
+	if (!match_any_class_permissions(avrule->perms, k->target_class, d->data))
+		goto exit;
+
+	rc = ebitmap_match_any(&avrule->stypes.types, &p->attr_type_map[k->source_type - 1]);
+	if (rc == 0)
+		goto exit;
+
+	if (avrule->flags == RULE_SELF) {
+		/* If the neverallow uses SELF, then it is not enough that the
+		 * neverallow's source matches the src and tgt of the rule being checked.
+		 * It must match the same thing in the src and tgt, so AND the source
+		 * and target together and check for a match on the result.
+		 */
+		ebitmap_t match;
+		rc = ebitmap_and(&match, &p->attr_type_map[k->source_type - 1], &p->attr_type_map[k->target_type - 1] );
+		if (rc) {
+			ebitmap_destroy(&match);
+			goto oom;
+		}
+		rc = ebitmap_match_any(&avrule->stypes.types, &match);
+		ebitmap_destroy(&match);
+	} else {
+		rc = ebitmap_match_any(&avrule->ttypes.types, &p->attr_type_map[k->target_type -1]);
+	}
+	if (rc == 0)
+		goto exit;
+
+	return 1;
+
+exit:
+	return 0;
+
+oom:
+	ERR(NULL, "Out of memory - unable to check neverallows");
+	return rc;
+}
+
+int check_assertion(policydb_t *p, avrule_t *avrule)
+{
+	int rc;
+	struct avtab_match_args args;
+
+	args.handle = NULL;
+	args.p = p;
+	args.avrule = avrule;
+	args.errors = 0;
+
+	rc = avtab_map(&p->te_avtab, check_assertion_avtab_match, &args);
+
+	if (rc == 0) {
+		rc = avtab_map(&p->te_cond_avtab, check_assertion_avtab_match, &args);
+	}
+
+	return rc;
 }
 
 int check_assertions(sepol_handle_t * handle, policydb_t * p,
 		     avrule_t * avrules)
 {
+	int rc;
 	avrule_t *a;
-	avtab_t te_avtab, te_cond_avtab;
-	ebitmap_node_t *snode, *tnode;
-	unsigned int i, j;
 	unsigned long errors = 0;
 
 	if (!avrules) {
@@ -111,54 +255,22 @@ int check_assertions(sepol_handle_t * handle, policydb_t * p,
 		return 0;
 	}
 
-	if (avrules) {
-		if (avtab_init(&te_avtab))
-			goto oom;
-		if (avtab_init(&te_cond_avtab)) {
-			avtab_destroy(&te_avtab);
-			goto oom;
-		}
-		if (expand_avtab(p, &p->te_avtab, &te_avtab) ||
-		    expand_avtab(p, &p->te_cond_avtab, &te_cond_avtab)) {
-			avtab_destroy(&te_avtab);
-			avtab_destroy(&te_cond_avtab);
-			goto oom;
-		}
-	}
-
 	for (a = avrules; a != NULL; a = a->next) {
-		ebitmap_t *stypes = &a->stypes.types;
-		ebitmap_t *ttypes = &a->ttypes.types;
-
 		if (!(a->specified & AVRULE_NEVERALLOW))
 			continue;
-
-		ebitmap_for_each_bit(stypes, snode, i) {
-			if (!ebitmap_node_get_bit(snode, i))
-				continue;
-			if (a->flags & RULE_SELF) {
-				errors += check_assertion_helper
-				    (handle, p, &te_avtab, &te_cond_avtab, i, i,
-				     a);
-			}
-			ebitmap_for_each_bit(ttypes, tnode, j) {
-				if (!ebitmap_node_get_bit(tnode, j))
-					continue;
-				errors += check_assertion_helper
-				    (handle, p, &te_avtab, &te_cond_avtab, i, j,
-				     a);
+		rc = check_assertion(p, a);
+		if (rc) {
+			rc = report_assertion_failures(handle, p, a);
+			if (rc < 0) {
+				ERR(handle, "Error occurred while checking neverallows");
+				return -1;
 			}
+			errors += rc;
 		}
 	}
 
 	if (errors)
 		ERR(handle, "%lu neverallow failures occurred", errors);
 
-	avtab_destroy(&te_avtab);
-	avtab_destroy(&te_cond_avtab);
 	return errors ? -1 : 0;
-
-      oom:
-	ERR(handle, "Out of memory - unable to check neverallows");
-	return -1;
 }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (2 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 03/10 v2] libsepol: Refactored neverallow checking James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-18 13:56   ` Stephen Smalley
  2015-06-17 19:58 ` [PATCH 05/10 v2] libsepol/cil: Add function to search the CIL AST for an AV rule James Carter
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

The largest change to the user and role bounds checking was to put
them in their own functions, so they could be called independently.

The type bounds checking was changed to check one type bounds at
a time. An expanded avtab is still created, but now only the rules
of the parent type are expanded. If violations are discovered,
a list of avtab_ptr_t's provides details. This list is used to
display error messages for backwards compatibility and will be
used by CIL to provide a more detailed error message.

Memory usage is reduced from 9,355M to 126M and time is reduced
from 9 sec to 2 sec.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/include/sepol/policydb/hierarchy.h |   11 +
 libsepol/src/hierarchy.c                    | 1000 +++++++++++++++++----------
 2 files changed, 631 insertions(+), 380 deletions(-)

diff --git a/libsepol/include/sepol/policydb/hierarchy.h b/libsepol/include/sepol/policydb/hierarchy.h
index b4eb9bc..88bc02e 100644
--- a/libsepol/include/sepol/policydb/hierarchy.h
+++ b/libsepol/include/sepol/policydb/hierarchy.h
@@ -25,11 +25,22 @@
 #ifndef _SEPOL_POLICYDB_HIERARCHY_H_
 #define _SEPOL_POLICYDB_HIERARCHY_H_
 
+#include <sepol/policydb/avtab.h>
 #include <sepol/policydb/policydb.h>
 #include <sys/cdefs.h>
 
 __BEGIN_DECLS
 
+extern int hierarchy_add_bounds(sepol_handle_t *handle, policydb_t *p);
+
+extern void bounds_destroy_bad(avtab_ptr_t cur);
+extern int bounds_check_type(sepol_handle_t *handle, policydb_t *p, uint32_t child,
+			     uint32_t parent, avtab_ptr_t *bad, int *numbad);
+
+extern int bounds_check_users(sepol_handle_t *handle, policydb_t *p);
+extern int bounds_check_roles(sepol_handle_t *handle, policydb_t *p);
+extern int bounds_check_types(sepol_handle_t *handle, policydb_t *p);
+
 extern int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p);
 
 __END_DECLS
diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
index d787a64..2436837 100644
--- a/libsepol/src/hierarchy.c
+++ b/libsepol/src/hierarchy.c
@@ -37,466 +37,706 @@
 
 #include "debug.h"
 
-typedef struct hierarchy_args {
-	policydb_t *p;
-	avtab_t *expa;		/* expanded avtab */
-	/* This tells check_avtab_hierarchy to check this list in addition to the unconditional avtab */
-	cond_av_list_t *opt_cond_list;
-	sepol_handle_t *handle;
-	int numerr;
-} hierarchy_args_t;
+#define BOUNDS_AVTAB_SIZE 1024
 
-/*
- * find_parent_(type|role|user)
- *
- * This function returns the parent datum of given XXX_datum_t
- * object or NULL, if it doesn't exist.
- *
- * If the given datum has a valid bounds, this function merely
- * returns the indicated object. Otherwise, it looks up the
- * parent based on the based hierarchy.
- */
-#define find_parent_template(prefix)				\
-int find_parent_##prefix(hierarchy_args_t *a,			\
-			 prefix##_datum_t *datum,		\
-			 prefix##_datum_t **parent)		\
-{								\
-	char *parent_name, *datum_name, *tmp;			\
-								\
-	if (datum->bounds)						\
-		*parent = a->p->prefix##_val_to_struct[datum->bounds - 1]; \
-	else {								\
-		datum_name = a->p->p_##prefix##_val_to_name[datum->s.value - 1]; \
-									\
-		tmp = strrchr(datum_name, '.');				\
-		/* no '.' means it has no parent */			\
-		if (!tmp) {						\
-			*parent = NULL;					\
-			return 0;					\
-		}							\
-									\
-		parent_name = strdup(datum_name);			\
-		if (!parent_name)					\
-			return -1;					\
-		parent_name[tmp - datum_name] = '\0';			\
-									\
-		*parent = hashtab_search(a->p->p_##prefix##s.table, parent_name); \
-		if (!*parent) {						\
-			/* Orphan type/role/user */			\
-			ERR(a->handle,					\
-			    "%s doesn't exist, %s is an orphan",	\
-			    parent_name,				\
-			    a->p->p_##prefix##_val_to_name[datum->s.value - 1]); \
-			free(parent_name);				\
-			return -1;					\
-		}							\
-		free(parent_name);					\
-	}								\
-									\
-	return 0;							\
+static int bounds_insert_helper(sepol_handle_t *handle, avtab_t *avtab,
+				avtab_key_t *avtab_key, avtab_datum_t *datum)
+{
+	int rc = avtab_insert(avtab, avtab_key, datum);
+	if (rc) {
+		if (rc == SEPOL_ENOMEM)
+			ERR(handle, "Insufficient memory");
+		else
+			ERR(handle, "Unexpected error (%d)", rc);
+	}
+	return rc;
 }
 
-static find_parent_template(type)
-static find_parent_template(role)
-static find_parent_template(user)
 
-static void compute_avtab_datum(hierarchy_args_t *args,
-				avtab_key_t *key,
-				avtab_datum_t *result)
+static int bounds_insert_rule(sepol_handle_t *handle, avtab_t *avtab,
+			      avtab_t *global, avtab_t *other,
+			      avtab_key_t *avtab_key, avtab_datum_t *datum)
 {
-	avtab_datum_t *avdatp;
-	uint32_t av = 0;
-
-	avdatp = avtab_search(args->expa, key);
-	if (avdatp)
-		av = avdatp->data;
-	if (args->opt_cond_list) {
-		avdatp = cond_av_list_search(key, args->opt_cond_list);
-		if (avdatp)
-			av |= avdatp->data;
+	int rc = 0;
+	avtab_datum_t *dup = avtab_search(avtab, avtab_key);
+
+	if (!dup) {
+		rc = bounds_insert_helper(handle, avtab, avtab_key, datum);
+		if (rc) goto exit;
+	} else {
+		dup->data |= datum->data;
 	}
 
-	result->data = av;
+	if (other) {
+		/* Search the other conditional avtab for the key and
+		 * add any common permissions to the global avtab
+		 */
+		uint32_t data = 0;
+		dup = avtab_search(other, avtab_key);
+		if (dup) {
+			data = dup->data & datum->data;
+			if (data) {
+				dup = avtab_search(global, avtab_key);
+				if (!dup) {
+					avtab_datum_t d;
+					d.data = data;
+					rc = bounds_insert_helper(handle, global,
+								  avtab_key, &d);
+					if (rc) goto exit;
+				} else {
+					dup->data |= data;
+				}
+			}
+		}
+	}
+
+exit:
+	return rc;
 }
 
-/* This function verifies that the type passed in either has a parent or is in the 
- * root of the namespace, 0 on success, 1 on orphan and -1 on error
- */
-static int check_type_hierarchy_callback(hashtab_key_t k, hashtab_datum_t d,
-					 void *args)
+static int bounds_expand_rule(sepol_handle_t *handle, policydb_t *p,
+			      avtab_t *avtab, avtab_t *global, avtab_t *other,
+			      uint32_t parent, uint32_t src, uint32_t tgt,
+			      uint32_t class, uint32_t data)
 {
-	hierarchy_args_t *a;
-	type_datum_t *t, *tp;
-
-	a = (hierarchy_args_t *) args;
-	t = (type_datum_t *) d;
+	int rc = 0;
+	avtab_key_t avtab_key;
+	avtab_datum_t datum;
+	ebitmap_node_t *tnode;
+	unsigned int i;
+
+	avtab_key.specified = AVTAB_ALLOWED;
+	avtab_key.target_class = class;
+	datum.data = data;
+
+	if (ebitmap_get_bit(&p->attr_type_map[src - 1], parent - 1)) {
+		avtab_key.source_type = parent;
+		ebitmap_for_each_bit(&p->attr_type_map[tgt - 1], tnode, i) {
+			if (!ebitmap_node_get_bit(tnode, i))
+				continue;
+			avtab_key.target_type = i + 1;
+			rc = bounds_insert_rule(handle, avtab, global, other,
+						&avtab_key, &datum);
+			if (rc) goto exit;
+		}
+	}
 
-	if (t->flavor == TYPE_ATTRIB) {
-		/* It's an attribute, we don't care */
-		return 0;
+	if (ebitmap_get_bit(&p->attr_type_map[tgt - 1], parent - 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;
+			rc = bounds_insert_rule(handle, avtab, global, other,
+						&avtab_key, &datum);
+			if (rc) goto exit;
+		}
 	}
-	if (find_parent_type(a, t, &tp) < 0)
-		return -1;
-
-	if (tp && tp->flavor == TYPE_ATTRIB) {
-		/* The parent is an attribute but the child isn't, not legal */
-		ERR(a->handle, "type %s is a child of an attribute %s",
-		    (char *) k, a->p->p_type_val_to_name[tp->s.value - 1]);
-		a->numerr++;
-		return -1;
+
+exit:
+	return rc;
+}
+
+static int bounds_expand_cond_rules(sepol_handle_t *handle, policydb_t *p,
+				    cond_av_list_t *cur, avtab_t *avtab,
+				    avtab_t *global, avtab_t *other,
+				    uint32_t parent)
+{
+	int rc = 0;
+
+	for (; cur; cur = cur->next) {
+		avtab_ptr_t n = cur->node;
+		rc = bounds_expand_rule(handle, p, avtab, global, other, parent,
+					n->key.source_type, n->key.target_type,
+					n->key.target_class, n->datum.data);
+		if (rc) goto exit;
 	}
-	return 0;
+
+exit:
+	return rc;
 }
 
-/* This function only verifies that the avtab node passed in does not violate any
- * hiearchy constraint via any relationship with other types in the avtab.
- * it should be called using avtab_map, returns 0 on success, 1 on violation and
- * -1 on error. opt_cond_list is an optional argument that tells this to check
- * a conditional list for the relationship as well as the unconditional avtab
- */
-static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d,
-					  void *args)
+struct bounds_expand_args {
+	sepol_handle_t *handle;
+	policydb_t *p;
+	avtab_t *avtab;
+	uint32_t parent;
+};
+
+static int bounds_expand_rule_callback(avtab_key_t *k, avtab_datum_t *d,
+				       void *args)
 {
-	avtab_key_t key;
-	hierarchy_args_t *a = (hierarchy_args_t *) args;
-	type_datum_t *s, *t1 = NULL, *t2 = NULL;
-	avtab_datum_t av;
+	struct bounds_expand_args *a = (struct bounds_expand_args *)args;
 
-	if (!(k->specified & AVTAB_ALLOWED)) {
-		/* This is not an allow rule, no checking done */
+	if (!(k->specified & AVTAB_ALLOWED))
 		return 0;
-	}
 
-	/* search for parent first */
-	s = a->p->type_val_to_struct[k->source_type - 1];
-	if (find_parent_type(a, s, &t1) < 0)
-		return -1;
-	if (t1) {
-		/*
-		 * search for access allowed between type 1's
-		 * parent and type 2.
-		 */
-		key.source_type = t1->s.value;
-		key.target_type = k->target_type;
-		key.target_class = k->target_class;
-		key.specified = AVTAB_ALLOWED;
-		compute_avtab_datum(a, &key, &av);
-
-		if ((av.data & d->data) == d->data)
-			return 0;
+	return bounds_expand_rule(a->handle, a->p, a->avtab, NULL, NULL,
+				  a->parent, k->source_type, k->target_type,
+				  k->target_class, d->data);
+}
+
+struct bounds_cond_info {
+	avtab_t true_avtab;
+	avtab_t false_avtab;
+	cond_list_t *cond_list;
+	struct bounds_cond_info *next;
+};
+
+static void bounds_destroy_cond_info(struct bounds_cond_info *cur)
+{
+	struct bounds_cond_info *next;
+
+	for (; cur; cur = next) {
+		next = cur->next;
+		avtab_destroy(&cur->true_avtab);
+		avtab_destroy(&cur->false_avtab);
+		cur->next = NULL;
+		free(cur);
 	}
+}
 
-	/* next we try type 1 and type 2's parent */
-	s = a->p->type_val_to_struct[k->target_type - 1];
-	if (find_parent_type(a, s, &t2) < 0)
-		return -1;
-	if (t2) {
-		/*
-		 * search for access allowed between type 1 and
-		 * type 2's parent.
-		 */
-		key.source_type = k->source_type;
-		key.target_type = t2->s.value;
-		key.target_class = k->target_class;
-		key.specified = AVTAB_ALLOWED;
-		compute_avtab_datum(a, &key, &av);
-
-		if ((av.data & d->data) == d->data)
-			return 0;
+static int bounds_expand_parent_rules(sepol_handle_t *handle, policydb_t *p,
+				      avtab_t *global_avtab,
+				      struct bounds_cond_info **cond_info,
+				      uint32_t child, uint32_t parent)
+{
+	int rc = 0;
+	struct bounds_expand_args args;
+	cond_list_t *cur;
+
+	avtab_init(global_avtab);
+	rc = avtab_alloc(global_avtab, BOUNDS_AVTAB_SIZE);
+	if (rc) goto oom;
+
+	args.handle = handle;
+	args.p = p;
+	args.avtab = global_avtab;
+	args.parent = parent;
+	rc = avtab_map(&p->te_avtab, bounds_expand_rule_callback, &args);
+	if (rc) goto exit;
+
+	*cond_info = NULL;
+	for (cur = p->cond_list; cur; cur = cur->next) {
+		struct bounds_cond_info *ci;
+		ci = malloc(sizeof(struct bounds_cond_info));
+		if (!ci) goto oom;
+		avtab_init(&ci->true_avtab);
+		avtab_init(&ci->false_avtab);
+		ci->cond_list = cur;
+		ci->next = *cond_info;
+		*cond_info = ci;
+		if (cur->true_list) {
+			rc = avtab_alloc(&ci->true_avtab, BOUNDS_AVTAB_SIZE);
+			if (rc) goto oom;
+			rc = bounds_expand_cond_rules(handle, p, cur->true_list,
+						      &ci->true_avtab, NULL,
+						      NULL, parent);
+			if (rc) goto exit;
+		}
+		if (cur->false_list) {
+			rc = avtab_alloc(&ci->false_avtab, BOUNDS_AVTAB_SIZE);
+			if (rc) goto oom;
+			rc = bounds_expand_cond_rules(handle, p, cur->false_list,
+						      &ci->false_avtab,
+						      global_avtab,
+						      &ci->true_avtab, parent);
+			if (rc) goto exit;
+		}
 	}
 
-	if (t1 && t2) {
-		/*
-                 * search for access allowed between type 1's parent
-                 * and type 2's parent.
-                 */
-		key.source_type = t1->s.value;
-		key.target_type = t2->s.value;
-		key.target_class = k->target_class;
-		key.specified = AVTAB_ALLOWED;
-		compute_avtab_datum(a, &key, &av);
-
-		if ((av.data & d->data) == d->data)
-			return 0;
+	return 0;
+
+oom:
+	ERR(handle, "Insufficient memory");
+
+exit:
+	ERR(handle,"Failed to expand parent rules\n");
+	avtab_destroy(global_avtab);
+	bounds_destroy_cond_info(*cond_info);
+	*cond_info = NULL;
+	return rc;
+}
+
+static int bounds_not_covered(policydb_t *p, avtab_t *global_avtab,
+			      avtab_t *cur_avtab, avtab_key_t *avtab_key,
+			      uint32_t data)
+{
+	avtab_datum_t *datum = avtab_search(cur_avtab, avtab_key);
+	if (datum)
+		data &= ~datum->data;
+	if (global_avtab && data) {
+		datum = avtab_search(global_avtab, avtab_key);
+		if (datum)
+			data &= ~datum->data;
 	}
 
-	/*
-	 * Neither one of these types have parents and 
-	 * therefore the hierarchical constraint does not apply
-	 */
-	if (!t1 && !t2)
-		return 0;
+	return data;
+}
+
+static int bounds_add_bad(sepol_handle_t *handle, uint32_t src, uint32_t tgt,
+			  uint32_t class, uint32_t data, avtab_ptr_t *bad)
+{
+	struct avtab_node *new = malloc(sizeof(struct avtab_node));
+	if (new == NULL) {
+		ERR(handle, "Insufficient memory");
+		return SEPOL_ENOMEM;
+	}
+	memset(new, 0, sizeof(struct avtab_node));
+	new->key.source_type = src;
+	new->key.target_type = tgt;
+	new->key.target_class = class;
+	new->datum.data = data;
+	new->next = *bad;
+	*bad = new;
 
-	/*
-	 * At this point there is a violation of the hierarchal
-	 * constraint, send error condition back
-	 */
-	ERR(a->handle,
-	    "hierarchy violation between types %s and %s : %s { %s }",
-	    a->p->p_type_val_to_name[k->source_type - 1],
-	    a->p->p_type_val_to_name[k->target_type - 1],
-	    a->p->p_class_val_to_name[k->target_class - 1],
-	    sepol_av_to_string(a->p, k->target_class, d->data & ~av.data));
-	a->numerr++;
 	return 0;
 }
 
-/*
- * If same permissions are allowed for same combination of
- * source and target, we can evaluate them as unconditional
- * one.
- * See the following example. A_t type is bounds of B_t type,
- * so B_t can never have wider permissions then A_t.
- * A_t has conditional permission on X_t, however, a part of
- * them (getattr and read) are unconditionaly allowed to A_t.
- *
- * Example)
- * typebounds A_t B_t;
- *
- * allow B_t X_t : file { getattr };
- * if (foo_bool) {
- *     allow A_t X_t : file { getattr read };
- * } else {
- *     allow A_t X_t : file { getattr read write };
- * }
- *
- * We have to pull up them as unconditional ones in this case,
- * because it seems to us B_t is violated to bounds constraints
- * during unconditional policy checking.
- */
-static int pullup_unconditional_perms(cond_list_t * cond_list,
-				      hierarchy_args_t * args)
+static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
+			     avtab_t *global_avtab, avtab_t *cur_avtab,
+			     uint32_t child, uint32_t parent, uint32_t src,
+			     uint32_t tgt, uint32_t class, uint32_t data,
+			     avtab_ptr_t *bad, int *numbad)
 {
-	cond_list_t *cur_node;
-	cond_av_list_t *cur_av, *expl_true = NULL, *expl_false = NULL;
-	avtab_t expa_true, expa_false;
-	avtab_datum_t *avdatp;
-	avtab_datum_t avdat;
-	avtab_ptr_t avnode;
-
-	for (cur_node = cond_list; cur_node; cur_node = cur_node->next) {
-		if (avtab_init(&expa_true))
-			goto oom0;
-		if (avtab_init(&expa_false))
-			goto oom1;
-		if (expand_cond_av_list(args->p, cur_node->true_list,
-					&expl_true, &expa_true))
-			goto oom2;
-		if (expand_cond_av_list(args->p, cur_node->false_list,
-					&expl_false, &expa_false))
-			goto oom3;
-		for (cur_av = expl_true; cur_av; cur_av = cur_av->next) {
-			avdatp = avtab_search(&expa_false,
-					      &cur_av->node->key);
-			if (!avdatp)
+	int rc = 0;
+	avtab_key_t avtab_key;
+	type_datum_t *td;
+	ebitmap_node_t *tnode;
+	unsigned int i;
+	uint32_t d;
+
+	avtab_key.specified = AVTAB_ALLOWED;
+	avtab_key.target_class = class;
+
+	if (ebitmap_get_bit(&p->attr_type_map[src - 1], child - 1)) {
+		avtab_key.source_type = parent;
+		ebitmap_for_each_bit(&p->attr_type_map[tgt - 1], tnode, i) {
+			if (!ebitmap_node_get_bit(tnode, i))
 				continue;
-
-			avdat.data = (cur_av->node->datum.data
-				      & avdatp->data);
-			if (!avdat.data)
+			avtab_key.target_type = i + 1;
+			d = bounds_not_covered(p, global_avtab, cur_avtab,
+					       &avtab_key, data);
+			if (!d) continue;
+			td = p->type_val_to_struct[i];
+			if (td && td->bounds) {
+				avtab_key.target_type = td->bounds;
+				d = bounds_not_covered(p, global_avtab,
+						       cur_avtab, &avtab_key,
+						       data);
+				if (!d) continue;
+			}
+			(*numbad)++;
+			rc = bounds_add_bad(handle, child, i+1, class, d, bad);
+			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;
-
-			avnode = avtab_search_node(args->expa,
-						   &cur_av->node->key);
-			if (avnode) {
-				avnode->datum.data |= avdat.data;
-			} else {
-				if (avtab_insert(args->expa,
-						 &cur_av->node->key,
-						 &avdat))
-					goto oom4;
+			avtab_key.source_type = i + 1;
+			if (avtab_key.source_type == child) {
+				/* Checked above */
+				continue;
+			}
+			d = bounds_not_covered(p, 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(p, 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;
 		}
-		cond_av_list_destroy(expl_false);
-		cond_av_list_destroy(expl_true);
-		avtab_destroy(&expa_false);
-		avtab_destroy(&expa_true);
 	}
-	return 0;
 
-oom4:
-	cond_av_list_destroy(expl_false);
-oom3:
-	cond_av_list_destroy(expl_true);
-oom2:
-	avtab_destroy(&expa_false);
-oom1:
-	avtab_destroy(&expa_true);
-oom0:
-	ERR(args->handle, "out of memory on conditional av list expansion");
-        return 1;
+exit:
+	return rc;
+}
+
+static int bounds_check_cond_rules(sepol_handle_t *handle, policydb_t *p,
+				   avtab_t *global_avtab, avtab_t *cond_avtab,
+				   cond_av_list_t *rules, uint32_t child,
+				   uint32_t parent, avtab_ptr_t *bad,
+				   int *numbad)
+{
+	int rc = 0;
+	cond_av_list_t *cur;
+
+	for (cur = rules; cur; cur = cur->next) {
+		avtab_ptr_t ap = cur->node;
+		avtab_key_t *key = &ap->key;
+		avtab_datum_t *datum = &ap->datum;
+		if (!(key->specified & AVTAB_ALLOWED))
+			continue;
+		rc = bounds_check_rule(handle, p, global_avtab, cond_avtab,
+				       child, parent, key->source_type,
+				       key->target_type, key->target_class,
+				       datum->data, bad, numbad);
+		if (rc) goto exit;
+	}
+
+exit:
+	return rc;
+}
+
+struct bounds_check_args {
+	sepol_handle_t *handle;
+	policydb_t *p;
+	avtab_t *cur_avtab;
+	uint32_t child;
+	uint32_t parent;
+	avtab_ptr_t bad;
+	int numbad;
+};
+
+static int bounds_check_rule_callback(avtab_key_t *k, avtab_datum_t *d,
+				      void *args)
+{
+	struct bounds_check_args *a = (struct bounds_check_args *)args;
+
+	if (!(k->specified & AVTAB_ALLOWED))
+		return 0;
+
+	return bounds_check_rule(a->handle, a->p, NULL, a->cur_avtab, a->child,
+				 a->parent, k->source_type, k->target_type,
+				 k->target_class, d->data, &a->bad, &a->numbad);
 }
 
-static int check_cond_avtab_hierarchy(cond_list_t * cond_list,
-				      hierarchy_args_t * args)
+static int bounds_check_child_rules(sepol_handle_t *handle, policydb_t *p,
+				    avtab_t *global_avtab,
+				    struct bounds_cond_info *cond_info,
+				    uint32_t child, uint32_t parent,
+				    avtab_ptr_t *bad, int *numbad)
 {
 	int rc;
-	cond_list_t *cur_node;
-	cond_av_list_t *cur_av, *expl = NULL;
-	avtab_t expa;
-	hierarchy_args_t *a = (hierarchy_args_t *) args;
-	avtab_datum_t avdat, *uncond;
-
-	for (cur_node = cond_list; cur_node; cur_node = cur_node->next) {
-		/*
-		 * Check true condition
-		 */
-		if (avtab_init(&expa))
-			goto oom;
-		if (expand_cond_av_list(args->p, cur_node->true_list,
-					&expl, &expa)) {
-			avtab_destroy(&expa);
-			goto oom;
-		}
-		args->opt_cond_list = expl;
-		for (cur_av = expl; cur_av; cur_av = cur_av->next) {
-			avdat.data = cur_av->node->datum.data;
-			uncond = avtab_search(a->expa, &cur_av->node->key);
-			if (uncond)
-				avdat.data |= uncond->data;
-			rc = check_avtab_hierarchy_callback(&cur_av->node->key,
-							    &avdat, args);
-			if (rc)
-				args->numerr++;
-		}
-		cond_av_list_destroy(expl);
-		avtab_destroy(&expa);
+	struct bounds_check_args args;
+	struct bounds_cond_info *cur;
 
-		/*
-		 * Check false condition
-		 */
-		if (avtab_init(&expa))
-			goto oom;
-		if (expand_cond_av_list(args->p, cur_node->false_list,
-					&expl, &expa)) {
-			avtab_destroy(&expa);
-			goto oom;
-		}
-		args->opt_cond_list = expl;
-		for (cur_av = expl; cur_av; cur_av = cur_av->next) {
-			avdat.data = cur_av->node->datum.data;
-			uncond = avtab_search(a->expa, &cur_av->node->key);
-			if (uncond)
-				avdat.data |= uncond->data;
-
-			rc = check_avtab_hierarchy_callback(&cur_av->node->key,
-							    &avdat, args);
-			if (rc)
-				a->numerr++;
+	args.handle = handle;
+	args.p = p;
+	args.cur_avtab = global_avtab;
+	args.child = child;
+	args.parent = parent;
+	args.bad = NULL;
+	args.numbad = 0;
+	rc = avtab_map(&p->te_avtab, bounds_check_rule_callback, &args);
+	if (rc) goto exit;
+
+	for (cur = cond_info; cur; cur = cur->next) {
+		cond_list_t *node = cur->cond_list;
+		rc = bounds_check_cond_rules(handle, p, global_avtab,
+					     &cur->true_avtab,
+					     node->true_list, child, parent,
+					     &args.bad, &args.numbad);
+		if (rc) goto exit;
+
+		rc = bounds_check_cond_rules(handle, p, global_avtab,
+					     &cur->false_avtab,
+					     node->false_list, child, parent,
+					     &args.bad, &args.numbad);
+		if (rc) goto exit;
+	}
+
+	*numbad += args.numbad;
+	*bad = args.bad;
+
+exit:
+	return rc;
+}
+
+int bounds_check_type(sepol_handle_t *handle, policydb_t *p, uint32_t child,
+		      uint32_t parent, avtab_ptr_t *bad, int *numbad)
+{
+	int rc = 0;
+	avtab_t global_avtab;
+	struct bounds_cond_info *cond_info = NULL;
+
+	rc = bounds_expand_parent_rules(handle, p, &global_avtab, &cond_info,
+					child, parent);
+	if (rc) goto exit;
+
+	rc = bounds_check_child_rules(handle, p, &global_avtab, cond_info,
+				      child, parent, bad, numbad);
+
+	bounds_destroy_cond_info(cond_info);
+	avtab_destroy(&global_avtab);
+
+exit:
+	return rc;
+}
+
+struct bounds_args {
+	sepol_handle_t *handle;
+	policydb_t *p;
+	int numbad;
+};
+
+static void bounds_report(sepol_handle_t *handle, policydb_t *p, uint32_t child,
+			  uint32_t parent, avtab_ptr_t cur)
+{
+	ERR(handle, "Child type %s exceeds bounds of parent %s in the following rules:",
+	    p->p_type_val_to_name[child - 1],
+	    p->p_type_val_to_name[parent - 1]);
+	for (; cur; cur = cur->next) {
+		ERR(handle, "    %s %s : %s { %s }",
+		    p->p_type_val_to_name[cur->key.source_type - 1],
+		    p->p_type_val_to_name[cur->key.target_type - 1],
+		    p->p_class_val_to_name[cur->key.target_class - 1],
+		    sepol_av_to_string(p, cur->key.target_class,
+				       cur->datum.data));
+	}
+}
+
+void bounds_destroy_bad(avtab_ptr_t cur)
+{
+	avtab_ptr_t next;
+
+	for (; cur; cur = next) {
+		next = cur->next;
+		cur->next = NULL;
+		free(cur);
+	}
+}
+
+static int bounds_check_type_callback(hashtab_key_t k, hashtab_datum_t d,
+				      void *args)
+{
+	int rc = 0;
+	struct bounds_args *a = (struct bounds_args *)args;
+	type_datum_t *t = (type_datum_t *)d;
+	avtab_ptr_t bad = NULL;
+
+	if (t->bounds) {
+		rc = bounds_check_type(a->handle, a->p, t->s.value, t->bounds,
+				       &bad, &a->numbad);
+		if (bad) {
+			bounds_report(a->handle, a->p, t->s.value, t->bounds,
+				      bad);
+			bounds_destroy_bad(bad);
 		}
-		cond_av_list_destroy(expl);
-		avtab_destroy(&expa);
 	}
 
-	return 0;
+	return rc;
+}
+
+int bounds_check_types(sepol_handle_t *handle, policydb_t *p)
+{
+	int rc;
+	struct bounds_args args;
+
+	args.handle = handle;
+	args.p = p;
+	args.numbad = 0;
+
+	rc = hashtab_map(p->p_types.table, bounds_check_type_callback, &args);
+	if (rc) goto exit;
+
+	if (args.numbad > 0) {
+		ERR(handle, "%d errors found during type bounds check",
+		    args.numbad);
+		rc = SEPOL_ERR;
+	}
 
-      oom:
-	ERR(args->handle, "out of memory on conditional av list expansion");
-	return 1;
+exit:
+	return rc;
 }
 
-/* The role hierarchy is defined as: a child role cannot have more types than it's parent.
- * This function should be called with hashtab_map, it will return 0 on success, 1 on 
- * constraint violation and -1 on error
+/* The role bounds is defined as: a child role cannot have a type that
+ * its parent doesn't have.
  */
-static int check_role_hierarchy_callback(hashtab_key_t k
-					 __attribute__ ((unused)),
-					 hashtab_datum_t d, void *args)
+static int bounds_check_role_callback(hashtab_key_t k __attribute__ ((unused)),
+				      hashtab_datum_t d, void *args)
 {
-	hierarchy_args_t *a;
-	role_datum_t *r, *rp;
+	struct bounds_args *a = (struct bounds_args *)args;
+	role_datum_t *r = (role_datum_t *) d;
+	role_datum_t *rp = NULL;
 
-	a = (hierarchy_args_t *) args;
-	r = (role_datum_t *) d;
+	if (!r->bounds)
+		return 0;
 
-	if (find_parent_role(a, r, &rp) < 0)
-		return -1;
+	rp = a->p->role_val_to_struct[r->bounds - 1];
 
 	if (rp && !ebitmap_contains(&rp->types.types, &r->types.types)) {
-		/* hierarchical constraint violation, return error */
-		ERR(a->handle, "Role hierarchy violation, %s exceeds %s",
-		    (char *) k, a->p->p_role_val_to_name[rp->s.value - 1]);
-		a->numerr++;
+		ERR(a->handle, "Role bounds violation, %s exceeds %s",
+		    (char *)k, a->p->p_role_val_to_name[rp->s.value - 1]);
+		a->numbad++;
+	}
+
+	return 0;
+}
+
+int bounds_check_roles(sepol_handle_t *handle, policydb_t *p)
+{
+	struct bounds_args args;
+
+	args.handle = handle;
+	args.p = p;
+	args.numbad = 0;
+
+	hashtab_map(p->p_roles.table, bounds_check_role_callback, &args);
+
+	if (args.numbad > 0) {
+		ERR(handle, "%d errors found during role bounds check",
+		    args.numbad);
+		return SEPOL_ERR;
 	}
+
 	return 0;
 }
 
-/* The user hierarchy is defined as: a child user cannot have a role that
- * its parent doesn't have.  This function should be called with hashtab_map,
- * it will return 0 on success, 1 on constraint violation and -1 on error.
+/* The user bounds is defined as: a child user cannot have a role that
+ * its parent doesn't have.
  */
-static int check_user_hierarchy_callback(hashtab_key_t k
-					 __attribute__ ((unused)),
-					 hashtab_datum_t d, void *args)
+static int bounds_check_user_callback(hashtab_key_t k __attribute__ ((unused)),
+				      hashtab_datum_t d, void *args)
 {
-	hierarchy_args_t *a;
-	user_datum_t *u, *up;
+	struct bounds_args *a = (struct bounds_args *)args;
+	user_datum_t *u = (user_datum_t *) d;
+	user_datum_t *up = NULL;
 
-	a = (hierarchy_args_t *) args;
-	u = (user_datum_t *) d;
+	if (!u->bounds)
+		return 0;
 
-	if (find_parent_user(a, u, &up) < 0)
-		return -1;
+	up = a->p->user_val_to_struct[u->bounds - 1];
 
 	if (up && !ebitmap_contains(&up->roles.roles, &u->roles.roles)) {
-		/* hierarchical constraint violation, return error */
-		ERR(a->handle, "User hierarchy violation, %s exceeds %s",
+		ERR(a->handle, "User bounds violation, %s exceeds %s",
 		    (char *) k, a->p->p_user_val_to_name[up->s.value - 1]);
-		a->numerr++;
+		a->numbad++;
 	}
+
 	return 0;
 }
 
-int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p)
+int bounds_check_users(sepol_handle_t *handle, policydb_t *p)
 {
-	hierarchy_args_t args;
-	avtab_t expa;
-
-	if (avtab_init(&expa))
-		goto oom;
-	if (expand_avtab(p, &p->te_avtab, &expa)) {
-		avtab_destroy(&expa);
-		goto oom;
-	}
+	struct bounds_args args;
 
-	args.p = p;
-	args.expa = &expa;
-	args.opt_cond_list = NULL;
 	args.handle = handle;
-	args.numerr = 0;
+	args.p = p;
+	args.numbad = 0;
 
-	if (hashtab_map(p->p_types.table, check_type_hierarchy_callback, &args))
-		goto bad;
+	hashtab_map(p->p_users.table, bounds_check_user_callback, &args);
 
-	if (pullup_unconditional_perms(p->cond_list, &args))
-		return -1;
+	if (args.numbad > 0) {
+		ERR(handle, "%d errors found during user bounds check",
+		    args.numbad);
+		return SEPOL_ERR;
+	}
 
-	if (avtab_map(&expa, check_avtab_hierarchy_callback, &args))
-		goto bad;
+	return 0;
+}
 
-	if (check_cond_avtab_hierarchy(p->cond_list, &args))
-		goto bad;
+#define add_hierarchy_callback_template(prefix)				\
+	int hierarchy_add_##prefix##_callback(hashtab_key_t k __attribute__ ((unused)), \
+					    hashtab_datum_t d, void *args) \
+{								\
+	struct bounds_args *a = (struct bounds_args *)args;		\
+	sepol_handle_t *handle = a->handle;				\
+	policydb_t *p = a->p;						\
+	prefix##_datum_t *datum = (prefix##_datum_t *)d;		\
+	prefix##_datum_t *parent;					\
+	char *parent_name, *datum_name, *tmp;				\
+									\
+	if (!datum->bounds) {						\
+		datum_name = p->p_##prefix##_val_to_name[datum->s.value - 1]; \
+									\
+		tmp = strrchr(datum_name, '.');				\
+		/* no '.' means it has no parent */			\
+		if (!tmp) return 0;					\
+									\
+		parent_name = strdup(datum_name);			\
+		if (!parent_name) {					\
+			ERR(handle, "Insufficient memory");		\
+			return SEPOL_ENOMEM;				\
+		}							\
+		parent_name[tmp - datum_name] = '\0';			\
+									\
+		parent = hashtab_search(p->p_##prefix##s.table, parent_name); \
+		if (!parent) {						\
+			/* Orphan type/role/user */			\
+			ERR(handle, "%s doesn't exist, %s is an orphan",\
+			    parent_name,				\
+			    p->p_##prefix##_val_to_name[datum->s.value - 1]); \
+			free(parent_name);				\
+			a->numbad++;					\
+			return 0;					\
+		}							\
+		datum->bounds = parent->s.value;			\
+		free(parent_name);					\
+	}								\
+									\
+	return 0;							\
+}								\
+
+static add_hierarchy_callback_template(type)
+static add_hierarchy_callback_template(role)
+static add_hierarchy_callback_template(user)
 
-	if (hashtab_map(p->p_roles.table, check_role_hierarchy_callback, &args))
-		goto bad;
+int hierarchy_add_bounds(sepol_handle_t *handle, policydb_t *p)
+{
+	int rc = 0;
+	struct bounds_args args;
+
+	args.handle = handle;
+	args.p = p;
+	args.numbad = 0;
+
+	rc = hashtab_map(p->p_users.table, hierarchy_add_user_callback, &args);
+	if (rc) goto exit;
+
+	rc = hashtab_map(p->p_roles.table, hierarchy_add_role_callback, &args);
+	if (rc) goto exit;
 
-	if (hashtab_map(p->p_users.table, check_user_hierarchy_callback, &args))
-		goto bad;
+	rc = hashtab_map(p->p_types.table, hierarchy_add_type_callback, &args);
+	if (rc) goto exit;
 
-	if (args.numerr) {
-		ERR(handle, "%d total errors found during hierarchy check",
-		    args.numerr);
-		goto bad;
+	if (args.numbad > 0) {
+		ERR(handle, "%d errors found while adding hierarchies",
+		    args.numbad);
+		rc = SEPOL_ERR;
 	}
 
-	avtab_destroy(&expa);
-	return 0;
+exit:
+	return rc;
+}
+
+int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p)
+{
+	int rc = 0;
+	int violation = 0;
+
+	rc = hierarchy_add_bounds(handle, p);
+	if (rc) goto exit;
+
+	rc = bounds_check_users(handle, p);
+	if (rc)
+		violation = 1;
+
+	rc = bounds_check_roles(handle, p);
+	if (rc)
+		violation = 1;
+
+	rc = bounds_check_types(handle, p);
+	if (rc) {
+		if (rc == SEPOL_ERR)
+			violation = 1;
+		else
+			goto exit;
+	}
 
-      bad:
-	avtab_destroy(&expa);
-	return -1;
+	if (violation)
+		rc = SEPOL_ERR;
 
-      oom:
-	ERR(handle, "Out of memory");
-	return -1;
+exit:
+	return rc;
 }
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 05/10 v2] libsepol/cil: Add function to search the CIL AST for an AV rule.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (3 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-17 19:58 ` [PATCH 06/10 v2] libsepol/cil: Refactored CIL neverallow checking and reporting James Carter
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

The search will be considered a success if any rule is found that
at least partially matches all parts (src type, tgt type, and class-
perms) of the target rule.

For example, for a target of (allow domain file_type (file (read write)
the rule (allow init_t init_exec_t (file (read exec)) will match.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/cil/src/cil_find.c | 305 ++++++++++++++++++++++++++++++++++++++++++++
 libsepol/cil/src/cil_find.h |  39 ++++++
 2 files changed, 344 insertions(+)
 create mode 100644 libsepol/cil/src/cil_find.c
 create mode 100644 libsepol/cil/src/cil_find.h

diff --git a/libsepol/cil/src/cil_find.c b/libsepol/cil/src/cil_find.c
new file mode 100644
index 0000000..a76dc37
--- /dev/null
+++ b/libsepol/cil/src/cil_find.c
@@ -0,0 +1,305 @@
+/*
+ * Copyright 2011 Tresys Technology, LLC. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ *    1. Redistributions of source code must retain the above copyright notice,
+ *       this list of conditions and the following disclaimer.
+ *
+ *    2. Redistributions in binary form must reproduce the above copyright notice,
+ *       this list of conditions and the following disclaimer in the documentation
+ *       and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY TRESYS TECHNOLOGY, LLC ``AS IS'' AND ANY EXPRESS
+ * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+ * EVENT SHALL TRESYS TECHNOLOGY, LLC OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
+ * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * The views and conclusions contained in the software and documentation are those
+ * of the authors and should not be interpreted as representing official policies,
+ * either expressed or implied, of Tresys Technology, LLC.
+ */
+
+#include <sepol/policydb/ebitmap.h>
+
+#include "cil_internal.h"
+#include "cil_flavor.h"
+#include "cil_list.h"
+#include "cil_log.h"
+#include "cil_symtab.h"
+
+struct cil_args_find {
+	enum cil_flavor flavor;
+	void *target;
+	struct cil_list *matching;
+	int match_self;
+};
+
+static int cil_type_match_any(struct cil_symtab_datum *d1, struct cil_symtab_datum *d2)
+{
+	enum cil_flavor f1 = ((struct cil_tree_node*)d1->nodes->head->data)->flavor;
+	enum cil_flavor f2 = ((struct cil_tree_node*)d2->nodes->head->data)->flavor;
+
+	if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_type *t1 = (struct cil_type *)d1;
+		struct cil_type *t2 = (struct cil_type *)d2;
+		if (t1->value == t2->value) {
+			return CIL_TRUE;
+		}
+	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_typeattribute *a = (struct cil_typeattribute *)d1;
+		struct cil_type *t = (struct cil_type *)d2;
+		if (ebitmap_get_bit(a->types, t->value)) {
+			return CIL_TRUE;
+		}
+	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
+		struct cil_type *t = (struct cil_type *)d1;
+		struct cil_typeattribute *a = (struct cil_typeattribute *)d2;
+		if (ebitmap_get_bit(a->types, t->value)) {
+			return CIL_TRUE;
+		}
+	} else {
+		/* Both are attributes */
+		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
+		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
+		return ebitmap_match_any(a1->types, a2->types);
+	}
+	return CIL_FALSE;
+}
+
+static int cil_type_matches(ebitmap_t *matches, struct cil_symtab_datum *d1, struct cil_symtab_datum *d2)
+{
+	int rc = SEPOL_OK;
+	enum cil_flavor f1 = ((struct cil_tree_node*)d1->nodes->head->data)->flavor;
+	enum cil_flavor f2 = ((struct cil_tree_node*)d2->nodes->head->data)->flavor;
+
+	if (f1 != CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_type *t1 = (struct cil_type *)d1;
+		struct cil_type *t2 = (struct cil_type *)d2;
+		if (t1->value == t2->value) {
+			ebitmap_set_bit(matches, t1->value, 1);
+		}
+	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
+		struct cil_typeattribute *a = (struct cil_typeattribute *)d1;
+		struct cil_type *t = (struct cil_type *)d2;
+		if (ebitmap_get_bit(a->types, t->value)) {
+			ebitmap_set_bit(matches, t->value, 1);
+		}
+	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
+		struct cil_type *t = (struct cil_type *)d1;
+		struct cil_typeattribute *a = (struct cil_typeattribute *)d2;
+		if (ebitmap_get_bit(a->types, t->value)) {
+			ebitmap_set_bit(matches, t->value, 1);
+		}
+	} else {
+		/* Both are attributes */
+		struct cil_typeattribute *a1 = (struct cil_typeattribute *)d1;
+		struct cil_typeattribute *a2 = (struct cil_typeattribute *)d2;
+		rc = ebitmap_and(matches, a1->types, a2->types);
+	}
+
+	return rc;
+}
+
+/* s1 is the src type that is matched with a self
+ * s2, and t2 are the source and type of the other rule
+ */
+static int cil_self_match_any(struct cil_symtab_datum *s1, struct cil_symtab_datum *s2, struct cil_symtab_datum *t2)
+{
+	int rc;
+	struct cil_tree_node *n1 = s1->nodes->head->data;
+	if (n1->flavor != CIL_TYPEATTRIBUTE) {
+		rc = cil_type_match_any(s1, t2);
+	} else {
+		struct cil_typeattribute *a = (struct cil_typeattribute *)s1;
+		ebitmap_t map;
+		ebitmap_init(&map);
+		rc = cil_type_matches(&map, s2, t2);
+		if (rc < 0) {
+			ebitmap_destroy(&map);
+			goto exit;
+		}
+		if (map.node == NULL) {
+			rc = CIL_FALSE;
+			goto exit;
+		}
+		rc = ebitmap_match_any(&map, a->types);
+		ebitmap_destroy(&map);
+	}
+
+exit:
+	return rc;
+}
+
+static int cil_classperms_match_any(struct cil_classperms *cp1, struct cil_classperms *cp2)
+{
+	struct cil_class *c1 = cp1->class;
+	struct cil_class *c2 = cp2->class;
+	struct cil_list_item *i1, *i2;
+
+	if (&c1->datum != &c2->datum) return CIL_FALSE;
+
+	cil_list_for_each(i1, cp1->perms) {
+		struct cil_perm *p1 = i1->data;
+		cil_list_for_each(i2, cp2->perms) {
+			struct cil_perm *p2 = i2->data;
+			if (&p1->datum == &p2->datum) return CIL_TRUE;
+		}
+	}
+	return CIL_FALSE;
+}
+
+static int __cil_classperms_list_match_any(struct cil_classperms *cp1, struct cil_list *cpl2)
+{
+	int rc;
+	struct cil_list_item *curr;
+
+	cil_list_for_each(curr, cpl2) {
+		if (curr->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *cp = curr->data;
+			if (FLAVOR(cp->class) == CIL_CLASS) {
+				rc = cil_classperms_match_any(cp1, cp);
+				if (rc == CIL_TRUE) return CIL_TRUE;
+			} else { /* MAP */
+				struct cil_list_item *i = NULL;
+				cil_list_for_each(i, cp->perms) {
+					struct cil_perm *cmp = i->data;
+					rc = __cil_classperms_list_match_any(cp1, cmp->classperms);
+					if (rc == CIL_TRUE) return CIL_TRUE;
+				}
+			}
+		} else { /* SET */
+			struct cil_classperms_set *cp_set = curr->data;
+			struct cil_classpermission *cp = cp_set->set;
+			rc = __cil_classperms_list_match_any(cp1, cp->classperms);
+			if (rc == CIL_TRUE) return CIL_TRUE;
+		}
+	}
+	return CIL_FALSE;
+}
+
+static int cil_classperms_list_match_any(struct cil_list *cpl1, struct cil_list *cpl2)
+{
+	int rc;
+	struct cil_list_item *curr;
+
+	cil_list_for_each(curr, cpl1) {
+		if (curr->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *cp = curr->data;
+			if (FLAVOR(cp->class) == CIL_CLASS) {
+				rc = __cil_classperms_list_match_any(cp, cpl2);
+				if (rc == CIL_TRUE) return CIL_TRUE;
+			} else { /* MAP */
+				struct cil_list_item *i = NULL;
+				cil_list_for_each(i, cp->perms) {
+					struct cil_perm *cmp = i->data;
+					rc = cil_classperms_list_match_any(cmp->classperms, cpl2);
+					if (rc == CIL_TRUE) return CIL_TRUE;
+				}
+			}
+		} else { /* SET */
+			struct cil_classperms_set *cp_set = curr->data;
+			struct cil_classpermission *cp = cp_set->set;
+			rc = cil_classperms_list_match_any(cp->classperms, cpl2);
+			if (rc == CIL_TRUE) return CIL_TRUE;
+		}
+	}
+	return CIL_FALSE;
+}
+
+int cil_find_matching_avrule(struct cil_tree_node *node, struct cil_avrule *avrule, struct cil_avrule *target, struct cil_list *matching, int match_self)
+{
+	int rc = SEPOL_OK;
+	struct cil_symtab_datum *s1 = avrule->src;
+	struct cil_symtab_datum *t1 = avrule->tgt;
+	struct cil_list *cp1 = avrule->classperms;
+	struct cil_symtab_datum *s2 = target->src;
+	struct cil_symtab_datum *t2 = target->tgt;
+	struct cil_list *cp2 = target->classperms;
+
+	if (match_self != CIL_TRUE && avrule == target) goto exit;
+
+	if (avrule->rule_kind != target->rule_kind) goto exit;
+
+	if (!cil_type_match_any(s1, s2)) goto exit;
+
+	if (t1->fqn != CIL_KEY_SELF && t2->fqn != CIL_KEY_SELF) {
+		if (!cil_type_match_any(t1, t2)) goto exit;
+	} else {
+		if (t1->fqn == CIL_KEY_SELF && t2->fqn == CIL_KEY_SELF) {
+			/* The earlier check whether s1 and s2 matches is all that is needed */
+		} else if (t1->fqn == CIL_KEY_SELF) {
+			rc = cil_self_match_any(s1, s2, t2);
+			if (rc < 0) {
+				goto exit;
+			} else if (rc == CIL_FALSE) {
+				rc = SEPOL_OK;
+				goto exit;
+			}
+		} else if (t2->fqn == CIL_KEY_SELF) {
+			rc = cil_self_match_any(s2, s1, t1);
+			if (rc < 0) {
+				goto exit;
+			} else if (rc == CIL_FALSE) {
+				rc = SEPOL_OK;
+				goto exit;
+			}
+		}
+	}
+
+	if (cil_classperms_list_match_any(cp1, cp2)) {
+		cil_list_append(matching, CIL_NODE, node);
+	}
+
+	rc = SEPOL_OK;
+
+exit:
+	return rc;
+}
+
+static int __cil_find_matching_avrule_in_ast(struct cil_tree_node *node,  __attribute__((unused)) uint32_t *finished, void *extra_args)
+{
+	int rc = SEPOL_OK;
+	struct cil_args_find *args = extra_args;
+
+	if (node->flavor == CIL_BLOCK) {
+		struct cil_block *blk = node->data;
+		if (blk->is_abstract == CIL_TRUE) {
+			*finished = CIL_TREE_SKIP_HEAD;
+			goto exit;
+		}
+	} else if (node->flavor == CIL_MACRO) {
+		*finished = CIL_TREE_SKIP_HEAD;
+		goto exit;
+	} else if (node->flavor == CIL_AVRULE) {
+		rc = cil_find_matching_avrule(node, node->data, args->target, args->matching, args->match_self);
+	}
+
+exit:
+	return rc;
+}
+
+int cil_find_matching_avrule_in_ast(struct cil_tree_node *current, enum cil_flavor flavor, void *target, struct cil_list *matching, int match_self)
+{
+	int rc;
+	struct cil_args_find args;
+
+	args.flavor = flavor;
+	args.target = target;
+	args.matching = matching;
+	args.match_self = match_self;
+
+	rc = cil_tree_walk(current, __cil_find_matching_avrule_in_ast, NULL, NULL, &args);
+	if (rc) {
+		cil_log(CIL_ERR, "An error occured while searching for avrule in AST\n");
+	}
+
+	return rc;
+}
diff --git a/libsepol/cil/src/cil_find.h b/libsepol/cil/src/cil_find.h
new file mode 100644
index 0000000..c8ca2d2
--- /dev/null
+++ b/libsepol/cil/src/cil_find.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2011 Tresys Technology, LLC. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ *    1. Redistributions of source code must retain the above copyright notice,
+ *       this list of conditions and the following disclaimer.
+ *
+ *    2. Redistributions in binary form must reproduce the above copyright notice,
+ *       this list of conditions and the following disclaimer in the documentation
+ *       and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY TRESYS TECHNOLOGY, LLC ``AS IS'' AND ANY EXPRESS
+ * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+ * EVENT SHALL TRESYS TECHNOLOGY, LLC OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
+ * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * The views and conclusions contained in the software and documentation are those
+ * of the authors and should not be interpreted as representing official policies,
+ * either expressed or implied, of Tresys Technology, LLC.
+ */
+
+#include "cil_flavor.h"
+#include "cil_tree.h"
+#include "cil_list.h"
+
+#ifndef CIL_FIND_H_
+#define CIL_FIND_H_
+
+int cil_find_matching_avrule_in_ast(struct cil_tree_node *current, enum cil_flavor flavor, void *target, struct cil_list *matching, int match_self);
+
+#endif
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 06/10 v2] libsepol/cil: Refactored CIL neverallow checking and reporting.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (4 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 05/10 v2] libsepol/cil: Add function to search the CIL AST for an AV rule James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-17 19:58 ` [PATCH 07/10 v2] libsepol/cil: Track number of classes and number of types and attributes James Carter
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

Use the libsepol neverallow checking to determine if a given neverallow
rule is violated. If a violation is found, use the function
cil_find_matching_avrule_in_ast() to find the AST node of the particular
rule that violates the neverallow. This allows CIL to provide a more
informative error message that includes the file and line number of the
node and all of its parents.

Example error report:
Neverallow check failed at line 31285 of cil.conf.neverallow
  (neverallow typeset4 self (memprotect (mmap_zero)))
    <root>
    booleanif at line 152094 of cil.conf.neverallow
    true at line 152095 of cil.conf.neverallow
    allow at line 152096 of cil.conf.neverallow
      (allow ada_t self (memprotect (mmap_zero)))

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/cil/src/cil_binary.c | 492 ++++++++++++++++++++++++++++--------------
 libsepol/cil/src/cil_binary.h |   4 +-
 2 files changed, 330 insertions(+), 166 deletions(-)

diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index 03f4924..2700b34 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -37,6 +37,8 @@
 #include <sepol/policydb/conditional.h>
 #include <sepol/policydb/constraint.h>
 #include <sepol/policydb/flask.h>
+#include <sepol/policydb/expand.h>
+#include <sepol/policydb/hierarchy.h>
 
 #include "cil_internal.h"
 #include "cil_flavor.h"
@@ -45,6 +47,7 @@
 #include "cil_tree.h"
 #include "cil_binary.h"
 #include "cil_symtab.h"
+#include "cil_find.h"
 
 /* There are 44000 filename_trans in current fedora policy. 1.33 times this is the recommended
  * size of a hashtable. The next power of 2 of this is 2 ** 16.
@@ -68,39 +71,9 @@ struct cil_args_booleanif {
 	policydb_t *pdb;
 	cond_node_t *cond_node;
 	enum cil_flavor cond_flavor;
-	struct cil_list *neverallows;
 	hashtab_t filename_trans_table;
 };
 
-struct cil_neverallow {
-	struct cil_tree_node *node;
-	struct cil_list *rules;
-};
-
-struct cil_neverallow_rule {
-	struct cil_symtab_datum *src;
-	struct cil_symtab_datum *tgt;
-	uint32_t class;
-	uint32_t perms;
-};
-
-void cil_neverallows_list_destroy(struct cil_list *neverallows)
-{
-	struct cil_list_item *i;
-	struct cil_list_item *j;
-
-	cil_list_for_each(i, neverallows) {
-		struct cil_neverallow *neverallow = i->data;
-		cil_list_for_each(j, neverallow->rules) {
-			struct cil_neverallow_rule *rule = j->data;
-			free(rule);
-		}
-		cil_list_destroy(&neverallow->rules, CIL_FALSE);
-		free(neverallow);
-	}
-	cil_list_destroy(&neverallows, CIL_FALSE);
-}
-
 static int __cil_get_sepol_user_datum(policydb_t *pdb, struct cil_symtab_datum *datum, user_datum_t **sepol_user)
 {
 	*sepol_user = hashtab_search(pdb->p_users.table, datum->fqn);
@@ -628,14 +601,21 @@ int __cil_typeattr_bitmap_init(policydb_t *pdb)
 	int rc = SEPOL_ERR;
 
 	pdb->type_attr_map = cil_malloc(pdb->p_types.nprim * sizeof(ebitmap_t));
+	pdb->attr_type_map = cil_malloc(pdb->p_types.nprim * sizeof(ebitmap_t));
 
 	uint32_t i = 0;
 	for (i = 0; i < pdb->p_types.nprim; i++) {
 		ebitmap_init(&pdb->type_attr_map[i]);
+		ebitmap_init(&pdb->attr_type_map[i]);
 		if (ebitmap_set_bit(&pdb->type_attr_map[i], i, 1)) {
 			rc = SEPOL_ERR;
 			goto exit;
 		}
+		if (ebitmap_set_bit(&pdb->attr_type_map[i], i, 1)) {
+			rc = SEPOL_ERR;
+			goto exit;
+		}
+
 	}
 
 	return SEPOL_OK;
@@ -675,6 +655,7 @@ int cil_typeattribute_to_bitmap(policydb_t *pdb, const struct cil_db *db, struct
 		if (rc != SEPOL_OK) goto exit;
 
 		ebitmap_set_bit(&pdb->type_attr_map[sepol_type->s.value - 1], value - 1, 1);
+		ebitmap_set_bit(&pdb->attr_type_map[value - 1], sepol_type->s.value - 1, 1);
 	}
 
 	rc = SEPOL_OK;
@@ -1324,89 +1305,7 @@ exit:
 	return rc;
 }
 
-static void __cil_neverallow_handle(struct cil_list *neverallows, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, uint32_t class, uint32_t perms)
-{
-	struct cil_neverallow *neverallow = neverallows->head->data;
-	struct cil_list *neverallow_rules = neverallow->rules;
-	struct cil_neverallow_rule *new = NULL;
-
-	new = cil_malloc(sizeof(*new));
-	new->src = src;
-	new->tgt = tgt;
-	new->class = class;
-	new->perms = perms;
-
-	cil_list_append(neverallow_rules, CIL_LIST_ITEM, new);
-}
-
-static int __cil_is_type_match(enum cil_flavor f1, struct cil_symtab_datum *t1, enum cil_flavor f2, struct cil_symtab_datum *t2)
-{
-	if (t1->fqn == t2->fqn) {
-		return CIL_TRUE;
-	} else if (f1 == CIL_TYPEATTRIBUTE && f2 != CIL_TYPEATTRIBUTE) {
-		struct cil_typeattribute *a = (struct cil_typeattribute *)t1;
-		struct cil_type *t = (struct cil_type *)t2;
-		if (ebitmap_get_bit(a->types, t->value)) {
-			return CIL_TRUE;
-		}
-	} else if (f1 != CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
-		struct cil_typeattribute *a = (struct cil_typeattribute *)t2;
-		struct cil_type *t = (struct cil_type *)t1;
-		if (ebitmap_get_bit(a->types, t->value)) {
-			return CIL_TRUE;
-		}
-	} else if (f1 == CIL_TYPEATTRIBUTE && f2 == CIL_TYPEATTRIBUTE) {
-		struct cil_typeattribute *a1 = (struct cil_typeattribute *)t2;
-		struct cil_typeattribute *a2 = (struct cil_typeattribute *)t1;
-		/* abusing the ebitmap abstraction for speed */
-		ebitmap_node_t *n1 = a1->types->node;
-		ebitmap_node_t *n2 = a2->types->node;
-		while (n1 && n2) {
-			if (n1->startbit < n2->startbit) {
-				n1 = n1->next;
-			} else if (n2->startbit < n1->startbit) {
-				n2 = n2->next;
-			} else {
-				if (n1->map & n2->map) {
-					return CIL_TRUE;
-				}
-				n1 = n1->next;
-				n2 = n2->next;
-			}
-		}
-	}
-	return CIL_FALSE;
-}
-
-static int __cil_check_neverallows(struct cil_list *neverallows, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, uint32_t class, uint32_t perms)
-{
-	struct cil_list_item *curr = NULL;
-	enum cil_flavor al_src_flavor = ((struct cil_tree_node*)src->nodes->head->data)->flavor;
-	enum cil_flavor al_tgt_flavor = ((struct cil_tree_node*)tgt->nodes->head->data)->flavor;
-	cil_list_for_each(curr, neverallows) {
-		struct cil_neverallow *neverallow = curr->data;
-		struct cil_tree_node *node = neverallow->node;
-		struct cil_list_item *curr_item = NULL;
-		cil_list_for_each(curr_item, neverallow->rules) {
-			struct cil_neverallow_rule *curr_rule = curr_item->data;
-			enum cil_flavor nv_src_flavor = ((struct cil_tree_node*)curr_rule->src->nodes->head->data)->flavor;
-			enum cil_flavor nv_tgt_flavor = ((struct cil_tree_node*)curr_rule->tgt->nodes->head->data)->flavor;
-			if ((curr_rule->perms & perms) && (class == curr_rule->class)) {
-				int src_match = __cil_is_type_match(al_src_flavor, src, nv_src_flavor, curr_rule->src);
-				if (src_match) {
-					int tgt_match = __cil_is_type_match(al_tgt_flavor, tgt, nv_tgt_flavor, curr_rule->tgt);
-					if (tgt_match) {
-						cil_log(CIL_ERR, "Neverallow found that matches avrule at line %d of %s\n", node->line, node->path);
-						return SEPOL_ERR;
-					}
-				}
-			}
-		}
-	}
-	return SEPOL_OK;
-}
-
-int __cil_avrule_expand_helper(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_classperms *cp, struct cil_list *neverallows, cond_node_t *cond_node, enum cil_flavor cond_flavor)
+int __cil_avrule_expand_helper(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_classperms *cp, cond_node_t *cond_node, enum cil_flavor cond_flavor)
 {
 	int rc = SEPOL_ERR;
 	type_datum_t *sepol_src = NULL;
@@ -1425,28 +1324,19 @@ int __cil_avrule_expand_helper(policydb_t *pdb, uint16_t kind, struct cil_symtab
 		return SEPOL_OK;
 	}
 
-	if (kind == CIL_AVRULE_NEVERALLOW) {
-		__cil_neverallow_handle(neverallows, src, tgt, sepol_class->s.value, data);
-	} else {
-		if (kind == CIL_AVRULE_DONTAUDIT) {
-			data = ~data;
-		} else if (neverallows != NULL && kind == CIL_AVRULE_ALLOWED) {
-			rc = __cil_check_neverallows(neverallows, src, tgt, sepol_class->s.value, data);
-			if (rc != SEPOL_OK) {
-				goto exit;
-			}
-		}
+	if (kind == CIL_AVRULE_DONTAUDIT) {
+		data = ~data;
+	}
 
-		rc = __cil_get_sepol_type_datum(pdb, src, &sepol_src);
-		if (rc != SEPOL_OK) goto exit;
+	rc = __cil_get_sepol_type_datum(pdb, src, &sepol_src);
+	if (rc != SEPOL_OK) goto exit;
 
-		rc = __cil_get_sepol_type_datum(pdb, tgt, &sepol_tgt);
-		if (rc != SEPOL_OK) goto exit;
+	rc = __cil_get_sepol_type_datum(pdb, tgt, &sepol_tgt);
+	if (rc != SEPOL_OK) goto exit;
 
-		rc = __cil_insert_avrule(pdb, kind, sepol_src->s.value, sepol_tgt->s.value, sepol_class->s.value, data, cond_node, cond_flavor);
-		if (rc != SEPOL_OK) {
-			goto exit;
-		}
+	rc = __cil_insert_avrule(pdb, kind, sepol_src->s.value, sepol_tgt->s.value, sepol_class->s.value, data, cond_node, cond_flavor);
+	if (rc != SEPOL_OK) {
+		goto exit;
 	}
 
 	return SEPOL_OK;
@@ -1456,7 +1346,7 @@ exit:
 }
 
 
-int __cil_avrule_expand(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms, struct cil_list *neverallows, cond_node_t *cond_node, enum cil_flavor cond_flavor)
+int __cil_avrule_expand(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum *src, struct cil_symtab_datum *tgt, struct cil_list *classperms, cond_node_t *cond_node, enum cil_flavor cond_flavor)
 {
 	int rc = SEPOL_ERR;
 	struct cil_list_item *curr;
@@ -1465,7 +1355,7 @@ int __cil_avrule_expand(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum
 		if (curr->flavor == CIL_CLASSPERMS) {
 			struct cil_classperms *cp = curr->data;
 			if (FLAVOR(cp->class) == CIL_CLASS) {
-				rc = __cil_avrule_expand_helper(pdb, kind, src, tgt, cp, neverallows, cond_node, cond_flavor);
+				rc = __cil_avrule_expand_helper(pdb, kind, src, tgt, cp, cond_node, cond_flavor);
 				if (rc != SEPOL_OK) {
 					goto exit;
 				}
@@ -1473,7 +1363,7 @@ int __cil_avrule_expand(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum
 				struct cil_list_item *i = NULL;
 				cil_list_for_each(i, cp->perms) {
 					struct cil_perm *cmp = i->data;
-					rc = __cil_avrule_expand(pdb, kind, src, tgt, cmp->classperms, neverallows, cond_node, cond_flavor);
+					rc = __cil_avrule_expand(pdb, kind, src, tgt, cmp->classperms, cond_node, cond_flavor);
 					if (rc != SEPOL_OK) {
 						goto exit;
 					}
@@ -1482,7 +1372,7 @@ int __cil_avrule_expand(policydb_t *pdb, uint16_t kind, struct cil_symtab_datum
 		} else { /* SET */
 			struct cil_classperms_set *cp_set = curr->data;
 			struct cil_classpermission *cp = cp_set->set;
-			rc = __cil_avrule_expand(pdb, kind, src, tgt, cp->classperms, neverallows, cond_node, cond_flavor);
+			rc = __cil_avrule_expand(pdb, kind, src, tgt, cp->classperms, cond_node, cond_flavor);
 			if (rc != SEPOL_OK) {
 				goto exit;
 			}
@@ -1495,7 +1385,7 @@ exit:
 	return rc;
 }
 
-int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_avrule *cil_avrule, struct cil_list *neverallows, cond_node_t *cond_node, enum cil_flavor cond_flavor)
+int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_avrule *cil_avrule, cond_node_t *cond_node, enum cil_flavor cond_flavor)
 {
 	int rc = SEPOL_ERR;
 	uint16_t kind = cil_avrule->rule_kind;
@@ -1509,12 +1399,6 @@ int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_a
 		goto exit;
 	}
 
-	if (cil_avrule->rule_kind == CIL_AVRULE_NEVERALLOW && db->disable_neverallow == CIL_TRUE) {
-		// ignore neverallow rules
-		rc = SEPOL_OK;
-		goto exit;
-	}
-
 	src = cil_avrule->src;
 	tgt = cil_avrule->tgt;
 
@@ -1530,7 +1414,7 @@ int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_a
 			if (!ebitmap_get_bit(&type_bitmap, i)) continue;
 
 			src = DATUM(db->val_to_type[i]);
-			rc = __cil_avrule_expand(pdb, kind, src, src, classperms, neverallows, cond_node, cond_flavor);
+			rc = __cil_avrule_expand(pdb, kind, src, src, classperms, cond_node, cond_flavor);
 			if (rc != SEPOL_OK) {
 				ebitmap_destroy(&type_bitmap);
 				goto exit;
@@ -1538,7 +1422,7 @@ int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct cil_a
 		}
 		ebitmap_destroy(&type_bitmap);
 	} else {
-		rc = __cil_avrule_expand(pdb, kind, src, tgt, classperms, neverallows, cond_node, cond_flavor);
+		rc = __cil_avrule_expand(pdb, kind, src, tgt, classperms, cond_node, cond_flavor);
 		if (rc != SEPOL_OK) goto exit;
 	}
 
@@ -1548,9 +1432,9 @@ exit:
 	return rc;
 }
 
-int cil_avrule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_avrule *cil_avrule, struct cil_list *neverallows)
+int cil_avrule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_avrule *cil_avrule)
 {
-	return __cil_avrule_to_avtab(pdb, db, cil_avrule, neverallows, NULL, CIL_FALSE);
+	return __cil_avrule_to_avtab(pdb, db, cil_avrule, NULL, CIL_FALSE);
 }
 
 int __cil_cond_to_policydb_helper(struct cil_tree_node *node, __attribute__((unused)) uint32_t *finished, void *extra_args)
@@ -1593,7 +1477,7 @@ int __cil_cond_to_policydb_helper(struct cil_tree_node *node, __attribute__((unu
 		break;
 	case CIL_AVRULE:
 		cil_avrule = node->data;
-		rc = __cil_avrule_to_avtab(pdb, db, cil_avrule, args->neverallows, cond_node, cond_flavor);
+		rc = __cil_avrule_to_avtab(pdb, db, cil_avrule, cond_node, cond_flavor);
 		if (rc != SEPOL_OK) {
 			cil_log(CIL_ERR, "Failed to insert avrule into avtab at line %d of %s\n", node->line, node->path);
 			goto exit;
@@ -1762,7 +1646,7 @@ static int __cil_cond_expr_to_sepol_expr(policydb_t *pdb, struct cil_list *cil_e
 	return SEPOL_OK;
 }
 
-int cil_booleanif_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_tree_node *node, struct cil_list *neverallows, hashtab_t filename_trans_table)
+int cil_booleanif_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_tree_node *node, hashtab_t filename_trans_table)
 {
 	int rc = SEPOL_ERR;
 	struct cil_args_booleanif bool_args;
@@ -1837,7 +1721,6 @@ int cil_booleanif_to_policydb(policydb_t *pdb, const struct cil_db *db, struct c
 	bool_args.db = db;
 	bool_args.pdb = pdb;
 	bool_args.cond_node = cond_node;
-	bool_args.neverallows = neverallows;
 	bool_args.filename_trans_table = filename_trans_table;
 
 	if (true_node != NULL) {
@@ -3202,17 +3085,9 @@ int __cil_node_to_policydb(struct cil_tree_node *node, void *extra_args)
 			break;
 		case CIL_AVRULE: {
 			struct cil_avrule *rule = node->data;
-			struct cil_list *neverallows = args->neverallows;
-			if (rule->rule_kind == CIL_AVRULE_NEVERALLOW) {
-				struct cil_neverallow *new_rule = NULL;
-
-				new_rule = cil_malloc(sizeof(*new_rule));
-				cil_list_init(&new_rule->rules, CIL_LIST_ITEM);
-				new_rule->node = node;
-
-				cil_list_prepend(neverallows, CIL_LIST_ITEM, new_rule);
-
-				rc = cil_avrule_to_policydb(pdb, db, node->data, neverallows);
+			if (db->disable_neverallow != CIL_TRUE && rule->rule_kind == CIL_AVRULE_NEVERALLOW) {
+				struct cil_list *neverallows = args->neverallows;
+				cil_list_prepend(neverallows, CIL_LIST_ITEM, node);
 			}
 			break;
 		}
@@ -3261,12 +3136,12 @@ int __cil_node_to_policydb(struct cil_tree_node *node, void *extra_args)
 	case 3:
 		switch (node->flavor) {
 		case CIL_BOOLEANIF:
-			rc = cil_booleanif_to_policydb(pdb, db, node, args->neverallows, filename_trans_table);
+			rc = cil_booleanif_to_policydb(pdb, db, node, filename_trans_table);
 			break;
 		case CIL_AVRULE: {
 				struct cil_avrule *rule = node->data;
 				if (rule->rule_kind != CIL_AVRULE_NEVERALLOW) {
-					rc = cil_avrule_to_policydb(pdb, db, node->data, args->neverallows);
+					rc = cil_avrule_to_policydb(pdb, db, node->data);
 				}
 			}
 			break;
@@ -3728,6 +3603,289 @@ exit:
 	return rc;
 }
 
+static void __cil_destroy_sepol_class_perms(class_perm_node_t *curr)
+{
+	class_perm_node_t *next;
+
+	while (curr) {
+		next = curr->next;
+		free(curr);
+		curr = next;
+	}
+}
+
+static int __cil_rule_to_sepol_class_perms(policydb_t *pdb, struct cil_list *classperms, class_perm_node_t **sepol_class_perms)
+{
+	int rc = SEPOL_ERR;
+	struct cil_list_item *i;
+	cil_list_for_each(i, classperms) {
+		if (i->flavor == CIL_CLASSPERMS) {
+			struct cil_classperms *cp = i->data;
+			if (FLAVOR(cp->class) == CIL_CLASS) {
+				class_perm_node_t *cpn = NULL;
+				class_datum_t *sepol_class = NULL;
+				uint32_t data = 0;
+
+				rc = __cil_get_sepol_class_datum(pdb, DATUM(cp->class), &sepol_class);
+				if (rc != SEPOL_OK) goto exit;
+
+				rc = __cil_perms_to_datum(cp->perms, sepol_class, &data);
+				if (rc != SEPOL_OK) goto exit;
+				if (data == 0) {
+					/* No permissions */
+					return SEPOL_OK;
+				}
+				cpn = cil_malloc(sizeof(class_perm_node_t));
+				cpn->tclass = sepol_class->s.value;
+				cpn->data = data;
+				cpn->next = *sepol_class_perms;
+				*sepol_class_perms = cpn;
+			} else { /* MAP */
+				struct cil_list_item *j = NULL;
+				cil_list_for_each(j, cp->perms) {
+					struct cil_perm *cmp = j->data;
+					rc = __cil_rule_to_sepol_class_perms(pdb, cmp->classperms, sepol_class_perms);
+					if (rc != SEPOL_OK) {
+						goto exit;
+					}
+				}
+			}
+		} else { /* SET */
+			struct cil_classperms_set *cp_set = i->data;
+			struct cil_classpermission *cp = cp_set->set;
+			rc = __cil_rule_to_sepol_class_perms(pdb, cp->classperms, sepol_class_perms);
+			if (rc != SEPOL_OK) {
+				goto exit;
+			}
+		}
+	}
+	return SEPOL_OK;
+
+exit:
+	return rc;
+}
+
+static void __cil_init_sepol_type_set(type_set_t *t)
+{
+	ebitmap_init(&t->types);
+	ebitmap_init(&t->negset);
+	t->flags = 0;
+}
+
+static int __cil_add_sepol_type(policydb_t *pdb, const struct cil_db *db, struct cil_symtab_datum *datum, ebitmap_t *map)
+{
+	int rc = SEPOL_ERR;
+	struct cil_tree_node *n = datum->nodes->head->data;
+	type_datum_t *sepol_datum = NULL;
+
+	if (n->flavor == CIL_TYPEATTRIBUTE) {
+		ebitmap_node_t *tnode;
+		unsigned int i;
+		struct cil_typeattribute *attr = (struct cil_typeattribute *)datum;
+		ebitmap_for_each_bit(attr->types, tnode, i) {
+			if (!ebitmap_get_bit(attr->types, i)) continue;
+			datum = DATUM(db->val_to_type[i]);
+			rc = __cil_get_sepol_type_datum(pdb, datum, &sepol_datum);
+			if (rc != SEPOL_OK) goto exit;
+			ebitmap_set_bit(map, sepol_datum->s.value - 1, 1);
+		}
+	} else {
+		rc = __cil_get_sepol_type_datum(pdb, datum, &sepol_datum);
+		if (rc != SEPOL_OK) goto exit;
+		ebitmap_set_bit(map, sepol_datum->s.value - 1, 1);
+	}
+
+	return SEPOL_OK;
+
+exit:
+	return rc;
+}
+
+static avrule_t *__cil_init_sepol_avrule(uint32_t kind, struct cil_tree_node *node)
+{
+	avrule_t *avrule;
+
+	avrule = cil_malloc(sizeof(avrule_t));
+	avrule->specified = kind;
+	avrule->flags = 0;
+	__cil_init_sepol_type_set(&avrule->stypes);
+	__cil_init_sepol_type_set(&avrule->ttypes);
+	avrule->perms = NULL;
+	avrule->line = node->line;
+	avrule->source_filename = node->path;
+	avrule->source_line = node->line;
+	avrule->next = NULL;
+	return avrule;
+}
+
+static void __cil_destroy_sepol_avrules(avrule_t *curr)
+{
+	avrule_t *next;
+
+	while (curr) {
+		next = curr->next;
+		ebitmap_destroy(&curr->stypes.types);
+		ebitmap_destroy(&curr->stypes.negset);
+		ebitmap_destroy(&curr->ttypes.types);
+		ebitmap_destroy(&curr->ttypes.negset);
+		__cil_destroy_sepol_class_perms(curr->perms);
+		free(curr);
+		curr = next;
+	}
+}
+
+static int __cil_rule_to_expanded_sepol_avrule(const struct cil_db *db, policydb_t *pdb, struct cil_tree_node *node, avrule_t **avrule)
+{
+	int rc = SEPOL_ERR;
+	struct cil_avrule *cil_rule = node->data;
+	struct cil_symtab_datum *tgt = cil_rule->tgt;
+	uint32_t kind;
+	avrule_t *rule;
+
+	*avrule = NULL;
+
+	switch (cil_rule->rule_kind) {
+	case CIL_AVRULE_AUDITALLOW:
+		kind = AVRULE_AUDITALLOW;
+		break;
+	case CIL_AVRULE_DONTAUDIT:
+		kind = AVRULE_AUDITDENY;
+		break;
+	case CIL_AVRULE_NEVERALLOW:
+		kind = AVRULE_NEVERALLOW;
+		break;
+	default:
+		kind = AVRULE_ALLOWED;
+		break;
+	}
+
+	rule = __cil_init_sepol_avrule(kind, node);
+
+	rc = __cil_rule_to_sepol_class_perms(pdb, cil_rule->classperms, &rule->perms);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	rc = __cil_add_sepol_type(pdb, db, cil_rule->src, &rule->stypes.types);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	if (tgt->fqn == CIL_KEY_SELF) {
+		rule->flags = RULE_SELF;
+	} else {
+		rc = __cil_add_sepol_type(pdb, db, cil_rule->tgt, &rule->ttypes.types);
+		if (rc != SEPOL_OK) {
+			goto exit;
+		}
+	}
+
+	rule->next = NULL;
+	*avrule = rule;
+
+	return SEPOL_OK;
+
+exit:
+	__cil_destroy_sepol_avrules(rule);
+	return rc;
+}
+
+static void __cil_print_parents(const char *pad, struct cil_tree_node *n)
+{
+	if (!n) return;
+
+	__cil_print_parents(pad, n->parent);
+
+	if (!n->path) {
+		cil_log(CIL_ERR,"%s%s\n", pad, cil_node_to_string(n));
+	} else {
+		cil_log(CIL_ERR,"%s%s at line %d of %s\n", pad, cil_node_to_string(n), n->line, n->path);
+	}
+}
+
+static void __cil_print_rule(const char *pad, const char *kind, struct cil_avrule *avrule)
+{
+	struct cil_list *cp_list = avrule->classperms;
+	struct cil_list_item *i1, *i2;
+
+	cil_log(CIL_ERR,"%s(%s ", pad, kind);
+	cil_log(CIL_ERR,"%s %s ", DATUM(avrule->src)->fqn, DATUM(avrule->tgt)->fqn);
+
+	i1 = cp_list->head;
+	if (i1->flavor == CIL_CLASSPERMS) {
+		struct cil_classperms *cp = i1->data;
+		cil_log(CIL_ERR,"(%s (", DATUM(cp->class)->fqn);
+		cil_list_for_each(i2, cp->perms) {
+			cil_log(CIL_ERR,"%s",DATUM(i2->data)->fqn);
+			if (i2 != cp->perms->tail) {
+				cil_log(CIL_ERR," ");
+			} else {
+				cil_log(CIL_ERR,"))");
+			}
+		}
+	} else {
+		struct cil_classperms_set *cp_set = i1->data;
+		cil_log(CIL_ERR,"%s", DATUM(cp_set->set)->fqn);
+	}
+
+	cil_log(CIL_ERR,")\n");
+}
+
+static int cil_check_neverallows(const struct cil_db *db, policydb_t *pdb, struct cil_list *neverallows)
+{
+	int rc = SEPOL_OK;
+	struct cil_list_item *i1;
+
+	cil_list_for_each(i1, neverallows) {
+		struct cil_tree_node *node = i1->data;
+		avrule_t *avrule;
+		rc = __cil_rule_to_expanded_sepol_avrule(db, pdb, node, &avrule);
+		if (rc != SEPOL_OK) {
+			cil_log(CIL_ERR, "Failed to create expanded sepol avrules to check neverallow rules\n");
+			goto exit;
+		}
+
+		rc = check_assertion(pdb, avrule);
+
+		if (rc == CIL_TRUE) {
+			struct cil_list_item *i2;
+			struct cil_list *matching;
+			struct cil_avrule *cil_rule = node->data;
+			struct cil_avrule target;
+			struct cil_tree_node *n2;
+			struct cil_avrule *r2;
+			target.rule_kind = CIL_AVRULE_ALLOWED;
+			target.src = cil_rule->src;
+			target.tgt = cil_rule->tgt;
+			target.classperms = cil_rule->classperms;
+			cil_log(CIL_ERR, "Neverallow check failed at line %d of %s\n", node->line, node->path);
+			__cil_print_rule("  ", "neverallow", cil_rule);
+			cil_list_init(&matching, CIL_NODE);
+			rc = cil_find_matching_avrule_in_ast(db->ast->root, CIL_AVRULE, &target, matching, CIL_FALSE);
+			if (rc) {
+				cil_log(CIL_ERR, "Error occurred while checking neverallow rules\n");
+				cil_list_destroy(&matching, CIL_FALSE);
+				__cil_destroy_sepol_avrules(avrule);
+				goto exit;
+			}
+	
+			cil_list_for_each(i2, matching) {
+				n2 = i2->data;
+				r2 = n2->data;
+				__cil_print_parents("    ", n2);
+				__cil_print_rule("      ", "allow", r2);
+			}
+			cil_log(CIL_ERR,"\n");
+			cil_list_destroy(&matching, CIL_FALSE);
+		}
+		__cil_destroy_sepol_avrules(avrule);
+		avrule = NULL;
+	}
+
+exit:
+	return rc;
+}
+
 // assumes policydb is already allocated and initialized properly with things
 // like policy type set to kernel and version set appropriately
 int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *policydb)
@@ -3822,13 +3980,19 @@ int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *p
 	cond_optimize_lists(pdb->cond_list);
 	__cil_set_conditional_state_and_flags(pdb);
 
+	if (db->disable_neverallow != CIL_TRUE) {
+		cil_log(CIL_INFO, "Checking Neverallows\n");
+		rc = cil_check_neverallows(db, pdb, neverallows);
+		if (rc != SEPOL_OK) goto exit;
+	}
+
 	rc = SEPOL_OK;
 
 exit:
 	hashtab_destroy(filename_trans_table);
 	hashtab_destroy(range_trans_table);
 	hashtab_destroy(role_trans_table);
-	cil_neverallows_list_destroy(neverallows);
+	cil_list_destroy(&neverallows, CIL_FALSE);
 
 	return rc;
 }
diff --git a/libsepol/cil/src/cil_binary.h b/libsepol/cil/src/cil_binary.h
index 5045c6e..72c0f95 100644
--- a/libsepol/cil/src/cil_binary.h
+++ b/libsepol/cil/src/cil_binary.h
@@ -250,7 +250,7 @@ int cil_type_rule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct c
  *
  * @return SEPOL_OK upon success or an error otherwise.
  */
-int cil_avrule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_avrule *cil_avrule, struct cil_list *neverallows);
+int cil_avrule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_avrule *cil_avrule);
 
 /**
  * Insert cil booleanif structure into sepol policydb.  This populates the
@@ -262,7 +262,7 @@ int cil_avrule_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_
  *
  * @return SEPOL_OK upon success or an error otherwise.
  */
-int cil_booleanif_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_tree_node *node, struct cil_list *neverallows, hashtab_t filename_trans_table);
+int cil_booleanif_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_tree_node *node, hashtab_t filename_trans_table);
 
 /**
  * Insert cil role transition structure into sepol policydb.
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 07/10 v2] libsepol/cil: Track number of classes and number of types and attributes.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (5 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 06/10 v2] libsepol/cil: Refactored CIL neverallow checking and reporting James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-17 19:58 ` [PATCH 08/10 v2] libsepol/cil: Add CIL bounds checking and reporting James Carter
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

These values are stored in the CIL db so they can be used to
determine how much memory is needed for mapping libsepol values
to CIL data.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/cil/src/cil.c          |  3 +-
 libsepol/cil/src/cil_binary.c   | 72 +++++++++++++++++++++++++++++++++--------
 libsepol/cil/src/cil_binary.h   |  4 +--
 libsepol/cil/src/cil_internal.h |  2 ++
 libsepol/cil/src/cil_post.c     | 26 +++++++++++----
 5 files changed, 84 insertions(+), 23 deletions(-)

diff --git a/libsepol/cil/src/cil.c b/libsepol/cil/src/cil.c
index 5c53bf3..be070de 100644
--- a/libsepol/cil/src/cil.c
+++ b/libsepol/cil/src/cil.c
@@ -257,7 +257,8 @@ void cil_db_init(struct cil_db **db)
 	cil_type_init(&(*db)->selftype);
 	(*db)->selftype->datum.name = CIL_KEY_SELF;
 	(*db)->selftype->datum.fqn = CIL_KEY_SELF;
-
+	(*db)->num_types_and_attrs = 0;
+	(*db)->num_classes = 0;
 	(*db)->num_types = 0;
 	(*db)->num_roles = 0;
 	(*db)->num_cats = 0;
diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index 2700b34..f2cd938 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -55,6 +55,7 @@
 #define FILENAME_TRANS_TABLE_SIZE 1 << 16
 #define RANGE_TRANS_TABLE_SIZE 1 << 13
 #define ROLE_TRANS_TABLE_SIZE 1 << 10
+#define PERMS_PER_CLASS 32
 
 struct cil_args_binary {
 	const struct cil_db *db;
@@ -64,6 +65,7 @@ struct cil_args_binary {
 	hashtab_t filename_trans_table;
 	hashtab_t range_trans_table;
 	hashtab_t role_trans_table;
+	void **type_value_to_cil;
 };
 
 struct cil_args_booleanif {
@@ -303,7 +305,7 @@ exit:
 	return rc;
 }
 
-int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db)
+int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db, struct cil_class *class_value_to_cil[], struct cil_perm **perm_value_to_cil[])
 {
 	int rc = SEPOL_ERR;
 	struct cil_list_item *curr_class;
@@ -312,8 +314,8 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db)
 		struct cil_class *cil_class = curr_class->data;
 		uint32_t value = 0;
 		char *key = NULL;
-		struct cil_tree_node *node = cil_class->datum.nodes->head->data;
-		struct cil_tree_node *cil_perm = node->cl_head;
+		int class_index;
+		struct cil_tree_node *curr;
 		common_datum_t *sepol_common = NULL;
 		class_datum_t *sepol_class = cil_malloc(sizeof(*sepol_class));
 		memset(sepol_class, 0, sizeof(class_datum_t));
@@ -326,6 +328,8 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db)
 			goto exit;
 		}
 		sepol_class->s.value = value;
+		class_index = value;
+		class_value_to_cil[class_index] = cil_class;
 
 		rc = symtab_init(&sepol_class->permissions, PERM_SYMTAB_SIZE);
 		if (rc != SEPOL_OK) {
@@ -333,6 +337,7 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db)
 		}
 
 		if (cil_class->common != NULL) {
+			int i;
 			struct cil_class *cil_common = cil_class->common;
 
 			key = cil_class->common->datum.fqn;
@@ -346,14 +351,19 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db)
 			sepol_class->comdatum = sepol_common;
 			sepol_class->comkey = cil_strdup(key);
 			sepol_class->permissions.nprim += sepol_common->permissions.nprim;
+
+			for (curr = NODE(cil_class->common)->cl_head, i = 1; curr; curr = curr->next, i++) {
+				struct cil_perm *cil_perm = curr->data;
+				perm_value_to_cil[class_index][i] = cil_perm;
+			}
 		}
 
-		while (cil_perm != NULL) {
-			struct cil_perm *curr_perm = cil_perm->data;
+		for (curr = NODE(cil_class)->cl_head; curr; curr = curr->next) {
+			struct cil_perm *cil_perm = curr->data;
 			perm_datum_t *sepol_perm = cil_malloc(sizeof(*sepol_perm));
 			memset(sepol_perm, 0, sizeof(perm_datum_t));
 
-			key = cil_strdup(curr_perm->datum.fqn);
+			key = cil_strdup(cil_perm->datum.fqn);
 			rc = hashtab_insert(sepol_class->permissions.table, key, sepol_perm);
 			if (rc != SEPOL_OK) {
 				free(sepol_perm);
@@ -362,7 +372,7 @@ int cil_classorder_to_policydb(policydb_t *pdb, const struct cil_db *db)
 			}
 			sepol_perm->s.value = sepol_class->permissions.nprim + 1;
 			sepol_class->permissions.nprim++;
-			cil_perm = cil_perm->next;
+			perm_value_to_cil[class_index][sepol_perm->s.value] = cil_perm;
 		}
 	}
 
@@ -463,7 +473,7 @@ exit:
 	return rc;
 }
 
-int cil_type_to_policydb(policydb_t *pdb, struct cil_type *cil_type)
+int cil_type_to_policydb(policydb_t *pdb, struct cil_type *cil_type, void *type_value_to_cil[])
 {
 	int rc = SEPOL_ERR;
 	uint32_t value = 0;
@@ -481,6 +491,8 @@ int cil_type_to_policydb(policydb_t *pdb, struct cil_type *cil_type)
 	sepol_type->s.value = value;
 	sepol_type->primary = 1;
 
+	type_value_to_cil[value] = cil_type;
+
 	return SEPOL_OK;
 
 exit:
@@ -564,7 +576,7 @@ exit:
 
 }
 
-int cil_typeattribute_to_policydb(policydb_t *pdb, struct cil_typeattribute *cil_attr)
+int cil_typeattribute_to_policydb(policydb_t *pdb, struct cil_typeattribute *cil_attr, void *type_value_to_cil[])
 {
 	int rc = SEPOL_ERR;
 	uint32_t value = 0;
@@ -588,6 +600,8 @@ int cil_typeattribute_to_policydb(policydb_t *pdb, struct cil_typeattribute *cil
 	sepol_attr->s.value = value;
 	sepol_attr->primary = 1;
 
+	type_value_to_cil[value] = cil_attr;
+
 	return SEPOL_OK;
 
 exit:
@@ -2998,12 +3012,15 @@ int __cil_node_to_policydb(struct cil_tree_node *node, void *extra_args)
 	hashtab_t filename_trans_table;
 	hashtab_t range_trans_table;
 	hashtab_t role_trans_table;
+	void **type_value_to_cil;
+
 	db = args->db;
 	pdb = args->pdb;
 	pass = args->pass;
 	filename_trans_table = args->filename_trans_table;
 	range_trans_table = args->range_trans_table;
 	role_trans_table = args->role_trans_table;
+	type_value_to_cil = args->type_value_to_cil;
 
 	if (node->flavor >= CIL_MIN_DECLARATIVE) {
 		if (node != DATUM(node->data)->nodes->head->data) {
@@ -3018,10 +3035,10 @@ int __cil_node_to_policydb(struct cil_tree_node *node, void *extra_args)
 			rc = cil_role_to_policydb(pdb, node->data);
 			break;
 		case CIL_TYPE:
-			rc = cil_type_to_policydb(pdb, node->data);
+			rc = cil_type_to_policydb(pdb, node->data, type_value_to_cil);
 			break;
 		case CIL_TYPEATTRIBUTE:
-			rc = cil_typeattribute_to_policydb(pdb, node->data);
+			rc = cil_typeattribute_to_policydb(pdb, node->data, type_value_to_cil);
 			break;
 		case CIL_POLICYCAP:
 			rc = cil_policycap_to_policydb(pdb, node->data);
@@ -3483,7 +3500,7 @@ exit:
 }
 
 
-int __cil_policydb_init(policydb_t *pdb, const struct cil_db *db)
+int __cil_policydb_init(policydb_t *pdb, const struct cil_db *db, struct cil_class *class_value_to_cil[], struct cil_perm **perm_value_to_cil[])
 {
 	int rc = SEPOL_ERR;
 
@@ -3493,7 +3510,7 @@ int __cil_policydb_init(policydb_t *pdb, const struct cil_db *db)
 	pdb->handle_unknown = db->handle_unknown;
 	pdb->mls = db->mls;
 
-	rc = cil_classorder_to_policydb(pdb, db);
+	rc = cil_classorder_to_policydb(pdb, db, class_value_to_cil, perm_value_to_cil);
 	if (rc != SEPOL_OK) {
 		goto exit;
 	}
@@ -3898,6 +3915,9 @@ int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *p
 	hashtab_t filename_trans_table = NULL;
 	hashtab_t range_trans_table = NULL;
 	hashtab_t role_trans_table = NULL;
+	void **type_value_to_cil;
+	struct cil_class **class_value_to_cil;
+	struct cil_perm ***perm_value_to_cil;
 
 	if (db == NULL || policydb == NULL) {
 		if (db == NULL) {
@@ -3908,7 +3928,22 @@ int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *p
 		return SEPOL_ERR;
 	}
 
-	rc = __cil_policydb_init(pdb, db);
+	/* libsepol values start at 1. Just allocate extra memory rather than
+	 * subtract 1 from the sepol value.
+	 */
+	type_value_to_cil = calloc(db->num_types_and_attrs+1, sizeof(*type_value_to_cil));
+	if (!type_value_to_cil) goto exit;
+
+	class_value_to_cil = calloc(db->num_classes+1, sizeof(*class_value_to_cil));
+	if (!class_value_to_cil) goto exit;
+
+	perm_value_to_cil = calloc(db->num_classes+1, sizeof(*perm_value_to_cil));
+	if (!perm_value_to_cil) goto exit;
+	for (i=1; i < db->num_classes+1; i++) {
+		perm_value_to_cil[i] = calloc(PERMS_PER_CLASS+1, sizeof(*perm_value_to_cil[i]));
+	}
+
+	rc = __cil_policydb_init(pdb, db, class_value_to_cil, perm_value_to_cil);
 	if (rc != SEPOL_OK) {
 		cil_log(CIL_ERR,"Problem in policydb_init\n");
 		goto exit;
@@ -3940,6 +3975,8 @@ int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *p
 	extra_args.filename_trans_table = filename_trans_table;
 	extra_args.range_trans_table = range_trans_table;
 	extra_args.role_trans_table = role_trans_table;
+	extra_args.type_value_to_cil = type_value_to_cil;
+
 	for (i = 1; i <= 3; i++) {
 		extra_args.pass = i;
 
@@ -3992,6 +4029,13 @@ exit:
 	hashtab_destroy(filename_trans_table);
 	hashtab_destroy(range_trans_table);
 	hashtab_destroy(role_trans_table);
+	free(type_value_to_cil);
+	free(class_value_to_cil);
+	/* Range is because libsepol values start at 1. */
+	for (i=1; i < db->num_classes+1; i++) {
+		free(perm_value_to_cil[i]);
+	}
+	free(perm_value_to_cil);
 	cil_list_destroy(&neverallows, CIL_FALSE);
 
 	return rc;
diff --git a/libsepol/cil/src/cil_binary.h b/libsepol/cil/src/cil_binary.h
index 72c0f95..33b43f9 100644
--- a/libsepol/cil/src/cil_binary.h
+++ b/libsepol/cil/src/cil_binary.h
@@ -112,7 +112,7 @@ int cil_roletype_to_policydb(policydb_t *pdb, const struct cil_db *db, struct ci
  *
  * @return SEPOL_OK upon success or an error otherwise.
  */
-int cil_type_to_policydb(policydb_t *pdb, struct cil_type *cil_type);
+int cil_type_to_policydb(policydb_t *pdb, struct cil_type *cil_type, void *type_value_to_cil[]);
 
 /**
  * Insert cil typealias structure into sepol policydb.
@@ -144,7 +144,7 @@ int cil_typepermissive_to_policydb(policydb_t *pdb, struct cil_typepermissive *c
  *
  * @return SEPOL_OK upon success or an error otherwise.
  */
-int cil_typeattribute_to_policydb(policydb_t *pdb, struct cil_typeattribute *cil_attr);
+int cil_typeattribute_to_policydb(policydb_t *pdb, struct cil_typeattribute *cil_attr, void *type_value_to_cil[]);
 
 /**
  * Insert cil attribute structure into sepol type->attribute bitmap.
diff --git a/libsepol/cil/src/cil_internal.h b/libsepol/cil/src/cil_internal.h
index a43d111..abe52de 100644
--- a/libsepol/cil/src/cil_internal.h
+++ b/libsepol/cil/src/cil_internal.h
@@ -279,6 +279,8 @@ struct cil_db {
 	struct cil_list *userprefixes;
 	struct cil_list *selinuxusers;
 	struct cil_list *names;
+	int num_types_and_attrs;
+	int num_classes;
 	int num_cats;
 	int num_types;
 	int num_roles;
diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index f91727f..e69b5a4 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -339,23 +339,37 @@ static int __cil_post_db_count_helper(struct cil_tree_node *node, uint32_t *fini
 	case CIL_MACRO:
 		*finished = CIL_TREE_SKIP_HEAD;
 		break;
+	case CIL_CLASS: {
+		struct cil_class *class = node->data;
+		if (class->datum.nodes->head->data == node) {
+			// Multiple nodes can point to the same datum. Only count once.
+			db->num_classes++;
+		}
+		break;
+	}
 	case CIL_TYPE: {
 		struct cil_type *type = node->data;
 		if (type->datum.nodes->head->data == node) {
-			// multiple AST nodes can point to the same cil_type data (like if
-			// copied from a macro). This check ensures we only count the
-			// duplicates once
+			// Multiple nodes can point to the same datum. Only count once.
 			type->value = db->num_types;
 			db->num_types++;
+			db->num_types_and_attrs++;
 		}
 		break;
 	}
+	case CIL_TYPEATTRIBUTE: {
+		struct cil_typeattribute *attr = node->data;
+		if (attr->datum.nodes->head->data == node) {
+			// Multiple nodes can point to the same datum. Only count once.
+			db->num_types_and_attrs++;
+		}
+		break;
+	}
+
 	case CIL_ROLE: {
 		struct cil_role *role = node->data;
 		if (role->datum.nodes->head->data == node) {
-			// multiple AST nodes can point to the same cil_role data (like if
-			// copied from a macro). This check ensures we only count the
-			// duplicates once
+			// Multiple nodes can point to the same datum. Only count once.
 			role->value = db->num_roles;
 			db->num_roles++;
 		}
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 08/10 v2] libsepol/cil: Add CIL bounds checking and reporting.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (6 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 07/10 v2] libsepol/cil: Track number of classes and number of types and attributes James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-17 19:58 ` [PATCH 09/10 v2] secilc: Add a CIL policy file to test neverallow checking James Carter
  2015-06-17 19:58 ` [PATCH 10/10 v2] secilc: Add a CIL policy file to test bounds checking James Carter
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

Use the libsepol bounds checking to check for and report user and
role bounds violations.

For type bounds checking, use libsepol bounds checking to determine
if there is a violation for a given type. For each violation display
an error message that includes the CIL AST from the root node to the
node of the rule causing the violation.

Example error report:
Child type b_t3_c exceeds bounds of parent b_t3
  (allow b_t3_c b_tc (file (write)))
    <root>
    booleanif at line 148633 of cil.conf.bounds
    true at line 148634 of cil.conf.bounds
    allow at line 148636 of cil.conf.bounds
      (allow b_t3_c b_tc (file (read write)))

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 libsepol/cil/src/cil_binary.c | 135 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index f2cd938..3fade7f 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -3903,6 +3903,130 @@ exit:
 	return rc;
 }
 
+static struct cil_list *cil_classperms_from_sepol(const struct cil_db *db, policydb_t *pdb, uint16_t class, uint32_t data, struct cil_class *class_value_to_cil[], struct cil_perm **perm_value_to_cil[])
+{
+	struct cil_classperms *cp;
+	struct cil_list *cp_list;
+	class_datum_t *sepol_class = pdb->class_val_to_struct[class - 1];
+	int i;
+
+	cil_classperms_init(&cp);
+
+	cp->class = class_value_to_cil[class];
+	if (!cp->class) goto exit;
+
+	cil_list_init(&cp->perms, CIL_PERM);
+	for (i = 0; i < sepol_class->permissions.nprim; i++) {
+		struct cil_perm *perm;
+		if ((data & (1 << i)) == 0) continue;
+		perm = perm_value_to_cil[class][i+1];
+		if (!perm) goto exit;
+		cil_list_append(cp->perms, CIL_PERM, perm);
+	}
+
+	cil_list_init(&cp_list, CIL_CLASSPERMS);
+	cil_list_append(cp_list, CIL_CLASSPERMS, cp);
+
+	return cp_list;
+
+exit:
+	cil_log(CIL_ERR,"Failed to create CIL class-permissions from sepol values\n");
+	return NULL;
+}
+
+static int cil_avrule_from_sepol(const struct cil_db *db, policydb_t *pdb, avtab_ptr_t sepol_rule, struct cil_avrule *cil_rule, void *type_value_to_cil[], struct cil_class *class_value_to_cil[], struct cil_perm **perm_value_to_cil[])
+{
+	int rc = SEPOL_ERR;
+	avtab_key_t *k = &sepol_rule->key;
+	avtab_datum_t *d = &sepol_rule->datum;
+	cil_rule->src = type_value_to_cil[k->source_type];
+	if (!cil_rule->src) goto exit;
+
+	cil_rule->tgt = type_value_to_cil[k->target_type];
+	if (!cil_rule->tgt) goto exit;
+
+	cil_rule->classperms = cil_classperms_from_sepol(db, pdb, k->target_class, d->data, class_value_to_cil, perm_value_to_cil);
+	if (!cil_rule->classperms) goto exit;
+
+	return SEPOL_OK;
+
+exit:
+	cil_log(CIL_ERR,"Failed to create CIL AV rule from sepol values\n");
+	return rc;
+}
+
+static int cil_check_type_bounds(const struct cil_db *db, policydb_t *pdb, void *type_value_to_cil, struct cil_class *class_value_to_cil[], struct cil_perm **perm_value_to_cil[])
+{
+	int rc = SEPOL_OK;
+	int i;
+
+	for (i = 0; i < db->num_types; i++) {
+		type_datum_t *child;
+		type_datum_t *parent;
+		avtab_ptr_t bad = NULL;
+		int numbad = 0;
+		struct cil_type *t = db->val_to_type[i];
+
+		if (!t->bounds) continue;
+
+		rc = __cil_get_sepol_type_datum(pdb, DATUM(t), &child);
+		if (rc != SEPOL_OK) goto exit;
+
+		rc = __cil_get_sepol_type_datum(pdb, DATUM(t->bounds), &parent);
+		if (rc != SEPOL_OK) goto exit;
+
+		rc = bounds_check_type(NULL, pdb, child->s.value, parent->s.value, &bad, &numbad);
+		if (rc != SEPOL_OK) goto exit;
+
+		if (bad) {
+			avtab_ptr_t cur;
+			struct cil_avrule target;
+
+			target.rule_kind = CIL_AVRULE_ALLOWED;
+			target.src_str = NULL;
+			target.tgt_str = NULL;
+
+			cil_log(CIL_ERR, "Child type %s exceeds bounds of parent %s\n",
+				t->datum.fqn, t->bounds->datum.fqn);
+			for (cur = bad; cur; cur = cur->next) {
+				struct cil_list_item *i2;
+				struct cil_list *matching;
+				struct cil_tree_node *n;
+
+				rc = cil_avrule_from_sepol(db, pdb, cur, &target, type_value_to_cil, class_value_to_cil, perm_value_to_cil);
+				if (rc != SEPOL_OK) {
+					cil_log(CIL_ERR, "Failed to convert sepol avrule to CIL\n");
+					goto exit;
+				}
+				__cil_print_rule("  ", "allow", &target);
+				cil_list_init(&matching, CIL_NODE);
+				rc = cil_find_matching_avrule_in_ast(db->ast->root, CIL_AVRULE, &target, matching, CIL_FALSE);
+				if (rc) {
+					cil_log(CIL_ERR, "Error occurred while checking type bounds\n");
+					cil_list_destroy(&matching, CIL_FALSE);
+					cil_list_destroy(&target.classperms, CIL_TRUE);
+					bounds_destroy_bad(bad);
+					goto exit;
+				}
+
+				cil_list_for_each(i2, matching) {
+					__cil_print_parents("    ", (struct cil_tree_node *)i2->data);
+				}
+				i2 = matching->tail;
+				n = i2->data;
+				__cil_print_rule("      ", "allow", n->data);
+				cil_log(CIL_ERR,"\n");
+				cil_list_destroy(&matching, CIL_FALSE);
+				cil_list_destroy(&target.classperms, CIL_TRUE);
+			}
+			bounds_destroy_bad(bad);
+		}
+	}
+
+exit:
+	return rc;
+}
+
 // assumes policydb is already allocated and initialized properly with things
 // like policy type set to kernel and version set appropriately
 int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *policydb)
@@ -4021,6 +4145,17 @@ int cil_binary_create_allocated_pdb(const struct cil_db *db, sepol_policydb_t *p
 		cil_log(CIL_INFO, "Checking Neverallows\n");
 		rc = cil_check_neverallows(db, pdb, neverallows);
 		if (rc != SEPOL_OK) goto exit;
+
+		cil_log(CIL_INFO, "Checking User Bounds\n");
+		bounds_check_users(NULL, pdb);
+
+		cil_log(CIL_INFO, "Checking Role Bounds\n");
+		bounds_check_roles(NULL, pdb);
+
+		cil_log(CIL_INFO, "Checking Type Bounds\n");
+		rc = cil_check_type_bounds(db, pdb, type_value_to_cil, class_value_to_cil, perm_value_to_cil);
+		if (rc != SEPOL_OK) goto exit;
+
 	}
 
 	rc = SEPOL_OK;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 09/10 v2] secilc: Add a CIL policy file to test neverallow checking.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (7 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 08/10 v2] libsepol/cil: Add CIL bounds checking and reporting James Carter
@ 2015-06-17 19:58 ` James Carter
  2015-06-17 19:58 ` [PATCH 10/10 v2] secilc: Add a CIL policy file to test bounds checking James Carter
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 secilc/test/neverallow.cil | 79 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 secilc/test/neverallow.cil

diff --git a/secilc/test/neverallow.cil b/secilc/test/neverallow.cil
new file mode 100644
index 0000000..6351558
--- /dev/null
+++ b/secilc/test/neverallow.cil
@@ -0,0 +1,79 @@
+(class CLASS (PERM))
+(classorder (CLASS))
+(sid SID)
+(sidorder (SID))
+(user USER)
+(role ROLE)
+(type TYPE)
+(category CAT)
+(categoryorder (CAT))
+(sensitivity SENS)
+(sensitivityorder (SENS))
+(sensitivitycategory SENS (CAT))
+(allow TYPE self (CLASS (PERM)))
+(roletype ROLE TYPE)
+(userrole USER ROLE)
+(userlevel USER (SENS))
+(userrange USER ((SENS)(SENS (CAT))))
+(sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))
+
+(class c1 (p1a p1b p1c))
+(class c2 (p2a p2b p2c))
+(class c3 (p3a p3b p3c))
+
+(classorder (CLASS c1 c2 c3))
+
+(classpermission cp1)
+(classpermissionset cp1 (c1 (p1a p1b)))
+(classpermissionset cp1 (c2 (p2a)))
+
+(classmap cm1 (mp1))
+(classmapping cm1 mp1
+	      (c1 (p1a)))
+
+(type t1)
+(type t2)
+(type t3)
+(type t4)
+(type t5)
+(type t6)
+(type t7)
+
+(typeattribute a1)
+(typeattribute a2)
+(typeattribute a3)
+(typeattribute a4)
+(typeattribute a5)
+(typeattribute a6)
+
+(typeattributeset a1 (t1 t2 t3 t4 t5))
+(typeattributeset a2 (t1 t2))
+(typeattributeset a3 (t3 t4))
+(typeattributeset a4 (t2 t3))
+(typeattributeset a5 (t5 t6))
+(typeattributeset a6 (t6 t7))
+
+(neverallow t1 t2 (c1 (p1a p1b)))
+(allow t1 t2 (c1 (p1a)))
+
+(neverallow t3 t4 (cm1 (mp1)))
+(allow t3 t4 (c1 (p1a)))
+
+(neverallow t5 t6 cp1)
+(allow t5 t6 (c1 (p1b)))
+(allow t5 t6 (c2 (p2a)))
+
+(neverallow a1 self (CLASS (PERM)))
+(allow t1 t1 (CLASS (PERM)))
+(allow t2 self (CLASS (PERM)))
+(allow a3 self (CLASS (PERM)))
+(allow a2 a4 (CLASS (PERM)))
+
+(neverallow a5 a6 (CLASS (PERM)))
+(allow t5 t7 (CLASS (PERM)))
+(allow t6 self (CLASS (PERM)))
+
+;; Should not call these violations
+(allow a1 self (c1 (p1a)))
+(allow a2 a3 (CLASS (PERM)))
+(allow t5 t6 (c2 (p2b)))
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 10/10 v2] secilc: Add a CIL policy file to test bounds checking.
  2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
                   ` (8 preceding siblings ...)
  2015-06-17 19:58 ` [PATCH 09/10 v2] secilc: Add a CIL policy file to test neverallow checking James Carter
@ 2015-06-17 19:58 ` James Carter
  9 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-17 19:58 UTC (permalink / raw)
  To: selinux

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
---
 secilc/test/bounds.cil | 241 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 241 insertions(+)
 create mode 100644 secilc/test/bounds.cil

diff --git a/secilc/test/bounds.cil b/secilc/test/bounds.cil
new file mode 100644
index 0000000..e72560e
--- /dev/null
+++ b/secilc/test/bounds.cil
@@ -0,0 +1,241 @@
+(class CLASS (PERM))
+(classorder (CLASS))
+(sid SID)
+(sidorder (SID))
+(user USER)
+(role ROLE)
+(type TYPE)
+(category CAT)
+(categoryorder (CAT))
+(sensitivity SENS)
+(sensitivityorder (SENS))
+(sensitivitycategory SENS (CAT))
+(allow TYPE self (CLASS (PERM)))
+(roletype ROLE TYPE)
+(userrole USER ROLE)
+(userlevel USER (SENS))
+(userrange USER ((SENS)(SENS (CAT))))
+(sidcontext SID (USER ROLE TYPE ((SENS)(SENS))))
+
+(class c1 (p1a p1b p1c))
+(class c2 (p2a p2b p2c))
+(class c3 (p3a p3b p3c))
+
+(classorder (CLASS c1 c2 c3))
+
+(classpermission cp1)
+(classpermissionset cp1 (c1 (p1a p1b)))
+(classpermissionset cp1 (c2 (p2a)))
+
+(classmap cm1 (mp1))
+(classmapping cm1 mp1
+	      (c1 (p1a)))
+
+(boolean b_b1 false)
+(boolean b_b2 false)
+(boolean b_b3 false)
+
+
+(type b_ta)
+(type b_tb)
+(type b_tc)
+(type b_td)
+
+
+;; All of these rules should pass the bounds check
+(type b_t1)
+(type b_t1_c)
+(typebounds b_t1 b_t1_c)
+
+(allow b_t1 self (CLASS (PERM)))
+(allow b_t1_c self (CLASS (PERM)))
+(allow b_t1 b_ta (CLASS (PERM)))
+(allow b_t1_c b_ta (CLASS (PERM)))
+(allow b_ta b_t1 (CLASS (PERM)))
+(allow b_ta b_t1_c (CLASS (PERM)))
+
+(booleanif b_b1
+  (false
+    (allow b_t1 b_tb (CLASS (PERM)))
+    (allow b_t1_c b_tb (CLASS (PERM)))
+    (allow b_tb b_t1 (CLASS (PERM)))
+    (allow b_tb b_t1_c (CLASS (PERM)))))
+
+(allow b_t1 b_tc (CLASS (PERM)))
+(allow b_tc b_t1 (CLASS (PERM)))
+(booleanif b_b2
+  (false
+    (allow b_t1_c b_tc (CLASS (PERM)))
+    (allow b_tc b_t1_c (CLASS (PERM)))))
+
+(allow b_t1_c b_td (CLASS (PERM)))
+(allow b_td b_t1_c (CLASS (PERM)))
+(booleanif b_b3
+  (true
+    (allow b_t1 b_td (CLASS (PERM)))
+    (allow b_td b_t1 (CLASS (PERM))))
+  (false
+    (allow b_t1 b_td (CLASS (PERM)))
+    (allow b_td b_t1 (CLASS (PERM)))))
+
+
+;; All of these rules should pass the bounds check
+(type b_t2)
+(type b_t2_c)
+(typebounds b_t2 b_t2_c)
+(typeattribute b_a2)
+(typeattribute b_a2_c)
+(typeattributeset b_a2 b_t2)
+(typeattributeset b_a2_c b_t2_c)
+
+(allow b_a2 self (CLASS (PERM)))
+(allow b_a2_c self (CLASS (PERM)))
+(allow b_a2 b_ta (CLASS (PERM)))
+(allow b_a2_c b_ta (CLASS (PERM)))
+(allow b_ta b_a2 (CLASS (PERM)))
+(allow b_ta b_a2_c (CLASS (PERM)))
+
+(booleanif b_b1
+  (false
+    (allow b_a2 b_tb (CLASS (PERM)))
+    (allow b_a2_c b_tb (CLASS (PERM)))
+    (allow b_tb b_a2 (CLASS (PERM)))
+    (allow b_tb b_a2_c (CLASS (PERM)))))
+
+(allow b_a2 b_tc (CLASS (PERM)))
+(allow b_tc b_a2 (CLASS (PERM)))
+(booleanif b_b2
+  (false
+    (allow b_a2_c b_tc (CLASS (PERM)))
+    (allow b_tc b_a2_c (CLASS (PERM)))))
+
+(allow b_a2_c b_td (CLASS (PERM)))
+(allow b_td b_a2_c (CLASS (PERM)))
+(booleanif b_b3
+  (true
+    (allow b_a2 b_td (CLASS (PERM)))
+    (allow b_td b_a2 (CLASS (PERM))))
+  (false
+    (allow b_a2 b_td (CLASS (PERM)))
+    (allow b_td b_a2 (CLASS (PERM)))))
+
+
+;; All of these rules should fail the bounds check
+(type b_t3)
+(type b_t3_c)
+(typebounds b_t3 b_t3_c)
+
+(allow b_t3 self (CLASS (PERM)))
+(allow b_t3_c self (c1 (p1a)))
+(allow b_t3 b_ta (CLASS (PERM)))
+(allow b_t3_c b_ta (c1 (p1a)))
+(allow b_ta b_t3 (CLASS (PERM)))
+(allow b_ta b_t3_c (c1 (p1a)))
+
+(booleanif b_b1
+  (false
+    (allow b_t3_c b_tb (c1 (p1a)))
+    (allow b_tb b_t3_c (c1 (p1a)))))
+
+(booleanif b_b2
+  (true
+    (allow b_t3_c b_tc (c1 (p1a)))
+    (allow b_tc b_t3_c (c1 (p1a))))
+  (false
+    (allow b_t3 b_tc (c1 (p1a)))
+    (allow b_tc b_t3 (c1 (p1a)))))
+
+(allow b_t3_c b_td (c1 (p1a)))
+(allow b_td b_t3_c (c1 (p1a)))
+(booleanif b_b3
+  (false
+    (allow b_t3 b_td (c1 (p1a)))
+    (allow b_td b_t3 (c1 (p1a)))))
+
+
+;; All of these rules should fail the bounds check
+(type b_t4)
+(type b_t4_c)
+(typebounds b_t4 b_t4_c)
+(typeattribute b_a4)
+(typeattribute b_a4_c)
+(typeattributeset b_a4 b_t4)
+(typeattributeset b_a4_c b_t4_c)
+
+(allow b_a4 self (CLASS (PERM)))
+(allow b_a4_c self (c1 (p1a)))
+(allow b_a4 b_ta (CLASS (PERM)))
+(allow b_a4_c b_ta (c1 (p1a)))
+(allow b_ta b_a4 (CLASS (PERM)))
+(allow b_ta b_a4_c (c1 (p1a)))
+
+(booleanif b_b1
+  (false
+    (allow b_a4_c b_tb (c1 (p1a)))
+    (allow b_tb b_a4_c (c1 (p1a)))))
+
+(booleanif b_b2
+  (true
+    (allow b_a4_c b_tc (c1 (p1a)))
+    (allow b_tc b_a4_c (c1 (p1a))))
+  (false
+    (allow b_a4 b_tc (c1 (p1a)))
+    (allow b_tc b_a4 (c1 (p1a)))))
+
+(allow b_a4_c b_td (c1 (p1a)))
+(allow b_td b_a4_c (c1 (p1a)))
+(booleanif b_b3
+  (false
+    (allow b_a4 b_td (c1 (p1a)))
+    (allow b_td b_a4 (c1 (p1a)))))
+
+
+;; Marked rules should fail, all others should pass
+(type b_t5)
+(type b_t5_c)
+(typebounds b_t5 b_t5_c)
+
+(allow b_t5 b_ta cp1)
+(allow b_t5_c b_ta (c1 (p1a)))
+(allow b_t5_c b_ta (c2 (p2a)))
+(allow b_t5_c b_ta (c2 (p2b))) ;; Fail
+(allow b_t5_c b_ta (c3 (p3a))) ;; Fail
+
+(allow b_t5 b_tb (c1 (p1a p1b)))
+(allow b_t5 b_tb (c2 (p2a)))
+(allow b_t5_c b_tb cp1)
+
+(allow b_t5 b_tc (cm1 (mp1)))
+(allow b_t5_c b_tc (c1 (p1a)))
+(allow b_t5_c b_tc (c1 (p1b))) ;; Fail
+(allow b_t5_c b_tc (c2 (p2a))) ;; Fail
+
+(allow b_t5 b_tc (c1 (p1a)))
+(allow b_t5_c b_tc (cm1 (mp1)))
+
+
+;; Marked rules should fail, all others should pass
+(type b_t6a)
+(type b_t6a_c)
+(type b_t6b)
+(type b_t6b_c)
+(typebounds b_t6a b_t6a_c)
+(typebounds b_t6b b_t6b_c)
+
+(allow b_t6a b_t6b (CLASS (PERM)))
+(allow b_t6a_c b_t6b_c (CLASS (PERM)))
+
+;; Needs: (allow b_t6a b_t6b (c1 (p1a)))
+(allow b_t6a_c b_t6b (c1 (p1a))) ;; Fail
+(allow b_t6a_c b_t6b_c (c1 (p1a))) ;; Fail
+
+;; Needs: (allow b_t6a b_t6b (c2 (p2a)))
+(allow b_t6a b_t6b_c (c2 (p2a))) ;; Fail
+(allow b_t6a_c b_t6b (c2 (p2a))) ;; Fail
+(allow b_t6a_c b_t6b_c (c2 (p2a)))
+
+;; Needs: (allow b_t6a b_t6b (c3 (p3c)))
+(allow b_t6a b_t6b (c3 (p3a p3b)))
+(allow b_t6a b_t6b_c (c3 (p3b p3c))) ;; Fail
+(allow b_t6a_c b_t6b (c3 (p3a p3c))) ;; Fail
+(allow b_t6a_c b_t6b_c (c3 (p3a p3b p3c))) ;; Fail
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any()
  2015-06-17 19:58 ` [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any() James Carter
@ 2015-06-18 13:23   ` Stephen Smalley
  0 siblings, 0 replies; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 13:23 UTC (permalink / raw)
  To: James Carter, selinux

On 06/17/2015 03:58 PM, James Carter wrote:
> This function returns true if there is a common bit that is set
> in both bitmaps.
> 
> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
>  libsepol/include/sepol/policydb/ebitmap.h |  1 +
>  libsepol/src/ebitmap.c                    | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/libsepol/include/sepol/policydb/ebitmap.h b/libsepol/include/sepol/policydb/ebitmap.h
> index 801438c..7b3508d 100644
> --- a/libsepol/include/sepol/policydb/ebitmap.h
> +++ b/libsepol/include/sepol/policydb/ebitmap.h
> @@ -86,6 +86,7 @@ extern unsigned int ebitmap_cardinality(ebitmap_t *e1);
>  extern int ebitmap_hamming_distance(ebitmap_t * e1, ebitmap_t * e2);
>  extern int ebitmap_cpy(ebitmap_t * dst, const ebitmap_t * src);
>  extern int ebitmap_contains(const ebitmap_t * e1, const ebitmap_t * e2);
> +extern int ebitmap_match_any(const ebitmap_t *e1, const ebitmap_t *e2);
>  extern int ebitmap_get_bit(const ebitmap_t * e, unsigned int bit);
>  extern int ebitmap_set_bit(ebitmap_t * e, unsigned int bit, int value);
>  extern void ebitmap_destroy(ebitmap_t * e);
> diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
> index be6b591..58f2fc4 100644
> --- a/libsepol/src/ebitmap.c
> +++ b/libsepol/src/ebitmap.c
> @@ -224,6 +224,28 @@ int ebitmap_contains(const ebitmap_t * e1, const ebitmap_t * e2)
>  	return 1;
>  }
>  
> +int ebitmap_match_any(const ebitmap_t *e1, const ebitmap_t *e2)
> +{
> +	ebitmap_node_t *n1 = e1->node;
> +	ebitmap_node_t *n2 = e2->node;
> +
> +	while (n1 && n2) {
> +		if (n1->startbit < n2->startbit) {
> +			n1 = n1->next;
> +		} else if (n2->startbit < n1->startbit) {
> +			n2 = n2->next;
> +		} else {
> +			if (n1->map & n2->map) {
> +				return 1;
> +			}
> +			n1 = n1->next;
> +			n2 = n2->next;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  int ebitmap_get_bit(const ebitmap_t * e, unsigned int bit)
>  {
>  	ebitmap_node_t *n;
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
  2015-06-17 19:58 ` [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map James Carter
@ 2015-06-18 13:41   ` Stephen Smalley
  2015-06-18 13:52     ` Stephen Smalley
  2015-06-18 20:16     ` James Carter
  0 siblings, 2 replies; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 13:41 UTC (permalink / raw)
  To: James Carter, selinux

On 06/17/2015 03:58 PM, James Carter wrote:
> Types are treated as attributes that contain only themselves. This
> is how types are already treated in the type_attr_map.
> 
> Treating types this way makes finding rules that apply to a given
> type much easier.

Can you provide an example of the difference it makes?

> 
> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
> ---
>  libsepol/src/expand.c   | 24 ++++++++++++++++--------
>  libsepol/src/policydb.c |  4 ++++
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index 478eaff..85fbe0f 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -2311,25 +2311,33 @@ static int type_attr_map(hashtab_key_t key
>  	policydb_t *p = state->out;
>  	unsigned int i;
>  	ebitmap_node_t *tnode;
> +	int value;
>  
>  	type = (type_datum_t *) datum;
> +	value = type->s.value;
> +
>  	if (type->flavor == TYPE_ATTRIB) {
> -		if (ebitmap_cpy(&p->attr_type_map[type->s.value - 1],
> -				&type->types)) {
> -			ERR(state->handle, "Out of memory!");
> -			return -1;
> +		if (ebitmap_cpy(&p->attr_type_map[value - 1], &type->types)) {
> +			goto out;
>  		}
>  		ebitmap_for_each_bit(&type->types, tnode, i) {
>  			if (!ebitmap_node_get_bit(tnode, i))
>  				continue;
> -			if (ebitmap_set_bit(&p->type_attr_map[i],
> -					    type->s.value - 1, 1)) {
> -				ERR(state->handle, "Out of memory!");
> -				return -1;
> +			if (ebitmap_set_bit(&p->type_attr_map[i], value - 1, 1)) {
> +				goto out;
>  			}
>  		}
> +	} else {
> +		if (ebitmap_set_bit(&p->attr_type_map[value - 1], value - 1, 1)) {
> +			goto out;
> +		}
>  	}
> +
>  	return 0;
> +
> +out:

Should probably call this err: or oom: instead to avoid confusion if
anyone later introduces an out: label for successful return path.

> +	ERR(state->handle, "Out of memory!");
> +	return -1;
>  }
>  
>  /* converts typeset using typemap and expands into ebitmap_t types using the attributes in the passed in policy.
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index 1677eb5..670aef8 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -3936,6 +3936,10 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose)
>  			/* add the type itself as the degenerate case */
>  			if (ebitmap_set_bit(&p->type_attr_map[i], i, 1))
>  				goto bad;
> +			if (p->type_val_to_struct[i]->flavor != TYPE_ATTRIB) {
> +				if (ebitmap_set_bit(&p->attr_type_map[i], i, 1))
> +					goto bad;
> +			}
>  		}
>  	}

It appears to me that this won't break any existing users of
attr_type_map AFAICS because they all check flavor == TYPE_ATTRIB first.

I was wondering though if we are being inconsistent with type_attr_map.
 We do set the type itself here upon policydb_read() of an existing
kernel policy, but do we do it when first generating the type_attr_map
by libsepol?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
  2015-06-18 13:41   ` Stephen Smalley
@ 2015-06-18 13:52     ` Stephen Smalley
  2015-06-18 20:16     ` James Carter
  1 sibling, 0 replies; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 13:52 UTC (permalink / raw)
  To: James Carter, selinux

On 06/18/2015 09:41 AM, Stephen Smalley wrote:
> On 06/17/2015 03:58 PM, James Carter wrote:
>> Types are treated as attributes that contain only themselves. This
>> is how types are already treated in the type_attr_map.
>>
>> Treating types this way makes finding rules that apply to a given
>> type much easier.
> 
> Can you provide an example of the difference it makes?

What I mean here is include the example or cite where this will be used
in the patch description itself.  I can see that you use it in the later
patches but it would be good to note the intended use case here, as
previously we never used attr_type_map unless flavor == TYPE_ATRIB.

> 
>>
>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>> ---
>>  libsepol/src/expand.c   | 24 ++++++++++++++++--------
>>  libsepol/src/policydb.c |  4 ++++
>>  2 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
>> index 478eaff..85fbe0f 100644
>> --- a/libsepol/src/expand.c
>> +++ b/libsepol/src/expand.c
>> @@ -2311,25 +2311,33 @@ static int type_attr_map(hashtab_key_t key
>>  	policydb_t *p = state->out;
>>  	unsigned int i;
>>  	ebitmap_node_t *tnode;
>> +	int value;
>>  
>>  	type = (type_datum_t *) datum;
>> +	value = type->s.value;
>> +
>>  	if (type->flavor == TYPE_ATTRIB) {
>> -		if (ebitmap_cpy(&p->attr_type_map[type->s.value - 1],
>> -				&type->types)) {
>> -			ERR(state->handle, "Out of memory!");
>> -			return -1;
>> +		if (ebitmap_cpy(&p->attr_type_map[value - 1], &type->types)) {
>> +			goto out;
>>  		}
>>  		ebitmap_for_each_bit(&type->types, tnode, i) {
>>  			if (!ebitmap_node_get_bit(tnode, i))
>>  				continue;
>> -			if (ebitmap_set_bit(&p->type_attr_map[i],
>> -					    type->s.value - 1, 1)) {
>> -				ERR(state->handle, "Out of memory!");
>> -				return -1;
>> +			if (ebitmap_set_bit(&p->type_attr_map[i], value - 1, 1)) {
>> +				goto out;
>>  			}
>>  		}
>> +	} else {
>> +		if (ebitmap_set_bit(&p->attr_type_map[value - 1], value - 1, 1)) {
>> +			goto out;
>> +		}
>>  	}
>> +
>>  	return 0;
>> +
>> +out:
> 
> Should probably call this err: or oom: instead to avoid confusion if
> anyone later introduces an out: label for successful return path.
> 
>> +	ERR(state->handle, "Out of memory!");
>> +	return -1;
>>  }
>>  
>>  /* converts typeset using typemap and expands into ebitmap_t types using the attributes in the passed in policy.
>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>> index 1677eb5..670aef8 100644
>> --- a/libsepol/src/policydb.c
>> +++ b/libsepol/src/policydb.c
>> @@ -3936,6 +3936,10 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose)
>>  			/* add the type itself as the degenerate case */
>>  			if (ebitmap_set_bit(&p->type_attr_map[i], i, 1))
>>  				goto bad;
>> +			if (p->type_val_to_struct[i]->flavor != TYPE_ATTRIB) {
>> +				if (ebitmap_set_bit(&p->attr_type_map[i], i, 1))
>> +					goto bad;
>> +			}
>>  		}
>>  	}
> 
> It appears to me that this won't break any existing users of
> attr_type_map AFAICS because they all check flavor == TYPE_ATTRIB first.
> 
> I was wondering though if we are being inconsistent with type_attr_map.
>  We do set the type itself here upon policydb_read() of an existing
> kernel policy, but do we do it when first generating the type_attr_map
> by libsepol?
> 
> 
> 
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code
  2015-06-17 19:58 ` [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code James Carter
@ 2015-06-18 13:56   ` Stephen Smalley
  2015-06-18 20:26     ` James Carter
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 13:56 UTC (permalink / raw)
  To: James Carter, selinux

On 06/17/2015 03:58 PM, James Carter wrote:
> The largest change to the user and role bounds checking was to put
> them in their own functions, so they could be called independently.
> 
> The type bounds checking was changed to check one type bounds at
> a time. An expanded avtab is still created, but now only the rules
> of the parent type are expanded. If violations are discovered,
> a list of avtab_ptr_t's provides details. This list is used to
> display error messages for backwards compatibility and will be
> used by CIL to provide a more detailed error message.
> 
> Memory usage is reduced from 9,355M to 126M and time is reduced
> from 9 sec to 2 sec.
> 
> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>

Can we optimize the case where there are no bounded users/roles/types at
all in the policy, and quickly return in that situation?  Seems like we
could just quickly walk them and check to see if any are bounded before
we start doing anything else.  Surprised we don't already do that.

Also, on this one, I get:
hierarchy.c: In function ‘bounds_expand_parent_rules’:
hierarchy.c:202:20: error: unused parameter ‘child’
[-Werror=unused-parameter]
           uint32_t child, uint32_t parent)
                    ^
hierarchy.c: In function ‘bounds_not_covered’:
hierarchy.c:261:43: error: unused parameter ‘p’ [-Werror=unused-parameter]
 static int bounds_not_covered(policydb_t *p, avtab_t *global_avtab,
                                           ^
hierarchy.c: In function ‘bounds_check_type_callback’:
hierarchy.c:510:53: error: unused parameter ‘k’ [-Werror=unused-parameter]
 static int bounds_check_type_callback(hashtab_key_t k, hashtab_datum_t d,
                                                     ^
cc1: all warnings being treated as errors



> ---
>  libsepol/include/sepol/policydb/hierarchy.h |   11 +
>  libsepol/src/hierarchy.c                    | 1000 +++++++++++++++++----------
>  2 files changed, 631 insertions(+), 380 deletions(-)
> 
> diff --git a/libsepol/include/sepol/policydb/hierarchy.h b/libsepol/include/sepol/policydb/hierarchy.h
> index b4eb9bc..88bc02e 100644
> --- a/libsepol/include/sepol/policydb/hierarchy.h
> +++ b/libsepol/include/sepol/policydb/hierarchy.h
> @@ -25,11 +25,22 @@
>  #ifndef _SEPOL_POLICYDB_HIERARCHY_H_
>  #define _SEPOL_POLICYDB_HIERARCHY_H_
>  
> +#include <sepol/policydb/avtab.h>
>  #include <sepol/policydb/policydb.h>
>  #include <sys/cdefs.h>
>  
>  __BEGIN_DECLS
>  
> +extern int hierarchy_add_bounds(sepol_handle_t *handle, policydb_t *p);
> +
> +extern void bounds_destroy_bad(avtab_ptr_t cur);
> +extern int bounds_check_type(sepol_handle_t *handle, policydb_t *p, uint32_t child,
> +			     uint32_t parent, avtab_ptr_t *bad, int *numbad);
> +
> +extern int bounds_check_users(sepol_handle_t *handle, policydb_t *p);
> +extern int bounds_check_roles(sepol_handle_t *handle, policydb_t *p);
> +extern int bounds_check_types(sepol_handle_t *handle, policydb_t *p);
> +
>  extern int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p);
>  
>  __END_DECLS
> diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
> index d787a64..2436837 100644
> --- a/libsepol/src/hierarchy.c
> +++ b/libsepol/src/hierarchy.c
> @@ -37,466 +37,706 @@
>  
>  #include "debug.h"
>  
> -typedef struct hierarchy_args {
> -	policydb_t *p;
> -	avtab_t *expa;		/* expanded avtab */
> -	/* This tells check_avtab_hierarchy to check this list in addition to the unconditional avtab */
> -	cond_av_list_t *opt_cond_list;
> -	sepol_handle_t *handle;
> -	int numerr;
> -} hierarchy_args_t;
> +#define BOUNDS_AVTAB_SIZE 1024
>  
> -/*
> - * find_parent_(type|role|user)
> - *
> - * This function returns the parent datum of given XXX_datum_t
> - * object or NULL, if it doesn't exist.
> - *
> - * If the given datum has a valid bounds, this function merely
> - * returns the indicated object. Otherwise, it looks up the
> - * parent based on the based hierarchy.
> - */
> -#define find_parent_template(prefix)				\
> -int find_parent_##prefix(hierarchy_args_t *a,			\
> -			 prefix##_datum_t *datum,		\
> -			 prefix##_datum_t **parent)		\
> -{								\
> -	char *parent_name, *datum_name, *tmp;			\
> -								\
> -	if (datum->bounds)						\
> -		*parent = a->p->prefix##_val_to_struct[datum->bounds - 1]; \
> -	else {								\
> -		datum_name = a->p->p_##prefix##_val_to_name[datum->s.value - 1]; \
> -									\
> -		tmp = strrchr(datum_name, '.');				\
> -		/* no '.' means it has no parent */			\
> -		if (!tmp) {						\
> -			*parent = NULL;					\
> -			return 0;					\
> -		}							\
> -									\
> -		parent_name = strdup(datum_name);			\
> -		if (!parent_name)					\
> -			return -1;					\
> -		parent_name[tmp - datum_name] = '\0';			\
> -									\
> -		*parent = hashtab_search(a->p->p_##prefix##s.table, parent_name); \
> -		if (!*parent) {						\
> -			/* Orphan type/role/user */			\
> -			ERR(a->handle,					\
> -			    "%s doesn't exist, %s is an orphan",	\
> -			    parent_name,				\
> -			    a->p->p_##prefix##_val_to_name[datum->s.value - 1]); \
> -			free(parent_name);				\
> -			return -1;					\
> -		}							\
> -		free(parent_name);					\
> -	}								\
> -									\
> -	return 0;							\
> +static int bounds_insert_helper(sepol_handle_t *handle, avtab_t *avtab,
> +				avtab_key_t *avtab_key, avtab_datum_t *datum)
> +{
> +	int rc = avtab_insert(avtab, avtab_key, datum);
> +	if (rc) {
> +		if (rc == SEPOL_ENOMEM)
> +			ERR(handle, "Insufficient memory");
> +		else
> +			ERR(handle, "Unexpected error (%d)", rc);
> +	}
> +	return rc;
>  }
>  
> -static find_parent_template(type)
> -static find_parent_template(role)
> -static find_parent_template(user)
>  
> -static void compute_avtab_datum(hierarchy_args_t *args,
> -				avtab_key_t *key,
> -				avtab_datum_t *result)
> +static int bounds_insert_rule(sepol_handle_t *handle, avtab_t *avtab,
> +			      avtab_t *global, avtab_t *other,
> +			      avtab_key_t *avtab_key, avtab_datum_t *datum)
>  {
> -	avtab_datum_t *avdatp;
> -	uint32_t av = 0;
> -
> -	avdatp = avtab_search(args->expa, key);
> -	if (avdatp)
> -		av = avdatp->data;
> -	if (args->opt_cond_list) {
> -		avdatp = cond_av_list_search(key, args->opt_cond_list);
> -		if (avdatp)
> -			av |= avdatp->data;
> +	int rc = 0;
> +	avtab_datum_t *dup = avtab_search(avtab, avtab_key);
> +
> +	if (!dup) {
> +		rc = bounds_insert_helper(handle, avtab, avtab_key, datum);
> +		if (rc) goto exit;
> +	} else {
> +		dup->data |= datum->data;
>  	}
>  
> -	result->data = av;
> +	if (other) {
> +		/* Search the other conditional avtab for the key and
> +		 * add any common permissions to the global avtab
> +		 */
> +		uint32_t data = 0;
> +		dup = avtab_search(other, avtab_key);
> +		if (dup) {
> +			data = dup->data & datum->data;
> +			if (data) {
> +				dup = avtab_search(global, avtab_key);
> +				if (!dup) {
> +					avtab_datum_t d;
> +					d.data = data;
> +					rc = bounds_insert_helper(handle, global,
> +								  avtab_key, &d);
> +					if (rc) goto exit;
> +				} else {
> +					dup->data |= data;
> +				}
> +			}
> +		}
> +	}
> +
> +exit:
> +	return rc;
>  }
>  
> -/* This function verifies that the type passed in either has a parent or is in the 
> - * root of the namespace, 0 on success, 1 on orphan and -1 on error
> - */
> -static int check_type_hierarchy_callback(hashtab_key_t k, hashtab_datum_t d,
> -					 void *args)
> +static int bounds_expand_rule(sepol_handle_t *handle, policydb_t *p,
> +			      avtab_t *avtab, avtab_t *global, avtab_t *other,
> +			      uint32_t parent, uint32_t src, uint32_t tgt,
> +			      uint32_t class, uint32_t data)
>  {
> -	hierarchy_args_t *a;
> -	type_datum_t *t, *tp;
> -
> -	a = (hierarchy_args_t *) args;
> -	t = (type_datum_t *) d;
> +	int rc = 0;
> +	avtab_key_t avtab_key;
> +	avtab_datum_t datum;
> +	ebitmap_node_t *tnode;
> +	unsigned int i;
> +
> +	avtab_key.specified = AVTAB_ALLOWED;
> +	avtab_key.target_class = class;
> +	datum.data = data;
> +
> +	if (ebitmap_get_bit(&p->attr_type_map[src - 1], parent - 1)) {
> +		avtab_key.source_type = parent;
> +		ebitmap_for_each_bit(&p->attr_type_map[tgt - 1], tnode, i) {
> +			if (!ebitmap_node_get_bit(tnode, i))
> +				continue;
> +			avtab_key.target_type = i + 1;
> +			rc = bounds_insert_rule(handle, avtab, global, other,
> +						&avtab_key, &datum);
> +			if (rc) goto exit;
> +		}
> +	}
>  
> -	if (t->flavor == TYPE_ATTRIB) {
> -		/* It's an attribute, we don't care */
> -		return 0;
> +	if (ebitmap_get_bit(&p->attr_type_map[tgt - 1], parent - 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;
> +			rc = bounds_insert_rule(handle, avtab, global, other,
> +						&avtab_key, &datum);
> +			if (rc) goto exit;
> +		}
>  	}
> -	if (find_parent_type(a, t, &tp) < 0)
> -		return -1;
> -
> -	if (tp && tp->flavor == TYPE_ATTRIB) {
> -		/* The parent is an attribute but the child isn't, not legal */
> -		ERR(a->handle, "type %s is a child of an attribute %s",
> -		    (char *) k, a->p->p_type_val_to_name[tp->s.value - 1]);
> -		a->numerr++;
> -		return -1;
> +
> +exit:
> +	return rc;
> +}
> +
> +static int bounds_expand_cond_rules(sepol_handle_t *handle, policydb_t *p,
> +				    cond_av_list_t *cur, avtab_t *avtab,
> +				    avtab_t *global, avtab_t *other,
> +				    uint32_t parent)
> +{
> +	int rc = 0;
> +
> +	for (; cur; cur = cur->next) {
> +		avtab_ptr_t n = cur->node;
> +		rc = bounds_expand_rule(handle, p, avtab, global, other, parent,
> +					n->key.source_type, n->key.target_type,
> +					n->key.target_class, n->datum.data);
> +		if (rc) goto exit;
>  	}
> -	return 0;
> +
> +exit:
> +	return rc;
>  }
>  
> -/* This function only verifies that the avtab node passed in does not violate any
> - * hiearchy constraint via any relationship with other types in the avtab.
> - * it should be called using avtab_map, returns 0 on success, 1 on violation and
> - * -1 on error. opt_cond_list is an optional argument that tells this to check
> - * a conditional list for the relationship as well as the unconditional avtab
> - */
> -static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d,
> -					  void *args)
> +struct bounds_expand_args {
> +	sepol_handle_t *handle;
> +	policydb_t *p;
> +	avtab_t *avtab;
> +	uint32_t parent;
> +};
> +
> +static int bounds_expand_rule_callback(avtab_key_t *k, avtab_datum_t *d,
> +				       void *args)
>  {
> -	avtab_key_t key;
> -	hierarchy_args_t *a = (hierarchy_args_t *) args;
> -	type_datum_t *s, *t1 = NULL, *t2 = NULL;
> -	avtab_datum_t av;
> +	struct bounds_expand_args *a = (struct bounds_expand_args *)args;
>  
> -	if (!(k->specified & AVTAB_ALLOWED)) {
> -		/* This is not an allow rule, no checking done */
> +	if (!(k->specified & AVTAB_ALLOWED))
>  		return 0;
> -	}
>  
> -	/* search for parent first */
> -	s = a->p->type_val_to_struct[k->source_type - 1];
> -	if (find_parent_type(a, s, &t1) < 0)
> -		return -1;
> -	if (t1) {
> -		/*
> -		 * search for access allowed between type 1's
> -		 * parent and type 2.
> -		 */
> -		key.source_type = t1->s.value;
> -		key.target_type = k->target_type;
> -		key.target_class = k->target_class;
> -		key.specified = AVTAB_ALLOWED;
> -		compute_avtab_datum(a, &key, &av);
> -
> -		if ((av.data & d->data) == d->data)
> -			return 0;
> +	return bounds_expand_rule(a->handle, a->p, a->avtab, NULL, NULL,
> +				  a->parent, k->source_type, k->target_type,
> +				  k->target_class, d->data);
> +}
> +
> +struct bounds_cond_info {
> +	avtab_t true_avtab;
> +	avtab_t false_avtab;
> +	cond_list_t *cond_list;
> +	struct bounds_cond_info *next;
> +};
> +
> +static void bounds_destroy_cond_info(struct bounds_cond_info *cur)
> +{
> +	struct bounds_cond_info *next;
> +
> +	for (; cur; cur = next) {
> +		next = cur->next;
> +		avtab_destroy(&cur->true_avtab);
> +		avtab_destroy(&cur->false_avtab);
> +		cur->next = NULL;
> +		free(cur);
>  	}
> +}
>  
> -	/* next we try type 1 and type 2's parent */
> -	s = a->p->type_val_to_struct[k->target_type - 1];
> -	if (find_parent_type(a, s, &t2) < 0)
> -		return -1;
> -	if (t2) {
> -		/*
> -		 * search for access allowed between type 1 and
> -		 * type 2's parent.
> -		 */
> -		key.source_type = k->source_type;
> -		key.target_type = t2->s.value;
> -		key.target_class = k->target_class;
> -		key.specified = AVTAB_ALLOWED;
> -		compute_avtab_datum(a, &key, &av);
> -
> -		if ((av.data & d->data) == d->data)
> -			return 0;
> +static int bounds_expand_parent_rules(sepol_handle_t *handle, policydb_t *p,
> +				      avtab_t *global_avtab,
> +				      struct bounds_cond_info **cond_info,
> +				      uint32_t child, uint32_t parent)
> +{
> +	int rc = 0;
> +	struct bounds_expand_args args;
> +	cond_list_t *cur;
> +
> +	avtab_init(global_avtab);
> +	rc = avtab_alloc(global_avtab, BOUNDS_AVTAB_SIZE);
> +	if (rc) goto oom;
> +
> +	args.handle = handle;
> +	args.p = p;
> +	args.avtab = global_avtab;
> +	args.parent = parent;
> +	rc = avtab_map(&p->te_avtab, bounds_expand_rule_callback, &args);
> +	if (rc) goto exit;
> +
> +	*cond_info = NULL;
> +	for (cur = p->cond_list; cur; cur = cur->next) {
> +		struct bounds_cond_info *ci;
> +		ci = malloc(sizeof(struct bounds_cond_info));
> +		if (!ci) goto oom;
> +		avtab_init(&ci->true_avtab);
> +		avtab_init(&ci->false_avtab);
> +		ci->cond_list = cur;
> +		ci->next = *cond_info;
> +		*cond_info = ci;
> +		if (cur->true_list) {
> +			rc = avtab_alloc(&ci->true_avtab, BOUNDS_AVTAB_SIZE);
> +			if (rc) goto oom;
> +			rc = bounds_expand_cond_rules(handle, p, cur->true_list,
> +						      &ci->true_avtab, NULL,
> +						      NULL, parent);
> +			if (rc) goto exit;
> +		}
> +		if (cur->false_list) {
> +			rc = avtab_alloc(&ci->false_avtab, BOUNDS_AVTAB_SIZE);
> +			if (rc) goto oom;
> +			rc = bounds_expand_cond_rules(handle, p, cur->false_list,
> +						      &ci->false_avtab,
> +						      global_avtab,
> +						      &ci->true_avtab, parent);
> +			if (rc) goto exit;
> +		}
>  	}
>  
> -	if (t1 && t2) {
> -		/*
> -                 * search for access allowed between type 1's parent
> -                 * and type 2's parent.
> -                 */
> -		key.source_type = t1->s.value;
> -		key.target_type = t2->s.value;
> -		key.target_class = k->target_class;
> -		key.specified = AVTAB_ALLOWED;
> -		compute_avtab_datum(a, &key, &av);
> -
> -		if ((av.data & d->data) == d->data)
> -			return 0;
> +	return 0;
> +
> +oom:
> +	ERR(handle, "Insufficient memory");
> +
> +exit:
> +	ERR(handle,"Failed to expand parent rules\n");
> +	avtab_destroy(global_avtab);
> +	bounds_destroy_cond_info(*cond_info);
> +	*cond_info = NULL;
> +	return rc;
> +}
> +
> +static int bounds_not_covered(policydb_t *p, avtab_t *global_avtab,
> +			      avtab_t *cur_avtab, avtab_key_t *avtab_key,
> +			      uint32_t data)
> +{
> +	avtab_datum_t *datum = avtab_search(cur_avtab, avtab_key);
> +	if (datum)
> +		data &= ~datum->data;
> +	if (global_avtab && data) {
> +		datum = avtab_search(global_avtab, avtab_key);
> +		if (datum)
> +			data &= ~datum->data;
>  	}
>  
> -	/*
> -	 * Neither one of these types have parents and 
> -	 * therefore the hierarchical constraint does not apply
> -	 */
> -	if (!t1 && !t2)
> -		return 0;
> +	return data;
> +}
> +
> +static int bounds_add_bad(sepol_handle_t *handle, uint32_t src, uint32_t tgt,
> +			  uint32_t class, uint32_t data, avtab_ptr_t *bad)
> +{
> +	struct avtab_node *new = malloc(sizeof(struct avtab_node));
> +	if (new == NULL) {
> +		ERR(handle, "Insufficient memory");
> +		return SEPOL_ENOMEM;
> +	}
> +	memset(new, 0, sizeof(struct avtab_node));
> +	new->key.source_type = src;
> +	new->key.target_type = tgt;
> +	new->key.target_class = class;
> +	new->datum.data = data;
> +	new->next = *bad;
> +	*bad = new;
>  
> -	/*
> -	 * At this point there is a violation of the hierarchal
> -	 * constraint, send error condition back
> -	 */
> -	ERR(a->handle,
> -	    "hierarchy violation between types %s and %s : %s { %s }",
> -	    a->p->p_type_val_to_name[k->source_type - 1],
> -	    a->p->p_type_val_to_name[k->target_type - 1],
> -	    a->p->p_class_val_to_name[k->target_class - 1],
> -	    sepol_av_to_string(a->p, k->target_class, d->data & ~av.data));
> -	a->numerr++;
>  	return 0;
>  }
>  
> -/*
> - * If same permissions are allowed for same combination of
> - * source and target, we can evaluate them as unconditional
> - * one.
> - * See the following example. A_t type is bounds of B_t type,
> - * so B_t can never have wider permissions then A_t.
> - * A_t has conditional permission on X_t, however, a part of
> - * them (getattr and read) are unconditionaly allowed to A_t.
> - *
> - * Example)
> - * typebounds A_t B_t;
> - *
> - * allow B_t X_t : file { getattr };
> - * if (foo_bool) {
> - *     allow A_t X_t : file { getattr read };
> - * } else {
> - *     allow A_t X_t : file { getattr read write };
> - * }
> - *
> - * We have to pull up them as unconditional ones in this case,
> - * because it seems to us B_t is violated to bounds constraints
> - * during unconditional policy checking.
> - */
> -static int pullup_unconditional_perms(cond_list_t * cond_list,
> -				      hierarchy_args_t * args)
> +static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
> +			     avtab_t *global_avtab, avtab_t *cur_avtab,
> +			     uint32_t child, uint32_t parent, uint32_t src,
> +			     uint32_t tgt, uint32_t class, uint32_t data,
> +			     avtab_ptr_t *bad, int *numbad)
>  {
> -	cond_list_t *cur_node;
> -	cond_av_list_t *cur_av, *expl_true = NULL, *expl_false = NULL;
> -	avtab_t expa_true, expa_false;
> -	avtab_datum_t *avdatp;
> -	avtab_datum_t avdat;
> -	avtab_ptr_t avnode;
> -
> -	for (cur_node = cond_list; cur_node; cur_node = cur_node->next) {
> -		if (avtab_init(&expa_true))
> -			goto oom0;
> -		if (avtab_init(&expa_false))
> -			goto oom1;
> -		if (expand_cond_av_list(args->p, cur_node->true_list,
> -					&expl_true, &expa_true))
> -			goto oom2;
> -		if (expand_cond_av_list(args->p, cur_node->false_list,
> -					&expl_false, &expa_false))
> -			goto oom3;
> -		for (cur_av = expl_true; cur_av; cur_av = cur_av->next) {
> -			avdatp = avtab_search(&expa_false,
> -					      &cur_av->node->key);
> -			if (!avdatp)
> +	int rc = 0;
> +	avtab_key_t avtab_key;
> +	type_datum_t *td;
> +	ebitmap_node_t *tnode;
> +	unsigned int i;
> +	uint32_t d;
> +
> +	avtab_key.specified = AVTAB_ALLOWED;
> +	avtab_key.target_class = class;
> +
> +	if (ebitmap_get_bit(&p->attr_type_map[src - 1], child - 1)) {
> +		avtab_key.source_type = parent;
> +		ebitmap_for_each_bit(&p->attr_type_map[tgt - 1], tnode, i) {
> +			if (!ebitmap_node_get_bit(tnode, i))
>  				continue;
> -
> -			avdat.data = (cur_av->node->datum.data
> -				      & avdatp->data);
> -			if (!avdat.data)
> +			avtab_key.target_type = i + 1;
> +			d = bounds_not_covered(p, global_avtab, cur_avtab,
> +					       &avtab_key, data);
> +			if (!d) continue;
> +			td = p->type_val_to_struct[i];
> +			if (td && td->bounds) {
> +				avtab_key.target_type = td->bounds;
> +				d = bounds_not_covered(p, global_avtab,
> +						       cur_avtab, &avtab_key,
> +						       data);
> +				if (!d) continue;
> +			}
> +			(*numbad)++;
> +			rc = bounds_add_bad(handle, child, i+1, class, d, bad);
> +			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;
> -
> -			avnode = avtab_search_node(args->expa,
> -						   &cur_av->node->key);
> -			if (avnode) {
> -				avnode->datum.data |= avdat.data;
> -			} else {
> -				if (avtab_insert(args->expa,
> -						 &cur_av->node->key,
> -						 &avdat))
> -					goto oom4;
> +			avtab_key.source_type = i + 1;
> +			if (avtab_key.source_type == child) {
> +				/* Checked above */
> +				continue;
> +			}
> +			d = bounds_not_covered(p, 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(p, 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;
>  		}
> -		cond_av_list_destroy(expl_false);
> -		cond_av_list_destroy(expl_true);
> -		avtab_destroy(&expa_false);
> -		avtab_destroy(&expa_true);
>  	}
> -	return 0;
>  
> -oom4:
> -	cond_av_list_destroy(expl_false);
> -oom3:
> -	cond_av_list_destroy(expl_true);
> -oom2:
> -	avtab_destroy(&expa_false);
> -oom1:
> -	avtab_destroy(&expa_true);
> -oom0:
> -	ERR(args->handle, "out of memory on conditional av list expansion");
> -        return 1;
> +exit:
> +	return rc;
> +}
> +
> +static int bounds_check_cond_rules(sepol_handle_t *handle, policydb_t *p,
> +				   avtab_t *global_avtab, avtab_t *cond_avtab,
> +				   cond_av_list_t *rules, uint32_t child,
> +				   uint32_t parent, avtab_ptr_t *bad,
> +				   int *numbad)
> +{
> +	int rc = 0;
> +	cond_av_list_t *cur;
> +
> +	for (cur = rules; cur; cur = cur->next) {
> +		avtab_ptr_t ap = cur->node;
> +		avtab_key_t *key = &ap->key;
> +		avtab_datum_t *datum = &ap->datum;
> +		if (!(key->specified & AVTAB_ALLOWED))
> +			continue;
> +		rc = bounds_check_rule(handle, p, global_avtab, cond_avtab,
> +				       child, parent, key->source_type,
> +				       key->target_type, key->target_class,
> +				       datum->data, bad, numbad);
> +		if (rc) goto exit;
> +	}
> +
> +exit:
> +	return rc;
> +}
> +
> +struct bounds_check_args {
> +	sepol_handle_t *handle;
> +	policydb_t *p;
> +	avtab_t *cur_avtab;
> +	uint32_t child;
> +	uint32_t parent;
> +	avtab_ptr_t bad;
> +	int numbad;
> +};
> +
> +static int bounds_check_rule_callback(avtab_key_t *k, avtab_datum_t *d,
> +				      void *args)
> +{
> +	struct bounds_check_args *a = (struct bounds_check_args *)args;
> +
> +	if (!(k->specified & AVTAB_ALLOWED))
> +		return 0;
> +
> +	return bounds_check_rule(a->handle, a->p, NULL, a->cur_avtab, a->child,
> +				 a->parent, k->source_type, k->target_type,
> +				 k->target_class, d->data, &a->bad, &a->numbad);
>  }
>  
> -static int check_cond_avtab_hierarchy(cond_list_t * cond_list,
> -				      hierarchy_args_t * args)
> +static int bounds_check_child_rules(sepol_handle_t *handle, policydb_t *p,
> +				    avtab_t *global_avtab,
> +				    struct bounds_cond_info *cond_info,
> +				    uint32_t child, uint32_t parent,
> +				    avtab_ptr_t *bad, int *numbad)
>  {
>  	int rc;
> -	cond_list_t *cur_node;
> -	cond_av_list_t *cur_av, *expl = NULL;
> -	avtab_t expa;
> -	hierarchy_args_t *a = (hierarchy_args_t *) args;
> -	avtab_datum_t avdat, *uncond;
> -
> -	for (cur_node = cond_list; cur_node; cur_node = cur_node->next) {
> -		/*
> -		 * Check true condition
> -		 */
> -		if (avtab_init(&expa))
> -			goto oom;
> -		if (expand_cond_av_list(args->p, cur_node->true_list,
> -					&expl, &expa)) {
> -			avtab_destroy(&expa);
> -			goto oom;
> -		}
> -		args->opt_cond_list = expl;
> -		for (cur_av = expl; cur_av; cur_av = cur_av->next) {
> -			avdat.data = cur_av->node->datum.data;
> -			uncond = avtab_search(a->expa, &cur_av->node->key);
> -			if (uncond)
> -				avdat.data |= uncond->data;
> -			rc = check_avtab_hierarchy_callback(&cur_av->node->key,
> -							    &avdat, args);
> -			if (rc)
> -				args->numerr++;
> -		}
> -		cond_av_list_destroy(expl);
> -		avtab_destroy(&expa);
> +	struct bounds_check_args args;
> +	struct bounds_cond_info *cur;
>  
> -		/*
> -		 * Check false condition
> -		 */
> -		if (avtab_init(&expa))
> -			goto oom;
> -		if (expand_cond_av_list(args->p, cur_node->false_list,
> -					&expl, &expa)) {
> -			avtab_destroy(&expa);
> -			goto oom;
> -		}
> -		args->opt_cond_list = expl;
> -		for (cur_av = expl; cur_av; cur_av = cur_av->next) {
> -			avdat.data = cur_av->node->datum.data;
> -			uncond = avtab_search(a->expa, &cur_av->node->key);
> -			if (uncond)
> -				avdat.data |= uncond->data;
> -
> -			rc = check_avtab_hierarchy_callback(&cur_av->node->key,
> -							    &avdat, args);
> -			if (rc)
> -				a->numerr++;
> +	args.handle = handle;
> +	args.p = p;
> +	args.cur_avtab = global_avtab;
> +	args.child = child;
> +	args.parent = parent;
> +	args.bad = NULL;
> +	args.numbad = 0;
> +	rc = avtab_map(&p->te_avtab, bounds_check_rule_callback, &args);
> +	if (rc) goto exit;
> +
> +	for (cur = cond_info; cur; cur = cur->next) {
> +		cond_list_t *node = cur->cond_list;
> +		rc = bounds_check_cond_rules(handle, p, global_avtab,
> +					     &cur->true_avtab,
> +					     node->true_list, child, parent,
> +					     &args.bad, &args.numbad);
> +		if (rc) goto exit;
> +
> +		rc = bounds_check_cond_rules(handle, p, global_avtab,
> +					     &cur->false_avtab,
> +					     node->false_list, child, parent,
> +					     &args.bad, &args.numbad);
> +		if (rc) goto exit;
> +	}
> +
> +	*numbad += args.numbad;
> +	*bad = args.bad;
> +
> +exit:
> +	return rc;
> +}
> +
> +int bounds_check_type(sepol_handle_t *handle, policydb_t *p, uint32_t child,
> +		      uint32_t parent, avtab_ptr_t *bad, int *numbad)
> +{
> +	int rc = 0;
> +	avtab_t global_avtab;
> +	struct bounds_cond_info *cond_info = NULL;
> +
> +	rc = bounds_expand_parent_rules(handle, p, &global_avtab, &cond_info,
> +					child, parent);
> +	if (rc) goto exit;
> +
> +	rc = bounds_check_child_rules(handle, p, &global_avtab, cond_info,
> +				      child, parent, bad, numbad);
> +
> +	bounds_destroy_cond_info(cond_info);
> +	avtab_destroy(&global_avtab);
> +
> +exit:
> +	return rc;
> +}
> +
> +struct bounds_args {
> +	sepol_handle_t *handle;
> +	policydb_t *p;
> +	int numbad;
> +};
> +
> +static void bounds_report(sepol_handle_t *handle, policydb_t *p, uint32_t child,
> +			  uint32_t parent, avtab_ptr_t cur)
> +{
> +	ERR(handle, "Child type %s exceeds bounds of parent %s in the following rules:",
> +	    p->p_type_val_to_name[child - 1],
> +	    p->p_type_val_to_name[parent - 1]);
> +	for (; cur; cur = cur->next) {
> +		ERR(handle, "    %s %s : %s { %s }",
> +		    p->p_type_val_to_name[cur->key.source_type - 1],
> +		    p->p_type_val_to_name[cur->key.target_type - 1],
> +		    p->p_class_val_to_name[cur->key.target_class - 1],
> +		    sepol_av_to_string(p, cur->key.target_class,
> +				       cur->datum.data));
> +	}
> +}
> +
> +void bounds_destroy_bad(avtab_ptr_t cur)
> +{
> +	avtab_ptr_t next;
> +
> +	for (; cur; cur = next) {
> +		next = cur->next;
> +		cur->next = NULL;
> +		free(cur);
> +	}
> +}
> +
> +static int bounds_check_type_callback(hashtab_key_t k, hashtab_datum_t d,
> +				      void *args)
> +{
> +	int rc = 0;
> +	struct bounds_args *a = (struct bounds_args *)args;
> +	type_datum_t *t = (type_datum_t *)d;
> +	avtab_ptr_t bad = NULL;
> +
> +	if (t->bounds) {
> +		rc = bounds_check_type(a->handle, a->p, t->s.value, t->bounds,
> +				       &bad, &a->numbad);
> +		if (bad) {
> +			bounds_report(a->handle, a->p, t->s.value, t->bounds,
> +				      bad);
> +			bounds_destroy_bad(bad);
>  		}
> -		cond_av_list_destroy(expl);
> -		avtab_destroy(&expa);
>  	}
>  
> -	return 0;
> +	return rc;
> +}
> +
> +int bounds_check_types(sepol_handle_t *handle, policydb_t *p)
> +{
> +	int rc;
> +	struct bounds_args args;
> +
> +	args.handle = handle;
> +	args.p = p;
> +	args.numbad = 0;
> +
> +	rc = hashtab_map(p->p_types.table, bounds_check_type_callback, &args);
> +	if (rc) goto exit;
> +
> +	if (args.numbad > 0) {
> +		ERR(handle, "%d errors found during type bounds check",
> +		    args.numbad);
> +		rc = SEPOL_ERR;
> +	}
>  
> -      oom:
> -	ERR(args->handle, "out of memory on conditional av list expansion");
> -	return 1;
> +exit:
> +	return rc;
>  }
>  
> -/* The role hierarchy is defined as: a child role cannot have more types than it's parent.
> - * This function should be called with hashtab_map, it will return 0 on success, 1 on 
> - * constraint violation and -1 on error
> +/* The role bounds is defined as: a child role cannot have a type that
> + * its parent doesn't have.
>   */
> -static int check_role_hierarchy_callback(hashtab_key_t k
> -					 __attribute__ ((unused)),
> -					 hashtab_datum_t d, void *args)
> +static int bounds_check_role_callback(hashtab_key_t k __attribute__ ((unused)),
> +				      hashtab_datum_t d, void *args)
>  {
> -	hierarchy_args_t *a;
> -	role_datum_t *r, *rp;
> +	struct bounds_args *a = (struct bounds_args *)args;
> +	role_datum_t *r = (role_datum_t *) d;
> +	role_datum_t *rp = NULL;
>  
> -	a = (hierarchy_args_t *) args;
> -	r = (role_datum_t *) d;
> +	if (!r->bounds)
> +		return 0;
>  
> -	if (find_parent_role(a, r, &rp) < 0)
> -		return -1;
> +	rp = a->p->role_val_to_struct[r->bounds - 1];
>  
>  	if (rp && !ebitmap_contains(&rp->types.types, &r->types.types)) {
> -		/* hierarchical constraint violation, return error */
> -		ERR(a->handle, "Role hierarchy violation, %s exceeds %s",
> -		    (char *) k, a->p->p_role_val_to_name[rp->s.value - 1]);
> -		a->numerr++;
> +		ERR(a->handle, "Role bounds violation, %s exceeds %s",
> +		    (char *)k, a->p->p_role_val_to_name[rp->s.value - 1]);
> +		a->numbad++;
> +	}
> +
> +	return 0;
> +}
> +
> +int bounds_check_roles(sepol_handle_t *handle, policydb_t *p)
> +{
> +	struct bounds_args args;
> +
> +	args.handle = handle;
> +	args.p = p;
> +	args.numbad = 0;
> +
> +	hashtab_map(p->p_roles.table, bounds_check_role_callback, &args);
> +
> +	if (args.numbad > 0) {
> +		ERR(handle, "%d errors found during role bounds check",
> +		    args.numbad);
> +		return SEPOL_ERR;
>  	}
> +
>  	return 0;
>  }
>  
> -/* The user hierarchy is defined as: a child user cannot have a role that
> - * its parent doesn't have.  This function should be called with hashtab_map,
> - * it will return 0 on success, 1 on constraint violation and -1 on error.
> +/* The user bounds is defined as: a child user cannot have a role that
> + * its parent doesn't have.
>   */
> -static int check_user_hierarchy_callback(hashtab_key_t k
> -					 __attribute__ ((unused)),
> -					 hashtab_datum_t d, void *args)
> +static int bounds_check_user_callback(hashtab_key_t k __attribute__ ((unused)),
> +				      hashtab_datum_t d, void *args)
>  {
> -	hierarchy_args_t *a;
> -	user_datum_t *u, *up;
> +	struct bounds_args *a = (struct bounds_args *)args;
> +	user_datum_t *u = (user_datum_t *) d;
> +	user_datum_t *up = NULL;
>  
> -	a = (hierarchy_args_t *) args;
> -	u = (user_datum_t *) d;
> +	if (!u->bounds)
> +		return 0;
>  
> -	if (find_parent_user(a, u, &up) < 0)
> -		return -1;
> +	up = a->p->user_val_to_struct[u->bounds - 1];
>  
>  	if (up && !ebitmap_contains(&up->roles.roles, &u->roles.roles)) {
> -		/* hierarchical constraint violation, return error */
> -		ERR(a->handle, "User hierarchy violation, %s exceeds %s",
> +		ERR(a->handle, "User bounds violation, %s exceeds %s",
>  		    (char *) k, a->p->p_user_val_to_name[up->s.value - 1]);
> -		a->numerr++;
> +		a->numbad++;
>  	}
> +
>  	return 0;
>  }
>  
> -int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p)
> +int bounds_check_users(sepol_handle_t *handle, policydb_t *p)
>  {
> -	hierarchy_args_t args;
> -	avtab_t expa;
> -
> -	if (avtab_init(&expa))
> -		goto oom;
> -	if (expand_avtab(p, &p->te_avtab, &expa)) {
> -		avtab_destroy(&expa);
> -		goto oom;
> -	}
> +	struct bounds_args args;
>  
> -	args.p = p;
> -	args.expa = &expa;
> -	args.opt_cond_list = NULL;
>  	args.handle = handle;
> -	args.numerr = 0;
> +	args.p = p;
> +	args.numbad = 0;
>  
> -	if (hashtab_map(p->p_types.table, check_type_hierarchy_callback, &args))
> -		goto bad;
> +	hashtab_map(p->p_users.table, bounds_check_user_callback, &args);
>  
> -	if (pullup_unconditional_perms(p->cond_list, &args))
> -		return -1;
> +	if (args.numbad > 0) {
> +		ERR(handle, "%d errors found during user bounds check",
> +		    args.numbad);
> +		return SEPOL_ERR;
> +	}
>  
> -	if (avtab_map(&expa, check_avtab_hierarchy_callback, &args))
> -		goto bad;
> +	return 0;
> +}
>  
> -	if (check_cond_avtab_hierarchy(p->cond_list, &args))
> -		goto bad;
> +#define add_hierarchy_callback_template(prefix)				\
> +	int hierarchy_add_##prefix##_callback(hashtab_key_t k __attribute__ ((unused)), \
> +					    hashtab_datum_t d, void *args) \
> +{								\
> +	struct bounds_args *a = (struct bounds_args *)args;		\
> +	sepol_handle_t *handle = a->handle;				\
> +	policydb_t *p = a->p;						\
> +	prefix##_datum_t *datum = (prefix##_datum_t *)d;		\
> +	prefix##_datum_t *parent;					\
> +	char *parent_name, *datum_name, *tmp;				\
> +									\
> +	if (!datum->bounds) {						\
> +		datum_name = p->p_##prefix##_val_to_name[datum->s.value - 1]; \
> +									\
> +		tmp = strrchr(datum_name, '.');				\
> +		/* no '.' means it has no parent */			\
> +		if (!tmp) return 0;					\
> +									\
> +		parent_name = strdup(datum_name);			\
> +		if (!parent_name) {					\
> +			ERR(handle, "Insufficient memory");		\
> +			return SEPOL_ENOMEM;				\
> +		}							\
> +		parent_name[tmp - datum_name] = '\0';			\
> +									\
> +		parent = hashtab_search(p->p_##prefix##s.table, parent_name); \
> +		if (!parent) {						\
> +			/* Orphan type/role/user */			\
> +			ERR(handle, "%s doesn't exist, %s is an orphan",\
> +			    parent_name,				\
> +			    p->p_##prefix##_val_to_name[datum->s.value - 1]); \
> +			free(parent_name);				\
> +			a->numbad++;					\
> +			return 0;					\
> +		}							\
> +		datum->bounds = parent->s.value;			\
> +		free(parent_name);					\
> +	}								\
> +									\
> +	return 0;							\
> +}								\
> +
> +static add_hierarchy_callback_template(type)
> +static add_hierarchy_callback_template(role)
> +static add_hierarchy_callback_template(user)
>  
> -	if (hashtab_map(p->p_roles.table, check_role_hierarchy_callback, &args))
> -		goto bad;
> +int hierarchy_add_bounds(sepol_handle_t *handle, policydb_t *p)
> +{
> +	int rc = 0;
> +	struct bounds_args args;
> +
> +	args.handle = handle;
> +	args.p = p;
> +	args.numbad = 0;
> +
> +	rc = hashtab_map(p->p_users.table, hierarchy_add_user_callback, &args);
> +	if (rc) goto exit;
> +
> +	rc = hashtab_map(p->p_roles.table, hierarchy_add_role_callback, &args);
> +	if (rc) goto exit;
>  
> -	if (hashtab_map(p->p_users.table, check_user_hierarchy_callback, &args))
> -		goto bad;
> +	rc = hashtab_map(p->p_types.table, hierarchy_add_type_callback, &args);
> +	if (rc) goto exit;
>  
> -	if (args.numerr) {
> -		ERR(handle, "%d total errors found during hierarchy check",
> -		    args.numerr);
> -		goto bad;
> +	if (args.numbad > 0) {
> +		ERR(handle, "%d errors found while adding hierarchies",
> +		    args.numbad);
> +		rc = SEPOL_ERR;
>  	}
>  
> -	avtab_destroy(&expa);
> -	return 0;
> +exit:
> +	return rc;
> +}
> +
> +int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p)
> +{
> +	int rc = 0;
> +	int violation = 0;
> +
> +	rc = hierarchy_add_bounds(handle, p);
> +	if (rc) goto exit;
> +
> +	rc = bounds_check_users(handle, p);
> +	if (rc)
> +		violation = 1;
> +
> +	rc = bounds_check_roles(handle, p);
> +	if (rc)
> +		violation = 1;
> +
> +	rc = bounds_check_types(handle, p);
> +	if (rc) {
> +		if (rc == SEPOL_ERR)
> +			violation = 1;
> +		else
> +			goto exit;
> +	}
>  
> -      bad:
> -	avtab_destroy(&expa);
> -	return -1;
> +	if (violation)
> +		rc = SEPOL_ERR;
>  
> -      oom:
> -	ERR(handle, "Out of memory");
> -	return -1;
> +exit:
> +	return rc;
>  }
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
  2015-06-18 13:41   ` Stephen Smalley
  2015-06-18 13:52     ` Stephen Smalley
@ 2015-06-18 20:16     ` James Carter
  2015-06-18 20:21       ` Stephen Smalley
  1 sibling, 1 reply; 21+ messages in thread
From: James Carter @ 2015-06-18 20:16 UTC (permalink / raw)
  To: Stephen Smalley, selinux

On 06/18/2015 09:41 AM, Stephen Smalley wrote:
> On 06/17/2015 03:58 PM, James Carter wrote:
>> Types are treated as attributes that contain only themselves. This
>> is how types are already treated in the type_attr_map.
>>
>> Treating types this way makes finding rules that apply to a given
>> type much easier.
>
> Can you provide an example of the difference it makes?
>

I'll add something like the following.


This allows the following statement:

ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);


which would have previously been written as:

if (p->type_val_to_struct[src-1]->flavor == TYPE_ATTRIB &&
     p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
	ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
} else if (p->type_val_to_struct[k->source_type - 1]->flavor == TYPE_ATTRIB) {
	if (ebitmap_get_bit(&p->attr_type_map[src-1], tgt-1)) {	
		ebitmap_set_bit(&m, tgt-1, 1);
	}
} else if (p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
	if (ebitmap_get_bit(&p->attr_type_map[tgt-1], src-1)) {	
		ebitmap_set_bit(&m, src-1, 1);
	}
} else {
	if (src == tgt) {
		ebitmap_set_bit(&m, src-1, 1);
	}
}	


>>
>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>> ---
>>   libsepol/src/expand.c   | 24 ++++++++++++++++--------
>>   libsepol/src/policydb.c |  4 ++++
>>   2 files changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
>> index 478eaff..85fbe0f 100644
>> --- a/libsepol/src/expand.c
>> +++ b/libsepol/src/expand.c
>> @@ -2311,25 +2311,33 @@ static int type_attr_map(hashtab_key_t key
>>   	policydb_t *p = state->out;
>>   	unsigned int i;
>>   	ebitmap_node_t *tnode;
>> +	int value;
>>
>>   	type = (type_datum_t *) datum;
>> +	value = type->s.value;
>> +
>>   	if (type->flavor == TYPE_ATTRIB) {
>> -		if (ebitmap_cpy(&p->attr_type_map[type->s.value - 1],
>> -				&type->types)) {
>> -			ERR(state->handle, "Out of memory!");
>> -			return -1;
>> +		if (ebitmap_cpy(&p->attr_type_map[value - 1], &type->types)) {
>> +			goto out;
>>   		}
>>   		ebitmap_for_each_bit(&type->types, tnode, i) {
>>   			if (!ebitmap_node_get_bit(tnode, i))
>>   				continue;
>> -			if (ebitmap_set_bit(&p->type_attr_map[i],
>> -					    type->s.value - 1, 1)) {
>> -				ERR(state->handle, "Out of memory!");
>> -				return -1;
>> +			if (ebitmap_set_bit(&p->type_attr_map[i], value - 1, 1)) {
>> +				goto out;
>>   			}
>>   		}
>> +	} else {
>> +		if (ebitmap_set_bit(&p->attr_type_map[value - 1], value - 1, 1)) {
>> +			goto out;
>> +		}
>>   	}
>> +
>>   	return 0;
>> +
>> +out:
>
> Should probably call this err: or oom: instead to avoid confusion if
> anyone later introduces an out: label for successful return path.
>

Sure, oom seems to be used a decent amount in other places.

>> +	ERR(state->handle, "Out of memory!");
>> +	return -1;
>>   }
>>
>>   /* converts typeset using typemap and expands into ebitmap_t types using the attributes in the passed in policy.
>> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
>> index 1677eb5..670aef8 100644
>> --- a/libsepol/src/policydb.c
>> +++ b/libsepol/src/policydb.c
>> @@ -3936,6 +3936,10 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose)
>>   			/* add the type itself as the degenerate case */
>>   			if (ebitmap_set_bit(&p->type_attr_map[i], i, 1))
>>   				goto bad;
>> +			if (p->type_val_to_struct[i]->flavor != TYPE_ATTRIB) {
>> +				if (ebitmap_set_bit(&p->attr_type_map[i], i, 1))
>> +					goto bad;
>> +			}
>>   		}
>>   	}
>
> It appears to me that this won't break any existing users of
> attr_type_map AFAICS because they all check flavor == TYPE_ATTRIB first.
>

That is my view as well.

> I was wondering though if we are being inconsistent with type_attr_map.
>   We do set the type itself here upon policydb_read() of an existing
> kernel policy, but do we do it when first generating the type_attr_map
> by libsepol?
>
>

I only see the type_attr_map being generated in policydb_read for a kernel 
policy and when expand_module() is called. This surprised me and made me think 
that there must be a better place to generate these mappings, but I can't think 
of a better place. Can you?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
  2015-06-18 20:16     ` James Carter
@ 2015-06-18 20:21       ` Stephen Smalley
  2015-06-18 20:23         ` Stephen Smalley
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 20:21 UTC (permalink / raw)
  To: James Carter, selinux

On 06/18/2015 04:16 PM, James Carter wrote:
> On 06/18/2015 09:41 AM, Stephen Smalley wrote:
>> Can you provide an example of the difference it makes?
>>
> 
> I'll add something like the following.
> 
> 
> This allows the following statement:
> 
> ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
> 
> 
> which would have previously been written as:
> 
> if (p->type_val_to_struct[src-1]->flavor == TYPE_ATTRIB &&
>     p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
>     ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
> } else if (p->type_val_to_struct[k->source_type - 1]->flavor ==
> TYPE_ATTRIB) {
>     if (ebitmap_get_bit(&p->attr_type_map[src-1], tgt-1)) {   
>         ebitmap_set_bit(&m, tgt-1, 1);
>     }
> } else if (p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
>     if (ebitmap_get_bit(&p->attr_type_map[tgt-1], src-1)) {   
>         ebitmap_set_bit(&m, src-1, 1);
>     }
> } else {
>     if (src == tgt) {
>         ebitmap_set_bit(&m, src-1, 1);
>     }
> }

Oh, you don't have to be that specific.  Could just note that this
simplified the implementation of neverallow checking in assertion.c or
something.


>> I was wondering though if we are being inconsistent with type_attr_map.
>>   We do set the type itself here upon policydb_read() of an existing
>> kernel policy, but do we do it when first generating the type_attr_map
>> by libsepol?
>>
>>
> 
> I only see the type_attr_map being generated in policydb_read for a
> kernel policy and when expand_module() is called. This surprised me and
> made me think that there must be a better place to generate these
> mappings, but I can't think of a better place. Can you?

No, I think it makes sense to generate it in the expand.c code (when
generating the kernel policy from modules) and in policydb_read (when
reading a kernel policy from a file/image).  But my question is whether
we are consistently setting the type itself in both places.  You did
that for attr_type_map, but are we doing it for type_attr_map?  And if
not, why not?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map.
  2015-06-18 20:21       ` Stephen Smalley
@ 2015-06-18 20:23         ` Stephen Smalley
  0 siblings, 0 replies; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 20:23 UTC (permalink / raw)
  To: James Carter, selinux

On 06/18/2015 04:21 PM, Stephen Smalley wrote:
> On 06/18/2015 04:16 PM, James Carter wrote:
>> On 06/18/2015 09:41 AM, Stephen Smalley wrote:
>>> Can you provide an example of the difference it makes?
>>>
>>
>> I'll add something like the following.
>>
>>
>> This allows the following statement:
>>
>> ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
>>
>>
>> which would have previously been written as:
>>
>> if (p->type_val_to_struct[src-1]->flavor == TYPE_ATTRIB &&
>>     p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
>>     ebitmap_and(&m, &p->attr_type_map[src-1], &p->attr_type_map[tgt-1]);
>> } else if (p->type_val_to_struct[k->source_type - 1]->flavor ==
>> TYPE_ATTRIB) {
>>     if (ebitmap_get_bit(&p->attr_type_map[src-1], tgt-1)) {   
>>         ebitmap_set_bit(&m, tgt-1, 1);
>>     }
>> } else if (p->type_val_to_struct[tgt-1]->flavor == TYPE_ATTRIB) {
>>     if (ebitmap_get_bit(&p->attr_type_map[tgt-1], src-1)) {   
>>         ebitmap_set_bit(&m, src-1, 1);
>>     }
>> } else {
>>     if (src == tgt) {
>>         ebitmap_set_bit(&m, src-1, 1);
>>     }
>> }
> 
> Oh, you don't have to be that specific.  Could just note that this
> simplified the implementation of neverallow checking in assertion.c or
> something.
> 
> 
>>> I was wondering though if we are being inconsistent with type_attr_map.
>>>   We do set the type itself here upon policydb_read() of an existing
>>> kernel policy, but do we do it when first generating the type_attr_map
>>> by libsepol?
>>>
>>>
>>
>> I only see the type_attr_map being generated in policydb_read for a
>> kernel policy and when expand_module() is called. This surprised me and
>> made me think that there must be a better place to generate these
>> mappings, but I can't think of a better place. Can you?
> 
> No, I think it makes sense to generate it in the expand.c code (when
> generating the kernel policy from modules) and in policydb_read (when
> reading a kernel policy from a file/image).  But my question is whether
> we are consistently setting the type itself in both places.  You did
> that for attr_type_map, but are we doing it for type_attr_map?  And if
> not, why not?

Actually, I see that we do set it in expand_module before calling
hashtab_map on the type_attr_map callback function.  Whereas you do it
for attr_type_map within type_attr_map callback.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code
  2015-06-18 13:56   ` Stephen Smalley
@ 2015-06-18 20:26     ` James Carter
  2015-06-18 20:29       ` Stephen Smalley
  0 siblings, 1 reply; 21+ messages in thread
From: James Carter @ 2015-06-18 20:26 UTC (permalink / raw)
  To: Stephen Smalley, selinux

On 06/18/2015 09:56 AM, Stephen Smalley wrote:
> On 06/17/2015 03:58 PM, James Carter wrote:
>> The largest change to the user and role bounds checking was to put
>> them in their own functions, so they could be called independently.
>>
>> The type bounds checking was changed to check one type bounds at
>> a time. An expanded avtab is still created, but now only the rules
>> of the parent type are expanded. If violations are discovered,
>> a list of avtab_ptr_t's provides details. This list is used to
>> display error messages for backwards compatibility and will be
>> used by CIL to provide a more detailed error message.
>>
>> Memory usage is reduced from 9,355M to 126M and time is reduced
>> from 9 sec to 2 sec.
>>
>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>
> Can we optimize the case where there are no bounded users/roles/types at
> all in the policy, and quickly return in that situation?  Seems like we
> could just quickly walk them and check to see if any are bounded before
> we start doing anything else.  Surprised we don't already do that.
>

I am not sure how to do it faster.

I am walking the types table [The statement: hashtab_map(p->p_types.table, 
bounds_check_type_callback, &args);] and only calling bounds_check_type() if the 
type has a bounds.

Is there a faster way?

> Also, on this one, I get:
> hierarchy.c: In function ‘bounds_expand_parent_rules’:
> hierarchy.c:202:20: error: unused parameter ‘child’
> [-Werror=unused-parameter]
>             uint32_t child, uint32_t parent)
>                      ^
> hierarchy.c: In function ‘bounds_not_covered’:
> hierarchy.c:261:43: error: unused parameter ‘p’ [-Werror=unused-parameter]
>   static int bounds_not_covered(policydb_t *p, avtab_t *global_avtab,
>                                             ^
> hierarchy.c: In function ‘bounds_check_type_callback’:
> hierarchy.c:510:53: error: unused parameter ‘k’ [-Werror=unused-parameter]
>   static int bounds_check_type_callback(hashtab_key_t k, hashtab_datum_t d,
>                                                       ^
> cc1: all warnings being treated as errors
>
>

I didn't realize that setting DEBUG=1 means that I would miss some warnings.

I obviously need to fix that.

>
>> ---
>>   libsepol/include/sepol/policydb/hierarchy.h |   11 +
>>   libsepol/src/hierarchy.c                    | 1000 +++++++++++++++++----------
>>   2 files changed, 631 insertions(+), 380 deletions(-)
>>
>> diff --git a/libsepol/include/sepol/policydb/hierarchy.h b/libsepol/include/sepol/policydb/hierarchy.h
>> index b4eb9bc..88bc02e 100644
>> --- a/libsepol/include/sepol/policydb/hierarchy.h
>> +++ b/libsepol/include/sepol/policydb/hierarchy.h
>> @@ -25,11 +25,22 @@
>>   #ifndef _SEPOL_POLICYDB_HIERARCHY_H_
>>   #define _SEPOL_POLICYDB_HIERARCHY_H_
>>
>> +#include <sepol/policydb/avtab.h>
>>   #include <sepol/policydb/policydb.h>
>>   #include <sys/cdefs.h>
>>
>>   __BEGIN_DECLS
>>
>> +extern int hierarchy_add_bounds(sepol_handle_t *handle, policydb_t *p);
>> +
>> +extern void bounds_destroy_bad(avtab_ptr_t cur);
>> +extern int bounds_check_type(sepol_handle_t *handle, policydb_t *p, uint32_t child,
>> +			     uint32_t parent, avtab_ptr_t *bad, int *numbad);
>> +
>> +extern int bounds_check_users(sepol_handle_t *handle, policydb_t *p);
>> +extern int bounds_check_roles(sepol_handle_t *handle, policydb_t *p);
>> +extern int bounds_check_types(sepol_handle_t *handle, policydb_t *p);
>> +
>>   extern int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p);
>>
>>   __END_DECLS
>> diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c
>> index d787a64..2436837 100644
>> --- a/libsepol/src/hierarchy.c
>> +++ b/libsepol/src/hierarchy.c
>> @@ -37,466 +37,706 @@
>>
>>   #include "debug.h"
>>
>> -typedef struct hierarchy_args {
>> -	policydb_t *p;
>> -	avtab_t *expa;		/* expanded avtab */
>> -	/* This tells check_avtab_hierarchy to check this list in addition to the unconditional avtab */
>> -	cond_av_list_t *opt_cond_list;
>> -	sepol_handle_t *handle;
>> -	int numerr;
>> -} hierarchy_args_t;
>> +#define BOUNDS_AVTAB_SIZE 1024
>>
>> -/*
>> - * find_parent_(type|role|user)
>> - *
>> - * This function returns the parent datum of given XXX_datum_t
>> - * object or NULL, if it doesn't exist.
>> - *
>> - * If the given datum has a valid bounds, this function merely
>> - * returns the indicated object. Otherwise, it looks up the
>> - * parent based on the based hierarchy.
>> - */
>> -#define find_parent_template(prefix)				\
>> -int find_parent_##prefix(hierarchy_args_t *a,			\
>> -			 prefix##_datum_t *datum,		\
>> -			 prefix##_datum_t **parent)		\
>> -{								\
>> -	char *parent_name, *datum_name, *tmp;			\
>> -								\
>> -	if (datum->bounds)						\
>> -		*parent = a->p->prefix##_val_to_struct[datum->bounds - 1]; \
>> -	else {								\
>> -		datum_name = a->p->p_##prefix##_val_to_name[datum->s.value - 1]; \
>> -									\
>> -		tmp = strrchr(datum_name, '.');				\
>> -		/* no '.' means it has no parent */			\
>> -		if (!tmp) {						\
>> -			*parent = NULL;					\
>> -			return 0;					\
>> -		}							\
>> -									\
>> -		parent_name = strdup(datum_name);			\
>> -		if (!parent_name)					\
>> -			return -1;					\
>> -		parent_name[tmp - datum_name] = '\0';			\
>> -									\
>> -		*parent = hashtab_search(a->p->p_##prefix##s.table, parent_name); \
>> -		if (!*parent) {						\
>> -			/* Orphan type/role/user */			\
>> -			ERR(a->handle,					\
>> -			    "%s doesn't exist, %s is an orphan",	\
>> -			    parent_name,				\
>> -			    a->p->p_##prefix##_val_to_name[datum->s.value - 1]); \
>> -			free(parent_name);				\
>> -			return -1;					\
>> -		}							\
>> -		free(parent_name);					\
>> -	}								\
>> -									\
>> -	return 0;							\
>> +static int bounds_insert_helper(sepol_handle_t *handle, avtab_t *avtab,
>> +				avtab_key_t *avtab_key, avtab_datum_t *datum)
>> +{
>> +	int rc = avtab_insert(avtab, avtab_key, datum);
>> +	if (rc) {
>> +		if (rc == SEPOL_ENOMEM)
>> +			ERR(handle, "Insufficient memory");
>> +		else
>> +			ERR(handle, "Unexpected error (%d)", rc);
>> +	}
>> +	return rc;
>>   }
>>
>> -static find_parent_template(type)
>> -static find_parent_template(role)
>> -static find_parent_template(user)
>>
>> -static void compute_avtab_datum(hierarchy_args_t *args,
>> -				avtab_key_t *key,
>> -				avtab_datum_t *result)
>> +static int bounds_insert_rule(sepol_handle_t *handle, avtab_t *avtab,
>> +			      avtab_t *global, avtab_t *other,
>> +			      avtab_key_t *avtab_key, avtab_datum_t *datum)
>>   {
>> -	avtab_datum_t *avdatp;
>> -	uint32_t av = 0;
>> -
>> -	avdatp = avtab_search(args->expa, key);
>> -	if (avdatp)
>> -		av = avdatp->data;
>> -	if (args->opt_cond_list) {
>> -		avdatp = cond_av_list_search(key, args->opt_cond_list);
>> -		if (avdatp)
>> -			av |= avdatp->data;
>> +	int rc = 0;
>> +	avtab_datum_t *dup = avtab_search(avtab, avtab_key);
>> +
>> +	if (!dup) {
>> +		rc = bounds_insert_helper(handle, avtab, avtab_key, datum);
>> +		if (rc) goto exit;
>> +	} else {
>> +		dup->data |= datum->data;
>>   	}
>>
>> -	result->data = av;
>> +	if (other) {
>> +		/* Search the other conditional avtab for the key and
>> +		 * add any common permissions to the global avtab
>> +		 */
>> +		uint32_t data = 0;
>> +		dup = avtab_search(other, avtab_key);
>> +		if (dup) {
>> +			data = dup->data & datum->data;
>> +			if (data) {
>> +				dup = avtab_search(global, avtab_key);
>> +				if (!dup) {
>> +					avtab_datum_t d;
>> +					d.data = data;
>> +					rc = bounds_insert_helper(handle, global,
>> +								  avtab_key, &d);
>> +					if (rc) goto exit;
>> +				} else {
>> +					dup->data |= data;
>> +				}
>> +			}
>> +		}
>> +	}
>> +
>> +exit:
>> +	return rc;
>>   }
>>
>> -/* This function verifies that the type passed in either has a parent or is in the
>> - * root of the namespace, 0 on success, 1 on orphan and -1 on error
>> - */
>> -static int check_type_hierarchy_callback(hashtab_key_t k, hashtab_datum_t d,
>> -					 void *args)
>> +static int bounds_expand_rule(sepol_handle_t *handle, policydb_t *p,
>> +			      avtab_t *avtab, avtab_t *global, avtab_t *other,
>> +			      uint32_t parent, uint32_t src, uint32_t tgt,
>> +			      uint32_t class, uint32_t data)
>>   {
>> -	hierarchy_args_t *a;
>> -	type_datum_t *t, *tp;
>> -
>> -	a = (hierarchy_args_t *) args;
>> -	t = (type_datum_t *) d;
>> +	int rc = 0;
>> +	avtab_key_t avtab_key;
>> +	avtab_datum_t datum;
>> +	ebitmap_node_t *tnode;
>> +	unsigned int i;
>> +
>> +	avtab_key.specified = AVTAB_ALLOWED;
>> +	avtab_key.target_class = class;
>> +	datum.data = data;
>> +
>> +	if (ebitmap_get_bit(&p->attr_type_map[src - 1], parent - 1)) {
>> +		avtab_key.source_type = parent;
>> +		ebitmap_for_each_bit(&p->attr_type_map[tgt - 1], tnode, i) {
>> +			if (!ebitmap_node_get_bit(tnode, i))
>> +				continue;
>> +			avtab_key.target_type = i + 1;
>> +			rc = bounds_insert_rule(handle, avtab, global, other,
>> +						&avtab_key, &datum);
>> +			if (rc) goto exit;
>> +		}
>> +	}
>>
>> -	if (t->flavor == TYPE_ATTRIB) {
>> -		/* It's an attribute, we don't care */
>> -		return 0;
>> +	if (ebitmap_get_bit(&p->attr_type_map[tgt - 1], parent - 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;
>> +			rc = bounds_insert_rule(handle, avtab, global, other,
>> +						&avtab_key, &datum);
>> +			if (rc) goto exit;
>> +		}
>>   	}
>> -	if (find_parent_type(a, t, &tp) < 0)
>> -		return -1;
>> -
>> -	if (tp && tp->flavor == TYPE_ATTRIB) {
>> -		/* The parent is an attribute but the child isn't, not legal */
>> -		ERR(a->handle, "type %s is a child of an attribute %s",
>> -		    (char *) k, a->p->p_type_val_to_name[tp->s.value - 1]);
>> -		a->numerr++;
>> -		return -1;
>> +
>> +exit:
>> +	return rc;
>> +}
>> +
>> +static int bounds_expand_cond_rules(sepol_handle_t *handle, policydb_t *p,
>> +				    cond_av_list_t *cur, avtab_t *avtab,
>> +				    avtab_t *global, avtab_t *other,
>> +				    uint32_t parent)
>> +{
>> +	int rc = 0;
>> +
>> +	for (; cur; cur = cur->next) {
>> +		avtab_ptr_t n = cur->node;
>> +		rc = bounds_expand_rule(handle, p, avtab, global, other, parent,
>> +					n->key.source_type, n->key.target_type,
>> +					n->key.target_class, n->datum.data);
>> +		if (rc) goto exit;
>>   	}
>> -	return 0;
>> +
>> +exit:
>> +	return rc;
>>   }
>>
>> -/* This function only verifies that the avtab node passed in does not violate any
>> - * hiearchy constraint via any relationship with other types in the avtab.
>> - * it should be called using avtab_map, returns 0 on success, 1 on violation and
>> - * -1 on error. opt_cond_list is an optional argument that tells this to check
>> - * a conditional list for the relationship as well as the unconditional avtab
>> - */
>> -static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d,
>> -					  void *args)
>> +struct bounds_expand_args {
>> +	sepol_handle_t *handle;
>> +	policydb_t *p;
>> +	avtab_t *avtab;
>> +	uint32_t parent;
>> +};
>> +
>> +static int bounds_expand_rule_callback(avtab_key_t *k, avtab_datum_t *d,
>> +				       void *args)
>>   {
>> -	avtab_key_t key;
>> -	hierarchy_args_t *a = (hierarchy_args_t *) args;
>> -	type_datum_t *s, *t1 = NULL, *t2 = NULL;
>> -	avtab_datum_t av;
>> +	struct bounds_expand_args *a = (struct bounds_expand_args *)args;
>>
>> -	if (!(k->specified & AVTAB_ALLOWED)) {
>> -		/* This is not an allow rule, no checking done */
>> +	if (!(k->specified & AVTAB_ALLOWED))
>>   		return 0;
>> -	}
>>
>> -	/* search for parent first */
>> -	s = a->p->type_val_to_struct[k->source_type - 1];
>> -	if (find_parent_type(a, s, &t1) < 0)
>> -		return -1;
>> -	if (t1) {
>> -		/*
>> -		 * search for access allowed between type 1's
>> -		 * parent and type 2.
>> -		 */
>> -		key.source_type = t1->s.value;
>> -		key.target_type = k->target_type;
>> -		key.target_class = k->target_class;
>> -		key.specified = AVTAB_ALLOWED;
>> -		compute_avtab_datum(a, &key, &av);
>> -
>> -		if ((av.data & d->data) == d->data)
>> -			return 0;
>> +	return bounds_expand_rule(a->handle, a->p, a->avtab, NULL, NULL,
>> +				  a->parent, k->source_type, k->target_type,
>> +				  k->target_class, d->data);
>> +}
>> +
>> +struct bounds_cond_info {
>> +	avtab_t true_avtab;
>> +	avtab_t false_avtab;
>> +	cond_list_t *cond_list;
>> +	struct bounds_cond_info *next;
>> +};
>> +
>> +static void bounds_destroy_cond_info(struct bounds_cond_info *cur)
>> +{
>> +	struct bounds_cond_info *next;
>> +
>> +	for (; cur; cur = next) {
>> +		next = cur->next;
>> +		avtab_destroy(&cur->true_avtab);
>> +		avtab_destroy(&cur->false_avtab);
>> +		cur->next = NULL;
>> +		free(cur);
>>   	}
>> +}
>>
>> -	/* next we try type 1 and type 2's parent */
>> -	s = a->p->type_val_to_struct[k->target_type - 1];
>> -	if (find_parent_type(a, s, &t2) < 0)
>> -		return -1;
>> -	if (t2) {
>> -		/*
>> -		 * search for access allowed between type 1 and
>> -		 * type 2's parent.
>> -		 */
>> -		key.source_type = k->source_type;
>> -		key.target_type = t2->s.value;
>> -		key.target_class = k->target_class;
>> -		key.specified = AVTAB_ALLOWED;
>> -		compute_avtab_datum(a, &key, &av);
>> -
>> -		if ((av.data & d->data) == d->data)
>> -			return 0;
>> +static int bounds_expand_parent_rules(sepol_handle_t *handle, policydb_t *p,
>> +				      avtab_t *global_avtab,
>> +				      struct bounds_cond_info **cond_info,
>> +				      uint32_t child, uint32_t parent)
>> +{
>> +	int rc = 0;
>> +	struct bounds_expand_args args;
>> +	cond_list_t *cur;
>> +
>> +	avtab_init(global_avtab);
>> +	rc = avtab_alloc(global_avtab, BOUNDS_AVTAB_SIZE);
>> +	if (rc) goto oom;
>> +
>> +	args.handle = handle;
>> +	args.p = p;
>> +	args.avtab = global_avtab;
>> +	args.parent = parent;
>> +	rc = avtab_map(&p->te_avtab, bounds_expand_rule_callback, &args);
>> +	if (rc) goto exit;
>> +
>> +	*cond_info = NULL;
>> +	for (cur = p->cond_list; cur; cur = cur->next) {
>> +		struct bounds_cond_info *ci;
>> +		ci = malloc(sizeof(struct bounds_cond_info));
>> +		if (!ci) goto oom;
>> +		avtab_init(&ci->true_avtab);
>> +		avtab_init(&ci->false_avtab);
>> +		ci->cond_list = cur;
>> +		ci->next = *cond_info;
>> +		*cond_info = ci;
>> +		if (cur->true_list) {
>> +			rc = avtab_alloc(&ci->true_avtab, BOUNDS_AVTAB_SIZE);
>> +			if (rc) goto oom;
>> +			rc = bounds_expand_cond_rules(handle, p, cur->true_list,
>> +						      &ci->true_avtab, NULL,
>> +						      NULL, parent);
>> +			if (rc) goto exit;
>> +		}
>> +		if (cur->false_list) {
>> +			rc = avtab_alloc(&ci->false_avtab, BOUNDS_AVTAB_SIZE);
>> +			if (rc) goto oom;
>> +			rc = bounds_expand_cond_rules(handle, p, cur->false_list,
>> +						      &ci->false_avtab,
>> +						      global_avtab,
>> +						      &ci->true_avtab, parent);
>> +			if (rc) goto exit;
>> +		}
>>   	}
>>
>> -	if (t1 && t2) {
>> -		/*
>> -                 * search for access allowed between type 1's parent
>> -                 * and type 2's parent.
>> -                 */
>> -		key.source_type = t1->s.value;
>> -		key.target_type = t2->s.value;
>> -		key.target_class = k->target_class;
>> -		key.specified = AVTAB_ALLOWED;
>> -		compute_avtab_datum(a, &key, &av);
>> -
>> -		if ((av.data & d->data) == d->data)
>> -			return 0;
>> +	return 0;
>> +
>> +oom:
>> +	ERR(handle, "Insufficient memory");
>> +
>> +exit:
>> +	ERR(handle,"Failed to expand parent rules\n");
>> +	avtab_destroy(global_avtab);
>> +	bounds_destroy_cond_info(*cond_info);
>> +	*cond_info = NULL;
>> +	return rc;
>> +}
>> +
>> +static int bounds_not_covered(policydb_t *p, avtab_t *global_avtab,
>> +			      avtab_t *cur_avtab, avtab_key_t *avtab_key,
>> +			      uint32_t data)
>> +{
>> +	avtab_datum_t *datum = avtab_search(cur_avtab, avtab_key);
>> +	if (datum)
>> +		data &= ~datum->data;
>> +	if (global_avtab && data) {
>> +		datum = avtab_search(global_avtab, avtab_key);
>> +		if (datum)
>> +			data &= ~datum->data;
>>   	}
>>
>> -	/*
>> -	 * Neither one of these types have parents and
>> -	 * therefore the hierarchical constraint does not apply
>> -	 */
>> -	if (!t1 && !t2)
>> -		return 0;
>> +	return data;
>> +}
>> +
>> +static int bounds_add_bad(sepol_handle_t *handle, uint32_t src, uint32_t tgt,
>> +			  uint32_t class, uint32_t data, avtab_ptr_t *bad)
>> +{
>> +	struct avtab_node *new = malloc(sizeof(struct avtab_node));
>> +	if (new == NULL) {
>> +		ERR(handle, "Insufficient memory");
>> +		return SEPOL_ENOMEM;
>> +	}
>> +	memset(new, 0, sizeof(struct avtab_node));
>> +	new->key.source_type = src;
>> +	new->key.target_type = tgt;
>> +	new->key.target_class = class;
>> +	new->datum.data = data;
>> +	new->next = *bad;
>> +	*bad = new;
>>
>> -	/*
>> -	 * At this point there is a violation of the hierarchal
>> -	 * constraint, send error condition back
>> -	 */
>> -	ERR(a->handle,
>> -	    "hierarchy violation between types %s and %s : %s { %s }",
>> -	    a->p->p_type_val_to_name[k->source_type - 1],
>> -	    a->p->p_type_val_to_name[k->target_type - 1],
>> -	    a->p->p_class_val_to_name[k->target_class - 1],
>> -	    sepol_av_to_string(a->p, k->target_class, d->data & ~av.data));
>> -	a->numerr++;
>>   	return 0;
>>   }
>>
>> -/*
>> - * If same permissions are allowed for same combination of
>> - * source and target, we can evaluate them as unconditional
>> - * one.
>> - * See the following example. A_t type is bounds of B_t type,
>> - * so B_t can never have wider permissions then A_t.
>> - * A_t has conditional permission on X_t, however, a part of
>> - * them (getattr and read) are unconditionaly allowed to A_t.
>> - *
>> - * Example)
>> - * typebounds A_t B_t;
>> - *
>> - * allow B_t X_t : file { getattr };
>> - * if (foo_bool) {
>> - *     allow A_t X_t : file { getattr read };
>> - * } else {
>> - *     allow A_t X_t : file { getattr read write };
>> - * }
>> - *
>> - * We have to pull up them as unconditional ones in this case,
>> - * because it seems to us B_t is violated to bounds constraints
>> - * during unconditional policy checking.
>> - */
>> -static int pullup_unconditional_perms(cond_list_t * cond_list,
>> -				      hierarchy_args_t * args)
>> +static int bounds_check_rule(sepol_handle_t *handle, policydb_t *p,
>> +			     avtab_t *global_avtab, avtab_t *cur_avtab,
>> +			     uint32_t child, uint32_t parent, uint32_t src,
>> +			     uint32_t tgt, uint32_t class, uint32_t data,
>> +			     avtab_ptr_t *bad, int *numbad)
>>   {
>> -	cond_list_t *cur_node;
>> -	cond_av_list_t *cur_av, *expl_true = NULL, *expl_false = NULL;
>> -	avtab_t expa_true, expa_false;
>> -	avtab_datum_t *avdatp;
>> -	avtab_datum_t avdat;
>> -	avtab_ptr_t avnode;
>> -
>> -	for (cur_node = cond_list; cur_node; cur_node = cur_node->next) {
>> -		if (avtab_init(&expa_true))
>> -			goto oom0;
>> -		if (avtab_init(&expa_false))
>> -			goto oom1;
>> -		if (expand_cond_av_list(args->p, cur_node->true_list,
>> -					&expl_true, &expa_true))
>> -			goto oom2;
>> -		if (expand_cond_av_list(args->p, cur_node->false_list,
>> -					&expl_false, &expa_false))
>> -			goto oom3;
>> -		for (cur_av = expl_true; cur_av; cur_av = cur_av->next) {
>> -			avdatp = avtab_search(&expa_false,
>> -					      &cur_av->node->key);
>> -			if (!avdatp)
>> +	int rc = 0;
>> +	avtab_key_t avtab_key;
>> +	type_datum_t *td;
>> +	ebitmap_node_t *tnode;
>> +	unsigned int i;
>> +	uint32_t d;
>> +
>> +	avtab_key.specified = AVTAB_ALLOWED;
>> +	avtab_key.target_class = class;
>> +
>> +	if (ebitmap_get_bit(&p->attr_type_map[src - 1], child - 1)) {
>> +		avtab_key.source_type = parent;
>> +		ebitmap_for_each_bit(&p->attr_type_map[tgt - 1], tnode, i) {
>> +			if (!ebitmap_node_get_bit(tnode, i))
>>   				continue;
>> -
>> -			avdat.data = (cur_av->node->datum.data
>> -				      & avdatp->data);
>> -			if (!avdat.data)
>> +			avtab_key.target_type = i + 1;
>> +			d = bounds_not_covered(p, global_avtab, cur_avtab,
>> +					       &avtab_key, data);
>> +			if (!d) continue;
>> +			td = p->type_val_to_struct[i];
>> +			if (td && td->bounds) {
>> +				avtab_key.target_type = td->bounds;
>> +				d = bounds_not_covered(p, global_avtab,
>> +						       cur_avtab, &avtab_key,
>> +						       data);
>> +				if (!d) continue;
>> +			}
>> +			(*numbad)++;
>> +			rc = bounds_add_bad(handle, child, i+1, class, d, bad);
>> +			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;
>> -
>> -			avnode = avtab_search_node(args->expa,
>> -						   &cur_av->node->key);
>> -			if (avnode) {
>> -				avnode->datum.data |= avdat.data;
>> -			} else {
>> -				if (avtab_insert(args->expa,
>> -						 &cur_av->node->key,
>> -						 &avdat))
>> -					goto oom4;
>> +			avtab_key.source_type = i + 1;
>> +			if (avtab_key.source_type == child) {
>> +				/* Checked above */
>> +				continue;
>> +			}
>> +			d = bounds_not_covered(p, 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(p, 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;
>>   		}
>> -		cond_av_list_destroy(expl_false);
>> -		cond_av_list_destroy(expl_true);
>> -		avtab_destroy(&expa_false);
>> -		avtab_destroy(&expa_true);
>>   	}
>> -	return 0;
>>
>> -oom4:
>> -	cond_av_list_destroy(expl_false);
>> -oom3:
>> -	cond_av_list_destroy(expl_true);
>> -oom2:
>> -	avtab_destroy(&expa_false);
>> -oom1:
>> -	avtab_destroy(&expa_true);
>> -oom0:
>> -	ERR(args->handle, "out of memory on conditional av list expansion");
>> -        return 1;
>> +exit:
>> +	return rc;
>> +}
>> +
>> +static int bounds_check_cond_rules(sepol_handle_t *handle, policydb_t *p,
>> +				   avtab_t *global_avtab, avtab_t *cond_avtab,
>> +				   cond_av_list_t *rules, uint32_t child,
>> +				   uint32_t parent, avtab_ptr_t *bad,
>> +				   int *numbad)
>> +{
>> +	int rc = 0;
>> +	cond_av_list_t *cur;
>> +
>> +	for (cur = rules; cur; cur = cur->next) {
>> +		avtab_ptr_t ap = cur->node;
>> +		avtab_key_t *key = &ap->key;
>> +		avtab_datum_t *datum = &ap->datum;
>> +		if (!(key->specified & AVTAB_ALLOWED))
>> +			continue;
>> +		rc = bounds_check_rule(handle, p, global_avtab, cond_avtab,
>> +				       child, parent, key->source_type,
>> +				       key->target_type, key->target_class,
>> +				       datum->data, bad, numbad);
>> +		if (rc) goto exit;
>> +	}
>> +
>> +exit:
>> +	return rc;
>> +}
>> +
>> +struct bounds_check_args {
>> +	sepol_handle_t *handle;
>> +	policydb_t *p;
>> +	avtab_t *cur_avtab;
>> +	uint32_t child;
>> +	uint32_t parent;
>> +	avtab_ptr_t bad;
>> +	int numbad;
>> +};
>> +
>> +static int bounds_check_rule_callback(avtab_key_t *k, avtab_datum_t *d,
>> +				      void *args)
>> +{
>> +	struct bounds_check_args *a = (struct bounds_check_args *)args;
>> +
>> +	if (!(k->specified & AVTAB_ALLOWED))
>> +		return 0;
>> +
>> +	return bounds_check_rule(a->handle, a->p, NULL, a->cur_avtab, a->child,
>> +				 a->parent, k->source_type, k->target_type,
>> +				 k->target_class, d->data, &a->bad, &a->numbad);
>>   }
>>
>> -static int check_cond_avtab_hierarchy(cond_list_t * cond_list,
>> -				      hierarchy_args_t * args)
>> +static int bounds_check_child_rules(sepol_handle_t *handle, policydb_t *p,
>> +				    avtab_t *global_avtab,
>> +				    struct bounds_cond_info *cond_info,
>> +				    uint32_t child, uint32_t parent,
>> +				    avtab_ptr_t *bad, int *numbad)
>>   {
>>   	int rc;
>> -	cond_list_t *cur_node;
>> -	cond_av_list_t *cur_av, *expl = NULL;
>> -	avtab_t expa;
>> -	hierarchy_args_t *a = (hierarchy_args_t *) args;
>> -	avtab_datum_t avdat, *uncond;
>> -
>> -	for (cur_node = cond_list; cur_node; cur_node = cur_node->next) {
>> -		/*
>> -		 * Check true condition
>> -		 */
>> -		if (avtab_init(&expa))
>> -			goto oom;
>> -		if (expand_cond_av_list(args->p, cur_node->true_list,
>> -					&expl, &expa)) {
>> -			avtab_destroy(&expa);
>> -			goto oom;
>> -		}
>> -		args->opt_cond_list = expl;
>> -		for (cur_av = expl; cur_av; cur_av = cur_av->next) {
>> -			avdat.data = cur_av->node->datum.data;
>> -			uncond = avtab_search(a->expa, &cur_av->node->key);
>> -			if (uncond)
>> -				avdat.data |= uncond->data;
>> -			rc = check_avtab_hierarchy_callback(&cur_av->node->key,
>> -							    &avdat, args);
>> -			if (rc)
>> -				args->numerr++;
>> -		}
>> -		cond_av_list_destroy(expl);
>> -		avtab_destroy(&expa);
>> +	struct bounds_check_args args;
>> +	struct bounds_cond_info *cur;
>>
>> -		/*
>> -		 * Check false condition
>> -		 */
>> -		if (avtab_init(&expa))
>> -			goto oom;
>> -		if (expand_cond_av_list(args->p, cur_node->false_list,
>> -					&expl, &expa)) {
>> -			avtab_destroy(&expa);
>> -			goto oom;
>> -		}
>> -		args->opt_cond_list = expl;
>> -		for (cur_av = expl; cur_av; cur_av = cur_av->next) {
>> -			avdat.data = cur_av->node->datum.data;
>> -			uncond = avtab_search(a->expa, &cur_av->node->key);
>> -			if (uncond)
>> -				avdat.data |= uncond->data;
>> -
>> -			rc = check_avtab_hierarchy_callback(&cur_av->node->key,
>> -							    &avdat, args);
>> -			if (rc)
>> -				a->numerr++;
>> +	args.handle = handle;
>> +	args.p = p;
>> +	args.cur_avtab = global_avtab;
>> +	args.child = child;
>> +	args.parent = parent;
>> +	args.bad = NULL;
>> +	args.numbad = 0;
>> +	rc = avtab_map(&p->te_avtab, bounds_check_rule_callback, &args);
>> +	if (rc) goto exit;
>> +
>> +	for (cur = cond_info; cur; cur = cur->next) {
>> +		cond_list_t *node = cur->cond_list;
>> +		rc = bounds_check_cond_rules(handle, p, global_avtab,
>> +					     &cur->true_avtab,
>> +					     node->true_list, child, parent,
>> +					     &args.bad, &args.numbad);
>> +		if (rc) goto exit;
>> +
>> +		rc = bounds_check_cond_rules(handle, p, global_avtab,
>> +					     &cur->false_avtab,
>> +					     node->false_list, child, parent,
>> +					     &args.bad, &args.numbad);
>> +		if (rc) goto exit;
>> +	}
>> +
>> +	*numbad += args.numbad;
>> +	*bad = args.bad;
>> +
>> +exit:
>> +	return rc;
>> +}
>> +
>> +int bounds_check_type(sepol_handle_t *handle, policydb_t *p, uint32_t child,
>> +		      uint32_t parent, avtab_ptr_t *bad, int *numbad)
>> +{
>> +	int rc = 0;
>> +	avtab_t global_avtab;
>> +	struct bounds_cond_info *cond_info = NULL;
>> +
>> +	rc = bounds_expand_parent_rules(handle, p, &global_avtab, &cond_info,
>> +					child, parent);
>> +	if (rc) goto exit;
>> +
>> +	rc = bounds_check_child_rules(handle, p, &global_avtab, cond_info,
>> +				      child, parent, bad, numbad);
>> +
>> +	bounds_destroy_cond_info(cond_info);
>> +	avtab_destroy(&global_avtab);
>> +
>> +exit:
>> +	return rc;
>> +}
>> +
>> +struct bounds_args {
>> +	sepol_handle_t *handle;
>> +	policydb_t *p;
>> +	int numbad;
>> +};
>> +
>> +static void bounds_report(sepol_handle_t *handle, policydb_t *p, uint32_t child,
>> +			  uint32_t parent, avtab_ptr_t cur)
>> +{
>> +	ERR(handle, "Child type %s exceeds bounds of parent %s in the following rules:",
>> +	    p->p_type_val_to_name[child - 1],
>> +	    p->p_type_val_to_name[parent - 1]);
>> +	for (; cur; cur = cur->next) {
>> +		ERR(handle, "    %s %s : %s { %s }",
>> +		    p->p_type_val_to_name[cur->key.source_type - 1],
>> +		    p->p_type_val_to_name[cur->key.target_type - 1],
>> +		    p->p_class_val_to_name[cur->key.target_class - 1],
>> +		    sepol_av_to_string(p, cur->key.target_class,
>> +				       cur->datum.data));
>> +	}
>> +}
>> +
>> +void bounds_destroy_bad(avtab_ptr_t cur)
>> +{
>> +	avtab_ptr_t next;
>> +
>> +	for (; cur; cur = next) {
>> +		next = cur->next;
>> +		cur->next = NULL;
>> +		free(cur);
>> +	}
>> +}
>> +
>> +static int bounds_check_type_callback(hashtab_key_t k, hashtab_datum_t d,
>> +				      void *args)
>> +{
>> +	int rc = 0;
>> +	struct bounds_args *a = (struct bounds_args *)args;
>> +	type_datum_t *t = (type_datum_t *)d;
>> +	avtab_ptr_t bad = NULL;
>> +
>> +	if (t->bounds) {
>> +		rc = bounds_check_type(a->handle, a->p, t->s.value, t->bounds,
>> +				       &bad, &a->numbad);
>> +		if (bad) {
>> +			bounds_report(a->handle, a->p, t->s.value, t->bounds,
>> +				      bad);
>> +			bounds_destroy_bad(bad);
>>   		}
>> -		cond_av_list_destroy(expl);
>> -		avtab_destroy(&expa);
>>   	}
>>
>> -	return 0;
>> +	return rc;
>> +}
>> +
>> +int bounds_check_types(sepol_handle_t *handle, policydb_t *p)
>> +{
>> +	int rc;
>> +	struct bounds_args args;
>> +
>> +	args.handle = handle;
>> +	args.p = p;
>> +	args.numbad = 0;
>> +
>> +	rc = hashtab_map(p->p_types.table, bounds_check_type_callback, &args);
>> +	if (rc) goto exit;
>> +
>> +	if (args.numbad > 0) {
>> +		ERR(handle, "%d errors found during type bounds check",
>> +		    args.numbad);
>> +		rc = SEPOL_ERR;
>> +	}
>>
>> -      oom:
>> -	ERR(args->handle, "out of memory on conditional av list expansion");
>> -	return 1;
>> +exit:
>> +	return rc;
>>   }
>>
>> -/* The role hierarchy is defined as: a child role cannot have more types than it's parent.
>> - * This function should be called with hashtab_map, it will return 0 on success, 1 on
>> - * constraint violation and -1 on error
>> +/* The role bounds is defined as: a child role cannot have a type that
>> + * its parent doesn't have.
>>    */
>> -static int check_role_hierarchy_callback(hashtab_key_t k
>> -					 __attribute__ ((unused)),
>> -					 hashtab_datum_t d, void *args)
>> +static int bounds_check_role_callback(hashtab_key_t k __attribute__ ((unused)),
>> +				      hashtab_datum_t d, void *args)
>>   {
>> -	hierarchy_args_t *a;
>> -	role_datum_t *r, *rp;
>> +	struct bounds_args *a = (struct bounds_args *)args;
>> +	role_datum_t *r = (role_datum_t *) d;
>> +	role_datum_t *rp = NULL;
>>
>> -	a = (hierarchy_args_t *) args;
>> -	r = (role_datum_t *) d;
>> +	if (!r->bounds)
>> +		return 0;
>>
>> -	if (find_parent_role(a, r, &rp) < 0)
>> -		return -1;
>> +	rp = a->p->role_val_to_struct[r->bounds - 1];
>>
>>   	if (rp && !ebitmap_contains(&rp->types.types, &r->types.types)) {
>> -		/* hierarchical constraint violation, return error */
>> -		ERR(a->handle, "Role hierarchy violation, %s exceeds %s",
>> -		    (char *) k, a->p->p_role_val_to_name[rp->s.value - 1]);
>> -		a->numerr++;
>> +		ERR(a->handle, "Role bounds violation, %s exceeds %s",
>> +		    (char *)k, a->p->p_role_val_to_name[rp->s.value - 1]);
>> +		a->numbad++;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int bounds_check_roles(sepol_handle_t *handle, policydb_t *p)
>> +{
>> +	struct bounds_args args;
>> +
>> +	args.handle = handle;
>> +	args.p = p;
>> +	args.numbad = 0;
>> +
>> +	hashtab_map(p->p_roles.table, bounds_check_role_callback, &args);
>> +
>> +	if (args.numbad > 0) {
>> +		ERR(handle, "%d errors found during role bounds check",
>> +		    args.numbad);
>> +		return SEPOL_ERR;
>>   	}
>> +
>>   	return 0;
>>   }
>>
>> -/* The user hierarchy is defined as: a child user cannot have a role that
>> - * its parent doesn't have.  This function should be called with hashtab_map,
>> - * it will return 0 on success, 1 on constraint violation and -1 on error.
>> +/* The user bounds is defined as: a child user cannot have a role that
>> + * its parent doesn't have.
>>    */
>> -static int check_user_hierarchy_callback(hashtab_key_t k
>> -					 __attribute__ ((unused)),
>> -					 hashtab_datum_t d, void *args)
>> +static int bounds_check_user_callback(hashtab_key_t k __attribute__ ((unused)),
>> +				      hashtab_datum_t d, void *args)
>>   {
>> -	hierarchy_args_t *a;
>> -	user_datum_t *u, *up;
>> +	struct bounds_args *a = (struct bounds_args *)args;
>> +	user_datum_t *u = (user_datum_t *) d;
>> +	user_datum_t *up = NULL;
>>
>> -	a = (hierarchy_args_t *) args;
>> -	u = (user_datum_t *) d;
>> +	if (!u->bounds)
>> +		return 0;
>>
>> -	if (find_parent_user(a, u, &up) < 0)
>> -		return -1;
>> +	up = a->p->user_val_to_struct[u->bounds - 1];
>>
>>   	if (up && !ebitmap_contains(&up->roles.roles, &u->roles.roles)) {
>> -		/* hierarchical constraint violation, return error */
>> -		ERR(a->handle, "User hierarchy violation, %s exceeds %s",
>> +		ERR(a->handle, "User bounds violation, %s exceeds %s",
>>   		    (char *) k, a->p->p_user_val_to_name[up->s.value - 1]);
>> -		a->numerr++;
>> +		a->numbad++;
>>   	}
>> +
>>   	return 0;
>>   }
>>
>> -int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p)
>> +int bounds_check_users(sepol_handle_t *handle, policydb_t *p)
>>   {
>> -	hierarchy_args_t args;
>> -	avtab_t expa;
>> -
>> -	if (avtab_init(&expa))
>> -		goto oom;
>> -	if (expand_avtab(p, &p->te_avtab, &expa)) {
>> -		avtab_destroy(&expa);
>> -		goto oom;
>> -	}
>> +	struct bounds_args args;
>>
>> -	args.p = p;
>> -	args.expa = &expa;
>> -	args.opt_cond_list = NULL;
>>   	args.handle = handle;
>> -	args.numerr = 0;
>> +	args.p = p;
>> +	args.numbad = 0;
>>
>> -	if (hashtab_map(p->p_types.table, check_type_hierarchy_callback, &args))
>> -		goto bad;
>> +	hashtab_map(p->p_users.table, bounds_check_user_callback, &args);
>>
>> -	if (pullup_unconditional_perms(p->cond_list, &args))
>> -		return -1;
>> +	if (args.numbad > 0) {
>> +		ERR(handle, "%d errors found during user bounds check",
>> +		    args.numbad);
>> +		return SEPOL_ERR;
>> +	}
>>
>> -	if (avtab_map(&expa, check_avtab_hierarchy_callback, &args))
>> -		goto bad;
>> +	return 0;
>> +}
>>
>> -	if (check_cond_avtab_hierarchy(p->cond_list, &args))
>> -		goto bad;
>> +#define add_hierarchy_callback_template(prefix)				\
>> +	int hierarchy_add_##prefix##_callback(hashtab_key_t k __attribute__ ((unused)), \
>> +					    hashtab_datum_t d, void *args) \
>> +{								\
>> +	struct bounds_args *a = (struct bounds_args *)args;		\
>> +	sepol_handle_t *handle = a->handle;				\
>> +	policydb_t *p = a->p;						\
>> +	prefix##_datum_t *datum = (prefix##_datum_t *)d;		\
>> +	prefix##_datum_t *parent;					\
>> +	char *parent_name, *datum_name, *tmp;				\
>> +									\
>> +	if (!datum->bounds) {						\
>> +		datum_name = p->p_##prefix##_val_to_name[datum->s.value - 1]; \
>> +									\
>> +		tmp = strrchr(datum_name, '.');				\
>> +		/* no '.' means it has no parent */			\
>> +		if (!tmp) return 0;					\
>> +									\
>> +		parent_name = strdup(datum_name);			\
>> +		if (!parent_name) {					\
>> +			ERR(handle, "Insufficient memory");		\
>> +			return SEPOL_ENOMEM;				\
>> +		}							\
>> +		parent_name[tmp - datum_name] = '\0';			\
>> +									\
>> +		parent = hashtab_search(p->p_##prefix##s.table, parent_name); \
>> +		if (!parent) {						\
>> +			/* Orphan type/role/user */			\
>> +			ERR(handle, "%s doesn't exist, %s is an orphan",\
>> +			    parent_name,				\
>> +			    p->p_##prefix##_val_to_name[datum->s.value - 1]); \
>> +			free(parent_name);				\
>> +			a->numbad++;					\
>> +			return 0;					\
>> +		}							\
>> +		datum->bounds = parent->s.value;			\
>> +		free(parent_name);					\
>> +	}								\
>> +									\
>> +	return 0;							\
>> +}								\
>> +
>> +static add_hierarchy_callback_template(type)
>> +static add_hierarchy_callback_template(role)
>> +static add_hierarchy_callback_template(user)
>>
>> -	if (hashtab_map(p->p_roles.table, check_role_hierarchy_callback, &args))
>> -		goto bad;
>> +int hierarchy_add_bounds(sepol_handle_t *handle, policydb_t *p)
>> +{
>> +	int rc = 0;
>> +	struct bounds_args args;
>> +
>> +	args.handle = handle;
>> +	args.p = p;
>> +	args.numbad = 0;
>> +
>> +	rc = hashtab_map(p->p_users.table, hierarchy_add_user_callback, &args);
>> +	if (rc) goto exit;
>> +
>> +	rc = hashtab_map(p->p_roles.table, hierarchy_add_role_callback, &args);
>> +	if (rc) goto exit;
>>
>> -	if (hashtab_map(p->p_users.table, check_user_hierarchy_callback, &args))
>> -		goto bad;
>> +	rc = hashtab_map(p->p_types.table, hierarchy_add_type_callback, &args);
>> +	if (rc) goto exit;
>>
>> -	if (args.numerr) {
>> -		ERR(handle, "%d total errors found during hierarchy check",
>> -		    args.numerr);
>> -		goto bad;
>> +	if (args.numbad > 0) {
>> +		ERR(handle, "%d errors found while adding hierarchies",
>> +		    args.numbad);
>> +		rc = SEPOL_ERR;
>>   	}
>>
>> -	avtab_destroy(&expa);
>> -	return 0;
>> +exit:
>> +	return rc;
>> +}
>> +
>> +int hierarchy_check_constraints(sepol_handle_t * handle, policydb_t * p)
>> +{
>> +	int rc = 0;
>> +	int violation = 0;
>> +
>> +	rc = hierarchy_add_bounds(handle, p);
>> +	if (rc) goto exit;
>> +
>> +	rc = bounds_check_users(handle, p);
>> +	if (rc)
>> +		violation = 1;
>> +
>> +	rc = bounds_check_roles(handle, p);
>> +	if (rc)
>> +		violation = 1;
>> +
>> +	rc = bounds_check_types(handle, p);
>> +	if (rc) {
>> +		if (rc == SEPOL_ERR)
>> +			violation = 1;
>> +		else
>> +			goto exit;
>> +	}
>>
>> -      bad:
>> -	avtab_destroy(&expa);
>> -	return -1;
>> +	if (violation)
>> +		rc = SEPOL_ERR;
>>
>> -      oom:
>> -	ERR(handle, "Out of memory");
>> -	return -1;
>> +exit:
>> +	return rc;
>>   }
>>


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code
  2015-06-18 20:26     ` James Carter
@ 2015-06-18 20:29       ` Stephen Smalley
  2015-06-18 20:35         ` James Carter
  0 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2015-06-18 20:29 UTC (permalink / raw)
  To: James Carter, selinux

On 06/18/2015 04:26 PM, James Carter wrote:
> On 06/18/2015 09:56 AM, Stephen Smalley wrote:
>> On 06/17/2015 03:58 PM, James Carter wrote:
>>> The largest change to the user and role bounds checking was to put
>>> them in their own functions, so they could be called independently.
>>>
>>> The type bounds checking was changed to check one type bounds at
>>> a time. An expanded avtab is still created, but now only the rules
>>> of the parent type are expanded. If violations are discovered,
>>> a list of avtab_ptr_t's provides details. This list is used to
>>> display error messages for backwards compatibility and will be
>>> used by CIL to provide a more detailed error message.
>>>
>>> Memory usage is reduced from 9,355M to 126M and time is reduced
>>> from 9 sec to 2 sec.
>>>
>>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>>
>> Can we optimize the case where there are no bounded users/roles/types at
>> all in the policy, and quickly return in that situation?  Seems like we
>> could just quickly walk them and check to see if any are bounded before
>> we start doing anything else.  Surprised we don't already do that.
>>
> 
> I am not sure how to do it faster.
> 
> I am walking the types table [The statement:
> hashtab_map(p->p_types.table, bounds_check_type_callback, &args);] and
> only calling bounds_check_type() if the type has a bounds.
> 
> Is there a faster way?

So is there any avtab allocation if there are no bounded types?
That's what I wanted to ensure we avoid or at least minimize.

I was surprised that you indicated that we have significant memory and
time usage from the old hierarchy checker since there are no bounded
types in the default policy; I had assumed that it would optimize for
that case, just as we quickly bail out of the assertion checker if there
are no neverallows before even doing an avtab_init.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code
  2015-06-18 20:29       ` Stephen Smalley
@ 2015-06-18 20:35         ` James Carter
  0 siblings, 0 replies; 21+ messages in thread
From: James Carter @ 2015-06-18 20:35 UTC (permalink / raw)
  To: Stephen Smalley, selinux

On 06/18/2015 04:29 PM, Stephen Smalley wrote:
> On 06/18/2015 04:26 PM, James Carter wrote:
>> On 06/18/2015 09:56 AM, Stephen Smalley wrote:
>>> On 06/17/2015 03:58 PM, James Carter wrote:
>>>> The largest change to the user and role bounds checking was to put
>>>> them in their own functions, so they could be called independently.
>>>>
>>>> The type bounds checking was changed to check one type bounds at
>>>> a time. An expanded avtab is still created, but now only the rules
>>>> of the parent type are expanded. If violations are discovered,
>>>> a list of avtab_ptr_t's provides details. This list is used to
>>>> display error messages for backwards compatibility and will be
>>>> used by CIL to provide a more detailed error message.
>>>>
>>>> Memory usage is reduced from 9,355M to 126M and time is reduced
>>>> from 9 sec to 2 sec.
>>>>
>>>> Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
>>>
>>> Can we optimize the case where there are no bounded users/roles/types at
>>> all in the policy, and quickly return in that situation?  Seems like we
>>> could just quickly walk them and check to see if any are bounded before
>>> we start doing anything else.  Surprised we don't already do that.
>>>
>>
>> I am not sure how to do it faster.
>>
>> I am walking the types table [The statement:
>> hashtab_map(p->p_types.table, bounds_check_type_callback, &args);] and
>> only calling bounds_check_type() if the type has a bounds.
>>
>> Is there a faster way?
>
> So is there any avtab allocation if there are no bounded types?
> That's what I wanted to ensure we avoid or at least minimize.
>

There is no avtab allocation unless there is a bounded type. And when there are, 
it is only rules that involve the parent that are expanded.

> I was surprised that you indicated that we have significant memory and
> time usage from the old hierarchy checker since there are no bounded
> types in the default policy; I had assumed that it would optimize for
> that case, just as we quickly bail out of the assertion checker if there
> are no neverallows before even doing an avtab_init.
>

I was surprised as well, but that is the case.


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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2015-06-18 20:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 19:58 [PATCH 00/10 v2] Improve libsepol and CIL neverallow and bounds checking James Carter
2015-06-17 19:58 ` [PATCH 01/10 v2] libsepol: Add new ebitmap function named ebitmap_match_any() James Carter
2015-06-18 13:23   ` Stephen Smalley
2015-06-17 19:58 ` [PATCH 02/10 v2] libsepol: Treat types like an attribute in the attr_type_map James Carter
2015-06-18 13:41   ` Stephen Smalley
2015-06-18 13:52     ` Stephen Smalley
2015-06-18 20:16     ` James Carter
2015-06-18 20:21       ` Stephen Smalley
2015-06-18 20:23         ` Stephen Smalley
2015-06-17 19:58 ` [PATCH 03/10 v2] libsepol: Refactored neverallow checking James Carter
2015-06-17 19:58 ` [PATCH 04/10 v2] libsepol: Refactored bounds (hierarchy) checking code James Carter
2015-06-18 13:56   ` Stephen Smalley
2015-06-18 20:26     ` James Carter
2015-06-18 20:29       ` Stephen Smalley
2015-06-18 20:35         ` James Carter
2015-06-17 19:58 ` [PATCH 05/10 v2] libsepol/cil: Add function to search the CIL AST for an AV rule James Carter
2015-06-17 19:58 ` [PATCH 06/10 v2] libsepol/cil: Refactored CIL neverallow checking and reporting James Carter
2015-06-17 19:58 ` [PATCH 07/10 v2] libsepol/cil: Track number of classes and number of types and attributes James Carter
2015-06-17 19:58 ` [PATCH 08/10 v2] libsepol/cil: Add CIL bounds checking and reporting James Carter
2015-06-17 19:58 ` [PATCH 09/10 v2] secilc: Add a CIL policy file to test neverallow checking James Carter
2015-06-17 19:58 ` [PATCH 10/10 v2] secilc: Add a CIL policy file to test bounds checking James Carter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.