[ SEMANAGE ] Replace semanage debugging system

[ SEMANAGE ] Replace semanage debugging system

[Ivan Gyurdiev]

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

The attached patch replaces the current semanage debugging system, with 
one based on callbacks. It breaks API, so the libsemanage.map file needs 
to be updated, but I'm not sure how to do that properly, so I left that 
part out...

Functions removed:
semanage_strerror

Functions that need to be exposed:
Everything in include/semanage/debug.h

Changes since last time we discussed this:
- added argument level (which is set in macros WARN, ERR, and INFO)
- added argument channel (auto-set by the macros to "libsemanage") - 
this will be helpful to separate out sepol messages
- replaced use of __FUNCTION__ with __func__, which should not be GNU 
specific
- function name argument is now auto-set by macros, so we don't have to 
worry about it.
- replaced existing usage of semanage_write_error with those macros

Unrelated changes in this patch:
- Renames policy_connection.h to policy.h
- moves conn into policy.h, which is a good place for it, I think.
- some #if0'ed code paths.

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

diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/include/semanage/debug.h exp/libsemanage/include/semanage/debug.h
--- old/libsemanage/include/semanage/debug.h	1969-12-31 19:00:00.000000000 -0500
+++ exp/libsemanage/include/semanage/debug.h	2005-10-11 02:06:34.000000000 -0400
@@ -0,0 +1,62 @@
+/* Author: Joshua Brindle <jbrindle@tresys.com>
+ *         Jason Tang     <jtang@tresys.com>
+ *         Ivan Gyurdiev  <ivg2@cornell.edu>
+ *
+ * Copyright (C) 2005 Tresys Technology, LLC
+ * Copyright (C) 2005 Red Hat Inc.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef _SEMANAGE_DEBUG_H_
+#define _SEMANAGE_DEBUG_H_
+
+#include <semanage/handle.h>
+
+struct semanage_message;
+typedef struct semanage_message semanage_message_t;
+
+#define SEMANAGE_MSG_ERR  1
+#define SEMANAGE_MSG_WARN 2
+#define SEMANAGE_MSG_INFO 3
+
+extern const char* semanage_msg_get_message(
+	semanage_message_t* msg);
+
+extern int semanage_msg_get_level(
+	semanage_message_t* msg);
+
+extern const char* semanage_msg_get_channel(
+	semanage_message_t* msg);
+
+extern const char* semanage_msg_get_fname(
+	semanage_message_t* msg);
+
+extern void semanage_msg_free(
+	semanage_message_t* msg);
+
+/* Set the messaging callback. 
+ * By the default, the callback will print
+ * the message on standard output, in a 
+ * particular format. Passing NULL here
+ * indicates that messaging should be suppressed */
+extern void semanage_msg_set_callback(
+	semanage_handle_t* handle,
+	void (*callback) (
+		void* varg,
+		semanage_message_t* msg),
+	void* callback_arg);
+
+#endif 
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/include/semanage/handle.h exp/libsemanage/include/semanage/handle.h
--- old/libsemanage/include/semanage/handle.h	2005-09-28 15:56:00.000000000 -0400
+++ exp/libsemanage/include/semanage/handle.h	2005-10-11 01:59:36.000000000 -0400
@@ -51,14 +51,6 @@
  * error. */
 int semanage_disconnect(semanage_handle_t *);
 
-/* Return a string describing the most recently encountered error
- * associated with a semanage_handle_t.	 The returned string must not
- * be modified by the caller.  Be aware that this string is not
- * persistent; future calls to this library may alter the buffer
- * contents, so make a copy of it if necessary.
- */
-const char *semanage_strerror(semanage_handle_t *);
-
 /* Attempt to obtain a transaction lock on the manager.	 If another
  * process has the lock then this function may block, depending upon
  * the timeout value in the handle.
@@ -75,18 +67,4 @@
  */
 int semanage_commit(semanage_handle_t *);
 
-/* META NOTES
- *
- * All of the other interfaces expect a semanage_handle as its first
- * parameter.  If an error occurs then the function returns a negative
- * value.  Call semanage_strerror() to retrieve a string that fully
- * describes the error.
- *
- * For all functions a non-negative number indicates success. For query 
- * fucntions a >=0 returned value is the "policy sequence number".  This
- * number keeps tracks of policy revisions and is used to detect if
- * one semanage client has committed policy changes while another is
- * still connected.
- */
-
 #endif
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/include/semanage/semanage.h exp/libsemanage/include/semanage/semanage.h
--- old/libsemanage/include/semanage/semanage.h	2005-09-28 15:56:00.000000000 -0400
+++ exp/libsemanage/include/semanage/semanage.h	2005-10-11 01:59:45.000000000 -0400
@@ -23,6 +23,7 @@
 
 #include <semanage/handle.h>
 #include <semanage/modules.h>
+#include <semanage/debug.h>
 
 /*
  * Explicit libsemanage initialization.
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/database_direct.c exp/libsemanage/src/database_direct.c
--- old/libsemanage/src/database_direct.c	2005-10-07 23:43:17.000000000 -0400
+++ exp/libsemanage/src/database_direct.c	2005-10-07 23:55:28.000000000 -0400
@@ -3,6 +3,7 @@
 #define DBASE_DEFINED
 
 #include <stdlib.h>
+#include <string.h>
 #include <stdio.h>
 #include <fcntl.h>
 #include <sys/types.h>
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/debug.c exp/libsemanage/src/debug.c
--- old/libsemanage/src/debug.c	2005-09-30 16:19:07.000000000 -0400
+++ exp/libsemanage/src/debug.c	2005-10-11 02:21:43.000000000 -0400
@@ -1,7 +1,9 @@
 /* Author: Joshua Brindle <jbrindle@tresys.co
  *         Jason Tang     <jtang@tresys.com>
+ *         Ivan Gyurdiev  <ivg2@cornell.edu> 
  *
  * Copyright (C) 2004-2005 Tresys Technology, LLC
+ * Copyright (C) 2005 Red Hat Inc.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -19,20 +21,109 @@
  */
 
 #include <stdarg.h>
+#include <stdlib.h>
 #include <stdio.h>
 #include "handle.h"
 #include "debug.h"
 
-/* FIXME: redesign with callbacks ? */
+#define SEMANAGE_ERRBUFSZ 1024
+
+struct semanage_message {
+	char message[SEMANAGE_ERRBUFSZ];
+	int level;
+	const char* channel;
+	const char* fname;
+};
+
+const char* semanage_msg_get_message(semanage_message_t* msg) {
+	return msg->message;
+};
+
+int semanage_msg_get_level(semanage_message_t* msg) {
+	return msg->level;
+}
+
+const char* semanage_msg_get_channel(semanage_message_t* msg) {
+	return msg->channel;
+}
+
+const char* semanage_msg_get_fname(semanage_message_t* msg) {
+	return msg->fname;
+}
+
+void semanage_msg_free(semanage_message_t* msg) {
+	if (!msg)
+		return;
+
+	free(msg);
+}
+
+void msg_default_handler(
+	void* varg,
+	semanage_message_t* msg) {
+
+	FILE* stream = NULL;
+
+	switch(semanage_msg_get_level(msg)) {
+	
+		case SEMANAGE_MSG_ERR:
+		case SEMANAGE_MSG_WARN:
+			stream = stderr;
+			break;
+		case SEMANAGE_MSG_INFO:
+		default:
+			stream = stdout;
+			break;
+	}
+
+	fprintf(stream, "%s.%s: %s\n",
+		semanage_msg_get_channel(msg),
+		semanage_msg_get_fname(msg),
+		semanage_msg_get_message(msg));
+
+	semanage_msg_free(msg);
+	varg = NULL;
+}
 
-/* Write an error message to the current error buffer, up to the
- * buffer's specified size. */
 #ifdef __GNUC__
-__attribute__ ((format (printf, 2, 3)))
+__attribute__ ((format (printf, 5, 6)))
 #endif
-void semanage_write_error(semanage_handle_t *sh, char *fmt, ...) {
+void msg_write(
+	semanage_handle_t* handle,
+	int level,
+	const char* channel,
+	const char* fname,
+	char* fmt,
+	...) {
+
+	semanage_message_t* msg;
+
+	if (!handle->callback)
+		return;
+
+	msg = (semanage_message_t*) malloc(sizeof(semanage_message_t));
+	if (!msg)
+		return;
+
+	msg->fname = fname;
+	msg->channel = channel;
+	msg->level = level;
+
 	va_list ap;
 	va_start(ap, fmt);
-	vsnprintf(sh->err_buf, SEMANAGE_ERRBUFSZ, fmt, ap);
+	vsnprintf(msg->message, SEMANAGE_ERRBUFSZ, fmt, ap);
 	va_end(ap);
+
+	handle->callback(handle->callback_arg, msg);
+}
+
+extern void semanage_msg_set_callback(
+	semanage_handle_t* handle,
+	void (*callback) (
+		void* varg,
+		semanage_message_t* msg),
+	void* callback_arg) {
+
+	handle->callback = callback;
+	handle->callback_arg = callback_arg;
 }
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/debug.h exp/libsemanage/src/debug.h
--- old/libsemanage/src/debug.h	2005-09-30 16:19:07.000000000 -0400
+++ exp/libsemanage/src/debug.h	2005-10-11 02:31:15.000000000 -0400
@@ -20,20 +20,42 @@
  *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
-#ifndef _SEPOL_INTERNAL_DEBUG_H_
-#define _SEPOL_INTERNAL_DEBUG_H_
+#ifndef _SEMANAGE_INTERNAL_DEBUG_H_
+#define _SEMANAGE_INTERNAL_DEBUG_H_
 
 #include "handle.h"
+#include <stdio.h>
+#include <semanage/debug.h>
 
 #define STATUS_SUCCESS 0
 #define STATUS_ERR -1
 #define STATUS_NODATA 1
 
-/* FIXME: redesign with level argument ? */
+#define ERR(handle, ...) \
+	msg_write(handle, SEMANAGE_MSG_ERR, "libsemanage", \
+	__func__, __VA_ARGS__)
+
+#define INFO(handle, fmt, ...) \
+	msg_write(handle, SEMANAGE_MSG_INFO, "libsemanage", \
+	__func__, __VA_ARGS__)
+
+#define WARN(handle, fmt, ...) \
+	msg_write(handle, SEMANAGE_MSG_WARN, "libsemanage", \
+	__func__, __VA_ARGS__)
 
 #ifdef __GNUC__
-__attribute__ ((format (printf, 2, 3)))
+__attribute__ ((format (printf, 5, 6)))
 #endif
-extern void semanage_write_error(semanage_handle_t *sh, char *fmt, ...);
+extern void msg_write(
+	semanage_handle_t* handle,
+	int level,
+	const char* channel,
+	const char* fname,
+	char* fmt,
+	...);
+
+extern void msg_default_handler(
+	void* varg,
+	semanage_message_t* msg);
 
 #endif 
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/direct_api.c exp/libsemanage/src/direct_api.c
--- old/libsemanage/src/direct_api.c	2005-10-07 18:37:59.000000000 -0400
+++ exp/libsemanage/src/direct_api.c	2005-10-11 02:34:29.000000000 -0400
@@ -33,7 +33,7 @@
 #include "modules.h"
 #include "direct_api.h"
 #include "semanage_store.h"
-#include "policy_connection.h"
+#include "policy.h"
 
 static void semanage_direct_destroy(semanage_handle_t *sh);
 static int semanage_direct_disconnect(semanage_handle_t *sh);
@@ -89,7 +89,7 @@
 	if (sh->is_in_transaction) {
 		/* destroy sandbox */
 		if (semanage_remove_directory(semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
-			semanage_write_error(sh, "Could not cleanly remove sandbox %s.", semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
+			ERR(sh, "Could not cleanly remove sandbox %s.", semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
 			return -1;
 		}
 		semanage_release_trans_lock(sh);
@@ -129,7 +129,7 @@
 	*module_name = *version = *filename = NULL;
 	
 	if (sepol_policy_file_create(&pf)) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		return -1;
 	}
 	sepol_policy_file_set_mem(pf, module_data, data_len);
@@ -138,19 +138,19 @@
 	    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.");
+		ERR(sh, "Could not parse module data.");
 		return -2;
 	}
 	sepol_policy_file_free(pf);
 	if (file_type != SEPOL_POLICY_MOD) {
-		semanage_write_error(sh, "Data did not represent a module.");
+		ERR(sh, "Data did not represent a module.");
 		return -2;
 	}
 	if ((module_path = semanage_path(SEMANAGE_TMP, SEMANAGE_MODULES)) == NULL) {
 		return -1;
 	}
 	if (asprintf(filename, "%s/%s.pp", module_path, *module_name) == -1) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		return -1;
 	}
 	return 0;
@@ -167,7 +167,7 @@
 	int file_type;
 
 	if (sepol_policy_file_create(&pf)) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		return -1;
 	}
 	sepol_policy_file_set_mem(pf, module_data, data_len);
@@ -176,14 +176,14 @@
 	    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.");
+		ERR(sh, "Could not parse base module data.");
 		return -2;
 	}
 	sepol_policy_file_free(pf);
 	free(module_name);
 	free(version);
 	if (file_type != SEPOL_POLICY_BASE) {
-		semanage_write_error(sh, "Data did not represent a module.");
+		ERR(sh, "Data did not represent a module.");
 		return -2;
 	}
 	return 0;
@@ -195,11 +195,11 @@
 		      const char *filename, char *data, size_t num_bytes) {
 	int out;
 	if ((out = open(filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR)) == -1) {
-		semanage_write_error(sh, "Could not open %s for writing.", filename);
+		ERR(sh, "Could not open %s for writing.", filename);
 		return -1;
 	}
 	if (write(out, data, num_bytes) == -1) {
-		semanage_write_error(sh, "Error while writing to %s.", filename);
+		ERR(sh, "Error while writing to %s.", filename);
 		close(out);
 		return -1;
 	}
@@ -217,19 +217,19 @@
 	FILE *outfile;
 	int retval;
 	if (sepol_policy_file_create(&pf)) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(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);
+		ERR(sh, "Could not open %s for writing.", filename);
 		return -1;
 	}
 	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);
+		ERR(sh, "Error while writing module to %s.", filename);
 		return -1;
 	}
 	return 0;
@@ -268,11 +268,23 @@
 		goto cleanup;
 	}
 
-	/* expand and verify the resulting policy */
-	if (semanage_expand_sandbox(sh, base) < 0 ||
-	    semanage_verify_kernel(sh) != 0) {
+	/* Expand the resulting policy */
+	if (semanage_expand_sandbox(sh, base) < 0)
+		goto cleanup;
+
+#if 0
+	/* Link components into base policy */
+	if (semanage_base_merge_components(sh, NULL /* FIXME */) < 0)
+		goto cleanup;
+
+	/* Commit changes to components */
+	if (semanage_commit_components(sh) < 0)
+		goto cleanup;
+#endif
+
+	/* Verify policy */
+	if (semanage_verify_kernel(sh) != 0)
 		goto cleanup;
-	}
 
 	retval = semanage_install_sandbox(sh);
 
@@ -346,14 +358,14 @@
 				break;
 			}
 			else {
-				semanage_write_error(sh, "Previous module %s is same or newer.", module_name);
+				ERR(sh, "Previous module %s is same or newer.", module_name);
 				retval = -4;
 				goto cleanup;
 			}
 		}
 	}
 	if (retval == -4) {
-		semanage_write_error(sh, "There does not already exist a module named %s.", module_name);
+		ERR(sh, "There does not already exist a module named %s.", module_name);
 		goto cleanup;
 	}
 	if (write_file(sh, filename, data, data_len) == -1) {
@@ -409,7 +421,7 @@
 	for (i = 0; i < num_mod_files; i++) {
 		char *base = strrchr(module_filenames[i], '/');
 		if (base == NULL) {
-			semanage_write_error(sh, "Could not read module names.");
+			ERR(sh, "Could not read module names.");
 			retval = -2;
 			goto cleanup;
 		}
@@ -417,14 +429,14 @@
 		if (memcmp(module_name, base, name_len) == 0 &&
 		    strcmp(base + name_len, ".pp") == 0) {
 			if (unlink(module_filenames[i]) == -1) {
-				semanage_write_error(sh, "Could not remove module file %s.", module_filenames[i]);
+				ERR(sh, "Could not remove module file %s.", module_filenames[i]);
 				retval = -2;
 			}
 			retval = 0;
 			goto cleanup;
 		}
 	}
-	semanage_write_error(sh, "Module %s was not found.", module_name);
+	ERR(sh, "Module %s was not found.", module_name);
 	retval = -2;				/* module not found */
  cleanup:
 	for (i = 0; module_filenames != NULL && i < num_mod_files; i++) {
@@ -463,12 +475,12 @@
 	}
 
 	if (sepol_policy_file_create(&pf)) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		goto cleanup;
 	}
 	
 	if ((*modinfo = calloc(num_mod_files, sizeof(**modinfo))) == NULL) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		goto cleanup;
 	}
 	
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/handle.c exp/libsemanage/src/handle.c
--- old/libsemanage/src/handle.c	2005-10-06 16:43:33.000000000 -0400
+++ exp/libsemanage/src/handle.c	2005-10-11 02:21:29.000000000 -0400
@@ -41,11 +41,11 @@
 #define SEMANAGE_COMMIT_READ_WAIT 5
 
 semanage_handle_t *semanage_handle_create(void) {
-	semanage_handle_t *sh;
-	const char *conf_name;
+	semanage_handle_t *sh = NULL;
+	const char *conf_name = NULL;
 
 	/* Allocate handle */
-	if ((sh = calloc(1, sizeof(*sh))) == NULL)
+	if ((sh = calloc(1, sizeof(semanage_handle_t))) == NULL)
 		goto omem;
 
 	/* Policy root */
@@ -66,6 +66,10 @@
 	/* Set timeout: some default value for now, later use config */
 	sh->timeout = SEMANAGE_COMMIT_READ_WAIT; 
 
+	/* Set callback */
+	sh->callback = msg_default_handler;
+	sh->callback_arg = NULL;
+
 	/* Configure object databases 
 	 * Hardcore DATA FILE backend for now */
 	if (user_file_dbase_init(&sh->dbase[DBASE_USERS]) < 0)
@@ -130,19 +134,12 @@
 	free(sh);
 }
 
-const char *semanage_strerror(semanage_handle_t *sh) {
-	if (sh == NULL) {
-		return "Could not parse semange.conf or out of memory.";
-	}
-	return sh->err_buf;
-}
-
 /********************* public transaction functions *********************/
 
 int semanage_begin_transaction(semanage_handle_t *sh) {
 	assert(sh != NULL && sh->funcs != NULL && sh->funcs->begin_trans != NULL);
 	if (!sh->is_connected) {
-		semanage_write_error(sh, "Not connected.");
+		ERR(sh, "Not connected.");
 		return -1;
 	}
 	if (sh->is_in_transaction) {
@@ -160,7 +157,7 @@
 	int retval;
 	assert(sh != NULL && sh->funcs != NULL && sh->funcs->commit != NULL);
 	if (!sh->is_in_transaction) {
-		semanage_write_error(sh, "Will not commit because caller does not have a tranaction lock yet.");
+		ERR(sh, "Will not commit because caller does not have a tranaction lock yet.");
 		return -1;
 	}
 	retval = sh->funcs->commit(sh);
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/handle.h exp/libsemanage/src/handle.h
--- old/libsemanage/src/handle.h	2005-10-04 10:51:22.000000000 -0400
+++ exp/libsemanage/src/handle.h	2005-10-06 08:02:08.000000000 -0400
@@ -25,19 +25,21 @@
 
 #include <stddef.h>
 #include <semanage/handle.h>
+#include <semanage/debug.h>
 #include "modules.h"
 #include "semanage_conf.h"
-#include "policy_connection.h"
+#include "policy.h"
 #include "database.h"
 
 struct semanage_handle {
 	int con_id;             /* Connection ID */
 	int policy_serial;      /* Policy serial number at connect time */
 
-	/* Error management */
-	/* FIXME: re-design error system using callbacks (?) */
-#define SEMANAGE_ERRBUFSZ 1024
-	char err_buf[SEMANAGE_ERRBUFSZ];
+	/* Error callback */
+	void (*callback) (
+		void* varg,
+		semanage_message_t* msg);
+	void* callback_arg;
 
 	/* one of these connections will actually be used while
 	 * working with the module store -- the particular one if
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/modules.c exp/libsemanage/src/modules.c
--- old/libsemanage/src/modules.c	2005-09-30 16:19:07.000000000 -0400
+++ exp/libsemanage/src/modules.c	2005-10-11 02:21:08.000000000 -0400
@@ -37,11 +37,11 @@
 int semanage_module_install(semanage_handle_t *sh,
 			    char *module_data, size_t data_len) {
 	if (sh->funcs->install == NULL) {
-		semanage_write_error(sh, "No install function defined for this connection type.");
+		ERR(sh, "No install function defined for this connection type.");
 		return -1;
 	}
 	else if (!sh->is_connected) {
-		semanage_write_error(sh, "Not connected.");
+		ERR(sh, "Not connected.");
 		return -1;
 	}
 	else if (!sh->is_in_transaction) {
@@ -55,11 +55,11 @@
 int semanage_module_upgrade(semanage_handle_t *sh,
 			    char *module_data, size_t data_len) {
 	if (sh->funcs->upgrade == NULL) {
-		semanage_write_error(sh, "No upgrade function defined for this connection type.");
+		ERR(sh, "No upgrade function defined for this connection type.");
 		return -1;
 	}
 	else if (!sh->is_connected) {
-		semanage_write_error(sh, "Not connected.");
+		ERR(sh, "Not connected.");
 		return -1;
 	}
 	else if (!sh->is_in_transaction) {
@@ -73,11 +73,11 @@
 int semanage_module_install_base(semanage_handle_t *sh,
 				 char *module_data, size_t data_len) {
 	if (sh->funcs->install_base == NULL) {
-		semanage_write_error(sh, "No install base function defined for this connection type.");
+		ERR(sh, "No install base function defined for this connection type.");
 		return -1;
 	}
 	else if (!sh->is_connected) {
-		semanage_write_error(sh, "Not connected.");
+		ERR(sh, "Not connected.");
 		return -1;
 	}
 	else if (!sh->is_in_transaction) {
@@ -91,11 +91,11 @@
 int semanage_module_remove(semanage_handle_t *sh,
 			   char *module_name) {
 	if (sh->funcs->remove == NULL) {
-		semanage_write_error(sh, "No remove function defined for this connection type.");
+		ERR(sh, "No remove function defined for this connection type.");
 		return -1;
 	}
 	else if (!sh->is_connected) {
-		semanage_write_error(sh, "Not connected.");
+		ERR(sh, "Not connected.");
 		return -1;
 	}
 	else if (!sh->is_in_transaction) {
@@ -109,11 +109,11 @@
 int semanage_module_list(semanage_handle_t *sh,
 			 semanage_module_info_t **modinfo, int *num_modules) {
 	if (sh->funcs->list == NULL) {
-		semanage_write_error(sh, "No list function defined for this connection type.");
+		ERR(sh, "No list function defined for this connection type.");
 		return -1;
 	}
 	else if (!sh->is_connected) {
-		semanage_write_error(sh, "Not connected.");
+		ERR(sh, "Not connected.");
 		return -1;
 	}
 	return sh->funcs->list(sh, modinfo, num_modules);
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/modules.h exp/libsemanage/src/modules.h
--- old/libsemanage/src/modules.h	2005-09-30 16:19:07.000000000 -0400
+++ exp/libsemanage/src/modules.h	2005-10-11 02:43:10.000000000 -0400
@@ -23,11 +23,6 @@
 
 #include <semanage/modules.h>
 
-struct semanage_module_conn {
-	int translock_file_fd;
-	int readlock_file_fd;
-};
-
 struct semanage_module_info {
 	char *name;             /* Key */
 	char *version;
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/policy_connection.h exp/libsemanage/src/policy_connection.h
--- old/libsemanage/src/policy_connection.h	2005-10-04 10:51:22.000000000 -0400
+++ exp/libsemanage/src/policy_connection.h	1969-12-31 19:00:00.000000000 -0500
@@ -1,55 +0,0 @@
-/* Author: Joshua Brindle <jbrindle@tresys.com>
- *         Jason Tang     <jtang@tresys.com>
- *
- * Copyright (C) 2005 Tresys Technology, LLC
- * Copyright (C) 2005 Red Hat Inc.
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Lesser General Public
- *  License as published by the Free Software Foundation; either
- *  version 2.1 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Lesser General Public License for more details.
- *
- *  You should have received a copy of the GNU Lesser General Public
- *  License along with this library; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
- */
-
-#ifndef _SEMANAGE_POLICY_CONNECTION_INTERNAL_H_
-#define _SEMANAGE_POLICY_CONNECTION_INTERNAL_H_
-
-struct semanage_policy_table {
-
-	/* Destroy a connection */	
-        void (*destroy)(semanage_handle_t *);
-
-	/* Disconnect from policy */
-        int (*disconnect)(semanage_handle_t *);
-
-	/* Begin a policy transaction */
-        int (*begin_trans)(semanage_handle_t *);
-
-	/* Commit a policy transaction */
-        int (*commit)(semanage_handle_t *);
-
-	/* Install a policy module */
-        int (*install)(semanage_handle_t *, char *, size_t);
-
-	/* Upgrade a policy module */
-        int (*upgrade)(semanage_handle_t *, char *, size_t);
-
-	/* Remove a policy module */
-	int (*remove)(semanage_handle_t *, char *);
-
-	/* List policy modules */
-	int (*list)(semanage_handle_t *, semanage_module_info_t **, int *);
-
-	/* Install base policy */
-        int (*install_base)(semanage_handle_t *, char *, size_t);
-};
-
-#endif
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/policy.h exp/libsemanage/src/policy.h
--- old/libsemanage/src/policy.h	1969-12-31 19:00:00.000000000 -0500
+++ exp/libsemanage/src/policy.h	2005-10-11 02:46:00.000000000 -0400
@@ -0,0 +1,77 @@
+/* Author: Joshua Brindle <jbrindle@tresys.com>
+ *         Jason Tang     <jtang@tresys.com>
+ *
+ * Copyright (C) 2005 Tresys Technology, LLC
+ * Copyright (C) 2005 Red Hat Inc.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef _SEMANAGE_POLICY_INTERNAL_H_
+#define _SEMANAGE_POLICY_INTERNAL_H_
+
+#include "modules.h"
+
+/* Circular dependency */
+struct semanage_handle;
+
+/* Connection Locking */
+struct semanage_module_conn {
+	int translock_file_fd;
+	int readlock_file_fd;
+};
+
+/* Backend dependent portion */
+struct semanage_policy_table {
+
+	/* Destroy a connection */	
+        void (*destroy)(struct semanage_handle*);
+
+	/* Disconnect from policy */
+        int (*disconnect)(struct semanage_handle*);
+
+	/* Begin a policy transaction */
+        int (*begin_trans)(struct semanage_handle*);
+
+	/* Commit a policy transaction */
+        int (*commit)(struct semanage_handle*);
+
+	/* Install a policy module */
+        int (*install)(struct semanage_handle*, char *, size_t);
+
+	/* Upgrade a policy module */
+        int (*upgrade)(struct semanage_handle*, char *, size_t);
+
+	/* Remove a policy module */
+	int (*remove)(struct semanage_handle*, char *);
+
+	/* List policy modules */
+	int (*list)(struct semanage_handle*, semanage_module_info_t **, int *);
+
+	/* Install base policy */
+        int (*install_base)(struct semanage_handle*, char *, size_t);
+};
+
+#if 0
+/* Should be backend independent */
+extern int semanage_base_merge_components(
+	struct semanage_handle* handle,
+	semanage_module_info_t* base);
+
+extern int semanage_commit_components(
+	struct semanage_handle* handle);
+#endif
+
+#endif
diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/semanage_store.c exp/libsemanage/src/semanage_store.c
--- old/libsemanage/src/semanage_store.c	2005-10-07 18:38:00.000000000 -0400
+++ exp/libsemanage/src/semanage_store.c	2005-10-11 02:20:49.000000000 -0400
@@ -205,18 +205,18 @@
 	if (stat(path, &sb) == -1) {
 		if (errno == ENOENT && create) {
 			if (mkdir(path, S_IRWXU) == -1) {
-				semanage_write_error(sh, "Could not create module store at %s.", path);
+				ERR(sh, "Could not create module store at %s.", path);
 				return -2;
 			}
 		}
 		else {
-			semanage_write_error(sh, "Could not read from module store at %s.", path);
+			ERR(sh, "Could not read from module store at %s.", path);
 			return -1;
 		}
 	}
 	else {
 		if (!S_ISDIR(sb.st_mode) || access(path, mode_mask) == -1) {
-			semanage_write_error(sh, "Could not access module store at %s, or it is not a directory.", path);
+			ERR(sh, "Could not access module store at %s, or it is not a directory.", path);
 			return -1;
 		}
 	}
@@ -224,18 +224,18 @@
 	if (stat(path, &sb) == -1) {
 		if (errno == ENOENT && create) {
 			if (mkdir(path, S_IRWXU) == -1) {
-				semanage_write_error(sh, "Could not create module store, active subdirectory at %s.", path);
+				ERR(sh, "Could not create module store, active subdirectory at %s.", path);
 				return -2;
 			}
 		}
 		else {
-			semanage_write_error(sh, "Could not read from module store, active subdirectory at %s.", path);
+			ERR(sh, "Could not read from module store, active subdirectory at %s.", path);
 			return -1;
 		}
 	}
 	else {
 		if (!S_ISDIR(sb.st_mode) || access(path, mode_mask) == -1) {
-			semanage_write_error(sh, "Could not access module store active subdirectory at %s, or it is not a directory.", path);
+			ERR(sh, "Could not access module store active subdirectory at %s, or it is not a directory.", path);
 			return -1;
 		}
 	}
@@ -243,18 +243,18 @@
 	if (stat (path, &sb) == -1) {
 		if (errno == ENOENT && create) {
 			if (mkdir(path, S_IRWXU) == -1) {
-				semanage_write_error(sh, "Could not create module store, active modules subdirectory at %s.", path);
+				ERR(sh, "Could not create module store, active modules subdirectory at %s.", path);
 				return -2;
 			}
 		}
 		else {
-			semanage_write_error(sh, "Could not read from module store, active modules subdirectory at %s.", path);
+			ERR(sh, "Could not read from module store, active modules subdirectory at %s.", path);
 			return -1;
 		}
 	}
 	else {
 		if (!S_ISDIR(sb.st_mode) || access(path, mode_mask) == -1) {
-			semanage_write_error(sh, "Could not access module store active modules subdirectory at %s, or it is not a directory.", path);
+			ERR(sh, "Could not access module store active modules subdirectory at %s, or it is not a directory.", path);
 			return -1;
 		}
 	}
@@ -395,21 +395,21 @@
 
 	if (stat(sandbox, &buf) == -1) {
 		if (errno != ENOENT) {
-			semanage_write_error(sh, "Error scanning directory %s.", sandbox);
+			ERR(sh, "Error scanning directory %s.", sandbox);
 			return -1;
 	       }
 	}
 	else {
 		/* remove the old sandbox */
 		if (semanage_remove_directory(sandbox) != 0) {
-			semanage_write_error(sh, "Error removing old sandbox directory %s.", sandbox);
+			ERR(sh, "Error removing old sandbox directory %s.", sandbox);
 			return -1;
 		}
 	}
 
 	if (mkdir(sandbox, S_IRWXU) == -1 ||
 	    semanage_copy_dir(semanage_path(SEMANAGE_ACTIVE, SEMANAGE_TOPLEVEL), sandbox) == -1) {
-		semanage_write_error(sh, "Could not copy files to sandbox %s.", sandbox);
+		ERR(sh, "Could not copy files to sandbox %s.", sandbox);
 		goto cleanup;
 	}
 	return 0;
@@ -441,7 +441,7 @@
 	*len = 0;
 	if ((num_files = scandir(modules_path, &namelist,
 				 semanage_filename_select, alphasort)) == -1) {
-		semanage_write_error(sh, "Error while scanning directory %s.", modules_path);
+		ERR(sh, "Error while scanning directory %s.", modules_path);
 		goto cleanup;
 	}
 	if (num_files == 0) {
@@ -449,7 +449,7 @@
 		goto cleanup;
 	}	
 	if ((*filenames = (char **) calloc(num_files, sizeof(**filenames))) == NULL) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		goto cleanup;
 	}	
 	for (i = 0; i < num_files; i++) {
@@ -459,7 +459,7 @@
 		snprintf(path, PATH_MAX, "%s/%s", modules_path, namelist[i]->d_name);
 		if ((filename = strdup(path)) == NULL) {
 			int j;
-			semanage_write_error(sh, "Out of memory!");
+			ERR(sh, "Out of memory!");
 			for (j = 0; j < i; j++) {
 				free((*filenames)[j]);
 			}
@@ -502,12 +502,12 @@
 	memset(write_buf, 0, sizeof(write_buf));
 	snprintf(write_buf, sizeof(write_buf), "%d", commit_number);
 	if ((fd = open(commit_filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR)) == -1) {
-		semanage_write_error(sh, "Could not open commit number file %s for writing.", commit_filename);
+		ERR(sh, "Could not open commit number file %s for writing.", commit_filename);
 		return -1;
 	}
 	amount_written = write(fd, write_buf, sizeof(write_buf));
 	if (amount_written == -1) {
-		semanage_write_error(sh, "Error while writing commit number to %s.", commit_filename);
+		ERR(sh, "Error while writing commit number to %s.", commit_filename);
 		close(fd);
 		return -1;
 	}
@@ -517,21 +517,21 @@
 	if (stat(backup, &buf) == 0) {
 		if (S_ISDIR(buf.st_mode) && 
 		    semanage_remove_directory(backup) != 0) {
-			semanage_write_error(sh, "Could not remove previous backup %s.", backup);
+			ERR(sh, "Could not remove previous backup %s.", backup);
 			return -1;
 		}
 	}
 	else if (errno != ENOENT) {
-		semanage_write_error(sh, "Could not stat directory %s.", backup);
+		ERR(sh, "Could not stat directory %s.", backup);
 		return -1;
 	}
 
 	if (rename(active, backup) == -1) {
-		semanage_write_error(sh, "Error while renaming %s to %s.", active, backup);
+		ERR(sh, "Error while renaming %s to %s.", active, backup);
 		return -1;
 	}
 	if (rename(sandbox, active) == -1) {
-		semanage_write_error(sh, "Error while renaming %s to %s.", sandbox, active);
+		ERR(sh, "Error while renaming %s to %s.", sandbox, active);
 		/* note that if an error occurs during the next
 		 * function then the store will be left in an
 		 * inconsistent state */
@@ -788,14 +788,14 @@
 	pid_t forkval;
 
 	if ((argv = split_args(e->path, e->args, new_name, old_name)) == NULL) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		return -1;
 	}
 	
 	/* no need to use pthread_atfork() -- child will not be using
 	 * any mutexes. */
 	if ((forkval = fork()) == -1) {
-		semanage_write_error(sh, "Error while forking process.");
+		ERR(sh, "Error while forking process.");
 		return -1;
 	}
 	else if (forkval == 0) {
@@ -809,7 +809,7 @@
 		int status = 0;
 		free_argv(argv);
 		if (waitpid(forkval, &status, 0) == -1 || !WIFEXITED(status)) {
-			semanage_write_error(sh, "Child process %s did not exit cleanly.", e->path);
+			ERR(sh, "Child process %s did not exit cleanly.", e->path);
 			return -1;
 		}
 		return WEXITSTATUS(status);
@@ -829,20 +829,20 @@
 	snprintf(running_policy, PATH_MAX, "%s.%d",
 		 selinux_binary_policy_path(), security_policyvers());
 	if (semanage_copy_file(active_kernel, running_policy) == -1) {
-		semanage_write_error(sh, "Could not copy %s to %s.", active_kernel, running_policy);
+		ERR(sh, "Could not copy %s to %s.", active_kernel, running_policy);
 		goto cleanup;
 	}
 	if (semanage_copy_file(active_fc, running_fc) == -1) {
-		semanage_write_error(sh, "Could not copy %s to %s.", active_fc, running_fc);
+		ERR(sh, "Could not copy %s to %s.", active_fc, running_fc);
 		goto cleanup;
 	}
 	if ((r = semanage_exec_prog(sh, sh->conf->load_policy, running_policy, "")) != 0) {
-		semanage_write_error(sh, "load_policy returned error code %d.", r);
+		ERR(sh, "load_policy returned error code %d.", r);
 		goto cleanup;
 	}
 	
 	if ((r = semanage_exec_prog(sh, sh->conf->setfiles, running_policy, running_fc)) != 0) {
-		semanage_write_error(sh, "setfiles returned error code %d.", r);
+		ERR(sh, "setfiles returned error code %d.", r);
 		goto cleanup;
 	}
 	retval = 0;
@@ -861,11 +861,11 @@
 	int retval = -1, new_commit_number;
 
 	if (sh->conf->load_policy == NULL) {
-		semanage_write_error(sh, "No load_policy program specified in configuration file.");
+		ERR(sh, "No load_policy program specified in configuration file.");
 		goto cleanup;
 	}
 	if (sh->conf->setfiles == NULL) {
-		semanage_write_error(sh, "No setfiles program specified in configuration file.");
+		ERR(sh, "No setfiles program specified in configuration file.");
 		goto cleanup;
 	}
 
@@ -904,7 +904,7 @@
 	int got_lock = 0;
 
 	if ((fd = open(lock_file, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR)) == -1) {
-		semanage_write_error(sh, "Could not open direct %s at %s.", lock_name, lock_file);
+		ERR(sh, "Could not open direct %s at %s.", lock_name, lock_file);
 		return -1;
 	}
 	if (sh->timeout == 0) {
@@ -923,7 +923,7 @@
 			break;
 		}
 		else if (errno != EAGAIN) {
-			semanage_write_error(sh, "Error obtaining direct %s at %s.", lock_name, lock_file);
+			ERR(sh, "Error obtaining direct %s at %s.", lock_name, lock_file);
 			close(fd);
 			return -1;
 		}
@@ -932,7 +932,7 @@
 				if (errno == EINTR) {
 					continue;
 				}
-				semanage_write_error(sh, "Error while waiting to get direct %s at %s.", lock_name, lock_file);
+				ERR(sh, "Error while waiting to get direct %s at %s.", lock_name, lock_file);
 				close(fd);
 				return -1;
 			}
@@ -940,7 +940,7 @@
 		}
 	} while (origtime.tv_sec > 0 || sh->timeout == -1);
 	if (!got_lock) {
-		semanage_write_error(sh, "Could not get direct %s at %s.", lock_name, lock_file);
+		ERR(sh, "Could not get direct %s at %s.", lock_name, lock_file);
 		close(fd);
 		return -1;
 	}
@@ -1030,14 +1030,14 @@
 			return 0;
 		}
 		else {
-			semanage_write_error(sh, "Could not open commit number file %s.", commit_filename);
+			ERR(sh, "Could not open commit number file %s.", commit_filename);
 			return -1;
 		}
 	}
 
 	amount_read = read(fd, buf, sizeof(buf));
 	if (amount_read == -1) {
-		semanage_write_error(sh, "Error while reading commit number from %s.", commit_filename);
+		ERR(sh, "Error while reading commit number from %s.", commit_filename);
 		commit_number = -1;
 	}
 	else if (sscanf(buf, "%d", &commit_number) != 1) {
@@ -1046,7 +1046,7 @@
 	}
 	else if (commit_number < 0) {
 		/* read file ought never have negative values */
-		semanage_write_error(sh, "Commit number file %s is corrupted; it should only contain a non-negative integer.", commit_filename);
+		ERR(sh, "Commit number file %s is corrupted; it should only contain a non-negative integer.", commit_filename);
 		commit_number = -1;
 	}
 
@@ -1068,22 +1068,22 @@
 
 	*package = NULL;
 	if (sepol_module_package_create(package) == -1) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		return -1;
 	}
 
 	if (sepol_policy_file_create(&pf)) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		goto cleanup;
 	}
 
 	if ((fp = fopen(filename, "rb")) == NULL) {
-		semanage_write_error(sh, "Could not open module file %s for reading.", filename);
+		ERR(sh, "Could not open module file %s for reading.", filename);
 		goto cleanup;
 	}
 	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);
+		ERR(sh, "Error while reading from module file %s.", filename);
 		fclose(fp);
 		goto cleanup;
 	}
@@ -1113,6 +1113,12 @@
 	int retval = -1, i;
 	int num_modules = 0;
 	sepol_module_package_t **mods = NULL;
+
+	/* FIXME: deprecated - replace with callback debugging
+	 * through a sepol state object */
+	char buffer[1024];
+	buffer[0] = '\0';
+
 	*base = NULL;
 
 	/* first make sure that base module is readable */
@@ -1120,7 +1126,7 @@
 		goto cleanup;
 	}
 	if (access(base_filename, R_OK) == -1) {
-		semanage_write_error(sh, "Could not access sandbox base file %s.", base_filename);
+		ERR(sh, "Could not access sandbox base file %s.", base_filename);
 		goto cleanup;
 	}
 
@@ -1135,7 +1141,7 @@
 		goto cleanup;
 	}
 	if ((mods = calloc(num_modules, sizeof(*mods))) == NULL) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(sh, "Out of memory!");
 		num_modules  = 0;
 		goto cleanup;
 	}
@@ -1145,10 +1151,15 @@
 		}
 	}
 
-	if (sepol_link_packages(*base, mods, num_modules, 0, sh->err_buf,
-				SEMANAGE_ERRBUFSZ) != 0) {
+	if (sepol_link_packages(*base, mods, num_modules, 
+			0, buffer, 1024) != 0) {
+		if (*buffer != '\0') 
+			ERR(sh, "%s", buffer);	
 		goto cleanup;
 	}
+	if (*buffer != '\0') 
+		WARN(sh, "%s", buffer);
+
 	retval = 0;
 
  cleanup:
@@ -1175,6 +1186,11 @@
 	int policyvers = security_policyvers();
 	FILE *outfile = NULL;
 
+	/* FIXME: deprecated - replace with callback debugging
+	 * through a sepol state object */
+	char buffer[1024];
+	buffer[0] = '\0';
+
 	if (policyvers < sepol_policy_kern_vers_min() || 
 	    policyvers > sepol_policy_kern_vers_max())
 		policyvers = sh->conf->policyvers;
@@ -1183,27 +1199,32 @@
 		return -1;
 	}
 	if (sepol_expand_module(base->policy, out, 0,
-			  sh->err_buf, SEMANAGE_ERRBUFSZ) == -1) {
+			  buffer, 1024) != -1) {
+		if (*buffer != '\0') 
+			ERR(sh, "%s", buffer);
 		goto cleanup;
 	}
+	if (*buffer != '\0') 
+		WARN(sh, "%s", buffer);
+
 	if (sepol_policydb_set_vers(out, policyvers)) {
-		semanage_write_error(sh, "Unknown/Invalid policy version %d.", sh->conf->policyvers);
+		ERR(sh, "Unknown/Invalid policy version %d.", sh->conf->policyvers);
 		goto cleanup;
 	}
 	if ((kernel_filename = semanage_path(SEMANAGE_TMP, SEMANAGE_KERNEL)) == NULL) {
 		goto cleanup;
 	}
 	if ((outfile = fopen(kernel_filename, "wb")) == NULL) {
-		semanage_write_error(sh, "Could not open kernel policy %s for writing.", kernel_filename);
+		ERR(sh, "Could not open kernel policy %s for writing.", kernel_filename);
 		goto cleanup;
 	}
 	if (sepol_policy_file_create(&pf)) {
-		semanage_write_error(sh, "Out of memory!");
+		ERR(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);
+		ERR(sh, "Error while writing kernel policy to %s.", kernel_filename);
 		goto cleanup;
 	}
 	retval = 0;

Re: [ SEPOL ] Another debugging system

[Ivan Gyurdiev]

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

Ivan Gyurdiev wrote:
> The attached patch replaces the current semanage debugging system, 
> with one based on callbacks. It breaks API, so the libsemanage.map 
> file needs to be updated, but I'm not sure how to do that properly, so 
> I left that part out...
Attached patch introduces the same debug system to sepol, except it does 
not break compatibility, or replace old usage here yet - this is a bit 
more difficult to do than in semanage, so I'd rather split it up in 
several patches. This one introduces a handle, but doesn't allow the 
handle to be passed in anywhere yet. A global handle is used for 
handling the compatibility functions. I've exported those interfaces via 
the map file, and included them in sepol.h.

Bugfix: can you delete the '\n' from the default handler in the semanage 
patch - it prints an extra newline.

Question: should sepol.h be including all (external api) sepol headers ? 
It does not do that currently. I added debug.h, but the others are still 
not included.

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

diff -Naur --exclude CVS libsepol/include/sepol/debug.h libsepol.new/include/sepol/debug.h
--- libsepol/include/sepol/debug.h	2005-07-18 10:28:43.000000000 -0400
+++ libsepol.new/include/sepol/debug.h	2005-10-11 04:25:10.000000000 -0400
@@ -1,9 +1,48 @@
 #ifndef _SEPOL_DEBUG_H_
 #define _SEPOL_DEBUG_H_
 
+#include <sepol/handle.h>
+
+/* Deprecated */
 extern void sepol_enable_debug(
 	void (*fn)(const char* fname, const char *fmt, ...));
 
 extern void sepol_disable_debug();
+extern void sepol_debug(int on);
+/* End deprecated */
+
+struct sepol_message;
+typedef struct sepol_message sepol_message_t;
+
+#define SEPOL_MSG_ERR  1
+#define SEPOL_MSG_WARN 2
+#define SEPOL_MSG_INFO 3
+
+extern const char* sepol_msg_get_message(
+	sepol_message_t* msg);
+
+extern int sepol_msg_get_level(
+	sepol_message_t* msg);
+
+extern const char* sepol_msg_get_channel(
+	sepol_message_t* msg);
+
+extern const char* sepol_msg_get_fname(
+	sepol_message_t* msg);
+
+extern void sepol_msg_free(
+	sepol_message_t* msg);
+
+/* Set the messaging callback. 
+ * By the default, the callback will print
+ * the message on standard output, in a 
+ * particular format. Passing NULL here
+ * indicates that messaging should be suppressed */
+extern void sepol_msg_set_callback(
+	sepol_handle_t* handle,
+	void (*callback) (
+		void* varg,
+		sepol_message_t* msg),
+	void* callback_arg);
 
-#endif /* _SEPOL_DEBUG_H_ */
+#endif 
diff -Naur --exclude CVS libsepol/include/sepol/handle.h libsepol.new/include/sepol/handle.h
--- libsepol/include/sepol/handle.h	1969-12-31 19:00:00.000000000 -0500
+++ libsepol.new/include/sepol/handle.h	2005-10-11 03:41:51.000000000 -0400
@@ -0,0 +1,13 @@
+#ifndef _SEPOL_HANDLE_H_
+#define _SEPOL_HANDLE_H_
+
+struct sepol_handle;
+typedef struct sepol_handle sepol_handle_t;
+
+/* Create and return a sepol handle. */
+sepol_handle_t *sepol_handle_create(void);
+
+/* Destroy a sepol handle. */
+void sepol_handle_destroy(sepol_handle_t *);
+
+#endif
diff -Naur --exclude CVS libsepol/include/sepol/sepol.h libsepol.new/include/sepol/sepol.h
--- libsepol/include/sepol/sepol.h	2005-09-14 11:44:44.000000000 -0400
+++ libsepol.new/include/sepol/sepol.h	2005-10-11 04:24:40.000000000 -0400
@@ -4,6 +4,8 @@
 #include <stddef.h>
 #include <stdio.h>
 
+#include <sepol/debug.h>
+
 /* Given an existing binary policy (starting at 'data', with length 'len')
    and a boolean configuration file named by 'boolpath', rewrite the binary
    policy for the boolean settings in the boolean configuration file.
@@ -37,6 +39,4 @@
 /* Check context validity against currently set binary policy. */
 extern int sepol_check_context(char *context);
 
-/* Turn on or off sepol error messages. */
-extern void sepol_debug(int on);
 #endif
diff -Naur --exclude CVS libsepol/src/debug.c libsepol.new/src/debug.c
--- libsepol/src/debug.c	2005-09-14 15:04:54.000000000 -0400
+++ libsepol.new/src/debug.c	2005-10-11 04:52:05.000000000 -0400
@@ -1,50 +1,143 @@
 #include <stdarg.h>
+#include <stdlib.h>
 #include <stdio.h>
-
-#include <sepol/sepol.h>
-#include <sepol/debug.h>
+#include "handle.h"
 #include "debug.h"
 
-#ifdef __GNUC__
-__attribute__ ((format (printf, 2, 3))) 
-#endif
-static void default_printf(
-	const char* fname, 
-	const char *fmt, ...) {
+/* Deprecated */
+void msg_compat_handler(
+	void* varg,
+	sepol_message_t* msg) {
+
+	void (*compat_fn)
+		(const char* fname, const char* fmt, ...) = varg;
+
+	if (compat_fn) {
+		compat_fn(sepol_msg_get_fname(msg),
+			"%s", sepol_msg_get_message(msg));
+	}
+}
 
-	va_list ap;
-	va_start(ap, fmt);
-	fprintf(stderr, "libsepol.%s: ", fname);
-	vfprintf (stderr, fmt, ap);
-	va_end(ap);
+struct sepol_handle compat_handle = {
+	.callback = msg_default_handler,
+	.callback_arg = NULL,
+};
+
+void sepol_debug(int on) {
+	compat_handle.callback = (on)? msg_default_handler : NULL;
 }
 
-#ifdef __GNUC__
-__attribute__ ((format (printf, 2, 3)))
-#endif
-static void suppress_printf(
-	const char* unused1, 
-	const char* unused2, ...) { 
-		unused1 = NULL;
-		unused2 = NULL;
+void sepol_enable_debug(
+	void (*fn)(const char* fname, const char *fmt, ...)) {
+
+	compat_handle.callback = (fn)? msg_compat_handler: msg_default_handler;
+	compat_handle.callback_arg = fn;
 }
 
-void (*DEBUG) (const char* fname, const char* fmt, ...) = default_printf;
+void sepol_disable_debug() {
+	compat_handle.callback = NULL;
+}
+/* End deprecated */
 
-/* Compatibility */
-void sepol_debug(int on) {
-        sepol_debug_compat(on);
+
+#define SEPOL_ERRBUFSZ 1024
+
+struct sepol_message {
+	char message[SEPOL_ERRBUFSZ];
+	int level;
+	const char* channel;
+	const char* fname;
+};
+
+const char* sepol_msg_get_message(sepol_message_t* msg) {
+	return msg->message;
 };
 
-void sepol_debug_compat(int on) {
-	DEBUG = (on)? default_printf : suppress_printf;
+int sepol_msg_get_level(sepol_message_t* msg) {
+	return msg->level;
 }
 
-void sepol_enable_debug(
-	void (*fn)(const char* fname, const char *fmt, ...)) {
-	DEBUG = (fn)? fn: default_printf;	
+const char* sepol_msg_get_channel(sepol_message_t* msg) {
+	return msg->channel;
 }
 
-void sepol_disable_debug() {
-	DEBUG = suppress_printf;
+const char* sepol_msg_get_fname(sepol_message_t* msg) {
+	return msg->fname;
+}
+
+void sepol_msg_free(sepol_message_t* msg) {
+	if (!msg)
+		return;
+
+	free(msg);
+}
+
+void msg_default_handler(
+	void* varg,
+	sepol_message_t* msg) {
+
+	FILE* stream = NULL;
+
+	switch(sepol_msg_get_level(msg)) {
+	
+		case SEPOL_MSG_ERR:
+		case SEPOL_MSG_WARN:
+			stream = stderr;
+			break;
+		case SEPOL_MSG_INFO:
+		default:
+			stream = stdout;
+			break;
+	}
+
+	fprintf(stream, "%s.%s: %s",
+		sepol_msg_get_channel(msg),
+		sepol_msg_get_fname(msg),
+		sepol_msg_get_message(msg));
+
+	sepol_msg_free(msg);
+	varg = NULL;
+}
+
+#ifdef __GNUC__
+__attribute__ ((format (printf, 5, 6)))
+#endif
+void msg_write(
+	sepol_handle_t* handle,
+	int level,
+	const char* channel,
+	const char* fname,
+	char* fmt,
+	...) {
+
+	sepol_message_t* msg;
+
+	if (!handle->callback)
+		return;
+
+	msg = (sepol_message_t*) malloc(sizeof(sepol_message_t));
+	if (!msg)
+		return;
+
+	msg->fname = fname;
+	msg->channel = channel;
+	msg->level = level;
+
+	va_list ap;
+	va_start(ap, fmt);
+	vsnprintf(msg->message, SEPOL_ERRBUFSZ, fmt, ap);
+	va_end(ap);
+
+	handle->callback(handle->callback_arg, msg);
+}
+
+extern void sepol_msg_set_callback(
+	sepol_handle_t* handle,
+	void (*callback) (
+		void* varg,
+		sepol_message_t* msg),
+	void* callback_arg) {
+
+	handle->callback = callback;
+	handle->callback_arg = callback_arg;
 }
diff -Naur --exclude CVS libsepol/src/debug.h libsepol.new/src/debug.h
--- libsepol/src/debug.h	2005-07-18 10:28:43.000000000 -0400
+++ libsepol.new/src/debug.h	2005-10-11 04:51:31.000000000 -0400
@@ -1,17 +1,52 @@
 #ifndef _SEPOL_INTERNAL_DEBUG_H_
 #define _SEPOL_INTERNAL_DEBUG_H_
 
+#include "handle.h"
+#include <stdio.h>
+#include <sepol/debug.h>
+
 #define STATUS_SUCCESS 0
 #define STATUS_ERR -1
 #define STATUS_NODATA 1
 
-extern void sepol_debug_compat(int on);
+#define ERR(handle, ...) \
+	msg_write(handle, SEPOL_MSG_ERR, "libsepol", \
+	__func__, __VA_ARGS__)
+
+#define INFO(handle, fmt, ...) \
+	msg_write(handle, SEPOL_MSG_INFO, "libsepol", \
+	__func__, __VA_ARGS__)
+
+#define WARN(handle, fmt, ...) \
+	msg_write(handle, SEPOL_MSG_WARN, "libsepol", \
+	__func__, __VA_ARGS__)
 
 #ifdef __GNUC__
-__attribute__ ((format (printf, 2, 3)))
+__attribute__ ((format (printf, 5, 6)))
 #endif
-extern void (*DEBUG) (
+extern void msg_write(
+	sepol_handle_t* handle,
+	int level,
+	const char* channel,
 	const char* fname,
-	const char* fmt, ...);
+	char* fmt,
+	...);
+
+extern void msg_default_handler(
+	void* varg,
+	sepol_message_t* msg);
+
+/* Deprecated */
+extern void msg_compat_handler(
+	void* varg,
+	sepol_message_t* msg);
+
+extern struct sepol_handle compat_handle;
+
+#define DEBUG(fname, ...) \
+	msg_write(&compat_handle, SEPOL_MSG_ERR, "libsepol", \
+	fname, __VA_ARGS__)
+/* End deprecated */
+
 
-#endif /* _SEPOL_INTERNAL_DEBUG_H_ */
+#endif 
diff -Naur --exclude CVS libsepol/src/handle.c libsepol.new/src/handle.c
--- libsepol/src/handle.c	1969-12-31 19:00:00.000000000 -0500
+++ libsepol.new/src/handle.c	2005-10-11 04:40:50.000000000 -0400
@@ -0,0 +1,21 @@
+#include <stdlib.h>
+#include "handle.h"
+#include "debug.h"
+
+sepol_handle_t *sepol_handle_create(void) {
+
+	sepol_handle_t *sh = malloc(sizeof(sepol_handle_t));
+	if (sh == NULL)
+		return NULL;
+	
+	/* Set callback */
+	sh->callback = msg_default_handler;
+	sh->callback_arg = NULL;
+
+	return sh;
+}
+
+void sepol_handle_destroy(sepol_handle_t *sh) {
+	free(sh);
+}
+
diff -Naur --exclude CVS libsepol/src/handle.h libsepol.new/src/handle.h
--- libsepol/src/handle.h	1969-12-31 19:00:00.000000000 -0500
+++ libsepol.new/src/handle.h	2005-10-11 04:31:25.000000000 -0400
@@ -0,0 +1,17 @@
+#ifndef _SEPOL_INTERNAL_HANDLE_H_
+#define _SEPOL_INTERNAL_HANDLE_H_
+
+#include <sepol/handle.h>
+#include <sepol/debug.h>
+
+struct sepol_handle {
+
+	/* Error callback */
+	void (*callback) (
+		void* varg,
+		sepol_message_t* msg);
+	void* callback_arg;
+
+};
+
+#endif
diff -Naur --exclude CVS libsepol/src/libsepol.map libsepol.new/src/libsepol.map
--- libsepol/src/libsepol.map	2005-10-07 18:38:02.000000000 -0400
+++ libsepol.new/src/libsepol.map	2005-10-11 04:54:39.000000000 -0400
@@ -17,5 +17,6 @@
 	sepol_bool*; sepol_context*; sepol_disable_debug; sepol_enable_debug;
 	sepol_iface*; sepol_port*; sepol_user*; sepol_clear_unused_users;
 	sepol_role_is_valid; sepol_set_delusers;
+	sepol_msg_*; sepol_handle_*;
   local: *;
 };

Re: [ SEPOL ] Another debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 05:03 -0400, Ivan Gyurdiev wrote:
> Attached patch introduces the same debug system to sepol, except it does 
> not break compatibility, or replace old usage here yet - this is a bit 
> more difficult to do than in semanage, so I'd rather split it up in 
> several patches. This one introduces a handle, but doesn't allow the 
> handle to be passed in anywhere yet. A global handle is used for 
> handling the compatibility functions. I've exported those interfaces via 
> the map file, and included them in sepol.h.
> 
> Bugfix: can you delete the '\n' from the default handler in the semanage 
> patch - it prints an extra newline.
> 
> Question: should sepol.h be including all (external api) sepol headers ? 
> It does not do that currently. I added debug.h, but the others are still 
> not included.

- Same comments apply as for the semanage patch.

- Yes, sepol.h should likely include all external API sepol headers, and
the current interfaces in sepol.h should likely be moved to separate
headers that are then included.

- The natural first target for adding the handles would be those
interfaces that already take error buffer arguments, i.e.
sepol_link_packages, sepol_link_modules, and sepol_expand_modules.

- I'm not sure it is worthwhile to add the handles to primitive
interfaces like sepol_policydb_create, where the only failure case is
out of memory.  In contrast, sepol_policydb_read/write and
sepol_policydb_from_image/to_image are more likely candidates.

- We'll need to preserve versions of the old sepol.h interfaces without
handles for backward compatibility, since there have been official
releases with those interfaces.  Some of those interfaces will be
obsoleted (genusers/genbools) but others will continue to be used
(genbools_array for load_policy, set_policydb_from_file and
check_context for setfiles -c).

-- 
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: [ SEPOL ] Another debugging system

[Ivan Gyurdiev]

> - Same comments apply as for the semanage patch.
>   
Allright, I'll redo both of those patches for tomorrow (no time today).
Thanks for the comments.
> - Yes, sepol.h should likely include all external API sepol headers, and
> the current interfaces in sepol.h should likely be moved to separate
> headers that are then included.
>   
Will change that in a later patch..

> - We'll need to preserve versions of the old sepol.h interfaces without
> handles for backward compatibility, since there have been official
> releases with those interfaces.  Some of those interfaces will be
> obsoleted (genusers/genbools) but others will continue to be used
> (genbools_array for load_policy, set_policydb_from_file and
> check_context for setfiles -c).
>   
I'm confused - what's the difference between an "old" interface that has 
a corresponding "new" interface... and an obsoleted interface. Seems 
like the same thing to me. Yes, we'll need need to preserve 
compatibility until all users are fixed, and then we can get rid of the 
old interfaces..right?


--
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: [ SEPOL ] Another debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 11:11 -0400, Ivan Gyurdiev wrote:
> I'm confused - what's the difference between an "old" interface that has 
> a corresponding "new" interface... and an obsoleted interface. Seems 
> like the same thing to me. Yes, we'll need need to preserve 
> compatibility until all users are fixed, and then we can get rid of the 
> old interfaces..right?

No, we should provide ABI stability, as discussed in section 3 of
http://people.redhat.com/drepper/dsohowto.pdf

Updated users can be limited to the new interfaces, but old binaries
should continue to work even with the new libsepol, i.e. the libsepol
DSO must continue to provide versions of those interfaces that are
consistent with its previously exported ABI.

Since some interfaces will be obsoleted by the transition to
regenerating policy on all local customizations, it won't be necessary
to provide new versions of those interfaces at all.

-- 
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: [ SEPOL ] Another debugging system

[Stephen Smalley]

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

On Tue, 2005-10-11 at 11:15 -0400, Stephen Smalley wrote:
> No, we should provide ABI stability, as discussed in section 3 of
> http://people.redhat.com/drepper/dsohowto.pdf
> 
> Updated users can be limited to the new interfaces, but old binaries
> should continue to work even with the new libsepol, i.e. the libsepol
> DSO must continue to provide versions of those interfaces that are
> consistent with its previously exported ABI.

Example:  Suppose that we wanted to introduce an explicit policydb
argument to sepol_check_context() rather than requiring use of a global
policydb within libsepol for that purpose.  We might modify libsepol as
in the following patch to achieve this without disturbing existing
binaries.  Any programs rebuilt against the new libsepol will be forced
to be updated to the new interface (public interface definition changed
in the header file), but existing binaries built against the old
libsepol will continue to fallback to the old interface.  Note that this
patch isn't complete (doesn't update chkcon example utility to the new
interface, or deal with the need to also have a non-global sidtab), but
is functional (I built libsepol with this patch, and then ran an
existing setfiles binary against it with the -c option to check contexts
against a binary policy, and setfiles continued to use the old interface
and work correctly).

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: libsepol-ex.patch --]
[-- Type: text/x-patch, Size: 4048 bytes --]

Index: libsepol/include/sepol/sepol.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/include/sepol/sepol.h,v
retrieving revision 1.8
diff -u -p -r1.8 sepol.h
--- libsepol/include/sepol/sepol.h	14 Sep 2005 15:18:55 -0000	1.8
+++ libsepol/include/sepol/sepol.h	11 Oct 2005 15:28:51 -0000
@@ -3,6 +3,7 @@
 
 #include <stddef.h>
 #include <stdio.h>
+#include <sepol/policydb.h>
 
 /* Given an existing binary policy (starting at 'data', with length 'len')
    and a boolean configuration file named by 'boolpath', rewrite the binary
@@ -35,7 +36,7 @@ extern void sepol_set_delusers(int on);
 extern int sepol_set_policydb_from_file(FILE *fp);
 
 /* Check context validity against currently set binary policy. */
-extern int sepol_check_context(char *context);
+extern int sepol_check_context(sepol_policydb_t *p, char *context);
 
 /* Turn on or off sepol error messages. */
 extern void sepol_debug(int on);
Index: libsepol/include/sepol/policydb/services.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/include/sepol/policydb/services.h,v
retrieving revision 1.1
diff -u -p -r1.1 services.h
--- libsepol/include/sepol/policydb/services.h	7 Oct 2005 20:10:15 -0000	1.1
+++ libsepol/include/sepol/policydb/services.h	11 Oct 2005 15:29:30 -0000
@@ -108,6 +108,7 @@ extern int sepol_sid_to_context(
  * has the string representation specified by `scontext'.
  */
 extern int sepol_context_to_sid(
+	policydb_t *p,
 	sepol_security_context_t scontext,		/* IN */
 	size_t  scontext_len,			/* IN */
 	sepol_security_id_t *out_sid);		/* OUT */
Index: libsepol/src/libsepol.map
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/src/libsepol.map,v
retrieving revision 1.6
diff -u -p -r1.6 libsepol.map
--- libsepol/src/libsepol.map	7 Oct 2005 20:10:15 -0000	1.6
+++ libsepol/src/libsepol.map	11 Oct 2005 15:27:05 -0000
@@ -1,4 +1,11 @@
-{
+LIBSEPOL_1.0 {
+  global: 
+	sepol_genbools*; sepol_set_policydb_from_file; sepol_check_context; sepol_genusers; sepol_debug; sepol_set_delusers;
+ local: 
+	*;
+};
+
+LIBSEPOL_1.1 {
   global: 
 	sepol_genbools*; sepol_set_policydb_from_file; sepol_check_context; sepol_genusers; sepol_debug; sepol_set_delusers;
 	sepol_policy_file_create; sepol_policy_file_free;
@@ -17,5 +24,4 @@
 	sepol_bool*; sepol_context*; sepol_disable_debug; sepol_enable_debug;
 	sepol_iface*; sepol_port*; sepol_user*; sepol_clear_unused_users;
 	sepol_role_is_valid; sepol_set_delusers;
-  local: *;
-};
+} LIBSEPOL_1.0;
Index: libsepol/src/services.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/src/services.c,v
retrieving revision 1.23
diff -u -p -r1.23 services.c
--- libsepol/src/services.c	7 Oct 2005 20:10:16 -0000	1.23
+++ libsepol/src/services.c	11 Oct 2005 15:24:21 -0000
@@ -538,7 +538,8 @@ out:
  * Return a SID associated with the security context that
  * has the string representation specified by `scontext'.
  */
-int sepol_context_to_sid(sepol_security_context_t scontext,
+int sepol_context_to_sid(policydb_t *policydb,
+	                    sepol_security_context_t scontext,
 			    size_t scontext_len,
 			    sepol_security_id_t * sid)
 {
@@ -566,10 +567,17 @@ int sepol_context_to_sid(sepol_security_
 	return STATUS_ERR;
 }
 
-int sepol_check_context(char *context) 
+int sepol_check_context1__ (char *context) 
 {
-	return sepol_context_to_sid(context, strlen(context)+1, NULL);
+	return sepol_context_to_sid(policydb, context, strlen(context)+1, NULL);
 }
+asm(".symver sepol_check_context1__,sepol_check_context@LIBSEPOL_1.0");
+
+int sepol_check_context2__ (sepol_policydb_t *p, char *context) 
+{
+	return sepol_context_to_sid(&p->p, context, strlen(context)+1, NULL);
+}
+asm(".symver sepol_check_context2__,sepol_check_context@@LIBSEPOL_1.1");
 
 static inline int compute_sid_handle_invalid_context(
 	context_struct_t *scontext,

Re: [ SEMANAGE ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 02:59 -0400, Ivan Gyurdiev wrote:
> The attached patch replaces the current semanage debugging system, with 
> one based on callbacks. It breaks API, so the libsemanage.map file needs 
> to be updated, but I'm not sure how to do that properly, so I left that 
> part out...

At present, you can just modify it without introducing a new interface
version, because the shared library interface has not been officially
released in any form yet.

> - replaced use of __FUNCTION__ with __func__, which should not be GNU 
> specific

OTOH, __func__ is not known to older versions of gcc, and we already
build with -D_GNU_SOURCE.  

An obvious concern about the new msg_write function is that it requires
memory allocation itself.  Thus, on a memory allocation failure in the
caller, we might easily end up dropping the out of memory error message
due to lack of available memory at this point.

-- 
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 ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 09:34 -0400, Stephen Smalley wrote:
> An obvious concern about the new msg_write function is that it requires
> memory allocation itself.  Thus, on a memory allocation failure in the
> caller, we might easily end up dropping the out of memory error message
> due to lack of available memory at this point.

More generally, I'm not sure why msg_write writes the formatted string
to a private buffer and passes the new msg type to the callback,
requiring the callback to call *_get methods, versus just having the
callback be a stdarg function that takes all of the arguments (plus
auxiliary data) directly.

Other comments:

diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/direct_api.c exp/libsemanage/src/direct_api.c
--- old/libsemanage/src/direct_api.c	2005-10-07 18:37:59.000000000 -0400
+++ exp/libsemanage/src/direct_api.c	2005-10-11 02:34:29.000000000 -0400
@@ -268,11 +268,23 @@
 		goto cleanup;
 	}
 
-	/* expand and verify the resulting policy */
-	if (semanage_expand_sandbox(sh, base) < 0 ||
-	    semanage_verify_kernel(sh) != 0) {
+	/* Expand the resulting policy */
+	if (semanage_expand_sandbox(sh, base) < 0)
+		goto cleanup;
+
+#if 0
+	/* Link components into base policy */
+	if (semanage_base_merge_components(sh, NULL /* FIXME */) < 0)
+		goto cleanup;
+
+	/* Commit changes to components */
+	if (semanage_commit_components(sh) < 0)
+		goto cleanup;
+#endif
+
+	/* Verify policy */
+	if (semanage_verify_kernel(sh) != 0)
 		goto cleanup;
-	}

Why wouldn't the above steps occur as part of semanage_expand_sandbox()?

-- 
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 ] Replace semanage debugging system

[Ivan Gyurdiev]

Stephen Smalley wrote:
> On Tue, 2005-10-11 at 09:34 -0400, Stephen Smalley wrote:
>   
>> An obvious concern about the new msg_write function is that it requires
>> memory allocation itself.  Thus, on a memory allocation failure in the
>> caller, we might easily end up dropping the out of memory error message
>> due to lack of available memory at this point.
>>     
>
> More generally, I'm not sure why msg_write writes the formatted string
> to a private buffer and passes the new msg type to the callback,
> requiring the callback to call *_get methods, versus just having the
> callback be a stdarg function that takes all of the arguments (plus
> auxiliary data) directly.
>   
Taking all of the arguments directly is bad - does not respond well to 
change, and I've already changed this at least 3 times. However, we 
could take 1) the void* arg, 2) the message structure (auxilary data) 2) 
the fmt, and 3) the variadic list..and I think that would be flexible 
enough. I changed it, because I thought this would be a simpler, and 
more intuitive interface, but I can change it back..

> Other comments:
>
> diff -Naur --exclude CVS --exclude ChangeLog --exclude VERSION --exclude libselinux --exclude policy_components.c old/libsemanage/src/direct_api.c exp/libsemanage/src/direct_api.c
> --- old/libsemanage/src/direct_api.c	2005-10-07 18:37:59.000000000 -0400
> +++ exp/libsemanage/src/direct_api.c	2005-10-11 02:34:29.000000000 -0400
> @@ -268,11 +268,23 @@
>  		goto cleanup;
>  	}
>  
> -	/* expand and verify the resulting policy */
> -	if (semanage_expand_sandbox(sh, base) < 0 ||
> -	    semanage_verify_kernel(sh) != 0) {
> +	/* Expand the resulting policy */
> +	if (semanage_expand_sandbox(sh, base) < 0)
> +		goto cleanup;
> +
> +#if 0
> +	/* Link components into base policy */
> +	if (semanage_base_merge_components(sh, NULL /* FIXME */) < 0)
> +		goto cleanup;
> +
> +	/* Commit changes to components */
> +	if (semanage_commit_components(sh) < 0)
> +		goto cleanup;
> +#endif
> +
> +	/* Verify policy */
> +	if (semanage_verify_kernel(sh) != 0)
>  		goto cleanup;
> -	}
>
> Why wouldn't the above steps occur as part of semanage_expand_sandbox()?
>   
...I suppose it could go there as well..

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

Re: [ SEMANAGE ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 10:29 -0400, Ivan Gyurdiev wrote:
> Taking all of the arguments directly is bad - does not respond well to 
> change, and I've already changed this at least 3 times. However, we 
> could take 1) the void* arg, 2) the message structure (auxilary data) 2) 
> the fmt, and 3) the variadic list..and I think that would be flexible 
> enough. I changed it, because I thought this would be a simpler, and 
> more intuitive interface, but I can change it back..

The auxiliary data could be directly embedded in the handle, avoiding
any need to introduce a new structure (or allocation), with the get
methods operating directly on the handle much like the old strerror
interface.  I don't think you want msg_write needing to allocate any
memory or performing any formatting string writing, as that should all
be done in the callback if desired.

-- 
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 ] Replace semanage debugging system

[Ivan Gyurdiev]

> The auxiliary data could be directly embedded in the handle, avoiding
> any need to introduce a new structure (or allocation), with the get
> methods operating directly on the handle much like the old strerror
> interface.  I don't think you want msg_write needing to allocate any
> memory or performing any formatting string writing, as that should all
> be done in the callback if desired.
>   

Okay, so it sounds like the optimal setup is:
- pass variadic list to callback
- remove msg field from message structure (rename?)
- embed message structure into handle to avoid allocation

Then handler takes four arguments instead of the current two:

- void* arg
- message structure (rename?)
- fmt
- variadic list






--
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 ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 10:57 -0400, Ivan Gyurdiev wrote:
> Okay, so it sounds like the optimal setup is:
> - pass variadic list to callback
> - remove msg field from message structure (rename?)
> - embed message structure into handle to avoid allocation
> 
> Then handler takes four arguments instead of the current two:
> 
> - void* arg
> - message structure (rename?)
> - fmt
> - variadic list

Why not just use the handle directly, and drop the separate message
structure entirely?

-- 
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 ] Replace semanage debugging system

[Ivan Gyurdiev]

> Why not just use the handle directly, and drop the separate message
> structure entirely?
>   
It's disorganized... the handle contains all kinds of things - I don't 
like adding random fields to it. It'd be more orderly to add a 
structure, and functions to work with it, in a separate file. I would 
think that nested structs have no runtime overhead...





--
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 ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 11:18 -0400, Ivan Gyurdiev wrote:
> > Why not just use the handle directly, and drop the separate message
> > structure entirely?
> >   
> It's disorganized... the handle contains all kinds of things - I don't 
> like adding random fields to it. It'd be more orderly to add a 
> structure, and functions to work with it, in a separate file. I would 
> think that nested structs have no runtime overhead...

Hmmm...well, I thought that one of the purposes of the handle was for
error handling, and you are adding the callback and callback arg there
in place of the old error buffer.  More generally, I was wondering if
the callback might want the handle without needing to explicitly specify
it as part of the callback arg.
  
-- 
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 ] Replace semanage debugging system

[Ivan Gyurdiev]

>>> Why not just use the handle directly, and drop the separate message
>>> structure entirely?
>>>   
>>>       
>> It's disorganized... the handle contains all kinds of things - I don't 
>> like adding random fields to it. It'd be more orderly to add a 
>> structure, and functions to work with it, in a separate file. I would 
>> think that nested structs have no runtime overhead...
>>     
>
> Hmmm...well, I thought that one of the purposes of the handle was for
> error handling, and you are adding the callback and callback arg there
> in place of the old error buffer. 
The handle's kind of used for everything right now.... It's a central 
merge point for anything that might need (or benefit from)
preserved state over multiple function calls. I don't feel very strongly 
about this -  I can pass back the handle if you prefer that - it's just 
an organizational preference (I might change my mind tomorrow when I see 
how it looks :)
>  More generally, I was wondering if
> the callback might want the handle without needing to explicitly specify
> it as part of the callback arg.
>   
The callback's job should be to print out the error. If it wants the 
handle, it's certainly free to pass it as part of the argument, but I'm 
not sure what it would be doing with it. I imagine the callback doing 
error _reporting_, while the normal function stack handles error _response_.


--
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 ] Replace semanage debugging system

[Ivan Gyurdiev]

> Okay, so it sounds like the optimal setup is:
> - pass variadic list to callback
> - remove msg field from message structure (rename?)
> - embed message structure into handle to avoid allocation
>
> Then handler takes four arguments instead of the current two:
>
> - void* arg
> - message structure (rename?)
> - fmt
> - variadic list
Wait.... how do I invoke a variadic function from a variadic function?
Should I pass the va_list as an argument?
I knew there was a reason I got rid of this in the first place...

--
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 ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 13:27 -0400, Ivan Gyurdiev wrote:
> > Okay, so it sounds like the optimal setup is:
> > - pass variadic list to callback
> > - remove msg field from message structure (rename?)
> > - embed message structure into handle to avoid allocation
> >
> > Then handler takes four arguments instead of the current two:
> >
> > - void* arg
> > - message structure (rename?)
> > - fmt
> > - variadic list
> Wait.... how do I invoke a variadic function from a variadic function?
> Should I pass the va_list as an argument?
> I knew there was a reason I got rid of this in the first place...

Why can't msg_write be a macro?

-- 
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 ] Replace semanage debugging system

[Ivan Gyurdiev]

> OTOH, __func__ is not known to older versions of gcc, and we already
> build with -D_GNU_SOURCE.  
>   
How much older? From the gcc webpage, I see it supported for 3.0+.
IMHO we should compile against a standard, not against a compiler..
> An obvious concern about the new msg_write function is that it requires
> memory allocation itself.  Thus, on a memory allocation failure in the
> caller, we might easily end up dropping the out of memory error message
> due to lack of available memory at this point.
>   
By the way, does the printf family require such a memory allocation?


--
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 ] Replace semanage debugging system

[Stephen Smalley]

On Tue, 2005-10-11 at 10:15 -0400, Ivan Gyurdiev wrote:
> > OTOH, __func__ is not known to older versions of gcc, and we already
> > build with -D_GNU_SOURCE.  
> >   
> How much older? From the gcc webpage, I see it supported for 3.0+.
> IMHO we should compile against a standard, not against a compiler..

info gcc says:

`__FUNCTION__' is another name for `__func__'.  Older versions of GCC
recognize only this name.  However, it is not standardized.  For
maximum portability, we recommend you use `__func__', but provide a
fallback definition with the preprocessor:

     #if __STDC_VERSION__ < 199901L
     # if __GNUC__ >= 2
     #  define __func__ __FUNCTION__
     # else
     #  define __func__ "<unknown>"
     # endif
     #endif

In any event, we have been using gcc and glibc specific extensions in
our code for some time, and I don't expect that to change.

> By the way, does the printf family require such a memory allocation?

I don't believe so, at least not in the common 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.