[PATCH-v3] libsepol: allow genfscon statements in modules

[PATCH-v3] libsepol: allow genfscon statements in modules

[Eric Paris]

This patch provides the libsepol support for the usage of genfscon
statements in policy modules.  The module must declare/require all of
the components of the context associated with the declaration but the
actual validation of that context is delayed until link time.

Comments and criticism appreciated.  (note that this patch may require
the recent bug fix from sds for mls_level_convert())

Signed-off-by: Eric Paris <eparis@redhat.com>
---

Example module with genfscon statements:

require {
	user user_u;
	role role_r;
	type type2_t;
	type type1_t;
	sensitivity s0;
}

genfscon proc /paris0000	user_u:role_r:type1_t:s0
genfscon proc /paris4444	user_u:role_r:type2_t:s0
genfscon proc /paris00000	user_u:role_r:type1_t:s0
genfscon proc /			user_u:role_r:type2_t:s0

 src/link.c     |  244 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/policydb.c |   30 +++----
 2 files changed, 259 insertions(+), 15 deletions(-)

diff -Naupr libsepol-2.0.26.orig/src/link.c libsepol-2.0.26/src/link.c
--- libsepol-2.0.26.orig/src/link.c	2008-03-27 13:20:03.000000000 -0400
+++ libsepol-2.0.26/src/link.c	2008-05-07 11:00:42.000000000 -0400
@@ -33,6 +33,7 @@
 #include <string.h>
 #include <assert.h>
 
+#include "context.h"
 #include "debug.h"
 
 #undef min
@@ -2120,6 +2125,234 @@ static int prepare_base(link_state_t * s
 	return 0;
 }
 
+/*
+ * We are working under the assumption that these types now exist in base
+ * at the very least it should have been taken care of by requires blocks
+ * in the policy module or previously been copied into base by the previous
+ * link time operations (so don't call this too early in the linking process
+ */
+static int context_copy_and_validate(link_state_t *state, context_struct_t *dst, context_struct_t *src)
+{
+	user_datum_t *base_user_datum;
+	role_datum_t *base_role_datum;
+	type_datum_t *base_type_datum;
+	char *key;
+	int ret;
+
+	key = state->cur->policy->p_user_val_to_name[src->user - 1];
+	base_user_datum = hashtab_search(state->base->p_users.table, key);
+	assert(base_user_datum);
+	dst->user = base_user_datum->s.value;
+
+	key = state->cur->policy->p_role_val_to_name[src->role - 1];
+	base_role_datum = hashtab_search(state->base->p_roles.table, key);
+	assert(base_role_datum);
+	dst->role = base_role_datum->s.value;
+
+	key = state->cur->policy->p_type_val_to_name[src->type - 1];
+	base_type_datum = hashtab_search(state->base->p_types.table, key);
+	assert(base_type_datum);
+	dst->type = base_type_datum->s.value;
+
+	ret = mls_context_cpy(dst, src);
+	if (ret)
+		return ret;
+
+	return context_is_valid(state->base, dst);
+}
+
+static void free_genfs(genfs_t *genfs)
+{
+	ocontext_t *con;
+	while (genfs) {
+		con = genfs->head;
+		while (con) {
+			free(con->u.name);
+			con->u.name = NULL;
+			free(con);
+			con = con->next;
+		}
+		genfs->head = NULL;
+		free(genfs->fstype);
+		genfs->fstype = NULL;
+		genfs = genfs->next;
+	}
+}
+
+/* merge 2 genfs sets with the same fstype, we also should free everything we
+ * can if it goes badly */
+static int genfs_merge(link_state_t *state, genfs_t *genfs, genfs_t *base_genfs)
+{
+	ocontext_t *con, *next_con, *base_con, *prev;
+	int len, base_len;
+
+	assert(!strcmp(genfs->fstype, base_genfs->fstype));
+
+	con = genfs->head;
+
+	/*
+	 * this loop is a bit odd formed because I have a list of items in genfs
+	 * that I am trying to merge into the list of items in base_genfs.  So
+	 * we have to grab next_con before we start inserting the object into
+	 * the other list
+	 */
+	while (con) {
+		next_con = con->next;
+		base_con = base_genfs->head;
+
+		/* insert at head of list because list is blank */
+		if (!base_con) {
+			base_genfs->head = con;
+			con = next_con;
+			continue;
+		}
+
+		/* test against the first entry in the list */
+		len = strlen(con->u.name);
+		base_len = strlen(base_con->u.name);
+		/* longer name goes at the head */
+		if (len > base_len) {
+			con->next = base_con;
+			base_genfs->head = con;
+			con = next_con;
+			continue;
+		}
+
+		/*
+		 * walk the base_con list until we need to insert between
+		 * base_con and base_con->next
+		 */
+		while (base_con->next) {
+			base_len = strlen(base_con->next->u.name);
+			if (len > base_len)
+				break;
+			base_con = base_con->next;
+			if (len < base_len)
+				continue;
+			/* (len == base_len) */
+			if (!strcmp(con->u.name, base_con->u.name) &&
+			    ((!con->v.sclass || !base_con->v.sclass || con->v.sclass == base_con->v.sclass))) {
+				ERR(state->handle, "dup genfs entry (%s,%s)", genfs->fstype, con->u.name);
+				/*
+				 * everything earlier in the list is now associated with
+				 * the base genfs, so we need to leave all of it alone and
+				 * let it get freed naturaly when we tear down the base.
+				 */
+				while (con) {
+					free(con->u.name);
+					prev = con;
+					con = con->next;
+					free(prev);
+				}
+				free(genfs->fstype);
+				free(genfs);
+				return -EINVAL;
+			}
+		}
+
+		if (!base_con->next || (len > base_len)) {
+			con->next = base_con->next;
+			base_con->next = con;
+			con = next_con;
+			continue;
+		} else {
+			assert(0);
+		}
+	}
+	/* free these since we are using the genfs and fstype already in the base */
+	free(genfs->fstype);
+	free(genfs);
+	return 0;
+}
+
+/* so this is going to merge genfs info from modules into the base policy */
+static int genfs_context_copy_helper(link_state_t *state)
+{
+	ocontext_t *c, *newc, *l;
+	genfs_t *genfs, *newgenfs, *base_genfs;
+	int cmp, ret;
+
+	for (genfs = state->cur->policy->genfs; genfs; genfs = genfs->next) {
+		/* build a newgenfs, we might free peices and parts
+		 * later if the base already had them */
+		newgenfs = calloc(1, sizeof(genfs_t));
+		if (!newgenfs) {
+			ERR(state->handle, "Out of memory!");
+			return -ENOMEM;
+		}
+		newgenfs->fstype = strdup(genfs->fstype);
+		if (!newgenfs->fstype) {
+			free_genfs(newgenfs);
+			ERR(state->handle, "Out of memory!");
+			return -ENOMEM;
+		}
+
+		l = NULL;
+		for (c = genfs->head; c; c = c->next) {
+			newc = calloc(1, sizeof(ocontext_t));
+			if (!newc) {
+				free_genfs(newgenfs);
+				ERR(state->handle, "Out of memory!");
+				return -ENOMEM;
+			}
+			newc->u.name = strdup(c->u.name);
+			if (!newc->u.name) {
+				free_genfs(newgenfs);
+				ERR(state->handle, "Out of memory!");
+				return -ENOMEM;
+			}
+			newc->v.sclass = c->v.sclass;
+			context_copy_and_validate(state, &newc->context[0], &c->context[0]);
+
+			if (l)
+				l->next = newc;
+			else
+				newgenfs->head = newc;
+			l = newc;
+		}
+
+		base_genfs = state->base->genfs;
+		/* base had no genfs, insert at front */
+		if (!base_genfs) {
+			state->base->genfs = newgenfs;
+			continue;
+		}
+		/* first base entry is after this entry, insert at front */
+		cmp = strcmp(newgenfs->fstype, base_genfs->fstype);
+		if (cmp < 0) {
+			newgenfs->next = base_genfs;
+			state->base->genfs = newgenfs;
+			continue;
+		} else if (cmp == 0) {
+			/* merge with head of list */
+			ret = genfs_merge(state, newgenfs, base_genfs);
+			if (ret)
+				return ret;
+			continue;
+		}
+		/* move down the list until we either need to be inserted between
+		 * base_genfs and base_genfs->next or we need to merge with
+		 * base_genfs->next */
+		while (base_genfs->next && (strcmp(newgenfs->fstype, base_genfs->next->fstype) > 0))
+			base_genfs = base_genfs->next;
+		/* insert between if appropiete */
+		if (!base_genfs->next || (strcmp(newgenfs->fstype, base_genfs->next->fstype) < 0)) {
+			newgenfs->next = base_genfs->next;
+			base_genfs->next = newgenfs;
+			continue;
+		}
+		/* this is the fun part, merge..... */
+		if (strcmp(newgenfs->fstype, base_genfs->next->fstype) == 0) {
+			ret = genfs_merge(state, newgenfs, base_genfs->next);
+			if (ret)
+				return ret;
+		} else {
+			assert(0);
+		}
+	}
+	return 0;
+}
+
 /* Link a set of modules into a base module. This process is somewhat
  * similar to an actual compiler: it requires a set of order dependent
  * steps.  The base and every module must have been indexed prior to
@@ -2202,8 +2435,19 @@ int link_modules(sepol_handle_t * handle
 	for (i = 0; i < len; i++) {
 		state.cur = modules[i];
 		state.cur_mod_name = modules[i]->policy->name;
-		ret =
-		    copy_identifiers(&state, modules[i]->policy->symtab, NULL);
+		ret = copy_identifiers(&state, modules[i]->policy->symtab, NULL);
+		if (ret) {
+			retval = ret;
+			goto cleanup;
+		}
+	}
+
+	/* copy genfs_context stuff into base */
+	for (i = 0; i < len; i++) {
+		state.cur = modules[i];
+		state.cur_mod_name = modules[i]->policy->name;
+
+		ret = genfs_context_copy_helper(&state);
 		if (ret) {
 			retval = ret;
 			goto cleanup;
diff -Naupr libsepol-2.0.26.orig/src/policydb.c libsepol-2.0.26/src/policydb.c
--- libsepol-2.0.26.orig/src/policydb.c	2008-03-27 13:20:03.000000000 -0400
+++ libsepol-2.0.26/src/policydb.c	2008-05-07 10:53:52.000000000 -0400
@@ -1563,8 +1563,9 @@ static int mls_range_to_semantic(mls_ran
  * Read and validate a security context structure
  * from a policydb binary representation file.
  */
-static int context_read_and_validate(context_struct_t * c,
-				     policydb_t * p, struct policy_file *fp)
+static int context_read_and_validate(context_struct_t *c,
+				     policydb_t *p, struct policy_file *fp,
+				     uint32_t validate)
 {
 	uint32_t buf[3];
 	int rc;
@@ -1588,7 +1589,7 @@ static int context_read_and_validate(con
 		}
 	}
 
-	if (!policydb_context_isvalid(p, c)) {
+	if (validate && !policydb_context_isvalid(p, c)) {
 		ERR(fp->handle, "invalid security context");
 		context_destroy(c);
 		return -1;
@@ -2076,7 +2077,7 @@ static int ocontext_read(struct policydb
 					return -1;
 				c->sid[0] = le32_to_cpu(buf[0]);
 				if (context_read_and_validate
-				    (&c->context[0], p, fp))
+				    (&c->context[0], p, fp, 1))
 					return -1;
 				break;
 			case OCON_FS:
@@ -2093,10 +2094,10 @@ static int ocontext_read(struct policydb
 					return -1;
 				c->u.name[len] = 0;
 				if (context_read_and_validate
-				    (&c->context[0], p, fp))
+				    (&c->context[0], p, fp, 1))
 					return -1;
 				if (context_read_and_validate
-				    (&c->context[1], p, fp))
+				    (&c->context[1], p, fp, 1))
 					return -1;
 				break;
 			case OCON_PORT:
@@ -2107,7 +2108,7 @@ static int ocontext_read(struct policydb
 				c->u.port.low_port = le32_to_cpu(buf[1]);
 				c->u.port.high_port = le32_to_cpu(buf[2]);
 				if (context_read_and_validate
-				    (&c->context[0], p, fp))
+				    (&c->context[0], p, fp, 1))
 					return -1;
 				break;
 			case OCON_NODE:
@@ -2117,7 +2118,7 @@ static int ocontext_read(struct policydb
 				c->u.node.addr = le32_to_cpu(buf[0]);
 				c->u.node.mask = le32_to_cpu(buf[1]);
 				if (context_read_and_validate
-				    (&c->context[0], p, fp))
+				    (&c->context[0], p, fp, 1))
 					return -1;
 				break;
 			case OCON_FSUSE:
@@ -2134,7 +2135,7 @@ static int ocontext_read(struct policydb
 					return -1;
 				c->u.name[len] = 0;
 				if (context_read_and_validate
-				    (&c->context[0], p, fp))
+				    (&c->context[0], p, fp, 1))
 					return -1;
 				break;
 			case OCON_NODE6:{
@@ -2151,7 +2152,7 @@ static int ocontext_read(struct policydb
 						c->u.node6.mask[k] =
 						    le32_to_cpu(buf[k + 4]);
 					if (context_read_and_validate
-					    (&c->context[0], p, fp))
+					    (&c->context[0], p, fp, 1))
 						return -1;
 					break;
 				}
@@ -2173,6 +2174,10 @@ static int genfs_read(policydb_t * p, st
 	ocontext_t *l, *c, *newc = NULL;
 	int rc;
 
+	/* don't validate full contexts in modules, will do that at link
+	 * time later */
+	int validate = (p->policy_type != POLICY_MOD);
+
 	rc = next_entry(buf, fp, sizeof(uint32_t));
 	if (rc < 0)
 		goto bad;
@@ -2240,10 +2245,9 @@ static int genfs_read(policydb_t * p, st
 			if (rc < 0)
 				goto bad;
 			newc->v.sclass = le32_to_cpu(buf[0]);
-			if (context_read_and_validate(&newc->context[0], p, fp))
+			if (context_read_and_validate(&newc->context[0], p, fp, validate))
 				goto bad;
-			for (l = NULL, c = newgenfs->head; c;
-			     l = c, c = c->next) {
+			for (l = NULL, c = newgenfs->head; c; l = c, c = c->next) {
 				if (!strcmp(newc->u.name, c->u.name) &&
 				    (!c->v.sclass || !newc->v.sclass ||
 				     newc->v.sclass == c->v.sclass)) {



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH-v3] libsepol: allow genfscon statements in modules

[Joshua Brindle]

Eric Paris wrote:
> This patch provides the libsepol support for the usage of genfscon
> statements in policy modules.  The module must declare/require all of
> the components of the context associated with the declaration but the
> actual validation of that context is delayed until link time.
>
> Comments and criticism appreciated.  (note that this patch may require
> the recent bug fix from sds for mls_level_convert())
>
>   

This work is appreciated, though I wonder if it is enough to just have 
genfscon in modules or if they should be part of the optional block 
grammar (and this part of the avrule_block struct, etc). This, of 
course, would require a module version bump. Other comments below.

> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
>
> Example module with genfscon statements:
>
> require {
> 	user user_u;
> 	role role_r;
> 	type type2_t;
> 	type type1_t;
> 	sensitivity s0;
> }
>
> genfscon proc /paris0000	user_u:role_r:type1_t:s0
> genfscon proc /paris4444	user_u:role_r:type2_t:s0
> genfscon proc /paris00000	user_u:role_r:type1_t:s0
> genfscon proc /			user_u:role_r:type2_t:s0
>
>  src/link.c     |  244 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/policydb.c |   30 +++----
>  2 files changed, 259 insertions(+), 15 deletions(-)
>
> diff -Naupr libsepol-2.0.26.orig/src/link.c libsepol-2.0.26/src/link.c
> --- libsepol-2.0.26.orig/src/link.c	2008-03-27 13:20:03.000000000 -0400
> +++ libsepol-2.0.26/src/link.c	2008-05-07 11:00:42.000000000 -0400
> @@ -33,6 +33,7 @@
>  #include <string.h>
>  #include <assert.h>
>  
> +#include "context.h"
>  #include "debug.h"
>  
>  #undef min
> @@ -2120,6 +2125,234 @@ static int prepare_base(link_state_t * s
>  	return 0;
>  }
>  
> +/*
> + * We are working under the assumption that these types now exist in base
> + * at the very least it should have been taken care of by requires blocks
> + * in the policy module or previously been copied into base by the previous
> + * link time operations (so don't call this too early in the linking process
> + */
> +static int context_copy_and_validate(link_state_t *state, context_struct_t *dst, context_struct_t *src)
> +{
> +	user_datum_t *base_user_datum;
> +	role_datum_t *base_role_datum;
> +	type_datum_t *base_type_datum;
> +	char *key;
> +	int ret;
> +
> +	key = state->cur->policy->p_user_val_to_name[src->user - 1];
> +	base_user_datum = hashtab_search(state->base->p_users.table, key);
> +	assert(base_user_datum);
> +	dst->user = base_user_datum->s.value;
> +
> +	key = state->cur->policy->p_role_val_to_name[src->role - 1];
> +	base_role_datum = hashtab_search(state->base->p_roles.table, key);
> +	assert(base_role_datum);
> +	dst->role = base_role_datum->s.value;
> +
> +	key = state->cur->policy->p_type_val_to_name[src->type - 1];
> +	base_type_datum = hashtab_search(state->base->p_types.table, key);
> +	assert(base_type_datum);
> +	dst->type = base_type_datum->s.value;
> +
> +	ret = mls_context_cpy(dst, src);
> +	if (ret)
> +		return ret;
> +
> +	return context_is_valid(state->base, dst);
> +}
> +
> +static void free_genfs(genfs_t *genfs)
>   

I don't think this should go in link.c

> +{
> +	ocontext_t *con;
> +	while (genfs) {
> +		con = genfs->head;
> +		while (con) {
> +			free(con->u.name);
> +			con->u.name = NULL;
> +			free(con);
> +			con = con->next;
> +		}
> +		genfs->head = NULL;
> +		free(genfs->fstype);
> +		genfs->fstype = NULL;
> +		genfs = genfs->next;
> +	}
> +}
> +
> +/* merge 2 genfs sets with the same fstype, we also should free everything we
> + * can if it goes badly */
> +static int genfs_merge(link_state_t *state, genfs_t *genfs, genfs_t *base_genfs)
> +{
> +	ocontext_t *con, *next_con, *base_con, *prev;
> +	int len, base_len;
> +
> +	assert(!strcmp(genfs->fstype, base_genfs->fstype));
> +
> +	con = genfs->head;
> +
> +	/*
> +	 * this loop is a bit odd formed because I have a list of items in genfs
> +	 * that I am trying to merge into the list of items in base_genfs.  So
> +	 * we have to grab next_con before we start inserting the object into
> +	 * the other list
> +	 */
> +	while (con) {
> +		next_con = con->next;
> +		base_con = base_genfs->head;
> +
> +		/* insert at head of list because list is blank */
> +		if (!base_con) {
> +			base_genfs->head = con;
> +			con = next_con;
> +			continue;
> +		}
> +
> +		/* test against the first entry in the list */
> +		len = strlen(con->u.name);
> +		base_len = strlen(base_con->u.name);
> +		/* longer name goes at the head */
> +		if (len > base_len) {
> +			con->next = base_con;
> +			base_genfs->head = con;
> +			con = next_con;
> +			continue;
> +		}
> +
> +		/*
> +		 * walk the base_con list until we need to insert between
> +		 * base_con and base_con->next
> +		 */
> +		while (base_con->next) {
> +			base_len = strlen(base_con->next->u.name);
> +			if (len > base_len)
> +				break;
> +			base_con = base_con->next;
> +			if (len < base_len)
> +				continue;
> +			/* (len == base_len) */
> +			if (!strcmp(con->u.name, base_con->u.name) &&
> +			    ((!con->v.sclass || !base_con->v.sclass || con->v.sclass == base_con->v.sclass))) {
>   

I don't get this. Shouldn't it be ((!con->v.sclass && 
!base_con->v.sclass)) ... or am I missing something?

> +				ERR(state->handle, "dup genfs entry (%s,%s)", genfs->fstype, con->u.name);
> +				/*
> +				 * everything earlier in the list is now associated with
> +				 * the base genfs, so we need to leave all of it alone and
> +				 * let it get freed naturaly when we tear down the base.
> +				 */
> +				while (con) {
> +					free(con->u.name);
> +					prev = con;
> +					con = con->next;
> +					free(prev);
> +				}
> +				free(genfs->fstype);
> +				free(genfs);
> +				return -EINVAL;
> +			}
> +		}
> +
> +		if (!base_con->next || (len > base_len)) {
> +			con->next = base_con->next;
> +			base_con->next = con;
> +			con = next_con;
> +			continue;
> +		} else {
> +			assert(0);
> +		}
> +	}
> +	/* free these since we are using the genfs and fstype already in the base */
> +	free(genfs->fstype);
> +	free(genfs);
> +	return 0;
> +}
> +
> +/* so this is going to merge genfs info from modules into the base policy */
> +static int genfs_context_copy_helper(link_state_t *state)
> +{
> +	ocontext_t *c, *newc, *l;
> +	genfs_t *genfs, *newgenfs, *base_genfs;
> +	int cmp, ret;
> +
> +	for (genfs = state->cur->policy->genfs; genfs; genfs = genfs->next) {
> +		/* build a newgenfs, we might free peices and parts
> +		 * later if the base already had them */
> +		newgenfs = calloc(1, sizeof(genfs_t));
>   

initializer preferred over calloc()

> +		if (!newgenfs) {
> +			ERR(state->handle, "Out of memory!");
> +			return -ENOMEM;
> +		}
> +		newgenfs->fstype = strdup(genfs->fstype);
> +		if (!newgenfs->fstype) {
> +			free_genfs(newgenfs);
> +			ERR(state->handle, "Out of memory!");
> +			return -ENOMEM;
> +		}
> +
> +		l = NULL;
> +		for (c = genfs->head; c; c = c->next) {
> +			newc = calloc(1, sizeof(ocontext_t));
>   

ditto

> +			if (!newc) {
> +				free_genfs(newgenfs);
> +				ERR(state->handle, "Out of memory!");
> +				return -ENOMEM;
> +			}
> +			newc->u.name = strdup(c->u.name);
> +			if (!newc->u.name) {
> +				free_genfs(newgenfs);
> +				ERR(state->handle, "Out of memory!");
> +				return -ENOMEM;
> +			}
> +			newc->v.sclass = c->v.sclass;
> +			context_copy_and_validate(state, &newc->context[0], &c->context[0]);
> +
> +			if (l)
> +				l->next = newc;
> +			else
> +				newgenfs->head = newc;
> +			l = newc;
> +		}
> +
> +		base_genfs = state->base->genfs;
> +		/* base had no genfs, insert at front */
> +		if (!base_genfs) {
> +			state->base->genfs = newgenfs;
> +			continue;
> +		}
> +		/* first base entry is after this entry, insert at front */
> +		cmp = strcmp(newgenfs->fstype, base_genfs->fstype);
> +		if (cmp < 0) {
> +			newgenfs->next = base_genfs;
> +			state->base->genfs = newgenfs;
> +			continue;
> +		} else if (cmp == 0) {
> +			/* merge with head of list */
> +			ret = genfs_merge(state, newgenfs, base_genfs);
> +			if (ret)
> +				return ret;
> +			continue;
> +		}
> +		/* move down the list until we either need to be inserted between
> +		 * base_genfs and base_genfs->next or we need to merge with
> +		 * base_genfs->next */
> +		while (base_genfs->next && (strcmp(newgenfs->fstype, base_genfs->next->fstype) > 0))
> +			base_genfs = base_genfs->next;
> +		/* insert between if appropiete */
> +		if (!base_genfs->next || (strcmp(newgenfs->fstype, base_genfs->next->fstype) < 0)) {
> +			newgenfs->next = base_genfs->next;
> +			base_genfs->next = newgenfs;
> +			continue;
> +		}
> +		/* this is the fun part, merge..... */
> +		if (strcmp(newgenfs->fstype, base_genfs->next->fstype) == 0) {
> +			ret = genfs_merge(state, newgenfs, base_genfs->next);
> +			if (ret)
> +				return ret;
> +		} else {
> +			assert(0);
> +		}
> +	}
> +	return 0;
> +}
> +
>  /* Link a set of modules into a base module. This process is somewhat
>   * similar to an actual compiler: it requires a set of order dependent
>   * steps.  The base and every module must have been indexed prior to
> @@ -2202,8 +2435,19 @@ int link_modules(sepol_handle_t * handle
>  	for (i = 0; i < len; i++) {
>  		state.cur = modules[i];
>  		state.cur_mod_name = modules[i]->policy->name;
> -		ret =
> -		    copy_identifiers(&state, modules[i]->policy->symtab, NULL);
> +		ret = copy_identifiers(&state, modules[i]->policy->symtab, NULL);
> +		if (ret) {
> +			retval = ret;
> +			goto cleanup;
> +		}
> +	}
> +
> +	/* copy genfs_context stuff into base */
> +	for (i = 0; i < len; i++) {
> +		state.cur = modules[i];
> +		state.cur_mod_name = modules[i]->policy->name;
> +
> +		ret = genfs_context_copy_helper(&state);
>   

Why isn't this part of copy_module?

>  		if (ret) {
>  			retval = ret;
>  			goto cleanup;
> diff -Naupr libsepol-2.0.26.orig/src/policydb.c libsepol-2.0.26/src/policydb.c
> --- libsepol-2.0.26.orig/src/policydb.c	2008-03-27 13:20:03.000000000 -0400
> +++ libsepol-2.0.26/src/policydb.c	2008-05-07 10:53:52.000000000 -0400
> @@ -1563,8 +1563,9 @@ static int mls_range_to_semantic(mls_ran
>   * Read and validate a security context structure
>   * from a policydb binary representation file.
>   */
> -static int context_read_and_validate(context_struct_t * c,
> -				     policydb_t * p, struct policy_file *fp)
> +static int context_read_and_validate(context_struct_t *c,
> +				     policydb_t *p, struct policy_file *fp,
> +				     uint32_t validate)
>  {
>  	uint32_t buf[3];
>  	int rc;
> @@ -1588,7 +1589,7 @@ static int context_read_and_validate(con
>  		}
>  	}
>  
> -	if (!policydb_context_isvalid(p, c)) {
> +	if (validate && !policydb_context_isvalid(p, c)) {
>  		ERR(fp->handle, "invalid security context");
>  		context_destroy(c);
>  		return -1;
> @@ -2076,7 +2077,7 @@ static int ocontext_read(struct policydb
>  					return -1;
>  				c->sid[0] = le32_to_cpu(buf[0]);
>  				if (context_read_and_validate
> -				    (&c->context[0], p, fp))
> +				    (&c->context[0], p, fp, 1))
>  					return -1;
>  				break;
>  			case OCON_FS:
> @@ -2093,10 +2094,10 @@ static int ocontext_read(struct policydb
>  					return -1;
>  				c->u.name[len] = 0;
>  				if (context_read_and_validate
> -				    (&c->context[0], p, fp))
> +				    (&c->context[0], p, fp, 1))
>  					return -1;
>  				if (context_read_and_validate
> -				    (&c->context[1], p, fp))
> +				    (&c->context[1], p, fp, 1))
>  					return -1;
>  				break;
>  			case OCON_PORT:
> @@ -2107,7 +2108,7 @@ static int ocontext_read(struct policydb
>  				c->u.port.low_port = le32_to_cpu(buf[1]);
>  				c->u.port.high_port = le32_to_cpu(buf[2]);
>  				if (context_read_and_validate
> -				    (&c->context[0], p, fp))
> +				    (&c->context[0], p, fp, 1))
>  					return -1;
>  				break;
>  			case OCON_NODE:
> @@ -2117,7 +2118,7 @@ static int ocontext_read(struct policydb
>  				c->u.node.addr = le32_to_cpu(buf[0]);
>  				c->u.node.mask = le32_to_cpu(buf[1]);
>  				if (context_read_and_validate
> -				    (&c->context[0], p, fp))
> +				    (&c->context[0], p, fp, 1))
>  					return -1;
>  				break;
>  			case OCON_FSUSE:
> @@ -2134,7 +2135,7 @@ static int ocontext_read(struct policydb
>  					return -1;
>  				c->u.name[len] = 0;
>  				if (context_read_and_validate
> -				    (&c->context[0], p, fp))
> +				    (&c->context[0], p, fp, 1))
>  					return -1;
>  				break;
>  			case OCON_NODE6:{
> @@ -2151,7 +2152,7 @@ static int ocontext_read(struct policydb
>  						c->u.node6.mask[k] =
>  						    le32_to_cpu(buf[k + 4]);
>  					if (context_read_and_validate
> -					    (&c->context[0], p, fp))
> +					    (&c->context[0], p, fp, 1))
>  						return -1;
>  					break;
>  				}
> @@ -2173,6 +2174,10 @@ static int genfs_read(policydb_t * p, st
>  	ocontext_t *l, *c, *newc = NULL;
>  	int rc;
>  
> +	/* don't validate full contexts in modules, will do that at link
> +	 * time later */
> +	int validate = (p->policy_type != POLICY_MOD);
> +
>  	rc = next_entry(buf, fp, sizeof(uint32_t));
>  	if (rc < 0)
>  		goto bad;
> @@ -2240,10 +2245,9 @@ static int genfs_read(policydb_t * p, st
>  			if (rc < 0)
>  				goto bad;
>  			newc->v.sclass = le32_to_cpu(buf[0]);
> -			if (context_read_and_validate(&newc->context[0], p, fp))
> +			if (context_read_and_validate(&newc->context[0], p, fp, validate))
>  				goto bad;
> -			for (l = NULL, c = newgenfs->head; c;
> -			     l = c, c = c->next) {
> +			for (l = NULL, c = newgenfs->head; c; l = c, c = c->next) {
>  				if (!strcmp(newc->u.name, c->u.name) &&
>  				    (!c->v.sclass || !newc->v.sclass ||
>  				     newc->v.sclass == c->v.sclass)) {
>
>
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>
>   



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH-v3] libsepol: allow genfscon statements in modules

[Eric Paris]

On Tue, 2008-05-13 at 16:06 -0400, Joshua Brindle wrote:
> > +static void free_genfs(genfs_t *genfs)
> >   
> 
> I don't think this should go in link.c

meaning what?  you want me to static inline it from a header file?
Doesn't matter to me...
> 
> > +{
> > +	ocontext_t *con;
> > +	while (genfs) {
> > +		con = genfs->head;
> > +		while (con) {
> > +			free(con->u.name);
> > +			con->u.name = NULL;
> > +			free(con);
> > +			con = con->next;
> > +		}
> > +		genfs->head = NULL;
> > +		free(genfs->fstype);
> > +		genfs->fstype = NULL;
> > +		genfs = genfs->next;
> > +	}
> > +}

[snip]

> > +		while (base_con->next) {
> > +			base_len = strlen(base_con->next->u.name);
> > +			if (len > base_len)
> > +				break;
> > +			base_con = base_con->next;
> > +			if (len < base_len)
> > +				continue;
> > +			/* (len == base_len) */
> > +			if (!strcmp(con->u.name, base_con->u.name) &&
> > +			    ((!con->v.sclass || !base_con->v.sclass || con->v.sclass == base_con->v.sclass))) {
> >   
> 
> I don't get this. Shouldn't it be ((!con->v.sclass && 
> !base_con->v.sclass)) ... or am I missing something?

I could make this more complex, but what I have is correct.

If con->v.sclass is 0 that means this rule is supposed to cover all file
types thus any rule already in base must overlap.  The converse is also
true.  If the value in the base is 0 that means that this rule from the
incoming module must already be covered by the rule in base.

The last part says, if both the rule in the module and the rule in the
base are non-zero we have an overlap if they are for the same file type.

> > +		newgenfs = calloc(1, sizeof(genfs_t));
> >   
> 
> initializer preferred over calloc()

grummbles about getting memset backwards and calloc always being right.


> > +		if (!newgenfs) {
> > +			ERR(state->handle, "Out of memory!");
> > +			return -ENOMEM;
> > +		}
> > +		newgenfs->fstype = strdup(genfs->fstype);
> > +		if (!newgenfs->fstype) {
> > +			free_genfs(newgenfs);
> > +			ERR(state->handle, "Out of memory!");
> > +			return -ENOMEM;
> > +		}
> > +
> > +		l = NULL;
> > +		for (c = genfs->head; c; c = c->next) {
> > +			newc = calloc(1, sizeof(ocontext_t));
> >   
> 
> ditto

grummble grubble, fine.

> > @@ -2202,8 +2435,19 @@ int link_modules(sepol_handle_t * handle
> >  	for (i = 0; i < len; i++) {
> >  		state.cur = modules[i];
> >  		state.cur_mod_name = modules[i]->policy->name;
> > -		ret =
> > -		    copy_identifiers(&state, modules[i]->policy->symtab, NULL);
> > +		ret = copy_identifiers(&state, modules[i]->policy->symtab, NULL);
> > +		if (ret) {
> > +			retval = ret;
> > +			goto cleanup;
> > +		}
> > +	}
> > +
> > +	/* copy genfs_context stuff into base */
> > +	for (i = 0; i < len; i++) {
> > +		state.cur = modules[i];
> > +		state.cur_mod_name = modules[i]->policy->name;
> > +
> > +		ret = genfs_context_copy_helper(&state);
> >   
> 
> Why isn't this part of copy_module?

No good reason.  I can put it there (and should have)


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH-v3] libsepol: allow genfscon statements in modules

[Joshua Brindle]

Eric Paris wrote:
> On Tue, 2008-05-13 at 16:06 -0400, Joshua Brindle wrote:
>   
>>> +static void free_genfs(genfs_t *genfs)
>>>   
>>>       
>> I don't think this should go in link.c
>>     
>
> meaning what?  you want me to static inline it from a header file?
> Doesn't matter to me...
>   

IMO put it in policydb.c and change policydb_destroy to use it as well.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH-v3] libsepol: allow genfscon statements in modules

[Eric Paris]

On Tue, 2008-05-13 at 16:06 -0400, Joshua Brindle wrote:
> Eric Paris wrote:
> > This patch provides the libsepol support for the usage of genfscon
> > statements in policy modules.  The module must declare/require all of
> > the components of the context associated with the declaration but the
> > actual validation of that context is delayed until link time.
> >
> > Comments and criticism appreciated.  (note that this patch may require
> > the recent bug fix from sds for mls_level_convert())

So I started to get back to this patch and realized it was pretty
seriously flawed.  I was not checking the validity of the context while
it was being linked into the base.  When I fixed to pay attention to the
return code of the return code of context_copy_and_validate() every
single context coming in from the module failed.  The reason being
because the MLS information is not getting written or read
(context_struct_t only reads/writes MLS info for monolithic or the base
module)

So, I've come to realize I need to start carrying around the
mls_semantic_range_t information with my genfs statements in the module
so that I can map those into the base and actually have/check MLS
validity.  I'm looking for any helpful suggestions or hints on how to do
this cleanly and things that people can think of off of the top of their
head of the gotchas when trying to carry around this MLS information.

So really if anyone has tips, tricks, pointers, gotchas, anything really
that might be interesting as I try to come up with a way for modules to
support full context strings let me know.

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.