[ libsepol 0/6] Context reorganization

[ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

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

The following patch moves functions relating to context_struct_t
into file contexts.c for better organization. It makes all of 
them take policydb as argument, and enforces the sepol_ prefix.
It uses the new debugging system. It adds a structure for
a high level representation of a context (sepol_ctxinfo_t), 
and a new function to convert from it (sepol_context_create),
and makes use of that function to make the conversion
from string (formerly sepol_context_to_sid) easier.

libsepol-1.1-context.diff

Disclaimer 1: 

The following bit, as taken from the current 
policydb_context_isvalid fails for me currently. I don't
understand the role of the cache here. Does genusers.c
need to be modified to do something with the cache,
when it loads the user? Do I need to correspondingly 
modify my own load_user function (which is based on genusers)?

+               if (!ebitmap_get_bit(&usrdatum->cache, c->role - 1))
+                       /* user may not be associated with role */
+                       return 0;

Disclaimer 2:

What purpose does the result_len serves in
to_string/from_string functions? Shouldn't all those
strings be null terminated? The former context_struct_to_string
was done this way, so I didn't change it, but I don't see
why this is necessary. Looking at mls_sid_to_context it
would appear that it is null terminated (uses strcpy).



[-- Attachment #2: libsepol-1.1-context.diff --]
[-- Type: text/x-patch, Size: 16681 bytes --]

diff -aru libsepol.work/include/sepol/context.h libsepol-1.1-context/include/sepol/context.h
--- libsepol.work/include/sepol/context.h	2005-07-18 15:05:31.000000000 -0400
+++ libsepol-1.1-context/include/sepol/context.h	2005-07-21 08:29:32.000000000 -0400
@@ -16,12 +16,24 @@
  * clients of the security server.
  */
 
-#ifndef _CONTEXT_H_
-#define _CONTEXT_H_
+#ifndef _SEPOL_CONTEXT_H_
+#define _SEPOL_CONTEXT_H_
 
 #include <sepol/ebitmap.h>
 #include <sepol/mls_types.h>
 
+/* 
+ * High level representation of the context 
+ * This structure provides sufficient information
+ * to construct a security context. 
+ */
+typedef struct sepol_ctxinfo {
+	const char* user;
+	const char* role;
+	const char* type;
+	const char* mls;
+} sepol_ctxinfo_t;
+
 /*
  * A security context consists of an authenticated user
  * identity, a role, a type and a MLS range.
@@ -33,7 +45,6 @@
 	mls_range_t range;
 } context_struct_t;
 
-
 static inline void mls_context_init(context_struct_t * c)
 {
 	memset(&c->range, 0, sizeof(c->range));
@@ -111,7 +122,34 @@
 		mls_context_cmp(c1, c2));
 }
 
-#endif	/* _CONTEXT_H_ */
+struct policydb;
+
+/* Create a context structure from high level representation */
+extern int sepol_context_create(
+	struct policydb *policydb,
+	context_struct_t** cptr,
+	sepol_ctxinfo_t* data);
+
+/* Create a context structure from string representation */
+extern int sepol_context_from_string(
+	struct policydb* policydb,
+	context_struct_t** cptr,
+	const char* con_str,
+	size_t con_str_len);
+
+/* Check if the provided context is valid for this policy */
+extern int sepol_context_is_valid(
+	struct policydb *policydb, 
+	context_struct_t *context);
+
+/* Extract the context as string */
+extern int sepol_context_to_string(
+	struct policydb *policydb,
+	context_struct_t * context,
+	char ** result,
+	size_t *result_len);
+
+#endif	/* _SEPOL_CONTEXT_H_ */
 
 /* FLASK */
 
diff -aru libsepol.work/include/sepol/policydb.h libsepol-1.1-context/include/sepol/policydb.h
--- libsepol.work/include/sepol/policydb.h	2005-07-18 15:05:31.000000000 -0400
+++ libsepol-1.1-context/include/sepol/policydb.h	2005-07-20 19:57:29.000000000 -0400
@@ -452,7 +452,11 @@
 
 extern int policydb_load_isids(policydb_t *p, sidtab_t *s);
 
-extern int policydb_context_isvalid(policydb_t *p, context_struct_t *c);
+/* Deprecated */
+static inline int policydb_context_isvalid(policydb_t *p, context_struct_t *c) {
+	return sepol_context_is_valid(p,c);
+}
+
 extern void symtabs_destroy(symtab_t *symtab);
 extern int scope_destroy(hashtab_key_t key, hashtab_datum_t datum, void *p);
 typedef void (*hashtab_destroy_func_t) (hashtab_key_t k, hashtab_datum_t d, void *args);
diff -aru libsepol.work/src/context.c libsepol-1.1-context/src/context.c
--- libsepol.work/src/context.c	2005-07-20 09:32:15.000000000 -0400
+++ libsepol-1.1-context/src/context.c	2005-07-21 12:19:09.000000000 -0400
@@ -0,0 +1,281 @@
+#include <stdlib.h>
+
+#include <sepol/policydb.h>
+#include <sepol/context.h>
+#include <sepol/mls.h>
+
+#include "debug.h"
+
+/*
+ * Return 1 if the fields in the security context
+ * structure `c' are valid.  Return 0 otherwise.
+ */
+int sepol_context_is_valid(policydb_t *p, context_struct_t *c)
+{
+	role_datum_t *role;
+	user_datum_t *usrdatum;
+	ebitmap_t types, roles;
+	int ret = 1;
+
+	ebitmap_init(&types);
+	ebitmap_init(&roles);
+	if (!c->role || c->role > p->p_roles.nprim)
+		return 0;
+
+	if (!c->user || c->user > p->p_users.nprim)
+		return 0;
+
+	if (!c->type || c->type > p->p_types.nprim)
+		return 0;
+
+	if (c->role != OBJECT_R_VAL) {
+		/*
+		 * Role must be authorized for the type.
+		 */
+		role = p->role_val_to_struct[c->role - 1];
+		if (!ebitmap_get_bit(&role->cache, c->type - 1))
+			/* role may not be associated with type */
+			return 0;
+
+		/*
+		 * User must be authorized for the role.
+		 */
+		usrdatum = p->user_val_to_struct[c->user - 1];
+		if (!usrdatum)
+			return 0;
+
+		if (!ebitmap_get_bit(&usrdatum->cache, c->role - 1)) 
+			/* user may not be associated with role */
+			return 0;
+	}
+
+	if (!mls_context_isvalid(p, c))
+		return 0;
+
+	return ret;
+}
+
+/*
+ * Write the security context string representation of
+ * the context structure `context' into a dynamically
+ * allocated string of the correct size.  Set `*scontext'
+ * to point to this string and set `*scontext_len' to
+ * the length of the string.
+ */
+int sepol_context_to_string(
+	policydb_t* policydb,
+	context_struct_t * context,
+	char **result,
+	size_t *result_len) {
+
+	char *scontext = NULL;
+	size_t scontext_len = 0;
+	char* ptr;
+
+	/* Compute the size of the context. */
+	scontext_len += strlen(policydb->p_user_val_to_name[context->user-1])+1;
+	scontext_len += strlen(policydb->p_role_val_to_name[context->role-1])+1;
+	scontext_len += strlen(policydb->p_type_val_to_name[context->type-1])+1;
+	scontext_len += mls_compute_context_len(policydb, context);
+
+	/* Allocate space for the context; caller must free this space. */
+	scontext = malloc(scontext_len);
+	if (!scontext) 
+		goto omem;
+
+	/*
+	 * Copy the user name, role name and type name into the context.
+	 */
+	ptr = scontext;
+	sprintf(ptr, "%s:%s:%s",
+		policydb->p_user_val_to_name[context->user - 1],
+		policydb->p_role_val_to_name[context->role - 1],
+		policydb->p_type_val_to_name[context->type - 1]);
+
+	ptr += 
+		strlen(policydb->p_user_val_to_name[context->user - 1]) + 1 + 
+		strlen(policydb->p_role_val_to_name[context->role - 1]) + 1 + 
+		strlen(policydb->p_type_val_to_name[context->type - 1]);
+
+	mls_sid_to_context(policydb, context, &ptr);
+
+	*result = scontext;
+	*result_len = scontext_len;
+	return STATUS_SUCCESS;
+
+	omem:
+	DEBUG(__FUNCTION__, "out of memory, could not convert "
+		"context to string\n");
+	free(scontext);
+	return STATUS_ERR;
+}
+
+
+/* Create a policy-dependent context structure, corresponding
+ * to the provided high level representation */
+
+int sepol_context_create(
+	policydb_t* policydb, 
+	context_struct_t** cptr, 
+	sepol_ctxinfo_t* data) {
+
+	context_struct_t* scontext = NULL;
+	user_datum_t* usrdatum;
+	role_datum_t* roldatum;
+	type_datum_t* typdatum;
+
+	/* Hashtab keys are not constant - suppress warnings */
+	char* user = strdup(data->user); 
+	char* role = strdup(data->role);
+	char* type = strdup(data->type);
+	char* mls = data->mls ? strdup(data->mls): NULL;
+
+	scontext = (context_struct_t*) malloc(sizeof(context_struct_t));
+	if (!user || !role || !type || (data->mls && !mls) || !scontext) {
+		DEBUG(__FUNCTION__, "out of memory\n"); 
+		goto err;
+	}
+	context_init(scontext);
+
+	/* User */
+	usrdatum = (user_datum_t*) hashtab_search(policydb->p_users.table,
+                                        (hashtab_key_t) user);
+	if (!usrdatum) {
+		DEBUG(__FUNCTION__, "user %s is not defined\n", user);
+		goto err_destroy;
+	}
+	scontext->user = usrdatum->value;
+
+	/* Role */
+	roldatum = (role_datum_t*) hashtab_search(policydb->p_roles.table,
+					(hashtab_key_t) role);
+	if (!roldatum) {
+		DEBUG(__FUNCTION__, "role %s is not defined\n", role);
+		goto err_destroy;
+	}
+	scontext->role = roldatum->value;
+
+	/* Type */
+	typdatum = (type_datum_t *) hashtab_search(policydb->p_types.table,
+					(hashtab_key_t) type);
+	if (!typdatum || typdatum->isattr) {
+		DEBUG(__FUNCTION__, "type %s is not defined\n", type);
+		goto err_destroy;
+	}
+	scontext->type = typdatum->value;
+
+	/* MLS */
+	if (mls && !sepol_mls_enabled()) {
+		DEBUG(__FUNCTION__, "mls context found, but mls is disabled\n");
+		mls = NULL;
+	}
+	else if (!mls && sepol_mls_enabled()) {
+		DEBUG(__FUNCTION__, "mls is enabled, but no mls context found\n");
+		goto err_destroy;
+	}
+	if (mls && (mls_context_to_sid(policydb, '$', &mls, scontext) < 0)) {
+		DEBUG(__FUNCTION__, "invalid mls context %s\n", mls);
+		goto err_destroy;
+	}
+
+	/* Validity check */
+	if (!sepol_context_is_valid(policydb, scontext)) {
+		if (mls)
+			DEBUG(__FUNCTION__, 
+				"invalid security context: %s:%s:%s:%s\n",
+				user, role, type, mls);
+		else
+			DEBUG(__FUNCTION__, 
+				"invalid security context: %s:%s:%s\n",
+				user, role, type);
+		goto err_destroy;
+	}
+
+	*cptr = scontext;
+	free(user);	
+	free(type);
+	free(role);
+	free(mls);
+	return STATUS_SUCCESS;
+
+	err_destroy:
+	context_destroy(scontext);
+
+	err: 
+	free(scontext);
+	free(user);
+	free(type);
+	free(role);
+	free(mls);	
+	DEBUG(__FUNCTION__, "error creating context structure\n");
+	return STATUS_ERR;
+}
+
+/*
+ * Create a context structure from the provided string.
+ */
+int sepol_context_from_string(
+	policydb_t* policydb,
+	context_struct_t** cptr,
+	const char* con_str,
+	size_t con_str_len) { 
+
+	char* con_cpy = NULL;
+	sepol_ctxinfo_t ctx_info;
+	char *p;
+
+	/* Copy the string so that we can modify the copy as we parse it.
+	   The string should already by null terminated, but we append a
+	   null suffix to the copy to avoid problems with the existing
+	   attr package, which doesn't view the null terminator as part
+	   of the attribute value. */
+	con_cpy = malloc(con_str_len + 1);
+	if (!con_cpy) {
+		DEBUG(__FUNCTION__, "out of memory\n");
+		goto err;
+	}
+	memcpy(con_cpy, con_str, con_str_len);
+	con_cpy[con_str_len] = '\0';
+	p = (char *) con_cpy;
+
+        /* Find user */
+	ctx_info.user = p;
+	while (*p && *p != ':')
+		p++;
+	if (*p == '\0')
+		goto parse_err;
+	*p++ = '\0';
+
+        /* Find role */
+	ctx_info.role = p;
+	while (*p && *p != ':')
+		p++;
+	if (*p == '\0')
+		goto parse_err;
+	*p++ = '\0';
+
+        /* Find type and MLS */
+	ctx_info.type = p;
+	while (*p && *p != ':')
+		p++;
+	if (sepol_mls_enabled() && (*p == ':'))
+		ctx_info.mls = (p + 1);
+	else
+		ctx_info.mls = NULL;
+	*p++ = '\0';
+
+	/* Now create from the data structure */
+	if (sepol_context_create(policydb, cptr, &ctx_info) < 0)
+		goto err;
+
+	free(con_cpy);
+	return STATUS_SUCCESS;
+	
+	parse_err:
+	DEBUG(__FUNCTION__, "invalid security context: %s\n", con_str);
+
+	err:
+	DEBUG(__FUNCTION__, "unable to create context structure\n");
+	free(con_cpy);
+	return STATUS_ERR;
+}
diff -aru libsepol.work/src/policydb.c libsepol-1.1-context/src/policydb.c
--- libsepol.work/src/policydb.c	2005-07-18 15:05:31.000000000 -0400
+++ libsepol-1.1-context/src/policydb.c	2005-07-20 19:57:49.000000000 -0400
@@ -985,56 +985,6 @@
 	return 0;
 }
 
-/*
- * Return 1 if the fields in the security context 
- * structure `c' are valid.  Return 0 otherwise.
- */
-int policydb_context_isvalid(policydb_t *p, context_struct_t *c)
-{
-	role_datum_t *role;
-	user_datum_t *usrdatum;
-	ebitmap_t types, roles;
-        int ret = 1;
-
-	ebitmap_init(&types);
-	ebitmap_init(&roles);
-	if (!c->role || c->role > p->p_roles.nprim)
-		return 0;
-
-	if (!c->user || c->user > p->p_users.nprim)
-		return 0;
-
-	if (!c->type || c->type > p->p_types.nprim)
-		return 0;
-
-	if (c->role != OBJECT_R_VAL) {
-		/*
-		 * Role must be authorized for the type.
-		 */
-		role = p->role_val_to_struct[c->role - 1];
-		if (!ebitmap_get_bit(&role->cache, c->type - 1))
-			/* role may not be associated with type */
-			return 0;
-		
-		/*
-		 * User must be authorized for the role.
-		 */
-		usrdatum = p->user_val_to_struct[c->user - 1];
-		if (!usrdatum)
-			return 0;
-
-		if (!ebitmap_get_bit(&usrdatum->cache, c->role - 1))
-			/* user may not be associated with role */
-			return 0;
-	}
-
-	if (!mls_context_isvalid(p, c))
-		return 0;
-
-	return ret;
-}
-
-
 /***********************************************************************/
 /* everything below is for policy reads */
 
diff -aru libsepol.work/src/services.c libsepol-1.1-context/src/services.c
--- libsepol.work/src/services.c	2005-07-18 15:05:30.000000000 -0400
+++ libsepol-1.1-context/src/services.c	2005-07-20 19:57:44.000000000 -0400
@@ -57,6 +57,7 @@
 #include <sepol/conditional.h>
 #include <sepol/flask.h>
 
+#include "debug.h"
 #include "private.h"
 #include "av_permissions.h"
 
@@ -481,52 +482,15 @@
 	return sepol_compute_av_reason(ssid, tsid, tclass, requested, avd, &reason);
 }
 
-/*
- * Write the security context string representation of 
- * the context structure `context' into a dynamically
- * allocated string of the correct size.  Set `*scontext'
- * to point to this string and set `*scontext_len' to
- * the length of the string.
- */
-int context_struct_to_string(context_struct_t * context,
-			     sepol_security_context_t * scontext,
-			     size_t *scontext_len)
-{
-	char *scontextp;
-
-	*scontext = 0;
-	*scontext_len = 0;
-
-	/* Compute the size of the context. */
-	*scontext_len += strlen(policydb->p_user_val_to_name[context->user - 1]) + 1;
-	*scontext_len += strlen(policydb->p_role_val_to_name[context->role - 1]) + 1;
-	*scontext_len += strlen(policydb->p_type_val_to_name[context->type - 1]) + 1;
-	*scontext_len += mls_compute_context_len(policydb, context);
-
-	/* Allocate space for the context; caller must free this space. */
-	scontextp = malloc(*scontext_len);
-	if (!scontextp) {
-		return -ENOMEM;
-	}
-	*scontext = (sepol_security_context_t) scontextp;
+/* Deprecated */
+static inline int context_struct_to_string(
+        context_struct_t* context,
+        char ** result,
+        size_t *result_len) {
 
-	/*
-	 * Copy the user name, role name and type name into the context.
-	 */
-	sprintf(scontextp, "%s:%s:%s",
-		policydb->p_user_val_to_name[context->user - 1],
-		policydb->p_role_val_to_name[context->role - 1],
-		policydb->p_type_val_to_name[context->type - 1]);
-	scontextp += strlen(policydb->p_user_val_to_name[context->user - 1]) + 1 + strlen(policydb->p_role_val_to_name[context->role - 1]) + 1 + strlen(policydb->p_type_val_to_name[context->type - 1]);
-
-	mls_sid_to_context(policydb, context, &scontextp);
-
-	*scontextp = 0;
-
-	return 0;
+        return sepol_context_to_string(policydb, context, result, result_len);
 }
 
-
 /*
  * Write the security context string representation of 
  * the context associated with `sid' into a dynamically
@@ -561,105 +525,25 @@
 			    size_t scontext_len,
 			    sepol_security_id_t * sid)
 {
-	sepol_security_context_t scontext2;
-	context_struct_t context;
-	role_datum_t *role;
-	type_datum_t *typdatum;
-	user_datum_t *usrdatum;
-	char *scontextp, *p, oldc;
-	int rc = 0;
-
-	if (sid)
-		*sid = SEPOL_SECSID_NULL;
-
-	/* Copy the string so that we can modify the copy as we parse it.
-	   The string should already by null terminated, but we append a
-	   null suffix to the copy to avoid problems with the existing
-	   attr package, which doesn't view the null terminator as part
-	   of the attribute value. */
-	scontext2 = malloc(scontext_len+1);
-	if (!scontext2) {
-		return -ENOMEM;
-	}
-	memcpy(scontext2, scontext, scontext_len);
-	scontext2[scontext_len] = 0;
-
-	context_init(&context);
-
-	/* Parse the security context. */
-
-	rc = -EINVAL;
-	scontextp = (char *) scontext2;
-
-	/* Extract the user. */
-	p = scontextp;
-	while (*p && *p != ':')
-		p++;
-
-	if (*p == 0)
-		goto out;
+	context_struct_t* context = NULL;
 
-	*p++ = 0;
-
-	usrdatum = (user_datum_t *) hashtab_search(policydb->p_users.table,
-					      (hashtab_key_t) scontextp);
-	if (!usrdatum)
-		goto out;
-
-	context.user = usrdatum->value;
-
-	/* Extract role. */
-	scontextp = p;
-	while (*p && *p != ':')
-		p++;
-
-	if (*p == 0)
-		goto out;
-
-	*p++ = 0;
-
-	role = (role_datum_t *) hashtab_search(policydb->p_roles.table,
-					       (hashtab_key_t) scontextp);
-	if (!role)
-		goto out;
-	context.role = role->value;
-
-	/* Extract type. */
-	scontextp = p;
-	while (*p && *p != ':')
-		p++;
-	oldc = *p;
-	*p++ = 0;
-
-	typdatum = (type_datum_t *) hashtab_search(policydb->p_types.table,
-					      (hashtab_key_t) scontextp);
-
-	if (!typdatum)
-		goto out;
-
-	context.type = typdatum->value;
-
-	rc = mls_context_to_sid(policydb, oldc, &p, &context);
-	if (rc)
-		goto out;
-
-	if ((p - scontext2) < scontext_len) {
-		rc = -EINVAL;
-		goto out;
-	}
+	/* First, create the context */
+	if (sepol_context_from_string(policydb, &context, 
+		scontext, scontext_len) < 0)
+		goto err;
 
-	/* Check the validity of the new context. */
-	if (!policydb_context_isvalid(policydb, &context)) {
-		rc = -EINVAL;
-		goto out;
-	}
-	/* Obtain the new sid. */
-	if (sid)
-		rc = sepol_sidtab_context_to_sid(sidtab, &context, sid);
-out:
-	context_destroy(&context);
-	free(scontext2);
-	return rc;
+	/* Obtain the new sid */
+	if (sid && (sepol_sidtab_context_to_sid(sidtab, context, sid) < 0))
+		goto err;		
+
+	context_destroy(context);
+	return STATUS_SUCCESS;
+
+	err:
+	if (context)
+		context_destroy(context);
+	DEBUG(__FUNCTION__, "could not convert %s to sid\n", scontext);
+	return STATUS_ERR;
 }
 
 int sepol_check_context(char *context) 

Re: [ libsepol 0/6] Context reorganization

[Joshua Brindle]

Ivan Gyurdiev wrote:

>The following patch moves functions relating to context_struct_t
>into file contexts.c for better organization. It makes all of 
>them take policydb as argument, and enforces the sepol_ prefix.
>It uses the new debugging system. It adds a structure for
>a high level representation of a context (sepol_ctxinfo_t), 
>and a new function to convert from it (sepol_context_create),
>and makes use of that function to make the conversion
>from string (formerly sepol_context_to_sid) easier.
>
>libsepol-1.1-context.diff
>
>Disclaimer 1: 
>
>The following bit, as taken from the current 
>policydb_context_isvalid fails for me currently. I don't
>understand the role of the cache here. Does genusers.c
>need to be modified to do something with the cache,
>when it loads the user? Do I need to correspondingly 
>modify my own load_user function (which is based on genusers)?
>
>+               if (!ebitmap_get_bit(&usrdatum->cache, c->role - 1))
>+                       /* user may not be associated with role */
>+                       return 0;
>
>  
>
The purpose of the cache is so that we don't have to do expansion of the 
type sets every time we want to validate a context (since this happens 
before expansion). The cache is built from policydb_index_others so if 
you are't calling that it won't be built. This should work fine with an 
already expanded policy since type_set_expand will just copy the type 
bitmap to the cache.



--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

> The purpose of the cache is so that we don't have to do expansion of the 
> type sets every time we want to validate a context (since this happens 
> before expansion). The cache is built from policydb_index_others so if 
> you are't calling that it won't be built. This should work fine with an 
> already expanded policy since type_set_expand will just copy the type 
> bitmap to the cache.

policydb_index_others is called when I do a policydb_read to
load the base policy, but at that point no users are loaded... 
then later when I load a user, and try to check a context, it fails
on the cache check.


--
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: [ libsepol 0/6] Context reorganization

[Joshua Brindle]

Ivan Gyurdiev wrote:

>>The purpose of the cache is so that we don't have to do expansion of the 
>>type sets every time we want to validate a context (since this happens 
>>before expansion). The cache is built from policydb_index_others so if 
>>you are't calling that it won't be built. This should work fine with an 
>>already expanded policy since type_set_expand will just copy the type 
>>bitmap to the cache.
>>    
>>
>
>policydb_index_others is called when I do a policydb_read to
>load the base policy, but at that point no users are loaded... 
>then later when I load a user, and try to check a context, it fails
>on the cache check.
>  
>
well, that is because the cache bitmap is empty. I'm not sure exactly 
what the order you do things in but you'll need to either call 
policydb_index_others after you've loaded all the users in or 
hashtab_map policydb_user_cache (not recommended), or fill in the cache 
bitmap yourself when loading the users in (really not recommended!)


--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

> well, that is because the cache bitmap is empty. I'm not sure exactly 
> what the order you do things in but you'll need to either call 
> policydb_index_others after you've loaded all the users 

I specifically don't want to do that - that's why I use realloc
and index users one by one... 

Notice how the new API deals with single users, and
does not provide a way to load many users at once. 
It's supposed to be used as part of an iterator
in libsemanage, and I don't want to loop over
the entire set of users every time I modify a single
one. 




--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

On Thu, 2005-07-21 at 14:14 -0400, Ivan Gyurdiev wrote:
> > well, that is because the cache bitmap is empty. I'm not sure exactly 
> > what the order you do things in but you'll need to either call 
> > policydb_index_others after you've loaded all the users 
> 
> I specifically don't want to do that - that's why I use realloc
> and index users one by one... 
> 
> Notice how the new API deals with single users, and
> does not provide a way to load many users at once. 
> It's supposed to be used as part of an iterator
> in libsemanage, and I don't want to loop over
> the entire set of users every time I modify a single
> one. 

Also, correct me if I'm wrong, but I don't think sepol_genusers_policydb
ever calls index_others either - it calls reindex_users directly.

Does that mean that this cache business breaks sepol_genusers_policydb
as is? Not sure how important that is, given that this function is never
actually exported via the shared API...




--
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: [ libsepol 0/6] Context reorganization

[Joshua Brindle]

Ivan Gyurdiev wrote:

>On Thu, 2005-07-21 at 14:14 -0400, Ivan Gyurdiev wrote:
>  
>
>>>well, that is because the cache bitmap is empty. I'm not sure exactly 
>>>what the order you do things in but you'll need to either call 
>>>policydb_index_others after you've loaded all the users 
>>>      
>>>
>>I specifically don't want to do that - that's why I use realloc
>>and index users one by one... 
>>
>>Notice how the new API deals with single users, and
>>does not provide a way to load many users at once. 
>>It's supposed to be used as part of an iterator
>>in libsemanage, and I don't want to loop over
>>the entire set of users every time I modify a single
>>one. 
>>    
>>
>
>Also, correct me if I'm wrong, but I don't think sepol_genusers_policydb
>ever calls index_others either - it calls reindex_users directly.
>
>  
>
>Does that mean that this cache business breaks sepol_genusers_policydb
>as is? Not sure how important that is, given that this function is never
>actually exported via the shared API...
>  
>
it also doesn't call context_isvalid() which means it doesn't need the 
cache.



--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

> it also doesn't call context_isvalid() which means it doesn't need the 
> cache.

I think any functions exposed by libsepol (intended for external use,
whether it's static linking, or shared linking), which manipulate 
the policydb should leave it  in a valid state after 
they're done. This is especially true if we move to a model 
where all changes occur on the in-memory policydb, 
instead of being written to disk (the new functions which I'm adding).

Basically, I want libsepol operations on the policydb to
be transactional in themselves, and you're saying that currently
sepol_genusers_policydb is broken, and so are the new user functions
that I submitted.



--
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: [ libsepol 0/6] Context reorganization

[Joshua Brindle]

Ivan Gyurdiev wrote:

>>it also doesn't call context_isvalid() which means it doesn't need the 
>>cache.
>>    
>>
>
>I think any functions exposed by libsepol (intended for external use,
>whether it's static linking, or shared linking), which manipulate 
>the policydb should leave it  in a valid state after 
>they're done. This is especially true if we move to a model 
>where all changes occur on the in-memory policydb, 
>instead of being written to disk (the new functions which I'm adding).
>
>Basically, I want libsepol operations on the policydb to
>be transactional in themselves, and you're saying that currently
>sepol_genusers_policydb is broken, and so are the new user functions
>that I submitted.
>  
>
you have to index the policy if you want to validate contexts, it's a 
necessity that won't go away.

--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

> you have to index the policy if you want to validate contexts, it's a 
> necessity that won't go away

I maintain the reverse tables properly - I just don't call 
the functions you expect me to, and neither does 
sepol_genusers_policydb. It looks like it's necessary to add:

        /* This pre-expands the roles and users for context validity
checking */
        if (hashtab_map(p->p_roles.table, policydb_role_cache, p))
                return -1;

to policydb_reindex_users().











--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

On Fri, 2005-07-22 at 10:15 -0400, Ivan Gyurdiev wrote:
> > you have to index the policy if you want to validate contexts, it's a 
> > necessity that won't go away
> 
> I maintain the reverse tables properly - I just don't call 
> the functions you expect me to, and neither does 
> sepol_genusers_policydb. It looks like it's necessary to add:
> 
>         /* This pre-expands the roles and users for context validity
> checking */
>         if (hashtab_map(p->p_roles.table, policydb_role_cache, p))
>                 return -1;
> 
> to policydb_reindex_users().


Well I meant the user cache one...
And then I'd have to expand roles manually when adding individual users.


--
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: [ libsepol 0/6] Context reorganization

[Joshua Brindle]

Ivan Gyurdiev wrote:

>>you have to index the policy if you want to validate contexts, it's a 
>>necessity that won't go away
>>    
>>
>
>I maintain the reverse tables properly - I just don't call 
>the functions you expect me to, and neither does 
>sepol_genusers_policydb. It looks like it's necessary to add:
>
>        /* This pre-expands the roles and users for context validity
>checking */
>        if (hashtab_map(p->p_roles.table, policydb_role_cache, p))
>                return -1;
>
>to policydb_reindex_users().
>  
>
No, this will expand the type_sets for each role, which should already 
be done by index_others. You'll want to use policydb_user_cache. I still 
don't recommend doing this but I'll have to look at the code later to 
see if there is something else that can be done.



--
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: [ libsepol 0/6] Context reorganization

[Ivan Gyurdiev]

Why is the cache expansion performed in index_others...
that function has to do with reconstructing reverse entry tables,
and I understand your function has to do with expanding * and ~ -
how is this related to maintaining the reverse entry tables?

Also, I think I'll move any code that has to do
with the users into users.c, and make index_others
call individual functions for roles, types, and users,
that all look like reindex_users (and then those functions
can be moved out of policydb.c)

Basically, I don't like how there is a big policydb.c file - 
this entire library is dedicated to maintaining the policydb
structure, so we should split up functionality into manageable
pieces.


--
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: [ libsepol 0/6] Context reorganization

[Joshua Brindle]

Ivan Gyurdiev wrote:

>Why is the cache expansion performed in index_others...
>that function has to do with reconstructing reverse entry tables,
>and I understand your function has to do with expanding * and ~ -
>how is this related to maintaining the reverse entry tables?
>  
>
Steve prefered I put it there rather than making policydb_read depend on 
another function

>Also, I think I'll move any code that has to do
>with the users into users.c, and make index_others
>call individual functions for roles, types, and users,
>that all look like reindex_users (and then those functions
>can be moved out of policydb.c)
>
>Basically, I don't like how there is a big policydb.c file - 
>this entire library is dedicated to maintaining the policydb
>structure, so we should split up functionality into manageable
>pieces.
>
>
>  
>


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

[ libsepol 10 ] Bugfixes for 0-9

[Ivan Gyurdiev]

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

On Tue, 2005-07-26 at 12:41 -0400, Stephen Smalley wrote:
> On Tue, 2005-07-26 at 12:30 -0400, Ivan Gyurdiev wrote:
> > Attached fix...applies on top of everything else.
> 
> Hmm...doesn't seem to be relative - looks like a complete replacement
> for the original diff?

You're right, I need to do this properly, and post the relative
patch on list - attached.

Depends on: 

libsepol-1.1-context.diff 
libsepol-1.2-ports.diff               
libsepol-1.2-interfaces.diff 
libsepol-1.2-fix_memory_leaks.diff 
libsepol-3.1-write_services_printf.diff


[-- Attachment #2: libsepol-1.3-3.2-bugfixes.diff --]
[-- Type: text/x-patch, Size: 3171 bytes --]

diff -p -aru libsepol.work/src/context.c libsepol-1.3-3.2-bugfixes/src/context.c
--- libsepol.work/src/context.c	2005-07-26 12:48:26.000000000 -0400
+++ libsepol-1.3-3.2-bugfixes/src/context.c	2005-07-26 12:48:25.000000000 -0400
@@ -75,13 +75,17 @@ int sepol_context_to_string(
 	/* Compute the size of the context. */
 	scontext_len += strlen(policydb->p_user_val_to_name[context->user-1])+1;
 	scontext_len += strlen(policydb->p_role_val_to_name[context->role-1])+1;
-	scontext_len += strlen(policydb->p_type_val_to_name[context->type-1])+1;
+	scontext_len += strlen(policydb->p_type_val_to_name[context->type-1]);
 	scontext_len += mls_compute_context_len(policydb, context);
+	
+	/* We must null terminate the string */
+	scontext_len += 1;
 
 	/* Allocate space for the context; caller must free this space. */
 	scontext = malloc(scontext_len);
 	if (!scontext) 
 		goto omem;
+	scontext[scontext_len-1] = '\0';
 
 	/*
 	 * Copy the user name, role name and type name into the context.
@@ -99,6 +103,9 @@ int sepol_context_to_string(
 
 	mls_sid_to_context(policydb, context, &ptr);
 
+	/* MLS may not null terminate the string */
+	*ptr = '\0';
+
 	*result = scontext;
 	*result_len = scontext_len;
 	return STATUS_SUCCESS;
diff -p -aru libsepol.work/src/interfaces.c libsepol-1.3-3.2-bugfixes/src/interfaces.c
--- libsepol.work/src/interfaces.c	2005-07-26 12:48:26.000000000 -0400
+++ libsepol-1.3-3.2-bugfixes/src/interfaces.c	2005-07-26 12:16:31.000000000 -0400
@@ -107,8 +107,6 @@ int sepol_iface_load(
 	if (rc < 0) 
 		goto err;
 
-#warning Is dup_match null terminated ??
-#warning if so, what's the purpose of dup_size
 	else if (rc != STATUS_NODATA) {
 		DEBUG(__FUNCTION__, "interface %s is already mapped to " 
 			"context %s with message context %s\n", 
diff -p -aru libsepol.work/src/ports.c libsepol-1.3-3.2-bugfixes/src/ports.c
--- libsepol.work/src/ports.c	2005-07-26 12:48:26.000000000 -0400
+++ libsepol-1.3-3.2-bugfixes/src/ports.c	2005-07-26 12:16:27.000000000 -0400
@@ -143,8 +143,6 @@ int sepol_port_load(
 	if (rc < 0) 
 		goto err;
 
-#warning Is dup_match null terminated ??
-#warning if so, what's the purpose of dup_size
 	else if (rc != STATUS_NODATA) {
 		DEBUG(__FUNCTION__, "port entry for %s %d-%d "
 			"is already mapped to context %s\n",
diff -p -aru libsepol.work/src/services.c libsepol-1.3-3.2-bugfixes/src/services.c
--- libsepol.work/src/services.c	2005-07-26 12:48:27.000000000 -0400
+++ libsepol-1.3-3.2-bugfixes/src/services.c	2005-07-26 12:16:18.000000000 -0400
@@ -502,7 +502,7 @@ int sepol_sid_to_context(sepol_security_
 
 	context = sepol_sidtab_search(sidtab, sid);
 	if (!context) {
-		DEBUG("unrecognized SID %d\n", sid);
+		DEBUG(__FUNCTION__, "unrecognized SID %d\n", sid);
 		rc = -EINVAL;
 		goto out;
 	}
diff -p -aru libsepol.work/src/write.c libsepol-1.3-3.2-bugfixes/src/write.c
--- libsepol.work/src/write.c	2005-07-26 12:48:27.000000000 -0400
+++ libsepol-1.3-3.2-bugfixes/src/write.c	2005-07-26 12:16:23.000000000 -0400
@@ -40,6 +40,7 @@
 #include <sepol/policydb.h>
 #include <sepol/conditional.h>
 
+#include "debug.h"
 #include "private.h"
 
 static unsigned int policyvers = POLICYDB_VERSION_MAX;

Re: [ libsepol 10 ] Bugfixes for 0-9

[Ivan Gyurdiev]

Actually that null-terminates it twice...at least one should stay
I think, because an mls_sid_to_context doesn't null-terminate.


--
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: [ libsepol 10 ] Bugfixes for 0-9

[Stephen Smalley]

On Tue, 2005-07-26 at 12:52 -0400, Ivan Gyurdiev wrote:
> You're right, I need to do this properly, and post the relative
> patch on list - attached.
> 
> Depends on: 
> 
> libsepol-1.1-context.diff 
> libsepol-1.2-ports.diff               
> libsepol-1.2-interfaces.diff 
> libsepol-1.2-fix_memory_leaks.diff 
> libsepol-3.1-write_services_printf.diff

Ok, all ten merged, plus the uninit variable patch posted by Dan.

-- 
Stephen Smalley
National Security Agency


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

Re: [ libsepol 10 ] Bugfixes for 0-9

[Ivan Gyurdiev]

On Tue, 2005-07-26 at 15:00 -0400, Stephen Smalley wrote:
> On Tue, 2005-07-26 at 12:52 -0400, Ivan Gyurdiev wrote:
> > You're right, I need to do this properly, and post the relative
> > patch on list - attached.
> > 
> > Depends on: 
> > 
> > libsepol-1.1-context.diff 
> > libsepol-1.2-ports.diff               
> > libsepol-1.2-interfaces.diff 
> > libsepol-1.2-fix_memory_leaks.diff 
> > libsepol-3.1-write_services_printf.diff
> 
> Ok, all ten merged, plus the uninit variable patch posted by Dan.

Remove unnecessary null-termination as pointed out (priv mail)?
If null termination is removed +1 strlen should also go away.


--
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: [ libsepol 10 ] Bugfixes for 0-9

[Stephen Smalley]

On Tue, 2005-07-26 at 15:01 -0400, Ivan Gyurdiev wrote:
> Remove unnecessary null-termination as pointed out (priv mail)?

Yes.

> If null termination is removed +1 strlen should also go away.

No.  Still need that for the nul terminator after the type when !
mls_enabled.  That is why it used to be included in that computation -
it was either going to be for the field separator or the nul terminator
after the type, depending on whether or not MLS was enabled.

-- 
Stephen Smalley
National Security Agency


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