[ SEMANAGE ] [ SEPOL ] More database work

[ SEMANAGE ] [ SEPOL ] More database work

[Ivan Gyurdiev]

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

The purpose of this patch is to resync against what I have here, which 
is a lot closer to being correct. It might have bugs, but the patch 
touches only unused code paths (the only code path that's used in the 
init/release path for databases, which doesn't do much). The main piece 
missing to make this testable/usable is commit() integration. I have 
file integration sort of working (meaning you can parse files, and 
hopefully modify them, and save the changes), but the changes aren't 
applied to policy yet. I will do further testing and debugging tomorrow. 
Until I've debugged and tested most of the merged code, commit() 
integration will stay out.

Patch is against the selinux-usr folder, since it touches sepol too.

Changes:

SEMANAGE:
==============

- Database interface: add a function called drop_cache(), which forces 
the database to reload the cache on the next call - this will be called 
after commit(), for example (because we're releasing the transaction lock)

- Direct database: remove double pointer policydb hack, since I don't 
know exactly how this is going to work, so I'll get rid of it for now
- Direct database: add cached/modified flags, and use those
- Direct database: try to implement cache function - haven't checked if 
it works yet...needs more things to test
- Direct database: implement iterate()

- File database: implement flush() - not sure if it works yet - looks 
right though.
- File database: fix a bug in list()

- Direct/File database: add construction of proper path, depending on 
whether we're in transaction or not
- Direct/File database: add functions called enter_ro(), exit_ro(), 
enter_rw(), and use those when the corresponding type of function is 
entered. Those functions check if user's connected, and if user's in 
transaction, and create/drop cache where appropriate. Read-only 
functions currently drop the cache on exit, since they don't hold any 
locks - I don't like this very much.

- Direct extension for records: register add() and modify() functions, 
and fill out the appropriate tables (with the functions from sepol). One 
thing that concerns me here is what to do with the key..which is 
currently unused in sepol.....missing for now.

- User and port databases - pass in the right filename suffix. In the 
direct case, allow an arbitrary suffix (so we can look in any module).
- User and port databases - implement parse() and print() functions. 
Those should work, based on testing in the past - I also just tested the 
user parse function today, and it seems to work (so, I can say that 
dbase_file_cache(), dbase_file_count(), and dbase_file_list() appear to 
work correctly...including the user parser)

SEPOL
==========
Rename functions to more consistent naming policy, that matches semanage 
(those names are now used by semanage). In particular, add() means add a 
new thing, and reject duplicates, modify() means add a new thing, or 
modify if it's already there (so it doesn't reject duplicates). load() 
is changed to either add or modify. Also makes a couple of internal 
functions static.





[-- Attachment #2: libsemanage.sync.diff --]
[-- Type: text/x-patch, Size: 40483 bytes --]

diff -Naur --exclude libselinux old/libsemanage/src/database_direct.c exp/libsemanage/src/database_direct.c
--- old/libsemanage/src/database_direct.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/database_direct.c	2005-10-06 11:18:10.000000000 -0400
@@ -3,30 +3,23 @@
 #define DBASE_DEFINED
 
 #include <stdlib.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <unistd.h>
 #include <sepol/policydb.h>
 #include "database_direct.h"
+#include "semanage_store.h"
+#include "handle.h"
 #include "debug.h"
 
 /* POLICY DIRECT dbase */
 struct dbase_direct {
 
-	/* Backing file */
-	const char* filename;
-
-	/* Address to store the policydb object, once it's
-	 * read from the file above. This is a double pointer,
-	 * because the address will be provided from outside.
-	 * This is done for two reasons:
-	 *
-	 * 1) We want to share the same policy dbase object
-	 * for multiple databases {user/interfaces/ports} -> same policydb
-	 * dbase. We don't want to re-construct the policy separately for
-	 * each.
-	 *
-	 * 2) This allows the policydb to updated outside the dbase_*
-	 * call chain...for example on commit(), after modifications are made.
-	 */
-	policydb_t** policy;
+	/* Backing file suffix */
+	const char* suffix;
 
 	/* Base record table */
 	record_table_t* rtable;
@@ -34,21 +27,183 @@
 	/* Policy extensions */
 	record_direct_table_t* rptable;
 
+	policydb_t* policy;
+	int cached;
+	int modified;
 };
 
+static int construct_filename(
+	semanage_handle_t* handle,
+	dbase_direct_t* dbase,
+	char** filename) {
+
+	const char* path = (handle->is_in_transaction)?
+		semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL):
+		semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
+	size_t fname_length = strlen(path) + strlen(dbase->suffix) + 2;
+
+	char* fname = malloc(fname_length);
+	if (!fname) {
+		/*  FIXME: handle error */
+		return STATUS_ERR;
+	}
+	snprintf(fname, fname_length, "%s/%s", path, dbase->suffix);
+
+	*filename = fname;
+	return STATUS_SUCCESS;
+}
+
+static int dbase_direct_cache(
+	semanage_handle_t* handle,
+	dbase_direct_t* dbase) {
+
+	int fd = -1;
+	struct stat sb;
+	void* data = NULL;
+	policydb_t* policydb = NULL;
+
+	char* fname = NULL;
+
+	/* Already cached */
+	if (dbase->cached)
+		return STATUS_SUCCESS;
+	
+	if (construct_filename(handle, dbase, &fname) < 0)
+		goto err;
+
+	/* Open file */
+	fd = open(fname, O_RDONLY);
+	if (fd < 0) {
+		/* FIXME: handle error */
+		goto err;
+	}
+
+	/* Stat */
+	if (fstat(fd, &sb) < 0) {
+		/* FIXME: handle error */
+		goto err;
+	}
+
+	/* Map file */
+	data = mmap(NULL, sb.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (data == MAP_FAILED) {
+		/* FIXME: handle error */
+		goto err;
+        }
+
+	/* Create policydb image */
+	policydb = (policydb_t*) malloc(sizeof(policydb_t));
+	if (!policydb)
+		goto omem;
+	if (policydb_from_image(data, sb.st_size, policydb) < 0)
+		goto err;
+	dbase->policy = policydb;
+
+	close(fd);
+	munmap(data, sb.st_size);
+	free(fname);
+	dbase->cached = 1;
+	return STATUS_SUCCESS;
+	
+	omem:
+	/* FIXME: handle error */
+	
+	err:
+	/* FIXME: handle error */
+	if (fd > 0)
+		close(fd);
+	if (data != NULL)
+		munmap(data, sb.st_size);
+	
+	free(policydb);
+	free(fname);
+	return STATUS_ERR;
+
+}
+
 static int dbase_direct_flush(
 	semanage_handle_t* handle,	
 	dbase_direct_t* dbase) {
 
+	if (!dbase->modified || !dbase->cached)
+		return STATUS_SUCCESS;
+
+	/* FIXME: policydb_to_image always writes a KERN policy */
+
 	/* Stub */
 	handle = NULL;
 	dbase = NULL;
 	return STATUS_SUCCESS;
 }
 
+
+static void dbase_direct_drop_cache(
+	semanage_handle_t* handle,       
+	 dbase_direct_t* dbase) {
+
+	if (dbase->cached) {
+		policydb_destroy(dbase->policy);
+		free(dbase->policy);
+		dbase->cached = 0;
+	}
+
+	handle = NULL;
+}
+
+static int enter_ro(
+	semanage_handle_t* handle,
+	dbase_direct_t* dbase) {
+
+	if (!handle->is_connected) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+	
+	//if (semanage_handle_get_read_lock(handle) < 0) {
+	//	/* FIXME: handle error */
+	//	return STATUS_ERR;
+	//}
+
+	if (dbase_direct_cache(handle, dbase) < 0) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+
+	return STATUS_SUCCESS;
+}
+
+static inline void exit_ro(
+	semanage_handle_t* handle,
+	dbase_direct_t* dbase) {
+
+	//semanage_release_read_lock(handle);
+	dbase_direct_drop_cache(handle, dbase);
+}
+
+static int enter_rw(
+	semanage_handle_t* handle,
+	dbase_direct_t* dbase) {
+
+	if (!handle->is_connected) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+	
+	if (!handle->is_in_transaction) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+
+	if (dbase_direct_cache(handle, dbase) < 0) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+
+	return STATUS_SUCCESS;	
+}
+
 int dbase_direct_init(
-	const char* filename,
-	policydb_t** policy_update_ptr,
+	const char* suffix,
 	record_table_t* rtable,
 	record_direct_table_t* rptable,
         dbase_direct_t** dbase) {
@@ -59,10 +214,12 @@
 	if (!tmp_dbase)
 		goto omem;
 
-	tmp_dbase->filename = filename;
+	tmp_dbase->suffix = suffix;
 	tmp_dbase->rtable = rtable;
 	tmp_dbase->rptable = rptable;
-	tmp_dbase->policy = policy_update_ptr;
+	tmp_dbase->policy = NULL;
+	tmp_dbase->cached = 0;
+	tmp_dbase->modified = 0;
 	*dbase = tmp_dbase;
 
 	return STATUS_SUCCESS;
@@ -76,8 +233,10 @@
 
 /* Release dbase resources */
 void dbase_direct_release(
+	semanage_handle_t* handle,
 	dbase_direct_t* dbase) {
 
+	dbase_direct_drop_cache(handle, dbase);
 	free(dbase);
 }
 
@@ -86,13 +245,18 @@
 	dbase_direct_t* dbase,
 	record_key_t* key,
 	record_t* data) {
+	
+	if (enter_rw(handle, dbase) < 0)
+		goto err;
 
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	key = NULL;
 	data = NULL;
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	return STATUS_ERR;
 }
 
 static int dbase_direct_modify (
@@ -101,12 +265,17 @@
 	record_key_t* key,
 	record_t* data) {
 
+	if (enter_rw(handle, dbase) < 0)
+		goto err;
+
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	key = NULL;
 	data = NULL;
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	return STATUS_ERR;
 }
 
 static int dbase_direct_del (
@@ -114,11 +283,16 @@
 	dbase_direct_t* dbase,
 	record_key_t* key) {
 
+	if (enter_rw(handle, dbase) < 0)
+		goto err;
+
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	key = NULL;
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	return STATUS_ERR;
 }
 
 static int dbase_direct_query (
@@ -127,12 +301,19 @@
 	record_key_t* key,
 	record_t** response) {
 
+	if (enter_ro(handle, dbase) < 0)
+		goto err;
+
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	key = NULL;
 	response = NULL;
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	exit_ro(handle, dbase);
+	return STATUS_ERR;
 }
 
 static int dbase_direct_exists (
@@ -141,24 +322,38 @@
 	record_key_t* key,
 	int* response) {
 
+	if (enter_ro(handle, dbase) < 0)
+		goto err;
+
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	key = NULL;
 	response = NULL;
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	exit_ro(handle, dbase);
+	return STATUS_ERR;
 }
 
 static int dbase_direct_count (
 	semanage_handle_t* handle,
 	dbase_direct_t* dbase,
 	int* response) {
-	
+
+	if (enter_ro(handle, dbase) < 0)
+		goto err;
+
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	response = NULL;
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	exit_ro(handle, dbase);
+	return STATUS_ERR;
 }
 
 static int dbase_direct_iterate(
@@ -167,13 +362,19 @@
 	int (*fn) (record_t* record, void* fn_arg),
 	void* arg) {
 
-	/* Stub */
-	handle = NULL;
-	dbase = NULL;
-	fn = NULL;
-	arg = NULL;
+	if (enter_ro(handle, dbase) < 0)
+		goto err;
+	
+	if (dbase->rptable->iterate(dbase->policy, fn, arg) < 0)
+		goto err;
 
+	exit_ro(handle, dbase);
         return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	exit_ro(handle, dbase);
+	return STATUS_ERR;
 }
 
 static int dbase_direct_list (
@@ -182,16 +383,24 @@
 	record_t*** records,
 	size_t* count) {
 
+	if (enter_ro(handle, dbase) < 0)
+		goto err;
+
 	/* Stub */
-	handle = NULL;
-	dbase = NULL;
 	records = NULL;
 	count = NULL;
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
+
+	err:
+	/* FIXME: handle error */
+	exit_ro(handle, dbase);
+	return STATUS_ERR;
 }
 
 /* DIRECT POLICY dbase - method table implementation */
 dbase_table_t SEMANAGE_DIRECT_DTABLE = {
+	.drop_cache = dbase_direct_drop_cache,
 	.flush = dbase_direct_flush,
 	.iterate = dbase_direct_iterate,
 	.exists = dbase_direct_exists,
diff -Naur --exclude libselinux old/libsemanage/src/database_direct.h exp/libsemanage/src/database_direct.h
--- old/libsemanage/src/database_direct.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/database_direct.h	2005-10-06 11:18:10.000000000 -0400
@@ -3,6 +3,7 @@
 
 #include <sepol/policydb.h>
 #include "database.h"
+#include "handle.h"
 
 struct dbase_direct;
 typedef struct dbase_direct dbase_direct_t;
@@ -10,8 +11,11 @@
 /* POLICY DIRECT extension to RECORD interface - method table */
 typedef struct record_direct_table {
 
-	/* Load record into the policy database  */
-	int (*load) (policydb_t* policy, record_t* record);
+	/* Add record into the policy database */
+	int (*add) (policydb_t* policy, record_t* record);
+
+	/* Modify record into the policy database */
+	int (*modify) (policydb_t* policydb, record_t* record);
 
 	/* Iterate over records */
 	int (*iterate) (
@@ -21,16 +25,16 @@
 
 } record_direct_table_t;
 
-/* POLICY DIRECT - initialization */
+/* Initialize database */
 extern int dbase_direct_init(
-	const char* filename,
-	policydb_t** policy_update_ptr,
+	const char* suffix,
 	record_table_t* rtable,
 	record_direct_table_t* rptable,
 	dbase_direct_t** dbase);
 
-/* POLICY DIRECT - release */
+/* Release allocated resources */
 extern void dbase_direct_release(
+	semanage_handle_t* handle,
 	dbase_direct_t* dbase);
 
 /* POLICY DIRECT - method table implementation */
diff -Naur --exclude libselinux old/libsemanage/src/database_file.c exp/libsemanage/src/database_file.c
--- old/libsemanage/src/database_file.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/database_file.c	2005-10-06 11:18:10.000000000 -0400
@@ -4,8 +4,10 @@
 
 #include <stdlib.h>
 #include "debug.h"
+#include "handle.h"
 #include "parse_utils.h"
 #include "database_file.h"
+#include "semanage_store.h"
 
 /* Representation of the database once loaded in memory */
 typedef struct cache_entry {
@@ -16,8 +18,8 @@
 /* FILE dbase */ 
 struct dbase_file {
 
-	/* Backing file */
-	const char* filename;
+	/* Backing file suffix */
+	const char* suffix;
 
 	/* Base record table */
 	record_table_t* rtable;
@@ -29,9 +31,31 @@
 	cache_entry_t* cache;
 	size_t cache_sz;
 	int cached;
-	int cache_invalid;
+	int modified;
 };
 
+static int construct_filename(
+	semanage_handle_t* handle,
+	dbase_file_t* dbase,
+	char** filename) {
+
+	const char* path = (handle->is_in_transaction)?
+		semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL):
+		semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL);
+	size_t fname_length = strlen(path) + strlen(dbase->suffix) + 2;
+
+	char* fname = malloc(fname_length);
+	if (!fname) {
+		/*  FIXME: handle error */
+		return STATUS_ERR;
+	}
+	snprintf(fname, fname_length, "%s/%s", path, dbase->suffix);
+
+	*filename = fname;
+	return STATUS_SUCCESS;
+}
+
+
 /* Helper for adding records to the cache */
 static int dbase_file_cache_add(
         dbase_file_t* dbase,
@@ -54,33 +78,47 @@
 }
 
 static int dbase_file_cache(
+	semanage_handle_t* handle,
 	dbase_file_t* dbase) {
 
 	/* Already cached */
-	if (dbase->cached && (!dbase->cache_invalid))
+	if (dbase->cached)
 		return STATUS_SUCCESS;
+	
+	dbase->cache_sz = 0;
+	dbase->cache = NULL;
 
 	int perr_fatal = 0;
 	/* FIXME: pass from caller? */
 
 	record_t* process_record = NULL;
 	int pstatus = STATUS_SUCCESS;
-	parse_info_t parse_info;
-	parse_info.filename = dbase->filename;
-	parse_info.parse_arg = NULL;
-	/* FIXME: pass from caller? */
 
-	if (parse_open(&parse_info) < 0)
+	parse_info_t* parse_info;
+	char* fname = NULL;
+
+	/* Already cached */
+	if (dbase->cached)
+		return STATUS_SUCCESS;
+
+	if (construct_filename(handle, dbase, &fname) < 0)
+		goto err;
+
+	if (parse_init(fname, NULL, &parse_info) < 0) 
+		goto err;
+
+	if (parse_open(parse_info) < 0)
                 goto err;
 
 	/* Main processing loop */
 	do {
+
 		/* Create record */
 		if (dbase->rtable->create(&process_record) < 0)
 			goto err;
 
 		/* Parse record */
-		pstatus = dbase->rftable->parse(&parse_info, process_record);
+		pstatus = dbase->rftable->parse(parse_info, process_record);
 
 		/* Parse error is fatal, exit */
 		if (perr_fatal && (pstatus < 0))
@@ -90,44 +128,142 @@
 		else if (pstatus < 0)
 			continue;
 
+		/* End of file */
+		else if (pstatus == STATUS_NODATA)
+			break;
+
 		/* Add record to list */
 		if (dbase_file_cache_add(dbase, process_record) < 0)
 			goto err;
 
 	} while (pstatus != STATUS_NODATA);
 
-	parse_close(&parse_info);
+	parse_close(parse_info);
+	parse_release(parse_info);
+	free(fname);
 	dbase->cached = 1;
-	dbase->cache_invalid = 0;
 	return STATUS_SUCCESS;
 
 	err:
 	/* FIXME: handle failure */
 	dbase->rtable->free(process_record);
-	parse_close(&parse_info);
+	parse_close(parse_info);
+	parse_release(parse_info);
+	free(fname);
 	return STATUS_ERR;
 }
 
+static void dbase_file_drop_cache(
+	semanage_handle_t* handle,
+	dbase_file_t* dbase) {
+
+	if (!dbase->cached)
+		return;
+
+	cache_entry_t *prev, *ptr = dbase->cache;
+	while (ptr != NULL) {
+		prev = ptr;
+		ptr = ptr->next;
+		dbase->rtable->free(prev->data);
+		free(prev);
+	}
+
+	dbase->cached = 0;
+	handle = NULL;
+}	
+
 /* Flush database to file */
 static int dbase_file_flush(
 	semanage_handle_t* handle,
 	dbase_file_t* dbase) {
 
-	/* Stub */
+	cache_entry_t* ptr;
+	char* fname = NULL;
+	FILE* str = NULL;
+
+	if (!dbase->modified || !dbase->cached)
+		return STATUS_SUCCESS;
+
+	if (!construct_filename(handle, dbase, &fname) < 0)
+		goto err;
+
+	str = fopen(fname, "w");
+	if (!str) {
+		/* FIXME: handle error condition */
+		goto err;
+	}
+
+	for (ptr = dbase->cache; ptr != NULL; ptr = ptr->next) {
+		if (dbase->rftable->print(ptr->data, str) < 0)
+			goto err;
+	}
+
+	fclose(str);
+	free(fname);
+	return STATUS_SUCCESS;
+
+	err:
 	handle = NULL;
-	dbase = NULL;
+	if (str != NULL)
+		fclose(str);
+	/* FIXME: handle error */
+	free(fname);
+	return STATUS_ERR;
+}
+
+static int enter_ro(
+	semanage_handle_t* handle,
+	dbase_file_t* dbase) {
+
+	//if (semanage_handle_get_read_lock(handle) < 0) {
+	//	/* FIXME: handle error */
+	//	return STATUS_ERR;
+	//}
+
+	if (dbase_file_cache(handle, dbase) < 0) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+
+	return STATUS_SUCCESS;
+}
+
+static inline void exit_ro(
+	semanage_handle_t* handle,
+	dbase_file_t* dbase) {
+
+	//semanage_release_read_lock(handle);
+	dbase_file_drop_cache(handle, dbase);
+}
+
+
+static int enter_rw(
+	semanage_handle_t* handle,
+	dbase_file_t* dbase) {
+
+	if (!handle->is_in_transaction) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+
+	if (dbase_file_cache(handle, dbase) < 0) {
+		/* FIXME: handle error */
+		return STATUS_ERR;
+	}
+
 	return STATUS_SUCCESS;
 }
 
 /* Helper for finding records in the cache */
 static int dbase_file_cache_locate(
+	semanage_handle_t* handle,
 	dbase_file_t* dbase,
 	record_key_t* key,
 	cache_entry_t** entry) {
 
 	cache_entry_t* ptr;
 
-	if (dbase_file_cache(dbase) < 0)
+	if (dbase_file_cache(handle, dbase) < 0)
 		goto err;
 
 	for (ptr = dbase->cache; ptr != NULL; ptr = ptr->next) {
@@ -144,7 +280,7 @@
 }
 
 int dbase_file_init(
-	const char* filename,
+	const char* suffix,
 	record_table_t* rtable,
 	record_file_table_t* rftable,
 	dbase_file_t** dbase) {
@@ -155,13 +291,12 @@
 	if (!tmp_dbase)
 		goto omem;
 
-	tmp_dbase->filename = filename;
+	tmp_dbase->suffix = suffix;
 	tmp_dbase->rtable = rtable;
 	tmp_dbase->rftable = rftable;
 	tmp_dbase->cache = NULL;
 	tmp_dbase->cache_sz = 0;
 	tmp_dbase->cached = 0;
-	tmp_dbase->cache_invalid = 0;
 
 	*dbase = tmp_dbase;
 	
@@ -172,19 +307,13 @@
 	free(tmp_dbase);
 	return STATUS_ERR;
 }
-		
+	
 /* Release dbase resources */
 void dbase_file_release(
+	semanage_handle_t* handle,
 	dbase_file_t* dbase) {
 
-	cache_entry_t *prev, *ptr = dbase->cache;
-	while (ptr != NULL) {
-		prev = ptr;
-		ptr = ptr->next;
-                dbase->rtable->free(prev->data);
-		free(prev);
-	}
-
+	dbase_file_drop_cache(handle, dbase);
 	free(dbase);
 }
 
@@ -197,19 +326,20 @@
 	cache_entry_t* entry;
 	int status;
 
-	if (dbase_file_cache(dbase) < 0)
+	if (enter_ro(handle, dbase) < 0)
 		goto err;
 
-	status = dbase_file_cache_locate(dbase, key, &entry);
+	status = dbase_file_cache_locate(handle, dbase, key, &entry);
 	if (status < 0)
 		goto err;
 
 	*response = (status != STATUS_NODATA);
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
 
 	err:
-	handle = NULL;
 	/* FIXME: handle error condition */
+	exit_ro(handle, dbase);
 	return STATUS_ERR;
 }
 
@@ -221,7 +351,7 @@
 
 	int exists;
 
-	if (dbase_file_cache(dbase) < 0)
+	if (enter_rw(handle, dbase) < 0)
 		goto err;
 
 	if (dbase_file_exists(handle, dbase, key, &exists) < 0)
@@ -235,10 +365,10 @@
 	if (dbase_file_cache_add(dbase, data) < 0)
 		goto err;
 
+	dbase->modified = 1;
 	return STATUS_SUCCESS;
 
 	err:
-	handle = NULL;
 	/* FIXME: handle error condition */
 	return STATUS_ERR;
 }
@@ -252,10 +382,10 @@
 	cache_entry_t* entry;
 	int status;
 
-	if (dbase_file_cache(dbase) < 0)
+	if (enter_rw(handle, dbase) < 0)
 		goto err;
 
-	status = dbase_file_cache_locate(dbase, key, &entry);
+	status = dbase_file_cache_locate(handle, dbase, key, &entry);
 	if (status < 0)
 		goto err;
 	if (status == STATUS_NODATA)
@@ -263,10 +393,10 @@
 	else
 		entry->data = data;
 
+	dbase->modified = 1;
 	return STATUS_SUCCESS;
 
 	err:
-	handle = NULL;
 	/* FIXME: handle error condition */
 	return STATUS_ERR;
 }
@@ -276,15 +406,16 @@
         dbase_file_t* dbase,
         int* response) {
 
-	if (dbase_file_cache(dbase) < 0)
+	if (enter_ro(handle, dbase) < 0)
 		goto err;
 
 	*response = dbase->cache_sz;
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
 
 	err:
-	handle = NULL;
 	/* FIXME: Handle error condition */
+	exit_ro(handle, dbase);
 	return STATUS_ERR;
 }
 
@@ -296,21 +427,23 @@
 
 	cache_entry_t* entry;
 	int status;
-        
-	if (dbase_file_cache(dbase) < 0)
+       
+	if (enter_ro(handle, dbase) < 0)
 		goto err;
 
-	status = dbase_file_cache_locate(dbase, key, &entry);
+	status = dbase_file_cache_locate(handle, dbase, key, &entry);
 	if (status < 0 || status == STATUS_NODATA)
 		goto err;
 
 	if (dbase->rtable->clone(entry->data, response) < 0)
 		goto err;
 
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
+
 	err:
-	handle = NULL;
 	/* FIXME: Handle error condition */
+	exit_ro(handle, dbase);
 	return STATUS_ERR;
 }
 
@@ -336,7 +469,7 @@
 
 	cache_entry_t *ptr, *prev = NULL;
 
-	if (dbase_file_cache(dbase) < 0)
+	if (enter_rw(handle, dbase) < 0)
 		goto err;
 
 	for (ptr = dbase->cache; ptr != NULL; ptr = ptr->next) {
@@ -349,6 +482,7 @@
 			dbase->rtable->free(ptr->data);
 			dbase->cache_sz--;
 			free(ptr);
+			dbase->modified = 1;
 			return STATUS_SUCCESS;
 		}
 		else
@@ -357,7 +491,6 @@
 
 	return STATUS_SUCCESS;
 	err:
-	handle = NULL;
 	/* FIXME: Handle error condition */
 	return STATUS_ERR;
 }
@@ -373,7 +506,7 @@
 	size_t tmp_count;
 	int i = 0;
 
-	if (dbase_file_cache(dbase) < 0)
+	if (enter_ro(handle, dbase) < 0)
 		goto err;
 
 	tmp_count = dbase->cache_sz;
@@ -384,23 +517,28 @@
 		if (tmp_records == NULL)
 			goto omem;
 
-		for (ptr = dbase->cache; ptr != NULL; ptr = ptr->next)
-			if (dbase->rtable->clone(ptr->data, &tmp_records[i++]) < 0)
+		for (ptr = dbase->cache; ptr != NULL; ptr = ptr->next) {
+
+			if (dbase->rtable->clone(ptr->data, &tmp_records[i]) < 0)
 				goto err;
+			i++;
+		}
 	}
+
 	*records = tmp_records;
 	*count = tmp_count;
 
+	exit_ro(handle, dbase);
 	return STATUS_SUCCESS;
 
 	omem:
-	handle = NULL;
 	/* FIXME: handle error condition */
 
 	err:
 	for (; i >= 0; i--)
 		dbase->rtable->free(tmp_records[i]);
 	free(tmp_records);
+	exit_ro(handle, dbase);
 	/* FIXME: handle error condition */
 	return STATUS_ERR;
 }
@@ -408,6 +546,7 @@
 
 /* FILE dbase - method table implementation */
 dbase_table_t SEMANAGE_FILE_DTABLE = {
+	.drop_cache = dbase_file_drop_cache,
 	.flush = dbase_file_flush,
 	.iterate = dbase_file_iterate,
 	.exists = dbase_file_exists, 
diff -Naur --exclude libselinux old/libsemanage/src/database_file.h exp/libsemanage/src/database_file.h
--- old/libsemanage/src/database_file.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/database_file.h	2005-10-06 11:18:10.000000000 -0400
@@ -4,6 +4,7 @@
 #include <stdio.h>
 #include "database.h"
 #include "parse_utils.h"
+#include "handle.h"
 
 struct dbase_file;
 typedef struct dbase_file dbase_file_t;
@@ -23,13 +24,14 @@
 
 /* FILE - initialization */
 extern int dbase_file_init(
-	const char* filename,
+	const char* suffix,
 	record_table_t* rtable,
 	record_file_table_t* rftable,
 	dbase_file_t** dbase);
 
 /* FILE - release */
 extern void dbase_file_release(
+	semanage_handle_t* handle,
 	dbase_file_t* dbase);
 
 /* FILE - method table implementation */
diff -Naur --exclude libselinux old/libsemanage/src/database.h exp/libsemanage/src/database.h
--- old/libsemanage/src/database.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/database.h	2005-10-06 11:18:10.000000000 -0400
@@ -14,6 +14,7 @@
 
 #include <stddef.h>
 
+/* Circular dependency */
 struct semanage_handle;
 
 /* RECORD interface - method table */
@@ -92,6 +93,10 @@
 		record_t*** records,
 		size_t* count);
 
+	void (*drop_cache) (
+		struct semanage_handle* handle,
+		dbase_t* dbase);
+
 	int (*flush) (
 		struct semanage_handle* handle,
 		dbase_t* dbase);
diff -Naur --exclude libselinux old/libsemanage/src/handle.c exp/libsemanage/src/handle.c
--- old/libsemanage/src/handle.c	2005-10-04 10:51:22.000000000 -0400
+++ exp/libsemanage/src/handle.c	2005-10-06 11:18:37.000000000 -0400
@@ -124,8 +124,8 @@
 	semanage_conf_destroy(sh->conf);
 
 	/* Free object databases */
-	user_file_dbase_release(&sh->dbase[DBASE_USERS]);
-	port_file_dbase_release(&sh->dbase[DBASE_PORTS]);
+	user_file_dbase_release(sh, &sh->dbase[DBASE_USERS]);
+	port_file_dbase_release(sh, &sh->dbase[DBASE_PORTS]);
 
 	free(sh);
 }
diff -Naur --exclude libselinux old/libsemanage/src/ports_direct.c exp/libsemanage/src/ports_direct.c
--- old/libsemanage/src/ports_direct.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/ports_direct.c	2005-10-06 11:18:10.000000000 -0400
@@ -13,23 +13,28 @@
 #include <sepol/policydb.h>
 #include "ports_direct.h"
 #include "debug.h"
+#include "handle.h"
 #include "database_direct.h"
+#include "semanage_store.h"
 
 /* PORT RECORD (SEPOL): method table (ports_policy.c) */
 extern record_table_t SEPOL_PORT_RTABLE;
 
 /* PORT RECORD (SEPOL): POLICY DIRECT extension : method table */
 record_direct_table_t SEMANAGE_PORT_DIRECT_RTABLE = {
-	.load        = sepol_port_load,
+	.add         = sepol_port_add,
+	.modify      = NULL, /* FIXME */
 	.iterate     = sepol_port_iterate,
 };
 
-int port_direct_dbase_init(dbase_config_t* dconfig) {
+int port_direct_dbase_init(
+	const char* suffix,
+	dbase_config_t* dconfig) {
+
 	if (dbase_direct_init(
-		NULL,                         /* FIXME: backing file       */
-		NULL,                         /* FIXME: policydb pointer   */
-		&SEPOL_PORT_RTABLE,           /* base record table         */
-		&SEMANAGE_PORT_DIRECT_RTABLE, /* direct extensions         */
+		suffix,
+		&SEPOL_PORT_RTABLE,
+		&SEMANAGE_PORT_DIRECT_RTABLE, 
 		&dconfig->dbase) < 0) 
 		return STATUS_ERR;
 
@@ -38,6 +43,9 @@
 	return STATUS_SUCCESS;
 }
 
-void port_direct_dbase_release(dbase_config_t* dconfig) {
-	dbase_direct_release(dconfig->dbase);
+void port_direct_dbase_release(
+	semanage_handle_t* handle,
+	dbase_config_t* dconfig) {
+
+	dbase_direct_release(handle, dconfig->dbase);
 }
diff -Naur --exclude libselinux old/libsemanage/src/ports_direct.h exp/libsemanage/src/ports_direct.h
--- old/libsemanage/src/ports_direct.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/ports_direct.h	2005-10-06 11:18:10.000000000 -0400
@@ -2,11 +2,14 @@
 #define _SEMANAGE_PORTS_DIRECT_H_
 
 #include "database.h"
+#include "handle.h"
 
 int port_direct_dbase_init(
+	const char* suffix,
 	dbase_config_t* dconfig);
 
 void port_direct_dbase_release(
+	semanage_handle_t* handle,
 	dbase_config_t* dconfig);
 
 #endif
diff -Naur --exclude libselinux old/libsemanage/src/ports_file.c exp/libsemanage/src/ports_file.c
--- old/libsemanage/src/ports_file.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/ports_file.c	2005-10-06 11:18:10.000000000 -0400
@@ -1,4 +1,5 @@
 #include <semanage/port_record.h>
+#include <semanage/context_record.h>
 
 typedef semanage_port_t record_t;
 typedef semanage_port_key_t record_key_t;
@@ -13,25 +14,114 @@
 #include "database_file.h"
 #include "parse_utils.h"
 #include "debug.h"
+#include "semanage_store.h"
+#include "handle.h"
 
 static int port_print(
 	semanage_port_t* port, 
 	FILE* str) {
 
-	/* Stub */
-	port = NULL;
-	str = NULL;
+	int low, high;
+	char* con_str = NULL;
+
+	if (fprintf(str, "portcon %s ", semanage_port_get_proto_str(port)) < 0)
+		goto err;
+
+	low = semanage_port_get_low(port);
+	high = semanage_port_get_high(port);
+
+	if (low == high) {
+		if (fprintf(str, "%d ", low) < 0)
+			goto err;
+	} else {
+		if (fprintf(str, "%d - %d ", low, high) < 0)
+			goto err;
+	}
+
+	con_str = semanage_context_to_string(semanage_port_get_con(port));
+	if (!con_str)
+		goto err;
+
+	if (fprintf(str, "%s\n", con_str) < 0)
+		goto err;
+
+	free(con_str);
 	return STATUS_SUCCESS;
+
+	err:
+        /* DEBUG(__FUNCTION__, "error writing to stream: %s\n", strerror(errno)); */
+	free(con_str);
+	return STATUS_ERR;
 }
 
 static int port_parse(
 	parse_info_t* info, 
 	semanage_port_t* port) {
 
-	/* Stub */
-	info = NULL;
-	port = NULL;
-	return STATUS_SUCCESS;	
+	int low, high, items;
+	char* proto = NULL;
+	char* context = NULL;
+	semanage_context_t* con = NULL;
+
+	if (parse_skip_space(info) < 0)
+		goto err;
+	if (!info->ptr)
+		goto last;
+
+	items = sscanf(info->ptr, "portcon %as %d - %d %as",
+			&proto, &low, &high, &context);
+	if (items != 4) {
+		free(proto);
+		free(context);
+		items = sscanf(info->ptr, "portcon %as %d %as", &proto, &low, &context);
+		if (items != 3) {
+			/* DEBUG(__FUNCTION__, "malformed line %u (%s): \n%s\n",
+				info->lineno, info->filename, info->orig_line); */
+			goto err;
+		}
+		semanage_port_set_port(port, low);
+	}
+	else semanage_port_set_range(port, low, high);
+
+	if (!strcasecmp(proto, "tcp"))
+		semanage_port_set_proto(port, SEMANAGE_PROTO_TCP);
+
+	else if (!strcasecmp(proto, "udp"))
+		semanage_port_set_proto(port, SEMANAGE_PROTO_UDP);
+
+	else {
+		/* DEBUG(__FUNCTION__, "invalid protocol %s on line %u (%s)\n",
+			 proto, info->lineno, info->filename); */
+		goto err;
+	}
+
+	if (semanage_context_from_string(context, &con) < 0)
+		goto err;
+
+	/* <<none>> is not allowed for ports */
+	if (!con)
+		goto err;
+
+	semanage_port_set_con(port, con);
+	con = NULL;
+
+	free(proto);
+	free(context);
+	parse_dispose_line(info);
+	return STATUS_SUCCESS;
+
+	last:
+	parse_dispose_line(info);
+	return STATUS_NODATA;
+
+	err:
+	/* DEBUG(__FUNCTION__, "error parsing port record\n"); */
+	free(proto);
+	free(context);
+	semanage_context_free(con);
+
+	parse_dispose_line(info);
+	return STATUS_ERR;
 }
 
 /* PORT RECORD: method table (ports.c) */
@@ -46,9 +136,9 @@
 int port_file_dbase_init(dbase_config_t* dconfig) {
 
 	if (dbase_file_init(
-		NULL,                         /* FIXME: backing file     */
-		&SEMANAGE_PORT_RTABLE,        /* record base table       */
-		&SEMANAGE_PORT_FILE_RTABLE,   /* file extensions         */
+		"port_contexts",
+		&SEMANAGE_PORT_RTABLE,
+		&SEMANAGE_PORT_FILE_RTABLE,
 		&dconfig->dbase) < 0)
 		return STATUS_ERR;
 
@@ -56,6 +146,9 @@
 	return STATUS_SUCCESS;
 }
 
-void port_file_dbase_release(dbase_config_t* dconfig) {
-	dbase_file_release(dconfig->dbase);
+void port_file_dbase_release(
+	semanage_handle_t* handle,
+	dbase_config_t* dconfig) {
+
+	dbase_file_release(handle, dconfig->dbase);
 }
diff -Naur --exclude libselinux old/libsemanage/src/ports_file.h exp/libsemanage/src/ports_file.h
--- old/libsemanage/src/ports_file.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/ports_file.h	2005-10-06 11:18:10.000000000 -0400
@@ -2,11 +2,13 @@
 #define _SEMANAGE_PORTS_FILE_H_
 
 #include "database.h"
+#include "handle.h"
 
 int port_file_dbase_init(
 	dbase_config_t* dconfig);
 
 void port_file_dbase_release(
+	semanage_handle_t* handle,
 	dbase_config_t* dconfig);
 
 #endif 
diff -Naur --exclude libselinux old/libsemanage/src/users_direct.c exp/libsemanage/src/users_direct.c
--- old/libsemanage/src/users_direct.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/users_direct.c	2005-10-06 11:18:10.000000000 -0400
@@ -14,22 +14,27 @@
 #include "users_direct.h"
 #include "debug.h"
 #include "database_direct.h"
+#include "handle.h"
+#include "semanage_store.h"
 
 /* USER RECORD (SEPOL): method table (users_policy.c) */
 extern record_table_t SEPOL_USER_RTABLE;
 
 /* USER RECRORD (SEPOL): POLICY DIRECT extension: method table */
 record_direct_table_t SEMANAGE_USER_DIRECT_RTABLE = {
-	.load        = sepol_user_load,
+	.add         = sepol_user_add,
+	.modify      = sepol_user_modify,
 	.iterate     = sepol_user_iterate,
 };
 
-int user_direct_dbase_init(dbase_config_t* dconfig) {
+int user_direct_dbase_init(
+	const char* suffix,
+	dbase_config_t* dconfig) {
+
 	if (dbase_direct_init(
-		NULL,                         /* FIXME: backing file         */
-		NULL,                         /* FIXME: policydb pointer     */
-		&SEPOL_USER_RTABLE,           /* record base table */
-		&SEMANAGE_USER_DIRECT_RTABLE, /* direct extensions */
+		suffix,
+		&SEPOL_USER_RTABLE, 
+		&SEMANAGE_USER_DIRECT_RTABLE, 
 		&dconfig->dbase) < 0)
 		return STATUS_ERR;
 
@@ -37,6 +42,9 @@
 	return STATUS_SUCCESS;
 }
 
-void user_direct_dbase_release(dbase_config_t* dconfig) {
-	dbase_direct_release(dconfig->dbase);
+void user_direct_dbase_release(
+	semanage_handle_t* handle, 
+	dbase_config_t* dconfig) {
+
+	dbase_direct_release(handle, dconfig->dbase);
 }
diff -Naur --exclude libselinux old/libsemanage/src/users_direct.h exp/libsemanage/src/users_direct.h
--- old/libsemanage/src/users_direct.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/users_direct.h	2005-10-06 11:18:10.000000000 -0400
@@ -2,11 +2,14 @@
 #define _SEMANAGE_USERS_DIRECT_H_
 
 #include "database.h"
+#include "handle.h"
 
 int user_direct_dbase_init(
+	const char* suffix,
 	dbase_config_t* dconfig);
 
 void user_direct_dbase_release(
+	semanage_handle_t* handle,
 	dbase_config_t* dconfig);
 
 #endif
diff -Naur --exclude libselinux old/libsemanage/src/users_file.c exp/libsemanage/src/users_file.c
--- old/libsemanage/src/users_file.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/users_file.c	2005-10-06 11:18:10.000000000 -0400
@@ -10,28 +10,211 @@
 
 #include <stdlib.h>
 #include <stdio.h>
+#include <ctype.h>
+#include <selinux/selinux.h>
 #include "database_file.h"
 #include "parse_utils.h"
 #include "debug.h"
+#include "semanage_store.h"
+#include "handle.h"
 
 static int user_print(
 	semanage_user_t* user, 
 	FILE* str) {
 
-	/* Stub */
-	user = NULL;
-	str = NULL;
+	const char** roles = NULL;
+	size_t i, nroles;
+
+	const char* name      = semanage_user_get_name(user);
+	const char* def_role  = semanage_user_get_defrole(user);
+	const char* mls_level = semanage_user_get_mlslevel(user);
+	const char* mls_range = semanage_user_get_mlsrange(user);
+
+	if (fprintf(str, "user %s roles { %s", name, def_role) < 0)
+		goto err;
+
+	if (semanage_user_get_roles(user, &roles, &nroles) < 0)
+		goto err;
+	
+	for (i = 0; i < nroles; i++) {
+		if (strcmp(roles[i], def_role) && 
+			fprintf(str, "%s ", roles[i]) < 0)
+			goto err;
+	}
+
+	if (fprintf(str, "}") < 0)
+		goto err;
+
+	/* MLS */
+	if (mls_level != NULL && mls_range != NULL)
+		if (fprintf(str, "level %s range %s", mls_level, mls_range) < 0)
+			goto err;
+
+	if (fprintf(str, ";\n") < 0)
+		goto err;
+
+	free(roles);
 	return STATUS_SUCCESS;
+
+	err:
+	free(roles);
+	/* DEBUG(__FUNCTION__, "error writing to stream: %s\n", strerror(errno)); */
+	return STATUS_ERR;
 }
 
 static int user_parse(
 	parse_info_t* info, 
 	semanage_user_t* user) {
 
-	/* Stub */
-	info = NULL;
-	user = NULL;
-	return STATUS_SUCCESS;	
+	int islist = 0;
+	char* mls = NULL;
+	char* start;
+
+	if (parse_skip_space(info) < 0)
+		goto err;
+	if (!info->ptr)
+		goto last;
+
+	/* Parse user name */
+	if (parse_assert_str(info, "user") < 0)
+		goto err;
+
+	if (parse_assert_space(info) < 0)
+		goto err;
+	if (parse_skip_space(info) < 0)
+		goto err;
+	if (parse_assert_noeof(info) < 0)
+		goto err;
+
+	if (semanage_user_set_name(user, parse_fetch_string_inplace(info)) < 0)
+                goto err;
+
+	/* Parse roles header */
+	if (parse_assert_str(info, "roles") < 0)
+		goto err;
+
+	if (parse_assert_space(info) < 0)
+		goto err;
+	if (parse_skip_space(info) < 0)
+		goto err;
+	if (parse_assert_noeof(info) < 0)
+		goto err;
+
+	islist = (parse_optional_ch(info,'{') != STATUS_NODATA);
+
+	/* For each role, loop */
+	do {
+		char delim;
+
+		if (parse_skip_space(info) < 0)
+			goto err;
+		if (parse_assert_noeof(info) < 0)
+			goto err;
+
+		start = info->ptr;
+		while (
+			*(info->ptr) &&
+			*(info->ptr) != ';' &&
+			*(info->ptr) != '}' &&
+			!isspace(*(info->ptr)))
+			info->ptr++;
+
+		delim = *(info->ptr);
+		*(info->ptr)++ = '\0';
+
+		if (semanage_user_add_role(user, start) < 0)
+			goto err;
+
+		if (delim && !isspace(delim)) {
+			if (islist && delim == '}')
+				break;
+			else if (!islist && delim == ';')
+				goto skip_semicolon;
+			else
+				goto err;
+		}
+
+		if (parse_skip_space(info) < 0)
+			goto err;
+		if (parse_assert_noeof(info) < 0)
+			goto err;
+
+		if (parse_optional_ch(info,';') != STATUS_NODATA)
+			goto skip_semicolon;
+
+		if (parse_optional_ch(info,'}') != STATUS_NODATA)
+			islist =0;
+
+	} while (islist);
+
+	/* Handle mls */
+	if (is_selinux_mls_enabled()) {
+
+		/* Parse level header */
+		if (parse_skip_space(info) < 0)
+			goto err;
+		if (parse_assert_noeof(info) < 0)
+			goto err;
+
+		if (parse_optional_str(info, "level") != STATUS_NODATA)
+			goto semicolon;
+
+		if (parse_assert_space(info) < 0)
+			goto err;
+		if (parse_skip_space(info) < 0)
+			goto err;
+		if (parse_assert_noeof(info) < 0)
+			goto err;
+
+		mls = parse_filter_space_until(info, "range");
+		if (!mls)
+			goto err;
+		if (semanage_user_set_mlslevel(user, mls) < 0)
+			goto err;
+		free(mls);
+
+		/* Parse range header */
+		if (parse_assert_str(info, "range") < 0)
+                       goto err;
+
+		if (parse_assert_space(info) < 0)
+			goto err;
+		if (parse_skip_space(info) < 0)
+			goto err;
+		if (parse_assert_noeof(info) < 0)
+			goto err;
+
+		mls = parse_filter_space_until(info, ";");
+		if (!mls)
+			goto err;
+		if (semanage_user_set_mlsrange(user, mls) < 0)
+			goto err;
+		free(mls);
+	}
+
+	/* Check for semicolon */
+	semicolon:
+	if (parse_skip_space(info) < 0)
+		goto err;
+	if (parse_assert_noeof(info) < 0)
+		goto err;
+	if (parse_assert_ch(info,';') < 0)
+		goto err;
+
+	info->ptr++;
+
+	skip_semicolon:
+	return STATUS_SUCCESS;
+
+	last:
+	parse_dispose_line(info);
+	return STATUS_NODATA;
+
+	err:
+        /* DEBUG(__FUNCTION__, "error parsing user record\n"); */
+	free(mls);
+	parse_dispose_line(info);
+	return STATUS_ERR;
 }
 
 /* USER RECORD: metod table (users.c) */
@@ -46,9 +229,9 @@
 int user_file_dbase_init(dbase_config_t* dconfig) {
 	
 	if (dbase_file_init(
-		NULL,                          /* FIXME: backing file  */
-		&SEMANAGE_USER_RTABLE,         /* record base table    */
-		&SEMANAGE_USER_FILE_RTABLE,    /* file extensions      */
+		"local.users",
+		&SEMANAGE_USER_RTABLE,
+		&SEMANAGE_USER_FILE_RTABLE, 
 		&dconfig->dbase) < 0)
 		return STATUS_ERR;
 
@@ -56,6 +239,9 @@
 	return STATUS_SUCCESS;
 }
 
-void user_file_dbase_release(dbase_config_t* dconfig) {
-	dbase_file_release(dconfig->dbase);
+void user_file_dbase_release(
+	semanage_handle_t* handle,
+	dbase_config_t* dconfig) {
+
+	dbase_file_release(handle, dconfig->dbase);
 }
diff -Naur --exclude libselinux old/libsemanage/src/users_file.h exp/libsemanage/src/users_file.h
--- old/libsemanage/src/users_file.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsemanage/src/users_file.h	2005-10-06 11:18:10.000000000 -0400
@@ -2,11 +2,13 @@
 #define _SEMANAGE_USERS_FILE_H_
 
 #include "database.h"
+#include "handle.h"
 
 int user_file_dbase_init(
 	dbase_config_t* dconfig);
 
 void user_file_dbase_release(
+	semanage_handle_t* handle,
 	dbase_config_t* dconfig);
 
 #endif 
diff -Naur --exclude libselinux old/libsepol/include/sepol/interfaces.h exp/libsepol/include/sepol/interfaces.h
--- old/libsepol/include/sepol/interfaces.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsepol/include/sepol/interfaces.h	2005-10-06 11:18:10.000000000 -0400
@@ -13,7 +13,7 @@
 	char** msgcon_str, size_t* msgcon_str_len);
 
 /* Load an interface into policy */
-extern int sepol_iface_load(
+extern int sepol_iface_add(
 	policydb_t* policydb,
 	sepol_iface_t* data);
 
diff -Naur --exclude libselinux old/libsepol/include/sepol/ports.h exp/libsepol/include/sepol/ports.h
--- old/libsepol/include/sepol/ports.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsepol/include/sepol/ports.h	2005-10-06 11:18:10.000000000 -0400
@@ -15,7 +15,7 @@
 	size_t* con_str_len);
 
 /* Load the given port into policy. No shadowing is allowed. */
-extern int sepol_port_load(
+extern int sepol_port_add(
 	policydb_t* policydb, 
 	sepol_port_t* data);
 
diff -Naur --exclude libselinux old/libsepol/include/sepol/users.h exp/libsepol/include/sepol/users.h
--- old/libsepol/include/sepol/users.h	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsepol/include/sepol/users.h	2005-10-06 11:18:10.000000000 -0400
@@ -19,7 +19,7 @@
 	policydb_t* policydb, 
 	const char *username);
 
-extern int sepol_user_load(
+extern int sepol_user_modify(
 	policydb_t* policydb, 
 	sepol_user_t* user);
 
diff -Naur --exclude libselinux old/libsepol/src/interfaces.c exp/libsepol/src/interfaces.c
--- old/libsepol/src/interfaces.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsepol/src/interfaces.c	2005-10-06 11:18:10.000000000 -0400
@@ -12,7 +12,7 @@
 
 /* Create a low level interface structure from
  * a high level representation */
-int sepol_iface_struct_create(
+static int sepol_iface_struct_create(
 	policydb_t* policydb,
 	ocontext_t** iface,
 	sepol_iface_t* data) {
@@ -91,7 +91,7 @@
 }
 
 /* Load an interface into policy */
-int sepol_iface_load(
+int sepol_iface_add(
 	policydb_t* policydb, 
 	sepol_iface_t* data) {
 
diff -Naur --exclude libselinux old/libsepol/src/ports.c exp/libsepol/src/ports.c
--- old/libsepol/src/ports.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsepol/src/ports.c	2005-10-06 11:18:10.000000000 -0400
@@ -38,7 +38,7 @@
 
 /* Create a low level port structure from
  * a high level representation */
-int sepol_port_struct_create(
+static int sepol_port_struct_create(
 	policydb_t* policydb,
 	ocontext_t** port,
 	sepol_port_t* data) {
@@ -132,7 +132,7 @@
 }
 
 /* Load a port into policy */
-int sepol_port_load(
+int sepol_port_add(
 	policydb_t* policydb, 
 	sepol_port_t* data) {
 
diff -Naur --exclude libselinux old/libsepol/src/users.c exp/libsepol/src/users.c
--- old/libsepol/src/users.c	2005-10-06 11:16:51.000000000 -0400
+++ exp/libsepol/src/users.c	2005-10-06 11:18:10.000000000 -0400
@@ -99,7 +99,7 @@
 		goto err;
 	}
 	
-	if (sepol_user_load(policydb, user) < 0) 
+	if (sepol_user_modify(policydb, user) < 0) 
 		goto err;
 
 	free(name);
@@ -157,7 +157,7 @@
  * which case the supplied data replaces the existing data. Alternatively,
  * the user could be new. */
 
-int sepol_user_load(policydb_t* policydb, sepol_user_t* user) {
+int sepol_user_modify(policydb_t* policydb, sepol_user_t* user) {
 
 	/* For user data */	
 	const char *tmp_mlslevel, *tmp_mlsrange;

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Ivan Gyurdiev]

> The purpose of this patch is to resync against what I have here,
The next patch will focus on:

- adding a new debug system to semanage
- testing and debugging the file database code path
- some commit() integration (I want changes and saving to files to work 
perfectly - then I'll worry about the policy).



--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Thu, 2005-10-06 at 12:01 -0400, Ivan Gyurdiev wrote:
> The purpose of this patch is to resync against what I have here, which 
> is a lot closer to being correct. It might have bugs, but the patch 
> touches only unused code paths (the only code path that's used in the 
> init/release path for databases, which doesn't do much). The main piece 
> missing to make this testable/usable is commit() integration. I have 
> file integration sort of working (meaning you can parse files, and 
> hopefully modify them, and save the changes), but the changes aren't 
> applied to policy yet. I will do further testing and debugging tomorrow. 
> Until I've debugged and tested most of the merged code, commit() 
> integration will stay out.

Ok, merged as of libsepol 1.9.12 and libsemanage 1.3.9.

BTW, I'd like to wean libsemanage off of its dependence on the static
libsepol, so that users of libsemanage (like semodule) no longer need to
link with the static libsepol even when they are using the shared
libsemanage.  This means:
- exporting your new libsepol interfaces via libsepol.map so that
libsemanage can use them without depending on the static lib,
- encapsulating the policydb internal types and functions that are
currently used by libsemanage.

I think I'll push all of the policydb internal types and function
interfaces down to include/sepol/policydb/*.h, and we'll introduce
opaque types and interfaces where needed in include/sepol/*.h.  So we
can still have a include/sepol/policydb.h, but it will become the opaque
type definition there, with the internals in
include/sepol/policydb/policydb.h.
 
-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Thu, 2005-10-06 at 15:27 -0400, Stephen Smalley wrote:
> BTW, I'd like to wean libsemanage off of its dependence on the static
> libsepol, so that users of libsemanage (like semodule) no longer need to
> link with the static libsepol even when they are using the shared
> libsemanage.  This means:
> - exporting your new libsepol interfaces via libsepol.map so that
> libsemanage can use them without depending on the static lib,
> - encapsulating the policydb internal types and functions that are
> currently used by libsemanage.
> 
> I think I'll push all of the policydb internal types and function
> interfaces down to include/sepol/policydb/*.h, and we'll introduce
> opaque types and interfaces where needed in include/sepol/*.h.  So we
> can still have a include/sepol/policydb.h, but it will become the opaque
> type definition there, with the internals in
> include/sepol/policydb/policydb.h.

Ok, I've made a first cut at the changes to libsepol and updated it and
checkpolicy to build again, and am starting to work through libsemanage.
As an example of what to expect, the diff below to database_direct.[ch]
was needed to get it to compile again.

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: ddirect.patch --]
[-- Type: text/x-patch, Size: 2881 bytes --]

Index: libsemanage/src/database_direct.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/database_direct.c,v
retrieving revision 1.5
diff -u -p -r1.5 database_direct.c
--- libsemanage/src/database_direct.c	6 Oct 2005 19:08:13 -0000	1.5
+++ libsemanage/src/database_direct.c	7 Oct 2005 14:19:07 -0000
@@ -9,6 +9,7 @@ typedef struct dbase_direct dbase_t;
 #include <sys/stat.h>
 #include <sys/mman.h>
 #include <unistd.h>
+#include <string.h>
 #include <sepol/policydb.h>
 #include "database_direct.h"
 #include "semanage_store.h"
@@ -27,7 +28,7 @@ struct dbase_direct {
 	/* Policy extensions */
 	record_direct_table_t* rptable;
 
-	policydb_t* policy;
+	sepol_policydb_t* policy;
 	int cached;
 	int modified;
 };
@@ -60,7 +61,7 @@ static int dbase_direct_cache(
 	int fd = -1;
 	struct stat sb;
 	void* data = NULL;
-	policydb_t* policydb = NULL;
+	sepol_policydb_t* policydb = NULL;
 
 	char* fname = NULL;
 
@@ -92,10 +93,9 @@ static int dbase_direct_cache(
         }
 
 	/* Create policydb image */
-	policydb = (policydb_t*) malloc(sizeof(policydb_t));
-	if (!policydb)
+	if (sepol_policydb_create(&policydb, SEPOL_POLICY_KERN))
 		goto omem;
-	if (policydb_from_image(data, sb.st_size, policydb) < 0)
+	if (sepol_policydb_from_image(data, sb.st_size, policydb) < 0)
 		goto err;
 	dbase->policy = policydb;
 
@@ -128,7 +128,7 @@ static int dbase_direct_flush(
 	if (!dbase->modified || !dbase->cached)
 		return STATUS_SUCCESS;
 
-	/* FIXME: policydb_to_image always writes a KERN policy */
+	/* FIXME: sepol_policydb_to_image always writes a KERN policy */
 
 	/* Stub */
 	handle = NULL;
@@ -142,8 +142,7 @@ static void dbase_direct_drop_cache(
 	 dbase_direct_t* dbase) {
 
 	if (dbase->cached) {
-		policydb_destroy(dbase->policy);
-		free(dbase->policy);
+		sepol_policydb_free(dbase->policy);
 		dbase->cached = 0;
 	}
 
Index: libsemanage/src/database_direct.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/database_direct.h,v
retrieving revision 1.4
diff -u -p -r1.4 database_direct.h
--- libsemanage/src/database_direct.h	6 Oct 2005 19:08:13 -0000	1.4
+++ libsemanage/src/database_direct.h	7 Oct 2005 14:10:41 -0000
@@ -12,14 +12,14 @@ typedef struct dbase_direct dbase_direct
 typedef struct record_direct_table {
 
 	/* Add record into the policy database */
-	int (*add) (policydb_t* policy, record_t* record);
+	int (*add) (sepol_policydb_t* policy, record_t* record);
 
 	/* Modify record into the policy database */
-	int (*modify) (policydb_t* policydb, record_t* record);
+	int (*modify) (sepol_policydb_t* policydb, record_t* record);
 
 	/* Iterate over records */
 	int (*iterate) (
-		policydb_t* policydb,
+		sepol_policydb_t* policydb,
 		int (*fn)(record_t* record, void* fn_arg),
 	void* arg);
 

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Fri, 2005-10-07 at 10:30 -0400, Stephen Smalley wrote:
> Ok, I've made a first cut at the changes to libsepol and updated it and
> checkpolicy to build again, and am starting to work through libsemanage.
> As an example of what to expect, the diff below to database_direct.[ch]
> was needed to get it to compile again.

A more interesting example - conversion of direct_api.c.

Note that I dropped sepol_set_policyvers() entirely from libsepol.  The
write function now propagate the policydb down as needed and use its
policy_type and policyvers fields to determine appropriate behavior.
policydb_init (not exported by the shared libsepol, but still used
internally and called by sepol_policydb_create) initializes the version
to the maximum supported for the specified policy type.  Thus, by
default, a newly created policydb will be written with the latest
version by functions that ultimately call policydb_write, while a
policydb created via policydb_read (not exported by the shared libsepol,
but still used internally and called by sepol_policydb_from_image) will
be written with its original version presrved by functions that
ultimately call policydb_write.  We'll still need a function to allow
selection of a non-default output version, but it will be per-policydb
now.
 
-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: dapi.patch --]
[-- Type: text/x-patch, Size: 6335 bytes --]

Index: libsemanage/src/direct_api.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/direct_api.c,v
retrieving revision 1.4
diff -u -p -r1.4 direct_api.c
--- libsemanage/src/direct_api.c	4 Oct 2005 12:16:17 -0000	1.4
+++ libsemanage/src/direct_api.c	7 Oct 2005 15:27:30 -0000
@@ -22,6 +22,8 @@
 #include <assert.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -121,22 +123,26 @@ static int semanage_direct_begintrans(se
 static int parse_module_headers(semanage_handle_t *sh, char *module_data,
 				size_t data_len, char **module_name,
 				char **version, char **filename) {
-	struct policy_file pf;
+	struct sepol_policy_file *pf;
 	int file_type;
 	const char *module_path;
 	*module_name = *version = *filename = NULL;
-	pf.type = PF_USE_MEMORY;
-	pf.data = module_data;
-	pf.len = data_len;
-	pf.fp = NULL;
+	
+	if (sepol_policy_file_create(&pf)) {
+		semanage_write_error(sh, "Out of memory!");
+		return -1;
+	}
+	sepol_policy_file_set_mem(pf, module_data, data_len);
 	if (module_data == NULL ||
 	    data_len == 0 ||
-	    sepol_module_package_info(&pf, &file_type, module_name,
+	    sepol_module_package_info(pf, &file_type, module_name,
 				      version) == -1) {
+		sepol_policy_file_free(pf);
 		semanage_write_error(sh, "Could not parse module data.");
 		return -2;
 	}
-	if (file_type != POLICY_MOD) {
+	sepol_policy_file_free(pf);
+	if (file_type != SEPOL_POLICY_MOD) {
 		semanage_write_error(sh, "Data did not represent a module.");
 		return -2;
 	}
@@ -156,23 +162,27 @@ static int parse_module_headers(semanage
  */
 static int parse_base_headers(semanage_handle_t *sh,
 			      char *module_data, size_t data_len) {
-	struct policy_file pf;
+	struct sepol_policy_file *pf;
 	char *module_name = NULL, *version = NULL;
 	int file_type;
-	pf.type = PF_USE_MEMORY;
-	pf.data = module_data;
-	pf.len = data_len;
-	pf.fp = NULL;
+
+	if (sepol_policy_file_create(&pf)) {
+		semanage_write_error(sh, "Out of memory!");
+		return -1;
+	}
+	sepol_policy_file_set_mem(pf, module_data, data_len);
 	if (module_data == NULL ||
 	    data_len == 0 ||
-	    sepol_module_package_info(&pf, &file_type,
+	    sepol_module_package_info(pf, &file_type,
 				      &module_name, &version) == -1) {
+		sepol_policy_file_free(pf);
 		semanage_write_error(sh, "Could not parse base module data.");
 		return -2;
 	}
+	sepol_policy_file_free(pf);
 	free(module_name);
 	free(version);
-	if (file_type != POLICY_BASE) {
+	if (file_type != SEPOL_POLICY_BASE) {
 		semanage_write_error(sh, "Data did not represent a module.");
 		return -2;
 	}
@@ -201,22 +211,22 @@ static int write_file(semanage_handle_t 
  * 'filename'.	Returns 0 on success, -1 if file could not be written.
  */
 static int semanage_write_module(semanage_handle_t *sh,
-				 const char *filename, sepol_module_package_t *package,
-				 uint32_t policy_type) {
-	struct policy_file pf;
+				 const char *filename, sepol_module_package_t *package)
+{
+	struct sepol_policy_file *pf;
 	FILE *outfile;
 	int retval;
-	if (sepol_set_policyvers(policy_type, MOD_POLICYDB_VERSION_MAX) != 0) {
-		semanage_write_error(sh, "Unknown/Invalid policy version %d.", sh->conf->policyvers);
+	if (sepol_policy_file_create(&pf)) {
+		semanage_write_error(sh, "Out of memory!");
 		return -1;
 	}
 	if ((outfile = fopen(filename, "wb")) == NULL) {
+		sepol_policy_file_free(pf);
 		semanage_write_error(sh, "Could not open %s for writing.", filename);
 		return -1;
 	}
-	pf.type = PF_USE_STDIO;
-	pf.fp = outfile;
-	retval = sepol_module_package_write(package, &pf);
+	sepol_policy_file_set_fp(pf, outfile);
+	retval = sepol_module_package_write(package, pf);
 	fclose(outfile);
 	if (retval == -1) {
 		semanage_write_error(sh, "Error while writing module to %s.", filename);
@@ -247,7 +257,7 @@ static int semanage_direct_commit(semana
 
 	/* write the linked base */
 	if ((linked_filename = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED)) == NULL ||
-	    semanage_write_module(sh, linked_filename, base, POLICY_BASE) == -1 ||
+	    semanage_write_module(sh, linked_filename, base) == -1 ||
 	    semanage_verify_linked(sh) != 0) {
 		goto cleanup;
 	}
@@ -271,7 +281,7 @@ static int semanage_direct_commit(semana
 		free(mod_filenames[i]);
 	}
 	free(mod_filenames);
-	sepol_module_package_destroy(base);
+	sepol_module_package_free(base);
 	semanage_release_trans_lock(sh);
 
 	/* regardless if the commit was successful or not, remove the
@@ -434,6 +444,7 @@ static int semanage_direct_remove(semana
  */
 static int semanage_direct_list(semanage_handle_t *sh,
 				semanage_module_info_t **modinfo, int *num_modules) {
+	struct sepol_policy_file *pf;
 	int i, retval = -1;
 	char **module_filenames = NULL;
 	int num_mod_files;
@@ -450,28 +461,35 @@ static int semanage_direct_list(semanage
 		retval = semanage_get_commit_number(sh);
 		goto cleanup;
 	}
+
+	if (sepol_policy_file_create(&pf)) {
+		semanage_write_error(sh, "Out of memory!");
+		goto cleanup;
+	}
+	
 	if ((*modinfo = calloc(num_mod_files, sizeof(**modinfo))) == NULL) {
 		semanage_write_error(sh, "Out of memory!");
 		goto cleanup;
 	}
+	
 	for (i = 0; i < num_mod_files; i++) {
+		FILE *fp;
 		char *name = NULL, *version = NULL;
-		struct policy_file pf;
 		int type;
-		if ((pf.fp = fopen(module_filenames[i], "rb")) == NULL) {
+		if ((fp = fopen(module_filenames[i], "rb")) == NULL) {
 			/* could not open this module file, so don't
 			 * report it */
 			continue;
 		}
-		pf.type = PF_USE_STDIO;
-		if (sepol_module_package_info(&pf, &type, &name, &version)) {
-			fclose(pf.fp);
+		sepol_policy_file_set_fp(pf, fp);
+		if (sepol_module_package_info(pf, &type, &name, &version)) {
+			fclose(fp);
 			free(name);
 			free(version);
 			continue;
 		}
-		fclose(pf.fp);
-		if (type == POLICY_MOD) {
+		fclose(fp);
+		if (type == SEPOL_POLICY_MOD) {
 			(*modinfo)[*num_modules].name = name;
 			(*modinfo)[*num_modules].version = version;
 			(*num_modules)++;
@@ -485,6 +503,7 @@ static int semanage_direct_list(semanage
 	retval = semanage_get_commit_number(sh);
 	
  cleanup:
+	sepol_policy_file_free(pf);
 	for (i = 0; module_filenames != NULL && i < num_mod_files; i++) {
 		free(module_filenames[i]);
 	}

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Fri, 2005-10-07 at 11:52 -0400, Stephen Smalley wrote:
> On Fri, 2005-10-07 at 10:30 -0400, Stephen Smalley wrote:
> > Ok, I've made a first cut at the changes to libsepol and updated it and
> > checkpolicy to build again, and am starting to work through libsemanage.
> > As an example of what to expect, the diff below to database_direct.[ch]
> > was needed to get it to compile again.
> 
> A more interesting example - conversion of direct_api.c.

Still more interesting example - conversion of semanage_store.c.
This brings in the expand/link interfaces.  A few things to note,
primarily directed at the Tresys folks since this was their code:
- AFAICS the calls to policydb_index* were unneeded; they are already
called as appropriate by the libsepol functions to update the indices.
Possibly legacy of earlier versions of the libsepol code?
- Enabling of the global branch moved into sepol_expand_module based on
a new parameter, so that libsemanage and other callers don't need access
to the policydb definition.  Not sure how you plan to selectively enable
in the future.
- expand_module already set the output type, so we only need to adjust
the version here (and I changed it to use the kernel's policyvers if
available instead of the config, as that is what will be loaded by
load_policy now).
- AFAICS the calls to load_isids were bogus/unneeded, unless you were
trying to use their error checking as a side effect, in which case we
should do that as part of expand_module.

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: store.patch --]
[-- Type: text/x-patch, Size: 5018 bytes --]

Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/semanage_store.c,v
retrieving revision 1.3
diff -u -p -r1.3 semanage_store.c
--- libsemanage/src/semanage_store.c	30 Sep 2005 19:23:36 -0000	1.3
+++ libsemanage/src/semanage_store.c	7 Oct 2005 18:15:29 -0000
@@ -29,8 +29,6 @@
 #include <selinux/selinux.h>
 #include <sepol/module.h>
 #include <sepol/sepol.h>
-#include <sepol/link.h>
-#include <sepol/expand.h>
 
 #include <assert.h>
 #include <ctype.h>
@@ -1065,44 +1063,38 @@ int semanage_get_commit_number(semanage_
  */
 static int semanage_load_module(semanage_handle_t *sh, const char *filename, sepol_module_package_t **package) {
 	int retval = 0;
-	struct policy_file pf;
+	FILE *fp;
+	struct sepol_policy_file *pf = NULL;
 
-	if ((*package = (sepol_module_package_t*) malloc(sizeof(**package))) == NULL) {
+	*package = NULL;
+	if (sepol_module_package_create(package) == -1) {
 		semanage_write_error(sh, "Out of memory!");
 		return -1;
 	}
-	if (sepol_module_package_init(*package) == -1) {
+
+	if (sepol_policy_file_create(&pf)) {
 		semanage_write_error(sh, "Out of memory!");
-		sepol_module_package_destroy(*package);
-		*package = NULL;
-		return -1;
+		goto cleanup;
 	}
 
-	if ((pf.fp = fopen(filename, "rb")) == NULL) {
+	if ((fp = fopen(filename, "rb")) == NULL) {
 		semanage_write_error(sh, "Could not open module file %s for reading.", filename);
-		sepol_module_package_destroy(*package);
-		*package = NULL;
-		return -1;
+		goto cleanup;
 	}
-	pf.type = PF_USE_STDIO;
-	if (sepol_module_package_read(*package, &pf, 0) == -1) {
+	sepol_policy_file_set_fp(pf, fp);
+	if (sepol_module_package_read(*package, pf, 0) == -1) {
 		semanage_write_error(sh, "Error while reading from module file %s.", filename);
-		fclose(pf.fp);
-		sepol_module_package_destroy(*package);
-		*package = NULL;
-		return -1;
-	}
-	fclose(pf.fp);
-	if ((*package)->policy->policy_type == POLICY_MOD) {
-		if (policydb_index_classes((*package)->policy) ||
-		    policydb_index_others((*package)->policy, 0)) {
-			semanage_write_error(sh, "Out of memory!");
-			sepol_module_package_destroy(*package);
-			*package = NULL;
-			return -1;
-		}
+		fclose(fp);
+		goto cleanup;
 	}
+	fclose(fp);
 	return retval;
+
+cleanup:
+	sepol_module_package_free(*package);
+	*package = NULL;
+	sepol_policy_file_free(pf);
+	return -1;
 }
 
 /* Links all of the modules within the sandbox into the base module.
@@ -1165,7 +1157,7 @@ int semanage_link_sandbox(semanage_handl
 	}
 	free(module_filenames);
 	for (i = 0; mods != NULL && i < num_modules; i++) {
-		sepol_module_package_destroy(mods[i]);
+		sepol_module_package_free(mods[i]);
 	}
 	free(mods);
 	return retval;
@@ -1176,31 +1168,24 @@ int semanage_link_sandbox(semanage_handl
  * error.
  */
 int semanage_expand_sandbox(semanage_handle_t *sh, sepol_module_package_t *base) {
-	policydb_t out;
+	struct sepol_policydb *out;
 	int retval = -1;
 	const char *kernel_filename = NULL;
-	sidtab_t sidtab;
-	struct policy_file pf;
+	struct sepol_policy_file *pf;
+	int policyvers = security_policyvers();
 	FILE *outfile = NULL;
 
-	memset(&out, 0, sizeof(out));
+	if (policyvers < 0)
+		policyvers = sh->conf->policyvers;
 
-	/* activate the global branch before expansion */
-	base->policy->global->branch_list->enabled = 1;
-	base->policy->global->enabled = base->policy->global->branch_list;
-	if (expand_module(base->policy, &out, 0,
-			  sh->err_buf, SEMANAGE_ERRBUFSZ) == -1) {
-		goto cleanup;
-	}
-	if (policydb_load_isids(&out, &sidtab) == -1) {
-		semanage_write_error(sh, "Error while loading initial SIDs.");
-		goto cleanup;
+	if (sepol_policydb_create(&out)) {
+		return -1;
 	}
-	if (policydb_index_others(&out, 0) == -1) {
-		semanage_write_error(sh, "Out of memory!");
+	if (sepol_expand_module(base->policy, out, 1, 0,
+			  sh->err_buf, SEMANAGE_ERRBUFSZ) == -1) {
 		goto cleanup;
 	}
-	if (sepol_set_policyvers(POLICY_KERN, sh->conf->policyvers)) {
+	if (sepol_policydb_set_vers(base->policy, policyvers)) {
 		semanage_write_error(sh, "Unknown/Invalid policy version %d.", sh->conf->policyvers);
 		goto cleanup;
 	}
@@ -1211,9 +1196,12 @@ int semanage_expand_sandbox(semanage_han
 		semanage_write_error(sh, "Could not open kernel policy %s for writing.", kernel_filename);
 		goto cleanup;
 	}
-	pf.type = PF_USE_STDIO;
-	pf.fp = outfile;
-	if (policydb_write(&out, &pf) == -1) {
+	if (sepol_policy_file_create(&pf)) {
+		semanage_write_error(sh, "Out of memory!");
+		goto cleanup;
+	}
+	sepol_policy_file_set_fp(pf, outfile);
+	if (sepol_policydb_write(out, pf) == -1) {
 		semanage_write_error(sh, "Error while writing kernel policy to %s.", kernel_filename);
 		goto cleanup;
 	}
@@ -1223,8 +1211,8 @@ int semanage_expand_sandbox(semanage_han
 	if (outfile != NULL) {
 		fclose(outfile);
 	}
-	policydb_destroy(&out);
-	sepol_sidtab_destroy(&sidtab);
+	sepol_policydb_free(out);
+	sepol_policy_file_free(pf);
 	return retval;
 }
 

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2005-10-07 at 11:52 -0400, Stephen Smalley wrote:
> 
>>On Fri, 2005-10-07 at 10:30 -0400, Stephen Smalley wrote:
>>
>>>Ok, I've made a first cut at the changes to libsepol and updated it and
>>>checkpolicy to build again, and am starting to work through libsemanage.
>>>As an example of what to expect, the diff below to database_direct.[ch]
>>>was needed to get it to compile again.
>>
>>A more interesting example - conversion of direct_api.c.
> 
> 
> Still more interesting example - conversion of semanage_store.c.
> This brings in the expand/link interfaces.  A few things to note,
> primarily directed at the Tresys folks since this was their code:
> - AFAICS the calls to policydb_index* were unneeded; they are already
> called as appropriate by the libsepol functions to update the indices.
> Possibly legacy of earlier versions of the libsepol code?

Yes, this is probably legacy from before everything was always indexed.

> - Enabling of the global branch moved into sepol_expand_module based on
> a new parameter, so that libsemanage and other callers don't need access
> to the policydb definition.  Not sure how you plan to selectively enable
> in the future.

I don't know of any reason to selectively enable the global branch, it 
should always be enabled or else there won't be any rules in the 
expanded policy.

> - expand_module already set the output type, so we only need to adjust
> the version here (and I changed it to use the kernel's policyvers if
> available instead of the config, as that is what will be loaded by
> load_policy now).

Hrm... Would it be better to always expand it to the latest format we 
know about and handle downgrading at write time? What if the kernel 
policyvery is lower than the requested write version? There is a 
possibility of lost information.

> - AFAICS the calls to load_isids were bogus/unneeded, unless you were
> trying to use their error checking as a side effect, in which case we
> should do that as part of expand_module.

yes, expand_module is probably a better place to do it (for error 
checking)..

<snip>
> @@ -1165,7 +1157,7 @@ int semanage_link_sandbox(semanage_handl
>  	}
>  	free(module_filenames);
>  	for (i = 0; mods != NULL && i < num_modules; i++) {
> -		sepol_module_package_destroy(mods[i]);
> +		sepol_module_package_free(mods[i]);
>  	}
>  	free(mods);
>  	return retval;

What is this?

> @@ -1176,31 +1168,24 @@ int semanage_link_sandbox(semanage_handl
>   * error.
>   */
>  int semanage_expand_sandbox(semanage_handle_t *sh, sepol_module_package_t *base) {
> -	policydb_t out;
> +	struct sepol_policydb *out;
>  	int retval = -1;
>  	const char *kernel_filename = NULL;
> -	sidtab_t sidtab;
> -	struct policy_file pf;
> +	struct sepol_policy_file *pf;
> +	int policyvers = security_policyvers();
>  	FILE *outfile = NULL;
>  
> -	memset(&out, 0, sizeof(out));
> +	if (policyvers < 0)
> +		policyvers = sh->conf->policyvers;
>  

Only if the kernel policyvers lookup fails we use the config file?

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 15:36 -0400, Joshua Brindle wrote:
> I don't know of any reason to selectively enable the global branch, it 
> should always be enabled or else there won't be any rules in the 
> expanded policy.

So what is the purpose of the current code in checkpolicy and
libsemanage (prior to my changes) to enable the branch?  Should
expand_module unconditionally enable this?

> Hrm... Would it be better to always expand it to the latest format we 
> know about and handle downgrading at write time? What if the kernel 
> policyvery is lower than the requested write version? There is a 
> possibility of lost information.

policydb representation is always the "latest"; version only matters at
read/write time.  So this setting of the version is just to decide how
it is written to the kernel policy file.  If we can write the kernel's
version, then we want to do that.  If not, then we want to write the
closest version to the kernel's version, so we'll go with the conf
version which will default to the max version supported by libsepol.  I
revised that logic after sending the diff.

> <snip>
> > @@ -1165,7 +1157,7 @@ int semanage_link_sandbox(semanage_handl
> >  	}
> >  	free(module_filenames);
> >  	for (i = 0; mods != NULL && i < num_modules; i++) {
> > -		sepol_module_package_destroy(mods[i]);
> > +		sepol_module_package_free(mods[i]);
> >  	}
> >  	free(mods);
> >  	return retval;
> 
> What is this?

I changed the interfaces on the libsepol side to be consistent with the
other public ones, so init/destroy -> create/free.  I don't personally
care about it, of course.

> Only if the kernel policyvers lookup fails we use the config file?

I revised this to use the conf setting (or default max) if the kernel
policy version falls outside the range supported by the shared libsepol.

BTW, I now have everything building again (except semodule_link/expand,
which I disabled for now), with semodule and semodule_package both using
only shared libs, and trivial tests of semodule/semodule_package seem to
be working as expected.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2005-10-07 at 15:36 -0400, Joshua Brindle wrote:
> 
>>I don't know of any reason to selectively enable the global branch, it 
>>should always be enabled or else there won't be any rules in the 
>>expanded policy.
> 
> 
> So what is the purpose of the current code in checkpolicy and
> libsemanage (prior to my changes) to enable the branch?  Should
> expand_module unconditionally enable this?
> 
Yes, I think it should.

> 
>>Hrm... Would it be better to always expand it to the latest format we 
>>know about and handle downgrading at write time? What if the kernel 
>>policyvery is lower than the requested write version? There is a 
>>possibility of lost information.
> 
> 
> policydb representation is always the "latest"; version only matters at
> read/write time.  So this setting of the version is just to decide how
> it is written to the kernel policy file. 

Right, we just have to be careful to not let anything use policyvers 
before policydb_write.

> If we can write the kernel's
> version, then we want to do that.  If not, then we want to write the
> closest version to the kernel's version, so we'll go with the conf
> version which will default to the max version supported by libsepol.  I
> revised that logic after sending the diff.
> 
> 
>><snip>
>>
>>>@@ -1165,7 +1157,7 @@ int semanage_link_sandbox(semanage_handl
>>> 	}
>>> 	free(module_filenames);
>>> 	for (i = 0; mods != NULL && i < num_modules; i++) {
>>>-		sepol_module_package_destroy(mods[i]);
>>>+		sepol_module_package_free(mods[i]);
>>> 	}
>>> 	free(mods);
>>> 	return retval;
>>
>>What is this?
> 
> 
> I changed the interfaces on the libsepol side to be consistent with the
> other public ones, so init/destroy -> create/free.  I don't personally
> care about it, of course.

Oh, ok, we had been doing destroy as free contents and node and free as 
free contents, which is why I was confused about the change.
> 
> 
>>Only if the kernel policyvers lookup fails we use the config file?
> 
> 
> I revised this to use the conf setting (or default max) if the kernel
> policy version falls outside the range supported by the shared libsepol.
> 
I'm not sure I understand. If the user sets the config option shouldn't 
it always override?

> BTW, I now have everything building again (except semodule_link/expand,
> which I disabled for now), with semodule and semodule_package both using
> only shared libs, and trivial tests of semodule/semodule_package seem to
> be working as expected.
> 

great. I don't think semodule_package actually needs to link against 
semanage though, that was probably leftover from semod.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 16:15 -0400, Joshua Brindle wrote:
> I'm not sure I understand. If the user sets the config option shouldn't 
> it always override?

Sorry, how does the code know whether the user has set the config
option?  The value is always set by conf-parse to something, even if the
conf file has no setting.  semanage_conf_init() initializes it to the
max version supported by libsepol.

Frankly, I'm not sure I understand the point of seting a fixed value in
a config file at all, given that it needs to change in response to the
versions supported by the current shared libsepol (now available via the
new functions I've introduced) and by the current kernel (already
available via security_policyvers).  If the user sets it to 20 in the
config file and then boots a 2.6.3 kernel, should we honor his setting?
My inclination is to just pick the kernel version always if libsepol
supports writing it, and otherwise fall back to the libsepol max
supported version (which should still be accepted by the kernel).

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2005-10-07 at 16:15 -0400, Joshua Brindle wrote:
> 
>>I'm not sure I understand. If the user sets the config option shouldn't 
>>it always override?
> 
> 
> Sorry, how does the code know whether the user has set the config
> option?  The value is always set by conf-parse to something, even if the
> conf file has no setting.  semanage_conf_init() initializes it to the
> max version supported by libsepol.
> 
> Frankly, I'm not sure I understand the point of seting a fixed value in
> a config file at all, given that it needs to change in response to the
> versions supported by the current shared libsepol (now available via the
> new functions I've introduced) and by the current kernel (already
> available via security_policyvers).  If the user sets it to 20 in the
> config file and then boots a 2.6.3 kernel, should we honor his setting?
> My inclination is to just pick the kernel version always if libsepol
> supports writing it, and otherwise fall back to the libsepol max
> supported version (which should still be accepted by the kernel).
> 

Sure, setting the policy version is probably not useful for most users, 
it's more of a development/debugging option than anything.

What if the user wants to build a policy for the new kernel he just 
installed? I guess a rebuild/reload after booting the kernel isn't bad, 
although we don't provide a way to do that without starting a 
transaction in the store.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Fri, 2005-10-07 at 16:41 -0400, Joshua Brindle wrote:
> Sure, setting the policy version is probably not useful for most users, 
> it's more of a development/debugging option than anything.
> 
> What if the user wants to build a policy for the new kernel he just 
> installed? I guess a rebuild/reload after booting the kernel isn't bad, 
> although we don't provide a way to do that without starting a 
> transaction in the store.

libsemanage doesn't appear to support just building a policy without
loading it AFAICS.  And load_policy no longer honors the path argument,
so it will always load a version <= security_policyvers() for the
current kernel.

Also, I just noticed that:
1) semanage_install_active() blindly uses security_policyvers() as the
version suffix for the running policy path even prior to my changes,
without considering sh->conf->policyvers at all, and
2) semanage_install_sandbox() similarly constructs a running policy path
based solely on security_policyvers(), but never uses it.

Patch below fixes this logic to make (1) consistent with the version
selection in semanage_expand_sandbox (i.e. use security_policyvers() if
it is in the range supported by the shared libsepol, otherwise use the
sh->conf->policyvers, which will be sepol_policy_kern_vers_max() by
default), and drops the dead code from (2).  But note that in the case
where semanage.conf sets the policy version to anything other than the
default, load_policy won't know about it, and no longer honors the path
argument, so it won't load that policy.  I think I'll change
selinux_mkload_policy to also check whether security_policyvers() falls
outside of the libsepol supported range and fall back to
sepol_policy_kern_vers_max() in that case.  But the policy version
setting in semanage.conf is still useless.

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: libsemanage-fixvers.patch --]
[-- Type: text/x-patch, Size: 1646 bytes --]

Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/semanage_store.c,v
retrieving revision 1.4
diff -u -p -r1.4 semanage_store.c
--- libsemanage/src/semanage_store.c	7 Oct 2005 20:22:39 -0000	1.4
+++ libsemanage/src/semanage_store.c	11 Oct 2005 18:51:09 -0000
@@ -826,8 +826,14 @@ static int semanage_install_active(seman
 	const char *active_fc = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_FC);
 	const char *running_fc = selinux_file_context_path();
 	int retval = -3, r;
+	int policyvers = security_policyvers();
+
+	if (policyvers < sepol_policy_kern_vers_min() || 
+	    policyvers > sepol_policy_kern_vers_max())
+		policyvers = sh->conf->policyvers;
+
 	snprintf(running_policy, PATH_MAX, "%s.%d",
-		 selinux_binary_policy_path(), security_policyvers());
+		 selinux_binary_policy_path(), policyvers);
 	if (semanage_copy_file(active_kernel, running_policy) == -1) {
 		semanage_write_error(sh, "Could not copy %s to %s.", active_kernel, running_policy);
 		goto cleanup;
@@ -857,7 +863,6 @@ static int semanage_install_active(seman
  * atomically.	Returns 0 on success, -1 on error.
  */
 int semanage_install_sandbox(semanage_handle_t *sh) {
-	char running_policy[PATH_MAX];
 	int retval = -1, new_commit_number;
 
 	if (sh->conf->load_policy == NULL) {
@@ -869,9 +874,6 @@ int semanage_install_sandbox(semanage_ha
 		goto cleanup;
 	}
 
-	snprintf(running_policy, PATH_MAX, "%s.%d",
-		 selinux_binary_policy_path(), security_policyvers());
-
 	if ((new_commit_number = semanage_commit_sandbox(sh)) < 0) {
 		goto cleanup;
 	}

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Tue, 2005-10-11 at 15:15 -0400, Stephen Smalley wrote:
> Patch below fixes this logic to make (1) consistent with the version
> selection in semanage_expand_sandbox (i.e. use security_policyvers() if
> it is in the range supported by the shared libsepol, otherwise use the
> sh->conf->policyvers, which will be sepol_policy_kern_vers_max() by
> default), and drops the dead code from (2).  But note that in the case
> where semanage.conf sets the policy version to anything other than the
> default, load_policy won't know about it, and no longer honors the path
> argument, so it won't load that policy.  I think I'll change
> selinux_mkload_policy to also check whether security_policyvers() falls
> outside of the libsepol supported range and fall back to
> sepol_policy_kern_vers_max() in that case.  But the policy version
> setting in semanage.conf is still useless.

Patch for libselinux is below.  The real cases are:
1) kernel policyvers falls within the libsepol supported range, in which
case libsemanage and libselinux will use the kernel policyvers for
generation and loading, or
2) kernel policyvers is higher than libsepol max, in which case
libsemanage and libselinux will use the libsepol max for generation and
loading (which the kernel will still accept),
3) kernel policyvers is less than libsepol min.  If this truly happens,
it is fatal, as it means that we cannot generate policy for the kernel.
However, it should never happen as libsepol provides backward
compatibility starting with the first policyvers ever supported by a 2.6
kernel.  At present, this also falls back to the libsepol max for
generation and loading as in (2); I suppose it should be altered to just
fail immediately.  I was originally thinking that we should still try
the libsepol max as a fallback in this case, as security_policyvers()
might return -1 due to a permission denial on /selinux/policyvers, but
immediate failure likely is more sensible.
 
-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: libselinux-1.27.8.diff --]
[-- Type: text/x-patch, Size: 2279 bytes --]

Index: libselinux/ChangeLog
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/ChangeLog,v
retrieving revision 1.149
retrieving revision 1.150
diff -u -p -r1.149 -r1.150
--- libselinux/ChangeLog	6 Oct 2005 18:19:32 -0000	1.149
+++ libselinux/ChangeLog	11 Oct 2005 19:38:16 -0000	1.150
@@ -1,3 +1,8 @@
+1.27.8 2005-10-11
+	* Changed selinux_mkload_policy to fall back to the maximum
+	policy version supported by libsepol if the kernel policy version
+	falls outside of the supported range.
+
 1.27.7 2005-10-06
 	* Changed getseuserbyname to fall back to the Linux username and
 	NULL level if seusers config file doesn't exist unless 
Index: libselinux/VERSION
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/VERSION,v
retrieving revision 1.67
retrieving revision 1.68
diff -u -p -r1.67 -r1.68
--- libselinux/VERSION	6 Oct 2005 18:19:32 -0000	1.67
+++ libselinux/VERSION	11 Oct 2005 19:38:16 -0000	1.68
@@ -1 +1 @@
-1.27.7
+1.27.8
Index: libselinux/src/load_policy.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/load_policy.c,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -p -r1.8 -r1.9
--- libselinux/src/load_policy.c	6 Oct 2005 16:18:13 -0000	1.8
+++ libselinux/src/load_policy.c	11 Oct 2005 19:38:17 -0000	1.9
@@ -11,6 +11,7 @@
 #include <errno.h>
 #include "selinux_internal.h"
 #include <sepol/sepol.h>
+#include <sepol/policydb.h>
 #include "policy.h"
 #include <limits.h>
 
@@ -43,13 +44,13 @@ int selinux_mkload_policy(int preservebo
 	void *map, *data;
 	int fd, rc = -1, *values, len, i, prot;
 
-	if (vers < 0)
-		return -1;
+	if (vers < sepol_policy_kern_vers_min() || vers > sepol_policy_kern_vers_max())
+		vers = sepol_policy_kern_vers_max();
 
 	snprintf(path, sizeof(path), "%s.%d", 
 		 selinux_binary_policy_path(), vers);
 	fd = open(path, O_RDONLY);
-	while (fd < 0 && errno == ENOENT && --vers > 0) {
+	while (fd < 0 && errno == ENOENT && --vers >= sepol_policy_kern_vers_min()) {
 		/* Check prior versions to see if old policy is available */
 		snprintf(path, sizeof(path), "%s.%d", 
 			 selinux_binary_policy_path(), vers);

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Tue, 2005-10-11 at 16:05 -0400, Stephen Smalley wrote:
> Patch for libselinux is below.  The real cases are:
> 1) kernel policyvers falls within the libsepol supported range, in which
> case libsemanage and libselinux will use the kernel policyvers for
> generation and loading, or
> 2) kernel policyvers is higher than libsepol max, in which case
> libsemanage and libselinux will use the libsepol max for generation and
> loading (which the kernel will still accept),
> 3) kernel policyvers is less than libsepol min.  If this truly happens,
> it is fatal, as it means that we cannot generate policy for the kernel.
> However, it should never happen as libsepol provides backward
> compatibility starting with the first policyvers ever supported by a 2.6
> kernel.  At present, this also falls back to the libsepol max for
> generation and loading as in (2); I suppose it should be altered to just
> fail immediately.  I was originally thinking that we should still try
> the libsepol max as a fallback in this case, as security_policyvers()
> might return -1 due to a permission denial on /selinux/policyvers, but
> immediate failure likely is more sensible.

Ok, so further patches for libsemanage and libselinux below.  Now it
treats case (3) as an immediate error, and also drops any use of the
conf value, since it is useless and won't be honored by libselinux for
loading anyway.

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: libsemanage-vers2.patch --]
[-- Type: text/x-patch, Size: 1787 bytes --]

Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/semanage_store.c,v
retrieving revision 1.5
diff -u -p -r1.5 semanage_store.c
--- libsemanage/src/semanage_store.c	11 Oct 2005 19:11:46 -0000	1.5
+++ libsemanage/src/semanage_store.c	11 Oct 2005 20:01:07 -0000
@@ -828,9 +828,10 @@ static int semanage_install_active(seman
 	int retval = -3, r;
 	int policyvers = security_policyvers();
 
-	if (policyvers < sepol_policy_kern_vers_min() || 
-	    policyvers > sepol_policy_kern_vers_max())
-		policyvers = sh->conf->policyvers;
+	if (policyvers < sepol_policy_kern_vers_min())
+		return retval;
+	if (policyvers > sepol_policy_kern_vers_max())
+		policyvers = sepol_policy_kern_vers_max();
 
 	snprintf(running_policy, PATH_MAX, "%s.%d",
 		 selinux_binary_policy_path(), policyvers);
@@ -1177,9 +1178,10 @@ int semanage_expand_sandbox(semanage_han
 	int policyvers = security_policyvers();
 	FILE *outfile = NULL;
 
-	if (policyvers < sepol_policy_kern_vers_min() || 
-	    policyvers > sepol_policy_kern_vers_max())
-		policyvers = sh->conf->policyvers;
+	if (policyvers < sepol_policy_kern_vers_min())
+		return retval;
+	if (policyvers > sepol_policy_kern_vers_max())
+		policyvers = sepol_policy_kern_vers_max();
 
 	if (sepol_policydb_create(&out)) {
 		return -1;
@@ -1189,7 +1191,7 @@ int semanage_expand_sandbox(semanage_han
 		goto cleanup;
 	}
 	if (sepol_policydb_set_vers(out, policyvers)) {
-		semanage_write_error(sh, "Unknown/Invalid policy version %d.", sh->conf->policyvers);
+		semanage_write_error(sh, "Unknown/Invalid policy version %d.", policyvers);
 		goto cleanup;
 	}
 	if ((kernel_filename = semanage_path(SEMANAGE_TMP, SEMANAGE_KERNEL)) == NULL) {

[-- Attachment #3: libselinux-vers2.patch --]
[-- Type: text/x-patch, Size: 741 bytes --]

Index: libselinux/src/load_policy.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/load_policy.c,v
retrieving revision 1.9
diff -u -p -r1.9 load_policy.c
--- libselinux/src/load_policy.c	11 Oct 2005 19:38:17 -0000	1.9
+++ libselinux/src/load_policy.c	11 Oct 2005 20:01:50 -0000
@@ -44,7 +44,9 @@ int selinux_mkload_policy(int preservebo
 	void *map, *data;
 	int fd, rc = -1, *values, len, i, prot;
 
-	if (vers < sepol_policy_kern_vers_min() || vers > sepol_policy_kern_vers_max())
+	if (vers < sepol_policy_kern_vers_min())
+		return -1;
+	if (vers > sepol_policy_kern_vers_max())
 		vers = sepol_policy_kern_vers_max();
 
 	snprintf(path, sizeof(path), "%s.%d", 

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Tue, 2005-10-11 at 16:05 -0400, Stephen Smalley wrote:
> 
>>Patch for libselinux is below.  The real cases are:
>>1) kernel policyvers falls within the libsepol supported range, in which
>>case libsemanage and libselinux will use the kernel policyvers for
>>generation and loading, or
>>2) kernel policyvers is higher than libsepol max, in which case
>>libsemanage and libselinux will use the libsepol max for generation and
>>loading (which the kernel will still accept),
>>3) kernel policyvers is less than libsepol min.  If this truly happens,
>>it is fatal, as it means that we cannot generate policy for the kernel.
>>However, it should never happen as libsepol provides backward
>>compatibility starting with the first policyvers ever supported by a 2.6
>>kernel.  At present, this also falls back to the libsepol max for
>>generation and loading as in (2); I suppose it should be altered to just
>>fail immediately.  I was originally thinking that we should still try
>>the libsepol max as a fallback in this case, as security_policyvers()
>>might return -1 due to a permission denial on /selinux/policyvers, but
>>immediate failure likely is more sensible.
> 
> 
> Ok, so further patches for libsemanage and libselinux below.  Now it
> treats case (3) as an immediate error, and also drops any use of the
> conf value, since it is useless and won't be honored by libselinux for
> loading anyway.

Fair enough, this should probably be removed entirely from the config 
parser and config struct.


--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Fri, 2005-10-07 at 16:41 -0400, Joshua Brindle wrote:
> 
>>Sure, setting the policy version is probably not useful for most users, 
>>it's more of a development/debugging option than anything.
>>
>>What if the user wants to build a policy for the new kernel he just 
>>installed? I guess a rebuild/reload after booting the kernel isn't bad, 
>>although we don't provide a way to do that without starting a 
>>transaction in the store.
> 
> 
> libsemanage doesn't appear to support just building a policy without
> loading it AFAICS.  And load_policy no longer honors the path argument,
> so it will always load a version <= security_policyvers() for the
> current kernel.
> 
right.

> Also, I just noticed that:
> 1) semanage_install_active() blindly uses security_policyvers() as the
> version suffix for the running policy path even prior to my changes,
> without considering sh->conf->policyvers at all, and
> 2) semanage_install_sandbox() similarly constructs a running policy path
> based solely on security_policyvers(), but never uses it.
> 

Why are we still adding the policy version if we aren't going to support 
building different versions anyway.

In the case of multiple kernels with different supported versions this 
could be detrimental in the case that, for example, a version 18 policy 
is built, the kernel is upgraded, a version 20 policy is built, the 
policy is changed, modules/users/whatever added. Then an older kernel is 
booted and loads the stale version 18 policy..

I'm not exactly sure how to handle this situation but this scenerio 
seems undesirable.

> Patch below fixes this logic to make (1) consistent with the version
> selection in semanage_expand_sandbox (i.e. use security_policyvers() if
> it is in the range supported by the shared libsepol, otherwise use the
> sh->conf->policyvers, which will be sepol_policy_kern_vers_max() by
> default), and drops the dead code from (2).  But note that in the case
> where semanage.conf sets the policy version to anything other than the
> default, load_policy won't know about it, and no longer honors the path
> argument, so it won't load that policy.  I think I'll change
> selinux_mkload_policy to also check whether security_policyvers() falls
> outside of the libsepol supported range and fall back to
> sepol_policy_kern_vers_max() in that case.  But the policy version
> setting in semanage.conf is still useless.
> 
> 

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Tue, 2005-10-11 at 18:51 -0400, Joshua Brindle wrote:
> Why are we still adding the policy version if we aren't going to support 
> building different versions anyway.

As you know, the policy version suffix allows multiple policies to exist
simultaneously on the disk for multiple kernels that support different
versions, and allows userspace to select the appropriate policy to load
based on the suffix without having to parse the policy itself.

> In the case of multiple kernels with different supported versions this 
> could be detrimental in the case that, for example, a version 18 policy 
> is built, the kernel is upgraded, a version 20 policy is built, the 
> policy is changed, modules/users/whatever added. Then an older kernel is 
> booted and loads the stale version 18 policy..

Certainly not ideal, but better than not having a policy that can be
loaded at all, thereby causing init to halt the system.

> I'm not exactly sure how to handle this situation but this scenerio 
> seems undesirable.

You've previously suggested having init/load_policy automatically
downgrade the binary policy image to the kernel's version (via
libsepol), but I had some concerns with that idea at the time.  I
suppose it could be looked at again.  Having libsemanage generate
multiple binary policies (as has been done in Fedora during transition
periods between kernels) doesn't seem to generalize well and would be
costly.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Tue, 2005-10-11 at 18:51 -0400, Joshua Brindle wrote:
> 
>>Why are we still adding the policy version if we aren't going to support 
>>building different versions anyway.
> 
> 
> As you know, the policy version suffix allows multiple policies to exist
> simultaneously on the disk for multiple kernels that support different
> versions, and allows userspace to select the appropriate policy to load
> based on the suffix without having to parse the policy itself.
> 
> 
>>In the case of multiple kernels with different supported versions this 
>>could be detrimental in the case that, for example, a version 18 policy 
>>is built, the kernel is upgraded, a version 20 policy is built, the 
>>policy is changed, modules/users/whatever added. Then an older kernel is 
>>booted and loads the stale version 18 policy..
> 
> 
> Certainly not ideal, but better than not having a policy that can be
> loaded at all, thereby causing init to halt the system.
> 
true, and I'm not suggesting it's good to bail, but loading a stale 
policy is a concern.

> 
>>I'm not exactly sure how to handle this situation but this scenerio 
>>seems undesirable.
> 
> 
> You've previously suggested having init/load_policy automatically
> downgrade the binary policy image to the kernel's version (via
> libsepol), but I had some concerns with that idea at the time.  I
> suppose it could be looked at again.  Having libsemanage generate
> multiple binary policies (as has been done in Fedora during transition
> periods between kernels) doesn't seem to generalize well and would be
> costly.
> 

Alternatively if an appropriate binary cannot be found one can be 
expanded from the linked policy in the module store. This would give a 
(hopefully) lossless and current binary policy. It would make startup 
slower in the case that a policy can't be found but seems better than 
loading a stale policy or none at all.

This follows on the theme of not mangling the binary image once it gets 
to load_policy/init, which I think is important. If semanage is managing 
the policy this should be included in that.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Wed, 2005-10-12 at 11:34 -0400, Joshua Brindle wrote:
> Alternatively if an appropriate binary cannot be found one can be 
> expanded from the linked policy in the module store. This would give a 
> (hopefully) lossless and current binary policy. It would make startup 
> slower in the case that a policy can't be found but seems better than 
> loading a stale policy or none at all.
> 
> This follows on the theme of not mangling the binary image once it gets 
> to load_policy/init, which I think is important. If semanage is managing 
> the policy this should be included in that.

I can't quite imagine init invoking libsemanage to generate a policy
file for it to load.  It would also create a circular dependency between
libselinux (which now contains all of the load policy logic and thus
would end up being modified for this purpose) and libsemanage (which
already depends on libselinux).  

As for modifying the binary policy image at load time, Karl already
agreed that the preservation of current boolean settings (in the
load_policy case, not the init case) has to remain part of that logic.
So regenerating to a different policy version in memory isn't especially
different/difficult there.

Offhand, I think that the only case where neither libsepol nor
libsemanage will yield the same policy as checkpolicy if downgrading
policy versions is the netlink class case, as checkpolicy does
manipulation during the policy source parsing itself there.  But that
change occurred circa 2.6.9 and was an unusual use of the version
mechanism, so it likely isn't much of a concern now.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Wed, 2005-10-12 at 11:34 -0400, Joshua Brindle wrote:
> 
>>Alternatively if an appropriate binary cannot be found one can be 
>>expanded from the linked policy in the module store. This would give a 
>>(hopefully) lossless and current binary policy. It would make startup 
>>slower in the case that a policy can't be found but seems better than 
>>loading a stale policy or none at all.
>>
>>This follows on the theme of not mangling the binary image once it gets 
>>to load_policy/init, which I think is important. If semanage is managing 
>>the policy this should be included in that.
>

Now that I think of it this can still lead to a stale policy if one was 
previously generated but the linked policy was since updated.

I think that we need to make libsemanage remove all binary policies in 
the binary policy directory when a new policy is install.

> 
> I can't quite imagine init invoking libsemanage to generate a policy
> file for it to load.  It would also create a circular dependency between
> libselinux (which now contains all of the load policy logic and thus
> would end up being modified for this purpose) and libsemanage (which
> already depends on libselinux).  
> 
> As for modifying the binary policy image at load time, Karl already
> agreed that the preservation of current boolean settings (in the
> load_policy case, not the init case) has to remain part of that logic.
> So regenerating to a different policy version in memory isn't especially
> different/difficult there.

The boolean preservation case is a special case, I think we should try 
to limit binary image mutating as much as possible though.

> 
> Offhand, I think that the only case where neither libsepol nor
> libsemanage will yield the same policy as checkpolicy if downgrading
> policy versions is the netlink class case, as checkpolicy does
> manipulation during the policy source parsing itself there.  But that
> change occurred circa 2.6.9 and was an unusual use of the version
> mechanism, so it likely isn't much of a concern now.
> 

hrm, should the logic in checkpolicy be moved to libsepol/write.c so 
that it will always be consistent?

I know the current versions allow downgrading that shouldn't affect the 
security properties of the policy but I don't see how it can be 
guaranteed that this is the case for every future version.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Wed, 2005-10-12 at 12:19 -0400, Joshua Brindle wrote:
> Now that I think of it this can still lead to a stale policy if one was 
> previously generated but the linked policy was since updated.
> 
> I think that we need to make libsemanage remove all binary policies in 
> the binary policy directory when a new policy is install.

No, we would just be changing the libsemanage and libselinux to generate
and load a binary policy path without any version suffix if we have the
load policy logic automatically downgrade the policy to the kernel
version (whether via libsemanage or via libsepol directly).  The load
policy logic would check first for the versionless file and use it if it
exists (which would indicate use of libsemanage on the system);
otherwise, it would fall through to the existing search for the
versioned file for backward compatibility with non-libsemanage'd
systems.

> The boolean preservation case is a special case, I think we should try 
> to limit binary image mutating as much as possible though.

Boolean preservation requires a policydb_read/write sequence already, so
it only requires setting the policyvers explicitly prior to the
policydb_write to simultaneously downgrade the policy.  Regardless I
think we have to do it in memory at init time, as we shouldn't be
writing to the filesystem at that point (fs is read-only then, right?).

> hrm, should the logic in checkpolicy be moved to libsepol/write.c so 
> that it will always be consistent?

Possibly, but may be irrelevant at this point.

> I know the current versions allow downgrading that shouldn't affect the 
> security properties of the policy but I don't see how it can be 
> guaranteed that this is the case for every future version.

That's a case for keeping the version suffixes and just using the
possibly stale file instead, so that you at least have an internally
consistent policy at every point in time, even if it isn't fully
up-to-date.

>From a compatibility POV, one could argue that booting an old kernel
should use that old policy in order to ensure that the kernel continues
to allow precisely what it used to allow (although that is an argument
for binding policies to kernel versions ala kernel modules and
rebuilding them on every kernel update, dropping the compatibility
support entirely from the kernel as suggested by James a while ago, but
no one seemed to want to bind kernel versions to policy versions).

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Wed, 2005-10-12 at 12:19 -0400, Joshua Brindle wrote:
> 
>>Now that I think of it this can still lead to a stale policy if one was 
>>previously generated but the linked policy was since updated.
>>
>>I think that we need to make libsemanage remove all binary policies in 
>>the binary policy directory when a new policy is install.
> 
> 
> No, we would just be changing the libsemanage and libselinux to generate
> and load a binary policy path without any version suffix if we have the
> load policy logic automatically downgrade the policy to the kernel
> version (whether via libsemanage or via libsepol directly).  The load
> policy logic would check first for the versionless file and use it if it
> exists (which would indicate use of libsemanage on the system);
> otherwise, it would fall through to the existing search for the
> versioned file for backward compatibility with non-libsemanage'd
> systems.
> 
Another idea, instead of downgrading the binary (because of the 
possibility of an incompatible downgrade in the future) what about using 
the expander in libsepol to build a kernel image from the linked module? 
This is basically the same as the libsemanage suggestion but without 
using libsemanage.

This seems to be getting fairly complicated, expanding a module 
(possibly) at boot time but stale or no policy, again, seems worse.

This also introduces more error conditions and something else would have 
to be done for unmanaged systems but in the long run this should 
guarantee that the policy is current and the kernel gets the version it 
needs, whether it was previously built or not.

OTOH, downgrading will probably work fine for now since there is 
compatibility and since the loading (and thus downgrading) will be 
encapsulated if there is ever a non-compatible policy version it can be 
switched to expanding the linked module. I think downgrading is probably 
the best way to do it now.

> 
>>The boolean preservation case is a special case, I think we should try 
>>to limit binary image mutating as much as possible though.
> 
> 
> Boolean preservation requires a policydb_read/write sequence already, so
> it only requires setting the policyvers explicitly prior to the
> policydb_write to simultaneously downgrade the policy.  Regardless I
> think we have to do it in memory at init time, as we shouldn't be
> writing to the filesystem at that point (fs is read-only then, right?).
> 
Right, and in the init case it would have to expand the module in memory 
and use that without writing to disk, this causes the situation where 
the latest policy version is never written to disk and the policy is 
re-expanded on every boot, which is not desirable.

Also, in the downgrade case, since init does not preserve booleans it 
doesn't have a need to read/write the image and could get away with 
loading the image straight off disk if an appropriate one is found, 
otherwise it will have to downgrade in memory.

> 
>>hrm, should the logic in checkpolicy be moved to libsepol/write.c so 
>>that it will always be consistent?
> 
> 
> Possibly, but may be irrelevant at this point.
> 
> 
>>I know the current versions allow downgrading that shouldn't affect the 
>>security properties of the policy but I don't see how it can be 
>>guaranteed that this is the case for every future version.
> 
> 
> That's a case for keeping the version suffixes and just using the
> possibly stale file instead, so that you at least have an internally
> consistent policy at every point in time, even if it isn't fully
> up-to-date.
> 
Expanding the linked policy could address this, ideally the module 
format should be able to support any binary version supported by libsepol.

>>From a compatibility POV, one could argue that booting an old kernel
> should use that old policy in order to ensure that the kernel continues
> to allow precisely what it used to allow (although that is an argument
> for binding policies to kernel versions ala kernel modules and
> rebuilding them on every kernel update, dropping the compatibility
> support entirely from the kernel as suggested by James a while ago, but
> no one seemed to want to bind kernel versions to policy versions).
> 

If policy updates have occured it is possible that the filesystem is not 
  labeled the same way, and could have invalid labels.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Wed, 2005-10-12 at 14:06 -0400, Joshua Brindle wrote: 
> Another idea, instead of downgrading the binary (because of the 
> possibility of an incompatible downgrade in the future) what about using 
> the expander in libsepol to build a kernel image from the linked module? 
> This is basically the same as the libsemanage suggestion but without 
> using libsemanage.

"Downgrading" just means setting the desired policy version for the
binary policy image (via sepol_policydb_set_vers) and then calling
sepol_policydb_write to write the image to memory or a file.  That is
what libsemanage already does for expansion after calling
sepol_expand_module (which merely expands to a policydb, not a binary
policy image).  The policydb is not versioned in its memory
representation; it merely carries around a version reflecting its source
for use in subsequent generation of an image - either the version of the
original policy image loaded into the policydb via a policydb_read or
the latest version supported by libsepol when creating a new image via
expand_module.  So if libsepol doesn't support a "downgrade" of a binary
policy image to an older version due to incompatibility, it isn't going
to support expanding to that version either.  It ultimately all comes
through policydb_write.

Also, libselinux shouldn't be pulling directly from the module store, so
it would still have some dependency on libsemanage to find that linked
module file.  At present, libsemanage depends on libselinux and
libsepol, libselinux depends on libsepol (for load_policy only), and
libsepol depends on nothing (other than libc, of course).   

> I think downgrading is probably 
> the best way to do it now.

I was thinking that the policy pathname could then become versionless,
but it occurs to me that the version suffix is still useful in allowing
libselinux to determine the policy version of the file (to decide
whether a downgrade is required) without needing to parse it, and
preserving the suffix also lets us unify the loading logic.  So, the
behavior of libsemanage and libselinux would be as follows:
- libsemanage always generates the latest policy version supported by
libsepol (the default), writing it to a versioned policy pathname with
that version number.  It doesn't use the kernel's policy version at all.

- libselinux always searches for a versioned filename with the highest
version <= the sepol max, opens and mmaps that file, uses the version
suffix to determine whether a downgrade is required, and downgrades if
necessary to the kernel's version.  This same logic applies both for the
managed case and the unmanaged case.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Wed, 2005-10-12 at 15:52 -0400, Stephen Smalley wrote:
> - libsemanage always generates the latest policy version supported by
> libsepol (the default), writing it to a versioned policy pathname with
> that version number.  It doesn't use the kernel's policy version at all.
> 
> - libselinux always searches for a versioned filename with the highest
> version <= the sepol max, opens and mmaps that file, uses the version
> suffix to determine whether a downgrade is required, and downgrades if
> necessary to the kernel's version.  This same logic applies both for the
> managed case and the unmanaged case.

And, if the downgrade fails, libselinux keeps searching for a lower
versioned filename until it finds one that can be downgraded or it finds
one that doesn't need a downgrade (i.e. already at or below the kernel's
version) or it passes the libsepol min (in which case it will just
fail).  That provides the fallback case for incompatible changes.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Wed, 2005-10-12 at 16:11 -0400, Stephen Smalley wrote:
> On Wed, 2005-10-12 at 15:52 -0400, Stephen Smalley wrote:
> > - libsemanage always generates the latest policy version supported by
> > libsepol (the default), writing it to a versioned policy pathname with
> > that version number.  It doesn't use the kernel's policy version at all.
> > 
> > - libselinux always searches for a versioned filename with the highest
> > version <= the sepol max, opens and mmaps that file, uses the version
> > suffix to determine whether a downgrade is required, and downgrades if
> > necessary to the kernel's version.  This same logic applies both for the
> > managed case and the unmanaged case.
> 
> And, if the downgrade fails, libselinux keeps searching for a lower
> versioned filename until it finds one that can be downgraded or it finds
> one that doesn't need a downgrade (i.e. already at or below the kernel's
> version) or it passes the libsepol min (in which case it will just
> fail).  That provides the fallback case for incompatible changes.

Patches below for libsemanage and libselinux (after merging Ivan's
diffs).  libsemanage goes back to always using sh->conf->policyvers,
which is initialized to sepol_policy_kern_vers_max() but can be changed
in the config file, and libselinux tries to automatically downgrade.


-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: libsemanage-confvers.patch --]
[-- Type: text/x-patch, Size: 1579 bytes --]

Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/semanage_store.c,v
retrieving revision 1.7
diff -u -p -r1.7 semanage_store.c
--- libsemanage/src/semanage_store.c	13 Oct 2005 15:56:30 -0000	1.7
+++ libsemanage/src/semanage_store.c	13 Oct 2005 15:59:51 -0000
@@ -826,12 +826,7 @@ static int semanage_install_active(seman
 	const char *active_fc = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_FC);
 	const char *running_fc = selinux_file_context_path();
 	int retval = -3, r;
-	int policyvers = security_policyvers();
-
-	if (policyvers < sepol_policy_kern_vers_min())
-		return retval;
-	if (policyvers > sepol_policy_kern_vers_max())
-		policyvers = sepol_policy_kern_vers_max();
+	int policyvers = sh->conf->policyvers;
 
 	snprintf(running_policy, PATH_MAX, "%s.%d",
 		 selinux_binary_policy_path(), policyvers);
@@ -1186,7 +1181,7 @@ int semanage_expand_sandbox(semanage_han
 	int retval = -1;
 	const char *kernel_filename = NULL;
 	struct sepol_policy_file *pf = NULL;
-	int policyvers = security_policyvers();
+	int policyvers = sh->conf->policyvers;
 	FILE *outfile = NULL;
 
  	/* FIXME: deprecated - replace with callback debugging
@@ -1194,11 +1189,6 @@ int semanage_expand_sandbox(semanage_han
  	char buffer[1024];
  	buffer[0] = '\0';
 
-	if (policyvers < sepol_policy_kern_vers_min())
-		return retval;
-	if (policyvers > sepol_policy_kern_vers_max())
-		policyvers = sepol_policy_kern_vers_max();
-
 	if (sepol_policydb_create(&out)) {
 		return -1;
 	}

[-- Attachment #3: libselinux-load.patch --]
[-- Type: text/x-patch, Size: 2436 bytes --]

Index: libselinux/src/load_policy.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/load_policy.c,v
retrieving revision 1.10
diff -u -p -r1.10 load_policy.c
--- libselinux/src/load_policy.c	11 Oct 2005 20:09:53 -0000	1.10
+++ libselinux/src/load_policy.c	13 Oct 2005 16:32:23 -0000
@@ -37,18 +37,17 @@ int load_setlocaldefs hidden = 1;
 
 int selinux_mkload_policy(int preservebools)
 {
-	int vers = security_policyvers();
+	int vers = sepol_policy_kern_vers_max();
+	int kernvers = security_policyvers();
 	char path[PATH_MAX], **names;
 	struct stat sb;
 	size_t size;
 	void *map, *data;
 	int fd, rc = -1, *values, len, i, prot;
+	sepol_policydb_t *policydb;
+	sepol_policy_file_t *pf;
 
-	if (vers < sepol_policy_kern_vers_min())
-		return -1;
-	if (vers > sepol_policy_kern_vers_max())
-		vers = sepol_policy_kern_vers_max();
-
+search:
 	snprintf(path, sizeof(path), "%s.%d", 
 		 selinux_binary_policy_path(), vers);
 	fd = open(path, O_RDONLY);
@@ -73,13 +72,46 @@ int selinux_mkload_policy(int preservebo
 	if (map == MAP_FAILED) 
 		goto close;
 
+	if (vers > kernvers) {
+		/* Need to downgrade to kernel-supported version. */
+		if (sepol_policy_file_create(&pf))
+			goto unmap;
+		if (sepol_policydb_create(&policydb)) {
+			sepol_policy_file_free(pf);
+			goto unmap;
+		}
+		sepol_policy_file_set_mem(pf, data, size);
+		if (sepol_policydb_read(policydb, pf)) {
+			sepol_policy_file_free(pf);
+			sepol_policydb_free(policydb);
+			goto unmap;
+		}
+		if (sepol_policydb_set_vers(policydb, kernvers) ||
+		    sepol_policydb_to_image(policydb, &data, &size)) {
+			/* Downgrade failed, keep searching. */
+			sepol_policy_file_free(pf);
+			sepol_policydb_free(policydb);
+			munmap(map, sb.st_size);
+			close(fd);			
+			vers--;
+			goto search;
+		}
+		sepol_policy_file_free(pf);
+		sepol_policydb_free(policydb);
+	}
+
 	if (load_setlocaldefs) {
-		rc = sepol_genusers(data, size, selinux_users_path(), &data, &size);
+		void *olddata = data;
+		size_t oldsize = size;
+		rc = sepol_genusers(olddata, oldsize, selinux_users_path(), &data, &size);
 		if (rc < 0) {
-			/* Fall back to the base image if genusers failed. */
-			data = map;
-			size = sb.st_size;
+			/* Fall back to the prior image if genusers failed. */
+			data = olddata;
+			size = oldsize;
 			rc = 0;
+		} else {
+			if (olddata != map)
+				free(olddata);
 		}
 	}
 

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Thu, 2005-10-13 at 12:43 -0400, Stephen Smalley wrote:
> Patches below for libsemanage and libselinux (after merging Ivan's
> diffs).  libsemanage goes back to always using sh->conf->policyvers,
> which is initialized to sepol_policy_kern_vers_max() but can be changed
> in the config file, and libselinux tries to automatically downgrade.

Ok, updated the SELinux userland on a FC4 system to latest cvs with
these changes, built a policy.20 with the updated checkpolicy, moved
aside the existing policy.19 (kernel only supports 19), and tried out
load_policy.  Downgraded the policy.20 in memory to a version 19 image
and loaded it successfully into the kernel.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Thu, 2005-10-13 at 14:43 -0400, Stephen Smalley wrote:
> On Thu, 2005-10-13 at 12:43 -0400, Stephen Smalley wrote:
> > Patches below for libsemanage and libselinux (after merging Ivan's
> > diffs).  libsemanage goes back to always using sh->conf->policyvers,
> > which is initialized to sepol_policy_kern_vers_max() but can be changed
> > in the config file, and libselinux tries to automatically downgrade.
> 
> Ok, updated the SELinux userland on a FC4 system to latest cvs with
> these changes, built a policy.20 with the updated checkpolicy, moved
> aside the existing policy.19 (kernel only supports 19), and tried out
> load_policy.  Downgraded the policy.20 in memory to a version 19 image
> and loaded it successfully into the kernel.

Also, the following sequence worked as expected on the FC4 system, with
libsemanage producing a new policy.20 file and load_policy downgrading
it automatically for the kernel:
checkmodule -o base.mod policy.conf
semodule_package base.pp base.mod file_contexts
semodule -b base.pp

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Wed, 2005-10-12 at 14:06 -0400, Joshua Brindle wrote: 
> 
>>Another idea, instead of downgrading the binary (because of the 
>>possibility of an incompatible downgrade in the future) what about using 
>>the expander in libsepol to build a kernel image from the linked module? 
>>This is basically the same as the libsemanage suggestion but without 
>>using libsemanage.
> 
> 
> "Downgrading" just means setting the desired policy version for the
> binary policy image (via sepol_policydb_set_vers) and then calling
> sepol_policydb_write to write the image to memory or a file.  That is
> what libsemanage already does for expansion after calling
> sepol_expand_module (which merely expands to a policydb, not a binary
> policy image).  The policydb is not versioned in its memory
> representation; it merely carries around a version reflecting its source
> for use in subsequent generation of an image - either the version of the
> original policy image loaded into the policydb via a policydb_read or
> the latest version supported by libsepol when creating a new image via
> expand_module.  So if libsepol doesn't support a "downgrade" of a binary
> policy image to an older version due to incompatibility, it isn't going
> to support expanding to that version either.  It ultimately all comes
> through policydb_write.
> 
This is true assuming policydb_write is always going to do the version 
compatibility. Since that will probably be the case for the foreseeable 
future this is fine.

> Also, libselinux shouldn't be pulling directly from the module store, so
> it would still have some dependency on libsemanage to find that linked
> module file.  At present, libsemanage depends on libselinux and
> libsepol, libselinux depends on libsepol (for load_policy only), and
> libsepol depends on nothing (other than libc, of course).   
> 
Ok, that is true
> 
>>I think downgrading is probably 
>>the best way to do it now.
> 
> 
> I was thinking that the policy pathname could then become versionless,
> but it occurs to me that the version suffix is still useful in allowing
> libselinux to determine the policy version of the file (to decide
> whether a downgrade is required) without needing to parse it, and
> preserving the suffix also lets us unify the loading logic.  So, the
> behavior of libsemanage and libselinux would be as follows:
> - libsemanage always generates the latest policy version supported by
> libsepol (the default), writing it to a versioned policy pathname with
> that version number.  It doesn't use the kernel's policy version at all.
> 
libselinux can do a partial parse to get the version, but this obviously 
isn't as easy as reading the suffix . but this will leave old (albeit 
unused) policies laying around.

it is probably a good idea for libselinux to parse the first few fields 
(magic number, version, etc) anyway before sending an image to the kernel.

> - libselinux always searches for a versioned filename with the highest
> version <= the sepol max, opens and mmaps that file, uses the version
> suffix to determine whether a downgrade is required, and downgrades if
> necessary to the kernel's version.  This same logic applies both for the
> managed case and the unmanaged case.
> 
so, if preserve booleans is on it reads the image, sets the policyvers 
to the kernel version, sets booleans and writes the image, this makes 
sense. If preserve booleans is not on it loads the policy, decides if a 
downgrade is needed and if not sends the image directly to the kernel. 
This sounds optimal for the init case. I'm comfortable with this.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Wed, 2005-10-12 at 16:16 -0400, Joshua Brindle wrote:
> libselinux can do a partial parse to get the version, but this obviously 
> isn't as easy as reading the suffix . but this will leave old (albeit 
> unused) policies laying around.

I viewed those files as the fallback position for incompatible policy
changes, i.e. when downgrade isn't supported from the latest file,
search the older files until you find one that can be downgraded or that
doesn't require downgrading.  Yes, it may be stale, but it may be your
only option for booting the system with that older kernel.

> it is probably a good idea for libselinux to parse the first few fields 
> (magic number, version, etc) anyway before sending an image to the kernel.

libsepol could provide an interface for just parsing the header and
extracting the version, but I'm not sure why libselinux would want to do
this (unless using versionless policy pathnames where we need it to
determine the version).  The kernel will check that information anyway
and reject the load attempt if it is invalid.

> so, if preserve booleans is on it reads the image, sets the policyvers 
> to the kernel version, sets booleans and writes the image, this makes 
> sense. If preserve booleans is not on it loads the policy, decides if a 
> downgrade is needed and if not sends the image directly to the kernel. 
> This sounds optimal for the init case. I'm comfortable with this.

In the short term, this logic will likely just precede the existing
genbools_array logic, so that all version selection and downgrading
happens first, and then genbools_array just operates on the resulting
(potentially downgraded) image.  Longer term, we'll likely want the
genbools interface revised so that we can avoid the multiple
policydb_read/write sequences when a downgrade is needed and boolean
preservation is enabled, but that is just an efficiency concern.
  
-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 15:54 -0400, Stephen Smalley wrote:
> BTW, I now have everything building again (except semodule_link/expand,
> which I disabled for now), with semodule and semodule_package both using
> only shared libs, and trivial tests of semodule/semodule_package seem to
> be working as expected.

I've committed what I have so far to sourceforge (libsepol 1.9.14 which
contains a bug fix for my first commit, checkpolicy 1.27.8, libsemanage
1.3.10, policycoreutils 1.27.6), so you can grab it from there, but keep
in mind that this is work in progress, interfaces may yet change, may
cause your computer to explode, etc.

The old policydb internal headers are under include/sepol/policydb/.
New sepol/policydb.h and sepol/module.h headers contain the new (or
converted) public types and interfaces for the shared libsepol.  These
functions plus Ivan's functions have been added to libsepol.map so that
I can link semodule against the shared libsepol, although I may have
gone overboard in that list (and yes, the interfaces can still change;
this is just a snapshot).

I left struct sepol_module_package publically defined, just wrapping its
policydb pointer, since it didn't seem to expose any internals
otherwise, so libsemanage and others users can still directly reference
p->policy, p->file_contexts, and p->file_contexts_len (but not anything
within p->policy).  Let me know if you think it should be made
completely opaque (or send a patch); if so, we'll have to update its
users to use get/set methods any time they directly reference the
fields.  Note that the create method allocates the struct and creates
its policydb for it.  

I didn't do anything about handles yet.  Note that the interfaces vary
quite a bit in what they take, with some taking sepol_policydb's, others
taking sepol_policy_file's, and others taking sepol_module_package's,
and some taking a combination.  Plus Ivan's interfaces, some of which
take sepol_policydb's but others just taking his new record structures.
So it seems like a handle would have to be separated if it is going to
be universal to all of the interfaces.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Ivan Gyurdiev]

> (and yes, the interfaces can still change;
> this is just a snapshot).
>   
I think this is important, in particular with respect to my functions - 
I may still change those. I haven't decided if the key structure should 
play a role in sepol. Some of the user functions may not be needed.
> I didn't do anything about handles yet.  Note that the interfaces vary
> quite a bit in what they take, with some taking sepol_policydb's, others
> taking sepol_policy_file's, and others taking sepol_module_package's,
> and some taking a combination.  Plus Ivan's interfaces, some of which
> take sepol_policydb's but others just taking his new record structures.
> So it seems like a handle would have to be separated if it is going to
> be universal to all of the interfaces.
>   
Hmm, that's true... we should separate the handle.
Are there any other uses for a handle, besides debugging settings?



--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 18:48 -0400, Ivan Gyurdiev wrote:
> Hmm, that's true... we should separate the handle.
> Are there any other uses for a handle, besides debugging settings?

Not for libsepol, I think.  So it likely could replace the verbose,
error_buf, and error_buf_size arguments for the sepol_link_packages,
sepol_link_modules, and sepol_expand_module interfaces.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 17:17 -0400, Stephen Smalley wrote:
> I left struct sepol_module_package publically defined, just wrapping its
> policydb pointer, since it didn't seem to expose any internals
> otherwise, so libsemanage and others users can still directly reference
> p->policy, p->file_contexts, and p->file_contexts_len (but not anything
> within p->policy).  Let me know if you think it should be made
> completely opaque (or send a patch); if so, we'll have to update its
> users to use get/set methods any time they directly reference the
> fields.  Note that the create method allocates the struct and creates
> its policydb for it.  

Hi all,

Any thoughts on the above question?  If we leave it publically defined,
then users can still directly allocate/free sepol_module_package's
rather than using the provided create/free interfaces and can directly
access the policy, file_contexts, and file_context_len fields.  Do we
anticipate sepol_module_package's including other information in the
future?

Also, I wanted to note that when I introduced create/free interfaces for
sepol_module_package, I had to rename the existing interface named
"sepol_module_package_create" to "sepol_module_package_create_file".
That interface was for creating a package file from a policy file and a
file contexts file, not for creating the struct itself.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

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

On Tue, 2005-10-11 at 08:51 -0400, Stephen Smalley wrote:
> Any thoughts on the above question?  If we leave it publically defined,
> then users can still directly allocate/free sepol_module_package's
> rather than using the provided create/free interfaces and can directly
> access the policy, file_contexts, and file_context_len fields.  Do we
> anticipate sepol_module_package's including other information in the
> future?
> 
> Also, I wanted to note that when I introduced create/free interfaces for
> sepol_module_package, I had to rename the existing interface named
> "sepol_module_package_create" to "sepol_module_package_create_file".
> That interface was for creating a package file from a policy file and a
> file contexts file, not for creating the struct itself.

Patch below hides the sepol_module_package type definition within
libsepol, committed to cvs.

We still need to decide what to do about the
sepol_module_package_create_file interface to make it extensible; one
option is to discard it and require the caller to build up a
sepol_module_package struct via a create/set_xx/set_yy sequence and then
use the write interface to write the final package file.  It appears the
we would only need/want a set_file_contexts interface at present, as the
policydb is allocated by the create interface and can be extracted via
get and then populated using the other policydb interfaces (read,
expand_module, etc).  The only user of the create_file interface
presently is semodule_package.

Also need to deal with the package file format itself, i.e. versioning,
sections, etc. per the discussion on fedora-selinux-list.

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: x.diff --]
[-- Type: text/x-patch, Size: 8775 bytes --]

Index: libsemanage/ChangeLog
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/ChangeLog,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -p -r1.24 -r1.25
--- libsemanage/ChangeLog	13 Oct 2005 16:47:59 -0000	1.24
+++ libsemanage/ChangeLog	13 Oct 2005 19:09:34 -0000	1.25
@@ -1,3 +1,6 @@
+1.3.14 2005-10-13
+	* Updated to use get interfaces for hidden sepol_module_package type.
+
 1.3.13 2005-10-13
 	* Changed semanage_expand_sandbox and semanage_install_active
 	to generate/install the latest policy version supported	by libsepol
Index: libsemanage/VERSION
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/VERSION,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -p -r1.21 -r1.22
--- libsemanage/VERSION	13 Oct 2005 16:47:59 -0000	1.21
+++ libsemanage/VERSION	13 Oct 2005 19:09:34 -0000	1.22
@@ -1 +1 @@
-1.3.13
+1.3.14
Index: libsemanage/src/direct_api.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/direct_api.c,v
retrieving revision 1.6
diff -u -p -r1.6 direct_api.c
--- libsemanage/src/direct_api.c	13 Oct 2005 15:56:29 -0000	1.6
+++ libsemanage/src/direct_api.c	13 Oct 2005 19:09:49 -0000
@@ -262,7 +262,7 @@ static int semanage_direct_commit(semana
 
 	/* write the linked file contexts */
 	if ((fc_filename = semanage_path(SEMANAGE_TMP, SEMANAGE_FC)) == NULL ||
-	    write_file(sh, fc_filename, base->file_contexts, base->file_contexts_len) == -1) {
+	    write_file(sh, fc_filename, sepol_module_package_get_file_contexts(base), sepol_module_package_get_file_contexts_len(base)) == -1) {
 		goto cleanup;
 	}
 
Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsemanage/src/semanage_store.c,v
retrieving revision 1.8
diff -u -p -r1.8 semanage_store.c
--- libsemanage/src/semanage_store.c	13 Oct 2005 16:47:59 -0000	1.8
+++ libsemanage/src/semanage_store.c	13 Oct 2005 19:09:49 -0000
@@ -1192,7 +1192,7 @@ int semanage_expand_sandbox(semanage_han
 	if (sepol_policydb_create(&out)) {
 		return -1;
 	}
-	if (sepol_expand_module(base->policy, out, 0,
+	if (sepol_expand_module(sepol_module_package_get_policy(base), out, 0,
 				buffer, sizeof(buffer)) == -1) {
 		if (*buffer != '\0')
 			ERR(sh, "%s", buffer);
Index: libsepol/ChangeLog
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/ChangeLog,v
retrieving revision 1.77
retrieving revision 1.78
diff -u -p -r1.77 -r1.78
--- libsepol/ChangeLog	13 Oct 2005 15:56:28 -0000	1.77
+++ libsepol/ChangeLog	13 Oct 2005 19:09:33 -0000	1.78
@@ -1,3 +1,6 @@
+1.9.17 2005-10-13
+	* Hid sepol_module_package type definition, and added get interfaces.
+
 1.9.16 2005-10-13
 	* Merged new callback-based error reporting system from Ivan
 	Gyurdiev.
Index: libsepol/VERSION
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/VERSION,v
retrieving revision 1.70
retrieving revision 1.71
diff -u -p -r1.70 -r1.71
--- libsepol/VERSION	13 Oct 2005 15:56:28 -0000	1.70
+++ libsepol/VERSION	13 Oct 2005 19:09:33 -0000	1.71
@@ -1 +1 @@
-1.9.16
+1.9.17
Index: libsepol/include/sepol/module.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/include/sepol/module.h,v
retrieving revision 1.3
diff -u -p -r1.3 module.h
--- libsepol/include/sepol/module.h	7 Oct 2005 20:10:14 -0000	1.3
+++ libsepol/include/sepol/module.h	13 Oct 2005 19:09:49 -0000
@@ -7,11 +7,7 @@
 
 #include <sepol/policydb.h> 
 
-struct sepol_module_package {
-	sepol_policydb_t *policy;
-	char 		*file_contexts;
-	uint32_t	file_contexts_len;
-};
+struct sepol_module_package;
 typedef struct sepol_module_package sepol_module_package_t;
 
 /* Module package public interfaces. */
@@ -20,6 +16,10 @@ extern int sepol_module_package_create(s
 
 extern void sepol_module_package_free(sepol_module_package_t *p);
 
+extern char *sepol_module_package_get_file_contexts(sepol_module_package_t *p);
+extern uint32_t sepol_module_package_get_file_contexts_len(sepol_module_package_t *p);
+extern sepol_policydb_t *sepol_module_package_get_policy(sepol_module_package_t *p);
+
 extern int sepol_link_packages(sepol_module_package_t *base,
                                sepol_module_package_t **modules, 
 			       int num_modules,
Index: libsepol/include/sepol/policydb/module.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/include/sepol/policydb/module.h,v
retrieving revision 1.1
diff -u -p -r1.1 module.h
--- libsepol/include/sepol/policydb/module.h	7 Oct 2005 20:10:14 -0000	1.1
+++ libsepol/include/sepol/policydb/module.h	13 Oct 2005 19:09:49 -0000
@@ -30,6 +30,12 @@
 
 #define SEPOL_MODULE_PACKAGE_MAGIC 0xf97cff8e
 
+struct sepol_module_package {
+	sepol_policydb_t *policy;
+	char 		*file_contexts;
+	uint32_t	file_contexts_len;
+};
+
 extern int sepol_module_package_init(sepol_module_package_t *p);
 
 #endif
Index: libsepol/src/libsepol.map
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/src/libsepol.map,v
retrieving revision 1.7
diff -u -p -r1.7 libsepol.map
--- libsepol/src/libsepol.map	13 Oct 2005 15:56:29 -0000	1.7
+++ libsepol/src/libsepol.map	13 Oct 2005 19:09:49 -0000
@@ -10,6 +10,7 @@
 	sepol_policydb_read; sepol_policydb_write;
 	sepol_policydb_from_image; sepol_policydb_to_image;
 	sepol_module_package_create; sepol_module_package_free; 
+	sepol_module_package_get_file_contexts; sepol_module_package_get_file_contexts_len; sepol_module_package_get_policy;
 	sepol_link_packages; 
 	sepol_module_package_read; sepol_module_package_info;
 	sepol_module_package_write; sepol_module_package_create_file;
Index: libsepol/src/module.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/src/module.c,v
retrieving revision 1.4
diff -u -p -r1.4 module.c
--- libsepol/src/module.c	11 Oct 2005 18:18:13 -0000	1.4
+++ libsepol/src/module.c	13 Oct 2005 19:09:49 -0000
@@ -54,6 +54,22 @@ void sepol_module_package_free(sepol_mod
         free(p);
 }
 
+char *sepol_module_package_get_file_contexts(sepol_module_package_t *p)
+{
+	return p->file_contexts;
+}
+
+
+uint32_t sepol_module_package_get_file_contexts_len(sepol_module_package_t *p)
+{
+	return p->file_contexts_len;
+}
+ 
+sepol_policydb_t *sepol_module_package_get_policy(sepol_module_package_t *p)
+{
+	return p->policy;
+}
+
 /* Append each of the file contexts from each module to the base
  * policy's file context.  'base_context' will be reallocated to a
  * larger size (and thus it is an in/out reference
Index: policycoreutils/ChangeLog
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/policycoreutils/ChangeLog,v
retrieving revision 1.138
retrieving revision 1.139
diff -u -p -r1.138 -r1.139
--- policycoreutils/ChangeLog	13 Oct 2005 17:57:55 -0000	1.138
+++ policycoreutils/ChangeLog	13 Oct 2005 19:09:34 -0000	1.139
@@ -1,3 +1,6 @@
+1.27.11 2005-10-13
+	* Updated semodule_expand to use get interfaces for hidden sepol_module_package type.
+
 1.27.10 2005-10-13
 	* Merged newrole and run_init pam config patches from Dan Walsh (Red Hat).
 
Index: policycoreutils/VERSION
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/policycoreutils/VERSION,v
retrieving revision 1.77
retrieving revision 1.78
diff -u -p -r1.77 -r1.78
--- policycoreutils/VERSION	13 Oct 2005 17:57:55 -0000	1.77
+++ policycoreutils/VERSION	13 Oct 2005 19:09:34 -0000	1.78
@@ -1 +1 @@
-1.27.10
+1.27.11
Index: policycoreutils/semodule_expand/semodule_expand.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/policycoreutils/semodule_expand/semodule_expand.c,v
retrieving revision 1.5
diff -u -p -r1.5 semodule_expand.c
--- policycoreutils/semodule_expand/semodule_expand.c	11 Oct 2005 18:21:52 -0000	1.5
+++ policycoreutils/semodule_expand/semodule_expand.c	13 Oct 2005 19:09:49 -0000
@@ -123,7 +123,7 @@ int main(int argc, char **argv)
         }
 
         error_buf [0] = '\0';
-        if (sepol_expand_module(base->policy, out, verbose, error_buf, sizeof (error_buf))) {
+        if (sepol_expand_module(sepol_module_package_get_policy(base), out, verbose, error_buf, sizeof (error_buf))) {
         	fprintf(stderr, "Error expanding policy: %s\n", error_buf);
         	exit(1);
         }

Re: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Tue, 2005-10-11 at 08:51 -0400, Stephen Smalley wrote:
> 
>>Any thoughts on the above question?  If we leave it publically defined,
>>then users can still directly allocate/free sepol_module_package's
>>rather than using the provided create/free interfaces and can directly
>>access the policy, file_contexts, and file_context_len fields.  Do we
>>anticipate sepol_module_package's including other information in the
>>future?
>>
>>Also, I wanted to note that when I introduced create/free interfaces for
>>sepol_module_package, I had to rename the existing interface named
>>"sepol_module_package_create" to "sepol_module_package_create_file".
>>That interface was for creating a package file from a policy file and a
>>file contexts file, not for creating the struct itself.
> 
> 
> Patch below hides the sepol_module_package type definition within
> libsepol, committed to cvs.
> 
> We still need to decide what to do about the
> sepol_module_package_create_file interface to make it extensible; one
> option is to discard it and require the caller to build up a
> sepol_module_package struct via a create/set_xx/set_yy sequence and then
> use the write interface to write the final package file.  It appears the
> we would only need/want a set_file_contexts interface at present, as the
> policydb is allocated by the create interface and can be extracted via
> get and then populated using the other policydb interfaces (read,
> expand_module, etc).  The only user of the create_file interface
> presently is semodule_package.
> 
> Also need to deal with the package file format itself, i.e. versioning,
> sections, etc. per the discussion on fedora-selinux-list.
> 
It also seems like the current CLI for semanage_package is insufficient. 
If we are building support into the format and API for sections and 
other data the semanage_package options should reflect what sections are 
you are filling in with what data, something like

-f file_contexts
-m module data

and anything else added later would obviously get an argument.



--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Thu, 2005-10-13 at 18:35 -0400, Joshua Brindle wrote:
> > We still need to decide what to do about the
> > sepol_module_package_create_file interface to make it extensible; one
> > option is to discard it and require the caller to build up a
> > sepol_module_package struct via a create/set_xx/set_yy sequence and then
> > use the write interface to write the final package file.  It appears the
> > we would only need/want a set_file_contexts interface at present, as the
> > policydb is allocated by the create interface and can be extracted via
> > get and then populated using the other policydb interfaces (read,
> > expand_module, etc).  The only user of the create_file interface
> > presently is semodule_package.
> > 
> > Also need to deal with the package file format itself, i.e. versioning,
> > sections, etc. per the discussion on fedora-selinux-list.
> > 
> It also seems like the current CLI for semanage_package is insufficient. 
> If we are building support into the format and API for sections and 
> other data the semanage_package options should reflect what sections are 
> you are filling in with what data, something like
> 
> -f file_contexts
> -m module data
> 
> and anything else added later would obviously get an argument.

Does it ever make sense to build a package without a module?  If not,
then I think we can leave the module as a required argument, and only
make things like file contexts and other components option-driven.  It
might also help to in some way more clearly distinguish the output file
from the input arguments to avoid accidentally clobbering a module (I've
done that before) with semodule_package, either via explicit -o option
like checkmodule/checkpolicy (with some default output filename) or have
semodule_package refuse to clobber an existing file.
.
-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Stephen Smalley wrote:
> On Thu, 2005-10-13 at 18:35 -0400, Joshua Brindle wrote:
> 
>>>We still need to decide what to do about the
>>>sepol_module_package_create_file interface to make it extensible; one
>>>option is to discard it and require the caller to build up a
>>>sepol_module_package struct via a create/set_xx/set_yy sequence and then
>>>use the write interface to write the final package file.  It appears the
>>>we would only need/want a set_file_contexts interface at present, as the
>>>policydb is allocated by the create interface and can be extracted via
>>>get and then populated using the other policydb interfaces (read,
>>>expand_module, etc).  The only user of the create_file interface
>>>presently is semodule_package.
>>>
>>>Also need to deal with the package file format itself, i.e. versioning,
>>>sections, etc. per the discussion on fedora-selinux-list.
>>>
>>
>>It also seems like the current CLI for semanage_package is insufficient. 
>>If we are building support into the format and API for sections and 
>>other data the semanage_package options should reflect what sections are 
>>you are filling in with what data, something like
>>
>>-f file_contexts
>>-m module data
>>
>>and anything else added later would obviously get an argument.
> 
> 
> Does it ever make sense to build a package without a module?  If not,
> then I think we can leave the module as a required argument, and only
> make things like file contexts and other components option-driven. 
Sure, I was expecting it to be required, but still use an argument, it 
doesn't matter to me though

> It
> might also help to in some way more clearly distinguish the output file
> from the input arguments to avoid accidentally clobbering a module (I've
> done that before) with semodule_package, either via explicit -o option
> like checkmodule/checkpolicy (with some default output filename) or have
> semodule_package refuse to clobber an existing file.
> .

Yes, another thing I expected to do but didn't mention because it wasn't 
relavent to the file format itself. I've also clobbered modules though :)


--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-14 at 09:33 -0400, Joshua Brindle wrote:
> Sure, I was expecting it to be required, but still use an argument, it 
> doesn't matter to me though

Usually an option argument is optional ;)

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 14:30 -0400, Stephen Smalley wrote:
> - expand_module already set the output type, so we only need to adjust
> the version here (and I changed it to use the kernel's policyvers if
> available instead of the config, as that is what will be loaded by
> load_policy now).

Ok, I've added sepol_policy_kern_vers_min/max() functions to libsepol so
that libsemanage can determine the range of versions supported by the
shared libsepol.  This is now used by conf-parse to set the default to
the max and to check the config value against the range. I also revised
semanage_expand_sandbox() to check the kernel version against that range
and fall back to the conf policyvers if the kernel version is outside of
that range, so that if the kernel version is higher than the one
supported by libsepol, we can still fall back to the older version
supported by libsepol.

load_policy functions in libselinux should likely do something similar
when deciding what version to load, at least as long as it continues to
use sepol* functions to manipulate boolean settings et al.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Ivan Gyurdiev]

> Ok, I've made a first cut at the changes to libsepol and updated it and
> checkpolicy to build again, and am starting to work through libsemanage.
> As an example of what to expect, the diff below to database_direct.[ch]
> was needed to get it to compile again.
>   
Ok...

On second thought that code was likely wrong to begin with, since Tresys 
put their modules in a special package container, and mark everything 
with a special magic, and use special functions to read everything.... 
I'll get it fixed later... not testing the direct case yet.

-    policydb = (policydb_t*) malloc(sizeof(policydb_t));
-    if (!policydb)
+    if (sepol_policydb_create(&policydb, SEPOL_POLICY_KERN))

Why do I get a feeling of deja-vu when I look at this code... :) I think 
I've already tried this once - I recall Karl convincing me it policydb 
shouldn't be opaque. Checkpolicy will be confusing to fix. By the way, 
is it really necessary to specify KERN type in the create() function. I 
thought you could detect the policy type, and mark it in the policydb 
object (rather than propagating it down from the caller).


--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 11:52 -0400, Ivan Gyurdiev wrote:
> On second thought that code was likely wrong to begin with, since Tresys 
> put their modules in a special package container, and mark everything 
> with a special magic, and use special functions to read everything.... 
> I'll get it fixed later... not testing the direct case yet.
> 
> -    policydb = (policydb_t*) malloc(sizeof(policydb_t));
> -    if (!policydb)
> +    if (sepol_policydb_create(&policydb, SEPOL_POLICY_KERN))
> 
> Why do I get a feeling of deja-vu when I look at this code... :) I think 
> I've already tried this once - I recall Karl convincing me it policydb 
> shouldn't be opaque. Checkpolicy will be confusing to fix. By the way, 
> is it really necessary to specify KERN type in the create() function. I 
> thought you could detect the policy type, and mark it in the policydb 
> object (rather than propagating it down from the caller).

checkpolicy doesn't require changes, other than the updating of the
header file locations to continue using the include/sepol/policydb/*.h
ones (already done in my working copy), and adjusting for the
elimination of a global sepol_set_policyvers() (also already done in my
working copy).  I'm not trying to enable checkpolicy to use the static
libsepol.  I only want to allow libsemanage (and thus its users) to use
the shared libsepol, so that semodule* no longer require the static
libsepol and any new libsemanage-based programs likewise don't require
it.  Fairly pointless to make libsemanage a shared lib if it depends on
a static lib as a backend.  I might also convert audit2why.  Nothing in
policycoreutils should really depend on the static libsepol, IMHO.

With regard to the policy type in the create interface, I was thinking
that sepol_policydb_create should include calling policydb_init, which
presently takes the type and sets it.  However, there isn't any obvious
reason to do that there, so possibly I should drop it from the
policydb_init internal function interface as well, and just let the
type/version be initialized as appropriate elsewhere (policydb_read for
policydb's created from an existing image; expand_module for policydb's
created from module expansion; possibly via separate set functions for
newly constructed ones if we need that support in libsemanage).

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 12:01 -0400, Stephen Smalley wrote:
> checkpolicy doesn't require changes, other than the updating of the
> header file locations to continue using the include/sepol/policydb/*.h
> ones (already done in my working copy), and adjusting for the
> elimination of a global sepol_set_policyvers() (also already done in my
> working copy).  I'm not trying to enable checkpolicy to use the static
> libsepol.  

Sorry, s/static/shared/ above.

> I only want to allow libsemanage (and thus its users) to use
> the shared libsepol, so that semodule* no longer require the static
> libsepol and any new libsemanage-based programs likewise don't require
> it.  Fairly pointless to make libsemanage a shared lib if it depends on
> a static lib as a backend.  I might also convert audit2why.  Nothing in
> policycoreutils should really depend on the static libsepol, IMHO.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Ivan Gyurdiev]

If you're changing the shared API, this might be a good time to 
introduce a handle object: sepol_handle_t, and pass it as an argument to 
the shared functions. It could contain a callback function, and the 
argument for it. You could also hide the policydb and/or policy_file in 
it, unless you prefer to keep them as a separate argument - I'm not sure 
which is better. If hidden within the handle, they could be NULL until 
initialized.

--
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: [ SEMANAGE ] [ SEPOL ] More database work

[Stephen Smalley]

On Fri, 2005-10-07 at 12:01 -0400, Stephen Smalley wrote:
> With regard to the policy type in the create interface, I was thinking
> that sepol_policydb_create should include calling policydb_init, which
> presently takes the type and sets it.  However, there isn't any obvious
> reason to do that there, so possibly I should drop it from the
> policydb_init internal function interface as well, and just let the
> type/version be initialized as appropriate elsewhere (policydb_read for
> policydb's created from an existing image; expand_module for policydb's
> created from module expansion; possibly via separate set functions for
> newly constructed ones if we need that support in libsemanage).

Ok, I dropped policy_type from sepol_policydb_create (and policydb_init
internally), and added sepol_policydb_set_typevers (set policy type and
automatically set policy version to max supported for the type) and
sepol_policydb_set_vers (change policy version to a different value than
the max) for explicit manipulation of the type and version.

-- 
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: [ SEMANAGE ] [ SEPOL ] More database work

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>> Ok, I've made a first cut at the changes to libsepol and updated it and
>> checkpolicy to build again, and am starting to work through libsemanage.
>> As an example of what to expect, the diff below to database_direct.[ch]
>> was needed to get it to compile again.
>>   
> 
> Ok...
> 
> On second thought that code was likely wrong to begin with, since Tresys 
> put their modules in a special package container, and mark everything 
> with a special magic, and use special functions to read everything.... 
> I'll get it fixed later... not testing the direct case yet.

It isn't just a special package container. The actual policy format for 
modules is different because it retains more information than the binary 
format. The 'package' format is the module with the file contexts 
prepended to it. The module format and the file contexts are needed to 
properly link and expand a policy.

Joshua

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